Skip to content

fix(mcp): prevent tool registration race condition on startup - #25622

Closed
vikarag wants to merge 2 commits into
NousResearch:mainfrom
vikarag:fix-mcp-race
Closed

fix(mcp): prevent tool registration race condition on startup#25622
vikarag wants to merge 2 commits into
NousResearch:mainfrom
vikarag:fix-mcp-race

Conversation

@vikarag

@vikarag vikarag commented May 14, 2026

Copy link
Copy Markdown

Root Cause

When an agent starts via CLI, is called. In , the flag was firing immediately after discovery, but before actually populated the ToolRegistry.

Because fetches the definitions immediately upon instantiation, the API payload was receiving because the servers had connected but had not registered their endpoints.

Fix

  1. Moved directly into the loop, executed immediately before .
  2. Added logic inside so that any detached instances or fast CLI calls properly block until MCP servers are fully registered.
  3. If an agent (e.g., cron background task) executes but discovery was never started, now synchronously invokes first.

This closes the gap ensuring the model never generates text-only responses missing tools.

vikarag and others added 2 commits May 14, 2026 17:44
Move _register_server_tools() call from _discover_and_register_server()
into MCPServerTask._register_tools(), invoked inside all four transport
paths (stdio, SSE, new HTTP, legacy HTTP) before _ready.set(). This
ensures the ToolRegistry is populated before start() returns, closing
the window where get_tool_definitions() could see an empty registry.

Also adds wait_for_mcp_ready() as defense-in-depth: get_tool_definitions()
blocks up to 30s for MCP discovery to finish if it's in progress.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround tool/mcp MCP client and OAuth comp/tools Tool registry, model_tools, toolsets comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

This looks implemented on current main by the later MCP startup/snapshot-refresh work. Automated hermes-sweeper review.

Evidence:

  • hermes_cli/mcp_startup.py:87 now waits for the background MCP discovery thread before the first tool snapshot, bounded by config so startup does not hang.
  • hermes_cli/cli_agent_setup_mixin.py:237 calls that wait before CLI agent construction.
  • tui_gateway/server.py:4056 adds a late MCP refresh that rebuilds the agent's tool snapshot and re-emits session info if discovery lands before the first turn.
  • agent/turn_context.py:176 refreshes registered MCP tools in the per-turn prologue before the next API call assembles tools=.
  • tools/mcp_tool.py:4535 centralizes the cache-aware snapshot rebuild in refresh_agent_mcp_tools().
  • The cron/detached-agent case mentioned in the PR body is covered by cron/scheduler.py:2455, which explicitly runs MCP discovery before building the cron job agent.
  • The hardening series is on main and shipped in v2026.6.19; the final hardening commit I found is 88d523220fddcfb42bd4f29e9ace4ae30ebbf1d9.

The exact _ready.set() ordering change from this PR was not adopted, but the behavioral guarantee it was aiming for — MCP tools not being silently absent from the agent payload because discovery/registration raced startup — is now covered on main.

@teknium1 teknium1 closed this Jun 29, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/tools Tool registry, model_tools, toolsets P1 High — major feature broken, no workaround sweeper:implemented-on-main Sweeper: behavior already present on current main 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