Skip to content

Commit d4b6872

Browse files
charliecreates[bot]CharlieHelpsmergify[bot]
authored
feat: add graceful stdio shutdown (#570)
* feat: add graceful stdio shutdown * fix: harden graceful stdio shutdown --------- Co-authored-by: CharlieHelps <charlie@charlielabs.ai> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 23cb9af commit d4b6872

7 files changed

Lines changed: 629 additions & 0 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"hevy-mcp": patch
3+
---
4+
5+
Gracefully close and flush the stdio transport on SIGINT or SIGTERM, with a
6+
bounded forced-exit fallback when shutdown stalls or other handles remain open.

src/index.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ const testDoubles = vi.hoisted(() => ({
2929
tool: vi.fn(),
3030
registerTool: vi.fn(),
3131
directRegisterToolCalls: 0,
32+
close: vi.fn().mockResolvedValue(undefined),
33+
installGracefulShutdown: vi.fn(() => ({
34+
cleanup: vi.fn(),
35+
getShutdownPromise: vi.fn(),
36+
})),
3237
sentry: {
3338
init: vi.fn(() => ({})),
3439
setUser: vi.fn(),
@@ -50,6 +55,10 @@ vi.mock("./utils/hevyClient.js", () => ({
5055
createClient: vi.fn().mockReturnValue({ mockedClient: true }),
5156
}));
5257

58+
vi.mock("./utils/graceful-shutdown.js", () => ({
59+
installGracefulShutdown: testDoubles.installGracefulShutdown,
60+
}));
61+
5362
vi.mock("./utils/telemetry.js", () => ({
5463
Sentry: testDoubles.sentry,
5564
tracer: {
@@ -100,6 +109,7 @@ vi.mock("@modelcontextprotocol/sdk/server/mcp.js", () => {
100109
}
101110

102111
connect = testDoubles.connect;
112+
close = testDoubles.close;
103113
isConnected = vi.fn(() => true);
104114
sendLoggingMessage = testDoubles.sendLoggingMessage;
105115
registerPrompt = testDoubles.registerPrompt;
@@ -132,6 +142,8 @@ describe("Server entry", () => {
132142
process.env = { ...originalEnv };
133143
process.argv = [...originalArgv];
134144
vi.clearAllMocks();
145+
testDoubles.connect.mockResolvedValue(undefined);
146+
testDoubles.close.mockResolvedValue(undefined);
135147
testDoubles.directRegisterToolCalls = 0;
136148
testDoubles.tool.mockImplementation(
137149
function (this: { registerTool: () => void }) {
@@ -290,6 +302,7 @@ describe("Server entry", () => {
290302
expect(exitSpy).not.toHaveBeenCalled();
291303
expect(createClient).not.toHaveBeenCalled();
292304
expect(testDoubles.startActiveSpan).not.toHaveBeenCalled();
305+
expect(testDoubles.installGracefulShutdown).not.toHaveBeenCalled();
293306

294307
const anyStdioModule = stdioModule as { __transports?: unknown[] };
295308
expect(anyStdioModule.__transports).toHaveLength(0);
@@ -323,6 +336,7 @@ describe("Server entry", () => {
323336
expect(helpText).toContain("Examples:");
324337
expect(createClient).not.toHaveBeenCalled();
325338
expect(testDoubles.startActiveSpan).not.toHaveBeenCalled();
339+
expect(testDoubles.installGracefulShutdown).not.toHaveBeenCalled();
326340

327341
const anyStdioModule = stdioModule as { __transports?: unknown[] };
328342
expect(anyStdioModule.__transports).toHaveLength(0);
@@ -357,6 +371,12 @@ describe("Server entry", () => {
357371
);
358372
expect(spanNames).toContain("mcp.server.run");
359373
expect(spanNames).toContain("mcp.server.connect");
374+
expect(testDoubles.installGracefulShutdown).toHaveBeenCalledWith({
375+
target: expect.objectContaining({ close: testDoubles.close }),
376+
});
377+
expect(testDoubles.connect.mock.invocationCallOrder[0]).toBeLessThan(
378+
testDoubles.installGracefulShutdown.mock.invocationCallOrder[0] ?? 0,
379+
);
360380
});
361381

362382
it("prefers CLI --hevy-api-key argument over environment variable", async () => {
@@ -393,6 +413,7 @@ describe("Server entry", () => {
393413

394414
await expect(runServer()).rejects.toThrow("connect failed");
395415
expect(testDoubles.connect).toHaveBeenCalled();
416+
expect(testDoubles.installGracefulShutdown).not.toHaveBeenCalled();
396417
expect(testDoubles.span.setStatus).toHaveBeenCalledWith({ code: 2 });
397418
});
398419

src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { registerTemplateTools } from "./tools/templates.js";
2323
import { registerUserTools } from "./tools/user.js";
2424
import { registerWorkoutTools } from "./tools/workouts.js";
2525
import { assertApiKey, parseConfig } from "./utils/config.js";
26+
import { installGracefulShutdown } from "./utils/graceful-shutdown.js";
2627
import { createClient } from "./utils/hevyClient.js";
2728
import { createMcpClientLogger } from "./utils/mcp-client-logger.js";
2829
import { createInstrumentedStdioTransport } from "./utils/stdio-observability.js";
@@ -307,6 +308,7 @@ export async function runServer() {
307308
}
308309
},
309310
);
311+
installGracefulShutdown({ target: server });
310312

311313
span.setStatus({ code: SpanStatusCode.OK });
312314
} catch (e) {
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { spawn } from "node:child_process";
2+
import path from "node:path";
3+
import { fileURLToPath } from "node:url";
4+
import { describe, expect, it } from "vitest";
5+
import type { ShutdownSignal } from "./graceful-shutdown.js";
6+
7+
const repositoryRoot = path.resolve(
8+
path.dirname(fileURLToPath(import.meta.url)),
9+
"../..",
10+
);
11+
const tsxCli = path.join(repositoryRoot, "node_modules/tsx/dist/cli.mjs");
12+
const childFixture = path.join(
13+
repositoryRoot,
14+
"tests/fixtures/graceful-shutdown-child.ts",
15+
);
16+
17+
describe("graceful stdio shutdown regression", () => {
18+
it.each(["SIGTERM", "SIGINT"] satisfies ShutdownSignal[])(
19+
"flushes backpressured JSON-RPC frames before exiting on %s",
20+
async (signal) => {
21+
const child = spawn(process.execPath, [tsxCli, childFixture], {
22+
cwd: repositoryRoot,
23+
stdio: ["pipe", "pipe", "pipe"],
24+
});
25+
const stdoutChunks: Buffer[] = [];
26+
const stderrChunks: Buffer[] = [];
27+
let exited = false;
28+
const exitResult = new Promise<{
29+
code: number | null;
30+
signal: NodeJS.Signals | null;
31+
}>((resolve) => {
32+
child.once("exit", (code, exitSignal) => {
33+
exited = true;
34+
resolve({ code, signal: exitSignal });
35+
});
36+
});
37+
38+
try {
39+
let expectedFrameCount = 0;
40+
await new Promise<void>((resolve, reject) => {
41+
const timeout = setTimeout(() => {
42+
reject(new Error("Timed out waiting for backpressure marker"));
43+
}, 5_000);
44+
45+
child.stderr.on("data", (chunk: Buffer) => {
46+
stderrChunks.push(chunk);
47+
const stderr = Buffer.concat(stderrChunks).toString("utf8");
48+
const marker = stderr.match(/BACKPRESSURED:(\d+)/);
49+
if (marker) {
50+
expectedFrameCount = Number(marker[1]);
51+
clearTimeout(timeout);
52+
resolve();
53+
}
54+
});
55+
child.once("error", reject);
56+
child.once("exit", (code, exitSignal) => {
57+
clearTimeout(timeout);
58+
reject(
59+
new Error(
60+
`Child exited before marker: code=${code}, signal=${exitSignal}`,
61+
),
62+
);
63+
});
64+
});
65+
66+
child.kill(signal);
67+
await new Promise((resolve) => setTimeout(resolve, 50));
68+
expect(exited).toBe(false);
69+
70+
child.stdout.on("data", (chunk: Buffer) => stdoutChunks.push(chunk));
71+
const timeout = setTimeout(() => child.kill("SIGKILL"), 10_000);
72+
const result = await exitResult;
73+
clearTimeout(timeout);
74+
75+
expect(result).toEqual({ code: 0, signal: null });
76+
const stdout = Buffer.concat(stdoutChunks).toString("utf8");
77+
const stderr = Buffer.concat(stderrChunks).toString("utf8");
78+
const frames = stdout.trimEnd().split("\n");
79+
80+
expect(expectedFrameCount).toBeGreaterThan(0);
81+
expect(frames).toHaveLength(expectedFrameCount);
82+
for (const [index, frame] of frames.entries()) {
83+
expect(JSON.parse(frame)).toEqual({
84+
jsonrpc: "2.0",
85+
id: index + 1,
86+
result: { payload: "x".repeat(128 * 1024) },
87+
});
88+
}
89+
expect(stdout).not.toContain("Shutting down gracefully");
90+
expect(stderr).toContain(`Shutting down gracefully after ${signal}`);
91+
} finally {
92+
if (!exited) {
93+
child.kill("SIGKILL");
94+
await exitResult;
95+
}
96+
}
97+
},
98+
10_000,
99+
);
100+
});

0 commit comments

Comments
 (0)