fix(mcp): terminate descendants after discovery timeout

[morluto]

What this PR does

When non-pooled MCP discovery times out for a stdio server, terminate processes below the transport wrapper before disconnecting the wrapper. The sweep is best-effort and reuses the existing bounded, cross-platform descendant-PID utilities. Remote transports continue directly to disconnect because they expose no process ID.

Why it's needed

Stdio launchers such as npx and uvx can run the MCP server as a grandchild. The SDK closes its direct child on timeout, but that does not terminate the grandchild, so a timed-out discovery can leave the actual server running.

Reviewer Test Plan

How to verify

• Trigger a discovery timeout with a stdio client that exposes a wrapper PID and two descendants. Both descendants should receive SIGTERM before the wrapper disconnect begins.
• Trigger the same timeout with a remote client that exposes no PID. PID enumeration and signaling should be skipped, while disconnect still runs.
• Make descendant enumeration fail. Disconnect should still run because process-tree cleanup is best-effort.

Regression proof on the base branch: the two stdio cases failed because descendant enumeration was never called, while the remote/no-PID control passed. With this change, all three cases pass. An independent SDK fallback also confirmed that direct-child close is bounded: it returned in 2017 ms, stopped the wrapper, and left the wrapper's grandchild alive.

Evidence (Before & After)

N/A — internal process-lifecycle change with no UI.

Tested on

OS	Status
🍏 macOS	⚠️ not tested
🪟 Windows	⚠️ not tested
🐧 Linux	✅ tested

Environment (optional)

Linux, Node.js 24.18.0. Focused timeout tests: 4/4 passed. Full MCP client manager tests: 116/116 passed. Core build, typecheck, formatting, and ESLint passed.

Risk & Scope

• Main risk or tradeoff: discovery timeout cleanup now sends SIGTERM to descendants of the stdio wrapper before closing the wrapper; PID enumeration and signaling remain bounded and best-effort.
• Not validated / out of scope: no changes to pooled lifecycle cleanup or the existing descendant-PID implementation; macOS and Windows were not run locally.
• Breaking changes / migration notes: none.

Linked Issues

Fixes #6918

中文说明

此 PR 的作用

当非池化 MCP 的 stdio 服务器发现过程超时时，在断开包装进程之前终止其下层进程。该清理为尽力而为，并复用现有的、有界且跨平台的后代 PID 工具。远程传输不提供进程 ID，因此仍直接执行断开连接。

为什么需要此更改

npx 和 uvx 等 stdio 启动器可能将 MCP 服务器作为孙进程运行。SDK 会在超时时关闭直接子进程，但不会终止孙进程，因此发现超时后实际服务器可能仍在运行。

审阅者测试计划

验证方法

• 使用一个提供包装进程 PID 和两个后代进程的 stdio 客户端触发发现超时。两个后代进程应在包装进程开始断开连接之前收到 SIGTERM。
• 使用不提供 PID 的远程客户端触发相同超时。应跳过 PID 枚举和信号发送，同时仍执行断开连接。
• 让后代进程枚举失败。由于进程树清理是尽力而为，仍应执行断开连接。

基线分支上的回归证明：两个 stdio 用例因从未调用后代进程枚举而失败，远程无 PID 对照用例通过。应用此更改后，三个用例全部通过。独立 SDK 回退验证还确认直接子进程关闭是有界的：关闭在 2017 毫秒内返回，包装进程退出，但其孙进程仍然存活。

证据（更改前与更改后）

不适用——内部进程生命周期更改，无 UI。

测试平台

操作系统	状态
🍏 macOS	⚠️ 未测试
🪟 Windows	⚠️ 未测试
🐧 Linux	✅ 已测试

环境（可选）

Linux，Node.js 24.18.0。聚焦超时测试 4/4 通过；完整 MCP 客户端管理器测试 116/116 通过；核心包构建、类型检查、格式检查和 ESLint 均通过。

风险与范围

• 主要风险或权衡：发现超时清理现在会在关闭 stdio 包装进程之前向其后代发送 SIGTERM；PID 枚举和信号发送仍然有界且为尽力而为。
• 未验证或范围外：未更改池化生命周期清理和现有后代 PID 实现；未在本地运行 macOS 和 Windows 测试。
• 破坏性更改或迁移说明：无。

关联问题

