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
This commit is contained in:
22
README.md
22
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.
|
MCP tools are automatically discovered and registered on startup. The LLM can use them alongside built-in tools — no extra configuration needed.
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -138,16 +138,46 @@ async def connect_mcp_servers(
|
|||||||
await session.initialize()
|
await session.initialize()
|
||||||
|
|
||||||
tools = await session.list_tools()
|
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
|
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:
|
for tool_def in tools.tools:
|
||||||
if enabled_tools and tool_def.name not in enabled_tools:
|
wrapped_name = f"mcp_{name}_{tool_def.name}"
|
||||||
logger.debug("MCP: skipping tool '{}' from server '{}' (not in enabledTools)", tool_def.name, 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
|
continue
|
||||||
wrapper = MCPToolWrapper(session, name, tool_def, tool_timeout=cfg.tool_timeout)
|
wrapper = MCPToolWrapper(session, name, tool_def, tool_timeout=cfg.tool_timeout)
|
||||||
registry.register(wrapper)
|
registry.register(wrapper)
|
||||||
logger.debug("MCP: registered tool '{}' from server '{}'", wrapper.name, name)
|
logger.debug("MCP: registered tool '{}' from server '{}'", wrapper.name, name)
|
||||||
registered_count += 1
|
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)
|
logger.info("MCP server '{}': connected, {} tools registered", name, registered_count)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -140,7 +140,7 @@ class MCPServerConfig(Base):
|
|||||||
url: str = "" # HTTP/SSE: endpoint URL
|
url: str = "" # HTTP/SSE: endpoint URL
|
||||||
headers: dict[str, str] = Field(default_factory=dict) # HTTP/SSE: custom headers
|
headers: dict[str, str] = Field(default_factory=dict) # HTTP/SSE: custom headers
|
||||||
tool_timeout: int = 30 # seconds before a tool call is cancelled
|
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_<server>_<tool> names; ["*"] = all tools; [] = no tools
|
||||||
|
|
||||||
class ToolsConfig(Base):
|
class ToolsConfig(Base):
|
||||||
"""Tools configuration."""
|
"""Tools configuration."""
|
||||||
|
|||||||
@@ -1,12 +1,15 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
from contextlib import AsyncExitStack, asynccontextmanager
|
||||||
import sys
|
import sys
|
||||||
from types import ModuleType, SimpleNamespace
|
from types import ModuleType, SimpleNamespace
|
||||||
|
|
||||||
import pytest
|
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:
|
class _FakeTextContent:
|
||||||
@@ -14,12 +17,63 @@ class _FakeTextContent:
|
|||||||
self.text = text
|
self.text = text
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def fake_mcp_runtime() -> dict[str, object | None]:
|
||||||
|
return {"session": None}
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
@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 = ModuleType("mcp")
|
||||||
mod.types = SimpleNamespace(TextContent=_FakeTextContent)
|
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)
|
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:
|
def _make_wrapper(session: object, *, timeout: float = 0.1) -> MCPToolWrapper:
|
||||||
tool_def = SimpleNamespace(
|
tool_def = SimpleNamespace(
|
||||||
@@ -97,3 +151,132 @@ async def test_execute_handles_generic_exception() -> None:
|
|||||||
result = await wrapper.execute()
|
result = await wrapper.execute()
|
||||||
|
|
||||||
assert result == "(MCP tool call failed: RuntimeError)"
|
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]
|
||||||
|
|||||||
Reference in New Issue
Block a user