fix: wait for MCP discovery in one-shot mode before building agent (#68137) - #71277
Closed
rkfshakti wants to merge 1 commit into
Closed
fix: wait for MCP discovery in one-shot mode before building agent (#68137)#71277rkfshakti wants to merge 1 commit into
rkfshakti wants to merge 1 commit into
Conversation
…ousResearch#68137) hermes -z (one-shot) snapshots the tool registry before background MCP discovery finishes. Fast servers (e.g. compiled binaries) usually make it in; slower stdio servers (Python-based MCP that takes 2-12s to boot) are silently dropped — the agent runs without their tools, with no error anywhere. Since the model never sees the tools, it sometimes fabricates a plausible answer as if it had called the tool. The interactive path (cli_agent_setup_mixin.py) already calls wait_for_mcp_discovery() before constructing the agent. The one-shot path (_run_agent in hermes_cli/oneshot.py) never called it. Fix: call wait_for_mcp_discovery() in _run_agent before constructing AIAgent, bounded by mcp_discovery_timeout so a dead server can't hang the one-shot. The wait is guarded with try/except so a discovery failure can't crash the one-shot run itself — mirroring the interactive path's resilience. Regression tests (tests/hermes_cli/test_oneshot_mcp_discovery.py): - test_waits_before_building_agent: records call order and asserts the discovery wait happens before AIAgent construction (fails on pre-fix code where the agent is built with no wait). - test_proceeds_when_discovery_wait_raises: asserts the agent is still built and a result returned when wait_for_mcp_discovery raises.
Contributor
Author
|
Reopening this fix as a fresh PR (the earlier #68168 was closed as a duplicate of #68162, but that one has stalled with no review activity and the bug is still live on I've rebased on current Can you review once again? |
Collaborator
Contributor
Author
|
Thanks for the triage note — #68162 is the earlier fix for the same issue. Closing this in favor of the original PR. Apologies for the duplicate. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
hermes -z(one-shot) snapshots the tool registry before background MCP discovery finishes. Fast servers (e.g. a compiled binary likemcp-grafana) usually make it in; slower stdio servers (a Python-based MCP that takes 2-12s to import/boot) are silently dropped — the agent runs without their tools, with no error anywhere.Worse: since the model never sees the tools, it sometimes refuses the task and sometimes fabricates a plausible answer as if it had called the tool.
Root cause
hermes_cli/main.py:_should_background_mcp_startupreturnsTrueforcommand in {None, "chat", "rl"}→ discovery runs in thecli-mcp-discoverybackground thread.hermes_cli/oneshot.py:run_oneshot/_run_agentbuildAIAgentdirectly and never callhermes_cli.mcp_startup.wait_for_mcp_discovery()(unlike the interactive path incli_agent_setup_mixin.py, which does).Fix
Call
wait_for_mcp_discovery()in_run_agentbefore constructingAIAgent, bounded bymcp_discovery_timeoutso a dead server can't hang the one-shot. The wait is guarded withtry/exceptso a discovery failure can't crash the one-shot run itself — mirroring the interactive path's resilience.This mirrors the interactive path at
cli_agent_setup_mixin.py.Tests
Added
tests/hermes_cli/test_oneshot_mcp_discovery.pywith 2 test cases:test_waits_before_building_agent— records call order and assertswait_for_mcp_discoveryis called beforeAIAgentconstruction. Fails on pre-fix code (agent built with no wait).test_proceeds_when_discovery_wait_raises— asserts the agent is still built and a result returned whenwait_for_mcp_discoveryraises, proving the guard works.I verified both tests fail on the pre-fix code and pass with the fix. Ruff is clean on the changed files.
Fixes #68137