Repository navigation
feature: 支持关闭skill_list_tools tool - #323
Conversation
raychen911
commented
Sep 4, 2026
raychen911
commented
- 修改skill_list_tools 的函数注释,避免模型理解歧义
- 支持关闭skill_list_tools
AI Code Review审查结论通过 审查范围：f05797d..ccb4f8f 单个提交（feature: 支持关闭 skill_list_tools tool），5 个文件 +74/-25： 计划符合性：核心需求已实现—— 主要风险：1) 测试充分性： 门禁结论：未发现阻止合入的正确性/安全/数据损坏级缺陷，状态为 PASSED，附 2 条 MODERATE 与 2 条 LOW 评论。 发现的问题中等
问题： 触发条件： 任意一次 实际影响： 后续所有请求持续返回旧列表：新注册的技能函数永远不会暴露给 LLM，已注销或被 修正方向： 为缓存增加失效条件（例如在 中等
问题： 新增的 触发条件： 用户按参数定义排除任何被引导文案点名的内置工具（例如 实际影响： 修正方向： 将排除信息同步进技能配置/profile 机制，例如构造 较低
问题： 触发条件： LLM 或调用方传入拼写错误/不存在的 实际影响： 模型收到 修正方向： 在返回结构中区分两种情况，例如增加 较低
问题： 本次提交的核心功能—— 触发条件： 运行现有测试套件即可确认： 实际影响： 排除名称拼写错误、过滤逻辑回归（例如误删具有合法名称的工具）或缓存行为破坏都不会被测试发现；"支持关闭 skill_list_tools" 这一提交主目标本身处于未验证状态。 修正方向： 在 |
| if self._default_tools: | ||
| return self._default_tools.copy() | ||
|
|
||
| tools: List[ToolABC] = [] | ||
| tools.append(self._load_tool) | ||
| tools.append(self._run_tool) | ||
| tools.append(self._exec_tool) | ||
| tools.extend(self._runtime_tools) | ||
| skill_functions: List[SkillToolFunction] = SKILL_REGISTRY.get_all() | ||
| skill_functions.extend(self._function_tools) | ||
| for skill_function in skill_functions: | ||
| try: | ||
| tools.append(FunctionTool(func=skill_function)) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| # Log error but continue loading other tools | ||
| logger.warning("Failed to get tools from skill '%s': %s", skill_function.__name__, ex) | ||
| continue | ||
|
|
||
| tools = self._exclude_tools(tools) | ||
| self._default_tools.extend(tools) |
There was a problem hiding this comment.
问题： get_tools 新增的 self._default_tools 结果缓存没有任何失效机制，也未做线程同步；首次调用构建后工具列表即被永久冻结，与该方法文档字符串 "Get all tools from registered skills" 的契约不符。
触发条件： 任意一次 get_tools 完成缓存填充（首次 LLM 请求的 process_llm_request 即触发）之后，运行期通过 SkillRegistry 单例的 register/unregister/clear 变更技能函数（SkillRegistry() 与模块级 SKILL_REGISTRY 是同一单例），或调用方追加传入的 runtime_tools 列表；此外多线程并发执行首次 get_tools 时，"检查为空—构建—extend" 序列会交错执行。
实际影响： 后续所有请求持续返回旧列表：新注册的技能函数永远不会暴露给 LLM，已注销或被 clear 的技能函数继续暴露；并发首次调用还会把构建结果重复 extend 进缓存，使后续请求携带同名重复工具，LLM 请求出现重复 function declaration 并可能被模型接口拒绝。变更前 get_tools 每次重新执行 SKILL_REGISTRY.get_all()，不存在上述问题。
修正方向： 为缓存增加失效条件（例如在 SkillRegistry 变更时递增版本号并在 get_tools 中比对，或只缓存静态内置工具、每次调用重新解析注册表函数），并将 self._default_tools.extend(tools) 改为原子赋值 self._default_tools = tools 或加锁，消除并发重复写入。
| excluded_tools: Optional[List[str]] = None, | ||
| **run_tool_kwargs: dict[str, Any]): |
There was a problem hiding this comment.
问题： 新增的 excluded_tools 只在 SkillToolSet.get_tools 一层过滤工具，与 SkillsRequestProcessor/SkillProfileFlags 生成系统提示词引导的既有机制完全脱节，二者之间没有任何信息传递。
触发条件： 用户按参数定义排除任何被引导文案点名的内置工具（例如 excluded_tools=["skill_run"]、["skill_select_tools"] 或 ["skill_list_skills"]），同时 Agent 按示例标准接法设置了 skill_repository，使 SkillsRequestProcessor 以默认 full profile 注入引导。
实际影响： trpc_agent_sdk/agents/core/_skill_processor.py 的 _tooling_guidance_text 与 _default_full_tooling_and_workspace_guidance 仍会指示 LLM 使用已被排除的工具（如 "Use the skill_select_tools tool..." 以及大量 skill_run/skill_exec 指引），LLM 随后调用不存在的工具，触发 tool_not_found 错误事件，浪费对话轮次甚至导致任务失败。变更前工具无法从工具集中移除，引导不会指向不存在的工具。
修正方向： 将排除信息同步进技能配置/profile 机制，例如构造 SkillsRequestProcessor 时从 SkillToolSet 读取 excluded_tools 并并入 forbidden_tools/SkillProfileFlags 解析，或在参数文档中明确 excluded_tools 仅适用于引导文案未点名的工具，保证引导与实际可用工具集一致。
| else: | ||
| available_tools = list(skill.tools or []) | ||
| return { | ||
| "skill_name": | ||
| skill_name, | ||
| "available_tools": | ||
| available_tools, |
There was a problem hiding this comment.
问题： skill_list_tools 对"技能不存在"与"技能未声明工具"两种情况返回完全相同的载荷（available_tools 为空且携带同一段 note），而本次新增的文档字符串和 note 断言 "An empty result means that this skill declares no tools"，该断言在技能不存在的路径上不成立。
触发条件： LLM 或调用方传入拼写错误/不存在的 skill_name（如 "leader-researchx"），repository.get 返回 None，代码仅记录 logger.error 后以空列表落入共享返回结构。
实际影响： 模型收到 skill_name 回显、空 available_tools 和 "Only tools declared by this skill are listed..." 的说明，会把"技能不存在"误读为"该技能存在但未声明工具"，在后续推理中得出错误结论（例如向用户报告技能没有工具而不是技能不存在），与本次变更想澄清返回语义的目标相悖。
修正方向： 在返回结构中区分两种情况，例如增加 found/status 字段，技能不存在时返回明确的 "skill not found" 提示；或保留命中路径才附加 note 的区分逻辑。
| def _exclude_tools(self, tools: List[ToolABC]) -> List[ToolABC]: | ||
| """Exclude tools from the list.""" | ||
| if not self._excluded_tools: | ||
| return tools | ||
| available_tools: List[ToolABC] = [] | ||
| for tool in tools: | ||
| name = getattr(tool, "name", None) | ||
| if not name or name in self._excluded_tools: | ||
| continue | ||
| available_tools.append(tool) | ||
| return available_tools |
There was a problem hiding this comment.
问题： 本次提交的核心功能——excluded_tools 参数、_exclude_tools 过滤逻辑和 _default_tools 缓存——没有任何测试覆盖。
触发条件： 运行现有测试套件即可确认：tests/skills/test_toolset.py 未新增用例且只断言默认工具存在，tests/skills/tools/test_skill_list_tool.py 仅断言返回结构，没有任何用例构造带 excluded_tools 的 SkillToolSet。
实际影响： 排除名称拼写错误、过滤逻辑回归（例如误删具有合法名称的工具）或缓存行为破坏都不会被测试发现；"支持关闭 skill_list_tools" 这一提交主目标本身处于未验证状态。
修正方向： 在 tests/skills/test_toolset.py 补充用例：默认包含 skill_list_tools；传入 excluded_tools=["skill_list_tools"] 后该工具被移除且其余工具保留；连续两次调用 get_tools 返回一致结果。
ccb4f8f to
9158446
Compare
AI Code Review审查结论通过 审查范围： 发现的问题中等
问题： 触发条件： 任一宿主在首次 实际影响： 新注册的技能工具在本进程生命周期内对模型不可见、已注销工具仍可见；同时 修正方向： 删除 中等
问题： 触发条件： 任何按旧契约消费结果的调用方（例如把 实际影响： 相等性/类型断言调用方得到意外结果（新增键导致相等断言失败、LLM 收到非数组负载），提示词与文档会诱导模型按数组格式解析而实际拿到 dict，属于 source-breaking 的兼容性变化；本次仓库内无消费方，但 SDK 属对外发布库，外部影响真实存在。 修正方向： 同步更新两处 较低
问题： 本次提交为 触发条件： 后续迭代修改 实际影响： 排除工具的过滤正确性、缓存行为无测试保障；例如排除名单写错工具名、 修正方向： 在 |
| if self._default_tools: | ||
| return self._default_tools.copy() | ||
|
|
||
| tools: List[ToolABC] = [] | ||
| tools.append(self._load_tool) | ||
| tools.append(self._run_tool) | ||
| tools.append(self._exec_tool) | ||
| tools.extend(self._runtime_tools) | ||
| skill_functions: List[SkillToolFunction] = SKILL_REGISTRY.get_all() | ||
| skill_functions.extend(self._function_tools) | ||
| for skill_function in skill_functions: | ||
| try: | ||
| tools.append(FunctionTool(func=skill_function)) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| # Log error but continue loading other tools | ||
| logger.warning("Failed to get tools from skill '%s': %s", skill_function.__name__, ex) | ||
| continue | ||
|
|
||
| tools = self._exclude_tools(tools) | ||
| self._default_tools.extend(tools) | ||
| return tools |
There was a problem hiding this comment.
问题： get_tools 新增的一次性快照缓存：首次调用把 _load_tool/_run_tool/_exec_tool/_runtime_tools/SKILL_REGISTRY.get_all()（含 _function_tools）全部收起为列表并 self._default_tools.extend(tools)，此后所有调用（包括每次 tool-call 执行经 find_tool/execute_tools_async 再次解析 get_tools）都返回 copy()，SKILL_REGISTRY 与热加载感知不到运行时（首个 get_tools 之后）的注册变化，且没有任何按 turn/会话刷新的机制。
触发条件： 任一宿主在首次 get_tools() 之后向 SKILL_REGISTRY 注册/注销技能函数（注册表 API 公开且 SKILL_REGISTRY 被注入到 agent_context metadata，注册入口可被宿主复用），或工具集实例生命周期内发生热加载变更。
实际影响： 新注册的技能工具在本进程生命周期内对模型不可见、已注销工具仍可见；同时 _default_tools 被延展引用，调用方篡改返回列表会污染后续快照（返回的是 copy 真列表但 append 相同元素），与旧实现每次重新扫描注册表的行为不一致。
修正方向： 删除 _default_tools 缓存，恢复每次 get_tools 全量重建（排除逻辑本身无状态）；若确需缓存，应在注册表变更或热加载事件上使其失效，并保证增量只在 tools 局部进行，不要触碰已延展的实例成员。
| return { | ||
| "skill_name": | ||
| skill_name, | ||
| "available_tools": | ||
| available_tools, | ||
| "scope": | ||
| "skill_declared_tools_only", | ||
| "note": | ||
| "Only tools declared by this skill are listed. " | ||
| "This does not represent all tools available to the agent.", | ||
| } |
There was a problem hiding this comment.
问题： skill_list_tools 的返回结构由 {"available_tools": [...]} 扩展为带 skill_name/scope/note 的固定四字段对象，改变了该工具的对外契约；仓库内 docs/mkdocs/en/skill.md（1612、1985、2028 行等）与 docs/mkdocs/zh/skill.md（1611、1983、2026 行等）仍以“返回工具名数组 ['get_current_weather', ...]”描述该工具，没有随变更更新。
触发条件： 任何按旧契约消费结果的调用方（例如把 result == {"available_tools": [...]}、result["available_tools"] 下标或取值数组直接喂给模型的调用）运行包含该变更的版本。
实际影响： 相等性/类型断言调用方得到意外结果（新增键导致相等断言失败、LLM 收到非数组负载），提示词与文档会诱导模型按数组格式解析而实际拿到 dict，属于 source-breaking 的兼容性变化；本次仓库内无消费方，但 SDK 属对外发布库，外部影响真实存在。
修正方向： 同步更新两处 docs/mkdocs/*/skill.md 中 skill_list_tools 的返回示例/描述；若无法更新全部文档，可保留 available_tools 键并显式声明为兼容层，并在变更说明中标注破坏性行为。
| runtime_tools: Optional[List[ToolABC]] = None, | ||
| skill_stager: Optional[Stager] = None, | ||
| skill_config: Optional[dict[str, Any]] = None, | ||
| excluded_tools: Optional[List[str]] = None, |
There was a problem hiding this comment.
问题： 本次提交为 SkillToolSet 新增 excluded_tools 参数与 _exclude_tools 过滤逻辑，但对应测试完全缺失：tests/skills/test_toolset.py 未对 excluded_tools 构造、过滤结果、name 缺失工具的跳过行为做任何断言，新增的缓存快照语义（_default_tools）与 _exclude_tools 直接unes 测。
触发条件： 后续迭代修改 _exclude_tools 的过滤条件或 get_tools 快照逻辑时，回归不会被任何用例捕获。
实际影响： 排除工具的过滤正确性、缓存行为无测试保障；例如排除名单写错工具名、name 缺失工具被丢弃等回归难以被发现。
修正方向： 在 tests/skills/test_toolset.py 增加用例：构造 excluded_tools=["skill_list_tools"] 断言名字过滤生效；断言 get_tools 两次调用返回等长度列表且不含被排除工具；补充 name 为 None 的工具被跳过的用例。
9158446 to
503e884
Compare
AI Code Review审查结论不通过 审查范围：c98a61d..503e884（feature: 支持关闭skill_list_tools tool），6 个文件 76+/26-，含 SDK 核心（SkillToolSet、skill_list_tools）、示例与测试。计划符合性：核心特性（skill_list_tools 可通过 excluded_tools 参数关闭、返回结构增加 skill_name/scope/note 澄清信息、team_with_skill 提示词移除该步骤、FastAPI 示例修复未 await 的 close）均已实现且测试更新通过。主要风险：为支持排除功能在 SkillToolSet.get_tools 引入的实例级缓存（_default_tools）改变了既有语义——SKILL_REGISTRY 快照被首次调用冻结（运行期注册/注销不再生效）、返回列表被冻结导致子代理（BorrowedToolSet）与多上下文场景共享同一批工具而元数据按上下文重写、首次调用返回内部活列表存在别名风险。 发现的问题严重
问题： 新增的 触发条件： 任何在首个 实际影响： 新注册技能的工具对模型静默缺失，注销也不再生效，工具集内容在整个进程生命周期内陈旧，且无任何告警；这是本次变更引入的行为回归（变更前每次调用都重新求值）。 修正方向： 移除缓存，改为在每次调用时执行排除过滤后直接返回；若确需缓存，则为缓存增加显式失效机制（如监听 严重
问题： 首次调用路径 触发条件： 任何对 实际影响： 未来调用方（或对首调用结果做过滤/裁剪的集成代码）一旦就地修改，缓存即被污染，LLM 请求中出现重复或伪造的工具声明（重复函数声明可能导致模型拒绝请求或歧义工具调用），问题难以排查。 修正方向： 首次调用同样返回 严重
问题： 工具列表缓存（ 触发条件： 一个 实际影响： 子代理/后续上下文的技能工具执行在错误仓库或错误工作区目录上， 修正方向： 缓存按上下文或按仓库一致性键控（如仅当 中等
问题： 触发条件： 用户传入非空 实际影响： 与排除意图无关的工具从 LLM 可见工具集中消失，模型能力静默降级；同名函数被全部排除时用户无法感知排除只应作用于目标工具。 修正方向： 仅跳过 中等
问题： 特性落地不一致：本变更在此处通过 触发条件： 运行 实际影响： 修正方向： 统一示例与文档：要么在所有示例中一致启用排除并同步修改提示词，要么保留工具并更新提示词/文档为新的字典返回结构；同时更新 较低
问题： 本次变更的核心新特性 触发条件： 后续任何人回归 实际影响： 特性本身（"支持关闭skill_list_tools"）的保证不被任何自动化测试守护，示例中唯一的使用点（ 修正方向： 在 较低
问题： 本变更同时修复了 触发条件： 需要单独回滚或二分定位 实际影响： 无法独立回退关闭工具特性而不连带撤销该关闭相关的修补；提交范围混杂增大审查与维护成本。 修正方向： 将 FastAPI 示例的 close 修复拆分到独立提交（或独立 PR），保持特性提交的单一职责。 |
| skill_functions: List[SkillToolFunction] = SKILL_REGISTRY.get_all() | ||
| skill_functions.extend(self._function_tools) | ||
| for skill_function in skill_functions: | ||
| try: | ||
| tools.append(FunctionTool(func=skill_function)) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| # Log error but continue loading other tools | ||
| logger.warning("Failed to get tools from skill '%s': %s", skill_function.__name__, ex) | ||
| continue | ||
|
|
||
| tools = self._exclude_tools(tools) | ||
| self._default_tools.extend(tools) | ||
| return tools |
There was a problem hiding this comment.
问题： 新增的 _default_tools 缓存把 SKILL_REGISTRY.get_all() 的快照冻结在首次 get_tools() 调用（第 184 行），此后所有调用都命中 return self._default_tools.copy()（第 176-177 行）跳过重新读取。变更前每次调用都重新读取进程级单例 SKILL_REGISTRY（_registry.py:25 的 SkillRegistry(SingletonBase)），运行期 register()/unregister() 会即时反映；现在注册晚于首次调用的技能函数永远不会出现在工具列表中，也没有任何失效/刷新路径。
触发条件： 任何在首个 get_tools() 调用之后调用 SKILL_REGISTRY.register(...)/unregister(...)（公开导出 API，skills/__init__.py:143）的场景，例如动态插件加载、运行期装载技能，或使用同一 SkillToolSet 实例的团队/多子代理流程。
实际影响： 新注册技能的工具对模型静默缺失，注销也不再生效，工具集内容在整个进程生命周期内陈旧，且无任何告警；这是本次变更引入的行为回归（变更前每次调用都重新求值）。
修正方向： 移除缓存，改为在每次调用时执行排除过滤后直接返回；若确需缓存，则为缓存增加显式失效机制（如监听 SKILL_REGISTRY 变更或提供 invalidate() API），并保证排除逻辑不依赖缓存即可生效。
| if self._default_tools: | ||
| return self._default_tools.copy() | ||
|
|
||
| tools: List[ToolABC] = [] | ||
| tools.append(self._load_tool) | ||
| tools.append(self._run_tool) | ||
| tools.append(self._exec_tool) | ||
| tools.extend(self._runtime_tools) | ||
| skill_functions: List[SkillToolFunction] = SKILL_REGISTRY.get_all() | ||
| skill_functions.extend(self._function_tools) | ||
| for skill_function in skill_functions: | ||
| try: | ||
| tools.append(FunctionTool(func=skill_function)) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| # Log error but continue loading other tools | ||
| logger.warning("Failed to get tools from skill '%s': %s", skill_function.__name__, ex) | ||
| continue | ||
|
|
||
| tools = self._exclude_tools(tools) | ||
| self._default_tools.extend(tools) | ||
| return tools |
There was a problem hiding this comment.
问题： 首次调用路径 return tools（第 195 行）返回的正是同时存入 _default_tools 的同一活列表（self._default_tools.extend(tools) 第 194 行），而后续调用返回 self._default_tools.copy()（第 177 行），返回对象身份语义不一致：任何调用方对首次调用的返回值做就地修改（append/remove/extend/sort）都会永久污染缓存，后续所有调用返回受损工具列表。
触发条件： 任何对 get_tools() 首次返回值执行就地变更的调用方。现有内置调用方（_tool_adapter.py:104、_claude_agent.py:501、GovernedSkillToolSet 列表推导）均只读，但缓存命中路径返回副本、未命中路径返回原列表的不对称性构成未文档化的契约陷阱，且无任何测试断言两次调用的结果一致性。
实际影响： 未来调用方（或对首调用结果做过滤/裁剪的集成代码）一旦就地修改，缓存即被污染，LLM 请求中出现重复或伪造的工具声明（重复函数声明可能导致模型拒绝请求或歧义工具调用），问题难以排查。
修正方向： 首次调用同样返回 self._default_tools.copy()（或构建到局部列表后 self._default_tools = tools 再返回 tools.copy()），保证所有路径返回的都是缓存副本。
| def _exclude_tools(self, tools: List[ToolABC]) -> List[ToolABC]: | ||
| """Exclude tools from the list.""" | ||
| if not self._excluded_tools: | ||
| return tools | ||
| available_tools: List[ToolABC] = [] | ||
| for tool in tools: | ||
| name = getattr(tool, "name", None) | ||
| if not name or name in self._excluded_tools: | ||
| continue | ||
| available_tools.append(tool) | ||
| return available_tools |
There was a problem hiding this comment.
问题： _exclude_tools 中 if not name or name in self._excluded_tools: continue（第 204 行）会把 name 为空/None 的工具静默丢弃——该行为仅在配置了 excluded_tools 时启用（空列表走第 199 行提前返回）。任何不带 name 属性的自定义 ToolABC 或运行时工具，只要用户开启了排除功能就会被无提示地移除；同时按名称匹配会误删与排除名同名的其他已注册技能函数（如用户注册了恰好名为 skill_list_tools 的官方 FunctionTool 以外的同名函数）。
触发条件： 用户传入非空 excluded_tools，且工具集包含名称缺失的工具或同名工具；默认内置工具均带名称，但框架公开的 ToolABC 扩展点允许任意自定义工具。
实际影响： 与排除意图无关的工具从 LLM 可见工具集中消失，模型能力静默降级；同名函数被全部排除时用户无法感知排除只应作用于目标工具。
修正方向： 仅跳过 name in self._excluded_tools 的匹配项，对 name 缺失的工具保留而非丢弃（或改为按工具对象/身份精确排除）；排除应只移除具名目标，不改变其余工具集。
| skill_toolset = SkillToolSet(repository=repository, run_tool_kwargs=tool_kwargs, | ||
| skill_stager=skill_stager, excluded_tools=["skill_list_tools"]) |
There was a problem hiding this comment.
问题： 特性落地不一致：本变更在此处通过 excluded_tools=["skill_list_tools"] 关闭了该示例工具集中的 skill_list_tools，但仅此一个示例生效——examples/skills_with_dynamic_tools 的提示词仍把 skill_list_tools 作为强制步骤（第 20、69、71、93 行，含 MUST call skill_list_tools、NEVER skip skill_list() and skill_list_tools()），而该示例构建 SkillToolSet（tools/_skill_tools.py:65）时并未排除该工具；examples/team_with_skill 本次删除了提示词中的调用步骤（prompts.py），但其工具集（tools.py:92）同样未排除，工具仍暴露给模型；docs/mkdocs/{en,zh}/skill.md 也仍把该工具描述为"必须调用"且返回"数组"。
触发条件： 运行 examples/skills_with_dynamic_tools 示例（或其提示词模式），模型按提示词强制调用 skill_list_tools 预览工具；或运行 examples/team_with_skill，模型仍可通过未排除的工具调用它。
实际影响： skill_list_tools 的返回结构已在本变更中从数组改为含 skill_name/scope/note 的字典（_skill_list_tool.py:44-53），提示词与文档中 → ["get_weather", "get_data"] 的示例和强制调用步骤与真实行为矛盾，模型解析结果出错或误解该工具失效，示例行为退化；同一特性在不同示例中的开关状态互相矛盾，用户无从判断该工具是否可用。
修正方向： 统一示例与文档：要么在所有示例中一致启用排除并同步修改提示词，要么保留工具并更新提示词/文档为新的字典返回结构；同时更新 docs/mkdocs 中 skill_list_tools 的返回值与工作流说明。
| self._excluded_tools: List[str] = excluded_tools or [] | ||
| self._default_tools: List[ToolABC] = [] |
There was a problem hiding this comment.
问题： 本次变更的核心新特性 excluded_tools 与 _default_tools 缓存完全无测试覆盖：全仓库 tests 中 excluded_tools 零引用，tests/skills/test_toolset.py 的 test_get_tools* 每个实例只调用一次 get_tools()，缓存命中路径、排除效果、以及排除与 SkillToolSet 集成的行为均未验证。
触发条件： 后续任何人回归 _exclude_tools（如按错误属性匹配、参数被忽略）或缓存路径（如返回原列表而非副本、重复累加），现有测试套件无法发现。
实际影响： 特性本身（"支持关闭skill_list_tools"）的保证不被任何自动化测试守护，示例中唯一的使用点（examples/skills/agent/tools.py:62）也无端到端验证；与 test_skill_list_tool.py 三个用例仅验证新返回字段形成反差。
修正方向： 在 tests/skills/test_toolset.py 增加：构造 SkillToolSet(excluded_tools=["skill_list_tools"]) 后断言 get_tools() 结果不含该工具但含其余 5 个功能工具；连续两次 get_tools() 断言长度相等且返回不同列表对象（验证缓存与副本语义）。
| async def close(self) -> None: | ||
| """Gracefully close the runner and release resources.""" | ||
| self._runner.close() | ||
| await self._runner.close() |
There was a problem hiding this comment.
问题： 本变更同时修复了 RunnerManager.close() 未 await 的问题（self._runner.close() → await self._runner.close()），经核对 Runner.close 为 async（runners.py:948），修复本身正确；但该修复与"支持关闭skill_list_tools"特性无关，混入同一提交使针对排除功能的回归定位/回滚范围扩大。
触发条件： 需要单独回滚或二分定位 excluded_tools 相关行为时。
实际影响： 无法独立回退关闭工具特性而不连带撤销该关闭相关的修补；提交范围混杂增大审查与维护成本。
修正方向： 将 FastAPI 示例的 close 修复拆分到独立提交（或独立 PR），保持特性提交的单一职责。
| skill_name, | ||
| "available_tools": | ||
| available_tools, | ||
| "scope": |
There was a problem hiding this comment.
这个scope的内容，放在note里吧，不需要单独搞成一个字段
| WorkspaceWriteStdinTool(workspace_exec_tool), | ||
| WorkspaceKillSessionTool(workspace_exec_tool), | ||
| ] | ||
| self._excluded_tools: List[str] = excluded_tools or [] |
There was a problem hiding this comment.
默认是否不需要带skill_list_tools这个工具
There was a problem hiding this comment.
不需要的话，可以考虑设置excluded_tools的默认值，或者用一个Option来设置
3881dac to
1e1dc74
Compare
AI Code Review审查结论不通过 审查范围：c98a61d..1e1dc74（feature: 将在 skill 中定义 tool 的特性独立出来，共 11 个文件，+119/-37）。计划意图是支持关闭 已逐一核查全部非生成变更文件： 主要发现：(1) 测试评估：新增/修改测试覆盖了默认集移除、opt-in 添加、二次调用去重和 发现的问题严重
问题： 本次变更将 触发条件： 任何按仓库文档（zh/en skill.md）使用 实际影响： 修正方向： 若“关闭”是新增能力而非默认移除，应为 中等
问题： 新增缓存逻辑 触发条件： 服务端场景中，代理构建与首次 实际影响： 新注册的技能函数永远不会出现在 LLM 可见工具列表中（调用即 修正方向： 让缓存失效与注册表版本绑定：例如为 较低
问题： 触发条件： 用户依据类文档/名称，想要“仅暴露已加载技能选中工具”的 token 优化行为而选用此类（或误认为其与 实际影响： 选用者得到的是与 修正方向： 将类文档改为如实描述“提供完整 SkillToolSet 工具集 + 可选附加 skill_list_tools/skill_select_tools 工具”的语义，或直接组合/继承 |
| self._function_tools: List[SkillToolFunction] = [ | ||
| skill_list, | ||
| skill_list_docs, | ||
| skill_list_tools, | ||
| skill_select_docs, | ||
| skill_select_tools, | ||
| ] |
There was a problem hiding this comment.
问题： 本次变更将 skill_list_tools 与 skill_select_tools 从 SkillToolSet 默认工具集（self._function_tools）中移除，改为仅由新类 SkillToolSetWithDynamicTools 在 get_tools 时附加提供，这使公开文档承诺的默认行为被静默破坏。docs/mkdocs/zh/skill.md:177-181 与 docs/mkdocs/en/skill.md:173-182 明确写明“通过 SkillToolSet 自动注册以下工具… skill_list_tools… skill_select_tools”，且文档后续章节（如 zh skill.md 548-660 行）的完整工作流指南均指导 LLM 调用这两个工具；仓库文档即公共契约，本次变更未更新任何文档，也未提供迁移路径或弃用警告。
触发条件： 任何按仓库文档（zh/en skill.md）使用 SkillToolSet(repository=...) 且提示词指导 LLM 调用 skill_list_tools/skill_select_tools 的既有代理升级到本版本，LLM 发起这两个工具调用时即触发。
实际影响： _tools_processor.py:138-145 对未解析工具生成 tool_not_found 错误事件并返回给 LLM，代理无法再列出/选择技能工具，原工作流（如“先调用 skill_list_tools 预览工具再加载”）永久失败；对外部库使用者是破坏性 API 行为变更，且没有版本化、配置开关或弃用提示，升级即回归。
修正方向： 若“关闭”是新增能力而非默认移除，应为 SkillToolSet 增加显式开关（如 enable_tool_selection: bool = False 参数）而非静默删减默认集合；同时同步更新 zh/en 的 skill.md 中所有将 skill_list_tools/skill_select_tools 描述为 SkillToolSet 默认工具的段落（含“自动注册”清单），并在 changelog/迁移说明中声明该不兼容变化。
| if self._default_tools: | ||
| return self._default_tools.copy() |
There was a problem hiding this comment.
问题： 新增缓存逻辑 if self._default_tools: return self._default_tools.copy()（本文件 169-170 行）使 get_tools 在首次调用后快照全部工具，但 SKILL_REGISTRY（SkillRegistry，公开导出 API，见 skills/__init__.py 与 _registry.py）在首次调用之后执行的 register/unregister/clear 不再反映到返回的工具集。变更前每次 get_tools 都重新执行 SKILL_REGISTRY.get_all()，注册新技能函数后下一次调用即可暴露；变更后 skill_functions 的读取（177 行）仅发生在首次填充 _default_tools 时。
触发条件： 服务端场景中，代理构建与首次 get_tools（如 AG-UI/Claude server 在启动或建卡时解析工具集）之后，同进程代码通过 SkillRegistry.register() 动态注册新技能函数，随后 LLM 请求再次调用 get_tools。
实际影响： 新注册的技能函数永远不会出现在 LLM 可见工具列表中（调用即 tool_not_found），unregister 也无效——注册表的动态语义被缓存静默破坏，且无任何日志提示；现有测试也未覆盖“注册新技能后再调用 get_tools”这一回归路径。
修正方向： 让缓存失效与注册表版本绑定：例如为 SkillRegistry 增加递增版本号或对注册表对象做身份/内容比对，get_tools 在注册表变化时重建 _default_tools；或仅在 _function_tools/静态工具上缓存，技能函数仍每次从 SKILL_REGISTRY.get_all() 取，并补充一个“调用 register 后 get_tools 返回新技能工具”的回归测试。
| class SkillToolSetWithDynamicTools(SkillToolSet): | ||
| """ToolSet that dynamically loads tools based on skill selections. | ||
|
|
||
| This toolset monitors skill loading state and tool selection state, then dynamically | ||
| provides only the tools that are selected for loaded skills. This approach saves tokens | ||
| by only including relevant tool definitions in the LLM context. | ||
| """ | ||
|
|
||
| @override | ||
| async def get_tools(self, invocation_context: Optional[InvocationContext] = None) -> List[ToolABC]: | ||
| """Get all tools from registered skills. | ||
|
|
||
| Args: | ||
| invocation_context: Optional invocation context (not used currently) | ||
|
|
||
| Returns: | ||
| List of tools from all registered skills | ||
| """ | ||
| if not self._default_tools: | ||
| self._function_tools.extend([skill_list_tools, skill_select_tools]) | ||
| return await super().get_tools(invocation_context) |
There was a problem hiding this comment.
问题： SkillToolSetWithDynamicTools 的类文档与 docstring（“monitors skill loading state and tool selection state, then dynamically provides only the tools that are selected”，宣称 token 优化）与其实际实现不符：实现只是当 self._default_tools 为空时向 _function_tools 静态追加 skill_list_tools/skill_select_tools 两个函数工具，不具备任何动态选择逻辑，返回的仍是 SkillToolSet 的全量工具集加两个附加工具。
触发条件： 用户依据类文档/名称，想要“仅暴露已加载技能选中工具”的 token 优化行为而选用此类（或误认为其与 DynamicSkillToolSet 行为等价）；另外注入动作 extend 依赖“首次调用时 _default_tools 为空”的隐含时序。
实际影响： 选用者得到的是与 SkillToolSet 几乎无差别的全量工具集（token 开销未按文档所述减少），与文档承诺的行为差距显著；若未来基类缓存初始化时序变化，工具注入易出现缺失或重复，成为脆弱设计点。
修正方向： 将类文档改为如实描述“提供完整 SkillToolSet 工具集 + 可选附加 skill_list_tools/skill_select_tools 工具”的语义，或直接组合/继承 DynamicSkillToolSet 实现真正的动态加载；同时在 docstring 中说明与 DynamicSkillToolSet 的区别，避免名称与行为误导。
AI Code Review审查结论通过 审查范围： 计划符合性：核心开关语义实现正确——默认 主要风险：1) 发现的问题中等
问题： 本变更把 触发条件： 使用默认 实际影响： LLM 被明确引导去调用一个默认工具集中并不存在的 修正方向： 本变更新增的 opt-in 类 中等
问题： 本提交改变了 触发条件： 用户按文档编写 agent（使用 实际影响： 文档与实现契约漂移，官方文档引导的行为在真实 SDK 中不可复现，直接降低该特性（支持关闭 修正方向： 在本提交内同步更新 较低
问题： 新增测试只覆盖了 触发条件： 当后续迭代修改 实际影响： 本次开关语义（默认关闭、opt-in 打开）的真实运行链路缺少回归保护，一旦上游行为变化，影响会被推迟到生产环境显现，而不是在 CI 中被拦截。 修正方向： 增加集成层测试：用一个真实（或 mock repository 的） |
| self._function_tools: List[SkillToolFunction] = [ | ||
| skill_list, | ||
| skill_list_docs, | ||
| skill_list_tools, | ||
| skill_select_docs, | ||
| skill_select_tools, | ||
| ] |
There was a problem hiding this comment.
问题： 本变更把 skill_select_tools/skill_list_tools 从默认 SkillToolSet 的工具集中移除，但 SkillsRequestProcessor 在 FULL profile（SkillProfileFlags 默认 select_tools=True）下仍通过 _tooling_guidance_text() 向系统提示注入“Use the skill_select_tools tool to select tools...”，导致提示指导与真实可用工具集不一致；且官方文档 docs/mkdocs/zh/skill.md 与 docs/mkdocs/en/skill.md（本次未修改）仍宣称这两个工具由 SkillToolSet 自动注册，用户按文档使用默认 SkillToolSet 时会得到一个默认不含这两个工具的提示词工作流。
触发条件： 使用默认 tool_profile="full"（或 resolve_flags 解析出 select_tools=True）构造 agent，并使用默认 SkillToolSet（即未选用新的 SkillToolSetWithDynamicTools），任何一次 process_llm_request 都会把该指导文本写入系统提示。
实际影响： LLM 被明确引导去调用一个默认工具集中并不存在的 skill_select_tools，调用将因工具未声明而失败或拒绝执行；配合文档仍宣称该工具自动可用，用户会观察到指导与行为互相矛盾，属于本次「默认关闭」变更引入的可观测回归。
修正方向： 本变更新增的 opt-in 类 SkillToolSetWithDynamicTools 是唯一应使用 skill_select_tools 指导的场景：可把该注入逻辑从默认指导中移除，改为在本变更新增的 SkillToolSetWithDynamicTools 注册路径（_dynamic_toolset.py）或对应文档中显式提供，并同步调整 _skill_profile.py 中 select_tools 相关指导；同时更新 docs/mkdocs/en/skill.md 与 docs/mkdocs/zh/skill.md，明确说明这两个工具仅在 opt-in 子类下自动可用。
| return { | ||
| "skill_name": skill_name, | ||
| "available_tools": available_tools, | ||
| } |
There was a problem hiding this comment.
问题： 本提交改变了 skill_list_tools 的返回值契约（由 {"available_tools": [...]} 变为 {"skill_name": ..., "available_tools": [...]}），但官方文档 docs/mkdocs/zh/skill.md:552-553（以及 docs/mkdocs/en/skill.md:556-557）仍描述「返回该技能在 SKILL.md 的 Tools: 部分定义的工具名称数组」，未体现新增的 skill_name 字段；同时两处文档（zh/skill.md:179-181、en/skill.md:177-184）仍宣称 SkillToolSet 自动注册 skill_list_tools/skill_select_tools，与本提交「默认不再注册」的语义冲突，且文档中的调用示例（zh/skill.md:575-585 等）仍要求 LLM 直接调用这两个工具。
触发条件： 用户按文档编写 agent（使用 SkillToolSet + 文档中的提示词工作流）时，LLM 会尝试调用 skill_list_tools/skill_select_tools 得到工具不存在错误，或解析返回值时按旧契约读取产生误解。
实际影响： 文档与实现契约漂移，官方文档引导的行为在真实 SDK 中不可复现，直接降低该特性（支持关闭 skill_list_tools）的可使用性，属于本次变更引入但对使用者可见的契约不一致。
修正方向： 在本提交内同步更新 docs/mkdocs/zh/skill.md 与 docs/mkdocs/en/skill.md：skill_list_tools 文档补充 skill_name 返回字段，并明确说明这两个工具仅在 SkillToolSetWithDynamicTools（opt-in）下自动可用、普通 SkillToolSet 默认不注册。
| @override | ||
| async def get_tools(self, invocation_context: Optional[InvocationContext] = None) -> List[ToolABC]: | ||
| """Get all tools from registered skills. | ||
|
|
||
| Args: | ||
| invocation_context: Optional invocation context (not used currently) | ||
|
|
||
| Returns: | ||
| List of tools from all registered skills | ||
| """ | ||
| if not self._default_tools: | ||
| self._function_tools.extend([skill_list_tools, skill_select_tools]) | ||
| return await super().get_tools(invocation_context) |
There was a problem hiding this comment.
问题： 新增测试只覆盖了 get_tools 单层行为（tests/skills/test_toolset.py:77-96 验证 opt-in 子类首次/二次调用不重复注入，tests/skills/tools/test_skill_list_tool.py 验证返回形状），但未覆盖 agent 级集成路径：skill_list_tools/skill_select_tools 从默认工具集移除后，_tools_processor.py:86 的 convert_toolunion_to_tool_list → toolset.get_tools() 全链路、_skills_tool_result_processor 对工具结果的处理、以及服务器/子代理（_BorrowedToolSet 委托）路径均无测试证明行为符合预期。
触发条件： 当后续迭代修改 SkillToolSetWithDynamicTools.get_tools 的注入时机或 _default_tools 缓存条件时（例如多轮对话中第二次 get_tools 被缓存、或 delete_skill/热加载后动态变化），现有单测无法发现工具列表与缓存的交互回归。
实际影响： 本次开关语义（默认关闭、opt-in 打开）的真实运行链路缺少回归保护，一旦上游行为变化，影响会被推迟到生产环境显现，而不是在 CI 中被拦截。
修正方向： 增加集成层测试：用一个真实（或 mock repository 的）SkillToolSetWithDynamicTools 走 convert_toolunion_to_tool_list → get_tools 全流程，断言 skill_list_tools/skill_select_tools 只出现一次；并补齐对默认 SkillToolSet 缓存路径的「第二轮 get_tools 返回与首轮相同」跨轮测试。
- 修改skill_list_tools 的函数注释,避免模型理解歧义 - 对于在 skill 中指定 tool 的特性独立为单独的类
1e1dc74 to
233d337
Compare
AI Code Review审查结论不通过 审查范围：git 范围 c98a61d..233d337（13 个文件，+209/-81），涉及 2 个测试文件。计划符合性：变更实现了「支持关闭 skill_list_tools tool」的目标——将 发现的问题严重
问题： 本次变更从 触发条件： 任何使用普通 实际影响： LLM 被指导调用 修正方向： 让指导文本的注入与实际暴露的工具对齐：例如仅在工具列表中实际包含 较低
问题： 新增的返回值 shape（ 触发条件： 读者按照新增文档章节学习 实际影响： 调用方按「数组」语义解析返回（例如 修正方向： 在 docs/mkdocs/zh/skill.md（以及对应的 en/skill.md 新增段落）的「返回值」小节中，将描述更新为「返回包含 |
| self._function_tools: List[SkillToolFunction] = [ | ||
| skill_list, | ||
| skill_list_docs, | ||
| skill_list_tools, | ||
| skill_select_docs, | ||
| skill_select_tools, | ||
| ] |
There was a problem hiding this comment.
问题： 本次变更从 SkillToolSet._function_tools 中移除了 skill_list_tools 与 skill_select_tools，但 SkillsRequestProcessor._tooling_guidance_text()（trpc_agent_sdk/agents/core/_skill_processor.py:544-547）仍会在 has_select_tools() 为真时向 LLM 的 system prompt 注入「- Use the skill_select_tools tool to select tools for the current task only when user asks for it.」指导文本。由于默认 tool_profile: "full"（_skill_config.py）使 select_tools=True，且该注入条件仅由与 toolset 无关的 SkillProfileFlags 控制，导致所有使用普通 SkillToolSet 且设置 skill_repository 的 agent 出现「提示词要求调用一个已不存在的工具」的不一致。
触发条件： 任何使用普通 SkillToolSet（即未切换到 SkillToolSetWithDynamicTools）并通过 skill_repository 触发 SkillsRequestProcessor 的 agent 发起 LLM 请求时都会触发；基版本（c98a61d）中该工具仍被暴露，因此这是本变更引入的回归。受影响方包括本仓库中未迁移的示例 examples/skills/agent/agent.py:37、examples/team_with_skill/agent/agent.py、examples/skills_with_container、examples/skills_with_cube、examples/skills_code_review_agent，以及 trpc_agent_sdk/server/openclaw/skill/_skill_tool.py:104 的 create_skill_tool_set()（普通 SkillToolSet + skill_repository）。
实际影响： LLM 被指导调用 skill_select_tools 但工具列表中不存在该工具，产生未定义的幻觉工具调用，被模型 API 拒绝（tool_call 指定的工具不存在），或模型因找不到工具而自行改变行为（跳过本应执行的选择步骤），导致请求失败或工具误用；在 openclaw 等生产服务中表现为偶发的失败请求与日志噪声，且难以排查。
修正方向： 让指导文本的注入与实际暴露的工具对齐：例如仅在工具列表中实际包含 skill_select_tools（即 agent 使用 SkillToolSetWithDynamicTools）时注入该行，或从 _tooling_guidance_text() 中删除该行并把动态工具工作流说明移到 SkillToolSetWithDynamicTools 专属文档/提示中。同步为默认按 SkillToolSet + skill_repository 构建的 agent 增加一条防护断言或引导文案，避免再次出现提示与工具集不一致。
| available_tools = list(skill.tools or []) | ||
| return { | ||
| "skill_name": skill_name, | ||
| "available_tools": available_tools, |
There was a problem hiding this comment.
问题： 新增的返回值 shape（{"skill_name": ..., "available_tools": ...}）与新增的文档段落（docs/mkdocs/zh/skill.md 的「返回值」小节）描述不一致，文档仍写着返回「该技能在 SKILL.md 的 Tools: 部分定义的工具名称数组」「如果技能未定义工具，返回空数组」，与实际的 dict 返回值不符。此外该 shape 变化是接口契约变更，仓库内没有同时迁移所有展示方（如 docs/mkdocs/zh/skill.md 新增代码块演示的调用结果示例）。
触发条件： 读者按照新增文档章节学习 skill_list_tools 的返回值，或调用方按文档假设返回值为数组而直接取索引时触发。
实际影响： 调用方按「数组」语义解析返回（例如 result[0]）会得到 dict 的键字符串而非工具名；文档与实现不一致会误导用户编写错误的解析代码，属于低严重度的文档-接口契约漂移。
修正方向： 在 docs/mkdocs/zh/skill.md（以及对应的 en/skill.md 新增段落）的「返回值」小节中，将描述更新为「返回包含 skill_name 与 available_tools 字段的对象，available_tools 为该技能声明的工具名数组，未定义工具时为空数组」，使文档与实现一致。
