From 6bc48a860d78c4d184ea750401cab13aed421a33 Mon Sep 17 00:00:00 2001 From: Arterialist Date: Wed, 5 Aug 2026 18:51:13 +0800 Subject: [PATCH] Sanitize MCP tool names and release 2.0.1 --- CHANGELOG.md | 5 ++ README.md | 7 +- mcphero/adapters/base_adapter.py | 94 ++++++++++++++---------- pyproject.toml | 2 +- tests/test_base_adapter.py | 119 +++++++++++++++++++++++++------ tests/test_openai_adapter.py | 13 ++++ 6 files changed, 179 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 891b794..3ce1635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [2.0.1] - 2026-08-05 +### Changed +- Normalize LLM-facing tool names to the OpenRouter-compatible character set, first-character rule, and 64-character limit while preserving original MCP names for calls. +- Always namespace colliding server/tool names; removed the unsafe collision-prefix opt-out. + ## [2.0.0] - 2026-03-14 ### Added - Generic MCP Adapter with no dependencies on openai/gemini/etc. diff --git a/README.md b/README.md index b215fbd..5dcc853 100644 --- a/README.md +++ b/README.md @@ -254,6 +254,8 @@ tools = await adapter.get_tool_definitions() # "search" becomes "weather__search" and "calendar__search" ``` +Tool names exposed to the LLM are normalized by default. Names use the OpenRouter-compatible ASCII letters, digits, underscores, and hyphens; they start with a letter or underscore, are limited to 64 characters, and are made unique after normalization. The original MCP name is retained for the `tools/call` request. + You can control this behavior: ```python @@ -261,14 +263,13 @@ You can control this behavior: adapter = MCPToolAdapterOpenAI(configs, prefix_separator="-") # "weather-search", "calendar-search" -# Disable auto-prefixing (will raise on collision) -adapter = MCPToolAdapterOpenAI(configs, auto_prefix_on_collision=False) - # Manual prefix via config (always applied, regardless of collisions) MCPServerConfig(url="...", tool_prefix="wx") # "wx__search" ``` +Collision prefixing is always enabled so the LLM never receives duplicate function names. There is no opt-out because duplicate provider names cannot be routed safely. + ## API Reference ### MCPToolAdapter diff --git a/mcphero/adapters/base_adapter.py b/mcphero/adapters/base_adapter.py index 6a11a18..b89559c 100644 --- a/mcphero/adapters/base_adapter.py +++ b/mcphero/adapters/base_adapter.py @@ -7,6 +7,7 @@ import asyncio import json +import re import uuid from dataclasses import dataclass, field from enum import Enum @@ -89,7 +90,7 @@ def __post_init__(self): class MCPToolDefinition: """A discovered MCP tool with routing information.""" - name: str # The name exposed to the LLM (possibly prefixed) + name: str # The provider-safe name exposed to the LLM (possibly prefixed) original_name: str # The original name on the MCP server server_name: str # Which server this tool belongs to description: str @@ -109,6 +110,35 @@ class ToolMapping: connection: MCPConnection +_TOOL_NAME_MAX_LENGTH = 64 +_INVALID_TOOL_NAME_CHARS = re.compile(r"[^a-zA-Z0-9_-]") + + +def _sanitize_tool_name(name: str) -> str: + """Return a provider-safe name while keeping it MCP-compatible.""" + sanitized = _INVALID_TOOL_NAME_CHARS.sub("_", name) + if not sanitized: + sanitized = "_mcp_tool" + if not (sanitized[0].isalpha() or sanitized[0] == "_"): + sanitized = f"_{sanitized}" + return sanitized[:_TOOL_NAME_MAX_LENGTH] + + +def _unique_tool_name(name: str, used_names: set[str]) -> str: + """Make a sanitized name unique without exceeding the provider limit.""" + base_name = _sanitize_tool_name(name) + candidate = base_name + suffix = 2 + while candidate in used_names: + suffix_text = f"_{suffix}" + candidate = ( + f"{base_name[: _TOOL_NAME_MAX_LENGTH - len(suffix_text)]}{suffix_text}" + ) + suffix += 1 + used_names.add(candidate) + return candidate + + class MCPConnection: """ Single MCP server connection handler. @@ -262,7 +292,6 @@ class BaseAdapter(Generic[ToolCallT, ResultT]): Args: servers: Single URL/config or list of URLs/configs. - auto_prefix_on_collision: Auto-prefix tools when names collide across servers. prefix_separator: Separator for prefixed names (default: "__"). """ @@ -270,7 +299,6 @@ def __init__( self, servers: str | MCPServerConfig | list[str | MCPServerConfig], *, - auto_prefix_on_collision: bool = True, prefix_separator: str = "__", ): if isinstance(servers, (str, MCPServerConfig)): @@ -279,7 +307,6 @@ def __init__( self._configs: list[MCPServerConfig] = [ MCPServerConfig(url=s) if isinstance(s, str) else s for s in servers ] - self.auto_prefix_on_collision = auto_prefix_on_collision self.prefix_separator = prefix_separator self._connections: dict[str, MCPConnection] = {} @@ -297,11 +324,6 @@ def _prepare_connections(self): counter = 1 while unique_name in seen_names: - if not self.auto_prefix_on_collision: - raise ValueError( - f"Duplicate server name '{base_name}'. " - "Provide unique names explicitly or enable auto_prefix_on_collision." - ) counter += 1 unique_name = f"{base_name}{self.prefix_separator}{counter}" @@ -330,52 +352,52 @@ def _make_prefixed_name(self, prefix: str | None, tool_name: str) -> str: def _resolve_tools( self, tools_by_server: dict[str, list[RawMCPTool]] ) -> list[MCPToolDefinition]: - """Resolve naming collisions and build typed tool definitions.""" - # Find collisions - tool_sources: dict[str, list[str]] = {} + """Resolve naming collisions and build provider-safe tool definitions.""" + tool_sources: dict[str, set[str]] = {} + for server_name, tools in tools_by_server.items(): config = next(c for c in self._configs if c.name == server_name) for tool in tools: - name = self._make_prefixed_name(config.tool_prefix, tool["name"]) - tool_sources.setdefault(name, []).append(server_name) + base_name = self._make_prefixed_name(config.tool_prefix, tool["name"]) + tool_sources.setdefault(base_name, set()).add(server_name) - collisions = { + colliding_names = { name for name, sources in tool_sources.items() if len(sources) > 1 } - # Build definitions definitions: list[MCPToolDefinition] = [] self._tool_map.clear() + used_names: set[str] = set() for server_name, tools in tools_by_server.items(): config = next(c for c in self._configs if c.name == server_name) - conn = self._connections[server_name] - for tool in tools: base_name = self._make_prefixed_name(config.tool_prefix, tool["name"]) - - if base_name in collisions and self.auto_prefix_on_collision: - final_name = f"{server_name}{self.prefix_separator}{tool['name']}" - else: - final_name = base_name - - definition = MCPToolDefinition( - name=final_name, - original_name=tool["name"], - server_name=server_name, - description=tool.get("description", ""), - input_schema=tool.get( - "inputSchema", {"type": "object", "properties": {}} - ), - raw=tool, + candidate_name = ( + self._make_prefixed_name(server_name, tool["name"]) + if base_name in colliding_names + else base_name + ) + final_name = _unique_tool_name(candidate_name, used_names) + + definitions.append( + MCPToolDefinition( + name=final_name, + original_name=tool["name"], + server_name=server_name, + description=tool.get("description", ""), + input_schema=tool.get( + "inputSchema", {"type": "object", "properties": {}} + ), + raw=tool, + ) ) - definitions.append(definition) self._tool_map[final_name] = ToolMapping( server_name=server_name, original_name=tool["name"], prefixed_name=final_name, - connection=conn, + connection=self._connections[server_name], ) return definitions @@ -402,7 +424,7 @@ async def fetch_one(name: str) -> tuple[str, list[RawMCPTool]]: name, tools = result tools_by_server[name] = tools else: - tools_by_server = {} + tools_by_server: dict[str, list[RawMCPTool]] = {} for name, conn in self._connections.items(): try: tools_by_server[name] = await conn.get_tools() diff --git a/pyproject.toml b/pyproject.toml index a1cff73..872b0bd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "mcphero" -version = "2.0.0" +version = "2.0.1" description = "Library to use MCP servers natively with AI clients as tools." readme = "README.md" requires-python = ">=3.11" diff --git a/tests/test_base_adapter.py b/tests/test_base_adapter.py index b30d179..e05d3f2 100644 --- a/tests/test_base_adapter.py +++ b/tests/test_base_adapter.py @@ -605,6 +605,20 @@ def test_connections_created_per_server(self): assert "weather" in adapter._connections assert "calendar" in adapter._connections + def test_duplicate_server_names_are_always_namespaced(self): + adapter = BaseAdapter( + [ + MCPServerConfig(url="https://a.com/weather", name="weather"), + MCPServerConfig(url="https://b.com/calendar", name="weather"), + ] + ) + + assert set(adapter._connections) == {"weather", "weather__2"} + assert [config.name for config in adapter._configs] == [ + "weather", + "weather__2", + ] + def test_tools_raises_before_discovery(self, base_url): adapter = BaseAdapter(base_url) with pytest.raises(RuntimeError, match="Must call discover_tools"): @@ -698,38 +712,101 @@ async def test_multi_server_collision_auto_prefix(self): assert names == {"weather__search", "calendar__search"} @respx.mock - async def test_multi_server_collision_disabled(self): - url_a = "https://a.com/mcp/weather" - url_b = "https://b.com/mcp/calendar" - - respx.post(url_a).mock( + async def test_sanitizes_tool_names(self): + url = "https://a.com/mcp/weather" + raw_names = [ + "-search", + "name with spaces", + "admin.tools.list", + "x/y", + "a" * 140, + "a/b", + "a_b", + ] + respx.post(url).mock( return_value=httpx.Response( 200, - json=_tools_response( - [{"name": "search", "description": "Search weather"}] - ), + json=_tools_response([{"name": name} for name in raw_names]), ) ) - respx.post(url_b).mock( - return_value=httpx.Response( - 200, - json=_tools_response( - [{"name": "search", "description": "Search events"}] - ), - ) + + adapter = BaseAdapter( + MCPServerConfig(url=url, name="weather", init_mode="none") ) + tools = await adapter.discover_tools() + + assert [tool.name for tool in tools] == [ + "_-search", + "name_with_spaces", + "admin_tools_list", + "x_y", + "a" * 64, + "a_b", + "a_b_2", + ] + assert [tool.original_name for tool in tools] == raw_names + + @respx.mock + async def test_prefix_collisions_get_unique_names(self): + url_a = "https://a.com/mcp/weather" + url_b = "https://b.com/mcp/calendar" + url_c = "https://c.com/mcp/other" + for url, tool_name in [ + (url_a, "search"), + (url_b, "search"), + (url_c, "weather__search"), + ]: + respx.post(url).mock( + return_value=httpx.Response( + 200, json=_tools_response([{"name": tool_name}]) + ) + ) adapter = BaseAdapter( [ MCPServerConfig(url=url_a, name="weather", init_mode="none"), MCPServerConfig(url=url_b, name="calendar", init_mode="none"), - ], - auto_prefix_on_collision=False, + MCPServerConfig(url=url_c, name="other", init_mode="none"), + ] ) - tools = await adapter.discover_tools() + tools = await adapter.discover_tools(parallel=False) + + assert [tool.name for tool in tools] == [ + "weather__search", + "calendar__search", + "weather__search_2", + ] + assert [tool.original_name for tool in tools] == [ + "search", + "search", + "weather__search", + ] + + @respx.mock + async def test_sanitized_name_routes_to_original_mcp_name(self): + url = "https://a.com/mcp/weather" + route = respx.post(url).mock( + side_effect=[ + httpx.Response( + 200, + json=_tools_response([{"name": "name with spaces"}]), + ), + httpx.Response( + 200, + json={"jsonrpc": "2.0", "id": "2", "result": {"ok": True}}, + ), + ] + ) + + adapter = BaseAdapter( + MCPServerConfig(url=url, name="weather", init_mode="none") + ) + await adapter.discover_tools() + result = await adapter.call_tool("name_with_spaces", {}) - names = [t.name for t in tools] - assert names.count("search") == 2 + assert result["result"]["ok"] is True + payload = json.loads(route.calls[1].request.content) + assert payload["params"]["name"] == "name with spaces" @respx.mock async def test_tool_prefix(self): @@ -767,7 +844,7 @@ async def test_custom_prefix_separator(self): ) tools = await adapter.discover_tools() - assert tools[0].name == "wx.search" + assert tools[0].name == "wx_search" # ── BaseAdapter: call_tool ─────────────────────────────────────── diff --git a/tests/test_openai_adapter.py b/tests/test_openai_adapter.py index 076a7ba..75b8eec 100644 --- a/tests/test_openai_adapter.py +++ b/tests/test_openai_adapter.py @@ -81,6 +81,19 @@ async def test_handles_empty_tools(self, base_url): tools = await adapter.get_tool_definitions() assert tools == [] + @respx.mock + async def test_sanitizes_provider_unsafe_name(self, base_url): + respx.post(base_url).mock( + return_value=httpx.Response( + 200, json=_tools_response([{"name": "name with spaces"}]) + ) + ) + + adapter = MCPToolAdapterOpenAI(MCPServerConfig(url=base_url, init_mode="none")) + tools = await adapter.get_tool_definitions() + + assert tools[0]["function"]["name"] == "name_with_spaces" + class TestProcessToolCalls: @respx.mock