mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
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>
118 lines
3.8 KiB
TypeScript
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,
|
|
);
|
|
});
|