fix: add namespace prefix to MCP tools to avoid conflicts with plugin tools#6007
Open
ChuwuYo wants to merge 8 commits intoAstrBotDevs:masterfrom
Open
fix: add namespace prefix to MCP tools to avoid conflicts with plugin tools#6007ChuwuYo wants to merge 8 commits intoAstrBotDevs:masterfrom
ChuwuYo wants to merge 8 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
Bot
added
size:M
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -446,11 +446,63 @@
- `on_task_canceled`：任务被取消时触发
- `on_task_result_ignored`：任务结果被忽略时触发（如状态已变化）
+#### MCP 工具命名空间前缀（PR #6007）
+
+[PR #6007](https://github.com/AstrBotDevs/AstrBot/pull/6007) 修复了 MCP 工具与插件工具同名时的冲突问题，通过为 MCP 工具添加命名空间前缀，确保两者可以同时启用。
+
+**命名空间前缀机制：**
+
+- **内部名称格式**：MCP 工具现在使用格式 `mcp_<server_name>__<tool_name>`（双下划线）作为内部唯一标识符
+- **服务器名称规范化**：服务器名称中的空格和特殊字符会被替换为下划线，仅保留字母、数字和下划线
+- **原始名称保留**：MCP 工具通过 `original_tool_name` 属性存储原始工具名称，用于调用 MCP 服务器和显示
+- **用户友好显示**：Dashboard UI 通过 `display_name` 字段显示格式化的工具名称（格式：`<ServerName>_<ToolName>`），用户看到的是友好名称而非内部的命名空间前缀名称
+
+**向后兼容性：**
+
+- **Persona 配置兼容**：旧的 Persona 配置引用 MCP 工具时可以继续使用原始工具名称（无前缀），系统会自动回退查找
+- **查找逻辑**：`ToolSet.get_func()` 方法包含回退机制：
+ 1. 优先按完整名称（带前缀）精确匹配
+ 2. 如果未找到，对 MCP 工具按 `original_tool_name` 回退查找
+ 3. 如果多个 MCP 服务器提供相同原始名称的工具，会记录警告并返回第一个匹配项
+- **无需立即更新**：现有配置无需立即修改，但建议新配置使用带前缀的完整名称以避免歧义
+
+**Dashboard UI 更新：**
+
+以下 UI 组件已更新为显示 `display_name` 字段，确保用户看到友好的工具名称：
+
+- **ToolTable 组件**：函数工具管理表格中显示 `display_name`
+- **PersonaForm 组件**：人格编辑表单的工具选择列表和已选工具标签使用 `display_name`
+- **PersonaQuickPreview 组件**：人格预览中显示 `display_name`
+- **PersonaManager 组件**：人格详情页面使用 `display_name` 显示工具名称
+
+**API 更新：**
+
+- **工具列表 API**（`/api/tools/list`）：返回结果中包含 `display_name` 字段，用于前端显示
+- **MCP 服务器详情 API**：MCP 服务器信息中的工具列表显示去除前缀的原始工具名称
+
+**技术实现：**
+
+- **MCPTool 类改动**（`astrbot/core/agent/mcp_client.py`）：
+ - 工具名称添加命名空间前缀：`f"mcp_{normalized_server_name}__{mcp_tool.name}"`
+ - 新增 `original_tool_name` 属性存储原始名称
+ - 调用 MCP 服务器时使用 `original_tool_name` 而非内部名称
+- **ToolSet 回退逻辑**（`astrbot/core/provider/func_tool_manager.py`）：
+ - `get_func()` 方法支持按原始名称查找 MCP 工具
+ - 处理多个 MCP 服务器提供同名工具的情况，记录警告并返回第一个匹配项
+
+**用户收益：**
+
+- MCP 工具和插件工具同名时不再冲突，可以同时启用
+- 多个 MCP 服务器可以提供同名工具，系统会自动区分
+- 旧的 Persona 配置无需修改即可继续工作
+- UI 显示友好的工具名称，提升用户体验
+
#### 注意事项
- 工具需在对应 SubAgent/Persona 配置中声明
- 动态注册工具时需确保配置同步更新
- 后台任务需正确设置 `background_task: true` 参数
- 后台任务执行通过 `HandoffExecutor.execute_queued_task()` 方法完成，该方法从数据库中恢复任务上下文并执行
+- **MCP 工具命名**：新配置建议使用带前缀的完整名称（`mcp_<server>__<tool>`）以避免歧义，但旧配置使用原始名称仍可正常工作
#### Neo 技能生命周期工具（PR #5028）
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并留下了一些整体性反馈：
- 在
get_mcp_servers中，从tool.name中剥离mcp_<server_name>__前缀的逻辑假设与MCPTool相同的规范化方式（去掉空格/短横线等），但目前这里是直接使用name；建议应用与MCPTool中相同的规范化逻辑，这样对于名称中包含空格或特殊字符的服务器，前缀检测才能稳定工作。 - 同样在
get_mcp_servers中，你在mcp_client.tools上拆分tool.name来移除命名空间前缀，但这些tools看起来是原始的 MCP tools（没有新的命名空间包装），因此startswith/split('__', 1)这段逻辑实际上是个空操作，可以简化，或者改为对你在其他地方实际使用的带命名空间的表示进行处理。
给 AI Agents 的提示词
Please address the comments from this code review:
## Overall Comments
- In `get_mcp_servers`, the logic that strips the `mcp_<server_name>__` prefix from `tool.name` assumes the same normalization as `MCPTool` (spaces/dashes removed etc.), but it currently uses `name` directly; consider applying the same normalization used in `MCPTool` so the prefix detection works reliably for servers whose names contain spaces or special characters.
- Also in `get_mcp_servers`, you are splitting `tool.name` to drop a namespace prefix on `mcp_client.tools`, but those `tools` appear to be the raw MCP tools (without the new namespaced wrapper), so the `startswith`/`split('__', 1)` block is effectively a no-op and can be simplified or updated to operate on the namespaced representation you actually use elsewhere.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/mcp_client.py" line_range="382-390" />
<code_context>
def __init__(
self, mcp_tool: mcp.Tool, mcp_client: MCPClient, mcp_server_name: str, **kwargs
) -> None:
+ # Add namespace prefix to avoid conflicts with plugin tools
+ # Normalize server name: remove spaces and special characters
+ normalized_server_name = mcp_server_name.replace(" ", "_").replace("-", "_")
+ # Remove any other special characters, keep only alphanumeric and underscore
+ normalized_server_name = "".join(
+ c for c in normalized_server_name if c.isalnum() or c == "_"
+ )
+ # Format: mcp_<normalized_server_name>__<tool_name>
+ namespaced_name = f"mcp_{normalized_server_name}__{mcp_tool.name}"
+
super().__init__(
</code_context>
<issue_to_address>
**issue (bug_risk):** Normalized MCP tool prefix can diverge from raw server name used in routing logic, causing mismatches.
This code normalizes `mcp_server_name`, but `dashboard/routes/tools.py:get_mcp_servers` de-namespaces tools using `tool.name.startswith(f"mcp_{name}__")`, where `name` is the raw server identifier from the route. If the route name contains spaces, hyphens, or other characters that are sanitized here, the prefix check will fail and tools won’t be matched. Consider centralizing this normalization (e.g., a shared helper) and applying it consistently both when building the tool name and when constructing `f"mcp_{name}__"` in the route, or store and reuse a single normalized server key.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/routes/tools.py" line_range="107-110" />
<code_context>
if name_key == name:
mcp_client = runtime.client
- server_info["tools"] = [tool.name for tool in mcp_client.tools]
+ # Display original tool names without namespace prefix
+ server_info["tools"] = [
+ tool.name.split("__", 1)[1]
+ if tool.name.startswith(f"mcp_{name}__")
+ else tool.name
+ for tool in mcp_client.tools
</code_context>
<issue_to_address>
**issue (bug_risk):** Prefix stripping for MCP tool names assumes raw server name matches the normalized prefix used in MCPTool.
Because `MCPTool` normalizes `mcp_server_name` (e.g., replacing spaces/hyphens and stripping special characters) before building the `mcp_{server}__` prefix, `startswith(f"mcp_{name}__")` may fail for server names that change under that normalization. In those cases, the prefix won’t be stripped and the UI will show the raw, prefixed name. Please normalize `name` the same way before constructing the prefix (or use a shared helper) so prefix creation and decoding stay consistent.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
get_mcp_servers, the logic that strips themcp_<server_name>__prefix fromtool.nameassumes the same normalization asMCPTool(spaces/dashes removed etc.), but it currently usesnamedirectly; consider applying the same normalization used inMCPToolso the prefix detection works reliably for servers whose names contain spaces or special characters. - Also in
get_mcp_servers, you are splittingtool.nameto drop a namespace prefix onmcp_client.tools, but thosetoolsappear to be the raw MCP tools (without the new namespaced wrapper), so thestartswith/split('__', 1)block is effectively a no-op and can be simplified or updated to operate on the namespaced representation you actually use elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_mcp_servers`, the logic that strips the `mcp_<server_name>__` prefix from `tool.name` assumes the same normalization as `MCPTool` (spaces/dashes removed etc.), but it currently uses `name` directly; consider applying the same normalization used in `MCPTool` so the prefix detection works reliably for servers whose names contain spaces or special characters.
- Also in `get_mcp_servers`, you are splitting `tool.name` to drop a namespace prefix on `mcp_client.tools`, but those `tools` appear to be the raw MCP tools (without the new namespaced wrapper), so the `startswith`/`split('__', 1)` block is effectively a no-op and can be simplified or updated to operate on the namespaced representation you actually use elsewhere.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/mcp_client.py" line_range="382-390" />
<code_context>
def __init__(
self, mcp_tool: mcp.Tool, mcp_client: MCPClient, mcp_server_name: str, **kwargs
) -> None:
+ # Add namespace prefix to avoid conflicts with plugin tools
+ # Normalize server name: remove spaces and special characters
+ normalized_server_name = mcp_server_name.replace(" ", "_").replace("-", "_")
+ # Remove any other special characters, keep only alphanumeric and underscore
+ normalized_server_name = "".join(
+ c for c in normalized_server_name if c.isalnum() or c == "_"
+ )
+ # Format: mcp_<normalized_server_name>__<tool_name>
+ namespaced_name = f"mcp_{normalized_server_name}__{mcp_tool.name}"
+
super().__init__(
</code_context>
<issue_to_address>
**issue (bug_risk):** Normalized MCP tool prefix can diverge from raw server name used in routing logic, causing mismatches.
This code normalizes `mcp_server_name`, but `dashboard/routes/tools.py:get_mcp_servers` de-namespaces tools using `tool.name.startswith(f"mcp_{name}__")`, where `name` is the raw server identifier from the route. If the route name contains spaces, hyphens, or other characters that are sanitized here, the prefix check will fail and tools won’t be matched. Consider centralizing this normalization (e.g., a shared helper) and applying it consistently both when building the tool name and when constructing `f"mcp_{name}__"` in the route, or store and reuse a single normalized server key.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/routes/tools.py" line_range="107-110" />
<code_context>
if name_key == name:
mcp_client = runtime.client
- server_info["tools"] = [tool.name for tool in mcp_client.tools]
+ # Display original tool names without namespace prefix
+ server_info["tools"] = [
+ tool.name.split("__", 1)[1]
+ if tool.name.startswith(f"mcp_{name}__")
+ else tool.name
+ for tool in mcp_client.tools
</code_context>
<issue_to_address>
**issue (bug_risk):** Prefix stripping for MCP tool names assumes raw server name matches the normalized prefix used in MCPTool.
Because `MCPTool` normalizes `mcp_server_name` (e.g., replacing spaces/hyphens and stripping special characters) before building the `mcp_{server}__` prefix, `startswith(f"mcp_{name}__")` may fail for server names that change under that normalization. In those cases, the prefix won’t be stripped and the UI will show the raw, prefixed name. Please normalize `name` the same way before constructing the prefix (or use a shared helper) so prefix creation and decoding stay consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where MCP tools and plugin tools with identical names would conflict, preventing their simultaneous use. The solution involves implementing a robust namespacing strategy for MCP tools internally, while meticulously ensuring full backward compatibility for existing configurations. Concurrently, the user interface has been refined to display the original, user-friendly names of these tools, significantly enhancing usability without exposing the internal namespaced identifiers. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the tool name conflict between MCP and plugin tools by introducing a namespacing scheme. The changes are comprehensive, touching both the backend and frontend to ensure backward compatibility and a good user experience.
My review has identified a few areas for improvement:
- There are a couple of bugs in the backend related to server name normalization that could lead to incorrect behavior or collisions.
- The frontend code has some duplication in data fetching and utility functions, which could be refactored for better maintainability and performance by using a centralized state management store.
- A minor logic improvement for deterministic behavior when handling multiple tool matches.
Overall, this is a solid implementation. Addressing the points in my review will make it more robust and maintainable.
ChuwuYo
changed the title
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体反馈：
- 在
MCPTool.__init__中，建议将urllib.parse.quote的导入移动到模块作用域，这样在每次实例化工具时就不会重复导入。 - 新的
getToolDisplayName逻辑目前在PersonaForm.vue和PersonaManager.vue中都有重复；建议将其集中到一个共享的 helper/composable 中，以便保持显示名称解析行为的一致性并更易于维护。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `MCPTool.__init__`, consider moving the `urllib.parse.quote` import to module scope so the import isn’t repeated on every tool instantiation.
- The new `getToolDisplayName` logic is now duplicated across `PersonaForm.vue` and `PersonaManager.vue`; consider centralizing this in a shared helper/composable so the display-name resolution behavior stays consistent and easier to maintain.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/extension/componentPanel/components/ToolTable.vue" line_range="44" />
<code_context>
{{ item.name.includes(':') ? 'mdi-server-network' : 'mdi-function-variant' }}
</v-icon>
<div>
</code_context>
<issue_to_address>
**issue (bug_risk):** The icon heuristic based on `name.includes(':')` may no longer correctly distinguish MCP tools after namespacing.
With MCP tools now named `mcp_<server>__<tool>`, this colon-based check will misclassify many items. Since the API exposes `origin`, `origin_name`, and `display_name`, icon selection should key off `origin === 'mcp'` vs `origin === 'plugin'` (or similar) instead of parsing `name`.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
MCPTool.__init__, consider moving theurllib.parse.quoteimport to module scope so the import isn’t repeated on every tool instantiation. - The new
getToolDisplayNamelogic is now duplicated acrossPersonaForm.vueandPersonaManager.vue; consider centralizing this in a shared helper/composable so the display-name resolution behavior stays consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `MCPTool.__init__`, consider moving the `urllib.parse.quote` import to module scope so the import isn’t repeated on every tool instantiation.
- The new `getToolDisplayName` logic is now duplicated across `PersonaForm.vue` and `PersonaManager.vue`; consider centralizing this in a shared helper/composable so the display-name resolution behavior stays consistent and easier to maintain.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/extension/componentPanel/components/ToolTable.vue" line_range="44" />
<code_context>
{{ item.name.includes(':') ? 'mdi-server-network' : 'mdi-function-variant' }}
</v-icon>
<div>
</code_context>
<issue_to_address>
**issue (bug_risk):** The icon heuristic based on `name.includes(':')` may no longer correctly distinguish MCP tools after namespacing.
With MCP tools now named `mcp_<server>__<tool>`, this colon-based check will misclassify many items. Since the API exposes `origin`, `origin_name`, and `display_name`, icon selection should key off `origin === 'mcp'` vs `origin === 'plugin'` (or similar) instead of parsing `name`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
Bot
added
size:L
Contributor
Author
|
🦌 Finish |
…in tools - Add namespace prefix format: mcp_<server_name>__<tool_name> to MCPTool - Store original tool name in MCPTool.original_tool_name for display and calling - Add backward compatibility in get_func() to support old persona tool references - Update tool list API to include display_name field showing original name - Update MCP server info to display original tool names without prefix - Fixes issue where MCP tools and plugin tools with same name would conflict
- Update frontend components to use display_name for user-friendly display - Add display_name field to ToolItem TypeScript interface - Update tool list display in: * PersonaForm.vue - tool selection list and selected tools * PersonaQuickPreview.vue - tool preview display * PersonaManager.vue - persona detail view * ToolTable.vue - function tools management table - Add getToolDisplayName() helper methods in PersonaForm and PersonaManager
- Use URL encoding for MCP server name normalization to avoid collisions - Add deterministic sorting for ambiguous MCP tool name resolution - Add type assertion to fix Pylance type error - Simplify MCP server tools display (use raw tool names directly)
- move quote import in MCPTool to module scope - use origin instead of name parsing for MCP tool icons - centralize tool display-name resolution in shared helper - centralize tool search matching for persona and component views - remove stale MCP-specific frontend search reference
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
FuncToolManager.is_builtin_tool, the new MCP fallback block appears after an earlyreturnand is therefore unreachable; consider moving this logic into a separate helper or above the return so it can actually run. - The MCP fallback lookup logic for
original_tool_nameis now duplicated (and slightly diverging) betweenget_funcand the unreachable block underis_builtin_tool; consider extracting a shared MCP lookup helper to keep behavior consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `FuncToolManager.is_builtin_tool`, the new MCP fallback block appears after an early `return` and is therefore unreachable; consider moving this logic into a separate helper or above the return so it can actually run.
- The MCP fallback lookup logic for `original_tool_name` is now duplicated (and slightly diverging) between `get_func` and the unreachable block under `is_builtin_tool`; consider extracting a shared MCP lookup helper to keep behavior consistent and easier to maintain.
## Individual Comments
### Comment 1
<location path="dashboard/src/utils/toolDisplayName.ts" line_range="4" />
<code_context>
+import type { ToolItem } from '@/components/extension/componentPanel/types';
+
+type ToolDisplayNameSource = Pick<ToolItem, 'name' | 'display_name'>;
+type ToolSearchSource = Pick<ToolItem, 'name' | 'display_name' | 'description' | 'origin' | 'origin_name'>;
+
+export function resolveToolDisplayName(
</code_context>
<issue_to_address>
**issue (bug_risk):** ToolSearchSource references `origin` fields that are not defined on ToolItem.
`ToolItem` in `types.ts` only defines `name`, `display_name?`, `description`, `active`, and `readonly?`, so `Pick<ToolItem, 'origin' | 'origin_name'>` will not type-check. Please either add `origin`/`origin_name` to `ToolItem` or define `ToolSearchSource` independently (or via a broader API type) so it accurately reflects the objects returned from `/api/tools/list`. Also update any callers that pass `ToolItem` into `matchesToolSearch` to use the corrected type.
</issue_to_address>
### Comment 2
<location path="astrbot/core/provider/func_tool_manager.py" line_range="334" />
<code_context>
if f.name == name:
return f
+
+ # Fallback: try to find MCP tool by original name for backward compatibility
+ # This handles cases where personas reference tools by their original names
+ if isinstance(name, str):
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the MCP lookup into a shared helper and keeping `is_builtin_tool` purely boolean to avoid duplication and mixed responsibilities.
You can keep the MCP fallback behavior while reducing duplication and fixing the `is_builtin_tool` confusion by:
1. Extracting the MCP lookup into a private helper.
2. Keeping `is_builtin_tool` purely boolean.
3. Calling the helper where the actual `MCPTool` instance is needed.
### 1. Extract a shared MCP lookup helper
```python
def _find_mcp_tool_by_original_name(self, name: str) -> MCPTool | None:
"""Best-effort lookup of an MCP tool by its original name."""
matches = [
f for f in self.func_list
if isinstance(f, MCPTool) and f.original_tool_name == name
]
if not matches:
return None
if len(matches) > 1:
# Sort for deterministic selection
matches.sort(key=lambda f: f.name)
logger.warning(
"Multiple MCP tools found with original name '%s': %s. Using %s",
name,
[f.name for f in matches],
matches[0].name,
)
return matches[0]
```
### 2. Simplify `get_func` to call the helper
Replace the MCP block in `get_func` with a call to the helper:
```python
def get_func(self, name) -> FuncTool | None:
# 优先返回已激活的工具（后加载的覆盖前面的，与 ToolSet.add_tool 保持一致）
# 使用 getattr(..., True) 与 ToolSet.add_tool 保持一致：没有 active 属性的工具视为已激活
for f in reversed(self.func_list):
if f.name == name and getattr(f, "active", True):
return f
# 退化则拿最后一个同名工具
for f in reversed(self.func_list):
if f.name == name:
return f
# Fallback: try to find MCP tool by original name for backward compatibility
if isinstance(name, str):
mcp_tool = self._find_mcp_tool_by_original_name(name)
if mcp_tool is not None:
return mcp_tool
try:
builtin_tool = self.get_builtin_tool(name)
except KeyError:
return None
if getattr(builtin_tool, "active", True):
return builtin_tool
return builtin_tool
return None
```
### 3. Keep `is_builtin_tool` as a boolean
Remove the MCP block from `is_builtin_tool` entirely and leave it as a pure predicate:
```python
def is_builtin_tool(self, name: str) -> bool:
ensure_builtin_tools_loaded()
return get_builtin_tool_class(name) is not None
```
If you truly need “does this name correspond to some tool, including MCP by original name”, introduce a clearly named method instead of overloading `is_builtin_tool`:
```python
def resolve_tool_by_name(self, name: str) -> FuncTool | None:
"""Resolve a tool by name, including MCP original names."""
tool = self.get_func(name)
if tool is not None:
return tool
if isinstance(name, str):
return self._find_mcp_tool_by_original_name(name)
return None
```
This keeps functionality intact, removes duplicated MCP matching logic, reduces branching, and restores `is_builtin_tool` to a single, predictable responsibility and return type.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- resolveToolDisplayName does a linear search over availableTools on every chip render (e.g., in PersonaForm and PersonaManager); consider precomputing a name→display_name map in component state/computed to avoid repeated O(n) scans during rendering.
- The new
assert mcp_client is not Nonein MCP server initialization will be stripped in optimized Python runs; if you intend this as a runtime safety check rather than a development-only assertion, replace it with an explicit conditional that raises a clear error or logs a critical failure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- resolveToolDisplayName does a linear search over availableTools on every chip render (e.g., in PersonaForm and PersonaManager); consider precomputing a name→display_name map in component state/computed to avoid repeated O(n) scans during rendering.
- The new `assert mcp_client is not None` in MCP server initialization will be stripped in optimized Python runs; if you intend this as a runtime safety check rather than a development-only assertion, replace it with an explicit conditional that raises a clear error or logs a critical failure.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/func_tool_manager.py" line_range="362-363" />
<code_context>
if isinstance(name, str):
+ # Fallback: look up MCP tool by original (pre-namespaced) name
+ # for backward compatibility with legacy persona configurations.
+ mcp_tool = self._find_mcp_tool_by_original_name(name)
+ if mcp_tool is not None:
+ return mcp_tool
+
</code_context>
<issue_to_address>
**issue (bug_risk):** MCP fallback lookup ignores the `active` flag and may violate the existing "active tools first" semantics.
The first loop over `reversed(self.func_list)` correctly respects `active` and load order, but `_find_mcp_tool_by_original_name` does not filter on `active`, so an inactive MCP tool can still be returned if its original name matches. To keep the "active tools first" behavior consistent, either filter MCP matches by `getattr(f, "active", True)` or apply the same active-precedence rule when choosing among MCP matches.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SourceryAI
reviewed
Apr 18, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
toolDisplayName.ts,ToolSearchSourceusesPick<ToolItem, 'origin' | 'origin_name'>butToolItemdoesn't declare those fields, which will either fail type-checking or rely on implicitany—consider extendingToolItemwithoriginandorigin_nameor changing the helper type to a looser interface that matches the actual API payload. - PersonaManager now fetches
/api/tools/listdirectly and maintains its ownavailableTools, which duplicates the tool-loading logic already used elsewhere; consider centralizing tool metadata in a store or shared composable so persona-related views all reuse the same source of truth.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `toolDisplayName.ts`, `ToolSearchSource` uses `Pick<ToolItem, 'origin' | 'origin_name'>` but `ToolItem` doesn't declare those fields, which will either fail type-checking or rely on implicit `any`—consider extending `ToolItem` with `origin` and `origin_name` or changing the helper type to a looser interface that matches the actual API payload.
- PersonaManager now fetches `/api/tools/list` directly and maintains its own `availableTools`, which duplicates the tool-loading logic already used elsewhere; consider centralizing tool metadata in a store or shared composable so persona-related views all reuse the same source of truth.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/persona/PersonaManager.vue" line_range="354-355" />
<code_context>
- skeletonTimer: null as ReturnType<typeof setTimeout> | null
+ skeletonTimer: null as ReturnType<typeof setTimeout> | null,
+
+ // 工具列表用于显示名称
+ availableTools: [] as any[]
};
},
</code_context>
<issue_to_address>
**suggestion:** Use a shared typed interface for availableTools instead of any[] to keep it aligned with backend tool metadata.
Since /api/tools/list appears to return the same structure as the ToolItem used in the extension panel (name, display_name, description, origin, origin_name, etc.), consider typing availableTools as ToolItem[] (or a narrowed variant) instead of any[]. This will give getToolDisplayName proper type checking and help catch mismatches if the backend tool schema changes.
Suggested implementation:
```
import { resolveToolDisplayName } from '@/utils/toolDisplayName';
import type { Folder, FolderTreeNode } from '@/components/folder/types';
import type { ToolItem } from '@/types/tool';
```
```
// 骨架屏延迟显示控制
showSkeleton: false,
skeletonTimer: null as ReturnType<typeof setTimeout> | null,
// 工具列表用于显示名称
availableTools: [] as ToolItem[]
```
1. Ensure that `ToolItem` is actually defined and exported in `@/types/tool` (or adjust the import path to wherever the shared tool metadata interface lives, e.g. a module already used by the extension panel).
2. Confirm that the `ToolItem` type matches the shape returned by `/api/tools/list` (fields like `name`, `display_name`, `description`, `origin`, `origin_name`, etc.). If the backend response is a superset, you can define a narrower interface that extends `ToolItem` and use that instead.
3. If `getToolDisplayName` (or `resolveToolDisplayName`) assumes specific fields on `availableTools`, make sure those fields are present on `ToolItem` so you get full type checking rather than falling back to `any`.
</issue_to_address>Hi @ChuwuYo! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.- Expose original_tool_names alongside namespaced tools in /api/tools/mcp/servers so the MCP "available tools" popup can render human-friendly names while persona tool matching continues to use namespaced identifiers. - Render the popup list from item.original_tool_names with a fallback to item.tools for forward compatibility. - Replace `assert mcp_client is not None` in _start_mcp_server with an explicit RuntimeError so the post-init invariant still holds when running under `python -O`, which strips asserts. - Tighten the _find_mcp_tool_by_original_name docstring to match the surrounding single-line style.
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- PersonaManager.vue now performs its own
/api/tools/listfetch solely for display names; consider reusing the existing tool list source (e.g., via a shared store/composable) to avoid duplicate requests and keep tool metadata in sync across views. - In
get_mcp_servers, buildingtoolsandoriginal_tool_namesby scanningtool_mgr.func_listfor each server introduces tight coupling to the internal storage and a potentialO(#servers * #tools)cost; it may be cleaner and cheaper to derive this from a pre-indexed structure or the MCP runtime/client directly. matchesToolSearchlowercases and trims the query internally while some callers (likecomponentPanel/index.vue) pre-normalize the input withnormalizeTextInput, leading to redundant work and slightly inconsistent normalization rules; consider standardizing normalization in one place (either always before or always inside the helper).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- PersonaManager.vue now performs its own `/api/tools/list` fetch solely for display names; consider reusing the existing tool list source (e.g., via a shared store/composable) to avoid duplicate requests and keep tool metadata in sync across views.
- In `get_mcp_servers`, building `tools` and `original_tool_names` by scanning `tool_mgr.func_list` for each server introduces tight coupling to the internal storage and a potential `O(#servers * #tools)` cost; it may be cleaner and cheaper to derive this from a pre-indexed structure or the MCP runtime/client directly.
- `matchesToolSearch` lowercases and trims the query internally while some callers (like `componentPanel/index.vue`) pre-normalize the input with `normalizeTextInput`, leading to redundant work and slightly inconsistent normalization rules; consider standardizing normalization in one place (either always before or always inside the helper).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ng in component panel
Contributor
Author
|
@seoeaa 原 issue 有人催促合并 |
Soulter
force-pushed
the
master
branch
2 times, most recently
from
faf411f to
0068960
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5689
Description / 描述
This PR fixes the conflict where MCP tools and plugin tools with the same name could not coexist, which previously caused one tool to overwrite the other during registration. The fix introduces namespaced internal names for MCP tools while preserving backward compatibility for existing persona configurations. It also includes follow-up UI, review-feedback, and maintainability fixes so friendly tool names, search behavior, and MCP-specific rendering stay consistent across the dashboard.
此 PR 修复了 MCP 工具和插件工具同名时无法共存的问题。此前工具注册时会发生覆盖，导致两者不能同时启用。修复方案为 MCP 工具引入命名空间内部名称，同时保持对现有 persona 配置的向后兼容；并补充了若干前端、审查反馈和可维护性修复，确保仪表盘中的友好名称展示、搜索行为以及 MCP 相关渲染保持一致。
Relation to #5925 / 与 #5925 的关系
#5925 已合并，通过在
get_func()中优先返回已激活的工具来缓解同名工具互相"连坐"的问题。它是冲突发生时的选择策略，保证至少一个工具能用。本 PR 解决的是更上一层的问题：通过为 MCP 工具加命名空间前缀mcp_<server>__<tool>，让同名的 MCP 工具和插件工具可以同时注册、同时被 LLM 调用，而不是二选一。两者不冲突——合并本 PR 后 #5925 的激活优先逻辑继续保留，作为同名场景下的通用 tie-breaker。#5925 is already merged and mitigates the "both disabled" problem by having
get_func()prefer the active tool when names collide — a resolution policy ensuring at least one tool remains usable. This PR addresses the layer above: by namespacing MCP tool internal names asmcp_<server>__<tool>, same-named MCP tools and plugin tools can both register and both be invoked by the LLM, instead of one overriding the other. The two changes are complementary — #5925's active-preference logic stays intact after this PR merges, continuing to serve as a generic tie-breaker for same-name scenarios.Root Cause / 根本原因
The
ToolSet.add_tool()method uses the tool name as the unique identifier. When an MCP tool and a plugin tool share the same name, the later registration replaces the earlier one instead of allowing both to coexist.ToolSet.add_tool()方法使用工具名称作为唯一标识。当 MCP 工具与插件工具同名时，后注册的工具会覆盖先注册的工具，而不是允许两者共存。Solution / 解决方案
mcp_<encoded_server_name>__<tool_name>to avoid collisions.MCPTool.original_tool_nameand is still used when calling the MCP server.get_func()falls back to a shared_find_mcp_tool_by_original_namehelper that prefers active MCP tools, so legacy persona configurations continue to resolve. The fallback sits after the upstream active-first exact match (introduced in fix: prevent accidental removal of MCP external tools due to name collisions with disabled built-in tools #5925), so it only triggers when the namespaced name isn't found — no behavior change for current configurations.nameand user-facingdisplay_name; the MCP "available tools" popup reads a paralleloriginal_tool_namesfield so it shows raw tool names while persona matching keeps using namespaced identifiers.origin === 'mcp'instead of parsing the tool name format._start_mcp_serverraises an explicitRuntimeErrorinstead of relying onassert, so it still holds underpython -O.Modifications / 改动点
Backend / 后端:
astrbot/core/agent/mcp_client.pymcp_<encoded_server_name>__<tool_name>original_tool_namequoteimport to module scopeastrbot/core/provider/func_tool_manager.py_find_mcp_tool_by_original_namehelper shared byget_func(); active matches take priority, deterministic tie-break with warning when multiple MCP servers expose the same original tool nameis_builtin_toolas a pure boolean predicate (remove prior unreachable MCP fallback)assert mcp_client is not Nonein_start_mcp_serverwith an explicitRuntimeErrorso the invariant survivespython -Oastrbot/dashboard/routes/tools.pydisplay_nameto the tool list APIdisplay_namein the builtin-tool branch to stay compatible with the builtin tool registry introduced upstreamget_mcp_serverspre-indexesMCPToolinstances by server once and uses dict lookup on the runtime view, avoiding anO(#servers × #tools)rescantools(namespaced, forpersonaForm.toolsset matching) andoriginal_tool_names(for UI display) on each MCP serverFrontend / 前端:
dashboard/src/components/extension/componentPanel/types.tsdisplay_nameto theToolIteminterfacedashboard/src/utils/toolDisplayName.tsmatchesToolSearchhandles all query normalization internallydashboard/src/components/shared/PersonaForm.vuedisplay_namefor tool selection and selected tool renderingdashboard/src/components/shared/PersonaQuickPreview.vuedisplay_namefor tool preview displaydashboard/src/views/persona/PersonaManager.vuedisplay_namefor persona detail displayavailableToolsasToolItem[]instead ofany[]dashboard/src/components/extension/componentPanel/components/ToolTable.vueorigin === 'mcp'for icon rendering instead of name parsingdisplay_namefor user-facing displaydashboard/src/components/extension/componentPanel/index.vuematchesToolSearchdashboard/src/components/extension/McpServersSection.vueoriginal_tool_nameswith a fallback totoolsfor forward compatibilityKey Improvements / 关键改进
MCP tools and plugin tools with the same original name can now coexist
Existing persona configurations continue to work without modification
MCP server names are encoded safely to avoid namespace collisions
UI consistently shows friendly MCP tool names instead of internal namespaced identifiers
The MCP "available tools" popup continues to show raw MCP tool names while namespaced names power set-based matching in persona forms
Tool search behavior matches what users actually see in the UI
MCP tool rendering no longer depends on fragile name-format heuristics
Shared frontend helpers reduce duplicated logic and future drift
_start_mcp_serverfails loudly underpython -Oinstead of silently skipping anassertThis is NOT a breaking change. / 这不是一个破坏性变更。
Validation / 验证情况
master; conflicts with the upstream builtin tool registry (refactor: improve astrbot builtin tool management #7418) and related refactors were resolved, preserving upstream's active-firstget_funcbehavior and the builtin-tool UI affordancesget_func()reviewed against legacy persona tool references; falls through only after the active-first exact match, so it's a no-op for existing configs that already resolve by current name/api/tools/list,/api/tools/mcp/servers, and related Vue consumersuv sync --group dev,ruff format --checkandruff checkpass on the modified Python files;vue-tsc --noEmitpasses for the touched frontend filespnpm run buildand copied intodata/dist/; startup smoke test againsthttp://localhost:6185succeeds with no initialization errors and both the tool management view and the MCP servers popup render the expected namesquoteimport moved to module scopeoriginis_builtin_toolrestored to a pure boolean predicate/api/tools/mcp/serversreturns a paralleloriginal_tool_namesfield so the popup keeps showing raw tool namesget_mcp_serversno longer rescansfunc_listper servermatchesToolSearchassert mcp_client is not Nonereplaced with an explicitRuntimeErrorScreenshots / 截图
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
确保名称相同的 MCP 工具和插件工具可以共存，同时保持向后兼容性，并改进 UI 中工具名称的显示。
新功能：
display_name供用户查看。缺陷修复：
增强改进：
display_name来显示工具名称，而不是内部名称。Original summary in English
Summary by Sourcery
Ensure MCP and plugin tools with identical names can coexist while preserving backward compatibility and improving tool name display in the UI.
New Features:
Bug Fixes:
Enhancements:
Summary by Sourcery
Allow MCP and plugin tools with identical names to coexist by namespacing MCP tool identifiers while keeping existing personas and UI behavior compatible.
Bug Fixes:
Enhancements:
Summary by Sourcery
Allow MCP tools and plugin tools with identical names to coexist by namespacing MCP tool identifiers while keeping existing personas and UI behavior compatible.
New Features:
Bug Fixes:
Enhancements: