Skip to content

feature: 支持关闭skill_list_tools tool - #323

Open
raychen911 wants to merge 1 commit into
mainfrom
feature/close_skill_list_tools
Open

feature: 支持关闭skill_list_tools tool#323
raychen911 wants to merge 1 commit into
mainfrom
feature/close_skill_list_tools

Conversation

@raychen911

Copy link
Copy Markdown
Contributor
  • 修改skill_list_tools 的函数注释,避免模型理解歧义
  • 支持关闭skill_list_tools

- 修改skill_list_tools 的函数注释,避免模型理解歧义
- 支持关闭skill_list_tools
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

通过

审查范围:f05797d..ccb4f8f 单个提交(feature: 支持关闭 skill_list_tools tool),5 个文件 +74/-25:SkillToolSet 新增 excluded_tools 参数、_exclude_tools 过滤方法与 get_tools 结果缓存 self._default_toolsskill_list_tools 返回结构增加 skill_name/scope/note 字段;examples/skills 示例启用排除;team_with_skill 提示词同步移除 skill_list_tools 步骤并重新编号;测试更新返回结构断言。

计划符合性:核心需求已实现——excluded_tools=["skill_list_tools"]FunctionTool 包装 skill_list_tools 函数得到的工具名一致,过滤生效;不传参数时默认行为与基线一致;skill_list_tools 返回结构变更为向后兼容的增量字段;示例与提示词改动内部自洽(步骤重编号正确,examples/skills 提示词不再引用该工具)。

主要风险:1) get_tools 新增缓存无失效、无同步,运行期通过 SkillRegistry 单例 register/unregister/clear 的技能函数在首次请求后被冻结(新注册不出现、已注销仍暴露),多线程并发首次调用可能产生重复工具列表;2) excluded_tools 仅在工具集层过滤,与 SkillsRequestProcessor 注入的系统提示词引导脱节,排除引导中点名的工具(skill_runskill_select_tools 等)会导致 LLM 调用不存在的工具并收到 tool_not_found 错误;3) skill_list_tools 将"技能不存在"与"技能未声明工具"混为同一响应,新增 note 的断言在 not-found 路径上不成立,可能误导模型;4) 提交主功能(排除与缓存)零测试覆盖。

测试充分性:test_skill_list_tool.py 覆盖了新返回结构的成功、未找到、无仓库、无工具路径,与实现一致;但 excluded_tools_exclude_tools 和缓存行为无任何用例。

门禁结论:未发现阻止合入的正确性/安全/数据损坏级缺陷,状态为 PASSED,附 2 条 MODERATE 与 2 条 LOW 评论。

发现的问题

中等

trpc_agent_sdk/skills/_toolset.py:175-193

问题: 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 或加锁,消除并发重复写入。

中等

trpc_agent_sdk/skills/_toolset.py:83-84

问题: 新增的 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 仅适用于引导文案未点名的工具,保证引导与实际可用工具集一致。

较低

trpc_agent_sdk/skills/tools/_skill_list_tool.py:42-48

问题: 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 的区分逻辑。

较低

trpc_agent_sdk/skills/_toolset.py:196-206

问题: 本次提交的核心功能——excluded_tools 参数、_exclude_tools 过滤逻辑和 _default_tools 缓存——没有任何测试覆盖。

触发条件: 运行现有测试套件即可确认:tests/skills/test_toolset.py 未新增用例且只断言默认工具存在,tests/skills/tools/test_skill_list_tool.py 仅断言返回结构,没有任何用例构造带 excluded_toolsSkillToolSet

实际影响: 排除名称拼写错误、过滤逻辑回归(例如误删具有合法名称的工具)或缓存行为破坏都不会被测试发现;"支持关闭 skill_list_tools" 这一提交主目标本身处于未验证状态。

修正方向:tests/skills/test_toolset.py 补充用例:默认包含 skill_list_tools;传入 excluded_tools=["skill_list_tools"] 后该工具被移除且其余工具保留;连续两次调用 get_tools 返回一致结果。

Comment on lines +175 to +193
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题: 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 或加锁,消除并发重复写入。

Comment on lines +83 to 84
excluded_tools: Optional[List[str]] = None,
**run_tool_kwargs: dict[str, Any]):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题: 新增的 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 仅适用于引导文案未点名的工具,保证引导与实际可用工具集一致。

Comment on lines +42 to +48
else:
available_tools = list(skill.tools or [])
return {
"skill_name":
skill_name,
"available_tools":
available_tools,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题: 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 的区分逻辑。

Comment on lines +196 to +206
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题: 本次提交的核心功能——excluded_tools 参数、_exclude_tools 过滤逻辑和 _default_tools 缓存——没有任何测试覆盖。

触发条件: 运行现有测试套件即可确认:tests/skills/test_toolset.py 未新增用例且只断言默认工具存在,tests/skills/tools/test_skill_list_tool.py 仅断言返回结构,没有任何用例构造带 excluded_toolsSkillToolSet

实际影响: 排除名称拼写错误、过滤逻辑回归(例如误删具有合法名称的工具)或缓存行为破坏都不会被测试发现;"支持关闭 skill_list_tools" 这一提交主目标本身处于未验证状态。

修正方向:tests/skills/test_toolset.py 补充用例:默认包含 skill_list_tools;传入 excluded_tools=["skill_list_tools"] 后该工具被移除且其余工具保留;连续两次调用 get_tools 返回一致结果。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants