diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index 774bf98938..a5e2c75bb9 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -116,6 +116,21 @@ class TestValidateToolset: def test_invalid(self): assert validate_toolset("nonexistent") is False + def test_mcp_alias_uses_live_registry(self, monkeypatch): + reg = ToolRegistry() + reg.register( + name="mcp_dynserver_ping", + toolset="mcp-dynserver", + schema=_make_schema("mcp_dynserver_ping", "Ping"), + handler=_dummy_handler, + ) + + monkeypatch.setattr("tools.registry.registry", reg) + + assert validate_toolset("dynserver") is True + assert validate_toolset("mcp-dynserver") is True + assert "mcp_dynserver_ping" in resolve_toolset("dynserver") + class TestGetToolsetInfo: def test_leaf(self): @@ -150,6 +165,23 @@ class TestCreateCustomToolset: del TOOLSETS["_test_custom"] +class TestRegistryOwnedToolsets: + def test_registry_membership_is_live(self, monkeypatch): + reg = ToolRegistry() + reg.register( + name="test_live_toolset_tool", + toolset="test-live-toolset", + schema=_make_schema("test_live_toolset_tool", "Live"), + handler=_dummy_handler, + ) + + monkeypatch.setattr("tools.registry.registry", reg) + + assert validate_toolset("test-live-toolset") is True + assert get_toolset("test-live-toolset")["tools"] == ["test_live_toolset_tool"] + assert resolve_toolset("test-live-toolset") == ["test_live_toolset_tool"] + + class TestToolsetConsistency: """Verify structural integrity of the built-in TOOLSETS dict.""" diff --git a/tests/tools/test_mcp_dynamic_discovery.py b/tests/tools/test_mcp_dynamic_discovery.py index c7c4ae86cd..991342bd0b 100644 --- a/tests/tools/test_mcp_dynamic_discovery.py +++ b/tests/tools/test_mcp_dynamic_discovery.py @@ -21,34 +21,19 @@ class TestRegisterServerTools: def mock_registry(self): return ToolRegistry() - @pytest.fixture - def mock_toolsets(self): - return { - "hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []}, - "hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []}, - "custom-toolset": {"tools": [], "description": "Other", "includes": []}, - } - - def test_injects_hermes_toolsets(self, mock_registry, mock_toolsets): - """Tools are injected into hermes-* toolsets but not custom ones.""" + def test_exposes_live_server_aliases(self, mock_registry): + """Registered MCP tools are reachable via live raw-server aliases.""" server = MCPServerTask("my_srv") server._tools = [_make_mcp_tool("my_tool", "desc")] server.session = MagicMock() + from toolsets import resolve_toolset, validate_toolset - with patch("tools.registry.registry", mock_registry), \ - patch("toolsets.create_custom_toolset"), \ - patch.dict("toolsets.TOOLSETS", mock_toolsets, clear=True): - + with patch("tools.registry.registry", mock_registry): registered = _register_server_tools("my_srv", server, {}) - - assert "mcp_my_srv_my_tool" in registered - assert "mcp_my_srv_my_tool" in mock_registry.get_all_tool_names() - - # Injected into hermes-* toolsets - assert "mcp_my_srv_my_tool" in mock_toolsets["hermes-cli"]["tools"] - assert "mcp_my_srv_my_tool" in mock_toolsets["hermes-telegram"]["tools"] - # NOT into non-hermes toolsets - assert "mcp_my_srv_my_tool" not in mock_toolsets["custom-toolset"]["tools"] + assert "mcp_my_srv_my_tool" in registered + assert "mcp_my_srv_my_tool" in mock_registry.get_all_tool_names() + assert validate_toolset("my_srv") is True + assert "mcp_my_srv_my_tool" in resolve_toolset("my_srv") class TestRefreshTools: @@ -58,19 +43,13 @@ class TestRefreshTools: def mock_registry(self): return ToolRegistry() - @pytest.fixture - def mock_toolsets(self): - return { - "hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []}, - "hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []}, - } - @pytest.mark.asyncio - async def test_nuke_and_repave(self, mock_registry, mock_toolsets): + async def test_nuke_and_repave(self, mock_registry): """Old tools are removed and new tools registered on refresh.""" server = MCPServerTask("live_srv") server._refresh_lock = asyncio.Lock() server._config = {} + from toolsets import resolve_toolset # Seed initial state: one old tool registered mock_registry.register( @@ -79,7 +58,6 @@ class TestRefreshTools: description="", emoji="", ) server._registered_tool_names = ["mcp_live_srv_old_tool"] - mock_toolsets["hermes-cli"]["tools"].append("mcp_live_srv_old_tool") # New tool list from server new_tool = _make_mcp_tool("new_tool", "new behavior") @@ -89,20 +67,13 @@ class TestRefreshTools: ) ) - with patch("tools.registry.registry", mock_registry), \ - patch("toolsets.create_custom_toolset"), \ - patch.dict("toolsets.TOOLSETS", mock_toolsets, clear=True): - + with patch("tools.registry.registry", mock_registry): await server._refresh_tools() - - # Old tool completely gone - assert "mcp_live_srv_old_tool" not in mock_registry.get_all_tool_names() - assert "mcp_live_srv_old_tool" not in mock_toolsets["hermes-cli"]["tools"] - - # New tool registered - assert "mcp_live_srv_new_tool" in mock_registry.get_all_tool_names() - assert "mcp_live_srv_new_tool" in mock_toolsets["hermes-cli"]["tools"] - assert server._registered_tool_names == ["mcp_live_srv_new_tool"] + assert "mcp_live_srv_old_tool" not in mock_registry.get_all_tool_names() + assert "mcp_live_srv_old_tool" not in resolve_toolset("live_srv") + assert "mcp_live_srv_new_tool" in mock_registry.get_all_tool_names() + assert "mcp_live_srv_new_tool" in resolve_toolset("live_srv") + assert server._registered_tool_names == ["mcp_live_srv_new_tool"] class TestMessageHandler: diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 43049c2c18..f5f15ea41d 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -365,10 +365,13 @@ class TestDiscoverAndRegister: _servers.pop("fs", None) - def test_toolset_created(self): - """A custom toolset is created for the MCP server.""" + def test_toolset_resolves_live_from_registry(self): + """MCP toolsets resolve through the live registry without TOOLSETS mutation.""" + from tools.registry import ToolRegistry from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask + from toolsets import resolve_toolset, validate_toolset + mock_registry = ToolRegistry() mock_tools = [_make_mcp_tool("ping", "Ping")] mock_session = MagicMock() @@ -378,16 +381,16 @@ class TestDiscoverAndRegister: server._tools = mock_tools return server - mock_create = MagicMock() with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \ - patch("toolsets.create_custom_toolset", mock_create): + patch("tools.registry.registry", mock_registry): 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" + assert validate_toolset("myserver") is True + assert validate_toolset("mcp-myserver") is True + assert "mcp_myserver_ping" in resolve_toolset("myserver") + assert "mcp_myserver_ping" in resolve_toolset("mcp-myserver") _servers.pop("myserver", None) @@ -550,12 +553,15 @@ class TestMCPServerTask: # --------------------------------------------------------------------------- class TestToolsetInjection: - def test_mcp_tools_added_to_all_hermes_toolsets(self): - """Discovered MCP tools are dynamically injected into all hermes-* toolsets.""" + def test_mcp_tools_resolve_through_server_aliases(self): + """Discovered MCP tools resolve through raw server-name aliases.""" from tools.mcp_tool import MCPServerTask + from tools.registry import ToolRegistry + from toolsets import resolve_toolset, validate_toolset mock_tools = [_make_mcp_tool("list_files", "List files")] mock_session = MagicMock() + mock_registry = ToolRegistry() fresh_servers = {} @@ -565,43 +571,32 @@ class TestToolsetInjection: server._tools = mock_tools return server - fake_toolsets = { - "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), \ 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): + patch("tools.registry.registry", mock_registry): from tools.mcp_tool import discover_mcp_tools 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"] - # Server name becomes a standalone toolset - assert "fs" in fake_toolsets - assert "mcp_fs_list_files" in fake_toolsets["fs"]["tools"] - assert fake_toolsets["fs"]["description"].startswith("MCP server '") + assert "mcp_fs_list_files" in result + assert validate_toolset("fs") is True + assert validate_toolset("mcp-fs") is True + assert "mcp_fs_list_files" in resolve_toolset("fs") + assert "mcp_fs_list_files" in resolve_toolset("mcp-fs") def test_server_toolset_skips_builtin_collision(self): - """MCP server named after a built-in toolset shouldn't overwrite it.""" + """MCP raw aliases never overwrite a built-in toolset name.""" from tools.mcp_tool import MCPServerTask + from tools.registry import ToolRegistry + from toolsets import resolve_toolset, validate_toolset mock_tools = [_make_mcp_tool("run", "Run command")] mock_session = MagicMock() fresh_servers = {} + mock_registry = ToolRegistry() async def fake_connect(name, config): server = MCPServerTask(name) @@ -620,12 +615,15 @@ class TestToolsetInjection: 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("tools.registry.registry", mock_registry), \ patch("toolsets.TOOLSETS", fake_toolsets): from tools.mcp_tool import discover_mcp_tools discover_mcp_tools() - # Built-in toolset preserved — description unchanged - assert fake_toolsets["terminal"]["description"] == "Terminal tools" + assert fake_toolsets["terminal"]["description"] == "Terminal tools" + assert "mcp_terminal_run" not in resolve_toolset("terminal") + assert validate_toolset("mcp-terminal") is True + assert "mcp_terminal_run" in resolve_toolset("mcp-terminal") def test_server_connection_failure_skipped(self): """If one server fails to connect, others still proceed.""" @@ -3038,14 +3036,22 @@ class TestSanitizeMcpNameComponent: assert "/" not in name assert "." not in name - def test_slash_in_sync_mcp_toolsets(self): - """_sync_mcp_toolsets uses sanitize consistently with _convert_mcp_schema.""" - from tools.mcp_tool import sanitize_mcp_name_component + def test_slash_in_server_alias_resolution(self): + """Server names with slashes resolve through their live MCP alias.""" + from tools.registry import ToolRegistry + from toolsets import resolve_toolset, validate_toolset - # Verify the prefix generation matches what _convert_mcp_schema produces - server_name = "ai.exa/exa" - safe_prefix = f"mcp_{sanitize_mcp_name_component(server_name)}_" - assert safe_prefix == "mcp_ai_exa_exa_" + reg = ToolRegistry() + reg.register( + name="mcp_ai_exa_exa_search", + toolset="mcp-ai.exa/exa", + schema={"name": "mcp_ai_exa_exa_search", "description": "Search", "parameters": {"type": "object", "properties": {}}}, + handler=lambda *_args, **_kwargs: "{}", + ) + + with patch("tools.registry.registry", reg): + assert validate_toolset("ai.exa/exa") is True + assert "mcp_ai_exa_exa_search" in resolve_toolset("ai.exa/exa") # --------------------------------------------------------------------------- diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index d6bdc89faf..263e4408f4 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -846,8 +846,7 @@ class MCPServerTask: After the initial ``await`` (list_tools), all mutations are synchronous — atomic from the event loop's perspective. """ - from tools.registry import registry, tool_error - from toolsets import TOOLSETS + from tools.registry import registry async with self._refresh_lock: # Capture old tool names for change diff @@ -857,16 +856,11 @@ class MCPServerTask: tools_result = await self.session.list_tools() new_mcp_tools = tools_result.tools if hasattr(tools_result, "tools") else [] - # 2. Remove old tools from hermes-* umbrella toolsets - for ts_name, ts in TOOLSETS.items(): - if ts_name.startswith("hermes-"): - ts["tools"] = [t for t in ts["tools"] if t not in self._registered_tool_names] - - # 3. Deregister old tools from the central registry + # 2. Deregister old tools from the central registry for prefixed_name in self._registered_tool_names: registry.deregister(prefixed_name) - # 4. Re-register with fresh tool list + # 3. Re-register with fresh tool list self._tools = new_mcp_tools self._registered_tool_names = _register_server_tools( self.name, self, self._config @@ -1671,57 +1665,6 @@ def _convert_mcp_schema(server_name: str, mcp_tool) -> dict: } -def _sync_mcp_toolsets(server_names: Optional[List[str]] = None) -> None: - """Expose each MCP server as a standalone toolset and inject into hermes-* sets. - - Creates a real toolset entry in TOOLSETS for each server name (e.g. - TOOLSETS["github"] = {"tools": ["mcp_github_list_files", ...]}). This - makes raw server names resolvable in platform_toolsets overrides. - - Also injects all MCP tools into hermes-* umbrella toolsets for the - default behavior. - - Skips server names that collide with built-in toolsets. - """ - from toolsets import TOOLSETS - - if server_names is None: - server_names = list(_load_mcp_config().keys()) - - existing = _existing_tool_names() - all_mcp_tools: List[str] = [] - - for server_name in server_names: - safe_prefix = f"mcp_{sanitize_mcp_name_component(server_name)}_" - server_tools = sorted( - t for t in existing if t.startswith(safe_prefix) - ) - all_mcp_tools.extend(server_tools) - - # Don't overwrite a built-in toolset that happens to share the name. - existing_ts = TOOLSETS.get(server_name) - if existing_ts and not str(existing_ts.get("description", "")).startswith("MCP server '"): - logger.warning( - "Skipping MCP toolset alias '%s' — a built-in toolset already uses that name", - server_name, - ) - continue - - TOOLSETS[server_name] = { - "description": f"MCP server '{server_name}' tools", - "tools": server_tools, - "includes": [], - } - - # Also inject into hermes-* umbrella toolsets for default behavior. - for ts_name, ts in TOOLSETS.items(): - if not ts_name.startswith("hermes-"): - continue - for tool_name in all_mcp_tools: - if tool_name not in ts["tools"]: - ts["tools"].append(tool_name) - - def _build_utility_schemas(server_name: str) -> List[dict]: """Build schemas for the MCP utility tools (resources & prompts). @@ -1874,16 +1817,16 @@ def _existing_tool_names() -> List[str]: def _register_server_tools(name: str, server: MCPServerTask, config: dict) -> List[str]: """Register tools from an already-connected server into the registry. - Handles include/exclude filtering, utility tools, toolset creation, - and hermes-* umbrella toolset injection. + Handles include/exclude filtering and utility tools. Toolset resolution + for ``mcp-{server}`` and raw server-name aliases is derived from the live + registry, rather than mutating ``toolsets.TOOLSETS`` at runtime. Used by both initial discovery and dynamic refresh (list_changed). Returns: List of registered prefixed tool names. """ - from tools.registry import registry, tool_error - from toolsets import create_custom_toolset, TOOLSETS + from tools.registry import registry registered_names: List[str] = [] toolset_name = f"mcp-{name}" @@ -1973,20 +1916,6 @@ def _register_server_tools(name: str, server: MCPServerTask, config: dict) -> Li ) registered_names.append(util_name) - # 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, - ) - # Inject into hermes-* umbrella toolsets for default behavior - for ts_name, ts in TOOLSETS.items(): - if ts_name.startswith("hermes-"): - for tool_name in registered_names: - if tool_name not in ts["tools"]: - ts["tools"].append(tool_name) - return registered_names @@ -2049,7 +1978,6 @@ def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: } if not new_servers: - _sync_mcp_toolsets(list(servers.keys())) return _existing_tool_names() # Start the background event loop for MCP connections @@ -2080,8 +2008,6 @@ def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: # The outer timeout is generous: 120s total for parallel discovery. _run_on_mcp_loop(_discover_all(), timeout=120) - _sync_mcp_toolsets(list(servers.keys())) - # Log a summary so ACP callers get visibility into what was registered. with _lock: connected = [n for n in new_servers if n in _servers] diff --git a/toolsets.py b/toolsets.py index 2e7a0a92a8..7c843fbfb7 100644 --- a/toolsets.py +++ b/toolsets.py @@ -409,8 +409,31 @@ def get_toolset(name: str) -> Optional[Dict[str, Any]]: Dict: Toolset definition with description, tools, and includes None: If toolset not found """ - # Return toolset definition - return TOOLSETS.get(name) + toolset = TOOLSETS.get(name) + if toolset: + return toolset + + try: + from tools.registry import registry + except Exception: + return None + + registry_toolset = name + description = f"Plugin toolset: {name}" + + if name not in _get_plugin_toolset_names(): + registry_toolset = _get_mcp_toolset_aliases().get(name) + if not registry_toolset: + return None + description = f"MCP server '{name}' tools" + elif name.startswith("mcp-"): + description = f"MCP server '{name[4:]}' tools" + + return { + "description": description, + "tools": registry.get_tool_names_for_toolset(registry_toolset), + "includes": [], + } def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]: @@ -438,7 +461,7 @@ def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]: # Use a fresh visited set per branch to avoid cross-branch contamination resolved = resolve_toolset(toolset_name, visited.copy()) all_tools.update(resolved) - return list(all_tools) + return sorted(all_tools) # Check for cycles / already-resolved (diamond deps). # Silently return [] — either this is a diamond (not a bug, tools already @@ -449,15 +472,8 @@ def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]: visited.add(name) # Get toolset definition - toolset = TOOLSETS.get(name) + toolset = get_toolset(name) if not toolset: - # Fall back to tool registry for plugin-provided toolsets - if name in _get_plugin_toolset_names(): - try: - from tools.registry import registry - return registry.get_tool_names_for_toolset(name) - except Exception: - pass return [] # Collect direct tools @@ -470,7 +486,7 @@ def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]: included_tools = resolve_toolset(included_name, visited) tools.update(included_tools) - return list(tools) + return sorted(tools) def resolve_multiple_toolsets(toolset_names: List[str]) -> List[str]: @@ -489,7 +505,7 @@ def resolve_multiple_toolsets(toolset_names: List[str]) -> List[str]: tools = resolve_toolset(name) all_tools.update(tools) - return list(all_tools) + return sorted(all_tools) def _get_plugin_toolset_names() -> Set[str]: @@ -509,6 +525,18 @@ def _get_plugin_toolset_names() -> Set[str]: return set() +def _get_mcp_toolset_aliases() -> Dict[str, str]: + """Map raw MCP server names to their live registry toolset names.""" + aliases = {} + for toolset_name in _get_plugin_toolset_names(): + if not toolset_name.startswith("mcp-"): + continue + alias = toolset_name[4:] + if alias and alias not in TOOLSETS: + aliases[alias] = toolset_name + return aliases + + def get_all_toolsets() -> Dict[str, Dict[str, Any]]: """ Get all available toolsets with their definitions. @@ -518,19 +546,18 @@ def get_all_toolsets() -> Dict[str, Dict[str, Any]]: Returns: Dict: All toolset definitions """ - result = TOOLSETS.copy() - # Add plugin-provided toolsets (synthetic entries) + result = dict(TOOLSETS) for ts_name in _get_plugin_toolset_names(): - if ts_name not in result: - try: - from tools.registry import registry - tools = registry.get_tool_names_for_toolset(ts_name) - result[ts_name] = { - "description": f"Plugin toolset: {ts_name}", - "tools": tools, - } - except Exception: - pass + display_name = ts_name + if ts_name.startswith("mcp-"): + alias = ts_name[4:] + if alias and alias not in TOOLSETS: + display_name = alias + if display_name in result: + continue + toolset = get_toolset(display_name) + if toolset: + result[display_name] = toolset return result @@ -544,7 +571,13 @@ def get_toolset_names() -> List[str]: List[str]: List of toolset names """ names = set(TOOLSETS.keys()) - names |= _get_plugin_toolset_names() + for ts_name in _get_plugin_toolset_names(): + if ts_name.startswith("mcp-"): + alias = ts_name[4:] + if alias and alias not in TOOLSETS: + names.add(alias) + continue + names.add(ts_name) return sorted(names) @@ -565,8 +598,9 @@ def validate_toolset(name: str) -> bool: return True if name in TOOLSETS: return True - # Check tool registry for plugin-provided toolsets - return name in _get_plugin_toolset_names() + if name in _get_plugin_toolset_names(): + return True + return name in _get_mcp_toolset_aliases() def create_custom_toolset(