Skip to content

fix(mcp): run MCP discovery synchronously for quiet-mode sessions - #36882

Closed
vanhoof wants to merge 2 commits into
NousResearch:mainfrom
vanhoof:fix/mcp-quiet-mode-race
Closed

fix(mcp): run MCP discovery synchronously for quiet-mode sessions#36882
vanhoof wants to merge 2 commits into
NousResearch:mainfrom
vanhoof:fix/mcp-quiet-mode-race

Conversation

@vanhoof

@vanhoof vanhoof commented Jun 1, 2026

Copy link
Copy Markdown

Problem

When chat -q "prompt" is used (kanban workers, cron jobs, scripted invocations), MCP tools from slow-starting servers are invisible to the agent.

The background MCP discovery introduced in 0c6e133 ("perf: stop eager MCP discovery from blocking agent-capable startup") uses a 750ms join timeout via wait_for_mcp_discovery(). Fast MCP servers (rover, slack, cp_strategy_coach) register in ~35ms, but slower ones like mcp-atlassian via uvx take 12+ seconds due to subprocess startup + HTTPS handshake.

get_tool_definitions() runs after the 750ms timeout expires, so only the tools from fast servers are present. Late-arriving tools are not in the direct schema and not behind tool_search -- they are completely invisible to the model.

Kanban workers are the primary victim

The kanban dispatcher spawns workers as:

hermes -p <profile> chat -q "work kanban task <id>"

Note: -q is --query (the prompt text), not -Q / --quiet (the quiet-mode flag). The existing _should_background_mcp_startup() had no check for query mode, so kanban workers always got the background optimization with the 750ms timeout -- exactly the sessions that can least afford it, since they build the tool list once at startup and never rebuild it.

Timeline from a real kanban worker session:

Time Event
11:40:56.985 cp_strategy_coach registered (5 tools)
11:40:56.990 slack registered (18 tools)
11:40:57 get_tool_definitions() runs (29 tools total)
11:41:05.533 google_workspace registered (123 tools) -- too late
11:41:08.836 mcp_atlassian registered (53 tools) -- too late

The worker could not post a Jira comment because all 53 mcp_mcp_atlassian_jira_* tools were missing from both the direct schema and tool_search.

Fix

Exempt both quiet mode (-Q / --quiet) and query mode (-q / --query) from background MCP startup. These are non-interactive sessions (kanban workers, cron jobs, scripted invocations) -- there is no user waiting at a prompt, so synchronous discovery has zero UX cost.

Interactive sessions (chat without -q or -Q) keep the background optimization.

Changes

  • hermes_cli/main.py: _should_background_mcp_startup() returns False when args.quiet or args.query is set
  • tests/hermes_cli/test_mcp_startup.py: regression test covering both -Q (quiet) and -q "prompt" (query) modes

Testing

All 5 test_mcp_startup tests pass (4 existing + 1 new with 2 sub-cases).

@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 labels Jun 1, 2026
@vanhoof
vanhoof force-pushed the fix/mcp-quiet-mode-race branch 2 times, most recently from da5d66d to 965fa4e Compare June 1, 2026 16:35
@vanhoof

vanhoof commented Jun 1, 2026

Copy link
Copy Markdown
Author

The kanban dispatcher spawns workers with -q (--query), not -Q (--quiet), they're different argparse flags. The original check only gated on args.quiet, so kanban workers and cron jobs still got the 750ms background timeout. Added args.query to the condition and split the test into two functions so each flag gets its own clean fixture lifecycle.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the -q/-Q distinction and providing focused regression coverage. The current chat -q path does retain the background-discovery race: hermes_cli/main.py:12430-12433 backgrounds all chat commands, while cli.py:16051-16243 runs a one-shot conversation and exits.

Problems

  • The new args.quiet branch also affects chat -Q without -q. -Q is output suppression, not a non-interactive-mode flag (hermes_cli/_parser.py:327-332), so this can synchronously block an interactive session.
  • Inline discovery removes the current bounded-startup protection. discover_mcp_tools() has a 120-second outer discovery bound (tools/mcp_tool.py:4963-4975), whereas wait_for_mcp_discovery() intentionally caps startup wait (hermes_cli/mcp_startup.py:87-100).
  • The proposed tests assert invocation choice only; they do not verify the real first-tool-snapshot ordering for a delayed MCP server.

