fix(tools): skip MCP keepalive during in-flight calls + fail orphaned calls on reconnect

[arminanton]

What does this PR do?

Stops the MCP idle keepalive from racing an in-flight tool call, and fails orphaned calls cleanly when the session is torn down. Together these close a failure mode where a single MCP tool call could hang the agent for up to tool_timeout (hours).

The bug

An MCP stdio session is a single JSON-RPC stream. The idle keepalive (list_tools / send_ping) runs on a timer independent of tool dispatch, so it can fire while a call_tool is in flight. The concurrent request wedges the stream, the in-flight call times out, that timeout triggers a reconnect, and the MCP SDK does not always fail the pending call_tool when its streams close — so its run_coroutine_threadsafe future never resolves and the calling agent thread polls to the full tool_timeout (up to hours). The user sees the agent "hang" on a tool that the server may have actually answered.

Relationship to existing code (what's already here vs. the remaining gap)

The current tree already serializes client-initiated RPCs via self._rpc_lock (tools/mcp_tool.py:1193, with the rationale comment at :1187-1192 describing exactly this "list_tools while a normal tool call is in flight" wedge). That lock is applied around the call_tool dispatch path.

But the keepalive loop itself does not participate in that protection. On current main:

• the keepalive's list_tools() (tools/mcp_tool.py:1398) and send_ping() (:1403) are not taken under _rpc_lock, so the keepalive can still issue a concurrent request on the shared stream while a call_tool runs;
• nothing skips the keepalive cycle when a call is active; and
• on reconnect/shutdown nothing cancels a pending call_tool, so it can be orphaned (the hang above).

This PR closes that remaining gap by extending the existing _rpc_lock discipline to the keepalive and adding orphan-cancellation on teardown. It builds on the _rpc_lock groundwork rather than duplicating it.

The fix

1. Track in-flight calls: new _inflight_tasks set and a _reconnecting flag on MCPServerTask.
2. The keepalive skips a cycle when a call is in flight (self._rpc_lock.locked() or self._inflight_tasks) — a server actively serving a call is provably alive — and otherwise runs the list_tools/send_ping probe under _rpc_lock, so it can never overlap a call_tool.
3. New _fail_inflight_calls() cancels pending call tasks on reconnect/shutdown instead of orphaning them.
4. _call() registers itself in _inflight_tasks and, when a deliberate teardown cancels it (_reconnecting), converts the CancelledError into a clean, retryable RuntimeError ("reconnected during the call; retry the tool") so the agent re-runs the tool on the freshly rebuilt session (self-healing) instead of hanging.

Related Issue

Related to #30268 ("All connected MCP servers fail keepalive simultaneously after Mac sleep/wake or network blip"). This PR doesn't fully fix the sleep/wake storm, but it partially mitigates it: skipping the keepalive while calls are active reduces spurious reconnect churn, and cancelling orphaned calls on reconnect means a post-wake reconnect no longer leaves a call hanging to tool_timeout.

Complementary to (not overlapping) the other open MCP-keepalive PRs:

• fix(mcp): stagger idle keepalive probes #30694 ("stagger idle keepalive probes") staggers probes across servers to avoid a thundering herd — orthogonal to this intra-server in-flight race.
• feat(mcp): allow configurable keepalive interval #26493 ("configurable keepalive interval") is a tuning knob and doesn't touch the race.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/mcp_tool.py:
  • MCPServerTask.__slots__ / __init__: add _inflight_tasks (set) and _reconnecting (bool).
  • keepalive loop: skip the cycle when a call is in flight; run the list_tools/send_ping probe under _rpc_lock.
  • new _fail_inflight_calls(reason) called on both the shutdown and reconnect exits.
  • the call_tool _call() coroutine: register/discard the task in _inflight_tasks and convert a deliberate-teardown CancelledError into a retryable RuntimeError.
• tests/tools/test_mcp_keepalive_inflight_race.py: 5 new tests (empty initial state; _fail_inflight_calls no-op when idle; cancels pending + flags teardown; add/discard bookkeeping; the _reconnecting flag distinguishing a deliberate teardown).

How to Test

1. With an MCP stdio server configured, start a long-running tool call.
2. Let the keepalive interval (180s) elapse during the call.
3. Before: the keepalive's list_tools/send_ping can wedge the stream → the call times out → reconnect → the call is orphaned and the agent hangs to tool_timeout. After: the keepalive is skipped while the call is active; if a reconnect does occur, the call is cancelled and surfaces a clean "retry the tool" error.

pytest tests/tools/test_mcp_keepalive_inflight_race.py -q

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(tools):)
• I searched for existing PRs to make sure this isn't a duplicate (reviewed fix(mcp): stagger idle keepalive probes #30694, feat(mcp): allow configurable keepalive interval #26493, test: add coverage for mcp keepalive failure #37740 — all keepalive-adjacent but none fix the in-flight race; this builds on the existing _rpc_lock at mcp_tool.py:1193)
• My PR contains only changes related to this fix (no unrelated commits)
• I've run pytest tests/ -q and all tests pass (5 new + 238 existing MCP tests across cancel/reconnect/stability/capability/tool suites, no regression)
• I've added tests for my changes
• I've tested on my platform: Ubuntu Linux (Python 3.11)

