From e5637e8477015552cefc3d274df953b1a19e5ea8 Mon Sep 17 00:00:00 2001 From: LeonSGP43 Date: Thu, 21 May 2026 18:02:05 +0800 Subject: [PATCH] Soft-fail slow MCP discovery at startup --- gateway/run.py | 8 ++++- hermes_cli/main.py | 2 +- tests/tools/test_mcp_tool.py | 53 +++++++++++++++++++++++++++++++++ tools/mcp_tool.py | 57 ++++++++++++++++++++++++++++++++---- 4 files changed, 112 insertions(+), 8 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index cca9901cb426..4ee8ee265f4c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -18081,7 +18081,13 @@ def restart_signal_handler(): try: from tools.mcp_tool import discover_mcp_tools _loop = asyncio.get_running_loop() - await _loop.run_in_executor(None, discover_mcp_tools) + await _loop.run_in_executor( + None, + lambda: discover_mcp_tools( + startup_timeout=5.0, + continue_in_background=True, + ), + ) except Exception as e: logger.debug("MCP tool discovery failed: %s", e) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 925f93e77c6b..7a948bf96729 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -13354,7 +13354,7 @@ def cmd_acp(args): # via the same lazy import path (#16856). from tools.mcp_tool import discover_mcp_tools - discover_mcp_tools() + discover_mcp_tools(startup_timeout=5.0, continue_in_background=True) except Exception: logger.debug( "MCP tool discovery failed at CLI startup", diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 3212a350c374..f67558aa3683 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -3214,6 +3214,59 @@ async def selective_register(name, cfg): _servers.pop("fail1", None) +class TestMCPStartupSoftFail: + """Startup-time discovery should not block Hermes on slow optional MCPs.""" + + def test_discover_timeout_continues_in_background(self): + from tools.mcp_tool import discover_mcp_tools, _servers, _ensure_mcp_loop + + fake_config = { + "slow_server": {"command": "npx", "args": ["slow"]}, + } + + async def slow_register(name, cfg): + await asyncio.sleep(0.05) + from tools.mcp_tool import MCPServerTask + server = MCPServerTask(name) + server.session = MagicMock() + server._registered_tool_names = ["mcp_slow_server_tool_a"] + _servers[name] = server + return server._registered_tool_names + + with patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ + patch("tools.mcp_tool._discover_and_register_server", side_effect=slow_register), \ + patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._existing_tool_names", side_effect=lambda: [ + "mcp_slow_server_tool_a" + ] if "slow_server" in _servers else []): + _ensure_mcp_loop() + + start = time.monotonic() + with patch("tools.mcp_tool.logger") as mock_logger: + result = discover_mcp_tools( + startup_timeout=0.01, + continue_in_background=True, + ) + elapsed = time.monotonic() - start + + assert elapsed < 0.05, ( + f"startup discovery should return quickly, took {elapsed:.3f}s" + ) + assert result == [] + warning_calls = [str(call) for call in mock_logger.warning.call_args_list] + assert any("continuing startup" in call for call in warning_calls), ( + f"expected startup soft-fail warning, got: {warning_calls}" + ) + + deadline = time.monotonic() + 1.0 + while "slow_server" not in _servers and time.monotonic() < deadline: + time.sleep(0.01) + + assert "slow_server" in _servers, "background discovery never finished" + + _servers.pop("slow_server", None) + + class TestMCPSelectiveToolLoading: """Tests for per-server MCP filtering and utility tool policies.""" diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index e50efc05a0c2..57812d8cd026 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -2155,7 +2155,12 @@ def _ensure_mcp_loop(): _mcp_thread.start() -def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): +def _run_on_mcp_loop( + coro_or_factory, + timeout: float = 30, + *, + cancel_on_timeout: bool = True, +): """Schedule a coroutine on the MCP event loop and block until done. Accepts either a coroutine object or a zero-arg callable that returns one. @@ -2196,7 +2201,8 @@ def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): if deadline is not None: remaining = deadline - time.monotonic() if remaining <= 0: - future.cancel() + if cancel_on_timeout: + future.cancel() elapsed = time.monotonic() - start_time raise TimeoutError( f"MCP call timed out after {elapsed:.1f}s " @@ -3186,7 +3192,12 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: # Public API # --------------------------------------------------------------------------- -def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: +def register_mcp_servers( + servers: Dict[str, dict], + *, + startup_timeout: Optional[float] = None, + continue_in_background: bool = False, +) -> List[str]: """Connect to explicit MCP servers and register their tools. Idempotent for already-connected server names. Servers with @@ -3194,6 +3205,12 @@ def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: Args: servers: Mapping of ``{server_name: server_config}``. + startup_timeout: Optional bounded wait for startup-time discovery. + When omitted, waits for the full discovery window. + continue_in_background: When True and ``startup_timeout`` expires, + keep discovery running on the MCP loop and return immediately so + Hermes startup is not blocked by a slow or unhealthy optional MCP + server. Returns: List of all currently registered MCP tool names. @@ -3258,8 +3275,23 @@ async def _discover_all(): _was_interrupted = _is_interrupted() if _was_interrupted: _set_interrupt(False) + discovery_wait = 120 if startup_timeout is None else float(startup_timeout) try: - _run_on_mcp_loop(_discover_all, timeout=120) + _run_on_mcp_loop( + _discover_all, + timeout=discovery_wait, + cancel_on_timeout=not continue_in_background, + ) + except TimeoutError: + if not continue_in_background: + raise + logger.warning( + "MCP discovery still running after %.1fs; continuing startup " + "without waiting for: %s", + discovery_wait, + ", ".join(sorted(new_servers)), + ) + return _existing_tool_names() finally: if _was_interrupted: _set_interrupt(True) @@ -3281,7 +3313,11 @@ async def _discover_all(): return _existing_tool_names() -def discover_mcp_tools() -> List[str]: +def discover_mcp_tools( + *, + startup_timeout: Optional[float] = None, + continue_in_background: bool = False, +) -> List[str]: """Entry point: load config, connect to MCP servers, register tools. Called from ``model_tools`` after ``discover_builtin_tools()``. Safe to call even when @@ -3290,6 +3326,11 @@ def discover_mcp_tools() -> List[str]: Idempotent for already-connected servers. If some servers failed on a previous call, only the missing ones are retried. + Args: + startup_timeout: Optional bounded wait for startup discovery. + continue_in_background: When True and ``startup_timeout`` expires, + continue startup while discovery finishes on the MCP loop. + Returns: List of all registered MCP tool names. """ @@ -3309,7 +3350,11 @@ def discover_mcp_tools() -> List[str]: if name not in _servers and _parse_boolish(cfg.get("enabled", True), default=True) ] - tool_names = register_mcp_servers(servers) + tool_names = register_mcp_servers( + servers, + startup_timeout=startup_timeout, + continue_in_background=continue_in_background, + ) if not new_server_names: return tool_names