feat: add SubAgent-scoped MCP tool isolation#5246
Closed
whatevertogo wants to merge 2 commits intoAstrBotDevs:masterfrom
Closed
feat: add SubAgent-scoped MCP tool isolation#5246whatevertogo wants to merge 2 commits intoAstrBotDevs:masterfrom
whatevertogo wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:XL
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
dosubot
bot
added
area:provider
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey——我发现了两个问题，并给出了一些整体性的反馈：
- 目前作用域（scope）的规范化 / 校验逻辑分别在前端（
normalizeAgentScope、正则校验）和后端（normalize_mcp_scope_value、MCP_SCOPE_WILDCARDS）各自实现，两者行为存在轻微差异（例如大小写处理、通配符语义）；建议考虑统一实现或至少对齐规则，以避免 UI 接受的内容与服务端实际应用的规则之间出现细微不一致。 - 在
_plugin_tool_fix和 handoff 的执行路径中，你已经通过is_mcp_tool_visible_to_agent对 MCP 工具做了限制，但其他为子 Agent 构建工具集的可能入口看起来仍然依赖全局的llm_tools.func_list；建议检查所有子 Agent 工具集构建路径是否都显式传入正确的agent_name，并统一使用同一个可见性辅助函数，以避免工具在全局范围内被意外暴露。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- The scope normalization/validation logic is now implemented separately in the frontend (`normalizeAgentScope`, regex validation) and backend (`normalize_mcp_scope_value`, `MCP_SCOPE_WILDCARDS`), which slightly diverge in behavior (e.g., case handling, wildcard semantics); consider consolidating or at least aligning the rules to avoid subtle inconsistencies between what the UI accepts and what the server applies.
- In `_plugin_tool_fix` and the handoff execution path you now gate MCP tools via `is_mcp_tool_visible_to_agent`, but other possible entry points that build toolsets for subagents still seem to rely on the global `llm_tools.func_list`; it would be good to double-check that all subagent toolset construction paths explicitly pass the appropriate `agent_name` and use the same visibility helper to avoid accidental global exposure.
## Individual Comments
### Comment 1
<location> `tests/test_mcp_scope.py:36-38` </location>
<code_context>
+ assert get_mcp_scopes_from_config(cfg) == ("agent_x",)
+
+
+def test_get_mcp_scopes_from_agent_scope_string():
+ cfg = {"agent_scope": "main"}
+ assert get_mcp_scopes_from_config(cfg) == ("main",)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing tests for `get_mcp_scopes_from_config` when config is not a mapping or does not contain any scope fields.
Since the function returns `None` for non-mapping configs and when both `scopes` and `agent_scope` are absent, please add tests for `get_mcp_scopes_from_config(None)`, `get_mcp_scopes_from_config([])`, and a config without those keys (e.g. `{"command": "python"}`) to cover these branches and document the default behavior.
</issue_to_address>
### Comment 2
<location> `astrbot/core/astr_main_agent.py:751` </location>
<code_context>
req.contexts = sanitized_contexts
-def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
+def _plugin_tool_fix(
+ event: AstrMessageEvent,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `_plugin_tool_fix` into smaller helpers that separately handle MCP visibility filtering, plugin-based filtering, and MCP injection to simplify the control flow and remove duplication.
You can reduce `_plugin_tool_fix` complexity and avoid duplicated MCP visibility logic by extracting small helpers and composing them linearly.
### 1. Centralize MCP visibility filtering
Right now MCP visibility is checked in two separate passes. You can extract this into a single helper and call it once at the top:
```python
def _filter_visible_mcp_tools(tools: ToolSet | None, agent_name: str) -> ToolSet | None:
if not tools:
return tools
filtered = ToolSet()
for tool in tools.tools:
if isinstance(tool, MCPTool) and not is_mcp_tool_visible_to_agent(tool, agent_name):
continue
filtered.add_tool(tool)
return filtered
```
Then at the start of `_plugin_tool_fix`:
```python
req.func_tool = _filter_visible_mcp_tools(req.func_tool, agent_name)
```
This removes the need to repeat `is_mcp_tool_visible_to_agent` in later branches.
### 2. Split plugin filtering from MCP concerns
The plugin filtering logic can be made MCP-agnostic by keeping MCP tools out of its responsibility entirely:
```python
def _filter_tools_by_plugins(tools: ToolSet, plugins_name: set[str]) -> ToolSet:
filtered = ToolSet()
for tool in tools.tools:
if isinstance(tool, MCPTool):
# MCP already filtered by visibility; always keep here
filtered.add_tool(tool)
continue
mp = tool.handler_module_path
if not mp:
continue
plugin = star_map.get(mp)
if not plugin:
continue
if plugin.name in plugins_name or plugin.reserved:
filtered.add_tool(tool)
return filtered
```
Then in `_plugin_tool_fix`:
```python
if event.plugins_name is not None and req.func_tool:
req.func_tool = _filter_tools_by_plugins(req.func_tool, event.plugins_name)
```
Note MCP visibility has already been applied once via `_filter_visible_mcp_tools`.
### 3. Isolate MCP injection and always return a new ToolSet
The injection branch can be made more consistent by not mutating `req.func_tool` in place and by always working on a new `ToolSet`:
```python
def _inject_global_mcp_tools(
base_tools: ToolSet | None,
agent_name: str,
) -> ToolSet:
result = ToolSet()
if base_tools:
for tool in base_tools.tools:
result.add_tool(tool)
for tool in llm_tools.func_list:
if isinstance(tool, MCPTool) and is_mcp_tool_visible_to_agent(tool, agent_name):
result.add_tool(tool)
return result
```
Then in `_plugin_tool_fix`:
```python
elif inject_mcp:
req.func_tool = _inject_global_mcp_tools(req.func_tool, agent_name)
```
### 4. Simplified `_plugin_tool_fix` orchestration
Putting it together, `_plugin_tool_fix` becomes a linear orchestrator of the three concerns:
```python
def _plugin_tool_fix(
event: AstrMessageEvent,
req: ProviderRequest,
inject_mcp: bool = True,
*,
agent_name: str = "main",
) -> None:
# 1. Agent-scoped MCP visibility
req.func_tool = _filter_visible_mcp_tools(req.func_tool, agent_name)
# 2. Plugin filtering (MCP passes through)
if event.plugins_name is not None and req.func_tool:
req.func_tool = _filter_tools_by_plugins(req.func_tool, event.plugins_name)
# 3. Optional global MCP injection
elif inject_mcp:
req.func_tool = _inject_global_mcp_tools(req.func_tool, agent_name)
```
This preserves all current behavior (including `inject_mcp` and agent scoping) while reducing branching, duplication, and the coupling of concerns inside a single function.
</issue_to_address>请帮我变得更有用！可以对每条评论点 👍 或 👎，我会根据反馈改进后续的 Review。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The scope normalization/validation logic is now implemented separately in the frontend (
normalizeAgentScope, regex validation) and backend (normalize_mcp_scope_value,MCP_SCOPE_WILDCARDS), which slightly diverge in behavior (e.g., case handling, wildcard semantics); consider consolidating or at least aligning the rules to avoid subtle inconsistencies between what the UI accepts and what the server applies. - In
_plugin_tool_fixand the handoff execution path you now gate MCP tools viais_mcp_tool_visible_to_agent, but other possible entry points that build toolsets for subagents still seem to rely on the globalllm_tools.func_list; it would be good to double-check that all subagent toolset construction paths explicitly pass the appropriateagent_nameand use the same visibility helper to avoid accidental global exposure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The scope normalization/validation logic is now implemented separately in the frontend (`normalizeAgentScope`, regex validation) and backend (`normalize_mcp_scope_value`, `MCP_SCOPE_WILDCARDS`), which slightly diverge in behavior (e.g., case handling, wildcard semantics); consider consolidating or at least aligning the rules to avoid subtle inconsistencies between what the UI accepts and what the server applies.
- In `_plugin_tool_fix` and the handoff execution path you now gate MCP tools via `is_mcp_tool_visible_to_agent`, but other possible entry points that build toolsets for subagents still seem to rely on the global `llm_tools.func_list`; it would be good to double-check that all subagent toolset construction paths explicitly pass the appropriate `agent_name` and use the same visibility helper to avoid accidental global exposure.
## Individual Comments
### Comment 1
<location> `tests/test_mcp_scope.py:36-38` </location>
<code_context>
+ assert get_mcp_scopes_from_config(cfg) == ("agent_x",)
+
+
+def test_get_mcp_scopes_from_agent_scope_string():
+ cfg = {"agent_scope": "main"}
+ assert get_mcp_scopes_from_config(cfg) == ("main",)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing tests for `get_mcp_scopes_from_config` when config is not a mapping or does not contain any scope fields.
Since the function returns `None` for non-mapping configs and when both `scopes` and `agent_scope` are absent, please add tests for `get_mcp_scopes_from_config(None)`, `get_mcp_scopes_from_config([])`, and a config without those keys (e.g. `{"command": "python"}`) to cover these branches and document the default behavior.
</issue_to_address>
### Comment 2
<location> `astrbot/core/astr_main_agent.py:751` </location>
<code_context>
req.contexts = sanitized_contexts
-def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
+def _plugin_tool_fix(
+ event: AstrMessageEvent,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `_plugin_tool_fix` into smaller helpers that separately handle MCP visibility filtering, plugin-based filtering, and MCP injection to simplify the control flow and remove duplication.
You can reduce `_plugin_tool_fix` complexity and avoid duplicated MCP visibility logic by extracting small helpers and composing them linearly.
### 1. Centralize MCP visibility filtering
Right now MCP visibility is checked in two separate passes. You can extract this into a single helper and call it once at the top:
```python
def _filter_visible_mcp_tools(tools: ToolSet | None, agent_name: str) -> ToolSet | None:
if not tools:
return tools
filtered = ToolSet()
for tool in tools.tools:
if isinstance(tool, MCPTool) and not is_mcp_tool_visible_to_agent(tool, agent_name):
continue
filtered.add_tool(tool)
return filtered
```
Then at the start of `_plugin_tool_fix`:
```python
req.func_tool = _filter_visible_mcp_tools(req.func_tool, agent_name)
```
This removes the need to repeat `is_mcp_tool_visible_to_agent` in later branches.
### 2. Split plugin filtering from MCP concerns
The plugin filtering logic can be made MCP-agnostic by keeping MCP tools out of its responsibility entirely:
```python
def _filter_tools_by_plugins(tools: ToolSet, plugins_name: set[str]) -> ToolSet:
filtered = ToolSet()
for tool in tools.tools:
if isinstance(tool, MCPTool):
# MCP already filtered by visibility; always keep here
filtered.add_tool(tool)
continue
mp = tool.handler_module_path
if not mp:
continue
plugin = star_map.get(mp)
if not plugin:
continue
if plugin.name in plugins_name or plugin.reserved:
filtered.add_tool(tool)
return filtered
```
Then in `_plugin_tool_fix`:
```python
if event.plugins_name is not None and req.func_tool:
req.func_tool = _filter_tools_by_plugins(req.func_tool, event.plugins_name)
```
Note MCP visibility has already been applied once via `_filter_visible_mcp_tools`.
### 3. Isolate MCP injection and always return a new ToolSet
The injection branch can be made more consistent by not mutating `req.func_tool` in place and by always working on a new `ToolSet`:
```python
def _inject_global_mcp_tools(
base_tools: ToolSet | None,
agent_name: str,
) -> ToolSet:
result = ToolSet()
if base_tools:
for tool in base_tools.tools:
result.add_tool(tool)
for tool in llm_tools.func_list:
if isinstance(tool, MCPTool) and is_mcp_tool_visible_to_agent(tool, agent_name):
result.add_tool(tool)
return result
```
Then in `_plugin_tool_fix`:
```python
elif inject_mcp:
req.func_tool = _inject_global_mcp_tools(req.func_tool, agent_name)
```
### 4. Simplified `_plugin_tool_fix` orchestration
Putting it together, `_plugin_tool_fix` becomes a linear orchestrator of the three concerns:
```python
def _plugin_tool_fix(
event: AstrMessageEvent,
req: ProviderRequest,
inject_mcp: bool = True,
*,
agent_name: str = "main",
) -> None:
# 1. Agent-scoped MCP visibility
req.func_tool = _filter_visible_mcp_tools(req.func_tool, agent_name)
# 2. Plugin filtering (MCP passes through)
if event.plugins_name is not None and req.func_tool:
req.func_tool = _filter_tools_by_plugins(req.func_tool, event.plugins_name)
# 3. Optional global MCP injection
elif inject_mcp:
req.func_tool = _inject_global_mcp_tools(req.func_tool, agent_name)
```
This preserves all current behavior (including `inject_mcp` and agent scoping) while reducing branching, duplication, and the coupling of concerns inside a single function.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+36
to
+38
| def test_get_mcp_scopes_from_agent_scope_string(): | ||
| cfg = {"agent_scope": "main"} | ||
| assert get_mcp_scopes_from_config(cfg) == ("main",) |
Contributor
There was a problem hiding this comment.
suggestion (testing): 当前缺少针对 get_mcp_scopes_from_config 的测试，用于覆盖配置不是映射类型，或不包含任何 scope 字段的情况。
由于该函数在配置不是映射类型，以及同时缺少 scopes 和 agent_scope 时会返回 None，请补充以下测试用例：get_mcp_scopes_from_config(None)、get_mcp_scopes_from_config([])，以及一个不包含这些键的配置（例如 {"command": "python"}），以覆盖这些分支并记录默认行为。
Original comment in English
suggestion (testing): Missing tests for get_mcp_scopes_from_config when config is not a mapping or does not contain any scope fields.
Since the function returns None for non-mapping configs and when both scopes and agent_scope are absent, please add tests for get_mcp_scopes_from_config(None), get_mcp_scopes_from_config([]), and a config without those keys (e.g. {"command": "python"}) to cover these branches and document the default behavior.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds SubAgent-level MCP tool isolation to address token consumption explosion in multi-agent orchestration scenarios. The implementation introduces a two-phase approach: a global opt-out switch (inject_global_mcp_tools) and per-server agent scope configuration (agent_scope).
Changes:
- Added MCP tool scope filtering by agent name with case-insensitive matching
- Added global configuration
inject_global_mcp_tools(defaultTrue) to control automatic MCP tool injection - Added per-server
agent_scopeconfiguration to restrict MCP tools to specific agents - Updated UI to support agent scope configuration with quick-select chips and validation
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| astrbot/core/agent/mcp_scope.py | New module providing scope normalization and visibility checking functions |
| astrbot/core/agent/mcp_client.py | Updated MCPTool to store and strip scope fields |
| astrbot/core/provider/func_tool_manager.py | Integrated scope extraction during MCP client initialization |
| astrbot/core/astr_main_agent.py | Updated _plugin_tool_fix to support inject_mcp flag and scope filtering |
| astrbot/core/astr_agent_tool_exec.py | Added scope filtering in handoff tool execution |
| astrbot/core/config/default.py | Added inject_global_mcp_tools config option |
| astrbot/dashboard/routes/tools.py | Exposed mcp_server_scopes in tool list API |
| dashboard/src/components/extension/McpServersSection.vue | Added agent scope UI with validation and quick-select |
| dashboard/src/i18n/locales/*/features/tool-use.json | Added i18n strings for scope UI |
| tests/test_plugin_tool_fix.py | Added unit tests for plugin tool fix logic |
| tests/test_mcp_scope.py | Added unit tests for scope normalization and filtering |
Contributor
Author
|
为啥不是agent拥有工具而是工具指定可用agent啊，困惑喵 |
Member
理解了，其实更合理的做法应该是在 人格页 页配置人格（其实就是agent）可用的mcp、tools、skills |
Contributor
Author
|
目前是： |
Contributor
Author
|
但是我不太了解项目第一次做贡献，所以我担心是否是因为历史遗留问题要这么写 |
Contributor
Author
|
所以我是按照issue的问题方式写的 |
Contributor
Author
|
我能改吗o(￣ヘ￣o#) |
Member
现在 AstrBot 的方式是，MCP Server 单独添加和管理，人格（Agent）可以选择添加的 MCP Server，我觉得 MCP Server 放在人格（Agent）管理比较好： 
我刚刚优化了一下这块，可以显示的清晰一些 ⬆️ 并且修复了 #5237。 不过还是感谢你的 PR ~ |
Contributor
Author
|
修好了我就关咯 |
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.
Summary / 概述
Fixes #5237
Adds SubAgent-level MCP tool isolation to address token consumption explosion and maintain clean SubAgent orchestration architecture.
Problem / 问题
Previously,
_plugin_tool_fixinastr_main_agent.pyunconditionally injected all MCP tools to the main agent, causing:Solution / 解决方案
Implemented two-phase approach:
Phase 1: Global Opt-out Switch
inject_global_mcp_toolsconfig (defaultTruefor backward compatibility)subagent_orchestratorconfig blockPhase 2: Per-Server Agent Scope
agent_scopeto MCP server configurationFiles Changed / 修改文件
astrbot/core/agent/mcp_client.py- Added scope filtering supportastrbot/core/agent/mcp_scope.py- New scope management moduleastrbot/core/astr_main_agent.py- Updated tool injection logicastrbot/core/config/default.py- Added config optionsastrbot/core/provider/func_tool_manager.py- Scope integrationastrbot/dashboard/routes/tools.py- API endpoints for scopedashboard/src/components/extension/McpServersSection.vue- UI for agent_scopetests/test_plugin_tool_fix.py- Unit tests for tool fixtests/test_mcp_scope.py- Unit tests for scope logicUsage / 使用方式
Disable global MCP injection:
{ "subagent_orchestrator": { "inject_global_mcp_tools": false } }Configure per-server scope:
{ "mcpServers": { "search_server": { "agent_scope": ["search_agent"] } } }Test Results / 测试结果
All 17 tests passing:
_plugin_tool_fixlogicSummary by Sourcery
引入以 SubAgent 为作用域的 MCP 工具可见性和配置机制，防止全局 MCP 工具注入并减少不必要的工具暴露。
New Features:
MCP agent_scope配置，用于控制哪些代理（agents）可以访问各 MCP 服务器的工具。Enhancements:
inject_global_mcp_tools标志位控制是否启用全局 MCP 工具自动注入。Tests:
Original summary in English
Summary by Sourcery
Introduce SubAgent-scoped MCP tool visibility and configuration to prevent global MCP tool injection and reduce unnecessary tool exposure.
New Features:
Enhancements:
Tests: