diff --git a/server/src/__tests__/fixtures/plugin-worker-persistent.cjs b/server/src/__tests__/fixtures/plugin-worker-persistent.cjs index 21535d908a..65eff19127 100644 --- a/server/src/__tests__/fixtures/plugin-worker-persistent.cjs +++ b/server/src/__tests__/fixtures/plugin-worker-persistent.cjs @@ -12,8 +12,16 @@ const readline = require("node:readline"); -/** How long the worker waits after acking `shutdown` before exiting. */ -const SHUTDOWN_EXIT_DELAY_MS = 300; +// How long the worker waits after acking `shutdown` before exiting. +// +// This has to sit inside the host's post-ack grace period: stopInternal() +// races the shutdown RPC (which resolves as soon as this ack lands) and then +// waits only 500ms more before escalating to SIGTERM. A delay close to that +// ceiling makes the negative-control test a timing race against a real process +// exit on a loaded CI runner, so keep the margin wide. The test does not +// depend on this window being long — it kills the pipe from a write hook the +// moment the shutdown is flushed, not after a poll. +const SHUTDOWN_EXIT_DELAY_MS = 100; /** Hard ceiling so a fixture never outlives the test run that spawned it. */ const MAX_LIFETIME_MS = 30_000; diff --git a/server/src/__tests__/plugin-worker-stdin-supervision.test.ts b/server/src/__tests__/plugin-worker-stdin-supervision.test.ts index 4249054316..9ae2fb50e5 100644 --- a/server/src/__tests__/plugin-worker-stdin-supervision.test.ts +++ b/server/src/__tests__/plugin-worker-stdin-supervision.test.ts @@ -86,6 +86,55 @@ function exitWithin(child: ChildProcess, timeoutMs: number): Promise { + const stdin = child.stdin; + if (!stdin) throw new Error("expected the forked child to have a stdin pipe"); + + return new Promise((resolve) => { + const originalWrite = stdin.write.bind(stdin) as typeof stdin.write; + let fired = false; + + stdin.write = ((chunk: unknown, ...rest: unknown[]) => { + const accepted = (originalWrite as (...a: unknown[]) => boolean)(chunk, ...rest); + + if (!fired && typeof chunk === "string" && chunk.includes('"shutdown"')) { + fired = true; + // Let the manager's own write callback run first, so the shutdown is + // fully handed off before the pipe dies. + setImmediate(() => { + const aliveAtDestroy = child.exitCode === null && child.signalCode === null; + stdin.destroy(epipe()); + resolve({ aliveAtDestroy }); + }); + } + + return accepted; + }) as typeof stdin.write; + }); +} + async function startPersistentWorker() { const before = forkedChildren.length; const handle = createPluginWorkerHandle("test.plugin", { @@ -182,20 +231,18 @@ describe("plugin worker stdin command-channel supervision", () => { handle.on("crash", (payload) => crashes.push(payload)); const exited = nextExit(child); - const stopping = handle.stop(); - - // stopInternal() sets intentionalStop before it writes the shutdown RPC. - await vi.waitFor(() => { - expect(handle.status).toBe("stopping"); - // ...and the shutdown must have drained out of the host before the pipe - // is killed, or this would be testing a dropped shutdown instead. - expect(child.stdin?.writableLength ?? 0).toBe(0); - }); // Kill the command channel mid-stop, while the fixture is still inside its // deferred-exit window. Without the intentionalStop guard this SIGKILLs a // worker that was already shutting down cleanly. - child.stdin?.destroy(epipe()); + const destroyed = destroyStdinOnShutdown(child); + const stopping = handle.stop(); + + const { aliveAtDestroy } = await destroyed; + expect( + aliveAtDestroy, + "fixture exited before the command channel was killed — the guard was never exercised", + ).toBe(true); await stopping; const { code, signal } = await exited;