Skip to content

fix: reconnect stale MCP sessions before retry - #27052

Closed
setclock wants to merge 1 commit into
NousResearch:mainfrom
setclock:fix/too-60-hindsight-mcp-reconnect
Closed

fix: reconnect stale MCP sessions before retry#27052
setclock wants to merge 1 commit into
NousResearch:mainfrom
setclock:fix/too-60-hindsight-mcp-reconnect

Conversation

@setclock

Copy link
Copy Markdown

Summary

Fixes stale MCP HTTP/stream session recovery for long-lived Hermes sessions. When an MCP server reports a session-expired / auth-recovery style failure, the retry path now clears readiness, signals reconnect, waits for a distinct fresh session, and avoids counting short reconnect windows as circuit-breaker failures.

This specifically addresses gateway/live-session cases where a fresh CLI hermes mcp test <server> succeeds while a long-lived chat process keeps retrying against a stale MCP session and surfaces errors such as Session terminated, not connected, or unreachable after consecutive failures.

Changes

  • Clear MCP server readiness during reconnect.
  • Add helper flow that waits for a distinct fresh session before retrying session-expired/auth-recovery paths.
  • Avoid circuit-breaker failure accounting during expected reconnect wait windows.
  • Add regression coverage proving stale sessions are not reused.
  • Update circuit-breaker test stubs to model readiness/fresh-session reconnect behavior.

Verification

Local verification already run before opening this PR:

python -m pytest tests/tools/test_mcp_tool_session_expired.py tests/tools/test_mcp_circuit_breaker.py tests/tools/test_mcp_reconnect_signal.py tests/tools/test_mcp_stability.py tests/tools/test_mcp_tool.py -q
# 235 passed

git diff --check -- tools/mcp_tool.py tests/tools/test_mcp_tool_session_expired.py tests/tools/test_mcp_circuit_breaker.py
# clean

hermes -p toolshantypm mcp test hindsight
# connected, tools discovered

hermes -p toolshantypm memory status
# Hindsight provider available

Post-gateway-restart live smoke from the affected Telegram profile also passed:

  • mcp_hindsight_get_bank succeeded.
  • mcp_hindsight_recall succeeded.
  • mcp_hindsight_list_operations succeeded.

Risk

Medium-low. This changes reconnect/retry behavior for MCP transports, but the patch is covered by focused stale-session and circuit-breaker regression tests. The intended behavior is only to force a genuinely fresh session before retrying after session-expired/auth-recovery failures, rather than reusing a stale one.

@cardtest15-coder

This comment was marked as spam.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth labels May 16, 2026
@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused MCP recovery fix. I agree the premise still exists on current main: _handle_session_expired_and_retry signals reconnect and then accepts any srv.session is not None and srv._ready.is_set() at tools/mcp_tool.py:2414-2419, while MCPServerTask.run() keeps _ready set across reconnects at tools/mcp_tool.py:1933-1939.

Problems

  • The PR clears _ready in the reconnect branch, but current main also uses _ready as the “already connected once” sentinel to skip Streamable HTTP preflight on reconnect at tools/mcp_tool.py:1891-1896. That invariant is covered by tests/tools/test_mcp_tool.py:1833-1840, so applying this as-is would regress the newer preflight-skip behavior.

Suggested changes

  • Keep the distinct-session wait, but avoid overloading _ready.clear() for reconnect freshness. A separate reconnect/session-generation signal, or a preflight guard that distinguishes initial connect from reconnect, would preserve both behaviors.
  • Add/update regression coverage so stale-session reconnect waits for a fresh session and the preflight-skip test remains true.

This is an automated hermes-sweeper review.

@teknium1

teknium1 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Salvaged into PR #59331 (merged) — your commit was cherry-picked onto current main with your authorship preserved in git log (43a4256). Your diagnosis was the only one in this cluster that survived the recent reconnect-resilience overhaul: _ready staying set across reconnects let the session-expired retry race into the stale session. Your _signal_reconnect_and_wait (clear readiness first, wait for a distinct session object) landed along with your tests. Thanks!

@teknium1 teknium1 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets 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