From 9c42960f501092711326e4661dd34aaa0394836d Mon Sep 17 00:00:00 2001 From: skygunner Date: Wed, 28 Oct 2026 05:47:09 +1000 Subject: [PATCH 1/2] feat(runtime): authorized Server Diagnose entry with a CLI provider diagnostic Implements the remaining #994 scope endorsed in the issue thread: one canonical Server/ToolRuntime entry that independently authorizes the exact Runner, provider generation, and authorized Project before forwarding the typed Runner Diagnose request from #1129, plus a CLI operator command as its first consumer. - diagnose_coding_agent: model-visible runtime tool with observe-level static authority (Require(coding_agent:run), PureRead). The optional include_live_acp flag escalates at runtime to the exact start_coding_agent scope policy plus a fresh per-Project permission decision before any provider process is launched; presence-only stays spawn-free. - Resolution order is fail-closed: validate input, gate the live flag, resolve the Project for the caller, require the exact Runner semantic view and CodingAgentRuns capability, look up the current provider generation from the Runner inventory, then forward through the existing enqueue_coding_agent fence (which re-verifies identity, generation, online window, and access). - The Server re-verifies Runner response bounds (option/choice counts, identity byte bounds, stable kind vocabulary, options-only-on-success, env map bound) before projecting; violations collapse into invalid_runner_response. CODING_AGENT_MAX_ENV_MAPPINGS moves to core and is shared with the Runner's ACP config validation so the output schema can never disagree with what was admitted. - Projection is project/provider_id/env_presence/live_acp only; the provider instance id never crosses to callers. - CLI: webcodex ops diagnose-provider (--project, --provider, --include-live-acp) calls the same entry through the existing /api/tools/call bridge and renders presence booleans, live option identities, and an ops verdict; values, stderr, and credentials never reach the output. --- crates/webcodex-cli/src/lib.rs | 86 ++- crates/webcodex-cli/src/webcodex_cli/mod.rs | 19 +- crates/webcodex-cli/src/webcodex_cli/ops.rs | 193 ++++++ .../src/webcodex_cli/tests/mod.rs | 1 + .../src/webcodex_cli/tests/ops.rs | 4 + .../src/webcodex_cli/tests/ops_diagnose.rs | 239 +++++++ crates/webcodex-cli/src/webcodex_cli/usage.rs | 33 +- crates/webcodex-core/src/coding_agent.rs | 4 + .../src/webcodex_runner/config.rs | 4 +- .../registry/output_schemas/coding_agents.rs | 44 +- .../src/tests/canonical_names.rs | 1 + .../src/tests/catalog_discovery.rs | 1 + .../src/tests/metadata_policy.rs | 5 + .../src/tests/output_schemas.rs | 1 + .../src/tests/policy_contracts.rs | 9 + .../src/tool_call/canonical_enum.rs | 23 + .../src/tool_call/parsing.rs | 2 + .../src/tool_definition/coding_agents.rs | 32 + .../src/tool_audit.rs | 9 + .../issue-994-server-diagnose-entry-design.md | 81 +++ src/auth/scopes.rs | 4 + src/mcp_tests/oauth_scope.rs | 3 + src/tool_runtime/coding_agent.rs | 451 +++++++++++++- src/tool_runtime/dispatch/execution.rs | 8 + src/tool_runtime/dispatch/routing.rs | 1 + src/tool_runtime/kernel.rs | 6 +- src/tool_runtime/sessions/tests.rs | 1 + .../tests/coding_agent_diagnose.rs | 587 ++++++++++++++++++ src/tool_runtime/tests/mod.rs | 1 + src/tool_runtime/tests/schema/discovery.rs | 8 + 30 files changed, 1807 insertions(+), 54 deletions(-) create mode 100644 crates/webcodex-cli/src/webcodex_cli/tests/ops_diagnose.rs create mode 100644 docs/design/issue-994-server-diagnose-entry-design.md create mode 100644 src/tool_runtime/tests/coding_agent_diagnose.rs diff --git a/crates/webcodex-cli/src/lib.rs b/crates/webcodex-cli/src/lib.rs index 067468b90..e38d33ed8 100644 --- a/crates/webcodex-cli/src/lib.rs +++ b/crates/webcodex-cli/src/lib.rs @@ -39,27 +39,27 @@ use webcodex_cli::{ client_profile_state_dir, client_profile_user_token_file, client_profile_user_token_file_for_scope, connect_usage, controller_usage, current_user_home, default_device_name, default_server_paths, disconnect_usage, discover_internal_binary, - is_effective_root, login_usage, logout_usage, ops_projects_usage, ops_runner_usage, - ops_runners_usage, ops_smoke_preflight_usage, ops_status_usage, ops_usage, ops_windows_usage, - pairing_create_usage, pairing_usage, parse_plugin_command, parse_plugin_init, - plugin_check_usage, plugin_describe_usage, plugin_init_usage, plugin_list_usage, - plugin_reload_usage, plugin_usage, project_activate_usage, project_register_usage, - read_env_file_value, render_token_generate, run_connect, run_controller_command, - run_disconnect, run_hosted_log_writer, run_internal_binary, run_login, run_logout, - run_ops_command, run_pairing_create, run_plugin_command, run_plugin_init, run_project_activate, - run_project_register, run_runner_install_service, run_runner_service, run_runner_status, - run_runner_token_create_local, run_server_init, run_server_install_service, run_server_service, - run_server_status, run_server_tunnel, run_status, run_token_create_local, + is_effective_root, login_usage, logout_usage, ops_diagnose_provider_usage, ops_projects_usage, + ops_runner_usage, ops_runners_usage, ops_smoke_preflight_usage, ops_status_usage, ops_usage, + ops_windows_usage, pairing_create_usage, pairing_usage, parse_plugin_command, + parse_plugin_init, plugin_check_usage, plugin_describe_usage, plugin_init_usage, + plugin_list_usage, plugin_reload_usage, plugin_usage, project_activate_usage, + project_register_usage, read_env_file_value, render_token_generate, run_connect, + run_controller_command, run_disconnect, run_hosted_log_writer, run_internal_binary, run_login, + run_logout, run_ops_command, run_pairing_create, run_plugin_command, run_plugin_init, + run_project_activate, run_project_register, run_runner_install_service, run_runner_service, + run_runner_status, run_runner_token_create_local, run_server_init, run_server_install_service, + run_server_service, run_server_status, run_server_tunnel, run_status, run_token_create_local, runner_config_for_scope, runner_init_usage, runner_install_service_usage, runner_service_file_for_scope, runner_status_usage, runner_usage, server_init_usage, server_install_service_usage, server_status_usage, server_tunnel_usage, server_usage, service_unit_name, status_usage, system_user_home, system_user_is_root, usage, validate_client_profile, validate_service_file_scope, write_connect_result, ConnectAuth, ConnectOptions, ControllerCommand, DisconnectOptions, LoginOptions, LogoutOptions, OpsCommand, - OpsCommonOptions, OpsRunnerOptions, OpsSmokePreflightOptions, OpsWindowsOptions, PluginCommand, - PluginInitOptions, ProjectActivateOptions, ProjectRegisterOptions, ServerStatusOptions, - ServiceControl, StatusOptions, DEFAULT_LOG_LINES, RUNNER_SERVICE_UNIT, SERVER_SERVICE_FILE, - SERVER_SERVICE_UNIT, + OpsCommonOptions, OpsDiagnoseProviderOptions, OpsRunnerOptions, OpsSmokePreflightOptions, + OpsWindowsOptions, PluginCommand, PluginInitOptions, ProjectActivateOptions, + ProjectRegisterOptions, ServerStatusOptions, ServiceControl, StatusOptions, DEFAULT_LOG_LINES, + RUNNER_SERVICE_UNIT, SERVER_SERVICE_FILE, SERVER_SERVICE_UNIT, }; const SETUP_GPT_SCOPES: &[&str] = &[ "runtime:read", @@ -1626,6 +1626,23 @@ fn parse_ops_subcommand(args: &[String]) -> CliAction { }, } } + "diagnose-provider" => { + if args.get(1).is_some_and(|a| a == "--help" || a == "-h") { + return CliAction::Exit { + code: 0, + stdout: ops_diagnose_provider_usage().to_string(), + stderr: String::new(), + }; + } + match parse_ops_diagnose_provider(&args[1..]) { + Ok(opts) => CliAction::Ops(OpsCommand::DiagnoseProvider(opts)), + Err(e) => CliAction::Exit { + code: 2, + stdout: String::new(), + stderr: format!("{}\n", e), + }, + } + } "smoke-preflight" => { if args.get(1).is_some_and(|a| a == "--help" || a == "-h") { return CliAction::Exit { @@ -1778,6 +1795,45 @@ fn parse_ops_windows(args: &[String]) -> Result { }) } +fn parse_ops_diagnose_provider(args: &[String]) -> Result { + let mut common = default_ops_common_options(); + let mut project = String::new(); + let mut provider = String::new(); + let mut include_live_acp = false; + let mut iter = args.iter(); + while let Some(arg) = iter.next() { + match arg.as_str() { + "--project" => project = next_value(&mut iter, arg)?, + "--provider" => provider = next_value(&mut iter, arg)?, + "--include-live-acp" => include_live_acp = true, + "--server-url" => common.server_url = next_value(&mut iter, arg)?, + "--proxy" => common.server_http.proxy = Some(next_value(&mut iter, arg)?), + "--no-system-proxy" => common.server_http.no_system_proxy = true, + "--env-file" => common.env_file = Some(PathBuf::from(next_value(&mut iter, arg)?)), + "--token-file" => common.token_file = Some(PathBuf::from(next_value(&mut iter, arg)?)), + "--token" => common.token = Some(next_value(&mut iter, arg)?), + "--json" => common.json = true, + "--strict" => common.strict = true, + other => return Err(format!("unknown ops diagnose-provider flag: {other}")), + } + } + common = validate_ops_common(&common)?; + let project = project.trim().to_string(); + if project.is_empty() { + return Err("--project is required".to_string()); + } + let provider = provider.trim().to_string(); + if provider.is_empty() { + return Err("--provider is required".to_string()); + } + Ok(OpsDiagnoseProviderOptions { + common, + project, + provider, + include_live_acp, + }) +} + fn parse_ops_smoke_preflight(args: &[String]) -> Result { let mut common = default_ops_common_options(); let mut project = String::new(); diff --git a/crates/webcodex-cli/src/webcodex_cli/mod.rs b/crates/webcodex-cli/src/webcodex_cli/mod.rs index 4fe801a5e..d081fbc81 100644 --- a/crates/webcodex-cli/src/webcodex_cli/mod.rs +++ b/crates/webcodex-cli/src/webcodex_cli/mod.rs @@ -76,8 +76,8 @@ pub(crate) use login::{ LogoutOptions, StatusOptions, }; pub(crate) use ops::{ - run_ops_command, OpsCommand, OpsCommonOptions, OpsRunnerOptions, OpsSmokePreflightOptions, - OpsWindowsOptions, + run_ops_command, OpsCommand, OpsCommonOptions, OpsDiagnoseProviderOptions, OpsRunnerOptions, + OpsSmokePreflightOptions, OpsWindowsOptions, }; #[cfg(test)] pub(crate) use output::RevisionComparison; @@ -134,13 +134,14 @@ pub(crate) use tokens::{ }; pub(crate) use usage::{ connect_usage, controller_usage, disconnect_usage, login_usage, logout_usage, - ops_projects_usage, ops_runner_usage, ops_runners_usage, ops_smoke_preflight_usage, - ops_status_usage, ops_usage, ops_windows_usage, pairing_create_usage, pairing_usage, - plugin_check_usage, plugin_describe_usage, plugin_init_usage, plugin_list_usage, - plugin_reload_usage, plugin_usage, project_activate_usage, project_register_usage, - runner_init_usage, runner_install_service_usage, runner_status_usage, runner_usage, - server_init_usage, server_install_service_usage, server_status_usage, server_tunnel_usage, - server_usage, status_usage, usage, + ops_diagnose_provider_usage, ops_projects_usage, ops_runner_usage, ops_runners_usage, + ops_smoke_preflight_usage, ops_status_usage, ops_usage, ops_windows_usage, + pairing_create_usage, pairing_usage, plugin_check_usage, plugin_describe_usage, + plugin_init_usage, plugin_list_usage, plugin_reload_usage, plugin_usage, + project_activate_usage, project_register_usage, runner_init_usage, + runner_install_service_usage, runner_status_usage, runner_usage, server_init_usage, + server_install_service_usage, server_status_usage, server_tunnel_usage, server_usage, + status_usage, usage, }; #[cfg(test)] diff --git a/crates/webcodex-cli/src/webcodex_cli/ops.rs b/crates/webcodex-cli/src/webcodex_cli/ops.rs index 760dda332..8ec141a12 100644 --- a/crates/webcodex-cli/src/webcodex_cli/ops.rs +++ b/crates/webcodex-cli/src/webcodex_cli/ops.rs @@ -24,6 +24,14 @@ pub(crate) struct OpsSmokePreflightOptions { pub(crate) project: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct OpsDiagnoseProviderOptions { + pub(crate) common: OpsCommonOptions, + pub(crate) project: String, + pub(crate) provider: String, + pub(crate) include_live_acp: bool, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct OpsWindowsOptions { pub(crate) common: OpsCommonOptions, @@ -46,6 +54,7 @@ pub(crate) enum OpsCommand { Projects(OpsCommonOptions), Windows(OpsWindowsOptions), SmokePreflight(OpsSmokePreflightOptions), + DiagnoseProvider(OpsDiagnoseProviderOptions), } impl OpsCommand { @@ -57,6 +66,7 @@ impl OpsCommand { OpsCommand::Runner(opts) => opts.common.strict, OpsCommand::Windows(opts) => opts.common.strict, OpsCommand::SmokePreflight(opts) => opts.common.strict, + OpsCommand::DiagnoseProvider(opts) => opts.common.strict, } } } @@ -337,6 +347,37 @@ pub(crate) async fn run_ops_command(command: OpsCommand) -> Result { + let token = resolve_ops_token(&opts.common)?; + let report = match call_runtime_tool( + &opts.common.server_url, + &opts.common.server_http, + token.as_deref(), + "diagnose_coding_agent", + json!({ + "project": opts.project, + "provider_id": opts.provider, + "include_live_acp": opts.include_live_acp, + }), + ) + .await + { + Ok(output) => ops_diagnose_provider_report( + &opts.common.server_url, + &opts.project, + &opts.provider, + opts.include_live_acp, + output.as_ref(), + ), + Err(failure) => ops_http_failure_report( + &opts.common.server_url, + "diagnose_coding_agent", + failure, + token.is_some(), + ), + }; + render_ops_command_output(report, opts.common.json, render_ops_diagnose_provider) + } } } @@ -1229,6 +1270,158 @@ pub(crate) fn render_ops_runner(report: &OpsReport, json_output: bool) -> Result Ok(out) } +/// Operator report for one `diagnose_coding_agent` call. Everything shown is +/// already the bounded Server projection: presence booleans, safe provider +/// inventory, and the optional live ACP diagnostic. No environment values, +/// provider stderr, or credentials ever appear here. +pub(crate) fn ops_diagnose_provider_report( + server_url: &str, + project: &str, + provider: &str, + include_live_acp: bool, + output: Option<&Value>, +) -> OpsReport { + let mut verdict = OpsVerdict::pass(); + let output = output.cloned().unwrap_or(Value::Null); + let summary = json!({ + "server_url": server_url, + "project": project, + "provider_id": provider, + "include_live_acp": include_live_acp, + "env_presence": output["env_presence"], + "live_acp": output["live_acp"], + "error_kind": output.get("error_kind").cloned().unwrap_or(Value::Null), + }); + if let Some(error_kind) = output["error_kind"].as_str() { + verdict.fail_reason( + error_kind, + "fix the reported input/authority problem and re-run; the Server never retried the diagnostic", + ); + return OpsReport { + verdict, + summary, + source: json!({ + "server_url": server_url, + "tool": "diagnose_coding_agent", + "provider_id": provider, + }), + }; + } + let mut absent: Vec = Vec::new(); + if let Some(map) = output["env_presence"].as_object() { + for (name, present) in map { + if present.as_bool() != Some(true) { + absent.push(name.clone()); + } + } + } + if !absent.is_empty() { + absent.sort(); + verdict.warn_reason( + "env_source_absent", + &format!( + "declared env_from_env sources absent from the running Runner process: {} (names only; values are never printed)", + absent.join(", ") + ), + ); + } + if include_live_acp { + let live = &output["live_acp"]; + let outcome = live["outcome"].as_str().unwrap_or("missing"); + if outcome != "ok" { + let phase = live["phase"].as_str().unwrap_or("unknown"); + verdict.fail_reason( + "live_acp_probe_failed", + &format!( + "live ACP probe stopped at outcome={outcome} phase={phase}; re-observe the Runner provider inventory and fix the provider configuration" + ), + ); + } else if live["options_truncated"] == true { + verdict.warn_reason( + "live_acp_options_truncated", + "the live option projection hit its bounded limit; unlisted options exist on the live session", + ); + } + } + OpsReport { + verdict, + summary, + source: json!({ + "server_url": server_url, + "tool": "diagnose_coding_agent", + "provider_id": provider, + }), + } +} + +pub(crate) fn render_ops_diagnose_provider( + report: &OpsReport, + json_output: bool, +) -> Result { + if json_output { + return render_ops_json(report); + } + let summary = &report.summary; + let mut out = render_overall_header(report); + out.push_str(&render_http_failure(report)); + out.push_str(&format!( + "Provider diagnose:\n project={} provider_id={} include_live_acp={}\n", + display_value(&summary["project"]), + display_value(&summary["provider_id"]), + display_value(&summary["include_live_acp"]), + )); + out.push_str("Environment mappings:\n"); + match summary["env_presence"].as_object() { + Some(map) if !map.is_empty() => { + for (name, present) in map { + out.push_str(&format!( + " {} {}\n", + name, + if present.as_bool() == Some(true) { + "present" + } else { + "ABSENT" + } + )); + } + } + _ => out.push_str(" (none declared)\n"), + } + let live = &summary["live_acp"]; + if summary["include_live_acp"] == true { + out.push_str("Live ACP:\n"); + if live.is_null() { + out.push_str(" (not reported)\n"); + } else { + out.push_str(&format!( + " outcome={} phase={} protocol_version={} options={} options_truncated={}\n", + display_value(&live["outcome"]), + display_value(&live["phase"]), + display_value(&live["protocol_version"]), + display_value(&json!(live["options"] + .as_array() + .map(Vec::len) + .unwrap_or(0))), + display_value(&live["options_truncated"]), + )); + for option in live["options"].as_array().into_iter().flatten() { + out.push_str(&format!( + " {} kind={} choices={} current={}\n", + display_value(&option["id"]), + display_value(&option["kind"]), + display_value(&json!(option["choices"] + .as_array() + .map(Vec::len) + .unwrap_or(0))), + display_value(&option["current"]), + )); + } + } + } + out.push_str(&render_reasons(report)); + Ok(out) +} + pub(crate) fn render_ops_projects(report: &OpsReport, json_output: bool) -> Result { if json_output { return render_ops_json(report); diff --git a/crates/webcodex-cli/src/webcodex_cli/tests/mod.rs b/crates/webcodex-cli/src/webcodex_cli/tests/mod.rs index a7c156535..2922595cd 100644 --- a/crates/webcodex-cli/src/webcodex_cli/tests/mod.rs +++ b/crates/webcodex-cli/src/webcodex_cli/tests/mod.rs @@ -2,6 +2,7 @@ mod support; mod connect; mod ops; +mod ops_diagnose; mod platform; mod plugin; mod profiles; diff --git a/crates/webcodex-cli/src/webcodex_cli/tests/ops.rs b/crates/webcodex-cli/src/webcodex_cli/tests/ops.rs index ea693b4d1..e08a72252 100644 --- a/crates/webcodex-cli/src/webcodex_cli/tests/ops.rs +++ b/crates/webcodex-cli/src/webcodex_cli/tests/ops.rs @@ -819,6 +819,10 @@ async fn run_ops_with_routes( opts.common.server_url = server_url; OpsCommand::SmokePreflight(opts) } + OpsCommand::DiagnoseProvider(mut opts) => { + opts.common.server_url = server_url; + OpsCommand::DiagnoseProvider(opts) + } }; let output = run_ops_command(command).await.unwrap().stdout; stop_tx.send(()).unwrap(); diff --git a/crates/webcodex-cli/src/webcodex_cli/tests/ops_diagnose.rs b/crates/webcodex-cli/src/webcodex_cli/tests/ops_diagnose.rs new file mode 100644 index 000000000..284771e11 --- /dev/null +++ b/crates/webcodex-cli/src/webcodex_cli/tests/ops_diagnose.rs @@ -0,0 +1,239 @@ +use super::support::*; +use crate::webcodex_cli::ops::ops_exit_code; +use crate::webcodex_cli::run_ops_command; +use std::time::Duration; + +fn diagnose_opts(server_url: String) -> OpsDiagnoseProviderOptions { + OpsDiagnoseProviderOptions { + common: OpsCommonOptions { + server_url, + server_http: direct_server_http(), + env_file: None, + token_file: None, + token: None, + json: false, + strict: false, + }, + project: "agent:mini:demo".to_string(), + provider: "codex".to_string(), + include_live_acp: false, + } +} + +fn diagnose_response_body() -> String { + json!({ + "success": true, + "output": { + "project": "agent:mini:demo", + "provider_id": "codex", + "env_presence": {"CODEX_PATH": true, "CODEX_HOME": false}, + "live_acp": null + } + }) + .to_string() +} + +// The CLI is only a consumer of the authorized Server entry: the fixture pins +// the exact tool name and typed arguments, and proves no credential is printed. +#[tokio::test] +async fn diagnose_provider_calls_the_canonical_server_entry_and_renders_presence() { + use std::io::BufRead as _; + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let address = listener.local_addr().unwrap(); + let (stop_tx, stop_rx) = std::sync::mpsc::channel(); + let server = thread::spawn(move || { + let mut bodies = Vec::new(); + loop { + match listener.accept() { + Ok((mut stream, _)) => { + stream.set_nonblocking(false).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + let mut reader = std::io::BufReader::new(&mut stream); + let mut length = 0; + loop { + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + if line == "\r\n" || line.is_empty() { + break; + } + if let Some(value) = + line.to_ascii_lowercase().strip_prefix("content-length:") + { + length = value.trim().parse::().unwrap(); + } + } + let mut body = vec![0u8; length]; + reader.read_exact(&mut body).unwrap(); + let request: Value = serde_json::from_slice(&body).unwrap(); + bodies.push(request.clone()); + let response = diagnose_response_body(); + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + response.len(), + response + ) + .unwrap(); + } + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + if stop_rx.try_recv().is_ok() { + break; + } + thread::sleep(Duration::from_millis(5)); + } + Err(error) => panic!("fixture accept: {error}"), + } + } + bodies + }); + let mut opts = diagnose_opts(format!("http://{address}")); + opts.common.token = Some("wc_pat_diagnose_fixture".into()); + let result = run_ops_command(OpsCommand::DiagnoseProvider(opts)) + .await + .unwrap(); + stop_tx.send(()).unwrap(); + let bodies = server.join().unwrap(); + assert_eq!(bodies.len(), 1); + assert_eq!(bodies[0]["tool"], "diagnose_coding_agent"); + assert_eq!(bodies[0]["params"]["project"], "agent:mini:demo"); + assert_eq!(bodies[0]["params"]["provider_id"], "codex"); + assert_eq!(bodies[0]["params"]["include_live_acp"], false); + // Human rendering names presence only; the token and any env value never + // appear (the fixture response carries no values, and the renderer prints + // only the boolean mapping). + assert!(result.stdout.contains("CODEX_PATH present")); + assert!(result.stdout.contains("CODEX_HOME ABSENT")); + assert!(result.stdout.contains("Overall: WARN")); + assert!(result.stdout.contains("env_source_absent")); + assert!(!result.stdout.contains("wc_pat_diagnose_fixture")); +} + +#[tokio::test] +async fn diagnose_provider_reports_server_side_failures_as_fail() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let response = json!({ + "success": false, + "error": "include_live_acp=true requires start_coding_agent authority: missing required scope: project:write", + "output": { + "error_kind": "coding_agent_live_probe_authority_required", + "run_id": null, + "execution_state": "not_started" + } + }) + .to_string(); + let server = thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + stream.set_nonblocking(false).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + let mut length = 0; + { + let mut reader = std::io::BufReader::new(&stream); + use std::io::BufRead as _; + loop { + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + if line == "\r\n" || line.is_empty() { + break; + } + if let Some(value) = line.to_ascii_lowercase().strip_prefix("content-length:") { + length = value.trim().parse::().unwrap(); + } + } + let mut body = vec![0u8; length]; + reader.read_exact(&mut body).unwrap(); + let request: Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(request["tool"], "diagnose_coding_agent"); + assert_eq!(request["params"]["include_live_acp"], true); + } + stream + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + response.len(), + response + ) + .as_bytes(), + ) + .unwrap(); + }); + let mut opts = diagnose_opts(format!("http://{address}")); + opts.common.token = Some("wc_pat_diagnose_fixture".into()); + opts.common.strict = true; + opts.include_live_acp = true; + let result = run_ops_command(OpsCommand::DiagnoseProvider(opts)) + .await + .unwrap(); + server.join().unwrap(); + assert_eq!(result.status, "fail"); + assert!(result + .stdout + .contains("coding_agent_live_probe_authority_required")); + assert_eq!( + ops_exit_code(true, "fail"), + 2, + "--strict must map a FAIL diagnose to exit 2" + ); +} + +#[test] +fn diagnose_provider_parses_flags_and_requires_project_and_provider() { + match cli_action([ + "ops", + "diagnose-provider", + "--project", + "agent:mini:demo", + "--provider", + "codex", + "--include-live-acp", + "--json", + "--strict", + ]) { + CliAction::Ops(OpsCommand::DiagnoseProvider(opts)) => { + assert_eq!(opts.project, "agent:mini:demo"); + assert_eq!(opts.provider, "codex"); + assert!(opts.include_live_acp); + assert!(opts.common.json); + assert!(opts.common.strict); + } + other => panic!("diagnose-provider did not parse: {other:?}"), + } + match cli_action(["ops", "diagnose-provider", "--project", "p"]) { + CliAction::Exit { code, stderr, .. } => { + assert_eq!(code, 2); + assert!(stderr.contains("--provider is required"), "{stderr}"); + } + other => panic!("missing provider must fail parsing: {other:?}"), + } + match cli_action(["ops", "diagnose-provider"]) { + CliAction::Exit { code, stderr, .. } => { + assert_eq!(code, 2); + assert!(stderr.contains("--project is required"), "{stderr}"); + } + other => panic!("missing project must fail parsing: {other:?}"), + } + match cli_action(["ops", "diagnose-provider", "--bogus"]) { + CliAction::Exit { code, stderr, .. } => { + assert_eq!(code, 2); + assert!( + stderr.contains("unknown ops diagnose-provider flag"), + "{stderr}" + ); + } + other => panic!("unknown flag must fail parsing: {other:?}"), + } +} + +#[test] +fn diagnose_provider_help_is_available() { + let out = cli_exit(["ops", "diagnose-provider", "--help"]).unwrap(); + assert!(out.contains("Usage: webcodex ops diagnose-provider")); + assert!(out.contains("--include-live-acp")); + let ops_help = cli_exit(["ops", "--help"]).unwrap(); + assert!(ops_help.contains("diagnose-provider")); +} diff --git a/crates/webcodex-cli/src/webcodex_cli/usage.rs b/crates/webcodex-cli/src/webcodex_cli/usage.rs index f06bd9097..4b584299f 100644 --- a/crates/webcodex-cli/src/webcodex_cli/usage.rs +++ b/crates/webcodex-cli/src/webcodex_cli/usage.rs @@ -199,7 +199,8 @@ pub(crate) fn ops_usage() -> &'static str { runner Show one exact Runner registration/build identity\n\ projects Show compact project inventory and smoke suitability\n\ windows Show observed ChatGPT Window activity for one project\n\ - smoke-preflight Check a project before deploy smoke validation\n\n\ + smoke-preflight Check a project before deploy smoke validation\n\ + diagnose-provider Diagnose one provider's env sources and optional live ACP options\n\n\ Common flags:\n\ --server-url URL WebCodex server URL [default: http://127.0.0.1:8080]\n\ --proxy http://HOST:PORT Explicit proxy override for Server requests\n\ @@ -245,6 +246,36 @@ pub(crate) fn ops_runners_usage() -> &'static str { -h, --help Print help and exit\n" } +pub(crate) fn ops_diagnose_provider_usage() -> &'static str { + "Usage: webcodex ops diagnose-provider --project PROJECT_ID --provider PROVIDER_ID [OPTIONS]\n\n\ + Diagnose one explicitly selected ACP provider for one exact authorized\n\ + Project through its configured Runner, via the same authorized Server\n\ + entry a Desktop Diagnose action would reuse.\n\n\ + Default (presence-only): report the provider's declared env_from_env\n\ + source names as present/absent booleans read from the running Runner\n\ + process. Values, unrelated names, and paths are never printed.\n\n\ + Options:\n\ + --project PROJECT_ID Exact registered Project id (required)\n\ + --provider PROVIDER_ID Logical Runner-advertised provider id, e.g. codex (required)\n\ + --include-live-acp ALSO launch one bounded ephemeral live ACP probe\n\ + (initialize + session/new only; requires the same\n\ + authority as start_coding_agent and can trigger\n\ + provider-side initialization effects such as a\n\ + login prompt; the process tree is always cleaned up)\n\ + --server-url URL WebCodex server URL [default: http://127.0.0.1:8080]\n\ + --proxy http://HOST:PORT Explicit proxy override for Server requests\n\ + --no-system-proxy Ignore proxy environment and connect directly\n\ + --env-file PATH Read user/API credential from WEBCODEX_TOKEN, then WEBCODEX_PAT\n\ + --token-file PATH Read bearer token from file\n\ + --token TOKEN Bearer token input; never printed\n\ + --json Print machine-readable output\n\ + --strict Exit 2 when the ops report status is FAIL\n\ + -h, --help Print help and exit\n\n\ + No prompt is sent, no provider tool runs, no configuration is written, and\n\ + there is no automatic retry. Stale Runner/provider/Project identities fail\n\ + closed with bounded reason codes.\n" +} + pub(crate) fn ops_runner_usage() -> &'static str { "Usage: webcodex ops runner --client-id CLIENT_ID [OPTIONS]\n\n\ Show one exact caller-visible Runner registration and build identity.\n\n\ diff --git a/crates/webcodex-core/src/coding_agent.rs b/crates/webcodex-core/src/coding_agent.rs index 45a7e6bed..dc25f487d 100644 --- a/crates/webcodex-core/src/coding_agent.rs +++ b/crates/webcodex-core/src/coding_agent.rs @@ -12,6 +12,10 @@ use std::collections::BTreeMap; pub const CODING_AGENT_MAX_PROVIDERS: usize = 8; pub const CODING_AGENT_MAX_PROVIDER_ID_BYTES: usize = 64; pub const CODING_AGENT_MAX_PROVIDER_NAME_BYTES: usize = 128; +/// Bound for one provider's declared `env_from_env` mappings; the Runner's +/// ACP config validation shares this constant, so the Diagnose presence map +/// and its output schema can never disagree with what was admitted. +pub const CODING_AGENT_MAX_ENV_MAPPINGS: usize = 64; pub const CODING_AGENT_MAX_RUN_ID_BYTES: usize = 96; pub const CODING_AGENT_MAX_INTENT_FINGERPRINT_BYTES: usize = 96; pub const CODING_AGENT_MAX_PROJECT_ID_BYTES: usize = 512; diff --git a/crates/webcodex-runner/src/webcodex_runner/config.rs b/crates/webcodex-runner/src/webcodex_runner/config.rs index a04a3dca6..786cc2368 100644 --- a/crates/webcodex-runner/src/webcodex_runner/config.rs +++ b/crates/webcodex-runner/src/webcodex_runner/config.rs @@ -129,7 +129,9 @@ pub(crate) struct RunnerConfig { pub(crate) acp: AcpConfig, } -const ACP_MAX_ENV_MAPPINGS: usize = 64; +// Shared with the CodingAgent Diagnose projection so its output schema can +// never disagree with what this validation admits. +const ACP_MAX_ENV_MAPPINGS: usize = webcodex_core::coding_agent::CODING_AGENT_MAX_ENV_MAPPINGS; const ACP_MAX_ENV_NAME_BYTES: usize = 256; const ACP_MAX_ARGS: usize = 64; const ACP_MAX_ARG_BYTES: usize = 4096; diff --git a/crates/webcodex-tool-contracts/src/registry/output_schemas/coding_agents.rs b/crates/webcodex-tool-contracts/src/registry/output_schemas/coding_agents.rs index f30680ddd..50dc522d4 100644 --- a/crates/webcodex-tool-contracts/src/registry/output_schemas/coding_agents.rs +++ b/crates/webcodex-tool-contracts/src/registry/output_schemas/coding_agents.rs @@ -1,7 +1,7 @@ use serde_json::{json, Value}; use webcodex_core::coding_agent::{ - CodingAgentEventKind, CodingAgentExecutionState, CodingAgentProviderSummary, - CodingAgentRunState, + CodingAgentEventKind, CodingAgentExecutionState, CodingAgentLiveAcpDiagnostic, + CodingAgentProviderSummary, CodingAgentRunState, CODING_AGENT_MAX_ENV_MAPPINGS, }; use crate::schema_generation::typed_host_schema; @@ -142,6 +142,46 @@ pub(super) fn output_schema_for_tool(name: &str) -> Option { )); Some(wrapped_output_schema(fields)) } + "diagnose_coding_agent" => { + // No Run exists for a diagnostic, so common_run_fields does not + // apply: the projection is the bounded diagnostic itself. + let fields = vec![ + ( + "project", + schema_type( + "string", + "Exact registered runtime Project id the diagnostic was authorized against.", + ), + ), + ( + "provider_id", + schema_type( + "string", + "Logical operator-configured provider id that was diagnosed.", + ), + ), + ( + "env_presence", + json!({ + "type": "object", + "description": "Declared env_from_env destination names mapped to presence booleans read from the running Runner process. Never values and never unrelated environment names.", + "additionalProperties": {"type": "boolean"}, + "maxProperties": CODING_AGENT_MAX_ENV_MAPPINGS + }), + ), + ( + "live_acp", + json!({ + "description": "Present only when the request explicitly authorized the live ACP probe; otherwise absent.", + "anyOf": [ + typed_host_schema::(), + {"type": "null"} + ] + }), + ), + ]; + Some(wrapped_output_schema(fields)) + } _ => None, } } diff --git a/crates/webcodex-tool-contracts/src/tests/canonical_names.rs b/crates/webcodex-tool-contracts/src/tests/canonical_names.rs index 3e8965252..fd503bda2 100644 --- a/crates/webcodex-tool-contracts/src/tests/canonical_names.rs +++ b/crates/webcodex-tool-contracts/src/tests/canonical_names.rs @@ -122,6 +122,7 @@ fn canonical_operation_names_are_unique_verb_first_and_route_independent() { "activate_", "manage_", "work_on_", + "diagnose_", ]; // Provider-specific validation and the three documented portable orchestration // entries are not noun-first CRUD. Gateways name a distinct callable namespace. diff --git a/crates/webcodex-tool-contracts/src/tests/catalog_discovery.rs b/crates/webcodex-tool-contracts/src/tests/catalog_discovery.rs index 60cfece27..615ad1674 100644 --- a/crates/webcodex-tool-contracts/src/tests/catalog_discovery.rs +++ b/crates/webcodex-tool-contracts/src/tests/catalog_discovery.rs @@ -921,6 +921,7 @@ fn coding_intent_has_independent_ordered_canonical_selection_surface() { "start_coding_agent", "observe_coding_agent", "cancel_coding_agent", + "diagnose_coding_agent", "get_session_assignment", "complete_session_message", ] { diff --git a/crates/webcodex-tool-contracts/src/tests/metadata_policy.rs b/crates/webcodex-tool-contracts/src/tests/metadata_policy.rs index e4c6fbb34..4c601457e 100644 --- a/crates/webcodex-tool-contracts/src/tests/metadata_policy.rs +++ b/crates/webcodex-tool-contracts/src/tests/metadata_policy.rs @@ -62,6 +62,11 @@ fn tool_specs_annotations_are_canonical_semantic_projections() { ToolEffect::Execute, ToolIdempotency::Keyed, ), + ( + "diagnose_coding_agent", + ToolEffect::Observe, + ToolIdempotency::PureRead, + ), ( "complete_session_message", ToolEffect::Mutate, diff --git a/crates/webcodex-tool-contracts/src/tests/output_schemas.rs b/crates/webcodex-tool-contracts/src/tests/output_schemas.rs index f651c8253..863866307 100644 --- a/crates/webcodex-tool-contracts/src/tests/output_schemas.rs +++ b/crates/webcodex-tool-contracts/src/tests/output_schemas.rs @@ -433,6 +433,7 @@ fn observation_schemas_do_not_repeat_static_continuation_semantics() { for name in [ "start_coding_agent", "observe_coding_agent", + "diagnose_coding_agent", "observe_session_messages", ] { let spec = spec_named(&specs, name); diff --git a/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs b/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs index 0588875f2..a36daa340 100644 --- a/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs +++ b/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs @@ -592,6 +592,15 @@ fn tool_definitions_drive_session_and_permission_policy() { assert_eq!(cancel.idempotency, ToolIdempotency::DesiredState); assert!(!runtime_tool_requires_permission("cancel_coding_agent")); + let diagnose = lookup_tool_definition("diagnose_coding_agent") + .unwrap() + .metadata(); + assert_eq!(diagnose.effect, ToolEffect::Observe); + assert_eq!(diagnose.risk, ToolRisk::Read); + assert_eq!(diagnose.approval, ToolApprovalPolicy::None); + assert_eq!(diagnose.idempotency, ToolIdempotency::PureRead); + assert!(!runtime_tool_requires_permission("diagnose_coding_agent")); + assert_eq!( runtime_tool_session_risk_class("__unknown__"), ToolRisk::Unknown.session_risk_class() diff --git a/crates/webcodex-tool-contracts/src/tool_call/canonical_enum.rs b/crates/webcodex-tool-contracts/src/tool_call/canonical_enum.rs index fa7d00227..e319926b6 100644 --- a/crates/webcodex-tool-contracts/src/tool_call/canonical_enum.rs +++ b/crates/webcodex-tool-contracts/src/tool_call/canonical_enum.rs @@ -941,6 +941,29 @@ pub enum ToolCall { #[schemars(regex(pattern = "^wc_agent_run_[A-Za-z0-9_.-]+$"))] run_id: String, }, + #[serde(rename = "diagnose_coding_agent")] + CodingAgentDiagnose { + /// Exact registered Project id. It selects the authorized Project whose configured Runner + /// performs the diagnostic; the Runner re-verifies that this exact Project identity and + /// root are still admitted before answering. + #[schemars(length(min = 1))] + project: String, + /// Logical Runner-advertised ACP provider id, for example codex. Executable, argv, + /// environment, credentials, and provider instance ids are Runner-owned and cannot be + /// supplied here. Choose providers only from the exact Runner inventory. + #[schemars(length(min = 1, max = 64))] + provider_id: String, + /// Authorize one bounded ephemeral live ACP probe: the Runner launches the configured + /// provider exactly like a Coding Run start, performs only `initialize` then + /// `session/new`, and always terminates the whole process tree. It never sends + /// `session/prompt`, runs provider tools, or writes configuration. This launch authority + /// is never implied by the presence-only observation authority of this tool: it requires + /// the same authority as start_coding_agent, and provider-side initialization effects + /// (for example an external login prompt) are possible. The result reports the bounded + /// phase where the probe stopped. + #[serde(default)] + include_live_acp: bool, + }, /// Execute bounded script content transported as typed data and written to /// a Runner-owned temporary file. The selected language is explicit and diff --git a/crates/webcodex-tool-contracts/src/tool_call/parsing.rs b/crates/webcodex-tool-contracts/src/tool_call/parsing.rs index 48216ba90..48f1741e3 100644 --- a/crates/webcodex-tool-contracts/src/tool_call/parsing.rs +++ b/crates/webcodex-tool-contracts/src/tool_call/parsing.rs @@ -482,6 +482,7 @@ impl ToolCall { Self::CodingAgentStart { .. } => "start_coding_agent", Self::CodingAgentObserve { .. } => "observe_coding_agent", Self::CodingAgentCancel { .. } => "cancel_coding_agent", + Self::CodingAgentDiagnose { .. } => "diagnose_coding_agent", Self::RunScript { .. } => "run_script", Self::RunShell { .. } => "run_shell", Self::OpenSessionShell { .. } => "open_session_shell", @@ -807,6 +808,7 @@ impl ToolCall { Self::RunProcess { project, .. } | Self::RunDetachedProcess { project, .. } | Self::CodingAgentStart { project, .. } + | Self::CodingAgentDiagnose { project, .. } | Self::StartAgentTaskCodingRun { project, .. } | Self::RunScript { project, .. } | Self::RunShell { project, .. } diff --git a/crates/webcodex-tool-contracts/src/tool_definition/coding_agents.rs b/crates/webcodex-tool-contracts/src/tool_definition/coding_agents.rs index 9104d57ec..a7f23e707 100644 --- a/crates/webcodex-tool-contracts/src/tool_definition/coding_agents.rs +++ b/crates/webcodex-tool-contracts/src/tool_definition/coding_agents.rs @@ -117,4 +117,36 @@ pub(super) const DEFINITIONS: &[ToolDefinition] = &[ ), PERMISSION_RISK_WRITE, ), + model_spec( + def( + "diagnose_coding_agent", + super::ToolAuditPolicy::typed_fields(&[ + super::ToolAuditResultField::value("project"), + super::ToolAuditResultField::value("provider_id"), + super::ToolAuditResultField::value("env_presence"), + super::ToolAuditResultField::value("live_acp"), + ]), + ModelVisible, + TOOL_CATEGORY_CODING_AGENT, + Some(CodingAgentRuns), + TOOL_PROVIDER_RUNNER, + // Presence-only is pure observation. The optional live ACP probe + // launches one ephemeral provider process, so the runtime gates + // include_live_acp=true on the same explicit authority as + // start_coding_agent; this static contract never grants it. + super::ToolSemanticContract { + effect: super::ToolEffect::Observe, + risk: Read, + approval: super::ToolApprovalPolicy::None, + idempotency: super::ToolIdempotency::PureRead, + }, + Some(CODING_AGENT_RUN), + true, + NoPath, + false, + false, + super::ToolSessionEvidencePolicy::NONE, + ), + "Diagnose one explicitly selected ACP provider for one exact authorized Project through its configured Runner. Default (include_live_acp=false): report only the provider's declared env_from_env source names as present/absent booleans read from the running Runner process - never values, unrelated names, paths, or process launches. include_live_acp=true additionally requires the same authority as start_coding_agent and launches one bounded ephemeral provider process exactly like a Run start, performing only initialize then session/new to project the live config-option identities (bounded ids, labels, kinds, and allowlisted choices); it never sends session/prompt, runs provider tools, or writes configuration, and the whole process tree is terminated on every exit path. One probe runs at a time per Runner within a fixed budget; a concurrent probe returns a bounded busy outcome. Results come from the live Runner, never from static configuration; stale Runner, provider-generation, or Project identities fail closed with bounded reason codes. No automatic retry or fallback: re-observe the Runner inventory and fix inputs instead.", + ), ]; diff --git a/crates/webcodex-tool-runtime-contracts/src/tool_audit.rs b/crates/webcodex-tool-runtime-contracts/src/tool_audit.rs index 5dbaf108e..163311a64 100644 --- a/crates/webcodex-tool-runtime-contracts/src/tool_audit.rs +++ b/crates/webcodex-tool-runtime-contracts/src/tool_audit.rs @@ -241,6 +241,15 @@ impl ToolCallAuditProjection for ToolCall { Self::CodingAgentCancel { run_id } => serde_json::json!({ "run_id": run_id, }), + Self::CodingAgentDiagnose { + project, + provider_id, + include_live_acp, + } => serde_json::json!({ + "project": project, + "provider_id": provider_id, + "include_live_acp": include_live_acp, + }), Self::RunScript { project, language, diff --git a/docs/design/issue-994-server-diagnose-entry-design.md b/docs/design/issue-994-server-diagnose-entry-design.md new file mode 100644 index 000000000..6288045a7 --- /dev/null +++ b/docs/design/issue-994-server-diagnose-entry-design.md @@ -0,0 +1,81 @@ +# Issue #994 stage three: the authorized Server Diagnose entry and its CLI consumer + +Follow-up to the merged Runner-side primitive (#1129). That PR shipped the +typed Runner fence (`CodingAgentRequest::Diagnose`), the presence-only +`env_from_env` projection, and the bounded ephemeral live ACP probe. This +round adds the missing middle: one canonical Server/ToolRuntime entry that +independently authorizes the exact Runner, provider generation, and +authorized Project before forwarding the typed request, plus the first +presentation surface (a CLI operator command). Desktop can reuse the same +entry later without new authority semantics. + +## The entry + +`diagnose_coding_agent` is a model-visible runtime tool next to +`observe_coding_agent`, with the same static authority shape: + +- OAuth policy `Require(coding_agent:run)` — runner observation authority, + never project-write; +- effect `Observe`, approval `None`, idempotency `PureRead`; +- Runner capability `CodingAgentRuns` required. + +The resolution order inside the entry is fail-closed and ordered so nothing +is observed or launched before its authority is established: + +1. provider id validation and principal binding; +2. `include_live_acp=true` gate — `check_runtime_tool_scope` against the + exact `start_coding_agent` scope policy, then a fresh + `PermissionEvaluator::evaluate_resolved_required("start_coding_agent", …)` + decision for the resolved Project. The decision (allow or deny) is + attached to the result like any permission-bearing execution; +3. Project resolution for the caller (`resolve_project_input_for_auth`); +4. exact Runner semantic view for the caller (offline or unauthorized → + bounded `coding_agent_runner_unavailable`); +5. provider generation from the Runner's advertised inventory + (`provider_instance_id` is looked up, never supplied); +6. typed dispatch through the existing `enqueue_coding_agent` fence, which + independently re-verifies Runner identity, provider generation, online + window, and access; +7. bounded wait (45 s > the Runner's 30 s probe budget), response identity + verification, and server-side bound re-verification of the payload. + +Project write authority is never required or implied — a diagnostic does not +mutate the Project, and the live probe's launch authority is the explicit +start gate above, not Project mutation. + +## Bound re-verification at the Server + +The Runner already projects inside fixed budgets. The Server re-verifies the +response before projecting it, so a malformed or oversized Runner answer can +never reach a caller as a diagnostic: option/choice counts, identity byte +bounds, the stable `select|boolean` kind vocabulary, the +"options only on success" invariant, and the environment-presence map bound +(`CODING_AGENT_MAX_ENV_MAPPINGS`, now shared with the Runner's ACP config +validation so the schema can never disagree with what was admitted). All +violations collapse into `invalid_runner_response`. + +## Projection + +The success payload is `project`, `provider_id`, `env_presence` +(destination name → boolean), and `live_acp` (the bounded +`CodingAgentLiveAcpDiagnostic`, `null` unless the caller authorized the live +probe). The provider instance id never crosses to the caller — it is the +Server-side identity anchor only, matching `safe_provider_inventory`. + +## CLI consumer + +`webcodex ops diagnose-provider --project --provider +[--include-live-acp]` calls the same authorized entry through the existing +`/api/tools/call` bridge with the operator credential. It renders presence +lines (`present` / `ABSENT`), the live option identities when requested, and +an ops verdict (`pass` / `warn` for absent env sources, `fail` for bounded +failures). Values, provider stderr, instance ids, and credentials never +reach the output. + +## Deliberately unchanged + +- The Runner fence, probe lifecycle, cleanup, and busy/timeout contracts + (#1129) are reused verbatim; no second ACP client exists. +- `env_from_env` presence stays spawn-free; nothing about + `include_live_acp=false` touches a process. +- No Desktop surface, no automatic retry, no configuration writes. diff --git a/src/auth/scopes.rs b/src/auth/scopes.rs index 3c6ba71f4..d99e5def0 100644 --- a/src/auth/scopes.rs +++ b/src/auth/scopes.rs @@ -800,6 +800,10 @@ mod tests { "cancel_coding_agent", OAuthToolScopePolicy::Require(SCOPE_CODING_AGENT_RUN), ), + ( + "diagnose_coding_agent", + OAuthToolScopePolicy::Require(SCOPE_CODING_AGENT_RUN), + ), ] { assert_eq!(oauth_scope_policy_for_runtime_tool(tool), policy, "{tool}"); } diff --git a/src/mcp_tests/oauth_scope.rs b/src/mcp_tests/oauth_scope.rs index e1e2c9f64..b3ac8abda 100644 --- a/src/mcp_tests/oauth_scope.rs +++ b/src/mcp_tests/oauth_scope.rs @@ -1003,6 +1003,7 @@ async fn oauth2_coding_agent_tools_require_independent_scope_in_catalog_and_dire "start_coding_agent", "observe_coding_agent", "cancel_coding_agent", + "diagnose_coding_agent", ] { assert!(!names.contains(name), "insufficient scopes leaked {name}"); } @@ -1033,6 +1034,7 @@ async fn oauth2_coding_agent_tools_require_independent_scope_in_catalog_and_dire "start_coding_agent", "observe_coding_agent", "cancel_coding_agent", + "diagnose_coding_agent", ] { assert!( !names.contains(name), @@ -1070,6 +1072,7 @@ async fn oauth2_coding_agent_tools_require_independent_scope_in_catalog_and_dire "start_coding_agent", "observe_coding_agent", "cancel_coding_agent", + "diagnose_coding_agent", ] { assert!( !names.contains(name), diff --git a/src/tool_runtime/coding_agent.rs b/src/tool_runtime/coding_agent.rs index 702dfaa5c..a511e1dd7 100644 --- a/src/tool_runtime/coding_agent.rs +++ b/src/tool_runtime/coding_agent.rs @@ -17,18 +17,24 @@ use tokio::sync::Mutex; use uuid::Uuid; use webcodex_core::coding_agent::{ merge_coding_agent_run_snapshot, validate_coding_agent_run_snapshot, CodingAgentCancelRequest, - CodingAgentConfigValue, CodingAgentDispatchState, CodingAgentEvent, CodingAgentExecutionState, - CodingAgentObservationMerge, CodingAgentObserveRequest, CodingAgentObserveResult, - CodingAgentRequest, CodingAgentResponse, CodingAgentResponsePayload, CodingAgentRunSnapshot, - CodingAgentRunState, CodingAgentStartRequest, CodingAgentTerminal, - CODING_AGENT_MAX_CONFIG_OPTIONS, CODING_AGENT_MAX_EVENTS_PER_RESPONSE, - CODING_AGENT_MAX_INVENTORY_RUNS, CODING_AGENT_OBSERVE_WAIT_MAX_SECS, - CODING_AGENT_TIMEOUT_MAX_SECS, CODING_AGENT_TIMEOUT_MIN_SECS, + CodingAgentConfigValue, CodingAgentDiagnoseRequest, CodingAgentDispatchState, CodingAgentEvent, + CodingAgentExecutionState, CodingAgentLiveAcpDiagnostic, CodingAgentObservationMerge, + CodingAgentObserveRequest, CodingAgentObserveResult, CodingAgentRequest, CodingAgentResponse, + CodingAgentResponsePayload, CodingAgentRunSnapshot, CodingAgentRunState, + CodingAgentStartRequest, CodingAgentTerminal, CODING_AGENT_MAX_CONFIG_KEY_BYTES, + CODING_AGENT_MAX_CONFIG_LABEL_BYTES, CODING_AGENT_MAX_CONFIG_OPTIONS, + CODING_AGENT_MAX_CONFIG_SELECT_CHOICES, CODING_AGENT_MAX_ENV_MAPPINGS, + CODING_AGENT_MAX_EVENTS_PER_RESPONSE, CODING_AGENT_MAX_INVENTORY_RUNS, + CODING_AGENT_OBSERVE_WAIT_MAX_SECS, CODING_AGENT_TIMEOUT_MAX_SECS, + CODING_AGENT_TIMEOUT_MIN_SECS, }; const IDEMPOTENCY_KEY_MAX_BYTES: usize = 256; const START_RESPONSE_WAIT_SECS: u64 = 32; const CONTROL_RESPONSE_WAIT_SECS: u64 = 65; +/// One live Diagnose probe is bounded to 30 s inside the Runner; the Server +/// wait only has to survive that budget plus transport margin. +const DIAGNOSE_RESPONSE_WAIT_SECS: u64 = 45; const DEFAULT_RUN_TIMEOUT_SECS: u64 = 300; const PUBLIC_TOKEN_PREFIX: &str = "wcar3_"; const PUBLIC_TOKEN_MAX_BYTES: usize = 54; @@ -1178,6 +1184,240 @@ impl ToolRuntime { } } + /// Canonical authorized Diagnose entry for one exact Project/provider. + /// Presence-only by default; `include_live_acp` additionally requires the + /// same explicit authority as start_coding_agent before any provider + /// process is launched. Forwards the typed Runner Diagnose fence and + /// never infers provider state from static configuration. + pub(crate) async fn coding_agent_diagnose( + &self, + project: String, + provider_id: String, + include_live_acp: bool, + auth: Option<&AuthContext>, + ) -> ToolResult { + if let Err(error) = webcodex_core::coding_agent::validate_provider_id(&provider_id) { + return coding_agent_error( + "invalid_coding_agent_diagnose", + error, + "not_started", + RecoveryKind::FixInput, + None, + ); + } + let principal = match stable_principal(auth) { + Ok(principal) => principal, + Err(error) => { + return coding_agent_error( + "coding_agent_identity_unavailable", + error, + "not_started", + RecoveryKind::FixInput, + None, + ) + } + }; + let authority = authority_fingerprint(&principal); + // The live probe launches one ephemeral provider process, so its + // launch authority is checked before anything is resolved or + // observed: presence-observation authority never implies it. This is + // exactly start_coding_agent's scope policy; the fresh permission + // decision for the resolved Project follows below. + if include_live_acp { + if let Err(error) = + crate::tool_runtime::kernel::check_runtime_tool_scope(auth, "start_coding_agent") + { + let detail = match error { + crate::tool_runtime::kernel::ToolCallErrorStatus::InsufficientScope { + description, + .. + } => description, + crate::tool_runtime::kernel::ToolCallErrorStatus::InvalidArguments { + message, + } => message, + }; + return coding_agent_error( + "coding_agent_live_probe_authority_required", + format!( + "include_live_acp=true requires start_coding_agent authority: {detail}" + ), + "not_started", + RecoveryKind::FixInput, + None, + ); + } + } + let resolved = match self.resolve_project_input_for_auth(&project, auth).await { + Ok(resolved) => resolved, + Err(error) => return error.into_tool_result(), + }; + // A diagnostic never requires or implies Project write authority; the + // optional live probe is gated on the explicit start authority above, + // not on Project mutation. + let client_id = resolved.config.client_id.as_str(); + let client = match self + .runner_registry + .get_runner_semantic_view_for_auth( + client_id, + crate::runner_http::runner_access_from_auth(auth).as_ref(), + ) + .await + { + Some(client) if client.view.connected => client, + _ => { + return coding_agent_error( + "coding_agent_runner_unavailable", + "exact Project Runner is offline or unauthorized", + "not_started", + RecoveryKind::Wait, + None, + ) + } + }; + if !client.supports(RunnerFeature::CodingAgentRuns) { + return coding_agent_error( + "coding_agent_unsupported", + "exact Project Runner does not advertise CodingAgentRun", + "not_started", + RecoveryKind::Reobserve, + None, + ); + } + let client = client.view; + let provider_instance_id = match client + .coding_agent_providers + .as_deref() + .unwrap_or(&[]) + .iter() + .find(|provider| provider.provider_id == provider_id) + { + Some(provider) => provider.provider_instance_id.clone(), + None => { + let mut error = coding_agent_error( + "coding_agent_provider_unavailable", + "Logical ACP provider is not advertised by the exact Project Runner. Re-observe that Runner's coding_agent_providers; choose an advertised provider_id according to the user's instructions, not an executable from PATH.", + "not_started", + RecoveryKind::Reobserve, + None, + ); + error.output["available_providers"] = + json!(webcodex_core::coding_agent::safe_provider_inventory( + client.coding_agent_providers.as_deref() + )); + error.output["suggested_call"] = SuggestedToolCall::fallback_recovery( + "get_runtime_status", + json!({"client_id": client.client_id, "compact": true}), + ) + .to_value(); + return error; + } + }; + let mut permission_decision = None; + if include_live_acp { + let decision = self.permission_evaluator.evaluate_resolved_required( + "start_coding_agent", + Some(resolved.resolved_id.as_str()), + webcodex_tool_contracts::tool_policy::runtime_tool_permission_risk( + "start_coding_agent", + ), + ); + if !decision.allows_execution() { + let mut result = + crate::tool_runtime::permissions::permission_execution_denied_result(&decision); + crate::tool_runtime::permissions::add_permission_to_result(&mut result, &decision); + return result; + } + permission_decision = Some(decision); + } + let run_id = diagnose_run_id(&principal, &provider_id, &provider_instance_id); + let mut result = self + .dispatch_coding_agent_diagnose( + &client, + &resolved.resolved_id, + &resolved.config.path, + &provider_id, + &provider_instance_id, + &run_id, + &authority, + include_live_acp, + auth, + ) + .await; + if let Some(decision) = permission_decision.as_ref() { + crate::tool_runtime::permissions::add_permission_to_result(&mut result, decision); + } + result + } + + #[allow(clippy::too_many_arguments)] + async fn dispatch_coding_agent_diagnose( + &self, + client: &crate::runner_protocol::RunnerView, + runtime_project_id: &str, + project_root: &str, + provider_id: &str, + provider_instance_id: &str, + run_id: &str, + authority: &str, + include_live_acp: bool, + auth: Option<&AuthContext>, + ) -> ToolResult { + let operation = CodingAgentRequest::Diagnose(CodingAgentDiagnoseRequest { + run_id: run_id.to_string(), + runtime_project_id: runtime_project_id.to_string(), + project_root: project_root.to_string(), + provider_id: provider_id.to_string(), + provider_instance_id: provider_instance_id.to_string(), + include_live_acp, + }); + let (_, receiver) = match self + .runner_registry + .enqueue_coding_agent( + &client.client_id, + &client.runner_instance_id, + provider_id, + provider_instance_id, + operation, + crate::runner_http::runner_access_from_auth(auth).as_ref(), + authority.to_string(), + ) + .await + { + Ok(value) => value, + Err(error) => { + return coding_agent_error( + "coding_agent_dispatch_rejected", + error, + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ) + } + }; + let response = + match tokio::time::timeout(Duration::from_secs(DIAGNOSE_RESPONSE_WAIT_SECS), receiver) + .await + { + Ok(Ok(response)) => response, + Ok(Err(_)) | Err(_) => { + return coding_agent_error( + "coding_agent_diagnose_response_lost", + "Runner did not answer the diagnostic in time; the Runner owns ephemeral probe cleanup on every exit path", + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ) + } + }; + diagnose_result_from_response( + response, + runtime_project_id, + provider_id, + provider_instance_id, + run_id, + ) + } + async fn start_waiter_lost_typed( &self, request_id: &str, @@ -1510,6 +1750,183 @@ fn deterministic_run_id(principal: &str, key: &str) -> String { ) } +/// Ephemeral per-call correlation id for one Diagnose exchange. It is never a +/// persisted Run: the Runner keeps no Diagnose run state, so uniqueness across +/// concurrent probes is all this id has to provide. +fn diagnose_run_id(principal: &str, provider_id: &str, provider_instance_id: &str) -> String { + let mut hasher = Sha256::new(); + hasher.update(b"webcodex-coding-agent-diagnose-v1\0"); + hasher.update(principal.as_bytes()); + hasher.update(b"\0"); + hasher.update(provider_id.as_bytes()); + hasher.update(b"\0"); + hasher.update(provider_instance_id.as_bytes()); + hasher.update(b"\0"); + hasher.update( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|elapsed| elapsed.as_nanos()) + .unwrap_or_default() + .to_le_bytes(), + ); + format!( + "wc_agent_run_{}", + webcodex_core::compact::encode(hasher.finalize()) + ) +} + +fn recovery_kind_from_wire(recovery: Option<&str>) -> RecoveryKind { + match recovery { + Some("fix_input") => RecoveryKind::FixInput, + Some("retry_same") => RecoveryKind::RetrySame, + Some("reconcile") => RecoveryKind::Reconcile, + Some("wait") => RecoveryKind::Wait, + Some("user_action") => RecoveryKind::UserAction, + Some("none") => RecoveryKind::NoAction, + _ => RecoveryKind::Reobserve, + } +} + +/// Server-side bound re-verification for a Runner-reported live diagnostic. +/// The Runner projects within these budgets; a response outside them is a +/// malformed result, never a projection input. Returns the bounded failure +/// message when a bound is violated. +pub(crate) fn live_acp_bound_violation(live: &CodingAgentLiveAcpDiagnostic) -> Option { + if live.options.len() > CODING_AGENT_MAX_CONFIG_OPTIONS { + return Some("live diagnostic exceeded the option bound".to_string()); + } + if matches!( + live.outcome, + webcodex_core::coding_agent::CodingAgentDiagnoseOutcome::Ok + ) && live.options.is_empty() + { + return Some("live diagnostic reported success without options".to_string()); + } + if !matches!( + live.outcome, + webcodex_core::coding_agent::CodingAgentDiagnoseOutcome::Ok + ) && !live.options.is_empty() + { + return Some("live diagnostic carried options for a non-success outcome".to_string()); + } + for option in &live.options { + if option.id.is_empty() + || option.id.len() > CODING_AGENT_MAX_CONFIG_KEY_BYTES + || option.label.as_ref().is_some_and(|label| { + label.is_empty() || label.len() > CODING_AGENT_MAX_CONFIG_LABEL_BYTES + }) + { + return Some("live diagnostic option identity exceeded its bound".to_string()); + } + if option.kind != "select" && option.kind != "boolean" { + return Some( + "live diagnostic option kind is outside the stable vocabulary".to_string(), + ); + } + if option.choices.len() > CODING_AGENT_MAX_CONFIG_SELECT_CHOICES { + return Some("live diagnostic exceeded the choice bound".to_string()); + } + for choice in &option.choices { + if choice.value.is_empty() || choice.value.len() > CODING_AGENT_MAX_CONFIG_KEY_BYTES { + return Some("live diagnostic choice identity exceeded its bound".to_string()); + } + if choice + .label + .as_ref() + .is_some_and(|label| label.len() > CODING_AGENT_MAX_CONFIG_LABEL_BYTES) + { + return Some("live diagnostic choice label exceeded its bound".to_string()); + } + } + } + None +} + +pub(crate) fn diagnose_projection( + runtime_project_id: &str, + provider_id: &str, + env_presence: BTreeMap, + live_acp: Option, +) -> Value { + json!({ + "project": runtime_project_id, + "provider_id": provider_id, + "env_presence": env_presence, + "live_acp": live_acp, + }) +} + +/// Map one Runner CodingAgent response onto the bounded Diagnose result. +/// Identity mismatches, oversized payloads, and Runner-reported failures all +/// become closed reason codes; a successful payload projects the bounded +/// diagnostic only (never the provider instance id or any environment value). +pub(crate) fn diagnose_result_from_response( + response: CodingAgentResponse, + runtime_project_id: &str, + provider_id: &str, + provider_instance_id: &str, + run_id: &str, +) -> ToolResult { + match response.payload { + Some(CodingAgentResponsePayload::Diagnose { + provider_id: response_provider, + provider_instance_id: response_instance, + env_presence, + live_acp, + }) => { + if response_provider != provider_id || response_instance != provider_instance_id { + return coding_agent_error( + "invalid_runner_response", + "Runner returned mismatched coding-agent diagnostic identity", + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ); + } + if env_presence.len() > CODING_AGENT_MAX_ENV_MAPPINGS { + return coding_agent_error( + "invalid_runner_response", + "diagnostic environment-presence map exceeded its bound", + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ); + } + if let Some(bound) = live_acp.as_ref().and_then(live_acp_bound_violation) { + return coding_agent_error( + "invalid_runner_response", + bound, + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ); + } + ToolResult::ok(diagnose_projection( + runtime_project_id, + provider_id, + env_presence, + live_acp, + )) + } + _ => match response.error { + Some(error) => coding_agent_error( + &error.code, + error.message, + "not_started", + recovery_kind_from_wire(error.recovery_kind.as_deref()), + Some(run_id), + ), + None => coding_agent_error( + "invalid_runner_response", + "Runner CodingAgentRun response contained no diagnostic result", + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ), + }, + } +} + fn intent_fingerprint( project: &str, provider: &str, @@ -1870,15 +2287,7 @@ fn coding_agent_start_failure_from_response( run_id: run_id.to_string(), }; }; - let recovery = match error.recovery_kind.as_deref() { - Some("fix_input") => RecoveryKind::FixInput, - Some("retry_same") => RecoveryKind::RetrySame, - Some("reconcile") => RecoveryKind::Reconcile, - Some("wait") => RecoveryKind::Wait, - Some("user_action") => RecoveryKind::UserAction, - Some("none") => RecoveryKind::NoAction, - _ => RecoveryKind::Reobserve, - }; + let recovery = recovery_kind_from_wire(error.recovery_kind.as_deref()); CodingAgentStartFailure { kind: error.code, message: error.message, @@ -1916,15 +2325,7 @@ fn response_to_tool_error( run_id, ); }; - let recovery = match error.recovery_kind.as_deref() { - Some("fix_input") => RecoveryKind::FixInput, - Some("retry_same") => RecoveryKind::RetrySame, - Some("reconcile") => RecoveryKind::Reconcile, - Some("wait") => RecoveryKind::Wait, - Some("user_action") => RecoveryKind::UserAction, - Some("none") => RecoveryKind::NoAction, - _ => RecoveryKind::Reobserve, - }; + let recovery = recovery_kind_from_wire(error.recovery_kind.as_deref()); coding_agent_error( &error.code, error.message, diff --git a/src/tool_runtime/dispatch/execution.rs b/src/tool_runtime/dispatch/execution.rs index 2c501f0df..04784b712 100644 --- a/src/tool_runtime/dispatch/execution.rs +++ b/src/tool_runtime/dispatch/execution.rs @@ -55,6 +55,14 @@ impl ToolRuntime { .await } ToolCall::CodingAgentCancel { run_id } => self.coding_agent_cancel(run_id, auth).await, + ToolCall::CodingAgentDiagnose { + project, + provider_id, + include_live_acp, + } => { + self.coding_agent_diagnose(project, provider_id, include_live_acp, auth) + .await + } call @ (ToolCall::RunProcess { .. } | ToolCall::RunDetachedProcess { .. } diff --git a/src/tool_runtime/dispatch/routing.rs b/src/tool_runtime/dispatch/routing.rs index 3f7e455d9..ecdcea53f 100644 --- a/src/tool_runtime/dispatch/routing.rs +++ b/src/tool_runtime/dispatch/routing.rs @@ -199,6 +199,7 @@ impl AuthorizedDispatch<'_, '_> { call @ (ToolCall::CodingAgentStart { .. } | ToolCall::CodingAgentObserve { .. } | ToolCall::CodingAgentCancel { .. } + | ToolCall::CodingAgentDiagnose { .. } | ToolCall::RunProcess { .. } | ToolCall::JobWriteInput { .. } | ToolCall::RunDetachedProcess { .. } diff --git a/src/tool_runtime/kernel.rs b/src/tool_runtime/kernel.rs index a41f6a7c0..ee107623e 100644 --- a/src/tool_runtime/kernel.rs +++ b/src/tool_runtime/kernel.rs @@ -915,7 +915,11 @@ mod tests { description: "missing required scope: project:write".to_string(), }) ); - for tool in ["observe_coding_agent", "cancel_coding_agent"] { + for tool in [ + "observe_coding_agent", + "cancel_coding_agent", + "diagnose_coding_agent", + ] { assert_eq!( check_runtime_tool_scope(Some(&run_only), tool), Ok(()), diff --git a/src/tool_runtime/sessions/tests.rs b/src/tool_runtime/sessions/tests.rs index 767f8d134..21e58cea3 100644 --- a/src/tool_runtime/sessions/tests.rs +++ b/src/tool_runtime/sessions/tests.rs @@ -118,6 +118,7 @@ fn session_tool_classification_uses_definition_policy() { #[cfg(feature = "workspace-checkpoints")] ("create_workspace_checkpoint", "checkpoint_manage"), ("cancel_coding_agent", "run_control"), + ("diagnose_coding_agent", "read_only"), ("write_project_file", "project_write"), ("apply_unified_diff", "project_write"), ("run_process", "job_run"), diff --git a/src/tool_runtime/tests/coding_agent_diagnose.rs b/src/tool_runtime/tests/coding_agent_diagnose.rs new file mode 100644 index 000000000..4821f9d4d --- /dev/null +++ b/src/tool_runtime/tests/coding_agent_diagnose.rs @@ -0,0 +1,587 @@ +use super::support::*; +use crate::auth::{AuthContext, AuthKind}; +use crate::runner_protocol::{RunnerCapabilities, RunnerRegisterRequest}; +use crate::tool_runtime::coding_agent::{diagnose_result_from_response, live_acp_bound_violation}; +use crate::tool_runtime::permissions::{AuthorityMode, PermissionEvaluator}; +use crate::tool_runtime::{ToolCall, ToolRuntime}; +use serde_json::{json, Value}; +use webcodex_core::coding_agent::{ + CodingAgentDiagnoseOutcome, CodingAgentDiagnosePhase, CodingAgentDispatchState, + CodingAgentLiveAcpDiagnostic, CodingAgentLiveConfigChoice, CodingAgentLiveConfigOption, + CodingAgentProvider, CodingAgentResponse, CodingAgentResponsePayload, + CODING_AGENT_MAX_CONFIG_KEY_BYTES, + CODING_AGENT_MAX_CONFIG_LABEL_BYTES, CODING_AGENT_MAX_CONFIG_OPTIONS, + CODING_AGENT_MAX_CONFIG_SELECT_CHOICES, CODING_AGENT_MAX_ENV_MAPPINGS, +}; + +fn oauth(scopes: &[&str]) -> AuthContext { + AuthContext { + user_id: Some("u".to_string()), + username: Some("alice".to_string()), + role: Some("user".to_string()), + scopes: scopes.iter().map(|s| s.to_string()).collect(), + token_kind: Some("oauth2".to_string()), + ..AuthContext::new(AuthKind::OAuth2Token) + } +} + +fn provider(id: &str) -> CodingAgentProvider { + CodingAgentProvider { + provider_id: id.into(), + provider_instance_id: format!("private-instance-{id}"), + name: format!("{id} Agent"), + } +} + +async fn register( + runtime: &ToolRuntime, + client_id: &str, + providers: Option>, +) { + let has_providers = providers + .as_ref() + .is_some_and(|providers| !providers.is_empty()); + runtime + .runner_registry + .register_with_auth( + RunnerRegisterRequest { + coding_agent_inventory: has_providers.then(Default::default), + coding_agent_providers: providers, + ..crate::test_support::runner_registration( + client_id, + &format!("inst-{client_id}"), + RunnerCapabilities { + coding_agent_runs: has_providers, + ..Default::default() + }, + ) + }, + None, + ) + .await + .unwrap(); +} + +async fn register_runner_with_project( + client_id: &str, + providers: Option>, +) -> ToolRuntime { + let runtime = test_runtime(); + register(&runtime, client_id, providers).await; + crate::test_support::apply_project_inventory_snapshot( + &runtime.runner_registry, + client_id, + &format!("inst-{client_id}"), + vec![registered_project("demo", "/tmp/acp-diagnose-demo")], + ) + .await; + runtime +} + +fn live_ok(options: Vec) -> CodingAgentLiveAcpDiagnostic { + CodingAgentLiveAcpDiagnostic { + outcome: CodingAgentDiagnoseOutcome::Ok, + phase: CodingAgentDiagnosePhase::Projection, + protocol_version: Some(1), + options, + options_truncated: false, + } +} + +fn select_option(id: &str, choices: Vec) -> CodingAgentLiveConfigOption { + CodingAgentLiveConfigOption { + id: id.to_string(), + label: Some(format!("{id} label")), + kind: "select".to_string(), + current: Some(choices.first().cloned().unwrap_or_default()), + choices: choices + .into_iter() + .map(|value| CodingAgentLiveConfigChoice { value, label: None }) + .collect(), + choices_truncated: false, + } +} + +fn success_response( + env_presence: serde_json::Map, + live: Option, +) -> CodingAgentResponse { + CodingAgentResponse { + dispatch_state: CodingAgentDispatchState::Completed, + payload: Some(CodingAgentResponsePayload::Diagnose { + provider_id: "codex".to_string(), + provider_instance_id: "private-instance-codex".to_string(), + env_presence: env_presence + .into_iter() + .map(|(name, value)| { + ( + name, + value.as_bool().expect("test env presence must be boolean"), + ) + }) + .collect(), + live_acp: live, + }), + error: None, + } +} + +// --------------------------------------------------------------------------- +// Authority: the live probe never inherits observation authority. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn diagnose_live_probe_requires_start_authority_before_any_resolution() { + let runtime = test_runtime(); + // The scope gate fires before Project resolution or Runner observation: + // an unknown project must not leak resolution details for an + // under-authorized live probe either. + let run_only = oauth(&["coding_agent:run"]); + let result = runtime + .coding_agent_diagnose( + "agent:mini:demo".into(), + "codex".into(), + true, + Some(&run_only), + ) + .await; + assert!(!result.success); + assert_eq!( + result.output["error_kind"], + "coding_agent_live_probe_authority_required" + ); + assert!(result + .error + .as_deref() + .unwrap() + .contains("start_coding_agent")); + let write_only = oauth(&["project:write"]); + let result = runtime + .coding_agent_diagnose( + "agent:mini:demo".into(), + "codex".into(), + true, + Some(&write_only), + ) + .await; + assert_eq!( + result.output["error_kind"], + "coding_agent_live_probe_authority_required" + ); +} + +#[tokio::test] +async fn diagnose_presence_only_needs_no_start_authority_and_authorized_live_probe_passes_the_gate() +{ + let runtime = test_runtime(); + // Presence-only under run-only authority proceeds past the authority gate; + // the unregistered Project then fails at resolution instead. + let run_only = oauth(&["coding_agent:run"]); + let result = runtime + .coding_agent_diagnose( + "agent:mini:demo".into(), + "codex".into(), + false, + Some(&run_only), + ) + .await; + assert!(!result.success); + assert_ne!( + result.output["error_kind"], + "coding_agent_live_probe_authority_required" + ); + // Fully authorized live probe passes the gate too (same resolver failure). + let start_allowed = oauth(&["coding_agent:run", "project:write"]); + let result = runtime + .coding_agent_diagnose( + "agent:mini:demo".into(), + "codex".into(), + true, + Some(&start_allowed), + ) + .await; + assert_ne!( + result.output["error_kind"], + "coding_agent_live_probe_authority_required" + ); +} + +#[tokio::test] +async fn diagnose_live_probe_is_permission_gated_per_project() { + let client_id = "diagnose-restricted"; + let runtime = test_runtime() + .with_permission_evaluator(PermissionEvaluator::with_mode(AuthorityMode::Restricted)); + register(&runtime, client_id, Some(vec![provider("codex")])).await; + crate::test_support::apply_project_inventory_snapshot( + &runtime.runner_registry, + client_id, + &format!("inst-{client_id}"), + vec![registered_project("demo", "/tmp/acp-diagnose-demo")], + ) + .await; + let bootstrap = auth_context(Some("operator"), true); + let result = runtime + .coding_agent_diagnose( + format!("agent:{client_id}:demo"), + "codex".into(), + true, + Some(&bootstrap), + ) + .await; + assert!(!result.success); + assert_eq!(result.output["error_kind"], "permission_denied"); + assert_eq!(result.output["permission"]["status"], "denied"); + // Presence-only stays permission-free even under restricted mode: the + // unadvertised provider fails at inventory resolution, not at a gate. + let result = runtime + .coding_agent_diagnose( + format!("agent:{client_id}:demo"), + "missing".into(), + false, + Some(&bootstrap), + ) + .await; + assert!(!result.success); + assert_eq!( + result.output["error_kind"], + "coding_agent_provider_unavailable" + ); + assert!(result.output.get("permission").is_none()); +} + +// --------------------------------------------------------------------------- +// Exact Runner/provider generation resolution. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn diagnose_rejects_runner_without_coding_agent_capability() { + let runtime = register_runner_with_project("plain-diagnose", None).await; + let bootstrap = auth_context(Some("operator"), true); + let result = runtime + .coding_agent_diagnose( + "agent:plain-diagnose:demo".into(), + "codex".into(), + false, + Some(&bootstrap), + ) + .await; + assert!(!result.success); + assert_eq!(result.output["error_kind"], "coding_agent_unsupported"); +} + +#[tokio::test] +async fn diagnose_unadvertised_provider_returns_safe_inventory_and_schema_valid_recovery() { + let runtime = + register_runner_with_project("mini", Some(vec![provider("pi"), provider("codex")])).await; + let bootstrap = auth_context(Some("operator"), true); + let result = runtime + .coding_agent_diagnose( + "agent:mini:demo".into(), + "missing".into(), + false, + Some(&bootstrap), + ) + .await; + assert!(!result.success); + assert_eq!( + result.output["error_kind"], + "coding_agent_provider_unavailable" + ); + let inventory = &result.output["available_providers"]; + assert_eq!( + inventory, + &json!([ + {"provider_id": "pi", "name": "pi Agent"}, + {"provider_id": "codex", "name": "codex Agent"} + ]) + ); + assert!(!result.output.to_string().contains("private-instance")); + let suggested = &result.output["suggested_call"]; + webcodex_tool_contracts::test_support::validate_generated_tool_call_against_registered_input_schema( + suggested, + ) + .expect("diagnose inventory recovery must pass get_runtime_status registered inputSchema"); +} + +// --------------------------------------------------------------------------- +// Response mapping: identity, bounds, closed failures, non-disclosure. +// --------------------------------------------------------------------------- + +#[test] +fn diagnose_response_with_mismatched_provider_identity_fails_closed() { + let mut response = success_response( + serde_json::Map::from_iter([("CODEX_PATH".to_string(), json!(true))]), + None, + ); + if let Some(CodingAgentResponsePayload::Diagnose { + provider_instance_id, + .. + }) = response.payload.as_mut() + { + *provider_instance_id = "replaced-instance".to_string(); + } + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert!(!result.success); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); +} + +#[test] +fn diagnose_response_maps_runner_failures_to_closed_reasons() { + let response = CodingAgentResponse::error( + CodingAgentDispatchState::NotStarted, + "stale_coding_agent_provider", + "ACP provider instance was replaced", + Some("stale_state"), + Some("reobserve"), + ); + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert!(!result.success); + assert_eq!(result.output["error_kind"], "stale_coding_agent_provider"); + assert_eq!(result.output["execution_state"], "not_started"); + + let response = CodingAgentResponse { + dispatch_state: CodingAgentDispatchState::Completed, + payload: None, + error: None, + }; + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); +} + +#[test] +fn diagnose_success_projection_is_bounded_and_discloses_nothing_sensitive() { + let env_presence = serde_json::Map::from_iter([ + ("CODEX_PATH".to_string(), json!(true)), + ("CODEX_HOME".to_string(), json!(false)), + ]); + let result = diagnose_result_from_response( + success_response( + env_presence, + Some(live_ok(vec![select_option( + "model", + vec!["gpt-6-sol".to_string(), "gpt-6.1-sol".to_string()], + )])), + ), + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert!(result.success); + let serialized = result.output.to_string(); + for forbidden in [ + "provider_instance_id", + "private-instance", + "CODEX_PATH=", + "executable", + ] { + assert!(!serialized.contains(forbidden), "leaked: {forbidden}"); + } + // Advertised live choice values (the model catalog) are the point of the + // live projection and are intentionally visible. + assert_eq!(result.output["project"], "agent:mini:demo"); + assert_eq!(result.output["provider_id"], "codex"); + assert_eq!( + result.output["env_presence"], + json!({"CODEX_PATH": true, "CODEX_HOME": false}) + ); + assert_eq!(result.output["live_acp"]["outcome"], "ok"); + assert_eq!(result.output["live_acp"]["phase"], "projection"); + assert_eq!(result.output["live_acp"]["protocol_version"], 1); + // Presence-only projection: no live section beyond null. + let result = diagnose_result_from_response( + success_response(serde_json::Map::new(), None), + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert_eq!(result.output["live_acp"], Value::Null); + + // The full success payload validates against the registered output schema. + let schema = webcodex_tool_contracts::registry::registered_tool_specs() + .into_iter() + .find(|spec| spec.name == "diagnose_coding_agent") + .expect("diagnose_coding_agent must be a registered tool") + .output_schema; + let live = live_ok(vec![select_option("model", vec!["m1".to_string()])]); + let payload = json!({ + "project": "agent:mini:demo", + "provider_id": "codex", + "env_presence": {"CODEX_PATH": true}, + "live_acp": serde_json::to_value(live).unwrap(), + }); + let wrapped = json!({"success": true, "output": payload}); + webcodex_tool_contracts::test_support::validate_schema_instance(&wrapped, &schema) + .expect("diagnose projection must match its registered output schema"); +} + +#[test] +fn diagnose_response_bounds_fail_closed() { + let oversized = |count: usize, choices: usize| -> Vec { + (0..count) + .map(|index| { + select_option( + &format!("option-{index}"), + (0..choices) + .map(|choice| format!("c{choice}")) + .collect::>(), + ) + }) + .collect() + }; + for (label, live) in [ + ( + "too many options", + live_ok(oversized(CODING_AGENT_MAX_CONFIG_OPTIONS + 1, 1)), + ), + ( + "too many choices", + live_ok(vec![select_option( + "model", + (0..CODING_AGENT_MAX_CONFIG_SELECT_CHOICES + 1) + .map(|choice| format!("c{choice}")) + .collect::>(), + )]), + ), + ("success without options", live_ok(Vec::new())), + ] { + let result = diagnose_result_from_response( + success_response(serde_json::Map::new(), Some(live)), + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert!(!result.success, "{label}"); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); + } + // A non-success outcome must never carry projected options. + let mut failed = live_ok(vec![select_option("model", vec!["m1".to_string()])]); + failed.outcome = CodingAgentDiagnoseOutcome::Timeout; + failed.phase = CodingAgentDiagnosePhase::Initialize; + let result = diagnose_result_from_response( + success_response(serde_json::Map::new(), Some(failed)), + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); + // Oversized environment-presence map. + let mut response = success_response(serde_json::Map::new(), None); + if let Some(CodingAgentResponsePayload::Diagnose { env_presence, .. }) = + response.payload.as_mut() + { + for index in 0..=CODING_AGENT_MAX_ENV_MAPPINGS { + env_presence.insert(format!("KEY_{index}"), true); + } + } + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + ); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); +} + +#[test] +fn live_acp_bound_violation_rejects_unstable_identities() { + let mut option = select_option("model", vec!["m1".to_string()]); + option.kind = "freeform".to_string(); + assert!(live_acp_bound_violation(&live_ok(vec![option])).is_some()); + + let mut option = select_option("model", vec!["m1".to_string()]); + option.id = "x".repeat(CODING_AGENT_MAX_CONFIG_KEY_BYTES + 1); + assert!(live_acp_bound_violation(&live_ok(vec![option])).is_some()); + + let mut option = select_option("model", vec!["m1".to_string()]); + option.label = Some("x".repeat(CODING_AGENT_MAX_CONFIG_LABEL_BYTES + 1)); + assert!(live_acp_bound_violation(&live_ok(vec![option])).is_some()); + + let mut option = select_option("model", vec!["m1".to_string()]); + option.choices[0].value = String::new(); + assert!(live_acp_bound_violation(&live_ok(vec![option])).is_some()); + + let mut diagnostic = live_ok(vec![select_option("model", vec!["m1".to_string()])]); + diagnostic.options_truncated = true; + assert!(live_acp_bound_violation(&diagnostic).is_none()); + + assert!(live_acp_bound_violation(&live_ok(vec![select_option( + "model", + vec!["m1".to_string()] + )])) + .is_none()); +} + +// Full governance path: the router pre-resolves the Project, then the runtime +// authority gate must still refuse the live probe. Presence-only observation +// authority (coding_agent:run alone) must never launch the provider process. +#[tokio::test] +async fn diagnose_live_probe_authority_holds_through_full_dispatch() { + let runtime = test_runtime(); + let run_only = oauth(&["coding_agent:run", "project:read"]); + runtime + .runner_registry + .register_with_auth( + RunnerRegisterRequest { + owner: run_only.username.clone(), + coding_agent_inventory: Some(Default::default()), + coding_agent_providers: Some(vec![provider("codex")]), + ..crate::test_support::runner_registration( + "mini", + "inst-mini", + RunnerCapabilities { + coding_agent_runs: true, + ..Default::default() + }, + ) + }, + Some(crate::test_support::runner_access(&run_only)).as_ref(), + ) + .await + .unwrap(); + crate::test_support::apply_project_inventory_snapshot( + &runtime.runner_registry, + "mini", + "inst-mini", + vec![registered_project("demo", "/tmp/acp-diagnose-demo")], + ) + .await; + let result = runtime + .dispatch_with_auth( + ToolCall::CodingAgentDiagnose { + project: "agent:mini:demo".into(), + provider_id: "codex".into(), + include_live_acp: true, + }, + Some(&run_only), + ) + .await; + assert!(!result.success); + assert_eq!( + result.output["error_kind"], + "coding_agent_live_probe_authority_required" + ); + assert!(result.error.as_deref().unwrap().contains("project:write")); +} diff --git a/src/tool_runtime/tests/mod.rs b/src/tool_runtime/tests/mod.rs index 6b7938039..22ff8e4a0 100644 --- a/src/tool_runtime/tests/mod.rs +++ b/src/tool_runtime/tests/mod.rs @@ -21,6 +21,7 @@ mod code_mode; #[cfg(feature = "experimental-code-mode")] mod code_mode_e2b; mod coding_agent_context; +mod coding_agent_diagnose; mod coding_agent_discovery; mod coding_task; mod coding_task_semantic_navigation; diff --git a/src/tool_runtime/tests/schema/discovery.rs b/src/tool_runtime/tests/schema/discovery.rs index 6c1866500..ef33e0943 100644 --- a/src/tool_runtime/tests/schema/discovery.rs +++ b/src/tool_runtime/tests/schema/discovery.rs @@ -2478,6 +2478,14 @@ async fn tool_manifest_projects_canonical_semantic_contracts() { "desired_state", false, ), + ( + "diagnose_coding_agent", + "observe", + "read_only", + "none", + "pure_read", + true, + ), ] { let result = runtime .dispatch(ToolCall::ToolManifest { From 47193b0e06ada0960df58530860ca2489450e4dd Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Wed, 28 Oct 2026 09:55:09 +0800 Subject: [PATCH 2/2] fix(review): fence ACP Diagnose results and lost-response certainty --- crates/webcodex-core/src/coding_agent.rs | 2 + .../src/webcodex_runner/config.rs | 2 +- src/tool_runtime/coding_agent.rs | 80 ++++++++++++++--- .../tests/coding_agent_diagnose.rs | 86 ++++++++++++++++++- 4 files changed, 151 insertions(+), 19 deletions(-) diff --git a/crates/webcodex-core/src/coding_agent.rs b/crates/webcodex-core/src/coding_agent.rs index dc25f487d..1ab02ad1f 100644 --- a/crates/webcodex-core/src/coding_agent.rs +++ b/crates/webcodex-core/src/coding_agent.rs @@ -16,6 +16,8 @@ pub const CODING_AGENT_MAX_PROVIDER_NAME_BYTES: usize = 128; /// ACP config validation shares this constant, so the Diagnose presence map /// and its output schema can never disagree with what was admitted. pub const CODING_AGENT_MAX_ENV_MAPPINGS: usize = 64; +/// Maximum byte length of a declared ACP environment variable name. +pub const CODING_AGENT_MAX_ENV_NAME_BYTES: usize = 256; pub const CODING_AGENT_MAX_RUN_ID_BYTES: usize = 96; pub const CODING_AGENT_MAX_INTENT_FINGERPRINT_BYTES: usize = 96; pub const CODING_AGENT_MAX_PROJECT_ID_BYTES: usize = 512; diff --git a/crates/webcodex-runner/src/webcodex_runner/config.rs b/crates/webcodex-runner/src/webcodex_runner/config.rs index 786cc2368..af8ac6749 100644 --- a/crates/webcodex-runner/src/webcodex_runner/config.rs +++ b/crates/webcodex-runner/src/webcodex_runner/config.rs @@ -132,7 +132,7 @@ pub(crate) struct RunnerConfig { // Shared with the CodingAgent Diagnose projection so its output schema can // never disagree with what this validation admits. const ACP_MAX_ENV_MAPPINGS: usize = webcodex_core::coding_agent::CODING_AGENT_MAX_ENV_MAPPINGS; -const ACP_MAX_ENV_NAME_BYTES: usize = 256; +const ACP_MAX_ENV_NAME_BYTES: usize = webcodex_core::coding_agent::CODING_AGENT_MAX_ENV_NAME_BYTES; const ACP_MAX_ARGS: usize = 64; const ACP_MAX_ARG_BYTES: usize = 4096; const ACP_MAX_ARGS_BYTES: usize = 16 * 1024; diff --git a/src/tool_runtime/coding_agent.rs b/src/tool_runtime/coding_agent.rs index a511e1dd7..b4811b326 100644 --- a/src/tool_runtime/coding_agent.rs +++ b/src/tool_runtime/coding_agent.rs @@ -24,9 +24,9 @@ use webcodex_core::coding_agent::{ CodingAgentStartRequest, CodingAgentTerminal, CODING_AGENT_MAX_CONFIG_KEY_BYTES, CODING_AGENT_MAX_CONFIG_LABEL_BYTES, CODING_AGENT_MAX_CONFIG_OPTIONS, CODING_AGENT_MAX_CONFIG_SELECT_CHOICES, CODING_AGENT_MAX_ENV_MAPPINGS, - CODING_AGENT_MAX_EVENTS_PER_RESPONSE, CODING_AGENT_MAX_INVENTORY_RUNS, - CODING_AGENT_OBSERVE_WAIT_MAX_SECS, CODING_AGENT_TIMEOUT_MAX_SECS, - CODING_AGENT_TIMEOUT_MIN_SECS, + CODING_AGENT_MAX_ENV_NAME_BYTES, CODING_AGENT_MAX_EVENTS_PER_RESPONSE, + CODING_AGENT_MAX_INVENTORY_RUNS, CODING_AGENT_OBSERVE_WAIT_MAX_SECS, + CODING_AGENT_TIMEOUT_MAX_SECS, CODING_AGENT_TIMEOUT_MIN_SECS, }; const IDEMPOTENCY_KEY_MAX_BYTES: usize = 256; @@ -1399,15 +1399,7 @@ impl ToolRuntime { .await { Ok(Ok(response)) => response, - Ok(Err(_)) | Err(_) => { - return coding_agent_error( - "coding_agent_diagnose_response_lost", - "Runner did not answer the diagnostic in time; the Runner owns ephemeral probe cleanup on every exit path", - "not_started", - RecoveryKind::Reobserve, - Some(run_id), - ) - } + Ok(Err(_)) | Err(_) => return diagnose_response_lost(include_live_acp, run_id), }; diagnose_result_from_response( response, @@ -1415,6 +1407,7 @@ impl ToolRuntime { provider_id, provider_instance_id, run_id, + include_live_acp, ) } @@ -1775,6 +1768,18 @@ fn diagnose_run_id(principal: &str, provider_id: &str, provider_instance_id: &st ) } +/// Once a live request was queued the Runner may have launched its provider, +/// even if the response channel was lost. Never claim that did not happen. +pub(crate) fn diagnose_response_lost(include_live_acp: bool, run_id: &str) -> ToolResult { + coding_agent_error( + "coding_agent_diagnose_response_lost", + "Runner did not answer the diagnostic in time; the Runner owns ephemeral probe cleanup on every exit path", + if include_live_acp { "outcome_unknown" } else { "not_started" }, + RecoveryKind::Reobserve, + Some(run_id), + ) +} + fn recovery_kind_from_wire(recovery: Option<&str>) -> RecoveryKind { match recovery { Some("fix_input") => RecoveryKind::FixInput, @@ -1838,6 +1843,33 @@ pub(crate) fn live_acp_bound_violation(live: &CodingAgentLiveAcpDiagnostic) -> O return Some("live diagnostic choice label exceeded its bound".to_string()); } } + match option.kind.as_str() { + "select" => { + if option.current.as_ref().is_some_and(|current| { + !option.choices.iter().any(|choice| &choice.value == current) + }) { + return Some( + "live diagnostic current select value was not an advertised choice" + .to_string(), + ); + } + } + "boolean" => { + if !option.choices.is_empty() + || option.choices_truncated + || option + .current + .as_deref() + .is_some_and(|value| value != "true" && value != "false") + { + return Some( + "live diagnostic boolean option contained invalid choices or current value" + .to_string(), + ); + } + } + _ => unreachable!("kind was validated above"), + } } None } @@ -1866,6 +1898,7 @@ pub(crate) fn diagnose_result_from_response( provider_id: &str, provider_instance_id: &str, run_id: &str, + include_live_acp: bool, ) -> ToolResult { match response.payload { Some(CodingAgentResponsePayload::Diagnose { @@ -1883,10 +1916,29 @@ pub(crate) fn diagnose_result_from_response( Some(run_id), ); } - if env_presence.len() > CODING_AGENT_MAX_ENV_MAPPINGS { + if env_presence.len() > CODING_AGENT_MAX_ENV_MAPPINGS + || env_presence.keys().any(|name| { + name.is_empty() + || name.len() > CODING_AGENT_MAX_ENV_NAME_BYTES + || !name.is_ascii() + || name.contains('\0') + || name.contains('=') + }) + { + return coding_agent_error( + "invalid_runner_response", + "diagnostic environment-presence names or count exceeded their bounds", + "not_started", + RecoveryKind::Reobserve, + Some(run_id), + ); + } + // Presence-only callers have no authority to request live option + // values; reject a mismatched Runner payload instead of projecting it. + if live_acp.is_some() != include_live_acp { return coding_agent_error( "invalid_runner_response", - "diagnostic environment-presence map exceeded its bound", + "Runner diagnostic live ACP payload did not match the requested mode", "not_started", RecoveryKind::Reobserve, Some(run_id), diff --git a/src/tool_runtime/tests/coding_agent_diagnose.rs b/src/tool_runtime/tests/coding_agent_diagnose.rs index 4821f9d4d..42ffaa9ea 100644 --- a/src/tool_runtime/tests/coding_agent_diagnose.rs +++ b/src/tool_runtime/tests/coding_agent_diagnose.rs @@ -1,7 +1,9 @@ use super::support::*; use crate::auth::{AuthContext, AuthKind}; use crate::runner_protocol::{RunnerCapabilities, RunnerRegisterRequest}; -use crate::tool_runtime::coding_agent::{diagnose_result_from_response, live_acp_bound_violation}; +use crate::tool_runtime::coding_agent::{ + diagnose_response_lost, diagnose_result_from_response, live_acp_bound_violation, +}; use crate::tool_runtime::permissions::{AuthorityMode, PermissionEvaluator}; use crate::tool_runtime::{ToolCall, ToolRuntime}; use serde_json::{json, Value}; @@ -9,9 +11,9 @@ use webcodex_core::coding_agent::{ CodingAgentDiagnoseOutcome, CodingAgentDiagnosePhase, CodingAgentDispatchState, CodingAgentLiveAcpDiagnostic, CodingAgentLiveConfigChoice, CodingAgentLiveConfigOption, CodingAgentProvider, CodingAgentResponse, CodingAgentResponsePayload, - CODING_AGENT_MAX_CONFIG_KEY_BYTES, - CODING_AGENT_MAX_CONFIG_LABEL_BYTES, CODING_AGENT_MAX_CONFIG_OPTIONS, - CODING_AGENT_MAX_CONFIG_SELECT_CHOICES, CODING_AGENT_MAX_ENV_MAPPINGS, + CODING_AGENT_MAX_CONFIG_KEY_BYTES, CODING_AGENT_MAX_CONFIG_LABEL_BYTES, + CODING_AGENT_MAX_CONFIG_OPTIONS, CODING_AGENT_MAX_CONFIG_SELECT_CHOICES, + CODING_AGENT_MAX_ENV_MAPPINGS, }; fn oauth(scopes: &[&str]) -> AuthContext { @@ -326,6 +328,7 @@ fn diagnose_response_with_mismatched_provider_identity_fails_closed() { "codex", "private-instance-codex", "wc_agent_run_x", + false, ); assert!(!result.success); assert_eq!(result.output["error_kind"], "invalid_runner_response"); @@ -346,6 +349,7 @@ fn diagnose_response_maps_runner_failures_to_closed_reasons() { "codex", "private-instance-codex", "wc_agent_run_x", + false, ); assert!(!result.success); assert_eq!(result.output["error_kind"], "stale_coding_agent_provider"); @@ -362,6 +366,7 @@ fn diagnose_response_maps_runner_failures_to_closed_reasons() { "codex", "private-instance-codex", "wc_agent_run_x", + false, ); assert_eq!(result.output["error_kind"], "invalid_runner_response"); } @@ -384,6 +389,7 @@ fn diagnose_success_projection_is_bounded_and_discloses_nothing_sensitive() { "codex", "private-instance-codex", "wc_agent_run_x", + true, ); assert!(result.success); let serialized = result.output.to_string(); @@ -413,6 +419,7 @@ fn diagnose_success_projection_is_bounded_and_discloses_nothing_sensitive() { "codex", "private-instance-codex", "wc_agent_run_x", + false, ); assert_eq!(result.output["live_acp"], Value::Null); @@ -470,6 +477,7 @@ fn diagnose_response_bounds_fail_closed() { "codex", "private-instance-codex", "wc_agent_run_x", + true, ); assert!(!result.success, "{label}"); assert_eq!(result.output["error_kind"], "invalid_runner_response"); @@ -484,6 +492,7 @@ fn diagnose_response_bounds_fail_closed() { "codex", "private-instance-codex", "wc_agent_run_x", + true, ); assert_eq!(result.output["error_kind"], "invalid_runner_response"); // Oversized environment-presence map. @@ -501,6 +510,7 @@ fn diagnose_response_bounds_fail_closed() { "codex", "private-instance-codex", "wc_agent_run_x", + false, ); assert_eq!(result.output["error_kind"], "invalid_runner_response"); } @@ -534,6 +544,74 @@ fn live_acp_bound_violation_rejects_unstable_identities() { .is_none()); } +#[test] +fn diagnose_response_lost_preserves_possible_live_launch() { + let live = diagnose_response_lost(true, "wc_agent_run_x"); + assert!(!live.success); + assert_eq!( + live.output["error_kind"], + "coding_agent_diagnose_response_lost" + ); + assert_eq!(live.output["execution_state"], "outcome_unknown"); + let presence = diagnose_response_lost(false, "wc_agent_run_x"); + assert_eq!(presence.output["execution_state"], "not_started"); +} + +#[test] +fn diagnose_response_rejects_mode_mismatch_and_unbounded_env_names() { + let live = live_ok(vec![select_option("model", vec!["m1".to_string()])]); + for (response, requested_live) in [ + (success_response(serde_json::Map::new(), Some(live)), false), + (success_response(serde_json::Map::new(), None), true), + ] { + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + requested_live, + ); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); + } + for invalid in [ + "X".repeat(webcodex_core::coding_agent::CODING_AGENT_MAX_ENV_NAME_BYTES + 1), + "HAS=EQUALS".to_string(), + "\u{4e2d}".to_string(), + ] { + let response = success_response(serde_json::Map::from_iter([(invalid, json!(true))]), None); + let result = diagnose_result_from_response( + response, + "agent:mini:demo", + "codex", + "private-instance-codex", + "wc_agent_run_x", + false, + ); + assert_eq!(result.output["error_kind"], "invalid_runner_response"); + } +} + +#[test] +fn live_diagnostic_current_must_be_from_advertised_choices() { + let mut unknown = select_option("model", vec!["model-a".to_string()]); + unknown.current = Some("unadvertised-private-value".into()); + assert!(live_acp_bound_violation(&live_ok(vec![unknown])).is_some()); + + let mut bool_option = select_option("auto", vec![]); + bool_option.kind = "boolean".into(); + bool_option.current = Some("true".into()); + assert!(live_acp_bound_violation(&live_ok(vec![bool_option.clone()])).is_none()); + bool_option.current = Some("secret".into()); + assert!(live_acp_bound_violation(&live_ok(vec![bool_option.clone()])).is_some()); + bool_option.current = Some("false".into()); + bool_option.choices.push(CodingAgentLiveConfigChoice { + value: "unexpected".into(), + label: None, + }); + assert!(live_acp_bound_violation(&live_ok(vec![bool_option])).is_some()); +} + // Full governance path: the router pre-resolves the Project, then the runtime // authority gate must still refuse the live probe. Presence-only observation // authority (coding_agent:run alone) must never launch the provider process.