mirror of
https://github.com/morluto/rea.git
synced 2026-11-02 21:06:04 +00:00
fix(mcp): keep a startup signal's exit status through the entry's normal return
Addresses the follow-on finding on the signal latch: the HTTP entry now owns its exit status, so a 0 returned by startup after a recorded signal keeps the signal's 130 or 143 while a real failure still wins; a process test sends the signal as the listening line appears. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
354b4d8c08
commit
136d073fa0
5 files changed
+72
-5
No files matched your search
@@ -243,3 +243,9 @@ 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).
|
||||
|
||||
Both reviewers then noted that the latched signal's status was overwritten by
|
||||
the entry's normal `0` return: the HTTP entry now owns its exit status through
|
||||
`createExitStatus`, where a `0` after a recorded signal keeps the signal's
|
||||
status and a real failure still wins (unit test), and a process test sends the
|
||||
signal the moment the listening line appears and asserts the signal status.
|
||||
+11
-5
@@ -22,9 +22,11 @@ import {
|
||||
import { createToolResultDelivery } from "./server/toolResult.js";
|
||||
import { createShutdown } from "./main/shutdown.js";
|
||||
import {
|
||||
createExitStatus,
|
||||
latchSignalShutdown,
|
||||
registerProcessShutdown,
|
||||
registerSignalShutdown,
|
||||
type ExitStatus,
|
||||
type SignalLatch,
|
||||
} from "./main/processShutdown.js";
|
||||
import {
|
||||
@@ -46,10 +48,12 @@ const runtimeDependencies = (): RuntimeDependencies => ({
|
||||
const httpRuntimeDependencies = (
|
||||
endpoint: McpHttpEndpoint,
|
||||
latch: SignalLatch,
|
||||
exit: ExitStatus,
|
||||
): RuntimeDependencies => {
|
||||
const base = runtimeDependencies();
|
||||
return {
|
||||
...base,
|
||||
setExitCode: exit.set,
|
||||
serve: async (factory, options) => {
|
||||
// Loaded here so the default stdio entry never loads the HTTP listener.
|
||||
const { serveStreamableHttp } =
|
||||
@@ -64,13 +68,14 @@ const httpRuntimeDependencies = (
|
||||
},
|
||||
registerShutdown: (request) => {
|
||||
const unregister = registerSignalShutdown((exitStatus) => {
|
||||
base.setExitCode(exitStatus);
|
||||
exit.signal(exitStatus);
|
||||
request();
|
||||
});
|
||||
// A signal that arrived during startup is honoured now that the lifecycle owner exists.
|
||||
// A signal that arrived during startup is honoured now that the lifecycle owner exists;
|
||||
// its status outlives the entry's own `0`, which startup returns right after this.
|
||||
const pending = latch.release();
|
||||
if (pending !== undefined) {
|
||||
base.setExitCode(pending);
|
||||
exit.signal(pending);
|
||||
request();
|
||||
}
|
||||
return unregister;
|
||||
@@ -190,11 +195,12 @@ export const runMcpHttpEntrypoint = async (
|
||||
}
|
||||
// Latched before any asynchronous startup so a signal cannot end the process without cleanup.
|
||||
const latch = latchSignalShutdown();
|
||||
const exit = createExitStatus(io.setExitCode);
|
||||
try {
|
||||
await runEntrypoint(
|
||||
() => run(httpRuntimeDependencies(endpoint.value, latch)),
|
||||
() => run(httpRuntimeDependencies(endpoint.value, latch, exit)),
|
||||
io.writeStderr,
|
||||
io.setExitCode,
|
||||
exit.set,
|
||||
);
|
||||
} finally {
|
||||
latch.release();
|
||||
|
||||
@@ -90,3 +90,28 @@ export const latchSignalShutdown = (
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
/** The exit status an entry reports: a signal's status survives the entry's normal `0` return. */
|
||||
export interface ExitStatus {
|
||||
/** The entry's own result; a `0` after a recorded signal keeps the signal's status. */
|
||||
readonly set: (code: number) => void;
|
||||
/** Record the exit status of a signal-requested shutdown. */
|
||||
readonly signal: (status: number) => void;
|
||||
}
|
||||
|
||||
export const createExitStatus = (
|
||||
setExitCode: (code: number) => void,
|
||||
): ExitStatus => {
|
||||
let signalStatus: number | undefined;
|
||||
return {
|
||||
set: (code) => {
|
||||
setExitCode(
|
||||
code === 0 && signalStatus !== undefined ? signalStatus : code,
|
||||
);
|
||||
},
|
||||
signal: (status) => {
|
||||
signalStatus = status;
|
||||
setExitCode(status);
|
||||
},
|
||||
};
|
||||
};
|
||||
@@ -4,6 +4,7 @@ import pino from "pino";
|
||||
|
||||
import { createShutdown } from "../main/shutdown.js";
|
||||
import {
|
||||
createExitStatus,
|
||||
latchSignalShutdown,
|
||||
registerProcessShutdown,
|
||||
registerSignalShutdown,
|
||||
@@ -237,3 +238,17 @@ it("latches the first startup signal, exits at once on a second, and hands the s
|
||||
const quiet = latchSignalShutdown(signals, (status) => exits.push(status));
|
||||
expect(quiet.release()).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps a signal's exit status through the entry's normal return and yields to a real failure", () => {
|
||||
const codes: number[] = [];
|
||||
const exit = createExitStatus((code) => codes.push(code));
|
||||
exit.set(0);
|
||||
exit.signal(143);
|
||||
exit.set(0);
|
||||
expect(codes).toEqual([0, 143, 143]);
|
||||
exit.set(1);
|
||||
expect(codes.at(-1)).toBe(1);
|
||||
const plain = createExitStatus((code) => codes.push(code));
|
||||
plain.set(0);
|
||||
expect(codes.at(-1)).toBe(0);
|
||||
});
|
||||
@@ -36,4 +36,19 @@ describe("rea mcp --http shutdown", () => {
|
||||
},
|
||||
SHUTDOWN_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
processTest(
|
||||
"exits with the signal status when the signal lands as startup completes",
|
||||
async ({ processes }) => {
|
||||
const started = await startMcpHttpProcess(processes);
|
||||
// No request first: the signal arrives at the edge between startup and the lifecycle owner.
|
||||
const [signal, status] = shutdownSignals[0]!;
|
||||
started.child.kill(signal);
|
||||
const [code, signalCode] = await once(started.child, "close");
|
||||
expect(code).toBe(status);
|
||||
expect(signalCode).toBeNull();
|
||||
expect(started.stderr()).not.toContain(MCP_SHUTDOWN_FAILED);
|
||||
},
|
||||
SHUTDOWN_TIMEOUT_MS,
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user