fix(coding-agent): restore terminal on SIGTERM

closes #5724
This commit is contained in:
Armin Ronacher
2026-06-15 00:47:35 +02:00
parent b5e13bcd8a
commit ba0ec61563
4 changed files with 97 additions and 3 deletions

View File

@@ -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)).

View File

@@ -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();
}
}

View File

@@ -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 () => {

View File

@@ -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"]);
});
});