Suggested changes

  • Scope the behavior to actual query mode and retain a finite deadline.
  • Add a temporary-HERMES_HOME, delayed-registration integration test proving first-query tool visibility and bounded behavior for a blocked server.

This is an automated hermes-sweeper review.

vanhoof added 2 commits July 13, 2026 18:07
When `chat -q` is used (kanban workers, cron jobs, scripted invocations),
MCP server registration must complete before `get_tool_definitions()`
builds the tool schema. The background MCP discovery introduced in
0c6e133 uses a 750ms join timeout that is too short for slow-starting
servers (e.g. mcp-atlassian via uvx takes 12+ seconds to register).

This causes a race condition where late-registering MCP tools are
invisible to the agent -- not in the direct tool schema and not behind
tool_search. The agent literally cannot call them.

The fix exempts quiet-mode sessions from background MCP startup. There
is no interactive prompt to rush to, so synchronous discovery has zero
UX cost and guarantees all configured MCP servers are registered before
the first agent turn.

Interactive sessions (chat without -q) keep the background optimization
since users benefit from the faster prompt display.

Signed-off-by: Chris van Hoof <vanhoof@ouwish.com>
…on tests

Address hermes-sweeper review feedback on PR NousResearch#36882:

- Remove args.quiet from the sync-discovery gate. -Q is output
  suppression for interactive sessions, not a non-interactive signal.
- Replace raw discover_mcp_tools() fallthrough with background
  discovery + join_mcp_discovery(timeout=30s) so a dead server
  cannot hang the process.
- Add integration tests: delayed server tools visible after startup,
  blocked server bounded by timeout cap, -Q keeps background optimization.

Signed-off-by: Chris van Hoof <vanhoof@ouwish.com>
@vanhoof
vanhoof force-pushed the fix/mcp-quiet-mode-race branch from 965fa4e to 2745c43 Compare July 13, 2026 22:17
@vanhoof

vanhoof commented Jul 13, 2026

Copy link
Copy Markdown
Author

Addressed all three points from the review:

1. -Q scoped out. args.quiet no longer triggers synchronous MCP discovery. -Q is output suppression for an otherwise interactive session, so it keeps the normal background optimization. New test (test_prepare_agent_startup_backgrounds_mcp_for_quiet_flag) confirms -Q returns fast with a background thread spawned.

2. Bounded deadline on the query-mode path. Query mode (-q "prompt") now uses start_background_mcp_discovery() + join_mcp_discovery(timeout=30) instead of raw discover_mcp_tools(). The 30s cap is a module-level constant (_QUERY_MODE_MCP_TIMEOUT) overridable in tests. A dead server cannot hang the process.

3. Delayed-registration and bounded-timeout integration tests added:

  • test_query_mode_delayed_mcp_server_tools_visible: 200ms-delayed server tools confirmed registered before _prepare_agent_startup returns.
  • test_query_mode_blocked_server_bounded_by_timeout: server that never completes, startup returns within the timeout cap, thread still alive.

Also cleaned up dead code: removed the now-redundant args.query check from _should_background_mcp_startup since the explicit query-mode branch in _prepare_agent_startup fires first.

9 tests pass (5 existing + 4 new/updated).

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 13, 2026
kshitijk4poor added a commit that referenced this pull request Aug 1, 2026
…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).
@vanhoof

vanhoof commented Aug 7, 2026

Copy link
Copy Markdown
Author

Closing as superseded upstream. This query-mode / kanban-worker MCP discovery race is resolved on main by #75933 (ensure MCP discovery completes before agent build in non-interactive sessions). Verified against current main during a runtime rebase — the bounded-timeout approach proposed here is no longer needed.

@vanhoof vanhoof closed this Aug 7, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…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).
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
…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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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