fix(terminal): retry Ctrl+C for processes needing multiple SIGINT (#266)

src/integrations/terminal/TerminalProcess.ts

@@ -6,7 +6,20 @@ import { BaseTerminalProcess } from "./BaseTerminalProcess"
 import { Terminal } from "./Terminal"
 
 export class TerminalProcess extends BaseTerminalProcess {
+	// #266: Some processes (interactive tools, programs that trap SIGINT and
+	// prompt for confirmation) need more than one Ctrl+C to actually exit. We
+	// send Ctrl+C up to this many times in TOTAL — the immediate send in abort()
+	// plus retries — checking between sends whether the process has exited, before
+	// giving up and letting dispose() proceed.
+	private static readonly CTRL_C_SEND_LIMIT = 3
+	// Delay between Ctrl+C re-sends. Kept short so cancel stays responsive; the
+	// retry window is bounded by (CTRL_C_SEND_LIMIT - 1) * ABORT_RETRY_DELAY_MS.
+	private static readonly ABORT_RETRY_DELAY_MS = 500
+
 	private terminalRef: WeakRef<Terminal>
+	// Guards against overlapping abort retry loops if abort() is called again
+	// while a previous loop is still re-sending Ctrl+C.
+	private aborting = false
 
 	constructor(terminal: Terminal) {
 		super()
@@ -256,9 +269,62 @@ export class TerminalProcess extends BaseTerminalProcess {
 	}
 
 	public override abort() {
-		if (this.isListening) {
-			// Send SIGINT using CTRL+C
-			this.terminal.terminal.sendText("\x03")
+		if (!this.isListening) {
+			return
+		}
+
+		// Send SIGINT using CTRL+C.
+		this.terminal.terminal.sendText("\x03")
+
+		// #266: A single Ctrl+C isn't always enough — some processes trap SIGINT
+		// and keep running. Kick off a bounded retry that re-sends Ctrl+C a few
+		// times, verifying between attempts whether the process actually exited
+		// (terminal.busy flips to false on completion). This is intentionally
+		// fire-and-forget so it never blocks the synchronous cancel path; the
+		// total retry window is bounded so dispose() is never delayed for long.
+		if (!this.aborting) {
+			this.aborting = true
+			void this.retryAbort()
+				.finally(() => {
+					this.aborting = false
+				})
+				.catch((err) => console.error("[TerminalProcess] retryAbort error:", err))
+		}
+	}
+
+	/**
+	 * Re-sends Ctrl+C after the immediate send in abort(), up to CTRL_C_SEND_LIMIT
+	 * total sends, waiting ABORT_RETRY_DELAY_MS between sends and stopping early once
+	 * the process exits (or once we stop listening). Bounded so it can never loop
+	 * indefinitely.
+	 */
+	private async retryAbort(): Promise<void> {
+		// abort() already sent Ctrl+C once, so `sent` starts at 1; re-send until we
+		// reach CTRL_C_SEND_LIMIT total.
+		for (let sent = 1; sent < TerminalProcess.CTRL_C_SEND_LIMIT; sent++) {
+			await new Promise((resolve) => setTimeout(resolve, TerminalProcess.ABORT_RETRY_DELAY_MS))
+
+			// Stop as soon as there's nothing left to interrupt. `isListening` (cleared
+			// by continue()) and `terminal.busy` (cleared by shellExecutionComplete() /
+			// the "completed" event) are set on different code paths and can diverge, so
+			// either one being false is a sufficient stop signal — we deliberately check
+			// both rather than collapsing them into one.
+			if (!this.isListening) {
+				return
+			}
+
+			const terminal = this.terminalRef.deref()
+
+			// Stop if the terminal is gone, idle, or has already moved on to a different
+			// command. If the original command exits and the terminal is reused before this
+			// tick fires, `terminal.busy` can be true for the NEW command while
+			// `terminal.process` points at a different TerminalProcess — re-sending Ctrl+C
+			// then would interrupt an unrelated command, so we bail out.
+			if (!terminal || !terminal.busy || terminal.process !== this) {
+				return
+			}
+
+			terminal.terminal.sendText("\x03")
 		}
 	}
 

src/integrations/terminal/__tests__/TerminalProcess.spec.ts

@@ -186,6 +186,122 @@ describe("TerminalProcess", () => {
 		})
 	})
 
+	describe("abort", () => {
+		// These MIRROR the private production constants in TerminalProcess.ts
+		// (ABORT_RETRY_DELAY_MS and CTRL_C_SEND_LIMIT) — they can't be imported, so if
+		// those values are ever tuned, update them here too or the timing assertions
+		// below will keep passing while asserting the wrong cadence.
+		const RETRY_DELAY_MS = 500 // mirrors ABORT_RETRY_DELAY_MS
+		const MAX_ATTEMPTS = 3 // mirrors CTRL_C_SEND_LIMIT (total Ctrl+C sends)
+
+		beforeEach(() => {
+			vi.useFakeTimers()
+			// abort() runs against the terminal's *current* process; mirror that wiring so
+			// the reuse guard (terminal.process === this) lets the retry loop proceed.
+			mockTerminalInfo.process = terminalProcess
+		})
+
+		afterEach(() => {
+			vi.runOnlyPendingTimers()
+			vi.useRealTimers()
+		})
+
+		it("sends a single Ctrl+C immediately and nothing else when the process exits (#266)", async () => {
+			// Process exits right away: terminal is no longer busy.
+			mockTerminalInfo.busy = false
+
+			terminalProcess.abort()
+
+			// Immediate Ctrl+C.
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(1)
+			expect(mockTerminal.sendText).toHaveBeenCalledWith("\x03")
+
+			// Advance past the whole retry window; no further Ctrl+C since not busy.
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * MAX_ATTEMPTS)
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(1)
+		})
+
+		it("re-sends Ctrl+C up to the bounded maximum while the process stays busy (#266)", async () => {
+			// Process keeps ignoring SIGINT: terminal stays busy throughout.
+			mockTerminalInfo.busy = true
+
+			terminalProcess.abort()
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(1)
+
+			// Each retry tick re-sends Ctrl+C while still busy, bounded by MAX_ATTEMPTS.
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * (MAX_ATTEMPTS + 2))
+
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(MAX_ATTEMPTS)
+			expect(mockTerminal.sendText).toHaveBeenCalledWith("\x03")
+		})
+
+		it("stops re-sending Ctrl+C once the process exits mid-retry (#266)", async () => {
+			mockTerminalInfo.busy = true
+
+			terminalProcess.abort()
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(1)
+
+			// First retry tick: still busy, re-send.
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS)
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2)
+
+			// Process exits before the next tick — drive the real completion lifecycle
+			// (shellExecutionComplete clears busy and releases terminal.process) rather than
+			// mutating busy directly, so the test exercises the production wiring.
+			mockTerminalInfo.shellExecutionComplete({ exitCode: 0 })
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * MAX_ATTEMPTS)
+
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2)
+		})
+
+		it("stops re-sending Ctrl+C if the terminal is reused for a different process (#266)", async () => {
+			mockTerminalInfo.busy = true
+
+			terminalProcess.abort()
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(1)
+
+			// First retry tick: still busy, re-send.
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS)
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2)
+
+			// The original command exits and the terminal is reused for a NEW command before
+			// the next tick: terminal stays busy, but terminal.process now points at a
+			// different process. The retry must not interrupt that unrelated command.
+			mockTerminalInfo.process = new TestTerminalProcess(mockTerminalInfo)
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * MAX_ATTEMPTS)
+
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2)
+		})
+
+		it("does nothing when the process is no longer listening (#266)", async () => {
+			terminalProcess["isListening"] = false
+
+			terminalProcess.abort()
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * MAX_ATTEMPTS)
+
+			expect(mockTerminal.sendText).not.toHaveBeenCalled()
+		})
+
+		it("does not start overlapping retry loops when abort() is called repeatedly (#266)", async () => {
+			mockTerminalInfo.busy = true
+
+			terminalProcess.abort()
+			terminalProcess.abort()
+
+			// Two immediate Ctrl+C from the two abort() calls, but only one retry loop.
+			// This count of 2 relies on the `aborting` guard being checked AFTER the
+			// immediate sendText in abort(): the second call still fires its own Ctrl+C
+			// before the guard short-circuits the duplicate retry loop. If the guard ever
+			// moves above the send, this would drop to 1 immediate send (total 3, not 4).
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2)
+
+			await vi.advanceTimersByTimeAsync(RETRY_DELAY_MS * (MAX_ATTEMPTS + 2))
+
+			// 2 immediate + (MAX_ATTEMPTS - 1) retries from the single loop.
+			expect(mockTerminal.sendText).toHaveBeenCalledTimes(2 + (MAX_ATTEMPTS - 1))
+		})
+	})
+
 	describe("getUnretrievedOutput", () => {
 		it("returns and clears unretrieved output", () => {
 			terminalProcess["fullOutput"] = `\x1b]633;C\x07previous\nnew output\x1b]633;D\x07`