diff --git a/clients/agent-runtime/src/gateway/mod.rs b/clients/agent-runtime/src/gateway/mod.rs index 332b531ff..87eb8436b 100644 --- a/clients/agent-runtime/src/gateway/mod.rs +++ b/clients/agent-runtime/src/gateway/mod.rs @@ -1713,6 +1713,255 @@ async fn finalize_generated_session_if_needed( } } +async fn ensure_webhook_session( + state: &AppState, + session_id: &str, + token_hash: Option<&str>, + reserved_idempotency_key: Option<&str>, +) -> Option { + if let Err(error) = state.mem.upsert_session(session_id, token_hash).await { + if token_hash.is_some() { + tracing::error!("session upsert failed for token-scoped request: {error:#}"); + release_idempotency_key(state, reserved_idempotency_key, false); + return Some(( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Session tracking failed"})), + )); + } + tracing::debug!("session upsert best-effort failed: {error}"); + } + + None +} + +async fn maybe_execute_legacy_http_ingress( + state: &AppState, + session_id: &str, + session_source: webhook_dispatch::WebhookSessionSource, + scrubbed_message: &str, + token_hash: Option<&str>, + reserved_idempotency_key: Option<&str>, +) -> Option { + let http_source = webhook_http_source(session_source); + let maybe_response = + maybe_handle_http_ingress(state, session_id, http_source, scrubbed_message, token_hash) + .await; + + if let Some((response, persist_idempotency)) = maybe_response { + release_idempotency_key(state, reserved_idempotency_key, persist_idempotency); + update_session_activity_if_persisted(state, session_id, token_hash, persist_idempotency) + .await; + return Some(response); + } + + None +} + +async fn execute_dispatcher_webhook( + state: &AppState, + config: &Config, + request: webhook_dispatch::WebhookTurnRequest, + reserved_idempotency_key: Option<&str>, +) -> WebhookResponse { + log_webhook_runtime_path(&request.session_id, true, "dispatcher_flag_enabled"); + let dispatch_result = webhook_dispatch::execute( + config, + Arc::clone(&state.provider), + Arc::clone(&state.mem), + Arc::clone(&state.observer), + state.cost_tracker.clone(), + &state.model, + request.clone(), + ) + .await; + log_webhook_terminal_outcome( + &request.session_id, + "dispatcher_agent", + webhook_outcome_label(&dispatch_result.outcome), + ); + let (response, persist_idempotency) = webhook_response_from_dispatch_result(dispatch_result); + release_idempotency_key(state, reserved_idempotency_key, persist_idempotency); + update_session_activity_if_persisted( + state, + &request.session_id, + request.caller_token_hash.as_deref(), + persist_idempotency, + ) + .await; + response +} + +async fn prepare_stream_request( + state: &AppState, + headers: &HeaderMap, + body: WebhookJsonBody, +) -> Result< + ( + WebhookBody, + String, + webhook_dispatch::WebhookSessionSource, + Option, + Config, + Vec, + ), + WebhookResponse, +> { + let webhook_body = parse_webhook_body(body)?; + let (session_id, session_source) = resolve_session_id(headers)?; + let token_hash = utils::extract_bearer_token(headers).map(|t| compute_token_hash(&t)); + + if let Err(error) = state + .mem + .upsert_session(&session_id, token_hash.as_deref()) + .await + { + if token_hash.is_some() { + tracing::error!("session upsert failed for token-scoped request: {error:#}"); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Session tracking failed"})), + )); + } + tracing::debug!("session upsert best-effort failed: {error}"); + } + + let config = state.config.lock().clone(); + let tool_snapshot = + crate::bootstrap::slash_tool_snapshot_from_config(&config).map_err(|_| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Failed to derive effective tool snapshot"})), + ) + })?; + + Ok(( + webhook_body, + session_id, + session_source, + token_hash, + config, + tool_snapshot, + )) +} + +async fn maybe_stream_handled_ingress_response( + state: &AppState, + handled_ingress: &crate::pre_execution::HandledIngress, + session_id: &str, + token_hash: Option<&str>, +) -> Option { + let crate::pre_execution::HandledIngress::Handled(handled) = handled_ingress else { + return None; + }; + + if let Err(e) = state + .mem + .update_session_activity(session_id, token_hash) + .await + { + tracing::debug!("session activity update best-effort failed: {e}"); + } + + let sid = session_id.to_string(); + let (events, status) = match handled { + crate::pre_execution::HandledIngressOutcome::SessionCommandSuccess(success) => { + let message_id = Uuid::new_v4().to_string(); + ( + vec![ + Ok::( + Event::default() + .event("chunk") + .data(success.message.clone()), + ), + Ok::( + Event::default() + .event("done") + .json_data(serde_json::json!({ + "message_id": message_id, + "session_id": sid, + "tools_called": [], + })) + .expect("serializable done event"), + ), + ], + StatusCode::OK, + ) + } + crate::pre_execution::HandledIngressOutcome::SessionCommandFailure { + class: _, + failure, + } => ( + vec![Ok::( + Event::default() + .event("error") + .json_data(serde_json::json!({ + "code": map_session_command_failure_code(&failure.kind), + "message": failure.message, + })) + .expect("serializable error event"), + )], + StatusCode::OK, + ), + crate::pre_execution::HandledIngressOutcome::Blocking(blocking) => match blocking { + crate::pre_execution::BlockingOutcome::ApprovalRequired { tool, reason } => ( + vec![Ok::( + Event::default() + .event("error") + .json_data(serde_json::json!({ + "code": "approval_required", + "tool": tool, + "reason": reason, + "message": format!("Approval required for tool `{tool}`: {reason}"), + })) + .expect("serializable error event"), + )], + StatusCode::OK, + ), + crate::pre_execution::BlockingOutcome::TimeoutAborted => ( + vec![Ok::( + Event::default() + .event("error") + .json_data(serde_json::json!({ + "code": "timeout", + "message": "Request timed out", + })) + .expect("serializable error event"), + )], + StatusCode::OK, + ), + crate::pre_execution::BlockingOutcome::Fallback { response } => { + let message_id = Uuid::new_v4().to_string(); + ( + vec![ + Ok::( + Event::default() + .event("chunk") + .data(scrub_sensitive_boundary_text(response)), + ), + Ok::( + Event::default() + .event("done") + .json_data(serde_json::json!({ + "message_id": message_id, + "session_id": sid, + "tools_called": [], + })) + .expect("serializable done event"), + ), + ], + StatusCode::OK, + ) + } + }, + }; + + let mut response = Sse::new(futures::stream::iter(events)) + .keep_alive(KeepAlive::default()) + .into_response(); + *response.status_mut() = status; + Some(response) +} + fn webhook_http_source( session_source: webhook_dispatch::WebhookSessionSource, ) -> crate::session_commands::CommandSessionSource { @@ -2064,7 +2313,6 @@ async fn handle_webhook( let server_execution_mode = config.agent.execution_mode; let dispatcher_enabled = webhook_dispatcher_enabled(&config); let token_hash = utils::extract_bearer_token(&headers).map(|t| compute_token_hash(&t)); - let http_source = webhook_http_source(session_source); let reserved_idempotency_key = match reserve_webhook_idempotency_key(&state, &headers) { Ok(key) => key, @@ -2072,53 +2320,46 @@ async fn handle_webhook( }; if is_preview && !dispatcher_enabled { - if let Some((response, persist_idempotency)) = maybe_handle_http_ingress( + if let Some(response) = ensure_webhook_session( &state, &session_id, - http_source, + token_hash.as_deref(), + reserved_idempotency_key.as_deref(), + ) + .await + { + return response; + } + + if let Some(response) = maybe_execute_legacy_http_ingress( + &state, + &session_id, + session_source, &scrubbed_message, token_hash.as_deref(), + reserved_idempotency_key.as_deref(), ) .await { - release_idempotency_key( - &state, - reserved_idempotency_key.as_deref(), - persist_idempotency, - ); return response; } } - // Track session lifecycle: create or touch session record. - // When a bearer token is present, session tracking is required for - // token-scoped ownership — fail the request if upsert fails. - // Without a token, tracking is best-effort/observational. - if let Err(e) = state - .mem - .upsert_session(&session_id, token_hash.as_deref()) - .await + if let Some(response) = ensure_webhook_session( + &state, + &session_id, + token_hash.as_deref(), + reserved_idempotency_key.as_deref(), + ) + .await { - if token_hash.is_some() { - tracing::error!("session upsert failed for token-scoped request: {e:#}"); - release_idempotency_key(&state, reserved_idempotency_key.as_deref(), false); - return ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({"error": "Session tracking failed"})), - ); - } - tracing::debug!("session upsert best-effort failed: {e}"); + return response; } if dispatcher_enabled { - log_webhook_runtime_path(&session_id, true, "dispatcher_flag_enabled"); - let dispatch_result = webhook_dispatch::execute( + return execute_dispatcher_webhook( + &state, &config, - Arc::clone(&state.provider), - Arc::clone(&state.mem), - Arc::clone(&state.observer), - state.cost_tracker.clone(), - &state.model, webhook_dispatch::WebhookTurnRequest { session_id: session_id.clone(), session_source, @@ -2130,54 +2371,23 @@ async fn handle_webhook( ), include_sse_frames: is_preview, }, - ) - .await; - log_webhook_terminal_outcome( - &session_id, - "dispatcher_agent", - webhook_outcome_label(&dispatch_result.outcome), - ); - let (response, persist_idempotency) = - webhook_response_from_dispatch_result(dispatch_result); - release_idempotency_key( - &state, reserved_idempotency_key.as_deref(), - persist_idempotency, - ); - update_session_activity_if_persisted( - &state, - &session_id, - token_hash.as_deref(), - persist_idempotency, ) .await; - return response; } // Intercept shared handled-ingress commands before plan/cost guards so they can // short-circuit without being blocked by execution-mode or cost checks. - let http_source = webhook_http_source(session_source); - if let Some((response, persist_idempotency)) = maybe_handle_http_ingress( + if let Some(response) = maybe_execute_legacy_http_ingress( &state, &session_id, - http_source, + session_source, &scrubbed_message, token_hash.as_deref(), + reserved_idempotency_key.as_deref(), ) .await { - release_idempotency_key( - &state, - reserved_idempotency_key.as_deref(), - persist_idempotency, - ); - update_session_activity_if_persisted( - &state, - &session_id, - token_hash.as_deref(), - persist_idempotency, - ) - .await; return response; } @@ -2266,41 +2476,12 @@ async fn handle_chat_stream( return Err(rejection); } - let webhook_body = parse_webhook_body(body)?; + let (webhook_body, session_id, session_source, token_hash, config, tool_snapshot) = + prepare_stream_request(&state, &headers, body).await?; let message = &webhook_body.message; let scrubbed_message = scrub_sensitive_boundary_text(message); - let (session_id, session_source) = resolve_session_id(&headers)?; - - // Track session lifecycle: create or touch session record. - // When a bearer token is present, session tracking is required for - // token-scoped ownership — fail the request if upsert fails. - // Without a token, tracking is best-effort/observational. - let token_hash = utils::extract_bearer_token(&headers).map(|t| compute_token_hash(&t)); - if let Err(e) = state - .mem - .upsert_session(&session_id, token_hash.as_deref()) - .await - { - if token_hash.is_some() { - tracing::error!("session upsert failed for token-scoped request: {e:#}"); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({"error": "Session tracking failed"})), - )); - } - tracing::debug!("session upsert best-effort failed: {e}"); - } - - let config = state.config.lock().clone(); let server_execution_mode = config.agent.execution_mode; let dispatcher_enabled = webhook_dispatcher_enabled(&config); - let tool_snapshot = - crate::bootstrap::slash_tool_snapshot_from_config(&config).map_err(|_| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({"error": "Failed to derive effective tool snapshot"})), - ) - })?; // ── Process message via existing dispatch ──────────── enum StreamProcessingOutcome { @@ -2332,111 +2513,14 @@ async fn handle_chat_stream( .await, ); - if let crate::pre_execution::HandledIngress::Handled(handled) = &handled_ingress { - // Update session activity before returning early - if let Err(e) = state - .mem - .update_session_activity(&session_id, token_hash.as_deref()) - .await - { - tracing::debug!("session activity update best-effort failed: {e}"); - } - - let sid = session_id.clone(); - let (events, status) = - match handled { - crate::pre_execution::HandledIngressOutcome::SessionCommandSuccess(success) => { - let message_id = Uuid::new_v4().to_string(); - ( - vec![ - Ok::( - Event::default() - .event("chunk") - .data(success.message.clone()), - ), - Ok::( - Event::default() - .event("done") - .json_data(serde_json::json!({ - "message_id": message_id, - "session_id": sid, - "tools_called": [], - })) - .expect("serializable done event"), - ), - ], - StatusCode::OK, - ) - } - crate::pre_execution::HandledIngressOutcome::SessionCommandFailure { - class: _, - failure, - } => ( - vec![Ok::( - Event::default() - .event("error") - .json_data(serde_json::json!({ - "code": map_session_command_failure_code(&failure.kind), - "message": failure.message, - })) - .expect("serializable error event"), - )], - StatusCode::OK, - ), - crate::pre_execution::HandledIngressOutcome::Blocking(blocking) => match blocking { - crate::pre_execution::BlockingOutcome::ApprovalRequired { tool, reason } => ( - vec![Ok::(Event::default() - .event("error") - .json_data(serde_json::json!({ - "code": "approval_required", - "tool": tool, - "reason": reason, - "message": format!("Approval required for tool `{tool}`: {reason}"), - })) - .expect("serializable error event"))], - StatusCode::OK, - ), - crate::pre_execution::BlockingOutcome::TimeoutAborted => ( - vec![Ok::( - Event::default() - .event("error") - .json_data(serde_json::json!({ - "code": "timeout", - "message": "Request timed out", - })) - .expect("serializable error event"), - )], - StatusCode::OK, - ), - crate::pre_execution::BlockingOutcome::Fallback { response } => { - let message_id = Uuid::new_v4().to_string(); - ( - vec![ - Ok::( - Event::default() - .event("chunk") - .data(scrub_sensitive_boundary_text(response)), - ), - Ok::( - Event::default() - .event("done") - .json_data(serde_json::json!({ - "message_id": message_id, - "session_id": sid, - "tools_called": [], - })) - .expect("serializable done event"), - ), - ], - StatusCode::OK, - ) - } - }, - }; - let mut response = Sse::new(futures::stream::iter(events)) - .keep_alive(KeepAlive::default()) - .into_response(); - *response.status_mut() = status; + if let Some(response) = maybe_stream_handled_ingress_response( + &state, + &handled_ingress, + &session_id, + token_hash.as_deref(), + ) + .await + { return Ok(response); } @@ -5094,14 +5178,16 @@ mod tests { .await .into_response(); - // SqliteMemory returns 422 for missing session - assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); + // Session upsert now runs before the preview legacy fast path, so slash-session + // commands can still short-circuit without invoking the provider. + assert_eq!(response.status(), StatusCode::OK); let body = response.into_body().collect().await.unwrap().to_bytes(); let payload: serde_json::Value = serde_json::from_slice(&body).unwrap(); assert_eq!(payload["session_id"], "preview-slash"); - assert_eq!(payload["error"]["code"], "unknown_session"); + assert!(payload.get("response").is_some()); + assert!(payload.get("error").is_none()); assert_eq!(provider_impl.calls.load(Ordering::SeqCst), 0); } diff --git a/clients/agent-runtime/src/gateway/webhook_dispatch.rs b/clients/agent-runtime/src/gateway/webhook_dispatch.rs index 5bf1a00bb..a28d51557 100644 --- a/clients/agent-runtime/src/gateway/webhook_dispatch.rs +++ b/clients/agent-runtime/src/gateway/webhook_dispatch.rs @@ -384,6 +384,40 @@ fn sanitize_sse_id(session_id: &str) -> String { .replace(['\r', '\n'], "") } +fn handled_ingress_to_webhook_result( + request: &WebhookTurnRequest, + model: &str, + handled: HandledIngress, +) -> Option { + match handled { + HandledIngress::Handled(HandledIngressOutcome::SessionCommandSuccess(success)) => { + Some(WebhookTurnResult { + session_id: request.session_id.clone(), + model: model.to_string(), + outcome: WebhookTerminalOutcome::Completed, + response_text: Some(success.message.clone()), + event_frames: Vec::new(), + tools_called: Vec::new(), + }) + } + HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { + class: _, + failure, + }) => Some(WebhookTurnResult { + session_id: request.session_id.clone(), + model: model.to_string(), + outcome: WebhookTerminalOutcome::Failed, + response_text: Some(failure.message), + event_frames: Vec::new(), + tools_called: Vec::new(), + }), + HandledIngress::Handled(HandledIngressOutcome::Blocking(blocking)) => Some( + map_canonical_result(request, model, CanonicalWebhookResult::Blocking(blocking)), + ), + HandledIngress::NotHandled => None, + } +} + pub(crate) async fn execute( config: &Config, provider: Arc, @@ -400,50 +434,10 @@ pub(crate) async fn execute( Err(result) => return result, }; - match evaluate_webhook_ingress(memory.as_ref(), &tool_snapshot, &request, clamped_mode).await { - HandledIngress::Handled(HandledIngressOutcome::SessionCommandSuccess(success)) => { - return WebhookTurnResult { - session_id: request.session_id.clone(), - model: model.to_string(), - outcome: WebhookTerminalOutcome::Completed, - response_text: Some(success.message.clone()), - event_frames: Vec::new(), - tools_called: Vec::new(), - }; - } - HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { - class: _, - failure, - }) => { - return WebhookTurnResult { - session_id: request.session_id.clone(), - model: model.to_string(), - outcome: WebhookTerminalOutcome::Failed, - response_text: Some(failure.message), - event_frames: Vec::new(), - tools_called: Vec::new(), - }; - } - HandledIngress::Handled(HandledIngressOutcome::Blocking(blocking)) => match blocking { - BlockingOutcome::ApprovalRequired { tool, reason } => { - return map_canonical_result( - &request, - model, - CanonicalWebhookResult::Blocking(BlockingOutcome::ApprovalRequired { - tool, - reason, - }), - ); - } - other => { - return map_canonical_result( - &request, - model, - CanonicalWebhookResult::Blocking(other), - ); - } - }, - HandledIngress::NotHandled => {} + let handled_ingress = + evaluate_webhook_ingress(memory.as_ref(), &tool_snapshot, &request, clamped_mode).await; + if let Some(result) = handled_ingress_to_webhook_result(&request, model, handled_ingress) { + return result; } let mut effective_config = config.clone(); @@ -660,6 +654,26 @@ mod tests { assert_eq!(frame.matches("id:").count(), 1); } + #[test] + fn handled_ingress_failure_maps_to_failed_webhook_result() { + let request = sample_request(WebhookSessionSource::Explicit); + let handled = HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { + class: crate::pre_execution::SessionCommandFailureClass::Failed, + failure: crate::session_commands::SessionCommandFailure { + kind: crate::session_commands::SessionCommandFailureKind::InvalidState, + message: "boom".into(), + command: "/tldr", + session_id: Some("webhook-123".into()), + }, + }); + + let result = handled_ingress_to_webhook_result(&request, "test-model", handled) + .expect("expected handled result"); + + assert_eq!(result.outcome, WebhookTerminalOutcome::Failed); + assert_eq!(result.response_text.as_deref(), Some("boom")); + } + #[test] fn maps_completed_agent_turn_into_completed_webhook_result() { let result = map_canonical_result( diff --git a/clients/agent-runtime/src/main.rs b/clients/agent-runtime/src/main.rs index a72252769..ce073a6d0 100644 --- a/clients/agent-runtime/src/main.rs +++ b/clients/agent-runtime/src/main.rs @@ -1632,6 +1632,41 @@ async fn maybe_handle_cli_handled_ingress( } } +async fn maybe_print_code_fast_path(config: &Config, message: Option<&str>) -> Result { + if let Some(raw_message) = message { + if let Some(result_message) = maybe_handle_cli_handled_ingress(config, raw_message).await? { + println!("{result_message}"); + return Ok(true); + } + } + + Ok(false) +} + +async fn run_code_message_or_interactive( + agent: &mut crate::agent::Agent, + message: Option, +) -> Result<()> { + let Some(message) = message else { + return agent.run_interactive().await; + }; + + let turn_result = agent + .turn_with_context(&message, crate::agent::TurnContext::default()) + .await; + + if let Ok(turn_result) = &turn_result { + if let Some(response) = turn_result.final_text.as_deref() { + println!("{response}"); + } + if let Some(err) = cli_blocking_error_from_turn_result(turn_result) { + return Err(err); + } + } + + turn_result.map(|_| ()) +} + async fn handle_code_command( config: Config, message: Option, @@ -1643,13 +1678,8 @@ async fn handle_code_command( ) -> Result<()> { let config = apply_code_session_config(config, provider, model, temperature, plan); - // Intercept shared handled-ingress commands before agent initialization. - if let Some(raw_message) = message.as_deref() { - if let Some(result_message) = maybe_handle_cli_handled_ingress(&config, raw_message).await? - { - println!("{result_message}"); - return Ok(()); - } + if maybe_print_code_fast_path(&config, message.as_deref()).await? { + return Ok(()); } info!("Starting code-specialist session (profile=code)"); @@ -1672,38 +1702,16 @@ async fn handle_code_command( agent.record_agent_start_event(&provider_name, &model_name); - let run_result = if let Some(msg) = message { - let turn_result = agent - .turn_with_context(&msg, crate::agent::TurnContext::default()) - .await; - if let Ok(turn_result) = &turn_result { - let blocking_err = cli_blocking_error_from_turn_result(turn_result); - if let Some(response) = turn_result.final_text.as_deref() { - println!("{response}"); - } - if let Some(err) = blocking_err { - let summary_result = agent.session_cost_summary(chrono::Utc::now()); - agent.record_agent_end_event(&provider_name, &model_name, session_start.elapsed()); - match summary_result { - Ok(summary) => print_cli_session_summary(summary, CliSessionSurface::Code), - Err(error) => { - tracing::warn!("Failed to load code session cost summary: {error}"); - } - } - return Err(err); - } - } - turn_result.map(|_| ()) - } else { - agent.run_interactive().await - }; + let run_result = run_code_message_or_interactive(&mut agent, message).await; - let summary_result = agent.session_cost_summary(chrono::Utc::now()); - agent.record_agent_end_event(&provider_name, &model_name, session_start.elapsed()); - match summary_result { - Ok(summary) => print_cli_session_summary(summary, CliSessionSurface::Code), - Err(error) => tracing::warn!("Failed to load code session cost summary: {error}"), - } + finish_cli_session( + &agent, + &provider_name, + &model_name, + session_start, + CliSessionSurface::Code, + "code", + ); run_result } diff --git a/clients/agent-runtime/src/security/policy.rs b/clients/agent-runtime/src/security/policy.rs index 71f5da683..ff38019e5 100644 --- a/clients/agent-runtime/src/security/policy.rs +++ b/clients/agent-runtime/src/security/policy.rs @@ -457,6 +457,43 @@ impl SecurityPolicy { .any(|w| w == "tee" || w.ends_with("/tee")) } + fn is_likely_path(arg: &str) -> bool { + (arg.contains('/') && !arg.contains(':')) + || arg.starts_with('~') + || arg.starts_with('.') + || arg.contains(std::path::MAIN_SEPARATOR) + } + + fn effective_path_arg(arg: &str) -> &str { + if arg.starts_with("--") { + arg.split_once('=').map(|(_, value)| value).unwrap_or(arg) + } else if arg.starts_with('-') && arg.len() > 2 { + arg.char_indices() + .nth(2) + .map(|(idx, _)| &arg[idx..]) + .unwrap_or("") + } else { + arg + } + } + + fn is_path_argument_safe(&self, effective_arg: &str) -> bool { + if !Self::is_likely_path(effective_arg) { + return true; + } + + if Path::new(effective_arg) + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + || (self.workspace_only + && (effective_arg.starts_with('/') || effective_arg.starts_with('~'))) + { + return false; + } + + !matches_any_forbidden_path(effective_arg, &self.forbidden_paths) + } + fn validate_command_segments(&self, command: &str) -> bool { let normalized = self.normalize_command(command); @@ -494,50 +531,18 @@ impl SecurityPolicy { .map(|arg| arg.to_ascii_lowercase()) .collect(); - // Helper to identify tokens that likely represent paths - fn is_likely_path(arg: &str) -> bool { - (arg.contains('/') && !arg.contains(':')) - || arg.starts_with('~') - || arg.starts_with('.') - || arg.contains(std::path::MAIN_SEPARATOR) - } - - // Ensure no argument is a forbidden path or a traversal attempt. - // We only check arguments that look like paths to avoid false positives - // on non-path tokens (e.g., git diff patterns, grep globs, brace literals). - for (_raw_arg, arg) in raw_args.iter().zip(normalized_args.iter()) { - // Extract potential path from flags (e.g. --file=/path or -C/path) - let effective_arg = if arg.starts_with("--") { - arg.split_once('=').map(|(_, v)| v).unwrap_or(arg) - } else if arg.starts_with('-') && arg.len() > 2 { - arg.char_indices() - .nth(2) - .map(|(idx, _)| &arg[idx..]) - .unwrap_or("") - } else { - arg - }; - - if !is_likely_path(effective_arg) { - continue; - } - - if Path::new(effective_arg) - .components() - .any(|c| matches!(c, std::path::Component::ParentDir)) - || (self.workspace_only - && (effective_arg.starts_with('/') || effective_arg.starts_with('~'))) - { + for arg in &normalized_args { + let effective_arg = Self::effective_path_arg(arg); + if !self.is_path_argument_safe(effective_arg) { return false; } + } - // Check against forbidden paths (e.g. /etc, ~/.ssh) - if matches_any_forbidden_path(effective_arg, &self.forbidden_paths) { - return false; - } + if !self.is_args_safe(base_raw, &args) { + return false; } - self.is_args_safe(base_raw, &args) + true } fn is_allowed_command(&self, base_raw: &str) -> bool { @@ -1434,6 +1439,12 @@ mod tests { // ── Edge cases: path traversal ────────────────────────── + #[test] + fn command_with_flag_embedded_absolute_path_is_blocked() { + let p = default_policy(); + assert!(!p.is_command_allowed("grep --file=/etc/passwd foo.txt")); + } + #[test] fn path_traversal_encoded_dots() { let p = default_policy(); diff --git a/clients/agent-runtime/src/tools/delegate_launch.rs b/clients/agent-runtime/src/tools/delegate_launch.rs index d243310ad..13daaf403 100644 --- a/clients/agent-runtime/src/tools/delegate_launch.rs +++ b/clients/agent-runtime/src/tools/delegate_launch.rs @@ -66,6 +66,92 @@ impl DelegateLaunchTool { })), } } + + fn parse_children_array(args: &serde_json::Value) -> anyhow::Result<&Vec> { + let children_val = match args.get("children") { + Some(v) => v, + None => anyhow::bail!("Missing 'children' parameter"), + }; + + match children_val.as_array() { + Some(items) if !items.is_empty() => Ok(items), + _ => anyhow::bail!("'children' must be a non-empty array"), + } + } + + fn child_rejects_streaming(item: &serde_json::Value) -> bool { + item.get("stream").is_some() + || item.get("stream_results").is_some() + || item.get("stream_tool_progress").is_some() + } + + fn parse_child_request( + item: &serde_json::Value, + launch_index: usize, + seen_ids: &mut std::collections::HashSet, + ) -> anyhow::Result { + if Self::child_rejects_streaming(item) { + anyhow::bail!("streaming payloads remain out of scope for this slice"); + } + + let child_id = match item.get("child_id").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child at index {launch_index} is missing a non-empty 'child_id'"), + }; + + if !seen_ids.insert(child_id.clone()) { + anyhow::bail!("Duplicate child_id '{child_id}'"); + } + + let agent_name = match item.get("agent_name").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child '{child_id}' is missing a non-empty 'agent_name'"), + }; + + let prompt = match item.get("prompt").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child '{child_id}' is missing a non-empty 'prompt'"), + }; + + let context = item + .get("context") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + let execution = item + .get("execution") + .cloned() + .map(serde_json::from_value::) + .transpose() + .map_err(|error| anyhow::anyhow!("invalid execution metadata: {error}"))?; + + let launch_index = u32::try_from(launch_index) + .map_err(|_| anyhow::anyhow!("Child '{child_id}' launch_index overflowed u32"))?; + + Ok(ChildLaunchRequest { + child_id: ChildAgentId(child_id), + agent_name, + prompt, + context, + launch_index, + execution, + }) + } + + fn validate_transport(request: &ChildLaunchRequest) -> Option { + if request + .execution + .as_ref() + .and_then(|spec| spec.transport.clone()) + == Some(CoordinatorTransport::RemoteBridge) + { + return Some(Self::structured_validation_error( + "remote_bridge_deferred", + "Requested child execution transport 'remote_bridge' is deferred and not available in the local orchestration slice", + )); + } + + None + } } #[async_trait] @@ -150,20 +236,9 @@ impl Tool for DelegateLaunchTool { } async fn execute(&self, args: serde_json::Value) -> anyhow::Result { - let children_val = match args.get("children") { - Some(v) => v, - None => { - return Ok(Self::validation_error("Missing 'children' parameter")); - } - }; - - let children_arr = match children_val.as_array() { - Some(a) if !a.is_empty() => a, - _ => { - return Ok(Self::validation_error( - "'children' must be a non-empty array", - )); - } + let children_arr = match Self::parse_children_array(&args) { + Ok(items) => items, + Err(error) => return Ok(Self::validation_error(error.to_string())), }; let mut child_requests: Vec = Vec::with_capacity(children_arr.len()); @@ -171,76 +246,16 @@ impl Tool for DelegateLaunchTool { std::collections::HashSet::with_capacity(children_arr.len()); for (launch_index, item) in children_arr.iter().enumerate() { - if item.get("stream").is_some() - || item.get("stream_results").is_some() - || item.get("stream_tool_progress").is_some() - { - return Ok(Self::validation_error( - "streaming payloads remain out of scope for this slice", - )); - } - - let child_id = match item.get("child_id").and_then(|v| v.as_str()) { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => { - return Ok(Self::validation_error(format!( - "Child at index {launch_index} is missing a non-empty 'child_id'" - ))); - } + let child_request = match Self::parse_child_request(item, launch_index, &mut seen_ids) { + Ok(request) => request, + Err(error) => return Ok(Self::validation_error(error.to_string())), }; - if !seen_ids.insert(child_id.clone()) { - return Ok(Self::validation_error(format!( - "Duplicate child_id '{child_id}'" - ))); + if let Some(result) = Self::validate_transport(&child_request) { + return Ok(result); } - let agent_name = match item.get("agent_name").and_then(|v| v.as_str()) { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => { - return Ok(Self::validation_error(format!( - "Child '{child_id}' is missing a non-empty 'agent_name'" - ))); - } - }; - - let prompt = match item.get("prompt").and_then(|v| v.as_str()) { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => { - return Ok(Self::validation_error(format!( - "Child '{child_id}' is missing a non-empty 'prompt'" - ))); - } - }; - - let context = item - .get("context") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()); - let execution = item - .get("execution") - .cloned() - .map(serde_json::from_value::) - .transpose() - .map_err(|error| anyhow::anyhow!("invalid execution metadata: {error}"))?; - - if execution.as_ref().and_then(|spec| spec.transport.clone()) - == Some(CoordinatorTransport::RemoteBridge) - { - return Ok(Self::structured_validation_error( - "remote_bridge_deferred", - "Requested child execution transport 'remote_bridge' is deferred and not available in the local orchestration slice", - )); - } - - child_requests.push(ChildLaunchRequest { - child_id: ChildAgentId(child_id), - agent_name, - prompt, - context, - launch_index: u32::try_from(launch_index).unwrap_or(u32::MAX), - execution, - }); + child_requests.push(child_request); } let request = CoordinatorLaunchRequest { @@ -396,6 +411,21 @@ mod tests { assert!(result.error.is_some()); } + #[tokio::test] + async fn rejects_child_without_agent_name_before_dispatch() { + let result = tool() + .execute(serde_json::json!({ + "children": [ + { "child_id": "a", "agent_name": "", "prompt": "p" } + ] + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("agent_name")); + } + #[tokio::test] async fn launch_rejects_remote_bridge_with_stable_reason_code() { let result = tool() diff --git a/clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt b/clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt index 7e9021c23..13c889348 100644 --- a/clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt +++ b/clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt @@ -72,12 +72,9 @@ class MobileRuntimeCoordinator( state = state.copy( bridgeSnapshot = - MobileBridgeSnapshot( - runtimeAvailable = readiness.runtimeAvailable, - linkEstablished = readiness.linkEstablished, - sessionCapable = readiness.sessionCapable, - sessionId = activeSessionId?.value, - environmentSupported = readiness.environmentSupported, + buildBridgeSnapshot( + readiness = readiness, + activeSessionId = activeSessionId, recoveryOverride = recoveryOverride, targetLabel = updatedLinkedMetadata?.targetId, ), @@ -88,6 +85,22 @@ class MobileRuntimeCoordinator( ) } + private fun buildBridgeSnapshot( + readiness: RuntimeReadinessSnapshot, + activeSessionId: RuntimeSessionId?, + recoveryOverride: MobileRecoveryKind?, + targetLabel: String?, + ): MobileBridgeSnapshot = + MobileBridgeSnapshot( + runtimeAvailable = readiness.runtimeAvailable, + linkEstablished = readiness.linkEstablished, + sessionCapable = readiness.sessionCapable, + sessionId = activeSessionId?.value, + environmentSupported = readiness.environmentSupported, + recoveryOverride = recoveryOverride, + targetLabel = targetLabel, + ) + private fun computeActiveSessionId( persistedSessionId: RuntimeSessionId?, sessions: List, @@ -140,12 +153,9 @@ class MobileRuntimeCoordinator( persistence.saveActiveSessionId(session.id) state = state.copy( - bridgeSnapshot = - state.bridgeSnapshot.copy(sessionId = session.id.value, recoveryOverride = null), + bridgeSnapshot = bridgeSnapshotForSession(session.id), activeSessionId = session.id, - resumableSessions = - (state.resumableSessions - - state.resumableSessions.filter { it.id == session.id }.toSet()) + session, + resumableSessions = replaceOrAppendSession(state.resumableSessions, session), messages = emptyList(), pendingApproval = null, ) @@ -159,11 +169,9 @@ class MobileRuntimeCoordinator( persistence.saveActiveSessionId(session.id) state = state.copy( - bridgeSnapshot = - state.bridgeSnapshot.copy(sessionId = session.id.value, recoveryOverride = null), + bridgeSnapshot = bridgeSnapshotForSession(session.id), activeSessionId = session.id, - resumableSessions = - state.resumableSessions.map { if (it.id == session.id) session else it }, + resumableSessions = replaceOrAppendSession(state.resumableSessions, session), pendingApproval = null, ) } @@ -177,7 +185,7 @@ class MobileRuntimeCoordinator( persistence.clearActiveSessionId() state = state.copy( - bridgeSnapshot = state.bridgeSnapshot.copy(sessionId = null, recoveryOverride = null), + bridgeSnapshot = clearedSessionBridgeSnapshot(), activeSessionId = null, messages = emptyList(), pendingApproval = null, @@ -224,6 +232,23 @@ class MobileRuntimeCoordinator( ) } + private fun bridgeSnapshotForSession(sessionId: RuntimeSessionId): MobileBridgeSnapshot = + state.bridgeSnapshot.copy(sessionId = sessionId.value, recoveryOverride = null) + + private fun clearedSessionBridgeSnapshot(): MobileBridgeSnapshot = + state.bridgeSnapshot.copy(sessionId = null, recoveryOverride = null) + + private fun replaceOrAppendSession( + sessions: List, + session: RuntimeSession, + ): List = (sessions - sessions.filter { it.id == session.id }.toSet()) + session + + private fun replaceExistingSession( + sessions: List, + session: RuntimeSession, + ): List = + sessions.map { existing -> if (existing.id == session.id) session else existing } + private fun applyTurnResult(result: RuntimeTurnResult) { val assistantMessages = mutableListOf() var pendingApproval: RuntimeApprovalRequest? = state.pendingApproval @@ -237,14 +262,9 @@ class MobileRuntimeCoordinator( assistantChatMessage(state.messages.size, event.text, assistantMessages.size) is RuntimeEvent.ApprovalPending -> pendingApproval = event.request - is RuntimeEvent.Failure -> { + is RuntimeEvent.Failure -> assistantMessages += - ChatMessage( - id = computeNextMessageId(state.messages.size, assistantMessages.size), - role = ChatRole.Assistant, - content = event.message, - ) - } + assistantChatMessage(state.messages.size, event.message, assistantMessages.size) } } state = diff --git a/clients/web/apps/dashboard/package.json b/clients/web/apps/dashboard/package.json index 1830c4baa..86a13d52d 100644 --- a/clients/web/apps/dashboard/package.json +++ b/clients/web/apps/dashboard/package.json @@ -11,8 +11,8 @@ "dev": "portless run --name dashboard vite", "build": "vue-tsc -b --noEmit && vite build", "preview": "portless run --name dashboard vite preview", - "format": "biome format --write src package.json components.json tsconfig*.json vite.config.ts index.html", - "check": "biome check src package.json components.json tsconfig*.json vite.config.ts index.html", + "format": "biome format --write src package.json components.json tsconfig*.json vite.config.ts vitest.config.ts index.html", + "check": "biome check src package.json components.json tsconfig*.json vite.config.ts vitest.config.ts index.html", "test": "vitest --run --environment happy-dom --exclude e2e/**", "test:a11y": "vitest --run --environment happy-dom src/App.spec.ts src/components/memory/MemoryList.spec.ts src/components/chat/ChatWorkspace.spec.ts", "test:coverage": "node ./scripts/run-coverage.mjs", diff --git a/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.spec.ts b/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.spec.ts index b86fc7ff6..102e19e14 100644 --- a/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.spec.ts +++ b/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.spec.ts @@ -55,6 +55,15 @@ describe("CerebroSessionActions", () => { expect(wrapper.text()).toContain("not_implemented"); expect(wrapper.text()).toContain("Context Lookup"); expect(wrapper.findAll("button").some((button) => button.text() === "Run")).toBe(false); + + const buttons = wrapper.findAll("button"); + expect(buttons).toHaveLength(4); + expect(buttons.map((button) => button.text())).toEqual([ + "not_implemented", + "not_implemented", + "not_implemented", + "not_implemented", + ]); }); it("disables deferred context lookup and surfaces deferred messaging", async () => { diff --git a/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.vue b/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.vue index eec584588..dc4cc1126 100644 --- a/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.vue +++ b/clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.vue @@ -21,9 +21,28 @@ const sessionTools: Array<{ tool: CerebroToolName; label: string }> = [ { tool: "mem_context", label: "Context Lookup" }, ]; +function getToolState(tool: CerebroToolName): string | undefined { + return props.status?.tools[tool]?.state; +} + +// biome-ignore lint/correctness/noUnusedVariables: Used in Vue template. +function getToolMessage(tool: CerebroToolName): string | undefined { + const toolStatus = props.status?.tools[tool]; + return toolStatus?.message ?? toolStatus?.state; +} + +function isToolDisabled(tool: CerebroToolName): boolean { + return getToolState(tool) !== "available" || pendingActions.value.has(tool); +} + +// biome-ignore lint/correctness/noUnusedVariables: Used in Vue template. +function getToolActionLabel(tool: CerebroToolName): string { + return getToolState(tool) === "available" ? "Run" : (getToolState(tool) ?? "unavailable"); +} + // biome-ignore lint/correctness/noUnusedVariables: Used in Vue template. async function invoke(tool: CerebroToolName) { - if (pendingActions.value.has(tool) || props.status?.tools[tool]?.state !== "available") { + if (isToolDisabled(tool)) { return; } @@ -61,13 +80,10 @@ async function invoke(tool: CerebroToolName) {
  • {{ entry.label }} -

    {{ status?.tools[entry.tool]?.message ?? status?.tools[entry.tool]?.state }}

    +

    {{ getToolMessage(entry.tool) }}

    -
    • diff --git a/clients/web/apps/dashboard/src/composables/useAdmin.spec.ts b/clients/web/apps/dashboard/src/composables/useAdmin.spec.ts index 9963297c8..76f0b417d 100644 --- a/clients/web/apps/dashboard/src/composables/useAdmin.spec.ts +++ b/clients/web/apps/dashboard/src/composables/useAdmin.spec.ts @@ -62,6 +62,23 @@ describe("useAdmin", () => { expect(new Headers(init?.headers).get("Authorization")).toBe("Bearer test-token"); }); + it("uses zero offset for the first page with a custom page size", async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ sessions: [], total: 0, limit: 25, offset: 0 }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + const admin = createAdmin(); + await admin.fetchSessions({ page: 1, per_page: 25 }); + + const [url] = fetchMock.mock.calls[0] ?? []; + const parsed = new URL(url as string); + expect(parsed.searchParams.get("limit")).toBe("25"); + expect(parsed.searchParams.get("offset")).toBe("0"); + }); + it("populates sessions ref and totalSessions on success", async () => { fetchMock.mockResolvedValueOnce( new Response( diff --git a/clients/web/apps/dashboard/src/composables/useAdmin.ts b/clients/web/apps/dashboard/src/composables/useAdmin.ts index feb354bc1..3338d2791 100644 --- a/clients/web/apps/dashboard/src/composables/useAdmin.ts +++ b/clients/web/apps/dashboard/src/composables/useAdmin.ts @@ -60,6 +60,13 @@ type RequestBucket = | "cerebroAction" | "cost"; +function resolvePageOffset(page?: number, perPage = 50): number | undefined { + if (!page) { + return undefined; + } + return (page - 1) * perPage; +} + function isNormalizedCerebroError(value: unknown): value is AdminCerebroActionError { if (!value || typeof value !== "object") { return false; @@ -233,7 +240,7 @@ export function useAdmin( { status: params.status, limit: params.per_page, - offset: params.page ? (params.page - 1) * (params.per_page ?? 50) : undefined, + offset: resolvePageOffset(params.page, params.per_page ?? 50), sort: params.sort, order: params.order, } @@ -321,7 +328,7 @@ export function useAdmin( session_id: params.session_id, q: params.search, limit: params.per_page, - offset: params.page ? (params.page - 1) * (params.per_page ?? 50) : undefined, + offset: resolvePageOffset(params.page, params.per_page ?? 50), } ); } catch (e: unknown) { diff --git a/clients/web/apps/dashboard/src/utils/playwrightEnv.ts b/clients/web/apps/dashboard/src/utils/playwrightEnv.ts index c4ea20d90..706cd7c12 100644 --- a/clients/web/apps/dashboard/src/utils/playwrightEnv.ts +++ b/clients/web/apps/dashboard/src/utils/playwrightEnv.ts @@ -1,3 +1,11 @@ -export function isPlaywrightFsAllowMode(env: Record = process.env) { +function readDefaultEnv(): Record { + return ( + (globalThis as { process?: { env?: Record } }).process?.env ?? {} + ); +} + +export function isPlaywrightFsAllowMode( + env: Record = readDefaultEnv() +) { return env.NODE_ENV === "test" || env.PLAYWRIGHT === "true"; } diff --git a/clients/web/apps/dashboard/vite.config.js b/clients/web/apps/dashboard/vite.config.js index 83fa4b2c2..72a6ea546 100644 --- a/clients/web/apps/dashboard/vite.config.js +++ b/clients/web/apps/dashboard/vite.config.js @@ -1,36 +1 @@ -import path from "node:path"; -import { fileURLToPath, URL } from "node:url"; -import vue from "@vitejs/plugin-vue"; -import { defineConfig } from "vite"; -const repoRoot = fileURLToPath(new URL("../../../../", import.meta.url)); -const isPlaywrightFsAllowMode = process.env.NODE_ENV === "test" || process.env.PLAYWRIGHT === "true"; -const fsAllow = isPlaywrightFsAllowMode - ? [repoRoot] - : [ - path.join(repoRoot, "openspec"), - path.join(repoRoot, "tmp"), - path.join(repoRoot, "clients/composeApp"), - ]; -export default defineConfig({ - plugins: [vue()], - server: { - fs: { - allow: fsAllow, - }, - }, - resolve: { - alias: { - "@": fileURLToPath(new URL("./src", import.meta.url)), - }, - }, - build: { - rollupOptions: { - output: { - manualChunks: { - vendor: ["vue", "vue-i18n"], - ui: ["@corvus/ui", "@corvus/locales"], - }, - }, - }, - }, -}); +export { default } from "./vite.config.ts"; diff --git a/clients/web/apps/dashboard/vite.config.ts b/clients/web/apps/dashboard/vite.config.ts index f3e3d25ac..f6d3b177f 100644 --- a/clients/web/apps/dashboard/vite.config.ts +++ b/clients/web/apps/dashboard/vite.config.ts @@ -28,31 +28,4 @@ export default defineConfig({ "@": fileURLToPath(new URL("./src", import.meta.url)), }, }, - build: { - rollupOptions: { - output: { - manualChunks: { - vendor: ["vue", "vue-i18n"], - ui: ["@corvus/ui", "@corvus/locales"], - }, - }, - }, - }, - test: { - environment: "happy-dom", - include: ["src/**/*.spec.ts"], - exclude: ["e2e/**"], - server: { - fs: { - // Tests need broader access; use repoRoot for test mode - allow: isTestMode - ? [repoRoot] - : [ - path.join(repoRoot, "openspec"), - path.join(repoRoot, "tmp"), - path.join(repoRoot, "clients/composeApp"), - ], - }, - }, - }, }); diff --git a/clients/web/apps/dashboard/vitest.config.ts b/clients/web/apps/dashboard/vitest.config.ts new file mode 100644 index 000000000..afb3c6ad0 --- /dev/null +++ b/clients/web/apps/dashboard/vitest.config.ts @@ -0,0 +1,35 @@ +import path from "node:path"; +import { fileURLToPath, URL } from "node:url"; + +import vue from "@vitejs/plugin-vue"; +import { defineConfig } from "vitest/config"; + +import { isPlaywrightFsAllowMode } from "./src/utils/playwrightEnv"; + +const repoRoot = fileURLToPath(new URL("../../../../", import.meta.url)); +const isTestMode = isPlaywrightFsAllowMode(); + +export default defineConfig({ + plugins: [vue()], + resolve: { + alias: { + "@": fileURLToPath(new URL("./src", import.meta.url)), + }, + }, + server: { + fs: { + allow: isTestMode + ? [repoRoot] + : [ + path.join(repoRoot, "openspec"), + path.join(repoRoot, "tmp"), + path.join(repoRoot, "clients/composeApp"), + ], + }, + }, + test: { + environment: "happy-dom", + include: ["src/**/*.spec.ts"], + exclude: ["e2e/**"], + }, +}); diff --git a/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-1-backend-critical.md b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-1-backend-critical.md new file mode 100644 index 000000000..ae0ac290b --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-1-backend-critical.md @@ -0,0 +1,13 @@ +--- +title: Plan de Implementación SonarQube Batch 1 Backend Critical +description: Plan de implementación para el primer lote de remediación de SonarQube centrado en issues críticos backend Rust del agent runtime. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: agent-runtime Rust remediation +docType: architecture +--- + +# Plan de Implementación SonarQube Batch 1 Backend Critical + +_Esta página refleja el plan en inglés mientras se prepara una traducción completa._ diff --git a/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md new file mode 100644 index 000000000..2424e2b14 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md @@ -0,0 +1,13 @@ +--- +title: Plan de Implementación SonarQube Batch 2 Frontend Critical y Accesibilidad +description: Plan de implementación para el segundo lote de remediación de SonarQube centrado en dashboard, rook-dashboard y accesibilidad. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: web dashboard and rook-dashboard remediation +docType: architecture +--- + +# Plan de Implementación SonarQube Batch 2 Frontend Critical y Accesibilidad + +_Esta página refleja el plan en inglés mientras se prepara una traducción completa._ diff --git a/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md new file mode 100644 index 000000000..550189ca6 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md @@ -0,0 +1,13 @@ +--- +title: Plan de Implementación SonarQube Batch 3 Scripts, Kotlin y CSS Residual +description: Plan de implementación para el tercer lote de remediación de SonarQube centrado en scripts shell, limpieza Kotlin y pequeños issues residuales de CSS. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: scripts and compose runtime remediation +docType: architecture +--- + +# Plan de Implementación SonarQube Batch 3 Scripts, Kotlin y CSS Residual + +_Esta página refleja el plan en inglés mientras se prepara una traducción completa._ diff --git a/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-remediation-plan.md b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-remediation-plan.md new file mode 100644 index 000000000..489c9b862 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/es/plans/2026-04-26-sonarqube-remediation-plan.md @@ -0,0 +1,55 @@ +--- +title: "Plan de remediación SonarQube" +date: 2026-04-26 +last_updated: 2026-04-26 +tags: [sonarqube, remediation, quality, plan] +status: draft +summary: "Plan por lotes para resolver todos los issues abiertos actuales de SonarQube en Corvus por prioridad y dominio." +description: "Plan de remediación de SonarQube a nivel de repositorio que cubre backend, frontend, accesibilidad, scripts y trabajo de seguimiento en Kotlin." +owner: team-platform +lastReviewed: 2026-04-26 +appliesTo: corvus runtime, web, and tooling remediation +docType: architecture +--- + +# Plan de remediación SonarQube + +_Esta página refleja el plan en inglés mientras se prepara una traducción completa._ + +## Objetivo + +Resolver todos los issues abiertos actuales de SonarQube para `dallay_corvus` en una secuencia controlada que reduzca primero el riesgo mientras mantiene cada lote de implementación coherente y revisable. + +## Alcance + +Este plan cubre el conjunto actual de issues abiertos ya identificados en SonarQube, incluyendo: + +- issues de complejidad en runtime/gateway/security de Rust +- issues frontend de dashboard y rook-dashboard +- issues de accesibilidad y CSS +- issues de mantenibilidad en scripts shell +- issue de duplicación en Kotlin + +## Estrategia de ejecución + +Usar **lotes por prioridad + dominio** en lugar de resolver todo en una sola pasada mixta. + +## Plan por lotes + +Seguir la misma secuencia descrita en la versión en inglés para Batch 1, Batch 2 y Batch 3. + +## Restricciones + +Mantener cada cambio enfocado, validable y alineado con el comportamiento existente. + +## Criterios de éxito + +El trabajo queda cerrado cuando los lotes planificados están implementados, validados y no quedan issues abiertos relevantes en SonarQube para este esfuerzo. + +## Riesgos y mitigaciones + +Los cambios de mayor riesgo son los de runtime, gateway y security; se deben validar con checks dirigidos y revertirse por lote si aparece una regresión. + +## Siguiente paso + +Usar este plan como guía de ejecución y como resumen del alcance para el seguimiento del remediation work. diff --git a/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-1-backend-critical.md b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-1-backend-critical.md new file mode 100644 index 000000000..cde0ea124 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-1-backend-critical.md @@ -0,0 +1,927 @@ +--- +title: SonarQube Batch 1 Backend Critical Implementation Plan +description: Implementation plan for the first SonarQube remediation batch focused on critical backend Rust issues in the agent runtime. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: agent-runtime Rust remediation +docType: architecture +--- + +# SonarQube Batch 1 Backend Critical Implementation Plan + +> **For agentic workers:** Implement this plan task-by-task using the `dispatching-parallel-agents` +> skill for independent tasks, or execute inline with review checkpoints. +> Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Resolve the current Rust CRITICAL SonarQube issues in the agent runtime by reducing cognitive complexity without changing CLI, gateway, orchestration, or security behavior. + +**Architecture:** Keep each fix local to its current module and favor extraction over rewrites. The main strategy is to split validation, early-return handling, outcome mapping, and path-argument checks into small pure helpers so Sonar complexity falls while behavior remains byte-for-byte equivalent at the contract level. + +**Tech Stack:** Rust, Tokio, Axum, Serde/serde_json, existing inline unit tests in `main.rs`, `gateway/mod.rs`, `gateway/webhook_dispatch.rs`, `security/policy.rs`, and `tools/delegate_launch.rs`, plus `cargo fmt`, `cargo clippy`, and targeted `cargo test`. + +--- + +## File Structure + +### Files to modify + +- `clients/agent-runtime/src/tools/delegate_launch.rs` + - Split child request validation/parsing out of `DelegateLaunchTool::execute`. + - Keep structured validation errors and launch contract unchanged. +- `clients/agent-runtime/src/main.rs` + - Split code-session fast path handling, run execution, and summary/finalization logic out of `handle_code_command`. +- `clients/agent-runtime/src/gateway/webhook_dispatch.rs` + - Split handled-ingress mapping logic out of `execute`. + - Keep webhook terminal outcome mapping unchanged. +- `clients/agent-runtime/src/gateway/mod.rs` + - Split auth/session/idempotency/dispatcher/legacy branches for `/webhook` and `/web/chat/stream`. + - Preserve current status codes, JSON bodies, and SSE behavior. +- `clients/agent-runtime/src/security/policy.rs` + - Split path-token parsing and path safety checks out of `is_segment_valid`. + - Preserve deny-by-default behavior. + +### Existing tests to extend in-place + +- `clients/agent-runtime/src/tools/delegate_launch.rs` +- `clients/agent-runtime/src/main.rs` +- `clients/agent-runtime/src/gateway/webhook_dispatch.rs` +- `clients/agent-runtime/src/gateway/mod.rs` +- `clients/agent-runtime/src/security/policy.rs` + +### Validation commands + +Run from: `clients/agent-runtime` + +```bash +cargo fmt --all -- --check +cargo clippy --all-targets -- -D warnings +cargo test delegate_launch::tests --lib +cargo test cli_shared_ingress_handles_compact_before_agent_execution --lib +cargo test cli_session_command_success_returns_message --lib +cargo test webhook_dispatch::tests --lib +cargo test gateway::mod::tests::legacy_webhook_preview_does_not_emit_synthetic_events_sse --lib +cargo test gateway::mod::tests::legacy_webhook_preview_intercepts_slash_session_commands --lib +cargo test gateway::mod::tests::webhook_non_preview_blocks_approval_and_keeps_session_id --lib +cargo test gateway::mod::tests::webhook_non_preview_unblocks_with_approval_override --lib +cargo test policy::tests --lib +``` + +Expected result: + +- `cargo fmt` exits 0 with no diff. +- `cargo clippy` exits 0. +- All targeted tests pass. + +--- + +## Task 1: Reduce complexity in `delegate_launch.rs` + +**Files:** +- Modify: `clients/agent-runtime/src/tools/delegate_launch.rs` +- Test: inline tests in `clients/agent-runtime/src/tools/delegate_launch.rs` + +- [ ] **Step 1: Add a focused validation test for child parsing before changing production code** + +Add one more validation-oriented test near the existing `delegate_launch` tests: + +```rust +#[tokio::test] +async fn rejects_child_without_agent_name_before_dispatch() { + let result = tool() + .execute(serde_json::json!({ + "children": [ + { "child_id": "a", "agent_name": "", "prompt": "p" } + ] + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result + .error + .as_deref() + .unwrap_or("") + .contains("agent_name")); +} +``` + +- [ ] **Step 2: Run the delegate launch test slice before refactoring** + +Run: + +```bash +cargo test delegate_launch::tests --lib +``` + +Expected: PASS on current tests, plus PASS on the new one after compilation. + +- [ ] **Step 3: Extract child array lookup and validation from `execute`** + +Inside `impl DelegateLaunchTool`, add small helpers above `execute`: + +```rust +fn parse_children_array<'a>(args: &'a serde_json::Value) -> anyhow::Result<&'a Vec> { + let children_val = match args.get("children") { + Some(v) => v, + None => return Err(anyhow::anyhow!("Missing 'children' parameter")), + }; + + match children_val.as_array() { + Some(items) if !items.is_empty() => Ok(items), + _ => Err(anyhow::anyhow!("'children' must be a non-empty array")), + } +} + +fn child_rejects_streaming(item: &serde_json::Value) -> bool { + item.get("stream").is_some() + || item.get("stream_results").is_some() + || item.get("stream_tool_progress").is_some() +} +``` + +Then replace the inlined `children` extraction in `execute` with: + +```rust +let children_arr = match Self::parse_children_array(&args) { + Ok(items) => items, + Err(error) => return Ok(Self::validation_error(error.to_string())), +}; +``` + +- [ ] **Step 4: Extract per-child parsing into a single helper** + +Add a helper that returns a ready `ChildLaunchRequest` and updates duplicate tracking through a mutable `HashSet` argument: + +```rust +fn parse_child_request( + item: &serde_json::Value, + launch_index: usize, + seen_ids: &mut std::collections::HashSet, +) -> anyhow::Result { + if Self::child_rejects_streaming(item) { + anyhow::bail!("streaming payloads remain out of scope for this slice"); + } + + let child_id = match item.get("child_id").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child at index {launch_index} is missing a non-empty 'child_id'"), + }; + + if !seen_ids.insert(child_id.clone()) { + anyhow::bail!("Duplicate child_id '{child_id}'"); + } + + let agent_name = match item.get("agent_name").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child '{child_id}' is missing a non-empty 'agent_name'"), + }; + + let prompt = match item.get("prompt").and_then(|v| v.as_str()) { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => anyhow::bail!("Child '{child_id}' is missing a non-empty 'prompt'"), + }; + + let context = item + .get("context") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + let execution = item + .get("execution") + .cloned() + .map(serde_json::from_value::) + .transpose() + .map_err(|error| anyhow::anyhow!("invalid execution metadata: {error}"))?; + + Ok(ChildLaunchRequest { + child_id: ChildAgentId(child_id), + agent_name, + prompt, + context, + launch_index: u32::try_from(launch_index).unwrap_or(u32::MAX), + execution, + }) +} +``` + +- [ ] **Step 5: Extract remote bridge rejection into a dedicated helper** + +Add: + +```rust +fn validate_transport(request: &ChildLaunchRequest) -> Option { + if request.execution.as_ref().and_then(|spec| spec.transport.clone()) + == Some(CoordinatorTransport::RemoteBridge) + { + return Some(Self::structured_validation_error( + "remote_bridge_deferred", + "Requested child execution transport 'remote_bridge' is deferred and not available in the local orchestration slice", + )); + } + + None +} +``` + +Then make the loop in `execute` look like: + +```rust +for (launch_index, item) in children_arr.iter().enumerate() { + let child_request = match Self::parse_child_request(item, launch_index, &mut seen_ids) { + Ok(request) => request, + Err(error) => return Ok(Self::validation_error(error.to_string())), + }; + + if let Some(result) = Self::validate_transport(&child_request) { + return Ok(result); + } + + child_requests.push(child_request); +} +``` + +- [ ] **Step 6: Re-run delegate launch tests after the extraction** + +Run: + +```bash +cargo test delegate_launch::tests --lib +``` + +Expected: PASS. No snapshot/structured output regressions. + +--- + +## Task 2: Reduce complexity in `main.rs` `handle_code_command` + +**Files:** +- Modify: `clients/agent-runtime/src/main.rs` +- Test: inline tests in `clients/agent-runtime/src/main.rs` + +- [ ] **Step 1: Preserve the handled-ingress fast path with an explicit helper contract** + +Add a helper above `handle_code_command`: + +```rust +async fn maybe_print_code_fast_path(config: &Config, message: Option<&str>) -> Result { + if let Some(raw_message) = message { + if let Some(result_message) = maybe_handle_cli_handled_ingress(config, raw_message).await? { + println!("{result_message}"); + return Ok(true); + } + } + + Ok(false) +} +``` + +Then replace the current nested block in `handle_code_command` with: + +```rust +if maybe_print_code_fast_path(&config, message.as_deref()).await? { + return Ok(()); +} +``` + +- [ ] **Step 2: Extract the message-vs-interactive execution path** + +Add a code-surface-specific helper instead of keeping the long inline block: + +```rust +async fn run_code_message_or_interactive( + agent: &mut crate::agent::Agent, + message: Option, + provider_name: &str, + model_name: &str, + session_start: Instant, +) -> Result<()> { + let Some(message) = message else { + return agent.run_interactive().await; + }; + + let turn_result = agent + .turn_with_context(&message, crate::agent::TurnContext::default()) + .await; + + if let Ok(turn_result) = &turn_result { + if let Some(response) = turn_result.final_text.as_deref() { + println!("{response}"); + } + if let Some(err) = cli_blocking_error_from_turn_result(turn_result) { + finish_cli_session( + agent, + provider_name, + model_name, + session_start, + CliSessionSurface::Code, + "code", + ); + return Err(err); + } + } + + turn_result.map(|_| ()) +} +``` + +- [ ] **Step 3: Simplify `handle_code_command` to orchestration-only flow** + +Make the function body use the same shape as the existing agent helper pattern: + +```rust +let mut agent = crate::agent::Agent::code_from_config(&config)?; +let session_start = Instant::now(); + +if override_budget { + apply_cli_budget_override(&agent, CliSessionSurface::Code)?; +} + +agent.record_agent_start_event(&provider_name, &model_name); +let run_result = run_code_message_or_interactive( + &mut agent, + message, + &provider_name, + &model_name, + session_start, +) +.await; +finish_cli_session( + &agent, + &provider_name, + &model_name, + session_start, + CliSessionSurface::Code, + "code", +); +run_result +``` + +This must replace the current long `if let Some(msg)` branch and the duplicated summary/finalization block. + +- [ ] **Step 4: Run the inline `main.rs` handled-ingress tests** + +Run: + +```bash +cargo test cli_shared_ingress_handles_compact_before_agent_execution --lib +cargo test cli_session_command_success_returns_message --lib +cargo test cli_resume_target_without_caller_scope_preserves_denied_error_path --lib +cargo test cli_unknown_slash_like_input_falls_through --lib +cargo test cli_tools_command_returns_effective_tool_listing --lib +``` + +Expected: PASS. These tests confirm the refactor preserved fast-path behavior and error propagation. + +--- + +## Task 3: Reduce complexity in `webhook_dispatch.rs` `execute` + +**Files:** +- Modify: `clients/agent-runtime/src/gateway/webhook_dispatch.rs` +- Test: inline tests in `clients/agent-runtime/src/gateway/webhook_dispatch.rs` + +- [ ] **Step 1: Extract handled-ingress mapping into a pure helper** + +Add a helper that converts the `HandledIngress` branch into an optional `WebhookTurnResult`: + +```rust +fn handled_ingress_to_webhook_result( + request: &WebhookTurnRequest, + model: &str, + handled: HandledIngress, +) -> Option { + match handled { + HandledIngress::Handled(HandledIngressOutcome::SessionCommandSuccess(success)) => { + Some(WebhookTurnResult { + session_id: request.session_id.clone(), + model: model.to_string(), + outcome: WebhookTerminalOutcome::Completed, + response_text: Some(success.message.clone()), + event_frames: Vec::new(), + tools_called: Vec::new(), + }) + } + HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { failure, .. }) => { + Some(WebhookTurnResult { + session_id: request.session_id.clone(), + model: model.to_string(), + outcome: WebhookTerminalOutcome::Failed, + response_text: Some(failure.message), + event_frames: Vec::new(), + tools_called: Vec::new(), + }) + } + HandledIngress::Handled(HandledIngressOutcome::Blocking(blocking)) => { + Some(map_canonical_result( + request, + model, + CanonicalWebhookResult::Blocking(blocking), + )) + } + HandledIngress::NotHandled => None, + } +} +``` + +- [ ] **Step 2: Replace the large ingress match in `execute` with the helper** + +Replace the current block beginning at: + +```rust +match evaluate_webhook_ingress(...).await { +``` + +with: + +```rust +let handled_ingress = evaluate_webhook_ingress( + memory.as_ref(), + &tool_snapshot, + &request, + clamped_mode, +) +.await; + +if let Some(result) = handled_ingress_to_webhook_result(&request, model, handled_ingress) { + return result; +} +``` + +- [ ] **Step 3: Add a narrow test for the extracted helper behavior** + +Add a test near the existing webhook dispatch tests: + +```rust +#[test] +fn handled_ingress_failure_maps_to_failed_webhook_result() { + let request = sample_request(WebhookSessionSource::Explicit); + let handled = HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { + class: crate::pre_execution::SessionCommandFailureClass::Failed, + failure: crate::session_commands::SessionCommandFailure { + kind: crate::session_commands::SessionCommandFailureKind::UnknownSession, + message: "boom".into(), + }, + }); + + let result = handled_ingress_to_webhook_result(&request, "test-model", handled) + .expect("expected handled result"); + + assert_eq!(result.outcome, WebhookTerminalOutcome::Failed); + assert_eq!(result.response_text.as_deref(), Some("boom")); +} +``` + +- [ ] **Step 4: Run the webhook dispatch test slice** + +Run: + +```bash +cargo test webhook_dispatch::tests --lib +``` + +Expected: PASS. Outcome mapping, SSE sanitization, and budget governance behavior stay identical. + +--- + +## Task 4: Reduce complexity in `security/policy.rs` `is_segment_valid` + +**Files:** +- Modify: `clients/agent-runtime/src/security/policy.rs` +- Test: inline tests in `clients/agent-runtime/src/security/policy.rs` + +- [ ] **Step 1: Add a focused path-flag regression test** + +Add a test that protects the flag-value path parsing behavior before refactoring: + +```rust +#[test] +fn command_with_flag_embedded_absolute_path_is_blocked() { + let p = default_policy(); + assert!(!p.is_command_allowed("grep --file=/etc/passwd foo.txt")); +} +``` + +- [ ] **Step 2: Extract likely-path and effective-arg parsing into helpers** + +Move the nested helper logic out of `is_segment_valid` into private methods: + +```rust +fn is_likely_path(arg: &str) -> bool { + (arg.contains('/') && !arg.contains(':')) + || arg.starts_with('~') + || arg.starts_with('.') + || arg.contains(std::path::MAIN_SEPARATOR) +} + +fn effective_path_arg<'a>(arg: &'a str) -> &'a str { + if arg.starts_with("--") { + arg.split_once('=').map(|(_, value)| value).unwrap_or(arg) + } else if arg.starts_with('-') && arg.len() > 2 { + arg.char_indices() + .nth(2) + .map(|(idx, _)| &arg[idx..]) + .unwrap_or("") + } else { + arg + } +} +``` + +- [ ] **Step 3: Extract path safety validation into a helper** + +Add: + +```rust +fn is_path_argument_safe(&self, effective_arg: &str) -> bool { + if !Self::is_likely_path(effective_arg) { + return true; + } + + if Path::new(effective_arg) + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + || (self.workspace_only && (effective_arg.starts_with('/') || effective_arg.starts_with('~'))) + { + return false; + } + + !matches_any_forbidden_path(effective_arg, &self.forbidden_paths) +} +``` + +Then replace the long `for` loop body in `is_segment_valid` with: + +```rust +for arg in &normalized_args { + let effective_arg = Self::effective_path_arg(arg); + if !self.is_path_argument_safe(effective_arg) { + return false; + } +} +``` + +- [ ] **Step 4: Re-run the policy test slice** + +Run: + +```bash +cargo test policy::tests --lib +``` + +Expected: PASS. Read-only blocking, allowlist behavior, path traversal blocking, and medium/high-risk command handling stay unchanged. + +--- + +## Task 5: Reduce complexity in `gateway/mod.rs` `handle_webhook` + +**Files:** +- Modify: `clients/agent-runtime/src/gateway/mod.rs` +- Test: inline tests in `clients/agent-runtime/src/gateway/mod.rs` + +- [ ] **Step 1: Extract session upsert failure policy into a helper** + +Add a helper near `handle_webhook`: + +```rust +async fn ensure_webhook_session( + state: &AppState, + session_id: &str, + token_hash: Option<&str>, + reserved_idempotency_key: Option<&str>, +) -> Option { + if let Err(error) = state.mem.upsert_session(session_id, token_hash).await { + if token_hash.is_some() { + tracing::error!("session upsert failed for token-scoped request: {error:#}"); + release_idempotency_key(state, reserved_idempotency_key, false); + return Some(( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Session tracking failed"})), + )); + } + tracing::debug!("session upsert best-effort failed: {error}"); + } + + None +} +``` + +- [ ] **Step 2: Extract the dispatcher branch into a helper** + +Add: + +```rust +async fn execute_dispatcher_webhook( + state: &AppState, + config: &Config, + session_id: &str, + session_source: webhook_dispatch::WebhookSessionSource, + token_hash: Option, + message: &str, + webhook_body_execution_mode: Option, + server_execution_mode: ExecutionMode, + is_preview: bool, + reserved_idempotency_key: Option<&str>, +) -> WebhookResponse { + log_webhook_runtime_path(session_id, true, "dispatcher_flag_enabled"); + let dispatch_result = webhook_dispatch::execute( + config, + Arc::clone(&state.provider), + Arc::clone(&state.mem), + Arc::clone(&state.observer), + state.cost_tracker.clone(), + &state.model, + webhook_dispatch::WebhookTurnRequest { + session_id: session_id.to_string(), + session_source, + caller_token_hash: token_hash.clone(), + message: message.to_string(), + execution_mode: resolve_webhook_execution_mode( + server_execution_mode, + webhook_body_execution_mode, + ), + include_sse_frames: is_preview, + }, + ) + .await; + log_webhook_terminal_outcome( + session_id, + "dispatcher_agent", + webhook_outcome_label(&dispatch_result.outcome), + ); + let (response, persist_idempotency) = webhook_response_from_dispatch_result(dispatch_result); + release_idempotency_key(state, reserved_idempotency_key, persist_idempotency); + update_session_activity_if_persisted(state, session_id, token_hash.as_deref(), persist_idempotency).await; + response +} +``` + +- [ ] **Step 3: Extract the shared handled-ingress short-circuit path** + +Add: + +```rust +async fn maybe_execute_legacy_http_ingress( + state: &AppState, + session_id: &str, + session_source: webhook_dispatch::WebhookSessionSource, + scrubbed_message: &str, + token_hash: Option<&str>, + reserved_idempotency_key: Option<&str>, +) -> Option { + let http_source = webhook_http_source(session_source); + let maybe_response = maybe_handle_http_ingress( + state, + session_id, + http_source, + scrubbed_message, + token_hash, + ) + .await; + + if let Some((response, persist_idempotency)) = maybe_response { + release_idempotency_key(state, reserved_idempotency_key, persist_idempotency); + update_session_activity_if_persisted(state, session_id, token_hash, persist_idempotency).await; + return Some(response); + } + + None +} +``` + +Then use it in both preview and non-preview handled-ingress branches. + +- [ ] **Step 4: Replace the main body of `handle_webhook` with a linear orchestrator flow** + +The refactored function should keep this shape: + +```rust +if let Some(rejection) = webhook_auth_rejection(&state, peer_addr, &headers) { + return rejection; +} + +let webhook_body = match parse_webhook_body(body) { + Ok(body) => body, + Err(rejection) => return rejection, +}; + +let (session_id, session_source) = match resolve_session_id(&headers) { + Ok(resolved) => resolved, + Err(response) => return response, +}; + +let reserved_idempotency_key = match reserve_webhook_idempotency_key(&state, &headers) { + Ok(key) => key, + Err(response) => return response, +}; + +if let Some(response) = ensure_webhook_session( + &state, + &session_id, + token_hash.as_deref(), + reserved_idempotency_key.as_deref(), +).await { + return response; +} + +if dispatcher_enabled { + return execute_dispatcher_webhook(...).await; +} + +if let Some(response) = maybe_execute_legacy_http_ingress(...).await { + return response; +} +``` + +Keep the existing plan-mode and cost-governance fail-closed branches intact after that. + +- [ ] **Step 5: Run the most relevant `/webhook` regression tests** + +Run: + +```bash +cargo test gateway::mod::tests::legacy_webhook_preview_does_not_emit_synthetic_events_sse --lib +cargo test gateway::mod::tests::legacy_webhook_preview_intercepts_slash_session_commands --lib +cargo test gateway::mod::tests::webhook_non_preview_blocks_approval_and_keeps_session_id --lib +cargo test gateway::mod::tests::webhook_non_preview_unblocks_with_approval_override --lib +cargo test gateway::mod::tests::webhook_non_preview_timeout_aborts_with_session_scope --lib +``` + +Expected: PASS. This confirms no regressions in preview behavior, slash interception, approval blocking, or timeout handling. + +--- + +## Task 6: Reduce complexity in `gateway/mod.rs` `handle_chat_stream` + +**Files:** +- Modify: `clients/agent-runtime/src/gateway/mod.rs` +- Test: inline tests in `clients/agent-runtime/src/gateway/mod.rs` + +- [ ] **Step 1: Extract stream session setup and tool snapshot creation** + +Add a helper: + +```rust +async fn prepare_stream_request( + state: &AppState, + headers: &HeaderMap, + body: WebhookJsonBody, +) -> Result< + ( + WebhookBody, + String, + webhook_dispatch::WebhookSessionSource, + Option, + Config, + crate::bootstrap::SlashToolSnapshot, + ), + WebhookResponse, +> { + let webhook_body = parse_webhook_body(body)?; + let (session_id, session_source) = resolve_session_id(headers)?; + let token_hash = utils::extract_bearer_token(headers).map(|t| compute_token_hash(&t)); + + if let Err(error) = state.mem.upsert_session(&session_id, token_hash.as_deref()).await { + if token_hash.is_some() { + tracing::error!("session upsert failed for token-scoped request: {error:#}"); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Session tracking failed"})), + )); + } + tracing::debug!("session upsert best-effort failed: {error}"); + } + + let config = state.config.lock().clone(); + let tool_snapshot = crate::bootstrap::slash_tool_snapshot_from_config(&config).map_err(|_| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": "Failed to derive effective tool snapshot"})), + ) + })?; + + Ok((webhook_body, session_id, session_source, token_hash, config, tool_snapshot)) +} +``` + +- [ ] **Step 2: Extract handled-ingress SSE response building** + +Move the `if let HandledIngress::Handled(...)` branch into: + +```rust +async fn maybe_stream_handled_ingress_response( + state: &AppState, + handled_ingress: &crate::pre_execution::HandledIngress, + session_id: &str, + token_hash: Option<&str>, +) -> Option { + // move the current event/status construction here unchanged +} +``` + +The body of this helper should preserve the current `chunk`, `done`, and `error` event payloads exactly. + +- [ ] **Step 3: Replace the nested front half of `handle_chat_stream` with a linear flow** + +The function should read like: + +```rust +if let Some(rejection) = webhook_auth_rejection(&state, peer_addr, &headers) { + return Err(rejection); +} + +let (webhook_body, session_id, session_source, token_hash, config, tool_snapshot) = + prepare_stream_request(&state, &headers, body).await?; + +let handled_ingress = crate::pre_execution::adapt_handled_ingress( + crate::pre_execution::evaluate_ingress( + state.mem.as_ref(), + &tool_snapshot, + ingress_context, + &scrubbed_message, + true, + ) + .await, +); + +if let Some(response) = maybe_stream_handled_ingress_response( + &state, + &handled_ingress, + &session_id, + token_hash.as_deref(), +) +.await { + return Ok(response); +} +``` + +Leave the dispatcher-vs-legacy stream outcome mapping below that branch unchanged except for variable plumbing. + +- [ ] **Step 4: Run the stream router regression test slice** + +Run at least the existing stream router tests that exercise `/web/chat/stream` in `gateway/mod.rs`. If there is no single named stream smoke test, run the full gateway lib test slice: + +```bash +cargo test gateway::mod::tests --lib +``` + +Expected: PASS. The SSE contract must remain stable. + +--- + +## Task 7: Full Batch 1 validation + +**Files:** +- No additional code changes expected +- Validate all modified Rust files + +- [ ] **Step 1: Run formatting** + +Run: + +```bash +cargo fmt --all -- --check +``` + +Expected: PASS. If it fails, run `cargo fmt --all`, then re-run the check. + +- [ ] **Step 2: Run lints** + +Run: + +```bash +cargo clippy --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 3: Run the targeted Batch 1 tests together** + +Run: + +```bash +cargo test delegate_launch::tests --lib && cargo test webhook_dispatch::tests --lib && cargo test policy::tests --lib && cargo test gateway::mod::tests --lib && cargo test cli_shared_ingress_handles_compact_before_agent_execution --lib && cargo test cli_session_command_success_returns_message --lib && cargo test cli_resume_target_without_caller_scope_preserves_denied_error_path --lib && cargo test cli_unknown_slash_like_input_falls_through --lib && cargo test cli_tools_command_returns_effective_tool_listing --lib +``` + +Expected: PASS. + +- [ ] **Step 4: Re-check SonarCloud after code is stable** + +Use the SonarQube MCP query for project `dallay_corvus` and confirm the Batch 1 CRITICAL issues no longer appear for: + +- `clients/agent-runtime/src/tools/delegate_launch.rs` +- `clients/agent-runtime/src/main.rs` +- `clients/agent-runtime/src/gateway/webhook_dispatch.rs` +- `clients/agent-runtime/src/gateway/mod.rs` +- `clients/agent-runtime/src/security/policy.rs` + +Expected: those CRITICAL complexity issues are cleared, or any residual issue count is smaller and traceable to a specific remaining function. + +--- + +## Self-review checklist + +- Batch 1 scope only: yes. +- No unrelated dependency additions: required. +- Security posture preserved in gateway/policy code: required. +- All five CRITICAL Rust targets covered by explicit tasks: yes. +- Validation commands are concrete and runnable from `clients/agent-runtime`: yes. diff --git a/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md new file mode 100644 index 000000000..6445dc323 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-2-frontend-critical-accessibility.md @@ -0,0 +1,185 @@ +--- +title: SonarQube Batch 2 Frontend Critical and Accessibility Implementation Plan +description: Implementation plan for the second SonarQube remediation batch focused on dashboard, rook-dashboard, and accessibility issues. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: web dashboard and rook-dashboard remediation +docType: architecture +--- + +# SonarQube Batch 2 Frontend Critical and Accessibility Implementation Plan + +> **For agentic workers:** Execute this batch on branch `maintenance/sonarqube-remediation` after the Batch 1 backend refactor commit. Keep changes localized, test first where practical, and validate each app independently before final combined verification. + +**Goal:** Resolve the current frontend-critical and accessibility Sonar issues in the dashboard and rook-dashboard apps by reducing control-flow complexity, improving semantics, and correcting likely contrast problems without changing the existing admin, pairing, session, or embedded-operator UX. + +**Architecture:** Prefer local component and composable fixes over shared design churn. Replace nested inline decision logic with named derived state/helpers, upgrade weak ARIA patterns to semantic structure when safe, and apply the smallest CSS/token changes that improve readability and contrast. Avoid broad design-system edits. + +**Tech Stack:** Vue 3, TypeScript, Vitest, Biome, Vite, app-local CSS, existing unit/a11y tests in `clients/web/apps/dashboard` and `clients/web/apps/rook-dashboard`. + +--- + +## File Structure + +### Files likely to modify + +#### Dashboard app +- `clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.vue` + - Replace inline conditional action-label/state rendering with explicit derived helpers/computed labels. + - Preserve button disabled behavior and current admin action contracts. +- `clients/web/apps/dashboard/src/components/chat/ChatWorkspace.vue` + - Review live-region usage, message rendering semantics, and any weak role assignment patterns. + - Preserve keyboard flow, screen-reader announcements, and existing chat/session behavior. +- `clients/web/apps/dashboard/src/composables/useAdmin.ts` + - Extract duplicated pagination/offset logic if Sonar is flagging repeated complexity. +- `clients/web/apps/dashboard/src/composables/useChat.ts` + - Replace terse nested/inline payload branching with explicit parsing helpers where Sonar complexity is triggered. +- `clients/web/apps/dashboard/src/App.vue` or related auth/session shell components if Sonar/a11y points there. + - Only touch if issues are confirmed during targeted inspection. + +#### Dashboard tests to extend +- `clients/web/apps/dashboard/src/components/sessions/CerebroSessionActions.spec.ts` +- `clients/web/apps/dashboard/src/components/chat/ChatWorkspace.spec.ts` +- `clients/web/apps/dashboard/src/App.spec.ts` +- `clients/web/apps/dashboard/src/composables/useAdmin.spec.ts` +- `clients/web/apps/dashboard/src/test/runAxe.ts` helpers if existing a11y tests need extension + +#### Rook dashboard app +- `clients/web/apps/rook-dashboard/src/style.css` + - Adjust local contrast-sensitive colors only where needed. + - Preserve dark visual identity and avoid re-theming the app. +- `clients/web/apps/rook-dashboard/src/features/**/` page components + - Replace nested presentation branching or weak semantic wrappers if Sonar flags them. +- `clients/web/apps/rook-dashboard/src/lib/api/client.spec.ts` + - Minor cleanup already identified (`?:` simplification / explicit readability) if tied to Sonar maintainability. + +#### Rook dashboard tests to extend +- `clients/web/apps/rook-dashboard/src/features/**/*.spec.ts` +- `clients/web/apps/rook-dashboard/src/lib/api/client.spec.ts` +- Add focused tests near touched components rather than creating broad new suites. + +### Files unlikely to modify +- Shared design-system packages +- Backend gateway/runtime code +- Unrelated docs beyond plan updates unless necessary + +--- + +## Implementation Strategy + +### Phase 1 — Confirm concrete issue surfaces +1. Inspect Sonar-targeted dashboard and rook-dashboard files for: + - nested ternaries and inline branching in templates/composables + - duplicated conditional parameter construction + - weak role usage where semantic HTML can replace it + - low-contrast foreground/background combinations in rook-dashboard CSS +2. Map each issue to the smallest safe file-local fix. +3. Do not start with CSS churn; first confirm markup/control-flow hotspots. + +### Phase 2 — Dashboard remediation +1. Add or extend tests first for any behavior-sensitive component: + - button labels/states in `CerebroSessionActions.vue` + - live region and message-list behavior in `ChatWorkspace.vue` + - any extracted pagination helper behavior in `useAdmin.ts` +2. Refactor inline conditional rendering into named helpers/computed state. +3. Where semantic HTML can replace ARIA/role duplication without changing behavior, prefer that swap. +4. Re-run dashboard checks before moving on. + +### Phase 3 — Rook dashboard remediation +1. Add/extend tests around any feature/page logic changed. +2. Fix local maintainability issues in components/specs first. +3. Adjust contrast-sensitive CSS values conservatively: + - improve text/background separation + - preserve dark theme hierarchy + - avoid broad hue changes unless required +4. Re-run rook-dashboard checks before final combined validation. + +### Phase 4 — Combined validation +Run formatting, lint/check, tests, and builds for both apps. If available and fast enough, run app-local accessibility-focused tests for dashboard as an extra guardrail. + +--- + +## Task Breakdown + +### Task 1: Inspect and pin down Batch 2 files +**Files:** read-only inspection across both app src trees + +**Actions:** +- Search for nested ternaries and inline branching in Vue templates and composables. +- Search for role/ARIA patterns that may be replaceable with semantic structure. +- Inspect rook-dashboard CSS for likely contrast offenders. +- Keep a short working list of exact files to touch. + +**Success criteria:** +- A bounded set of candidate files is identified before implementation begins. + +### Task 2: Remediate dashboard control-flow complexity +**Files:** likely `CerebroSessionActions.vue`, `useAdmin.ts`, `useChat.ts`, possibly `App.vue` or session/chat supporting components + +**Actions:** +- Add/extend tests before refactoring behavior-sensitive logic. +- Extract derived labels, status lookup, or payload parsing helpers. +- Replace nested inline decision logic with explicit branches. +- Preserve all current text, disabled states, request parameters, and emitted events. + +**Success criteria:** +- Sonar-style complexity hotspots are reduced. +- Dashboard tests remain green. +- No UX copy or state regressions. + +### Task 3: Remediate dashboard accessibility semantics +**Files:** likely `ChatWorkspace.vue` and any touched auth/session shell components + +**Actions:** +- Review live region usage, role duplication, labels, and semantic wrappers. +- Replace weak role usage with semantic elements only when behavior remains equivalent. +- Keep screen-reader announcements and existing tests stable; extend tests if semantics change. + +**Success criteria:** +- Accessibility posture improves without introducing noisy announcements or broken focus flow. + +### Task 4: Remediate rook-dashboard maintainability and contrast +**Files:** `src/style.css`, plus feature/page components/specs as confirmed during inspection + +**Actions:** +- Apply minimal CSS changes to improve contrast where needed. +- Simplify small maintainability issues in specs/components flagged by Sonar. +- Keep the embedded dashboard visually consistent and operationally unchanged. + +**Success criteria:** +- Build/test/check remain green. +- Styling remains recognizably the same, only clearer/more accessible. + +### Task 5: Final verification +**Run from `clients/web/apps/dashboard`:** +```bash +pnpm check +pnpm test +pnpm test:a11y +pnpm build +``` + +**Run from `clients/web/apps/rook-dashboard`:** +```bash +pnpm check +pnpm test +pnpm build +``` + +**Optional if UI-sensitive changes warrant it:** +- run app preview/dev server and perform a quick visual sanity pass + +**Expected result:** +- Both apps pass formatting/lint-style checks, tests, and builds. +- No obvious semantic/styling regressions. + +--- + +## Implementation Notes + +- Prefer additive helper functions/computed state over aggressive component decomposition unless complexity clearly warrants it. +- Do not rewrite shared CSS foundations imported from `@corvus/shared` unless a local fix is impossible. +- When changing semantics, preserve existing selectors/test hooks where feasible to avoid unnecessary test churn. +- Favor tests that prove user-visible behavior over snapshot-style assertions. +- Keep commits focused if the batch needs to be split mid-flight, but implementation can still land on the same maintenance branch. diff --git a/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md new file mode 100644 index 000000000..bd8c197f2 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-batch-3-scripts-kotlin-residual-css.md @@ -0,0 +1,142 @@ +--- +title: SonarQube Batch 3 Scripts, Kotlin, and Residual CSS Implementation Plan +description: Implementation plan for the third SonarQube remediation batch focused on shell scripts, Kotlin cleanup, and small residual CSS issues. +owner: team-platform +status: draft +lastReviewed: 2026-04-26 +appliesTo: scripts and compose runtime remediation +docType: architecture +--- + +# SonarQube Batch 3 Scripts, Kotlin, and Residual CSS Implementation Plan + +> **For agentic workers:** Execute this batch on branch `maintenance/sonarqube-remediation` after the Batch 1 and Batch 2 remediation commits. Keep edits minimal and behavior-preserving. Prefer local helper extraction and explicit shell intent over broad rewrites. + +**Goal:** Resolve the remaining non-critical Sonar issues in the shell scripts, mobile runtime coordinator Kotlin logic, and any small residual CSS duplication with the smallest safe edits that preserve automation and runtime behavior. + +**Architecture:** Treat shell scripts as operational entrypoints whose semantics must remain unchanged. Favor named local variables, explicit returns, and safer conditionals over refactoring control flow wholesale. In Kotlin, collapse duplicated branches into small pure helpers while preserving runtime state semantics exactly. For CSS, only consolidate selectors if the duplication is obvious and low risk. + +**Tech Stack:** Bash, Kotlin Multiplatform / Compose commonMain runtime code, Gradle, existing project tests/build tasks, and local CSS in web apps if residual duplication is confirmed. + +--- + +## File Structure + +### Files to modify +- `scripts/mobile-smoke-test.sh` + - Clarify parameter handling and explicit success/failure returns. + - Preserve smoke-check behavior, logs, and exit codes. +- `scripts/check-tools.sh` + - Reduce maintainability friction in version parsing / status printing helpers. + - Preserve output intent and failure accumulation semantics. +- `clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt` + - Collapse duplicated runtime/session/approval/message mapping branches into focused helpers. + - Preserve state transitions and recovery semantics exactly. +- Residual CSS file(s) only if a concrete small duplication is confirmed during inspection. + +### Tests and validation targets +- Shell syntax validation: + - `bash -n scripts/mobile-smoke-test.sh` + - `bash -n scripts/check-tools.sh` +- Kotlin validation proportional to scope: + - existing tests near `MobileRuntimeCoordinator` if present + - otherwise the smallest Gradle task that compiles/tests the touched source set or module +- CSS validation only through existing frontend checks if CSS is actually touched + +--- + +## Implementation Strategy + +### Phase 1 — Shell maintainability cleanup +1. Inspect both scripts for: + - positional parameters used directly multiple times + - implicit success/failure flows Sonar tends to flag + - test expressions where `[[ ... ]]` is safer and already idiomatic in the file +2. Add only behavior-preserving changes: + - assign args to local names when useful + - make returns explicit inside helper functions + - keep log messages and exit semantics intact +3. Validate both scripts with `bash -n` immediately after edits. + +### Phase 2 — Kotlin branch deduplication +1. Inspect `MobileRuntimeCoordinator.kt` for repeated branch logic across: + - readiness/session loading + - pending approval handling + - chat message synthesis/recovery logic + - state update construction +2. If tests exist nearby, extend them first for behavior-sensitive changes. +3. Extract small pure helpers that reduce duplication/cognitive load without changing public behavior. +4. Run the smallest relevant Gradle validation for the touched file/module. + +### Phase 3 — Residual CSS duplication +1. Only proceed if inspection finds a concrete duplicated selector block tied to Sonar residual issues. +2. Consolidate selectors minimally. +3. Re-run only the relevant frontend validation if CSS changes occur. + +--- + +## Task Breakdown + +### Task 1: Inspect and remediate `mobile-smoke-test.sh` +**Actions:** +- Confirm argument handling and helper return paths. +- Replace outdated conditional/test patterns only where safe. +- Avoid changing smoke command order, adb/xcode interactions, or output meaning. + +**Success criteria:** +- Script remains syntax-valid and operationally equivalent. +- Maintainability warnings targeted by Sonar are reduced. + +### Task 2: Inspect and remediate `check-tools.sh` +**Actions:** +- Review helper functions for repeated parsing/branching patterns. +- Prefer named locals and explicit result handling. +- Keep tool-version threshold logic and failure accumulation unchanged. + +**Success criteria:** +- Script remains syntax-valid and behaviorally equivalent. +- Output format and non-zero failure behavior remain intact. + +### Task 3: Refactor `MobileRuntimeCoordinator.kt` +**Actions:** +- Identify duplicated branch/state-construction logic. +- Add/extend tests if available before behavior-sensitive refactors. +- Extract helper functions with narrow responsibilities. +- Keep all runtime readiness, active session, approval, and recovery semantics stable. + +**Success criteria:** +- Reduced duplication/complexity with no state-machine regression. +- Relevant Kotlin validation passes. + +### Task 4: Residual CSS cleanup if warranted +**Actions:** +- Touch CSS only if a specific duplicated block is confirmed. +- Consolidate with the smallest selector grouping possible. + +**Success criteria:** +- Less duplication, no visual churn, no unnecessary app-wide styling changes. + +### Task 5: Final verification +**Run from repo root:** +```bash +bash -n scripts/mobile-smoke-test.sh +bash -n scripts/check-tools.sh +``` + +**Run Kotlin validation with smallest suitable scope discovered during inspection.** + +**If CSS is touched, run the relevant app validation only for that app.** + +**Expected result:** +- Shell scripts parse cleanly. +- Kotlin touched scope compiles/tests cleanly. +- Any CSS changes remain low risk and validated. + +--- + +## Implementation Notes + +- Shell changes should read as clearer intent, not as modernization theater. +- Do not alter operational command ordering unless strictly necessary for correctness. +- In Kotlin, prefer helper extraction over introducing new abstractions or classes. +- If no meaningful residual CSS duplication is found, explicitly skip that sub-slice rather than inventing cleanup. diff --git a/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-remediation-plan.md b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-remediation-plan.md new file mode 100644 index 000000000..3871645e4 --- /dev/null +++ b/clients/web/apps/docs/src/content/docs/plans/2026-04-26-sonarqube-remediation-plan.md @@ -0,0 +1,169 @@ +--- +title: "SonarQube remediation plan" +date: 2026-04-26 +last_updated: 2026-04-26 +tags: [sonarqube, remediation, quality, plan] +status: draft +summary: "Batch-based plan to resolve all current open SonarQube issues in Corvus by priority and domain." +description: "Repository-wide SonarQube remediation plan covering backend, frontend, accessibility, scripts, and Kotlin follow-up work." +owner: team-platform +lastReviewed: 2026-04-26 +appliesTo: corvus runtime, web, and tooling remediation +docType: architecture +--- + +# SonarQube remediation plan + +## Goal + +Resolve all currently open SonarQube issues for `dallay_corvus` in a controlled sequence that reduces risk first while keeping each implementation batch coherent and reviewable. + +## Scope + +This plan covers the current open issue set already identified in SonarQube, including: + +- Rust runtime/gateway/security complexity issues +- Dashboard and rook-dashboard frontend issues +- Accessibility and CSS issues +- Shell script maintainability issues +- Kotlin duplication issue + +This plan does not include unrelated cleanup or opportunistic refactors unless they are necessary to safely close a Sonar issue. + +## Recommended execution strategy + +Use **priority + domain batching** instead of resolving issues in a single mixed pass. + +Reasoning: + +- Critical runtime/security issues carry the highest maintenance and regression risk. +- Frontend accessibility and dashboard issues are easier to validate once backend-critical work is stable. +- Shell/Kotlin/CSS tail work is mostly mechanical and lower risk. +- Smaller coherent batches reduce review cost and make regressions easier to isolate. + +## Batch plan + +### Batch 1 — Backend critical + +Target all current **CRITICAL** issues in Rust runtime surfaces: + +- `clients/agent-runtime/src/tools/delegate_launch.rs` +- `clients/agent-runtime/src/main.rs` +- `clients/agent-runtime/src/gateway/webhook_dispatch.rs` +- `clients/agent-runtime/src/gateway/mod.rs` +- `clients/agent-runtime/src/security/policy.rs` + +Primary remediation pattern: + +- extract helper functions from large control-flow blocks +- isolate validation and normalization from execution logic +- flatten nested branching where possible +- preserve CLI and gateway behavior exactly +- avoid broad architectural rewrites + +Validation expectations: + +- relevant Rust formatting and lint checks +- targeted Rust tests for touched modules when available +- no security posture weakening in gateway or policy code + +### Batch 2 — Frontend critical + accessibility + +Target current dashboard and rook-dashboard issues: + +- `clients/web/apps/dashboard/src/**/*` +- `clients/web/apps/rook-dashboard/src/**/*` + +Primary remediation pattern: + +- replace nested ternaries with explicit control flow +- extract duplicated logic into named helpers when needed +- replace weak ARIA-role usage with semantic HTML where appropriate +- fix contrast issues with minimal token/style changes +- prefer local component fixes over shared design churn + +Validation expectations: + +- relevant frontend lint/test/build commands for touched apps +- visual sanity check for semantics and styling-sensitive changes +- keep existing admin/pairing UX intact + +### Batch 3 — Scripts, Kotlin, and residual CSS + +Target remaining non-critical issues: + +- `scripts/mobile-smoke-test.sh` +- `scripts/check-tools.sh` +- `clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt` +- residual CSS duplication issues + +Primary remediation pattern: + +- add explicit shell returns where required +- assign positional shell parameters to local names +- replace `[` with `[[` where appropriate and safe +- collapse duplicated Kotlin branch logic without changing state semantics +- remove CSS duplication with the smallest possible selector consolidation + +Validation expectations: + +- shell syntax checks where applicable +- Kotlin/project build validation proportional to touched scope +- no behavior changes to automation scripts beyond style-compliant equivalence + +### Batch 4 — Global verification + +After all implementation batches: + +- re-query SonarQube for remaining open issues +- verify the original issue set is fully resolved +- address any residual or newly surfaced issues caused by remediation work +- summarize any issues intentionally deferred, if any remain + +## Implementation constraints + +- Prefer minimal, behavior-preserving edits. +- Avoid new dependencies unless required. +- Do not mix unrelated refactors with Sonar remediation. +- Treat `gateway`, `security`, and tool execution surfaces as high-risk. +- Keep the existing secure-by-default runtime posture intact. + +## Success criteria + +A batch is complete when: + +1. The targeted issues for that batch are resolved in code. +2. Relevant local validation passes or any skipped validation is explicitly documented. +3. No obvious regression is introduced in the touched domain. + +The overall effort is complete when the current SonarQube open issue set for `dallay_corvus` has been cleared or reduced to only explicitly reviewed exceptions. + +## Risks and mitigations + +### Risk: behavior changes while reducing complexity + +Mitigation: + +- use extraction instead of rewrites +- preserve function boundaries and public contracts +- validate with targeted tests/builds after each batch + +### Risk: accessibility fixes alter UI structure unexpectedly + +Mitigation: + +- prefer semantic substitutions that preserve layout +- keep CSS changes minimal and localized +- run a visual sanity check on changed screens when practical + +### Risk: Sonar reports shift after refactors + +Mitigation: + +- finish and validate one batch at a time +- re-check Sonar after meaningful milestones +- avoid cascading style rewrites + +## Planned next step + +Begin with **Batch 1 — Backend critical**, inspect the affected Rust files, and prepare a focused implementation plan before editing. diff --git a/clients/web/apps/rook-dashboard/src/style.css b/clients/web/apps/rook-dashboard/src/style.css index 60af3d72e..a85c4dd36 100644 --- a/clients/web/apps/rook-dashboard/src/style.css +++ b/clients/web/apps/rook-dashboard/src/style.css @@ -100,7 +100,7 @@ code { .account-meta, .empty-state p, .detail-grid dd { - color: #bfd2e8; + color: #d5e3f2; line-height: 1.6; } @@ -110,7 +110,7 @@ code { font-size: 0.78rem; text-transform: uppercase; letter-spacing: 0.08em; - color: #7dd3fc; + color: #93ddff; } .session-card, @@ -292,13 +292,13 @@ input { } .state-banner.info { - background: rgb(14 165 233 / 12%); - color: #c8f1ff; + background: rgb(14 165 233 / 18%); + color: #e0f7ff; } .state-banner.danger { - background: rgb(239 68 68 / 14%); - color: #fecaca; + background: rgb(239 68 68 / 18%); + color: #ffe0e0; } .empty-state { diff --git a/scripts/check-tools.sh b/scripts/check-tools.sh index fb1b837c9..1d13e348f 100644 --- a/scripts/check-tools.sh +++ b/scripts/check-tools.sh @@ -62,8 +62,13 @@ numeric_prefix() { echo "${num:-0}" } +has_command() { + local command_name="$1" + command -v "$command_name" >/dev/null 2>&1 +} + # 1. Java -if command -v java >/dev/null 2>&1; then +if has_command java; then java_ver_raw=$(java -version 2>&1 | awk -F '"' '/version/ {print $2; exit}') java_major=$(echo "$java_ver_raw" | cut -d. -f1) if [[ "$java_major" = "1" ]]; then @@ -81,7 +86,7 @@ else fi # 2. Git -if command -v git >/dev/null 2>&1; then +if has_command git; then git_ver=$(git --version | awk '{print $3}') print_status "Git" 0 "v$git_ver" else @@ -139,14 +144,21 @@ check_required_major_minor_version() { minor_part=${current_version#*.} current_minor=$(numeric_prefix "${minor_part%%.*}") - if [[ "$current_major" -lt "$min_major" || ( "$current_major" -eq "$min_major" && "$current_minor" -lt "$min_minor" ) ]]; then + local version_too_low=false + if [[ "$current_major" -lt "$min_major" ]]; then + version_too_low=true + elif [[ "$current_major" -eq "$min_major" && "$current_minor" -lt "$min_minor" ]]; then + version_too_low=true + fi + + if [[ "$version_too_low" == "true" ]]; then print_status "$tool_name" 1 "Found v$current_version, need v$min_major.$min_minor+" || FAILED=1 else print_status "$tool_name" 0 "v$current_version" fi } -if command -v node >/dev/null 2>&1; then +if has_command node; then node_full=$(node -v) check_required_major_version "Node.js" "${node_full#v}" "$MIN_NODE" else @@ -154,7 +166,7 @@ else fi # 4. pnpm -if command -v pnpm >/dev/null 2>&1; then +if has_command pnpm; then pnpm_ver=$(pnpm --version) check_required_major_version "pnpm" "$pnpm_ver" "$MIN_PNPM" else @@ -162,7 +174,7 @@ else fi # 5. Rust -if command -v rustc >/dev/null 2>&1; then +if has_command rustc; then rust_full=$(rustc --version | awk '{print $2}') check_required_major_minor_version "Rust" "$rust_full" "$MIN_RUST_MAJOR" "$MIN_RUST_MINOR" else @@ -170,7 +182,7 @@ else fi # 6. Docker (Optional) -if command -v docker >/dev/null 2>&1; then +if has_command docker; then docker_ver=$(docker --version | awk '{print $3}' | tr -d ',') print_status "Docker" 0 "v$docker_ver" else @@ -179,7 +191,7 @@ fi # 7. Xcode CLI Tools (macOS only, optional) if [[ "$(uname -s 2>/dev/null || echo unknown)" = "Darwin" ]]; then - if command -v xcodebuild >/dev/null 2>&1; then + if has_command xcodebuild; then xcode_ver=$(xcodebuild -version 2>/dev/null | awk 'NR==1{print $2}') print_status "Xcode" 0 "v${xcode_ver:-unknown}" else diff --git a/scripts/mobile-smoke-test.sh b/scripts/mobile-smoke-test.sh index f9c3553a6..79fb8a077 100755 --- a/scripts/mobile-smoke-test.sh +++ b/scripts/mobile-smoke-test.sh @@ -69,14 +69,16 @@ check_android_prerequisites() { test_android() { log_info "=== ANDROID SMOKE VALIDATION ===" + local device_count + # Check device connection - DEVICE_COUNT=$(adb devices 2>/dev/null | grep -c "device$" || true) - if [ "$DEVICE_COUNT" -eq 0 ]; then + device_count=$(adb devices 2>/dev/null | grep -c "device$" || true) + if [[ "$device_count" -eq 0 ]]; then log_error "No Android devices connected" log_info "Connect device via USB and enable ADB debugging" exit 1 fi - log_success "Found $DEVICE_COUNT Android device(s)" + log_success "Found $device_count Android device(s)" # Deploy corvus binary log_info "Deploying corvus binary to device..." @@ -120,16 +122,18 @@ test_android() { test_ios() { log_info "=== iOS SMOKE VALIDATION ===" + local config_file="clients/iosApp/Configuration/Config.xcconfig" + local device_list + # Check for connected devices - DEVICE_LIST=$(xcrun xctrace list devices 2>/dev/null | grep -A 100 "== Devices ==" | grep -B 100 "== " | head -20) - if echo "$DEVICE_LIST" | grep -q "Offline"; then + device_list=$(xcrun xctrace list devices 2>/dev/null | grep -A 100 "== Devices ==" | grep -B 100 "== " | head -20) + if grep -q "Offline" <<< "$device_list"; then log_warn "iOS devices detected but offline" log_info "Connect device via USB and trust this computer" fi # Check TEAM_ID configuration - local config_file="clients/iosApp/Configuration/Config.xcconfig" - if [ ! -f "$config_file" ]; then + if [[ ! -f "$config_file" ]]; then log_error "Config file not found: $config_file" log_info "Ensure iOS configuration exists before running smoke validation" exit 1 @@ -145,7 +149,7 @@ test_ios() { } ' "$config_file") - if [ -z "$TEAM_ID" ]; then + if [[ -z "$TEAM_ID" ]]; then log_error "TEAM_ID not configured in clients/iosApp/Configuration/Config.xcconfig" log_info "Add your Apple Developer Team ID to proceed with device builds" log_info "simulator builds work without TEAM_ID"