Fix MCP shutdown after timed-out agent runs

[kamilnowak05]

Summary

• start the MCP event loop inside its worker thread and wait until it is ready before scheduling work
• cancel and drain pending MCP-loop tasks before stopping/closing the loop, preventing destroyed pending subprocess pipe tasks after timeouts
• harden hosted MCP OAuth handling by normalizing confidential client metadata and serializing auth flows across providers

Root cause

A Linear-triggered Hermes agent run hit repeated non-streaming model-call timeouts. During process cleanup, shutdown_mcp_servers() stopped and closed the MCP background loop while stdio MCP subprocess startup/reconnect tasks were still pending. That left asyncio subprocess pipe coroutines half-open and produced the observed BaseSubprocessTransport._connect_pipes / coroutine ignored GeneratorExit warnings.

Tests

• pytest tests/tools/test_mcp_tool.py::TestShutdown tests/tools/test_mcp_oauth_manager.py -q — 14 passed
• pytest tests/tools/test_mcp_tool.py::TestShutdown::test_shutdown_cancels_pending_loop_tasks_before_closing_loop -q — 1 passed
• pytest tests/tools/test_mcp_tool.py -q — 184 passed
• pytest tests/tools/test_mcp_probe.py tests/tools/test_mcp_oauth.py tests/tools/test_mcp_oauth_manager.py tests/tools/test_mcp_oauth_cold_load_expiry.py tests/tools/test_mcp_oauth_metadata.py tests/tools/test_mcp_oauth_bidirectional.py -q — 80 passed, 1 existing warning
• python -m py_compile tools/mcp_tool.py tools/mcp_oauth.py tools/mcp_oauth_manager.py tests/tools/test_mcp_tool.py tests/tools/test_mcp_oauth_manager.py
• git diff --check

Full suite note: pytest -q was also run and reached completion, but the existing repository-wide suite currently reports unrelated failures outside this change area: 100 failed, 22242 passed, 77 skipped, 214 warnings.

[alt-glitch]

Resubmission of closed #25625 by the same author. Related to #20269 (MCP multi-server coroutine timeout causes event loop starvation) and #6665 (cancel timed-out coroutines).

[teknium1]

Thanks for the detailed MCP lifecycle investigation. The shutdown implementation needs a current-main adaptation rather than a direct salvage.

Problems

• Current main already tracks the relevant server task: MCPServerTask.start() cancels its detached run() task on caller cancellation (tools/mcp_tool.py:2879-2894), while shutdown() waits for/cancels/awaits that task and drains tracked refresh work (:2898-2924). Commit 4638f3b43 also bounds the abandoned-start class across HTTP transports.
• The new shutdown test creates an arbitrary unowned task; it does not exercise the stdio startup/transport path described in the report.
• The separate OAuth changes are not behaviorally covered: the added test only compares lock identity, and no added test covers confidential-client metadata normalization.

Suggested changes

• Reproduce any remaining lifecycle leak on current main and test that real owned task path; do not transplant a blanket asyncio.all_tasks() cancellation sweep.
• Split and test the OAuth behavior independently, including concurrent flow serialization and read/write normalization.

Automated hermes-sweeper review.

[teknium1]

This only proves that a manually-created, unowned loop task is cancelled. Please exercise the reported MCPServerTask stdio startup/transport path instead; current main now owns and cancels that task class through MCPServerTask.start()/shutdown().

[teknium1]

This asserts shared lock identity but never drives two async_auth_flow generators. Add a deterministic concurrent-flow test that proves serialization while preserving bidirectional response forwarding.

[GottZ]

This was generated by AI during triage.

Summary

Two PRs address the reported MCP cleanup failure, and both contain the same combined diff: background-loop startup and blanket pending-task cancellation for the shutdown symptom, plus separate OAuth serialization and confidential-client metadata normalization changes. Neither PR has a recorded Verify verdict; the visible keep_open decision on #25652 is a PR review verdict.

Related pull requests

• #25625 [closed] duplicate — (+139/-39) — superseded duplicate: This closed PR remains relevant as the original submission of the identical lifecycle and OAuth changes, but it was resubmitted as #25652. Its shutdown test covers an arbitrary unowned task rather than the reported stdio startup/transport path.
• #25652 related — (+139/-39) — keep open with a salvage path: The diff attempts to prevent timeout cleanup warnings by starting the event loop in its worker thread and cancelling every pending loop task, while also adding unrelated OAuth serialization and metadata normalization. Consistent with the keep_open PR review on #25652, current main already tracks and drains owned server work at tools/mcp_tool.py:2879-2924, with commit 4638f3b covering abandoned HTTP starts; the remaining lifecycle bug therefore needs a current-main reproduction on the real owned stdio path, and the OAuth behavior needs independent behavioral tests.

Duplicates

#25625 and #25652 contain the same +139/-39 change set; #25625 is the closed original submission and #25652 is its resubmission.

Suggested consolidation

Keep #25652 open with a salvage path, and treat closed #25625 as superseded by duplicate #25652. For #25652, replace the blanket asyncio.all_tasks() cancellation with a current-main fix demonstrated against the real owned stdio startup/transport path, then split the OAuth serialization and confidential-client metadata normalization into independently tested changes, including concurrent-flow behavior and metadata normalization.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup25625 ["PRs duplicating each other"]
        P25625["PR #25625 (closed)"]
        P25652["PR #25652 (open)"]
    end
    class P25625 closed
    class P25652 open
    class P25652 target
    click P25625 "https://github.com/NousResearch/hermes-agent/pull/25625"
    click P25652 "https://github.com/NousResearch/hermes-agent/pull/25652"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 24 kB of PR diffs, 4 kB of issue/PR text, 1 kB of discussion (2 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.