Fixes #6918

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: Real, observed gap — the non-pooled McpClientManager discovery-timeout path doesn't clean up descendant processes (e.g. grandchildren spawned by npx/uvx wrappers), while the pooled path (PoolEntry.sweepAndDisconnect) already does. Issue #6918 documents this with references to the specific source commit and prior PRs (#4336, #4411) that introduced the descendant-PID sweep only for the pooled path. Maintainer wenshao independently reproduced the bug on main with a real stdio wrapper leaking a real grandchild, and confirmed the fix closes it deterministically (3/3 trials). Not theoretical.

Direction: Aligned. This is a parity fix bringing the non-pooled timeout cleanup in line with the pooled lifecycle. Reuses existing cross-platform utilities (listDescendantPids, sigtermPids from pid-descendants.ts) rather than introducing a parallel implementation. Issue #6918 is labeled type/bug, scope/mcp, priority/P2.

Size: 29 production lines (in mcp-client-manager.ts) + 116 test lines (in mcp-client-manager.test.ts). Small, focused change — not applicable for maintainer awareness thresholds.

Approach: Scope feels right. The diff does exactly one thing: adds a best-effort descendant PID sweep before client.disconnect() in the timeout handler, matching the pattern in sweepAndDisconnect. Remote transports (no PID) are correctly skipped. Error handling is best-effort with appropriate debug/warn logging. The new partial-signal warning (when sigtermPids reports fewer successes than descendants found) is a useful addition not present in the pooled path. Four focused tests cover: stdio descendants signaled before disconnect, remote transports skip PID enumeration, no-descendants case skips signaling, and disconnect still runs when enumeration fails. No scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题：已确认的差距——非池化 McpClientManager 的发现超时路径没有清理后代进程（如 npx/uvx 包装器产生的孙进程），而池化路径（PoolEntry.sweepAndDisconnect）已经做到了。Issue #6918 记录了这一点，引用了具体的源码提交和先前的 PR（#4336、#4411）。维护者 wenshao 独立在 main 上用真实 stdio 包装器复现了此 bug（泄漏真实孙进程），并确认修复确定性地解决了问题（3/3 次试验）。不是理论性问题。

方向：对齐。这是一个对等修复——将非池化超时清理与池化生命周期保持一致。复用现有的跨平台工具（pid-descendants.ts 中的 listDescendantPids、sigtermPids），而非引入并行实现。Issue #6918 标记为 type/bug、scope/mcp、priority/P2。

规模：29 行生产代码（mcp-client-manager.ts）+ 116 行测试代码（mcp-client-manager.test.ts）。小而集中的更改——无需维护者关注。

方案：范围合理。diff 只做一件事：在超时处理程序的 client.disconnect() 之前添加尽力而为的后代 PID 清理，与 sweepAndDisconnect 中的模式匹配。远程传输（无 PID）被正确跳过。错误处理是尽力而为的，附带适当的 debug/warn 日志。新增的部分信号警告（当 sigtermPids 报告的成功数少于找到的后代数时）是池化路径中没有的有用补充。四个聚焦的测试覆盖：stdio 后代在断开连接前收到信号、远程传输跳过 PID 枚举、无后代时跳过信号发送、枚举失败时仍执行断开连接。没有范围蔓延。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

