pi-mono/packages/mcp/test/stdio.test.ts
robopipi[bot] 42849bb840
fix(mcp): clean up server children on stdio close after the server exited (#11724)
On POSIX, stdio MCP servers run in their own process group so that
closing the transport can kill the whole tree. A server's main process
can exit while a child it started is still running and still holds the
inherited stdout/stderr. In that case `StdioTransport.close()` returned
early, because the direct child already had an exit code. It skipped the
process-group SIGTERM/SIGKILL cleanup, so the child kept running. The
transport's close event also never fired.

A second problem: the group was removed from the set the host exit hook
kills as soon as the server process exited. So the leftover child also
survived pi exiting.

This change:
- On POSIX, skips the early return when the server already exited, so
the existing stdin-close → SIGTERM → SIGKILL group cleanup runs and
`close()` resolves once the pipes close. Windows keeps the early return.
- Keeps the process group tracked until the child's `close` event (all
pipes closed) instead of `exit`.

Testing:
- Added a test in `packages/mcp/test/stdio.test.ts` with a new fixture,
`exiting-parent-server.mjs`. The fixture starts a helper that inherits
stdout/stderr, then exits. The test checks that after `close()` the
helper is gone and the close event fired exactly once. Without the fix
it fails: the helper is still alive.
- `packages/mcp` tests pass (47 tests). `npm run check` passes.
- Manual reproduction with a shell-script server that backgrounds a
long-running process and exits. Before: `close()` returned in 0 ms, the
process stayed alive, no close event. After: it is killed and the close
event fires.

Fixes #11721

Requested by @badlogic

Co-authored-by: triagent <triagent@localhost>
2026-11-03 09:31:10 +01:00

118 lines
3.8 KiB
TypeScript

import { fileURLToPath } from "node:url";
import { describe, expect, it } from "vitest";
import { McpClient, StdioTransport } from "../src/index.ts";
const fixture = fileURLToPath(new URL("./fixtures/stdio-server.mjs", import.meta.url));
const stubborn = fileURLToPath(new URL("./fixtures/stubborn-server.mjs", import.meta.url));
const exitingParent = fileURLToPath(new URL("./fixtures/exiting-parent-server.mjs", import.meta.url));
function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
async function waitFor(condition: () => boolean, timeoutMs = 2_000): Promise<boolean> {
const deadline = Date.now() + timeoutMs;
while (!condition()) {
if (Date.now() > deadline) return false;
await new Promise((resolve) => setTimeout(resolve, 10));
}
return true;
}
describe("StdioTransport", () => {
it("connects to a newline-delimited MCP server and captures stderr", async () => {
const stderr: string[] = [];
const transport = new StdioTransport({
command: process.execPath,
args: [fixture],
onStderr: (chunk) => stderr.push(chunk),
});
const client = new McpClient({ name: "stdio-test", version: "1.0.0" });
await client.connect(transport);
expect(await client.listTools()).toEqual([{ name: "echo", inputSchema: { type: "object" } }]);
expect(await client.callTool("echo", { text: "hello" })).toEqual({
content: [{ type: "text", text: "hello" }],
});
expect(transport.pid).toBeTypeOf("number");
await new Promise((resolve) => setTimeout(resolve, 10));
expect(stderr.join("")).toContain("stdio fixture ready");
expect(transport.stderr).toContain("stdio fixture ready");
await client.close();
expect(client.connectionState).toBe("closed");
});
it.skipIf(process.platform === "win32")(
"kills a server that ignores shutdown, including its children",
async () => {
const transport = new StdioTransport({
command: process.execPath,
args: [stubborn],
closeTimeoutMs: 100,
});
const client = new McpClient({ name: "stdio-test", version: "1.0.0" });
await client.connect(transport);
let grandchild: number | undefined;
for (let i = 0; i < 100 && grandchild === undefined; i++) {
const match = /grandchild (\d+)/.exec(transport.stderr);
if (match) grandchild = Number(match[1]);
else await new Promise((resolve) => setTimeout(resolve, 10));
}
expect(grandchild).toBeTypeOf("number");
const startedAt = Date.now();
await client.close();
expect(Date.now() - startedAt).toBeLessThan(5_000);
let alive = true;
for (let i = 0; i < 100 && alive; i++) {
try {
process.kill(grandchild as number, 0);
await new Promise((resolve) => setTimeout(resolve, 20));
} catch {
alive = false;
}
}
expect(alive).toBe(false);
},
10_000,
);
it.skipIf(process.platform === "win32")(
"kills children still holding the pipes after the server process exited",
async () => {
const transport = new StdioTransport({
command: process.execPath,
args: [exitingParent],
closeTimeoutMs: 100,
});
let closeEvents = 0;
transport.onClose(() => closeEvents++);
await transport.start();
const serverPid = transport.pid as number;
let helper: number | undefined;
await waitFor(() => {
const match = /helper (\d+)/.exec(transport.stderr);
if (match) helper = Number(match[1]);
return helper !== undefined;
});
expect(helper).toBeTypeOf("number");
expect(await waitFor(() => !isAlive(serverPid))).toBe(true);
try {
expect(isAlive(helper as number)).toBe(true);
await transport.close();
expect(await waitFor(() => !isAlive(helper as number))).toBe(true);
expect(closeEvents).toBe(1);
} finally {
try {
process.kill(helper as number, "SIGKILL");
} catch {}
}
},
10_000,
);
});