Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
53 changes: 53 additions & 0 deletions tests/tools/test_mcp_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
57 changes: 51 additions & 6 deletions tools/mcp_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -3186,14 +3192,25 @@ 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
``enabled: false`` are skipped without disconnecting existing sessions.

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.
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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.
"""
Expand All @@ -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

Expand Down
Loading