fix(mcp): recover closed stdio sessions after keepalive failures

[TurgutKural]

Summary

Follow-up to #62811 and replacement for #64989.

#62811 confirms the same MCP failure class: keepalive and an active RPC can interfere on the single JSON-RPC stream, causing a wedged/closed transport, false reconnects, and failed tool calls. This PR keeps the _rpc_lock serialization from #64989 and adds the missing transport-recovery path for the resulting closed-stream exceptions.

Changes

• Preserve _rpc_lock serialization for keepalive ping / list_tools probes.
• Classify empty-message transport exceptions (anyio.ClosedResourceError, BrokenResourceError, EndOfStream, BrokenPipeError, EOFError) as reconnectable transport failures.
• Use the diagnostic exception formatter in keepalive logs so the failure type is not lost.
• Add regression tests for closed-resource and broken-pipe classification.

The existing reconnect path remains bounded: one reconnect/retry at the tool-call layer, the existing reconnect budget, circuit breaker, and parked self-probe behavior remain unchanged.

Validation

• 278 targeted MCP/reconnect tests passed.
• py_compile passed.
• git diff --check passed.

Refs: #62811, closes #64989

[teknium1]

Thanks for carrying the closed-stream recovery follow-up. The underlying serialization premise remains on current main: keepalive RPCs are outside the shared lock (tools/mcp_tool.py:2004-2026), while normal calls hold it (tools/mcp_tool.py:4100-4108).

Problems

• tools/mcp_tool.py:3507: the new anyio/"Resource" predicate matches any AnyIO exception with Resource in its name, not only the closed-stream failures named in the PR. A match enters the reconnect-and-retry path (tools/mcp_tool.py:3540-3588), so this can turn an unrelated error into transport churn.

Suggested changes

• Limit this branch to the explicit closed-stream exception names/types and add a negative test for a non-target AnyIO-style resource exception.
• Preserve #62811's active-RPC skip when salvaging serialization; its focused regression establishes that an in-flight RPC is the liveness signal and a periodic probe should not queue behind it.

This is an automated hermes-sweeper review.

[TurgutKural]

Rebased onto current upstream/main (head f82e29cc5). All required CI checks pass.

Changes in this rebase:

