diff --git a/.changeset/stdio-kill-process-tree.md b/.changeset/stdio-kill-process-tree.md new file mode 100644 index 0000000000..6200318f1c --- /dev/null +++ b/.changeset/stdio-kill-process-tree.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': minor +--- + +Add an opt-in `killProcessTree` option to `StdioClientTransport`. When enabled, `close()` tears down the entire process tree — the child is spawned as its own process-group leader on POSIX (signalled via the process group) and torn down with `taskkill /T /F` on Windows — preventing orphaned server processes when the server is launched through a wrapper such as `npx`, `uvx`, or `python -m`. Defaults to `false`, preserving existing signal-propagation behaviour. Fixes #2023. diff --git a/packages/client/src/client/stdio.ts b/packages/client/src/client/stdio.ts index a4664e1c93..6229d2470f 100644 --- a/packages/client/src/client/stdio.ts +++ b/packages/client/src/client/stdio.ts @@ -46,6 +46,19 @@ export type StdioServerParameters = { * Defaults to 10 MB. */ maxBufferSize?: number; + + /** + * Kill the entire process tree when {@linkcode StdioClientTransport.close} is called. + * + * MCP servers are commonly launched through a wrapper (`npx`, `uvx`, `python -m`). + * `ChildProcess.kill()` signals only the direct child, so the wrapper's children survive + * as orphans. When this is `true`, the child is spawned as its own process-group leader + * (POSIX) and `close()` signals the whole group; on Windows the tree is torn down with + * `taskkill /T /F`. + * + * Defaults to `false`, preserving the current signal-propagation behaviour. + */ + killProcessTree?: boolean; }; /** @@ -135,6 +148,9 @@ export class StdioClientTransport implements Transport { }, stdio: ['pipe', 'pipe', this._serverParams.stderr ?? 'inherit'], shell: false, + // Own process group, so close() can signal the whole tree. Windows has no + // process groups in this sense; `taskkill /T` covers it there. + detached: this._serverParams.killProcessTree === true && process.platform !== 'win32', windowsHide: process.platform === 'win32', cwd: this._serverParams.cwd }); @@ -201,6 +217,38 @@ export class StdioClientTransport implements Transport { return this._process?.pid ?? null; } + /** + * Signal the child, or its whole tree when `killProcessTree` is set. + * Always falls back to the plain single-process kill. + */ + private _signalProcess(proc: ChildProcess, signal: 'SIGTERM' | 'SIGKILL'): void { + const pid = proc.pid; + + if (!this._serverParams.killProcessTree || pid === undefined) { + proc.kill(signal); + return; + } + + if (process.platform === 'win32') { + try { + spawn('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + return; + } catch { + // fall through to the direct kill below + } + } else { + try { + // Negative pid addresses the group created by `detached: true`. + process.kill(-pid, signal); + return; + } catch { + // Group already gone, or we never became the leader. + } + } + + proc.kill(signal); + } + private processReadBuffer() { while (true) { try { @@ -292,7 +340,7 @@ export class StdioClientTransport implements Transport { if (processToClose.exitCode === null) { try { - processToClose.kill('SIGTERM'); + this._signalProcess(processToClose, 'SIGTERM'); } catch { // ignore } @@ -302,7 +350,7 @@ export class StdioClientTransport implements Transport { if (processToClose.exitCode === null) { try { - processToClose.kill('SIGKILL'); + this._signalProcess(processToClose, 'SIGKILL'); } catch { // ignore } diff --git a/packages/client/test/client/stdioKillProcessTree.test.ts b/packages/client/test/client/stdioKillProcessTree.test.ts new file mode 100644 index 0000000000..f9e3895ca7 --- /dev/null +++ b/packages/client/test/client/stdioKillProcessTree.test.ts @@ -0,0 +1,42 @@ +import { existsSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; + +import { StdioClientTransport } from '../../src/client/stdio'; + +test('killProcessTree terminates grandchildren spawned by a wrapper', async () => { + // The npx/uvx anatomy: the direct child is a wrapper that spawns the real server. + // Without process-group teardown the grandchild outlives close() as an orphan. + if (process.platform === 'win32') return; // taskkill path is covered manually + + const pidFile = `${tmpdir()}/mcp-tree-${process.pid}-${Date.now()}`; + const WRAPPER_SCRIPT = String.raw` + const { spawn } = require('child_process'); + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + require('fs').writeFileSync(${JSON.stringify(pidFile)}, String(child.pid)); + setInterval(() => {}, 1000); + `; + + const transport = new StdioClientTransport({ + command: process.execPath, + args: ['-e', WRAPPER_SCRIPT], + killProcessTree: true + }); + await transport.start(); + + while (!existsSync(pidFile)) await new Promise(resolve => setTimeout(resolve, 25)); + const grandchildPid = Number(readFileSync(pidFile, 'utf8')); + expect(() => process.kill(grandchildPid, 0)).not.toThrow(); + + await transport.close(); + + // The group signal is delivered asynchronously; give it a moment to land. + for (let i = 0; i < 40; i++) { + try { + process.kill(grandchildPid, 0); + } catch { + return; // gone — the tree was reaped + } + await new Promise(resolve => setTimeout(resolve, 25)); + } + throw new Error(`grandchild ${grandchildPid} survived close()`); +}, 15_000);