From 3c252ae44b524ed20861e681b14c4d66a6fb4bdf Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:03:14 +0300 Subject: [PATCH 01/12] feat: add MCP (Model Context Protocol) client support Connect to external MCP servers via stdio transport, discover their tools at startup, and register them into the hermes-agent tool registry. - New tools/mcp_tool.py: config loading, server connection via background event loop, tool handler factories, discovery, and graceful shutdown - model_tools.py: trigger MCP discovery after built-in tool imports - cli.py: call shutdown_mcp_servers in _run_cleanup - pyproject.toml: add mcp>=1.2.0 as optional dependency - 27 unit tests covering config, schema conversion, handlers, registration, SDK interaction, toolset injection, graceful fallback, and shutdown Config format (in ~/.hermes/config.yaml): mcp_servers: filesystem: command: "npx" args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] --- cli.py | 5 + model_tools.py | 7 + pyproject.toml | 2 + tests/tools/test_mcp_tool.py | 588 +++++++++++++++++++++++++++++++++++ tools/mcp_tool.py | 380 ++++++++++++++++++++++ uv.lock | 82 ++++- 6 files changed, 1063 insertions(+), 1 deletion(-) create mode 100644 tests/tools/test_mcp_tool.py create mode 100644 tools/mcp_tool.py diff --git a/cli.py b/cli.py index faa6586d..a2519460 100755 --- a/cli.py +++ b/cli.py @@ -386,6 +386,11 @@ def _run_cleanup(): _cleanup_all_browsers() except Exception: pass + try: + from tools.mcp_tool import shutdown_mcp_servers + shutdown_mcp_servers() + except Exception: + pass # ============================================================================ # ASCII Art & Branding diff --git a/model_tools.py b/model_tools.py index 036bb34b..8da3d67e 100644 --- a/model_tools.py +++ b/model_tools.py @@ -105,6 +105,13 @@ def _discover_tools(): _discover_tools() +# MCP tool discovery (external MCP servers from config) +try: + from tools.mcp_tool import discover_mcp_tools + discover_mcp_tools() +except Exception as e: + logger.debug("MCP tool discovery failed: %s", e) + # ============================================================================= # Backward-compat constants (built once after discovery) diff --git a/pyproject.toml b/pyproject.toml index 152b4730..2f241b3a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,7 @@ cli = ["simple-term-menu"] tts-premium = ["elevenlabs"] pty = ["ptyprocess>=0.7.0"] honcho = ["honcho-ai>=2.0.1"] +mcp = ["mcp>=1.2.0"] all = [ "hermes-agent[modal]", "hermes-agent[messaging]", @@ -57,6 +58,7 @@ all = [ "hermes-agent[slack]", "hermes-agent[pty]", "hermes-agent[honcho]", + "hermes-agent[mcp]", ] [project.scripts] diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py new file mode 100644 index 00000000..caaffd48 --- /dev/null +++ b/tests/tools/test_mcp_tool.py @@ -0,0 +1,588 @@ +"""Tests for the MCP (Model Context Protocol) client support. + +All tests use mocks -- no real MCP servers or subprocesses are started. +""" + +import asyncio +import json +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_mcp_tool(name="read_file", description="Read a file", input_schema=None): + """Create a fake MCP Tool object matching the SDK interface.""" + tool = SimpleNamespace() + tool.name = name + tool.description = description + tool.inputSchema = input_schema or { + "type": "object", + "properties": { + "path": {"type": "string", "description": "File path"}, + }, + "required": ["path"], + } + return tool + + +def _make_call_result(text="file contents here", is_error=False): + """Create a fake MCP CallToolResult.""" + block = SimpleNamespace(text=text) + return SimpleNamespace(content=[block], isError=is_error) + + +# --------------------------------------------------------------------------- +# Config loading +# --------------------------------------------------------------------------- + +class TestLoadMCPConfig: + def test_no_config_returns_empty(self): + """No mcp_servers key in config -> empty dict.""" + with patch("tools.mcp_tool.load_config", create=True) as mock_lc: + # Patch the actual import inside the function + with patch("hermes_cli.config.load_config", return_value={"model": "test"}): + from tools.mcp_tool import _load_mcp_config + result = _load_mcp_config() + assert result == {} + + def test_valid_config_parsed(self): + """Valid mcp_servers config is returned as-is.""" + servers = { + "filesystem": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + "env": {}, + } + } + with patch("hermes_cli.config.load_config", return_value={"mcp_servers": servers}): + from tools.mcp_tool import _load_mcp_config + result = _load_mcp_config() + assert "filesystem" in result + assert result["filesystem"]["command"] == "npx" + + def test_mcp_servers_not_dict_returns_empty(self): + """mcp_servers set to non-dict value -> empty dict.""" + with patch("hermes_cli.config.load_config", return_value={"mcp_servers": "invalid"}): + from tools.mcp_tool import _load_mcp_config + result = _load_mcp_config() + assert result == {} + + +# --------------------------------------------------------------------------- +# Schema conversion +# --------------------------------------------------------------------------- + +class TestSchemaConversion: + def test_converts_mcp_tool_to_hermes_schema(self): + from tools.mcp_tool import _convert_mcp_schema + + mcp_tool = _make_mcp_tool(name="read_file", description="Read a file") + schema = _convert_mcp_schema("filesystem", mcp_tool) + + assert schema["name"] == "mcp_filesystem_read_file" + assert schema["description"] == "Read a file" + assert "properties" in schema["parameters"] + + def test_empty_input_schema_gets_default(self): + from tools.mcp_tool import _convert_mcp_schema + + mcp_tool = _make_mcp_tool(name="ping", description="Ping", input_schema=None) + mcp_tool.inputSchema = None + schema = _convert_mcp_schema("test", mcp_tool) + + assert schema["parameters"]["type"] == "object" + assert schema["parameters"]["properties"] == {} + + def test_tool_name_prefix_format(self): + from tools.mcp_tool import _convert_mcp_schema + + mcp_tool = _make_mcp_tool(name="list_dir") + schema = _convert_mcp_schema("my_server", mcp_tool) + + assert schema["name"] == "mcp_my_server_list_dir" + + def test_hyphens_sanitized_to_underscores(self): + """Hyphens in tool/server names are replaced with underscores for LLM compat.""" + from tools.mcp_tool import _convert_mcp_schema + + mcp_tool = _make_mcp_tool(name="get-sum") + schema = _convert_mcp_schema("my-server", mcp_tool) + + assert schema["name"] == "mcp_my_server_get_sum" + assert "-" not in schema["name"] + + +# --------------------------------------------------------------------------- +# Check function +# --------------------------------------------------------------------------- + +class TestCheckFunction: + def test_disconnected_returns_false(self): + from tools.mcp_tool import _make_check_fn, _connections + + # Ensure no connection exists + _connections.pop("test_server", None) + check = _make_check_fn("test_server") + assert check() is False + + def test_connected_returns_true(self): + from tools.mcp_tool import _make_check_fn, _connections, MCPConnection + + conn = MCPConnection( + server_name="test_server", + session=MagicMock(), + stack=MagicMock(), + ) + _connections["test_server"] = conn + try: + check = _make_check_fn("test_server") + assert check() is True + finally: + _connections.pop("test_server", None) + + def test_session_none_returns_false(self): + from tools.mcp_tool import _make_check_fn, _connections, MCPConnection + + conn = MCPConnection( + server_name="test_server", + session=None, + stack=MagicMock(), + ) + _connections["test_server"] = conn + try: + check = _make_check_fn("test_server") + assert check() is False + finally: + _connections.pop("test_server", None) + + +# --------------------------------------------------------------------------- +# Tool handler (async) +# --------------------------------------------------------------------------- + +class TestToolHandler: + """Tool handlers are sync functions that schedule work on the MCP loop.""" + + def _patch_mcp_loop(self, coro_side_effect=None): + """Return a patch for _run_on_mcp_loop that runs the coroutine directly.""" + def fake_run(coro, timeout=30): + return asyncio.get_event_loop().run_until_complete(coro) + if coro_side_effect: + return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=coro_side_effect) + return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=fake_run) + + def test_successful_call(self): + from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + + mock_session = MagicMock() + mock_session.call_tool = AsyncMock( + return_value=_make_call_result("hello world", is_error=False) + ) + conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) + _connections["test_srv"] = conn + + try: + handler = _make_tool_handler("test_srv", "greet") + with self._patch_mcp_loop(): + result = json.loads(handler({"name": "world"})) + assert result["result"] == "hello world" + mock_session.call_tool.assert_called_once_with("greet", arguments={"name": "world"}) + finally: + _connections.pop("test_srv", None) + + def test_mcp_error_result(self): + from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + + mock_session = MagicMock() + mock_session.call_tool = AsyncMock( + return_value=_make_call_result("something went wrong", is_error=True) + ) + conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) + _connections["test_srv"] = conn + + try: + handler = _make_tool_handler("test_srv", "fail_tool") + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert "error" in result + assert "something went wrong" in result["error"] + finally: + _connections.pop("test_srv", None) + + def test_disconnected_server(self): + from tools.mcp_tool import _make_tool_handler, _connections + + _connections.pop("ghost", None) + handler = _make_tool_handler("ghost", "any_tool") + # Disconnected check happens before _run_on_mcp_loop, no patch needed + result = json.loads(handler({})) + assert "error" in result + assert "not connected" in result["error"] + + def test_exception_during_call(self): + from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + + mock_session = MagicMock() + mock_session.call_tool = AsyncMock(side_effect=RuntimeError("connection lost")) + conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) + _connections["test_srv"] = conn + + try: + handler = _make_tool_handler("test_srv", "broken_tool") + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert "error" in result + assert "connection lost" in result["error"] + finally: + _connections.pop("test_srv", None) + + +# --------------------------------------------------------------------------- +# Tool registration (discovery + register) +# --------------------------------------------------------------------------- + +class TestDiscoverAndRegister: + def test_tools_registered_in_registry(self): + """_discover_and_register_server registers tools with correct names.""" + from tools.registry import ToolRegistry, registry as real_registry + from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + + mock_registry = ToolRegistry() + mock_tools = [ + _make_mcp_tool("read_file", "Read a file"), + _make_mcp_tool("write_file", "Write a file"), + ] + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) + ) + + async def fake_connect(name, config): + return MCPConnection(name, session=mock_session, stack=MagicMock()) + + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.registry.registry", mock_registry): + registered = asyncio.run( + _discover_and_register_server("fs", {"command": "npx", "args": []}) + ) + + assert "mcp_fs_read_file" in registered + assert "mcp_fs_write_file" in registered + assert "mcp_fs_read_file" in mock_registry.get_all_tool_names() + assert "mcp_fs_write_file" in mock_registry.get_all_tool_names() + + _connections.pop("fs", None) + + def test_toolset_created(self): + """A custom toolset is created for the MCP server.""" + from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + + mock_tools = [_make_mcp_tool("ping", "Ping")] + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) + ) + + async def fake_connect(name, config): + return MCPConnection(name, session=mock_session, stack=MagicMock()) + + mock_create = MagicMock() + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("toolsets.create_custom_toolset", mock_create): + asyncio.run( + _discover_and_register_server("myserver", {"command": "test"}) + ) + + mock_create.assert_called_once() + call_kwargs = mock_create.call_args + assert call_kwargs[1]["name"] == "mcp-myserver" or call_kwargs[0][0] == "mcp-myserver" + + _connections.pop("myserver", None) + + def test_schema_format_correct(self): + """Registered schemas have the correct format.""" + from tools.registry import ToolRegistry, registry as real_registry + from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + + mock_registry = ToolRegistry() + mock_tools = [_make_mcp_tool("do_thing", "Do something")] + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) + ) + + async def fake_connect(name, config): + return MCPConnection(name, session=mock_session, stack=MagicMock()) + + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.registry.registry", mock_registry): + asyncio.run( + _discover_and_register_server("srv", {"command": "test"}) + ) + + entry = mock_registry._tools.get("mcp_srv_do_thing") + assert entry is not None + assert entry.schema["name"] == "mcp_srv_do_thing" + assert "parameters" in entry.schema + assert entry.is_async is False + assert entry.toolset == "mcp-srv" + + _connections.pop("srv", None) + + +# --------------------------------------------------------------------------- +# _connect_server (SDK interaction) +# --------------------------------------------------------------------------- + +class TestConnectServer: + def test_calls_sdk_with_correct_params(self): + """_connect_server creates StdioServerParameters and calls stdio_client.""" + from tools.mcp_tool import _connect_server, MCPConnection + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + + mock_read = MagicMock() + mock_write = MagicMock() + + with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ + patch("tools.mcp_tool.stdio_client") as mock_stdio, \ + patch("tools.mcp_tool.ClientSession") as mock_cs, \ + patch("tools.mcp_tool.AsyncExitStack") as mock_stack_cls: + + mock_stack = MagicMock() + mock_stack.enter_async_context = AsyncMock( + side_effect=[(mock_read, mock_write), mock_session] + ) + mock_stack_cls.return_value = mock_stack + + conn = asyncio.run(_connect_server("test_srv", { + "command": "npx", + "args": ["-y", "some-server"], + "env": {"MY_KEY": "secret"}, + })) + + # StdioServerParameters called with correct values + mock_params.assert_called_once_with( + command="npx", + args=["-y", "some-server"], + env={"MY_KEY": "secret"}, + ) + # ClientSession created with the streams + mock_cs.assert_called_once_with(mock_read, mock_write) + # initialize() was called + mock_session.initialize.assert_called_once() + # Returned connection is valid + assert conn.server_name == "test_srv" + assert conn.session is mock_session + + def test_no_command_raises(self): + """Missing 'command' in config raises ValueError.""" + from tools.mcp_tool import _connect_server + + with pytest.raises(ValueError, match="no 'command'"): + asyncio.run(_connect_server("bad", {"args": []})) + + def test_empty_env_passed_as_none(self): + """Empty env dict is passed as None to StdioServerParameters.""" + from tools.mcp_tool import _connect_server + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + + with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ + patch("tools.mcp_tool.stdio_client"), \ + patch("tools.mcp_tool.ClientSession", return_value=mock_session), \ + patch("tools.mcp_tool.AsyncExitStack") as mock_stack_cls: + + mock_stack = MagicMock() + mock_stack.enter_async_context = AsyncMock( + side_effect=[ + (MagicMock(), MagicMock()), + mock_session, + ] + ) + mock_stack_cls.return_value = mock_stack + + asyncio.run(_connect_server("srv", { + "command": "node", + "env": {}, + })) + + # Empty dict -> None + assert mock_params.call_args[1]["env"] is None or \ + mock_params.call_args.kwargs.get("env") is None + + +# --------------------------------------------------------------------------- +# discover_mcp_tools toolset injection +# --------------------------------------------------------------------------- + +class TestToolsetInjection: + def test_mcp_tools_added_to_platform_toolsets(self): + """Discovered MCP tools are injected into hermes-cli and platform toolsets.""" + from tools.mcp_tool import _connections, MCPConnection + + mock_tools = [_make_mcp_tool("list_files", "List files")] + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) + ) + + async def fake_connect(name, config): + return MCPConnection(name, session=mock_session, stack=MagicMock()) + + fake_toolsets = { + "hermes-cli": {"tools": ["terminal", "web_search"], "description": "CLI", "includes": []}, + "hermes-telegram": {"tools": ["terminal"], "description": "Telegram", "includes": []}, + } + fake_config = { + "fs": {"command": "npx", "args": []}, + } + + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ + patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.mcp_tool.TOOLSETS", fake_toolsets, create=True), \ + patch("toolsets.TOOLSETS", fake_toolsets): + from tools.mcp_tool import discover_mcp_tools + result = discover_mcp_tools() + + assert "mcp_fs_list_files" in result + assert "mcp_fs_list_files" in fake_toolsets["hermes-cli"]["tools"] + assert "mcp_fs_list_files" in fake_toolsets["hermes-telegram"]["tools"] + # Original tools preserved + assert "terminal" in fake_toolsets["hermes-cli"]["tools"] + + _connections.pop("fs", None) + + def test_server_connection_failure_skipped(self): + """If one server fails to connect, others still proceed.""" + from tools.mcp_tool import _connections, MCPConnection + + mock_tools = [_make_mcp_tool("ping", "Ping")] + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) + ) + + call_count = 0 + + async def flaky_connect(name, config): + nonlocal call_count + call_count += 1 + if name == "broken": + raise ConnectionError("cannot reach server") + return MCPConnection(name, session=mock_session, stack=MagicMock()) + + fake_config = { + "broken": {"command": "bad"}, + "good": {"command": "npx", "args": []}, + } + fake_toolsets = { + "hermes-cli": {"tools": [], "description": "CLI", "includes": []}, + } + + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ + patch("tools.mcp_tool._connect_server", side_effect=flaky_connect), \ + patch("toolsets.TOOLSETS", fake_toolsets): + from tools.mcp_tool import discover_mcp_tools + result = discover_mcp_tools() + + # Only good server's tool registered + assert "mcp_good_ping" in result + assert "mcp_broken_ping" not in result + assert call_count == 2 # Both were attempted + + _connections.pop("good", None) + + +# --------------------------------------------------------------------------- +# Graceful fallback +# --------------------------------------------------------------------------- + +class TestGracefulFallback: + def test_mcp_unavailable_returns_empty(self): + """When _MCP_AVAILABLE is False, discover_mcp_tools is a no-op.""" + with patch("tools.mcp_tool._MCP_AVAILABLE", False): + from tools.mcp_tool import discover_mcp_tools + result = discover_mcp_tools() + assert result == [] + + def test_no_servers_returns_empty(self): + """No MCP servers configured -> empty list.""" + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._load_mcp_config", return_value={}): + from tools.mcp_tool import discover_mcp_tools + result = discover_mcp_tools() + assert result == [] + + +# --------------------------------------------------------------------------- +# Shutdown +# --------------------------------------------------------------------------- + +class TestShutdown: + def test_no_connections_safe(self): + """shutdown_mcp_servers with no connections does nothing.""" + from tools.mcp_tool import shutdown_mcp_servers, _connections + + _connections.clear() + shutdown_mcp_servers() # Should not raise + + def test_shutdown_clears_connections(self): + """shutdown_mcp_servers closes stacks and clears the dict.""" + import tools.mcp_tool as mcp_mod + from tools.mcp_tool import shutdown_mcp_servers, _connections, MCPConnection + + _connections.clear() + mock_stack = MagicMock() + mock_stack.aclose = AsyncMock() + conn = MCPConnection("test", session=MagicMock(), stack=mock_stack) + _connections["test"] = conn + + # Start a real background loop so shutdown can schedule on it + mcp_mod._ensure_mcp_loop() + try: + shutdown_mcp_servers() + finally: + # _stop_mcp_loop is called by shutdown, but ensure cleanup + mcp_mod._mcp_loop = None + mcp_mod._mcp_thread = None + + assert len(_connections) == 0 + mock_stack.aclose.assert_called_once() + + def test_shutdown_handles_errors(self): + """shutdown_mcp_servers handles errors during close gracefully.""" + import tools.mcp_tool as mcp_mod + from tools.mcp_tool import shutdown_mcp_servers, _connections, MCPConnection + + _connections.clear() + mock_stack = MagicMock() + mock_stack.aclose = AsyncMock(side_effect=RuntimeError("close failed")) + conn = MCPConnection("broken", session=MagicMock(), stack=mock_stack) + _connections["broken"] = conn + + mcp_mod._ensure_mcp_loop() + try: + shutdown_mcp_servers() # Should not raise + finally: + mcp_mod._mcp_loop = None + mcp_mod._mcp_thread = None + + assert len(_connections) == 0 diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py new file mode 100644 index 00000000..eecbaa29 --- /dev/null +++ b/tools/mcp_tool.py @@ -0,0 +1,380 @@ +#!/usr/bin/env python3 +""" +MCP (Model Context Protocol) Client Support + +Connects to external MCP servers via stdio transport, discovers their tools, +and registers them into the hermes-agent tool registry so the agent can call +them like any built-in tool. + +Configuration is read from ~/.hermes/config.yaml under the ``mcp_servers`` key. +The ``mcp`` Python package is optional -- if not installed, this module is a +no-op and logs a debug message. + +Example config:: + + mcp_servers: + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + env: {} + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_..." + +Architecture: + A dedicated background event loop (_mcp_loop) runs in a daemon thread. + All MCP connections live on this loop. Tool handlers schedule coroutines + onto it via run_coroutine_threadsafe(), so they work from any thread. +""" + +import asyncio +import json +import logging +import threading +from typing import Any, Dict, List, Optional + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Graceful import -- MCP SDK is an optional dependency +# --------------------------------------------------------------------------- + +_MCP_AVAILABLE = False +try: + from mcp import ClientSession, StdioServerParameters + from mcp.client.stdio import stdio_client + from contextlib import AsyncExitStack + _MCP_AVAILABLE = True +except ImportError: + logger.debug("mcp package not installed -- MCP tool support disabled") + + +# --------------------------------------------------------------------------- +# Connection tracking +# --------------------------------------------------------------------------- + +class MCPConnection: + """Holds a live MCP server connection and its async resource stack.""" + + __slots__ = ("server_name", "session", "stack") + + def __init__(self, server_name: str, session: Any, stack: Any): + self.server_name = server_name + self.session: Optional[Any] = session + self.stack: Optional[Any] = stack + + +_connections: Dict[str, MCPConnection] = {} + +# Dedicated event loop running in a background daemon thread. +# All MCP async operations (connect, call_tool, shutdown) run here. +_mcp_loop: Optional[asyncio.AbstractEventLoop] = None +_mcp_thread: Optional[threading.Thread] = None + + +def _ensure_mcp_loop(): + """Start the background event loop thread if not already running.""" + global _mcp_loop, _mcp_thread + if _mcp_loop is not None and _mcp_loop.is_running(): + return + _mcp_loop = asyncio.new_event_loop() + _mcp_thread = threading.Thread( + target=_mcp_loop.run_forever, + name="mcp-event-loop", + daemon=True, + ) + _mcp_thread.start() + + +def _run_on_mcp_loop(coro, timeout: float = 30): + """Schedule a coroutine on the MCP event loop and block until done.""" + if _mcp_loop is None or not _mcp_loop.is_running(): + raise RuntimeError("MCP event loop is not running") + future = asyncio.run_coroutine_threadsafe(coro, _mcp_loop) + return future.result(timeout=timeout) + + +# --------------------------------------------------------------------------- +# Config loading +# --------------------------------------------------------------------------- + +def _load_mcp_config() -> Dict[str, dict]: + """Read ``mcp_servers`` from the Hermes config file. + + Returns a dict of ``{server_name: {command, args, env}}`` or empty dict. + """ + try: + from hermes_cli.config import load_config + config = load_config() + servers = config.get("mcp_servers") + if not servers or not isinstance(servers, dict): + return {} + return servers + except Exception as exc: + logger.debug("Failed to load MCP config: %s", exc) + return {} + + +# --------------------------------------------------------------------------- +# Server connection +# --------------------------------------------------------------------------- + +async def _connect_server(name: str, config: dict) -> MCPConnection: + """Start an MCP server subprocess and initialize a ClientSession. + + Args: + name: Logical server name (e.g. "filesystem"). + config: Dict with ``command``, ``args``, and optional ``env``. + + Returns: + An ``MCPConnection`` with a live session. + + Raises: + Exception on connection or initialization failure. + """ + command = config.get("command") + args = config.get("args", []) + env = config.get("env") + + if not command: + raise ValueError(f"MCP server '{name}' has no 'command' in config") + + server_params = StdioServerParameters( + command=command, + args=args, + env=env if env else None, + ) + + stack = AsyncExitStack() + stdio_transport = await stack.enter_async_context(stdio_client(server_params)) + read_stream, write_stream = stdio_transport + session = await stack.enter_async_context(ClientSession(read_stream, write_stream)) + await session.initialize() + + return MCPConnection(server_name=name, session=session, stack=stack) + + +# --------------------------------------------------------------------------- +# Handler / check-fn factories +# --------------------------------------------------------------------------- + +def _make_tool_handler(server_name: str, tool_name: str): + """Return a sync handler that calls an MCP tool via the background loop. + + The handler conforms to the registry's dispatch interface: + ``handler(args_dict, **kwargs) -> str`` + """ + + def _handler(args: dict, **kwargs) -> str: + conn = _connections.get(server_name) + if not conn or not conn.session: + return json.dumps({ + "error": f"MCP server '{server_name}' is not connected" + }) + + async def _call(): + result = await conn.session.call_tool(tool_name, arguments=args) + # MCP CallToolResult has .content (list of content blocks) and .isError + if result.isError: + error_text = "" + for block in (result.content or []): + if hasattr(block, "text"): + error_text += block.text + return json.dumps({"error": error_text or "MCP tool returned an error"}) + + # Collect text from content blocks + parts: List[str] = [] + for block in (result.content or []): + if hasattr(block, "text"): + parts.append(block.text) + return json.dumps({"result": "\n".join(parts) if parts else ""}) + + try: + return _run_on_mcp_loop(_call(), timeout=120) + except Exception as exc: + logger.error("MCP tool %s/%s call failed: %s", server_name, tool_name, exc) + return json.dumps({"error": f"MCP call failed: {type(exc).__name__}: {exc}"}) + + return _handler + + +def _make_check_fn(server_name: str): + """Return a check function that verifies the MCP connection is alive.""" + + def _check() -> bool: + conn = _connections.get(server_name) + return conn is not None and conn.session is not None + + return _check + + +# --------------------------------------------------------------------------- +# Discovery & registration +# --------------------------------------------------------------------------- + +def _convert_mcp_schema(server_name: str, mcp_tool) -> dict: + """Convert an MCP tool listing to the Hermes registry schema format. + + Args: + server_name: The logical server name for prefixing. + mcp_tool: An MCP ``Tool`` object with ``.name``, ``.description``, + and ``.inputSchema``. + + Returns: + A dict suitable for ``registry.register(schema=...)``. + """ + # Sanitize: replace hyphens and dots with underscores for LLM API compatibility + safe_tool_name = mcp_tool.name.replace("-", "_").replace(".", "_") + safe_server_name = server_name.replace("-", "_").replace(".", "_") + prefixed_name = f"mcp_{safe_server_name}_{safe_tool_name}" + return { + "name": prefixed_name, + "description": mcp_tool.description or f"MCP tool {mcp_tool.name} from {server_name}", + "parameters": mcp_tool.inputSchema if mcp_tool.inputSchema else { + "type": "object", + "properties": {}, + }, + } + + +async def _discover_and_register_server(name: str, config: dict) -> List[str]: + """Connect to a single MCP server, discover tools, and register them. + + Returns list of registered tool names. + """ + from tools.registry import registry + from toolsets import create_custom_toolset + + conn = await _connect_server(name, config) + _connections[name] = conn + + # Discover tools + tools_result = await conn.session.list_tools() + tools = tools_result.tools if hasattr(tools_result, "tools") else [] + + registered_names: List[str] = [] + toolset_name = f"mcp-{name}" + + for mcp_tool in tools: + schema = _convert_mcp_schema(name, mcp_tool) + tool_name_prefixed = schema["name"] + + registry.register( + name=tool_name_prefixed, + toolset=toolset_name, + schema=schema, + handler=_make_tool_handler(name, mcp_tool.name), + check_fn=_make_check_fn(name), + is_async=False, + description=schema["description"], + ) + registered_names.append(tool_name_prefixed) + + # Create a custom toolset so these tools are discoverable + if registered_names: + create_custom_toolset( + name=toolset_name, + description=f"MCP tools from {name} server", + tools=registered_names, + ) + + logger.info( + "MCP server '%s': registered %d tool(s): %s", + name, len(registered_names), ", ".join(registered_names), + ) + return registered_names + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + +def discover_mcp_tools() -> List[str]: + """Entry point: load config, connect to MCP servers, register tools. + + Called from ``model_tools._discover_tools()``. Safe to call even when + the ``mcp`` package is not installed (returns empty list). + + Returns: + List of all registered MCP tool names. + """ + if not _MCP_AVAILABLE: + logger.debug("MCP SDK not available -- skipping MCP tool discovery") + return [] + + servers = _load_mcp_config() + if not servers: + logger.debug("No MCP servers configured") + return [] + + # Start the background event loop for MCP connections + _ensure_mcp_loop() + + all_tools: List[str] = [] + + async def _discover_all(): + for name, cfg in servers.items(): + try: + registered = await _discover_and_register_server(name, cfg) + all_tools.extend(registered) + except Exception as exc: + logger.warning("Failed to connect to MCP server '%s': %s", name, exc) + + _run_on_mcp_loop(_discover_all(), timeout=60) + + if all_tools: + # Add MCP tools to hermes-cli and other platform toolsets + from toolsets import TOOLSETS + for ts_name in ("hermes-cli", "hermes-telegram", "hermes-discord", + "hermes-whatsapp", "hermes-slack"): + ts = TOOLSETS.get(ts_name) + if ts: + for tool_name in all_tools: + if tool_name not in ts["tools"]: + ts["tools"].append(tool_name) + + return all_tools + + +def shutdown_mcp_servers(): + """Close all MCP server connections and stop the background loop.""" + global _mcp_loop, _mcp_thread + + if not _connections: + _stop_mcp_loop() + return + + async def _shutdown(): + for name, conn in list(_connections.items()): + try: + if conn.stack: + await conn.stack.aclose() + except Exception as exc: + logger.debug("Error closing MCP server '%s': %s", name, exc) + finally: + conn.session = None + conn.stack = None + _connections.clear() + + if _mcp_loop is not None and _mcp_loop.is_running(): + try: + future = asyncio.run_coroutine_threadsafe(_shutdown(), _mcp_loop) + future.result(timeout=10) + except Exception as exc: + logger.debug("Error during MCP shutdown: %s", exc) + + _stop_mcp_loop() + + +def _stop_mcp_loop(): + """Stop the background event loop and join its thread.""" + global _mcp_loop, _mcp_thread + if _mcp_loop is not None: + _mcp_loop.call_soon_threadsafe(_mcp_loop.stop) + if _mcp_thread is not None: + _mcp_thread.join(timeout=5) + _mcp_thread = None + _mcp_loop.close() + _mcp_loop = None diff --git a/uv.lock b/uv.lock index 54863389..a768b72c 100644 --- a/uv.lock +++ b/uv.lock @@ -1015,6 +1015,7 @@ all = [ { name = "discord-py" }, { name = "elevenlabs" }, { name = "honcho-ai" }, + { name = "mcp" }, { name = "ptyprocess" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -1037,6 +1038,9 @@ dev = [ honcho = [ { name = "honcho-ai" }, ] +mcp = [ + { name = "mcp" }, +] messaging = [ { name = "aiohttp" }, { name = "discord-py" }, @@ -1072,6 +1076,7 @@ requires-dist = [ { name = "hermes-agent", extras = ["cron"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["dev"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["honcho"], marker = "extra == 'all'" }, + { name = "hermes-agent", extras = ["mcp"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["messaging"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["modal"], marker = "extra == 'all'" }, { name = "hermes-agent", extras = ["pty"], marker = "extra == 'all'" }, @@ -1081,6 +1086,7 @@ requires-dist = [ { name = "httpx" }, { name = "jinja2" }, { name = "litellm", specifier = ">=1.75.5" }, + { name = "mcp", marker = "extra == 'mcp'", specifier = ">=1.2.0" }, { name = "openai" }, { name = "platformdirs" }, { name = "prompt-toolkit" }, @@ -1103,7 +1109,7 @@ requires-dist = [ { name = "tenacity" }, { name = "typer" }, ] -provides-extras = ["modal", "dev", "messaging", "cron", "slack", "cli", "tts-premium", "pty", "honcho", "all"] +provides-extras = ["modal", "dev", "messaging", "cron", "slack", "cli", "tts-premium", "pty", "honcho", "mcp", "all"] [[package]] name = "hf-xet" @@ -1522,6 +1528,31 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/70/bc/6f1c2f612465f5fa89b95bead1f44dcb607670fd42891d8fdcd5d039f4f4/markupsafe-3.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:32001d6a8fc98c8cb5c947787c5d08b0a50663d139f1305bac5885d98d9b40fa", size = 14146, upload-time = "2025-09-27T18:37:28.327Z" }, ] +[[package]] +name = "mcp" +version = "1.26.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "httpx" }, + { name = "httpx-sse" }, + { name = "jsonschema" }, + { name = "pydantic" }, + { name = "pydantic-settings" }, + { name = "pyjwt", extra = ["crypto"] }, + { name = "python-multipart" }, + { name = "pywin32", marker = "sys_platform == 'win32'" }, + { name = "sse-starlette" }, + { name = "starlette" }, + { name = "typing-extensions" }, + { name = "typing-inspection" }, + { name = "uvicorn", marker = "sys_platform != 'emscripten'" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/fc/6d/62e76bbb8144d6ed86e202b5edd8a4cb631e7c8130f3f4893c3f90262b10/mcp-1.26.0.tar.gz", hash = "sha256:db6e2ef491eecc1a0d93711a76f28dec2e05999f93afd48795da1c1137142c66", size = 608005, upload-time = "2026-01-24T19:40:32.468Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fd/d9/eaa1f80170d2b7c5ba23f3b59f766f3a0bb41155fbc32a69adfa1adaaef9/mcp-1.26.0-py3-none-any.whl", hash = "sha256:904a21c33c25aa98ddbeb47273033c435e595bbacfdb177f4bd87f6dceebe1ca", size = 233615, upload-time = "2026-01-24T19:40:30.652Z" }, +] + [[package]] name = "mdurl" version = "0.1.2" @@ -2114,6 +2145,20 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/36/c7/cfc8e811f061c841d7990b0201912c3556bfeb99cdcb7ed24adc8d6f8704/pydantic_core-2.41.5-pp311-pypy311_pp73-win_amd64.whl", hash = "sha256:56121965f7a4dc965bff783d70b907ddf3d57f6eba29b6d2e5dabfaf07799c51", size = 2145302, upload-time = "2025-11-04T13:43:46.64Z" }, ] +[[package]] +name = "pydantic-settings" +version = "2.13.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pydantic" }, + { name = "python-dotenv" }, + { name = "typing-inspection" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/52/6d/fffca34caecc4a3f97bda81b2098da5e8ab7efc9a66e819074a11955d87e/pydantic_settings-2.13.1.tar.gz", hash = "sha256:b4c11847b15237fb0171e1462bf540e294affb9b86db4d9aa5c01730bdbe4025", size = 223826, upload-time = "2026-02-19T13:45:08.055Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/00/4b/ccc026168948fec4f7555b9164c724cf4125eac006e176541483d2c959be/pydantic_settings-2.13.1-py3-none-any.whl", hash = "sha256:d56fd801823dbeae7f0975e1f8c8e25c258eb75d278ea7abb5d9cebb01b56237", size = 58929, upload-time = "2026-02-19T13:45:06.034Z" }, +] + [[package]] name = "pygments" version = "2.19.2" @@ -2221,6 +2266,28 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/81/c4/34e93fe5f5429d7570ec1fa436f1986fb1f00c3e0f43a589fe2bbcd22c3f/pytz-2025.2-py2.py3-none-any.whl", hash = "sha256:5ddf76296dd8c44c26eb8f4b6f35488f3ccbf6fbbd7adee0b7262d43f0ec2f00", size = 509225, upload-time = "2025-03-25T02:24:58.468Z" }, ] +[[package]] +name = "pywin32" +version = "311" +source = { registry = "https://pypi.org/simple" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/7b/40/44efbb0dfbd33aca6a6483191dae0716070ed99e2ecb0c53683f400a0b4f/pywin32-311-cp310-cp310-win32.whl", hash = "sha256:d03ff496d2a0cd4a5893504789d4a15399133fe82517455e78bad62efbb7f0a3", size = 8760432, upload-time = "2025-07-14T20:13:05.9Z" }, + { url = "https://files.pythonhosted.org/packages/5e/bf/360243b1e953bd254a82f12653974be395ba880e7ec23e3731d9f73921cc/pywin32-311-cp310-cp310-win_amd64.whl", hash = "sha256:797c2772017851984b97180b0bebe4b620bb86328e8a884bb626156295a63b3b", size = 9590103, upload-time = "2025-07-14T20:13:07.698Z" }, + { url = "https://files.pythonhosted.org/packages/57/38/d290720e6f138086fb3d5ffe0b6caa019a791dd57866940c82e4eeaf2012/pywin32-311-cp310-cp310-win_arm64.whl", hash = "sha256:0502d1facf1fed4839a9a51ccbcc63d952cf318f78ffc00a7e78528ac27d7a2b", size = 8778557, upload-time = "2025-07-14T20:13:11.11Z" }, + { url = "https://files.pythonhosted.org/packages/7c/af/449a6a91e5d6db51420875c54f6aff7c97a86a3b13a0b4f1a5c13b988de3/pywin32-311-cp311-cp311-win32.whl", hash = "sha256:184eb5e436dea364dcd3d2316d577d625c0351bf237c4e9a5fabbcfa5a58b151", size = 8697031, upload-time = "2025-07-14T20:13:13.266Z" }, + { url = "https://files.pythonhosted.org/packages/51/8f/9bb81dd5bb77d22243d33c8397f09377056d5c687aa6d4042bea7fbf8364/pywin32-311-cp311-cp311-win_amd64.whl", hash = "sha256:3ce80b34b22b17ccbd937a6e78e7225d80c52f5ab9940fe0506a1a16f3dab503", size = 9508308, upload-time = "2025-07-14T20:13:15.147Z" }, + { url = "https://files.pythonhosted.org/packages/44/7b/9c2ab54f74a138c491aba1b1cd0795ba61f144c711daea84a88b63dc0f6c/pywin32-311-cp311-cp311-win_arm64.whl", hash = "sha256:a733f1388e1a842abb67ffa8e7aad0e70ac519e09b0f6a784e65a136ec7cefd2", size = 8703930, upload-time = "2025-07-14T20:13:16.945Z" }, + { url = "https://files.pythonhosted.org/packages/e7/ab/01ea1943d4eba0f850c3c61e78e8dd59757ff815ff3ccd0a84de5f541f42/pywin32-311-cp312-cp312-win32.whl", hash = "sha256:750ec6e621af2b948540032557b10a2d43b0cee2ae9758c54154d711cc852d31", size = 8706543, upload-time = "2025-07-14T20:13:20.765Z" }, + { url = "https://files.pythonhosted.org/packages/d1/a8/a0e8d07d4d051ec7502cd58b291ec98dcc0c3fff027caad0470b72cfcc2f/pywin32-311-cp312-cp312-win_amd64.whl", hash = "sha256:b8c095edad5c211ff31c05223658e71bf7116daa0ecf3ad85f3201ea3190d067", size = 9495040, upload-time = "2025-07-14T20:13:22.543Z" }, + { url = "https://files.pythonhosted.org/packages/ba/3a/2ae996277b4b50f17d61f0603efd8253cb2d79cc7ae159468007b586396d/pywin32-311-cp312-cp312-win_arm64.whl", hash = "sha256:e286f46a9a39c4a18b319c28f59b61de793654af2f395c102b4f819e584b5852", size = 8710102, upload-time = "2025-07-14T20:13:24.682Z" }, + { url = "https://files.pythonhosted.org/packages/a5/be/3fd5de0979fcb3994bfee0d65ed8ca9506a8a1260651b86174f6a86f52b3/pywin32-311-cp313-cp313-win32.whl", hash = "sha256:f95ba5a847cba10dd8c4d8fefa9f2a6cf283b8b88ed6178fa8a6c1ab16054d0d", size = 8705700, upload-time = "2025-07-14T20:13:26.471Z" }, + { url = "https://files.pythonhosted.org/packages/e3/28/e0a1909523c6890208295a29e05c2adb2126364e289826c0a8bc7297bd5c/pywin32-311-cp313-cp313-win_amd64.whl", hash = "sha256:718a38f7e5b058e76aee1c56ddd06908116d35147e133427e59a3983f703a20d", size = 9494700, upload-time = "2025-07-14T20:13:28.243Z" }, + { url = "https://files.pythonhosted.org/packages/04/bf/90339ac0f55726dce7d794e6d79a18a91265bdf3aa70b6b9ca52f35e022a/pywin32-311-cp313-cp313-win_arm64.whl", hash = "sha256:7b4075d959648406202d92a2310cb990fea19b535c7f4a78d3f5e10b926eeb8a", size = 8709318, upload-time = "2025-07-14T20:13:30.348Z" }, + { url = "https://files.pythonhosted.org/packages/c9/31/097f2e132c4f16d99a22bfb777e0fd88bd8e1c634304e102f313af69ace5/pywin32-311-cp314-cp314-win32.whl", hash = "sha256:b7a2c10b93f8986666d0c803ee19b5990885872a7de910fc460f9b0c2fbf92ee", size = 8840714, upload-time = "2025-07-14T20:13:32.449Z" }, + { url = "https://files.pythonhosted.org/packages/90/4b/07c77d8ba0e01349358082713400435347df8426208171ce297da32c313d/pywin32-311-cp314-cp314-win_amd64.whl", hash = "sha256:3aca44c046bd2ed8c90de9cb8427f581c479e594e99b5c0bb19b29c10fd6cb87", size = 9656800, upload-time = "2025-07-14T20:13:34.312Z" }, + { url = "https://files.pythonhosted.org/packages/c0/d2/21af5c535501a7233e734b8af901574572da66fcc254cb35d0609c9080dd/pywin32-311-cp314-cp314-win_arm64.whl", hash = "sha256:a508e2d9025764a8270f93111a970e1d0fbfc33f4153b388bb649b7eec4f9b42", size = 8932540, upload-time = "2025-07-14T20:13:36.379Z" }, +] + [[package]] name = "pyyaml" version = "6.0.3" @@ -2639,6 +2706,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/e9/44/75a9c9421471a6c4805dbf2356f7c181a29c1879239abab1ea2cc8f38b40/sniffio-1.3.1-py3-none-any.whl", hash = "sha256:2f6da418d1f1e0fddd844478f41680e794e6051915791a034ff65e5f100525a2", size = 10235, upload-time = "2024-02-25T23:20:01.196Z" }, ] +[[package]] +name = "sse-starlette" +version = "3.3.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "starlette" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/5a/9f/c3695c2d2d4ef70072c3a06992850498b01c6bc9be531950813716b426fa/sse_starlette-3.3.2.tar.gz", hash = "sha256:678fca55a1945c734d8472a6cad186a55ab02840b4f6786f5ee8770970579dcd", size = 32326, upload-time = "2026-02-28T11:24:34.36Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/61/28/8cb142d3fe80c4a2d8af54ca0b003f47ce0ba920974e7990fa6e016402d1/sse_starlette-3.3.2-py3-none-any.whl", hash = "sha256:5c3ea3dad425c601236726af2f27689b74494643f57017cafcb6f8c9acfbb862", size = 14270, upload-time = "2026-02-28T11:24:32.984Z" }, +] + [[package]] name = "starlette" version = "0.52.1" From 0eb0bec74cac9e5022087e40deba27ff466d4f6b Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:06:17 +0300 Subject: [PATCH 02/12] feat(gateway): add MCP server shutdown on gateway exit Ensures MCP subprocess connections are closed when the messaging gateway shuts down, preventing orphan processes. --- gateway/run.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/gateway/run.py b/gateway/run.py index 8154b76f..2a40149e 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2202,7 +2202,14 @@ async def start_gateway(config: Optional[GatewayConfig] = None) -> bool: # Stop cron ticker cleanly cron_stop.set() cron_thread.join(timeout=5) - + + # Close MCP server connections + try: + from tools.mcp_tool import shutdown_mcp_servers + shutdown_mcp_servers() + except Exception: + pass + return True From aa2ecaef29fd13eae1df704857039cbba6c05849 Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:22:00 +0300 Subject: [PATCH 03/12] fix: resolve orphan subprocess leak on MCP server shutdown Refactor MCP connections from AsyncExitStack to task-per-server architecture. Each server now runs as a long-lived asyncio Task with `async with stdio_client(...)`, ensuring anyio cancel-scope cleanup happens in the same Task that opened the connection. --- tests/tools/test_mcp_tool.py | 358 +++++++++++++++++++---------------- tools/mcp_tool.py | 194 ++++++++++++------- 2 files changed, 319 insertions(+), 233 deletions(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index caaffd48..f12a6c93 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -36,6 +36,15 @@ def _make_call_result(text="file contents here", is_error=False): return SimpleNamespace(content=[block], isError=is_error) +def _make_mock_server(name, session=None, tools=None): + """Create an MCPServerTask with mock attributes for testing.""" + from tools.mcp_tool import MCPServerTask + server = MCPServerTask(name) + server.session = session + server._tools = tools or [] + return server + + # --------------------------------------------------------------------------- # Config loading # --------------------------------------------------------------------------- @@ -43,12 +52,10 @@ def _make_call_result(text="file contents here", is_error=False): class TestLoadMCPConfig: def test_no_config_returns_empty(self): """No mcp_servers key in config -> empty dict.""" - with patch("tools.mcp_tool.load_config", create=True) as mock_lc: - # Patch the actual import inside the function - with patch("hermes_cli.config.load_config", return_value={"model": "test"}): - from tools.mcp_tool import _load_mcp_config - result = _load_mcp_config() - assert result == {} + with patch("hermes_cli.config.load_config", return_value={"model": "test"}): + from tools.mcp_tool import _load_mcp_config + result = _load_mcp_config() + assert result == {} def test_valid_config_parsed(self): """Valid mcp_servers config is returned as-is.""" @@ -123,46 +130,37 @@ class TestSchemaConversion: class TestCheckFunction: def test_disconnected_returns_false(self): - from tools.mcp_tool import _make_check_fn, _connections + from tools.mcp_tool import _make_check_fn, _servers - # Ensure no connection exists - _connections.pop("test_server", None) + _servers.pop("test_server", None) check = _make_check_fn("test_server") assert check() is False def test_connected_returns_true(self): - from tools.mcp_tool import _make_check_fn, _connections, MCPConnection + from tools.mcp_tool import _make_check_fn, _servers - conn = MCPConnection( - server_name="test_server", - session=MagicMock(), - stack=MagicMock(), - ) - _connections["test_server"] = conn + server = _make_mock_server("test_server", session=MagicMock()) + _servers["test_server"] = server try: check = _make_check_fn("test_server") assert check() is True finally: - _connections.pop("test_server", None) + _servers.pop("test_server", None) def test_session_none_returns_false(self): - from tools.mcp_tool import _make_check_fn, _connections, MCPConnection + from tools.mcp_tool import _make_check_fn, _servers - conn = MCPConnection( - server_name="test_server", - session=None, - stack=MagicMock(), - ) - _connections["test_server"] = conn + server = _make_mock_server("test_server", session=None) + _servers["test_server"] = server try: check = _make_check_fn("test_server") assert check() is False finally: - _connections.pop("test_server", None) + _servers.pop("test_server", None) # --------------------------------------------------------------------------- -# Tool handler (async) +# Tool handler # --------------------------------------------------------------------------- class TestToolHandler: @@ -171,20 +169,24 @@ class TestToolHandler: def _patch_mcp_loop(self, coro_side_effect=None): """Return a patch for _run_on_mcp_loop that runs the coroutine directly.""" def fake_run(coro, timeout=30): - return asyncio.get_event_loop().run_until_complete(coro) + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() if coro_side_effect: return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=coro_side_effect) return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=fake_run) def test_successful_call(self): - from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + from tools.mcp_tool import _make_tool_handler, _servers mock_session = MagicMock() mock_session.call_tool = AsyncMock( return_value=_make_call_result("hello world", is_error=False) ) - conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) - _connections["test_srv"] = conn + server = _make_mock_server("test_srv", session=mock_session) + _servers["test_srv"] = server try: handler = _make_tool_handler("test_srv", "greet") @@ -193,17 +195,17 @@ class TestToolHandler: assert result["result"] == "hello world" mock_session.call_tool.assert_called_once_with("greet", arguments={"name": "world"}) finally: - _connections.pop("test_srv", None) + _servers.pop("test_srv", None) def test_mcp_error_result(self): - from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + from tools.mcp_tool import _make_tool_handler, _servers mock_session = MagicMock() mock_session.call_tool = AsyncMock( return_value=_make_call_result("something went wrong", is_error=True) ) - conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) - _connections["test_srv"] = conn + server = _make_mock_server("test_srv", session=mock_session) + _servers["test_srv"] = server try: handler = _make_tool_handler("test_srv", "fail_tool") @@ -212,25 +214,24 @@ class TestToolHandler: assert "error" in result assert "something went wrong" in result["error"] finally: - _connections.pop("test_srv", None) + _servers.pop("test_srv", None) def test_disconnected_server(self): - from tools.mcp_tool import _make_tool_handler, _connections + from tools.mcp_tool import _make_tool_handler, _servers - _connections.pop("ghost", None) + _servers.pop("ghost", None) handler = _make_tool_handler("ghost", "any_tool") - # Disconnected check happens before _run_on_mcp_loop, no patch needed result = json.loads(handler({})) assert "error" in result assert "not connected" in result["error"] def test_exception_during_call(self): - from tools.mcp_tool import _make_tool_handler, _connections, MCPConnection + from tools.mcp_tool import _make_tool_handler, _servers mock_session = MagicMock() mock_session.call_tool = AsyncMock(side_effect=RuntimeError("connection lost")) - conn = MCPConnection("test_srv", session=mock_session, stack=MagicMock()) - _connections["test_srv"] = conn + server = _make_mock_server("test_srv", session=mock_session) + _servers["test_srv"] = server try: handler = _make_tool_handler("test_srv", "broken_tool") @@ -239,7 +240,7 @@ class TestToolHandler: assert "error" in result assert "connection lost" in result["error"] finally: - _connections.pop("test_srv", None) + _servers.pop("test_srv", None) # --------------------------------------------------------------------------- @@ -249,23 +250,21 @@ class TestToolHandler: class TestDiscoverAndRegister: def test_tools_registered_in_registry(self): """_discover_and_register_server registers tools with correct names.""" - from tools.registry import ToolRegistry, registry as real_registry - from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + from tools.registry import ToolRegistry + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask mock_registry = ToolRegistry() mock_tools = [ _make_mcp_tool("read_file", "Read a file"), _make_mcp_tool("write_file", "Write a file"), ] - mock_session = MagicMock() - mock_session.initialize = AsyncMock() - mock_session.list_tools = AsyncMock( - return_value=SimpleNamespace(tools=mock_tools) - ) async def fake_connect(name, config): - return MCPConnection(name, session=mock_session, stack=MagicMock()) + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ patch("tools.registry.registry", mock_registry): @@ -278,22 +277,20 @@ class TestDiscoverAndRegister: assert "mcp_fs_read_file" in mock_registry.get_all_tool_names() assert "mcp_fs_write_file" in mock_registry.get_all_tool_names() - _connections.pop("fs", None) + _servers.pop("fs", None) def test_toolset_created(self): """A custom toolset is created for the MCP server.""" - from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask mock_tools = [_make_mcp_tool("ping", "Ping")] - mock_session = MagicMock() - mock_session.initialize = AsyncMock() - mock_session.list_tools = AsyncMock( - return_value=SimpleNamespace(tools=mock_tools) - ) async def fake_connect(name, config): - return MCPConnection(name, session=mock_session, stack=MagicMock()) + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server mock_create = MagicMock() with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ @@ -306,24 +303,22 @@ class TestDiscoverAndRegister: call_kwargs = mock_create.call_args assert call_kwargs[1]["name"] == "mcp-myserver" or call_kwargs[0][0] == "mcp-myserver" - _connections.pop("myserver", None) + _servers.pop("myserver", None) def test_schema_format_correct(self): """Registered schemas have the correct format.""" - from tools.registry import ToolRegistry, registry as real_registry - from tools.mcp_tool import _discover_and_register_server, _connections, MCPConnection + from tools.registry import ToolRegistry + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask mock_registry = ToolRegistry() mock_tools = [_make_mcp_tool("do_thing", "Do something")] - mock_session = MagicMock() - mock_session.initialize = AsyncMock() - mock_session.list_tools = AsyncMock( - return_value=SimpleNamespace(tools=mock_tools) - ) async def fake_connect(name, config): - return MCPConnection(name, session=mock_session, stack=MagicMock()) + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ patch("tools.registry.registry", mock_registry): @@ -338,91 +333,125 @@ class TestDiscoverAndRegister: assert entry.is_async is False assert entry.toolset == "mcp-srv" - _connections.pop("srv", None) + _servers.pop("srv", None) # --------------------------------------------------------------------------- -# _connect_server (SDK interaction) +# MCPServerTask (run / start / shutdown) # --------------------------------------------------------------------------- -class TestConnectServer: - def test_calls_sdk_with_correct_params(self): - """_connect_server creates StdioServerParameters and calls stdio_client.""" - from tools.mcp_tool import _connect_server, MCPConnection +class TestMCPServerTask: + """Test the MCPServerTask lifecycle with mocked MCP SDK.""" + def _mock_stdio_and_session(self, session): + """Return patches for stdio_client and ClientSession as async CMs.""" + mock_read, mock_write = MagicMock(), MagicMock() + + mock_stdio_cm = MagicMock() + mock_stdio_cm.__aenter__ = AsyncMock(return_value=(mock_read, mock_write)) + mock_stdio_cm.__aexit__ = AsyncMock(return_value=False) + + mock_cs_cm = MagicMock() + mock_cs_cm.__aenter__ = AsyncMock(return_value=session) + mock_cs_cm.__aexit__ = AsyncMock(return_value=False) + + return ( + patch("tools.mcp_tool.stdio_client", return_value=mock_stdio_cm), + patch("tools.mcp_tool.ClientSession", return_value=mock_cs_cm), + mock_read, mock_write, + ) + + def test_start_connects_and_discovers_tools(self): + """start() creates a Task that connects, discovers tools, and waits.""" + from tools.mcp_tool import MCPServerTask + + mock_tools = [_make_mcp_tool("echo")] mock_session = MagicMock() mock_session.initialize = AsyncMock() - - mock_read = MagicMock() - mock_write = MagicMock() - - with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ - patch("tools.mcp_tool.stdio_client") as mock_stdio, \ - patch("tools.mcp_tool.ClientSession") as mock_cs, \ - patch("tools.mcp_tool.AsyncExitStack") as mock_stack_cls: - - mock_stack = MagicMock() - mock_stack.enter_async_context = AsyncMock( - side_effect=[(mock_read, mock_write), mock_session] - ) - mock_stack_cls.return_value = mock_stack - - conn = asyncio.run(_connect_server("test_srv", { - "command": "npx", - "args": ["-y", "some-server"], - "env": {"MY_KEY": "secret"}, - })) - - # StdioServerParameters called with correct values - mock_params.assert_called_once_with( - command="npx", - args=["-y", "some-server"], - env={"MY_KEY": "secret"}, + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=mock_tools) ) - # ClientSession created with the streams - mock_cs.assert_called_once_with(mock_read, mock_write) - # initialize() was called - mock_session.initialize.assert_called_once() - # Returned connection is valid - assert conn.server_name == "test_srv" - assert conn.session is mock_session + + p_stdio, p_cs, _, _ = self._mock_stdio_and_session(mock_session) + + async def _test(): + with patch("tools.mcp_tool.StdioServerParameters"), p_stdio, p_cs: + server = MCPServerTask("test_srv") + await server.start({"command": "npx", "args": ["-y", "test"]}) + + assert server.session is mock_session + assert len(server._tools) == 1 + assert server._tools[0].name == "echo" + mock_session.initialize.assert_called_once() + + await server.shutdown() + assert server.session is None + + asyncio.run(_test()) def test_no_command_raises(self): """Missing 'command' in config raises ValueError.""" - from tools.mcp_tool import _connect_server + from tools.mcp_tool import MCPServerTask - with pytest.raises(ValueError, match="no 'command'"): - asyncio.run(_connect_server("bad", {"args": []})) + async def _test(): + server = MCPServerTask("bad") + with pytest.raises(ValueError, match="no 'command'"): + await server.start({"args": []}) + + asyncio.run(_test()) def test_empty_env_passed_as_none(self): """Empty env dict is passed as None to StdioServerParameters.""" - from tools.mcp_tool import _connect_server + from tools.mcp_tool import MCPServerTask mock_session = MagicMock() mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=[]) + ) - with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ - patch("tools.mcp_tool.stdio_client"), \ - patch("tools.mcp_tool.ClientSession", return_value=mock_session), \ - patch("tools.mcp_tool.AsyncExitStack") as mock_stack_cls: + p_stdio, p_cs, _, _ = self._mock_stdio_and_session(mock_session) - mock_stack = MagicMock() - mock_stack.enter_async_context = AsyncMock( - side_effect=[ - (MagicMock(), MagicMock()), - mock_session, - ] - ) - mock_stack_cls.return_value = mock_stack + async def _test(): + with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ + p_stdio, p_cs: + server = MCPServerTask("srv") + await server.start({"command": "node", "env": {}}) - asyncio.run(_connect_server("srv", { - "command": "node", - "env": {}, - })) + # Empty dict -> None + call_kwargs = mock_params.call_args + assert call_kwargs.kwargs.get("env") is None - # Empty dict -> None - assert mock_params.call_args[1]["env"] is None or \ - mock_params.call_args.kwargs.get("env") is None + await server.shutdown() + + asyncio.run(_test()) + + def test_shutdown_signals_task_exit(self): + """shutdown() signals the event and waits for task completion.""" + from tools.mcp_tool import MCPServerTask + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock( + return_value=SimpleNamespace(tools=[]) + ) + + p_stdio, p_cs, _, _ = self._mock_stdio_and_session(mock_session) + + async def _test(): + with patch("tools.mcp_tool.StdioServerParameters"), p_stdio, p_cs: + server = MCPServerTask("srv") + await server.start({"command": "npx"}) + + assert server.session is not None + assert not server._task.done() + + await server.shutdown() + + assert server.session is None + assert server._task.done() + + asyncio.run(_test()) # --------------------------------------------------------------------------- @@ -432,17 +461,16 @@ class TestConnectServer: class TestToolsetInjection: def test_mcp_tools_added_to_platform_toolsets(self): """Discovered MCP tools are injected into hermes-cli and platform toolsets.""" - from tools.mcp_tool import _connections, MCPConnection + from tools.mcp_tool import _servers, MCPServerTask mock_tools = [_make_mcp_tool("list_files", "List files")] mock_session = MagicMock() - mock_session.initialize = AsyncMock() - mock_session.list_tools = AsyncMock( - return_value=SimpleNamespace(tools=mock_tools) - ) async def fake_connect(name, config): - return MCPConnection(name, session=mock_session, stack=MagicMock()) + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server fake_toolsets = { "hermes-cli": {"tools": ["terminal", "web_search"], "description": "CLI", "includes": []}, @@ -455,7 +483,6 @@ class TestToolsetInjection: with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ - patch("tools.mcp_tool.TOOLSETS", fake_toolsets, create=True), \ patch("toolsets.TOOLSETS", fake_toolsets): from tools.mcp_tool import discover_mcp_tools result = discover_mcp_tools() @@ -466,18 +493,14 @@ class TestToolsetInjection: # Original tools preserved assert "terminal" in fake_toolsets["hermes-cli"]["tools"] - _connections.pop("fs", None) + _servers.pop("fs", None) def test_server_connection_failure_skipped(self): """If one server fails to connect, others still proceed.""" - from tools.mcp_tool import _connections, MCPConnection + from tools.mcp_tool import _servers, MCPServerTask mock_tools = [_make_mcp_tool("ping", "Ping")] mock_session = MagicMock() - mock_session.initialize = AsyncMock() - mock_session.list_tools = AsyncMock( - return_value=SimpleNamespace(tools=mock_tools) - ) call_count = 0 @@ -486,7 +509,10 @@ class TestToolsetInjection: call_count += 1 if name == "broken": raise ConnectionError("cannot reach server") - return MCPConnection(name, session=mock_session, stack=MagicMock()) + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server fake_config = { "broken": {"command": "bad"}, @@ -508,7 +534,7 @@ class TestToolsetInjection: assert "mcp_broken_ping" not in result assert call_count == 2 # Both were attempted - _connections.pop("good", None) + _servers.pop("good", None) # --------------------------------------------------------------------------- @@ -533,50 +559,46 @@ class TestGracefulFallback: # --------------------------------------------------------------------------- -# Shutdown +# Shutdown (public API) # --------------------------------------------------------------------------- class TestShutdown: - def test_no_connections_safe(self): - """shutdown_mcp_servers with no connections does nothing.""" - from tools.mcp_tool import shutdown_mcp_servers, _connections + def test_no_servers_safe(self): + """shutdown_mcp_servers with no servers does nothing.""" + from tools.mcp_tool import shutdown_mcp_servers, _servers - _connections.clear() + _servers.clear() shutdown_mcp_servers() # Should not raise - def test_shutdown_clears_connections(self): - """shutdown_mcp_servers closes stacks and clears the dict.""" + def test_shutdown_clears_servers(self): + """shutdown_mcp_servers calls shutdown() on each server and clears dict.""" import tools.mcp_tool as mcp_mod - from tools.mcp_tool import shutdown_mcp_servers, _connections, MCPConnection + from tools.mcp_tool import shutdown_mcp_servers, _servers - _connections.clear() - mock_stack = MagicMock() - mock_stack.aclose = AsyncMock() - conn = MCPConnection("test", session=MagicMock(), stack=mock_stack) - _connections["test"] = conn + _servers.clear() + mock_server = MagicMock() + mock_server.shutdown = AsyncMock() + _servers["test"] = mock_server - # Start a real background loop so shutdown can schedule on it mcp_mod._ensure_mcp_loop() try: shutdown_mcp_servers() finally: - # _stop_mcp_loop is called by shutdown, but ensure cleanup mcp_mod._mcp_loop = None mcp_mod._mcp_thread = None - assert len(_connections) == 0 - mock_stack.aclose.assert_called_once() + assert len(_servers) == 0 + mock_server.shutdown.assert_called_once() def test_shutdown_handles_errors(self): """shutdown_mcp_servers handles errors during close gracefully.""" import tools.mcp_tool as mcp_mod - from tools.mcp_tool import shutdown_mcp_servers, _connections, MCPConnection + from tools.mcp_tool import shutdown_mcp_servers, _servers - _connections.clear() - mock_stack = MagicMock() - mock_stack.aclose = AsyncMock(side_effect=RuntimeError("close failed")) - conn = MCPConnection("broken", session=MagicMock(), stack=mock_stack) - _connections["broken"] = conn + _servers.clear() + mock_server = MagicMock() + mock_server.shutdown = AsyncMock(side_effect=RuntimeError("close failed")) + _servers["broken"] = mock_server mcp_mod._ensure_mcp_loop() try: @@ -585,4 +607,4 @@ class TestShutdown: mcp_mod._mcp_loop = None mcp_mod._mcp_thread = None - assert len(_connections) == 0 + assert len(_servers) == 0 diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index eecbaa29..5225d63f 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -25,8 +25,13 @@ Example config:: Architecture: A dedicated background event loop (_mcp_loop) runs in a daemon thread. - All MCP connections live on this loop. Tool handlers schedule coroutines - onto it via run_coroutine_threadsafe(), so they work from any thread. + Each MCP server runs as a long-lived asyncio Task on this loop, keeping + its ``async with stdio_client(...)`` context alive. Tool call coroutines + are scheduled onto the loop via ``run_coroutine_threadsafe()``. + + On shutdown, each server Task is signalled to exit its ``async with`` + block, ensuring the anyio cancel-scope cleanup happens in the *same* + Task that opened the connection (required by anyio). """ import asyncio @@ -45,31 +50,114 @@ _MCP_AVAILABLE = False try: from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client - from contextlib import AsyncExitStack _MCP_AVAILABLE = True except ImportError: logger.debug("mcp package not installed -- MCP tool support disabled") # --------------------------------------------------------------------------- -# Connection tracking +# Server task -- each MCP server lives in one long-lived asyncio Task # --------------------------------------------------------------------------- -class MCPConnection: - """Holds a live MCP server connection and its async resource stack.""" +class MCPServerTask: + """Manages a single MCP server connection in a dedicated asyncio Task. - __slots__ = ("server_name", "session", "stack") + The entire connection lifecycle (connect, discover, serve, disconnect) + runs inside one asyncio Task so that anyio cancel-scopes created by + ``stdio_client`` are entered and exited in the same Task context. + """ - def __init__(self, server_name: str, session: Any, stack: Any): - self.server_name = server_name - self.session: Optional[Any] = session - self.stack: Optional[Any] = stack + __slots__ = ( + "name", "session", + "_task", "_ready", "_shutdown_event", "_tools", "_error", + ) + + def __init__(self, name: str): + self.name = name + self.session: Optional[Any] = None + self._task: Optional[asyncio.Task] = None + self._ready = asyncio.Event() + self._shutdown_event = asyncio.Event() + self._tools: list = [] + self._error: Optional[Exception] = None + + async def run(self, config: dict): + """Long-lived coroutine: connect, discover tools, wait, disconnect.""" + command = config.get("command") + args = config.get("args", []) + env = config.get("env") + + if not command: + self._error = ValueError( + f"MCP server '{self.name}' has no 'command' in config" + ) + self._ready.set() + return + + server_params = StdioServerParameters( + command=command, + args=args, + env=env if env else None, + ) + + try: + async with stdio_client(server_params) as (read_stream, write_stream): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + self.session = session + + tools_result = await session.list_tools() + self._tools = ( + tools_result.tools + if hasattr(tools_result, "tools") + else [] + ) + + # Signal that connection is ready + self._ready.set() + + # Block until shutdown is requested -- this keeps the + # async-with contexts alive on THIS Task. + await self._shutdown_event.wait() + except Exception as exc: + self._error = exc + self._ready.set() + finally: + self.session = None + + async def start(self, config: dict): + """Create the background Task and wait until ready (or failed).""" + self._task = asyncio.ensure_future(self.run(config)) + await self._ready.wait() + if self._error: + raise self._error + + async def shutdown(self): + """Signal the Task to exit and wait for clean resource teardown.""" + self._shutdown_event.set() + if self._task and not self._task.done(): + try: + await asyncio.wait_for(self._task, timeout=10) + except asyncio.TimeoutError: + logger.warning( + "MCP server '%s' shutdown timed out, cancelling task", + self.name, + ) + self._task.cancel() + try: + await self._task + except asyncio.CancelledError: + pass + self.session = None -_connections: Dict[str, MCPConnection] = {} +# --------------------------------------------------------------------------- +# Module-level state +# --------------------------------------------------------------------------- + +_servers: Dict[str, MCPServerTask] = {} # Dedicated event loop running in a background daemon thread. -# All MCP async operations (connect, call_tool, shutdown) run here. _mcp_loop: Optional[asyncio.AbstractEventLoop] = None _mcp_thread: Optional[threading.Thread] = None @@ -118,42 +206,22 @@ def _load_mcp_config() -> Dict[str, dict]: # --------------------------------------------------------------------------- -# Server connection +# Server connection helper # --------------------------------------------------------------------------- -async def _connect_server(name: str, config: dict) -> MCPConnection: - """Start an MCP server subprocess and initialize a ClientSession. +async def _connect_server(name: str, config: dict) -> MCPServerTask: + """Create an MCPServerTask, start it, and return when ready. - Args: - name: Logical server name (e.g. "filesystem"). - config: Dict with ``command``, ``args``, and optional ``env``. - - Returns: - An ``MCPConnection`` with a live session. + The server Task keeps the subprocess alive in the background. + Call ``server.shutdown()`` (on the same event loop) to tear it down. Raises: - Exception on connection or initialization failure. + ValueError: if ``command`` is missing from *config*. + Exception: on connection or initialization failure. """ - command = config.get("command") - args = config.get("args", []) - env = config.get("env") - - if not command: - raise ValueError(f"MCP server '{name}' has no 'command' in config") - - server_params = StdioServerParameters( - command=command, - args=args, - env=env if env else None, - ) - - stack = AsyncExitStack() - stdio_transport = await stack.enter_async_context(stdio_client(server_params)) - read_stream, write_stream = stdio_transport - session = await stack.enter_async_context(ClientSession(read_stream, write_stream)) - await session.initialize() - - return MCPConnection(server_name=name, session=session, stack=stack) + server = MCPServerTask(name) + await server.start(config) + return server # --------------------------------------------------------------------------- @@ -168,14 +236,14 @@ def _make_tool_handler(server_name: str, tool_name: str): """ def _handler(args: dict, **kwargs) -> str: - conn = _connections.get(server_name) - if not conn or not conn.session: + server = _servers.get(server_name) + if not server or not server.session: return json.dumps({ "error": f"MCP server '{server_name}' is not connected" }) async def _call(): - result = await conn.session.call_tool(tool_name, arguments=args) + result = await server.session.call_tool(tool_name, arguments=args) # MCP CallToolResult has .content (list of content blocks) and .isError if result.isError: error_text = "" @@ -204,8 +272,8 @@ def _make_check_fn(server_name: str): """Return a check function that verifies the MCP connection is alive.""" def _check() -> bool: - conn = _connections.get(server_name) - return conn is not None and conn.session is not None + server = _servers.get(server_name) + return server is not None and server.session is not None return _check @@ -247,17 +315,13 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: from tools.registry import registry from toolsets import create_custom_toolset - conn = await _connect_server(name, config) - _connections[name] = conn - - # Discover tools - tools_result = await conn.session.list_tools() - tools = tools_result.tools if hasattr(tools_result, "tools") else [] + server = await _connect_server(name, config) + _servers[name] = server registered_names: List[str] = [] toolset_name = f"mcp-{name}" - for mcp_tool in tools: + for mcp_tool in server._tools: schema = _convert_mcp_schema(name, mcp_tool) tool_name_prefixed = schema["name"] @@ -339,29 +403,29 @@ def discover_mcp_tools() -> List[str]: def shutdown_mcp_servers(): - """Close all MCP server connections and stop the background loop.""" + """Close all MCP server connections and stop the background loop. + + Each server Task is signalled to exit its ``async with`` block so that + the anyio cancel-scope cleanup happens in the same Task that opened it. + """ global _mcp_loop, _mcp_thread - if not _connections: + if not _servers: _stop_mcp_loop() return async def _shutdown(): - for name, conn in list(_connections.items()): + for name, server in list(_servers.items()): try: - if conn.stack: - await conn.stack.aclose() + await server.shutdown() except Exception as exc: logger.debug("Error closing MCP server '%s': %s", name, exc) - finally: - conn.session = None - conn.stack = None - _connections.clear() + _servers.clear() if _mcp_loop is not None and _mcp_loop.is_running(): try: future = asyncio.run_coroutine_threadsafe(_shutdown(), _mcp_loop) - future.result(timeout=10) + future.result(timeout=15) except Exception as exc: logger.debug("Error during MCP shutdown: %s", exc) From 593c549bc466f6e0b8c517320393c16731597c74 Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:34:21 +0300 Subject: [PATCH 04/12] fix: make discover_mcp_tools idempotent to prevent duplicate connections When discover_mcp_tools() is called multiple times (e.g. direct call then model_tools import), return existing tool names instead of opening new connections that would orphan the previous ones. --- tools/mcp_tool.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 5225d63f..5cdce4a3 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -361,6 +361,9 @@ def discover_mcp_tools() -> List[str]: Called from ``model_tools._discover_tools()``. Safe to call even when the ``mcp`` package is not installed (returns empty list). + Idempotent: if servers are already connected, returns the existing + tool names without creating duplicate connections. + Returns: List of all registered MCP tool names. """ @@ -368,6 +371,15 @@ def discover_mcp_tools() -> List[str]: logger.debug("MCP SDK not available -- skipping MCP tool discovery") return [] + # Already connected -- return existing tool names (idempotent) + if _servers: + existing: List[str] = [] + for name, server in _servers.items(): + for mcp_tool in server._tools: + schema = _convert_mcp_schema(name, mcp_tool) + existing.append(schema["name"]) + return existing + servers = _load_mcp_config() if not servers: logger.debug("No MCP servers configured") From 151e8d896ca2296eeb836097bdb8049e70ef40f0 Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:38:01 +0300 Subject: [PATCH 05/12] fix(tests): isolate discover_mcp_tools tests from global _servers state Patch _servers to empty dict in tests that call discover_mcp_tools() with mocked config, preventing interference from real MCP connections that may exist when running within the full test suite. --- tests/tools/test_mcp_tool.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index f12a6c93..2e52272b 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -461,11 +461,14 @@ class TestMCPServerTask: class TestToolsetInjection: def test_mcp_tools_added_to_platform_toolsets(self): """Discovered MCP tools are injected into hermes-cli and platform toolsets.""" - from tools.mcp_tool import _servers, MCPServerTask + from tools.mcp_tool import MCPServerTask mock_tools = [_make_mcp_tool("list_files", "List files")] mock_session = MagicMock() + # Fresh _servers dict to bypass idempotency guard + fresh_servers = {} + async def fake_connect(name, config): server = MCPServerTask(name) server.session = mock_session @@ -481,6 +484,7 @@ class TestToolsetInjection: } with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._servers", fresh_servers), \ patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ patch("toolsets.TOOLSETS", fake_toolsets): @@ -493,15 +497,15 @@ class TestToolsetInjection: # Original tools preserved assert "terminal" in fake_toolsets["hermes-cli"]["tools"] - _servers.pop("fs", None) - def test_server_connection_failure_skipped(self): """If one server fails to connect, others still proceed.""" - from tools.mcp_tool import _servers, MCPServerTask + from tools.mcp_tool import MCPServerTask mock_tools = [_make_mcp_tool("ping", "Ping")] mock_session = MagicMock() + # Fresh _servers dict to bypass idempotency guard + fresh_servers = {} call_count = 0 async def flaky_connect(name, config): @@ -523,6 +527,7 @@ class TestToolsetInjection: } with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._servers", fresh_servers), \ patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ patch("tools.mcp_tool._connect_server", side_effect=flaky_connect), \ patch("toolsets.TOOLSETS", fake_toolsets): @@ -534,8 +539,6 @@ class TestToolsetInjection: assert "mcp_broken_ping" not in result assert call_count == 2 # Both were attempted - _servers.pop("good", None) - # --------------------------------------------------------------------------- # Graceful fallback @@ -552,6 +555,7 @@ class TestGracefulFallback: def test_no_servers_returns_empty(self): """No MCP servers configured -> empty list.""" with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._servers", {}), \ patch("tools.mcp_tool._load_mcp_config", return_value={}): from tools.mcp_tool import discover_mcp_tools result = discover_mcp_tools() From 11a2ecb936d6bc97f67ce2574630767091a504ec Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Mon, 2 Mar 2026 22:08:32 +0300 Subject: [PATCH 06/12] fix: resolve thread safety issues and shutdown deadlock in MCP client - Add threading.Lock protecting all shared state (_servers, _mcp_loop, _mcp_thread) - Fix deadlock in shutdown_mcp_servers: _stop_mcp_loop was called inside a _lock block but also acquires _lock (non-reentrant) - Fix race condition in _ensure_mcp_loop with concurrent callers - Change idempotency to per-server (retry failed servers, skip connected) - Dynamic toolset injection via startswith("hermes-") instead of hardcoded list - Parallel shutdown via asyncio.gather instead of sequential loop - Add tests for partial failure retry, parallel shutdown, dynamic injection --- tests/tools/test_mcp_tool.py | 108 +++++++++++++++++++++++++--- tools/mcp_tool.py | 134 +++++++++++++++++++++++------------ 2 files changed, 184 insertions(+), 58 deletions(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 2e52272b..065baf4a 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -459,14 +459,13 @@ class TestMCPServerTask: # --------------------------------------------------------------------------- class TestToolsetInjection: - def test_mcp_tools_added_to_platform_toolsets(self): - """Discovered MCP tools are injected into hermes-cli and platform toolsets.""" + def test_mcp_tools_added_to_all_hermes_toolsets(self): + """Discovered MCP tools are dynamically injected into all hermes-* toolsets.""" from tools.mcp_tool import MCPServerTask mock_tools = [_make_mcp_tool("list_files", "List files")] mock_session = MagicMock() - # Fresh _servers dict to bypass idempotency guard fresh_servers = {} async def fake_connect(name, config): @@ -476,12 +475,12 @@ class TestToolsetInjection: return server fake_toolsets = { - "hermes-cli": {"tools": ["terminal", "web_search"], "description": "CLI", "includes": []}, - "hermes-telegram": {"tools": ["terminal"], "description": "Telegram", "includes": []}, - } - fake_config = { - "fs": {"command": "npx", "args": []}, + "hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []}, + "hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []}, + "hermes-gateway": {"tools": [], "description": "GW", "includes": []}, + "non-hermes": {"tools": [], "description": "other", "includes": []}, } + fake_config = {"fs": {"command": "npx", "args": []}} with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ patch("tools.mcp_tool._servers", fresh_servers), \ @@ -492,8 +491,12 @@ class TestToolsetInjection: result = discover_mcp_tools() assert "mcp_fs_list_files" in result + # All hermes-* toolsets get injection assert "mcp_fs_list_files" in fake_toolsets["hermes-cli"]["tools"] assert "mcp_fs_list_files" in fake_toolsets["hermes-telegram"]["tools"] + assert "mcp_fs_list_files" in fake_toolsets["hermes-gateway"]["tools"] + # Non-hermes toolset should NOT get injection + assert "mcp_fs_list_files" not in fake_toolsets["non-hermes"]["tools"] # Original tools preserved assert "terminal" in fake_toolsets["hermes-cli"]["tools"] @@ -504,7 +507,6 @@ class TestToolsetInjection: mock_tools = [_make_mcp_tool("ping", "Ping")] mock_session = MagicMock() - # Fresh _servers dict to bypass idempotency guard fresh_servers = {} call_count = 0 @@ -534,10 +536,62 @@ class TestToolsetInjection: from tools.mcp_tool import discover_mcp_tools result = discover_mcp_tools() - # Only good server's tool registered assert "mcp_good_ping" in result assert "mcp_broken_ping" not in result - assert call_count == 2 # Both were attempted + assert call_count == 2 + + def test_partial_failure_retry_on_second_call(self): + """Failed servers are retried on subsequent discover_mcp_tools() calls.""" + from tools.mcp_tool import MCPServerTask + + mock_tools = [_make_mcp_tool("ping", "Ping")] + mock_session = MagicMock() + + # Use a real dict so idempotency logic works correctly + fresh_servers = {} + call_count = 0 + broken_fixed = False + + async def flaky_connect(name, config): + nonlocal call_count + call_count += 1 + if name == "broken" and not broken_fixed: + raise ConnectionError("cannot reach server") + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server + + fake_config = { + "broken": {"command": "bad"}, + "good": {"command": "npx", "args": []}, + } + fake_toolsets = { + "hermes-cli": {"tools": [], "description": "CLI", "includes": []}, + } + + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._servers", fresh_servers), \ + patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \ + patch("tools.mcp_tool._connect_server", side_effect=flaky_connect), \ + patch("toolsets.TOOLSETS", fake_toolsets): + from tools.mcp_tool import discover_mcp_tools + + # First call: good connects, broken fails + result1 = discover_mcp_tools() + assert "mcp_good_ping" in result1 + assert "mcp_broken_ping" not in result1 + first_attempts = call_count + + # "Fix" the broken server + broken_fixed = True + call_count = 0 + + # Second call: should retry broken, skip good + result2 = discover_mcp_tools() + assert "mcp_good_ping" in result2 + assert "mcp_broken_ping" in result2 + assert call_count == 1 # Only broken retried # --------------------------------------------------------------------------- @@ -581,6 +635,7 @@ class TestShutdown: _servers.clear() mock_server = MagicMock() + mock_server.name = "test" mock_server.shutdown = AsyncMock() _servers["test"] = mock_server @@ -601,6 +656,7 @@ class TestShutdown: _servers.clear() mock_server = MagicMock() + mock_server.name = "broken" mock_server.shutdown = AsyncMock(side_effect=RuntimeError("close failed")) _servers["broken"] = mock_server @@ -612,3 +668,33 @@ class TestShutdown: mcp_mod._mcp_thread = None assert len(_servers) == 0 + + def test_shutdown_is_parallel(self): + """Multiple servers are shut down in parallel via asyncio.gather.""" + import tools.mcp_tool as mcp_mod + from tools.mcp_tool import shutdown_mcp_servers, _servers + import time + + _servers.clear() + + # 3 servers each taking 1s to shut down + for i in range(3): + mock_server = MagicMock() + mock_server.name = f"srv_{i}" + async def slow_shutdown(): + await asyncio.sleep(1) + mock_server.shutdown = slow_shutdown + _servers[f"srv_{i}"] = mock_server + + mcp_mod._ensure_mcp_loop() + try: + start = time.monotonic() + shutdown_mcp_servers() + elapsed = time.monotonic() - start + finally: + mcp_mod._mcp_loop = None + mcp_mod._mcp_thread = None + + assert len(_servers) == 0 + # Parallel: ~1s, not ~3s. Allow some margin. + assert elapsed < 2.5, f"Shutdown took {elapsed:.1f}s, expected ~1s (parallel)" diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 5cdce4a3..4ab55215 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -32,6 +32,12 @@ Architecture: On shutdown, each server Task is signalled to exit its ``async with`` block, ensuring the anyio cancel-scope cleanup happens in the *same* Task that opened the connection (required by anyio). + +Thread safety: + _servers and _mcp_loop/_mcp_thread are accessed from both the MCP + background thread and caller threads. All mutations are protected by + _lock so the code is safe regardless of GIL presence (e.g. Python 3.13+ + free-threading). """ import asyncio @@ -161,26 +167,32 @@ _servers: Dict[str, MCPServerTask] = {} _mcp_loop: Optional[asyncio.AbstractEventLoop] = None _mcp_thread: Optional[threading.Thread] = None +# Protects _mcp_loop, _mcp_thread, and _servers from concurrent access. +_lock = threading.Lock() + def _ensure_mcp_loop(): """Start the background event loop thread if not already running.""" global _mcp_loop, _mcp_thread - if _mcp_loop is not None and _mcp_loop.is_running(): - return - _mcp_loop = asyncio.new_event_loop() - _mcp_thread = threading.Thread( - target=_mcp_loop.run_forever, - name="mcp-event-loop", - daemon=True, - ) - _mcp_thread.start() + with _lock: + if _mcp_loop is not None and _mcp_loop.is_running(): + return + _mcp_loop = asyncio.new_event_loop() + _mcp_thread = threading.Thread( + target=_mcp_loop.run_forever, + name="mcp-event-loop", + daemon=True, + ) + _mcp_thread.start() def _run_on_mcp_loop(coro, timeout: float = 30): """Schedule a coroutine on the MCP event loop and block until done.""" - if _mcp_loop is None or not _mcp_loop.is_running(): + with _lock: + loop = _mcp_loop + if loop is None or not loop.is_running(): raise RuntimeError("MCP event loop is not running") - future = asyncio.run_coroutine_threadsafe(coro, _mcp_loop) + future = asyncio.run_coroutine_threadsafe(coro, loop) return future.result(timeout=timeout) @@ -236,7 +248,8 @@ def _make_tool_handler(server_name: str, tool_name: str): """ def _handler(args: dict, **kwargs) -> str: - server = _servers.get(server_name) + with _lock: + server = _servers.get(server_name) if not server or not server.session: return json.dumps({ "error": f"MCP server '{server_name}' is not connected" @@ -272,7 +285,8 @@ def _make_check_fn(server_name: str): """Return a check function that verifies the MCP connection is alive.""" def _check() -> bool: - server = _servers.get(server_name) + with _lock: + server = _servers.get(server_name) return server is not None and server.session is not None return _check @@ -307,6 +321,16 @@ def _convert_mcp_schema(server_name: str, mcp_tool) -> dict: } +def _existing_tool_names() -> List[str]: + """Return tool names for all currently connected servers.""" + names: List[str] = [] + for sname, server in _servers.items(): + for mcp_tool in server._tools: + schema = _convert_mcp_schema(sname, mcp_tool) + names.append(schema["name"]) + return names + + async def _discover_and_register_server(name: str, config: dict) -> List[str]: """Connect to a single MCP server, discover tools, and register them. @@ -316,7 +340,8 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: from toolsets import create_custom_toolset server = await _connect_server(name, config) - _servers[name] = server + with _lock: + _servers[name] = server registered_names: List[str] = [] toolset_name = f"mcp-{name}" @@ -361,8 +386,8 @@ def discover_mcp_tools() -> List[str]: Called from ``model_tools._discover_tools()``. Safe to call even when the ``mcp`` package is not installed (returns empty list). - Idempotent: if servers are already connected, returns the existing - tool names without creating duplicate connections. + Idempotent for already-connected servers. If some servers failed on a + previous call, only the missing ones are retried. Returns: List of all registered MCP tool names. @@ -371,27 +396,25 @@ def discover_mcp_tools() -> List[str]: logger.debug("MCP SDK not available -- skipping MCP tool discovery") return [] - # Already connected -- return existing tool names (idempotent) - if _servers: - existing: List[str] = [] - for name, server in _servers.items(): - for mcp_tool in server._tools: - schema = _convert_mcp_schema(name, mcp_tool) - existing.append(schema["name"]) - return existing - servers = _load_mcp_config() if not servers: logger.debug("No MCP servers configured") return [] + # Only attempt servers that aren't already connected + with _lock: + new_servers = {k: v for k, v in servers.items() if k not in _servers} + + if not new_servers: + return _existing_tool_names() + # Start the background event loop for MCP connections _ensure_mcp_loop() all_tools: List[str] = [] async def _discover_all(): - for name, cfg in servers.items(): + for name, cfg in new_servers.items(): try: registered = await _discover_and_register_server(name, cfg) all_tools.extend(registered) @@ -401,17 +424,16 @@ def discover_mcp_tools() -> List[str]: _run_on_mcp_loop(_discover_all(), timeout=60) if all_tools: - # Add MCP tools to hermes-cli and other platform toolsets + # Dynamically inject into all hermes-* platform toolsets from toolsets import TOOLSETS - for ts_name in ("hermes-cli", "hermes-telegram", "hermes-discord", - "hermes-whatsapp", "hermes-slack"): - ts = TOOLSETS.get(ts_name) - if ts: + for ts_name, ts in TOOLSETS.items(): + if ts_name.startswith("hermes-"): for tool_name in all_tools: if tool_name not in ts["tools"]: ts["tools"].append(tool_name) - return all_tools + # Return ALL registered tools (existing + newly discovered) + return _existing_tool_names() def shutdown_mcp_servers(): @@ -419,24 +441,39 @@ def shutdown_mcp_servers(): Each server Task is signalled to exit its ``async with`` block so that the anyio cancel-scope cleanup happens in the same Task that opened it. + All servers are shut down in parallel via ``asyncio.gather``. """ - global _mcp_loop, _mcp_thread + with _lock: + if not _servers: + # No servers -- just stop the loop. _stop_mcp_loop() also + # acquires _lock, so we must release it first. + pass + else: + servers_snapshot = list(_servers.values()) + # Fast path: nothing to shut down. if not _servers: _stop_mcp_loop() return async def _shutdown(): - for name, server in list(_servers.items()): - try: - await server.shutdown() - except Exception as exc: - logger.debug("Error closing MCP server '%s': %s", name, exc) - _servers.clear() + results = await asyncio.gather( + *(server.shutdown() for server in servers_snapshot), + return_exceptions=True, + ) + for server, result in zip(servers_snapshot, results): + if isinstance(result, Exception): + logger.debug( + "Error closing MCP server '%s': %s", server.name, result, + ) + with _lock: + _servers.clear() - if _mcp_loop is not None and _mcp_loop.is_running(): + with _lock: + loop = _mcp_loop + if loop is not None and loop.is_running(): try: - future = asyncio.run_coroutine_threadsafe(_shutdown(), _mcp_loop) + future = asyncio.run_coroutine_threadsafe(_shutdown(), loop) future.result(timeout=15) except Exception as exc: logger.debug("Error during MCP shutdown: %s", exc) @@ -447,10 +484,13 @@ def shutdown_mcp_servers(): def _stop_mcp_loop(): """Stop the background event loop and join its thread.""" global _mcp_loop, _mcp_thread - if _mcp_loop is not None: - _mcp_loop.call_soon_threadsafe(_mcp_loop.stop) - if _mcp_thread is not None: - _mcp_thread.join(timeout=5) - _mcp_thread = None - _mcp_loop.close() + with _lock: + loop = _mcp_loop + thread = _mcp_thread _mcp_loop = None + _mcp_thread = None + if loop is not None: + loop.call_soon_threadsafe(loop.stop) + if thread is not None: + thread.join(timeout=5) + loop.close() From 64ff8f065b1f4506626fbef29cc509032cdf145e Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 18:40:03 -0800 Subject: [PATCH 07/12] feat(mcp): add HTTP transport, reconnection, security hardening Upgrades the MCP client implementation from PR #291 with: - HTTP/Streamable HTTP transport: support 'url' key in config for remote MCP servers (Notion, Slack, Sentry, Supabase, etc.) - Automatic reconnection with exponential backoff (1s-60s, 5 retries) when a server connection drops unexpectedly - Environment variable filtering: only pass safe vars (PATH, HOME, etc.) plus user-specified env to stdio subprocesses (prevents secret leaks) - Credential stripping: sanitize error messages before returning to the LLM (strips GitHub PATs, OpenAI keys, Bearer tokens, etc.) - Configurable per-server timeouts: 'timeout' and 'connect_timeout' keys - Fix shutdown race condition in servers_snapshot variable scoping Test coverage: 50 tests (up from 30), including new tests for env filtering, credential sanitization, HTTP config detection, reconnection logic, and configurable timeouts. All 1162 tests pass (1162 passed, 3 skipped, 0 failed). --- tests/tools/test_mcp_tool.py | 374 ++++++++++++++++++++++++++++++++++- tools/mcp_tool.py | 302 ++++++++++++++++++++++------ 2 files changed, 611 insertions(+), 65 deletions(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 065baf4a..4b7e2c72 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -5,6 +5,7 @@ All tests use mocks -- no real MCP servers or subprocesses are started. import asyncio import json +import os from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch @@ -189,7 +190,7 @@ class TestToolHandler: _servers["test_srv"] = server try: - handler = _make_tool_handler("test_srv", "greet") + handler = _make_tool_handler("test_srv", "greet", 120) with self._patch_mcp_loop(): result = json.loads(handler({"name": "world"})) assert result["result"] == "hello world" @@ -208,7 +209,7 @@ class TestToolHandler: _servers["test_srv"] = server try: - handler = _make_tool_handler("test_srv", "fail_tool") + handler = _make_tool_handler("test_srv", "fail_tool", 120) with self._patch_mcp_loop(): result = json.loads(handler({})) assert "error" in result @@ -220,7 +221,7 @@ class TestToolHandler: from tools.mcp_tool import _make_tool_handler, _servers _servers.pop("ghost", None) - handler = _make_tool_handler("ghost", "any_tool") + handler = _make_tool_handler("ghost", "any_tool", 120) result = json.loads(handler({})) assert "error" in result assert "not connected" in result["error"] @@ -234,7 +235,7 @@ class TestToolHandler: _servers["test_srv"] = server try: - handler = _make_tool_handler("test_srv", "broken_tool") + handler = _make_tool_handler("test_srv", "broken_tool", 120) with self._patch_mcp_loop(): result = json.loads(handler({})) assert "error" in result @@ -400,8 +401,8 @@ class TestMCPServerTask: asyncio.run(_test()) - def test_empty_env_passed_as_none(self): - """Empty env dict is passed as None to StdioServerParameters.""" + def test_empty_env_gets_safe_defaults(self): + """Empty env dict gets safe default env vars (PATH, HOME, etc.).""" from tools.mcp_tool import MCPServerTask mock_session = MagicMock() @@ -414,13 +415,18 @@ class TestMCPServerTask: async def _test(): with patch("tools.mcp_tool.StdioServerParameters") as mock_params, \ - p_stdio, p_cs: + p_stdio, p_cs, \ + patch.dict("os.environ", {"PATH": "/usr/bin", "HOME": "/home/test"}, clear=False): server = MCPServerTask("srv") await server.start({"command": "node", "env": {}}) - # Empty dict -> None + # Empty dict -> safe env vars (not None) call_kwargs = mock_params.call_args - assert call_kwargs.kwargs.get("env") is None + env_arg = call_kwargs.kwargs.get("env") + assert env_arg is not None + assert isinstance(env_arg, dict) + assert "PATH" in env_arg + assert "HOME" in env_arg await server.shutdown() @@ -698,3 +704,353 @@ class TestShutdown: assert len(_servers) == 0 # Parallel: ~1s, not ~3s. Allow some margin. assert elapsed < 2.5, f"Shutdown took {elapsed:.1f}s, expected ~1s (parallel)" + + +# --------------------------------------------------------------------------- +# _build_safe_env +# --------------------------------------------------------------------------- + +class TestBuildSafeEnv: + """Tests for _build_safe_env() environment filtering.""" + + def test_only_safe_vars_passed(self): + """Only safe baseline vars and XDG_* from os.environ are included.""" + from tools.mcp_tool import _build_safe_env + + fake_env = { + "PATH": "/usr/bin", + "HOME": "/home/test", + "USER": "test", + "LANG": "en_US.UTF-8", + "LC_ALL": "C", + "TERM": "xterm", + "SHELL": "/bin/bash", + "TMPDIR": "/tmp", + "XDG_DATA_HOME": "/home/test/.local/share", + "SECRET_KEY": "should_not_appear", + "AWS_ACCESS_KEY_ID": "AKIAIOSFODNN7EXAMPLE", + } + with patch.dict("os.environ", fake_env, clear=True): + result = _build_safe_env(None) + + # Safe vars present + assert result["PATH"] == "/usr/bin" + assert result["HOME"] == "/home/test" + assert result["USER"] == "test" + assert result["LANG"] == "en_US.UTF-8" + assert result["XDG_DATA_HOME"] == "/home/test/.local/share" + # Unsafe vars excluded + assert "SECRET_KEY" not in result + assert "AWS_ACCESS_KEY_ID" not in result + + def test_user_env_merged(self): + """User-specified env vars are merged into the safe env.""" + from tools.mcp_tool import _build_safe_env + + with patch.dict("os.environ", {"PATH": "/usr/bin"}, clear=True): + result = _build_safe_env({"MY_CUSTOM_VAR": "hello"}) + + assert result["PATH"] == "/usr/bin" + assert result["MY_CUSTOM_VAR"] == "hello" + + def test_user_env_overrides_safe(self): + """User env can override safe defaults.""" + from tools.mcp_tool import _build_safe_env + + with patch.dict("os.environ", {"PATH": "/usr/bin"}, clear=True): + result = _build_safe_env({"PATH": "/custom/bin"}) + + assert result["PATH"] == "/custom/bin" + + def test_none_user_env(self): + """None user_env still returns safe vars from os.environ.""" + from tools.mcp_tool import _build_safe_env + + with patch.dict("os.environ", {"PATH": "/usr/bin", "HOME": "/root"}, clear=True): + result = _build_safe_env(None) + + assert isinstance(result, dict) + assert result["PATH"] == "/usr/bin" + assert result["HOME"] == "/root" + + def test_secret_vars_excluded(self): + """Sensitive env vars from os.environ are NOT passed through.""" + from tools.mcp_tool import _build_safe_env + + fake_env = { + "PATH": "/usr/bin", + "AWS_SECRET_ACCESS_KEY": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "GITHUB_TOKEN": "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + "OPENAI_API_KEY": "sk-proj-abc123", + "DATABASE_URL": "postgres://user:pass@localhost/db", + "API_SECRET": "supersecret", + } + with patch.dict("os.environ", fake_env, clear=True): + result = _build_safe_env(None) + + assert "PATH" in result + assert "AWS_SECRET_ACCESS_KEY" not in result + assert "GITHUB_TOKEN" not in result + assert "OPENAI_API_KEY" not in result + assert "DATABASE_URL" not in result + assert "API_SECRET" not in result + + +# --------------------------------------------------------------------------- +# _sanitize_error +# --------------------------------------------------------------------------- + +class TestSanitizeError: + """Tests for _sanitize_error() credential stripping.""" + + def test_strips_github_pat(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("Error with ghp_abc123def456") + assert result == "Error with [REDACTED]" + + def test_strips_openai_key(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("key sk-projABC123xyz") + assert result == "key [REDACTED]" + + def test_strips_bearer_token(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("Authorization: Bearer eyJabc123def") + assert result == "Authorization: [REDACTED]" + + def test_strips_token_param(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("url?token=secret123") + assert result == "url?[REDACTED]" + + def test_no_credentials_unchanged(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("normal error message") + assert result == "normal error message" + + def test_multiple_credentials(self): + from tools.mcp_tool import _sanitize_error + result = _sanitize_error("ghp_abc123 and sk-projXyz789 and token=foo") + assert "ghp_" not in result + assert "sk-" not in result + assert "token=" not in result + assert result.count("[REDACTED]") == 3 + + +# --------------------------------------------------------------------------- +# HTTP config +# --------------------------------------------------------------------------- + +class TestHTTPConfig: + """Tests for HTTP transport detection and handling.""" + + def test_is_http_with_url(self): + from tools.mcp_tool import MCPServerTask + server = MCPServerTask("remote") + server._config = {"url": "https://example.com/mcp"} + assert server._is_http() is True + + def test_is_stdio_with_command(self): + from tools.mcp_tool import MCPServerTask + server = MCPServerTask("local") + server._config = {"command": "npx", "args": []} + assert server._is_http() is False + + def test_http_unavailable_raises(self): + from tools.mcp_tool import MCPServerTask + + server = MCPServerTask("remote") + config = {"url": "https://example.com/mcp"} + + async def _test(): + with patch("tools.mcp_tool._MCP_HTTP_AVAILABLE", False): + with pytest.raises(ImportError, match="HTTP transport"): + await server._run_http(config) + + asyncio.run(_test()) + + +# --------------------------------------------------------------------------- +# Reconnection logic +# --------------------------------------------------------------------------- + +class TestReconnection: + """Tests for automatic reconnection behavior in MCPServerTask.run().""" + + def test_reconnect_on_disconnect(self): + """After initial success, a connection drop triggers reconnection.""" + from tools.mcp_tool import MCPServerTask + + run_count = 0 + target_server = None + + original_run_stdio = MCPServerTask._run_stdio + + async def patched_run_stdio(self_srv, config): + nonlocal run_count, target_server + run_count += 1 + if target_server is not self_srv: + return await original_run_stdio(self_srv, config) + if run_count == 1: + # First connection succeeds, then simulate disconnect + self_srv.session = MagicMock() + self_srv._tools = [] + self_srv._ready.set() + raise ConnectionError("connection dropped") + else: + # Reconnection succeeds; signal shutdown so run() exits + self_srv.session = MagicMock() + self_srv._shutdown_event.set() + await self_srv._shutdown_event.wait() + + async def _test(): + nonlocal target_server + server = MCPServerTask("test_srv") + target_server = server + + with patch.object(MCPServerTask, "_run_stdio", patched_run_stdio), \ + patch("asyncio.sleep", new_callable=AsyncMock): + await server.run({"command": "test"}) + + assert run_count >= 2 # At least one reconnection attempt + + asyncio.run(_test()) + + def test_no_reconnect_on_shutdown(self): + """If shutdown is requested, don't attempt reconnection.""" + from tools.mcp_tool import MCPServerTask + + run_count = 0 + target_server = None + + original_run_stdio = MCPServerTask._run_stdio + + async def patched_run_stdio(self_srv, config): + nonlocal run_count, target_server + run_count += 1 + if target_server is not self_srv: + return await original_run_stdio(self_srv, config) + self_srv.session = MagicMock() + self_srv._tools = [] + self_srv._ready.set() + raise ConnectionError("connection dropped") + + async def _test(): + nonlocal target_server + server = MCPServerTask("test_srv") + target_server = server + server._shutdown_event.set() # Shutdown already requested + + with patch.object(MCPServerTask, "_run_stdio", patched_run_stdio), \ + patch("asyncio.sleep", new_callable=AsyncMock): + await server.run({"command": "test"}) + + # Should not retry because shutdown was set + assert run_count == 1 + + asyncio.run(_test()) + + def test_no_reconnect_on_initial_failure(self): + """First connection failure reports error immediately, no retry.""" + from tools.mcp_tool import MCPServerTask + + run_count = 0 + target_server = None + + original_run_stdio = MCPServerTask._run_stdio + + async def patched_run_stdio(self_srv, config): + nonlocal run_count, target_server + run_count += 1 + if target_server is not self_srv: + return await original_run_stdio(self_srv, config) + raise ConnectionError("cannot connect") + + async def _test(): + nonlocal target_server + server = MCPServerTask("test_srv") + target_server = server + + with patch.object(MCPServerTask, "_run_stdio", patched_run_stdio), \ + patch("asyncio.sleep", new_callable=AsyncMock): + await server.run({"command": "test"}) + + # Only one attempt, no retry on initial failure + assert run_count == 1 + assert server._error is not None + assert "cannot connect" in str(server._error) + + asyncio.run(_test()) + + +# --------------------------------------------------------------------------- +# Configurable timeouts +# --------------------------------------------------------------------------- + +class TestConfigurableTimeouts: + """Tests for configurable per-server timeouts.""" + + def test_default_timeout(self): + """Server with no timeout config gets _DEFAULT_TOOL_TIMEOUT.""" + from tools.mcp_tool import MCPServerTask, _DEFAULT_TOOL_TIMEOUT + + server = MCPServerTask("test_srv") + assert server.tool_timeout == _DEFAULT_TOOL_TIMEOUT + assert server.tool_timeout == 120 + + def test_custom_timeout(self): + """Server with timeout=180 in config gets 180.""" + from tools.mcp_tool import MCPServerTask + + target_server = None + + original_run_stdio = MCPServerTask._run_stdio + + async def patched_run_stdio(self_srv, config): + if target_server is not self_srv: + return await original_run_stdio(self_srv, config) + self_srv.session = MagicMock() + self_srv._tools = [] + self_srv._ready.set() + await self_srv._shutdown_event.wait() + + async def _test(): + nonlocal target_server + server = MCPServerTask("test_srv") + target_server = server + + with patch.object(MCPServerTask, "_run_stdio", patched_run_stdio): + task = asyncio.ensure_future( + server.run({"command": "test", "timeout": 180}) + ) + await server._ready.wait() + assert server.tool_timeout == 180 + server._shutdown_event.set() + await task + + asyncio.run(_test()) + + def test_timeout_passed_to_handler(self): + """The tool handler uses the server's configured timeout.""" + from tools.mcp_tool import _make_tool_handler, _servers, MCPServerTask + + mock_session = MagicMock() + mock_session.call_tool = AsyncMock( + return_value=_make_call_result("ok", is_error=False) + ) + server = _make_mock_server("test_srv", session=mock_session) + server.tool_timeout = 180 + _servers["test_srv"] = server + + try: + handler = _make_tool_handler("test_srv", "my_tool", 180) + with patch("tools.mcp_tool._run_on_mcp_loop") as mock_run: + mock_run.return_value = json.dumps({"result": "ok"}) + handler({}) + # Verify timeout=180 was passed + call_kwargs = mock_run.call_args + assert call_kwargs.kwargs.get("timeout") == 180 or \ + (len(call_kwargs.args) > 1 and call_kwargs.args[1] == 180) or \ + call_kwargs[1].get("timeout") == 180 + finally: + _servers.pop("test_srv", None) diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 4ab55215..1419327c 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -2,9 +2,9 @@ """ MCP (Model Context Protocol) Client Support -Connects to external MCP servers via stdio transport, discovers their tools, -and registers them into the hermes-agent tool registry so the agent can call -them like any built-in tool. +Connects to external MCP servers via stdio or HTTP/StreamableHTTP transport, +discovers their tools, and registers them into the hermes-agent tool registry +so the agent can call them like any built-in tool. Configuration is read from ~/.hermes/config.yaml under the ``mcp_servers`` key. The ``mcp`` Python package is optional -- if not installed, this module is a @@ -17,17 +17,32 @@ Example config:: command: "npx" args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] env: {} + timeout: 120 # per-tool-call timeout in seconds (default: 120) + connect_timeout: 60 # initial connection timeout (default: 60) github: command: "npx" args: ["-y", "@modelcontextprotocol/server-github"] env: GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_..." + remote_api: + url: "https://my-mcp-server.example.com/mcp" + headers: + Authorization: "Bearer sk-..." + timeout: 180 + +Features: + - Stdio transport (command + args) and HTTP/StreamableHTTP transport (url) + - Automatic reconnection with exponential backoff (up to 5 retries) + - Environment variable filtering for stdio subprocesses (security) + - Credential stripping in error messages returned to the LLM + - Configurable per-server timeouts for tool calls and connections + - Thread-safe architecture with dedicated background event loop Architecture: A dedicated background event loop (_mcp_loop) runs in a daemon thread. Each MCP server runs as a long-lived asyncio Task on this loop, keeping - its ``async with stdio_client(...)`` context alive. Tool call coroutines - are scheduled onto the loop via ``run_coroutine_threadsafe()``. + its transport context alive. Tool call coroutines are scheduled onto the + loop via ``run_coroutine_threadsafe()``. On shutdown, each server Task is signalled to exit its ``async with`` block, ensuring the anyio cancel-scope cleanup happens in the *same* @@ -43,6 +58,8 @@ Thread safety: import asyncio import json import logging +import os +import re import threading from typing import Any, Dict, List, Optional @@ -53,13 +70,81 @@ logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- _MCP_AVAILABLE = False +_MCP_HTTP_AVAILABLE = False try: from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client _MCP_AVAILABLE = True + try: + from mcp.client.streamable_http import streamablehttp_client + _MCP_HTTP_AVAILABLE = True + except ImportError: + _MCP_HTTP_AVAILABLE = False except ImportError: logger.debug("mcp package not installed -- MCP tool support disabled") +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +_DEFAULT_TOOL_TIMEOUT = 120 # seconds for tool calls +_DEFAULT_DISCOVERY_TIMEOUT = 60 # seconds for server discovery +_DEFAULT_CONNECT_TIMEOUT = 60 # seconds for initial connection +_MAX_RECONNECT_RETRIES = 5 +_MAX_BACKOFF_SECONDS = 60 + +# Environment variables that are safe to pass to stdio subprocesses +_SAFE_ENV_KEYS = frozenset({ + "PATH", "HOME", "USER", "LANG", "LC_ALL", "TERM", "SHELL", "TMPDIR", +}) + +# Regex for credential patterns to strip from error messages +_CREDENTIAL_PATTERN = re.compile( + r"(?:" + r"ghp_[A-Za-z0-9_]{1,255}" # GitHub PAT + r"|sk-[A-Za-z0-9_]{1,255}" # OpenAI-style key + r"|Bearer\s+\S+" # Bearer token + r"|token=[^\s&,;\"']{1,255}" # token=... + r"|key=[^\s&,;\"']{1,255}" # key=... + r"|API_KEY=[^\s&,;\"']{1,255}" # API_KEY=... + r"|password=[^\s&,;\"']{1,255}" # password=... + r"|secret=[^\s&,;\"']{1,255}" # secret=... + r")", + re.IGNORECASE, +) + + +# --------------------------------------------------------------------------- +# Security helpers +# --------------------------------------------------------------------------- + +def _build_safe_env(user_env: Optional[dict]) -> dict: + """Build a filtered environment dict for stdio subprocesses. + + Only passes through safe baseline variables (PATH, HOME, etc.) and XDG_* + variables from the current process environment, plus any variables + explicitly specified by the user in the server config. + + This prevents accidentally leaking secrets like API keys, tokens, or + credentials to MCP server subprocesses. + """ + env = {} + for key, value in os.environ.items(): + if key in _SAFE_ENV_KEYS or key.startswith("XDG_"): + env[key] = value + if user_env: + env.update(user_env) + return env + + +def _sanitize_error(text: str) -> str: + """Strip credential-like patterns from error text before returning to LLM. + + Replaces tokens, keys, and other secrets with [REDACTED] to prevent + accidental credential exposure in tool error responses. + """ + return _CREDENTIAL_PATTERN.sub("[REDACTED]", text) + # --------------------------------------------------------------------------- # Server task -- each MCP server lives in one long-lived asyncio Task @@ -70,66 +155,152 @@ class MCPServerTask: The entire connection lifecycle (connect, discover, serve, disconnect) runs inside one asyncio Task so that anyio cancel-scopes created by - ``stdio_client`` are entered and exited in the same Task context. + the transport client are entered and exited in the same Task context. + + Supports both stdio and HTTP/StreamableHTTP transports. """ __slots__ = ( - "name", "session", - "_task", "_ready", "_shutdown_event", "_tools", "_error", + "name", "session", "tool_timeout", + "_task", "_ready", "_shutdown_event", "_tools", "_error", "_config", ) def __init__(self, name: str): self.name = name self.session: Optional[Any] = None + self.tool_timeout: float = _DEFAULT_TOOL_TIMEOUT self._task: Optional[asyncio.Task] = None self._ready = asyncio.Event() self._shutdown_event = asyncio.Event() self._tools: list = [] self._error: Optional[Exception] = None + self._config: dict = {} - async def run(self, config: dict): - """Long-lived coroutine: connect, discover tools, wait, disconnect.""" + def _is_http(self) -> bool: + """Check if this server uses HTTP transport.""" + return "url" in self._config + + async def _run_stdio(self, config: dict): + """Run the server using stdio transport.""" command = config.get("command") args = config.get("args", []) - env = config.get("env") + user_env = config.get("env") if not command: - self._error = ValueError( + raise ValueError( f"MCP server '{self.name}' has no 'command' in config" ) - self._ready.set() - return + safe_env = _build_safe_env(user_env) server_params = StdioServerParameters( command=command, args=args, - env=env if env else None, + env=safe_env if safe_env else None, ) - try: - async with stdio_client(server_params) as (read_stream, write_stream): - async with ClientSession(read_stream, write_stream) as session: - await session.initialize() - self.session = session + async with stdio_client(server_params) as (read_stream, write_stream): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + self.session = session + await self._discover_tools() + self._ready.set() + await self._shutdown_event.wait() - tools_result = await session.list_tools() - self._tools = ( - tools_result.tools - if hasattr(tools_result, "tools") - else [] - ) + async def _run_http(self, config: dict): + """Run the server using HTTP/StreamableHTTP transport.""" + if not _MCP_HTTP_AVAILABLE: + raise ImportError( + f"MCP server '{self.name}' requires HTTP transport but " + "mcp.client.streamable_http is not available. " + "Upgrade the mcp package to get HTTP support." + ) - # Signal that connection is ready + url = config["url"] + headers = config.get("headers") + connect_timeout = config.get("connect_timeout", _DEFAULT_CONNECT_TIMEOUT) + + async with streamablehttp_client( + url, + headers=headers, + timeout=float(connect_timeout), + ) as (read_stream, write_stream, _get_session_id): + async with ClientSession(read_stream, write_stream) as session: + await session.initialize() + self.session = session + await self._discover_tools() + self._ready.set() + await self._shutdown_event.wait() + + async def _discover_tools(self): + """Discover tools from the connected session.""" + if self.session is None: + return + tools_result = await self.session.list_tools() + self._tools = ( + tools_result.tools + if hasattr(tools_result, "tools") + else [] + ) + + async def run(self, config: dict): + """Long-lived coroutine: connect, discover tools, wait, disconnect. + + Includes automatic reconnection with exponential backoff if the + connection drops unexpectedly (unless shutdown was requested). + """ + self._config = config + self.tool_timeout = config.get("timeout", _DEFAULT_TOOL_TIMEOUT) + retries = 0 + backoff = 1.0 + + while True: + try: + if self._is_http(): + await self._run_http(config) + else: + await self._run_stdio(config) + # Normal exit (shutdown requested) -- break out + break + except Exception as exc: + self.session = None + + # If this is the first connection attempt, report the error + if not self._ready.is_set(): + self._error = exc self._ready.set() + return - # Block until shutdown is requested -- this keeps the - # async-with contexts alive on THIS Task. - await self._shutdown_event.wait() - except Exception as exc: - self._error = exc - self._ready.set() - finally: - self.session = None + # If shutdown was requested, don't reconnect + if self._shutdown_event.is_set(): + logger.debug( + "MCP server '%s' disconnected during shutdown: %s", + self.name, exc, + ) + return + + retries += 1 + if retries > _MAX_RECONNECT_RETRIES: + logger.warning( + "MCP server '%s' failed after %d reconnection attempts, " + "giving up: %s", + self.name, _MAX_RECONNECT_RETRIES, exc, + ) + return + + logger.warning( + "MCP server '%s' connection lost (attempt %d/%d), " + "reconnecting in %.0fs: %s", + self.name, retries, _MAX_RECONNECT_RETRIES, + backoff, exc, + ) + await asyncio.sleep(backoff) + backoff = min(backoff * 2, _MAX_BACKOFF_SECONDS) + + # Check again after sleeping + if self._shutdown_event.is_set(): + return + finally: + self.session = None async def start(self, config: dict): """Create the background Task and wait until ready (or failed).""" @@ -203,7 +374,10 @@ def _run_on_mcp_loop(coro, timeout: float = 30): def _load_mcp_config() -> Dict[str, dict]: """Read ``mcp_servers`` from the Hermes config file. - Returns a dict of ``{server_name: {command, args, env}}`` or empty dict. + Returns a dict of ``{server_name: server_config}`` or empty dict. + Server config can contain either ``command``/``args``/``env`` for stdio + transport or ``url``/``headers`` for HTTP transport, plus optional + ``timeout`` and ``connect_timeout`` overrides. """ try: from hermes_cli.config import load_config @@ -224,11 +398,12 @@ def _load_mcp_config() -> Dict[str, dict]: async def _connect_server(name: str, config: dict) -> MCPServerTask: """Create an MCPServerTask, start it, and return when ready. - The server Task keeps the subprocess alive in the background. + The server Task keeps the connection alive in the background. Call ``server.shutdown()`` (on the same event loop) to tear it down. Raises: - ValueError: if ``command`` is missing from *config*. + ValueError: if required config keys are missing. + ImportError: if HTTP transport is needed but not available. Exception: on connection or initialization failure. """ server = MCPServerTask(name) @@ -240,7 +415,7 @@ async def _connect_server(name: str, config: dict) -> MCPServerTask: # Handler / check-fn factories # --------------------------------------------------------------------------- -def _make_tool_handler(server_name: str, tool_name: str): +def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): """Return a sync handler that calls an MCP tool via the background loop. The handler conforms to the registry's dispatch interface: @@ -263,7 +438,11 @@ def _make_tool_handler(server_name: str, tool_name: str): for block in (result.content or []): if hasattr(block, "text"): error_text += block.text - return json.dumps({"error": error_text or "MCP tool returned an error"}) + return json.dumps({ + "error": _sanitize_error( + error_text or "MCP tool returned an error" + ) + }) # Collect text from content blocks parts: List[str] = [] @@ -273,10 +452,17 @@ def _make_tool_handler(server_name: str, tool_name: str): return json.dumps({"result": "\n".join(parts) if parts else ""}) try: - return _run_on_mcp_loop(_call(), timeout=120) + return _run_on_mcp_loop(_call(), timeout=tool_timeout) except Exception as exc: - logger.error("MCP tool %s/%s call failed: %s", server_name, tool_name, exc) - return json.dumps({"error": f"MCP call failed: {type(exc).__name__}: {exc}"}) + logger.error( + "MCP tool %s/%s call failed: %s", + server_name, tool_name, exc, + ) + return json.dumps({ + "error": _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {exc}" + ) + }) return _handler @@ -339,7 +525,11 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: from tools.registry import registry from toolsets import create_custom_toolset - server = await _connect_server(name, config) + connect_timeout = config.get("connect_timeout", _DEFAULT_CONNECT_TIMEOUT) + server = await asyncio.wait_for( + _connect_server(name, config), + timeout=connect_timeout, + ) with _lock: _servers[name] = server @@ -354,7 +544,7 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: name=tool_name_prefixed, toolset=toolset_name, schema=schema, - handler=_make_tool_handler(name, mcp_tool.name), + handler=_make_tool_handler(name, mcp_tool.name, server.tool_timeout), check_fn=_make_check_fn(name), is_async=False, description=schema["description"], @@ -369,9 +559,11 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: tools=registered_names, ) + transport_type = "HTTP" if "url" in config else "stdio" logger.info( - "MCP server '%s': registered %d tool(s): %s", - name, len(registered_names), ", ".join(registered_names), + "MCP server '%s' (%s): registered %d tool(s): %s", + name, transport_type, len(registered_names), + ", ".join(registered_names), ) return registered_names @@ -419,9 +611,12 @@ def discover_mcp_tools() -> List[str]: registered = await _discover_and_register_server(name, cfg) all_tools.extend(registered) except Exception as exc: - logger.warning("Failed to connect to MCP server '%s': %s", name, exc) + logger.warning( + "Failed to connect to MCP server '%s': %s", + name, exc, + ) - _run_on_mcp_loop(_discover_all(), timeout=60) + _run_on_mcp_loop(_discover_all(), timeout=_DEFAULT_DISCOVERY_TIMEOUT) if all_tools: # Dynamically inject into all hermes-* platform toolsets @@ -444,15 +639,10 @@ def shutdown_mcp_servers(): All servers are shut down in parallel via ``asyncio.gather``. """ with _lock: - if not _servers: - # No servers -- just stop the loop. _stop_mcp_loop() also - # acquires _lock, so we must release it first. - pass - else: - servers_snapshot = list(_servers.values()) + servers_snapshot = list(_servers.values()) # Fast path: nothing to shut down. - if not _servers: + if not servers_snapshot: _stop_mcp_loop() return From 63f5e14c6993bcec5c5a51d2e27d86a2be6897ca Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 18:52:33 -0800 Subject: [PATCH 08/12] docs: add comprehensive MCP documentation and examples - docs/mcp.md: Full MCP documentation covering prerequisites, configuration, transports (stdio + HTTP), security (env filtering, credential stripping), reconnection, troubleshooting, popular servers, and advanced usage - README.md: Add MCP section with quick config example and install instructions - cli-config.yaml.example: Add commented mcp_servers section with examples for stdio, HTTP, and authenticated server configs - docs/tools.md: Add MCP to Tool Categories table and MCP Tools section - skills/mcp/native-mcp/SKILL.md: Create native MCP client skill with full configuration reference, transport types, security, troubleshooting - skills/mcp/DESCRIPTION.md: Update category description to cover both native MCP client and mcporter bridge approaches --- README.md | 17 ++ cli-config.yaml.example | 35 +++ docs/mcp.md | 527 +++++++++++++++++++++++++++++++++ docs/tools.md | 18 ++ skills/mcp/DESCRIPTION.md | 2 +- skills/mcp/native-mcp/SKILL.md | 330 +++++++++++++++++++++ 6 files changed, 928 insertions(+), 1 deletion(-) create mode 100644 docs/mcp.md create mode 100644 skills/mcp/native-mcp/SKILL.md diff --git a/README.md b/README.md index d038cd58..c6891b83 100644 --- a/README.md +++ b/README.md @@ -496,6 +496,23 @@ hermes tools **Available toolsets:** `web`, `terminal`, `file`, `browser`, `vision`, `image_gen`, `moa`, `skills`, `tts`, `todo`, `memory`, `session_search`, `cronjob`, `code_execution`, `delegation`, `clarify`, and more. +### 🔌 MCP (Model Context Protocol) + +Connect to any MCP-compatible server to extend Hermes with external tools. Just add servers to your config: + +```yaml +mcp_servers: + time: + command: uvx + args: ["mcp-server-time"] + notion: + url: https://mcp.notion.com/mcp +``` + +Supports stdio and HTTP transports, auto-reconnection, and env var filtering. See [docs/mcp.md](docs/mcp.md) for details. + +Install MCP support: `pip install hermes-agent[mcp]` + ### 🖥️ Terminal & Process Management The terminal tool can execute commands in different environments, with full background process management via the `process` tool: diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 9fcf11d5..170c142b 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -442,6 +442,41 @@ toolsets: # toolsets: # - safe +# ============================================================================= +# MCP (Model Context Protocol) Servers +# ============================================================================= +# Connect to external MCP servers to add tools from the MCP ecosystem. +# Each server's tools are automatically discovered and registered. +# See docs/mcp.md for full documentation. +# +# Stdio servers (spawn a subprocess): +# command: the executable to run +# args: command-line arguments +# env: environment variables (only these + safe defaults passed to subprocess) +# +# HTTP servers (connect to a URL): +# url: the MCP server endpoint +# headers: HTTP headers (e.g., for authentication) +# +# Optional per-server settings: +# timeout: tool call timeout in seconds (default: 120) +# connect_timeout: initial connection timeout (default: 60) +# +# mcp_servers: +# time: +# command: uvx +# args: ["mcp-server-time"] +# filesystem: +# command: npx +# args: ["-y", "@modelcontextprotocol/server-filesystem", "/home/user"] +# notion: +# url: https://mcp.notion.com/mcp +# github: +# command: npx +# args: ["-y", "@modelcontextprotocol/server-github"] +# env: +# GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_..." + # ============================================================================= # Voice Transcription (Speech-to-Text) # ============================================================================= diff --git a/docs/mcp.md b/docs/mcp.md new file mode 100644 index 00000000..1017f61c --- /dev/null +++ b/docs/mcp.md @@ -0,0 +1,527 @@ +# MCP (Model Context Protocol) Support + +MCP lets Hermes Agent connect to external tool servers — giving the agent access to databases, APIs, filesystems, and more without any code changes. + +## Overview + +The [Model Context Protocol](https://modelcontextprotocol.io/) (MCP) is an open standard for connecting AI agents to external tools and data sources. MCP servers expose tools over a lightweight RPC protocol, and Hermes Agent can connect to any compliant server automatically. + +What this means for you: + +- **Thousands of ready-made tools** — browse the [MCP server directory](https://github.com/modelcontextprotocol/servers) for servers covering GitHub, Slack, databases, file systems, web scraping, and more. +- **No code changes needed** — add a few lines to `~/.hermes/config.yaml` and the tools appear alongside built-in ones. +- **Mix and match** — run multiple MCP servers simultaneously, combining stdio-based and HTTP-based servers. +- **Secure by default** — environment variables are filtered and credentials are stripped from error messages returned to the LLM. + +## Prerequisites + +Install MCP support as an optional dependency: + +```bash +pip install hermes-agent[mcp] +``` + +Depending on which MCP servers you want to use, you may need additional runtimes: + +| Server Type | Runtime Needed | Example | +|-------------|---------------|---------| +| HTTP/remote | Nothing extra | `url: "https://mcp.example.com"` | +| npm-based (npx) | Node.js 18+ | `command: "npx"` | +| Python-based | uv (recommended) | `command: "uvx"` | + +Most popular MCP servers are distributed as npm packages and launched via `npx`. Python-based servers typically use `uvx` (from the [uv](https://docs.astral.sh/uv/) package manager). + +## Configuration + +MCP servers are configured in `~/.hermes/config.yaml` under the `mcp_servers` key. Each entry is a named server with its connection details. + +### Stdio Servers (command + args + env) + +Stdio servers run as local subprocesses. Communication happens over stdin/stdout. + +```yaml +mcp_servers: + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/home/user/projects"] + env: {} + + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxx" +``` + +| Key | Required | Description | +|-----|----------|-------------| +| `command` | Yes | Executable to run (e.g., `npx`, `uvx`, `python`) | +| `args` | No | List of command-line arguments | +| `env` | No | Environment variables to pass to the subprocess | + +**Note:** Only explicitly listed `env` variables plus a safe baseline (PATH, HOME, USER, LANG, SHELL, TMPDIR, XDG_*) are passed to the subprocess. Your shell's API keys, tokens, and secrets are **not** leaked. See [Security](#security) for details. + +### HTTP Servers (url + headers) + +HTTP servers run remotely and are accessed over HTTP/StreamableHTTP. + +```yaml +mcp_servers: + remote_api: + url: "https://my-mcp-server.example.com/mcp" + headers: + Authorization: "Bearer sk-xxxxxxxxxxxx" +``` + +| Key | Required | Description | +|-----|----------|-------------| +| `url` | Yes | Full URL of the MCP HTTP endpoint | +| `headers` | No | HTTP headers to include (e.g., auth tokens) | + +### Per-Server Timeouts + +Each server can have custom timeouts: + +```yaml +mcp_servers: + slow_database: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-postgres"] + env: + DATABASE_URL: "postgres://user:pass@localhost/mydb" + timeout: 300 # Tool call timeout in seconds (default: 120) + connect_timeout: 90 # Initial connection timeout in seconds (default: 60) +``` + +| Key | Default | Description | +|-----|---------|-------------| +| `timeout` | 120 | Maximum seconds to wait for a single tool call to complete | +| `connect_timeout` | 60 | Maximum seconds to wait for the initial connection and tool discovery | + +### Mixed Configuration Example + +You can combine stdio and HTTP servers freely: + +```yaml +mcp_servers: + # Local filesystem access via stdio + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + + # GitHub API via stdio with auth + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxx" + + # Remote database via HTTP + company_db: + url: "https://mcp.internal.company.com/db" + headers: + Authorization: "Bearer sk-xxxxxxxxxxxx" + timeout: 180 + + # Python-based server via uvx + memory: + command: "uvx" + args: ["mcp-server-memory"] +``` + +## Config Translation (Claude/Cursor JSON → Hermes YAML) + +Many MCP server docs show configuration in Claude Desktop JSON format. Here's how to translate: + +**Claude Desktop JSON** (`claude_desktop_config.json`): + +```json +{ + "mcpServers": { + "filesystem": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + "env": {} + }, + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github"], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "ghp_xxxxxxxxxxxx" + } + } + } +} +``` + +**Hermes Agent YAML** (`~/.hermes/config.yaml`): + +```yaml +mcp_servers: # mcpServers → mcp_servers (snake_case) + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + env: {} + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxx" +``` + +Translation rules: + +1. **Key name**: `mcpServers` → `mcp_servers` (snake_case) +2. **Format**: JSON → YAML (remove braces/brackets, use indentation) +3. **Arrays**: `["a", "b"]` stays the same in YAML flow style, or use block style with `- a` +4. **Everything else**: Keys (`command`, `args`, `env`) are identical + +## How It Works + +### Startup & Discovery + +When Hermes Agent starts, the tool discovery system calls `discover_mcp_tools()`: + +1. **Config loading** — Reads `mcp_servers` from `~/.hermes/config.yaml` +2. **Background loop** — Spins up a dedicated asyncio event loop in a daemon thread for MCP connections +3. **Connection** — Connects to each configured server (stdio subprocess or HTTP) +4. **Session init** — Initializes the MCP client session (protocol handshake) +5. **Tool discovery** — Calls `list_tools()` on each server to get available tools +6. **Registration** — Registers each MCP tool into the Hermes tool registry with a prefixed name + +### Tool Registration + +Each discovered MCP tool is registered with a prefixed name following this pattern: + +``` +mcp_{server_name}_{tool_name} +``` + +Hyphens and dots in both server and tool names are replaced with underscores for API compatibility. For example: + +| Server Name | MCP Tool Name | Registered As | +|-------------|--------------|---------------| +| `filesystem` | `read_file` | `mcp_filesystem_read_file` | +| `github` | `create-issue` | `mcp_github_create_issue` | +| `my-api` | `query.data` | `mcp_my_api_query_data` | + +Tools appear alongside built-in tools — the agent sees them in its tool list and can call them like any other tool. + +### Tool Calling + +When the agent calls an MCP tool: + +1. The handler is invoked by the tool registry (sync interface) +2. The handler schedules the actual MCP `call_tool()` RPC on the background event loop +3. The call blocks (with timeout) until the MCP server responds +4. Response content blocks are collected and returned as JSON +5. Errors are sanitized to strip credentials before returning to the LLM + +### Shutdown + +On agent exit, `shutdown_mcp_servers()` is called: + +1. All server tasks are signalled to exit via their shutdown events +2. Each server's `async with` context manager exits, cleaning up transports +3. The background event loop is stopped and its thread is joined +4. All server state is cleared + +## Security + +### Environment Variable Filtering + +When launching stdio MCP servers, Hermes does **not** pass your full shell environment to the subprocess. The `_build_safe_env()` function constructs a minimal environment: + +**Always passed through** (from your current environment): +- `PATH`, `HOME`, `USER`, `LANG`, `LC_ALL`, `TERM`, `SHELL`, `TMPDIR` +- Any variable starting with `XDG_` + +**Explicitly added**: Any variables you list in the server's `env` config. + +**Everything else is excluded** — your `OPENAI_API_KEY`, `AWS_SECRET_ACCESS_KEY`, database passwords, and other secrets are never leaked to MCP server subprocesses unless you explicitly add them. + +```yaml +mcp_servers: + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + # Only this token is passed — nothing else from your shell + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxx" +``` + +### Credential Stripping in Errors + +If an MCP tool call fails, the error message is sanitized by `_sanitize_error()` before being returned to the LLM. The following patterns are replaced with `[REDACTED]`: + +- GitHub PATs (`ghp_...`) +- OpenAI-style keys (`sk-...`) +- Bearer tokens (`Bearer ...`) +- Query parameters (`token=...`, `key=...`, `API_KEY=...`, `password=...`, `secret=...`) + +This prevents accidental credential exposure through error messages in the conversation. + +## Transport Types + +### Stdio Transport + +The default transport for locally-installed MCP servers. The server runs as a subprocess and communicates over stdin/stdout. + +```yaml +mcp_servers: + my_server: + command: "npx" # or "uvx", "python", any executable + args: ["-y", "package"] + env: + MY_VAR: "value" +``` + +**Pros:** Simple setup, no network needed, works offline. +**Cons:** Server must be installed locally, one process per server. + +### HTTP / StreamableHTTP Transport + +For remote MCP servers accessible over HTTP. Uses the StreamableHTTP protocol from the MCP SDK. + +```yaml +mcp_servers: + my_remote: + url: "https://mcp.example.com/endpoint" + headers: + Authorization: "Bearer token" +``` + +**Pros:** No local installation needed, shared servers, cloud-hosted. +**Cons:** Requires network, slightly higher latency, needs `mcp` package with HTTP support. + +**Note:** If HTTP transport is not available in your installed `mcp` package version, Hermes will log a clear error and skip that server. + +## Reconnection + +If an MCP server connection drops after initial setup (e.g., process crash, network hiccup), Hermes automatically attempts to reconnect with exponential backoff: + +| Attempt | Delay Before Retry | +|---------|--------------------| +| 1 | 1 second | +| 2 | 2 seconds | +| 3 | 4 seconds | +| 4 | 8 seconds | +| 5 | 16 seconds | + +- Maximum of **5 retry attempts** before giving up +- Backoff is capped at **60 seconds** (relevant if the formula exceeds this) +- Reconnection only triggers for **established connections** that drop — initial connection failures are reported immediately without retries +- If shutdown is requested during reconnection, the retry loop exits cleanly + +## Troubleshooting + +### Common Errors + +**"mcp package not installed"** + +``` +MCP SDK not available -- skipping MCP tool discovery +``` + +Solution: Install the MCP optional dependency: + +```bash +pip install hermes-agent[mcp] +``` + +--- + +**"command not found" or server fails to start** + +The MCP server command (`npx`, `uvx`, etc.) is not on PATH. + +Solution: Install the required runtime: + +```bash +# For npm-based servers +npm install -g npx # or ensure Node.js 18+ is installed + +# For Python-based servers +pip install uv # then use "uvx" as the command +``` + +--- + +**"MCP server 'X' has no 'command' in config"** + +Your stdio server config is missing the `command` key. + +Solution: Check your `~/.hermes/config.yaml` indentation and ensure `command` is present: + +```yaml +mcp_servers: + my_server: + command: "npx" # <-- required for stdio servers + args: ["-y", "package-name"] +``` + +--- + +**Server connects but tools fail with authentication errors** + +Your API key or token is missing or invalid. + +Solution: Ensure the key is in the server's `env` block (not your shell env): + +```yaml +mcp_servers: + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_your_actual_token" # <-- check this +``` + +--- + +**"MCP server 'X' is not connected"** + +The server disconnected and reconnection failed (or was never established). + +Solution: +1. Check the Hermes logs for connection errors (`hermes --verbose`) +2. Verify the server works standalone (e.g., run the `npx` command manually) +3. Increase `connect_timeout` if the server is slow to start + +--- + +**Connection timeout during discovery** + +``` +Failed to connect to MCP server 'X': TimeoutError +``` + +Solution: Increase the `connect_timeout` for slow-starting servers: + +```yaml +mcp_servers: + slow_server: + command: "npx" + args: ["-y", "heavy-server-package"] + connect_timeout: 120 # default is 60 +``` + +--- + +**HTTP transport not available** + +``` +mcp.client.streamable_http is not available +``` + +Solution: Upgrade the `mcp` package to a version that includes HTTP support: + +```bash +pip install --upgrade mcp +``` + +## Popular MCP Servers + +Here are some popular free MCP servers you can use immediately: + +| Server | Package | Description | +|--------|---------|-------------| +| Filesystem | `@modelcontextprotocol/server-filesystem` | Read/write/search local files | +| GitHub | `@modelcontextprotocol/server-github` | Issues, PRs, repos, code search | +| Git | `@modelcontextprotocol/server-git` | Git operations on local repos | +| Fetch | `@modelcontextprotocol/server-fetch` | HTTP fetching and web content extraction | +| Memory | `@modelcontextprotocol/server-memory` | Persistent key-value memory | +| SQLite | `@modelcontextprotocol/server-sqlite` | Query SQLite databases | +| PostgreSQL | `@modelcontextprotocol/server-postgres` | Query PostgreSQL databases | +| Brave Search | `@modelcontextprotocol/server-brave-search` | Web search via Brave API | +| Puppeteer | `@modelcontextprotocol/server-puppeteer` | Browser automation | +| Sequential Thinking | `@modelcontextprotocol/server-sequential-thinking` | Step-by-step reasoning | + +### Example Configs for Popular Servers + +```yaml +mcp_servers: + # Filesystem — no API key needed + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/home/user/projects"] + + # Git — no API key needed + git: + command: "uvx" + args: ["mcp-server-git", "--repository", "/home/user/my-repo"] + + # GitHub — requires a personal access token + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxx" + + # Fetch — no API key needed + fetch: + command: "uvx" + args: ["mcp-server-fetch"] + + # SQLite — no API key needed + sqlite: + command: "uvx" + args: ["mcp-server-sqlite", "--db-path", "/home/user/data.db"] + + # Brave Search — requires API key (free tier available) + brave_search: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-brave-search"] + env: + BRAVE_API_KEY: "BSA_xxxxxxxxxxxx" +``` + +## Advanced + +### Multiple Servers + +You can run as many MCP servers as you want simultaneously. Each server gets its own subprocess (stdio) or HTTP connection, and all tools are registered into a single unified namespace. + +Servers are connected sequentially during startup. If one server fails to connect, the others still work — failed servers are logged as warnings and skipped. + +### Tool Naming Convention + +All MCP tools follow the naming pattern: + +``` +mcp_{server_name}_{tool_name} +``` + +Both the server name and tool name are sanitized: hyphens (`-`) and dots (`.`) are replaced with underscores (`_`). This ensures compatibility with LLM function-calling APIs that restrict tool name characters. + +If you configure a server named `my-api` that exposes a tool called `query.users`, the agent will see it as `mcp_my_api_query_users`. + +### Configurable Timeouts + +Fine-tune timeouts per server based on expected response times: + +```yaml +mcp_servers: + fast_cache: + command: "npx" + args: ["-y", "mcp-server-redis"] + timeout: 30 # Fast lookups — short timeout + connect_timeout: 15 + + slow_analysis: + url: "https://analysis.example.com/mcp" + timeout: 600 # Long-running analysis — generous timeout + connect_timeout: 120 +``` + +### Idempotent Discovery + +`discover_mcp_tools()` is idempotent — calling it multiple times only connects to servers that aren't already running. Already-connected servers keep their existing connections and tool registrations. + +### Custom Toolsets + +Each MCP server's tools are automatically grouped into a toolset named `mcp-{server_name}`. These toolsets are also injected into all `hermes-*` platform toolsets, so MCP tools are available in CLI, Telegram, Discord, and other platforms. + +### Thread Safety + +The MCP subsystem is fully thread-safe. A dedicated background event loop runs in a daemon thread, and all server state is protected by a lock. This works correctly even with Python 3.13+ free-threading builds. diff --git a/docs/tools.md b/docs/tools.md index d0cad2cd..0b96550b 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -55,6 +55,7 @@ async def web_search(query: str) -> dict: | **Clarify** | `clarify_tool.py` | `clarify` (interactive multiple-choice / open-ended questions, CLI-only) | | **Code Execution** | `code_execution_tool.py` | `execute_code` (run Python scripts that call tools via RPC sandbox) | | **Delegation** | `delegate_tool.py` | `delegate_task` (spawn subagents with isolated context, single + parallel batch) | +| **MCP (External)** | `tools/mcp_tool.py` | Auto-discovered from configured MCP servers | ## Tool Registration @@ -414,3 +415,20 @@ The Skills Hub enables searching, installing, and managing skills from online re **CLI:** `hermes skills search|install|inspect|list|audit|uninstall|publish|snapshot|tap` **Slash:** `/skills search|install|inspect|list|audit|uninstall|publish|snapshot|tap` + +## MCP Tools + +MCP (Model Context Protocol) tools are **dynamically registered** from external MCP servers configured in `cli-config.yaml`. Unlike built-in tools which are defined in Python source files, MCP tools are discovered at startup by connecting to each configured server and querying its available tools. + +Each MCP tool is automatically wrapped with an OpenAI-compatible schema and registered in the tool registry under the `mcp` toolset. Tool names are prefixed with the server name (e.g., `time__get_current_time`) to avoid collisions. + +**Key characteristics:** +- Tools are discovered and registered at agent startup — no code changes needed +- Supports both stdio (subprocess) and HTTP (streamable HTTP) transports +- Auto-reconnects on connection failures with exponential backoff +- Environment variables passed to stdio servers are filtered for security +- Each server can have independent timeout settings + +**Configuration:** Add servers to `mcp_servers` in `cli-config.yaml`. See [docs/mcp.md](mcp.md) for full documentation. + +**Installation:** MCP support requires the optional `mcp` extra: `pip install hermes-agent[mcp]` diff --git a/skills/mcp/DESCRIPTION.md b/skills/mcp/DESCRIPTION.md index 7c668b92..627c20ea 100644 --- a/skills/mcp/DESCRIPTION.md +++ b/skills/mcp/DESCRIPTION.md @@ -1,3 +1,3 @@ --- -description: Skills for working with MCP (Model Context Protocol) servers, tools, and integrations. +description: Skills for working with MCP (Model Context Protocol) servers, tools, and integrations. Includes the built-in native MCP client (configure servers in config.yaml for automatic tool discovery) and the mcporter CLI bridge for ad-hoc server interaction. --- diff --git a/skills/mcp/native-mcp/SKILL.md b/skills/mcp/native-mcp/SKILL.md new file mode 100644 index 00000000..4362c6cf --- /dev/null +++ b/skills/mcp/native-mcp/SKILL.md @@ -0,0 +1,330 @@ +--- +name: native-mcp +description: Built-in MCP (Model Context Protocol) client that connects to external MCP servers, discovers their tools, and registers them as native Hermes Agent tools. Supports stdio and HTTP transports with automatic reconnection, security filtering, and zero-config tool injection. +version: 1.0.0 +author: Hermes Agent +license: MIT +metadata: + hermes: + tags: [MCP, Tools, Integrations] + related_skills: [mcporter] +--- + +# Native MCP Client + +Hermes Agent has a built-in MCP client that connects to MCP servers at startup, discovers their tools, and makes them available as first-class tools the agent can call directly. No bridge CLI needed -- tools from MCP servers appear alongside built-in tools like `terminal`, `read_file`, etc. + +## When to Use + +Use this whenever you want to: +- Connect to MCP servers and use their tools from within Hermes Agent +- Add external capabilities (filesystem access, GitHub, databases, APIs) via MCP +- Run local stdio-based MCP servers (npx, uvx, or any command) +- Connect to remote HTTP/StreamableHTTP MCP servers +- Have MCP tools auto-discovered and available in every conversation + +For ad-hoc, one-off MCP tool calls from the terminal without configuring anything, see the `mcporter` skill instead. + +## Prerequisites + +- **mcp Python package** -- optional dependency; install with `pip install mcp`. If not installed, MCP support is silently disabled. +- **Node.js** -- required for `npx`-based MCP servers (most community servers) +- **uv** -- required for `uvx`-based MCP servers (Python-based servers) + +Install the MCP SDK: + +```bash +pip install mcp +# or, if using uv: +uv pip install mcp +``` + +## Quick Start + +Add MCP servers to `~/.hermes/config.yaml` under the `mcp_servers` key: + +```yaml +mcp_servers: + time: + command: "uvx" + args: ["mcp-server-time"] +``` + +Restart Hermes Agent. On startup it will: +1. Connect to the server +2. Discover available tools +3. Register them with the prefix `mcp_time_*` +4. Inject them into all platform toolsets + +You can then use the tools naturally -- just ask the agent to get the current time. + +## Configuration Reference + +Each entry under `mcp_servers` is a server name mapped to its config. There are two transport types: **stdio** (command-based) and **HTTP** (url-based). + +### Stdio Transport (command + args) + +```yaml +mcp_servers: + server_name: + command: "npx" # (required) executable to run + args: ["-y", "pkg-name"] # (optional) command arguments, default: [] + env: # (optional) environment variables for the subprocess + SOME_API_KEY: "value" + timeout: 120 # (optional) per-tool-call timeout in seconds, default: 120 + connect_timeout: 60 # (optional) initial connection timeout in seconds, default: 60 +``` + +### HTTP Transport (url) + +```yaml +mcp_servers: + server_name: + url: "https://my-server.example.com/mcp" # (required) server URL + headers: # (optional) HTTP headers + Authorization: "Bearer sk-..." + timeout: 180 # (optional) per-tool-call timeout in seconds, default: 120 + connect_timeout: 60 # (optional) initial connection timeout in seconds, default: 60 +``` + +### All Config Options + +| Option | Type | Default | Description | +|-------------------|--------|---------|---------------------------------------------------| +| `command` | string | -- | Executable to run (stdio transport, required) | +| `args` | list | `[]` | Arguments passed to the command | +| `env` | dict | `{}` | Extra environment variables for the subprocess | +| `url` | string | -- | Server URL (HTTP transport, required) | +| `headers` | dict | `{}` | HTTP headers sent with every request | +| `timeout` | int | `120` | Per-tool-call timeout in seconds | +| `connect_timeout` | int | `60` | Timeout for initial connection and discovery | + +Note: A server config must have either `command` (stdio) or `url` (HTTP), not both. + +## How It Works + +### Startup Discovery + +When Hermes Agent starts, `discover_mcp_tools()` is called during tool initialization: + +1. Reads `mcp_servers` from `~/.hermes/config.yaml` +2. For each server, spawns a connection in a dedicated background event loop +3. Initializes the MCP session and calls `list_tools()` to discover available tools +4. Registers each tool in the Hermes tool registry + +### Tool Naming Convention + +MCP tools are registered with the naming pattern: + +``` +mcp_{server_name}_{tool_name} +``` + +Hyphens and dots in names are replaced with underscores for LLM API compatibility. + +Examples: +- Server `filesystem`, tool `read_file` → `mcp_filesystem_read_file` +- Server `github`, tool `list-issues` → `mcp_github_list_issues` +- Server `my-api`, tool `fetch.data` → `mcp_my_api_fetch_data` + +### Auto-Injection + +After discovery, MCP tools are automatically injected into all `hermes-*` platform toolsets (CLI, Discord, Telegram, etc.). This means MCP tools are available in every conversation without any additional configuration. + +### Connection Lifecycle + +- Each server runs as a long-lived asyncio Task in a background daemon thread +- Connections persist for the lifetime of the agent process +- If a connection drops, automatic reconnection with exponential backoff kicks in (up to 5 retries, max 60s backoff) +- On agent shutdown, all connections are gracefully closed + +### Idempotency + +`discover_mcp_tools()` is idempotent -- calling it multiple times only connects to servers that aren't already connected. Failed servers are retried on subsequent calls. + +## Transport Types + +### Stdio Transport + +The most common transport. Hermes launches the MCP server as a subprocess and communicates over stdin/stdout. + +```yaml +mcp_servers: + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/home/user/projects"] +``` + +The subprocess inherits a **filtered** environment (see Security section below) plus any variables you specify in `env`. + +### HTTP / StreamableHTTP Transport + +For remote or shared MCP servers. Requires the `mcp` package to include HTTP client support (`mcp.client.streamable_http`). + +```yaml +mcp_servers: + remote_api: + url: "https://mcp.example.com/mcp" + headers: + Authorization: "Bearer sk-..." +``` + +If HTTP support is not available in your installed `mcp` version, the server will fail with an ImportError and other servers will continue normally. + +## Security + +### Environment Variable Filtering + +For stdio servers, Hermes does NOT pass your full shell environment to MCP subprocesses. Only safe baseline variables are inherited: + +- `PATH`, `HOME`, `USER`, `LANG`, `LC_ALL`, `TERM`, `SHELL`, `TMPDIR` +- Any `XDG_*` variables + +All other environment variables (API keys, tokens, secrets) are excluded unless you explicitly add them via the `env` config key. This prevents accidental credential leakage to untrusted MCP servers. + +```yaml +mcp_servers: + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + # Only this token is passed to the subprocess + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_..." +``` + +### Credential Stripping in Error Messages + +If an MCP tool call fails, any credential-like patterns in the error message are automatically redacted before being shown to the LLM. This covers: + +- GitHub PATs (`ghp_...`) +- OpenAI-style keys (`sk-...`) +- Bearer tokens +- Generic `token=`, `key=`, `API_KEY=`, `password=`, `secret=` patterns + +## Troubleshooting + +### "MCP SDK not available -- skipping MCP tool discovery" + +The `mcp` Python package is not installed. Install it: + +```bash +pip install mcp +``` + +### "No MCP servers configured" + +No `mcp_servers` key in `~/.hermes/config.yaml`, or it's empty. Add at least one server. + +### "Failed to connect to MCP server 'X'" + +Common causes: +- **Command not found**: The `command` binary isn't on PATH. Ensure `npx`, `uvx`, or the relevant command is installed. +- **Package not found**: For npx servers, the npm package may not exist or may need `-y` in args to auto-install. +- **Timeout**: The server took too long to start. Increase `connect_timeout`. +- **Port conflict**: For HTTP servers, the URL may be unreachable. + +### "MCP server 'X' requires HTTP transport but mcp.client.streamable_http is not available" + +Your `mcp` package version doesn't include HTTP client support. Upgrade: + +```bash +pip install --upgrade mcp +``` + +### Tools not appearing + +- Check that the server is listed under `mcp_servers` (not `mcp` or `servers`) +- Ensure the YAML indentation is correct +- Look at Hermes Agent startup logs for connection messages +- Tool names are prefixed with `mcp_{server}_{tool}` -- look for that pattern + +### Connection keeps dropping + +The client retries up to 5 times with exponential backoff (1s, 2s, 4s, 8s, 16s, capped at 60s). If the server is fundamentally unreachable, it gives up after 5 attempts. Check the server process and network connectivity. + +## Examples + +### Time Server (uvx) + +```yaml +mcp_servers: + time: + command: "uvx" + args: ["mcp-server-time"] +``` + +Registers tools like `mcp_time_get_current_time`. + +### Filesystem Server (npx) + +```yaml +mcp_servers: + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/home/user/documents"] + timeout: 30 +``` + +Registers tools like `mcp_filesystem_read_file`, `mcp_filesystem_write_file`, `mcp_filesystem_list_directory`. + +### GitHub Server with Authentication + +```yaml +mcp_servers: + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxxxxxxxxxx" + timeout: 60 +``` + +Registers tools like `mcp_github_list_issues`, `mcp_github_create_pull_request`, etc. + +### Remote HTTP Server + +```yaml +mcp_servers: + company_api: + url: "https://mcp.mycompany.com/v1/mcp" + headers: + Authorization: "Bearer sk-xxxxxxxxxxxxxxxxxxxx" + X-Team-Id: "engineering" + timeout: 180 + connect_timeout: 30 +``` + +### Multiple Servers + +```yaml +mcp_servers: + time: + command: "uvx" + args: ["mcp-server-time"] + + filesystem: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + + github: + command: "npx" + args: ["-y", "@modelcontextprotocol/server-github"] + env: + GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_xxxxxxxxxxxxxxxxxxxx" + + company_api: + url: "https://mcp.internal.company.com/mcp" + headers: + Authorization: "Bearer sk-xxxxxxxxxxxxxxxxxxxx" + timeout: 300 +``` + +All tools from all servers are registered and available simultaneously. Each server's tools are prefixed with its name to avoid collisions. + +## Notes + +- MCP tools are called synchronously from the agent's perspective but run asynchronously on a dedicated background event loop +- Tool results are returned as JSON with either `{"result": "..."}` or `{"error": "..."}` +- The native MCP client is independent of `mcporter` -- you can use both simultaneously +- Server connections are persistent and shared across all conversations in the same agent process +- Adding or removing servers requires restarting the agent (no hot-reload currently) From 60effcfc4427c5dee2ce95c1751454f2e5fb67a3 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 19:02:28 -0800 Subject: [PATCH 09/12] fix(mcp): parallel discovery, user-visible logging, config validation - Discovery is now parallel (asyncio.gather) instead of sequential, fixing the 60s shared timeout issue with multiple servers - Startup messages use print() so users see connection status even with default log levels (the 'tools' logger is set to ERROR) - Summary line shows total tools and failed servers count - Validate conflicting config: warn if both 'url' and 'command' are present (HTTP takes precedence) - Update TODO.md: mark MCP as implemented, list remaining work - Add test for conflicting config detection (51 tests total) All 1163 tests pass. --- TODO.md | 38 +++++++++------------ tests/tools/test_mcp_tool.py | 9 +++++ tools/mcp_tool.py | 65 +++++++++++++++++++++++++++++------- 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/TODO.md b/TODO.md index 01153c68..f6ec5e55 100644 --- a/TODO.md +++ b/TODO.md @@ -63,33 +63,27 @@ Full Python plugin interface that goes beyond the current hook system. - `hermes plugin list|install|uninstall|create` CLI commands - Plugin discovery and validation on startup -### Phase 3: MCP support (industry standard) -- MCP client that can connect to external MCP servers (stdio, SSE, HTTP) -- This is the big one -- Codex, Cline, and OpenCode all support MCP -- Allows Hermes to use any MCP-compatible tool server (hundreds exist) -- Config: `mcp_servers` list in config.yaml with connection details -- Each MCP server's tools get registered as a new toolset +### Phase 3: MCP support (industry standard) ✅ DONE +- ✅ MCP client that connects to external MCP servers (stdio + HTTP/StreamableHTTP) +- ✅ Config: `mcp_servers` in config.yaml with connection details +- ✅ Each MCP server's tools auto-registered as a dynamic toolset +- Future: Resources, Prompts, Progress notifications, `hermes mcp` CLI command --- -## 6. MCP (Model Context Protocol) Support 🔗 +## 6. MCP (Model Context Protocol) Support 🔗 ✅ DONE -**Status:** Not started -**Priority:** High -- this is becoming an industry standard +**Status:** Implemented (PR #301) +**Priority:** Complete -MCP is the protocol that Codex, Cline, and OpenCode all support for connecting to external tool servers. Supporting MCP would instantly give Hermes access to hundreds of community tool servers. +Native MCP client support with stdio and HTTP/StreamableHTTP transports, auto-discovery, reconnection with exponential backoff, env var filtering, and credential stripping. See `docs/mcp.md` for full documentation. -**What other agents do:** -- **Codex**: Full MCP integration with skill dependencies -- **Cline**: `use_mcp_tool` / `access_mcp_resource` / `load_mcp_documentation` tools -- **OpenCode**: MCP client support (stdio, SSE, StreamableHTTP transports), OAuth auth - -**Our approach:** -- Implement an MCP client that can connect to external MCP servers -- Config: list of MCP servers in `~/.hermes/config.yaml` with transport type and connection details -- Each MCP server's tools auto-registered as a dynamic toolset -- Start with stdio transport (most common), then add SSE and HTTP -- Could also be part of the Plugin system (#5, Phase 3) since MCP is essentially a plugin protocol +**Still TODO:** +- `hermes mcp` CLI subcommand (list/test/status) +- `hermes tools` UI integration for MCP toolsets +- MCP Resources and Prompts support +- OAuth authentication for remote servers +- Progress notifications for long-running tools --- @@ -121,7 +115,7 @@ Automatic filesystem snapshots after each agent loop iteration so the user can r ### Tier 1: Next Up -1. MCP Support -- #6 +1. ~~MCP Support -- #6~~ ✅ Done (PR #301) ### Tier 2: Quality of Life diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 4b7e2c72..74c380d6 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -856,6 +856,15 @@ class TestHTTPConfig: server._config = {"command": "npx", "args": []} assert server._is_http() is False + def test_conflicting_url_and_command_warns(self): + """Config with both url and command logs a warning and uses HTTP.""" + from tools.mcp_tool import MCPServerTask + server = MCPServerTask("conflict") + config = {"url": "https://example.com/mcp", "command": "npx", "args": []} + # url takes precedence + server._config = config + assert server._is_http() is True + def test_http_unavailable_raises(self): from tools.mcp_tool import MCPServerTask diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 1419327c..7c87e0ff 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -88,8 +88,7 @@ except ImportError: # --------------------------------------------------------------------------- _DEFAULT_TOOL_TIMEOUT = 120 # seconds for tool calls -_DEFAULT_DISCOVERY_TIMEOUT = 60 # seconds for server discovery -_DEFAULT_CONNECT_TIMEOUT = 60 # seconds for initial connection +_DEFAULT_CONNECT_TIMEOUT = 60 # seconds for initial connection per server _MAX_RECONNECT_RETRIES = 5 _MAX_BACKOFF_SECONDS = 60 @@ -250,6 +249,15 @@ class MCPServerTask: """ self._config = config self.tool_timeout = config.get("timeout", _DEFAULT_TOOL_TIMEOUT) + + # Validate: warn if both url and command are present + if "url" in config and "command" in config: + logger.warning( + "MCP server '%s' has both 'url' and 'command' in config. " + "Using HTTP transport ('url'). Remove 'command' to silence " + "this warning.", + self.name, + ) retries = 0 backoff = 1.0 @@ -604,19 +612,43 @@ def discover_mcp_tools() -> List[str]: _ensure_mcp_loop() all_tools: List[str] = [] + failed_count = 0 + + async def _discover_one(name: str, cfg: dict) -> List[str]: + """Connect to a single server and return its registered tool names.""" + transport_desc = cfg.get("url", f'{cfg.get("command", "?")} {" ".join(cfg.get("args", [])[:2])}') + try: + registered = await _discover_and_register_server(name, cfg) + transport_type = "HTTP" if "url" in cfg else "stdio" + print(f" MCP: '{name}' ({transport_type}) — {len(registered)} tool(s)") + return registered + except Exception as exc: + print(f" MCP: '{name}' — FAILED: {exc}") + logger.warning( + "Failed to connect to MCP server '%s': %s", + name, exc, + ) + return [] async def _discover_all(): - for name, cfg in new_servers.items(): - try: - registered = await _discover_and_register_server(name, cfg) - all_tools.extend(registered) - except Exception as exc: - logger.warning( - "Failed to connect to MCP server '%s': %s", - name, exc, - ) + nonlocal failed_count + # Connect to all servers in PARALLEL + results = await asyncio.gather( + *(_discover_one(name, cfg) for name, cfg in new_servers.items()), + return_exceptions=True, + ) + for result in results: + if isinstance(result, Exception): + failed_count += 1 + logger.warning("MCP discovery error: %s", result) + elif isinstance(result, list): + all_tools.extend(result) + else: + failed_count += 1 - _run_on_mcp_loop(_discover_all(), timeout=_DEFAULT_DISCOVERY_TIMEOUT) + # Per-server timeouts are handled inside _discover_and_register_server. + # The outer timeout is generous: 120s total for parallel discovery. + _run_on_mcp_loop(_discover_all(), timeout=120) if all_tools: # Dynamically inject into all hermes-* platform toolsets @@ -627,6 +659,15 @@ def discover_mcp_tools() -> List[str]: if tool_name not in ts["tools"]: ts["tools"].append(tool_name) + # Print summary + total_servers = len(new_servers) + ok_servers = total_servers - failed_count + if all_tools or failed_count: + summary = f" MCP: {len(all_tools)} tool(s) from {ok_servers} server(s)" + if failed_count: + summary += f" ({failed_count} failed)" + print(summary) + # Return ALL registered tools (existing + newly discovered) return _existing_tool_names() From 7df14227a957b5efb0096d0199304262776cfbc0 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 19:15:59 -0800 Subject: [PATCH 10/12] feat(mcp): banner integration, /reload-mcp command, resources & prompts Banner integration: - MCP Servers section in CLI startup banner between Tools and Skills - Shows each server with transport type, tool count, connection status - Failed servers shown in red; section hidden when no MCP configured - Summary line includes MCP server count - Removed raw print() calls from discovery (banner handles display) /reload-mcp command: - New slash command in both CLI and gateway - Disconnects all MCP servers, re-reads config.yaml, reconnects - Reports what changed (added/removed/reconnected servers) - Allows adding/removing MCP servers without restarting Resources & Prompts support: - 4 utility tools registered per server: list_resources, read_resource, list_prompts, get_prompt - Exposes MCP Resources (data sources) and Prompts (templates) as tools - Proper parameter schemas (uri for read_resource, name for get_prompt) - Handles text and binary resource content - 23 new tests covering schemas, handlers, and registration Test coverage: 74 MCP tests total, 1186 tests pass overall. --- cli.py | 42 ++++ gateway/run.py | 51 ++++- hermes_cli/banner.py | 29 ++- tests/tools/test_mcp_tool.py | 426 +++++++++++++++++++++++++++++++++++ tools/mcp_tool.py | 326 ++++++++++++++++++++++++++- 5 files changed, 869 insertions(+), 5 deletions(-) diff --git a/cli.py b/cli.py index 591ab3e1..64e90c1f 100755 --- a/cli.py +++ b/cli.py @@ -690,6 +690,7 @@ COMMANDS = { "/cron": "Manage scheduled tasks (list, add, remove)", "/skills": "Search, install, inspect, or manage skills from online registries", "/platforms": "Show gateway/messaging platform status", + "/reload-mcp": "Reload MCP servers from config.yaml", "/quit": "Exit the CLI (also: /exit, /q)", } @@ -1770,6 +1771,8 @@ class HermesCLI: self._manual_compress() elif cmd_lower == "/usage": self._show_usage() + elif cmd_lower == "/reload-mcp": + self._reload_mcp() else: # Check for skill slash commands (/gif-search, /axolotl, etc.) base_cmd = cmd_lower.split()[0] @@ -1891,6 +1894,45 @@ class HermesCLI: for quiet_logger in ('tools', 'minisweagent', 'run_agent', 'trajectory_compressor', 'cron', 'hermes_cli'): logging.getLogger(quiet_logger).setLevel(logging.ERROR) + def _reload_mcp(self): + """Reload MCP servers: disconnect all, re-read config.yaml, reconnect.""" + try: + from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _load_mcp_config, _servers, _lock + + # Capture old server names + with _lock: + old_servers = set(_servers.keys()) + + print("🔄 Reloading MCP servers...") + + # Shutdown existing connections + shutdown_mcp_servers() + + # Reconnect (reads config.yaml fresh) + new_tools = discover_mcp_tools() + + # Compute what changed + with _lock: + connected_servers = set(_servers.keys()) + + added = connected_servers - old_servers + removed = old_servers - connected_servers + reconnected = connected_servers & old_servers + + if reconnected: + print(f" ♻️ Reconnected: {', '.join(sorted(reconnected))}") + if added: + print(f" ➕ Added: {', '.join(sorted(added))}") + if removed: + print(f" ➖ Removed: {', '.join(sorted(removed))}") + if not connected_servers: + print(" (._.) No MCP servers connected.") + else: + print(f" 🔧 {len(new_tools)} tool(s) available from {len(connected_servers)} server(s)") + + except Exception as e: + print(f" ❌ MCP reload failed: {e}") + def _clarify_callback(self, question, choices): """ Platform callback for the clarify tool. Called from the agent thread. diff --git a/gateway/run.py b/gateway/run.py index 61027c4f..83b97372 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -645,7 +645,7 @@ class GatewayRunner: # Emit command:* hook for any recognized slash command _known_commands = {"new", "reset", "help", "status", "stop", "model", "personality", "retry", "undo", "sethome", "set-home", - "compress", "usage"} + "compress", "usage", "reload-mcp"} if command and command in _known_commands: await self.hooks.emit(f"command:{command}", { "platform": source.platform.value if source.platform else "", @@ -686,6 +686,9 @@ class GatewayRunner: if command == "usage": return await self._handle_usage_command(event) + + if command == "reload-mcp": + return await self._handle_reload_mcp_command(event) # Skill slash commands: /skill-name loads the skill and sends to agent if command: @@ -1086,6 +1089,7 @@ class GatewayRunner: "`/sethome` — Set this chat as the home channel", "`/compress` — Compress conversation context", "`/usage` — Show token usage for this session", + "`/reload-mcp` — Reload MCP servers from config", "`/help` — Show this message", ] try: @@ -1379,6 +1383,51 @@ class GatewayRunner: ) return "No usage data available for this session." + async def _handle_reload_mcp_command(self, event: MessageEvent) -> str: + """Handle /reload-mcp command -- disconnect and reconnect all MCP servers.""" + loop = asyncio.get_event_loop() + try: + from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _load_mcp_config, _servers, _lock + + # Capture old server names before shutdown + with _lock: + old_servers = set(_servers.keys()) + + # Read new config before shutting down, so we know what will be added/removed + new_config = _load_mcp_config() + new_server_names = set(new_config.keys()) + + # Shutdown existing connections + await loop.run_in_executor(None, shutdown_mcp_servers) + + # Reconnect by discovering tools (reads config.yaml fresh) + new_tools = await loop.run_in_executor(None, discover_mcp_tools) + + # Compute what changed + with _lock: + connected_servers = set(_servers.keys()) + + added = connected_servers - old_servers + removed = old_servers - connected_servers + reconnected = connected_servers & old_servers + + lines = ["🔄 **MCP Servers Reloaded**\n"] + if reconnected: + lines.append(f"♻️ Reconnected: {', '.join(sorted(reconnected))}") + if added: + lines.append(f"➕ Added: {', '.join(sorted(added))}") + if removed: + lines.append(f"➖ Removed: {', '.join(sorted(removed))}") + if not connected_servers: + lines.append("No MCP servers connected.") + else: + lines.append(f"\n🔧 {len(new_tools)} tool(s) available from {len(connected_servers)} server(s)") + return "\n".join(lines) + + except Exception as e: + logger.warning("MCP reload failed: %s", e) + return f"❌ MCP reload failed: {e}" + def _set_session_env(self, context: SessionContext) -> None: """Set environment variables for the current session.""" os.environ["HERMES_SESSION_PLATFORM"] = context.source.platform.value diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index 974dfaa1..be1b3a95 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -196,6 +196,28 @@ def build_welcome_banner(console: Console, model: str, cwd: str, if remaining_toolsets > 0: right_lines.append(f"[dim #B8860B](and {remaining_toolsets} more toolsets...)[/]") + # MCP Servers section (only if configured) + try: + from tools.mcp_tool import get_mcp_status + mcp_status = get_mcp_status() + except Exception: + mcp_status = [] + + if mcp_status: + right_lines.append("") + right_lines.append("[bold #FFBF00]MCP Servers[/]") + for srv in mcp_status: + if srv["connected"]: + right_lines.append( + f"[dim #B8860B]{srv['name']}[/] [#FFF8DC]({srv['transport']})[/] " + f"[dim #B8860B]—[/] [#FFF8DC]{srv['tools']} tool(s)[/]" + ) + else: + right_lines.append( + f"[red]{srv['name']}[/] [dim]({srv['transport']})[/] " + f"[red]— failed[/]" + ) + right_lines.append("") right_lines.append("[bold #FFBF00]Available Skills[/]") skills_by_category = get_available_skills() @@ -216,7 +238,12 @@ def build_welcome_banner(console: Console, model: str, cwd: str, right_lines.append("[dim #B8860B]No skills installed[/]") right_lines.append("") - right_lines.append(f"[dim #B8860B]{len(tools)} tools · {total_skills} skills · /help for commands[/]") + mcp_connected = sum(1 for s in mcp_status if s["connected"]) if mcp_status else 0 + summary_parts = [f"{len(tools)} tools", f"{total_skills} skills"] + if mcp_connected: + summary_parts.append(f"{mcp_connected} MCP servers") + summary_parts.append("/help for commands") + right_lines.append(f"[dim #B8860B]{' · '.join(summary_parts)}[/]") right_content = "\n".join(right_lines) layout_table.add_row(left_content, right_content) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 74c380d6..7da383a9 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -1063,3 +1063,429 @@ class TestConfigurableTimeouts: call_kwargs[1].get("timeout") == 180 finally: _servers.pop("test_srv", None) + + +# --------------------------------------------------------------------------- +# Utility tool schemas (Resources & Prompts) +# --------------------------------------------------------------------------- + +class TestUtilitySchemas: + """Tests for _build_utility_schemas() and the schema format of utility tools.""" + + def test_builds_four_utility_schemas(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("myserver") + assert len(schemas) == 4 + names = [s["schema"]["name"] for s in schemas] + assert "mcp_myserver_list_resources" in names + assert "mcp_myserver_read_resource" in names + assert "mcp_myserver_list_prompts" in names + assert "mcp_myserver_get_prompt" in names + + def test_hyphens_sanitized_in_utility_names(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("my-server") + names = [s["schema"]["name"] for s in schemas] + for name in names: + assert "-" not in name + assert "mcp_my_server_list_resources" in names + + def test_list_resources_schema_no_required_params(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("srv") + lr = next(s for s in schemas if s["handler_key"] == "list_resources") + params = lr["schema"]["parameters"] + assert params["type"] == "object" + assert params["properties"] == {} + assert "required" not in params + + def test_read_resource_schema_requires_uri(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("srv") + rr = next(s for s in schemas if s["handler_key"] == "read_resource") + params = rr["schema"]["parameters"] + assert "uri" in params["properties"] + assert params["properties"]["uri"]["type"] == "string" + assert params["required"] == ["uri"] + + def test_list_prompts_schema_no_required_params(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("srv") + lp = next(s for s in schemas if s["handler_key"] == "list_prompts") + params = lp["schema"]["parameters"] + assert params["type"] == "object" + assert params["properties"] == {} + assert "required" not in params + + def test_get_prompt_schema_requires_name(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("srv") + gp = next(s for s in schemas if s["handler_key"] == "get_prompt") + params = gp["schema"]["parameters"] + assert "name" in params["properties"] + assert params["properties"]["name"]["type"] == "string" + assert "arguments" in params["properties"] + assert params["properties"]["arguments"]["type"] == "object" + assert params["required"] == ["name"] + + def test_schemas_have_descriptions(self): + from tools.mcp_tool import _build_utility_schemas + + schemas = _build_utility_schemas("test_srv") + for entry in schemas: + desc = entry["schema"]["description"] + assert desc and len(desc) > 0 + assert "test_srv" in desc + + +# --------------------------------------------------------------------------- +# Utility tool handlers (Resources & Prompts) +# --------------------------------------------------------------------------- + +class TestUtilityHandlers: + """Tests for the MCP Resources & Prompts handler functions.""" + + def _patch_mcp_loop(self): + """Return a patch for _run_on_mcp_loop that runs the coroutine directly.""" + def fake_run(coro, timeout=30): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=fake_run) + + # -- list_resources -- + + def test_list_resources_success(self): + from tools.mcp_tool import _make_list_resources_handler, _servers + + mock_resource = SimpleNamespace( + uri="file:///tmp/test.txt", name="test.txt", + description="A test file", mimeType="text/plain", + ) + mock_session = MagicMock() + mock_session.list_resources = AsyncMock( + return_value=SimpleNamespace(resources=[mock_resource]) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_list_resources_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert "resources" in result + assert len(result["resources"]) == 1 + assert result["resources"][0]["uri"] == "file:///tmp/test.txt" + assert result["resources"][0]["name"] == "test.txt" + finally: + _servers.pop("srv", None) + + def test_list_resources_empty(self): + from tools.mcp_tool import _make_list_resources_handler, _servers + + mock_session = MagicMock() + mock_session.list_resources = AsyncMock( + return_value=SimpleNamespace(resources=[]) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_list_resources_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert result["resources"] == [] + finally: + _servers.pop("srv", None) + + def test_list_resources_disconnected(self): + from tools.mcp_tool import _make_list_resources_handler, _servers + _servers.pop("ghost", None) + handler = _make_list_resources_handler("ghost", 120) + result = json.loads(handler({})) + assert "error" in result + assert "not connected" in result["error"] + + # -- read_resource -- + + def test_read_resource_success(self): + from tools.mcp_tool import _make_read_resource_handler, _servers + + content_block = SimpleNamespace(text="Hello from resource") + mock_session = MagicMock() + mock_session.read_resource = AsyncMock( + return_value=SimpleNamespace(contents=[content_block]) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_read_resource_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({"uri": "file:///tmp/test.txt"})) + assert result["result"] == "Hello from resource" + mock_session.read_resource.assert_called_once_with("file:///tmp/test.txt") + finally: + _servers.pop("srv", None) + + def test_read_resource_missing_uri(self): + from tools.mcp_tool import _make_read_resource_handler, _servers + + server = _make_mock_server("srv", session=MagicMock()) + _servers["srv"] = server + + try: + handler = _make_read_resource_handler("srv", 120) + result = json.loads(handler({})) + assert "error" in result + assert "uri" in result["error"].lower() + finally: + _servers.pop("srv", None) + + def test_read_resource_disconnected(self): + from tools.mcp_tool import _make_read_resource_handler, _servers + _servers.pop("ghost", None) + handler = _make_read_resource_handler("ghost", 120) + result = json.loads(handler({"uri": "test://x"})) + assert "error" in result + assert "not connected" in result["error"] + + # -- list_prompts -- + + def test_list_prompts_success(self): + from tools.mcp_tool import _make_list_prompts_handler, _servers + + mock_prompt = SimpleNamespace( + name="summarize", description="Summarize text", + arguments=[ + SimpleNamespace(name="text", description="Text to summarize", required=True), + ], + ) + mock_session = MagicMock() + mock_session.list_prompts = AsyncMock( + return_value=SimpleNamespace(prompts=[mock_prompt]) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_list_prompts_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert "prompts" in result + assert len(result["prompts"]) == 1 + assert result["prompts"][0]["name"] == "summarize" + assert result["prompts"][0]["arguments"][0]["name"] == "text" + finally: + _servers.pop("srv", None) + + def test_list_prompts_empty(self): + from tools.mcp_tool import _make_list_prompts_handler, _servers + + mock_session = MagicMock() + mock_session.list_prompts = AsyncMock( + return_value=SimpleNamespace(prompts=[]) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_list_prompts_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({})) + assert result["prompts"] == [] + finally: + _servers.pop("srv", None) + + def test_list_prompts_disconnected(self): + from tools.mcp_tool import _make_list_prompts_handler, _servers + _servers.pop("ghost", None) + handler = _make_list_prompts_handler("ghost", 120) + result = json.loads(handler({})) + assert "error" in result + assert "not connected" in result["error"] + + # -- get_prompt -- + + def test_get_prompt_success(self): + from tools.mcp_tool import _make_get_prompt_handler, _servers + + mock_msg = SimpleNamespace( + role="assistant", + content=SimpleNamespace(text="Here is a summary of your text."), + ) + mock_session = MagicMock() + mock_session.get_prompt = AsyncMock( + return_value=SimpleNamespace(messages=[mock_msg], description=None) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_get_prompt_handler("srv", 120) + with self._patch_mcp_loop(): + result = json.loads(handler({"name": "summarize", "arguments": {"text": "hello"}})) + assert "messages" in result + assert len(result["messages"]) == 1 + assert result["messages"][0]["role"] == "assistant" + assert "summary" in result["messages"][0]["content"].lower() + mock_session.get_prompt.assert_called_once_with( + "summarize", arguments={"text": "hello"} + ) + finally: + _servers.pop("srv", None) + + def test_get_prompt_missing_name(self): + from tools.mcp_tool import _make_get_prompt_handler, _servers + + server = _make_mock_server("srv", session=MagicMock()) + _servers["srv"] = server + + try: + handler = _make_get_prompt_handler("srv", 120) + result = json.loads(handler({})) + assert "error" in result + assert "name" in result["error"].lower() + finally: + _servers.pop("srv", None) + + def test_get_prompt_disconnected(self): + from tools.mcp_tool import _make_get_prompt_handler, _servers + _servers.pop("ghost", None) + handler = _make_get_prompt_handler("ghost", 120) + result = json.loads(handler({"name": "test"})) + assert "error" in result + assert "not connected" in result["error"] + + def test_get_prompt_default_arguments(self): + from tools.mcp_tool import _make_get_prompt_handler, _servers + + mock_session = MagicMock() + mock_session.get_prompt = AsyncMock( + return_value=SimpleNamespace(messages=[], description=None) + ) + server = _make_mock_server("srv", session=mock_session) + _servers["srv"] = server + + try: + handler = _make_get_prompt_handler("srv", 120) + with self._patch_mcp_loop(): + handler({"name": "test_prompt"}) + # arguments defaults to {} when not provided + mock_session.get_prompt.assert_called_once_with( + "test_prompt", arguments={} + ) + finally: + _servers.pop("srv", None) + + +# --------------------------------------------------------------------------- +# Utility tools registration in _discover_and_register_server +# --------------------------------------------------------------------------- + +class TestUtilityToolRegistration: + """Verify utility tools are registered alongside regular MCP tools.""" + + def test_utility_tools_registered(self): + """_discover_and_register_server registers all 4 utility tools.""" + from tools.registry import ToolRegistry + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask + + mock_registry = ToolRegistry() + mock_tools = [_make_mcp_tool("read_file", "Read a file")] + mock_session = MagicMock() + + async def fake_connect(name, config): + server = MCPServerTask(name) + server.session = mock_session + server._tools = mock_tools + return server + + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.registry.registry", mock_registry): + registered = asyncio.run( + _discover_and_register_server("fs", {"command": "npx", "args": []}) + ) + + # Regular tool + 4 utility tools + assert "mcp_fs_read_file" in registered + assert "mcp_fs_list_resources" in registered + assert "mcp_fs_read_resource" in registered + assert "mcp_fs_list_prompts" in registered + assert "mcp_fs_get_prompt" in registered + assert len(registered) == 5 + + # All in the registry + all_names = mock_registry.get_all_tool_names() + for name in registered: + assert name in all_names + + _servers.pop("fs", None) + + def test_utility_tools_in_same_toolset(self): + """Utility tools belong to the same mcp-{server} toolset.""" + from tools.registry import ToolRegistry + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask + + mock_registry = ToolRegistry() + mock_session = MagicMock() + + async def fake_connect(name, config): + server = MCPServerTask(name) + server.session = mock_session + server._tools = [] + return server + + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.registry.registry", mock_registry): + asyncio.run( + _discover_and_register_server("myserv", {"command": "test"}) + ) + + # Check that utility tools are in the right toolset + for tool_name in ["mcp_myserv_list_resources", "mcp_myserv_read_resource", + "mcp_myserv_list_prompts", "mcp_myserv_get_prompt"]: + entry = mock_registry._tools.get(tool_name) + assert entry is not None, f"{tool_name} not found in registry" + assert entry.toolset == "mcp-myserv" + + _servers.pop("myserv", None) + + def test_utility_tools_have_check_fn(self): + """Utility tools have a working check_fn.""" + from tools.registry import ToolRegistry + from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask + + mock_registry = ToolRegistry() + mock_session = MagicMock() + + async def fake_connect(name, config): + server = MCPServerTask(name) + server.session = mock_session + server._tools = [] + return server + + with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.registry.registry", mock_registry): + asyncio.run( + _discover_and_register_server("chk", {"command": "test"}) + ) + + entry = mock_registry._tools.get("mcp_chk_list_resources") + assert entry is not None + # Server is connected, check_fn should return True + assert entry.check_fn() is True + + # Disconnect the server + _servers["chk"].session = None + assert entry.check_fn() is False + + _servers.pop("chk", None) diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 7c87e0ff..55e1f7d5 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -475,6 +475,190 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): return _handler +def _make_list_resources_handler(server_name: str, tool_timeout: float): + """Return a sync handler that lists resources from an MCP server.""" + + def _handler(args: dict, **kwargs) -> str: + with _lock: + server = _servers.get(server_name) + if not server or not server.session: + return json.dumps({ + "error": f"MCP server '{server_name}' is not connected" + }) + + async def _call(): + result = await server.session.list_resources() + resources = [] + for r in (result.resources if hasattr(result, "resources") else []): + entry = {} + if hasattr(r, "uri"): + entry["uri"] = str(r.uri) + if hasattr(r, "name"): + entry["name"] = r.name + if hasattr(r, "description") and r.description: + entry["description"] = r.description + if hasattr(r, "mimeType") and r.mimeType: + entry["mimeType"] = r.mimeType + resources.append(entry) + return json.dumps({"resources": resources}) + + try: + return _run_on_mcp_loop(_call(), timeout=tool_timeout) + except Exception as exc: + logger.error( + "MCP %s/list_resources failed: %s", server_name, exc, + ) + return json.dumps({ + "error": _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {exc}" + ) + }) + + return _handler + + +def _make_read_resource_handler(server_name: str, tool_timeout: float): + """Return a sync handler that reads a resource by URI from an MCP server.""" + + def _handler(args: dict, **kwargs) -> str: + with _lock: + server = _servers.get(server_name) + if not server or not server.session: + return json.dumps({ + "error": f"MCP server '{server_name}' is not connected" + }) + + uri = args.get("uri") + if not uri: + return json.dumps({"error": "Missing required parameter 'uri'"}) + + async def _call(): + result = await server.session.read_resource(uri) + # read_resource returns ReadResourceResult with .contents list + parts: List[str] = [] + contents = result.contents if hasattr(result, "contents") else [] + for block in contents: + if hasattr(block, "text"): + parts.append(block.text) + elif hasattr(block, "blob"): + parts.append(f"[binary data, {len(block.blob)} bytes]") + return json.dumps({"result": "\n".join(parts) if parts else ""}) + + try: + return _run_on_mcp_loop(_call(), timeout=tool_timeout) + except Exception as exc: + logger.error( + "MCP %s/read_resource failed: %s", server_name, exc, + ) + return json.dumps({ + "error": _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {exc}" + ) + }) + + return _handler + + +def _make_list_prompts_handler(server_name: str, tool_timeout: float): + """Return a sync handler that lists prompts from an MCP server.""" + + def _handler(args: dict, **kwargs) -> str: + with _lock: + server = _servers.get(server_name) + if not server or not server.session: + return json.dumps({ + "error": f"MCP server '{server_name}' is not connected" + }) + + async def _call(): + result = await server.session.list_prompts() + prompts = [] + for p in (result.prompts if hasattr(result, "prompts") else []): + entry = {} + if hasattr(p, "name"): + entry["name"] = p.name + if hasattr(p, "description") and p.description: + entry["description"] = p.description + if hasattr(p, "arguments") and p.arguments: + entry["arguments"] = [ + { + "name": a.name, + **({"description": a.description} if hasattr(a, "description") and a.description else {}), + **({"required": a.required} if hasattr(a, "required") else {}), + } + for a in p.arguments + ] + prompts.append(entry) + return json.dumps({"prompts": prompts}) + + try: + return _run_on_mcp_loop(_call(), timeout=tool_timeout) + except Exception as exc: + logger.error( + "MCP %s/list_prompts failed: %s", server_name, exc, + ) + return json.dumps({ + "error": _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {exc}" + ) + }) + + return _handler + + +def _make_get_prompt_handler(server_name: str, tool_timeout: float): + """Return a sync handler that gets a prompt by name from an MCP server.""" + + def _handler(args: dict, **kwargs) -> str: + with _lock: + server = _servers.get(server_name) + if not server or not server.session: + return json.dumps({ + "error": f"MCP server '{server_name}' is not connected" + }) + + name = args.get("name") + if not name: + return json.dumps({"error": "Missing required parameter 'name'"}) + arguments = args.get("arguments", {}) + + async def _call(): + result = await server.session.get_prompt(name, arguments=arguments) + # GetPromptResult has .messages list + messages = [] + for msg in (result.messages if hasattr(result, "messages") else []): + entry = {} + if hasattr(msg, "role"): + entry["role"] = msg.role + if hasattr(msg, "content"): + content = msg.content + if hasattr(content, "text"): + entry["content"] = content.text + elif isinstance(content, str): + entry["content"] = content + else: + entry["content"] = str(content) + messages.append(entry) + resp = {"messages": messages} + if hasattr(result, "description") and result.description: + resp["description"] = result.description + return json.dumps(resp) + + try: + return _run_on_mcp_loop(_call(), timeout=tool_timeout) + except Exception as exc: + logger.error( + "MCP %s/get_prompt failed: %s", server_name, exc, + ) + return json.dumps({ + "error": _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {exc}" + ) + }) + + return _handler + + def _make_check_fn(server_name: str): """Return a check function that verifies the MCP connection is alive.""" @@ -515,6 +699,77 @@ def _convert_mcp_schema(server_name: str, mcp_tool) -> dict: } +def _build_utility_schemas(server_name: str) -> List[dict]: + """Build schemas for the MCP utility tools (resources & prompts). + + Returns a list of (schema, handler_factory_name) tuples encoded as dicts + with keys: schema, handler_key. + """ + safe_name = server_name.replace("-", "_").replace(".", "_") + return [ + { + "schema": { + "name": f"mcp_{safe_name}_list_resources", + "description": f"List available resources from MCP server '{server_name}'", + "parameters": { + "type": "object", + "properties": {}, + }, + }, + "handler_key": "list_resources", + }, + { + "schema": { + "name": f"mcp_{safe_name}_read_resource", + "description": f"Read a resource by URI from MCP server '{server_name}'", + "parameters": { + "type": "object", + "properties": { + "uri": { + "type": "string", + "description": "URI of the resource to read", + }, + }, + "required": ["uri"], + }, + }, + "handler_key": "read_resource", + }, + { + "schema": { + "name": f"mcp_{safe_name}_list_prompts", + "description": f"List available prompts from MCP server '{server_name}'", + "parameters": { + "type": "object", + "properties": {}, + }, + }, + "handler_key": "list_prompts", + }, + { + "schema": { + "name": f"mcp_{safe_name}_get_prompt", + "description": f"Get a prompt by name from MCP server '{server_name}'", + "parameters": { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "Name of the prompt to retrieve", + }, + "arguments": { + "type": "object", + "description": "Optional arguments to pass to the prompt", + }, + }, + "required": ["name"], + }, + }, + "handler_key": "get_prompt", + }, + ] + + def _existing_tool_names() -> List[str]: """Return tool names for all currently connected servers.""" names: List[str] = [] @@ -522,12 +777,18 @@ def _existing_tool_names() -> List[str]: for mcp_tool in server._tools: schema = _convert_mcp_schema(sname, mcp_tool) names.append(schema["name"]) + # Also include utility tool names + for entry in _build_utility_schemas(sname): + names.append(entry["schema"]["name"]) return names async def _discover_and_register_server(name: str, config: dict) -> List[str]: """Connect to a single MCP server, discover tools, and register them. + Also registers utility tools for MCP Resources and Prompts support + (list_resources, read_resource, list_prompts, get_prompt). + Returns list of registered tool names. """ from tools.registry import registry @@ -559,6 +820,30 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: ) registered_names.append(tool_name_prefixed) + # Register MCP Resources & Prompts utility tools + _handler_factories = { + "list_resources": _make_list_resources_handler, + "read_resource": _make_read_resource_handler, + "list_prompts": _make_list_prompts_handler, + "get_prompt": _make_get_prompt_handler, + } + check_fn = _make_check_fn(name) + for entry in _build_utility_schemas(name): + schema = entry["schema"] + handler_key = entry["handler_key"] + handler = _handler_factories[handler_key](name, server.tool_timeout) + + registry.register( + name=schema["name"], + toolset=toolset_name, + schema=schema, + handler=handler, + check_fn=check_fn, + is_async=False, + description=schema["description"], + ) + registered_names.append(schema["name"]) + # Create a custom toolset so these tools are discoverable if registered_names: create_custom_toolset( @@ -620,10 +905,8 @@ def discover_mcp_tools() -> List[str]: try: registered = await _discover_and_register_server(name, cfg) transport_type = "HTTP" if "url" in cfg else "stdio" - print(f" MCP: '{name}' ({transport_type}) — {len(registered)} tool(s)") return registered except Exception as exc: - print(f" MCP: '{name}' — FAILED: {exc}") logger.warning( "Failed to connect to MCP server '%s': %s", name, exc, @@ -666,12 +949,49 @@ def discover_mcp_tools() -> List[str]: summary = f" MCP: {len(all_tools)} tool(s) from {ok_servers} server(s)" if failed_count: summary += f" ({failed_count} failed)" - print(summary) + logger.info(summary) # Return ALL registered tools (existing + newly discovered) return _existing_tool_names() +def get_mcp_status() -> List[dict]: + """Return status of all configured MCP servers for banner display. + + Returns a list of dicts with keys: name, transport, tools, connected. + Includes both successfully connected servers and configured-but-failed ones. + """ + result: List[dict] = [] + + # Get configured servers from config + configured = _load_mcp_config() + if not configured: + return result + + with _lock: + active_servers = dict(_servers) + + for name, cfg in configured.items(): + transport = "http" if "url" in cfg else "stdio" + server = active_servers.get(name) + if server and server.session is not None: + result.append({ + "name": name, + "transport": transport, + "tools": len(server._tools), + "connected": True, + }) + else: + result.append({ + "name": name, + "transport": transport, + "tools": 0, + "connected": False, + }) + + return result + + def shutdown_mcp_servers(): """Close all MCP server connections and stop the background loop. From eec31b008910df8805220fad321fa94f34e63188 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 19:25:06 -0800 Subject: [PATCH 11/12] fix(mcp): /reload-mcp now updates agent tools + injects history message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLI: After reload, refreshes self.agent.tools and valid_tool_names so the model sees updated tools on its next API call - Both CLI and Gateway: Appends a [SYSTEM: ...] message at the END of conversation history explaining what changed (added/removed/ reconnected servers, tool count). This preserves prompt-cache for the system prompt and earlier messages — only the tail changes. - Gateway already creates a new AIAgent per message so tools refresh naturally; the injected message provides context for the model --- cli.py | 38 ++++++++++++++++++++++++++++++++++++-- gateway/run.py | 25 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/cli.py b/cli.py index 64e90c1f..1808d91a 100755 --- a/cli.py +++ b/cli.py @@ -1895,7 +1895,11 @@ class HermesCLI: logging.getLogger(quiet_logger).setLevel(logging.ERROR) def _reload_mcp(self): - """Reload MCP servers: disconnect all, re-read config.yaml, reconnect.""" + """Reload MCP servers: disconnect all, re-read config.yaml, reconnect. + + After reconnecting, refreshes the agent's tool list so the model + sees the updated tools on the next turn. + """ try: from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _load_mcp_config, _servers, _lock @@ -1926,10 +1930,40 @@ class HermesCLI: if removed: print(f" ➖ Removed: {', '.join(sorted(removed))}") if not connected_servers: - print(" (._.) No MCP servers connected.") + print(" No MCP servers connected.") else: print(f" 🔧 {len(new_tools)} tool(s) available from {len(connected_servers)} server(s)") + # Refresh the agent's tool list so the model can call new tools + if self.agent is not None: + from model_tools import get_tool_definitions + self.agent.tools = get_tool_definitions( + enabled_toolsets=self.agent.enabled_toolsets + if hasattr(self.agent, "enabled_toolsets") else None, + quiet_mode=True, + ) + self.agent.valid_tool_names = { + tool["function"]["name"] for tool in self.agent.tools + } if self.agent.tools else set() + + # Inject a message at the END of conversation history so the + # model knows tools changed. Appended after all existing + # messages to preserve prompt-cache for the prefix. + change_parts = [] + if added: + change_parts.append(f"Added servers: {', '.join(sorted(added))}") + if removed: + change_parts.append(f"Removed servers: {', '.join(sorted(removed))}") + if reconnected: + change_parts.append(f"Reconnected servers: {', '.join(sorted(reconnected))}") + tool_summary = f"{len(new_tools)} MCP tool(s) now available" if new_tools else "No MCP tools available" + change_detail = ". ".join(change_parts) + ". " if change_parts else "" + self.conversation_history.append({ + "role": "user", + "content": f"[SYSTEM: MCP servers have been reloaded. {change_detail}{tool_summary}. The tool list for this conversation has been updated accordingly.]", + }) + print(f" ✅ Agent updated — {len(self.agent.tools if self.agent else [])} tool(s) available") + except Exception as e: print(f" ❌ MCP reload failed: {e}") diff --git a/gateway/run.py b/gateway/run.py index 83b97372..7471bc55 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1422,6 +1422,31 @@ class GatewayRunner: lines.append("No MCP servers connected.") else: lines.append(f"\n🔧 {len(new_tools)} tool(s) available from {len(connected_servers)} server(s)") + + # Inject a message at the END of the session history so the + # model knows tools changed on its next turn. Appended after + # all existing messages to preserve prompt-cache for the prefix. + change_parts = [] + if added: + change_parts.append(f"Added servers: {', '.join(sorted(added))}") + if removed: + change_parts.append(f"Removed servers: {', '.join(sorted(removed))}") + if reconnected: + change_parts.append(f"Reconnected servers: {', '.join(sorted(reconnected))}") + tool_summary = f"{len(new_tools)} MCP tool(s) now available" if new_tools else "No MCP tools available" + change_detail = ". ".join(change_parts) + ". " if change_parts else "" + reload_msg = { + "role": "user", + "content": f"[SYSTEM: MCP servers have been reloaded. {change_detail}{tool_summary}. The tool list for this conversation has been updated accordingly.]", + } + try: + session_entry = self.session_store.get_or_create_session(event.source) + self.session_store.append_to_transcript( + session_entry.session_id, reload_msg + ) + except Exception: + pass # Best-effort; don't fail the reload over a transcript write + return "\n".join(lines) except Exception as e: From 3ead3401e0b0d1e1059c0b28c183b8ca5b6b3c7b Mon Sep 17 00:00:00 2001 From: teknium1 Date: Mon, 2 Mar 2026 21:31:23 -0800 Subject: [PATCH 12/12] fix(mcp): persist updated tools to session log immediately after reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After /reload-mcp updates self.agent.tools, immediately call _persist_session() so the session JSON file at ~/.hermes/sessions/ reflects the new tools list. Without this, the tools field in the session log would only update on the next conversation turn — if the user quit after reloading, the log would have stale tools. --- cli.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cli.py b/cli.py index 1808d91a..4079d89c 100755 --- a/cli.py +++ b/cli.py @@ -1962,6 +1962,18 @@ class HermesCLI: "role": "user", "content": f"[SYSTEM: MCP servers have been reloaded. {change_detail}{tool_summary}. The tool list for this conversation has been updated accordingly.]", }) + + # Persist session immediately so the session log reflects the + # updated tools list (self.agent.tools was refreshed above). + if self.agent is not None: + try: + self.agent._persist_session( + self.conversation_history, + self.conversation_history, + ) + except Exception: + pass # Best-effort + print(f" ✅ Agent updated — {len(self.agent.tools if self.agent else [])} tool(s) available") except Exception as e: