fix(#2405): prefix MCP tool names with mcp__{server}__ for proper tool filtering [FaaFyfxR9WAQrL7FcAgEHJvztd8cVMxvjHRS55rw1nwH] - #2428
Conversation
…or proper tool filtering [FaaFyfxR9WAQrL7FcAgEHJvztd8cVMxvjHRS55rw1nwH]
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
不能这样改工具的名字,因为调用mcpServer时也将使用你修改后的工具名字,这样的话,mcpSever将报错:undefinedError: MCP tool error: Unknown tool: invalid_tool_name |
|
CI Failure — Spotless formatting Build fails on Fix: run |
oss-maintainer
left a comment
There was a problem hiding this comment.
CI Failure: Spotless Formatting
The build fails due to a Spotless formatting violation in McpClientManager.java:
- mcpTool.name(),
+ mcpClientWrapper.getName() + "__" + mcpTool.name(),The concatenated string exceeds the line length limit. Please run:
mvn spotless:applyto auto-format, then push the fix.
Code Review
The approach of prefixing MCP tool names with mcp__{server}__ is sound — it provides clear namespacing for tool filtering. A few observations:
-
Breaking change for existing users: Tool names change format. Users relying on the original tool name (e.g., in
PermissionContextStaterules) will need to update their configurations. Consider documenting this in the PR description or migration notes. -
Separator choice: Using
__(double underscore) is good — unlikely to collide with real tool names. -
Test coverage: Please verify that the tool name prefix is correctly handled by
PermissionContextStaterule matching, especially the scenario described in #2454 whereaddAskRuleis used with MCP tools.
oss-maintainer
left a comment
There was a problem hiding this comment.
CI Failure: Spotless Formatting
The build fails due to a Spotless formatting violation in McpClientManager.java. The concatenated string for the tool name prefix exceeds the line length limit. Please run:
mvn spotless:applyto auto-format, then push the fix.
Code Review
The approach of prefixing MCP tool names with mcp__{server}__ is sound for namespacing. A few notes:
-
Breaking change: Tool names change format. Users relying on original names (e.g. in
PermissionContextStaterules) need to update configs. Consider documenting this. -
Test coverage: Verify the prefix is correctly handled by
PermissionContextStaterule matching, especially the scenario in #2454 whereaddAskRuleis used with MCP tools.

Closes #2405
Summary
When registering custom MCP servers, the tools inside them were not renamed to
mcp__{server_name}__{tool_name}format as documented in https://java.agentscope.io/v2/zh/docs/building-blocks/tool.html#mcp. This caused theToolsConfig.allowfilter to be unable to properly include MCP server tools.Root Cause
In
McpClientManager.registerMcpClient(), theMcpToolwas created with the original tool name from the MCP server (e.g.,getTime,listHoliday), without themcp__{server_name}__prefix that the documentation specifies.Fix
Changed
McpClientManager.javaline 177 to prefix the tool name with"mcp__" + mcpClientWrapper.getName() + "__":The
enableToolsfiltering inMcpServerConfigstill works correctly because it filters by the original MCP tool name (before prefixing), and theToolFilterreads the final prefixed tool name from the toolkit.Testing
enableToolsfromMcpServerConfigstill filters by original tool names correctlyToolsConfig.allow/denynow works with the prefixed namesMcpClientManagerTestremain unaffected (they don't assert tool names directly)