@sigitcodereview / sigit / commits / f19fe5c

Kill a running command when the turn is cancelled

A foreground run_command was only observed after it exited, so pressing stop left it running for up to the two minute timeout. A cancel or session close now raises a per-session abort signal. The local poll loop kills the process tree on it, and a command in the client's terminal gets terminal/kill, then the output is read and the terminal released as on a timeout. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: siGit Code v1.6.6-acp <noreply@sigit.si>

Seto Elkahfi committed Oct 10, 2026 at 17:05 UTC f19fe5c40b5bf95b83ffe4de4b5b30888f6b4238

6 files changed +277 -8

.agents/AGENTS.md

+6 -1

@@ -335,7 +335,12 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

335 the top of `handle_prompt`, before it waits on either lock, and a `PromptRegistration`

336 guard clears it on every return, so a `session/cancel` that arrives while the turn

337 is still queued, switching threads, loading a model or running a slash command ends

338 - it as `cancelled`. The checks sit after each of those waits. Tool

338 + it as `cancelled`. The checks sit after each of those waits. A cancel (and

339 + `begin_close`) also raises the session's `tools::AbortSignal`, which the local

340 + `run_command` poll loop and `client_terminal::Route::run` watch, so a running

341 + foreground command is killed instead of finishing first. The signal is reset at the

342 + top of each turn, not when a tool starts, so a cancel that lands during a permission

343 + prompt still stops the command that follows. Tool

344 execution, subagents included, still runs one session at a time.

345 - **`src/instructions.rs`** — project instruction files, the always-on counterpart to skills.

