MCP tool discovery racing agent tool snapshot in one-shot/TUI

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

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

  merged to main

  files: agent/agent_init.py, agent/background_review.py, agent/turn_context.py, cli.py, gateway/run.py, hermes_cli/config.py, hermes_cli/mcp_startup.py, tests/agent/test_turn_context.py, tests/tools/test_refresh_agent_mcp_tools.py, tools/mcp_tool.py, tui_gateway/entry.py, tui_gateway/server.py

  Show the diff

  • agent/agent_init.py

    @@ -531,7 +531,14 @@ def init_agent(
         agent._last_activity_desc: str = "initializing"
         agent._current_tool: str | None = None
         agent._api_call_count: int = 0
    -
    +    # Opt-out flag for the between-turns MCP tool refresh (build_turn_context).
    +    # Set on internal forks (e.g. background_review) that must keep ``tools[]``
    +    # byte-identical to a parent for provider cache parity.
    +    agent._skip_mcp_refresh = False
    +    # Registry generation the current tool snapshot was derived from. Lets a
    +    # late/concurrent refresh reject a stale (older-generation) rebuild instead
    +    # of clobbering a newer one. Set adjacent to the tool snapshot below.
    +    agent._tool_snapshot_generation = 0
         # Rate limit tracking — updated from x-ratelimit-* response headers
         # after each API call.  Accessed by /usage slash command.
         agent._rate_limit_state: Optional["RateLimitState"] = None
    @@ -953,7 +960,14 @@ def init_agent(
                 print(f"🔄 Fallback chain ({len(agent._fallback_chain)} providers): " +
                       " → ".join(f"{f['model']} ({f['provider']})" for f in agent._fallback_chain))
     
    -    # Get available tools with filtering
    +    # Get available tools with filtering. Capture the registry generation this
    +    # snapshot is derived from FIRST, so a later concurrent refresh can tell
    +    # whether it holds a newer or staler view (see refresh_agent_mcp_tools).
    +    try:
    +        from tools.registry import registry as _snapshot_registry
    +        agent._tool_snapshot_generation = _snapshot_registry._generation
    +    except Exception:
    +        agent._tool_snapshot_generation = 0
         agent.tools = _ra().get_tool_definitions(
             enabled_toolsets=enabled_toolsets,
             disabled_toolsets=disabled_toolsets,

  • agent/background_review.py

    @@ -535,6 +535,13 @@ def _bg_review_auto_deny(command, description, **kwargs):
                 )
                 review_agent._memory_write_origin = "background_review"
                 review_agent._memory_write_context = "background_review"
    +            # The review fork pins the parent's cached system prompt and keeps
    +            # ``tools[]`` byte-identical to the parent so its outbound request
    +            # hits the same provider cache prefix (see the toolset-parity note
    +            # above). The between-turns MCP refresh in build_turn_context would
    +            # add late-connecting MCP tools to this fork and break that parity,
    +            # so opt the review fork out of it.
    +            review_agent._skip_mcp_refresh = True
                 review_agent._memory_store = agent._memory_store
                 review_agent._memory_enabled = agent._memory_enabled
                 review_agent._user_profile_enabled = agent._user_profile_enabled

  • agent/turn_context.py

    @@ -112,6 +112,24 @@ def build_turn_context(
         # Restore the primary runtime if the previous turn activated fallback.
         agent._restore_primary_runtime()
     
    +    # Between-turns MCP refresh: an MCP server that finished connecting since
    +    # the previous turn (slow HTTP/OAuth servers routinely take 2-6s on a cold
    +    # connect, missing the bounded startup wait) lands in THIS turn's tool
    +    # snapshot.  This is cache-safe by construction: it runs in the per-turn
    +    # prologue, before this turn's first API call assembles ``tools=``, so it
    +    # only ever extends a fresh request prefix — it never mutates the cached
    +    # prefix of an in-flight turn.  No-op when no MCP servers are registered
    +    # (the common case, gated by the cheap ``has_registered_mcp_tools`` check)
    +    # or when the tool set is unchanged (``refresh_agent_mcp_tools`` diffs by
    +    # name and leaves the snapshot untouched on no-change).
    +    try:
    +        if not getattr(agent, "_skip_mcp_refresh", False):
    +            from tools.mcp_tool import has_registered_mcp_tools, refresh_agent_mcp_tools
    +            if has_registered_mcp_tools():
    +                refresh_agent_mcp_tools(agent, quiet_mode=True)
    +    except Exception:
    +        logger.debug("between-turns MCP tool refresh skipped", exc_info=True)
    +
         # Sanitize surrogate characters from user input.
         if isinstance(user_message, str):
             user_message = sanitize_surrogates(user_message)

  • cli.py

    @@ -9661,16 +9661,35 @@ def _reload_mcp(self):
                 else:
                     print(f"  🔧 {len(new_tools)} tool(s) available from {len(connected_servers)} server(s)")
     
    -            # Refresh the agent's tool list so the model can call new tools
    +            # Refresh the agent's tool list so the model can call new tools.
    +            # Route through the shared helper so this CLI /reload-mcp path stays
    +            # in lockstep with the TUI RPC / gateway reload / late-binding paths
    +            # (name-diff, thread-safe, and — critically — additive-preserving so
    +            # memory-provider and context-engine tools survive the rebuild).
                 if self.agent is not None:
    -                self.agent.tools = get_tool_definitions(
    -                    enabled_toolsets=self.agent.enabled_toolsets
    -                    if hasattr(self.agent, "enabled_toolsets") else None,
    +                from tools.mcp_tool import refresh_agent_mcp_tools
    +                # Explicit reload: pick up MCP servers the user ENABLED in config
    +                # this session. self.enabled_toolsets was resolved once at
    +                # startup; merge in any now-connected server names (unless the
    +                # user pinned `all`/`*`, which already includes everything) so a
    +                # freshly-added server isn't filtered out. Mirrors startup, where
    +                # MCP server names are part of enabled_toolsets (see __init__).
    +                enabled_override = None
    +                et = self.enabled_toolsets
    +                if et and "all" not in et and "*" not in et:
    +                    merged = list(et)
    +                    for _name in sorted(connected_servers):
    +                        if _name not in merged:
    +                            merged.append(_name)
    +                    enabled_override = merged
    +                refresh_agent_mcp_tools(
    +                    self.agent,
    +                    enabled_override=enabled_override,
                         quiet_mode=True,
                     )
    -                self.agent.valid_tool_names = {
    -                    tool["function"]["name"] for tool in self.agent.tools
    -                } if self.agent.tools else set()
    +                # Keep the CLI's own list in sync with what the agent now uses.
    +                if enabled_override is not None:
    +                    self.enabled_toolsets = enabled_override
     
                 # Inject a message at the END of conversation history so the
                 # model knows tools changed.  Appended after all existing

  • gateway/run.py

    @@ -11656,7 +11656,7 @@ async def _execute_mcp_reload(self, event: MessageEvent) -> str:
                 # consented to the prompt-cache invalidation via the slash-confirm
                 # gate in _handle_reload_mcp_command before we reach this point.
                 try:
    -                from model_tools import get_tool_definitions
    +                from tools.mcp_tool import refresh_agent_mcp_tools
                     _cache = getattr(self, "_agent_cache", None)
                     _cache_lock = getattr(self, "_agent_cache_lock", None)
                     if _cache_lock is not None and _cache:
    @@ -11668,15 +11668,16 @@ async def _execute_mcp_reload(self, event: MessageEvent) -> str:
                                     continue
                                 if _agent is None:
                                     continue
    -                            new_defs = get_tool_definitions(
    -                                enabled_toolsets=getattr(_agent, "enabled_toolsets", None),
    -                                disabled_toolsets=getattr(_agent, "disabled_toolsets", None),
    -                                quiet_mode=True,
    -                            )
    -                            _agent.tools = new_defs
    -                            _agent.valid_tool_names = {
    -                                t["function"]["name"] for t in new_defs
    -                            } if new_defs else set()
    +                            # Preserve each cached agent's build-time toolset
    +                            # selection EXACTLY: a gateway session built with a
    +                            # restricted enabled_toolsets (e.g. ["safe"]) must
    +                            # NOT silently gain tools after a reload. This is the
    +                            # opposite of the interactive CLI/TUI /reload-mcp,
    +                            # which is a single user re-applying their own config
    +                            # edit; gateway agents are per-session and may be
    +                            # deliberately locked down. (Contract is asserted by
    +                            # test_reload_mcp_preserves_per_agent_toolset_overrides.)
    +                            refresh_agent_mcp_tools(_agent, quiet_mode=True)
                 except Exception as _exc:
                     logger.debug(
                         "Failed to update cached agent tools after MCP reload: %s",

  • hermes_cli/config.py

    @@ -1201,6 +1201,21 @@ def _ensure_hermes_home_managed(home: Path):
         # 100K chars ≈ 25–35K tokens across typical tokenisers.
         "file_read_max_chars": 100_000,
     
    +    # Seconds to wait at agent-build time for in-flight MCP server discovery
    +    # to finish before the agent snapshots its tool list.  MCP discovery runs
    +    # in a background thread so a slow/dead server can't freeze startup; this
    +    # bounds how long the first agent build blocks on it.  The wait returns
    +    # the INSTANT discovery completes, so users with no MCP servers (the common
    +    # case) or fast servers pay ~0s regardless of this value — the bound is
    +    # only reached when a server is genuinely still connecting.  The old 0.75s
    +    # default was a touch short for HTTP/OAuth servers on a cold connect; a
    +    # modest bump lets more of them land in the FIRST turn's snapshot.  This is
    +    # only a turn-1 latency/UX knob: a server that misses this window is still
    +    # picked up automatically on the next turn by the between-turns refresh
    +    # (see agent/turn_context.py), so correctness never depends on it.  Keep it
    +    # small so a slow/dead server adds little to first-response latency.
    +    "mcp_discovery_timeout": 1.5,
    +
         # Tool-output truncation thresholds. When terminal output or a
         # single read_file page exceeds these limits, Hermes truncates the
         # payload sent to the model (keeping head + tail for terminal,

  • hermes_cli/mcp_startup.py

    @@ -51,9 +51,38 @@ def _discover() -> None:
             thread.start()
     
     
    -def wait_for_mcp_discovery(timeout: float = 0.75) -> None:
    -    """Briefly wait for background MCP discovery before the first tool snapshot."""
    +def _resolve_discovery_timeout(explicit: "float | None") -> float:
    +    """Resolve the MCP discovery wait bound: explicit arg > config > default.
    +
    +    Reads ``mcp_discovery_timeout`` from config.yaml, defaulting to the value in
    +    ``DEFAULT_CONFIG`` (single source of truth) when the key is absent. Kept lazy
    +    and fail-safe — a missing/invalid value or a broken config falls back to a
    +    short safe bound so startup can never hang or crash.
    +    """
    +    if explicit is not None:
    +        return explicit
    +    try:
    +        from hermes_cli.config import load_config, DEFAULT_CONFIG
    +
    +        default = float(DEFAULT_CONFIG.get("mcp_discovery_timeout", 1.5))
    +        raw = (load_config() or {}).get("mcp_discovery_timeout", default)
    +        val = float(raw)
    +        return val if val > 0 else default
    +    except Exception:
    +        return 1.5
    +
    +
    +def wait_for_mcp_discovery(timeout: "float | None" = None) -> None:
    +    """Wait for background MCP discovery before the first tool snapshot.
    +
    +    ``thread.join(timeout)`` returns the INSTANT discovery completes, so this
    +    only ever blocks for the real connect time of a still-pending server —
    +    users with no MCP servers or fast servers pay ~0s.  The bound (from
    +    ``mcp_discovery_timeout`` in config) just caps the wait so a dead server
    +    can't freeze startup; servers that miss it are picked up by the automatic
    +    late-binding refresh.
    +    """
         thread = _mcp_discovery_thread
         if thread is None or not thread.is_alive():
             return
    -    thread.join(timeout=timeout)
    +    thread.join(timeout=_resolve_discovery_timeout(timeout))

  • tests/agent/test_turn_context.py

    @@ -47,6 +47,9 @@ def __init__(self):
             self.max_iterations = 90
             self.tools = []
             self.valid_tool_names = set()
    +        self.enabled_toolsets = None
    +        self.disabled_toolsets = None
    +        self._skip_mcp_refresh = False
             self.compression_enabled = False
             self.context_compressor = types.SimpleNamespace(
                 protect_first_n=2, protect_last_n=2
    @@ -185,3 +188,74 @@ def test_no_review_when_memory_disabled():
         agent = _FakeAgent()
         ctx = _build(agent)
         assert ctx.should_review_memory is False
    +
    +
    +# ── Between-turns MCP refresh (cache-safe late-binding) ──────────────────────
    +#
    +# A slow MCP server that connects after the agent's build-time tool snapshot
    +# must become callable by the user's NEXT turn — without mutating an in-flight
    +# turn's cached request prefix. The prologue is exactly that boundary, so the
    +# refresh hook lives here. These assert the contract (R1/R2/R6 in the spec),
    +# not timing permutations.
    +
    +
    +def test_between_turns_refresh_adds_late_tool_when_servers_registered():
    +    """R1: a tool that registered since build lands in this turn's snapshot."""
    +    agent = _FakeAgent()
    +
    +    new_def = {"type": "function", "function": {"name": "mcp_x_tool", "description": "", "parameters": {}}}
    +
    +    import model_tools
    +    with patch("tools.mcp_tool.has_registered_mcp_tools", return_value=True), \
    +         patch.object(model_tools, "get_tool_definitions", return_value=[new_def]):
    +        _build(agent)
    +
    +    assert "mcp_x_tool" in agent.valid_tool_names
    +    assert any(t["function"]["name"] == "mcp_x_tool" for t in agent.tools)
    +
    +
    +def test_between_turns_refresh_skipped_when_no_servers():
    +    """R6: the common case (no MCP servers) never walks the registry."""
    +    agent = _FakeAgent()
    +    import model_tools
    +
    +    with patch("tools.mcp_tool.has_registered_mcp_tools", return_value=False), \
    +         patch.object(model_tools, "get_tool_definitions") as gtd:
    +        _build(agent)
    +
    +    gtd.assert_not_called()
    +
    +
    +def test_between_turns_refresh_skipped_when_skip_flag_set():
    +    """Internal forks (background_review) set _skip_mcp_refresh to keep tools[]
    +    byte-identical to the parent for cache parity — the hook must honor it even
    +    when MCP servers are registered."""
    +    agent = _FakeAgent()
    +    agent._skip_mcp_refresh = True
    +    import model_tools
    +
    +    with patch("tools.mcp_tool.has_registered_mcp_tools", return_value=True), \
    +         patch.object(model_tools, "get_tool_definitions") as gtd:
    +        _build(agent)
    +
    +    gtd.assert_not_called()
    +
    +
    +def test_between_turns_refresh_no_churn_when_unchanged():
    +    """R2: an unchanged tool set leaves the snapshot object identity intact
    +    (no needless swap → nothing for the next request prefix to diff against)."""
    +    agent = _FakeAgent()
    +    same = [{"type": "function", "function": {"name": "a", "description": "", "parameters": {}}}]
    +    agent.tools = same
    +    agent.valid_tool_names = {"a"}
    +
    +    import model_tools
    +    with patch("tools.mcp_tool.has_registered_mcp_tools", return_value=True), \
    +         patch.object(
    +             model_tools, "get_tool_definitions",
    +             return_value=[{"type": "function", "function": {"name": "a", "description": "", "parameters": {}}}],
    +         ):
    +        _build(agent)
    +
    +    assert agent.tools is same  # not replaced → no churn
    +

  • tests/tools/test_refresh_agent_mcp_tools.py

    @@ -0,0 +1,298 @@
    +"""Tests for the shared MCP agent-tool refresh helper and discovery-wait bound.
    +
    +``refresh_agent_mcp_tools`` is the single rebuild path used by the TUI
    +``reload.mcp`` RPC, the gateway reload, and the late-binding refresh thread —
    +so a slow MCP server that connects after the agent's one-time tool snapshot is
    +picked up everywhere identically.  These assert the *contracts* those callers
    +rely on (name-based diff, in-place mutation, agent-scoped filtering) rather than
    +freezing any particular tool list.
    +"""
    +
    +import threading
    +import types
    +
    +from tools import mcp_tool
    +
    +
    +def _tool(name):
    +    return {"type": "function", "function": {"name": name, "description": "", "parameters": {}}}
    +
    +
    +def _agent(tool_names, *, enabled=None, disabled=None):
    +    a = types.SimpleNamespace()
    +    a.tools = [_tool(n) for n in tool_names]
    +    a.valid_tool_names = set(tool_names)
    +    a.enabled_toolsets = enabled
    +    a.disabled_toolsets = disabled
    +    return a
    +
    +
    +def test_refresh_adds_late_landing_tools(monkeypatch):
    +    """A server that registers after build → its tools land in the snapshot."""
    +    agent = _agent(["read_file", "terminal"])
    +
    +    new_defs = [_tool(n) for n in ("read_file", "terminal", "mcp_granola_get_account_info")]
    +    monkeypatch.setattr(mcp_tool, "get_tool_definitions", lambda **kw: new_defs, raising=False)
    +    # get_tool_definitions is imported inside the helper from model_tools, so patch there too.
    +    import model_tools
    +    monkeypatch.setattr(model_tools, "get_tool_definitions", lambda **kw: new_defs)
    +
    +    added = mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    assert added == {"mcp_granola_get_account_info"}
    +    assert "mcp_granola_get_account_info" in agent.valid_tool_names
    +    assert len(agent.tools) == 3
    +
    +
    +def test_refresh_no_change_returns_empty_and_leaves_agent_untouched(monkeypatch):
    +    """No new tools → empty set, and the snapshot object is not swapped."""
    +    agent = _agent(["read_file", "terminal"])
    +    original_tools = agent.tools
    +
    +    import model_tools
    +    monkeypatch.setattr(
    +        model_tools, "get_tool_definitions",
    +        lambda **kw: [_tool("read_file"), _tool("terminal")],
    +    )
    +
    +    added = mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    assert added == set()
    +    assert agent.tools is original_tools  # not replaced → no churn / no cache thrash
    +
    +
    +def test_refresh_detects_equal_size_swap(monkeypatch):
    +    """Name-based diff catches an add+remove of equal count (count-compare can't)."""
    +    agent = _agent(["a", "old_mcp_tool"])  # 2 tools
    +
    +    import model_tools
    +    # Same COUNT (2) but a different membership: old_mcp_tool removed, new added.
    +    monkeypatch.setattr(
    +        model_tools, "get_tool_definitions",
    +        lambda **kw: [_tool("a"), _tool("new_mcp_tool")],
    +    )
    +
    +    added = mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    assert added == {"new_mcp_tool"}
    +    assert agent.valid_tool_names == {"a", "new_mcp_tool"}
    +    assert "old_mcp_tool" not in agent.valid_tool_names
    +
    +
    +def test_refresh_passes_agent_toolset_filters(monkeypatch):
    +    """The rebuild re-derives with the agent's OWN enabled/disabled toolsets."""
    +    agent = _agent(["a"], enabled=["coding", "granola"], disabled=["messaging"])
    +    seen = {}
    +
    +    import model_tools
    +
    +    def _capture(**kw):
    +        seen.update(kw)
    +        return [_tool("a"), _tool("b")]
    +
    +    monkeypatch.setattr(model_tools, "get_tool_definitions", _capture)
    +
    +    mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    assert seen["enabled_toolsets"] == ["coding", "granola"]
    +    assert seen["disabled_toolsets"] == ["messaging"]
    +
    +
    +def test_refresh_preserves_memory_provider_and_context_engine_tools(monkeypatch):
    +    """B1 regression: a rebuild must NOT drop post-build-injected tools.
    +
    +    get_tool_definitions() returns only the registry-derived tools. agent_init
    +    appends memory-provider tools (mem0/honcho/…) and context-engine tools
    +    (lcm_*) directly onto agent.tools AFTER that. A naive
    +    `agent.tools = get_tool_definitions()` would silently delete them on every
    +    refresh. The helper must re-inject them.
    +    """
    +    # Agent already carries: a built-in, a memory-provider tool, a context tool.
    +    agent = _agent(["read_file", "memory_search", "lcm_grep"])
    +
    +    # Provider exposes its schemas; context compressor exposes lcm_*.
    +    agent._memory_manager = types.SimpleNamespace(
    +        get_all_tool_schemas=lambda: [
    +            {"name": "memory_search", "description": "", "parameters": {}}
    +        ]
    +    )
    +    agent.context_compressor = types.SimpleNamespace(
    +        get_tool_schemas=lambda: [
    +            {"name": "lcm_grep", "description": "", "parameters": {}}
    +        ]
    +    )
    +    agent._context_engine_tool_names = {"lcm_grep"}
    +
    +    import model_tools
    +    # The registry now ALSO has a newly-connected MCP tool, but does NOT contain
    +    # the memory/context tools (they're never in get_tool_definitions output).
    +    monkeypatch.setattr(
    +        model_tools, "get_tool_definitions",
    +        lambda **kw: [_tool("read_file"), _tool("mcp_new_server_tool")],
    +    )
    +
    +    added = mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    # The new MCP tool landed AND the injected families survived.
    +    assert "mcp_new_server_tool" in agent.valid_tool_names
    +    assert "memory_search" in agent.valid_tool_names   # not clobbered
    +    assert "lcm_grep" in agent.valid_tool_names         # not clobbered
    +    assert added == {"mcp_new_server_tool"}
    +
    +
    +def test_refresh_respects_context_engine_toolset_gate(monkeypatch):
    +    """#5544: context-engine tools must NOT be re-injected on a restricted
    +    toolset. A platform with enabled_toolsets that excludes context_engine
    +    must not get lcm_* leaked back in by a refresh."""
    +    agent = _agent(["read_file"], enabled=["coding"])  # context_engine NOT enabled
    +    agent.context_compressor = types.SimpleNamespace(
    +        get_tool_schemas=lambda: [{"name": "lcm_grep", "description": "", "parameters": {}}]
    +    )
    +    agent._context_engine_tool_names = set()
    +
    +    import model_tools
    +    monkeypatch.setattr(
    +        model_tools, "get_tool_definitions",
    +        lambda **kw: [_tool("read_file"), _tool("mcp_new_tool")],
    +    )
    +
    +    mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    assert "mcp_new_tool" in agent.valid_tool_names  # MCP tool still lands
    +    assert "lcm_grep" not in agent.valid_tool_names   # gated out (#5544)
    +
    +
    +def test_refreshed_tool_is_callable_through_valid_tool_names_guard(monkeypatch):
    +    """The whole point: a late tool, once refreshed, passes the name guard the
    +    run loop uses to accept/reject tool calls (agent.valid_tool_names)."""
    +    agent = _agent(["read_file"])
    +
    +    import model_tools
    +    monkeypatch.setattr(
    +        model_tools, "get_tool_definitions",
    +        lambda **kw: [_tool("read_file"), _tool("mcp_granola_list_meetings")],
    +    )
    +
    +    # Before refresh the run loop would reject the call ("Tool does not exist").
    +    assert "mcp_granola_list_meetings" not in agent.valid_tool_names
    +
    +    mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    # After refresh the same guard accepts it AND it's in the tools= payload.
    +    assert "mcp_granola_list_meetings" in agent.valid_tool_names
    +    assert any(t["function"]["name"] == "mcp_granola_list_meetings" for t in agent.tools)
    +
    +
    +def test_refresh_is_thread_safe_under_concurrent_calls(monkeypatch):
    +    """Concurrent refreshes keep tools / valid_tool_names coherent.
    +
    +    The registry alternates between two DIFFERENT tool sets every call, so the
    +    write path (publish) runs repeatedly rather than short-circuiting on the
    +    no-change early return — this actually exercises the lock. The invariant:
    +    a reader of ``valid_tool_names`` must always match ``agent.tools``, and the
    +    final published pair must be one of the two valid sets (never a mix).
    +    """
    +    agent = _agent(["a"])
    +
    +    import itertools
    +    set_a = [_tool("a"), _tool("b")]
    +    set_b = [_tool("a"), _tool("c")]
    +    flip = itertools.cycle([set_a, set_b])
    +    flip_lock = threading.Lock()
    +
    +    def _gtd(**kw):
    +        with flip_lock:
    +            return list(next(flip))
    +
    +    import model_tools
    +    monkeypatch.setattr(model_tools, "get_tool_definitions", _gtd)
    +
    +    errors = []
    +
    +    def _worker():
    +        try:
    +            for _ in range(50):
    +                mcp_tool.refresh_agent_mcp_tools(agent)
    +                # Coherence invariant: the name set must match the tool list
    +                # at every observation, never a torn cross-attribute state.
    +                names = {t["function"]["name"] for t in agent.tools}
    +                assert agent.valid_tool_names == names
    +                assert names in ({"a", "b"}, {"a", "c"})
    +        except Exception as exc:  # pragma: no cover - failure path
    +            errors.append(exc)
    +
    +    threads = [threading.Thread(target=_worker) for _ in range(4)]
    +    for t in threads:
    +        t.start()
    +    for t in threads:
    +        t.join(timeout=10)
    +
    +    assert not errors
    +    assert agent.valid_tool_names in ({"a", "b"}, {"a", "c"})
    +
    +
    +# ── discovery-wait bound (mcp_discovery_timeout config) ──────────────────────
    +
    +
    +def test_resolve_discovery_timeout_explicit_wins(monkeypatch):
    +    from hermes_cli import mcp_startup
    +
    +    assert mcp_startup._resolve_discovery_timeout(2.5) == 2.5
    +
    +
    +def test_resolve_discovery_timeout_reads_config(monkeypatch):
    +    from hermes_cli import mcp_startup
    +    import hermes_cli.config as cfg
    +
    +    monkeypatch.setattr(cfg, "load_config", lambda: {"mcp_discovery_timeout": 8.0})
    +
    +    assert mcp_startup._resolve_discovery_timeout(None) == 8.0
    +
    +
    +def test_resolve_discovery_timeout_falls_back_on_bad_value(monkeypatch):
    +    from hermes_cli import mcp_startup
    +    import hermes_cli.config as cfg
    +
    +    # Non-positive / unparsable → DEFAULT_CONFIG value, never hang.
    +    default = float(cfg.DEFAULT_CONFIG.get("mcp_discovery_timeout", 1.5))
    +    monkeypatch.setattr(cfg, "load_config", lambda: {"mcp_discovery_timeout": 0})
    +    assert mcp_startup._resolve_discovery_timeout(None) == default
    +
    +    monkeypatch.setattr(cfg, "load_config", lambda: {"mcp_discovery_timeout": "oops"})
    +    assert mcp_startup._resolve_discovery_timeout(None) == default
    +
    +
    +def test_stale_generation_refresh_does_not_clobber_newer(monkeypatch):
    +    """A slower refresh that computed an OLDER registry generation must not
    +    overwrite a snapshot a newer-generation refresh already published."""
    +    from tools import registry as _reg_mod
    +
    +    agent = _agent(["read_file"])
    +    # A newer refresh already published generation = current+5, with two tools.
    +    agent._tool_snapshot_generation = _reg_mod.registry._generation + 5
    +    agent.tools = [_tool("read_file"), _tool("mcp_new_tool")]
    +    agent.valid_tool_names = {"read_file", "mcp_new_tool"}
    +
    +    import model_tools
    +    # This (stale) refresh computes only the old single-tool set.
    +    monkeypatch.setattr(model_tools, "get_tool_definitions", lambda **kw: [_tool("read_file")])
    +
    +    added = mcp_tool.refresh_agent_mcp_tools(agent)
    +
    +    # Stale write rejected: the newer tool survives.
    +    assert added == set()
    +    assert "mcp_new_tool" in agent.valid_tool_names
    +
    +
    +def test_wait_returns_instantly_when_no_discovery_thread(monkeypatch):
    +    """The common case (no MCP / discovery done) pays ~0s regardless of bound."""
    +    import time
    +    from hermes_cli import mcp_startup
    +
    +    monkeypatch.setattr(mcp_startup, "_mcp_discovery_thread", None)
    +    import hermes_cli.config as cfg
    +    monkeypatch.setattr(cfg, "load_config", lambda: {"mcp_discovery_timeout": 999.0})
    +
    +    t0 = time.time()
    +    mcp_startup.wait_for_mcp_discovery()
    +    assert time.time() - t0 < 0.2  # never blocks on the bound when nothing's pending

  • tools/mcp_tool.py

    @@ -3994,6 +3994,215 @@ async def _probe_all():
         return result
     
     
    +# Serializes in-place mutation of an agent's tool snapshot.  The reload RPC,
    +# the gateway reload, and the late-binding refresh thread all swap
    +# ``agent.tools`` / ``agent.valid_tool_names`` after the agent was built; the
    +# agent's run loop reads those during tool iteration, so a concurrent write
    +# mid-read could otherwise expose a half-updated list.
    +_agent_tools_lock = threading.Lock()
    +
    +
    +def has_registered_mcp_tools() -> bool:
    +    """True if any MCP server has actually registered tools into the registry.
    +
    +    Cheap — checks the global MCP-tool→server name map under ``_lock``, no
    +    registry walk.  Used by the per-turn refresh hook so a session with no MCP
    +    tools (the common case, and also a connected-but-zero-tool/prompt-only
    +    server) skips the ``get_tool_definitions`` rebuild entirely.  Checks
    +    registered TOOLS, not connected servers, so a server that registers no tools
    +    doesn't keep the hook firing every turn.
    +    """
    +    with _lock:
    +        return bool(_mcp_tool_server_names)
    +
    +
    +def refresh_agent_mcp_tools(
    +    agent,
    +    *,
    +    enabled_override=None,
    +    disabled_override=None,
    +    quiet_mode: bool = True,
    +) -> set:
    +    """Re-derive an already-built agent's tool snapshot from the live registry.
    +
    +    The agent snapshots ``agent.tools`` once at build time and never re-reads
    +    the registry (see ``run_agent`` / ``agent_init``).  When MCP servers connect
    +    *after* that snapshot — a slow HTTP/OAuth server that misses the bounded
    +    startup wait, or a ``/reload-mcp`` — their tools are invisible until the
    +    snapshot is rebuilt.  This is the single shared rebuild used by every such
    +    caller (the TUI ``reload.mcp`` RPC, the gateway reload, the late-binding
    +    refresh thread, and the per-turn between-turns refresh) so they can't drift
    +    apart again.
    +
    +    The rebuild respects the agent's own ``enabled_toolsets`` /
    +    ``disabled_toolsets`` (the same filtering it was built with) and diffs by
    +    tool **name** (not count — a count compare misses an equal-size add/remove
    +    swap).
    +
    +    Crucially it is **additive-preserving**: ``get_tool_definitions`` returns
    +    only the registry-derived tools, but ``agent_init`` appends two further
    +    families directly onto ``agent.tools`` *after* that — external
    +    memory-provider tools (mem0/honcho/…) and context-engine tools
    +    (``lcm_*``).  A naive ``agent.tools = get_tool_definitions(...)`` would
    +    silently DELETE those.  So after rebuilding the registry set we re-run the
    +    same post-build injectors ``agent_init`` used, reconstructing the full
    +    surface.  The new ``(tools, valid_tool_names)`` pair is published together
    +    under ``_agent_tools_lock`` so a concurrent reader never sees a
    +    cross-attribute half-swap.
    +
    +    Returns the set of newly-added tool names (empty when nothing changed), so
    +    callers can decide whether to notify the user / re-emit session info.  The
    +    caller owns the prompt-cache contract: this helper does NOT check turn state,
    +    because each caller has a different policy (``/reload-mcp`` rebuilds after
    +    explicit user consent; the late-binding and between-turns paths only rebuild
    +    at a turn boundary, before that turn's ``tools=`` prefix is assembled).
    +    """
    +    from model_tools import get_tool_definitions
    +    from tools.registry import registry
    +
    +    # Explicit reloads (/reload-mcp) pass freshly-resolved toolsets so a server
    +    # the user just ENABLED in config is picked up; the agent's stored selection
    +    # is then updated to match. The automatic paths (between-turns, late-binding)
    +    # pass nothing and reuse the agent's build-time selection unchanged.
    +    if enabled_override is not None or disabled_override is not None:
    +        enabled = enabled_override if enabled_override is not None else getattr(agent, "enabled_toolsets", None)
    +        disabled = disabled_override if disabled_override is not None else getattr(agent, "disabled_toolsets", None)
    +        agent.enabled_toolsets = enabled
    +        agent.disabled_toolsets = disabled
    +    else:
    +        enabled = getattr(agent, "enabled_toolsets", None)
    +        disabled = getattr(agent, "disabled_toolsets", None)
    +
    +    # Capture the registry generation this rebuild is derived from BEFORE the
    +    # (potentially slow) get_tool_definitions call. Used at publish time to
    +    # reject a stale write: if two callers race (e.g. the late-refresh daemon
    +    # and the between-turns prologue around turn 1), a slower caller that
    +    # computed an OLDER set must not clobber a newer set another caller already
    +    # published. ``registry._generation`` bumps on every (de)register.
    +    snapshot_generation = registry._generation
    +
    +    # Registry-derived tools (built-ins + MCP), filtered to the agent's toolsets.
    +    # Computed OUTSIDE the lock (get_tool_definitions can be slow); the diff and
    +    # publish below happen together in ONE critical section so two concurrent
    +    # callers can't torn-publish or compute overlapping ``added`` sets.
    +    new_defs = list(
    +        get_tool_definitions(
    +            enabled_toolsets=enabled,
    +            disabled_toolsets=disabled,
    +            quiet_mode=quiet_mode,
    +        )
    +        or []
    +    )
    +    new_names = {t["function"]["name"] for t in new_defs}
    +
    +    # Re-append the post-build injected families that get_tool_definitions does
    +    # NOT reproduce, so a refresh never strips them (memory-provider + context-
    +    # engine tools). Staged entirely on LOCALS — the live ``agent.tools`` /
    +    # ``valid_tool_names`` / ``_context_engine_tool_names`` are never touched
    +    # until the single atomic publish below, so a concurrent reader
    +    # (``build_api_kwargs``) can't see a partial rebuild or a cross-attribute
    +    # half-swap. ``staged_engine_names`` are the context-engine routing names
    +    # this rebuild actually appended (matching agent_init's dedup-aware add).
    +    staged_engine_names = _reinject_post_build_tools(agent, new_defs, new_names)
    +
    +    # Single atomic read-diff-publish so the returned ``added`` is consistent
    +    # with what was actually published, even under concurrent callers, and a
    +    # stale (older-generation) rebuild can't overwrite a newer published one.
    +    with _agent_tools_lock:
    +        # Defensive: the published generation should be an int, but tolerate an
    +        # agent that never set it (or set a non-int, e.g. a test mock) rather
    +        # than throwing TypeError on the comparison and silently failing the
    +        # whole refresh.
    +        published_gen_raw = getattr(agent, "_tool_snapshot_generation", -1)
    +        published_gen = published_gen_raw if isinstance(published_gen_raw, int) else -1
    +        if snapshot_generation < published_gen:
    +            # A newer snapshot already won; our set is stale — drop it.
    +            return set()
    +        current = {
    +            t["function"]["name"]
    +            for t in (getattr(agent, "tools", None) or [])
    +        }
    +        if new_names == current:
    +            # No change → leave the live snapshot untouched (no churn), but
    +            # record the generation so an in-flight older caller can't clobber.
    +            agent._tool_snapshot_generation = max(published_gen, snapshot_generation)
    +            return set()
    +        agent.tools = new_defs
    +        agent.valid_tool_names = new_names
    +        # Publish context-engine routing names atomically with the snapshot.
    +        engine_names = getattr(agent, "_context_engine_tool_names", None)
    +        if isinstance(engine_names, set):
    +            engine_names.clear()
    +            engine_names.update(staged_engine_names)
    +        agent._tool_snapshot_generation = max(published_gen, snapshot_generation)
    +        return new_names - current
    +
    +
    +def _reinject_post_build_tools(agent, tools_list: list, name_set: set) -> set:
    +    """Append memory-provider and context-engine tools onto staged locals.
    +
    +    Mirrors the post-``get_tool_definitions`` injection in ``agent_init`` so a
    +    snapshot rebuild reconstructs the FULL tool surface, not just the
    +    registry-derived subset. Operates ONLY on the caller's staged ``tools_list``
    +    / ``name_set`` (never the live agent attributes) so the rebuild stays atomic.
    +    Idempotent (skips names already present) and fail-soft.
    +
    +    Returns the set of context-engine routing names actually appended by THIS
    +    rebuild — matching ``agent_init``'s dedup behavior (a name already provided
    +    by a registry/plugin tool is NOT claimed for context-engine routing). The
    +    caller publishes this into ``agent._context_engine_tool_names`` atomically
    +    with the snapshot.
    +    """
    +    def _add(schema: dict) -> bool:
    +        name = schema.get("name", "")
    +        if not name or name in name_set:
    +            return False
    +        tools_list.append({"type": "function", "function": schema})
    +        name_set.add(name)
    +        return True
    +
    +    # Memory-provider tools (mem0/honcho/byterover/supermemory/…).
    +    try:
    +        memory_manager = getattr(agent, "_memory_manager", None)
    +        get_mem_schemas = getattr(memory_manager, "get_all_tool_schemas", None) if memory_manager else None
    +        if callable(get_mem_schemas):
    +            # Honor the same enablement gate inject_memory_provider_tools uses.
    +            from agent.memory_manager import memory_provider_tools_enabled
    +            if "memory" in name_set or memory_provider_tools_enabled(getattr(agent, "enabled_toolsets", None)):
    +                for schema in get_mem_schemas():
    +                    if isinstance(schema, dict):
    +                        _add(schema)
    +    except Exception:
    +        logger.debug("Memory-provider tool re-injection skipped", exc_info=True)
    +
    +    # Context-engine tools (lcm_grep/lcm_describe/…) — the `context_engine`
    +    # toolset is intentionally empty, so these only exist via this append.
    +    # Honor the same enabled_toolsets gate agent_init uses (#5544): without it a
    +    # restricted-toolset platform (e.g. platform_toolsets: telegram: []) would
    +    # re-leak lcm_* tools the build deliberately excluded, and pay the local-
    +    # model latency penalty.
    +    staged_engine_names: set = set()
    +    try:
    +        enabled = getattr(agent, "enabled_toolsets", None)
    +        context_engine_allowed = enabled is None or "context_engine" in enabled
    +        compressor = getattr(agent, "context_compressor", None)
    +        get_schemas = getattr(compressor, "get_tool_schemas", None) if compressor else None
    +        if context_engine_allowed and callable(get_schemas):
    +            for schema in get_schemas():
    +                if not isinstance(schema, dict):
    +                    continue
    +                name = schema.get("name", "")
    +                # Only claim the routing name when WE appended the schema, so a
    +                # name already owned by a registry/plugin tool keeps its own
    +                # dispatch (matches agent_init.py's `continue`-before-claim).
    +                if _add(schema) and name:
    +                    staged_engine_names.add(name)
    +    except Exception:
    +        logger.debug("Context-engine tool re-injection skipped", exc_info=True)
    +
    +    return staged_engine_names
    +
    +
     def shutdown_mcp_servers():
         """Close all MCP server connections and stop the background loop.
     

  • tui_gateway/entry.py

    @@ -192,22 +192,32 @@ def _log_exit(reason: str) -> None:
         print(f"[gateway-exit] {reason}", file=sys.stderr, flush=True)
     
     
    -def wait_for_mcp_discovery(timeout: float = 0.75) -> None:
    -    """Briefly block until background MCP discovery finishes, up to ``timeout``.
    +def wait_for_mcp_discovery(timeout: "float | None" = None) -> None:
    +    """Block until background MCP discovery finishes, up to the resolved bound.
     
         MCP discovery runs in a daemon thread spawned at startup (see main()) so a
         slow/dead server can't freeze ``gateway.ready``.  But the agent snapshots
         its tool list ONCE at build time and never re-reads it, so a reachable-but-
         slow server that finishes connecting *after* the first prompt would be
    -    invisible for the whole session.  Joining with a short bounded timeout
    -    before the first agent build lets already-spawning fast servers land
    -    without re-introducing the startup hang: a dead server simply isn't waited
    -    on beyond ``timeout``.  No-op when no discovery thread was started.
    +    invisible for the whole session.  Joining with a bounded timeout before the
    +    first agent build lets already-spawning servers land without re-introducing
    +    the startup hang: ``thread.join(timeout)`` returns the instant discovery
    +    completes (so fast/no-MCP startups pay ~0s), and a dead server is simply not
    +    waited on beyond the bound.  No-op when no discovery thread was started.
    +
    +    The bound comes from ``mcp_discovery_timeout`` in config (shared with the
    +    CLI path via ``hermes_cli.mcp_startup``); ``timeout`` overrides it.
         """
         thread = _mcp_discovery_thread
         if thread is None or not thread.is_alive():
             return
    -    thread.join(timeout=timeout)
    +    try:
    +        from hermes_cli.mcp_startup import _resolve_discovery_timeout
    +
    +        bound = _resolve_discovery_timeout(timeout)
    +    except Exception:
    +        bound = timeout if timeout is not None else 0.75
    +    thread.join(timeout=bound)
     
     
     def mcp_discovery_in_flight() -> bool:

  • tui_gateway/server.py

    @@ -3557,26 +3557,19 @@ def _wait_then_refresh() -> None:
                 ):
                     return
                 try:
    -                from model_tools import get_tool_definitions
    +                from tools.mcp_tool import refresh_agent_mcp_tools
     
    -                new_defs = get_tool_definitions(
    -                    enabled_toolsets=_load_enabled_toolsets(),
    -                    quiet_mode=True,
    -                )
    +                added = refresh_agent_mcp_tools(agent, quiet_mode=True)
                 except Exception as exc:
                     logger.warning(
    -                    "Late MCP refresh: get_tool_definitions failed for %s: %s",
    +                    "Late MCP refresh: tool snapshot rebuild failed for %s: %s",
                         sid,
                         exc,
                     )
                     return
    -            # No change (discovery added nothing new) → don't churn the client.
    -            if len(new_defs or []) == len(getattr(agent, "tools", []) or []):
    +            # No new tools landed (discovery added nothing) → don't churn the client.
    +            if not added:
                     return
    -            agent.tools = new_defs
    -            agent.valid_tool_names = (
    -                {t["function"]["name"] for t in new_defs} if new_defs else set()
    -            )
                 info = _session_info(agent, session)
             # Emit outside the lock — write_json must not block under _sessions_lock.
             _emit("session.info", sid, info)
    @@ -8414,16 +8407,15 @@ def _(rid, params: dict) -> dict:
                 # The user already consented to the prompt-cache invalidation via
                 # the confirm gate above.  Mirrors gateway/run.py::_execute_mcp_reload.
                 try:
    -                from model_tools import get_tool_definitions
    +                from tools.mcp_tool import refresh_agent_mcp_tools
     
    -                new_defs = get_tool_definitions(
    -                    enabled_toolsets=_load_enabled_toolsets(),
    +                # Explicit reload: re-resolve enabled toolsets so a server the
    +                # user just enabled in config this session is picked up.
    +                refresh_agent_mcp_tools(
    +                    agent,
    +                    enabled_override=_load_enabled_toolsets(),
                         quiet_mode=True,
                     )
    -                agent.tools = new_defs
    -                agent.valid_tool_names = (
    -                    {t["function"]["name"] for t in new_defs} if new_defs else set()
    -                )
                 except Exception as _exc:
                     logger.warning(
                         "Failed to refresh cached agent tools after /reload-mcp: %s",

• #75933 fix(mcp): ensure MCP discovery completes before agent build in non-interactive sessions

  merged to main

  files: cli.py, hermes_cli/cli_agent_setup_mixin.py, hermes_cli/config_defaults.py, hermes_cli/mcp_startup.py, hermes_cli/oneshot.py, tests/hermes_cli/test_mcp_discovery_timing.py