fix(mcp): stdio discovery cooldown, message-less transport classification, zero-connected retry latch - #68660
Merged
Merged
Conversation
alt-glitch
added
type/bug
Contributor
૮ >ﻌ< ა ci reviewrunning on d8d64bc looks good to me! |
Builds on trevorgordon981's #50589 (cherry-picked as the previous commit). The #50394 cooldown reset only ran inside the async _shutdown coroutine, which is skipped on the empty-_servers fast path — the most common state when a server failed to connect (failed servers are never recorded in _servers). It was also skipped when the MCP loop wasn't running. Clear _server_connect_retry_after/_server_connect_failures on the fast path and in a final unconditional sweep so a full shutdown/restart always re-attempts every configured server immediately. Adds regression tests for both paths.
Builds on diffen77's #66547 (cherry-picked as the previous commits). Extend _is_session_expired_error's iterative traversal to follow __cause__/__context__ in addition to ExceptionGroup .exceptions — SDK wrappers often raise a generic RuntimeError *from* the message-less ClosedResourceError, leaving the transport signal reachable only via the chain. The identity-visited set guards chain cycles (handlers re-raising previously seen exceptions), and a bounded node budget (_EXC_TRAVERSAL_MAX_NODES) caps pathological acyclic graphs. Adds regression tests: cause/context chain detection, interruption precedence through chains, cyclic cause/context termination, and budget-bounded termination.
… nothing start_background_mcp_discovery() sets _mcp_discovery_started once and never resets it. If the first background run exits without connecting any MCP server (startup cancellation, OOM restart, transient network failure), every later call returns immediately and the process is permanently stuck with zero MCP tools until a full restart. Fix: when discovery is marked started but the thread is dead and no server is connected, reset the flag and spawn a fresh discovery thread. Also log a WARNING when a discovery run completes with zero connected servers, so the condition is visible instead of silent. Caught in production on a long-running gateway fleet where a gateway restarted under memory pressure and came back with all MCP tools missing.
Review follow-up: the stdio hermes --tui path spawned its own one-shot discovery thread, so the retry-after-zero-connected semantics added to start_background_mcp_discovery() did not cover it. Spawn TUI discovery through the shared owner and make the entry-side wait_for_mcp_discovery() fall through to the shared owner when no local thread exists (mcp_discovery_in_flight/join_mcp_discovery already consult both owners). Keeps the cheap no-mcp-servers config guard on the TUI path. Adds regression tests for the entry-side wait delegation.
Builds on fazerluga-creator's #66981 (cherry-picked as the previous two commits). start_background_mcp_discovery()'s retry allowance only fires when the function is CALLED again, but tui_gateway/entry.py main() calls it exactly once at startup — so a first discovery run that connected nothing still latched the stdio TUI MCP-less for the whole session. Re-invoke the idempotent spawn from wait_for_mcp_discovery() (the per-agent-build wait) when the process is MCP-enabled, gated on a flag set in main() so non-MCP sessions never pay the MCP import on the wait path. Adds regression tests for both the retry re-invocation and the non-MCP skip.
The TUI retry-allowance follow-up made tui_gateway.entry.wait_for_mcp_discovery delegate to hermes_cli.mcp_startup unconditionally when no entry-local thread exists. But server._make_agent already calls the startup wait directly for dashboard /api/ws sessions, so every non-stdio agent build paid the bounded wait twice (caught by test_make_agent_waits_for_shared_mcp_discovery). Gate the retry-spawn AND the delegated wait on _mcp_discovery_enabled, which only the stdio TUI arms in main().
teknium1
force-pushed
the
salvage/mcp-discovery-classifier-fixes
branch
from
July 21, 2026 14:09
ea78cf6 to
d8d64bc
Compare
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three MCP reliability fixes: failed stdio servers stop being re-spawned on every discovery pass (cooldown), message-less anyio ClosedResource/BrokenResource errors are classified correctly instead of bypassing the reconnect classifier, and background discovery can retry after a run that connected nothing (CLI latch + the TUI's own discovery thread).
Salvages #50589 by @trevorgordon981, #66547 by @diffen77, #66981 by @fazerluga-creator (authorship preserved).
Changes
tools/mcp_tool.py: stdio failure cooldown (fix(mcp): isolate a single failing stdio server from the bridge #50589) + follow-up: cooldown state now resets on the empty-_serversshutdown fast path toohermes_cli/mcp_startup.py: one-shot latch allows retry after zero-connected runs (fix(mcp): allow background discovery retry after a run that connected nothing #66981) + follow-up: same allowance for the TUI discovery thread (tui_gateway/entry.py)Validation
Targeted MCP + startup + TUI-entry suites green (see per-file tests in the diff).
Infographic