_{Reviewed at 863667595a7fbf62f5febf712e287d4a2e176bf2 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review: I independently proposed adding a best-effort descendant PID sweep before client.disconnect() in the timeout handler — get root PID via getTransportPid(), list descendants, SIGTERM them, wrap in try/catch, always proceed to disconnect. The PR's approach is identical to my proposal and faithfully mirrors the existing PoolEntry.sweepAndDisconnect pattern (mcp-pool-entry.ts:822). No critical issues, no AGENTS.md violations.

The diff is clean: one import line, 28 lines of new logic (try/catch for the PID sweep, then the existing disconnect block), and four focused tests. Reuses the existing cross-platform listDescendantPids/sigtermPids utilities — no new abstractions. Error handling is appropriately best-effort: the outer try/catch ensures disconnect() always runs even if PID enumeration throws. The partial-signal warning (when sigtermPids returns fewer successes than descendants found) is a useful addition not present in the pooled path — good defensive logging. Remote transports (no PID) correctly skip enumeration via the rootPid !== undefined guard and log a debug message.

One observation: the PR adds a debug log when no PID is available ("Skipping descendant pid sweep") and a warn log on partial signal coverage, both of which the pooled sweepAndDisconnect lacks. These are appropriate for the non-pooled path since it lacks the SweepResult structure the pooled path uses for structured reporting — the logs are the observability channel here.

Tests:

$ cd packages/core && npx vitest run src/tools/mcp-client-manager.test.ts

 ✓ src/tools/mcp-client-manager.test.ts (116 tests) 913ms

 Test Files  1 passed (1)
      Tests  116 passed (116)
   Start at  23:40:21
   Duration  9.13s

All 116 tests pass, including the 4 new tests covering:

• Stdio wrapper descendants are signaled before disconnect after timeout
• Remote transports skip PID enumeration entirely
• No-descendants case skips signaling but still disconnects
• Disconnect still runs when descendant enumeration throws

Real-scenario testing: This is an internal process-lifecycle change (no UI) — the behavior is about signaling descendant PIDs during a discovery timeout, which cannot be meaningfully observed through tmux capture-pane. The unit tests with mocked PID values comprehensively verify the wiring (signal → disconnect ordering, PID enumeration skip for remote, error resilience). Maintainer wenshao independently performed a real end-to-end controlled experiment on macOS at head 863667595 — driving the real McpClientManager → real StdioClientTransport → real PID sweep against a real process tree — confirming: on main the grandchild leaks (alive, reparented to init), on this PR the grandchild receives SIGTERM and is killed (3/3 trials). The load-bearing check (reverting only the sweep hunk) flips exactly the 3 stdio-path tests red while the remote control stays green.

中文说明

代码审查： 我独立提出的方案是在超时处理程序的 client.disconnect() 之前添加尽力而为的后代 PID 清理——通过 getTransportPid() 获取根 PID、列出后代、发送 SIGTERM、包裹在 try/catch 中、始终继续断开连接。PR 的方案与我的提案完全一致，并忠实地镜像了 PoolEntry.sweepAndDisconnect（mcp-pool-entry.ts:822）中的现有模式。无关键问题，无 AGENTS.md 违规。

Diff 干净：一行 import、28 行新逻辑（PID 清理的 try/catch，然后是现有的 disconnect 块）、四个聚焦的测试。复用现有的跨平台 listDescendantPids/sigtermPids 工具——没有新的抽象。错误处理适当地采用尽力而为模式：外层 try/catch 确保即使 PID 枚举抛出异常，disconnect() 也始终执行。部分信号警告（当 sigtermPids 返回的成功数少于找到的后代数时）是池化路径中没有的有用补充——良好的防御性日志。远程传输（无 PID）通过 rootPid !== undefined 守卫正确跳过枚举并记录 debug 消息。

一个观察：PR 添加了无 PID 可用时的 debug 日志（"Skipping descendant pid sweep"）和部分信号覆盖时的 warn 日志，这两者池化 sweepAndDisconnect 路径中都没有。这对非池化路径是合适的，因为它缺少池化路径用于结构化报告的 SweepResult 结构——日志是这里的可观测性通道。

测试： 全部 116 个测试通过（包括 4 个新增测试），覆盖：stdio 包装进程后代在超时后被信号通知再断开连接、远程传输完全跳过 PID 枚举、无后代情况跳过信号发送但仍断开连接、后代枚举失败时仍执行断开连接。

真实场景测试： 这是内部进程生命周期更改（无 UI）——行为涉及发现超时期间的后代 PID 信号发送，无法通过 tmux capture-pane 有意义地观察。带模拟 PID 值的单元测试全面验证了接线（信号→断开连接顺序、远程传输跳过 PID 枚举、错误弹性）。维护者 wenshao 在 head 863667595 上独立进行了真实端到端对照实验——驱动真实 McpClientManager → 真实 StdioClientTransport → 真实 PID 清理作用于真实进程树——确认：在 main 上孙进程泄漏（存活，被 reparent 到 init），在本 PR 下孙进程收到 SIGTERM 并被终止（3/3 次试验）。承重性检查（仅回退 sweep 代码段）恰好使 3 个 stdio 路径测试变红，而远程对照保持绿色。

— Qwen Code · qwen3.7-max

_{Reviewed at 863667595a7fbf62f5febf712e287d4a2e176bf2 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — Clean across every stage. Minimal, focused parity fix with strong test coverage and independent maintainer verification.

This is a textbook bugfix: a documented gap between the pooled and non-pooled MCP lifecycle paths, fixed by reusing existing utilities with no new abstractions. The 29-line production change does exactly one thing — best-effort descendant PID sweep before disconnect — and the four tests comprehensively cover the stdio, remote, no-descendants, and error paths. The diff faithfully mirrors the sweepAndDisconnect pattern in mcp-pool-entry.ts, which is exactly what you'd want for a parity fix. The partial-signal warning is a thoughtful addition that improves observability on the non-pooled path. No scope creep, no drive-by refactors, no over-engineering. Maintainer wenshao independently verified with a real end-to-end test on macOS at this exact commit, confirming the grandchild leak on main and the fix on this branch. Ships it.

中文说明

信心度：5/5 — 每个阶段都干净。最小化的对等修复，测试覆盖充分，且有维护者的独立验证。

这是一个教科书式的 bugfix：池化和非池化 MCP 生命周期路径之间的已记录差距，通过复用现有工具修复，无新抽象。29 行生产代码只做一件事——断开连接前尽力而为地清理后代 PID——四个测试全面覆盖了 stdio、远程、无后代和错误路径。diff 忠实地镜像了 mcp-pool-entry.ts 中的 sweepAndDisconnect 模式，这是对等修复应有的样子。部分信号警告是一个周到的补充，改善了非池化路径的可观测性。无范围蔓延，无顺手重构，无过度工程。维护者 wenshao 在完全相同的提交上独立进行了 macOS 真实端到端测试，确认了 main 上的孙进程泄漏和本分支上的修复。可以合并。

— Qwen Code · qwen3.7-max

_{Reviewed at 863667595a7fbf62f5febf712e287d4a2e176bf2 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Reviewed. Suggestions are inline. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.

— qwen3.7-max via Qwen Code /review

[github-actions]

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。

[qwen-code-ci-bot]

Reviewed. Suggestions are inline. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Reviewed. Suggestions are inline. Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.

— qwen3.7-max via Qwen Code /review

[qqqys]

Blocking: the current head d569b24 leaves a duplicated partial-signal fragment at packages/core/src/tools/mcp-client-manager.ts:2402-2406 after the complete if block ending at line 2401. This is syntactically invalid TypeScript; the required Ubuntu and web-shell jobs fail at install/build time. Remove the stray duplicate lines and rebalance the braces before merge.

[morluto]

Addressed the blocking review in commit 8636675. The five stray lines introduced at d569b24 were removed, restoring balanced TypeScript syntax while preserving the complete partial-signal warning block. I reproduced the relevant gates locally: MCP client manager tests 116/116, repository build, repository typecheck, focused ESLint, Prettier, diff-check, and final autoreview all pass.

[qwen-code-ci-bot]

Reviewed. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.

— qwen3.7-max via Qwen Code /review

[wenshao]

✅ Local real-build verification — confirmed

I verified this PR locally with a real build (not just the mocked unit tests) at head 863667595.

TL;DR: The fix works end-to-end in the real runtime path. A real stdio wrapper that leaks a real grandchild has that grandchild SIGTERM'd by the sweep and reaped with this PR, whereas on main the grandchild leaks (reparented to init). Full suite, ESLint, and typecheck are green. Also cross-checked on macOS, which the PR notes as "not tested".

Environment: macOS (Darwin 24.6.0) · Node v22.23.1 · @modelcontextprotocol/sdk 1.29.0 · core built via npm run build (tsc --build + esbuild).

1. Real end-to-end controlled experiment (the key evidence)

The PR's unit tests mock listDescendantPids/sigtermPids, so they prove the wiring but not the behavior. I drove the real McpClientManager → real McpClient → real StdioClientTransport → real pid sweep against a real process tree:

• A fake stdio server (an npx/uvx-shaped wrapper) spawns a real grandchild node process, then hangs the MCP initialize handshake so single-session discovery times out.
• The grandchild installs a SIGTERM handler that records the signal — proving it was signalled by the sweep, not merely reaped when its parent died.

Only the sweep hunk differs between the two builds (the "before" source is byte-identical to main):

Build	sigtermReceived	grandchild after cleanup	3 trials
BEFORE (== main, no sweep)	false	🔴 LEAKED (alive)	3/3
AFTER (this PR)	true	🟢 KILLED	3/3

This reproduces #6918 on main and demonstrates the fix closes it, deterministically.

2. Load-bearing check (tests genuinely exercise the fix)

Reverting only the sweep hunk (keeping the new tests) flips exactly the three stdio-path tests red, while the remote/no-PID control stays green:

× signals stdio wrapper descendants before disconnecting after timeout
✓ disconnects remote transports without enumerating pids   ← control stays green
× skips signaling when a stdio transport has no descendants
× still disconnects when descendant enumeration fails
Tests  3 failed | 1 passed

3. Full suite + gates at head

• vitest run mcp-client-manager.test.ts → 116 passed (116) (incl. the 4 new tests)
• eslint mcp-client-manager.ts mcp-client-manager.test.ts → clean (exit 0)
• npm run build (tsc --build + esbuild) → success

4. Code review

The change is a faithful mirror of the pooled path's PoolEntry.sweepAndDisconnect: SIGTERM descendants (best-effort) → then disconnect, reusing the existing bounded, cross-platform listDescendantPids/sigtermPids utilities. The sweep is wrapped in try/catch so disconnect() always runs, and the remote (no-PID) path correctly skips enumeration. Same-as-pooled semantics: descendants get SIGTERM only (no SIGKILL escalation) — a deliberate scope match, not a regression.

Verdict: LGTM — ready to merge from a verification standpoint.

🇨🇳 中文版本（点击展开）

✅ 本地真实构建验证 —— 已确认

我在本地对本 PR 进行了真实构建验证（不仅仅是被 mock 的单元测试），基于 head 863667595。

结论： 该修复在真实运行路径中端到端有效。一个泄漏真实孙进程的真实 stdio 包装器，在本 PR 下其孙进程会收到 SIGTERM 并被回收；而在 main 上孙进程会泄漏（被 reparent 到 init）。完整测试、ESLint 与类型检查全绿。另外在 PR 标注为"未测试"的 macOS 上也做了交叉验证。

环境： macOS (Darwin 24.6.0) · Node v22.23.1 · @modelcontextprotocol/sdk 1.29.0 · 通过 npm run build（tsc --build + esbuild）构建 core。

1. 真实端到端对照实验（核心证据）

PR 的单元测试mock 了 listDescendantPids/sigtermPids，因此只验证了接线而非行为。我驱动了真实的 McpClientManager → 真实 McpClient → 真实 StdioClientTransport → 真实的 pid sweep，作用于真实的进程树：

• 一个仿真 stdio 服务器（npx/uvx 形状的包装器）派生一个真实孙进程 node，然后挂起 MCP initialize 握手，使单会话发现超时。
• 孙进程安装了 SIGTERM 处理器记录信号 —— 证明它是被 sweep 主动发信号，而非父进程死亡后被顺带回收。

两个构建之间仅 sweep 这一段代码不同（"before" 源码与 main 逐字节相同）：

构建	sigtermReceived	清理后孙进程	3 次试验
BEFORE（== main，无 sweep）	false	🔴 泄漏（存活）	3/3
AFTER（本 PR）	true	🟢 被终止	3/3

这在 main 上复现了 #6918，并确定性地证明了修复将其闭合。

2. 承重性检查（测试确实验证了修复）

仅回退 sweep 这一段（保留新测试），恰好使三个 stdio 路径测试变红，而 remote/无 PID 的对照用例保持绿色：

× signals stdio wrapper descendants before disconnecting after timeout
✓ disconnects remote transports without enumerating pids   ← 对照，保持绿色
× skips signaling when a stdio transport has no descendants
× still disconnects when descendant enumeration fails
Tests  3 failed | 1 passed

3. Head 上的完整测试与门禁

• vitest run mcp-client-manager.test.ts → 116 passed (116)（含 4 个新测试）
• eslint mcp-client-manager.ts mcp-client-manager.test.ts → 干净（exit 0）
• npm run build（tsc --build + esbuild）→ 成功

4. 代码审查

该更改忠实地镜像了池化路径的 PoolEntry.sweepAndDisconnect：先 SIGTERM 后代（尽力而为）→ 再断开连接，复用了现有的、有界的、跨平台的 listDescendantPids/sigtermPids 工具。sweep 包裹在 try/catch 中，确保 disconnect() 始终执行；remote（无 PID）路径正确跳过枚举。与池化路径语义一致：后代仅收 SIGTERM（不升级到 SIGKILL）—— 这是有意的范围对齐，而非回归。

结论：从验证角度看 LGTM —— 可以合并。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

fixed