diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index c15c069e5..aa8bdeaad 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -985,12 +985,19 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): if len(platform_keys) > 1: platform_choices.append("Configure all platforms (global)") platform_choices.append("Reconfigure an existing tool's provider or API key") + + # Show MCP option if any MCP servers are configured + _has_mcp = bool(config.get("mcp_servers")) + if _has_mcp: + platform_choices.append("Configure MCP server tools") + platform_choices.append("Done") # Index offsets for the extra options after per-platform entries _global_idx = len(platform_keys) if len(platform_keys) > 1 else -1 _reconfig_idx = len(platform_keys) + (1 if len(platform_keys) > 1 else 0) - _done_idx = _reconfig_idx + 1 + _mcp_idx = (_reconfig_idx + 1) if _has_mcp else -1 + _done_idx = _reconfig_idx + (2 if _has_mcp else 1) while True: idx = _prompt_choice("Select an option:", platform_choices, default=0) @@ -1005,6 +1012,12 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): print() continue + # "Configure MCP tools" selected + if idx == _mcp_idx: + _configure_mcp_tools_interactive(config) + print() + continue + # "Configure all platforms (global)" selected if idx == _global_idx: # Use the union of all platforms' current tools as the starting state @@ -1091,6 +1104,137 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): print() +# ─── MCP Tools Interactive Configuration ───────────────────────────────────── + + +def _configure_mcp_tools_interactive(config: dict): + """Probe MCP servers for available tools and let user toggle them on/off. + + Connects to each configured MCP server, discovers tools, then shows + a per-server curses checklist. Writes changes back as ``tools.exclude`` + entries in config.yaml. + """ + from hermes_cli.curses_ui import curses_checklist + + mcp_servers = config.get("mcp_servers") or {} + if not mcp_servers: + _print_info("No MCP servers configured.") + return + + # Count enabled servers + enabled_names = [ + k for k, v in mcp_servers.items() + if v.get("enabled", True) not in (False, "false", "0", "no", "off") + ] + if not enabled_names: + _print_info("All MCP servers are disabled.") + return + + print() + print(color(" Discovering tools from MCP servers...", Colors.YELLOW)) + print(color(f" Connecting to {len(enabled_names)} server(s): {', '.join(enabled_names)}", Colors.DIM)) + + try: + from tools.mcp_tool import probe_mcp_server_tools + server_tools = probe_mcp_server_tools() + except Exception as exc: + _print_error(f"Failed to probe MCP servers: {exc}") + return + + if not server_tools: + _print_warning("Could not discover tools from any MCP server.") + _print_info("Check that server commands/URLs are correct and dependencies are installed.") + return + + # Report discovery results + failed = [n for n in enabled_names if n not in server_tools] + if failed: + for name in failed: + _print_warning(f" Could not connect to '{name}'") + + total_tools = sum(len(tools) for tools in server_tools.values()) + print(color(f" Found {total_tools} tool(s) across {len(server_tools)} server(s)", Colors.GREEN)) + print() + + any_changes = False + + for server_name, tools in server_tools.items(): + if not tools: + _print_info(f" {server_name}: no tools found") + continue + + srv_cfg = mcp_servers.get(server_name, {}) + tools_cfg = srv_cfg.get("tools") or {} + include_list = tools_cfg.get("include") or [] + exclude_list = tools_cfg.get("exclude") or [] + + # Build checklist labels + labels = [] + for tool_name, description in tools: + desc_short = description[:70] + "..." if len(description) > 70 else description + if desc_short: + labels.append(f"{tool_name} ({desc_short})") + else: + labels.append(tool_name) + + # Determine which tools are currently enabled + pre_selected: Set[int] = set() + tool_names = [t[0] for t in tools] + for i, tool_name in enumerate(tool_names): + if include_list: + # Include mode: only included tools are selected + if tool_name in include_list: + pre_selected.add(i) + elif exclude_list: + # Exclude mode: everything except excluded + if tool_name not in exclude_list: + pre_selected.add(i) + else: + # No filter: all enabled + pre_selected.add(i) + + chosen = curses_checklist( + f"MCP Server: {server_name} ({len(tools)} tools)", + labels, + pre_selected, + cancel_returns=pre_selected, + ) + + if chosen == pre_selected: + _print_info(f" {server_name}: no changes") + continue + + # Compute new exclude list based on unchecked tools + new_exclude = [tool_names[i] for i in range(len(tool_names)) if i not in chosen] + + # Update config + srv_cfg = mcp_servers.setdefault(server_name, {}) + tools_cfg = srv_cfg.setdefault("tools", {}) + + if new_exclude: + tools_cfg["exclude"] = new_exclude + # Remove include if present — we're switching to exclude mode + tools_cfg.pop("include", None) + else: + # All tools enabled — clear filters + tools_cfg.pop("exclude", None) + tools_cfg.pop("include", None) + + enabled_count = len(chosen) + disabled_count = len(tools) - enabled_count + _print_success( + f" {server_name}: {enabled_count} enabled, {disabled_count} disabled" + ) + any_changes = True + + if any_changes: + save_config(config) + print() + print(color(" ✓ MCP tool configuration saved", Colors.GREEN)) + else: + print(color(" No changes to MCP tools", Colors.DIM)) + + # ─── Non-interactive disable/enable ────────────────────────────────────────── diff --git a/tests/hermes_cli/test_mcp_tools_config.py b/tests/hermes_cli/test_mcp_tools_config.py new file mode 100644 index 000000000..d7be938ad --- /dev/null +++ b/tests/hermes_cli/test_mcp_tools_config.py @@ -0,0 +1,291 @@ +"""Tests for MCP tools interactive configuration in hermes_cli.tools_config.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from hermes_cli.tools_config import _configure_mcp_tools_interactive + +# Patch targets: imports happen inside the function body, so patch at source +_PROBE = "tools.mcp_tool.probe_mcp_server_tools" +_CHECKLIST = "hermes_cli.curses_ui.curses_checklist" +_SAVE = "hermes_cli.tools_config.save_config" + + +def test_no_mcp_servers_prints_info(capsys): + """Returns immediately when no MCP servers are configured.""" + config = {} + _configure_mcp_tools_interactive(config) + captured = capsys.readouterr() + assert "No MCP servers configured" in captured.out + + +def test_all_servers_disabled_prints_info(capsys): + """Returns immediately when all configured servers have enabled=false.""" + config = { + "mcp_servers": { + "github": {"command": "npx", "enabled": False}, + "slack": {"command": "npx", "enabled": "false"}, + } + } + _configure_mcp_tools_interactive(config) + captured = capsys.readouterr() + assert "disabled" in captured.out + + +def test_probe_failure_shows_warning(capsys): + """Shows warning when probe returns no tools.""" + config = {"mcp_servers": {"github": {"command": "npx"}}} + with patch(_PROBE, return_value={}): + _configure_mcp_tools_interactive(config) + captured = capsys.readouterr() + assert "Could not discover" in captured.out + + +def test_probe_exception_shows_error(capsys): + """Shows error when probe raises an exception.""" + config = {"mcp_servers": {"github": {"command": "npx"}}} + with patch(_PROBE, side_effect=RuntimeError("MCP not installed")): + _configure_mcp_tools_interactive(config) + captured = capsys.readouterr() + assert "Failed to probe" in captured.out + + +def test_no_changes_when_checklist_cancelled(capsys): + """No config changes when user cancels (ESC) the checklist.""" + config = { + "mcp_servers": { + "github": {"command": "npx", "args": ["-y", "server-github"]}, + } + } + tools = [("create_issue", "Create an issue"), ("search_repos", "Search repos")] + + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, return_value={0, 1}), \ + patch(_SAVE) as mock_save: + _configure_mcp_tools_interactive(config) + mock_save.assert_not_called() + captured = capsys.readouterr() + assert "no changes" in captured.out.lower() + + +def test_disabling_tool_writes_exclude_list(capsys): + """Unchecking a tool adds it to the exclude list.""" + config = { + "mcp_servers": { + "github": {"command": "npx"}, + } + } + tools = [ + ("create_issue", "Create an issue"), + ("delete_repo", "Delete a repo"), + ("search_repos", "Search repos"), + ] + + # User unchecks delete_repo (index 1) + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, return_value={0, 2}), \ + patch(_SAVE) as mock_save: + _configure_mcp_tools_interactive(config) + + mock_save.assert_called_once() + tools_cfg = config["mcp_servers"]["github"]["tools"] + assert tools_cfg["exclude"] == ["delete_repo"] + assert "include" not in tools_cfg + + +def test_enabling_all_clears_filters(capsys): + """Checking all tools clears both include and exclude lists.""" + config = { + "mcp_servers": { + "github": { + "command": "npx", + "tools": {"exclude": ["delete_repo"], "include": ["create_issue"]}, + }, + } + } + tools = [("create_issue", "Create"), ("delete_repo", "Delete")] + + # User checks all tools — pre_selected would be {0} (include mode), + # so returning {0, 1} is a change + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, return_value={0, 1}), \ + patch(_SAVE) as mock_save: + _configure_mcp_tools_interactive(config) + + mock_save.assert_called_once() + tools_cfg = config["mcp_servers"]["github"]["tools"] + assert "exclude" not in tools_cfg + assert "include" not in tools_cfg + + +def test_pre_selection_respects_existing_exclude(capsys): + """Tools in exclude list start unchecked.""" + config = { + "mcp_servers": { + "github": { + "command": "npx", + "tools": {"exclude": ["delete_repo"]}, + }, + } + } + tools = [("create_issue", "Create"), ("delete_repo", "Delete"), ("search", "Search")] + captured_pre_selected = {} + + def fake_checklist(title, labels, pre_selected, **kwargs): + captured_pre_selected["value"] = set(pre_selected) + return pre_selected # No changes + + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, side_effect=fake_checklist), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + # create_issue (0) and search (2) should be pre-selected, delete_repo (1) should not + assert captured_pre_selected["value"] == {0, 2} + + +def test_pre_selection_respects_existing_include(capsys): + """Only tools in include list start checked.""" + config = { + "mcp_servers": { + "github": { + "command": "npx", + "tools": {"include": ["search"]}, + }, + } + } + tools = [("create_issue", "Create"), ("delete_repo", "Delete"), ("search", "Search")] + captured_pre_selected = {} + + def fake_checklist(title, labels, pre_selected, **kwargs): + captured_pre_selected["value"] = set(pre_selected) + return pre_selected # No changes + + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, side_effect=fake_checklist), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + # Only search (2) should be pre-selected + assert captured_pre_selected["value"] == {2} + + +def test_multiple_servers_each_get_checklist(capsys): + """Each server gets its own checklist.""" + config = { + "mcp_servers": { + "github": {"command": "npx"}, + "slack": {"url": "https://mcp.example.com"}, + } + } + checklist_calls = [] + + def fake_checklist(title, labels, pre_selected, **kwargs): + checklist_calls.append(title) + return pre_selected # No changes + + with patch( + _PROBE, + return_value={ + "github": [("create_issue", "Create")], + "slack": [("send_message", "Send")], + }, + ), patch(_CHECKLIST, side_effect=fake_checklist), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + assert len(checklist_calls) == 2 + assert any("github" in t for t in checklist_calls) + assert any("slack" in t for t in checklist_calls) + + +def test_failed_server_shows_warning(capsys): + """Servers that fail to connect show warnings.""" + config = { + "mcp_servers": { + "github": {"command": "npx"}, + "broken": {"command": "nonexistent"}, + } + } + + # Only github succeeds + with patch( + _PROBE, return_value={"github": [("create_issue", "Create")]}, + ), patch(_CHECKLIST, return_value={0}), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + captured = capsys.readouterr() + assert "broken" in captured.out + + +def test_description_truncation_in_labels(): + """Long descriptions are truncated in checklist labels.""" + config = { + "mcp_servers": { + "github": {"command": "npx"}, + } + } + long_desc = "A" * 100 + captured_labels = {} + + def fake_checklist(title, labels, pre_selected, **kwargs): + captured_labels["value"] = labels + return pre_selected + + with patch( + _PROBE, return_value={"github": [("my_tool", long_desc)]}, + ), patch(_CHECKLIST, side_effect=fake_checklist), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + label = captured_labels["value"][0] + assert "..." in label + assert len(label) < len(long_desc) + 30 # truncated + tool name + parens + + +def test_switching_from_include_to_exclude(capsys): + """When user modifies selection, include list is replaced by exclude list.""" + config = { + "mcp_servers": { + "github": { + "command": "npx", + "tools": {"include": ["create_issue"]}, + }, + } + } + tools = [("create_issue", "Create"), ("search", "Search"), ("delete", "Delete")] + + # User selects create_issue and search (deselects delete) + # pre_selected would be {0} (only create_issue from include), so {0, 1} is a change + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, return_value={0, 1}), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + tools_cfg = config["mcp_servers"]["github"]["tools"] + assert tools_cfg["exclude"] == ["delete"] + assert "include" not in tools_cfg + + +def test_empty_tools_server_skipped(capsys): + """Server with no tools shows info message and skips checklist.""" + config = { + "mcp_servers": { + "empty": {"command": "npx"}, + } + } + checklist_calls = [] + + def fake_checklist(title, labels, pre_selected, **kwargs): + checklist_calls.append(title) + return pre_selected + + with patch(_PROBE, return_value={"empty": []}), \ + patch(_CHECKLIST, side_effect=fake_checklist), \ + patch(_SAVE): + _configure_mcp_tools_interactive(config) + + assert len(checklist_calls) == 0 + captured = capsys.readouterr() + assert "no tools found" in captured.out diff --git a/tests/tools/test_mcp_probe.py b/tests/tools/test_mcp_probe.py new file mode 100644 index 000000000..a592c5dca --- /dev/null +++ b/tests/tools/test_mcp_probe.py @@ -0,0 +1,210 @@ +"""Tests for probe_mcp_server_tools() in tools.mcp_tool.""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +@pytest.fixture(autouse=True) +def _reset_mcp_state(): + """Ensure clean MCP module state before/after each test.""" + import tools.mcp_tool as mcp + old_loop = mcp._mcp_loop + old_thread = mcp._mcp_thread + old_servers = dict(mcp._servers) + yield + mcp._servers.clear() + mcp._servers.update(old_servers) + mcp._mcp_loop = old_loop + mcp._mcp_thread = old_thread + + +class TestProbeMcpServerTools: + """Tests for the lightweight probe_mcp_server_tools function.""" + + def test_returns_empty_when_mcp_not_available(self): + with patch("tools.mcp_tool._MCP_AVAILABLE", False): + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + assert result == {} + + def test_returns_empty_when_no_config(self): + with patch("tools.mcp_tool._load_mcp_config", return_value={}): + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + assert result == {} + + def test_returns_empty_when_all_servers_disabled(self): + config = { + "github": {"command": "npx", "enabled": False}, + "slack": {"command": "npx", "enabled": "off"}, + } + with patch("tools.mcp_tool._load_mcp_config", return_value=config): + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + assert result == {} + + def test_returns_tools_from_successful_server(self): + """Successfully probed server returns its tools list.""" + config = { + "github": {"command": "npx", "connect_timeout": 5}, + } + mock_tool_1 = SimpleNamespace(name="create_issue", description="Create a new issue") + mock_tool_2 = SimpleNamespace(name="search_repos", description="Search repositories") + + mock_server = MagicMock() + mock_server._tools = [mock_tool_1, mock_tool_2] + mock_server.shutdown = AsyncMock() + + async def fake_connect(name, cfg): + return mock_server + + with patch("tools.mcp_tool._load_mcp_config", return_value=config), \ + patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop") as mock_run, \ + patch("tools.mcp_tool._stop_mcp_loop"): + + # Simulate running the async probe + def run_coro(coro, timeout=120): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + mock_run.side_effect = run_coro + + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + + assert "github" in result + assert len(result["github"]) == 2 + assert result["github"][0] == ("create_issue", "Create a new issue") + assert result["github"][1] == ("search_repos", "Search repositories") + mock_server.shutdown.assert_awaited_once() + + def test_failed_server_omitted_from_results(self): + """Servers that fail to connect are silently skipped.""" + config = { + "github": {"command": "npx", "connect_timeout": 5}, + "broken": {"command": "nonexistent", "connect_timeout": 5}, + } + mock_tool = SimpleNamespace(name="create_issue", description="Create") + mock_server = MagicMock() + mock_server._tools = [mock_tool] + mock_server.shutdown = AsyncMock() + + async def fake_connect(name, cfg): + if name == "broken": + raise ConnectionError("Server not found") + return mock_server + + with patch("tools.mcp_tool._load_mcp_config", return_value=config), \ + patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop") as mock_run, \ + patch("tools.mcp_tool._stop_mcp_loop"): + + def run_coro(coro, timeout=120): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + mock_run.side_effect = run_coro + + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + + assert "github" in result + assert "broken" not in result + + def test_handles_tool_without_description(self): + """Tools without descriptions get empty string.""" + config = {"github": {"command": "npx", "connect_timeout": 5}} + mock_tool = SimpleNamespace(name="my_tool") # no description attribute + + mock_server = MagicMock() + mock_server._tools = [mock_tool] + mock_server.shutdown = AsyncMock() + + async def fake_connect(name, cfg): + return mock_server + + with patch("tools.mcp_tool._load_mcp_config", return_value=config), \ + patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop") as mock_run, \ + patch("tools.mcp_tool._stop_mcp_loop"): + + def run_coro(coro, timeout=120): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + mock_run.side_effect = run_coro + + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + + assert result["github"][0] == ("my_tool", "") + + def test_cleanup_called_even_on_failure(self): + """_stop_mcp_loop is called even when probe fails.""" + config = {"github": {"command": "npx", "connect_timeout": 5}} + + with patch("tools.mcp_tool._load_mcp_config", return_value=config), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop", side_effect=RuntimeError("boom")), \ + patch("tools.mcp_tool._stop_mcp_loop") as mock_stop: + + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + + assert result == {} + mock_stop.assert_called_once() + + def test_skips_disabled_servers(self): + """Disabled servers are not probed.""" + config = { + "github": {"command": "npx", "connect_timeout": 5}, + "disabled_one": {"command": "npx", "enabled": False}, + } + mock_tool = SimpleNamespace(name="create_issue", description="Create") + mock_server = MagicMock() + mock_server._tools = [mock_tool] + mock_server.shutdown = AsyncMock() + + connect_calls = [] + + async def fake_connect(name, cfg): + connect_calls.append(name) + return mock_server + + with patch("tools.mcp_tool._load_mcp_config", return_value=config), \ + patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop") as mock_run, \ + patch("tools.mcp_tool._stop_mcp_loop"): + + def run_coro(coro, timeout=120): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + mock_run.side_effect = run_coro + + from tools.mcp_tool import probe_mcp_server_tools + result = probe_mcp_server_tools() + + assert "github" in result + assert "disabled_one" not in result + assert "disabled_one" not in connect_calls diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 7294e8be5..7ff8103b2 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -1624,6 +1624,72 @@ def get_mcp_status() -> List[dict]: return result +def probe_mcp_server_tools() -> Dict[str, List[tuple]]: + """Temporarily connect to configured MCP servers and list their tools. + + Designed for ``hermes tools`` interactive configuration — connects to each + enabled server, grabs tool names and descriptions, then disconnects. + Does NOT register tools in the Hermes registry. + + Returns: + Dict mapping server name to list of (tool_name, description) tuples. + Servers that fail to connect are omitted from the result. + """ + if not _MCP_AVAILABLE: + return {} + + servers_config = _load_mcp_config() + if not servers_config: + return {} + + enabled = { + k: v for k, v in servers_config.items() + if _parse_boolish(v.get("enabled", True), default=True) + } + if not enabled: + return {} + + _ensure_mcp_loop() + + result: Dict[str, List[tuple]] = {} + probed_servers: List[MCPServerTask] = [] + + async def _probe_all(): + names = list(enabled.keys()) + coros = [] + for name, cfg in enabled.items(): + ct = cfg.get("connect_timeout", _DEFAULT_CONNECT_TIMEOUT) + coros.append(asyncio.wait_for(_connect_server(name, cfg), timeout=ct)) + + outcomes = await asyncio.gather(*coros, return_exceptions=True) + + for name, outcome in zip(names, outcomes): + if isinstance(outcome, Exception): + logger.debug("Probe: failed to connect to '%s': %s", name, outcome) + continue + probed_servers.append(outcome) + tools = [] + for t in outcome._tools: + desc = getattr(t, "description", "") or "" + tools.append((t.name, desc)) + result[name] = tools + + # Shut down all probed connections + await asyncio.gather( + *(s.shutdown() for s in probed_servers), + return_exceptions=True, + ) + + try: + _run_on_mcp_loop(_probe_all(), timeout=120) + except Exception as exc: + logger.debug("MCP probe failed: %s", exc) + finally: + _stop_mcp_loop() + + return result + + def shutdown_mcp_servers(): """Close all MCP server connections and stop the background loop.