diff --git a/plugin/scripts/session-end.mjs b/plugin/scripts/session-end.mjs index 019149c30..1a8882482 100755 --- a/plugin/scripts/session-end.mjs +++ b/plugin/scripts/session-end.mjs @@ -51,7 +51,7 @@ async function main() { headers: authHeaders(), signal: AbortSignal.timeout(3e4) }).catch(() => {}); - setTimeout(() => process.exit(0), 1500).unref(); + setTimeout(() => process.exit(0), 500).unref(); } main(); diff --git a/src/hooks/session-end.ts b/src/hooks/session-end.ts index 6a9fc429c..39b9aef78 100644 --- a/src/hooks/session-end.ts +++ b/src/hooks/session-end.ts @@ -63,7 +63,7 @@ async function main() { }).catch(() => {}); } - setTimeout(() => process.exit(0), 1500).unref(); + setTimeout(() => process.exit(0), 500).unref(); } main(); diff --git a/test/session-end-exit-timing.test.ts b/test/session-end-exit-timing.test.ts new file mode 100644 index 000000000..85a03dcaa --- /dev/null +++ b/test/session-end-exit-timing.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { spawn } from "node:child_process"; +import { createServer, type Server } from "node:http"; +import { join } from "node:path"; +import { once } from "node:events"; + +// Runs the BUILT hook artifact (what ships), not the TypeScript source. CI +// builds before testing; locally run `npm run build` after editing +// src/hooks/session-end.ts or this exercises stale code. +const HOOK = join( + import.meta.dirname, + "..", + "plugin", + "scripts", + "session-end.mjs", +); + +// A server that accepts the connection and reads the request but never sends a +// response, so the hook's fire-and-forget `fetch` stays in flight. With the +// request hanging, the only thing that ends the hook process is its deferred +// `setTimeout(() => process.exit(0), N).unref()` — which lets us measure N. +let blackHole: Server; +let blackHoleUrl: string; + +beforeAll(async () => { + blackHole = createServer(() => { + // Intentionally never respond. + }); + blackHole.listen(0, "127.0.0.1"); + await once(blackHole, "listening"); + const address = blackHole.address(); + const port = typeof address === "object" && address ? address.port : 0; + blackHoleUrl = `http://127.0.0.1:${port}`; +}); + +afterAll(async () => { + blackHole.close(); + await once(blackHole, "close"); +}); + +function runHook( + stdin: string, + env: Record, +): Promise<{ exitCode: number | null; tookMs: number }> { + return new Promise((resolve, reject) => { + const start = Date.now(); + const child = spawn(process.execPath, [HOOK], { + env: { PATH: process.env["PATH"] ?? "", ...env }, + stdio: ["pipe", "ignore", "ignore"], + }); + child.on("error", reject); + child.on("close", (exitCode) => { + resolve({ exitCode, tookMs: Date.now() - start }); + }); + child.stdin.write(stdin); + child.stdin.end(); + }); +} + +describe("session-end hook exits within Claude Code's shutdown grace (#991)", () => { + it("exits well under the old 1500ms cap even when the memory server hangs", async () => { + const payload = JSON.stringify({ session_id: "ses_timing_test" }); + const { exitCode, tookMs } = await runHook(payload, { + AGENTMEMORY_URL: blackHoleUrl, + }); + + expect(exitCode).toBe(0); + // Positive control: the hung request must actually hold the process open to + // the deferred-exit timer, otherwise the timing assertion is meaningless. + expect(tookMs).toBeGreaterThan(250); + // The deferred-exit timer must fire comfortably inside Claude Code's + // SessionEnd shutdown grace. The bound sits between the fixed 500ms timer + // (plus startup) and the old 1500ms cap that overran the grace, so the + // harness killed the hook and reported "Hook cancelled" (#991). + expect(tookMs).toBeLessThan(1400); + }); +});