From 4fda7efcbdbfa92a1fcbfc26ab67aa281064a331 Mon Sep 17 00:00:00 2001 From: fazerluga-creator Date: Sat, 18 Jul 2026 17:55:02 +0300 Subject: [PATCH] fix(mcp): allow background discovery retry after a run that connected nothing start_background_mcp_discovery() sets _mcp_discovery_started once and never resets it. If the first background run exits without connecting any MCP server (startup cancellation, OOM restart, transient network failure), every later call returns immediately and the process is permanently stuck with zero MCP tools until a full restart. Fix: when discovery is marked started but the thread is dead and no server is connected, reset the flag and spawn a fresh discovery thread. Also log a WARNING when a discovery run completes with zero connected servers, so the condition is visible instead of silent. Caught in production on a long-running gateway fleet where a gateway restarted under memory pressure and came back with all MCP tools missing. --- hermes_cli/mcp_startup.py | 40 ++++++++++++++- tests/hermes_cli/test_mcp_startup.py | 73 ++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/hermes_cli/mcp_startup.py b/hermes_cli/mcp_startup.py index ad9996e65b96..a8744161d563 100644 --- a/hermes_cli/mcp_startup.py +++ b/hermes_cli/mcp_startup.py @@ -25,12 +25,35 @@ def _has_configured_mcp_servers() -> bool: def start_background_mcp_discovery(*, logger, thread_name: str) -> None: - """Spawn one shared background MCP discovery thread for this process.""" + """Spawn one shared background MCP discovery thread for this process. + + If the first background discovery run exits without connecting any MCP + server (for example after startup cancellation / OOM restart), later calls + are allowed to retry instead of permanently pinning the process in a + "discovery already started" state with zero MCP tools. + """ global _mcp_discovery_started, _mcp_discovery_thread with _mcp_discovery_lock: if _mcp_discovery_started: - return + thread = _mcp_discovery_thread + if thread is not None and thread.is_alive(): + return + try: + from tools.mcp_tool import get_mcp_status + + status = get_mcp_status() or [] + if any(entry.get("connected") for entry in status): + return + except Exception: + return + logger.warning( + "Background MCP discovery previously exited with no connected " + "servers; retrying discovery thread" + ) + _mcp_discovery_started = False + _mcp_discovery_thread = None + _mcp_discovery_started = True if not _has_configured_mcp_servers(): return @@ -38,8 +61,21 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None: def _discover() -> None: try: _discover_mcp_tools_without_interactive_oauth() + try: + from tools.mcp_tool import get_mcp_status + status = get_mcp_status() or [] + if not any(entry.get("connected") for entry in status): + logger.warning( + "Background MCP discovery completed with zero connected servers" + ) + except Exception: + logger.debug("Failed to inspect MCP status after background discovery", exc_info=True) except Exception: logger.debug("Background MCP tool discovery failed", exc_info=True) + finally: + with _mcp_discovery_lock: + global _mcp_discovery_thread, _mcp_discovery_started + _mcp_discovery_thread = None thread = threading.Thread( target=_discover, diff --git a/tests/hermes_cli/test_mcp_startup.py b/tests/hermes_cli/test_mcp_startup.py index 2dcc40f71261..6b2c2c0f0920 100644 --- a/tests/hermes_cli/test_mcp_startup.py +++ b/tests/hermes_cli/test_mcp_startup.py @@ -222,3 +222,76 @@ def test_init_agent_waits_for_mcp_discovery_before_agent_build(monkeypatch): monkeypatch.setattr(cli_mod, "AIAgent", _fake_agent) assert cli._init_agent() is True + + +def _retry_logger(): + return types.SimpleNamespace( + debug=lambda *_a, **_k: None, + warning=lambda *_a, **_k: None, + ) + + +def _install_retry_stubs(monkeypatch, *, connected: bool, calls: dict): + monkeypatch.setitem( + sys.modules, + "hermes_cli.config", + types.SimpleNamespace( + read_raw_config=lambda: {"mcp_servers": {"demo": {"transport": "stdio"}}}, + ), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_oauth", + types.SimpleNamespace(suppress_interactive_oauth=lambda: nullcontext()), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_tool", + types.SimpleNamespace( + discover_mcp_tools=lambda: calls.__setitem__("mcp", calls["mcp"] + 1), + get_mcp_status=lambda: [{"connected": connected}], + ), + ) + + +def test_background_discovery_retries_after_dead_thread_with_zero_connected(monkeypatch): + """A finished discovery run that connected nothing must not pin the + process in a 'discovery already started' state: the next call should be + allowed to retry (e.g. after startup cancellation or an OOM restart).""" + calls = {"mcp": 0} + _install_retry_stubs(monkeypatch, connected=False, calls=calls) + + mcp_startup.start_background_mcp_discovery( + logger=_retry_logger(), thread_name="test-mcp-retry-1" + ) + thread = mcp_startup._mcp_discovery_thread + if thread is not None: + thread.join(timeout=1.0) + assert calls["mcp"] == 1 + + mcp_startup.start_background_mcp_discovery( + logger=_retry_logger(), thread_name="test-mcp-retry-2" + ) + thread = mcp_startup._mcp_discovery_thread + if thread is not None: + thread.join(timeout=1.0) + assert calls["mcp"] == 2 + + +def test_background_discovery_does_not_retry_when_servers_connected(monkeypatch): + """Once at least one MCP server is connected, repeat calls stay no-ops.""" + calls = {"mcp": 0} + _install_retry_stubs(monkeypatch, connected=True, calls=calls) + + mcp_startup.start_background_mcp_discovery( + logger=_retry_logger(), thread_name="test-mcp-noretry-1" + ) + thread = mcp_startup._mcp_discovery_thread + if thread is not None: + thread.join(timeout=1.0) + assert calls["mcp"] == 1 + + mcp_startup.start_background_mcp_discovery( + logger=_retry_logger(), thread_name="test-mcp-noretry-2" + ) + assert calls["mcp"] == 1