diff --git a/README.md b/README.md index e2bcaff27..0b46cd3f4 100644 --- a/README.md +++ b/README.md @@ -401,6 +401,7 @@ See the [Sandbox Configuration Guide](backend/docs/CONFIGURATION.md#sandbox) to DeerFlow supports configurable MCP servers and skills to extend its capabilities. For HTTP/SSE MCP servers, OAuth token flows are supported (`client_credentials`, `refresh_token`). For stdio MCP servers, per-tool call timeouts can be configured with `tool_call_timeout`. +MCP tool names are prefixed with `_` by default to prevent collisions across servers. If a server already namespaces its own tools, set `tool_name_prefix: false` on that server in `extensions_config.json` to keep the original names. Disable the prefix only when the resulting names remain unique across all enabled servers. Settings > Tools updates one MCP server at a time: an invalid stdio command on one server no longer blocks toggling another, while enabling that invalid server remains protected by the command allowlist and surfaces the backend validation message in the UI. Targeted updates accept both DeerFlow's `type` field and the MCP-spec `transport` field for SSE/HTTP servers. Runtime MCP and skill updates replace `extensions_config.json` atomically, so an interrupted write cannot leave the shared configuration truncated or partially written. diff --git a/backend/AGENTS.md b/backend/AGENTS.md index d652a1401..1fa04f19b 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -772,6 +772,7 @@ E2B output sync records remote file versions and actual host file metadata in a - **Lazy initialization**: Tools loaded on first use via `get_cached_mcp_tools()` - **Cache invalidation**: Detects extensions-config changes by comparing the resolved config path and a `(mtime, size, sha256)` content signature against the values recorded at initialization, not a strict mtime `>` comparison. This catches same-second edits, mtime that stays put or moves backward (`git checkout`, `cp -p` / backup restore, `tar` / `rsync`, object-store / network mounts), and a switch to a different config file with an equal-or-older mtime. The signature helper (`config/file_signature.py::get_config_signature`) is shared with `config/app_config.py::get_app_config()` for the sibling runtime-editable config file, rather than each maintaining its own copy. `ExtensionsConfig.resolve_config_path()` raises `FileNotFoundError` for an explicit `config_path`/`DEER_FLOW_EXTENSIONS_CONFIG_PATH` that points at a missing file — an operator-asserted path going missing is a real misconfiguration, so this is intentionally loud for callers that load the config for actual use (e.g. `from_file()` via `get_mcp_tools()`); only the fallback search mode returns `None`. The MCP cache's own path resolution (`mcp/cache.py::_resolve_config_path`) is narrower: it catches that specific `FileNotFoundError` locally and treats it the same as "unconfigured", so this staleness check degrades to "not stale" instead of propagating an exception when a previously-valid explicit/env-var config disappears mid-run - **Transports**: stdio (command-based), SSE, HTTP +- **Per-server tool-name prefixing**: `mcpServers..tool_name_prefix` defaults to `true`, preserving the collision-safe `_` prefix. Servers whose tools already carry a stable namespace may set it to `false`; discovery then calls `langchain_mcp_adapters.tools.load_mcp_tools` with that server's flag. Source routing and stdio session-pool wrapping are based on the producing server and transport, never on whether the visible tool name starts with the server prefix. - **OAuth (HTTP/SSE)**: Supports token endpoint flows (`client_credentials`, `refresh_token`) with automatic token refresh + Authorization header injection - **Routing hints**: `extensions_config.json -> mcpServers..routing` and `tools..routing` are soft preference metadata. The effective diff --git a/backend/app/gateway/routers/mcp.py b/backend/app/gateway/routers/mcp.py index 415290ee3..91ed94e46 100644 --- a/backend/app/gateway/routers/mcp.py +++ b/backend/app/gateway/routers/mcp.py @@ -377,6 +377,7 @@ class McpServerConfigResponse(BaseModel): description: str = Field(default="", description="Human-readable description of what this MCP server provides") routing: McpRoutingConfig = Field(default_factory=McpRoutingConfig, description="Soft routing hints for tools from this MCP server") tools: dict[str, McpToolOverride] = Field(default_factory=dict, description="Per-original-tool MCP configuration overrides") + tool_name_prefix: bool = Field(default=True, description="Whether to prefix discovered tool names with the MCP server name") tool_call_timeout: float | None = Field(default=None, description="Timeout in seconds for individual stdio MCP tool calls") model_config = ConfigDict(extra="allow") diff --git a/backend/docs/MCP_SERVER.md b/backend/docs/MCP_SERVER.md index cc04597e8..d91028884 100644 --- a/backend/docs/MCP_SERVER.md +++ b/backend/docs/MCP_SERVER.md @@ -70,6 +70,32 @@ top-level `config.yaml -> tool_search.auto_promote_top_k` setting. - `tool_search.auto_promote_top_k`: global limit for auto-promoted deferred MCP schemas per model call. Default `3`; valid range `1..5`. +## Tool Name Prefixes + +DeerFlow prefixes discovered MCP tool names with `_` by default. +This avoids collisions when two enabled servers expose tools with the same +name. A server that already namespaces its own tools can opt out: + +```json +{ + "mcpServers": { + "semantic-scholar": { + "type": "stdio", + "command": "uvx", + "args": ["s2-mcp-server"], + "tool_name_prefix": false + } + } +} +``` + +With this setting, a server tool named `semantic_scholar_search_papers` keeps +that name instead of becoming +`semantic-scholar_semantic_scholar_search_papers`. The default is `true` for +backward compatibility. Disable it only when every resulting tool name remains +unique across the enabled servers. Stdio tools continue to use DeerFlow's +persistent per-thread session pool regardless of this setting. + ## Per-Tool Timeout (Stdio MCP Servers) For `stdio` MCP servers, set `tool_call_timeout` to limit each individual MCP tool call in seconds: diff --git a/backend/packages/harness/deerflow/config/extensions_config.py b/backend/packages/harness/deerflow/config/extensions_config.py index 05c237a31..5d6f0b715 100644 --- a/backend/packages/harness/deerflow/config/extensions_config.py +++ b/backend/packages/harness/deerflow/config/extensions_config.py @@ -98,6 +98,10 @@ class McpServerConfig(BaseModel): description: str = Field(default="", description="Human-readable description of what this MCP server provides") routing: McpRoutingConfig = Field(default_factory=McpRoutingConfig, description="Soft routing hints for tools from this MCP server") tools: dict[str, McpToolOverride] = Field(default_factory=dict, description="Per-original-tool MCP configuration overrides") + tool_name_prefix: bool = Field( + default=True, + description="Whether to prefix discovered tool names with the MCP server name to avoid cross-server collisions", + ) tool_call_timeout: float | None = Field( default=None, description="Timeout in seconds for individual stdio MCP tool calls. HTTP/SSE servers use transport-level timeouts. None means no timeout.", diff --git a/backend/packages/harness/deerflow/mcp/tools.py b/backend/packages/harness/deerflow/mcp/tools.py index 0072616e4..ee04a51b4 100644 --- a/backend/packages/harness/deerflow/mcp/tools.py +++ b/backend/packages/harness/deerflow/mcp/tools.py @@ -431,6 +431,7 @@ def _make_session_pool_tool( connection: dict[str, Any], tool_interceptors: list[Any] | None = None, tool_call_timeout: float | None = None, + tool_name_prefix: bool = True, ) -> BaseTool: """Wrap an MCP tool so it reuses a persistent session from the pool. @@ -442,10 +443,11 @@ def _make_session_pool_tool( The configured ``tool_interceptors`` (OAuth, custom) are preserved and applied on every call before invoking the pooled session. """ - # Strip the server-name prefix to recover the original MCP tool name. + # Strip only prefixes added by the adapter. An unprefixed server may expose + # a tool whose own name happens to start with ``_``. original_name = tool.name prefix = f"{server_name}_" - if original_name.startswith(prefix): + if tool_name_prefix and original_name.startswith(prefix): original_name = original_name[len(prefix) :] pool = get_session_pool() @@ -582,6 +584,7 @@ async def get_mcp_tools() -> list[BaseTool]: """ try: from langchain_mcp_adapters.client import MultiServerMCPClient + from langchain_mcp_adapters.tools import load_mcp_tools except ImportError: logger.warning("langchain-mcp-adapters not installed. Install it to enable MCP tools: pip install langchain-mcp-adapters") return [] @@ -648,7 +651,18 @@ async def get_mcp_tools() -> list[BaseTool]: async def load_server_tools(server_name: str) -> list[BaseTool]: try: - return await client.get_tools(server_name=server_name) + server_cfg = extensions_config.mcp_servers.get(server_name) + tool_name_prefix = server_cfg.tool_name_prefix if server_cfg is not None else True + if tool_name_prefix: + return await client.get_tools(server_name=server_name) + return await load_mcp_tools( + None, + connection=servers_config[server_name], + callbacks=client.callbacks, + server_name=server_name, + tool_interceptors=client.tool_interceptors, + tool_name_prefix=False, + ) except Exception as e: logger.warning( f"Skipping MCP server '{server_name}' after tool discovery failed: {e}", @@ -672,11 +686,11 @@ async def get_mcp_tools() -> list[BaseTool]: # scanning servers_config for a name prefix is ambiguous when one server name is a # prefix of another (e.g. "web" vs "web_scraper" → "web_scraper_search".startswith( # "web_") matches "web" first), which pools the tool under the wrong server. Using the - # source grouping makes routing exact; the prefix guard preserves the previous - # behavior of leaving unprefixed tools unwrapped. + # source grouping makes routing exact even when a server opts out of name prefixing. for source_name, server_tools in zip(servers_config.keys(), tools_by_server, strict=True): transport = servers_config[source_name].get("transport", "stdio") server_cfg = extensions_config.mcp_servers.get(source_name) + tool_name_prefix = server_cfg.tool_name_prefix if server_cfg is not None else True for tool in server_tools: if not _VALID_MCP_TOOL_NAME.fullmatch(tool.name or ""): logger.warning( @@ -688,13 +702,22 @@ async def get_mcp_tools() -> list[BaseTool]: continue tag_mcp_tool(tool) prefix = f"{source_name}_" - original_name = tool.name[len(prefix) :] if tool.name.startswith(prefix) else tool.name + original_name = tool.name[len(prefix) :] if tool_name_prefix and tool.name.startswith(prefix) else tool.name routing = resolve_effective_mcp_routing(server_cfg, original_name) if routing.get("mode") != "off": tag_mcp_routing(tool, routing) - if tool.name.startswith(f"{source_name}_") and transport == "stdio": + if transport == "stdio": _timeout = server_cfg.tool_call_timeout if server_cfg else None - wrapped_tools.append(_make_session_pool_tool(tool, source_name, servers_config[source_name], tool_interceptors, tool_call_timeout=_timeout)) + wrapped_tools.append( + _make_session_pool_tool( + tool, + source_name, + servers_config[source_name], + tool_interceptors, + tool_call_timeout=_timeout, + tool_name_prefix=tool_name_prefix, + ) + ) else: if transport != "stdio" and server_cfg and server_cfg.tool_call_timeout is not None: logger.warning( diff --git a/backend/tests/test_mcp_session_pool.py b/backend/tests/test_mcp_session_pool.py index b45846346..f19882d93 100644 --- a/backend/tests/test_mcp_session_pool.py +++ b/backend/tests/test_mcp_session_pool.py @@ -1730,7 +1730,7 @@ async def test_mcp_tools_routed_to_source_server_with_prefix_overlap(): routed: list[tuple[str, str]] = [] - def fake_wrap(tool, server_name, connection, interceptors, tool_call_timeout=None): + def fake_wrap(tool, server_name, connection, interceptors, tool_call_timeout=None, tool_name_prefix=True): routed.append((tool.name, server_name)) return tool diff --git a/backend/tests/test_mcp_sync_wrapper.py b/backend/tests/test_mcp_sync_wrapper.py index 3e909594b..b5f253b2f 100644 --- a/backend/tests/test_mcp_sync_wrapper.py +++ b/backend/tests/test_mcp_sync_wrapper.py @@ -36,7 +36,15 @@ def test_mcp_tool_sync_wrapper_generation(): with ( patch("langchain_mcp_adapters.client.MultiServerMCPClient", return_value=mock_client_instance), patch("deerflow.config.extensions_config.ExtensionsConfig.from_file"), - patch("deerflow.mcp.tools.build_servers_config", return_value={"test-server": {}}), + patch( + "deerflow.mcp.tools.build_servers_config", + return_value={ + "test-server": { + "transport": "http", + "url": "https://example.test/mcp", + } + }, + ), patch("deerflow.mcp.tools.get_initial_oauth_headers", new_callable=AsyncMock, return_value={}), ): # Run the async function manually with asyncio.run diff --git a/backend/tests/test_mcp_tool_name_prefix.py b/backend/tests/test_mcp_tool_name_prefix.py new file mode 100644 index 000000000..30319192f --- /dev/null +++ b/backend/tests/test_mcp_tool_name_prefix.py @@ -0,0 +1,165 @@ +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from langchain_core.tools import StructuredTool +from pydantic import BaseModel, Field + +from app.gateway.routers.mcp import McpServerConfigResponse +from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig +from deerflow.mcp.tools import _make_session_pool_tool, get_mcp_tools +from deerflow.tools.mcp_metadata import get_mcp_routing + + +class _Args(BaseModel): + query: str = Field(..., description="query") + + +def _tool(name: str) -> StructuredTool: + async def _call(query: str) -> str: + return query + + return StructuredTool( + name=name, + description="Search", + args_schema=_Args, + coroutine=_call, + ) + + +def test_mcp_tool_name_prefix_is_an_explicit_default_true_field() -> None: + assert "tool_name_prefix" in McpServerConfig.model_fields + assert McpServerConfig().tool_name_prefix is True + assert McpServerConfig(tool_name_prefix=False).model_dump()["tool_name_prefix"] is False + + +def test_gateway_mcp_config_preserves_tool_name_prefix() -> None: + response = McpServerConfigResponse.model_validate(McpServerConfig(tool_name_prefix=False).model_dump()) + + assert response.tool_name_prefix is False + + +@pytest.mark.asyncio +async def test_mcp_tool_name_prefix_can_be_disabled_per_server_without_disabling_stdio_pooling() -> None: + extensions_config = ExtensionsConfig.model_validate( + { + "mcpServers": { + "semantic_scholar": { + "type": "stdio", + "command": "uvx", + "args": ["s2-mcp-server"], + "tool_name_prefix": False, + "routing": {"mode": "prefer", "priority": 10}, + "tools": { + "semantic_scholar_search_papers": { + "routing": {"priority": 99}, + } + }, + }, + "github": { + "type": "http", + "url": "https://example.test/mcp", + }, + } + } + ) + servers_config = { + "semantic_scholar": { + "transport": "stdio", + "command": "uvx", + "args": ["s2-mcp-server"], + }, + "github": { + "transport": "http", + "url": "https://example.test/mcp", + }, + } + raw_tools = { + "semantic_scholar": _tool("semantic_scholar_search_papers"), + "github": _tool("search_repositories"), + } + + class FakeClient: + def __init__( + self, + connections, + *, + callbacks=None, + tool_interceptors=None, + tool_name_prefix=False, + ) -> None: + self.connections = connections + self.callbacks = callbacks + self.tool_interceptors = tool_interceptors or [] + self.tool_name_prefix = tool_name_prefix + + async def get_tools(self, *, server_name=None): + tool = raw_tools[server_name] + name = f"{server_name}_{tool.name}" if self.tool_name_prefix else tool.name + return [_tool(name)] + + async def fake_load_mcp_tools( + session, + *, + connection, + callbacks=None, + tool_interceptors=None, + server_name=None, + tool_name_prefix=False, + ): + assert session is None + assert connection is servers_config[server_name] + tool = raw_tools[server_name] + name = f"{server_name}_{tool.name}" if tool_name_prefix else tool.name + return [_tool(name)] + + with ( + patch("deerflow.mcp.tools.ExtensionsConfig.from_file", return_value=extensions_config), + patch("deerflow.mcp.tools.build_servers_config", return_value=servers_config), + patch("deerflow.mcp.tools.get_initial_oauth_headers", new_callable=AsyncMock, return_value={}), + patch("deerflow.mcp.tools.build_oauth_tool_interceptor", return_value=None), + patch("langchain_mcp_adapters.client.MultiServerMCPClient", FakeClient), + patch("langchain_mcp_adapters.tools.load_mcp_tools", side_effect=fake_load_mcp_tools), + patch("deerflow.mcp.tools._make_session_pool_tool", side_effect=lambda tool, *_args, **_kwargs: tool) as wrap_tool, + ): + tools = await get_mcp_tools() + + assert {tool.name for tool in tools} == { + "semantic_scholar_search_papers", + "github_search_repositories", + } + semantic_scholar_tool = next(tool for tool in tools if tool.name == "semantic_scholar_search_papers") + routing = get_mcp_routing(semantic_scholar_tool) + assert routing is not None + assert routing["priority"] == 99 + wrap_tool.assert_called_once() + assert wrap_tool.call_args.args[1] == "semantic_scholar" + assert wrap_tool.call_args.kwargs["tool_name_prefix"] is False + + +@pytest.mark.asyncio +async def test_unprefixed_stdio_tool_keeps_server_like_original_name(tmp_path: Path) -> None: + """Pooling must not strip a prefix that belongs to the MCP tool itself.""" + original_tool = _tool("github_search") + mock_session = AsyncMock() + mock_session.call_tool = AsyncMock(return_value=MagicMock(content=[], isError=False, structuredContent=None)) + mock_pool = MagicMock() + mock_pool.get_session = AsyncMock(return_value=mock_session) + + with ( + patch("deerflow.mcp.tools.get_session_pool", return_value=mock_pool), + patch("deerflow.mcp.tools.get_paths", return_value=MagicMock()), + patch( + "deerflow.mcp.tools._prepare_stdio_workspace", + return_value=(tmp_path, tmp_path / "tmp", {}), + ), + ): + wrapped = _make_session_pool_tool( + original_tool, + "github", + {"transport": "stdio", "command": "mcp-server", "args": []}, + tool_name_prefix=False, + ) + await wrapped.coroutine(query="repositories") + + mock_session.call_tool.assert_awaited_once_with("github_search", {"query": "repositories"}) diff --git a/extensions_config.example.json b/extensions_config.example.json index 53f4e92d2..08c8b7f5b 100644 --- a/extensions_config.example.json +++ b/extensions_config.example.json @@ -15,6 +15,7 @@ "env": { "GITHUB_TOKEN": "$GITHUB_TOKEN" }, + "tool_name_prefix": true, "tool_call_timeout": 60, "description": "GitHub MCP server for repository operations" },