diff --git a/src/resources/extensions/gsd/auto-supervisor.ts b/src/resources/extensions/gsd/auto-supervisor.ts index 05e0713fb..4777f68e2 100644 --- a/src/resources/extensions/gsd/auto-supervisor.ts +++ b/src/resources/extensions/gsd/auto-supervisor.ts @@ -1,16 +1,24 @@ /** - * Auto-mode Supervisor — SIGTERM handling and working-tree activity detection. + * Auto-mode Supervisor — signal handling and working-tree activity detection. * * Pure functions — no module-level globals or AutoContext dependency. */ import { clearLock } from "./crash-recovery.js"; +import { releaseSessionLock } from "./session-lock.js"; import { nativeHasChanges } from "./native-git-bridge.js"; -// ─── SIGTERM Handling ───────────────────────────────────────────────────────── +// ─── Signal Handling ───────────────────────────────────────────────────────── + +/** Signals that should trigger lock cleanup on process termination. */ +const CLEANUP_SIGNALS: NodeJS.Signals[] = ["SIGTERM", "SIGHUP", "SIGINT"]; /** - * Register a SIGTERM handler that clears the lock file and exits cleanly. + * Register signal handlers that clear lock files and exit cleanly. + * Installs handlers on SIGTERM, SIGHUP, and SIGINT so that lock files + * are cleaned up regardless of how the process is terminated (normal kill, + * parent process death, or Ctrl+C). + * * Captures the active base path at registration time so the handler * always references the correct path even if the module variable changes. * Removes any previously registered handler before installing the new one. @@ -21,19 +29,22 @@ export function registerSigtermHandler( currentBasePath: string, previousHandler: (() => void) | null, ): () => void { - if (previousHandler) process.off("SIGTERM", previousHandler); + if (previousHandler) { + for (const sig of CLEANUP_SIGNALS) process.off(sig, previousHandler); + } const handler = () => { clearLock(currentBasePath); + releaseSessionLock(currentBasePath); process.exit(0); }; - process.on("SIGTERM", handler); + for (const sig of CLEANUP_SIGNALS) process.on(sig, handler); return handler; } -/** Deregister the SIGTERM handler (called on stop/pause). */ +/** Deregister signal handlers from all cleanup signals (called on stop/pause). */ export function deregisterSigtermHandler(handler: (() => void) | null): void { if (handler) { - process.off("SIGTERM", handler); + for (const sig of CLEANUP_SIGNALS) process.off(sig, handler); } } diff --git a/src/resources/extensions/gsd/tests/signal-handlers.test.ts b/src/resources/extensions/gsd/tests/signal-handlers.test.ts new file mode 100644 index 000000000..d6d409e53 --- /dev/null +++ b/src/resources/extensions/gsd/tests/signal-handlers.test.ts @@ -0,0 +1,103 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + registerSigtermHandler, + deregisterSigtermHandler, +} from "../auto-supervisor.ts"; + +/** + * Tests for signal handler registration (SIGTERM, SIGHUP, SIGINT). + * + * Validates that registerSigtermHandler installs handlers on all three + * signals and deregisterSigtermHandler removes them from all three. + * Fixes #1797 — stranded lock files on VSCode crash due to missing + * SIGHUP and SIGINT handlers. + */ + +test("registerSigtermHandler installs handlers on SIGTERM, SIGHUP, and SIGINT", () => { + const before = { + SIGTERM: process.listenerCount("SIGTERM"), + SIGHUP: process.listenerCount("SIGHUP"), + SIGINT: process.listenerCount("SIGINT"), + }; + + const handler = registerSigtermHandler("/tmp/test-signal-handlers", null); + + assert.equal( + process.listenerCount("SIGTERM"), + before.SIGTERM + 1, + "SIGTERM listener should be added", + ); + assert.equal( + process.listenerCount("SIGHUP"), + before.SIGHUP + 1, + "SIGHUP listener should be added", + ); + assert.equal( + process.listenerCount("SIGINT"), + before.SIGINT + 1, + "SIGINT listener should be added", + ); + + // Clean up + deregisterSigtermHandler(handler); +}); + +test("deregisterSigtermHandler removes handlers from all three signals", () => { + const handler = registerSigtermHandler("/tmp/test-signal-handlers", null); + + const during = { + SIGTERM: process.listenerCount("SIGTERM"), + SIGHUP: process.listenerCount("SIGHUP"), + SIGINT: process.listenerCount("SIGINT"), + }; + + deregisterSigtermHandler(handler); + + assert.equal( + process.listenerCount("SIGTERM"), + during.SIGTERM - 1, + "SIGTERM listener should be removed", + ); + assert.equal( + process.listenerCount("SIGHUP"), + during.SIGHUP - 1, + "SIGHUP listener should be removed", + ); + assert.equal( + process.listenerCount("SIGINT"), + during.SIGINT - 1, + "SIGINT listener should be removed", + ); +}); + +test("registerSigtermHandler deregisters previous handler from all signals", () => { + const before = { + SIGTERM: process.listenerCount("SIGTERM"), + SIGHUP: process.listenerCount("SIGHUP"), + SIGINT: process.listenerCount("SIGINT"), + }; + + const handler1 = registerSigtermHandler("/tmp/test-signal-handlers", null); + const handler2 = registerSigtermHandler("/tmp/test-signal-handlers-2", handler1); + + // Should still only have one extra listener per signal (old one removed, new one added) + assert.equal( + process.listenerCount("SIGTERM"), + before.SIGTERM + 1, + "SIGTERM should have exactly one handler after re-registration", + ); + assert.equal( + process.listenerCount("SIGHUP"), + before.SIGHUP + 1, + "SIGHUP should have exactly one handler after re-registration", + ); + assert.equal( + process.listenerCount("SIGINT"), + before.SIGINT + 1, + "SIGINT should have exactly one handler after re-registration", + ); + + // Clean up + deregisterSigtermHandler(handler2); +});