From 430bd94679f6bda6a97c1443449238b270f68348 Mon Sep 17 00:00:00 2001 From: calvinsturm Date: Tue, 30 Jun 2026 15:39:14 -0700 Subject: [PATCH 1/2] Add shell command timeout and cleanup Replace the unbounded `command.output().await` host shell execution with a managed child: piped stdout/stderr drained into shared buffers, `kill_on_drop` enabled, and an optional per-call `timeout_ms`. - ShellReq gains `timeout_ms` (u64; 0 = unbounded, preserving prior behavior). Exposed as an optional `timeout_ms` arg on the `shell` tool (catalog + schema + strict validation; integer >= 0 so negatives are unrepresentable). - On timeout the direct child is killed and reaped, drain tasks get a short grace then abort (so a surviving grandchild holding the pipe can never block return), and a structured result is returned: ok=false, timed_out=true, timeout_ms, partial stdout/stderr, and an actionable hint. - New ShellExecTimeout tool error code; classify_shell_target_error detects the timeout envelope before other shell error classification. - kill_on_drop(true) means a cancelled turn no longer silently orphans the direct child. Limitation: termination is best-effort on the direct child only; commands run through a shell wrapper (sh -c / cmd /C) may leave grandchildren until they exit. Robust process-tree cleanup (unix process groups / Windows job objects) and docker-target timeout enforcement are follow-ups. Tests: host command under timeout succeeds; exceeding timeout returns promptly (<2s for a 200ms budget) with timed_out + hint; timeout_ms=0 stays unbounded. --- src/target.rs | 302 ++++++++++++++++++++++++++++++++++++---- src/tools.rs | 2 + src/tools/catalog.rs | 5 +- src/tools/exec_shell.rs | 27 ++++ src/tools/schema.rs | 8 +- 5 files changed, 313 insertions(+), 31 deletions(-) diff --git a/src/target.rs b/src/target.rs index 8b703ca..6baa6ba 100644 --- a/src/target.rs +++ b/src/target.rs @@ -68,6 +68,11 @@ pub struct ShellReq { pub args: Vec, pub cwd: Option, pub max_tool_output_bytes: usize, + /// Wall-clock timeout for the command, in milliseconds. `0` means unbounded + /// (the historical default). The `u64` type makes negative values + /// unrepresentable. Timeout enforcement currently applies to the host + /// execution target only; the docker target ignores this field (follow-up). + pub timeout_ms: u64, } #[derive(Debug, Clone)] @@ -144,34 +149,14 @@ impl ExecTarget for HostTarget { None => req.workdir.clone(), }; command.current_dir(cwd); - match command.output().await { - Ok(output) => { - let stdout_raw = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr_raw = String::from_utf8_lossy(&output.stderr).to_string(); - let (stdout, stdout_truncated) = - truncate_utf8_to_bytes(&stdout_raw, req.max_tool_output_bytes); - let (stderr, stderr_truncated) = - truncate_utf8_to_bytes(&stderr_raw, req.max_tool_output_bytes); - TargetResult { - ok: output.status.success(), - content: json!({ - "status": output.status.code(), - "stdout": stdout, - "stderr": stderr, - "stdout_truncated": stdout_truncated, - "stderr_truncated": stderr_truncated, - "max_tool_output_bytes": req.max_tool_output_bytes - }) - .to_string(), - truncated: stdout_truncated || stderr_truncated, - bytes: Some((output.stdout.len() + output.stderr.len()) as u64), - exit_code: output.status.code(), - stderr_truncated: Some(stderr_truncated), - stdout_truncated: Some(stdout_truncated), - execution_target: ExecTargetKind::Host, - docker: None, - } - } + match spawn_and_wait_managed(command, req.timeout_ms, None).await { + Ok(managed) => build_shell_target_result( + ExecTargetKind::Host, + None, + managed, + req.timeout_ms, + req.max_tool_output_bytes, + ), Err(e) => TargetResult::failed( ExecTargetKind::Host, format!("shell execution failed: {e}"), @@ -1005,6 +990,181 @@ fn path_is_workdir_scoped(path: &str) -> bool { }) } +/// Captured result of a managed child process, including whether the wait was +/// terminated by a timeout. On timeout, `status` is `None` and partial +/// stdout/stderr captured before termination are still returned. +struct ManagedOutput { + status: Option, + stdout: Vec, + stderr: Vec, + timed_out: bool, +} + +/// Spawn `command` with piped stdout/stderr and `kill_on_drop(true)`, drain its +/// output concurrently, and wait for it to exit. +/// +/// When `timeout_ms == 0` the wait is unbounded (historical behavior). When +/// `timeout_ms > 0` and the deadline elapses, the direct child is killed and +/// reaped, and `ManagedOutput::timed_out` is set. +/// +/// Limitation: this kills and reaps the *direct* child only. A child launched +/// through a shell wrapper (e.g. `sh -c "..."` or `cmd /C "..."`) may leave +/// grandchild processes running until they exit on their own. Robust +/// process-tree termination (unix process groups / Windows job objects) is a +/// follow-up. +async fn spawn_and_wait_managed( + mut command: Command, + timeout_ms: u64, + stdin_bytes: Option<&[u8]>, +) -> std::io::Result { + use std::sync::{Arc, Mutex}; + use tokio::io::AsyncReadExt; + + command + .stdin(if stdin_bytes.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + + let mut child = command.spawn()?; + + if let Some(data) = stdin_bytes { + if let Some(mut stdin) = child.stdin.take() { + stdin.write_all(data).await?; + // `stdin` is dropped here, closing the pipe so the child sees EOF. + } + } + + // Drain stdout/stderr into shared buffers via incremental reads. Shared + // buffers (rather than `read_to_end` returning the bytes) let the timeout + // path recover whatever was captured so far even when the drain task is + // still blocked on a pipe held open by a surviving grandchild process. + fn drain(pipe: Option) -> (Arc>>, tokio::task::JoinHandle<()>) + where + R: tokio::io::AsyncRead + Unpin + Send + 'static, + { + let buf = Arc::new(Mutex::new(Vec::new())); + let sink = buf.clone(); + let handle = tokio::spawn(async move { + if let Some(mut p) = pipe { + let mut chunk = [0u8; 8192]; + loop { + match p.read(&mut chunk).await { + Ok(0) | Err(_) => break, + Ok(n) => { + if let Ok(mut guard) = sink.lock() { + guard.extend_from_slice(&chunk[..n]); + } + } + } + } + } + }); + (buf, handle) + } + + let (stdout_buf, stdout_task) = drain(child.stdout.take()); + let (stderr_buf, stderr_task) = drain(child.stderr.take()); + let stdout_abort = stdout_task.abort_handle(); + let stderr_abort = stderr_task.abort_handle(); + + let (status, timed_out) = if timeout_ms == 0 { + (Some(child.wait().await?), false) + } else { + match tokio::time::timeout(std::time::Duration::from_millis(timeout_ms), child.wait()).await + { + Ok(res) => (Some(res?), false), + Err(_elapsed) => { + // Best-effort terminate the direct child, then reap it. + let _ = child.start_kill(); + let _ = child.wait().await; + (None, true) + } + } + }; + + if timed_out { + // The direct child is dead, but a grandchild may still hold the pipes + // open. Give the drain tasks a short grace period to flush, then abort + // so a surviving grandchild can never block our return. + let grace = std::time::Duration::from_millis(200); + let _ = tokio::time::timeout(grace, async { + let _ = stdout_task.await; + let _ = stderr_task.await; + }) + .await; + stdout_abort.abort(); + stderr_abort.abort(); + } else { + // Normal exit: the pipes reach EOF, so the drain tasks complete. + let _ = stdout_task.await; + let _ = stderr_task.await; + } + + let stdout = stdout_buf.lock().map(|g| g.clone()).unwrap_or_default(); + let stderr = stderr_buf.lock().map(|g| g.clone()).unwrap_or_default(); + + Ok(ManagedOutput { + status, + stdout, + stderr, + timed_out, + }) +} + +/// Build the standard shell `TargetResult` JSON envelope, adding timeout +/// metadata and an actionable recovery hint when the command was terminated by +/// a timeout. +fn build_shell_target_result( + kind: ExecTargetKind, + docker: Option, + managed: ManagedOutput, + timeout_ms: u64, + max_tool_output_bytes: usize, +) -> TargetResult { + let stdout_raw = String::from_utf8_lossy(&managed.stdout).to_string(); + let stderr_raw = String::from_utf8_lossy(&managed.stderr).to_string(); + let (stdout, stdout_truncated) = truncate_utf8_to_bytes(&stdout_raw, max_tool_output_bytes); + let (stderr, stderr_truncated) = truncate_utf8_to_bytes(&stderr_raw, max_tool_output_bytes); + let status_code = managed.status.and_then(|s| s.code()); + let ok = !managed.timed_out && managed.status.map(|s| s.success()).unwrap_or(false); + let mut content = json!({ + "status": status_code, + "stdout": stdout, + "stderr": stderr, + "stdout_truncated": stdout_truncated, + "stderr_truncated": stderr_truncated, + "max_tool_output_bytes": max_tool_output_bytes + }); + if managed.timed_out { + if let Some(obj) = content.as_object_mut() { + obj.insert("timed_out".to_string(), json!(true)); + obj.insert("timeout_ms".to_string(), json!(timeout_ms)); + obj.insert( + "hint".to_string(), + json!(format!( + "Shell command exceeded the {timeout_ms} ms timeout and was terminated before it finished. Re-run with a larger timeout_ms, make the command non-interactive, or narrow its scope." + )), + ); + } + } + TargetResult { + ok, + content: content.to_string(), + truncated: stdout_truncated || stderr_truncated, + bytes: Some((managed.stdout.len() + managed.stderr.len()) as u64), + exit_code: status_code, + stderr_truncated: Some(stderr_truncated), + stdout_truncated: Some(stdout_truncated), + execution_target: kind, + docker, + } +} + fn truncate_utf8_to_bytes(input: &str, max_bytes: usize) -> (String, bool) { if max_bytes == 0 { return (input.to_string(), false); @@ -1070,12 +1230,98 @@ mod tests { args: vec!["hi".to_string()], cwd: Some("../".to_string()), max_tool_output_bytes: 200_000, + timeout_ms: 0, }) .await; assert!(!out.ok); assert!(out.content.contains("must stay within workdir")); } + fn fast_echo_shell_req() -> ShellReq { + if cfg!(windows) { + ShellReq { + workdir: PathBuf::from("."), + cmd: "cmd".to_string(), + args: vec!["/C".to_string(), "echo ok".to_string()], + cwd: None, + max_tool_output_bytes: 200_000, + timeout_ms: 0, + } + } else { + ShellReq { + workdir: PathBuf::from("."), + cmd: "sh".to_string(), + args: vec!["-c".to_string(), "echo ok".to_string()], + cwd: None, + max_tool_output_bytes: 200_000, + timeout_ms: 0, + } + } + } + + fn slow_sleep_shell_req(timeout_ms: u64) -> ShellReq { + if cfg!(windows) { + ShellReq { + workdir: PathBuf::from("."), + cmd: "cmd".to_string(), + args: vec!["/C".to_string(), "ping -n 6 127.0.0.1 >NUL".to_string()], + cwd: None, + max_tool_output_bytes: 200_000, + timeout_ms, + } + } else { + ShellReq { + workdir: PathBuf::from("."), + cmd: "sh".to_string(), + args: vec!["-c".to_string(), "sleep 5".to_string()], + cwd: None, + max_tool_output_bytes: 200_000, + timeout_ms, + } + } + } + + #[tokio::test] + async fn host_shell_under_timeout_succeeds_normally() { + let mut req = fast_echo_shell_req(); + req.timeout_ms = 5_000; + let out = HostTarget.exec_shell(req).await; + assert!(out.ok, "fast command should succeed: {}", out.content); + assert!(out.content.contains("ok")); + assert!(!out.content.contains("timed_out")); + } + + #[tokio::test] + async fn host_shell_zero_timeout_is_unbounded_and_back_compatible() { + // timeout_ms == 0 preserves historical unbounded behavior; a fast + // command still completes normally. + let out = HostTarget.exec_shell(fast_echo_shell_req()).await; + assert!(out.ok, "command should succeed: {}", out.content); + assert!(out.content.contains("ok")); + assert!(!out.content.contains("timed_out")); + } + + #[tokio::test] + async fn host_shell_exceeding_timeout_returns_timeout_failure() { + let start = std::time::Instant::now(); + let out = HostTarget.exec_shell(slow_sleep_shell_req(200)).await; + let elapsed = start.elapsed(); + assert!( + elapsed < std::time::Duration::from_secs(2), + "timed-out command should return promptly, took {elapsed:?}" + ); + assert!(!out.ok, "timed-out command must not report success"); + let parsed: serde_json::Value = + serde_json::from_str(&out.content).expect("timeout content is JSON"); + assert_eq!(parsed["timed_out"], serde_json::json!(true)); + assert_eq!(parsed["timeout_ms"], serde_json::json!(200)); + let hint = parsed["hint"].as_str().unwrap_or_default(); + assert!( + hint.contains("timeout_ms"), + "timeout result should include an actionable hint, got: {hint}" + ); + } + #[test] fn docker_command_assembly_is_deterministic() { let t = DockerTarget::new( diff --git a/src/tools.rs b/src/tools.rs index 9a7902a..10577f3 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -127,6 +127,7 @@ pub enum ToolErrorCode { ShellExecNotFound, ShellExecOsError, ShellExecNonZeroExit, + ShellExecTimeout, } impl ToolErrorCode { @@ -145,6 +146,7 @@ impl ToolErrorCode { Self::ShellExecNotFound => "shell_exec_not_found", Self::ShellExecOsError => "shell_exec_os_error", Self::ShellExecNonZeroExit => "shell_exec_non_zero_exit", + Self::ShellExecTimeout => "shell_exec_timeout", } } } diff --git a/src/tools/catalog.rs b/src/tools/catalog.rs index 79cd99c..d69f695 100644 --- a/src/tools/catalog.rs +++ b/src/tools/catalog.rs @@ -68,13 +68,14 @@ pub fn builtin_tools_enabled(enable_write_tools: bool, enable_shell_tool: bool) if enable_shell_tool { tools.push(ToolDef { name: "shell".to_string(), - description: "Run a shell command with optional args and cwd.".to_string(), + description: "Run a shell command with optional args, cwd, and timeout_ms. timeout_ms bounds the command's wall-clock runtime in milliseconds; omit or use 0 for no timeout.".to_string(), parameters: json!({ "type":"object", "properties":{ "cmd":{"type":"string"}, "args":{"type":"array","items":{"type":"string"}}, - "cwd":{"type":"string"} + "cwd":{"type":"string"}, + "timeout_ms":{"type":"integer","minimum":0} }, "required":["cmd"] }), diff --git a/src/tools/exec_shell.rs b/src/tools/exec_shell.rs index d5850fd..4347a23 100644 --- a/src/tools/exec_shell.rs +++ b/src/tools/exec_shell.rs @@ -39,12 +39,16 @@ pub(super) async fn run_shell(rt: &ToolRuntime, args: &Value) -> ToolExecution { .get("cwd") .and_then(|v| v.as_str()) .map(ToString::to_string); + // `timeout_ms` is optional; absent or 0 preserves unbounded behavior. The + // `as_u64` parse makes negative values unrepresentable. + let timeout_ms = args.get("timeout_ms").and_then(|v| v.as_u64()).unwrap_or(0); let req = ShellReq { workdir: rt.workdir.clone(), cmd: cmd.to_string(), args: arg_list.clone(), cwd: cwd.clone(), max_tool_output_bytes: rt.max_tool_output_bytes, + timeout_ms, }; let mut out = rt.exec_target.exec_shell(req).await; if !out.ok && shell_spawn_not_found(&out.content) { @@ -59,6 +63,7 @@ pub(super) async fn run_shell(rt: &ToolRuntime, args: &Value) -> ToolExecution { args: repair_args, cwd, max_tool_output_bytes: rt.max_tool_output_bytes, + timeout_ms, }) .await; out = annotate_shell_repair(repaired, repair_strategy); @@ -71,6 +76,9 @@ pub(super) fn classify_shell_target_error( content: &str, exit_code: Option, ) -> ToolErrorDetail { + if let Some(detail) = timeout_error_from_content(content) { + return detail; + } let lower = content.to_ascii_lowercase(); let spawn_failed = lower.contains("shell execution failed:"); let not_found = lower.contains("program not found") @@ -109,6 +117,25 @@ pub(super) fn classify_shell_target_error( } } +fn timeout_error_from_content(content: &str) -> Option { + let parsed = serde_json::from_str::(content).ok()?; + if parsed.get("timed_out").and_then(|v| v.as_bool()) != Some(true) { + return None; + } + let message = match parsed.get("timeout_ms").and_then(|v| v.as_u64()) { + Some(ms) => format!("Shell command exceeded the {ms} ms timeout and was terminated."), + None => "Shell command exceeded its timeout and was terminated.".to_string(), + }; + Some(ToolErrorDetail { + code: ToolErrorCode::ShellExecTimeout, + message, + expected_schema: None, + received_args: None, + minimal_example: minimal_builtin_example("shell"), + available_tools: None, + }) +} + fn shell_status_code_from_content(content: &str) -> Option { let parsed = serde_json::from_str::(content).ok()?; parsed diff --git a/src/tools/schema.rs b/src/tools/schema.rs index 151fbac..5b78eba 100644 --- a/src/tools/schema.rs +++ b/src/tools/schema.rs @@ -34,7 +34,8 @@ pub fn compact_builtin_schema(tool_name: &str) -> Option { "properties":{ "cmd":{"type":"string"}, "args":{"type":"array","items":{"type":"string"}}, - "cwd":{"type":"string"} + "cwd":{"type":"string"}, + "timeout_ms":{"type":"integer","minimum":0} } })), "write_file" => Some(json!({ @@ -186,6 +187,11 @@ pub fn validate_builtin_tool_args( return Err("cwd must be a string".to_string()); } } + if let Some(v) = obj.get("timeout_ms") { + if v.as_u64().is_none() { + return Err("timeout_ms must be a non-negative integer".to_string()); + } + } } "write_file" => { require_non_empty_string(obj, "path")?; From 64e8238665c846a29b169cce77c41644044b40fd Mon Sep 17 00:00:00 2001 From: calvinsturm Date: Tue, 30 Jun 2026 15:47:10 -0700 Subject: [PATCH 2/2] Stabilize shell lifecycle validation Make the shell auto-repair test deterministic and give the docker target a clear answer for timeout requests. - tools/tests: the windows builtin auto-repair test used `echo`, which is frequently shadowed by an MSYS/Git `echo.exe` on PATH; the direct spawn then succeeds and the repair path is never exercised (the test failed on this machine and on base main). Switch to `ver`, a cmd builtin with no standalone executable counterpart, so the initial spawn deterministically fails and forces windows_cmd_c repair regardless of host PATH. Assertion still proves repair_attempted + windows_cmd_c strategy and real command output. - target: DockerTarget::exec_shell now rejects timeout_ms > 0 with a structured, classifiable error (error=timeout_unsupported) instead of silently ignoring a requested bound. Returns before any docker invocation, so the new test is deterministic without a docker daemon. Shared run_container read/write/list/ patch behavior is untouched. - New ShellExecTimeoutUnsupported tool error code; classify_shell_target_error maps the docker rejection envelope to it with an actionable message. --- src/target.rs | 52 +++++++++++++++++++++++++++++++++++++++++ src/tools.rs | 2 ++ src/tools/exec_shell.rs | 24 +++++++++++++++++++ src/tools/tests.rs | 13 +++++++++-- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/target.rs b/src/target.rs index 6baa6ba..4e63df5 100644 --- a/src/target.rs +++ b/src/target.rs @@ -608,6 +608,30 @@ impl ExecTarget for DockerTarget { } async fn exec_shell(&self, req: ShellReq) -> TargetResult { + // Timeout enforcement is not implemented for the docker target (killing + // the `docker run` client would not reliably stop the container). Rather + // than silently ignore a requested bound, reject it with a structured, + // classifiable error so the model/operator gets a clear signal instead + // of an unexpectedly unbounded command. + if req.timeout_ms > 0 { + return TargetResult { + ok: false, + content: json!({ + "error": "timeout_unsupported", + "execution_target": "docker", + "timeout_ms": req.timeout_ms, + "hint": "timeout_ms is not supported on the docker execution target. Re-run on the host target, or omit timeout_ms." + }) + .to_string(), + truncated: false, + bytes: None, + exit_code: None, + stderr_truncated: None, + stdout_truncated: None, + execution_target: ExecTargetKind::Docker, + docker: Some(self.meta.clone()), + }; + } let args = req .args .iter() @@ -1355,6 +1379,34 @@ mod tests { ); } + #[tokio::test] + async fn docker_shell_rejects_timeout_with_structured_error() { + // The guard returns before any docker invocation, so this is + // deterministic without docker installed. + let t = DockerTarget::new( + "ubuntu:24.04".to_string(), + "/work".to_string(), + "none".to_string(), + None, + ); + let out = t + .exec_shell(ShellReq { + workdir: PathBuf::from("."), + cmd: "echo".to_string(), + args: vec!["hi".to_string()], + cwd: None, + max_tool_output_bytes: 200_000, + timeout_ms: 500, + }) + .await; + assert!(!out.ok, "timeout on docker target must be rejected"); + let parsed: serde_json::Value = + serde_json::from_str(&out.content).expect("structured docker rejection is JSON"); + assert_eq!(parsed["error"], serde_json::json!("timeout_unsupported")); + assert_eq!(parsed["execution_target"], serde_json::json!("docker")); + assert_eq!(parsed["timeout_ms"], serde_json::json!(500)); + } + #[test] fn docker_mount_rejects_root_paths() { let t = DockerTarget::new( diff --git a/src/tools.rs b/src/tools.rs index 10577f3..007694d 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -128,6 +128,7 @@ pub enum ToolErrorCode { ShellExecOsError, ShellExecNonZeroExit, ShellExecTimeout, + ShellExecTimeoutUnsupported, } impl ToolErrorCode { @@ -147,6 +148,7 @@ impl ToolErrorCode { Self::ShellExecOsError => "shell_exec_os_error", Self::ShellExecNonZeroExit => "shell_exec_non_zero_exit", Self::ShellExecTimeout => "shell_exec_timeout", + Self::ShellExecTimeoutUnsupported => "shell_exec_timeout_unsupported", } } } diff --git a/src/tools/exec_shell.rs b/src/tools/exec_shell.rs index 4347a23..0477095 100644 --- a/src/tools/exec_shell.rs +++ b/src/tools/exec_shell.rs @@ -76,6 +76,9 @@ pub(super) fn classify_shell_target_error( content: &str, exit_code: Option, ) -> ToolErrorDetail { + if let Some(detail) = timeout_unsupported_error_from_content(content) { + return detail; + } if let Some(detail) = timeout_error_from_content(content) { return detail; } @@ -117,6 +120,27 @@ pub(super) fn classify_shell_target_error( } } +fn timeout_unsupported_error_from_content(content: &str) -> Option { + let parsed = serde_json::from_str::(content).ok()?; + if parsed.get("error").and_then(|v| v.as_str()) != Some("timeout_unsupported") { + return None; + } + let target = parsed + .get("execution_target") + .and_then(|v| v.as_str()) + .unwrap_or("this"); + Some(ToolErrorDetail { + code: ToolErrorCode::ShellExecTimeoutUnsupported, + message: format!( + "timeout_ms is not supported on the {target} execution target. Re-run on the host target or omit timeout_ms." + ), + expected_schema: None, + received_args: None, + minimal_example: minimal_builtin_example("shell"), + available_tools: None, + }) +} + fn timeout_error_from_content(content: &str) -> Option { let parsed = serde_json::from_str::(content).ok()?; if parsed.get("timed_out").and_then(|v| v.as_bool()) != Some(true) { diff --git a/src/tools/tests.rs b/src/tools/tests.rs index e3bd42a..b94a01c 100644 --- a/src/tools/tests.rs +++ b/src/tools/tests.rs @@ -771,10 +771,15 @@ async fn shell_auto_repair_wraps_windows_builtin() { exec_target_kind: ExecTargetKind::Host, exec_target: std::sync::Arc::new(HostTarget), }; + // Use `ver` rather than `echo`: both are cmd builtins, but `echo` is often + // shadowed by an MSYS/Git `echo.exe` on PATH, which lets the direct spawn + // succeed and bypasses the repair path. `ver` has no standalone executable + // counterpart, so the initial spawn deterministically fails regardless of + // host PATH contents, forcing the windows_cmd_c auto-repair. let tc = ToolCall { id: "tc_shell_auto_repair_win".to_string(), name: "shell".to_string(), - arguments: json!({"cmd":"echo","args":["hi-manual-test"]}), + arguments: json!({"cmd":"ver"}), }; let msg = execute_tool(&rt, &tc).await; let envelope: Value = serde_json::from_str(&msg.content.expect("content")).expect("json"); @@ -788,7 +793,11 @@ async fn shell_auto_repair_wraps_windows_builtin() { .get("stdout") .and_then(|v| v.as_str()) .unwrap_or_default(); - assert!(stdout.contains("hi-manual-test")); + // `cmd /c ver` prints a line like "Microsoft Windows [Version 10.0.x]". + assert!( + stdout.to_ascii_lowercase().contains("windows"), + "expected `ver` output to mention Windows, got: {stdout}" + ); assert_eq!( inner.get("repair_attempted").and_then(|v| v.as_bool()), Some(true)