From a1241ee68ccb333abd905f526823583e56c8220b Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 14 Mar 2026 10:26:15 +0000 Subject: [PATCH] fix(mcp): clarify enabledTools filtering semantics - support both raw and wrapped MCP tool names - treat [\"*\"] as all tools and [] as no tools - add warnings, tests, and README docs for enabledTools --- README.md | 22 +++++ nanobot/agent/tools/mcp.py | 36 ++++++- nanobot/config/schema.py | 2 +- tests/test_mcp_tool.py | 187 ++++++++++++++++++++++++++++++++++++- 4 files changed, 241 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e7bb41d..bc27255 100644 --- a/README.md +++ b/README.md @@ -1112,6 +1112,28 @@ Use `toolTimeout` to override the default 30s per-call timeout for slow servers: } ``` +Use `enabledTools` to register only a subset of tools from an MCP server: + +```json +{ + "tools": { + "mcpServers": { + "filesystem": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/path/to/dir"], + "enabledTools": ["read_file", "mcp_filesystem_write_file"] + } + } + } +} +``` + +`enabledTools` accepts either the raw MCP tool name (for example `read_file`) or the wrapped nanobot tool name (for example `mcp_filesystem_write_file`). + +- Omit `enabledTools`, or set it to `["*"]`, to register all tools. +- Set `enabledTools` to `[]` to register no tools from that server. +- Set `enabledTools` to a non-empty list of names to register only that subset. + MCP tools are automatically discovered and registered on startup. The LLM can use them alongside built-in tools — no extra configuration needed. diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 8c5c6ba..cebfbd2 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -138,16 +138,46 @@ async def connect_mcp_servers( await session.initialize() tools = await session.list_tools() - enabled_tools = set(cfg.enabled_tools) if cfg.enabled_tools else None + enabled_tools = set(cfg.enabled_tools) + allow_all_tools = "*" in enabled_tools registered_count = 0 + matched_enabled_tools: set[str] = set() + available_raw_names = [tool_def.name for tool_def in tools.tools] + available_wrapped_names = [f"mcp_{name}_{tool_def.name}" for tool_def in tools.tools] for tool_def in tools.tools: - if enabled_tools and tool_def.name not in enabled_tools: - logger.debug("MCP: skipping tool '{}' from server '{}' (not in enabledTools)", tool_def.name, name) + wrapped_name = f"mcp_{name}_{tool_def.name}" + if ( + not allow_all_tools + and tool_def.name not in enabled_tools + and wrapped_name not in enabled_tools + ): + logger.debug( + "MCP: skipping tool '{}' from server '{}' (not in enabledTools)", + wrapped_name, + name, + ) continue wrapper = MCPToolWrapper(session, name, tool_def, tool_timeout=cfg.tool_timeout) registry.register(wrapper) logger.debug("MCP: registered tool '{}' from server '{}'", wrapper.name, name) registered_count += 1 + if enabled_tools: + if tool_def.name in enabled_tools: + matched_enabled_tools.add(tool_def.name) + if wrapped_name in enabled_tools: + matched_enabled_tools.add(wrapped_name) + + if enabled_tools and not allow_all_tools: + unmatched_enabled_tools = sorted(enabled_tools - matched_enabled_tools) + if unmatched_enabled_tools: + logger.warning( + "MCP server '{}': enabledTools entries not found: {}. Available raw names: {}. " + "Available wrapped names: {}", + name, + ", ".join(unmatched_enabled_tools), + ", ".join(available_raw_names) or "(none)", + ", ".join(available_wrapped_names) or "(none)", + ) logger.info("MCP server '{}': connected, {} tools registered", name, registered_count) except Exception as e: diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index aa3e676..033fb63 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -140,7 +140,7 @@ class MCPServerConfig(Base): url: str = "" # HTTP/SSE: endpoint URL headers: dict[str, str] = Field(default_factory=dict) # HTTP/SSE: custom headers tool_timeout: int = 30 # seconds before a tool call is cancelled - enabled_tools: list[str] = Field(default_factory=list) # Only register these tools; empty = all tools + enabled_tools: list[str] = Field(default_factory=lambda: ["*"]) # Only register these tools; accepts raw MCP names or wrapped mcp__ names; ["*"] = all tools; [] = no tools class ToolsConfig(Base): """Tools configuration.""" diff --git a/tests/test_mcp_tool.py b/tests/test_mcp_tool.py index bf68425..d014f58 100644 --- a/tests/test_mcp_tool.py +++ b/tests/test_mcp_tool.py @@ -1,12 +1,15 @@ from __future__ import annotations import asyncio +from contextlib import AsyncExitStack, asynccontextmanager import sys from types import ModuleType, SimpleNamespace import pytest -from nanobot.agent.tools.mcp import MCPToolWrapper +from nanobot.agent.tools.mcp import MCPToolWrapper, connect_mcp_servers +from nanobot.agent.tools.registry import ToolRegistry +from nanobot.config.schema import MCPServerConfig class _FakeTextContent: @@ -14,12 +17,63 @@ class _FakeTextContent: self.text = text +@pytest.fixture +def fake_mcp_runtime() -> dict[str, object | None]: + return {"session": None} + + @pytest.fixture(autouse=True) -def _fake_mcp_module(monkeypatch: pytest.MonkeyPatch) -> None: +def _fake_mcp_module( + monkeypatch: pytest.MonkeyPatch, fake_mcp_runtime: dict[str, object | None] +) -> None: mod = ModuleType("mcp") mod.types = SimpleNamespace(TextContent=_FakeTextContent) + + class _FakeStdioServerParameters: + def __init__(self, command: str, args: list[str], env: dict | None = None) -> None: + self.command = command + self.args = args + self.env = env + + class _FakeClientSession: + def __init__(self, _read: object, _write: object) -> None: + self._session = fake_mcp_runtime["session"] + + async def __aenter__(self) -> object: + return self._session + + async def __aexit__(self, exc_type, exc, tb) -> bool: + return False + + @asynccontextmanager + async def _fake_stdio_client(_params: object): + yield object(), object() + + @asynccontextmanager + async def _fake_sse_client(_url: str, httpx_client_factory=None): + yield object(), object() + + @asynccontextmanager + async def _fake_streamable_http_client(_url: str, http_client=None): + yield object(), object(), object() + + mod.ClientSession = _FakeClientSession + mod.StdioServerParameters = _FakeStdioServerParameters monkeypatch.setitem(sys.modules, "mcp", mod) + client_mod = ModuleType("mcp.client") + stdio_mod = ModuleType("mcp.client.stdio") + stdio_mod.stdio_client = _fake_stdio_client + sse_mod = ModuleType("mcp.client.sse") + sse_mod.sse_client = _fake_sse_client + streamable_http_mod = ModuleType("mcp.client.streamable_http") + streamable_http_mod.streamable_http_client = _fake_streamable_http_client + + monkeypatch.setitem(sys.modules, "mcp.client", client_mod) + monkeypatch.setitem(sys.modules, "mcp.client.stdio", stdio_mod) + monkeypatch.setitem(sys.modules, "mcp.client.sse", sse_mod) + monkeypatch.setitem(sys.modules, "mcp.client.streamable_http", streamable_http_mod) + def _make_wrapper(session: object, *, timeout: float = 0.1) -> MCPToolWrapper: tool_def = SimpleNamespace( @@ -97,3 +151,132 @@ async def test_execute_handles_generic_exception() -> None: result = await wrapper.execute() assert result == "(MCP tool call failed: RuntimeError)" + + +def _make_tool_def(name: str) -> SimpleNamespace: + return SimpleNamespace( + name=name, + description=f"{name} tool", + inputSchema={"type": "object", "properties": {}}, + ) + + +def _make_fake_session(tool_names: list[str]) -> SimpleNamespace: + async def initialize() -> None: + return None + + async def list_tools() -> SimpleNamespace: + return SimpleNamespace(tools=[_make_tool_def(name) for name in tool_names]) + + return SimpleNamespace(initialize=initialize, list_tools=list_tools) + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_supports_raw_names( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"]) + registry = ToolRegistry() + stack = AsyncExitStack() + await stack.__aenter__() + try: + await connect_mcp_servers( + {"test": MCPServerConfig(command="fake", enabled_tools=["demo"])}, + registry, + stack, + ) + finally: + await stack.aclose() + + assert registry.tool_names == ["mcp_test_demo"] + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_defaults_to_all( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"]) + registry = ToolRegistry() + stack = AsyncExitStack() + await stack.__aenter__() + try: + await connect_mcp_servers( + {"test": MCPServerConfig(command="fake")}, + registry, + stack, + ) + finally: + await stack.aclose() + + assert registry.tool_names == ["mcp_test_demo", "mcp_test_other"] + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_supports_wrapped_names( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"]) + registry = ToolRegistry() + stack = AsyncExitStack() + await stack.__aenter__() + try: + await connect_mcp_servers( + {"test": MCPServerConfig(command="fake", enabled_tools=["mcp_test_demo"])}, + registry, + stack, + ) + finally: + await stack.aclose() + + assert registry.tool_names == ["mcp_test_demo"] + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_empty_list_registers_none( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"]) + registry = ToolRegistry() + stack = AsyncExitStack() + await stack.__aenter__() + try: + await connect_mcp_servers( + {"test": MCPServerConfig(command="fake", enabled_tools=[])}, + registry, + stack, + ) + finally: + await stack.aclose() + + assert registry.tool_names == [] + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_warns_on_unknown_entries( + fake_mcp_runtime: dict[str, object | None], monkeypatch: pytest.MonkeyPatch +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo"]) + registry = ToolRegistry() + warnings: list[str] = [] + + def _warning(message: str, *args: object) -> None: + warnings.append(message.format(*args)) + + monkeypatch.setattr("nanobot.agent.tools.mcp.logger.warning", _warning) + + stack = AsyncExitStack() + await stack.__aenter__() + try: + await connect_mcp_servers( + {"test": MCPServerConfig(command="fake", enabled_tools=["unknown"])}, + registry, + stack, + ) + finally: + await stack.aclose() + + assert registry.tool_names == [] + assert warnings + assert "enabledTools entries not found: unknown" in warnings[-1] + assert "Available raw names: demo" in warnings[-1] + assert "Available wrapped names: mcp_test_demo" in warnings[-1]