Documentation & Housekeeping

• I've updated relevant documentation (inline comments + the _fail_inflight_calls docstring explaining the orphan-hang mechanism) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config keys)
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) — N/A (pure asyncio, no OS primitives; passes the Windows-footgun check)
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

---

⚠️ Apply-time conflict note (operator-acknowledged)

When cherry-picking/rebasing this PR onto v0.17.0 (2bd1977d8), expect one conflict in tools/mcp_tool.py. This is EXPECTED and documented: origin/main commit 40722058e refactored our inline keepalive into self._keepalive_probe(), so the same region is touched on both sides.

Resolution (keep-both): retain v0.17.0's _keepalive_probe() helper structure AND this PR's _inflight_tasks skip-guard + orphan-fail-on-reconnect logic. The load-bearing guard (if self._rpc_lock.locked() or self._inflight_tasks: continue) sits just above the conflict and merges clean. A ready-to-pull pre-resolved branch is published at forward-compat/48069-on-v0.17.0 on the fork (0 conflict markers, subsystem tests green). Full recipe: #50111:verification/ + the drifted-PR reference.

[arminanton]

PR CAMPAIGN — INDEPENDENT CHERRY-PICK RE-APPLICATION VERDICT (correction, 2026-06-21)

This SUPERSEDES the §2 "36/37 clean" claim from the earlier git-apply proof. An independent
cherry-pick (real commit-object 3-way merge, NOT git apply of patch text) onto v0.17.0
(2bd1977) gives a STRICTER and more honest verdict. git apply --3way was too lenient and
MASKED one real conflict (#48069). Method: per-PR isolated cherry-pick onto a clean v0.17
worktree (proof_cp_isolated.sh).

TRUE ISOLATED VERDICT vs v0.17.0: 35 CLEAN / 2 CONFLICT (of 37)

CONFLICT #1 — #50056 (sqlite-driver): tests/hermes_cli/test_kanban_db.py
TRIVIAL: a single import line (v0.17 import sqlite3 vs PR import subprocess).
Resolution = keep both. 512 tests pass on the merged tree.

CONFLICT #2 — #48069 (mcp-keepalive-inflight-race): tools/mcp_tool.py [SUBSTANTIVE — was hidden by git-apply]
REAL semantic overlap: v0.17 added _pending_call_context (contextvars snapshot for
elicitation gateway-routing) in the SAME 4 regions the PR rewrites for _inflight_tasks/
_reconnecting keepalive-suppression: slots, init, the keepalive probe, and _call().
RESOLUTION = keep BOTH features (verified semantically compatible):
- slots/init: both _pending_call_context AND _inflight_tasks+_reconnecting.
- keepalive probe: keep v0.17's _keepalive_probe() helper (a strict superset of the PR's
inlined send_ping/list_tools — the PR's real value, the if _rpc_lock.locked() or _inflight_tasks: continue suppression guard, already merged cleanly at the call site).
- _call(): inflight-task add/discard + CancelledError->retryable wraps v0.17's
_pending_call_context snapshot inside _rpc_lock around session.call_tool().
VERIFIED: 0 conflict markers, AST parses, and pytest tests/tools/test_mcp_tool.py tests/tools/test_mcp_keepalive_inflight_race.py => 205 passed on the v0.17-merged tree
(includes the PR's own in-flight-race regression test).

HONEST IMPLICATION

"Re-appliable onto v0.17" is TRUE but NOT "trivial for all": 35 PRs apply with zero edits,
#50056 needs a 1-line import merge, and #48069 needs a real (but mechanical, ~4-region,
keep-both) semantic merge in mcp_tool.py. Both conflicts are RESOLVED and TEST-VERIFIED here.
The PRs themselves target current main (where they were built and apply clean); these
conflicts only arise when re-pulling specifically onto the OLDER v0.17 snapshot, which is the
expected forward-drift a re-application manifest exists to absorb.

STILL OUTSIDE AGENT CONTROL

CI: all 37 fork PRs = "no checks reported" (NousResearch holds fork-PR workflow runs pending
maintainer approval — verified from repo workflow config). The cherry-pick build+test above is
the strongest INDEPENDENT verification available without a maintainer triggering CI.

Reproduce: bash /mnt/devvm/custom/tmp/proof_cp_isolated.sh (per-PR verdict);
then cherry-pick #48069 onto v0.17, resolve the 4 regions keep-both, pytest tests/tools/test_mcp*.

[arminanton]

Forward-compat: pulling this PR onto v0.17.0 (2bd1977)

This PR targets main and applies cleanly there. Re-pulling it onto the older v0.17.0 snapshot has a substantive conflict in tools/mcp_tool.py: v0.17.0 independently added _pending_call_context (a contextvars snapshot for elicitation gateway-routing) in the same 4 regions this PR rewrites for _inflight_tasks/_reconnecting (keepalive-suppression) — __slots__, __init__, the keepalive probe, and _call(). The two features are independent; the resolution is keep both (inflight-task add/discard + CancelledError→retryable wraps the _pending_call_context snapshot inside _rpc_lock; keep v0.17's _keepalive_probe() helper, a superset of this PR's inlined probe).

A ready-to-pull branch with the keep-both resolution committed is published:
arminanton/hermes-agent : forward-compat/48069-on-v0.17.0 (@ 69a42e10d). Verified: pytest tests/tools/test_mcp_tool.py tests/tools/test_mcp_keepalive_inflight_race.py → 205 passed.

Note for re-appliers: this PR's branch head (71cdbfa3a) is a merge commit (origin/main was merged in); the actual change is the single fix commit ccc162f4e — cherry-pick that, not the full range, to avoid replaying upstream drift.

[0xquinto]

Thanks for documenting the in-flight keepalive race and the broader orphan-call failure mode. I reproduced the same stream wedge on current main.

The current #48069 head is 2,923 commits behind and git merge-tree reports a content conflict in tools/mcp_tool.py, so I opened #62811 as a narrow current-main salvage of only the keepalive serialization invariant, with explicit credit here and without duplicating the reconnect/orphan-cancellation portion.

The direct regression was RED→GREEN; 265 targeted MCP tests, including both lock orderings, ruff, the Windows-footgun scan, and a fresh-process real completion canary pass.

[teknium1]

Thanks for documenting and isolating the MCP stream race. The premise remains present on current main: the lifecycle loop invokes _keepalive_probe() without _rpc_lock (tools/mcp_tool.py:1939-1942), while tool calls acquire that lock (tools/mcp_tool.py:3934-3945).

Problems

• The added _inflight_tasks tracking is limited to session.call_tool(), but resource/prompt handlers use the same background-loop and RPC-lock pattern at tools/mcp_tool.py:4055-4268. They are not cancelled during the proposed reconnect/shutdown teardown.
• tests/tools/test_mcp_keepalive_inflight_race.py tests synthetic task-set bookkeeping only. It does not drive _wait_for_lifecycle_event() or verify either RPC/keepalive lock ordering.

Suggested changes

• Track/cancel every user-visible MCP request coroutine, with reconnect and shutdown handler-path coverage.
• Rework the serialization against current main's _keepalive_probe() helper and add direct ordering coverage. The linked fix(mcp): serialize keepalive with active RPCs #62811 already isolates that current-main serialization work.

This is an automated hermes-sweeper review.