fix(mcp): bound reconnect transport setup - #63823
Closed
oralzl wants to merge 1 commit into
Closed
Conversation
tonydwb
reviewed
Jul 13, 2026
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
Bounding MCP reconnect transport setup prevents a runaway retry loop from blocking connection. 2 files, 95 lines. Title matches diff.
Strengths
- A timeout on the reconnect transport path is the correct place — not the broader connection lifecycle.
Reviewed by Hermes Agent in batch mode
Contributor
|
Thanks for the focused MCP reconnect analysis and regression coverage. Automated hermes-sweeper review found the reported pre-session wedge already addressed on current main:
The proposed outer watchdog is therefore redundant with the current-main behavior for this reconnect failure mode. |
13 tasks
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
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.
What does this PR do?
Prevents a long-lived MCP server task from hanging forever when a fresh transport wedges before publishing
server.session.The reconnect loop previously awaited
_run_http()/_run_stdio()directly. If SDK, OAuth, or transport setup blocked without returning or raising, the loop never advanced past reconnect attempt 1; later reconnect signals had no effect and only a gateway restart recovered the server.This change adds a setup-only watchdog:
self.sessionis published is bounded byconnect_timeout;"recycle"pass through unchanged.This is complementary to #63495, which fixes the known OAuth inner-generator leak behind #38193. That root fix prevents one concrete deadlock; this PR keeps any future SDK/OAuth/transport setup wedge from freezing Hermes' entire reconnect state machine.
Related Issue
Related: #38193, #31987, #49543
Complementary to: #63495
Type of Change
Changes Made
tools/mcp_tool.py_run_transport_with_connect_timeout();tests/tools/test_mcp_reconnect_signal.pyHow to Test
Run the MCP tool suite:
python -m pytest tests/tools/test_mcp_reconnect_signal.py tests/tools/test_mcp*.py -qResult: 582 passed, 4 pre-existing unknown-mark warnings.
Run static and cross-platform checks:
Result: all pass.
Production validation: on macOS arm64 / Python 3.11.15, an OAuth-backed MCP transport emitted the AnyIO lock-cleanup error and then logged only reconnect attempt 1 indefinitely. With the setup watchdog loaded, the gateway restarted cleanly and the live Discord session completed MCP format-guide, search, and memo-by-ID calls directly without a fresh-process fallback.
Checklist
Code
pytest tests/ -qsuite — the complete MCP tool suite passesDocumentation & Housekeeping
cli-config.yaml.exampleupdate — N/A; no config keys addedCONTRIBUTING.md/AGENTS.mdupdate — N/AScreenshots / Logs
Observed failure signature:
No attempt 2 or fresh-session registration followed until gateway restart.