diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a87b1ed16..333cdd432 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -25,6 +25,10 @@ - Fixed follow-up shortcut submission of builtin slash commands so `/goal set ...` applies goal mode instead of queueing as plain text. +### Fixed + +- Fixed Ctrl+Z crashing the agent on Windows with `TypeError: Unknown signal: SIGTSTP`. `InputController.handleCtrlZ` called `process.kill(0, "SIGTSTP")` unconditionally, but `SIGTSTP` is POSIX job-control and Bun/Node on Windows rejects the signal name from the JS side; the throw propagated out of the TUI input dispatcher as an uncaught exception. The handler now no-ops with a "Suspend (Ctrl+Z) is not supported on this platform" status on Windows, and on POSIX wraps `process.kill` in a try/catch that detaches the registered SIGCONT resume hook and re-`start()`s the TUI on failure so a rejected signal can never leave the UI stranded with a leaked listener ([#2036](https://github.com/can1357/oh-my-pi/issues/2036)). + ## [15.10.1] - 2026-06-07 ### Added diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index d1e0ef6ad..c2df60aa4 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -499,17 +499,44 @@ export class InputController { } handleCtrlZ(): void { - // Set up handler to restore TUI when resumed - process.once("SIGCONT", () => { + // SIGTSTP is POSIX job-control: Windows has no equivalent and + // `process.kill(_, "SIGTSTP")` throws `TypeError: Unknown signal: + // SIGTSTP` there, taking the whole agent down via an uncaught + // exception (issue #2036). No-op on platforms that cannot suspend. + if (process.platform === "win32") { + this.ctx.showStatus("Suspend (Ctrl+Z) is not supported on this platform"); + return; + } + + // Capture the listener so we can detach it if the signal never + // fires; otherwise a failed suspend would leave a stale SIGCONT + // handler that fires on the next unrelated continue and tries to + // re-`start()` an already-running TUI. + const onResume = (): void => { this.ctx.ui.start(); this.ctx.ui.requestRender(true); - }); + }; + process.once("SIGCONT", onResume); - // Stop the TUI (restore terminal to normal mode) + // Stop the TUI (restore terminal to normal mode) before sending the + // signal so the parent shell sees a sane terminal state. this.ctx.ui.stop(); - // Send SIGTSTP to process group (pid=0 means all processes in group) - process.kill(0, "SIGTSTP"); + try { + // pid=0 → entire foreground process group; the shell receives + // SIGTSTP and parks the job. + process.kill(0, "SIGTSTP"); + } catch (err) { + // Either the runtime refused the signal or the kernel rejected + // it (some sandboxes block sending to pid=0). Tear the resume + // hook down and bring the TUI back so the user is not stranded + // on a frozen prompt. + process.removeListener("SIGCONT", onResume); + this.ctx.ui.start(); + this.ctx.ui.requestRender(true); + const reason = err instanceof Error ? err.message : String(err); + this.ctx.showError(`Failed to suspend: ${reason}`); + } } handleDequeue(): void { diff --git a/packages/coding-agent/test/input-controller-suspend.test.ts b/packages/coding-agent/test/input-controller-suspend.test.ts new file mode 100644 index 000000000..73945e9c9 --- /dev/null +++ b/packages/coding-agent/test/input-controller-suspend.test.ts @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, it, type Mock, vi } from "bun:test"; +import { InputController } from "../src/modes/controllers/input-controller"; +import type { InteractiveModeContext } from "../src/modes/types"; + +interface SuspendCtx { + ctx: InteractiveModeContext; + ui: { + start: Mock<() => void>; + stop: Mock<() => void>; + requestRender: Mock<(force?: boolean) => void>; + }; + showStatus: Mock<(message: string) => void>; + showError: Mock<(message: string) => void>; +} + +function createCtx(): SuspendCtx { + const ui = { + start: vi.fn(), + stop: vi.fn(), + requestRender: vi.fn(), + }; + const showStatus = vi.fn(); + const showError = vi.fn(); + const ctx = { + ui: ui as unknown as InteractiveModeContext["ui"], + showStatus, + showError, + } as unknown as InteractiveModeContext; + return { ctx, ui, showStatus, showError }; +} + +const originalPlatform = process.platform; + +function setPlatform(value: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { value, configurable: true, writable: true }); +} + +afterEach(() => { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true, writable: true }); + vi.restoreAllMocks(); + // Drop any SIGCONT listener a passing test left behind so a later test + // (or the next file) doesn't get spurious callbacks. + process.removeAllListeners("SIGCONT"); +}); + +describe("InputController.handleCtrlZ", () => { + it("no-ops on Windows so the unsupported SIGTSTP signal can't crash the process (#2036)", () => { + setPlatform("win32"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw new Error("process.kill must not be called on win32"); + }); + const onceSpy = vi.spyOn(process, "once"); + const { ctx, ui, showStatus, showError } = createCtx(); + + const controller = new InputController(ctx); + expect(() => controller.handleCtrlZ()).not.toThrow(); + + expect(killSpy).not.toHaveBeenCalled(); + expect(onceSpy).not.toHaveBeenCalledWith("SIGCONT", expect.anything()); + expect(ui.stop).not.toHaveBeenCalled(); + expect(ui.start).not.toHaveBeenCalled(); + expect(showStatus).toHaveBeenCalledTimes(1); + expect(showStatus.mock.calls[0]?.[0]).toMatch(/not supported/i); + expect(showError).not.toHaveBeenCalled(); + }); + + it("sends SIGTSTP to the process group and registers a SIGCONT resume hook on POSIX", () => { + setPlatform("linux"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true); + const onceSpy = vi.spyOn(process, "once"); + const { ctx, ui, showError } = createCtx(); + + const controller = new InputController(ctx); + controller.handleCtrlZ(); + + // Resume hook registered BEFORE the signal is sent so a same-tick + // SIGCONT delivery can't race past us. + expect(onceSpy).toHaveBeenCalledWith("SIGCONT", expect.any(Function)); + const sigcontOrder = onceSpy.mock.invocationCallOrder[0] ?? Infinity; + const stopOrder = ui.stop.mock.invocationCallOrder[0] ?? Infinity; + const killOrder = killSpy.mock.invocationCallOrder[0] ?? Infinity; + expect(sigcontOrder).toBeLessThan(stopOrder); + expect(stopOrder).toBeLessThan(killOrder); + + expect(killSpy).toHaveBeenCalledTimes(1); + expect(killSpy).toHaveBeenCalledWith(0, "SIGTSTP"); + expect(ui.start).not.toHaveBeenCalled(); + expect(showError).not.toHaveBeenCalled(); + + // Simulating the kernel-delivered SIGCONT drives the TUI back up. + const resume = onceSpy.mock.calls.find(([sig]) => sig === "SIGCONT")?.[1] as (() => void) | undefined; + expect(resume).toBeDefined(); + resume?.(); + expect(ui.start).toHaveBeenCalledTimes(1); + expect(ui.requestRender).toHaveBeenCalledWith(true); + }); + + it("restores the TUI and drops the SIGCONT listener when process.kill rejects the signal", () => { + setPlatform("linux"); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw new Error("Unknown signal: SIGTSTP"); + }); + const onceSpy = vi.spyOn(process, "once"); + const removeSpy = vi.spyOn(process, "removeListener"); + const { ctx, ui, showError, showStatus } = createCtx(); + + const controller = new InputController(ctx); + // Critical contract: the failure must not bubble up to the caller — + // otherwise the TUI's stdin reader (which invoked us) crashes the + // whole process via `[Uncaught Exception]`. + expect(() => controller.handleCtrlZ()).not.toThrow(); + + // The exact listener we registered for SIGCONT is the one we + // remove; otherwise a leaked handler would fire on the next + // unrelated continue and re-`start()` an already-running TUI. + const registered = onceSpy.mock.calls.find(([sig]) => sig === "SIGCONT")?.[1]; + expect(registered).toBeDefined(); + expect(removeSpy).toHaveBeenCalledWith("SIGCONT", registered); + + expect(killSpy).toHaveBeenCalledTimes(1); + expect(ui.stop).toHaveBeenCalledTimes(1); + expect(ui.start).toHaveBeenCalledTimes(1); + expect(ui.requestRender).toHaveBeenCalledWith(true); + expect(showError).toHaveBeenCalledTimes(1); + expect(showError.mock.calls[0]?.[0]).toMatch(/Failed to suspend/); + expect(showStatus).not.toHaveBeenCalled(); + }); +});