Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,10 @@ leave you answering one call blind.
One question, one answer, then the process ends. `sandbx tui` takes the same
flags and draws that turn on a screen instead of streaming it, where ctrl-c or
escape ends it mid-flight — `--session` keeping the rounds it finished but not
the one it stopped in, and a running tool call left to finish unseen. When the
turn ends the screen goes and the transcript is printed in its place, so the
answer stays in your scrollback. It needs a terminal on stdout, so it refuses a
the one it stopped in, and a running `bash` command killed, where any other tool
call finishes first. When the turn ends the screen goes and the transcript is
printed in its place, so the answer stays in your scrollback. It needs a
terminal on stdout, so it refuses a
piped run. `--approve call` asks on the screen, in rows under the transcript,
and one key answers: `y`, `n` or `a`, escape or ctrl-c refusing the call and
ending the turn ([#225](https://github.com/danczw/sandbx/issues/225)):
Expand Down
16 changes: 11 additions & 5 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,15 @@ architecture the filter gates on.
round it finished, under a stop naming an interrupt or a hung-up terminal, and
loses the round it stopped in whole: whatever it streamed, including the part
of the answer you read, and every tool call it asked for. The calls it had
already run did run, and one still running finishes, but the transcript
records the effect of neither. A stop before the first round finishes stores
already run did run, and one still running is killed if it is a `bash` and
finishes if it is not, but the transcript records the effect of neither. A
stop before the first round finishes stores
nothing, the prompt included, and leaves no trace in the session that a call
ran ([#271](https://github.com/danczw/sandbx/issues/271)). Where a round was
stored, resuming says so on stderr; the model is told nothing of it. What the
turn showed is printed back to the terminal once the screen is gone, and the
running call is named on stderr but never its result. Where the terminal is
call a stop stood on is named there as it ended, run or cancelled, but never
with its output. Where the terminal is
what went away, only a redirected stderr still has either. A stop during the
wrap-up round after `--max-rounds` keeps the rounds before it as the cap left
them, and loses only the wrap-up's prose
Expand Down Expand Up @@ -376,8 +378,12 @@ architecture the filter gates on.
Nothing else is capped: no CPU bound, no memory bound, no limit on processes
spawned. A fork bomb is unbounded while the call lasts; bounded is that it does
not outlive it.
- **A running tool call cannot be interrupted.** Only its own deadline stops it;
no way to cancel one from outside
- **Only a spawned command can be interrupted.** A stop on `tui` (ctrl-c,
escape, or a hung-up terminal) kills a running `bash` command with the kill a
timeout makes, and starts no call behind it. The six in-process tools cannot
be cancelled: one already running finishes, so a `write` under way lands, and
the stop waits for it — for as long as a stalled filesystem takes. Nothing a
killed command had already done is undone
([#26](https://github.com/danczw/sandbx/issues/26)).
- **A bound on the harness's own loop is not a containment claim.** A turn is
bounded — `TurnLimits::max_rounds` caps the requests one turn may make of the
Expand Down
16 changes: 8 additions & 8 deletions context/decision-approval-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ that ask it and implement it.
In `answer_calls`, between resolving the name and `spawn_blocking`.

A blocking task cannot be cancelled (#26): dropping the `JoinHandle` leaves the
task running to completion. So a gate consulted *concurrently* with the call —
`select!` on a decision and a `JoinHandle` — would answer "denied" about a `write`
that had already landed. Before the spawn is the only position where a refusal
means anything.
task running to completion, and only a `bash` has a kill to reach for besides.
So a gate consulted *concurrently* with the call — `select!` on a decision and a
`JoinHandle` — would answer "denied" about a `write` that had already landed.
Before the spawn is the only position where a refusal means anything.

Two refusals sit *above* the gate, so a caller's closure never sees either and
never has to invent a verdict for one:
Expand Down Expand Up @@ -203,8 +203,8 @@ It also keeps less. The abort is a `TurnStop`, so the turn returns and its
messages, usage and `withheld` are stored. A `tui` stop ends the turn through
`run_turn_until`, which keeps the rounds before it and drops the one it lands
in, so the round the operator was watching is stored nowhere, for a hangup as
for an interrupt (#271). A tool call already running finishes unseen (#26) on
both surfaces alike.
for an interrupt (#271). A tool call already running is killed if it is a `bash`
and finishes if it is not (#26), on both surfaces alike.

## Deny by default, and the honest claim

Expand Down Expand Up @@ -364,8 +364,8 @@ descriptor no task feeds, and nothing else is in flight during `approve`, since
- **A stop given earlier outranks an `a`.** Before the blanket is read, the gate
asks whether the turn was interrupted or the terminal has gone, and refuses or
aborts at once. An `a` was the operator's word about a tool, not a way past a
stop they gave since: a covered call would otherwise run, and a running call
cannot be cancelled (#26).
stop they gave since: a covered call would otherwise run, and an in-process
call cannot be cancelled (#26).
- **Refusal outranks an answer already pressed.** An interrupt beats a queued
`y`, and a hangup beats both. Each leaves the call refused, the direction a
doubtful reading must lean.
Expand Down
2 changes: 1 addition & 1 deletion context/decision-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ program that checks.
### Narrow by construction, at one site

Three `Command`s are built on the spawn path — the harness's spawn of the helper
(`helper_command`, shared by `run_to_completion` and `run_with_deadline`), the
(`helper_command`, shared by `run_to_completion` and `run_supervised`), the
supervisor's spawn of stage 2, and stage 2's spawn of the real program. None of
them calls `Command::new`: all go through `spawn::command`, which clears and
re-adds as it builds, so a narrowed environment is a property of every `Command`
Expand Down
2 changes: 1 addition & 1 deletion context/guide-sandboxing.md
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ construction rather than a call each site has to remember:
sandbx ──► helper stage 1 ──► stage 2 ──► the command
↑ ↑ ↑
helper_command() inner stage .spawn() ◄── the load-bearing one
(output, run_with_deadline)
(output, run_supervised)
└──────────────────┴───────────┘
all three via spawn::command
```
Expand Down
1 change: 1 addition & 0 deletions crates/sandbx-cli/src/agent_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ fn report(call: Settled<'_>) -> String {
ToolError::TimedOut { after, .. } => {
format!("{head} — timed out after {after:?} and was killed")
}
ToolError::Cancelled { .. } => format!("{head} — cancelled and killed"),
},
}
}
Expand Down
14 changes: 14 additions & 0 deletions crates/sandbx-cli/src/agent_gate_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,20 @@ fn a_policy_refusal_is_not_reported_as_a_call_that_ran() {
);
}

/// The operator who pressed ctrl-C reads this line, so it must say the command was killed
/// and not that it ran or that a limit was hit.
#[test]
fn a_cancelled_call_is_reported_as_killed() {
let error = ToolError::Cancelled {
subject: "run `sleep 80`".to_string(),
};

assert_eq!(
reported_bash("sleep 80", Outcome::Errored(&error)),
"bash sleep 80 — cancelled and killed"
);
}

/// Neither of these reaches `approve`, so this line is the operator's only account (#169).
#[test]
fn the_two_refusals_above_the_gate_are_reported() {
Expand Down
7 changes: 4 additions & 3 deletions crates/sandbx-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,10 @@ pub enum Command {
///
/// Interrupting costs two things, which sandbx prints again once the screen goes:
/// `--session` keeps the rounds the turn finished but not the one it stopped
/// in, and a tool already running finishes — sandbx cannot cancel one
/// mid-call, and the process waits for it before it exits. A turn interrupted
/// while it is being summarised after `--max-rounds` loses only the summary.
/// in, and a tool already running is dealt with first: a `bash` command is
/// killed, any other tool call finishes, and the process waits for it before
/// it exits. A turn interrupted while it is being summarised after
/// `--max-rounds` loses only the summary.
///
/// `--show-thinking` reads differently here: it draws the reasoning in the
/// pane rather than on stderr.
Expand Down
54 changes: 9 additions & 45 deletions crates/sandbx-cli/src/tui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
//! reprint; repainted only on an event, so the screen is still for a tool call.

use std::io::{self, Write};
use std::mem;
use std::sync::Mutex;

use sandbx_anthropic::AnthropicClient;
Expand Down Expand Up @@ -193,7 +192,6 @@ impl Tui {
Device::Terminal => approved.to_vec(),
Device::Redirected => Vec::new(),
},
running: Running::default(),
over: false,
});
let mut keys = Keys::listen();
Expand All @@ -205,8 +203,8 @@ impl Tui {
matches!(self.run.approve, Approve::Call).then(|| Modal::new(&pane, keys.answers()));

// Inside the loop rather than raced against it, so the rounds before the stop come
// back to be stored. A running tool still finishes, and the process waits on it,
// `Runtime::drop` having no timeout (#26).
// back to be stored. The stop kills a running `bash` and waits for an in-process
// tool, so the call it stood on settles, and is drawn, before the turn returns (#26).
let mut seen = None;
let first = run_turn_until(
&mut open,
Expand Down Expand Up @@ -274,7 +272,7 @@ impl Tui {
// a write that can stall on a paused terminal while the trail is still held.
pane.over = true;

let (code, outcome, closing) = match accounted(driven, mem::take(&mut pane.running)) {
let (code, outcome, closing) = match accounted(driven) {
Ok(Accounted { code, kept, said }) => (Ok(code), kept, said),
Err(error) => {
// In the reprint only where stderr went elsewhere: `main` prints the error
Expand Down Expand Up @@ -356,20 +354,18 @@ struct Accounted {
said: Vec<String>,
}

/// Account for what the turn's futures came to, `running` being the call a stop stood on.
/// Account for what the turn's futures came to.
///
/// Apart from `drive` so what a stop keeps is an assertion: the screen it is drawn on needs
/// a terminal, and `SECURITY.md` claims the difference.
fn accounted(driven: Driven, running: Running) -> Result<Accounted, TurnError> {
fn accounted(driven: Driven) -> Result<Accounted, TurnError> {
Ok(match driven {
Driven::Stopped(stopped, outcome) => {
let Ending { code, said } = stop(stopped, false);
// The call the dropped round stood on never settles, so this is the only line
// it gets.
Accounted {
code,
kept: Some((outcome, stored(stopped))),
said: said.into_iter().chain(running.stopped()).collect(),
said,
}
}
// The cap's own lines too, as the same turn stored under `--no-wrap-up` has them. The
Expand Down Expand Up @@ -432,8 +428,8 @@ fn stop(stopped: Stopped, wrapping: bool) -> Ending {
} else {
format!(
"{how}. Under `--session` the rounds it finished are kept and the one it stopped \
in is lost, and a tool call already running finishes unseen: the process waits \
for it before it exits (#26)"
in is lost. A `bash` command already running is killed; any other tool call \
already running finishes first, and what it did stays (#26)"
)
};

Expand Down Expand Up @@ -592,7 +588,6 @@ struct Pane {
screen: Screen,
/// Every line of the account, in order, for a stderr the reprint does not reach.
said: Vec<String>,
running: Running,
/// Whether the turn is over, after which a line is noted and not drawn.
over: bool,
}
Expand All @@ -610,32 +605,6 @@ impl Notes for Pane {
}
}

/// The call the gate let through and that has not settled yet.
///
/// Apart from [`Pane`] so a test can drive it without a screen.
#[derive(Default)]
struct Running(Option<String>);

impl Running {
/// Set on `Allow` alone: a refused call settles before anything runs.
fn approved(&mut self, call: ToolCall<'_>, verdict: &ApprovalDecision) {
if matches!(verdict, ApprovalDecision::Allow) {
self.0 = Some(gate::describe(call.tool, call.input));
}
}

fn settled(&mut self) {
self.0 = None;
}

/// The line a stop owes the call it stood on, if one was running.
fn stopped(self) -> Option<String> {
self.0.map(|call| {
format!("sandbx: {call} — still running when the turn stopped, and finishes unseen")
})
}
}

/// The pane as the seams share it: a line is noted as [`paint`] draws, not at all under a
/// poisoned lock.
impl Notes for &Mutex<Pane> {
Expand Down Expand Up @@ -683,19 +652,14 @@ impl CallGate for Gate<'_> {
/// Delegated, never re-derived: one `--allow-tool` approving two different sets across
/// two subcommands is the divergence nobody notices until a `bash` runs.
fn approve(&mut self, call: ToolCall<'_>) -> ApprovalDecision {
let verdict = self.argv.approve(call);
if let Ok(mut pane) = self.pane.lock() {
pane.running.approved(call, &verdict);
}
verdict
self.argv.approve(call)
}

/// The same line `agent-run` writes, through the pane: `ArgvGate::settled` puts it on
/// stderr, which the alternate screen neither redirects nor gives back.
fn settled(&mut self, call: Settled<'_>) {
let line = gate::line(call);
if let Ok(mut pane) = self.pane.lock() {
pane.running.settled();
pane.note(&line);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/sandbx-cli/src/tui_modal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<S: Shows, R: Replies> Operator for Modal<S, R> {
fn ask(&mut self, call: ToolCall<'_>) -> ApprovalDecision {
// Before the `a` that covers a tool: that was the operator's word about the tool,
// not a way past a stop they gave since, or a terminal that has gone. A covered
// call would otherwise run, and a running call cannot be cancelled (#26).
// call would otherwise run, and an in-process call cannot be cancelled (#26).
match self.replies.stopped() {
Some(Reply::Interrupted) => return deny(INTERRUPTED),
Some(Reply::Closed) => return abort(CLOSED),
Expand Down
64 changes: 14 additions & 50 deletions crates/sandbx-cli/src/tui_tests.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use super::super::render::Unfinished;
use super::super::{Device, INCOMPLETE, NO_CONSENT, gate};
use super::{
Accounted, Driven, Ending, Running, Tui, accounted, counted, ending, given_back, held, stop,
};
use super::{Accounted, Driven, Ending, Tui, accounted, counted, ending, given_back, held, stop};
use crate::{AgentError, Cli, Command};
use clap::Parser;
use sandbx_approval::{ApprovalDecision, ToolCall};
use sandbx_event::StopReason;
use sandbx_session_message::TurnStop as StoredStop;
use sandbx_tools::BuiltinTool;
use sandbx_tui_input::Stopped;
use sandbx_turn::{TurnError, TurnOutcome, TurnStop};
use std::io;
Expand Down Expand Up @@ -340,7 +336,7 @@ fn a_terminal_that_went_away_is_not_reported_as_an_interrupt() {
#[test]
fn every_stop_keeps_the_rounds_it_finished() {
let capped = TurnStop::RoundLimit { rounds: 4 };
let kept = |driven| accounted(driven, Running::default()).expect("not a failure");
let kept = |driven| accounted(driven).expect("not a failure");

for (stopped, code, stored) in [
(Stopped::Pressed, 2, StoredStop::Interrupted),
Expand Down Expand Up @@ -379,24 +375,26 @@ fn every_stop_keeps_the_rounds_it_finished() {
said[0].contains("the one it stopped in is lost"),
"{said:?}"
);
// What the operator is told about a call under way: the mechanism's whole claim.
assert!(
said[0].contains("`bash` command already running is killed"),
"{said:?}"
);
}
}

/// A round cut at `--max-tokens` and then capped says so even when the wrap-up is stopped,
/// the turn kept being the same one `--no-wrap-up` would have kept.
#[test]
fn a_stopped_wrap_up_still_names_a_cut_round() {
let said = accounted(
Driven::StoppedWrapping(
Stopped::Pressed,
outcome(
TurnStop::RoundLimit { rounds: 2 },
Some(StopReason::MaxTokens),
),
2,
let said = accounted(Driven::StoppedWrapping(
Stopped::Pressed,
outcome(
TurnStop::RoundLimit { rounds: 2 },
Some(StopReason::MaxTokens),
),
Running::default(),
)
2,
))
.expect("not a failure")
.said;
assert_eq!(said.len(), 3, "{said:?}");
Expand Down Expand Up @@ -461,37 +459,3 @@ fn compact_at_is_one_flag_under_both() {
);
assert_eq!(drawn.budget_tokens, 5000);
}

/// The call a stop lands on never settles, so the tracker is the only account it gets; one
/// the gate refused settled before it could be running.
#[test]
fn only_a_call_let_through_and_unsettled_is_named_on_a_stop() {
let input = serde_json::json!({ "command": "sleep 20" });
let call = ToolCall {
tool: BuiltinTool::Bash,
id: "toolu_1",
input: &input,
};

let mut running = Running::default();
running.approved(call, &ApprovalDecision::Allow);
let named = running.stopped().expect("an approved call is running");
assert!(
named.starts_with("sandbx: bash sleep 20 — still running"),
"{named}"
);

let mut refused = Running::default();
refused.approved(
call,
&ApprovalDecision::Deny {
reason: "no".to_string(),
},
);
assert_eq!(refused.stopped(), None);

let mut finished = Running::default();
finished.approved(call, &ApprovalDecision::Allow);
finished.settled();
assert_eq!(finished.stopped(), None);
}
Loading
Loading