fix(cli): wait for background MCP discovery before building the one-shot agent (#68137) - #68162
fix(cli): wait for background MCP discovery before building the one-shot agent (#68137)#68162PRATHAMESH75 wants to merge 1 commit into
Conversation
…hot agent CLI startup kicks MCP tool discovery onto the cli-mcp-discovery background thread for the one-shot command, but hermes_cli/oneshot.py:_run_agent built AIAgent — which snapshots the tool registry at construction — without joining that thread. Fast MCP servers usually made it in; slower stdio servers (a Python module that takes several seconds to import/boot) were silently dropped, so the model ran without their tools and would sometimes refuse or fabricate. Mirror the interactive path (cli_agent_setup_mixin) by calling wait_for_mcp_discovery() before constructing the agent. It is a no-op when no discovery is in flight and is bounded by mcp_discovery_timeout, so a dead server still can't hang the one-shot. Fixes NousResearch#68137
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the direct one-shot construction path. Current main still has the pre-snapshot gap: hermes_cli/main.py:10376-10435 backgrounds discovery, while hermes_cli/oneshot.py:394-410 reaches AIAgent without a wait; agent/agent_init.py:1393-1397 then snapshots tools.
Problems
- The new call remains bounded by
mcp_discovery_timeout(hermes_cli/mcp_startup.py:151-164), whose default is 1.5 seconds (hermes_cli/config_defaults.py:433-446). For a one-shot, a server completing after that bound cannot arrive through the regular next-turn refresh (agent/turn_context.py:399-425), so the reported multi-second startup case remains unresolved.
Suggested changes
- Define a one-shot completion policy that covers slow configured MCP servers, or narrow the scope to the configured bounded-wait behavior and test both sides of the timeout.
- Add a delayed-discovery behavioral test asserting the constructed agent receives the registered MCP schema.
This is an automated hermes-sweeper review.
| from hermes_cli.mcp_startup import wait_for_mcp_discovery | ||
|
|
||
| wait_for_mcp_discovery() | ||
| except Exception: |
There was a problem hiding this comment.
wait_for_mcp_discovery() is bounded by the 1.5-second default. Because one-shot has no later user turn for agent/turn_context.py's between-turn refresh, a configured MCP server completing after that bound is still absent from this run. Please either make the one-shot policy sufficient for that case or narrow and test the stated guarantee.
|
Closing as superseded. Upstream commit Its approach is also strictly broader than mine: this PR called Thanks @kshitijk4poor — closing in favor of the merged fix. |
What does this PR do?
hermes -z(one-shot) snapshots the tool registry before background MCPdiscovery finishes. CLI startup kicks MCP discovery onto the
cli-mcp-discoverybackground thread for the one-shot command(
_should_background_mcp_startup), buthermes_cli/oneshot.py:_run_agentbuilt
AIAgent— which snapshots the tool registry at construction — withoutever joining that thread.
Fast servers (a compiled binary like
mcp-grafana) usually make it in; slowerstdio servers (a Python-based MCP that takes several seconds to import/boot)
are silently dropped — the agent runs without their tools, with no error
anywhere. 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.
The interactive path already waits (
cli_agent_setup_mixincallswait_for_mcp_discovery()); the one-shot path just never got the sametreatment. This PR adds that wait before constructing the agent.
Related Issue
Fixes #68137
Type of Change
Changes Made
hermes_cli/oneshot.py— in_run_agent, callwait_for_mcp_discovery()before buildingAIAgent. The wait is a no-opwhen no discovery thread is in flight and is bounded by
mcp_discovery_timeout, so a dead server can't hang the one-shot; failuresare swallowed (debug-logged) so they never abort the run.
tests/hermes_cli/test_oneshot_mcp_wait.py— new regression tests: the waitis invoked before the agent is constructed, and a wait that raises does
not break the one-shot run.
How to Test
Both tests pass. Verified the ordering test genuinely fails without the
source change (stashed the fix →
test_waits_before_building_agentfails).Preflight (windows-footguns, ruff, affected tests) is green.
Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ANotes
The issue also flags a related quirk: under
-p <profile>,load_config()resolves the profile config, so a globally-set
mcp_discovery_timeoutmaynot be honored by profile runs. That is a separate config-resolution concern
and is intentionally left out of this focused fix.