From 26a95796e011ed4ccf724634ba082ddab42fce90 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:27:05 +0100 Subject: [PATCH 1/8] feat(core): cancel a running command from a token A raised Cancel kills the command's process group through the same path as a timeout, and a token raised beforehand starts nothing. Refs: #26 --- context/decision-policy.md | 2 +- context/guide-sandboxing.md | 2 +- crates/sandbx-core/public-api.txt | 20 +++++ crates/sandbx-core/src/cancel.rs | 37 ++++++++ crates/sandbx-core/src/cancel_tests.rs | 27 ++++++ crates/sandbx-core/src/helper_refusal.rs | 2 + crates/sandbx-core/src/lib.rs | 2 + crates/sandbx-core/src/sandbox_error.rs | 11 +++ crates/sandbx-core/src/sandbox_error_tests.rs | 4 + crates/sandbx-core/src/sandboxed_command.rs | 90 +++++++++++++------ crates/sandbx-core/tests/command.rs | 22 ++++- crates/sandbx-helper/tests/command.rs | 90 ++++++++++++++++++- 12 files changed, 278 insertions(+), 31 deletions(-) create mode 100644 crates/sandbx-core/src/cancel.rs create mode 100644 crates/sandbx-core/src/cancel_tests.rs diff --git a/context/decision-policy.md b/context/decision-policy.md index ae85b003..6333d488 100644 --- a/context/decision-policy.md +++ b/context/decision-policy.md @@ -523,7 +523,7 @@ program that checks. ### Narrow by construction, at one site Three `Command`s are built on the spawn path — the harness's spawn of the helper -(`helper_command`, shared by `run_to_completion` and `run_with_deadline`), the +(`helper_command`, shared by `run_to_completion` and `run_supervised`), the supervisor's spawn of stage 2, and stage 2's spawn of the real program. None of them calls `Command::new`: all go through `spawn::command`, which clears and re-adds as it builds, so a narrowed environment is a property of every `Command` diff --git a/context/guide-sandboxing.md b/context/guide-sandboxing.md index c351f9fb..242269cc 100644 --- a/context/guide-sandboxing.md +++ b/context/guide-sandboxing.md @@ -436,7 +436,7 @@ construction rather than a call each site has to remember: sandbx ──► helper stage 1 ──► stage 2 ──► the command ↑ ↑ ↑ helper_command() inner stage .spawn() ◄── the load-bearing one - (output, run_with_deadline) + (output, run_supervised) └──────────────────┴───────────┘ all three via spawn::command ``` diff --git a/crates/sandbx-core/public-api.txt b/crates/sandbx-core/public-api.txt index 56fb3192..c289b0af 100644 --- a/crates/sandbx-core/public-api.txt +++ b/crates/sandbx-core/public-api.txt @@ -212,6 +212,7 @@ impl core::panic::unwind_safe::UnwindSafe for sandbx_core::Pinning pub enum sandbx_core::SandboxError pub sandbx_core::SandboxError::BadHelperArgs pub sandbx_core::SandboxError::BadHelperArgs::detail: &'static str +pub sandbx_core::SandboxError::Cancelled pub sandbx_core::SandboxError::ExecFailed pub sandbx_core::SandboxError::ExecFailed::source: core::io::error::Error pub sandbx_core::SandboxError::GrantBoundByResolver @@ -288,6 +289,24 @@ impl core::marker::Unpin for sandbx_core::SandboxError impl core::marker::UnsafeUnpin for sandbx_core::SandboxError impl !core::panic::unwind_safe::RefUnwindSafe for sandbx_core::SandboxError impl !core::panic::unwind_safe::UnwindSafe for sandbx_core::SandboxError +pub struct sandbx_core::Cancel(_) +impl sandbx_core::Cancel +pub fn sandbx_core::Cancel::is_raised(&self) -> bool +pub fn sandbx_core::Cancel::new() -> Self +pub fn sandbx_core::Cancel::raise(&self) +impl core::clone::Clone for sandbx_core::Cancel +pub fn sandbx_core::Cancel::clone(&self) -> sandbx_core::Cancel +impl core::default::Default for sandbx_core::Cancel +pub fn sandbx_core::Cancel::default() -> sandbx_core::Cancel +impl core::fmt::Debug for sandbx_core::Cancel +pub fn sandbx_core::Cancel::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Freeze for sandbx_core::Cancel +impl core::marker::Send for sandbx_core::Cancel +impl core::marker::Sync for sandbx_core::Cancel +impl core::marker::Unpin for sandbx_core::Cancel +impl core::marker::UnsafeUnpin for sandbx_core::Cancel +impl core::panic::unwind_safe::RefUnwindSafe for sandbx_core::Cancel +impl core::panic::unwind_safe::UnwindSafe for sandbx_core::Cancel pub struct sandbx_core::CommandLine pub sandbx_core::CommandLine::argv: alloc::vec::Vec pub sandbx_core::CommandLine::helper: std::path::PathBuf @@ -513,6 +532,7 @@ pub struct sandbx_core::SandboxedCommand impl sandbx_core::SandboxedCommand pub fn sandbx_core::SandboxedCommand::arg(self, impl core::convert::Into) -> Self pub fn sandbx_core::SandboxedCommand::args(self, I) -> Self where I: core::iter::traits::collect::IntoIterator, S: core::convert::Into +pub fn sandbx_core::SandboxedCommand::cancel_on(self, sandbx_core::Cancel) -> Self pub fn sandbx_core::SandboxedCommand::command_line(&self) -> core::result::Result pub fn sandbx_core::SandboxedCommand::helper(self, impl core::convert::AsRef) -> Self pub fn sandbx_core::SandboxedCommand::new(impl core::convert::Into, sandbx_core::SandboxPolicy) -> Self diff --git a/crates/sandbx-core/src/cancel.rs b/crates/sandbx-core/src/cancel.rs new file mode 100644 index 00000000..812fd0a3 --- /dev/null +++ b/crates/sandbx-core/src/cancel.rs @@ -0,0 +1,37 @@ +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// A flag one side raises to stop a command another side is waiting on. +/// +/// Every clone is the same flag: the caller keeps one and hands another to +/// [`SandboxedCommand::cancel_on`](crate::SandboxedCommand::cancel_on). Raised once and for +/// good, so a token is minted per unit of work to stop, never reused. +/// +/// A polled flag and not a channel: a run already wakes every few milliseconds to ask the +/// child whether it has exited, and a second thing to block on would need a second thing +/// to wake it. +#[derive(Debug, Clone, Default)] +pub struct Cancel(Arc); + +impl Cancel { + /// A token nobody has raised. + pub fn new() -> Self { + Self::default() + } + + /// Ask whoever holds a clone to stop. Idempotent. + pub fn raise(&self) { + // `Relaxed`: the flag publishes nothing else, so there is no other write for a + // reader to be ordered against. + self.0.store(true, Ordering::Relaxed); + } + + /// Whether any clone has been raised. + pub fn is_raised(&self) -> bool { + self.0.load(Ordering::Relaxed) + } +} + +#[cfg(test)] +#[path = "cancel_tests.rs"] +mod tests; diff --git a/crates/sandbx-core/src/cancel_tests.rs b/crates/sandbx-core/src/cancel_tests.rs new file mode 100644 index 00000000..4c835546 --- /dev/null +++ b/crates/sandbx-core/src/cancel_tests.rs @@ -0,0 +1,27 @@ +use super::Cancel; + +#[test] +fn a_new_token_is_not_raised() { + assert!(!Cancel::new().is_raised()); +} + +/// The point of the type: the turn keeps one end and every spawned command holds another. +#[test] +fn raising_one_clone_raises_them_all() { + let held = Cancel::new(); + let handed_out = held.clone(); + + held.raise(); + + assert!(handed_out.is_raised()); +} + +#[test] +fn a_separate_token_is_unaffected() { + let first = Cancel::new(); + let second = Cancel::new(); + + first.raise(); + + assert!(!second.is_raised()); +} diff --git a/crates/sandbx-core/src/helper_refusal.rs b/crates/sandbx-core/src/helper_refusal.rs index 04906f1a..57005db3 100644 --- a/crates/sandbx-core/src/helper_refusal.rs +++ b/crates/sandbx-core/src/helper_refusal.rs @@ -173,6 +173,8 @@ impl SandboxError { Self::GrantBoundByResolver { .. } => Parent("grant_bound_by_resolver"), // The word the operator typed and the docs use, so a trail reader greps for it. Self::TimedOut { .. } => Parent("timeout"), + // Decided by the caller's token, not by anything the command did. + Self::Cancelled => Parent("cancelled"), // The relay's own output: reporting it would be a second crossing. Borrowed, so a // trail filtered by `reason=` cannot tell which side of the channel decided it. Self::HelperRefused { refusal, .. } => Parent(refusal.label()), diff --git a/crates/sandbx-core/src/lib.rs b/crates/sandbx-core/src/lib.rs index 427f4a7d..85585af7 100644 --- a/crates/sandbx-core/src/lib.rs +++ b/crates/sandbx-core/src/lib.rs @@ -19,6 +19,7 @@ compile_error!( ); mod audit_event; +mod cancel; mod command; mod concealment; mod degradation; @@ -33,6 +34,7 @@ mod spawn; mod spawn_id; pub use audit_event::{AUDIT_TARGET, AuditEvent, Pinning}; +pub use cancel::Cancel; pub use command::{ CommandLine, HELPER_FLAG, HELPER_INNER_FLAG, SandboxedCommand, with_helper_dispatch, }; diff --git a/crates/sandbx-core/src/sandbox_error.rs b/crates/sandbx-core/src/sandbox_error.rs index c5e5dce0..fa6d533d 100644 --- a/crates/sandbx-core/src/sandbox_error.rs +++ b/crates/sandbx-core/src/sandbox_error.rs @@ -239,6 +239,15 @@ pub enum SandboxError { after: Duration, }, + /// The [`Cancel`](crate::Cancel) handed to [`SandboxedCommand::cancel_on`] was raised, so + /// the command was killed — or never started, where it was raised beforehand. + /// + /// Not [`TimedOut`](Self::TimedOut): the caller chose this, and a retry with the same + /// token can only be refused again. + /// + /// [`SandboxedCommand::cancel_on`]: crate::SandboxedCommand::cancel_on + Cancelled, + /// This kernel cannot enforce a sandbox, returned instead of running unsandboxed. A /// kernel and not a platform — the crate refuses to build for non-Linux at all — so: too /// old for the Landlock baseline, Landlock disabled at boot, or a ruleset accepted and @@ -288,6 +297,7 @@ impl fmt::Display for SandboxError { Self::TimedOut { after } => { write!(f, "command exceeded its {after:?} limit and was killed") } + Self::Cancelled => f.write_str("command was cancelled and killed"), Self::Unsupported { detail } => write!(f, "sandboxing is not available here: {detail}"), Self::UnboundedResolution { detail } => { write!(f, "a name allowlist would bound nothing here: {detail}") @@ -426,6 +436,7 @@ impl std::error::Error for SandboxError { | Self::ProcessHardening { .. } | Self::ProcessConcealment { .. } | Self::TimedOut { .. } + | Self::Cancelled | Self::PinMismatch { .. } | Self::PinnedScript { .. } | Self::HelperRefused { .. } diff --git a/crates/sandbx-core/src/sandbox_error_tests.rs b/crates/sandbx-core/src/sandbox_error_tests.rs index 91c50765..cb046433 100644 --- a/crates/sandbx-core/src/sandbox_error_tests.rs +++ b/crates/sandbx-core/src/sandbox_error_tests.rs @@ -101,6 +101,7 @@ fn every_variant() -> Vec { SandboxError::TimedOut { after: Duration::from_secs(1), }, + SandboxError::Cancelled, SandboxError::Unsupported { detail: "sample" }, SandboxError::UnboundedResolution { detail: "sample" }, SandboxError::GrantBoundByResolver { @@ -136,6 +137,7 @@ fn every_variant() -> Vec { | SandboxError::PinUnreadable { .. } | SandboxError::PinnedScript { .. } | SandboxError::TimedOut { .. } + | SandboxError::Cancelled | SandboxError::HelperRefused { .. } | SandboxError::UnboundedResolution { .. } | SandboxError::GrantBoundByResolver { .. } @@ -181,6 +183,7 @@ fn each_variant_keeps_the_label_a_trail_filters_by() { SandboxError::PinUnreadable { .. } => "pin_unreadable", SandboxError::PinnedScript { .. } => "pinned_script", SandboxError::TimedOut { .. } => "timeout", + SandboxError::Cancelled => "cancelled", SandboxError::Unsupported { .. } => "unsupported", SandboxError::UnboundedResolution { .. } => "unbounded_resolution", SandboxError::GrantBoundByResolver { .. } => "grant_bound_by_resolver", @@ -262,6 +265,7 @@ fn the_reasons_the_helper_does_not_decide_are_not_refusals() { | SandboxError::UnboundedResolution { .. } | SandboxError::GrantBoundByResolver { .. } | SandboxError::TimedOut { .. } + | SandboxError::Cancelled ); assert_eq!( diff --git a/crates/sandbx-core/src/sandboxed_command.rs b/crates/sandbx-core/src/sandboxed_command.rs index 7535b847..27911eae 100644 --- a/crates/sandbx-core/src/sandboxed_command.rs +++ b/crates/sandbx-core/src/sandboxed_command.rs @@ -14,7 +14,7 @@ use std::thread::JoinHandle; use std::time::{Duration, Instant}; use std::{fs, io, mem, thread}; -use crate::{HelperArgs, SandboxError, SandboxPolicy, Sha256Digest}; +use crate::{Cancel, HelperArgs, SandboxError, SandboxPolicy, Sha256Digest}; use super::dispatch::HELPER_FLAG; @@ -31,6 +31,7 @@ pub struct SandboxedCommand { policy: SandboxPolicy, helper: Option, timeout: Option, + cancel: Option, pin: Option, session: Option, } @@ -60,6 +61,7 @@ impl SandboxedCommand { policy, helper: None, timeout: None, + cancel: None, pin: None, session: None, } @@ -102,6 +104,17 @@ impl SandboxedCommand { self } + /// Kill the command once `cancel` is raised, from any thread; unset by default. + /// + /// Reaches the same group kill a timeout does, on a different condition, and is noticed + /// within a poll interval. Setting one puts the run on the polled path even with no + /// [`timeout`](Self::timeout), the unpolled one having no way to look. + #[must_use] + pub fn cancel_on(mut self, cancel: Cancel) -> Self { + self.cancel = Some(cancel); + self + } + /// Name the session this runs under, so a record pairs with a transcript; unset by /// default, which is what `sandbox-run` is — it has no session to name. #[must_use] @@ -157,17 +170,25 @@ impl SandboxedCommand { /// Run the command to completion and collect its output. Blocks until the command exits, /// or — with a [`timeout`](Self::timeout) set — until the limit expires, which kills it - /// and returns [`SandboxError::TimedOut`]. A helper stage that refused returns + /// and returns [`SandboxError::TimedOut`]. A [`cancel_on`](Self::cancel_on) token raised + /// does the same and returns [`SandboxError::Cancelled`] — before anything starts, where + /// it was raised already. A helper stage that refused returns /// [`SandboxError::HelperRefused`] rather than the status it exited with, which is /// indistinguishable from the command's own. /// - /// Untimed, nothing kills anything: no process group is made, and namespace teardown is - /// the only thing that ends a descendant. + /// Neither timed nor cancellable, nothing kills anything: no process group is made, and + /// namespace teardown is the only thing that ends a descendant. /// /// A run is two records (#96): `spawned`, then any `degraded`, then exactly one `exited` /// or `failed`, emitted here and nowhere else. pub fn output(&self) -> Result { let CommandLine { helper, argv } = self.command_line()?; + + // Ahead of the `spawned` record, so a run that never started leaves no pair to close. + if self.cancel.as_ref().is_some_and(Cancel::is_raised) { + return Err(SandboxError::Cancelled); + } + let (audit, write_end) = audit_channel()?; // Minted here and not in `new`: this takes `&self`, so one builder can be run more @@ -188,9 +209,16 @@ impl SandboxedCommand { ) .emit(); - let result = match self.timeout { - None => run_to_completion(&helper, &argv, &self.policy, write_end), - Some(limit) => run_with_deadline(&helper, &argv, &self.policy, limit, write_end), + let result = match (self.timeout, &self.cancel) { + (None, None) => run_to_completion(&helper, &argv, &self.policy, write_end), + (limit, cancel) => run_supervised( + &helper, + &argv, + &self.policy, + limit, + cancel.as_ref(), + write_end, + ), }; // Both paths wait before returning, so nothing drains the pipe while the helper @@ -242,7 +270,7 @@ fn helper_command(helper: &Path, argv: &[OsString], policy: &SandboxPolicy) -> C /// Spawn the helper and wait for it, with no deadline. `Command::output` reads both pipes /// to EOF before it waits, so a command that never exits wedges the caller; -/// [`run_with_deadline`] is the path that cannot. +/// [`run_supervised`] is the path that cannot. fn run_to_completion( helper: &Path, argv: &[OsString], @@ -310,14 +338,15 @@ fn record_reports( refused } -/// Spawn the helper and wait for it, giving up after `limit`. Not `output()`: -/// `std::process` has no timed wait, so this drains both pipes on their own threads and -/// polls for exit until the deadline. -fn run_with_deadline( +/// Spawn the helper and wait for it, giving up after `limit` or once `cancel` is raised. +/// Not `output()`: `std::process` has no timed wait, so this drains both pipes on their own +/// threads and polls for exit, for the deadline and the token alike. +fn run_supervised( helper: &Path, argv: &[OsString], policy: &SandboxPolicy, - limit: Duration, + limit: Option, + cancel: Option<&Cancel>, write_end: PipeWriter, ) -> Result { use std::os::unix::process::CommandExt; @@ -348,33 +377,40 @@ fn run_with_deadline( let out = drain(child.stdout.take()); let err = drain(child.stderr.take()); - let deadline = Instant::now() + limit; + let expiry = limit.map(|limit| (Instant::now() + limit, limit)); let finished = loop { match child.try_wait() { - Ok(Some(status)) => break Ok(Some(status)), - Ok(None) if Instant::now() >= deadline => break Ok(None), - Ok(None) => thread::sleep(POLL_INTERVAL), + // Ahead of the token: a command that already exited was not stopped by it. + Ok(Some(status)) => break Ok(status), + Ok(None) if cancel.is_some_and(Cancel::is_raised) => { + break Err(SandboxError::Cancelled); + } + Ok(None) => { + if let Some((_, after)) = expiry.filter(|&(deadline, _)| Instant::now() >= deadline) + { + break Err(SandboxError::TimedOut { after }); + } + thread::sleep(POLL_INTERVAL); + } // Broken out and not returned, so the kill below is not skipped: the caller // reads the audit channel to an EOF only a dead helper gives. Err(source) => break Err(spawn_failed(source)), } }; - // On every path out of the loop, not just the timeout: a command may background work and - // exit inside its deadline, leaving behind processes that inherited the pipe write ends - // — the readers would otherwise never see EOF. + // On every path out of the loop, not just a timeout or a cancel: a command may background + // work and exit inside its deadline, leaving behind processes that inherited the pipe + // write ends — the readers would otherwise never see EOF. kill_group(group); let status = match finished { - Ok(Some(status)) => status, - // A refused `try_wait` and a timeout leave the same thing behind: no status, a child - // to reap and readers to let go. - outcome => { + Ok(status) => status, + // A refused `try_wait`, a timeout and a cancel leave the same thing behind: no + // status, a child to reap and readers to let go. + Err(error) => { let _ = child.wait(); settle(&out.reader, &err.reader); - return Err(outcome - .err() - .unwrap_or(SandboxError::TimedOut { after: limit })); + return Err(error); } }; diff --git a/crates/sandbx-core/tests/command.rs b/crates/sandbx-core/tests/command.rs index c27579ea..c5d090fb 100644 --- a/crates/sandbx-core/tests/command.rs +++ b/crates/sandbx-core/tests/command.rs @@ -3,7 +3,9 @@ //! The half that spawns the real helper is `sandbx-helper`'s `tests/command.rs`, where //! `CARGO_BIN_EXE_sandbx-helper` resolves (#347). -use sandbx_core::{CommandLine, HELPER_FLAG, SandboxPolicy, SandboxedCommand}; +use sandbx_core::{ + Cancel, CommandLine, HELPER_FLAG, SandboxError, SandboxPolicy, SandboxedCommand, +}; use std::ffi::OsString; use std::path::Path; use std::process::ExitCode; @@ -221,3 +223,21 @@ fn a_policy_granting_a_file_its_own_resolver_replaces_cannot_be_turned_into_an_a "the refusal names no path to go and look at, or no way forward: {message}" ); } + +/// Refused ahead of the spawn: a helper that does not exist would otherwise come back as +/// `SpawnFailed`, so the variant is the proof nothing was started. +#[test] +fn a_cancel_already_raised_starts_nothing() { + let cancel = Cancel::new(); + cancel.raise(); + + let result = SandboxedCommand::new("/bin/true", SandboxPolicy::default()) + .helper("/nonexistent/helper") + .cancel_on(cancel) + .output(); + + assert!( + matches!(result, Err(SandboxError::Cancelled)), + "expected a refusal before the spawn, got: {result:?}" + ); +} diff --git a/crates/sandbx-helper/tests/command.rs b/crates/sandbx-helper/tests/command.rs index aee36404..654a0fc7 100644 --- a/crates/sandbx-helper/tests/command.rs +++ b/crates/sandbx-helper/tests/command.rs @@ -9,7 +9,7 @@ //! off, which `-D warnings` refuses. #![cfg(all(feature = "sandbox-integration", target_os = "linux"))] -use sandbx_core::{SandboxPolicy, SandboxedCommand}; +use sandbx_core::{Cancel, SandboxError, SandboxPolicy, SandboxedCommand}; use std::path::Path; use std::time::{Duration, Instant}; use std::{env, fs, thread}; @@ -139,6 +139,94 @@ fn a_command_that_outruns_its_timeout_is_killed() { ); } +/// Raise `cancel` once the command has had time to start. +fn raise_soon(cancel: &Cancel) -> thread::JoinHandle<()> { + let raiser = cancel.clone(); + thread::spawn(move || { + thread::sleep(Duration::from_millis(200)); + raiser.raise(); + }) +} + +/// Cancellation is the deadline's kill on a different condition, so it must not need the +/// deadline: no timeout is set, and the elapsed time is the guarantee, as above. +#[test] +fn a_raised_cancel_kills_a_command_with_no_timeout() { + let cancel = Cancel::new(); + let stopper = raise_soon(&cancel); + let started = Instant::now(); + + let result = SandboxedCommand::new( + "/bin/sh", + SandboxPolicy::default().allow_system_executables(), + ) + .arg("-c") + .arg("sleep 30") + .helper(env!("CARGO_BIN_EXE_sandbx-helper")) + .cancel_on(cancel) + .output(); + + let elapsed = started.elapsed(); + stopper.join().unwrap(); + + assert!( + matches!(result, Err(SandboxError::Cancelled)), + "expected a cancellation, got: {result:?}" + ); + assert!( + elapsed < Duration::from_secs(5), + "cancelled but only after {elapsed:?} — the caller was still blocked" + ); +} + +/// The group kill, not just the direct child: a pipeline leaves a grandchild holding the +/// pipes, which would keep the readers waiting past the cancel. +#[test] +fn a_raised_cancel_frees_the_call_from_a_grandchild() { + let cancel = Cancel::new(); + let stopper = raise_soon(&cancel); + let started = Instant::now(); + + let result = SandboxedCommand::new( + "/bin/sh", + SandboxPolicy::default().allow_system_executables(), + ) + .arg("-c") + .arg("sleep 30 | cat") + .helper(env!("CARGO_BIN_EXE_sandbx-helper")) + .timeout(Duration::from_secs(60)) + .cancel_on(cancel) + .output(); + + let elapsed = started.elapsed(); + stopper.join().unwrap(); + + assert!( + matches!(result, Err(SandboxError::Cancelled)), + "expected a cancellation, got: {result:?}" + ); + assert!( + elapsed < Duration::from_secs(5), + "a grandchild kept the cancelled call blocked for {elapsed:?}" + ); +} + +/// A token nobody raises changes nothing, so every existing caller keeps its behaviour. +#[test] +fn an_unraised_cancel_lets_a_command_finish() { + let output = SandboxedCommand::new( + "/bin/echo", + SandboxPolicy::default().allow_system_executables(), + ) + .arg("done") + .helper(env!("CARGO_BIN_EXE_sandbx-helper")) + .cancel_on(Cancel::new()) + .output() + .expect("a command nobody cancelled must run"); + + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "done"); +} + /// Grandchildren inherit the pipes, so killing only the direct child leaves the reader /// threads waiting on an EOF that never comes. A pipeline rather than `cmd &`: /// backgrounding in `sh` redirects the job's stdin from `/dev/null`, which this policy From 3ca9b5bc8ee5ecb458c589b4d4f1cedea2985234 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:30:32 +0100 Subject: [PATCH 2/8] feat(tools): stop a running bash call from the context's token ExecutionContext carries a Cancel that every spawn it hands out observes, and bash reports a killed command as ToolError::Cancelled. Refs: #26 --- crates/sandbx-cli/src/agent_gate.rs | 1 + crates/sandbx-cli/src/agent_gate_tests.rs | 14 ++++++ crates/sandbx-tools/public-api.txt | 5 ++ crates/sandbx-tools/src/execution_context.rs | 33 ++++++++++--- crates/sandbx-tools/src/lib.rs | 2 + crates/sandbx-tools/src/tool_error.rs | 12 +++++ crates/sandbx-tools/src/tools_bash.rs | 1 + crates/sandbx-tools/src/tools_bash_tests.rs | 12 +++++ crates/sandbx-tools/tests/bash.rs | 50 +++++++++++++++++++- 9 files changed, 122 insertions(+), 8 deletions(-) diff --git a/crates/sandbx-cli/src/agent_gate.rs b/crates/sandbx-cli/src/agent_gate.rs index 5adacecc..53881bd8 100644 --- a/crates/sandbx-cli/src/agent_gate.rs +++ b/crates/sandbx-cli/src/agent_gate.rs @@ -194,6 +194,7 @@ fn report(call: Settled<'_>) -> String { ToolError::TimedOut { after, .. } => { format!("{head} — timed out after {after:?} and was killed") } + ToolError::Cancelled { .. } => format!("{head} — cancelled and killed"), }, } } diff --git a/crates/sandbx-cli/src/agent_gate_tests.rs b/crates/sandbx-cli/src/agent_gate_tests.rs index d2f45fd2..680b0051 100644 --- a/crates/sandbx-cli/src/agent_gate_tests.rs +++ b/crates/sandbx-cli/src/agent_gate_tests.rs @@ -359,6 +359,20 @@ fn a_policy_refusal_is_not_reported_as_a_call_that_ran() { ); } +/// The operator who pressed ctrl-C reads this line, so it must say the command was killed +/// and not that it ran or that a limit was hit. +#[test] +fn a_cancelled_call_is_reported_as_killed() { + let error = ToolError::Cancelled { + subject: "run `sleep 80`".to_string(), + }; + + assert_eq!( + reported_bash("sleep 80", Outcome::Errored(&error)), + "bash sleep 80 — cancelled and killed" + ); +} + /// Neither of these reaches `approve`, so this line is the operator's only account (#169). #[test] fn the_two_refusals_above_the_gate_are_reported() { diff --git a/crates/sandbx-tools/public-api.txt b/crates/sandbx-tools/public-api.txt index fdb2a8c1..563234fe 100644 --- a/crates/sandbx-tools/public-api.txt +++ b/crates/sandbx-tools/public-api.txt @@ -1,4 +1,5 @@ pub mod sandbx_tools +pub use sandbx_tools::Cancel pub enum sandbx_tools::BuiltinTool pub sandbx_tools::BuiltinTool::Bash pub sandbx_tools::BuiltinTool::Edit @@ -58,6 +59,8 @@ impl core::panic::unwind_safe::UnwindSafe for sandbx_tools::RiskLevel pub enum sandbx_tools::ToolError pub sandbx_tools::ToolError::BadInput pub sandbx_tools::ToolError::BadInput::detail: alloc::string::String +pub sandbx_tools::ToolError::Cancelled +pub sandbx_tools::ToolError::Cancelled::subject: alloc::string::String pub sandbx_tools::ToolError::Denied pub sandbx_tools::ToolError::Denied::reason: alloc::string::String pub sandbx_tools::ToolError::Denied::subject: alloc::string::String @@ -81,11 +84,13 @@ impl core::panic::unwind_safe::RefUnwindSafe for sandbx_tools::ToolError impl core::panic::unwind_safe::UnwindSafe for sandbx_tools::ToolError pub struct sandbx_tools::ExecutionContext impl sandbx_tools::ExecutionContext +pub fn sandbx_tools::ExecutionContext::cancelled(&self) -> bool pub fn sandbx_tools::ExecutionContext::for_session(sandbx_core::policy::sandbox_policy::SandboxPolicy, alloc::string::String) -> Self pub fn sandbx_tools::ExecutionContext::guard(&self) -> &sandbx_core::fs_guard::FsGuard pub fn sandbx_tools::ExecutionContext::limits(&self) -> &sandbx_tools::ToolLimits pub fn sandbx_tools::ExecutionContext::new(sandbx_core::policy::sandbox_policy::SandboxPolicy) -> Self pub fn sandbx_tools::ExecutionContext::sandboxed_command(&self, alloc::string::String) -> sandbx_core::command::sandboxed_command::SandboxedCommand +pub fn sandbx_tools::ExecutionContext::with_cancel(self, sandbx_core::cancel::Cancel) -> Self pub fn sandbx_tools::ExecutionContext::with_helper(self, std::path::PathBuf) -> Self pub fn sandbx_tools::ExecutionContext::with_limits(self, sandbx_tools::ToolLimits) -> Self pub fn sandbx_tools::ExecutionContext::with_timeout(self, core::time::Duration) -> Self diff --git a/crates/sandbx-tools/src/execution_context.rs b/crates/sandbx-tools/src/execution_context.rs index 8e412ab0..048eb0d3 100644 --- a/crates/sandbx-tools/src/execution_context.rs +++ b/crates/sandbx-tools/src/execution_context.rs @@ -1,4 +1,4 @@ -use sandbx_core::{FsGuard, SandboxPolicy, SandboxedCommand}; +use sandbx_core::{Cancel, FsGuard, SandboxPolicy, SandboxedCommand}; use std::path::PathBuf; use std::time::Duration; @@ -25,6 +25,7 @@ pub struct ExecutionContext { helper: Option, limits: ToolLimits, timeout: Duration, + cancel: Cancel, session: Option, } @@ -68,6 +69,7 @@ impl ExecutionContext { helper: None, limits: ToolLimits::default(), timeout: DEFAULT_TIMEOUT, + cancel: Cancel::new(), session, } } @@ -95,6 +97,23 @@ impl ExecutionContext { self } + /// Stop what this context spawns once `cancel` is raised, instead of a token nobody holds. + /// + /// Every clone shares the token, so one raised stops a command started through any of + /// them. Only a spawned command can be stopped: the in-process tools are bounded by + /// work and finish what they started. + #[must_use] + pub fn with_cancel(mut self, cancel: Cancel) -> Self { + self.cancel = cancel; + self + } + + /// Whether the token given to [`with_cancel`](Self::with_cancel) has been raised, for a + /// caller choosing whether to start another call. + pub fn cancelled(&self) -> bool { + self.cancel.is_raised() + } + /// How much tools may do, and how much they may return. pub fn limits(&self) -> &ToolLimits { &self.limits @@ -105,17 +124,19 @@ impl ExecutionContext { &self.guard } - /// A command to spawn, with the policy, the helper and the timeout already - /// applied. + /// A command to spawn, with the policy, the helper, the timeout and the cancel token + /// already applied. /// /// The only route to the policy, and one that spends it rather than lending it /// out: an accessor returning `&SandboxPolicy` would let an in-process tool read /// the path lists and open files itself, bypassing the TOCTOU-safe handles - /// [`FsGuard`] hands back (#56). Timeout, helper and session are applied here so a - /// second spawning built-in cannot forget them. + /// [`FsGuard`] hands back (#56). Timeout, cancel, helper and session are applied here so + /// a second spawning built-in cannot forget them. #[must_use] pub fn sandboxed_command(&self, program: String) -> SandboxedCommand { - let mut command = SandboxedCommand::new(program, self.policy.clone()).timeout(self.timeout); + let mut command = SandboxedCommand::new(program, self.policy.clone()) + .timeout(self.timeout) + .cancel_on(self.cancel.clone()); if let Some(helper) = &self.helper { command = command.helper(helper); } diff --git a/crates/sandbx-tools/src/lib.rs b/crates/sandbx-tools/src/lib.rs index 577fc5d7..92d30330 100644 --- a/crates/sandbx-tools/src/lib.rs +++ b/crates/sandbx-tools/src/lib.rs @@ -15,6 +15,8 @@ mod tools; pub use builtin_tool::{BuiltinTool, RiskLevel, ToolOutput}; pub use execution_context::{DEFAULT_TIMEOUT, ExecutionContext}; +/// Re-exported so a caller stopping a call needs no dependency on the crate that defines it. +pub use sandbx_core::Cancel; pub use tool_error::ToolError; pub use tool_limits::ToolLimits; diff --git a/crates/sandbx-tools/src/tool_error.rs b/crates/sandbx-tools/src/tool_error.rs index 6b44774e..354d4824 100644 --- a/crates/sandbx-tools/src/tool_error.rs +++ b/crates/sandbx-tools/src/tool_error.rs @@ -43,6 +43,15 @@ pub enum ToolError { /// The limit it exceeded. after: Duration, }, + + /// The operation was killed because the caller stopped waiting for it. + /// + /// Separate from [`TimedOut`](Self::TimedOut): no limit was hit, so there is nothing to + /// narrow or lengthen, and the operation may have changed state before it died. + Cancelled { + /// What was attempted. + subject: String, + }, } impl fmt::Display for ToolError { @@ -56,6 +65,9 @@ impl fmt::Display for ToolError { Self::TimedOut { subject, after } => { write!(f, "{subject} timed out after {after:?} and was killed") } + Self::Cancelled { subject } => { + write!(f, "{subject} was cancelled and killed before it finished") + } } } } diff --git a/crates/sandbx-tools/src/tools_bash.rs b/crates/sandbx-tools/src/tools_bash.rs index bd9c79c6..80b2a903 100644 --- a/crates/sandbx-tools/src/tools_bash.rs +++ b/crates/sandbx-tools/src/tools_bash.rs @@ -94,6 +94,7 @@ fn sandbox_error(command: &str, error: sandbx_core::SandboxError) -> ToolError { subject: ran(), after, }, + SandboxError::Cancelled => ToolError::Cancelled { subject: ran() }, // Exhaustive over the refusals and not over `SandboxError`, most of which is // `FsGuard`'s and unreachable from a spawn: an arm for one would claim an unreachable // path is handled. diff --git a/crates/sandbx-tools/src/tools_bash_tests.rs b/crates/sandbx-tools/src/tools_bash_tests.rs index 8778cc40..509fa777 100644 --- a/crates/sandbx-tools/src/tools_bash_tests.rs +++ b/crates/sandbx-tools/src/tools_bash_tests.rs @@ -151,3 +151,15 @@ fn a_timeout_names_the_command_that_ran() { "a command killed on its deadline did run: {error:?}" ); } + +/// Its own kind, not a timeout: the limit was not what stopped it, and "might succeed +/// given longer" would be the wrong thing to tell a model whose operator pressed ctrl-C. +#[test] +fn a_cancel_names_the_command_that_was_stopped() { + let error = sandbox_error(COMMAND, SandboxError::Cancelled); + + assert!( + matches!(&error, ToolError::Cancelled { subject } if subject == &format!("run `{COMMAND}`")), + "a command stopped from outside did run: {error:?}" + ); +} diff --git a/crates/sandbx-tools/tests/bash.rs b/crates/sandbx-tools/tests/bash.rs index 31587837..e6da1bf5 100644 --- a/crates/sandbx-tools/tests/bash.rs +++ b/crates/sandbx-tools/tests/bash.rs @@ -6,11 +6,11 @@ #![cfg(all(feature = "sandbox-integration", target_os = "linux"))] use sandbx_core::{SandboxPolicy, VettedPath}; -use sandbx_tools::{BuiltinTool, ExecutionContext, ToolError}; +use sandbx_tools::{BuiltinTool, Cancel, ExecutionContext, ToolError}; use serde_json::json; -use std::fs; use std::path::Path; use std::time::{Duration, Instant}; +use std::{fs, thread}; /// `path`, pinned to the object it names — the shape every grant takes (#212). fn vetted(path: impl AsRef) -> VettedPath { @@ -153,6 +153,52 @@ fn a_command_that_outruns_the_timeout_says_so() { ); } +/// The token reaches the spawn through the context, so a stop raised elsewhere kills the +/// command the tool is blocked on. No timeout is lowered: the default 90 seconds is what +/// the elapsed time is measured against. +#[test] +fn a_raised_cancel_kills_the_command_the_tool_is_blocked_on() { + let cancel = Cancel::new(); + let ctx = + context(SandboxPolicy::default().allow_system_executables()).with_cancel(cancel.clone()); + let stopper = thread::spawn(move || { + thread::sleep(Duration::from_millis(200)); + cancel.raise(); + }); + let started = Instant::now(); + + let result = BuiltinTool::Bash.execute(json!({ "command": "sleep 30" }), &ctx); + + let elapsed = started.elapsed(); + stopper.join().unwrap(); + + assert!( + matches!(result, Err(ToolError::Cancelled { .. })), + "expected a cancellation, got: {result:?}" + ); + assert!( + elapsed < Duration::from_secs(5), + "the tool call stayed blocked for {elapsed:?}" + ); +} + +/// A context cloned before the stop is raised is the same context to it: the turn clones +/// one per call. +#[test] +fn a_cancel_raised_beforehand_runs_nothing() { + let cancel = Cancel::new(); + let ctx = + context(SandboxPolicy::default().allow_system_executables()).with_cancel(cancel.clone()); + cancel.raise(); + + let result = BuiltinTool::Bash.execute(json!({ "command": "echo ran" }), &ctx.clone()); + + assert!( + matches!(result, Err(ToolError::Cancelled { .. })), + "expected a cancellation, got: {result:?}" + ); +} + /// The Messages API rejects an empty `tool_result`, so a silent success — `touch`, /// `mkdir -p`, `true` — would otherwise kill the turn. #[test] From 07ca1bb246a6cf56af2694f7fcd35650cc89a09f Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:34:05 +0100 Subject: [PATCH 3/8] feat(turn): kill a running bash call when the turn is stopped run_turn_until raises a per-turn Cancel when the stop resolves while answering, waits for the running call to end, and starts none behind it. Refs: #26 --- crates/sandbx-turn/src/tools.rs | 10 ++- crates/sandbx-turn/src/turn.rs | 49 ++++++++++++--- crates/sandbx-turn/tests/turn_stop.rs | 88 ++++++++++++++++++++++++--- 3 files changed, 126 insertions(+), 21 deletions(-) diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index b2efa25d..1b174ddc 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -32,6 +32,12 @@ where let mut aborted: Option = None; for block in blocks { + // A stop raised during the last call: nothing behind it is asked about or run. The + // round is dropped whole by the caller, so no `tool_result` is owed for these. + if ctx.cancelled() { + break; + } + let ContentBlock::ToolUse { id, name, input } = block else { continue; }; @@ -59,8 +65,8 @@ where continue; } - // Before `spawn_blocking`, never racing it: a blocking task cannot be cancelled, so - // a late decision would not stop the call it refused (#26). + // Before `spawn_blocking`, never racing it: an in-process tool cannot be cancelled, + // so a late decision would not stop the call it refused (#26). let verdict = match &aborted { Some(reason) => ApprovalDecision::Deny { reason: reason.clone(), diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index f7dac261..0a0ed3ad 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -15,7 +15,7 @@ use sandbx_compact::{Compaction, PromptUsage, over_budget, plan_cut, window}; use sandbx_event::{AgentEvent, StopReason}; use sandbx_prompt::{Prompt, RequestMessage, Role, Thinking, ToolChoice, ToolDefinition}; use sandbx_provider::{EventStream, ProviderError}; -use sandbx_tools::{BuiltinTool, ExecutionContext}; +use sandbx_tools::{BuiltinTool, Cancel, ExecutionContext}; use crate::TurnError; use crate::accumulate::accumulate; @@ -141,8 +141,8 @@ pub enum TurnStop { /// /// The transcript is every round before it, as legal to send again as /// [`RoundLimit`](Self::RoundLimit)'s, or empty. Not in it: anything of the dropped - /// round, the calls it had already run included, and a call it had started, which still - /// runs to completion (#26). + /// round, the calls it had already run included. A `bash` it had started is killed; any + /// other call it had started finishes, and its effect stays (#26). Interrupted, } @@ -164,8 +164,8 @@ pub struct TurnLimits { /// /// Bounds one round's *consumption*, which `sandbx-anthropic` leaves to a caller: its own /// read timeout resets on every chunk, so a connection kept warm while producing nothing - /// is not bounded by it. Not a bound on the turn itself: an outer deadline would not help a - /// tool call either, since `spawn_blocking` cannot be cancelled (#26). + /// is not bounded by it. Not a bound on the turn itself: an outer deadline would not help an + /// in-process tool call either, which cannot be cancelled (#26). pub stream_timeout: Duration, /// Whether to withhold the oldest history from a request that has outgrown a budget, @@ -215,8 +215,9 @@ impl Default for TurnLimits { /// [`TurnStop::GateAborted`]. `context/decision-approval-gate.md` has the rest. /// /// Tools run on `spawn_blocking`, uncancellable: dropping this future still lets the -/// blocking task finish, so a turn abandoned mid-tool applies the `write` anyway, recorded -/// only in the audit trail (#26). [`run_turn_until`] stops a turn and keeps its rounds. +/// blocking task finish, so a turn abandoned mid-tool applies the `write` anyway, or lets a +/// `bash` run out its timeout, recorded only in the audit trail (#26). [`run_turn_until`] +/// stops a turn instead: it keeps the rounds before the stop and kills a running `bash`. /// /// Needs a tokio runtime with the time driver enabled: the per-round bound is /// `tokio::time::timeout`, which panics with "there is no timer running" otherwise. @@ -241,8 +242,10 @@ where /// /// Raced against each round's request and its tool calls, `stop` first, so a stop that /// has already resolved opens no further request. The round it lands in is dropped whole, -/// calls already answered in it included; a call already running goes on unseen, -/// `spawn_blocking` being what it is (#26), and [`CallGate::settled`] never hears of it. +/// calls already answered in it included. A call already running is waited for rather than +/// abandoned, and reported to [`CallGate::settled`] as it ends: a `bash` is killed, but an +/// in-process tool cannot be cancelled (#26), so it finishes and its effect stays. No call +/// behind it is put to the gate or run. /// /// [`TurnOutcome::usage`] keeps the dropped round's figure when it reported one, its request /// being exactly the messages kept. [`TurnOutcome::withheld`] is the cut of the last request @@ -267,6 +270,11 @@ where G: CallGate, { let mut stop = pin!(stop); + // One per turn, never reused: raised for good, so a token carried over would stop the + // next turn's first call before it started. The context is cloned to carry it, which + // leaves the caller's untouched. + let cancel = Cancel::new(); + let ctx = &ctx.clone().with_cancel(cancel.clone()); // Built once: the offered set does not change between rounds. let definitions: Vec = turn.tools.iter().copied().map(definition).collect(); let mut produced: Vec = Vec::new(); @@ -369,7 +377,7 @@ where // Before pushing: answering borrows `round.blocks`, pushing moves them. let answered = answer_calls(&round.blocks, ctx, turn.tools, &mut gate); - let Some(answers) = unless(stop.as_mut(), answered).await else { + let Some(answers) = unless_cancelling(stop.as_mut(), &cancel, answered).await else { return Ok(outcome( produced, usage, @@ -438,6 +446,27 @@ async fn unless( } } +/// [`unless`], for work that can be told to stop: once `stop` has resolved, raise `cancel` and +/// wait for `work` to wind down, rather than drop it with a call still running on a thread +/// nothing can reach. +/// +/// What `work` returns then is discarded, the round it belongs to being dropped whole. +async fn unless_cancelling( + stop: Pin<&mut impl Future>, + cancel: &Cancel, + work: impl Future, +) -> Option { + let mut work = pin!(work); + match select(stop, work.as_mut()).await { + Either::Left(((), _)) => { + cancel.raise(); + work.await; + None + } + Either::Right((done, _)) => Some(done), + } +} + /// The one exit from [`run_turn_until`], so no path can return reasoning to a caller. fn outcome( mut produced: Vec, diff --git a/crates/sandbx-turn/tests/turn_stop.rs b/crates/sandbx-turn/tests/turn_stop.rs index a5646448..1f1c191f 100644 --- a/crates/sandbx-turn/tests/turn_stop.rs +++ b/crates/sandbx-turn/tests/turn_stop.rs @@ -14,7 +14,7 @@ use sandbx_core::SandboxPolicy; use sandbx_event::{AgentEvent, StopReason}; use sandbx_prompt::{ContentBlock, RequestMessage, Role}; use sandbx_provider::EventStream; -use sandbx_tools::BuiltinTool; +use sandbx_tools::{BuiltinTool, ExecutionContext}; use sandbx_turn::{Compaction, Turn, TurnLimits, TurnOutcome, TurnStop, run_turn_until}; use sandbx_turn_fixture::{AllowAll, Script, call, ctx, stop, text, turn, vetted}; @@ -96,13 +96,39 @@ impl CallGate for RaisingOn<'_> { } } +/// What a stopped turn handed back, and what its gate saw of the calls. +struct Stopped { + outcome: TurnOutcome, + asked: usize, + settled: usize, +} + /// `turn` over `script`, with `root` readable, stopped inside its second `ls`. /// +/// Asserts the stopped call settled, which is the whole of what the stop owes it: the +/// call is waited for rather than left to finish unseen (#26). +fn stopped_in_second_call(script: &mut Script, turn: Turn<'_>, root: &Path) -> TurnOutcome { + let stopped = stopped_in_second_call_under( + script, + turn, + &ctx(SandboxPolicy::default().allow_read(vetted(root))), + ); + + assert_eq!(stopped.settled, 2, "the stopped call was not waited for"); + stopped.outcome +} + +/// `turn` over `script`, stopped inside the second call its gate is asked about. +/// /// On one blocking thread, which is what the plug holds. Under a timeout, since a stop /// that is never polled leaves the plug in place and the call behind it pending forever. -/// Asserts the first call alone settled: the second finishing would mean the plug did not -/// hold it, and the race #497 describes is back. -fn stopped_in_second_call(script: &mut Script, turn: Turn<'_>, root: &Path) -> TurnOutcome { +/// The plug is what keeps the second call from finishing before the stop lands: without +/// it the race #497 describes is back, and the call settles on its own. +fn stopped_in_second_call_under( + script: &mut Script, + turn: Turn<'_>, + ctx: &ExecutionContext, +) -> Stopped { // Declared before `interrupt`, so a panic below drops the plug before the runtime, // whose drop waits for every blocking thread, the plugged one included. let runtime = tokio::runtime::Builder::new_current_thread() @@ -126,7 +152,7 @@ fn stopped_in_second_call(script: &mut Script, turn: Turn<'_>, root: &Path) -> T run_turn_until( async |r| script.open(r).await, turn, - &ctx(SandboxPolicy::default().allow_read(vetted(root))), + ctx, |_| {}, &mut gate, interrupt.raised(), @@ -137,8 +163,11 @@ fn stopped_in_second_call(script: &mut Script, turn: Turn<'_>, root: &Path) -> T .expect("the stop ended the turn") .expect("a scripted turn that only reads"); - assert_eq!(gate.settled, 1, "the stopped call finished before the stop"); - outcome + Stopped { + outcome, + asked: gate.asked, + settled: gate.settled, + } } #[tokio::test] @@ -161,8 +190,49 @@ async fn a_stop_already_resolved_sends_nothing() { assert!(script.sent.is_empty(), "a request went out after the stop"); } -/// Round one alone: the stop drops the stopped call's answer, and the call is left to -/// finish rather than cancelled (#26). +/// The stop reaches a round that has more calls behind the stopped one: the stopped call +/// is waited for and settles, and nothing behind it is put to the gate or run (#26). A +/// `write` behind it, because an `ls` leaves no mark to look for. +#[test] +fn a_stop_mid_call_starts_no_call_behind_it() { + let root = tempfile::tempdir().unwrap(); + let behind = root.path().join("behind.txt"); + let ctx = ctx(SandboxPolicy::default() + .allow_read(vetted(root.path())) + .allow_write(vetted(root.path()))); + let listed = serde_json::json!({ "path": root.path().to_str().unwrap() }); + let mut script = Script::new([ + vec![ + call("ls", listed.clone()), + call("ls", listed), + call( + "write", + serde_json::json!({ "path": behind.to_str().unwrap(), "content": "behind it" }), + ), + stop(StopReason::ToolUse), + ], + vec![text("never asked"), stop(StopReason::EndTurn)], + ]); + + let stopped = stopped_in_second_call_under( + &mut script, + turn(&[], &[BuiltinTool::Ls, BuiltinTool::Write]), + &ctx, + ); + + assert_eq!(stopped.outcome.stop, TurnStop::Interrupted); + assert_eq!(stopped.settled, 2, "the stopped call was not waited for"); + assert_eq!(stopped.asked, 2, "a call behind the stop reached the gate"); + assert!(!behind.exists(), "a call behind the stop ran"); + assert!( + stopped.outcome.messages.is_empty(), + "the stopped round was kept: {:?}", + stopped.outcome.messages + ); +} + +/// Round one alone: the stop drops the stopped round's answers, the call it found running +/// having been waited for first (#26). #[test] fn a_stop_mid_call_keeps_the_rounds_before_it() { let root = tempfile::tempdir().unwrap(); From 94bd4e601b605a1450ef80e8ea224e8d1033b91c Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:38:26 +0100 Subject: [PATCH 4/8] refactor(cli): drop the unseen-call account a stop no longer owes A stop now waits for the call it stood on, so that call settles and is drawn like any other; the tracker that named it as finishing unseen had nothing left to say. The stop line and the tui help now say a running bash command is killed. Refs: #26 --- crates/sandbx-cli/src/cli.rs | 7 ++-- crates/sandbx-cli/src/tui.rs | 54 +++++-------------------- crates/sandbx-cli/src/tui_modal.rs | 2 +- crates/sandbx-cli/src/tui_tests.rs | 64 +++++++----------------------- 4 files changed, 28 insertions(+), 99 deletions(-) diff --git a/crates/sandbx-cli/src/cli.rs b/crates/sandbx-cli/src/cli.rs index ecabc430..c76cff57 100644 --- a/crates/sandbx-cli/src/cli.rs +++ b/crates/sandbx-cli/src/cli.rs @@ -97,9 +97,10 @@ pub enum Command { /// /// Interrupting costs two things, which sandbx prints again once the screen goes: /// `--session` keeps the rounds the turn finished but not the one it stopped - /// in, and a tool already running finishes — sandbx cannot cancel one - /// mid-call, and the process waits for it before it exits. A turn interrupted - /// while it is being summarised after `--max-rounds` loses only the summary. + /// in, and a tool already running is dealt with first: a `bash` command is + /// killed, any other tool call finishes, and the process waits for it before + /// it exits. A turn interrupted while it is being summarised after + /// `--max-rounds` loses only the summary. /// /// `--show-thinking` reads differently here: it draws the reasoning in the /// pane rather than on stderr. diff --git a/crates/sandbx-cli/src/tui.rs b/crates/sandbx-cli/src/tui.rs index 952fddc6..d2a982bd 100644 --- a/crates/sandbx-cli/src/tui.rs +++ b/crates/sandbx-cli/src/tui.rs @@ -16,7 +16,6 @@ //! reprint; repainted only on an event, so the screen is still for a tool call. use std::io::{self, Write}; -use std::mem; use std::sync::Mutex; use sandbx_anthropic::AnthropicClient; @@ -193,7 +192,6 @@ impl Tui { Device::Terminal => approved.to_vec(), Device::Redirected => Vec::new(), }, - running: Running::default(), over: false, }); let mut keys = Keys::listen(); @@ -205,8 +203,8 @@ impl Tui { matches!(self.run.approve, Approve::Call).then(|| Modal::new(&pane, keys.answers())); // Inside the loop rather than raced against it, so the rounds before the stop come - // back to be stored. A running tool still finishes, and the process waits on it, - // `Runtime::drop` having no timeout (#26). + // back to be stored. The stop kills a running `bash` and waits for an in-process + // tool, so the call it stood on settles, and is drawn, before the turn returns (#26). let mut seen = None; let first = run_turn_until( &mut open, @@ -274,7 +272,7 @@ impl Tui { // a write that can stall on a paused terminal while the trail is still held. pane.over = true; - let (code, outcome, closing) = match accounted(driven, mem::take(&mut pane.running)) { + let (code, outcome, closing) = match accounted(driven) { Ok(Accounted { code, kept, said }) => (Ok(code), kept, said), Err(error) => { // In the reprint only where stderr went elsewhere: `main` prints the error @@ -356,20 +354,18 @@ struct Accounted { said: Vec, } -/// Account for what the turn's futures came to, `running` being the call a stop stood on. +/// Account for what the turn's futures came to. /// /// Apart from `drive` so what a stop keeps is an assertion: the screen it is drawn on needs /// a terminal, and `SECURITY.md` claims the difference. -fn accounted(driven: Driven, running: Running) -> Result { +fn accounted(driven: Driven) -> Result { Ok(match driven { Driven::Stopped(stopped, outcome) => { let Ending { code, said } = stop(stopped, false); - // The call the dropped round stood on never settles, so this is the only line - // it gets. Accounted { code, kept: Some((outcome, stored(stopped))), - said: said.into_iter().chain(running.stopped()).collect(), + said, } } // The cap's own lines too, as the same turn stored under `--no-wrap-up` has them. The @@ -432,8 +428,8 @@ fn stop(stopped: Stopped, wrapping: bool) -> Ending { } else { format!( "{how}. Under `--session` the rounds it finished are kept and the one it stopped \ - in is lost, and a tool call already running finishes unseen: the process waits \ - for it before it exits (#26)" + in is lost. A `bash` command already running is killed; any other tool call \ + already running finishes first, and what it did stays (#26)" ) }; @@ -592,7 +588,6 @@ struct Pane { screen: Screen, /// Every line of the account, in order, for a stderr the reprint does not reach. said: Vec, - running: Running, /// Whether the turn is over, after which a line is noted and not drawn. over: bool, } @@ -610,32 +605,6 @@ impl Notes for Pane { } } -/// The call the gate let through and that has not settled yet. -/// -/// Apart from [`Pane`] so a test can drive it without a screen. -#[derive(Default)] -struct Running(Option); - -impl Running { - /// Set on `Allow` alone: a refused call settles before anything runs. - fn approved(&mut self, call: ToolCall<'_>, verdict: &ApprovalDecision) { - if matches!(verdict, ApprovalDecision::Allow) { - self.0 = Some(gate::describe(call.tool, call.input)); - } - } - - fn settled(&mut self) { - self.0 = None; - } - - /// The line a stop owes the call it stood on, if one was running. - fn stopped(self) -> Option { - self.0.map(|call| { - format!("sandbx: {call} — still running when the turn stopped, and finishes unseen") - }) - } -} - /// The pane as the seams share it: a line is noted as [`paint`] draws, not at all under a /// poisoned lock. impl Notes for &Mutex { @@ -683,11 +652,7 @@ impl CallGate for Gate<'_> { /// Delegated, never re-derived: one `--allow-tool` approving two different sets across /// two subcommands is the divergence nobody notices until a `bash` runs. fn approve(&mut self, call: ToolCall<'_>) -> ApprovalDecision { - let verdict = self.argv.approve(call); - if let Ok(mut pane) = self.pane.lock() { - pane.running.approved(call, &verdict); - } - verdict + self.argv.approve(call) } /// The same line `agent-run` writes, through the pane: `ArgvGate::settled` puts it on @@ -695,7 +660,6 @@ impl CallGate for Gate<'_> { fn settled(&mut self, call: Settled<'_>) { let line = gate::line(call); if let Ok(mut pane) = self.pane.lock() { - pane.running.settled(); pane.note(&line); } } diff --git a/crates/sandbx-cli/src/tui_modal.rs b/crates/sandbx-cli/src/tui_modal.rs index 27f39fa4..9f588abe 100644 --- a/crates/sandbx-cli/src/tui_modal.rs +++ b/crates/sandbx-cli/src/tui_modal.rs @@ -74,7 +74,7 @@ impl Operator for Modal { fn ask(&mut self, call: ToolCall<'_>) -> ApprovalDecision { // Before the `a` that covers a tool: that was the operator's word about the tool, // not a way past a stop they gave since, or a terminal that has gone. A covered - // call would otherwise run, and a running call cannot be cancelled (#26). + // call would otherwise run, and an in-process call cannot be cancelled (#26). match self.replies.stopped() { Some(Reply::Interrupted) => return deny(INTERRUPTED), Some(Reply::Closed) => return abort(CLOSED), diff --git a/crates/sandbx-cli/src/tui_tests.rs b/crates/sandbx-cli/src/tui_tests.rs index 7b497393..66160ee5 100644 --- a/crates/sandbx-cli/src/tui_tests.rs +++ b/crates/sandbx-cli/src/tui_tests.rs @@ -1,14 +1,10 @@ use super::super::render::Unfinished; use super::super::{Device, INCOMPLETE, NO_CONSENT, gate}; -use super::{ - Accounted, Driven, Ending, Running, Tui, accounted, counted, ending, given_back, held, stop, -}; +use super::{Accounted, Driven, Ending, Tui, accounted, counted, ending, given_back, held, stop}; use crate::{AgentError, Cli, Command}; use clap::Parser; -use sandbx_approval::{ApprovalDecision, ToolCall}; use sandbx_event::StopReason; use sandbx_session_message::TurnStop as StoredStop; -use sandbx_tools::BuiltinTool; use sandbx_tui_input::Stopped; use sandbx_turn::{TurnError, TurnOutcome, TurnStop}; use std::io; @@ -340,7 +336,7 @@ fn a_terminal_that_went_away_is_not_reported_as_an_interrupt() { #[test] fn every_stop_keeps_the_rounds_it_finished() { let capped = TurnStop::RoundLimit { rounds: 4 }; - let kept = |driven| accounted(driven, Running::default()).expect("not a failure"); + let kept = |driven| accounted(driven).expect("not a failure"); for (stopped, code, stored) in [ (Stopped::Pressed, 2, StoredStop::Interrupted), @@ -379,6 +375,11 @@ fn every_stop_keeps_the_rounds_it_finished() { said[0].contains("the one it stopped in is lost"), "{said:?}" ); + // What the operator is told about a call under way: the mechanism's whole claim. + assert!( + said[0].contains("`bash` command already running is killed"), + "{said:?}" + ); } } @@ -386,17 +387,14 @@ fn every_stop_keeps_the_rounds_it_finished() { /// the turn kept being the same one `--no-wrap-up` would have kept. #[test] fn a_stopped_wrap_up_still_names_a_cut_round() { - let said = accounted( - Driven::StoppedWrapping( - Stopped::Pressed, - outcome( - TurnStop::RoundLimit { rounds: 2 }, - Some(StopReason::MaxTokens), - ), - 2, + let said = accounted(Driven::StoppedWrapping( + Stopped::Pressed, + outcome( + TurnStop::RoundLimit { rounds: 2 }, + Some(StopReason::MaxTokens), ), - Running::default(), - ) + 2, + )) .expect("not a failure") .said; assert_eq!(said.len(), 3, "{said:?}"); @@ -461,37 +459,3 @@ fn compact_at_is_one_flag_under_both() { ); assert_eq!(drawn.budget_tokens, 5000); } - -/// The call a stop lands on never settles, so the tracker is the only account it gets; one -/// the gate refused settled before it could be running. -#[test] -fn only_a_call_let_through_and_unsettled_is_named_on_a_stop() { - let input = serde_json::json!({ "command": "sleep 20" }); - let call = ToolCall { - tool: BuiltinTool::Bash, - id: "toolu_1", - input: &input, - }; - - let mut running = Running::default(); - running.approved(call, &ApprovalDecision::Allow); - let named = running.stopped().expect("an approved call is running"); - assert!( - named.starts_with("sandbx: bash sleep 20 — still running"), - "{named}" - ); - - let mut refused = Running::default(); - refused.approved( - call, - &ApprovalDecision::Deny { - reason: "no".to_string(), - }, - ); - assert_eq!(refused.stopped(), None); - - let mut finished = Running::default(); - finished.approved(call, &ApprovalDecision::Allow); - finished.settled(); - assert_eq!(finished.stopped(), None); -} From 3c858bc7c01af3d9e94bafa9504272e9d1d676a3 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:38:31 +0100 Subject: [PATCH 5/8] docs: a stop kills a running bash call SECURITY.md claims only a spawned command can be interrupted, and says what the six in-process tools and a killed command leave behind. Refs: #26 --- README.md | 7 ++++--- SECURITY.md | 16 +++++++++++----- context/decision-approval-gate.md | 16 ++++++++-------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 502a7437..18acb7ee 100644 --- a/README.md +++ b/README.md @@ -306,9 +306,10 @@ leave you answering one call blind. One question, one answer, then the process ends. `sandbx tui` takes the same flags and draws that turn on a screen instead of streaming it, where ctrl-c or escape ends it mid-flight — `--session` keeping the rounds it finished but not -the one it stopped in, and a running tool call left to finish unseen. When the -turn ends the screen goes and the transcript is printed in its place, so the -answer stays in your scrollback. It needs a terminal on stdout, so it refuses a +the one it stopped in, and a running `bash` command killed, where any other tool +call finishes first. When the turn ends the screen goes and the transcript is +printed in its place, so the answer stays in your scrollback. It needs a +terminal on stdout, so it refuses a piped run. `--approve call` asks on the screen, in rows under the transcript, and one key answers: `y`, `n` or `a`, escape or ctrl-c refusing the call and ending the turn ([#225](https://github.com/danczw/sandbx/issues/225)): diff --git a/SECURITY.md b/SECURITY.md index aa46066a..4995879f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -332,13 +332,15 @@ architecture the filter gates on. round it finished, under a stop naming an interrupt or a hung-up terminal, and loses the round it stopped in whole: whatever it streamed, including the part of the answer you read, and every tool call it asked for. The calls it had - already run did run, and one still running finishes, but the transcript - records the effect of neither. A stop before the first round finishes stores + already run did run, and one still running is killed if it is a `bash` and + finishes if it is not, but the transcript records the effect of neither. A + stop before the first round finishes stores nothing, the prompt included, and leaves no trace in the session that a call ran ([#271](https://github.com/danczw/sandbx/issues/271)). Where a round was stored, resuming says so on stderr; the model is told nothing of it. What the turn showed is printed back to the terminal once the screen is gone, and the - running call is named on stderr but never its result. Where the terminal is + call a stop stood on is named there as it ended, run or cancelled, but never + with its output. Where the terminal is what went away, only a redirected stderr still has either. A stop during the wrap-up round after `--max-rounds` keeps the rounds before it as the cap left them, and loses only the wrap-up's prose @@ -376,8 +378,12 @@ architecture the filter gates on. Nothing else is capped: no CPU bound, no memory bound, no limit on processes spawned. A fork bomb is unbounded while the call lasts; bounded is that it does not outlive it. -- **A running tool call cannot be interrupted.** Only its own deadline stops it; - no way to cancel one from outside +- **Only a spawned command can be interrupted.** A stop on `tui` (ctrl-c, + escape, or a hung-up terminal) kills a running `bash` command with the kill a + timeout makes, and starts no call behind it. The six in-process tools cannot + be cancelled: one already running finishes, so a `write` under way lands, and + the stop waits for it — for as long as a stalled filesystem takes. Nothing a + killed command had already done is undone ([#26](https://github.com/danczw/sandbx/issues/26)). - **A bound on the harness's own loop is not a containment claim.** A turn is bounded — `TurnLimits::max_rounds` caps the requests one turn may make of the diff --git a/context/decision-approval-gate.md b/context/decision-approval-gate.md index 8bda3192..d66c5995 100644 --- a/context/decision-approval-gate.md +++ b/context/decision-approval-gate.md @@ -16,10 +16,10 @@ that ask it and implement it. In `answer_calls`, between resolving the name and `spawn_blocking`. A blocking task cannot be cancelled (#26): dropping the `JoinHandle` leaves the -task running to completion. So a gate consulted *concurrently* with the call — -`select!` on a decision and a `JoinHandle` — would answer "denied" about a `write` -that had already landed. Before the spawn is the only position where a refusal -means anything. +task running to completion, and only a `bash` has a kill to reach for besides. +So a gate consulted *concurrently* with the call — `select!` on a decision and a +`JoinHandle` — would answer "denied" about a `write` that had already landed. +Before the spawn is the only position where a refusal means anything. Two refusals sit *above* the gate, so a caller's closure never sees either and never has to invent a verdict for one: @@ -203,8 +203,8 @@ It also keeps less. The abort is a `TurnStop`, so the turn returns and its messages, usage and `withheld` are stored. A `tui` stop ends the turn through `run_turn_until`, which keeps the rounds before it and drops the one it lands in, so the round the operator was watching is stored nowhere, for a hangup as -for an interrupt (#271). A tool call already running finishes unseen (#26) on -both surfaces alike. +for an interrupt (#271). A tool call already running is killed if it is a `bash` +and finishes if it is not (#26), on both surfaces alike. ## Deny by default, and the honest claim @@ -364,8 +364,8 @@ descriptor no task feeds, and nothing else is in flight during `approve`, since - **A stop given earlier outranks an `a`.** Before the blanket is read, the gate asks whether the turn was interrupted or the terminal has gone, and refuses or aborts at once. An `a` was the operator's word about a tool, not a way past a - stop they gave since: a covered call would otherwise run, and a running call - cannot be cancelled (#26). + stop they gave since: a covered call would otherwise run, and an in-process + call cannot be cancelled (#26). - **Refusal outranks an answer already pressed.** An interrupt beats a queued `y`, and a hangup beats both. Each leaves the call refused, the direction a doubtful reading must lean. From 1ed2f6b75bda9782f538263666a3d0907140dd3f Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:42:46 +0100 Subject: [PATCH 6/8] test(helper): a cancel kills a descendant that left the process group SECURITY.md says a stop kills what a timeout kills; this pins it for the setsid escapee the timeout test already covers. Refs: #26 --- crates/sandbx-helper/tests/command.rs | 46 +++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crates/sandbx-helper/tests/command.rs b/crates/sandbx-helper/tests/command.rs index 654a0fc7..9fbf321d 100644 --- a/crates/sandbx-helper/tests/command.rs +++ b/crates/sandbx-helper/tests/command.rs @@ -468,6 +468,52 @@ fn a_descendant_that_leaves_the_group_is_still_killed() { ); } +/// The same escapee, stopped from outside: `SECURITY.md` claims a cancel kills what a +/// timeout kills, and it is the shared `kill_group` that makes that true. +#[test] +fn a_descendant_that_leaves_the_group_is_still_killed_by_a_cancel() { + let dir = scratch(); + let canary = dir.path().join("canary"); + let cancel = Cancel::new(); + let stopper = raise_soon(&cancel); + let started = Instant::now(); + + let result = SandboxedCommand::new( + "/bin/sh", + SandboxPolicy::default() + .allow_system_executables() + .allow_read(vetted("/dev/null")) + .allow_write(vetted("/dev/null")) + .allow_write(vetted(dir.path())), + ) + .arg("-c") + .arg(format!( + "setsid sh -c 'sleep 2; echo alive > {}' & sleep 30", + canary.display() + )) + .helper(env!("CARGO_BIN_EXE_sandbx-helper")) + .cancel_on(cancel) + .output(); + + let elapsed = started.elapsed(); + stopper.join().unwrap(); + + assert!( + matches!(result, Err(SandboxError::Cancelled)), + "expected a cancellation, got: {result:?}" + ); + assert!( + elapsed < Duration::from_secs(5), + "a setsid escapee held the cancelled call open for {elapsed:?}" + ); + + thread::sleep(Duration::from_secs(4)); + assert!( + !canary.exists(), + "a setsid descendant escaped the cancel and outlived the tool call" + ); +} + /// An `exec` from a second thread sheds the parent death signal; the init still dies, timed. #[test] fn a_command_that_sheds_its_lifetime_still_dies() { From 70129b14cf9b1013968eb934cf8a16590f1a6a0f Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 11:42:58 +0100 Subject: [PATCH 7/8] docs: say what a cancel leaves out A run cancelled before it starts leaves no record, the turn replaces a caller's token with its own, and a cancelled round must be dropped. Refs: #26 --- crates/sandbx-core/src/sandboxed_command.rs | 5 ++++- crates/sandbx-turn/src/tools.rs | 4 ++++ crates/sandbx-turn/src/turn.rs | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/sandbx-core/src/sandboxed_command.rs b/crates/sandbx-core/src/sandboxed_command.rs index 27911eae..a8489ebf 100644 --- a/crates/sandbx-core/src/sandboxed_command.rs +++ b/crates/sandbx-core/src/sandboxed_command.rs @@ -172,7 +172,8 @@ impl SandboxedCommand { /// or — with a [`timeout`](Self::timeout) set — until the limit expires, which kills it /// and returns [`SandboxError::TimedOut`]. A [`cancel_on`](Self::cancel_on) token raised /// does the same and returns [`SandboxError::Cancelled`] — before anything starts, where - /// it was raised already. A helper stage that refused returns + /// it was raised already, and then with no record on the trail, as for a policy refused + /// in [`command_line`](Self::command_line). A helper stage that refused returns /// [`SandboxError::HelperRefused`] rather than the status it exited with, which is /// indistinguishable from the command's own. /// @@ -382,6 +383,8 @@ fn run_supervised( match child.try_wait() { // Ahead of the token: a command that already exited was not stopped by it. Ok(Some(status)) => break Ok(status), + // Ahead of the deadline: when both are due, the caller's act is the cause worth + // recording, not the overrun it happened to coincide with. Ok(None) if cancel.is_some_and(Cancel::is_raised) => { break Err(SandboxError::Cancelled); } diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index 1b174ddc..c67c43b1 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -17,6 +17,10 @@ use crate::TurnError; /// every other task — hence `spawn_blocking`, whose uncancellability `run_turn` documents. /// Every answered block reports to [`CallGate::settled`], including the two refusals above /// the gate, which the gate's own verdict cannot account for. +/// +/// A cancelled `ctx` ends the loop early, leaving a `tool_use` with no `tool_result`: the +/// caller must drop the round, which `run_turn_until` does and is the only caller to hold +/// the token. pub(crate) async fn answer_calls( blocks: &[ContentBlock], ctx: &ExecutionContext, diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index 0a0ed3ad..351cb068 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -240,6 +240,10 @@ where /// [`run_turn`], ended as [`TurnStop::Interrupted`] once `stop` resolves. /// +/// The turn owns the stop: it installs a token of its own on `ctx` for the length of the +/// turn, replacing one the caller set with [`ExecutionContext::with_cancel`], so `stop` is +/// the only way to cancel a call from here. +/// /// Raced against each round's request and its tool calls, `stop` first, so a stop that /// has already resolved opens no further request. The round it lands in is dropped whole, /// calls already answered in it included. A call already running is waited for rather than From ba0519c2c15af07ddced2482b58b8b41891c6c39 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 12:09:22 +0100 Subject: [PATCH 8/8] docs: trim two comments the cancel branch repeated Refs: #26 --- crates/sandbx-turn/src/tools.rs | 3 +-- crates/sandbx-turn/src/turn.rs | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index c67c43b1..0ccad249 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -36,8 +36,7 @@ where let mut aborted: Option = None; for block in blocks { - // A stop raised during the last call: nothing behind it is asked about or run. The - // round is dropped whole by the caller, so no `tool_result` is owed for these. + // Nothing behind a stopped call is asked about or run. if ctx.cancelled() { break; } diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index 351cb068..ae304097 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -274,9 +274,8 @@ where G: CallGate, { let mut stop = pin!(stop); - // One per turn, never reused: raised for good, so a token carried over would stop the - // next turn's first call before it started. The context is cloned to carry it, which - // leaves the caller's untouched. + // One per turn: raised for good, so a carried-over token would stop the next turn's + // first call before it started. Cloned, so the caller's context is untouched. let cancel = Cancel::new(); let ctx = &ctx.clone().with_cancel(cancel.clone()); // Built once: the offered set does not change between rounds.