fix(mcp): expose late-connecting MCP tools to the agent (TUI/CLI/gateway) by alt-glitch · Pull Request #49160 · NousResearch/hermes-agent

alt-glitch · 2026-06-19T16:34:19Z

TL;DR

MCP servers that connect after the agent's one-time tool snapshot were invisible to the agent for the whole session (banner could even show them while the agent couldn't call them). This raises the startup discovery wait to a config-driven default of 5.0s (which costs ~0s for the common case — see below), and consolidates the three duplicated tool-snapshot rebuilds into one shared, thread-safe, name-diffing helper so every reload/late-bind path behaves identically.

The bug

The agent snapshots agent.tools / agent.valid_tool_names once at build time and never re-reads the registry (run_agent / agent_init). MCP discovery runs in a background thread, and the build waited only 0.75s (wait_for_mcp_discovery) for it. HTTP/OAuth servers routinely take 2–6s on a cold connect (measured ~2.5s for a Granola streamable-HTTP server with OAuth token reload), so they missed the window and their tools never entered the snapshot. hermes chat masks this because it re-derives get_tool_definitions() per turn through a wrapper that re-waits; the TUI/gateway agent does not.

Behavior

Startup scenario	Before (0.75s flat)	After (5.0s bound, config)
No MCP servers configured (the common case)	instant	instant (the `thread is None` short-circuit; the bound is never reached)
Fast MCP server (<0.75s)	works	works (early-return on thread completion)
Slow HTTP/OAuth server (2–6s)	tools missing all session 🐛	tools present on the first turn
Dead / hung server	bounded at 0.75s	bounded at 5.0s (still can't freeze startup)

thread.join(timeout) returns the instant discovery completes, so the higher bound is only ever reached when a server is genuinely still connecting — fast and no-MCP startups pay ~0s regardless. The bound is exposed as mcp_discovery_timeout in config.yaml (default 5.0).

A second, independent defect: the late-binding refresh and the two reload paths each had their own copy of the agent.tools rebuild, and the late-refresh detected change by tool count (len(new) == len(old)), so an equal-size add/remove swap was silently skipped. All three now call one shared helper that diffs by name.

Why the bug existed

The 0.75s bound was chosen to stop a dead server from freezing startup — but a flat timeout also throttles the reachable-but-slow server, which is the actual reported failure. The rebuild logic was copy-pasted across three call sites and drifted (count- vs name-based diffing).

What is NOT changed / weakened

Prompt caching stays sacred. The late-binding refresh keeps its pre-first-turn guard — it never rebuilds the tool list once a turn/API call has started, so a cached prompt prefix is never invalidated mid-conversation. The shared helper deliberately does not check turn state; each caller owns that policy (/reload-mcp rebuilds after explicit user consent; the late-refresh only pre-first-turn).
Dead-server startup hang protection is preserved (the wait is still bounded; only the ceiling moved).
No new core tool, no new HERMES_* env var — the one knob is a config.yaml value with a safe fallback.
The shared helper now resolves enabled_toolsets/disabled_toolsets from agent.* (what the agent was actually built with) instead of re-resolving _load_enabled_toolsets() at refresh time. This is equivalent today and matches the gateway path; it's also more correct if coding-context/cwd shifted between build and reload.

Rejected alternatives

fix(mcp): increase wait_for_mcp_discovery timeout from 0.75s to 3.0s #41630 (flat 0.75s → 3.0s): right direction but a magic constant with no config and no dedup; this PR makes it configurable and fixes the duplicated rebuild + count-diff bug too. Supersedes fix(mcp): increase wait_for_mcp_discovery timeout from 0.75s to 3.0s #41630.
feat: auto-refresh MCP tools for slow servers via async late-binding #42802 (async late-binding): the shared-helper + thread-safety + name-diff + dedup ideas are excellent and adopted here directly — this PR builds on that foundation. The difference is when/where the refresh fires: feat: auto-refresh MCP tools for slow servers via async late-binding #42802 protects the prompt cache via a deferred (~10s) thread that checks _first_turn_started(); this PR fires the refresh at the turn boundary in build_turn_context (so late tools land on the user's next turn) and adds additive-preservation of memory/context-engine tools, a generation-aware publish, and config-driven timeout. Not a "dropped guard" — a different refresh-lifecycle. Two items from feat: auto-refresh MCP tools for slow servers via async late-binding #42802 are intentionally out of scope here and worth a follow-up: the cli_agent_setup_mixin.py CLI integration and the entry.py↔mcp_startup discovery-thread state sync. Supersedes feat: auto-refresh MCP tools for slow servers via async late-binding #42802.
fix(tui): rebuild agent tool schema when background MCP discovery completes (fixes #38945) #39007 (threading.Event rebuild): subset of the late-refresh already on main; folded in.

Scope

Upstream TUI + CLI + gateway only. The Desktop/WebSocket startup-discovery gap (#42694/#42703/#38301) and hermes -z oneshot (#38448) are the same family but distinct entry points; left for focused follow-ups so this PR stays reviewable.

Tests

tests/tools/test_refresh_agent_mcp_tools.py (new) asserts the contracts, not snapshots:

late-landing tools are added; no-change returns empty and does not swap the snapshot object (no churn)
name-based diff catches an equal-count add/remove swap
rebuild uses the agent's own enabled/disabled toolsets
thread-safety: 4 threads × 50 concurrent refreshes never desync tools vs valid_tool_names
mcp_discovery_timeout resolution (explicit > config > safe fallback) and instant return when nothing is pending

75 passed across the touched areas (new tests + test_mcp_startup, test_tui_mcp_late_refresh, test_mcp_reload_refreshes_cached_agents, test_mcp_reload_confirm_gate, test_mcp_config, test_cli_mcp_config_watch).

Fixes #41625
Fixes #47121
Supersedes #41630
Supersedes #42802

…way) MCP servers that connect after the agent's one-time tool snapshot were invisible for the whole session. Two root causes, fixed together: 1. The startup discovery wait was a flat 0.75s. HTTP/OAuth servers commonly take 2-6s on a cold connect, so they missed the window and their tools never entered the agent's snapshot. `thread.join(timeout)` already returns the instant discovery completes, so raising the bound costs ~0s for the common case (no MCP / fast servers) and only ever blocks for a genuinely-pending server, capped so a dead server can't freeze startup. The bound is now configurable via `mcp_discovery_timeout` (config.yaml, default 5.0s). 2. Three call sites duplicated the agent tool-snapshot rebuild (the TUI `reload.mcp` RPC, the gateway reload, and the TUI late-binding refresh thread), and the late-refresh detected changes by tool COUNT — missing an equal-size add/remove swap. Consolidated into one shared `tools.mcp_tool.refresh_agent_mcp_tools(agent)` helper that diffs by tool NAME, mutates the agent under a lock (thread-safe), and respects the agent's own enabled/disabled toolsets. The late-binding refresh keeps its pre-first-turn cache-safety guard: it never rebuilds the tool list once a turn has started, so the cached prompt prefix is never invalidated mid-conversation. Tests: new tests/tools/test_refresh_agent_mcp_tools.py covers the name-based diff, in-place mutation, agent-scoped filtering, thread safety, and the config-driven discovery bound (incl. instant-return when nothing is pending). 75 passed across the touched areas.

github-actions · 2026-06-19T16:35:07Z

🔎 Lint report: `fix/mcp-late-binding-canonical` vs `origin/main`

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11197 on HEAD, 11157 on base (🆕 +40)

🆕 New issues (41):

Rule	Count
`not-subscriptable`	17
`unsupported-operator`	11
`unresolved-attribute`	8
`invalid-argument-type`	5

First entries

tests/tools/test_browser_lightpanda.py:243: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/cli/test_fast_command.py:484: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/tools/test_web_providers.py:223: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/cli/test_fast_command.py:485: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/tools/test_web_providers.py:218: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["search_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/hermes_cli/test_mcp_reload_confirm_gate.py:33: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/tools/test_browser_hardening.py:125: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/hermes_cli/test_aux_config.py:54: [unresolved-attribute] unresolved-attribute: Attribute `keys` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/hermes_cli/test_mcp_reload_confirm_gate.py:25: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/tools/test_browser_lightpanda.py:242: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["engine"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/tools/test_refresh_agent_mcp_tools.py:257: [invalid-argument-type] invalid-argument-type: Argument to constructor `float.__new__` is incorrect: Expected `str | Buffer | SupportsFloat | SupportsIndex`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/gateway/test_whatsapp_reply_prefix.py:119: [unsupported-operator] unsupported-operator: Operator `>=` is not supported between objects of type `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements` and `int`
agent/background_review.py:544: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_skip_mcp_refresh` on type `AIAgent`
tests/cli/test_resume_display.py:716: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["resume_display"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/agent/test_openrouter_response_cache.py:232: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
cli.py:7546: [invalid-argument-type] invalid-argument-type: Argument to function `build_welcome_banner` is incorrect: Expected `list[str]`, found `list[str] | list[str | Unknown] | set[str]`
tests/hermes_cli/test_destructive_slash_confirm_gate.py:26: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/hermes_cli/test_kanban_core_functionality.py:3375: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/hermes_cli/test_setup_noninteractive.py:122: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/hermes_cli/test_destructive_slash_confirm_gate.py:32: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
hermes_cli/config.py:6419: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tools/browser_tool.py:1184: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
tests/agent/test_onboarding.py:311: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tests/tools/test_browser_console.py:341: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["record_sessions"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
hermes_cli/config.py:4957: [unresolved-attribute] unresolved-attribute: Attribute `items` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int`, `float` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 34 union elements`
... and 16 more

✅ Fixed issues (21):

Rule	Count
`unsupported-operator`	11
`unresolved-attribute`	7
`invalid-argument-type`	3

First entries

tests/tools/test_browser_console.py:341: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["record_sessions"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/hermes_cli/test_aux_config.py:37: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["title_generation"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/tools/test_web_providers.py:218: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["search_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/tools/test_browser_lightpanda.py:242: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["engine"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/agent/test_curator.py:1105: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["curator"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/tools/test_web_providers.py:217: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/cli/test_reasoning_command.py:552: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
cli.py:7546: [invalid-argument-type] invalid-argument-type: Argument to function `build_welcome_banner` is incorrect: Expected `list[str]`, found `list[str] | set[str]`
tests/hermes_cli/test_aux_config.py:54: [unresolved-attribute] unresolved-attribute: Attribute `keys` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/hermes_cli/test_mcp_reload_confirm_gate.py:33: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/gateway/test_whatsapp_reply_prefix.py:119: [unsupported-operator] unsupported-operator: Operator `>=` is not supported between objects of type `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements` and `int`
tests/cli/test_fast_command.py:484: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/hermes_cli/test_aux_config.py:47: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["session_search"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/cli/test_resume_display.py:716: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["resume_display"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/hermes_cli/test_destructive_slash_confirm_gate.py:32: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tools/browser_tool.py:1184: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/hermes_cli/test_kanban_core_functionality.py:3375: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
hermes_cli/config.py:4942: [unresolved-attribute] unresolved-attribute: Attribute `items` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
hermes_cli/config.py:4952: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `None`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/tools/test_web_providers.py:219: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["extract_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`
tests/cron/test_suggestions.py:197: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["monitor"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 33 union elements`

Unchanged: 5826 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

alt-glitch · 2026-06-19T16:41:14Z

Related: this PR consolidates the MCP late-discovery cluster. Supersedes #41630 (flat 0.75s->3.0s timeout bump — this PR makes it config-driven via mcp_discovery_timeout and also fixes the duplicated rebuild + count-vs-name diff bug), #42802 (async late-binding — adopts its shared-helper/thread-safety/name-diff ideas while keeping the pre-first-turn cache-safety guard it dropped), and folds in #39007 (threading.Event rebuild, a subset already on main). Fixes #41625 and #47121 (dup of #41625).

Out of scope / left for focused follow-ups: the Desktop/WebSocket startup-discovery gap (#42694/#42703/#38301) and hermes -z oneshot (#38448) — same family, distinct entry points.

…binding) A slow MCP server (HTTP/OAuth, 2-6s cold connect) that finishes connecting after the agent's one-time tool snapshot was uncallable for the rest of the session. The merged pre-first-turn late-refresh only helps during the dead air before the user's first keystroke; once a turn starts it bails to protect the prompt cache, so a user who types before the server connects never gets the tools without a manual /reload-mcp. Refresh the snapshot in the per-turn prologue (build_turn_context), before this turn's first API call assembles tools=. This is cache-safe by construction: the refresh only ever extends a fresh request prefix at a turn boundary, never mutates the cached prefix of an in-flight turn. So late tools become callable on the user's NEXT turn automatically, with no /reload-mcp and no cache cost. - tools/mcp_tool.py: has_registered_mcp_tools() — cheap guard so sessions with no MCP servers (the common case) skip the rebuild entirely. - agent/turn_context.py: call the shared refresh_agent_mcp_tools() helper at the top of the prologue when MCP servers are registered. - tests: 3 contract tests through the real build_turn_context (adds late tool; skipped when no servers; no snapshot churn when unchanged). .hermes/plans/: SPEC + PLAN documenting the root cause, the cache-safety constraint, and why the existing fixes (#48403/#41630/#42802) don't close it.

…ces) Consolidated findings from three independent reviewers (Codex, Claude Code, a Hermes subagent w/ the hermes-agent-dev skill): - BLOCKING: refresh_agent_mcp_tools rebuilt only the registry subset, silently dropping post-build-injected memory-provider (mem0/honcho/…) and context- engine (lcm_*) tools on every refresh. Now additive-preserving: re-applies the same injectors agent_init uses, staged on locals and published atomically. - Re-injection now honors the #5544 enabled_toolsets gate for context-engine tools, so a restricted-toolset platform can't get lcm_* leaked back in. - Atomic read-diff-publish under one lock: the returned `added` set and the (tools, valid_tool_names) pair are consistent even under concurrent callers (no half-swap, no TOCTOU). - background_review fork opts out (_skip_mcp_refresh) so its byte-identical tools[] cache parity with the parent is preserved. - CLI /reload-mcp routed through the shared helper (was a 4th divergent copy with the same clobber bug + missing disabled_toolsets). - Explicit reloads (TUI RPC + CLI) pass enabled_override so a server the user just enabled in config this session is picked up; automatic paths reuse the agent's build-time selection. - mcp_discovery_timeout default 5.0 -> 1.5s: correctness now comes from the between-turns refresh, so the startup wait is only a small turn-1 UX bump rather than a heavy dead-server latency penalty. - has_registered_mcp_tools checks registered TOOLS (not connected servers) so a zero-tool/prompt-only server doesn't make the per-turn hook fire forever. - Tests: rewrote the thread-safety test to actually exercise the write path (alternating tool sets), added the #5544-gate regression, the memory/context preservation regression, and a "callable next turn via valid_tool_names" contract; removed a dead monkeypatch line.

…ity holes) Second review pass (Codex + Hermes subagent). Codex reproduced a real race with a two-thread harness; both converged on the remaining issues. - Generation-aware publish (fixes a lost-update race): two refresh callers (the late-refresh daemon and the between-turns prologue around turn 1) could each compute a snapshot outside the lock; a SLOWER caller holding an OLDER registry generation could acquire the publish lock after a newer caller and clobber it, deleting just-landed tools. refresh_agent_mcp_tools now captures registry._generation before computing and refuses to publish a stale set; agent._tool_snapshot_generation tracks the published generation. - Context-engine routing names (_context_engine_tool_names) are now staged on a local and published atomically with the snapshot, and only claimed when this rebuild actually appended the schema — matching agent_init's dedup so a registry/plugin tool of the same name keeps its own dispatch. (Previously mutated live, before the publish lock, and on no-change refreshes.) - CLI /reload-mcp: self.enabled_toolsets is resolved once at startup, so a server newly ENABLED in config mid-session wasn't picked up (TUI already re-resolved). Merge now-connected MCP server names into the override (unless the user pinned all/*), mirroring startup, and keep self.enabled_toolsets in sync. Closes the CLI/TUI parity hole. - ACP (acp_adapter/server.py) routed through the shared helper — it was a 5th sibling rebuild that re-injected memory tools but NOT context-engine tools and bypassed the atomic/name-diff path (inert today, fragile). - mcp_startup._resolve_discovery_timeout pulls its default from DEFAULT_CONFIG (single source of truth) instead of a stale hardcoded 5.0 literal. - Tests: stale-generation-no-clobber, _skip_mcp_refresh honored, timeout fallback uses DEFAULT_CONFIG.

…tract note Third review pass (Hermes subagent) declared convergence: no BLOCKING, the round-2 generation-aware publish / context-engine staging / CLI reload / ACP routing all verified correct by hand and by test. - agent_init: capture _tool_snapshot_generation immediately before the tool snapshot (was ~425 lines earlier); removes a harmless skew window so the recorded generation always matches the snapshot it describes. - gateway/run.py _execute_mcp_reload: keep preserving each cached agent's build-time enabled_toolsets EXACTLY (do NOT merge newly-connected servers like CLI/TUI do) and document WHY — gateway sessions can be deliberately locked down, and test_reload_mcp_preserves_per_agent_toolset_overrides asserts this. A reviewer suggested "parity" here; it would have violated that contract.

397239396 · 2026-06-19T17:47:51Z

Great work on this! 🎉 Glad to see the core ideas from #42802 carried forward — the shared refresh function with thread-safe lock, name-based diff, and dedup are exactly the right foundation.

One small correction to the PR description: #42802 did include the pre-first-turn cache-safety guard — spawn_late_mcp_refresh() checks _first_turn_started() and uses a deferred thread (10s delay) to protect prompt cache. The guard wasn't dropped, just restructured into the deferred thread lifecycle. Minor point, just wanted to clarify.

I'm happy to see the timeout becoming config-driven via mcp_discovery_timeout in config.yaml — that's a much better UX than the hardcoded constant in #42802. And the refresh_agent_mcp_tools() living in tools/mcp_tool.py makes sense architecturally since it's a general-purpose tool rebuild operation, not CLI-specific.

If this gets merged first, I'll close #42802. There are a couple of things from #42802 that aren't covered here (CLI path integration in cli_agent_setup_mixin.py, entry.py discovery-thread state sync fix), but those are minor and can be addressed by anyone in follow-ups — they don't block MCP loading.

Thanks for picking this up! 👍

alt-glitch · 2026-06-19T17:54:19Z

Correction re: #42802 + credit and scope notes

A correction to my PR description, flagged by @397239396 (author of #42802): I wrote that #42802 "dropped" the pre-first-turn cache-safety guard. That's not accurate. #42802 did protect the prompt cache — spawn_late_mcp_refresh() checks _first_turn_started() and uses a deferred (~10s) thread lifecycle to do so. The guard wasn't removed, it was restructured into the deferred-thread mechanism. The real difference between the two approaches is when/where the refresh fires (a deferred timer vs. the turn-boundary prologue in build_turn_context), not whether one is cache-safe and the other isn't. Apologies for the mischaracterization — I've struck that line.

Credit where due: the core foundation here is straight from #42802 — the shared refresh function, the thread-safe lock, the name-based diff, and thread dedup. Those were the right call and this PR builds directly on them. What this PR adds on top:

the refresh runs at the turn boundary (build_turn_context) so late tools land on the user's next turn;
it's additive-preserving (re-injects memory-provider + context-engine tools, which get_tool_definitions doesn't reproduce — this was the round-1 review's BLOCKING finding);
a generation-aware publish to reject a stale concurrent rebuild;
the timeout is config-driven via mcp_discovery_timeout in config.yaml rather than a hardcoded constant;
refresh_agent_mcp_tools() lives in tools/mcp_tool.py as a general-purpose rebuild used by all reload paths (TUI RPC, gateway, CLI /reload-mcp, late-refresh, ACP).

Not covered here (good follow-ups, don't block MCP loading):

CLI agent-setup integration in cli_agent_setup_mixin.py (from feat: auto-refresh MCP tools for slow servers via async late-binding #42802);
the entry.py ↔ hermes_cli.mcp_startup discovery-thread state-sync fix (from feat: auto-refresh MCP tools for slow servers via async late-binding #42802; also independently flagged in review as a latent issue).

If this lands first, #42802 can close — but the two items above are worth salvaging from it in a follow-up.

CI caught 3 ACP test failures (tests/acp/test_server.py, tests/acp/test_mcp_e2e.py). Root cause: routing ACP's tool-surface rebuild through the shared refresh_agent_mcp_tools helper (added in the round-2 pass) broke a deliberate, pre-existing ACP contract: - the ACP tests assert `agent.tools is <get_tool_definitions return>` (object identity) and an exact get_tool_definitions(enabled_toolsets=[...], disabled_toolsets=..., quiet_mode=True) call signature; the shared helper list()-copies and re-derives differently, breaking identity; and - the tests use a MagicMock agent whose _tool_snapshot_generation is a mock, so the new `int < published_gen` generation guard raised TypeError and the whole ACP refresh silently failed. ACP already preserves memory-provider tools (its own inject call) and excludes context_engine, so there was no bug to fix there — only over-reach. Reverted ACP to its original rebuild. (Same lesson as the gateway path: leave call sites that carry their own tested contract alone; a reviewer's "inert today, fragile" note meant leave-it, not change-it.) Also hardened the generation guard defensively: tolerate a non-int _tool_snapshot_generation (mock / partially-built agent) instead of throwing TypeError and silently failing the refresh.

teknium1 · 2026-06-19T18:58:01Z

Merged via #49208 — your 6 commits were cherry-picked onto current main with your authorship preserved in git log (16642e2…93d6e73). Thanks for the thorough, self-reviewed implementation. #49208

alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels Jun 19, 2026

alt-glitch added 4 commits June 19, 2026 22:17

alt-glitch marked this pull request as ready for review June 19, 2026 17:55

teknium1 mentioned this pull request Jun 19, 2026

fix(mcp): expose late-connecting MCP tools to the agent (TUI/CLI/gateway) #49208

Merged

teknium1 closed this Jun 19, 2026

teknium1 mentioned this pull request Jun 19, 2026

docs(mcp): fix stale 0.75s discovery-wait reference after #49208 #49216

Closed

This was referenced Aug 3, 2026

fix(cli): wait for MCP discovery in one-shot mode #69174

Open

fix: keep one-shot MCP resources alive through tool calls #66047

Open

coderabbitai Bot mentioned this pull request Aug 10, 2026

feat: Support MCP Authentication NVIDIA/NeMo-Fabric#194

Closed

2 tasks

Navigation Menu

Uh oh!

fix(mcp): expose late-connecting MCP tools to the agent (TUI/CLI/gateway) - #49160

fix(mcp): expose late-connecting MCP tools to the agent (TUI/CLI/gateway)#49160
alt-glitch wants to merge 6 commits into
mainfrom
fix/mcp-late-binding-canonical

alt-glitch commented Jun 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

alt-glitch commented Jun 19, 2026

Uh oh!

397239396 commented Jun 19, 2026

Uh oh!

alt-glitch commented Jun 19, 2026

Uh oh!

teknium1 commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

alt-glitch commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

The bug

Behavior

Why the bug existed

What is NOT changed / weakened

Rejected alternatives

Scope

Tests

Uh oh!

github-actions Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔎 Lint report: fix/mcp-late-binding-canonical vs origin/main

ruff

ty (type checker)

Uh oh!

alt-glitch commented Jun 19, 2026

Uh oh!

397239396 commented Jun 19, 2026

Uh oh!

alt-glitch commented Jun 19, 2026

Uh oh!

teknium1 commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alt-glitch commented Jun 19, 2026 •

edited

Loading

github-actions Bot commented Jun 19, 2026 •

edited

Loading

🔎 Lint report: `fix/mcp-late-binding-canonical` vs `origin/main`