Skip to content

fix(mcp): isolate a single failing stdio server from the bridge - #50589

Closed
trevorgordon981 wants to merge 1 commit into
NousResearch:mainfrom
trevorgordon981:fix/50394-mcp-bridge-single-failure
Closed

fix(mcp): isolate a single failing stdio server from the bridge#50589
trevorgordon981 wants to merge 1 commit into
NousResearch:mainfrom
trevorgordon981:fix/50394-mcp-bridge-single-failure

Conversation

@trevorgordon981

@trevorgordon981 trevorgordon981 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Closes #50394.

A failing stdio MCP server (bad PATH / exec-not-found / crash-on-start) was never recorded in _servers — _connect_server → MCPServerTask.start() raises before the _servers[name] = server line runs. So the new-servers filter never skipped it, and every discover_mcp_tools() call (one per agent worker session, every few seconds) re-spawned it from scratch — a restart storm of unreaped subprocesses + event-loop churn that intermittently knocked healthy co-located servers' tools into "Unknown tool". The existing circuit breaker only guarded runtime tool-call dispatch, not re-discovery.

Fix: a per-server connection-retry cooldown with exponential backoff (30s → 600s). register_mcp_servers skips a server whose cooldown is active (isolating the failure; healthy servers untouched); the failure path arms the backoff, success clears it, and shutdown_mcp_servers clears the maps so /reload-mcp and restarts re-attempt immediately.

Tests: +6 (test_mcp_bridge_single_failure.py) incl. the regression (failing server not re-spawned on the 2nd discovery pass, but retried after cooldown expiry); 228 pass across the MCP suite.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 22, 2026
@alt-glitch

alt-glitch commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Duplicate of #50482 — both PRs fix #50394 (a single failing stdio MCP server churning the whole bridge) by adding a per-server connection-failure cooldown/circuit-breaker in tools/mcp_tool.py that skips a persistently-broken server during discover_mcp_tools() until a backoff elapses. Same file, same issue, same mechanism. #50482 was opened earlier (2026-06-22 00:26 vs 04:01), so it's the canonical version.

teknium1
teknium1 reviewed Jul 14, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the cross-discovery failure mode. The premise still exists on current main: _servers[name] is assigned only after _connect_server() returns (tools/mcp_tool.py:4850-4858), but start() raises an initial-connect error (tools/mcp_tool.py:2879-2896); the registration filter then retries any enabled name absent from _servers (tools/mcp_tool.py:4900-4904).

Problems

  • The added reset at tools/mcp_tool.py:4652-4658 is skipped in the exact only-failing-server case. shutdown_mcp_servers() returns when its _servers snapshot is empty before its nested _shutdown() coroutine runs (tools/mcp_tool.py:5425-5431 on current main). The connection cooldown therefore survives the reload path the PR says should retry immediately.

Suggested changes

  • Clear the retry maps before the empty-server fast path and add a regression test for shutdown after a failed-only discovery.
  • Reconcile this with current main's parked initial-connect task flow (tools/mcp_tool.py:2772-2799) during salvage.

Automated hermes-sweeper review.

Comment thread tools/mcp_tool.py
_server_connect_retry_after.clear()
_server_connect_failures.clear()

with _lock:

@teknium1 teknium1 Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reset is unreachable when the failed server is the only configured server: shutdown_mcp_servers() returns through its empty-_servers fast path before _shutdown() runs. Clear these maps before that fast path and cover failed-only shutdown/reload with a regression test.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
teknium1 added a commit that referenced this pull request Jul 21, 2026
@teknium1
fix(mcp): clear connect-cooldown state on every shutdown path
79d7995 
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.
@teknium1 teknium1 mentioned this pull request Jul 21, 2026
Merged
teknium1 added a commit that referenced this pull request Jul 21, 2026
@teknium1
fix(mcp): clear connect-cooldown state on every shutdown path
e46bdef 
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.
teknium1 added a commit that referenced this pull request Jul 21, 2026
@teknium1
fix(mcp): clear connect-cooldown state on every shutdown path
ee23bff 
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.
@teknium1

teknium1 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Merged via #68660. Your stdio failure cooldown was cherry-picked with authorship preserved; we added a follow-up so cooldown state also resets on the empty-_servers shutdown fast path.

@teknium1 teknium1 closed this Jul 21, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@teknium1
fix(mcp): clear connect-cooldown state on every shutdown path
0da8c71 
Builds on trevorgordon981's NousResearch#50589 (cherry-picked as the previous commit).
The NousResearch#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.
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
@teknium1
fix(mcp): clear connect-cooldown state on every shutdown path
41fea13 
Builds on trevorgordon981's NousResearch#50589 (cherry-picked as the previous commit).
The NousResearch#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.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
@melon-xf
fix(mcp): clear connect-cooldown state on every shutdown path
971bb5f 
Builds on trevorgordon981's NousResearch#50589 (cherry-picked as the previous commit).
The NousResearch#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A single failing stdio MCP server churns the whole MCP bridge — healthy servers' tools become "Unknown tool" at runtime

3 participants

@trevorgordon981 @alt-glitch @teknium1