• Added the closed-event-loop guard in _wait_for_reconnect_or_shutdown so a parked MCP task woken after the loop closes returns "shutdown" instead of raising Event loop is closed (fix(mcp): recover closed stdio sessions after keepalive failures #65111).
• Narrowed the session-expiry classifier to the explicit closed/broken/end-of-stream types (review feedback: the previous anyio + "Resource" substring predicate matched unrelated AnyIO errors and could churn the transport). Added a negative regression test.

[TurgutKural]

Deep upstream-already-fixed analysis — verdict: PARTIALLY_FIXED (gap still real, PR is needed)

I read current upstream/main (c0c76a4715) tools/mcp_tool.py (5913 lines), traced the SDK stdio receive path (mcp 1.27.1), and empirically executed both the upstream classifier and the PR-head classifier against the real bare exceptions a dead stdio transport raises.

What upstream ALREADY has (the reconnect plumbing is in place)

• MCPServerTask lifecycle loop: _reconnect_event + _run_stdio/_run_http tear down and rebuild the streamablehttp_client/ClientSession pair and respawn a dead stdio subprocess (tools/mcp_tool.py:1748, 2110, 2212, 2539, 2899+).
• Tool/resource/prompt handlers route every call exception through _handle_session_expired_and_retry → (mcp_tool.py:4299, 4363, 4430, 4495, 4566), which signals reconnect and retries once.
• Idle recycle: 6c731fe59 "Recycle idle MCP stdio servers"; lazy reconnect on recycled state _get_connected_server_for_call/_request_lazy_reconnect (:4081, 4057).
• Keepalive probe _keepalive_probe (:2029) surfaces closed transport as a liveness failure that triggers reconnect.

The partial fix that's already upstream

Commit a1f85ef2b "fix(mcp): retry stale pipe transport failures" (May 5 2026) added string markers to _SESSION_EXPIRED_MARKERS (tools/mcp_tool.py:3516-3529): "closedresourceerror", "closed resource", "transport is closed", "connection closed", "broken pipe", "end of file". Intent was exactly this PR's case.

The gap — why the PR is still needed (empirically verified)

The classifier _is_session_expired_error (tools/mcp_tool.py:3532-3555) does:

msg = str(exc).lower()
if not msg:
    return False          # <-- empty message => NOT recoverable
return any(marker in msg for marker in _SESSION_EXPIRED_MARKERS)

When a stdio subprocess dies, the SDK stdio stdout_reader ends (mcp/client/stdio/__init__.py:163) and the base session _receive_loop (mcp/shared/session.py:351) yields the bare AnyIO exceptions — anyio.ClosedResourceError, anyio.EndOfStream, anyio.BrokenResourceError (BrokenPipeError/EOFError on the write side). Critically, these exceptions have an empty str():

>>> str(anyio.ClosedResourceError())
''
>>> str(anyio.EndOfStream())
''
>>> str(anyio.BrokenResourceError())
''

I executed the actual upstream function against these real exceptions:

anyio.ClosedResourceError()  -> False
anyio.EndOfStream()          -> False
anyio.BrokenResourceError()  -> False
BrokenPipeError()            -> False
EOFError()                   -> False

So on upstream/main, a closed stdio session still raises on the next call_tool/list_tools/keepalive instead of reconnecting — the empty-message early-return defeats the a1f85ef2b markers. Upstream has no except anyio.ClosedResourceError / isinstance(...) branch anywhere (grep for except anyio/isinstance(...ClosedResource...) across the file returns nothing).

Why the PR is correct and should stay open

PR #65111 makes _is_session_expired_error type-based first (PR head tools/mcp_tool.py:3561+):

if type(exc).__name__ in {"ClosedResourceError","BrokenResourceError","EndOfStream"}
   or isinstance(exc, (BrokenPipeError, EOFError)):
    return True

I executed the actual PR-head function against the same real exceptions:

anyio.ClosedResourceError()  -> True
anyio.EndOfStream()          -> True
anyio.BrokenResourceError()  -> True
BrokenPipeError()            -> True
EOFError()                   -> True
RuntimeError('invalid or expired session') -> True   (legacy markers preserved)
RuntimeError('Tool failed')  -> False                (no false positives)

It keeps _rpc_lock serialization (from #62811/#64989) and bounds the path exactly as upstream does (one reconnect/retry at call layer, existing reconnect budget, circuit breaker, parked self-probe).

Conclusion

ALREADY_FIXED_DIFFERENTLY does not apply: the reconnect mechanism exists, but its classifier gate cannot match the real closed-stdio exception types because they carry empty messages. The PR closes a genuine, reproducible gap. Recommended: keep open / merge — it is not redundant with a1f85ef2b.

Evidence table:

Item	File:line (upstream/main c0c76a4715)	Commit
Reconnect lifecycle loop	tools/mcp_tool.py:1748,2110,2212,2539	743c116fb, 6c731fe59
Session-expired retry routing on every call	tools/mcp_tool.py:4299,4363,4430,4495,4566	27beeb183
String-marker classifier w/ empty-msg early return	tools/mcp_tool.py:3516-3555	a1f85ef2b
SDK stdio reader ends on process exit	mcp/client/stdio/__init__.py:163	—
SDK base receive loop	mcp/shared/session.py:351	—
PR type-based classifier (the fix)	PR fix/mcp-transport-recovery tools/mcp_tool.py:3561+	#65111

[GottZ]

This was generated by AI during triage.

Thanks for the follow-up — the core classifier fix is sound and I verified the failure path on current main: _is_session_expired_error bails at if not msg: return False (tools/mcp_tool.py:3553), so bare anyio.ClosedResourceError / EndOfStream / BrokenResourceError (empty str()) never reach the _SESSION_EXPIRED_MARKERS added in a1f85ef2b. The type-based branch plus the _exc_str switch close exactly that gap, and the narrowed exact-name matching with the negative test resolves the earlier "any AnyIO Resource error" concern.

Two things before merge:

1. Keepalive skip vs. queue. This still wraps the probe in a plain async with self._rpc_lock (tools/mcp_tool.py:2047, :2069) rather than the active-RPC skip (if self._rpc_lock.locked(): return). fix(mcp): serialize keepalive with active RPCs #62811 argues — and covers with a regression — that an in-flight RPC already proves liveness, so the periodic probe should not queue behind a long-running tools/call. Could we adopt the skip guard here so the two PRs converge on one keepalive semantics?
2. Overlap. fix(mcp): serialize keepalive with active RPCs #62811 (still open, different author) touches the same _keepalive_probe, so exactly one of these can merge cleanly; and fix(mcp): make MCP subprocess death invisible to the agent #10250 rewrites large parts of tools/mcp_tool.py. It would help to state explicitly how this PR relates to fix(mcp): serialize keepalive with active RPCs #62811 (supersede / rebase-on-top) and to note that the transport-recovery classifier here is the piece fix(mcp): serialize keepalive with active RPCs #62811 lacks. The recovery classifier is the genuinely novel and needed part — I'd suggest it not be blocked on the keepalive-serialization question, which fix(mcp): serialize keepalive with active RPCs #62811 handles more precisely.

Minor: the closed-event-loop guard returns from a finally block, which would swallow a propagating exception — fine here since it only triggers on a closed/absent loop during shutdown, but worth a one-line note. Also consider an integration test that drives the full closed-session → keepalive-failure → reconnect path; current tests exercise the classifier and the lock in isolation. Relevant issue context: this partially addresses #30268 (Mac sleep/wake keepalive failures) via the now-recognized dead-transport exceptions.

[TurgutKural]

Close as absorbed upstream

This PR's functionality has been independently implemented on upstream/main through these commits:

• f26fb901e (fix(mcp): reconnect message-less closed transports) — Added type-based exception handling in _is_session_expired_error for anyio.BrokenResourceError, ClosedResourceError, and EndOfStream, exactly matching the approach in this PR.
• fbe086f7c (fix(mcp): cycle-guard cause/context traversal in transport classifier) — Added iterative exception-chain traversal with identity-visited set and node budget, matching the PR's traversal approach.
• ee23bffee (fix(mcp): clear connect-cooldown state on every shutdown path) — Related shutdown cleanup.
• The event loop closed guard (_mcp_loop_exception_handler) was already present from an earlier commit.

What upstream has that the PR also proposed:

• ✅ Type-based check for ClosedResourceError/EndOfStream/BrokenResourceError (not string-based)
• ✅ Iterative exception chain traversal with cycle detection
• ✅ Event loop closed suppression during shutdown

No remaining gap — closing.