From a052836559683da8f98b538b3aee795aede42fde Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Thu, 24 Sep 2026 23:51:37 -0500 Subject: [PATCH] fix(mcp): connect servers added to config on the next agent build A long-lived backend (Desktop's tui_gateway) runs MCP discovery once. Every agent build re-enters start_background_mcp_discovery, but that returned as soon as any server was connected, so a server added with `hermes mcp add` after startup never reached a new session; only /reload-mcp or a restart picked it up. Re-entry now also runs discovery when an enabled configured server is not live and its connect cooldown has lapsed. Discovery is additive, so live servers and open sessions are untouched; the new session gets the tools. The pending-server computation is factored out of reconcile_mcp_servers_with_config so both callers share it. Refs #76954 --- hermes_cli/mcp_startup.py | 27 ++++++++++++++++----- tests/hermes_cli/test_mcp_startup.py | 35 ++++++++++++++++++++++++++++ tools/mcp_tool_discovery.py | 33 +++++++++++++++++++------- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/hermes_cli/mcp_startup.py b/hermes_cli/mcp_startup.py index 33d9ada840..b1cbbb39f0 100644 --- a/hermes_cli/mcp_startup.py +++ b/hermes_cli/mcp_startup.py @@ -83,11 +83,21 @@ def _any_mcp_connected() -> bool: return _discovery_registered_servers(get_mcp_status() or []) +def _servers_awaiting_connect() -> list[str]: + from tools.mcp_tool_discovery import mcp_servers_awaiting_connect + + pending = mcp_servers_awaiting_connect() + return pending if _mcp_server_filter is None else [n for n in pending if n in _mcp_server_filter] + + def start_background_mcp_discovery(*, logger, thread_name: str) -> None: """Spawn one background MCP discovery thread per profile home. If the first run exits without connecting any server (e.g. startup cancellation / OOM restart), later calls may retry instead of pinning the profile in "already started" with zero MCP tools. + Likewise a server added to ``mcp_servers`` after that run (``hermes mcp add`` against a running + Desktop backend) is connected by the next call, which every agent build makes, so a new session + gets its tools without a reload (#76954). Discovery is additive: live servers are untouched. """ home_key = hermes_home_key() with _mcp_discovery_lock: @@ -96,14 +106,19 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None: if thread is not None and thread.is_alive(): return try: - if _any_mcp_connected(): - return + connected = _any_mcp_connected() + pending = _servers_awaiting_connect() if connected else [] except Exception: return - logger.warning( - "Background MCP discovery previously exited with no connected " - "servers; retrying discovery thread" - ) + if connected and not pending: + return + if connected: + logger.info("MCP server(s) %s not connected yet; running discovery", ", ".join(pending)) + else: + logger.warning( + "Background MCP discovery previously exited with no connected " + "servers; retrying discovery thread" + ) _mcp_discovery_started.discard(home_key) _mcp_discovery_thread.pop(home_key, None) diff --git a/tests/hermes_cli/test_mcp_startup.py b/tests/hermes_cli/test_mcp_startup.py index 6666dba3b6..54c023dc69 100644 --- a/tests/hermes_cli/test_mcp_startup.py +++ b/tests/hermes_cli/test_mcp_startup.py @@ -465,3 +465,38 @@ def test_lazy_only_discovery_counts_as_usable_at_both_startup_sites(monkeypatch, assert calls["mcp"] == (2 if retried else 1) assert any("zero connected" in w for w in warnings) is retried assert any("retrying discovery thread" in w for w in warnings) is retried + + +def test_server_added_after_discovery_is_connected_by_the_next_agent_build(monkeypatch, tmp_path): + """#76954: ``hermes mcp add`` against a running Desktop backend. Discovery already ran and left + ``github`` live, so the re-entry every agent build makes returned early and the new server's + tools never reached a new session. It must run discovery again, and only while something + configured is still unconnected.""" + from tools import mcp_tool + from tools import mcp_tool_config as _config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "config.yaml").write_text("mcp_servers:\n github:\n url: https://mcp.example.test/gh\n") + configured = {"github": {"url": "https://mcp.example.test/gh"}} + monkeypatch.setattr(_config, "_load_mcp_config", lambda: dict(configured)) + monkeypatch.setitem(mcp_tool._servers, "github", object()) + monkeypatch.setitem(mcp_tool._server_scope_keys, "github", None) + monkeypatch.setattr(mcp_startup, "_any_mcp_connected", lambda: True) + runs: list = [] + monkeypatch.setattr(mcp_startup, "_discover_mcp_tools_without_interactive_oauth", lambda: runs.append(1)) + logger = types.SimpleNamespace(debug=lambda *_a, **_k: None, info=lambda *_a, **_k: None, + warning=lambda *_a, **_k: None) + + def build_agent(): + mcp_startup.start_background_mcp_discovery(logger=logger, thread_name="t") + thread = mcp_startup._current_home_thread() + if thread is not None: + thread.join(timeout=5.0) + + build_agent() # backend start + build_agent() # new session, config unchanged: github is live, nothing to do + assert len(runs) == 1 + + configured["linear"] = {"url": "https://mcp.example.test/linear"} # hermes mcp add linear + build_agent() + assert len(runs) == 2 diff --git a/tools/mcp_tool_discovery.py b/tools/mcp_tool_discovery.py index 0ae4be65a7..a2aacbbfb8 100644 --- a/tools/mcp_tool_discovery.py +++ b/tools/mcp_tool_discovery.py @@ -10,7 +10,7 @@ import logging import time from contextlib import contextmanager from pathlib import Path -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Set, Tuple from tools.mcp_tool_common import _core, _parse_boolish, mcp_server_enabled from tools import mcp_tool_config as _config from tools import mcp_tool_errors as _errors @@ -629,6 +629,14 @@ def reconcile_mcp_servers_with_config() -> Dict[str, List[str]]: _lifecycle.shutdown_mcp_servers(scope=scope, names=set(stale)) for key in lazy: _forget_lazy_server(key) + added = _awaiting_connect(wanted, scope) + if added: + discover_mcp_tools() + return {"removed": stale + sorted(_key_name(k) for k in lazy), "added": added, + "pending": sorted(connecting - wanted)} + + +def _awaiting_connect(wanted: Set[str], scope) -> List[str]: with _core._lock: # Same resolution ``_select_new_servers`` applies: this scope's own connection OR a shared # one it adopted from another profile counts as live. Owner==scope alone misses the adopted @@ -637,15 +645,22 @@ def reconcile_mcp_servers_with_config() -> Dict[str, List[str]]: known = {name for name in wanted if (key := _resolve_server_key(name, scope, current=False)) in _core._servers or key in _core._server_connecting or key in _core._lazy_server_configs} - # A configured server that is not live is retried here — this is the only reviver for one whose - # FIRST connect failed (#112445) — but only once its connect cooldown lapsed: ``discover_mcp_tools`` + # A configured server that is not live is retried — the only way one whose FIRST connect + # failed comes back (#112445) — but only once its connect cooldown lapsed: ``discover_mcp_tools`` # would skip it anyway, and entering it takes the cross-process discovery lock (up to 120 s of - # waiting when another process holds it) and logs a failed pass, every tick, for nothing. - added = sorted(name for name in wanted - known if not _connect_cooldown_active(name)) - if added: - discover_mcp_tools() - return {"removed": stale + sorted(_key_name(k) for k in lazy), "added": added, - "pending": sorted(connecting - wanted)} + # waiting when another process holds it) and logs a failed pass, every call, for nothing. + return sorted(name for name in wanted - known if not _connect_cooldown_active(name)) + + +def mcp_servers_awaiting_connect() -> List[str]: + """Enabled ``mcp_servers`` entries the current registry scope has no live, connecting or lazy + registration for, whose connect cooldown has lapsed: added to config since the last discovery + run, or one whose earlier connect failed. Read-only — what the next :func:`discover_mcp_tools` + would connect.""" + with _owner_secret_scope(): + servers = _config._load_mcp_config() + wanted = {name for name, cfg in servers.items() if mcp_server_enabled(cfg)} + return _awaiting_connect(wanted, _core._mcp_registry_scope()) def _forget_lazy_server(key) -> None: