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

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

Some processes (interactive tools, programs that trap SIGINT and prompt
for confirmation) need more than one Ctrl+C to exit. The cancel path sent
a single Ctrl+C then disposed immediately, leaving such processes running
and the terminal stuck busy.

abort() now kicks off a bounded, fire-and-forget retry that re-sends
Ctrl+C up to 3 times (500ms apart), stopping early once the process exits
or we stop listening. The synchronous cancel path is never blocked and the
retry window is bounded so dispose() is never delayed indefinitely. The
ExecaTerminal backend is unaffected (it sends SIGKILL directly).

Follow-up to #245 / #261.

Closes #266

* refactor(terminal): clarify Ctrl+C retry naming and comments per review (#266)

- rename ABORT_MAX_ATTEMPTS -> CTRL_C_SEND_LIMIT (total sends) and start the
  retry loop at sent=1 so the bound reads naturally
- document why both isListening and terminal.busy are checked
- cross-reference the mirrored test constants to the production ones
- note the double-abort send-count assumption in the test
- drop the unused changeset

* fix(terminal): skip Ctrl+C retry when terminal is reused by a different process (#266)

* Update src/integrations/terminal/TerminalProcess.ts

---------

Co-authored-by: Armando Vaquera <263793884+proyectoauraorg@users.noreply.github.com>
Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>

Author: proyectoauraorg

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`