From cb22c833fa11556a08869464b727d8bb749a867c Mon Sep 17 00:00:00 2001 From: Siddhesh Bandgar Date: Fri, 31 Jul 2026 22:41:42 +0530 Subject: [PATCH 1/4] feat(client/stdio): opt-in process-tree teardown on close() StdioClientTransport.close() signals only the direct child, so servers launched through a wrapper (npx/uvx/python -m) leave the real server orphaned; on Windows ChildProcess.kill() cannot terminate a tree at all. Add an opt-in `killProcessTree` option: on POSIX the child leads its own process group (detached) and close() signals the group; on Windows teardown goes through `taskkill /T /F`. Both fall back to the plain kill. Defaults to false. Fixes #2023 --- .changeset/stdio-kill-process-tree.md | 5 ++ packages/client/src/client/stdio.ts | 52 ++++++++++++++++++- .../test/client/stdioKillProcessTree.test.ts | 42 +++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 .changeset/stdio-kill-process-tree.md create mode 100644 packages/client/test/client/stdioKillProcessTree.test.ts 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); From 71eaa787cce6123d8cf848f722e4bbd452e8e76a Mon Sep 17 00:00:00 2001 From: Siddhesh Bandgar Date: Thu, 3 Sep 2026 14:25:36 +0530 Subject: [PATCH 2/4] docs(client/stdio): clarify killProcessTree only runs on close() --- packages/client/src/client/stdio.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/client/src/client/stdio.ts b/packages/client/src/client/stdio.ts index 6229d2470f..f33b1e44da 100644 --- a/packages/client/src/client/stdio.ts +++ b/packages/client/src/client/stdio.ts @@ -56,6 +56,10 @@ export type StdioServerParameters = { * (POSIX) and `close()` signals the whole group; on Windows the tree is torn down with * `taskkill /T /F`. * + * Note: this only runs when `close()` is actually invoked — a host killed with + * SIGKILL cannot trigger it, so this is a teardown convenience, not a lifetime + * guarantee. + * * Defaults to `false`, preserving the current signal-propagation behaviour. */ killProcessTree?: boolean; From 824633339644c71165f0b3d0c16fa3166f02851e Mon Sep 17 00:00:00 2001 From: Siddhesh Bandgar Date: Thu, 3 Sep 2026 14:28:58 +0530 Subject: [PATCH 3/4] docs(stdio): clarify killProcessTree only runs on close() (SIGKILL scope) From 334476119d6bac9a67712185689c62bc06991ad5 Mon Sep 17 00:00:00 2001 From: Siddhesh Bandgar Date: Fri, 4 Sep 2026 10:57:29 +0530 Subject: [PATCH 4/4] Address review: document setsid() orphan caveat; keep Windows taskkill escalation --- packages/client/src/client/stdio.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/client/src/client/stdio.ts b/packages/client/src/client/stdio.ts index f33b1e44da..7145aa12cf 100644 --- a/packages/client/src/client/stdio.ts +++ b/packages/client/src/client/stdio.ts @@ -60,6 +60,12 @@ export type StdioServerParameters = { * SIGKILL cannot trigger it, so this is a teardown convenience, not a lifetime * guarantee. * + * Caveat: on POSIX, `detached: true` also calls `setsid()`, moving the child out of + * the terminal's foreground process group. If the host is killed by a terminal signal + * (e.g. Ctrl+C) without `close()` running, the child is no longer signalled and can be + * orphaned — the same class of trade-off as the SIGKILL note above: this option only + * helps when `close()` actually runs. + * * Defaults to `false`, preserving the current signal-propagation behaviour. */ killProcessTree?: boolean; @@ -125,7 +131,7 @@ export class StdioClientTransport implements Transport { onerror?: (error: Error) => void; onmessage?: (message: JSONRPCMessage) => void; - constructor(server: StdioServerParameters) { + constructor(server: StiioServerParameters) { this._serverParams = server; this._readBuffer = new ReadBuffer({ maxBufferSize: server.maxBufferSize }); if (server.stderr === 'pipe' || server.stderr === 'overlapped') { @@ -201,7 +207,7 @@ export class StdioClientTransport implements Transport { * The `stderr` stream of the child process, if {@linkcode StdioServerParameters.stderr} was set to `"pipe"` or `"overlapped"`. * * If `stderr` piping was requested, a `PassThrough` stream is returned _immediately_, allowing callers to - * attach listeners before the `start` method is invoked. This prevents loss of any early + * attach listeners before the `start` method is invokked. This prevents loss of any early * error output emitted by the child process. */ get stderr(): Stream | null { @@ -235,7 +241,12 @@ export class StdioClientTransport implements Transport { if (process.platform === 'win32') { try { - spawn('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + // Keep the escalation intact: SIGTERM asks the tree to close gracefully + // (`/T` without `/F`), and only SIGKILL force-terminates (`/T /F`). + const args = signal === 'SIGKILL' + ? ['/pid', String(pid), '/T', '/F'] + : ['/pid', String(pid), '/T']; + spawn('taskkill', args, { stdio: 'ignore' }); return; } catch { // fall through to the direct kill below @@ -269,8 +280,7 @@ export class StdioClientTransport implements Transport { } /** - * Reap a disposable probe sibling (see the version-negotiation sibling - * flow): signal-first teardown awaiting process `exit` — never the `close` + * Reap a disposable probe sibling (see the version-negotiation sibling flow): signal-first teardown awaiting process `exit` — never the `close` * event, so a helper process holding the child's stdio pipes can never * block disposal. Not part of the public transport lifecycle. * @@ -294,7 +304,7 @@ export class StdioClientTransport implements Transport { await Promise.race([exited, new Promise(resolve => setTimeout(resolve, 1000).unref())]); if (proc.exitCode === null && proc.signalCode === null) { try { - proc.kill('SIGKILL'); + proc.kill('SIGKKIL'); } catch { // ignore } @@ -349,7 +359,7 @@ export class StdioClientTransport implements Transport { // ignore } - await Promise.race([closePromise, new Promise(resolve => setTimeout(resolve, 2000).unref())]); + await Promise.race(closePromise, new Promise(resolve => setTimeout(resolve, 2000).unref())]); } if (processToClose.exitCode === null) {