From 8e5c7c9bf174243c757a0eac5cfaaf7c17ff7d2d Mon Sep 17 00:00:00 2001
From: roboomp <omp@can.ac>
Date: Mon, 25 May 2026 18:40:22 +0000
Subject: [PATCH] fix(bash): kept persistent shells after cancel

Stopped marking persistent bash sessions as permanently broken when the JavaScript abort or timeout race wins.

Stopped the Rust descendant kill-wave helper once no cancellation targets remain so later commands are not swept into old cancels.

Fixes #1347
---
 crates/pi-shell/src/shell.rs                  | 19 ++++++++++---------
 .../coding-agent/src/exec/bash-executor.ts    |  3 ---
 .../coding-agent/test/bash-executor.test.ts   | 14 ++++++++++++++
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs
index 4efa4f343..a813a2670 100644
--- a/crates/pi-shell/src/shell.rs
+++ b/crates/pi-shell/src/shell.rs
@@ -972,21 +972,22 @@ async fn read_output_bytes(
 }
 
 // Rescan-and-signal loop for cancellation. Each pass picks up descendants
-// spawned during the previous wave's grace period; empty scans keep going so a
-// cancellation that wins before `run_string` exposes its child still escalates.
+// spawned during the previous wave's grace period, then exits as soon as no
+// targets remain so unrelated later commands are not swept into old cancels.
 async fn terminate_new_descendants<S: std::hash::BuildHasher>(baseline: &HashSet<i32, S>) {
 	const WAVES: u32 = 3;
 	for wave in 0..WAVES {
 		let mut targets = process::TerminationTargets::new();
 		process::add_new_descendants(&mut targets, baseline);
-		if !targets.is_empty() {
-			let signal = if wave == 0 {
-				process::TERM_SIGNAL
-			} else {
-				process::KILL_SIGNAL
-			};
-			targets.signal(signal);
+		if targets.is_empty() {
+			return;
 		}
+		let signal = if wave == 0 {
+			process::TERM_SIGNAL
+		} else {
+			process::KILL_SIGNAL
+		};
+		targets.signal(signal);
 		if wave + 1 < WAVES {
 			let pause = if wave == 0 {
 				Duration::from_millis(75)
diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts
index e5453d20d..a3db8e24d 100644
--- a/packages/coding-agent/src/exec/bash-executor.ts
+++ b/packages/coding-agent/src/exec/bash-executor.ts
@@ -207,9 +207,6 @@ export async function executeBash(command: string, options?: BashExecutorOptions
 			void runPromise.catch(() => undefined);
 			if (shellSession) {
 				resetSession = true;
-				// Fall back to one-shot execution for the rest of the process once
-				// a persistent session has stopped responding to cancellation.
-				brokenShellSessions.add(sessionKey);
 			}
 			return {
 				exitCode: undefined,
diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts
index 67ec0b76a..49d7700b5 100644
--- a/packages/coding-agent/test/bash-executor.test.ts
+++ b/packages/coding-agent/test/bash-executor.test.ts
@@ -197,6 +197,20 @@ describe("executeBash", () => {
 			expect(raced.result.output).toContain("Command cancelled");
 		}
 		expect(abortSpy).toHaveBeenCalled();
+
+		vi.restoreAllMocks();
+
+		await executeBash("export PI_AFTER_ABORT=still_persistent", {
+			cwd: tempDir,
+			timeout: 5000,
+			sessionKey: "hung-native-abort",
+		});
+		const next = await executeBash("printf '%s\n' \"$PI_AFTER_ABORT\"", {
+			cwd: tempDir,
+			timeout: 5000,
+			sessionKey: "hung-native-abort",
+		});
+		expect(next.output.trim()).toBe("still_persistent");
 	});
 
 	it("returns at the JavaScript timeout when native timeout cleanup stalls", async () => {