fix(cli): wait for MCP before first-turn agent builds - #60017
Conversation
d5537f8 to
7b17581
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating a real first-turn MCP race. Current main backgrounds CLI MCP discovery at hermes_cli/main.py:12571-12584, but oneshot builds its agent at hermes_cli/oneshot.py:393; the agent snapshots schemas at agent/agent_init.py:1187-1199.
Problems
hermes_cli/oneshot.py:389always joins when any MCP server is configured, even when this invocation cannot expose MCP tools.hermes_cli/tools_config.py:1884-1900makesno_mcpan explicit opt-out, and explicit non-MCP--toolsetshave the same issue. A slow configured server can therefore delay a deliberately MCP-free one-shot.hermes_cli/config.py:1351changes the shared default to 15s.tui_gateway/server.py:4537-4546uses that shared wait without an override, extending the change to TUI first-agent startup as well.
Suggested changes
- Gate the one-shot wait on the effective enabled MCP server selection, including explicit toolsets,
enabled: false, andno_mcp; add skip-path coverage. - Scope or explicitly validate the global timeout increase. The timeline-linked #61944 has an effective-toolset-gated one-shot approach worth consolidating with.
- Add a temp-
HERMES_HOMEreal-path regression rather than only mocked ordering tests, consistent withAGENTS.md:84-87.
Automated hermes-sweeper review.
|
|
||
| from hermes_cli.mcp_startup import ensure_mcp_discovery_before_agent_build | ||
|
|
||
| ensure_mcp_discovery_before_agent_build( |
There was a problem hiding this comment.
This joins discovery based only on configured MCP servers, not on toolsets_list. A one-shot with explicit non-MCP toolsets, or a CLI config containing no_mcp, cannot expose MCP schemas but can now wait up to the shared bound. Gate this on the effective enabled MCP-server selection and add skip-path tests.
| # snapshot cannot be used on that only turn. ``thread.join(timeout)`` returns | ||
| # as soon as discovery completes, so reachable servers only wait for their | ||
| # real handshake time while unavailable servers remain bounded. | ||
| "mcp_discovery_timeout": 15.0, |
There was a problem hiding this comment.
This is a shared timeout: tui_gateway/server.py calls the same resolver before every TUI agent build. Please scope the longer wait to the intended first-only CLI path, or add coverage and rationale for the TUI latency change.
…teractive sessions Non-interactive sessions (hermes chat -q, hermes -z) snapshot the tool registry at AIAgent construction time. If background MCP discovery hasn't finished, MCP tools are invisible for the entire session — and unlike interactive mode, there is no between-turns late-binding refresh to recover. Root cause: wait_for_mcp_discovery() only joins an already-created discovery thread, so it no-ops if a direct/single-query path reaches agent construction before MCP startup created that thread. Oneshot._run_agent() didn't call it at all. Fix: - Add ensure_mcp_discovery_before_agent_build() helper to mcp_startup.py: idempotently starts discovery if needed + bounded wait. Fail-open on errors. - Add single_query parameter to _resolve_discovery_timeout/wait_for_mcp_discovery: uses mcp_single_query_discovery_timeout (default 15s) instead of the interactive mcp_discovery_timeout (1.5s) because one-shot sessions have no second turn to recover. - Wire into CLI _init_agent (single_query from _single_query_mode flag set in cli.py's single-query path) and oneshot._run_agent (single_query=True). - Interactive sessions unchanged: keep 1.5s bound (between-turns refresh covers). Closes #38448, #51316, #37013, #68137 Composite salvage of #60017 (chrishart0), #51322 (Bartok9), #38620 (buptwz), #43544 (halonke), #36882 (vanhoof).
…teractive sessions Non-interactive sessions (hermes chat -q, hermes -z) snapshot the tool registry at AIAgent construction time. If background MCP discovery hasn't finished, MCP tools are invisible for the entire session — and unlike interactive mode, there is no between-turns late-binding refresh to recover. Root cause: wait_for_mcp_discovery() only joins an already-created discovery thread, so it no-ops if a direct/single-query path reaches agent construction before MCP startup created that thread. Oneshot._run_agent() didn't call it at all. Fix: - Add ensure_mcp_discovery_before_agent_build() helper to mcp_startup.py: idempotently starts discovery if needed + bounded wait. Fail-open on errors. - Add single_query parameter to _resolve_discovery_timeout/wait_for_mcp_discovery: uses mcp_single_query_discovery_timeout (default 15s) instead of the interactive mcp_discovery_timeout (1.5s) because one-shot sessions have no second turn to recover. - Wire into CLI _init_agent (single_query from _single_query_mode flag set in cli.py's single-query path) and oneshot._run_agent (single_query=True). - Interactive sessions unchanged: keep 1.5s bound (between-turns refresh covers). Closes NousResearch#38448, NousResearch#51316, NousResearch#37013, NousResearch#68137 Composite salvage of NousResearch#60017 (chrishart0), NousResearch#51322 (Bartok9), NousResearch#38620 (buptwz), NousResearch#43544 (halonke), NousResearch#36882 (vanhoof).
…teractive sessions Non-interactive sessions (hermes chat -q, hermes -z) snapshot the tool registry at AIAgent construction time. If background MCP discovery hasn't finished, MCP tools are invisible for the entire session — and unlike interactive mode, there is no between-turns late-binding refresh to recover. Root cause: wait_for_mcp_discovery() only joins an already-created discovery thread, so it no-ops if a direct/single-query path reaches agent construction before MCP startup created that thread. Oneshot._run_agent() didn't call it at all. Fix: - Add ensure_mcp_discovery_before_agent_build() helper to mcp_startup.py: idempotently starts discovery if needed + bounded wait. Fail-open on errors. - Add single_query parameter to _resolve_discovery_timeout/wait_for_mcp_discovery: uses mcp_single_query_discovery_timeout (default 15s) instead of the interactive mcp_discovery_timeout (1.5s) because one-shot sessions have no second turn to recover. - Wire into CLI _init_agent (single_query from _single_query_mode flag set in cli.py's single-query path) and oneshot._run_agent (single_query=True). - Interactive sessions unchanged: keep 1.5s bound (between-turns refresh covers). Closes NousResearch#38448, NousResearch#51316, NousResearch#37013, NousResearch#68137 Composite salvage of NousResearch#60017 (chrishart0), NousResearch#51322 (Bartok9), NousResearch#38620 (buptwz), NousResearch#43544 (halonke), NousResearch#36882 (vanhoof).
What does this PR do?
Fixes a first-turn MCP readiness race for non-interactive CLI runs.
hermes chat -qand top-levelhermes -z/--oneshotcan constructAIAgentbefore configured MCP servers finish registering their dynamic tools. Because the agent snapshots its tool registry at construction time, the first and only model turn can miss nativemcp__...tools even when the MCP server itself is healthy and the profile/toolset is configured correctly.Why this matters
This is not just a missing-tool edge case.
chat -qis the natural command for scripted agent runs, evals, batch tests, kanban/worker-style invocations, and other automation where there is no second turn to recover from late MCP discovery. The failure is especially confusing because out-of-band checks can pass: a configured MCP server can test successfully, but the agent still snapshots its tools too early and never sees that server's tools for the run. When MCP tools are absent from that first snapshot, the model may silently degrade into slower or less reliable workarounds such as shell/Python calls, or conclude that required native tools are unavailable. In a real evaluation workflow this caused multiple runs to be invalid, wasted hours of debugging, and burned substantial model tokens before the root cause was visible:chat -qlooked like configuredchat, but its first-turn tool schema did not reliably include configured MCP tools.The expected contract is simple: if
hermes chatwould expose a configured MCP toolset, thenhermes chat -q "..."should expose the same tools for its first and only prompt.Fix layer
The fix adds a small shared helper that idempotently starts the existing background MCP discovery path and bounded-waits at the point of agent construction, then calls it from both CLI agent setup and oneshot. This is slightly stronger than the old
wait_for_mcp_discovery()call:wait_for_mcp_discovery()only joins an already-created discovery thread, so it no-ops if a direct/single-query path reaches agent construction before MCP startup created that thread. The new helper makes the construction site self-sufficient: start discovery if needed, then wait up to the existing bound.Putting the guarantee at the construction site keeps the fix narrow: it composes existing MCP startup primitives, is a no-op if discovery already ran, remains bounded by
mcp_discovery_timeout, skips work when no MCP servers are configured, and stays fail-open on errors.Real-world validation showed that the previous 1.5-second default was too short for an otherwise healthy remote HTTP MCP cold handshake taking roughly 2–5 seconds. The agent still snapshotted its tools before registration completed. This PR therefore raises the default bound to 15 seconds. This is a ceiling, not a fixed sleep:
thread.join(timeout)returns immediately when discovery completes, so healthy servers incur only their actual handshake time, while unavailable servers remain capped at 15 seconds.Related Issue
Fixes #38448
Also covers the
hermes chat -qaffected path described in the issue discussion.Type of Change
Changes Made
hermes_cli/mcp_startup.py: addsensure_mcp_discovery_before_agent_build(), composing the existing background MCP discovery and bounded wait while preserving OAuth-prompt suppression and fail-open semantics.hermes_cli/cli_agent_setup_mixin.py: calls the helper before CLIAIAgent(...)construction, coveringhermes chat -qand the interactive first turn.hermes_cli/oneshot.py: calls the same helper before oneshotAIAgent(...)construction, coveringhermes -z/--oneshot.hermes_cli/config.py: raises the bounded discovery default from 1.5 to 15 seconds so ordinary remote MCP cold starts can complete before the first-only tool snapshot.tui_gateway/entry.pyandtui_gateway/server.py: keep fallback behavior and inline documentation aligned with the shared default.tests/hermes_cli/test_mcp_startup.py: adds regression coverage for the helper's real bounded discovery path, including OAuth-suppression ordering, plus ordering checks that discovery precedes agent build in both CLI and oneshot paths.Sibling commands sharing this contract
hermes chat -q, top-levelhermes -z/--oneshot, and the interactive first turn all need configured MCP tools available before the first model call. All three now route through the shared helper beforeAIAgentsnapshots tools.How to Test
scripts/run_tests.sh tests/hermes_cli/test_mcp_startup.py tests/hermes_cli/test_tui_resume_flow.py -q scripts/run_tests.sh tests/hermes_cli/test_oneshot_usage_file.py tests/agent/test_oneshot.py -q scripts/run_tests.sh tests/tools/test_refresh_agent_mcp_tools.py tests/test_tui_mcp_late_refresh.py tests/tui_gateway/test_wait_for_mcp_discovery.py tests/tui_gateway/test_mcp_late_refresh_thread_owner.py -q python -m pytest tests/test_tui_gateway_server.py::test_make_agent_waits_for_shared_mcp_discovery -q -o 'addopts=' git diff --check python -m compileall hermes_cli/config.py hermes_cli/mcp_startup.py hermes_cli/cli_agent_setup_mixin.py hermes_cli/oneshot.py tui_gateway/entry.py tui_gateway/server.pyManual end-to-end smoke against a profile with a healthy remote HTTP MCP toolset:
Verification Output
compileallandgit diff --checkpassed. The branch was rebased onto current upstreammainbefore final verification.