fix(coding-agent): restore terminal on SIGTERM

closes earendil-works#5724

Author: mitsuhiko

packages/coding-agent/CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixed
 
+- Fixed SIGTERM/SIGHUP interactive shutdown to keep signal handlers installed until terminal cleanup completes, preventing `signal-exit` from re-sending the signal and leaving the terminal in raw/Kitty keyboard mode ([#5724](https://github.com/earendil-works/pi/issues/5724)).
 - Fixed extensions documentation to clarify that `pi.getActiveTools()` returns active tool names while `pi.getAllTools()` returns tool metadata ([#5729](https://github.com/earendil-works/pi/issues/5729)).
 - Fixed package commands such as `pi list`, `pi install`, and `pi update` to terminate after completing even if an extension leaves background handles open ([#5687](https://github.com/earendil-works/pi/issues/5687)).
 - Fixed `pi update` for pnpm global installs whose configured `global-bin-dir` no longer matches the active pnpm home ([#5689](https://github.com/earendil-works/pi/issues/5689)).

packages/coding-agent/src/modes/interactive/interactive-mode.ts

@@ -3362,7 +3362,9 @@ export class InteractiveMode {
 	private async shutdown(options?: { fromSignal?: boolean }): Promise<void> {
 		if (this.isShuttingDown) return;
 		this.isShuttingDown = true;
-		this.unregisterSignalHandlers();
+		// Keep signal handlers registered until terminal cleanup has completed.
+		// `signal-exit` checks the listener list during the same SIGTERM/SIGHUP
+		// dispatch and re-sends the signal if only its own listeners remain.
 
 		if (options?.fromSignal) {
 			// Signal-triggered shutdown (SIGTERM/SIGHUP). Emit extension cleanup
@@ -5742,7 +5744,6 @@ export class InteractiveMode {
 	}
 
 	stop(): void {
-		this.unregisterSignalHandlers();
 		if (this.settingsManager.getShowTerminalProgress()) {
 			this.ui.terminal.setProgress(false);
 		}
@@ -5760,5 +5761,6 @@ export class InteractiveMode {
 			this.ui.stop();
 			this.isInitialized = false;
 		}
+		this.unregisterSignalHandlers();
 	}
 }

packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts

@@ -116,7 +116,6 @@ describe("InteractiveMode.shutdown ordering (#5080)", () => {
 
 		expect(order).toEqual(["dispose", "drainInput", "stop"]);
 		expect(context.isShuttingDown).toBe(true);
-		expect(context.unregisterSignalHandlers).toHaveBeenCalledTimes(1);
 	});
 
 	test("interactive quit stops the TUI before emitting session_shutdown", async () => {

packages/coding-agent/test/suite/regressions/5724-sigterm-signal-exit.test.ts

@@ -0,0 +1,92 @@
+import { afterEach, describe, expect, test, vi } from "vitest";
+import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts";
+
+// Regression for https://github.com/earendil-works/pi/issues/5724
+//
+// `proper-lockfile` installs `signal-exit`, whose signal listener re-sends
+// SIGTERM/SIGHUP when it observes no other process listeners during the same
+// signal dispatch. InteractiveMode must therefore keep its signal handlers
+// registered until async terminal cleanup has completed.
+
+type ShutdownThis = {
+	isShuttingDown: boolean;
+	unregisterSignalHandlers: () => void;
+	runtimeHost: { dispose: () => Promise<void> };
+	ui: { terminal: { drainInput: (ms: number) => Promise<void> } };
+	stop: () => void;
+};
+
+type InteractiveModePrototypeWithShutdown = {
+	shutdown(this: ShutdownThis, options?: { fromSignal?: boolean }): Promise<void>;
+};
+
+const interactiveModePrototype = InteractiveMode.prototype as unknown;
+
+class ProcessExitError extends Error {}
+
+function deferred(): { promise: Promise<void>; resolve: () => void } {
+	let resolve: (() => void) | undefined;
+	const promise = new Promise<void>((res) => {
+		resolve = res;
+	});
+	return {
+		promise,
+		resolve: () => resolve?.(),
+	};
+}
+
+async function callShutdown(context: ShutdownThis, options?: { fromSignal?: boolean }): Promise<void> {
+	try {
+		await (interactiveModePrototype as InteractiveModePrototypeWithShutdown).shutdown.call(context, options);
+	} catch (error) {
+		if (!(error instanceof ProcessExitError)) throw error;
+	}
+}
+
+describe("InteractiveMode SIGTERM shutdown with signal-exit (#5724)", () => {
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
+	test("keeps signal handlers registered while signal-triggered cleanup is pending", async () => {
+		vi.spyOn(process, "exit").mockImplementation((() => {
+			throw new ProcessExitError();
+		}) as typeof process.exit);
+
+		const order: string[] = [];
+		const dispose = deferred();
+		const context: ShutdownThis = {
+			isShuttingDown: false,
+			unregisterSignalHandlers: vi.fn(() => {
+				order.push("unregister");
+			}),
+			runtimeHost: {
+				dispose: vi.fn(() => {
+					order.push("dispose");
+					return dispose.promise;
+				}),
+			},
+			ui: {
+				terminal: {
+					drainInput: vi.fn(async () => {
+						order.push("drainInput");
+					}),
+				},
+			},
+			stop: vi.fn(() => {
+				order.push("stop");
+			}),
+		};
+
+		const shutdownPromise = callShutdown(context, { fromSignal: true });
+		await Promise.resolve();
+
+		expect(order).toEqual(["dispose"]);
+		expect(context.unregisterSignalHandlers).not.toHaveBeenCalled();
+
+		dispose.resolve();
+		await shutdownPromise;
+
+		expect(order).toEqual(["dispose", "drainInput", "stop"]);
+	});
+});