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