fix(core): 修复子智能体工具去重失效及内置工具误删问题#5459
Conversation
- 调整 build_main_agent 逻辑顺序,确保在去重装饰逻辑执行前已加载内置工具(如 Computer Use、Cron 等)。 - 修正人格(Persona)工具查找逻辑,使其能正确识别并处理人格配置为“全部工具”的情况。 - 优化 _plugin_tool_fix,确保在插件过滤时保留没有模块路径的内置工具和 MCP 工具。 - 解决子智能体开启去重后主智能体依然保留重复内置工具的问题。
Summary of ChangesHello @SXP-Simon, 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! 此拉取请求主要解决了智能体工具管理中的几个关键问题,包括内置工具的去重失效、人格工具识别不准确以及插件过滤误删工具。通过调整核心逻辑顺序和优化工具查找与过滤机制,确保了智能体工具的正确加载、去重和分配,提升了工具管理的健壮性和准确性。 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
|
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 在
_ensure_persona_and_skills中,persona_info_found加上persona_info_tools的组合虽然可用,但语义有些不够清晰;可以考虑使用一个哨兵值(例如object())或者一个小的辅助函数,来明确区分“未找到 persona”和“已找到 persona 且 tools=None(表示所有工具)”,从而让意图更加一目了然。 - 在
build_main_agent中,先应用 computer/cron/proactive 工具,再调用_apply_kb和_decorate_llm_request,这改变了之前的执行顺序;如果能加一条简短的注释说明为什么装饰步骤必须在内置工具(以及 KB)之后执行,会对今后的阅读者有帮助,也能避免之后不小心改动这一顺序。 - 在
_plugin_tool_fix中,对于handler_module_path为空的工具,直接将其加入new_tool_set这一特殊处理方式比较容易被误解;建议补充一句简短注释,说明这样做的目的是保留缺少插件映射的内置/MCP 工具,避免它们在插件过滤过程中被删除。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- In `_ensure_persona_and_skills`, the `persona_info_found` + `persona_info_tools` pattern works but is a bit opaque; consider using a sentinel value (e.g., `object()`) or a small helper to clearly distinguish “persona not found” from “persona found with tools=None (meaning all tools)” to make the intent more self-explanatory.
- The reordering in `build_main_agent` (applying computer/cron/proactive tools before `_apply_kb` and `_decorate_llm_request`) changes the previous sequencing; it would help future readers if you add a short comment explaining why decoration must run after built-in tools (and after KB) so that this order isn’t accidentally changed later.
- In `_plugin_tool_fix`, the special-case of adding tools with an empty `handler_module_path` directly to `new_tool_set` is easy to misinterpret; consider adding a brief comment that this is intended to preserve built-in/MCP tools that lack a plugin mapping so they are not removed during plugin filtering.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈来改进后续的代码评审。
Original comment in English
Hey - I've left some high level feedback:
- In
_ensure_persona_and_skills, thepersona_info_found+persona_info_toolspattern works but is a bit opaque; consider using a sentinel value (e.g.,object()) or a small helper to clearly distinguish “persona not found” from “persona found with tools=None (meaning all tools)” to make the intent more self-explanatory. - The reordering in
build_main_agent(applying computer/cron/proactive tools before_apply_kband_decorate_llm_request) changes the previous sequencing; it would help future readers if you add a short comment explaining why decoration must run after built-in tools (and after KB) so that this order isn’t accidentally changed later. - In
_plugin_tool_fix, the special-case of adding tools with an emptyhandler_module_pathdirectly tonew_tool_setis easy to misinterpret; consider adding a brief comment that this is intended to preserve built-in/MCP tools that lack a plugin mapping so they are not removed during plugin filtering.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_ensure_persona_and_skills`, the `persona_info_found` + `persona_info_tools` pattern works but is a bit opaque; consider using a sentinel value (e.g., `object()`) or a small helper to clearly distinguish “persona not found” from “persona found with tools=None (meaning all tools)” to make the intent more self-explanatory.
- The reordering in `build_main_agent` (applying computer/cron/proactive tools before `_apply_kb` and `_decorate_llm_request`) changes the previous sequencing; it would help future readers if you add a short comment explaining why decoration must run after built-in tools (and after KB) so that this order isn’t accidentally changed later.
- In `_plugin_tool_fix`, the special-case of adding tools with an empty `handler_module_path` directly to `new_tool_set` is easy to misinterpret; consider adding a brief comment that this is intended to preserve built-in/MCP tools that lack a plugin mapping so they are not removed during plugin filtering.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -351,7 +351,78 @@
---
-### 7. 其他优化
+### 7. 工具加载顺序与去重修复(PR #5459)
+
+#### 问题背景
+在 [PR #5459](https://github.com/AstrBotDevs/AstrBot/pull/5459) 之前,子智能体去重功能(`remove_main_duplicate_tools`)存在以下问题:
+
+1. **内置工具去重失效**:当子智能体开启去重后,主智能体仍然保留与子智能体重复的内置工具(Computer Use、Cron、send_message_to_user 等)
+2. **人格工具查找兼容性问题**:人格(Persona)配置为"全部工具"(`tools=None`)时,工具查找逻辑无法正确区分"人格不存在"和"配置了全部工具"
+3. **插件过滤误删内置工具**:在插件过滤时,缺少模块路径的内置工具和 MCP 工具被错误过滤掉
+
+#### 主要改动
+
+##### 调整 build_main_agent 逻辑顺序
+修复的核心是调整内置工具的加载时机,确保在去重装饰逻辑(`_decorate_llm_request`)执行前已加载所有内置工具。调整后的执行顺序为:
+
+1. **内置工具加载**(提前至去重前):
+ - Computer Use 工具(sandbox/local 运行时)
+ - Cron 工具(定时任务工具)
+ - send_message_to_user 工具(主动消息工具)
+
+2. **知识库应用**(`_apply_kb`)
+
+3. **去重装饰逻辑**(`_decorate_llm_request`):
+ - 此时所有内置工具已完整加载
+ - 子智能体去重逻辑可以正确识别并移除主智能体中与子智能体重复的内置工具
+
+修复前,内置工具在去重逻辑之后添加,导致去重功能无法识别这些工具,造成重复加载。
+
+##### 修复人格工具查找逻辑
+改进了 `_ensure_persona_and_skills()` 方法中的人格工具查找逻辑:
+
+- **修复前**:使用 `next()` 函数查找人格,当人格配置 `tools=None` 时,无法区分"人格不存在"和"配置了全部工具"两种情况
+- **修复后**:引入 `persona_info_found` 标志位,通过显式循环查找人格:
+ - `persona_info_found=True` 且 `persona_info_tools=None`:表示人格存在且配置为"全部工具"
+ - `persona_info_found=False`:表示人格不存在,使用智能体自身的工具配置
+
+这样确保人格配置的语义正确传递,与主智能体的行为保持一致。
+
+##### 优化 _plugin_tool_fix 插件过滤
+在 `_plugin_tool_fix()` 方法中,对于 `tool.handler_module_path` 为空的工具,直接添加到新工具集中,不进行插件过滤:
+
+```python
+if not mp:
+ new_tool_set.add_tool(tool)
+ continue
+```
+
+这样可以保留没有模块路径的内置工具和 MCP 工具,避免误删除。
+
+#### 技术细节
+
+**内置工具加载时机调整:**
+- Computer Use 工具(`_apply_sandbox_tools` / `_apply_local_env_tools`)
+- Cron 工具(`_proactive_cron_job_tools`)
+- send_message_to_user 工具(`SEND_MESSAGE_TO_USER_TOOL`)
+
+这些工具现在在 `await _decorate_llm_request()` 之前添加,确保子智能体的去重逻辑能够正确处理这些工具。
+
+**知识库应用调用位置:**
+知识库应用(`_apply_kb`)的调用位置保持在去重逻辑之前,确保知识库工具也能被正确去重。
+
+#### 影响范围
+此修复确保子智能体去重功能(`remove_main_duplicate_tools`)正常工作,解决了以下问题:
+
+- 内置工具重复加载问题
+- 人格配置兼容性问题(`tools=None` 语义)
+- 插件过滤时误删内置工具和 MCP 工具的问题
+
+修复后,当启用 `remove_main_duplicate_tools: true` 时,主智能体会正确移除与子智能体重复的所有工具(包括内置工具),仅保留独有工具和 handoff 工具。
+
+---
+
+### 8. 其他优化
- JWT 处理和错误处理机制增强,提升系统安全性和稳定性
- UI 细节优化,提升用户体验
- 日志与异常处理增强,便于问题追踪Note: You must be authenticated to accept/decline updates. |
There was a problem hiding this comment.
Code Review
本次 PR 修复了子智能体工具去重失效及内置工具误删的问题。核心改动包括:调整 build_main_agent 的逻辑顺序,确保内置工具在去重逻辑执行前加载;优化了人格(Persona)的工具查找逻辑,以正确处理“全部工具”的配置;并修正了 _plugin_tool_fix 以保留没有模块路径的内置工具。这些改动从根本上解决了子智能体与主智能体之间工具重复或丢失的问题。
整体来看,代码改动方向正确,逻辑清晰。我针对人格工具查找部分提出了一点代码简化建议,以提高可读性。除此之外,其他修改均有效且实现良好。
- 为内置工具加载顺序、工具保留原则等关键逻辑点添加详细说明文档。 - 简化子智能体人格工具的判定写法,提升代码可读性。
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
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
改进主代理工具初始化和角色工具解析,以修复工具去重问题以及内置工具被意外移除的问题。
Bug 修复:
Original summary in English
Summary by Sourcery
Improve main agent tool initialization and persona tool resolution to fix tool deduplication and unintended removal of built-in tools.
Bug Fixes: