From 28318291d714ba66ece6dfd7de21833a04b7efcf Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Fri, 9 Oct 2026 12:17:24 +0200 Subject: [PATCH 1/3] fix(bash): kill the command's whole process group on timeout, cancel and drop On Unix, bash leads its own process group (process_group(0)); a GroupKill guard, declared after the child so it fires before the child is reaped, killpg's the group on timeout, cancel or drop and is disarmed when the command finishes on its own. Pipeline stages, && lists and background jobs no longer outlive a cancelled call; a setsid process still escapes. Windows unchanged. libc is a cfg(unix) dependency (already in the tree). Closes #277. Suggested by @shahidcodes. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- CHANGELOG.md | 4 ++ CLAUDE.md | 2 +- Cargo.toml | 5 ++ docs/concepts/tools.md | 2 +- src/tools/bash.rs | 57 +++++++++++++++-- tests/tools_test.rs | 137 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 200 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9314957..9323b2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ adheres to [Semantic Versioning](https://semver.org/). ## Unreleased +### Fixed + +- **`BashTool` kills what a command started** (#277, suggested by @shahidcodes). On Unix, `bash` now leads its own process group, and a timeout, a cancel or dropping the call kills the whole group: pipeline stages, `&&` lists and background jobs no longer keep running. Only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves its background jobs alone. Windows is unchanged (the `bash` process only). Adds `libc` as a Unix-only dependency (already in the tree through tokio). Tests: timeout, cancel, drop and normal completion in `tests/tools_test.rs`, mutation-checked. + ### Tests - **HTTP MCP on wasm32.** `tests/wasm32.rs` runs an agent that connects to an HTTP MCP server, discovers its tool and calls it from the loop, through the host's `fetch` (a scripted server replaces the global `fetch`): handshake, `Mcp-Session-Id` replay and the tool result. The Workers guide notes that a stalled MCP stream has no idle read timeout of its own on wasm32, and that custom headers are not supported yet (#275). diff --git a/CLAUDE.md b/CLAUDE.md index c0a30c2..e87df7d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -240,7 +240,7 @@ Tests that mutate the global layers or assert on logs live in their own serializ - Sandboxed tools do their I/O on `PathSandbox::io_path` (the checked path); unsandboxed tools use the caller's spelling. - `search` passes `--regexp=` / `-e`, then `--`, then the path. It has no per-file `--max-count`; it reads lines until `max_results` and kills the search at the next one (`details.truncated`). An exit 2 (some file errored) with matches returns the matches plus stderr under `Warnings:` (`details.warnings` = that text, capped, or null; kept on the early-stop path too); only an error with no matches fails. - `list_files` guards `find` with `not_an_option`. `find`'s stderr is read: errors (an unreadable subdirectory) are appended to the listing under `Warnings` and in `details.warnings` (capped), so a partial listing never reads as complete; only an error with no files fails (#260). - - `bash` spawns with `kill_on_drop`, which kills the `bash` process only, and with `stdin(Stdio::null())` (`spawn` inherits stdin, while `output()` did not; `search` sets it too). Each stream goes through `Capture` (bytes kept ≤ `max_output_bytes`, the rest drained; a cut flag and a read error survive a timeout), and output is decoded after cutting. + - `bash` spawns with `kill_on_drop` and, on Unix, `process_group(0)`: a `GroupKill` guard (declared after the child, so it fires before the child is reaped and the pgid cannot be reused) `killpg`s the whole group on timeout, cancel or drop, and is disarmed when the command finishes on its own (its detached background jobs survive); `libc` is a `cfg(unix)` dependency, the crate's only `unsafe`. Also `stdin(Stdio::null())` (`spawn` inherits stdin, while `output()` did not; `search` sets it too). Each stream goes through `Capture` (bytes kept ≤ `max_output_bytes`, the rest drained; a cut flag and a read error survive a timeout), and output is decoded after cutting. - Tool `execute` is built and awaited inside `catch_unwind` in `execute_single_tool`, the same as middleware. A panic is an error result carrying the payload text, logged at `error!` with the `tool_call_id`. - The Gemini key goes in `x-goog-api-key`, trimmed, never the URL. It is skipped when the user's headers set `x-goog-api-key`, or when the key is empty. An `Authorization` header doesn't count. - Retry logic (`retry.rs`) uses exponential backoff with ±20% jitter; only retries `RateLimited` and `Network` errors (`classify` maps 429, 503 and 529 to `RateLimited` and `warn!`s the body, since `RateLimited` carries no message; `RATE_LIMIT_CODES` includes Anthropic's `overloaded_error` and Google's `unavailable`, and an in-stream numeric `code` of 503/529 counts — 429 / `RESOURCE_EXHAUSTED` in-stream stays `Api` to keep its message; other 5xx stay `Api`). In `stream_assistant_response` each attempt's forwarder is **drained, never aborted** (what consumers see must not depend on the scheduler), an attempt that sent `MessageStart` without `MessageEnd` is closed with `StopReason::Error` (`AttemptEvents::needs_end`), and a retried attempt is followed by `AgentEvent::ProviderRetry` — consumers that treat an error `MessageEnd` as failure look at the next event first (GASP holds it back; `release_smoke` / `long_horizon` clear it; `retry::RetrySafeEvents` / `retry_safe_events(rx)` is the consumer-side filter for append-only sinks: a private `Held` enum (Nothing / Open / Failed) holds an assistant attempt until its `MessageEnd`, drops a retried one down to its `ProviderRetry`, releases a final failure as start + `Error`/`Aborted` end without deltas, and drops (never releases, `warn!`s) an attempt left open by a retry, a second start or the stream's end; other events overtake a held attempt; a sub-agent's `ToolExecutionUpdate` text is not covered; the documented terminal pattern (abort on a `ProviderRetry` after streamed text) is tested in `tests/retry_safe_test.rs`). An `Ok` attempt that left its message open (a provider that never sent `Done`) is closed with the returned message; a failed forwarder `JoinError` is `warn!`ed. The backoff sleep races `cancel`; a cancellation ends the turn as `StopReason::Aborted` (not `Error`, so `on_error` is not called). A cancel caught at the top of a turn (while tools ran, or between turns) appends `CANCELLED_MARKER` (`[Agent stopped: cancelled]`, via `push_stop_marker`) when the run produced an assistant message; `sub_agent::extract_error` treats it, like a loop abort, as a failure (a limit marker is still partial success). `Batched { size: 0 }` runs as size 1. Providers must drop every `tx` clone before `stream` returns, or the drain waits diff --git a/Cargo.toml b/Cargo.toml index 27880f7..af24e98 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -190,6 +190,11 @@ opt-level = 1 # wasm32 (e.g. Cloudflare Workers): randomness from the JS host, and a # JS-backed executor/timer in place of a Tokio runtime. +# `BashTool` kills a command's whole process group on timeout or cancel +# (`killpg`). Already in the tree through tokio. +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom_04 = { package = "getrandom", version = "0.4", features = ["wasm_js"] } getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] } diff --git a/docs/concepts/tools.md b/docs/concepts/tools.md index b0a9ac3..6bfa245 100644 --- a/docs/concepts/tools.md +++ b/docs/concepts/tools.md @@ -196,7 +196,7 @@ async fn execute(&self, params: serde_json::Value, _ctx: ToolContext) -> Result< } ``` -**Exception: BashTool.** The built-in `BashTool` returns `Ok` even on non-zero exit codes, with both stdout and stderr in the result. This is intentional — the LLM needs to see the actual error output (compilation errors, test failures, etc.) to diagnose and fix issues. Only failures outside the command return `Err`: a matched deny pattern, a refused confirmation, a timeout (its message carries the output printed so far), cancellation, or `bash` failing to start. A command that does not exist is an ordinary non-zero exit (127). A timeout or cancel kills the `bash` process; what it started (pipeline stages, commands in a `&&` list, background jobs) can keep running. +**Exception: BashTool.** The built-in `BashTool` returns `Ok` even on non-zero exit codes, with both stdout and stderr in the result. This is intentional — the LLM needs to see the actual error output (compilation errors, test failures, etc.) to diagnose and fix issues. Only failures outside the command return `Err`: a matched deny pattern, a refused confirmation, a timeout (its message carries the output printed so far), cancellation, or `bash` failing to start. A command that does not exist is an ordinary non-zero exit (127). On Unix, a timeout, a cancel or dropping the call kills the command's whole process group — pipeline stages, commands in a `&&` list and background jobs included; only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves what it started in the background alone (a server it launched, say). On Windows only the `bash` process is killed. **A panicking tool** is contained by the loop, the same as a panicking middleware. The call ends as an error result (`tool '' panicked: …`) that the model sees, and the run continues. diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 757eff8..9bd6b3b 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -187,12 +187,15 @@ impl AgentTool for BashTool { // Capture output cmd.stdout(std::process::Stdio::piped()); cmd.stderr(std::process::Stdio::piped()); - // A timeout or cancel must not leave the command running. This kills - // the `bash` process only: what it started — pipeline stages, - // commands in a `;` / `&&` list, background jobs — is not in that - // kill and can keep running. Run the agent in a container if that - // matters. + // A timeout or cancel must not leave the command running. On Unix + // `bash` leads its own process group and `GroupKill` kills the whole + // group: non-interactive bash has no job control, so pipeline stages, + // `;` / `&&` lists and background jobs stay in it. Only a process that + // starts its own session (`setsid`) escapes. Elsewhere this kills the + // `bash` process only. cmd.kill_on_drop(true); + #[cfg(unix)] + cmd.process_group(0); let timeout = self.timeout; let max_bytes = self.max_output_bytes; @@ -203,6 +206,9 @@ impl AgentTool for BashTool { let mut child = cmd .spawn() .map_err(|e| ToolError::Failed(format!("Failed to execute: {}", e)))?; + // Declared after `child`, so it is dropped first: the group is killed + // while `bash` is not yet reaped, and its id cannot have been reused. + let mut group = GroupKill::new(child.id()); let (Some(child_out), Some(child_err)) = (child.stdout.take(), child.stderr.take()) else { return Err(ToolError::Failed("Failed to capture output".into())); }; @@ -236,6 +242,9 @@ impl AgentTool for BashTool { ))); } Some(Ok(status)) => { + // Finished on its own: what it left running in the background + // (a server it started, say) is the command's business. + group.disarm(); status.map_err(|e| ToolError::Failed(format!("Failed to execute: {}", e)))? } }; @@ -250,6 +259,44 @@ impl AgentTool for BashTool { } } +/// Kills the command's process group when dropped — on a timeout, a cancel, +/// or the tool's future being dropped — unless the command finished first. +struct GroupKill { + #[cfg(unix)] + pgid: Option, +} + +impl GroupKill { + fn new(pid: Option) -> Self { + #[cfg(not(unix))] + let _ = pid; + Self { + #[cfg(unix)] + pgid: pid.and_then(|p| libc::pid_t::try_from(p).ok()), + } + } + + fn disarm(&mut self) { + #[cfg(unix)] + { + self.pgid = None; + } + } +} + +impl Drop for GroupKill { + fn drop(&mut self) { + #[cfg(unix)] + if let Some(pgid) = self.pgid { + // SAFETY: `killpg` takes two integers and touches no memory of + // ours; a failure (the group already gone) is fine to ignore. + unsafe { + libc::killpg(pgid, libc::SIGKILL); + } + } + } +} + /// One output stream: the bytes kept, whether more were discarded, and a /// read error if the stream ended on one. #[derive(Default)] diff --git a/tests/tools_test.rs b/tests/tools_test.rs index dd3918e..d0694eb 100644 --- a/tests/tools_test.rs +++ b/tests/tools_test.rs @@ -1109,3 +1109,140 @@ async fn bash_env_allowlist_hides_other_variables() { std::env::remove_var("YOAGENT_TEST_KEEP"); } } + +// --------------------------------------------------------------------------- +// BashTool: what a command started goes with it (#277) +// --------------------------------------------------------------------------- + +/// Whether `pid` still exists (`kill -0`). +#[cfg(unix)] +fn alive(pid: &str) -> bool { + std::process::Command::new("kill") + .args(["-0", pid]) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()) +} + +/// Waits until the command has written its background job's pid. +#[cfg(unix)] +async fn background_pid(file: &std::path::Path) -> String { + for _ in 0..200 { + if let Ok(pid) = std::fs::read_to_string(file) { + if !pid.trim().is_empty() { + return pid.trim().to_owned(); + } + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + panic!("the command never wrote its background pid"); +} + +/// A SIGKILL is delivered asynchronously and the orphan is reaped by init. +#[cfg(unix)] +async fn assert_gone(pid: &str) { + for _ in 0..200 { + if !alive(pid) { + return; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + panic!("background job {pid} outlived the command"); +} + +#[cfg(unix)] +fn backgrounding_command(pid_file: &std::path::Path) -> serde_json::Value { + serde_json::json!({ + "command": format!("sleep 30 & echo $! > {}; sleep 30 | cat", pid_file.display()) + }) +} + +#[cfg(unix)] +#[tokio::test] +async fn bash_timeout_kills_background_jobs_and_pipeline_stages() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("pid"); + // Long enough that `bash` is up and has written the pid even when other + // tests load the machine; the pid is read before the timeout fires. + let run = tokio::spawn({ + let args = backgrounding_command(&pid_file); + async move { + BashTool::new() + .with_timeout(std::time::Duration::from_secs(3)) + .execute(args, ctx("bash")) + .await + } + }); + let pid = background_pid(&pid_file).await; + assert!(alive(&pid)); + assert!(run + .await + .unwrap() + .unwrap_err() + .to_string() + .contains("timed out")); + assert_gone(&pid).await; +} + +#[cfg(unix)] +#[tokio::test] +async fn bash_cancel_kills_background_jobs() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("pid"); + let cancel = CancellationToken::new(); + let run = tokio::spawn({ + let (cancel, args) = (cancel.clone(), backgrounding_command(&pid_file)); + async move { + BashTool::new() + .execute(args, ctx_with_cancel("bash", cancel)) + .await + } + }); + let pid = background_pid(&pid_file).await; + assert!(alive(&pid)); + cancel.cancel(); + assert!(matches!(run.await.unwrap(), Err(ToolError::Cancelled))); + assert_gone(&pid).await; +} + +#[cfg(unix)] +#[tokio::test] +async fn dropping_the_bash_call_kills_background_jobs() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("pid"); + let run = tokio::spawn({ + let args = backgrounding_command(&pid_file); + async move { BashTool::new().execute(args, ctx("bash")).await } + }); + let pid = background_pid(&pid_file).await; + assert!(alive(&pid)); + // The caller gives up: the call's future is dropped mid-run. + run.abort(); + assert!(run.await.unwrap_err().is_cancelled()); + assert_gone(&pid).await; +} + +/// A command that finishes on its own may leave a job running on purpose +/// (a server it started): only a timeout, cancel or drop kills the group. +#[cfg(unix)] +#[tokio::test] +async fn a_finished_command_leaves_its_detached_background_job_alone() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("pid"); + let result = BashTool::new() + .execute( + serde_json::json!({ + "command": format!("sleep 30 >/dev/null 2>&1 & echo $! > {}", pid_file.display()) + }), + ctx("bash"), + ) + .await + .unwrap(); + assert_eq!(result.details["exit_code"], 0); + let pid = background_pid(&pid_file).await; + assert!(alive(&pid), "the job outlives a command that finished"); + std::process::Command::new("kill") + .arg(&pid) + .status() + .unwrap(); +} From c198998b4707baaf8bbc81aa9558a2d99ba3f512 Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Fri, 9 Oct 2026 12:21:47 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(bash):=20review=20fixes=20=E2=80=94=20s?= =?UTF-8?q?td=20process=5Fgroup,=20Ctrl+C=20in=20the=20CLI,=20precise=20do?= =?UTF-8?q?cs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - std's CommandExt::process_group (Rust 1.64) instead of tokio's, which needs tokio 1.40 while the crate asks for tokio = "1". - The command is outside the terminal's foreground group, so the terminal's Ctrl+C no longer reaches it: examples/cli.rs now aborts the run on Ctrl+C during a run (exits only at the prompt); documented, with /dev/tty prompts. - Docs: a background job survives a finished command only if its output is redirected; one holding the pipes keeps the call open until the timeout. - Cargo.toml: the unix block no longer splits the wasm32 comment. - Tests: a zombie counts as gone (no init in a container). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- CHANGELOG.md | 2 +- CLAUDE.md | 2 +- Cargo.toml | 4 ++-- docs/concepts/tools.md | 2 +- examples/cli.rs | 37 +++++++++++++++++++++++++++++++------ src/tools/bash.rs | 9 ++++++--- tests/tools_test.rs | 17 ++++++++++++++--- 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9323b2a..0b84770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ adheres to [Semantic Versioning](https://semver.org/). ### Fixed -- **`BashTool` kills what a command started** (#277, suggested by @shahidcodes). On Unix, `bash` now leads its own process group, and a timeout, a cancel or dropping the call kills the whole group: pipeline stages, `&&` lists and background jobs no longer keep running. Only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves its background jobs alone. Windows is unchanged (the `bash` process only). Adds `libc` as a Unix-only dependency (already in the tree through tokio). Tests: timeout, cancel, drop and normal completion in `tests/tools_test.rs`, mutation-checked. +- **`BashTool` kills what a command started** (#277, suggested by @shahidcodes). On Unix, `bash` now leads its own process group, and a timeout, a cancel or dropping the call kills the whole group: pipeline stages, `&&` lists and background jobs no longer keep running. Only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves a background job alone when the job's output is redirected (`cmd >log 2>&1 &`); one still writing to the tool's output keeps the call open until the timeout, which now kills it. **Behaviour change:** the command is no longer in the terminal's foreground group, so the terminal's Ctrl+C does not reach it — cancel or drop the call on Ctrl+C (the `cli` example now aborts the run), and a command prompting on `/dev/tty` (`sudo`, SSH) stops until the timeout. Windows is unchanged (the `bash` process only). Adds `libc` as a Unix-only dependency (already in the tree through tokio). Tests: timeout, cancel, drop and normal completion in `tests/tools_test.rs`, mutation-checked. ### Tests diff --git a/CLAUDE.md b/CLAUDE.md index e87df7d..9bdf1af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -240,7 +240,7 @@ Tests that mutate the global layers or assert on logs live in their own serializ - Sandboxed tools do their I/O on `PathSandbox::io_path` (the checked path); unsandboxed tools use the caller's spelling. - `search` passes `--regexp=` / `-e`, then `--`, then the path. It has no per-file `--max-count`; it reads lines until `max_results` and kills the search at the next one (`details.truncated`). An exit 2 (some file errored) with matches returns the matches plus stderr under `Warnings:` (`details.warnings` = that text, capped, or null; kept on the early-stop path too); only an error with no matches fails. - `list_files` guards `find` with `not_an_option`. `find`'s stderr is read: errors (an unreadable subdirectory) are appended to the listing under `Warnings` and in `details.warnings` (capped), so a partial listing never reads as complete; only an error with no files fails (#260). - - `bash` spawns with `kill_on_drop` and, on Unix, `process_group(0)`: a `GroupKill` guard (declared after the child, so it fires before the child is reaped and the pgid cannot be reused) `killpg`s the whole group on timeout, cancel or drop, and is disarmed when the command finishes on its own (its detached background jobs survive); `libc` is a `cfg(unix)` dependency, the crate's only `unsafe`. Also `stdin(Stdio::null())` (`spawn` inherits stdin, while `output()` did not; `search` sets it too). Each stream goes through `Capture` (bytes kept ≤ `max_output_bytes`, the rest drained; a cut flag and a read error survive a timeout), and output is decoded after cutting. + - `bash` spawns with `kill_on_drop` and, on Unix, `process_group(0)`: a `GroupKill` guard (declared after the child, so it fires before the child is reaped and the pgid cannot be reused) `killpg`s the whole group on timeout, cancel or drop, and is disarmed when the command finishes on its own (a background job survives only if its output is redirected — one holding the pipes keeps the call open until the timeout kills it); std's `CommandExt::process_group` (tokio's needs 1.40); the command is outside the terminal's foreground group, so hosts cancel on Ctrl+C (`examples/cli.rs` aborts the run); `libc` is a `cfg(unix)` dependency, the crate's only `unsafe`. Also `stdin(Stdio::null())` (`spawn` inherits stdin, while `output()` did not; `search` sets it too). Each stream goes through `Capture` (bytes kept ≤ `max_output_bytes`, the rest drained; a cut flag and a read error survive a timeout), and output is decoded after cutting. - Tool `execute` is built and awaited inside `catch_unwind` in `execute_single_tool`, the same as middleware. A panic is an error result carrying the payload text, logged at `error!` with the `tool_call_id`. - The Gemini key goes in `x-goog-api-key`, trimmed, never the URL. It is skipped when the user's headers set `x-goog-api-key`, or when the key is empty. An `Authorization` header doesn't count. - Retry logic (`retry.rs`) uses exponential backoff with ±20% jitter; only retries `RateLimited` and `Network` errors (`classify` maps 429, 503 and 529 to `RateLimited` and `warn!`s the body, since `RateLimited` carries no message; `RATE_LIMIT_CODES` includes Anthropic's `overloaded_error` and Google's `unavailable`, and an in-stream numeric `code` of 503/529 counts — 429 / `RESOURCE_EXHAUSTED` in-stream stays `Api` to keep its message; other 5xx stay `Api`). In `stream_assistant_response` each attempt's forwarder is **drained, never aborted** (what consumers see must not depend on the scheduler), an attempt that sent `MessageStart` without `MessageEnd` is closed with `StopReason::Error` (`AttemptEvents::needs_end`), and a retried attempt is followed by `AgentEvent::ProviderRetry` — consumers that treat an error `MessageEnd` as failure look at the next event first (GASP holds it back; `release_smoke` / `long_horizon` clear it; `retry::RetrySafeEvents` / `retry_safe_events(rx)` is the consumer-side filter for append-only sinks: a private `Held` enum (Nothing / Open / Failed) holds an assistant attempt until its `MessageEnd`, drops a retried one down to its `ProviderRetry`, releases a final failure as start + `Error`/`Aborted` end without deltas, and drops (never releases, `warn!`s) an attempt left open by a retry, a second start or the stream's end; other events overtake a held attempt; a sub-agent's `ToolExecutionUpdate` text is not covered; the documented terminal pattern (abort on a `ProviderRetry` after streamed text) is tested in `tests/retry_safe_test.rs`). An `Ok` attempt that left its message open (a provider that never sent `Done`) is closed with the returned message; a failed forwarder `JoinError` is `warn!`ed. The backoff sleep races `cancel`; a cancellation ends the turn as `StopReason::Aborted` (not `Error`, so `on_error` is not called). A cancel caught at the top of a turn (while tools ran, or between turns) appends `CANCELLED_MARKER` (`[Agent stopped: cancelled]`, via `push_stop_marker`) when the run produced an assistant message; `sub_agent::extract_error` treats it, like a loop abort, as a failure (a limit marker is still partial success). `Batched { size: 0 }` runs as size 1. Providers must drop every `tx` clone before `stream` returns, or the drain waits diff --git a/Cargo.toml b/Cargo.toml index af24e98..d1bfce2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -188,13 +188,13 @@ inherits = "test" debug = false opt-level = 1 -# wasm32 (e.g. Cloudflare Workers): randomness from the JS host, and a -# JS-backed executor/timer in place of a Tokio runtime. # `BashTool` kills a command's whole process group on timeout or cancel # (`killpg`). Already in the tree through tokio. [target.'cfg(unix)'.dependencies] libc = "0.2" +# wasm32 (e.g. Cloudflare Workers): randomness from the JS host, and a +# JS-backed executor/timer in place of a Tokio runtime. [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom_04 = { package = "getrandom", version = "0.4", features = ["wasm_js"] } getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] } diff --git a/docs/concepts/tools.md b/docs/concepts/tools.md index 6bfa245..e9b781e 100644 --- a/docs/concepts/tools.md +++ b/docs/concepts/tools.md @@ -196,7 +196,7 @@ async fn execute(&self, params: serde_json::Value, _ctx: ToolContext) -> Result< } ``` -**Exception: BashTool.** The built-in `BashTool` returns `Ok` even on non-zero exit codes, with both stdout and stderr in the result. This is intentional — the LLM needs to see the actual error output (compilation errors, test failures, etc.) to diagnose and fix issues. Only failures outside the command return `Err`: a matched deny pattern, a refused confirmation, a timeout (its message carries the output printed so far), cancellation, or `bash` failing to start. A command that does not exist is an ordinary non-zero exit (127). On Unix, a timeout, a cancel or dropping the call kills the command's whole process group — pipeline stages, commands in a `&&` list and background jobs included; only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves what it started in the background alone (a server it launched, say). On Windows only the `bash` process is killed. +**Exception: BashTool.** The built-in `BashTool` returns `Ok` even on non-zero exit codes, with both stdout and stderr in the result. This is intentional — the LLM needs to see the actual error output (compilation errors, test failures, etc.) to diagnose and fix issues. Only failures outside the command return `Err`: a matched deny pattern, a refused confirmation, a timeout (its message carries the output printed so far), cancellation, or `bash` failing to start. A command that does not exist is an ordinary non-zero exit (127). On Unix, a timeout, a cancel or dropping the call kills the command's whole process group — pipeline stages, commands in a `&&` list and background jobs included; only a process that starts its own session (`setsid`) escapes. A command that finishes on its own leaves a background job alone if the job's output is redirected away from the tool (`npm run dev >dev.log 2>&1 &`); a job still writing to the tool's output keeps the call open until the timeout, which kills it. Because the command runs in its own process group, the terminal's Ctrl+C no longer reaches it: a host should cancel or drop the call on Ctrl+C (the CLI example aborts the run), and a command that prompts on `/dev/tty` (`sudo`, an SSH passphrase) stops until the timeout. On Windows only the `bash` process is killed. **A panicking tool** is contained by the loop, the same as a panicking middleware. The call ends as an error result (`tool '' panicked: …`) that the model sees, and the run continues. diff --git a/examples/cli.rs b/examples/cli.rs index 2733900..9363b14 100644 --- a/examples/cli.rs +++ b/examples/cli.rs @@ -151,11 +151,22 @@ async fn main() { .with_skills(skills.clone()) .with_tools(default_tools()); - // Graceful Ctrl+C exit - tokio::spawn(async { - tokio::signal::ctrl_c().await.ok(); - println!("\n{DIM} bye 👋{RESET}\n"); - std::process::exit(0); + // Ctrl+C at the prompt exits. During a run it stops the run instead + // (the event loop below aborts it): commands `bash` started are in their + // own process group, so the terminal's Ctrl+C does not reach them, and + // exiting here would leave them running. + let running = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + tokio::spawn({ + let running = running.clone(); + async move { + loop { + tokio::signal::ctrl_c().await.ok(); + if !running.load(std::sync::atomic::Ordering::SeqCst) { + println!("\n{DIM} bye 👋{RESET}\n"); + std::process::exit(0); + } + } + } }); print_banner(); @@ -210,8 +221,21 @@ async fn main() { let mut rx = agent.prompt(input).await; let mut session_stats = SessionStats::default(); let mut in_text = false; + running.store(true, std::sync::atomic::Ordering::SeqCst); - while let Some(event) = rx.recv().await { + loop { + let event = tokio::select! { + event = rx.recv() => match event { + Some(event) => event, + None => break, + }, + _ = tokio::signal::ctrl_c() => { + // Cancels the run, which kills a running command's whole + // process group; the run then ends as aborted. + agent.abort(); + continue; + } + }; match event { AgentEvent::ToolExecutionStart { tool_name, args, .. @@ -309,6 +333,7 @@ async fn main() { _ => {} } } + running.store(false, std::sync::atomic::Ordering::SeqCst); if in_text { println!(); diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 9bd6b3b..567cbf6 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -161,7 +161,12 @@ impl AgentTool for BashTool { } } - let mut cmd = Command::new("bash"); + let mut cmd = std::process::Command::new("bash"); + // std's `process_group` (Rust 1.64): tokio's own needs tokio 1.40, + // newer than the `tokio = "1"` this crate asks for. + #[cfg(unix)] + std::os::unix::process::CommandExt::process_group(&mut cmd, 0); + let mut cmd = Command::from(cmd); cmd.arg("-c").arg(command); // Keep credentials out of model-authored commands when configured. @@ -194,8 +199,6 @@ impl AgentTool for BashTool { // starts its own session (`setsid`) escapes. Elsewhere this kills the // `bash` process only. cmd.kill_on_drop(true); - #[cfg(unix)] - cmd.process_group(0); let timeout = self.timeout; let max_bytes = self.max_output_bytes; diff --git a/tests/tools_test.rs b/tests/tools_test.rs index d0694eb..a9e5b74 100644 --- a/tests/tools_test.rs +++ b/tests/tools_test.rs @@ -1114,14 +1114,25 @@ async fn bash_env_allowlist_hides_other_variables() { // BashTool: what a command started goes with it (#277) // --------------------------------------------------------------------------- -/// Whether `pid` still exists (`kill -0`). +/// Whether `pid` still runs. A killed process nobody has reaped yet (no init +/// in a container, say) is a zombie: `kill -0` still succeeds on it, so a +/// `Z` state counts as gone. #[cfg(unix)] fn alive(pid: &str) -> bool { - std::process::Command::new("kill") + let exists = std::process::Command::new("kill") .args(["-0", pid]) .stderr(std::process::Stdio::null()) .status() - .is_ok_and(|s| s.success()) + .is_ok_and(|s| s.success()); + let zombie = std::process::Command::new("ps") + .args(["-o", "stat=", "-p", pid]) + .output() + .is_ok_and(|out| { + String::from_utf8_lossy(&out.stdout) + .trim_start() + .starts_with('Z') + }); + exists && !zombie } /// Waits until the command has written its background job's pid. From f364acbb76b4c6660ac685969905a3a57a9d40cd Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Fri, 9 Oct 2026 13:27:17 +0200 Subject: [PATCH 3/3] fix(bash): no unused-mut on Windows (process_group is Unix-only) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- src/tools/bash.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 567cbf6..a75e09b 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -161,6 +161,7 @@ impl AgentTool for BashTool { } } + #[cfg_attr(not(unix), allow(unused_mut))] let mut cmd = std::process::Command::new("bash"); // std's `process_group` (Rust 1.64): tokio's own needs tokio 1.40, // newer than the `tokio = "1"` this crate asks for.