Skip to content

fix: correct inverted liveness check in _stdio_children_dead - #95012

Closed
RafaelGali-Dextra wants to merge 2 commits into
NousResearch:mainfrom
RafaelGali-Dextra:fix/mcp-stdio-children-dead-inverted-liveness
Closed

RafaelGali-Dextra wants to merge 2 commits into
NousResearch:mainfrom
RafaelGali-Dextra:fix/mcp-stdio-children-dead-inverted-liveness

Conversation

@RafaelGali-Dextra

Copy link
Copy Markdown

Summary

MCPServerTask._stdio_children_dead() is meant to return True only when every stdio child the MCP server spawned has exited, so the fast-fail path (#81995) can fail a tool call immediately against a known-dead process instead of waiting out the full tool timeout.

The loop had its return values inverted: as soon as it found a live pid (psutil.pid_exists() True), it returned True ("all dead") instead of False ("at least one still alive"). The intended False branch sat right after an unconditional return, so it was unreachable dead code.

for pid in pids:
    if not psutil.pid_exists(pid):
        continue  # this one is dead
    return True  # <- wrong: process is ALIVE, should not report "dead"
    return False  # <- unreachable dead code
return True

Impact

Any stdio MCP server with a captured child PID fails its very first real tool call with:

MCP stdio subprocess for '<name>' has exited; failing the call fast instead of waiting 300s

even though the subprocess is alive and would answer normally. Connection/tool-discovery (hermes mcp test, initial tool list) is unaffected because it doesn't go through this fast-fail path — which is why the server looks perfectly healthy right up until the first tool call, making this look like an auth/connectivity problem rather than a liveness-check bug.

Reproduced end-to-end with sooperset/mcp-atlassian (Jira) run over stdio via uvx: every jira_* tool call failed immediately post-connect until this fix was applied (verified against the live Jira REST API in parallel — credentials/auth were fine the whole time). Tool calls succeed normally after the fix.

Changes

  • tools/mcp_tool.py: fix the inverted return value, drop the unreachable duplicate return.
  • tests/tools/test_mcp_tool.py: add TestStdioChildrenDead covering the three states (child alive, all children exited, no pids captured).

Test Plan

  • New unit tests pass: pytest tests/tools/test_mcp_tool.py -k TestStdioChildrenDead -v (3 passed)
  • Full file suite green: pytest tests/tools/test_mcp_tool.py -q (101 passed)
  • Manually verified against a real stdio MCP server (mcp-atlassian/Jira) before and after the fix

_stdio_children_dead() is meant to return True only when every stdio
child the MCP server spawned has exited (used to fast-fail a tool call
against a known-dead process instead of waiting out the full timeout,
see NousResearch#81995).

The loop had the return values inverted: as soon as it found a LIVE
pid (psutil.pid_exists() True), it returned True ("all dead") instead
of False ("at least one still alive"). The correct False branch was
placed after an unconditional return and was dead code, unreachable.

Net effect: any stdio MCP server with a captured child PID fails its
very first real tool call with:

  MCP stdio subprocess for '<name>' has exited; failing the call fast
  instead of waiting 300s

even though the subprocess is alive and would have answered normally.
Connection/listing (hermes mcp test, tool discovery) is unaffected
since it does not go through this fast-fail path, which is why the
server appears healthy right up until the first tool call.

Fix: return False when a live pid is found (mirrors the docstring:
"True when every stdio child we spawned has exited"), remove the
unreachable duplicate return.

Reproduced with sooperset/mcp-atlassian (Jira) run over stdio via
uvx: every jira_* tool call failed immediately post-connect until
this fix was applied; tool calls succeeded normally afterward.
Regression coverage for the inverted-liveness bug fixed in the
previous commit: a live child pid must yield False (not dead), all
children exited must yield True, and no captured pids must yield
False (unknown -> don't fast-fail).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth duplicate This issue or pull request already exists labels Aug 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #94339. Both repair the identical live-PID return inversion in _stdio_children_dead; #94339 is the earlier open canonical fix.

@SassoSte

Copy link
Copy Markdown

Independent reproduction, different platform/servers than the reports above: hit this today on macOS via two unrelated stdio servers — x_api (xurl mcp bridge) and github (@modelcontextprotocol/server-github). Same exact error text on both (MCP stdio subprocess for '<name>' has exited; failing the call fast...), first call, every time — enumerate-only hermes mcp test passed cleanly on both the whole time, which is what made it look server-specific at first.

Confirmed root cause matches this PR: _stdio_children_dead() in tools/mcp_tool.py returns True on the first live PID it checks. Also confirmed a full Hermes desktop restart (fresh serve PID, fresh stdio child PIDs, no inherited state) does not clear it — the bug is deterministic in the code, not a stuck/wedged process.

Applied the same fix locally (swap the inverted return, drop the unreachable dead branch) and verified live: get_posts_by_id and list_issues both return real data immediately post-patch. +1 for merging — this affects any stdio MCP server with a captured child PID, not just the servers named in the existing reports.

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Closing — the _stdio_children_dead polarity inversion was fixed on main today via #94339 (@liuhao1024's carrier: same un-inversion plus fail-open hardening for psutil/probe failures, with an 8-test regression suite; merge commit ef46ec0). This PR's diagnosis of the inverted live-child branch was correct — it was independently found by many contributors, and #94339 was selected as the canonical carrier per the duplicate-PR rule (tests + completeness + community verification). Thanks for the report and fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants