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'