diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 4d0680d0ac..106372ba51 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -459,7 +459,10 @@ name them. Listener errors reach the runtime's connection-loss handler only after a successful bind; a bind failure prints the listen-failure message alone. Request bodies are bounded by the SDK's 4 MiB limit, and the response budget keeps its stdio-derived floor. The listener ends on SIGINT or SIGTERM only, not on stdin EOF, and exits `130` or -`143` after cleanup settles. +`143` after cleanup settles. A signal that arrives while startup is still +running is recorded and acted on as soon as the listener and its providers are +owned, so startup never ends without cleanup; a second signal during startup +exits immediately. ## Progress and cancellation diff --git a/docs/specs/agent-environments/http-transport/spec.md b/docs/specs/agent-environments/http-transport/spec.md index 930b4380a4..59421e60d3 100644 --- a/docs/specs/agent-environments/http-transport/spec.md +++ b/docs/specs/agent-environments/http-transport/spec.md @@ -42,7 +42,9 @@ client registration keep stdio. checks replace an open session; refuse requests once the slot is closed, join an arming session during shutdown, and keep close stages retryable. - End the HTTP listener on SIGINT/SIGTERM only, never on stdin EOF, and exit with - the signal status (`130`/`143`) after cleanup settles. + the signal status (`130`/`143`) after cleanup settles; latch a signal that + arrives during startup so it is acted on as soon as the lifecycle owner exists, + and exit at once on a second one. - Export `serveStreamableHttp({ host, port, createMcpServer, routes, logger })` returning `{ url, close }` so an agent environment server can add plain routes beside `/mcp` without a second bridge. diff --git a/docs/specs/agent-environments/http-transport/verification.md b/docs/specs/agent-environments/http-transport/verification.md index 3292ee3e5b..0afdefde2c 100644 --- a/docs/specs/agent-environments/http-transport/verification.md +++ b/docs/specs/agent-environments/http-transport/verification.md @@ -234,3 +234,12 @@ request to drain (test: a 4.5 MB sessionless initialize is refused promptly while the connected client keeps echoing); and the node:http bridge passes repeated `Set-Cookie` values as an array instead of collapsing them (test: two cookies from a composed route both arrive). + +A sixth round (Codex, one P1) was addressed next: signal handlers were only +installed after the asynchronous startup returned, so a SIGINT or SIGTERM +during target opening, provider loading or the HTTP bind ended the process +without cleanup. The HTTP entry now latches signals before startup, the +lifecycle owner picks the recorded signal up as soon as it registers, and a +second signal during startup exits at once (test: the latch records `143`, +exits on the second signal, hands the status over on release and leaves no +listeners behind). diff --git a/src/main.ts b/src/main.ts index 0a7c42337a..ee767f0c90 100644 --- a/src/main.ts +++ b/src/main.ts @@ -22,8 +22,10 @@ import { import { createToolResultDelivery } from "./server/toolResult.js"; import { createShutdown } from "./main/shutdown.js"; import { + latchSignalShutdown, registerProcessShutdown, registerSignalShutdown, + type SignalLatch, } from "./main/processShutdown.js"; import { parseMcpHttpEndpoint, @@ -43,6 +45,7 @@ const runtimeDependencies = (): RuntimeDependencies => ({ /** Replace stdio with the streamable HTTP listener; signals alone end it, never stdin. */ const httpRuntimeDependencies = ( endpoint: McpHttpEndpoint, + latch: SignalLatch, ): RuntimeDependencies => { const base = runtimeDependencies(); return { @@ -59,11 +62,19 @@ const httpRuntimeDependencies = ( ...options, }); }, - registerShutdown: (request) => - registerSignalShutdown((exitStatus) => { + registerShutdown: (request) => { + const unregister = registerSignalShutdown((exitStatus) => { base.setExitCode(exitStatus); request(); - }), + }); + // A signal that arrived during startup is honoured now that the lifecycle owner exists. + const pending = latch.release(); + if (pending !== undefined) { + base.setExitCode(pending); + request(); + } + return unregister; + }, }; }; @@ -177,11 +188,17 @@ export const runMcpHttpEntrypoint = async ( io.setExitCode(1); return; } - await runEntrypoint( - () => run(httpRuntimeDependencies(endpoint.value)), - io.writeStderr, - io.setExitCode, - ); + // Latched before any asynchronous startup so a signal cannot end the process without cleanup. + const latch = latchSignalShutdown(); + try { + await runEntrypoint( + () => run(httpRuntimeDependencies(endpoint.value, latch)), + io.writeStderr, + io.setExitCode, + ); + } finally { + latch.release(); + } }; /** Return false when the runtime's argv path is synthetic or no longer exists. */ diff --git a/src/main/processShutdown.ts b/src/main/processShutdown.ts index 1b39f90da9..f60e87ea70 100644 --- a/src/main/processShutdown.ts +++ b/src/main/processShutdown.ts @@ -61,3 +61,32 @@ export const registerSignalShutdown = ( }, { signals }, ); + +/** A signal recorded while startup is still running, before the lifecycle owner can act on it. */ +export interface SignalLatch { + /** Stop recording; returns the exit status of the first signal seen, if any. */ + readonly release: () => number | undefined; +} + +/** Record the first SIGINT or SIGTERM during startup and exit at once on a second one, so startup never dies without its cleanup. */ +export const latchSignalShutdown = ( + signals: ShutdownSignalSource = process, + exit: (status: number) => void = (status) => process.exit(status), +): SignalLatch => { + let pending: number | undefined; + const record = (signal: ShutdownSignal) => (): void => { + if (pending === undefined) pending = SIGNAL_EXIT_STATUS[signal]; + else exit(pending); + }; + const onInterrupt = record("SIGINT"); + const onTerminate = record("SIGTERM"); + signals.on("SIGINT", onInterrupt); + signals.on("SIGTERM", onTerminate); + return { + release: () => { + signals.off("SIGINT", onInterrupt); + signals.off("SIGTERM", onTerminate); + return pending; + }, + }; +}; diff --git a/src/server/shutdownRegistration.test.ts b/src/server/shutdownRegistration.test.ts index 41eb94ecec..f3dda49b80 100644 --- a/src/server/shutdownRegistration.test.ts +++ b/src/server/shutdownRegistration.test.ts @@ -4,6 +4,7 @@ import pino from "pino"; import { createShutdown } from "../main/shutdown.js"; import { + latchSignalShutdown, registerProcessShutdown, registerSignalShutdown, } from "../main/processShutdown.js"; @@ -220,3 +221,19 @@ it("joins concurrent shutdown events and retries only failed owners", async () = expect(input.listenerCount("end")).toBe(0); expect(input.listenerCount("close")).toBe(0); }); + +it("latches the first startup signal, exits at once on a second, and hands the status over on release", () => { + const signals = new EventEmitter(); + const exits: number[] = []; + const latch = latchSignalShutdown(signals, (status) => exits.push(status)); + expect(signals.listenerCount("SIGINT")).toBe(1); + signals.emit("SIGTERM"); + expect(exits).toEqual([]); + signals.emit("SIGINT"); + expect(exits).toEqual([143]); + expect(latch.release()).toBe(143); + expect(signals.listenerCount("SIGINT")).toBe(0); + expect(signals.listenerCount("SIGTERM")).toBe(0); + const quiet = latchSignalShutdown(signals, (status) => exits.push(status)); + expect(quiet.release()).toBeUndefined(); +});