From d61d9d9fd351ee96267fe225fdb01b42b76b5341 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 13:56:55 +0100 Subject: [PATCH 01/12] feat(tools): stop a grep between files when the turn is stopped --- crates/sandbx-tools/src/tools_grep.rs | 9 ++++++ crates/sandbx-tools/tests/search.rs | 43 ++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/sandbx-tools/src/tools_grep.rs b/crates/sandbx-tools/src/tools_grep.rs index ab7d0855..22a65d4c 100644 --- a/crates/sandbx-tools/src/tools_grep.rs +++ b/crates/sandbx-tools/src/tools_grep.rs @@ -39,6 +39,15 @@ pub(crate) fn execute(input: GrepInput, ctx: &ExecutionContext) -> Result= ctx.limits().max_bytes_scanned { diff --git a/crates/sandbx-tools/tests/search.rs b/crates/sandbx-tools/tests/search.rs index 1fac77c0..eeb1ca8f 100644 --- a/crates/sandbx-tools/tests/search.rs +++ b/crates/sandbx-tools/tests/search.rs @@ -9,7 +9,7 @@ #![allow(clippy::disallowed_methods)] 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::os::unix::fs as unix_fs; @@ -83,6 +83,47 @@ fn grep_reports_no_matches_rather_than_failing() { assert_eq!(out.content(), "no matches"); } +/// A stop is an error and not a short listing: a search that gave up must not read as one +/// that found nothing. +#[test] +fn grep_stops_between_files_once_cancelled() { + let root = tempfile::tempdir().unwrap(); + fs::write(root.path().join("a.txt"), "needle\n").unwrap(); + fs::write(root.path().join("b.txt"), "needle\n").unwrap(); + + let cancel = Cancel::new(); + let ctx = context(SandboxPolicy::default().allow_read(vetted(root.path()))) + .with_cancel(cancel.clone()); + cancel.raise(); + + let result = BuiltinTool::Grep.execute( + json!({ "path": root.path().to_str().unwrap(), "pattern": "needle" }), + &ctx, + ); + + assert!( + matches!(result, Err(ToolError::Cancelled { .. })), + "expected a cancellation, got: {result:?}" + ); +} + +#[test] +fn grep_is_unchanged_by_a_token_nobody_raised() { + let root = tempfile::tempdir().unwrap(); + fs::write(root.path().join("a.txt"), "needle\n").unwrap(); + + let ctx = context(SandboxPolicy::default().allow_read(vetted(root.path()))) + .with_cancel(Cancel::new()); + let out = BuiltinTool::Grep + .execute( + json!({ "path": root.path().to_str().unwrap(), "pattern": "needle" }), + &ctx, + ) + .unwrap(); + + assert!(out.content().contains("a.txt:1"), "got: {}", out.content()); +} + #[cfg(unix)] #[test] fn grep_does_not_follow_a_symlink_out_of_the_root() { From 6ecd945440ecf200a545a31b5a03007e092b39d8 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 13:58:22 +0100 Subject: [PATCH 02/12] docs: say a stop reaches a grep between files --- SECURITY.md | 20 +++++++++++--------- context/decision-approval-gate.md | 8 +++++--- crates/sandbx-tools/src/execution_context.rs | 4 ++-- crates/sandbx-turn/src/turn.rs | 6 +++--- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 4995879f..d5502f50 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -332,8 +332,9 @@ 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 is killed if it is a `bash` and - finishes if it is not, but the transcript records the effect of neither. A + already run did run, and one still running is killed if it is a `bash`, + stopped if it is a `grep`, and finishes if it is neither, but the transcript + records the effect of none. 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 @@ -378,13 +379,14 @@ 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. -- **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)). +- **Only a spawned command and a `grep` 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, stops a `grep` before its next file, and starts no + call behind either. The other five 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, and a `grep` is held the + same way inside the file it is reading. 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 model, `TurnLimits::stream_timeout` how long one of them may spend streaming diff --git a/context/decision-approval-gate.md b/context/decision-approval-gate.md index d66c5995..968c1d0e 100644 --- a/context/decision-approval-gate.md +++ b/context/decision-approval-gate.md @@ -16,7 +16,8 @@ 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, and only a `bash` has a kill to reach for besides. +task running to completion, and only a `bash` and a `grep` can be stopped +besides, a `write` not at all. 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. @@ -203,8 +204,9 @@ 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 is killed if it is a `bash` -and finishes if it is not (#26), on both surfaces alike. +for an interrupt (#271). A tool call already running is killed if it is a `bash`, +stopped if it is a `grep`, and finishes if it is neither (#26), on both surfaces +alike. ## Deny by default, and the honest claim diff --git a/crates/sandbx-tools/src/execution_context.rs b/crates/sandbx-tools/src/execution_context.rs index 048eb0d3..9f382286 100644 --- a/crates/sandbx-tools/src/execution_context.rs +++ b/crates/sandbx-tools/src/execution_context.rs @@ -100,8 +100,8 @@ impl ExecutionContext { /// 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. + /// them. A spawned command is killed, and `grep` stops between files; the other + /// in-process tools are bounded by work and finish what they started (#26). #[must_use] pub fn with_cancel(mut self, cancel: Cancel) -> Self { self.cancel = cancel; diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index ae304097..eb87df4b 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -247,9 +247,9 @@ 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 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. +/// abandoned, and reported to [`CallGate::settled`] as it ends: a `bash` is killed and a +/// `grep` stops between files, but the other in-process tools cannot be cancelled (#26), so +/// one 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 From 3ff1af03f1dc54a4281d59122f3041731590ae95 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:02:45 +0100 Subject: [PATCH 03/12] fix(tools): do not say a stopped grep was killed --- README.md | 4 ++-- SECURITY.md | 4 ++-- crates/sandbx-cli/src/agent_gate.rs | 2 +- crates/sandbx-cli/src/agent_gate_tests.rs | 8 ++++---- crates/sandbx-tools/src/tool_error.rs | 7 ++++--- crates/sandbx-tools/tests/search.rs | 3 +++ 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 18acb7ee..1f652c2d 100644 --- a/README.md +++ b/README.md @@ -306,8 +306,8 @@ 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 `bash` command killed, where any other tool -call finishes first. When the turn ends the screen goes and the transcript is +the one it stopped in, and a running `bash` command killed and a `grep` stopped +before its next file, 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, diff --git a/SECURITY.md b/SECURITY.md index d5502f50..bb4ed08d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -381,8 +381,8 @@ architecture the filter gates on. not outlive it. - **Only a spawned command and a `grep` 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, stops a `grep` before its next file, and starts no - call behind either. The other five in-process tools cannot be cancelled: one + the kill a timeout makes, stops a `grep` before its next file (not during the + directory walk before it), and starts no call behind either. The other five 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, and a `grep` is held the same way inside the file it is reading. Nothing a killed command had already diff --git a/crates/sandbx-cli/src/agent_gate.rs b/crates/sandbx-cli/src/agent_gate.rs index 53881bd8..1c495a1c 100644 --- a/crates/sandbx-cli/src/agent_gate.rs +++ b/crates/sandbx-cli/src/agent_gate.rs @@ -194,7 +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"), + ToolError::Cancelled { .. } => format!("{head} — cancelled"), }, } } diff --git a/crates/sandbx-cli/src/agent_gate_tests.rs b/crates/sandbx-cli/src/agent_gate_tests.rs index 680b0051..53673b0f 100644 --- a/crates/sandbx-cli/src/agent_gate_tests.rs +++ b/crates/sandbx-cli/src/agent_gate_tests.rs @@ -359,17 +359,17 @@ 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. +/// The operator who pressed ctrl-C reads this line, so it must say the call was cancelled +/// and not that it ran or that a limit was hit. Not "killed": a `grep` is stopped, not killed. #[test] -fn a_cancelled_call_is_reported_as_killed() { +fn a_cancelled_call_is_reported_as_cancelled() { 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" + "bash sleep 80 — cancelled" ); } diff --git a/crates/sandbx-tools/src/tool_error.rs b/crates/sandbx-tools/src/tool_error.rs index 354d4824..60636fd0 100644 --- a/crates/sandbx-tools/src/tool_error.rs +++ b/crates/sandbx-tools/src/tool_error.rs @@ -44,10 +44,11 @@ pub enum ToolError { after: Duration, }, - /// The operation was killed because the caller stopped waiting for it. + /// The operation was stopped because the caller stopped waiting for it: a command is + /// killed, a `grep` returns before its next file. /// /// 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. + /// narrow or lengthen, and a command may have changed state before it died. Cancelled { /// What was attempted. subject: String, @@ -66,7 +67,7 @@ impl fmt::Display for ToolError { write!(f, "{subject} timed out after {after:?} and was killed") } Self::Cancelled { subject } => { - write!(f, "{subject} was cancelled and killed before it finished") + write!(f, "{subject} was cancelled before it finished") } } } diff --git a/crates/sandbx-tools/tests/search.rs b/crates/sandbx-tools/tests/search.rs index eeb1ca8f..14127bc5 100644 --- a/crates/sandbx-tools/tests/search.rs +++ b/crates/sandbx-tools/tests/search.rs @@ -105,6 +105,9 @@ fn grep_stops_between_files_once_cancelled() { matches!(result, Err(ToolError::Cancelled { .. })), "expected a cancellation, got: {result:?}" ); + // The model and the operator both read this, and nothing was killed. + let message = result.unwrap_err().to_string(); + assert!(!message.contains("killed"), "got: {message}"); } #[test] From 1b6370a9b8e09c52c22a0fe4cf41cb8b0a44dfa8 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:06:42 +0100 Subject: [PATCH 04/12] docs: wrap the lines the grep change left over width --- README.md | 6 +++--- SECURITY.md | 11 ++++++----- context/decision-approval-gate.md | 6 +++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1f652c2d..b84eb6f5 100644 --- a/README.md +++ b/README.md @@ -307,9 +307,9 @@ 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 `bash` command killed and a `grep` stopped -before its next file, 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 +before its next file, 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 bb4ed08d..1b7aecb2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -382,11 +382,12 @@ architecture the filter gates on. - **Only a spawned command and a `grep` 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, stops a `grep` before its next file (not during the - directory walk before it), and starts no call behind either. The other five 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, and a `grep` is held the - same way inside the file it is reading. Nothing a killed command had already - done is undone ([#26](https://github.com/danczw/sandbx/issues/26)). + directory walk before it), and starts no call behind either. The other five + 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, and a `grep` is held the same way inside the file it is + reading. 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 model, `TurnLimits::stream_timeout` how long one of them may spend streaming diff --git a/context/decision-approval-gate.md b/context/decision-approval-gate.md index 968c1d0e..ff096ff8 100644 --- a/context/decision-approval-gate.md +++ b/context/decision-approval-gate.md @@ -204,9 +204,9 @@ 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 is killed if it is a `bash`, -stopped if it is a `grep`, and finishes if it is neither (#26), on both surfaces -alike. +for an interrupt (#271). A tool call already running is killed if it is a +`bash`, stopped if it is a `grep`, and finishes if it is neither (#26), on both +surfaces alike. ## Deny by default, and the honest claim From 29fee435014659d58987685b36af97ed25abba1b Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:20:42 +0100 Subject: [PATCH 05/12] feat(turn): keep the round a stop lands in, every call answered --- crates/sandbx-session-bridge/src/lib.rs | 16 +-- .../src/session_bridge_tests.rs | 8 +- crates/sandbx-session-message/src/lib.rs | 3 +- crates/sandbx-turn/src/tools.rs | 57 +++++++++- crates/sandbx-turn/src/turn.rs | 63 +++++++---- crates/sandbx-turn/tests/turn_stop.rs | 106 +++++++++++++++--- 6 files changed, 199 insertions(+), 54 deletions(-) diff --git a/crates/sandbx-session-bridge/src/lib.rs b/crates/sandbx-session-bridge/src/lib.rs index 1f83ec08..9da452a3 100644 --- a/crates/sandbx-session-bridge/src/lib.rs +++ b/crates/sandbx-session-bridge/src/lib.rs @@ -97,17 +97,17 @@ fn resume_note(pending: bool, stop: Option) -> Option<&'static str> { "its last turn ended with no one left to approve a call; this prompt goes to \ the model beside a tool call it never answered" } - // The dropped round is named because its tool calls may have run (#26), and - // nothing the model is sent says so. + // The round is kept with every call answered (#26): a call running at the stop shows + // its output withheld, so the operator is told what that record leaves out. Some(TurnStop::Interrupted) => { - "its last turn was interrupted, and the round it stopped in is not stored, nor \ - what any tool call in it did; this prompt goes to the model beside a tool call it \ - never answered" + "its last turn was interrupted; the round it stopped in is stored, a call running \ + at the stop with its output withheld and none behind it run; this prompt goes to \ + the model beside those results" } Some(TurnStop::HungUp) => { - "its last turn stopped when its terminal went away, and the round it stopped in \ - is not stored, nor what any tool call in it did; this prompt goes to the model \ - beside a tool call it never answered" + "its last turn stopped when its terminal went away; the round it stopped in is \ + stored, a call running at the stop with its output withheld and none behind it \ + run; this prompt goes to the model beside those results" } // An answer beside an unanswered call is a hand edit, so it says no more than no // record does. diff --git a/crates/sandbx-session-bridge/src/session_bridge_tests.rs b/crates/sandbx-session-bridge/src/session_bridge_tests.rs index 70087c21..392d823b 100644 --- a/crates/sandbx-session-bridge/src/session_bridge_tests.rs +++ b/crates/sandbx-session-bridge/src/session_bridge_tests.rs @@ -207,9 +207,10 @@ fn an_unanswered_call_is_explained_by_the_stop_recorded() { assert!(!note(None).contains("rounds")); } -/// A hangup is not reported as a keypress, and both name the round they lost (#271). +/// A hangup is not reported as a keypress, and both say the round was kept and what the +/// call the stop landed on left out (#271, #26). #[test] -fn a_stopped_turn_names_its_stop_and_what_it_lost() { +fn a_stopped_turn_names_its_stop_and_what_it_kept() { let pressed = resume_note(true, Some(TurnStop::Interrupted)).expect("a note"); let gone = resume_note(true, Some(TurnStop::HungUp)).expect("a note"); @@ -217,7 +218,8 @@ fn a_stopped_turn_names_its_stop_and_what_it_lost() { assert!(gone.contains("terminal went away"), "{gone}"); assert!(!gone.contains("interrupted"), "{gone}"); for note in [pressed, gone] { - assert!(note.contains("is not stored"), "{note}"); + assert!(note.contains("output withheld"), "{note}"); + assert!(!note.contains("not stored"), "{note}"); assert!(!note.contains("rounds"), "{note}"); } } diff --git a/crates/sandbx-session-message/src/lib.rs b/crates/sandbx-session-message/src/lib.rs index 05e79904..b9094e63 100644 --- a/crates/sandbx-session-message/src/lib.rs +++ b/crates/sandbx-session-message/src/lib.rs @@ -93,7 +93,8 @@ pub enum TurnStop { Summarised, /// The gate ended the turn, having lost whatever it decides with. GateAborted, - /// The operator stopped the turn, and the round it was in is not stored. + /// The operator stopped the turn. A round it stopped mid-stream is not stored; one it + /// stopped mid-call is, every call answered. Interrupted, /// The terminal went away, which stopped the turn as [`Interrupted`](Self::Interrupted) /// does. diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index 0ccad249..cc3b60a8 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -5,7 +5,7 @@ use sandbx_approval::{ApprovalDecision, CallGate, Outcome, Settled, ToolCall}; use sandbx_prompt::{ContentBlock, ToolDefinition}; -use sandbx_tools::{BuiltinTool, ExecutionContext}; +use sandbx_tools::{BuiltinTool, ExecutionContext, ToolError}; use crate::TurnError; @@ -105,22 +105,52 @@ where name: name.to_string(), })?; + // Read after the join, never before the spawn: a call that finished a moment before + // the stop is withheld too, which errs toward saying less of what the operator stopped. + let stopped = ctx.cancelled(); results.push(match outcome { Ok(output) => { gate.settled(settle(Some(tool), Outcome::Ran)); - ContentBlock::ToolResult { - tool_use_id: id.clone(), - content: output.into_content(), - is_error: None, + if stopped { + refused(id, FINISHED_AFTER_STOP.to_string()) + } else { + ContentBlock::ToolResult { + tool_use_id: id.clone(), + content: output.into_content(), + is_error: None, + } } } Err(error) => { gate.settled(settle(Some(tool), Outcome::Errored(&error))); - refused(id, error.to_string()) + match (stopped, &error) { + (false, _) => refused(id, error.to_string()), + (true, ToolError::Cancelled { .. }) => refused(id, CANCELLED.to_string()), + (true, _) => refused(id, FINISHED_AFTER_STOP.to_string()), + } } }); } + // A stop ends the loop above early. Answered here, in the order asked, because a + // `tool_use` with no `tool_result` is a transcript no provider takes back, and a + // stopped round is stored and resumed. + // By position and not by id: each call leaves exactly one result, in order, so the + // unanswered are the suffix. + let behind: Vec<&String> = blocks + .iter() + .filter_map(|block| match block { + ContentBlock::ToolUse { id, .. } => Some(id), + _ => None, + }) + .skip(results.len()) + .collect(); + results.extend( + behind + .into_iter() + .map(|id| refused(id, NOT_RUN.to_string())), + ); + Ok(Answers { results, aborted: aborted.is_some(), @@ -136,6 +166,21 @@ pub(crate) struct Answers { pub(crate) aborted: bool, } +// What a stopped round stores in place of a call's own result. Fixed text and never a +// path, a command or a tool's output: a resume sends these to the provider, and the +// operator stopped before reading any of it. + +/// A call behind the one the stop landed on, never put to the gate or run. +const NOT_RUN: &str = "not run: the turn was stopped"; + +/// A call the stop killed or interrupted before it could finish. +const CANCELLED: &str = "cancelled: the turn was stopped before it finished"; + +/// A call that finished once the stop was raised. Says its effect stands, because a `write` +/// that landed and was recorded as not run would be done again. +const FINISHED_AFTER_STOP: &str = "was running when the turn was stopped; it finished, its \ + effect stands, its output was withheld"; + fn refused(id: &str, content: String) -> ContentBlock { ContentBlock::ToolResult { tool_use_id: id.to_string(), diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index eb87df4b..a6ecd6b3 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -348,13 +348,7 @@ where })? }; let Some(round) = unless(stop.as_mut(), streamed).await else { - return Ok(outcome( - produced, - usage, - streamed_cut, - TurnStop::Interrupted, - last_stop, - )); + return Ok(interrupted(produced, usage, streamed_cut, last_stop)); }; let round = round?; streamed_cut = cut; @@ -380,16 +374,20 @@ 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_cancelling(stop.as_mut(), &cancel, answered).await else { - return Ok(outcome( - produced, - usage, - streamed_cut, - TurnStop::Interrupted, - last_stop, - )); + let (answers, stopped) = unless_cancelling(stop.as_mut(), &cancel, answered).await; + let answers = match answers { + Ok(answers) => answers, + // A panicked tool during a stop: the round has no answers to keep. + Err(_) if stopped => return Ok(interrupted(produced, usage, streamed_cut, last_stop)), + Err(error) => return Err(error), }; - let Answers { results, aborted } = answers?; + let Answers { results, aborted } = answers; + + // No result means no call in the round, so nothing a stop could have left unsaid: it + // is dropped as it was before calls were kept, not stored as a bare assistant turn. + if stopped && results.is_empty() { + return Ok(interrupted(produced, usage, streamed_cut, last_stop)); + } produced.push(RequestMessage { role: Role::Assistant, @@ -413,7 +411,12 @@ where }); // After the push, so the transcript ends on this round's results, as `RoundLimit` - // already hands back. + // already hands back. Before `aborted`: a stop that lands in the round the gate + // ended is the stop, not the abort. + if stopped { + return Ok(interrupted(produced, usage, streamed_cut, last_stop)); + } + if aborted { return Ok(outcome( produced, @@ -453,23 +456,39 @@ async fn unless( /// 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. +/// `work`'s output and whether it was stopped. A stopped `work` has answered every call it was +/// asked, so the round it belongs to is kept. async fn unless_cancelling( stop: Pin<&mut impl Future>, cancel: &Cancel, work: impl Future, -) -> Option { +) -> (T, bool) { let mut work = pin!(work); match select(stop, work.as_mut()).await { Either::Left(((), _)) => { cancel.raise(); - work.await; - None + (work.await, true) } - Either::Right((done, _)) => Some(done), + Either::Right((done, _)) => (done, false), } } +/// [`TurnStop::Interrupted`], over the rounds kept so far. +fn interrupted( + produced: Vec, + usage: Option, + streamed_cut: usize, + last_stop: Option, +) -> TurnOutcome { + outcome( + produced, + usage, + streamed_cut, + TurnStop::Interrupted, + last_stop, + ) +} + /// 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 1f1c191f..796508d7 100644 --- a/crates/sandbx-turn/tests/turn_stop.rs +++ b/crates/sandbx-turn/tests/turn_stop.rs @@ -1,5 +1,6 @@ //! Public contract of `run_turn_until`: what a stopped turn keeps. +use std::fs; use std::future::{self, Future}; use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; @@ -29,6 +30,20 @@ fn listing(root: &tempfile::TempDir) -> Vec { ] } +/// The `(content, is_error)` of each `tool_result` in a user turn. +fn answers(message: &RequestMessage) -> Vec<(&str, Option)> { + message + .content + .iter() + .map(|block| match block { + ContentBlock::ToolResult { + content, is_error, .. + } => (content.as_str(), *is_error), + other => panic!("not a tool_result: {other:?}"), + }) + .collect() +} + /// A stop raised inside a tool call, landing before that call can finish (#497). /// /// Raising a flag alone races the call: an `ls` that completes before its `JoinHandle` is @@ -193,9 +208,14 @@ async fn a_stop_already_resolved_sends_nothing() { /// 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. +/// +/// The round is kept with every call answered: the one that finished before the stop says +/// what it found, the stopped one says only that it finished, and the one behind it says it +/// did not run. #[test] fn a_stop_mid_call_starts_no_call_behind_it() { let root = tempfile::tempdir().unwrap(); + fs::write(root.path().join("seen.txt"), "").unwrap(); let behind = root.path().join("behind.txt"); let ctx = ctx(SandboxPolicy::default() .allow_read(vetted(root.path())) @@ -224,15 +244,75 @@ fn a_stop_mid_call_starts_no_call_behind_it() { 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"); + + let [assistant, user] = &stopped.outcome.messages[..] else { + panic!( + "expected the stopped round, got {:?}", + stopped.outcome.messages + ); + }; + assert_eq!(assistant.role, Role::Assistant); + let [before, during, after] = &answers(user)[..] else { + panic!("a result for each of three calls, got {user:?}"); + }; + assert!(before.0.contains("seen.txt"), "got {before:?}"); + assert_eq!(before.1, None); + assert!(during.0.contains("withheld"), "got {during:?}"); assert!( - stopped.outcome.messages.is_empty(), - "the stopped round was kept: {:?}", - stopped.outcome.messages + !during.0.contains("seen.txt"), + "the stopped call's output got through" ); + assert_eq!(during.1, Some(true)); + assert!(after.0.contains("not run"), "got {after:?}"); + assert_eq!(after.1, Some(true)); } -/// Round one alone: the stop drops the stopped round's answers, the call it found running -/// having been waited for first (#26). +/// A `write` the stop landed on is waited for and lands (#26), and what it says is not +/// stored: the operator stopped before reading it, and a resume would send it on. +#[test] +fn a_stop_mid_write_lets_it_land_and_withholds_what_it_says() { + let root = tempfile::tempdir().unwrap(); + let target = root.path().join("landed.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), + call( + "write", + serde_json::json!({ "path": target.to_str().unwrap(), "content": "landed" }), + ), + 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!(fs::read_to_string(&target).unwrap(), "landed"); + let [_, user] = &stopped.outcome.messages[..] else { + panic!( + "expected the stopped round, got {:?}", + stopped.outcome.messages + ); + }; + let [_, write] = &answers(user)[..] else { + panic!("a result for each of two calls, got {user:?}"); + }; + assert!(write.0.contains("withheld"), "got {write:?}"); + assert!( + !write.0.contains("landed"), + "the write's own report got through: {write:?}" + ); +} + +/// Round one answered, the stop in round two's only call: both rounds are kept (#26). #[test] fn a_stop_mid_call_keeps_the_rounds_before_it() { let root = tempfile::tempdir().unwrap(); @@ -246,15 +326,13 @@ fn a_stop_mid_call_keeps_the_rounds_before_it() { assert_eq!(outcome.stop, TurnStop::Interrupted); let roles: Vec = outcome.messages.iter().map(|m| m.role).collect(); - assert_eq!(roles, [Role::Assistant, Role::User], "round one alone"); - assert!( - matches!( - outcome.messages[1].content[..], - [ContentBlock::ToolResult { .. }] - ), - "got {:?}", - outcome.messages + assert_eq!( + roles, + [Role::Assistant, Role::User, Role::Assistant, Role::User] ); + let last = answers(&outcome.messages[3]); + assert_eq!(last.len(), 1, "got {:?}", outcome.messages); + assert!(last[0].0.contains("withheld"), "got {last:?}"); assert_eq!(script.sent.len(), 2, "no third request after the stop"); } @@ -311,7 +389,7 @@ fn a_stopped_turn_returns_no_reasoning() { let outcome = stopped_in_second_call(&mut script, turn(&[], &[BuiltinTool::Ls]), root.path()); assert_eq!(outcome.stop, TurnStop::Interrupted); - assert_eq!(outcome.messages.len(), 2, "got {:?}", outcome.messages); + assert_eq!(outcome.messages.len(), 4, "got {:?}", outcome.messages); assert!( !outcome .messages From 6974be80805d327af48a371d91c868540ddea698 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:23:19 +0100 Subject: [PATCH 06/12] fix(cli): say a stop keeps the round it landed in while tools ran --- crates/sandbx-cli/src/tui.rs | 16 +++++++++------- crates/sandbx-cli/src/tui_tests.rs | 11 ++++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/sandbx-cli/src/tui.rs b/crates/sandbx-cli/src/tui.rs index d2a982bd..dbe83f3a 100644 --- a/crates/sandbx-cli/src/tui.rs +++ b/crates/sandbx-cli/src/tui.rs @@ -203,8 +203,9 @@ 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. 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). + // back to be stored. The stop kills a running `bash`, stops a `grep` and waits for + // another 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, @@ -336,7 +337,7 @@ fn held(stderr: Device) -> Option { /// What the turn's futures came to, before any of it is accounted for. enum Driven { /// Stopped before the turn had finished, which keeps the rounds before the one it - /// stopped in. + /// stopped in, and that one too if it was running tools. Stopped(Stopped, TurnOutcome), /// Stopped in the wrap-up round of a turn that ran out after `usize` rounds, which is /// kept as the cap left it. @@ -404,7 +405,7 @@ fn stored(stopped: Stopped) -> StoredStop { /// The code a stop exits with, and its line. `wrapping` is whether it found the turn in /// its wrap-up round, which keeps every round before it, where any other stop loses the -/// round it lands in. +/// round it lands in if the model was still writing it. /// /// A hangup has its own line and not the interrupt's: the operator did not stop this turn, /// and telling them they did is the one reading of a hangup that is certainly wrong (#264). @@ -427,9 +428,10 @@ 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. A `bash` command already running is killed; any other tool call \ - already running finishes first, and what it did stays (#26)" + "{how}. Under `--session` the rounds it finished are kept, and the one it stopped \ + in is kept if it was running tools and is lost if the model was still writing. A \ + `bash` command already running is killed and a `grep` stops before its next file; \ + any other tool call already running finishes first, and what it did stays (#26)" ) }; diff --git a/crates/sandbx-cli/src/tui_tests.rs b/crates/sandbx-cli/src/tui_tests.rs index 66160ee5..8e835f34 100644 --- a/crates/sandbx-cli/src/tui_tests.rs +++ b/crates/sandbx-cli/src/tui_tests.rs @@ -371,8 +371,13 @@ fn every_stop_keeps_the_rounds_it_finished() { let (_, stop) = turn.expect("the finished rounds are kept"); assert_eq!(stop, stored, "{stopped:?}"); assert_eq!(mid_turn, code, "{stopped:?}"); + // Both stops are named: one in the stream loses its round, one in a tool call keeps it. assert!( - said[0].contains("the one it stopped in is lost"), + said[0].contains("is lost if the model was still writing"), + "{said:?}" + ); + assert!( + said[0].contains("is kept if it was running tools"), "{said:?}" ); // What the operator is told about a call under way: the mechanism's whole claim. @@ -380,6 +385,10 @@ fn every_stop_keeps_the_rounds_it_finished() { said[0].contains("`bash` command already running is killed"), "{said:?}" ); + assert!( + said[0].contains("`grep` stops before its next file"), + "{said:?}" + ); } } From 037dda5a37f1d7ad878267dd08d8bc302004419c Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:23:23 +0100 Subject: [PATCH 07/12] docs: a stopped round is kept, with the stopped call's output withheld --- README.md | 7 ++++--- SECURITY.md | 23 +++++++++++++---------- context/decision-approval-gate.md | 11 ++++++----- crates/sandbx-turn/src/tools.rs | 7 ++++--- crates/sandbx-turn/src/turn.rs | 21 +++++++++++++-------- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index b84eb6f5..52e1bd20 100644 --- a/README.md +++ b/README.md @@ -305,8 +305,9 @@ 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 `bash` command killed and a `grep` stopped +escape ends it mid-flight — `--session` keeping the rounds it finished, and the +one it stopped in if it was running tools, with the output of the call it +stopped on withheld, and a running `bash` command killed and a `grep` stopped before its next file, 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 @@ -335,7 +336,7 @@ resuming one another user can write is refused — [SECURITY.md](SECURITY.md). |---|---| | `0` | the model finished its answer | | `2` | a bound cut the turn short, named on stderr — `--max-tokens` or `--max-rounds`. Out of rounds, one more request answers the turn with no tool, so stdout usually holds a summary a blank line below what arrived before the cap; `--no-wrap-up` skips it, leaving nothing at all if the model opened with a tool call ([#204](https://github.com/danczw/sandbx/pull/204)). Under `tui` it also means you interrupted the turn | -| `3` | the terminal went away, so the turn ended there and nobody was watching it. Under `--approve call` that is the terminal it asks on: that call and the ones behind it were refused and no further request was sent. What the turn did before is on stdout and in `--session`. Under `tui` it is the screen or the keyboard hanging up mid-turn, which ends the turn where it stood: `--session` keeps the rounds it finished and loses the one it stopped in, so a hangup before the first round finishes stores nothing, the prompt included ([#271](https://github.com/danczw/sandbx/issues/271)). In the wrap-up round after `--max-rounds` it loses only the wrap-up. The audit trail of what it touched is released to stderr rather than dropped unwritten, so `2>` a file to keep it: where stderr is the terminal that died, the write fails and it goes with it | +| `3` | the terminal went away, so the turn ended there and nobody was watching it. Under `--approve call` that is the terminal it asks on: that call and the ones behind it were refused and no further request was sent. What the turn did before is on stdout and in `--session`. Under `tui` it is the screen or the keyboard hanging up mid-turn, which ends the turn where it stood: `--session` keeps the rounds it finished and the one it stopped in if it was running tools, and loses it if the model was still writing, so a hangup before the first round finishes streaming stores nothing, the prompt included ([#271](https://github.com/danczw/sandbx/issues/271)). In the wrap-up round after `--max-rounds` it loses only the wrap-up. The audit trail of what it touched is released to stderr rather than dropped unwritten, so `2>` a file to keep it: where stderr is the terminal that died, the write fails and it goes with it | | `64` | sandbx would not take the command line, so no turn was started and nothing was spawned. The complaint is on stderr, as clap wrote it. `EX_USAGE`, and the same code under every subcommand | | anything else | it failed before or during the turn, with the reason on stderr | diff --git a/SECURITY.md b/SECURITY.md index 1b7aecb2..9510ef99 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -329,16 +329,19 @@ architecture the filter gates on. Whatever a tool read is in it: a token in a config, a `.env` under `--allow-read`, a key a `bash` printed. No encryption, no redaction, no expiry, no deletion. A turn stopped on `tui`'s screen stores the prompt and every - 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 is killed if it is a `bash`, - stopped if it is a `grep`, and finishes if it is neither, but the transcript - records the effect of none. 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 + round it finished, under a stop naming an interrupt or a hung-up terminal. A + stop while the model is still writing loses that round whole: whatever it + streamed, including the part of the answer you read, and every tool call it + asked for. A stop while its tools run keeps the round with every call + answered: a call that finished before the stop keeps its output; one running + at the stop is killed if it is a `bash`, stopped if it is a `grep` and + finishes if it is neither, and is stored with its output withheld, since you + stopped before reading it and a resume would send it to the provider; one + behind it is stored as not run. What a withheld call did still stands. A stop + before the first round finishes streaming stores nothing, the prompt + included ([#271](https://github.com/danczw/sandbx/issues/271)). Where a round + was stored, resuming says so on stderr, and the model sees those results and + nothing more of the stop. What the turn showed is printed back to the terminal once the screen is gone, and the call a stop stood on is named there as it ended, run or cancelled, but never with its output. Where the terminal is diff --git a/context/decision-approval-gate.md b/context/decision-approval-gate.md index ff096ff8..5b8f28f0 100644 --- a/context/decision-approval-gate.md +++ b/context/decision-approval-gate.md @@ -202,11 +202,12 @@ case without `--approve call`; with it, a hangup lands on a question and is the 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 is killed if it is a -`bash`, stopped if it is a `grep`, and finishes if it is neither (#26), on both -surfaces alike. +`run_turn_until`, which keeps the rounds before it, and the one it lands in if +that was running tools, every call answered and the one under way with its +output withheld. A round the model was still writing is stored nowhere, for a +hangup as for an interrupt (#271). A tool call already running is killed if it +is a `bash`, stopped if it is a `grep`, and finishes if it is neither (#26), on +both surfaces alike. ## Deny by default, and the honest claim diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index cc3b60a8..8ff4b703 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -18,9 +18,10 @@ use crate::TurnError; /// 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. +/// A cancelled `ctx` ends the loop early. Every call still gets a result, so the round stays +/// legal to store: a call behind the stop says it did not run, and one that settled once +/// the token was raised says its output was withheld, whatever it returned. Only +/// `run_turn_until` holds 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 a6ecd6b3..6baf5b69 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -137,12 +137,15 @@ pub enum TurnStop { /// `tool_result`. GateAborted, - /// [`run_turn_until`]'s `stop` resolved, and the round it found running was dropped. + /// [`run_turn_until`]'s `stop` resolved. /// - /// 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. A `bash` it had started is killed; any - /// other call it had started finishes, and its effect stays (#26). + /// A stop that lands while the model is streaming drops that round: the transcript is + /// every round before it, or empty. One that lands in a round's tool calls keeps the + /// round, every call answered, so it ends on a user turn of results, as legal to send + /// again as [`RoundLimit`](Self::RoundLimit)'s. A call that finished before the stop + /// says what it found; one running at it is waited for, a `bash` killed and any other + /// finishing with its effect standing (#26), and says only that its output was + /// withheld; one behind it was never put to the gate or run, and says so. Interrupted, } @@ -217,7 +220,8 @@ impl Default for TurnLimits { /// 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, 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`. +/// stops a turn instead: it keeps the rounds before the stop, the one it landed in among +/// them, 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. @@ -245,8 +249,9 @@ where /// 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 +/// has already resolved opens no further request. A round it lands in while streaming is +/// dropped whole; one it lands in during its calls is kept, [`TurnStop::Interrupted`] says +/// how. A call already running is waited for rather than /// abandoned, and reported to [`CallGate::settled`] as it ends: a `bash` is killed and a /// `grep` stops between files, but the other in-process tools cannot be cancelled (#26), so /// one finishes and its effect stays. No call behind it is put to the gate or run. From 3d23769852dac7ddc66f249679e3f6cf98edbb99 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:36:49 +0100 Subject: [PATCH 08/12] feat(turn): a stop does not wait for a read-only call --- crates/sandbx-cli/src/main.rs | 12 +- crates/sandbx-cli/src/main_tests.rs | 19 +++ crates/sandbx-turn/src/tools.rs | 48 +++++++- crates/sandbx-turn/tests/turn_stop.rs | 159 +++++++++++++++++--------- 4 files changed, 177 insertions(+), 61 deletions(-) diff --git a/crates/sandbx-cli/src/main.rs b/crates/sandbx-cli/src/main.rs index 088a0e1b..066a0614 100644 --- a/crates/sandbx-cli/src/main.rs +++ b/crates/sandbx-cli/src/main.rs @@ -122,13 +122,19 @@ fn report(result: Result, failure: u8) -> ExitCode { fn block_on( future: impl Future>, ) -> Result { - tokio::runtime::Builder::new_current_thread() + let runtime = tokio::runtime::Builder::new_current_thread() .enable_all() .build() // Named at the one call site that can produce it rather than by a blanket // `From`, which would label any later io error as this one. - .map_err(sandbx_cli::AgentError::Runtime)? - .block_on(future) + .map_err(sandbx_cli::AgentError::Runtime)?; + let result = runtime.block_on(future); + + // Dropping a runtime waits for every blocking task, and a read-only tool call a stop + // abandoned may be parked in one for as long as a stalled filesystem takes (#26). Only + // those are left by now, every other call having been awaited. + runtime.shutdown_background(); + result } #[cfg(test)] diff --git a/crates/sandbx-cli/src/main_tests.rs b/crates/sandbx-cli/src/main_tests.rs index 7d02df1f..85991123 100644 --- a/crates/sandbx-cli/src/main_tests.rs +++ b/crates/sandbx-cli/src/main_tests.rs @@ -49,3 +49,22 @@ fn an_argv_sandbx_accepts_is_not_a_usage_error() { Cli::try_parse_from(["sandbx", "sandbox-run", "--", "true"]) .expect("a well-formed sandbox-run"); } + +/// A read abandoned on a stop is still parked on its blocking thread, and dropping the +/// runtime waits for every such thread. The process must not (#26). +#[test] +fn the_runtime_does_not_wait_for_a_blocking_task_left_running() { + let started = std::time::Instant::now(); + + let result = super::block_on(async { + tokio::task::spawn_blocking(|| std::thread::sleep(std::time::Duration::from_secs(3))); + Ok(0) + }); + + assert!(matches!(result, Ok(0))); + assert!( + started.elapsed() < std::time::Duration::from_secs(1), + "the runtime waited {:?} for a task nobody awaits", + started.elapsed() + ); +} diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index 8ff4b703..1c6ec43b 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -3,9 +3,13 @@ //! The crate's only `spawn_blocking` site. Tools are synchronous, so the sync/async //! boundary lives here rather than spread through seven tool bodies. +use futures_util::future::{Either, select}; use sandbx_approval::{ApprovalDecision, CallGate, Outcome, Settled, ToolCall}; use sandbx_prompt::{ContentBlock, ToolDefinition}; -use sandbx_tools::{BuiltinTool, ExecutionContext, ToolError}; +use sandbx_tools::{BuiltinTool, ExecutionContext, RiskLevel, ToolError}; +use std::pin::pin; +use std::time::Duration; +use tokio::task::{JoinError, JoinHandle}; use crate::TurnError; @@ -100,11 +104,22 @@ where // consumes it; an `Arc` would only pay off past a few path lists. let arguments = input.clone(); let context = ctx.clone(); - let outcome = tokio::task::spawn_blocking(move || tool.execute(arguments, &context)) - .await - .map_err(|_| TurnError::ToolPanicked { + let mut running = tokio::task::spawn_blocking(move || tool.execute(arguments, &context)); + let joined = match tool.risk() { + RiskLevel::ReadOnly => unless_stopped(&mut running, ctx).await, + RiskLevel::Writes | RiskLevel::Executes => Some(running.await), + }; + let outcome = match joined { + Some(joined) => joined.map_err(|_| TurnError::ToolPanicked { name: name.to_string(), - })?; + })?, + // Left running on its blocking thread, its result never read. Safe only because it + // is read-only: abandoning a `write` is what the gate's position ahead of the + // spawn exists to rule out. Exhaustive above, so a fourth level must pick a side. + None => Err(ToolError::Cancelled { + subject: name.to_string(), + }), + }; // Read after the join, never before the spawn: a call that finished a moment before // the stop is withheld too, which errs toward saying less of what the operator stopped. @@ -158,6 +173,29 @@ where }) } +/// How often a running read-only call is checked for a stop. +/// +/// A poll and not a wake-up because [`Cancel`](sandbx_tools::Cancel) is a polled flag by +/// design. Bounds how long a stop waits on a call that cannot finish. +const STOP_POLL: Duration = Duration::from_millis(50); + +/// `call`'s result, or `None` once `ctx` is cancelled while it is still running. +/// +/// `call` is borrowed and so left running, not aborted: nothing can interrupt a blocking +/// thread, only stop waiting for it. +async fn unless_stopped( + call: &mut JoinHandle, + ctx: &ExecutionContext, +) -> Option> { + loop { + match select(&mut *call, pin!(tokio::time::sleep(STOP_POLL))).await { + Either::Left((joined, _)) => return Some(joined), + Either::Right(((), _)) if ctx.cancelled() => return None, + Either::Right(((), _)) => {} + } + } +} + /// One round's answers, and whether the gate ended the turn giving them. pub(crate) struct Answers { /// One `tool_result` per `tool_use` block, in the order the model asked. diff --git a/crates/sandbx-turn/tests/turn_stop.rs b/crates/sandbx-turn/tests/turn_stop.rs index 796508d7..dfbdae6a 100644 --- a/crates/sandbx-turn/tests/turn_stop.rs +++ b/crates/sandbx-turn/tests/turn_stop.rs @@ -1,12 +1,16 @@ //! Public contract of `run_turn_until`: what a stopped turn keeps. +// `mkfifo` is spawned to build a fixture: std cannot create a named pipe. Not code +// escaping the sandbox, which is what the ban on `Command::new` exists to stop. +#![allow(clippy::disallowed_methods)] -use std::fs; use std::future::{self, Future}; use std::path::Path; +use std::process::Command; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Mutex, mpsc}; use std::task::Poll; -use std::time::Duration; +use std::time::{Duration, Instant}; +use std::{fs, thread}; use futures_util::task::AtomicWaker; use futures_util::{StreamExt, stream}; @@ -30,6 +34,27 @@ fn listing(root: &tempfile::TempDir) -> Vec { ] } +/// Opens `fifo` for writing once `delay` has passed, which wakes a reader parked on it, and +/// joins that thread when dropped. +struct Release(Option>); + +impl Release { + fn after(delay: Duration, fifo: std::path::PathBuf) -> Self { + Self(Some(thread::spawn(move || { + thread::sleep(delay); + let _ = fs::OpenOptions::new().write(true).open(fifo); + }))) + } +} + +impl Drop for Release { + fn drop(&mut self) { + if let Some(thread) = self.0.take() { + let _ = thread.join(); + } + } +} + /// The `(content, is_error)` of each `tool_result` in a user turn. fn answers(message: &RequestMessage) -> Vec<(&str, Option)> { message @@ -206,8 +231,9 @@ async fn a_stop_already_resolved_sends_nothing() { } /// 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. +/// is waited for and settles, and nothing behind it is put to the gate or run (#26). The +/// stopped call is a `write`, which is always waited for and lands; a read-only call is +/// abandoned instead, so it could settle either way. /// /// The round is kept with every call answered: the one that finished before the stop says /// what it found, the stopped one says only that it finished, and the one behind it says it @@ -216,6 +242,7 @@ async fn a_stop_already_resolved_sends_nothing() { fn a_stop_mid_call_starts_no_call_behind_it() { let root = tempfile::tempdir().unwrap(); fs::write(root.path().join("seen.txt"), "").unwrap(); + let landed = root.path().join("landed.txt"); let behind = root.path().join("behind.txt"); let ctx = ctx(SandboxPolicy::default() .allow_read(vetted(root.path())) @@ -223,8 +250,11 @@ fn a_stop_mid_call_starts_no_call_behind_it() { 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": landed.to_str().unwrap(), "content": "landed" }), + ), call( "write", serde_json::json!({ "path": behind.to_str().unwrap(), "content": "behind it" }), @@ -243,6 +273,7 @@ fn a_stop_mid_call_starts_no_call_behind_it() { 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_eq!(fs::read_to_string(&landed).unwrap(), "landed"); assert!(!behind.exists(), "a call behind the stop ran"); let [assistant, user] = &stopped.outcome.messages[..] else { @@ -257,61 +288,17 @@ fn a_stop_mid_call_starts_no_call_behind_it() { }; assert!(before.0.contains("seen.txt"), "got {before:?}"); assert_eq!(before.1, None); + // What the operator stopped before reading, and a resume would send on. assert!(during.0.contains("withheld"), "got {during:?}"); assert!( - !during.0.contains("seen.txt"), - "the stopped call's output got through" + !during.0.contains("landed"), + "the write's own report got through: {during:?}" ); assert_eq!(during.1, Some(true)); assert!(after.0.contains("not run"), "got {after:?}"); assert_eq!(after.1, Some(true)); } -/// A `write` the stop landed on is waited for and lands (#26), and what it says is not -/// stored: the operator stopped before reading it, and a resume would send it on. -#[test] -fn a_stop_mid_write_lets_it_land_and_withholds_what_it_says() { - let root = tempfile::tempdir().unwrap(); - let target = root.path().join("landed.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), - call( - "write", - serde_json::json!({ "path": target.to_str().unwrap(), "content": "landed" }), - ), - 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!(fs::read_to_string(&target).unwrap(), "landed"); - let [_, user] = &stopped.outcome.messages[..] else { - panic!( - "expected the stopped round, got {:?}", - stopped.outcome.messages - ); - }; - let [_, write] = &answers(user)[..] else { - panic!("a result for each of two calls, got {user:?}"); - }; - assert!(write.0.contains("withheld"), "got {write:?}"); - assert!( - !write.0.contains("landed"), - "the write's own report got through: {write:?}" - ); -} - /// Round one answered, the stop in round two's only call: both rounds are kept (#26). #[test] fn a_stop_mid_call_keeps_the_rounds_before_it() { @@ -330,12 +317,78 @@ fn a_stop_mid_call_keeps_the_rounds_before_it() { roles, [Role::Assistant, Role::User, Role::Assistant, Role::User] ); + // An `ls` is abandoned or finished, whichever the poll finds first: either says nothing of + // what it listed. let last = answers(&outcome.messages[3]); assert_eq!(last.len(), 1, "got {:?}", outcome.messages); - assert!(last[0].0.contains("withheld"), "got {last:?}"); + assert_eq!(last[0].1, Some(true), "got {last:?}"); + assert!( + last[0].0.contains("withheld") || last[0].0.contains("cancelled"), + "got {last:?}" + ); assert_eq!(script.sent.len(), 2, "no third request after the stop"); } +/// A `read` of a FIFO with no writer waits in `open` for as long as it takes someone to +/// write, which no stop can interrupt. It is read-only and writes nothing, so the stop +/// abandons it instead of waiting (#26). +#[tokio::test] +async fn a_stop_does_not_wait_for_a_read_that_cannot_finish() { + let root = tempfile::tempdir().unwrap(); + let fifo = root.path().join("pipe"); + let made = Command::new("mkfifo") + .arg(&fifo) + .status() + .expect("mkfifo should run"); + assert!(made.success()); + let mut script = Script::new([ + vec![ + call( + "read", + serde_json::json!({ "path": fifo.to_str().unwrap() }), + ), + stop(StopReason::ToolUse), + ], + vec![text("never asked"), stop(StopReason::EndTurn)], + ]); + // Frees the thread the read is parked on after two seconds, a stop that waits for it + // then failing the bound below instead of hanging the runtime's drop. Declared after + // `root`, so it is joined before the directory goes: with the FIFO unlinked the open + // fails, and the read stays parked. + let _release = Release::after(Duration::from_secs(2), fifo.clone()); + let started = Instant::now(); + + let outcome = tokio::time::timeout( + Duration::from_secs(10), + run_turn_until( + async |r| script.open(r).await, + turn(&[], &[BuiltinTool::Read]), + &ctx(SandboxPolicy::default().allow_read(vetted(root.path()))), + |_| {}, + AllowAll, + tokio::time::sleep(Duration::from_millis(200)), + ), + ) + .await + .expect("the stop ended the turn") + .unwrap(); + let elapsed = started.elapsed(); + + assert_eq!(outcome.stop, TurnStop::Interrupted); + assert!( + elapsed < Duration::from_secs(1), + "the stop waited {elapsed:?} for a read" + ); + let [_, user] = &outcome.messages[..] else { + panic!("expected the stopped round, got {:?}", outcome.messages); + }; + let [result] = &answers(user)[..] else { + panic!("a result for the one call, got {user:?}"); + }; + assert!(result.0.contains("cancelled"), "got {result:?}"); + assert_eq!(result.1, Some(true)); +} + /// A round that streams forever: only the stop can end it under a 300-second bound. fn stalled() -> EventStream { Box::pin(stream::pending()) From b5fed82c8e9dee1d496bbc794842acf368649994 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:37:35 +0100 Subject: [PATCH 09/12] fix(cli): say a stop leaves a read-only call running --- crates/sandbx-cli/src/tui.rs | 5 +++-- crates/sandbx-cli/src/tui_tests.rs | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/sandbx-cli/src/tui.rs b/crates/sandbx-cli/src/tui.rs index dbe83f3a..4b79a83b 100644 --- a/crates/sandbx-cli/src/tui.rs +++ b/crates/sandbx-cli/src/tui.rs @@ -430,8 +430,9 @@ fn stop(stopped: Stopped, wrapping: bool) -> Ending { format!( "{how}. Under `--session` the rounds it finished are kept, and the one it stopped \ in is kept if it was running tools and is lost if the model was still writing. A \ - `bash` command already running is killed and a `grep` stops before its next file; \ - any other tool call already running finishes first, and what it did stays (#26)" + `bash` command already running is killed, and a call that only reads is left \ + running and not waited for; a `write` or `edit` already running finishes first, \ + and what it did stays (#26)" ) }; diff --git a/crates/sandbx-cli/src/tui_tests.rs b/crates/sandbx-cli/src/tui_tests.rs index 8e835f34..5078c1d0 100644 --- a/crates/sandbx-cli/src/tui_tests.rs +++ b/crates/sandbx-cli/src/tui_tests.rs @@ -386,7 +386,11 @@ fn every_stop_keeps_the_rounds_it_finished() { "{said:?}" ); assert!( - said[0].contains("`grep` stops before its next file"), + said[0].contains("a call that only reads is left running and not waited for"), + "{said:?}" + ); + assert!( + said[0].contains("a `write` or `edit` already running finishes first"), "{said:?}" ); } From efb38fc473604737338e2e22538a14bf8b84f0ce Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:39:25 +0100 Subject: [PATCH 10/12] docs: a stop leaves a read-only call running and waits for a write --- README.md | 4 +-- SECURITY.md | 30 +++++++++++--------- context/decision-approval-gate.md | 11 +++---- crates/sandbx-cli/src/cli.rs | 9 +++--- crates/sandbx-cli/src/tui.rs | 6 ++-- crates/sandbx-cli/src/tui_modal.rs | 2 +- crates/sandbx-tools/src/execution_context.rs | 4 ++- crates/sandbx-turn/src/tools.rs | 3 +- crates/sandbx-turn/src/turn.rs | 18 ++++++------ 9 files changed, 49 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 52e1bd20..eb66e4c6 100644 --- a/README.md +++ b/README.md @@ -307,8 +307,8 @@ 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, and the one it stopped in if it was running tools, with the output of the call it -stopped on withheld, and a running `bash` command killed and a `grep` stopped -before its next file, where any other tool call finishes first. When the turn +stopped on withheld, and a running `bash` command killed and a call that only +reads left running, where a `write` or `edit` 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, diff --git a/SECURITY.md b/SECURITY.md index 9510ef99..7c41a2b1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -334,8 +334,8 @@ architecture the filter gates on. streamed, including the part of the answer you read, and every tool call it asked for. A stop while its tools run keeps the round with every call answered: a call that finished before the stop keeps its output; one running - at the stop is killed if it is a `bash`, stopped if it is a `grep` and - finishes if it is neither, and is stored with its output withheld, since you + at the stop is killed if it is a `bash`, left running if it only reads and + finishes if it writes, and is stored with its output withheld, since you stopped before reading it and a resume would send it to the provider; one behind it is stored as not run. What a withheld call did still stands. A stop before the first round finishes streaming stores nothing, the prompt @@ -378,19 +378,23 @@ architecture the filter gates on. instead — a cap on files visited and bytes read for `grep` and `find`, the single file or directory for the rest ([`ToolLimits`](crates/sandbx-tools/src/tool_limits.rs)) — on a thread that - cannot be cancelled, so one read on a stalled filesystem hangs indefinitely. - Nothing else is capped: no CPU bound, no memory bound, no limit on processes + cannot be cancelled, so one read on a stalled filesystem hangs indefinitely, + holding that thread until the process exits; a stop stops waiting for it, + below. 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. -- **Only a spawned command and a `grep` 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, stops a `grep` before its next file (not during the - directory walk before it), and starts no call behind either. The other five - 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, and a `grep` is held the same way inside the file it is - reading. Nothing a killed command had already done is undone - ([#26](https://github.com/danczw/sandbx/issues/26)). +- **A `write` or `edit` cannot be interrupted, and a read is abandoned, not + stopped.** A stop on `tui` (ctrl-c, escape, or a hung-up terminal) kills a + running `bash` command with the kill a timeout makes, and stops waiting for a + running `read`, `ls`, `grep` or `find`: the call is left on its blocking + thread with its result unread, a `grep` also stopping before its next file + (not during the directory walk before it). It starts no call behind either. A + `write` or `edit` already running finishes, so a `write` under way lands, and + the stop waits for it. An abandoned read goes on until it finishes or the + process exits, so a stop is not a promise that nothing is still reading: its + access was recorded in the audit trail before it began, and a stalled one + holds a blocking thread until exit. 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 model, `TurnLimits::stream_timeout` how long one of them may spend streaming diff --git a/context/decision-approval-gate.md b/context/decision-approval-gate.md index 5b8f28f0..c19a3945 100644 --- a/context/decision-approval-gate.md +++ b/context/decision-approval-gate.md @@ -16,8 +16,9 @@ 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, and only a `bash` and a `grep` can be stopped -besides, a `write` not at all. +task running to completion. A stop abandons that handle only for a call that +reads, which leaves nothing behind it; a `write` or `edit` is waited for, and +that is the case this position guards. 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. @@ -206,7 +207,7 @@ messages, usage and `withheld` are stored. A `tui` stop ends the turn through that was running tools, every call answered and the one under way with its output withheld. A round the model was still writing is stored nowhere, for a hangup as for an interrupt (#271). A tool call already running is killed if it -is a `bash`, stopped if it is a `grep`, and finishes if it is neither (#26), on +is a `bash`, left running if it only reads, and finishes if it writes (#26), on both surfaces alike. ## Deny by default, and the honest claim @@ -367,8 +368,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 an in-process - call cannot be cancelled (#26). + stop they gave since: a covered call would otherwise run, and a `write` or + `edit` 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. diff --git a/crates/sandbx-cli/src/cli.rs b/crates/sandbx-cli/src/cli.rs index c76cff57..d11dba0f 100644 --- a/crates/sandbx-cli/src/cli.rs +++ b/crates/sandbx-cli/src/cli.rs @@ -96,10 +96,11 @@ pub enum Command { /// screen goes, and what it showed is printed in its place. /// /// 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 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 + /// `--session` keeps the rounds the turn finished, and the one it stopped in + /// if it was running tools, and a tool already running is dealt with first: + /// a `bash` command is killed, a call that only reads is left running, and a + /// `write` or `edit` finishes, the process waiting 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 diff --git a/crates/sandbx-cli/src/tui.rs b/crates/sandbx-cli/src/tui.rs index 4b79a83b..25ad08d7 100644 --- a/crates/sandbx-cli/src/tui.rs +++ b/crates/sandbx-cli/src/tui.rs @@ -203,9 +203,9 @@ 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. The stop kills a running `bash`, stops a `grep` and waits for - // another in-process tool, so the call it stood on settles, and is drawn, before the - // turn returns (#26). + // back to be stored. The stop kills a running `bash`, leaves a read-only call running + // and waits for a `write` or `edit`, 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, diff --git a/crates/sandbx-cli/src/tui_modal.rs b/crates/sandbx-cli/src/tui_modal.rs index 9f588abe..d2a46907 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 an in-process call cannot be cancelled (#26). + // call would otherwise run, and a `write` or `edit` 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-tools/src/execution_context.rs b/crates/sandbx-tools/src/execution_context.rs index 9f382286..47ede407 100644 --- a/crates/sandbx-tools/src/execution_context.rs +++ b/crates/sandbx-tools/src/execution_context.rs @@ -101,7 +101,9 @@ impl ExecutionContext { /// /// Every clone shares the token, so one raised stops a command started through any of /// them. A spawned command is killed, and `grep` stops between files; the other - /// in-process tools are bounded by work and finish what they started (#26). + /// in-process tools never read the token, and `write` and `edit` finish what they + /// started (#26). A caller may stop waiting for a read-only one instead, as + /// `sandbx-turn` does. #[must_use] pub fn with_cancel(mut self, cancel: Cancel) -> Self { self.cancel = cancel; diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index 1c6ec43b..c38d0ffc 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -19,6 +19,7 @@ use crate::TurnError; /// `ExecutionContext` settled first (#242). `BuiltinTool::execute` may sit in a `write`, a /// directory walk or a 90-second command, which on a current-thread runtime would freeze /// every other task — hence `spawn_blocking`, whose uncancellability `run_turn` documents. +/// A stop abandons a read-only call there and still waits for a `write` or `edit`. /// Every answered block reports to [`CallGate::settled`], including the two refusals above /// the gate, which the gate's own verdict cannot account for. /// @@ -73,7 +74,7 @@ where continue; } - // Before `spawn_blocking`, never racing it: an in-process tool cannot be cancelled, + // Before `spawn_blocking`, never racing it: a `write` or `edit` cannot be cancelled, // so a late decision would not stop the call it refused (#26). let verdict = match &aborted { Some(reason) => ApprovalDecision::Deny { diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index 6baf5b69..9bfbba23 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -143,9 +143,10 @@ pub enum TurnStop { /// every round before it, or empty. One that lands in a round's tool calls keeps the /// round, every call answered, so it ends on a user turn of results, as legal to send /// again as [`RoundLimit`](Self::RoundLimit)'s. A call that finished before the stop - /// says what it found; one running at it is waited for, a `bash` killed and any other - /// finishing with its effect standing (#26), and says only that its output was - /// withheld; one behind it was never put to the gate or run, and says so. + /// says what it found; one running at it is killed if it is a `bash`, left running if it + /// only reads, and waited for if it writes, its effect standing (#26), and says only + /// that its output was withheld or that it was cancelled; one behind it was never put to + /// the gate or run, and says so. Interrupted, } @@ -153,7 +154,7 @@ pub enum TurnStop { /// /// None of them bounds a turn's total wall-clock time, and no product of them does: the /// in-process tools are bounded by work, not time, so a single `read` on a stalled filesystem -/// is bounded by nothing here. +/// is bounded by nothing here, though a stop no longer waits for it. #[derive(Debug, Clone, Copy)] pub struct TurnLimits { /// How many times the model may be asked within one turn. @@ -251,10 +252,11 @@ where /// Raced against each round's request and its tool calls, `stop` first, so a stop that /// has already resolved opens no further request. A round it lands in while streaming is /// dropped whole; one it lands in during its calls is kept, [`TurnStop::Interrupted`] says -/// how. A call already running is waited for rather than -/// abandoned, and reported to [`CallGate::settled`] as it ends: a `bash` is killed and a -/// `grep` stops between files, but the other in-process tools cannot be cancelled (#26), so -/// one finishes and its effect stays. No call behind it is put to the gate or run. +/// how. A call already running is reported to [`CallGate::settled`] as it ends: a `bash` is +/// killed, and a read-only call (`read`, `ls`, `grep`, `find`) is abandoned on its blocking +/// thread with its result unread, a `grep` also stopping at its next file. A `write` or +/// `edit` cannot be cancelled (#26), so it is waited for 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 From 273e430cfdaee5f2a584d33affb6b7f68b6af5d4 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:47:39 +0100 Subject: [PATCH 11/12] fix(turn): store a call that ends after a stop as what it was --- crates/sandbx-turn/src/tools.rs | 34 +++++++++++++++----- crates/sandbx-turn/src/turn.rs | 25 +++++++-------- crates/sandbx-turn/tests/turn_stop.rs | 46 +++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 21 deletions(-) diff --git a/crates/sandbx-turn/src/tools.rs b/crates/sandbx-turn/src/tools.rs index c38d0ffc..5d385c87 100644 --- a/crates/sandbx-turn/src/tools.rs +++ b/crates/sandbx-turn/src/tools.rs @@ -20,8 +20,9 @@ use crate::TurnError; /// directory walk or a 90-second command, which on a current-thread runtime would freeze /// every other task — hence `spawn_blocking`, whose uncancellability `run_turn` documents. /// A stop abandons a read-only call there and still waits for a `write` or `edit`. -/// Every answered block reports to [`CallGate::settled`], including the two refusals above -/// the gate, which the gate's own verdict cannot account for. +/// Every call that was put to the gate, or refused above it, reports to +/// [`CallGate::settled`]. A call behind a stop is answered as not run and reports nothing: +/// it never reached the gate, so there is no verdict for the account to close. /// /// A cancelled `ctx` ends the loop early. Every call still gets a result, so the round stays /// legal to store: a call behind the stop says it did not run, and one that settled once @@ -111,9 +112,18 @@ where RiskLevel::Writes | RiskLevel::Executes => Some(running.await), }; let outcome = match joined { - Some(joined) => joined.map_err(|_| TurnError::ToolPanicked { - name: name.to_string(), - })?, + Some(Ok(outcome)) => outcome, + // Answered and not propagated: the calls before it in this round have landed, and + // a round dropped for one panic would leave a resume knowing nothing of them. + Some(Err(_)) if ctx.cancelled() => Err(ToolError::Failed { + subject: name.to_string(), + detail: "the tool panicked".to_string(), + }), + Some(Err(_)) => { + return Err(TurnError::ToolPanicked { + name: name.to_string(), + }); + } // Left running on its blocking thread, its result never read. Safe only because it // is read-only: abandoning a `write` is what the gate's position ahead of the // spawn exists to rule out. Exhaustive above, so a fourth level must pick a side. @@ -143,7 +153,7 @@ where match (stopped, &error) { (false, _) => refused(id, error.to_string()), (true, ToolError::Cancelled { .. }) => refused(id, CANCELLED.to_string()), - (true, _) => refused(id, FINISHED_AFTER_STOP.to_string()), + (true, _) => refused(id, ERRORED_AFTER_STOP.to_string()), } } }); @@ -213,14 +223,22 @@ pub(crate) struct Answers { /// A call behind the one the stop landed on, never put to the gate or run. const NOT_RUN: &str = "not run: the turn was stopped"; -/// A call the stop killed or interrupted before it could finish. -const CANCELLED: &str = "cancelled: the turn was stopped before it finished"; +/// A call the stop killed or abandoned before it finished. Says it may have changed +/// something, because a `bash` killed halfway recorded as plainly cancelled would be rerun +/// on top of what it had done. +const CANCELLED: &str = "cancelled: the turn was stopped before it finished; anything it \ + had already done may remain"; /// A call that finished once the stop was raised. Says its effect stands, because a `write` /// that landed and was recorded as not run would be done again. const FINISHED_AFTER_STOP: &str = "was running when the turn was stopped; it finished, its \ effect stands, its output was withheld"; +/// A call that failed once the stop was raised. Claims no effect either way: a failed +/// `write` changed nothing, a timed-out `bash` may have. +const ERRORED_AFTER_STOP: &str = "was running when the turn was stopped; it did not succeed, \ + and its error was withheld"; + fn refused(id: &str, content: String) -> ContentBlock { ContentBlock::ToolResult { tool_use_id: id.to_string(), diff --git a/crates/sandbx-turn/src/turn.rs b/crates/sandbx-turn/src/turn.rs index 9bfbba23..ccfadf5c 100644 --- a/crates/sandbx-turn/src/turn.rs +++ b/crates/sandbx-turn/src/turn.rs @@ -142,11 +142,11 @@ pub enum TurnStop { /// A stop that lands while the model is streaming drops that round: the transcript is /// every round before it, or empty. One that lands in a round's tool calls keeps the /// round, every call answered, so it ends on a user turn of results, as legal to send - /// again as [`RoundLimit`](Self::RoundLimit)'s. A call that finished before the stop - /// says what it found; one running at it is killed if it is a `bash`, left running if it - /// only reads, and waited for if it writes, its effect standing (#26), and says only - /// that its output was withheld or that it was cancelled; one behind it was never put to - /// the gate or run, and says so. + /// again as [`RoundLimit`](Self::RoundLimit)'s. A call that had settled when the stop + /// was raised says what it found; one running then is killed if it is a `bash`, left + /// running if it only reads, and waited for if it writes, its effect standing (#26), and + /// says only that its output was withheld or that it was cancelled; one behind it was + /// never put to the gate or run, and says so. Interrupted, } @@ -258,6 +258,10 @@ where /// `edit` cannot be cancelled (#26), so it is waited for and its effect stays. No call /// behind it is put to the gate or run. /// +/// An abandoned call keeps its blocking thread, and dropping a runtime waits for every one: +/// a caller that must exit promptly after a stop ends its runtime with +/// `shutdown_background`, as `sandbx` does. +/// /// [`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 /// that finished streaming, the carried floor before one did: a cut planned for a round @@ -382,13 +386,9 @@ where // Before pushing: answering borrows `round.blocks`, pushing moves them. let answered = answer_calls(&round.blocks, ctx, turn.tools, &mut gate); let (answers, stopped) = unless_cancelling(stop.as_mut(), &cancel, answered).await; - let answers = match answers { - Ok(answers) => answers, - // A panicked tool during a stop: the round has no answers to keep. - Err(_) if stopped => return Ok(interrupted(produced, usage, streamed_cut, last_stop)), - Err(error) => return Err(error), - }; - let Answers { results, aborted } = answers; + // A tool that panics after the stop is answered as a failure, so an error here is one + // that came before it. + let Answers { results, aborted } = answers?; // No result means no call in the round, so nothing a stop could have left unsaid: it // is dropped as it was before calls were kept, not stored as a bare assistant turn. @@ -480,7 +480,6 @@ async fn unless_cancelling( } } -/// [`TurnStop::Interrupted`], over the rounds kept so far. fn interrupted( produced: Vec, usage: Option, diff --git a/crates/sandbx-turn/tests/turn_stop.rs b/crates/sandbx-turn/tests/turn_stop.rs index dfbdae6a..10d671fe 100644 --- a/crates/sandbx-turn/tests/turn_stop.rs +++ b/crates/sandbx-turn/tests/turn_stop.rs @@ -299,6 +299,50 @@ fn a_stop_mid_call_starts_no_call_behind_it() { assert_eq!(after.1, Some(true)); } +/// A `write` the stop landed on that then fails changed nothing, and is not stored as a +/// call whose effect stands: a resume would take it for done, and never redo it. +#[test] +fn a_call_that_fails_after_the_stop_is_not_stored_as_done() { + let root = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + let refused = outside.path().join("refused.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), + call( + "write", + serde_json::json!({ "path": refused.to_str().unwrap(), "content": "no" }), + ), + 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!(!refused.exists(), "the write was outside the policy"); + let [_, user] = &stopped.outcome.messages[..] else { + panic!( + "expected the stopped round, got {:?}", + stopped.outcome.messages + ); + }; + let [_, write] = &answers(user)[..] else { + panic!("a result for each of two calls, got {user:?}"); + }; + assert!(!write.0.contains("effect stands"), "got {write:?}"); + assert!(write.0.contains("did not succeed"), "got {write:?}"); + assert_eq!(write.1, Some(true)); +} + /// Round one answered, the stop in round two's only call: both rounds are kept (#26). #[test] fn a_stop_mid_call_keeps_the_rounds_before_it() { @@ -386,6 +430,8 @@ async fn a_stop_does_not_wait_for_a_read_that_cannot_finish() { panic!("a result for the one call, got {user:?}"); }; assert!(result.0.contains("cancelled"), "got {result:?}"); + // A killed `bash` shares this text, and may have changed state before it died. + assert!(result.0.contains("already done"), "got {result:?}"); assert_eq!(result.1, Some(true)); } From cbf4c51b3cce4c276c6b5af348a81c285fa00d72 Mon Sep 17 00:00:00 2001 From: danczw Date: Thu, 12 Nov 2026 14:47:44 +0100 Subject: [PATCH 12/12] docs: bound the stored-round claim by when the stop was raised --- SECURITY.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 7c41a2b1..11700226 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -333,11 +333,13 @@ architecture the filter gates on. stop while the model is still writing loses that round whole: whatever it streamed, including the part of the answer you read, and every tool call it asked for. A stop while its tools run keeps the round with every call - answered: a call that finished before the stop keeps its output; one running - at the stop is killed if it is a `bash`, left running if it only reads and - finishes if it writes, and is stored with its output withheld, since you - stopped before reading it and a resume would send it to the provider; one - behind it is stored as not run. What a withheld call did still stands. A stop + answered: a call that had settled when the stop was raised keeps its output; + one still running then, or settling as it was raised, is killed if it is a + `bash`, left running if it only reads and finishes if it writes, and is + stored with its output withheld, since you stopped before reading it and a + resume would send it to the provider; one behind it is stored as not run. + What a withheld call did still stands, and a killed or abandoned one is + stored as cancelled with anything it had done possibly remaining. A stop before the first round finishes streaming stores nothing, the prompt included ([#271](https://github.com/danczw/sandbx/issues/271)). Where a round was stored, resuming says so on stderr, and the model sees those results and @@ -380,9 +382,9 @@ architecture the filter gates on. ([`ToolLimits`](crates/sandbx-tools/src/tool_limits.rs)) — on a thread that cannot be cancelled, so one read on a stalled filesystem hangs indefinitely, holding that thread until the process exits; a stop stops waiting for it, - below. 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. + below. 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 `write` or `edit` cannot be interrupted, and a read is abandoned, not stopped.** A stop on `tui` (ctrl-c, escape, or a hung-up terminal) kills a running `bash` command with the kill a timeout makes, and stops waiting for a