diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index d202191b9..465611c2f 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/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 + // 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 { + // 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") } } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index ba410b715..890d76d6e 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/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`