fix: prevent MCP tool name collision by prefixing with server name#9104
fix: prevent MCP tool name collision by prefixing with server name#91042722550596 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider centralizing the logic for constructing the prefixed tool name (e.g., a helper like
get_prefixed_tool_name(server_name, tool_name)) so the prefix format stays consistent if you need to modify it later. - Using
:as a separator between server and tool names may be brittle if either component can contain:; you might want to either validate/disallow:in names or choose a safer delimiter/encoding scheme.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the logic for constructing the prefixed tool name (e.g., a helper like `get_prefixed_tool_name(server_name, tool_name)`) so the prefix format stays consistent if you need to modify it later.
- Using `:` as a separator between server and tool names may be brittle if either component can contain `:`; you might want to either validate/disallow `:` in names or choose a safer delimiter/encoding scheme.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request prefixes MCP tool names with their server name using a colon separator in both the client registration and the dashboard tools service. The reviewer identified a critical issue where using a colon in tool names violates the strict naming constraints of major LLM APIs, which can lead to API errors. They suggested refactoring this logic to use a shared helper function that cleans and formats the tool names safely to ensure compatibility and avoid code duplication.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ) -> None: | ||
| super().__init__( | ||
| name=mcp_tool.name, | ||
| name=f"{mcp_server_name}:{mcp_tool.name}", |
There was a problem hiding this comment.
问题分析
在大多数主流的大语言模型(LLM)API(如 OpenAI、Anthropic、Gemini、Groq 等)中,工具/函数名称(name)有非常严格的命名规范限制:
- 字符限制:通常只允许字母、数字、下划线和连字符,即满足正则表达式
^[a-zA-Z0-9_-]+$。冒号:是不被允许的非法字符。 - 长度限制:通常最大长度为 64 个字符。
如果直接使用 f"{mcp_server_name}:{mcp_tool.name}" 作为工具名称,包含冒号 : 会直接导致 LLM API 返回 400 Bad Request 错误。
解决方案
建议使用双下划线 __ 作为命名空间分隔符,并对服务器名称和工具名称进行清洗(将所有非 [a-zA-Z0-9_-] 的字符替换为下划线 _),同时限制总长度不超过 64 字符。
为了避免在 mcp_client.py 和 tools_service.py 中重复编写相同的清洗逻辑,应当将此清洗逻辑重构为一个公共的辅助函数(例如 clean_tool_name),以避免代码重复。
| name=f"{mcp_server_name}:{mcp_tool.name}", | |
| name=clean_tool_name(mcp_server_name, mcp_tool.name), |
References
- When implementing similar functionality for different cases, refactor the logic into a shared helper function to avoid code duplication.
| if name_key == name: | ||
| mcp_client = runtime.client | ||
| server_info["tools"] = [tool.name for tool in mcp_client.tools] | ||
| server_info["tools"] = [f"{name}:{tool.name}" for tool in mcp_client.tools] |
There was a problem hiding this comment.
为了与 astrbot/core/agent/mcp_client.py 中的工具注册名称保持一致,并避免冒号 : 导致 LLM API 报错,这里也需要采用相同的清洗逻辑。为了避免代码重复,建议调用公共的辅助函数 clean_tool_name,而不是在此处重复编写相同的正则表达式清洗逻辑。
| server_info["tools"] = [f"{name}:{tool.name}" for tool in mcp_client.tools] | |
| server_info["tools"] = [ | |
| clean_tool_name(name, tool.name) | |
| for tool in mcp_client.tools | |
| ] |
References
- When implementing similar functionality for different cases, refactor the logic into a shared helper function to avoid code duplication.
When multiple MCP servers expose tools with the same name (e.g., `read_memory`), selecting one would inadvertently select the other because both backend registration and the frontend-facing API used the raw tool name without any server identifier. Changes: - Added `clean_tool_name()` helper: sanitizes server/tool names to [a-zA-Z0-9_-], uses __ as separator, truncates to 64 chars max - MCPTool.__init__: uses clean_tool_name() for tool registration - ToolsService.get_mcp_servers: uses clean_tool_name() for API return The actual MCP call still uses the original tool name (self.mcp_tool.name), so server communication is unaffected.
343ef32 to
f059ca1
Compare
问题
当添加多个 MCP 服务器时,如果不同服务器暴露了同名工具(如
read_memory),前端选中一个会意外地把另一个也选中。原因是后端注册和前端 API 都只用了原生的工具名,没有加服务器标识。修改
astrbot/core/agent/mcp_client.py— MCPTool 注册名加上服务器前缀:astrbot/dashboard/services/tools_service.py— MCP 服务器列表 API 也返回前缀名:实际调用 MCP server 时仍使用原始工具名(
self.mcp_tool.name),不影响通信。Summary by Sourcery
Bug Fixes: