From e4db66137c29186bf9473989a6e675e82e5476f8 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Thu, 6 Aug 2026 10:19:08 +0700 Subject: [PATCH] fix: stop swallowing interrupts forever when an agent won't exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runAgent registered no-op SIGINT/SIGTERM handlers for the whole lifetime of the child so the terminal's interrupt would reach the agent without tearing the hub down first. The handlers were never escalated, so if the child stopped responding the hub could not be interrupted at all: the promise never settled and no number of ctrl+c presses could reach it. The user's only way out was closing the terminal window. Windows is where this bites. A hub-launched agent runs through four nested cmd.exe batch shims (our .cmd shim, npm's codevhub.cmd, the shell:true wrapper, npm's codev.cmd), and a batch host that catches a console break stops at "Terminate batch job (Y/N)?" rather than exiting. Swallow the first interrupt as before, then stop: a second one abandons the wait, tells the user the agent may still be shutting down, and resolves 130. The child is deliberately not killed — on Windows that would TerminateProcess the cmd.exe wrapper and orphan the real agent still attached to the console. The escalation does not fire during a normal session: interactive agents hold the terminal in raw mode, where ctrl+c arrives as input rather than as a signal. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/run.ts | 45 +++++++++++++++++++++----- tests/lib/run.test.ts | 74 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/src/lib/run.ts b/src/lib/run.ts index 84cd858..0615885 100644 --- a/src/lib/run.ts +++ b/src/lib/run.ts @@ -95,16 +95,45 @@ export function runAgent(cmd: string, args: string[]): Promise { }) : spawner.spawn(cmd, args, { stdio: "inherit", env }); - // The child shares our process group, so the terminal already delivers - // SIGINT/SIGTERM to it. Swallow them in the parent so we don't exit - // before the child finishes its own cleanup. - const swallow = () => {}; - process.on("SIGINT", swallow); - process.on("SIGTERM", swallow); + // The child shares our process group (POSIX) / console (Windows), so the + // terminal already delivers SIGINT/SIGTERM to it. Swallow the first one + // in the parent so we don't exit before the child finishes its cleanup. + // + // Never swallow indefinitely, though: an unconditional swallow makes the + // hub unkillable whenever the child fails to exit, and the user is left + // with a frozen terminal and no way out but closing the window. Windows + // is where this bites — the launch chain runs through several cmd.exe + // batch shims (our own .cmd shim, npm's codevhub.cmd, the shell:true + // wrapper below, npm's codev.cmd), and a batch host that catches a + // console break stops at "Terminate batch job (Y/N)?" instead of + // exiting. A second interrupt hands the terminal back. + // + // This does not fire during a normal agent session: interactive agents + // hold the terminal in raw mode, where ctrl+c is delivered as input + // rather than as a signal, so the count only moves once the terminal is + // generating real interrupts. + let interrupts = 0; + const onInterrupt = () => { + interrupts += 1; + if (interrupts < 2) return; + cleanup(); + process.stderr.write( + `\nStopped waiting for ${label}; it may still be shutting down in the background.\n`, + ); + logWarn(`abandoned ${label} after repeated interrupts`, { + action: "process.exit", + eventType: "end", + outcome: "failure", + extra: { agent: cmd, exit_code: 130 }, + }); + resolve(130); + }; + process.on("SIGINT", onInterrupt); + process.on("SIGTERM", onInterrupt); const cleanup = () => { - process.off("SIGINT", swallow); - process.off("SIGTERM", swallow); + process.off("SIGINT", onInterrupt); + process.off("SIGTERM", onInterrupt); }; child.once("error", (err: NodeJS.ErrnoException) => { diff --git a/tests/lib/run.test.ts b/tests/lib/run.test.ts index e9489d5..df3ed4e 100644 --- a/tests/lib/run.test.ts +++ b/tests/lib/run.test.ts @@ -104,6 +104,80 @@ describe("runAgent", () => { }, ); + // A fake child that never emits "exit" stands in for a wedged launch chain + // (on Windows, a cmd.exe shim sitting at "Terminate batch job (Y/N)?"). + // `process.emit` invokes our listeners without asking the OS for a real + // signal, so these run identically on Windows. + function withWedgedChild(): { + child: ChildProcess; + restore: () => void; + stderr: string[]; + } { + const child = new EventEmitter() as unknown as ChildProcess; + const spawnSpy = vi + .spyOn(spawner, "spawn") + .mockImplementation((() => child) as unknown as typeof spawner.spawn); + const original = process.stderr.write.bind(process.stderr); + const stderr: string[] = []; + process.stderr.write = ((chunk: string | Uint8Array) => { + stderr.push(typeof chunk === "string" ? chunk : chunk.toString()); + return true; + }) as typeof process.stderr.write; + return { + child, + stderr, + restore: () => { + process.stderr.write = original; + spawnSpy.mockRestore(); + }, + }; + } + + test("swallows a single interrupt so the child can clean up", async () => { + const { child, restore } = withWedgedChild(); + try { + const pending = runAgent("codev", []); + process.emit("SIGINT"); + await new Promise((r) => setImmediate(r)); + // Still waiting: the child's own exit code wins, not the interrupt. + child.emit("exit", 0, null); + expect(await pending).toBe(0); + } finally { + restore(); + } + }); + + test("stops waiting for a wedged child after a second interrupt", async () => { + const { restore, stderr } = withWedgedChild(); + try { + const pending = runAgent("codev", []); + process.emit("SIGINT"); + await new Promise((r) => setImmediate(r)); + process.emit("SIGINT"); + expect(await pending).toBe(130); + expect( + stderr.some((m) => m.includes("Stopped waiting for CoDev Code")), + ).toBe(true); + } finally { + restore(); + } + }); + + test("removes its signal listeners once it stops waiting", async () => { + const before = process.listenerCount("SIGINT"); + const { restore } = withWedgedChild(); + try { + const pending = runAgent("codev", []); + process.emit("SIGINT"); + await new Promise((r) => setImmediate(r)); + process.emit("SIGINT"); + await pending; + expect(process.listenerCount("SIGINT")).toBe(before); + } finally { + restore(); + } + }); + test("prints a startup banner to stderr before launching", async () => { // vi.spyOn(process.stderr, 'write') doesn't reliably intercept under // vitest's stdio handling, so swap the method directly. stdio:'inherit'