346 Reads `AGENTS.md` (the cross-tool [agents.md](https://agents.md) standard) and `CLAUDE.md`,

CHANGELOG.md

@@ -9,6 +9,11 @@

9 inference. A cancel that lands while the turn is queued on the workspace,

10 switching threads, loading a model, or running a slash command ends it with

11 `cancelled` instead of `end_turn`.

12 +- **Cancelling a turn stops the command it is running.** A foreground

13 + `run_command` used to run to its own end (up to two minutes) before the turn

14 + noticed the cancel. A cancel or `session/close` now kills the process tree, or

15 + sends `terminal/kill` when the command runs in the editor's terminal, and the

16 + turn ends `cancelled` within a moment.

17 - **A prompt that is only audio is answered.** A clip with no text or image used

18 to end the turn at once with nothing sent to the model, even though the agent

19 advertises audio.

src/client_terminal.rs

+68 -4

@@ -141,6 +141,8 @@ pub enum Run {

141 },

142 /// The command was still running at the timeout and was killed.

143 TimedOut { terminal_id: String, output: Output },

144 + /// The user cancelled the turn while the command ran, and it was killed.

145 + Cancelled { terminal_id: String, output: Output },

146 /// The terminal started but the client failed a later request. The

147 /// command may have run, so it must not be run again.

148 Failed { terminal_id: String, error: String },

@@ -178,9 +180,27 @@ impl Route {

180 self.terminal.embed(session, tool_call_id, id).await;

181 }

182

181 - let waited = tokio::time::timeout(timeout, self.terminal.wait_for_exit(session, id)).await;

183 + let abort = crate::tools::abort_signal(session);

184 + let waited = tokio::select! {

185 + waited = tokio::time::timeout(timeout, self.terminal.wait_for_exit(session, id)) => {

186 + Some(waited)

187 + }

188 + () = abort.aborted() => None,

189 + };

190 let run = match waited {

183 - Ok(Ok(status)) => match self.read_output(id).await {

191 + None => {

192 + if let Err(error) = self

193 + .ask("terminal/kill", self.terminal.kill(session, id))

194 + .await

195 + {

196 + log::warn!("could not kill terminal {id}: {error}");

197 + }

198 + Run::Cancelled {

199 + terminal_id: terminal_id.clone(),

200 + output: self.read_output(id).await.unwrap_or_default(),

201 + }

202 + }

203 + Some(Ok(Ok(status))) => match self.read_output(id).await {

204 Ok(output) => Run::Exited {

205 terminal_id: terminal_id.clone(),

206 status,

@@ -191,11 +211,11 @@ impl Route {

211 error,

212 },

213 },

194 - Ok(Err(error)) => Run::Failed {

214 + Some(Ok(Err(error))) => Run::Failed {

215 terminal_id: terminal_id.clone(),

216 error: format!("terminal/wait_for_exit failed: {error}"),

217 },

198 - Err(_) => {

218 + Some(Err(_)) => {

219 // Kill before reading, so the output is what the command

220 // printed up to the timeout and nothing more.

221 if let Err(error) = self

@@ -404,6 +424,50 @@ mod tests {

424 assert_eq!(limit, 1000);

425 }

426

427 + #[tokio::test]

428 + async fn a_cancel_kills_the_terminal_then_reads_what_it_printed() {

429 + let terminal = Arc::new(FakeTerminal {

430 + exit: None,

431 + output: "partial\n",

432 + ..FakeTerminal::default()

433 + });

434 + let route = Route {

435 + terminal: Arc::clone(&terminal) as Arc<dyn ClientTerminal>,

436 + session_id: "session-cancel".to_string(),

437 + };

438 + crate::tools::reset_abort("session-cancel");

439 + let cwd = std::env::temp_dir();

440 +

441 + let (run, ()) = tokio::join!(

442 + route.run("sleep 999", &cwd, None, Duration::from_secs(30), 1000,),

443 + async {

444 + tokio::time::sleep(Duration::from_millis(50)).await;

445 + crate::tools::abort_session_tools("session-cancel");

446 + }

447 + );

448 +

449 + assert_eq!(

450 + run,

451 + Run::Cancelled {

452 + terminal_id: "term-1".to_string(),

453 + output: Output {

454 + output: "partial\n".to_string(),

455 + truncated: false

456 + },

457 + }

458 + );

459 + assert_eq!(

460 + terminal.calls(),

461 + [

462 + "create session-cancel",

463 + "wait term-1",

464 + "kill term-1",

465 + "output term-1",

466 + "release term-1",

467 + ]

468 + );

469 + }

470 +

471 #[tokio::test]

472 async fn a_command_past_the_timeout_is_killed_then_read() {

473 let terminal = Arc::new(FakeTerminal {

src/main.rs

+7 -3

@@ -3048,6 +3048,7 @@ impl SiGitAgent {

3048 permissions::reset_session(&key);

3049 self.announced_titles.lock().unwrap().remove(&key);

3050 let killed = tools::kill_session_tasks(&key);

3051 + tools::forget_abort(&key);

3052 self.closing_sessions.lock().unwrap().remove(&key);

3053

3054 log::info!(

@@ -3090,6 +3091,7 @@ impl SiGitAgent {

3091 cancellation.cancelled.store(true, Ordering::Release);

3092 cancellation.notify.notify_one();

3093 }

3094 + tools::abort_session_tools(&key);

3095 }

3096

3097 async fn handle_fork_session(

@@ -3231,6 +3233,7 @@ impl SiGitAgent {

3233 // sessions or loading a model is seen. The session lock is held, so the

3234 // slot can only be this turn's. The guard clears it on every return.

3235 let cancellation = Arc::new(PromptCancellation::default());

3236 + tools::reset_abort(&session_id.to_string());

3237 self.prompt_cancellations

3238 .lock()

3239 .unwrap()

@@ -3917,9 +3920,9 @@ impl SiGitAgent {

3920 tool_call_id: tc.id.clone(),

3921 content: output,

3922 });

3920 - // Cancellation cannot preempt a synchronous foreground tool:

3921 - // it is observed here as soon as that tool exits. Background

3922 - // commands remain cancellable through `kill_command`.

3923 + // A cancel aborts a running foreground command (see

3924 + // `tools::abort_session_tools`), so it exits and is observed

3925 + // here. Background commands are stopped with `kill_command`.

3926 if cancellation.cancelled.load(Ordering::Acquire) {

3927 for pending in &result.tool_calls[call_index + 1..] {

3928 tool_results.push(BackendToolResult {

@@ -4279,6 +4282,7 @@ impl SiGitAgent {

4282 cancellation.cancelled.store(true, Ordering::Release);

4283 cancellation.notify.notify_one();

4284 }

4285 + tools::abort_session_tools(&args.session_id.to_string());

4286 Ok(())

4287 }

4288

src/tools.rs

+106

@@ -2295,6 +2295,8 @@ fn exec_delete_file(arguments: &str) -> String {

2295

2296 // ── run_command ──────────────────────────────────────────────────────────────

2297

2298 +/// What a foreground command reports when the user cancelled the turn under it.

2299 +const CANCELLED_BY_USER: &str = "Error: command was cancelled by the user and killed.";

2300 const COMMAND_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(120);

2301 const COMMAND_OUTPUT_LIMIT: usize = 50_000;

2302 /// How long to keep waiting for the output drains once the command itself has

@@ -2679,11 +2681,16 @@ fn exec_run_command(arguments: &str, owner: Option<&str>) -> String {

2681 drain(1, child.stderr.take().map(|pipe| Box::new(pipe) as _));

2682 drop(chunks_tx);

2683

2684 + let abort = owner.map(abort_signal);

2685 let start = std::time::Instant::now();

2686 let status = loop {

2687 match child.try_wait() {

2688 Ok(Some(status)) => break status,

2689 Ok(None) => {

2690 + if abort.as_ref().is_some_and(|signal| signal.is_aborted()) {

2691 + kill_shell_tree(&mut child);

2692 + return CANCELLED_BY_USER.to_string();

2693 + }

2694 if start.elapsed() >= COMMAND_TIMEOUT {

2695 kill_shell_tree(&mut child);

2696 return format!(

@@ -2809,6 +2816,18 @@ async fn exec_run_command_via_client(

2816 }

2817 (result, terminal_id)

2818 }

2819 + Run::Cancelled {

2820 + terminal_id,

2821 + output,

2822 + } => {

2823 + let mut result = CANCELLED_BY_USER.to_string();

2824 + let combined = client_output_text(output);

2825 + if !combined.is_empty() {

2826 + result.push_str("\nOutput before it was killed:\n");

2827 + result.push_str(&combined);

2828 + }

2829 + (result, terminal_id)

2830 + }

2831 Run::Failed { terminal_id, error } => (format!("Error: {error}"), terminal_id),

2832 };

2833

@@ -2880,6 +2899,64 @@ fn active_session() -> Option<String> {

2899 .clone()

2900 }

2901

2902 +/// Raised when a session's turn is cancelled or its session closed, so a

2903 +/// foreground tool does not run on to its own end with nobody waiting for it.

2904 +#[derive(Default)]

2905 +pub struct AbortSignal {

2906 + aborted: std::sync::atomic::AtomicBool,

2907 + notify: tokio::sync::Notify,

2908 +}

2909 +

2910 +impl AbortSignal {

2911 + pub fn is_aborted(&self) -> bool {

2912 + self.aborted.load(Ordering::Acquire)

2913 + }

2914 +

2915 + /// Resolves once the signal is raised. `notify_one` keeps a permit when

2916 + /// nothing is waiting, so a signal raised just before this is polled still

2917 + /// wakes it.

2918 + pub async fn aborted(&self) {

2919 + while !self.is_aborted() {

2920 + self.notify.notified().await;

2921 + }

2922 + }

2923 +}

2924 +

2925 +fn aborts() -> &'static Mutex<HashMap<String, Arc<AbortSignal>>> {

2926 + static ABORTS: OnceLock<Mutex<HashMap<String, Arc<AbortSignal>>>> = OnceLock::new();

2927 + ABORTS.get_or_init(|| Mutex::new(HashMap::new()))

2928 +}

2929 +

2930 +fn lock_aborts() -> std::sync::MutexGuard<'static, HashMap<String, Arc<AbortSignal>>> {

2931 + aborts()

2932 + .lock()

2933 + .unwrap_or_else(|poisoned| poisoned.into_inner())

2934 +}

2935 +

2936 +/// The abort signal of `session`'s current turn. Created on first use, so a

2937 +/// signal raised before any tool asks for it is still there when one does.

2938 +pub fn abort_signal(session: &str) -> Arc<AbortSignal> {

2939 + Arc::clone(lock_aborts().entry(session.to_string()).or_default())

2940 +}

2941 +

2942 +/// Start a turn with a clean slate: a cancel aimed at an earlier turn must not

2943 +/// kill this turn's first command.

2944 +pub fn reset_abort(session: &str) {

2945 + lock_aborts().insert(session.to_string(), Arc::new(AbortSignal::default()));

2946 +}

2947 +

2948 +/// Tell `session`'s running foreground tool to stop.

2949 +pub fn abort_session_tools(session: &str) {

2950 + let signal = abort_signal(session);

2951 + signal.aborted.store(true, Ordering::Release);

2952 + signal.notify.notify_one();

2953 +}

2954 +

2955 +/// Forget `session`'s signal once the session is gone.

2956 +pub fn forget_abort(session: &str) {

2957 + lock_aborts().remove(session);

2958 +}

2959 +

2960 /// Process-global background task table (same pattern as `mcp::MCP`).

2961 fn tasks() -> &'static Mutex<HashMap<u64, BackgroundTask>> {

2962 static TASKS: OnceLock<Mutex<HashMap<u64, BackgroundTask>>> = OnceLock::new();

@@ -4028,6 +4105,35 @@ mod tests {

4105 );

4106 }

4107

4108 + #[cfg(unix)]

4109 + #[test]

4110 + fn a_cancel_kills_a_running_foreground_command() {

4111 + reset_abort("tools-abort");

4112 + let trigger = std::thread::spawn(|| {

4113 + std::thread::sleep(std::time::Duration::from_millis(300));

4114 + abort_session_tools("tools-abort");

4115 + });

4116 + let started = std::time::Instant::now();

4117 + let result = exec_run_command(r#"{"command": "sleep 30"}"#, Some("tools-abort"));

4118 + trigger.join().unwrap();

4119 +

4120 + assert_eq!(result, CANCELLED_BY_USER);

4121 + assert!(

4122 + started.elapsed() < std::time::Duration::from_secs(10),

4123 + "the command was not killed: {:?}",

4124 + started.elapsed()

4125 + );

4126 + }

4127 +

4128 + #[cfg(unix)]

4129 + #[test]

4130 + fn a_new_turn_is_not_aborted_by_an_earlier_cancel() {

4131 + abort_session_tools("tools-stale");

4132 + reset_abort("tools-stale");

4133 + let result = exec_run_command(r#"{"command": "echo fine"}"#, Some("tools-stale"));

4134 + assert!(result.contains("fine"), "{result}");

4135 + }

4136 +

4137 #[test]

4138 fn test_run_command_success() {

4139 let result = exec_run_command(r#"{"command": "echo hello"}"#, None);

tests/acp_client_terminal.rs

+85

@@ -294,6 +294,56 @@ impl AgentUnderTest {

294 .to_string()

295 }

296

297 + /// Send a prompt, then `session/cancel` as soon as the agent starts

298 + /// waiting on the editor's terminal, which never answers. Returns the

299 + /// prompt's response.

300 + fn prompt_cancelled_mid_command(&mut self, session_id: &str, editor: &mut Editor) -> Value {

301 + let id = self.request(

302 + "session/prompt",

303 + json!({"sessionId": session_id, "prompt": [{"type": "text", "text": "go"}]}),

304 + );

305 + let deadline = Instant::now() + TIMEOUT;

306 + loop {

307 + let remaining = deadline.saturating_duration_since(Instant::now());

308 + let Ok(message) = self.incoming.recv_timeout(remaining) else {

309 + panic!("timed out: the cancel did not end the prompt");

310 + };

311 + if message["id"] == id && message.get("method").is_none() {

312 + return message;

313 + }

314 + let Some(method) = message["method"].as_str() else {

315 + continue;

316 + };

317 + if method == "session/update" {

318 + editor.updates.push(message.clone());

319 + continue;

320 + }

321 + if !method.starts_with("terminal/") {

322 + continue;

323 + }

324 + editor.requests.push(message.clone());

325 + let request_id = message["id"].clone();

326 + let result = match method {

327 + "terminal/create" => json!({"terminalId": "term-1"}),

328 + "terminal/wait_for_exit" => {

329 + // The command is still running; the user presses stop.

330 + self.send(json!({

331 + "jsonrpc": "2.0",

332 + "method": "session/cancel",

333 + "params": {"sessionId": session_id},

334 + }));

335 + continue;

336 + }

337 + "terminal/output" => json!({

338 + "output": "partial\n",

339 + "truncated": false,

340 + }),

341 + _ => json!({}),

342 + };

343 + self.send(json!({"jsonrpc": "2.0", "id": request_id, "result": result}));

344 + }

345 + }

346 +

347 fn prompt(&mut self, session_id: &str, editor: &mut Editor) {

348 let id = self.request(

349 "session/prompt",

@@ -502,3 +552,38 @@ fn sigit_client_terminal_off_keeps_commands_local() {

552 assert!(result.contains("ran-locally"), "run_command said: {result}");

553 assert!(editor.requests.is_empty(), "{:?}", editor.requests);

554 }

555 +

556 +#[test]

557 +fn a_cancel_mid_command_kills_the_client_terminal_and_ends_the_turn_cancelled() {

558 + let scratch = Scratch::new("cancel");

559 + let endpoint = start_fake_endpoint(vec![

560 + echo_call("call_1", &scratch.work, false),

561 + sse_text("never asked for"),

562 + ]);

563 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

564 + let session_id = agent.open_session(true, &scratch.work);

565 +

566 + let mut editor = Editor::default();

567 + let response = agent.prompt_cancelled_mid_command(&session_id, &mut editor);

568 +

569 + assert_eq!(response["result"]["stopReason"], "cancelled", "{response}");

570 + assert_eq!(

571 + editor.methods(),

572 + [

573 + "terminal/create",

574 + "terminal/wait_for_exit",

575 + "terminal/kill",

576 + "terminal/output",

577 + "terminal/release",

578 + ]

579 + );

580 + assert_eq!(

581 + editor.request("terminal/kill")["params"]["terminalId"],

582 + "term-1"

583 + );

584 + assert_eq!(

585 + endpoint.requests.lock().unwrap().len(),

586 + 1,

587 + "a cancelled turn must not ask the model for another round"

588 + );

589 +}