Skip to content

fix(mcp): reuse stdio watcher coroutine - #98088

Closed
Serph91P wants to merge 1 commit into
NousResearch:mainfrom
Serph91P:fix/mcp-warning-cleanup
Closed

Serph91P wants to merge 1 commit into
NousResearch:mainfrom
Serph91P:fix/mcp-warning-cleanup

Conversation

@Serph91P

@Serph91P Serph91P commented Aug 29, 2026

Copy link
Copy Markdown

Summary

  • create the stdio child-watcher coroutine exactly once
  • reuse that coroutine for awaitability validation and task scheduling
  • prevent the discarded probe coroutine from emitting RuntimeWarning: coroutine ... was never awaited
  • assert in the reconnect regression test that the watcher is constructed once

The MCP-prefixed pre-discovery validation changes were removed because they are already covered more broadly by #44044.

Verification

  • uv run pytest -q tests/tools/test_mcp_stdio_fastfail_reconnect.py (2 passed)
  • uv run pytest -q tests/tools/test_mcp*.py (596 passed)
  • uv run ruff check tools/mcp_tool.py tests/tools/test_mcp_stdio_fastfail_reconnect.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/mcp MCP client and OAuth area/config Config system, migrations, profiles duplicate This issue or pull request already exists labels Aug 29, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #44044 for the configured mcp-<server> pre-discovery validation repair; #44044 is the broader open implementation across startup validators. This PR also carries separate stdio watcher-warning cleanup, which should be preserved if work is consolidated.

@Serph91P
Serph91P force-pushed the fix/mcp-warning-cleanup branch from 296ae8b to 4d507dd Compare August 29, 2026 19:59
@Serph91P Serph91P changed the title fix(mcp): eliminate startup and coroutine warnings fix(mcp): reuse stdio watcher coroutine Aug 29, 2026
@Serph91P

Copy link
Copy Markdown
Author

Thanks for flagging this. I removed the duplicate MCP-prefixed validation changes. This PR now contains only the independent stdio watcher coroutine cleanup; #44044 remains the implementation for pre-discovery validation.

@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Closing with credit — the leak this fixes landed on main via #100920 (fe3e5dc), carrying @loulanyue's earlier #96044 fix (Aug 27) with a different shape: the probe now uses inspect.iscoroutinefunction(_watch_children) instead of creating the coroutine once and reusing it. Same bug, same outcome (0 leaked coroutines, verified live). Thanks @Serph91P for the independent fix.

@teknium1 teknium1 closed this Sep 2, 2026
@Serph91P
Serph91P deleted the fix/mcp-warning-cleanup branch September 2, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists 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.

3 participants