Skip to content

feat(tools): 增加按需工具搜索与加载能力 - #368

Merged
weimch merged 1 commit into
mainfrom
feat/tool_search
Nov 6, 2026
Merged

weimch merged 1 commit into
mainfrom
feat/tool_search

Conversation

@raychen911

Copy link
Copy Markdown
Contributor
  • 背景: 当 Agent 配置大量工具时,完整工具定义会增加 Token
    消耗、降低工具选择准确率,并影响 Prompt Cache 稳定性。

  • 方案: 新增 ToolSearchToolSet,支持关键词及语义检索、Native 和 Dispatch
    调用模式、Session 状态持久化、权限过滤及动态 MCP
    工具加载;补充单测和中英文文档。

  • 示例: 新增 Tool Search
    示例,演示模型按需发现并加载天气工具,再完成多日天气查询。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

不通过

审查范围:base_commit 594bfb09..head_commit c615be8(feat(tools): 增加按需工具搜索与加载能力),17 个新文件、2335 行,核心为 trpc_agent_sdk/tools/tool_search/(_toolset.py 1060 行、_types.py、_semantic.py)、tools/__init__.py 导出、tests/tools/tool_search/test_tool_search.py(550 行)以及 examples/tool_search/ 与 README。

计划符合性:按需搜索(NATIVE/DISPATCH 双模式)、会话状态持久化发现结果、权限过滤、语义/关键词检索、MCP 动态工具命名空间等目标均有实现且有对应单元测试,整体方向符合计划。

主要风险(按严重性):(1) DISPATCH 模式下 _CallTool 自行排干 StreamingProgressTool 生成器,丢失所有进度事件与 skip_summarization 语义(SEVERE),且有测试固化了该有损行为;(2) _CallTool 不设置 ctx.function_call_id,依赖该 id 的 deferred 工具(Skill/artifact)归因错乱;(3) _RenamedLongRunningTool 初始化链断裂导致 is_long_running/func 缺失而 isinstance 仍为真,NATIVE+MCP 场景触发 LongRunningEvent 提前终止并可能在 AG-UI 侧重复执行;(4) catalog_in_description=False 时目录在每轮迭代被重复追加进系统指令,token 持续膨胀;(5) close() 不清 _dynamic_cache,关闭后列取失败仍回退到失效工具;(6) 工具重名冲突推迟到首个 get_tools 运行时才爆炸;(7) tool_names 与 queries 并集场景静默丢结果;(8) 示例 calculator 的 eval 支持 **,可被远程输入引发幂运算 DoS。

验证方法:对每个候选均在提交前/提交后代码路径上直接验证(_tools_processor.py 的 _split_calls_by_streaming/_execute_progress_streaming_tool/_execute_tool 三路径、_llm_agent.py 长任务与 skip_summarization 循环、_base_session_service.py 状态增量持久化、_base_tool.py 的 run_async 包装与参数错误响应、FunctionTool/LongRunningFunctionTool/StreamingProgressTool 构造链、session 服务的 state_delta 合并),排除的候选包括:dispatch 中 call_tool 的工具参数错误(run_async 包装仍会走 _get_argument_error_response,与 _execute_tool 的 _get_argument_error_response 语义等价)、system_instruction 为 Content 类型的损坏(LlmAgent 校验器强制其为 str)、tool_filter 语义(ToolSearchToolSet 的过滤语义与其它工具集不同但属设计差异)、.state 读取竞态(同一轮并行 tool_search+call_tool 竞态已在代码层面降低风险)。

测试充分性:单元测试覆盖面较好(关键词、语义、权限、状态、branch 隔离、close),但均为表级断言,缺少经完整 LlmAgent 循环的集成测试,导致 dispatch 流式/长任务语义偏差、重复目录追加、跨工具集重复声明等行为不被发现;test_dispatch_aggregates_progress_streaming_tool 反而固化了有损语义。

门禁结论:存在一个 SEVERE 问题(DISPATCH 模式丢失 progress-streaming 语义)与多个 MODERATE 行为偏差/资源问题,建议修复后合入。

发现的问题

严重

trpc_agent_sdk/tools/tool_search/_toolset.py:296-345

问题: _CallTool._run_async_impl 在 DISPATCH 模式下绕过了框架的 progress-streaming 执行路径:当目标是 StreamingProgressTool(is_progress_streaming=True)时,它直接在工具内部 async for ... entry.tool.run_streaming(...) 中把生成器排干,只把最后一个 yield 值作为结果返回(_toolset.py:334-341)。而 StreamingProgressTool 的契约(_streaming_progress_tool.py 模块文档与 _run_async_impl 的 RuntimeError 说明)要求必须经 ToolsProcessor._execute_progress_streaming_tool 驱动:每个 yield 生成一个 partial=True 的进度 Event,最终事件带 skip_summarization(_streaming_progress_tool.py:147-153)。同时 ToolsProcessor._split_calls_by_streaming 只会检查调用名对应的工具是否为 call_tool(其 is_progress_streaming=False),因此 call_tool 永远不会被路由到 streaming 桶。

触发条件: DISPATCH 模式下把 StreamingProgressTool(或声明 skip_summarization=True 的流式工具)放入 deferred 目录,模型调用 tool_search 加载后再调用 call_tool 执行。

实际影响: 客户端收不到任何进度事件(长任务只能白屏等待到结束);skip_summarization=True 被丢失,agent 循环不会提前终止,还会额外发起一轮 LLM 总结调用,产生与直接模式(NATIVE)不同的行为;且该路径没有先做 _execute_progress_streaming_tool 的错误兜底(_tools_processor.py:505-526 的参数错误 event 分支),run_streaming 内的异常会直接冒泡。且测试 test_dispatch_aggregates_progress_streaming_tool(tests/tools/tool_search/test_tool_search.py:374-401)断言的正是在这个有损行为,等于把缺陷固化进测试。

修正方向: 在 _CallTool._run_async_impl 中对 is_progress_streaming 条目不要自行排干生成器:改为把结果通过 context.event_actions(或为 call_tool 声明 is_progress_streaming=True 并让 ToolsProcessor 在 _split_calls_by_streaming 能解析到目标工具后按 streaming 路径执行)转发,保证 partial 事件与 skip_summarization 语义与 NATIVE 模式一致;同时修正该测试以断言完整语义。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:296-345

问题: _CallTool._run_async_impl 从不设置 tool_context.function_call_id,而 ToolsProcessor._execute_tool(_tools_processor.py:369-370)在每次执行前都会设置 context.function_call_id = tool_call.id。DISPATCH 模式下 call_tool 的调用体是 entry.tool.run_async(...)(_toolset.py:342),该调用把 tool_context 原样传给被调工具,被调工具(如 skills/_skill_run.py:199、tools/_save_artifact.py:98)读取 ctx.function_call_id 时会得到上一次执行工具遗留的 id 或 None。

触发条件: DISPATCH 模式下加载并调用一个内部依赖 ctx.function_call_id 做关联(如保存产物、Skill 运行记录)的 deferred 工具,且前一工具调用已发生。

实际影响: 本次调用被记录在上一个工具调用的 function_call id 下,导致产物、轨迹、历史归因错乱,无法正确关联到本次 call_tool 调用。

修正方向: 在 _CallTool._run_async_impl 开头用本次调用对应的调用标识设置 tool_context.function_call_id(或由 ToolsProcessor 在解析后的目标工具上统一设置),再执行 run_async/run_streaming。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:89-160

问题: _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool)(_toolset.py:159)的 MRO 链从不调用 LongRunningFunctionTool.__init__:_RenamedTool.__init__(_toolset.py:92-99)直接调用 BaseTool.__init__(未走 super()),因此 LongRunningFunctionTool.__init__(_long_running_tool.py:37-49)设置的 self.func、self.is_long_running 属性在包装实例上缺失,而 isinstance(tool, LongRunningFunctionTool) 仍为 True。

触发条件: NATIVE 模式下 MCP toolbox 返回 LongRunningFunctionTool,经 _rename_dynamic_tool(_toolset.py:680-683)包装、经 tool_search 加载后由模型调用该重命名工具。

实际影响: _llm_agent.py:643-647 的长期运行检测按 isinstance 命中该工具,把调用 id 加入 long_running_tool_ids,随后在 _llm_agent.py:674-690 生成 LongRunningEvent 并 return 提前终止 agent;但该实例并非真正的长期运行工具,且 AG-UI 的 EventTranslator.translate_lro_function_calls 可能据此在客户端重复执行一次工具(工具被调用两次)。测试 test_mcp_toolbox_preserves_long_running_marker 只用 isinstance 断言,无法发现该问题。

修正方向: 让 _RenamedLongRunningTool 调用 LongRunningFunctionTool.__init__(或显式设置 self.func/self.is_long_running = True),并补一个断言属性而不只是 isinstance 的测试。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:1034-1049

问题: _replace_catalog_placeholder 的无占位符分支(_toolset.py:1049)在每一轮请求/每个 process_request 调用时都无条件执行 request.append_instructions([catalog])。LlmAgent 主循环每轮迭代都会重建 LlmRequest 并重新运行 ToolsProcessor.process_llm_request(_llm_agent.py:487-507、_tools_processor.py:86-96),而系统指令在每轮都会重新组装且永远不含已注入的 catalog,因此目录文本在每个 loop 迭代(以及多轮对话)中重复追加一次。

触发条件: catalog_in_description=False(默认值)且使用含 deferred 工具目录的 ToolSearchToolSet;代理遇到工具循环/多轮会话(例如先 tool_search 再执行工具,再继续下一轮 LLM 调用)。

实际影响: 系统提示中的 catalog 文本随轮次线性累积(每轮一次完整目录),大目录场景下 token 消耗持续膨胀;与 README 声明“目录在 {deferred_tools_section} 占位符处注入一次”的语义不符。测试只覆盖了占位符存在(test_catalog_replaces_placeholder)与单次追加(test_tools_processor_refreshes_native_tools_after_search),未覆盖多轮重复追加。

修正方向: 在追加前检查系统指令是否已包含目录标记(如 <available-deferred-tools>/<toolbox-catalog> 头),已存在则跳过;或在 LlmRequest 上记录已注入标记。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:414-421

问题: close() 只关闭 MCP toolbox,不清理 _dynamic_cache。_build_snapshot 的异常分支(_toolset.py:470-481)在 MCP 列取失败时回退到 _dynamic_cache 中的最近一次非空快照;一旦工具集被关闭(runners.py:933 的 _cleanup_toolsets 会调用 ToolSearchToolSet.close()),后续任何 get_tools 调用若 MCP 列取失败,仍会静默使用缓存中指向已关闭连接的包装工具。

触发条件: agent 生命周期结束后(工具集已 close),又因延迟触发的请求(如 tool_search/call_tool 执行前重新解析工具、或行尾清理)调用 get_tools,且 MCP 服务器列取失败。

实际影响: 模型拿到已失效的工具并继续调用,执行时抛出连接已关闭的底层错误(而非干净的“工具不可用”拒绝),_dynamic_cache 中还残留已关闭工具的引用;同时每次 get_tools 都会在 MCP 列取上做 async with self._dynamic_refresh_locks[namespace] 等待,关闭后无法恢复。

修正方向: close() 中清空 _dynamic_cache 并置关闭标记,_build_snapshot 在已关闭状态下直接抛错而不是回退缓存;或让缓存失效逻辑与 close 联动。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:87

问题: _validate_configuration(_toolset.py:517-535)只校验 preset/deferred/toolbox 内部及跨 preset-deferred 的重名,没有校验「preset 工具与 toolbox 工具」「preset 与 deferred」之外的 toolbox 工具与 preset 工具之间的重名;而 _add_entry(_toolset.py:690-701)在 tool.name in entries 时直接 raise ValueError。工具箱工具名与 preset 工具名冲突时,构造期不报错,到首次 get_tools()——即第一轮 LLM 请求发起时——才抛出。

触发条件: toolbox 中声明了一个与 preset(如示例中的 calculator)同名的工具,或两个不同 namespace 的工具重名。

实际影响: 本应在配置期快速失败的错误被推迟到运行时,导致每轮请求都报 ValueError/请求失败,且错误信息出现在线上运行环境中而非开发配置阶段。

修正方向: 在 _validate_configuration 中把 toolbox 与 preset/deferred 的名字合并到同一 seen_names 集合统一校验,使冲突在构造期即抛出。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:562-620

问题: tool_names 优先分支(_toolset.py:575-578)完全忽略同时传入的 queries:_select_by_name 只按精确名解析,queries 被静默丢弃。_keyword_search(_toolset.py:796-830)则对复合词(含 -/_ 的 term)做了不对称处理:required 词经 _split_compound 拆分后逐部分匹配(_toolset.py:821),而 optional 词不拆分直接用于 _score_metadata。

触发条件: 模型/调用方同时传 tool_names 与 queries(希望“精确名+相关匹配”并集);或查询含 list-reminders 这类复合 optional 词需要按部分匹配召回。

实际影响: 前一种场景下相关工具永远不会被发现(静默丢失结果);后一种场景下仅按原始子串匹配,描述中分散出现各组成部分的工具得 0 分,排序行为与 required 词不一致,结果不稳定。测试仅覆盖单一查询形式(test_required_terms_and_overflow),未覆盖双参数组合。

修正方向: 在 _search 中对 tool_names 与 queries 做并集处理(分别选取后合并去重),并对 optional 复合词应用与 required 相同的 _split_compound 拆分逻辑。

较低

examples/tool_search/agent/tools.py:16-23

问题: 示例 calculator 工具(examples/tool_search/agent/tools.py:16-23)用 eval(expression, {"__builtins__": {}}, {}) 求值且字符集允许 * 与 ( )。虽然空 __builtins__ 与字符白名单(仅 0123456789.+-*/() )排除了属性访问/标识符,RCE 不可达,但 ** 幂运算链未被限制:9**9**9 右结合为 9^(9^9),结果约 3.7 亿位(实测 90 秒超时未完成、内存数百 MB);9**9**9**9 的指数本身即约 3.7 亿位,直接触发天文分配或 OOM。

触发条件: 任意聊天用户诱导模型调用 calculator(它是永远可见的 preset 工具),输入 9**9**9 或 9**9**9**9 这类表达式。

实际影响: 单进程 demo 服务 CPU/内存被占满直至超时或 OOM-kill,阻塞事件循环(eval 为同步调用),同进程其他请求全部卡死——远程可触发的拒绝服务。

修正方向: 移除 **(或整个幂运算)支持;限制表达式长度与整数位数;用 asyncio.wait_for 加执行超时;优先改为无 eval 的受限求值器。

Comment on lines +296 to +345
async def _run_async_impl(
self,
*,
tool_context: InvocationContext,
args: dict[str, Any],
) -> Any:
tool_name = str(args.get("tool_name", "")).strip()
params = args.get("params")
if not tool_name:
return {
"status": "error",
"message": "tool_name is required.",
}
if not isinstance(params, dict):
return {
"status": "error",
"message": "params must be an object.",
}
entry = _entry_by_name(self._snapshot, tool_name)
if entry is None:
return {
"status": "error",
"message": f"Unknown tool {tool_name!r}. Use tool_search to find it.",
}
if entry.deferred:
if self._allowed is not None and not self._allowed.get(entry.name, False):
return {
"status": "error",
"message": f"You do not have permission to use {entry.name!r}.",
}
loaded = self._owner._load_discovered(tool_context)
if entry.name not in loaded:
return {
"status":
"error",
"message": (f"Tool {entry.name!r} is not loaded yet. Call tool_search "
f"with tool_names=[{entry.name!r}] first."),
}
if entry.tool.is_progress_streaming:
last_value: Any = None
async for value in entry.tool.run_streaming(
tool_context=tool_context,
args=params,
):
last_value = value
return last_value
return await entry.tool.run_async(
tool_context=tool_context,
args=params,
)

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.

问题: _CallTool._run_async_impl 在 DISPATCH 模式下绕过了框架的 progress-streaming 执行路径:当目标是 StreamingProgressTool(is_progress_streaming=True)时,它直接在工具内部 async for ... entry.tool.run_streaming(...) 中把生成器排干,只把最后一个 yield 值作为结果返回(_toolset.py:334-341)。而 StreamingProgressTool 的契约(_streaming_progress_tool.py 模块文档与 _run_async_impl 的 RuntimeError 说明)要求必须经 ToolsProcessor._execute_progress_streaming_tool 驱动:每个 yield 生成一个 partial=True 的进度 Event,最终事件带 skip_summarization(_streaming_progress_tool.py:147-153)。同时 ToolsProcessor._split_calls_by_streaming 只会检查调用名对应的工具是否为 call_tool(其 is_progress_streaming=False),因此 call_tool 永远不会被路由到 streaming 桶。

触发条件: DISPATCH 模式下把 StreamingProgressTool(或声明 skip_summarization=True 的流式工具)放入 deferred 目录,模型调用 tool_search 加载后再调用 call_tool 执行。

实际影响: 客户端收不到任何进度事件(长任务只能白屏等待到结束);skip_summarization=True 被丢失,agent 循环不会提前终止,还会额外发起一轮 LLM 总结调用,产生与直接模式(NATIVE)不同的行为;且该路径没有先做 _execute_progress_streaming_tool 的错误兜底(_tools_processor.py:505-526 的参数错误 event 分支),run_streaming 内的异常会直接冒泡。且测试 test_dispatch_aggregates_progress_streaming_tool(tests/tools/tool_search/test_tool_search.py:374-401)断言的正是在这个有损行为,等于把缺陷固化进测试。

修正方向: 在 _CallTool._run_async_impl 中对 is_progress_streaming 条目不要自行排干生成器:改为把结果通过 context.event_actions(或为 call_tool 声明 is_progress_streaming=True 并让 ToolsProcessor 在 _split_calls_by_streaming 能解析到目标工具后按 streaming 路径执行)转发,保证 partial 事件与 skip_summarization 语义与 NATIVE 模式一致;同时修正该测试以断言完整语义。

Comment on lines +296 to +345
async def _run_async_impl(
self,
*,
tool_context: InvocationContext,
args: dict[str, Any],
) -> Any:
tool_name = str(args.get("tool_name", "")).strip()
params = args.get("params")
if not tool_name:
return {
"status": "error",
"message": "tool_name is required.",
}
if not isinstance(params, dict):
return {
"status": "error",
"message": "params must be an object.",
}
entry = _entry_by_name(self._snapshot, tool_name)
if entry is None:
return {
"status": "error",
"message": f"Unknown tool {tool_name!r}. Use tool_search to find it.",
}
if entry.deferred:
if self._allowed is not None and not self._allowed.get(entry.name, False):
return {
"status": "error",
"message": f"You do not have permission to use {entry.name!r}.",
}
loaded = self._owner._load_discovered(tool_context)
if entry.name not in loaded:
return {
"status":
"error",
"message": (f"Tool {entry.name!r} is not loaded yet. Call tool_search "
f"with tool_names=[{entry.name!r}] first."),
}
if entry.tool.is_progress_streaming:
last_value: Any = None
async for value in entry.tool.run_streaming(
tool_context=tool_context,
args=params,
):
last_value = value
return last_value
return await entry.tool.run_async(
tool_context=tool_context,
args=params,
)

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.

问题: _CallTool._run_async_impl 从不设置 tool_context.function_call_id,而 ToolsProcessor._execute_tool(_tools_processor.py:369-370)在每次执行前都会设置 context.function_call_id = tool_call.id。DISPATCH 模式下 call_tool 的调用体是 entry.tool.run_async(...)(_toolset.py:342),该调用把 tool_context 原样传给被调工具,被调工具(如 skills/_skill_run.py:199、tools/_save_artifact.py:98)读取 ctx.function_call_id 时会得到上一次执行工具遗留的 id 或 None。

触发条件: DISPATCH 模式下加载并调用一个内部依赖 ctx.function_call_id 做关联(如保存产物、Skill 运行记录)的 deferred 工具,且前一工具调用已发生。

实际影响: 本次调用被记录在上一个工具调用的 function_call id 下,导致产物、轨迹、历史归因错乱,无法正确关联到本次 call_tool 调用。

修正方向: 在 _CallTool._run_async_impl 开头用本次调用对应的调用标识设置 tool_context.function_call_id(或由 ToolsProcessor 在解析后的目标工具上统一设置),再执行 run_async/run_streaming。

Comment on lines +89 to +160
class _RenamedTool(BaseTool):
"""Expose a dynamic tool under an MCP-style globally unique name."""

def __init__(self, original: BaseTool, name: str) -> None:
BaseTool.__init__(
self,
name=name,
description=original.description,
filters=list(original.filters),
)
self._original = original

@property
@override
def is_streaming(self) -> bool:
return self._original.is_streaming

@property
@override
def is_progress_streaming(self) -> bool:
return self._original.is_progress_streaming

@property
@override
def api_variant(self) -> str:
return self._original.api_variant

@property
def skip_summarization(self) -> bool:
"""Preserve execution behavior of wrapped dynamic tools."""
return bool(getattr(self._original, "skip_summarization", False))

@override
def _get_declaration(self) -> Optional[FunctionDeclaration]:
declaration = self._original._get_declaration() # pylint: disable=protected-access
if declaration is None:
return None
return declaration.model_copy(
update={"name": self.name},
deep=True,
)

@override
async def _run_async_impl(
self,
*,
tool_context: InvocationContext,
args: dict[str, Any],
) -> Any:
return await self._original._run_async_impl( # pylint: disable=protected-access
tool_context=tool_context,
args=args,
)

async def run_streaming(
self,
*,
tool_context: InvocationContext,
args: dict[str, Any],
):
run_streaming = getattr(self._original, "run_streaming", None)
if run_streaming is None:
raise TypeError(f"Tool {self._original.name!r} does not support progress streaming", )
async for value in run_streaming(
tool_context=tool_context,
args=args,
):
yield value


class _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool):
"""Preserve the framework's long-running marker after namespace."""

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.

问题: _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool)(_toolset.py:159)的 MRO 链从不调用 LongRunningFunctionTool.__init__:_RenamedTool.__init__(_toolset.py:92-99)直接调用 BaseTool.__init__(未走 super()),因此 LongRunningFunctionTool.__init__(_long_running_tool.py:37-49)设置的 self.func、self.is_long_running 属性在包装实例上缺失,而 isinstance(tool, LongRunningFunctionTool) 仍为 True。

触发条件: NATIVE 模式下 MCP toolbox 返回 LongRunningFunctionTool,经 _rename_dynamic_tool(_toolset.py:680-683)包装、经 tool_search 加载后由模型调用该重命名工具。

实际影响: _llm_agent.py:643-647 的长期运行检测按 isinstance 命中该工具,把调用 id 加入 long_running_tool_ids,随后在 _llm_agent.py:674-690 生成 LongRunningEvent 并 return 提前终止 agent;但该实例并非真正的长期运行工具,且 AG-UI 的 EventTranslator.translate_lro_function_calls 可能据此在客户端重复执行一次工具(工具被调用两次)。测试 test_mcp_toolbox_preserves_long_running_marker 只用 isinstance 断言,无法发现该问题。

修正方向: 让 _RenamedLongRunningTool 调用 LongRunningFunctionTool.__init__(或显式设置 self.func/self.is_long_running = True),并补一个断言属性而不只是 isinstance 的测试。

Comment on lines +1034 to +1049
def _replace_catalog_placeholder(
request: LlmRequest,
catalog: str,
) -> None:
if request.config is None:
request.append_instructions([catalog] if catalog else [])
return
current = request.config.system_instruction
if current is not None and TOOL_SEARCH_PLACEHOLDER in str(current):
request.config.system_instruction = str(current).replace(
TOOL_SEARCH_PLACEHOLDER,
catalog,
1,
)
elif catalog:
request.append_instructions([catalog])

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.

问题: _replace_catalog_placeholder 的无占位符分支(_toolset.py:1049)在每一轮请求/每个 process_request 调用时都无条件执行 request.append_instructions([catalog])。LlmAgent 主循环每轮迭代都会重建 LlmRequest 并重新运行 ToolsProcessor.process_llm_request(_llm_agent.py:487-507、_tools_processor.py:86-96),而系统指令在每轮都会重新组装且永远不含已注入的 catalog,因此目录文本在每个 loop 迭代(以及多轮对话)中重复追加一次。

触发条件: catalog_in_description=False(默认值)且使用含 deferred 工具目录的 ToolSearchToolSet;代理遇到工具循环/多轮会话(例如先 tool_search 再执行工具,再继续下一轮 LLM 调用)。

实际影响: 系统提示中的 catalog 文本随轮次线性累积(每轮一次完整目录),大目录场景下 token 消耗持续膨胀;与 README 声明“目录在 {deferred_tools_section} 占位符处注入一次”的语义不符。测试只覆盖了占位符存在(test_catalog_replaces_placeholder)与单次追加(test_tools_processor_refreshes_native_tools_after_search),未覆盖多轮重复追加。

修正方向: 在追加前检查系统指令是否已包含目录标记(如 <available-deferred-tools>/<toolbox-catalog> 头),已存在则跳过;或在 LlmRequest 上记录已注入标记。

Comment on lines +414 to +421
async def close(self) -> None:
closed: set[int] = set()
for box in self._mcp_toolboxes:
if id(box.toolset) in closed:
continue
closed.add(id(box.toolset))
await box.toolset.close()

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.

问题: close() 只关闭 MCP toolbox,不清理 _dynamic_cache。_build_snapshot 的异常分支(_toolset.py:470-481)在 MCP 列取失败时回退到 _dynamic_cache 中的最近一次非空快照;一旦工具集被关闭(runners.py:933 的 _cleanup_toolsets 会调用 ToolSearchToolSet.close()),后续任何 get_tools 调用若 MCP 列取失败,仍会静默使用缓存中指向已关闭连接的包装工具。

触发条件: agent 生命周期结束后(工具集已 close),又因延迟触发的请求(如 tool_search/call_tool 执行前重新解析工具、或行尾清理)调用 get_tools,且 MCP 服务器列取失败。

实际影响: 模型拿到已失效的工具并继续调用,执行时抛出连接已关闭的底层错误(而非干净的“工具不可用”拒绝),_dynamic_cache 中还残留已关闭工具的引用;同时每次 get_tools 都会在 MCP 列取上做 async with self._dynamic_refresh_locks[namespace] 等待,关闭后无法恢复。

修正方向: close() 中清空 _dynamic_cache 并置关闭标记,_build_snapshot 在已关闭状态下直接抛错而不是回退缓存;或让缓存失效逻辑与 close 联动。

namespace_tools: dict[str, frozenset[str]]
namespace_descriptions: dict[str, str]
namespace_order: tuple[str, ...]

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.

问题: _validate_configuration(_toolset.py:517-535)只校验 preset/deferred/toolbox 内部及跨 preset-deferred 的重名,没有校验「preset 工具与 toolbox 工具」「preset 与 deferred」之外的 toolbox 工具与 preset 工具之间的重名;而 _add_entry(_toolset.py:690-701)在 tool.name in entries 时直接 raise ValueError。工具箱工具名与 preset 工具名冲突时,构造期不报错,到首次 get_tools()——即第一轮 LLM 请求发起时——才抛出。

触发条件: toolbox 中声明了一个与 preset(如示例中的 calculator)同名的工具,或两个不同 namespace 的工具重名。

实际影响: 本应在配置期快速失败的错误被推迟到运行时,导致每轮请求都报 ValueError/请求失败,且错误信息出现在线上运行环境中而非开发配置阶段。

修正方向: 在 _validate_configuration 中把 toolbox 与 preset/deferred 的名字合并到同一 seen_names 集合统一校验,使冲突在构造期即抛出。

Comment on lines +562 to +620
async def _search(
self,
ctx: InvocationContext,
snapshot: _Snapshot,
allowed: Optional[dict[str, bool]],
args: dict[str, Any],
) -> dict[str, Any]:
tool_names = _string_list(args.get("tool_names"))
queries = _string_list(args.get("queries"))
namespace = str(args.get("namespace", "")).strip()
requested_max = _positive_int(args.get("max_results"))
max_results = min(requested_max, _MAX_REQUEST_RESULTS) if requested_max else self.options.max_results

if tool_names:
selected = _select_by_name(snapshot, tool_names, allowed)
overflow: list[str] = []
else:
error = _validate_namespace(snapshot, namespace)
if error is not None:
return error
if namespace and not queries:
ranked = _namespace_names(snapshot, namespace, allowed)
selected, overflow = _split_results(ranked, max_results)
elif queries:
try:
selected, overflow = await self._search_queries(
snapshot,
allowed,
namespace,
queries,
max_results,
)
except Exception as exc: # pylint: disable=broad-except
if not self.options.embedding_fail_open or self._semantic_index is None:
raise
logger.warning(
"Semantic tool search failed; falling back to keyword search: %s",
exc,
)
ranked = _keyword_search(
snapshot,
namespace,
queries,
allowed,
)
selected, overflow = _split_results(ranked, max_results)
else:
selected, overflow = [], []

previously_loaded = set(self._load_discovered(ctx))
deferred_selected = [name for name in selected if name in snapshot.deferred_names]
self._append_discovered(ctx, deferred_selected)
return _format_result(
snapshot,
selected,
overflow,
previously_loaded,
self.options.invocation_mode,
)

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.

问题: tool_names 优先分支(_toolset.py:575-578)完全忽略同时传入的 queries:_select_by_name 只按精确名解析,queries 被静默丢弃。_keyword_search(_toolset.py:796-830)则对复合词(含 -/_ 的 term)做了不对称处理:required 词经 _split_compound 拆分后逐部分匹配(_toolset.py:821),而 optional 词不拆分直接用于 _score_metadata。

触发条件: 模型/调用方同时传 tool_names 与 queries(希望“精确名+相关匹配”并集);或查询含 list-reminders 这类复合 optional 词需要按部分匹配召回。

实际影响: 前一种场景下相关工具永远不会被发现(静默丢失结果);后一种场景下仅按原始子串匹配,描述中分散出现各组成部分的工具得 0 分,排序行为与 required 词不一致,结果不稳定。测试仅覆盖单一查询形式(test_required_terms_and_overflow),未覆盖双参数组合。

修正方向: 在 _search 中对 tool_names 与 queries 做并集处理(分别选取后合并去重),并对 optional 复合词应用与 required 相同的 _split_compound 拆分逻辑。

Comment on lines +16 to +23


async def calculator(expression: str) -> dict:
"""Evaluate a basic arithmetic expression."""
allowed = set("0123456789.+-*/() ")
if not expression or any(char not in allowed for char in expression):
return {"error": "Only basic arithmetic expressions are supported."}
return {"expression": expression, "result": eval(expression, {"__builtins__": {}}, {})}

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.

问题: 示例 calculator 工具(examples/tool_search/agent/tools.py:16-23)用 eval(expression, {"__builtins__": {}}, {}) 求值且字符集允许 * 与 ( )。虽然空 __builtins__ 与字符白名单(仅 0123456789.+-*/() )排除了属性访问/标识符,RCE 不可达,但 ** 幂运算链未被限制:9**9**9 右结合为 9^(9^9),结果约 3.7 亿位(实测 90 秒超时未完成、内存数百 MB);9**9**9**9 的指数本身即约 3.7 亿位,直接触发天文分配或 OOM。

触发条件: 任意聊天用户诱导模型调用 calculator(它是永远可见的 preset 工具),输入 9**9**9 或 9**9**9**9 这类表达式。

实际影响: 单进程 demo 服务 CPU/内存被占满直至超时或 OOM-kill,阻塞事件循环(eval 为同步调用),同进程其他请求全部卡死——远程可触发的拒绝服务。

修正方向: 移除 **(或整个幂运算)支持;限制表达式长度与整数位数;用 asyncio.wait_for 加执行超时;优先改为无 eval 的受限求值器。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

通过

本次审查范围为 b33f164..4aaa193(feat(tools): 增加按需工具搜索与加载能力),共 19 个变更文件(约 4323 行新增),核心为新增包 trpc_agent_sdk/tools/tool_search(_toolset.py/_semantic.py/_types.py)、ToolsProcessor 的 get_unresolved_tool 回退钩子、测试、示例与文档。计划符合性:核心设计(工具搜索、加载状态写入 Session State、NATIVE/DISPATCH 双模式、permission_filter、语义索引与降级、MCP 动态工具重命名与指纹清理)均按设计文档实现,经逐路径追踪未发现单一实例常规路径下的阻断性正确性/安全缺陷;测试(943 行)覆盖了搜索、加载、权限、并发、语义降级、MCP 指纹失效等主要场景,但未覆盖多 ToolSearchToolSet 实例(默认 name)共存的隔离场景,也未覆盖 MCP 工具下线/重命名后的 call_tool 行为。主要风险集中在多实例配置下的状态键冲突(第 2.11 节声明的 toolset_name 隔离在默认构造下不成立)、MCP 目录在每次 get_tools/find_tool 时全量重建与指纹重算的性能开销、_dynamic_tool_names 无界增长以及 embedding_fail_open 的宽泛异常吞噬。所有问题均为有界影响(MODERATE/LOW),无高置信的 SEVERE 缺陷,门禁结论为通过(PASSED)。

发现的问题

未发现需要反馈的代码问题。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

通过

审查范围:base_commit b33f164..head_commit 316b7fa 共 19 个文件(4345 行新增)。核心为新增的 ToolSearchToolSet 按需工具搜索与加载能力(_toolset.py 1403 行、_semantic.py 246 行、_types.py 92 行)、ToolsProcessor 新增 get_unresolved_tool 声明免加载回退机制(_tools_processor.py)、965 行测试及示例/文档。已逐行审查全部非生成变更文件,并追踪了 LlmAgent 工具执行循环、Session/State 提交链路、MCP ToolSet、StreamingProgressTool/LongRunningFunctionTool、权限过滤与并发路径。

计划符合性:与 design doc 的高层契约(快照每请求重建、状态键每工具独立写入、权限前置过滤、指纹校验防陈旧向量、embedding fail-open、并发搜索可交换 delta)实现一致,测试覆盖了主要成功/边界/失败路径,实现质量较好。

主要风险(均非阻塞):1) Dispatch 模式下 LongRunningFunctionTool 协议静默失效——_CallTool 直接执行目标工具,LlmAgent 的 isinstance(tool, LongRunningFunctionTool) 检测恒不命中 call_tool,长运行工具的暂停-恢复/LongRunningEvent 语义丢失,且无对应测试;2) 未解析工具调用路径每个 call 触发两次全量 _build_snapshot(含 MCP 串行网络 listing),批次含多个未解析调用时有明显延迟;3) _dynamic_tool_names 与指纹缓存只增不减,长生命周期进程内存缓慢增长;4) tool_names/queries 传非数组(模型格式错误)时静默返回空结果无错误引导;5) InMemoryToolSearchVectorStore.search 持锁做全量余弦计算,大目录并发搜索锁竞争。

测试充分性:30 个用例覆盖关键词/语义搜索、权限、并发状态合并、MCP 刷新、指纹防陈旧、两种调用模式,但缺少 Dispatch 模式 + LongRunningFunctionTool、以及 tool_names 非法输入格式用例。

门禁结论:未发现高置信的 SEVERE 级正确性/安全/数据损坏缺陷,状态为 PASSED;上述 MODERATE/LOW 问题建议合入前修复或追踪。

发现的问题

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:316-347

问题: _CallTool._run_async_impl(Dispatch 模式)直接执行 entry.tool.run_async(...),绕过了框架的 LongRunningFunctionTool 检测协议。LlmAgent._llm_agent.py:645-646 通过 find_tool 返回的工具是否 isinstance(tool, LongRunningFunctionTool) 来识别长运行调用并产出 LongRunningEvent(暂停 Agent 等人工介入);而 Dispatch 模式下 find_tool 返回的必然是 _CallTool 实例,long_running_tool_ids 恒为空,长运行工具的暂停-恢复契约静默失效。对比 Native 模式专门实现了 _RenamedLongRunningTool 保留该标记(_toolset.py:160),Dispatch 路径是本次变更新引入的行为缺口。

触发条件: InvocationMode.DISPATCH + 目录中含 LongRunningFunctionTool(或 MCP 服务器暴露长运行工具),模型通过 call_tool(tool_name, params) 调用该工具。

实际影响: 工具执行结果被当作普通工具响应继续驱动对话循环,永远不会产生 LongRunningEvent,框架层的暂停-人工介入-恢复流程失效;工具声明中"Do not call this tool again if it has already returned intermediate or pending status"的指令还会诱导模型放弃后续轮询,长运行任务结果可能被静默丢弃。

修正方向: 让 LlmAgent 的长运行检测识别 _CallTool 的目标工具(例如在 _CallTool 上暴露 resolved_tool 属性,检测时解包后做 isinstance 判断),或在 Dispatch 模式下对 entry.tool 为 LongRunningFunctionTool 的执行路径同样产出 LongRunningEvent,并补充对应测试。

中等

trpc_agent_sdk/agents/core/_tools_processor.py:348-363

问题: 新增的 _find_unresolved_tool(_tools_processor.py:348-363)在 __invoke_tools 和 find_tool 两个路径中对每个未解析的 tool_call 都会调用 ToolSearchToolSet.get_unresolved_tool → _build_snapshot,而后者会对每个 MCP toolbox 执行一次 box.toolset.get_tools(ctx) 网络调用,且被 _dynamic_refresh_locks(asyncio.Lock)串行化。LlmAgent 的 long-running 检测循环(_llm_agent.py:645)对同一批次每个 call 再叠加一轮 find_tool,导致含 N 个未解析调用的批次产生 2N 次全量 MCP listing。

触发条件: Native 模式下模型直接调用尚未加载的 deferred 工具(每个此类调用),且配置了 MCP toolbox;网络延迟越高、batch 中未解析调用越多,影响越大。

实际影响: 每次未解析调用额外付出 2 次完整目录重建和串行 MCP 网络往返(数百毫秒级),且同一批次内重复调用 get_tools 返回的 _UnloadedTool 均相同,工作完全冗余;对含多个未解析调用的批次构成可感知的延迟退化。

修正方向: _find_unresolved_tool 按工具名对结果做进程内缓存/复用(或在一次调用内批量查询多个名字),避免每个 call 重建 snapshot;get_unresolved_tool 可复用本次请求已构建的 snapshot(如通过 context 元数据传递)。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:540-556

问题: _build_snapshot 中 self._dynamic_tool_names.update(previous_fingerprints) 与 update(fresh_fingerprints) 只增不减:MCP 工具下线或更名时,旧名称从不从 _dynamic_tool_names 集合中移除(changed | removed 仅用于 forget_names 传给语义索引清理,集合本身不清理)。

触发条件: 长生命周期进程中,MCP 服务器工具列表反复增删/更名(例如灰度发布、服务重启后工具集变化)。

实际影响: _dynamic_tool_names 无限增长(每个历史工具名一个条目),且 _semantic_candidate_is_current(_toolset.py:938-947)每次搜索对其遍历,进程内存缓慢泄漏并增加语义候选校验开销。

修正方向: 在更新时按 cache_key 先 difference_update 掉已移除的名称(remove = set(previous_fingerprints) - set(fresh_fingerprints) 时同步从 _dynamic_tool_names 剔除),或直接用 _dynamic_fingerprint_cache 的值集合替代该集合。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:1016-1019

问题: _string_list(_toolset.py:1016-1019)对非 list/tuple 输入(例如模型按声明理解错误、把 tool_names 传成字符串 "get_forecast",或 queries 传成单个字符串)直接返回 [],_search 随后静默走"空查询"分支返回 "No matches. Try different queries, namespace, or tool_names.",模型无法得知是自己传参格式错误,可能反复重试造成额外 LLM 轮次浪费。

触发条件: tool_search 被调用时 tool_names 或 queries 字段为字符串而非数组(LLM 多轮生成中格式漂移是已知常见现象)。

实际影响: 本应明确报错的参数错误被吞掉,模型得到误导性"无匹配"结果并可能重复搜索,浪费 token 与轮次。

修正方向: 对非数组输入返回结构化错误(如 {"status": "invalid_args", "message": "tool_names must be an array of strings..."}),或在声明中为 tool_names/queries 明确 required 并加格式说明后,将非法输入映射为显式错误提示。

较低

trpc_agent_sdk/tools/tool_search/_semantic.py:62-75

问题: InMemoryToolSearchVectorStore.search(_semantic.py:62-75)在持有 threading.Lock 期间对全量向量计算余弦相似度(O(N·d)),锁粒度覆盖整个扫描+排序过程;SemanticToolIndex 的 search 每次查询还会对全部候选重新计算。

触发条件: 目录较大(数千个工具)且多请求并发执行语义搜索(每次搜索都完整遍历向量表)。

实际影响: 大目录下单个搜索持锁时间与候选数成正比,并发请求相互阻塞,语义检索吞吐下降;对含语义索引的在线服务场景构成可测量的性能退化。

修正方向: 先在锁内拷贝 self._vectors 快照(或在锁外基于不可变映射),锁外完成余弦计算与排序;或将 sorted 结果按需惰性计算,避免每次搜索全量排序。

Comment on lines +316 to +347
entry = _entry_by_name(self._snapshot, tool_name)
if entry is None:
return {
"status": "error",
"message": f"Unknown tool {tool_name!r}. Use tool_search to find it.",
}
if entry.deferred:
if self._allowed is not None and not self._allowed.get(entry.name, False):
return {
"status": "error",
"message": f"You do not have permission to use {entry.name!r}.",
}
loaded = self._owner._load_discovered(tool_context)
if entry.name not in loaded:
return {
"status":
"error",
"message": (f"Tool {entry.name!r} is not loaded yet. Call tool_search "
f"with tool_names=[{entry.name!r}] first."),
}
if entry.tool.is_progress_streaming:
chunks: list[Any] = []
async for value in entry.tool.run_streaming(
tool_context=tool_context,
args=params,
):
chunks.append(value)
return {"chunks": chunks}
return await entry.tool.run_async(
tool_context=tool_context,
args=params,
)

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.

问题: _CallTool._run_async_impl(Dispatch 模式)直接执行 entry.tool.run_async(...),绕过了框架的 LongRunningFunctionTool 检测协议。LlmAgent._llm_agent.py:645-646 通过 find_tool 返回的工具是否 isinstance(tool, LongRunningFunctionTool) 来识别长运行调用并产出 LongRunningEvent(暂停 Agent 等人工介入);而 Dispatch 模式下 find_tool 返回的必然是 _CallTool 实例,long_running_tool_ids 恒为空,长运行工具的暂停-恢复契约静默失效。对比 Native 模式专门实现了 _RenamedLongRunningTool 保留该标记(_toolset.py:160),Dispatch 路径是本次变更新引入的行为缺口。

触发条件: InvocationMode.DISPATCH + 目录中含 LongRunningFunctionTool(或 MCP 服务器暴露长运行工具),模型通过 call_tool(tool_name, params) 调用该工具。

实际影响: 工具执行结果被当作普通工具响应继续驱动对话循环,永远不会产生 LongRunningEvent,框架层的暂停-人工介入-恢复流程失效;工具声明中"Do not call this tool again if it has already returned intermediate or pending status"的指令还会诱导模型放弃后续轮询,长运行任务结果可能被静默丢弃。

修正方向: 让 LlmAgent 的长运行检测识别 _CallTool 的目标工具(例如在 _CallTool 上暴露 resolved_tool 属性,检测时解包后做 isinstance 判断),或在 Dispatch 模式下对 entry.tool 为 LongRunningFunctionTool 的执行路径同样产出 LongRunningEvent,并补充对应测试。

Comment on lines +348 to +363
async def _find_unresolved_tool(
self,
tool_call: FunctionCall,
context: InvocationContext,
) -> Optional[BaseTool]:
"""Ask dynamic toolsets for a declaration-free fallback tool."""
for configured in self.tools:
if not isinstance(configured, BaseToolSet):
continue
get_unresolved_tool = getattr(configured, "get_unresolved_tool", None)
if get_unresolved_tool is None:
continue
fallback = await get_unresolved_tool(tool_call.name, context)
if fallback is not None:
return fallback
return None

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.

问题: 新增的 _find_unresolved_tool(_tools_processor.py:348-363)在 __invoke_tools 和 find_tool 两个路径中对每个未解析的 tool_call 都会调用 ToolSearchToolSet.get_unresolved_tool → _build_snapshot,而后者会对每个 MCP toolbox 执行一次 box.toolset.get_tools(ctx) 网络调用,且被 _dynamic_refresh_locks(asyncio.Lock)串行化。LlmAgent 的 long-running 检测循环(_llm_agent.py:645)对同一批次每个 call 再叠加一轮 find_tool,导致含 N 个未解析调用的批次产生 2N 次全量 MCP listing。

触发条件: Native 模式下模型直接调用尚未加载的 deferred 工具(每个此类调用),且配置了 MCP toolbox;网络延迟越高、batch 中未解析调用越多,影响越大。

实际影响: 每次未解析调用额外付出 2 次完整目录重建和串行 MCP 网络往返(数百毫秒级),且同一批次内重复调用 get_tools 返回的 _UnloadedTool 均相同,工作完全冗余;对含多个未解析调用的批次构成可感知的延迟退化。

修正方向: _find_unresolved_tool 按工具名对结果做进程内缓存/复用(或在一次调用内批量查询多个名字),避免每个 call 重建 snapshot;get_unresolved_tool 可复用本次请求已构建的 snapshot(如通过 context 元数据传递)。

Comment on lines +540 to +556
fresh_fingerprints = {tool.name: _tool_fingerprint(tool) for tool in wrapped}
with self._dynamic_cache_lock:
previous_fingerprints = self._dynamic_fingerprint_cache.get(
cache_key,
{},
)
changed = {
name
for name, fingerprint in fresh_fingerprints.items()
if (name in previous_fingerprints and previous_fingerprints[name] != fingerprint)
}
removed = set(previous_fingerprints) - set(fresh_fingerprints)
self._dynamic_cache[cache_key] = wrapped
self._dynamic_fingerprint_cache[cache_key] = fresh_fingerprints
self._dynamic_tool_names.update(previous_fingerprints)
self._dynamic_tool_names.update(fresh_fingerprints)
forget_names = tuple(sorted(changed | removed))

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.

问题: _build_snapshot 中 self._dynamic_tool_names.update(previous_fingerprints) 与 update(fresh_fingerprints) 只增不减:MCP 工具下线或更名时,旧名称从不从 _dynamic_tool_names 集合中移除(changed | removed 仅用于 forget_names 传给语义索引清理,集合本身不清理)。

触发条件: 长生命周期进程中,MCP 服务器工具列表反复增删/更名(例如灰度发布、服务重启后工具集变化)。

实际影响: _dynamic_tool_names 无限增长(每个历史工具名一个条目),且 _semantic_candidate_is_current(_toolset.py:938-947)每次搜索对其遍历,进程内存缓慢泄漏并增加语义候选校验开销。

修正方向: 在更新时按 cache_key 先 difference_update 掉已移除的名称(remove = set(previous_fingerprints) - set(fresh_fingerprints) 时同步从 _dynamic_tool_names 剔除),或直接用 _dynamic_fingerprint_cache 的值集合替代该集合。

Comment on lines +1016 to +1019
def _string_list(value: Any) -> list[str]:
if not isinstance(value, (list, tuple)):
return []
return [str(item).strip() for item in value if str(item).strip()]

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.

问题: _string_list(_toolset.py:1016-1019)对非 list/tuple 输入(例如模型按声明理解错误、把 tool_names 传成字符串 "get_forecast",或 queries 传成单个字符串)直接返回 [],_search 随后静默走"空查询"分支返回 "No matches. Try different queries, namespace, or tool_names.",模型无法得知是自己传参格式错误,可能反复重试造成额外 LLM 轮次浪费。

触发条件: tool_search 被调用时 tool_names 或 queries 字段为字符串而非数组(LLM 多轮生成中格式漂移是已知常见现象)。

实际影响: 本应明确报错的参数错误被吞掉,模型得到误导性"无匹配"结果并可能重复搜索,浪费 token 与轮次。

修正方向: 对非数组输入返回结构化错误(如 {"status": "invalid_args", "message": "tool_names must be an array of strings..."}),或在声明中为 tool_names/queries 明确 required 并加格式说明后,将非法输入映射为显式错误提示。

Comment on lines +62 to +75
def search(
self,
query_vector: Sequence[float],
candidate_names: Sequence[str],
limit: int,
) -> list[str]:
allowed = set(candidate_names)
with self._lock:
scored = [(
_cosine_similarity(query_vector, vector),
name,
) for name, vector in self._vectors.items() if name in allowed]
scored.sort(key=lambda item: (-item[0], item[1]))
return [name for _, name in scored[:limit]]

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.

问题: InMemoryToolSearchVectorStore.search(_semantic.py:62-75)在持有 threading.Lock 期间对全量向量计算余弦相似度(O(N·d)),锁粒度覆盖整个扫描+排序过程;SemanticToolIndex 的 search 每次查询还会对全部候选重新计算。

触发条件: 目录较大(数千个工具)且多请求并发执行语义搜索(每次搜索都完整遍历向量表)。

实际影响: 大目录下单个搜索持锁时间与候选数成正比,并发请求相互阻塞,语义检索吞吐下降;对含语义索引的在线服务场景构成可测量的性能退化。

修正方向: 先在锁内拷贝 self._vectors 快照(或在锁外基于不可变映射),锁外完成余弦计算与排序;或将 sorted 结果按需惰性计算,避免每次搜索全量排序。

│ └─ namespace:限定 Toolbox │
│ │ │
│ ▼ │
│ InvocationContext.state:记录发现结果 │

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.

为啥要用state呢?看起来没有必要使用吧

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.

相当于是 一轮里,先让LLM看下一下要用哪个工具,然后再调一次,不需要跨轮会话保存的话,是不是放在 ctx其他栈字段上?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

当需要跨轮次、跨请求保留的业务状态,例如已发现工具。避免多次搜索

- 背景:
当 Agent 配置大量工具时,完整工具定义会增加 Token
消耗、降低工具选择准确率,并影响 Prompt Cache 稳定性。

- 方案:
新增 ToolSearchToolSet,支持关键词及语义检索、Native 和 Dispatch
调用模式、Session 状态持久化、权限过滤及动态 MCP
工具加载;补充单测和中英文文档。

- 示例:
新增 Tool Search
示例,演示模型按需发现并加载天气工具,再完成多日天气查询。

@weimch weimch left a comment

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.

Approve

@weimch
weimch merged commit 0eed179 into main Nov 6, 2026
5 checks passed
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

不通过

审查范围:base b33f164..head 2c24b31(19 个文件,+4382/-1),核心为新增的 ToolSearch 按需工具搜索/加载能力(trpc_agent_sdk/tools/tool_search/_toolset.py、_semantic.py、_types.py、_tools_processor.py 改造、示例与 965 行测试)。计划符合性:NATIVE/DISPATCH 双模式、静态 Toolbox + 动态 MCP、语义/关键词检索、State 持久化 discovered 名称、fingerprint 失效、权限过滤、进度流式与 guard-tool 等设计要点均落地且与 docs/design/02-tool-search-design.md 基本一致。主要风险:(1)SEVERE — 示例工具 calculator 对模型可控输入直接 eval,字符白名单放行 **,实测 9**9**9 可致 CPU/内存耗尽(>90s/1.5GB);该示例是特性文档的推荐用法,易被直接复制部署。(2)MODERATE — DISPATCH 模式经 _CallTool 直接 run_async 调用目录内工具,绕过框架 long_running_tool_ids/LongRunningEvent 协议,长期运行工具(如 ClientProxyTool)丢失进度事件与循环提前结束语义;DISPATCH 流式工具被排空为单条 {"chunks": [...]} 响应,丢失 skip_summarization 语义。(3)MODERATE — _find_unresolved_tool 无异常隔离,任一 MCP namespace 列表失败会把原本的按调用 tool_not_found 升级为整轮 agent_execution_failed。(4)MODERATE — namespace=null(模型常见行为)被 str() 成 "None",返回误导性错误。(5)LOW — _replace_catalog_placeholder 对 Content 类型 system_instruction 做 str() 破坏结构化指令;ToolPermissionFilter 未从 trpc_agent_sdk.tools 导出;_string_list 静默丢弃字符串参数;_RenamedLongRunningTool 未设置文档化的 is_long_running 属性;options.max_results 超过 10 时可绕过 _MAX_REQUEST_RESULTS 上限。测试充分性:965 行测试覆盖并行搜索状态合并、权限过滤、语义排序、MCP forget、DISPATCH chunks、NATIVE guard 引导等,质量高;但未覆盖 catalog_in_description 占位符替换分支、DISPATCH 下 get_unresolved_tool/长期运行/流式语义、跨 turn 持久化边界。门禁结论:存在 SEVERE 缺陷(示例代码可被构造输入造成拒绝服务),state = REVIEW_CODE_STATE_FAILED,建议修复后合入。审查过程仅做本地只读分析,未修改任何文件、依赖或 Git 状态。

发现的问题

严重

examples/tool_search/agent/tools.py:21-24

问题: 新增示例工具 calculator 对模型可控的 expression 直接调用内建 eval(expression, {"__builtins__": {}}, {})。字符白名单 set("0123456789.+-*/() ") 允许 * 与 **,指数运算可产生天文数字中间值,且 eval 仍可访问 ().__class__.__mro__ 等对象图导致信息泄露面扩大;该示例是本次 ToolSearch 特性的官方用法样例,会被直接复制进生产 agent。

触发条件: 用户/模型输入任意通过白名单的表达式,例如 9**9**9**9(仅含数字与 **,白名单放行);实测 9**9**9 在 30 秒超时下无法返回并持续消耗 CPU,1.5GB 内存上限下亦可打满内存。

实际影响: 单次工具调用即可拖垮承载该示例的 agent 进程或宿主(拒绝服务);多轮调用并发时影响叠加。1/0 之类会抛 ZeroDivisionError 的情况虽被框架包装成错误事件,但指数爆炸路径无任何终止条件。

修正方向: 移除 eval,改用 ast.literal_eval 配合受限求值器(如对 AST 节点做白名单校验后按二元运算逐步计算),并限制操作数位数/运算数;或在文档中明确示例仅供教学、不得直接部署。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:344-347

问题: DISPATCH 模式下 _CallTool._run_async_impl 对目录内工具直接 await entry.tool.run_async(...),绕过了框架基于 long_running_tool_ids + isinstance(tool, LongRunningFunctionTool) 的长期运行检测(_llm_agent.py:643-691):被调用的目标工具(如动态 MCP 的 ClientProxyTool,它是 LongRunningFunctionTool 子类)永远不会进入 long_running_tool_ids,也永远不会产生 LongRunningEvent。

触发条件: 以 InvocationMode.DISPATCH 配置包含长期运行工具(跨进程 RPC 型,如 client proxy)的目录,模型经 call_tool 调用它。

实际影响: NATIVE 模式下同样的工具调用会立即产出 LongRunningEvent 并结束本次 agent 循环、由 runner 异步接管;DISPATCH 模式则把整个 RPC 时长同步挂进 agent 循环,客户端进度事件丢失,取消/超时语义也与框架长期运行协议不一致,两种模式行为显著分叉且无文档说明。

修正方向: 在 _CallTool._run_async_impl 中检测 isinstance(entry.tool, LongRunningFunctionTool),将工具 id 上报到 long_running_tool_ids/运行器,或显式标注 DISPATCH 不支持长期运行协议并在文档与校验中拒绝此类工具组合。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:336-343

问题: DISPATCH 模式下 _CallTool._run_async_impl 把 StreamingProgressTool 的 run_streaming 整体排空进 {"chunks": [...]} 单条 function_response,既不产出阶段性的 progress 事件,也不设置 skip_summarization,与 NATIVE 模式下 _execute_progress_streaming_tool(_tools_processor.py:600-604 设置 final_event.actions.skip_summarization = True)的行为不同。

触发条件: 目录中包含 StreamingProgressTool,以 DISPATCH 模式运行时模型经 call_tool 调用之。

实际影响: 用户的最终答案(流式末值)与中间 chunk 一起被打包回给模型后,模型总是被要求再做一次总结性 LLM 往返(NATIVE 下本应直接以工具输出收尾);chunks 与“最终应答”的区分(last_value 语义)也丢失,长流式任务下体验与成本均退化,且同一工具在两种模式下的循环行为不一致。

修正方向: 在 DISPATCH 流式排空路径中保留末值作为最终 function_response、将中间 chunk 作为 progress 事件产出,并按框架约定设置 skip_summarization,使两种模式的收敛语义一致。

中等

trpc_agent_sdk/agents/core/_tools_processor.py:348-363

问题: 新增的 _find_unresolved_tool 对每个配置的 toolset 依次 await get_unresolved_tool(...),无任何 try/except 隔离;而 ToolSearchToolSet.get_unresolved_tool(_toolset.py:440-462)内部会执行 _build_snapshot,该过程会调用 MCP box.toolset.get_tools(ctx)(网络调用)且首列失败时直接 raise,_normalize_dynamic_tools 在 strict_validation 下对冲突声明也会抛 ValueError。

触发条件: 任一 MCP namespace 瞬时不可达、列表为空且无缓存(首列),或动态声明冲突触发 ValueError;此时只要模型调用了未知名工具(进入本回退),异常即从 __invoke_tools 第 138-140 行(位于该函数 try: 之外)或 find_tool(_llm_agent.py:645 长期运行预检复用)向上传播。

实际影响: 变更前未知工具调用只会产生按调用的 tool_not_found 错误事件;变更后同一个瞬态故障会升级为整轮 agent_execution_failed/run 中止,即使出错的是无关的辅助 toolset、目标工具可由其他 toolset 服务。

修正方向: 对每个 toolset 的 get_unresolved_tool 调用包 try/except,失败时记录日志并 continue,保持回退机制 best-effort 语义;同时让 _build_snapshot 的首列失败也走“降级为上次快照”而非向上抛出。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:792

问题: _search 中 namespace = str(args.get("namespace", "")).strip():当模型对可选参数显式传 namespace: null 时,args.get 返回 None,str(None) 得到字符串 "None",随后 _validate_namespace(snapshot, "None") 因不存在该命名空间而返回误导性错误。

触发条件: 工具调用模型(尤其是按结构化参数填充全部字段的模型)对可选字段 namespace 输出 null——这是常见行为,schema 中该字段为可选字符串。

实际影响: 用户本来只是想在全局范围搜索,却收到形如“Unknown namespace 'None'”的报错,模型据此反复重试或放弃,搜索结果功能在此输入下不可用;错误信息还会把不存在的命名空间伪造成真实存在。

修正方向: 对 falsy 值统一归空,例如 namespace = str(args.get("namespace") or "").strip(),并补充 namespace=null 的测试用例。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:1384-1392

问题: _replace_catalog_placeholder 在 system_instruction 非 None 且含占位符时执行 request.config.system_instruction = str(current).replace(...);GenerateContentConfig.system_instruction 的类型是 Optional[ContentUnion](可为 Content 或 Part 列表,google-genai 类型),对结构化类型执行 str() 得到的是 repr 字符串,随后直接覆盖原指令。

触发条件: 调用方以 Content/Part 列表形式(examples 之外更复杂的使用方式,如与多段系统指令组合)配置包含 TOOL_SEARCH_PLACEHOLDER 的 system_instruction,模型请求经 process_request 过滤。

实际影响: 原本应原样渲染为文本的结构化系统指令被替换成 Content(parts=[Part(text=...)]...) 形式的 repr,模型收到的系统提示变成转义过的垃圾文本,指令语义被静默破坏;该分支无任何测试覆盖。

修正方向: 仅当 current 为 str 时做原地替换,非 str 时提取文本后再拼装(或调用方在构造时就以字符串占位),并在测试中覆盖 Content 类型 system_instruction 的替换路径。

较低

trpc_agent_sdk/tools/__init__.py:79-91

问题: 本次变更从 trpc_agent_sdk.tools 重新导出了 ToolSearchOptions 等类型(_types.py 位于 tools/tool_search/),但 ToolSearchOptions.permission_filter 字段所引用的 ToolPermissionFilter 类型别名未随其他名称一并导出:grep 确认 trpc_agent_sdk/tools/__init__.py 中零处出现该名称,用户只能从 trpc_agent_sdk.tools.tool_search 深路径导入。

触发条件: 用户按公共面习惯写 from trpc_agent_sdk.tools import ToolPermissionFilter 以标注 permission_filter 回调签名。

实际影响: 公共 API 导出不一致,类型标注场景出现 ImportError 或被迫依赖内部路径;对依赖此 SDK API 面的兼容性造成不必要的摩擦。

修正方向: 在 trpc_agent_sdk/tools/__init__.py 的 __all__ 中一并导出 ToolPermissionFilter(连同其他 ToolSearch* 名称)。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:1016-1019

问题: _string_list 对非 list/tuple 入参一律返回 []:若模型把 tool_names 以裸字符串(如 "read_file")而非数组形式传出,该参数被静默丢弃,与 namespace(str() 强转)、max_results(_positive_int 强转)等兄弟解析器的容错行为不一致。

触发条件: 宽松模式下模型对 tool_names 输出字符串字面量或空值。

实际影响: 搜索静默退化为空查询的“No matches”,返回给模型的提示(如“Try different queries, namespace, or tool_names”)未指明入参被丢弃,模型无法自我纠正,多轮空转。

修正方向: 将裸字符串容错为 [value],或对非数组类型返回显式错误字典(如 {"status": "error", "message": "tool_names must be a list"})给模型可恢复信号。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:160-161

问题: _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool) 没有自己的 __init__,实例化走 _RenamedTool.__init__ → BaseTool.__init__,因此 LongRunningFunctionTool.__init__ 中设置的文档化公开属性 self.is_long_running = True(_long_running_tool.py:49)在改名后的实例上从未被设置——MRO 模拟确认 getattr(tool, "is_long_running", False) 为 False,尽管 isinstance(tool, LongRunningFunctionTool) 为 True。

触发条件: 任一 NATIVE 模式消费者(框架代码或用户代码)按 is_long_running 属性而非 isinstance 判断重命名后的长期运行工具。

实际影响: 类声明的契约与实际属性不一致:_llm_agent.py 等当前消费者恰好都用 isinstance,此前依赖没有暴露问题,但任何按属性探测的消费方(包括用户对 get_tools 输出的自检)都会把长期运行 MCP 工具误判为普通工具,长期运行语义被静默丢失。

修正方向: 为 _RenamedLongRunningTool 补 __init__(调用 LongRunningFunctionTool.__init__ 或显式设置 self.is_long_running = True),使属性和类型检查一致。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:793-794

问题: _search 中 max_results = min(requested_max, _MAX_REQUEST_RESULTS) if requested_max else self.options.max_results:每次请求的 _MAX_REQUEST_RESULTS = 10 硬上限只作用于模型显式传入的 max_results,而配置项 ToolSearchOptions.max_results(默认 5,可配成任意大)在模型未传参数时被直接采用,不受 10 的上限约束。

触发条件: 操作者将 options.max_results 配置为大于 10,且模型搜索时不传 max_results(正常省略行为)。

实际影响: 单次搜索可返回超过 _MAX_REQUEST_RESULTS 预设上限的候选结果,每次搜索响应中渲染的文档声明与描述随之膨胀,超出设计预期 token 开销;_split_results 的 overflow 逻辑也与“上限 10”的意图不一致。

修正方向: 对配置默认值同样施加 _MAX_REQUEST_RESULTS 上限(或对 max_results 上限进行配置校验),保证两种取值路径的上界一致。

Comment on lines +21 to +24
if not expression or any(char not in allowed for char in expression):
return {"error": "Only basic arithmetic expressions are supported."}
return {"expression": expression, "result": eval(expression, {"__builtins__": {}}, {})}

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.

问题: 新增示例工具 calculator 对模型可控的 expression 直接调用内建 eval(expression, {"__builtins__": {}}, {})。字符白名单 set("0123456789.+-*/() ") 允许 * 与 **,指数运算可产生天文数字中间值,且 eval 仍可访问 ().__class__.__mro__ 等对象图导致信息泄露面扩大;该示例是本次 ToolSearch 特性的官方用法样例,会被直接复制进生产 agent。

触发条件: 用户/模型输入任意通过白名单的表达式,例如 9**9**9**9(仅含数字与 **,白名单放行);实测 9**9**9 在 30 秒超时下无法返回并持续消耗 CPU,1.5GB 内存上限下亦可打满内存。

实际影响: 单次工具调用即可拖垮承载该示例的 agent 进程或宿主(拒绝服务);多轮调用并发时影响叠加。1/0 之类会抛 ZeroDivisionError 的情况虽被框架包装成错误事件,但指数爆炸路径无任何终止条件。

修正方向: 移除 eval,改用 ast.literal_eval 配合受限求值器(如对 AST 节点做白名单校验后按二元运算逐步计算),并限制操作数位数/运算数;或在文档中明确示例仅供教学、不得直接部署。

Comment on lines +344 to +347
return await entry.tool.run_async(
tool_context=tool_context,
args=params,
)

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.

问题: DISPATCH 模式下 _CallTool._run_async_impl 对目录内工具直接 await entry.tool.run_async(...),绕过了框架基于 long_running_tool_ids + isinstance(tool, LongRunningFunctionTool) 的长期运行检测(_llm_agent.py:643-691):被调用的目标工具(如动态 MCP 的 ClientProxyTool,它是 LongRunningFunctionTool 子类)永远不会进入 long_running_tool_ids,也永远不会产生 LongRunningEvent。

触发条件: 以 InvocationMode.DISPATCH 配置包含长期运行工具(跨进程 RPC 型,如 client proxy)的目录,模型经 call_tool 调用它。

实际影响: NATIVE 模式下同样的工具调用会立即产出 LongRunningEvent 并结束本次 agent 循环、由 runner 异步接管;DISPATCH 模式则把整个 RPC 时长同步挂进 agent 循环,客户端进度事件丢失,取消/超时语义也与框架长期运行协议不一致,两种模式行为显著分叉且无文档说明。

修正方向: 在 _CallTool._run_async_impl 中检测 isinstance(entry.tool, LongRunningFunctionTool),将工具 id 上报到 long_running_tool_ids/运行器,或显式标注 DISPATCH 不支持长期运行协议并在文档与校验中拒绝此类工具组合。

Comment on lines +336 to +343
if entry.tool.is_progress_streaming:
chunks: list[Any] = []
async for value in entry.tool.run_streaming(
tool_context=tool_context,
args=params,
):
chunks.append(value)
return {"chunks": chunks}

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.

问题: DISPATCH 模式下 _CallTool._run_async_impl 把 StreamingProgressTool 的 run_streaming 整体排空进 {"chunks": [...]} 单条 function_response,既不产出阶段性的 progress 事件,也不设置 skip_summarization,与 NATIVE 模式下 _execute_progress_streaming_tool(_tools_processor.py:600-604 设置 final_event.actions.skip_summarization = True)的行为不同。

触发条件: 目录中包含 StreamingProgressTool,以 DISPATCH 模式运行时模型经 call_tool 调用之。

实际影响: 用户的最终答案(流式末值)与中间 chunk 一起被打包回给模型后,模型总是被要求再做一次总结性 LLM 往返(NATIVE 下本应直接以工具输出收尾);chunks 与“最终应答”的区分(last_value 语义)也丢失,长流式任务下体验与成本均退化,且同一工具在两种模式下的循环行为不一致。

修正方向: 在 DISPATCH 流式排空路径中保留末值作为最终 function_response、将中间 chunk 作为 progress 事件产出,并按框架约定设置 skip_summarization,使两种模式的收敛语义一致。

Comment on lines +348 to +363
async def _find_unresolved_tool(
self,
tool_call: FunctionCall,
context: InvocationContext,
) -> Optional[BaseTool]:
"""Ask dynamic toolsets for a declaration-free fallback tool."""
for configured in self.tools:
if not isinstance(configured, BaseToolSet):
continue
get_unresolved_tool = getattr(configured, "get_unresolved_tool", None)
if get_unresolved_tool is None:
continue
fallback = await get_unresolved_tool(tool_call.name, context)
if fallback is not None:
return fallback
return None

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.

问题: 新增的 _find_unresolved_tool 对每个配置的 toolset 依次 await get_unresolved_tool(...),无任何 try/except 隔离;而 ToolSearchToolSet.get_unresolved_tool(_toolset.py:440-462)内部会执行 _build_snapshot,该过程会调用 MCP box.toolset.get_tools(ctx)(网络调用)且首列失败时直接 raise,_normalize_dynamic_tools 在 strict_validation 下对冲突声明也会抛 ValueError。

触发条件: 任一 MCP namespace 瞬时不可达、列表为空且无缓存(首列),或动态声明冲突触发 ValueError;此时只要模型调用了未知名工具(进入本回退),异常即从 __invoke_tools 第 138-140 行(位于该函数 try: 之外)或 find_tool(_llm_agent.py:645 长期运行预检复用)向上传播。

实际影响: 变更前未知工具调用只会产生按调用的 tool_not_found 错误事件;变更后同一个瞬态故障会升级为整轮 agent_execution_failed/run 中止,即使出错的是无关的辅助 toolset、目标工具可由其他 toolset 服务。

修正方向: 对每个 toolset 的 get_unresolved_tool 调用包 try/except,失败时记录日志并 continue,保持回退机制 best-effort 语义;同时让 _build_snapshot 的首列失败也走“降级为上次快照”而非向上抛出。

) -> dict[str, Any]:
tool_names = _string_list(args.get("tool_names"))
queries = _string_list(args.get("queries"))
namespace = str(args.get("namespace", "")).strip()

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.

问题: _search 中 namespace = str(args.get("namespace", "")).strip():当模型对可选参数显式传 namespace: null 时,args.get 返回 None,str(None) 得到字符串 "None",随后 _validate_namespace(snapshot, "None") 因不存在该命名空间而返回误导性错误。

触发条件: 工具调用模型(尤其是按结构化参数填充全部字段的模型)对可选字段 namespace 输出 null——这是常见行为,schema 中该字段为可选字符串。

实际影响: 用户本来只是想在全局范围搜索,却收到形如“Unknown namespace 'None'”的报错,模型据此反复重试或放弃,搜索结果功能在此输入下不可用;错误信息还会把不存在的命名空间伪造成真实存在。

修正方向: 对 falsy 值统一归空,例如 namespace = str(args.get("namespace") or "").strip(),并补充 namespace=null 的测试用例。

Comment on lines +1384 to +1392
current = request.config.system_instruction
if current is not None and TOOL_SEARCH_PLACEHOLDER in str(current):
request.config.system_instruction = str(current).replace(
TOOL_SEARCH_PLACEHOLDER,
catalog,
1,
)
elif catalog:
request.append_instructions([catalog])

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.

问题: _replace_catalog_placeholder 在 system_instruction 非 None 且含占位符时执行 request.config.system_instruction = str(current).replace(...);GenerateContentConfig.system_instruction 的类型是 Optional[ContentUnion](可为 Content 或 Part 列表,google-genai 类型),对结构化类型执行 str() 得到的是 repr 字符串,随后直接覆盖原指令。

触发条件: 调用方以 Content/Part 列表形式(examples 之外更复杂的使用方式,如与多段系统指令组合)配置包含 TOOL_SEARCH_PLACEHOLDER 的 system_instruction,模型请求经 process_request 过滤。

实际影响: 原本应原样渲染为文本的结构化系统指令被替换成 Content(parts=[Part(text=...)]...) 形式的 repr,模型收到的系统提示变成转义过的垃圾文本,指令语义被静默破坏;该分支无任何测试覆盖。

修正方向: 仅当 current 为 str 时做原地替换,非 str 时提取文本后再拼装(或调用方在构造时就以字符串占位),并在测试中覆盖 Content 类型 system_instruction 的替换路径。

Comment on lines +79 to 91
from .tool_search import InvocationMode as ToolSearchInvocationMode
from .tool_search import InMemoryToolSearchVectorStore
from .tool_search import MCPToolbox as ToolSearchMCPToolbox
from .tool_search import SemanticToolIndex
from .tool_search import Toolbox as ToolSearchToolbox
from .tool_search import ToolSearchEmbeddingUsage
from .tool_search import ToolSearchOptions
from .tool_search import ToolSearchToolSet
from .tool_search import ToolSearchVectorStore
from .tool_search import TOOL_SEARCH_PLACEHOLDER
from .tool_search import TOOL_SEARCH_TOOL_NAME
from .tool_search import CALL_TOOL_NAME
from .goal_tools import GoalOptions

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.

问题: 本次变更从 trpc_agent_sdk.tools 重新导出了 ToolSearchOptions 等类型(_types.py 位于 tools/tool_search/),但 ToolSearchOptions.permission_filter 字段所引用的 ToolPermissionFilter 类型别名未随其他名称一并导出:grep 确认 trpc_agent_sdk/tools/__init__.py 中零处出现该名称,用户只能从 trpc_agent_sdk.tools.tool_search 深路径导入。

触发条件: 用户按公共面习惯写 from trpc_agent_sdk.tools import ToolPermissionFilter 以标注 permission_filter 回调签名。

实际影响: 公共 API 导出不一致,类型标注场景出现 ImportError 或被迫依赖内部路径;对依赖此 SDK API 面的兼容性造成不必要的摩擦。

修正方向: 在 trpc_agent_sdk/tools/__init__.py 的 __all__ 中一并导出 ToolPermissionFilter(连同其他 ToolSearch* 名称)。

Comment on lines +1016 to +1019
def _string_list(value: Any) -> list[str]:
if not isinstance(value, (list, tuple)):
return []
return [str(item).strip() for item in value if str(item).strip()]

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.

问题: _string_list 对非 list/tuple 入参一律返回 []:若模型把 tool_names 以裸字符串(如 "read_file")而非数组形式传出,该参数被静默丢弃,与 namespace(str() 强转)、max_results(_positive_int 强转)等兄弟解析器的容错行为不一致。

触发条件: 宽松模式下模型对 tool_names 输出字符串字面量或空值。

实际影响: 搜索静默退化为空查询的“No matches”,返回给模型的提示(如“Try different queries, namespace, or tool_names”)未指明入参被丢弃,模型无法自我纠正,多轮空转。

修正方向: 将裸字符串容错为 [value],或对非数组类型返回显式错误字典(如 {"status": "error", "message": "tool_names must be a list"})给模型可恢复信号。

Comment on lines +160 to +161
class _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool):
"""Preserve the framework's long-running marker after namespace."""

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.

问题: _RenamedLongRunningTool(_RenamedTool, LongRunningFunctionTool) 没有自己的 __init__,实例化走 _RenamedTool.__init__ → BaseTool.__init__,因此 LongRunningFunctionTool.__init__ 中设置的文档化公开属性 self.is_long_running = True(_long_running_tool.py:49)在改名后的实例上从未被设置——MRO 模拟确认 getattr(tool, "is_long_running", False) 为 False,尽管 isinstance(tool, LongRunningFunctionTool) 为 True。

触发条件: 任一 NATIVE 模式消费者(框架代码或用户代码)按 is_long_running 属性而非 isinstance 判断重命名后的长期运行工具。

实际影响: 类声明的契约与实际属性不一致:_llm_agent.py 等当前消费者恰好都用 isinstance,此前依赖没有暴露问题,但任何按属性探测的消费方(包括用户对 get_tools 输出的自检)都会把长期运行 MCP 工具误判为普通工具,长期运行语义被静默丢失。

修正方向: 为 _RenamedLongRunningTool 补 __init__(调用 LongRunningFunctionTool.__init__ 或显式设置 self.is_long_running = True),使属性和类型检查一致。

Comment on lines +793 to +794
requested_max = _positive_int(args.get("max_results"))
max_results = min(requested_max, _MAX_REQUEST_RESULTS) if requested_max else self.options.max_results

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.

问题: _search 中 max_results = min(requested_max, _MAX_REQUEST_RESULTS) if requested_max else self.options.max_results:每次请求的 _MAX_REQUEST_RESULTS = 10 硬上限只作用于模型显式传入的 max_results,而配置项 ToolSearchOptions.max_results(默认 5,可配成任意大)在模型未传参数时被直接采用,不受 10 的上限约束。

触发条件: 操作者将 options.max_results 配置为大于 10,且模型搜索时不传 max_results(正常省略行为)。

实际影响: 单次搜索可返回超过 _MAX_REQUEST_RESULTS 预设上限的候选结果,每次搜索响应中渲染的文档声明与描述随之膨胀,超出设计预期 token 开销;_split_results 的 overflow 逻辑也与“上限 10”的意图不一致。

修正方向: 对配置默认值同样施加 _MAX_REQUEST_RESULTS 上限(或对 max_results 上限进行配置校验),保证两种取值路径的上界一致。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

不通过

审查范围:base_commit b33f164..head_commit e1fcc24「feat(tools): 增加按需工具搜索与加载能力」,19 个文件、+4579/-1,核心为新增 trpc_agent_sdk/tools/tool_search/(_toolset.py 1403 行、_semantic.py、_types.py)及 agents/core/_tools_processor.py 的改动,附带 965 行新测试。计划符合性:按需工具搜索(ToolSearchToolSet + _ToolSearchTool)、目录化延迟加载(deferred/toolbox、NATIVE/DISPATCH 双模式)、语义检索(SemanticToolIndex + fingerprint verifier)、fallback 钩子(_find_unresolved_tool/get_unresolved_tool)、会话态持久化按需加载等设计意图均已实现;核心正路径(加载/搜索/分发/状态合并/离线回退等约 33 个用例)覆盖充分。主要风险与结论:1) SEVERE:新 fallback 钩子使「未知工具名」这一常规事件触发 MCP 实时列表,且 _build_snapshot 在缓存为空时 re-raise,异常逃逸 per-tool try 与 asyncio.gather,把原本的单工具 tool_not_found 事件升级为整个 turn 以 tool_execution_failed 终止;2) SEVERE:每轮对每个 MCP 工具集重复实时列表(无正向缓存),单 turn 可产生 10+ 次 RPC,且任一 namespace 列表失败(缓存为空)会使整轮工具调用失败;3) MODERATE:DISPATCH 模式 _CallTool 绕过框架 progress-streaming 校验/过滤器链,缺参调用抛未处理异常并内存缓冲整个流;4) MODERATE:max_results 显式 0 被静默当 5、tool_names 路径绕过上限;5) MODERATE:_normalize_dynamic_tools 内联指纹在 else 分支抛 ValueError 不受失败兜底保护,触发即断业务;6) MODERATE:包装工具丢失 filters_name/tool.var 语义;7) MODERATE:canonical_names 大小写折叠静默覆盖同名工具;8) LOW:forget 只删指纹不删向量、_dynamic_tool_names 只增不减;9) 若干 LOW(+ 必选语法失效、namespace 字段污染参数、鸭子类型钩子无异常防护、NaN 排序污染)。测试充分性:正路径测试较充分,但缺口明显:无 MCP 列表异常/缓存空 re-raise 时整轮失败的测试、无 tool_names 超上限测试、无 DISPATCH progress-streaming 缺参错误事件测试、无搜索返回元数据字段污染参数的端到端测试。门禁结论:存在 2 条 SEVERE 级缺陷,按规则判定 state=FAILED,建议修复后合入。

发现的问题

严重

trpc_agent_sdk/agents/core/_tools_processor.py:138-141

问题: 新加的 _find_unresolved_tool 回退调用位于 __invoke_tools 的 try/except 之外(第 140 行),而 ToolSearchToolSet.get_unresolved_tool 内部通过 _build_snapshot 对每个 MCP 工具集执行实时 tools/list,当列表失败且 _dynamic_cache 中没有任何历史快照时,_toolset.py:527 的裸 raise 会直接穿透整个工具调用批次,取代此前针对未知工具的每次调用 tool_not_found 错误事件。

触发条件: NATIVE 模式下配置 MCP 工具集且尚未成功列出过工具(缓存为空),MCP 服务器不可达或握手失败;LLM 发起一个不在已解析列表中的工具调用(agent 循环中常见的名称缺失事件),进入 _find_unresolved_tool → _build_snapshot → box.toolset.get_tools 抛异常 → 缓存为空 → re-raise。顺序模式经 find_tool(:327-330)同样可达;并行模式中该异常逃出 asyncio.gather(只捕获 RunLimitException),同时取消同一批次的其他工具任务。

实际影响: 整个 agent turn 被 _llm_agent.py:739 的 except Exception 捕获并产生单个 tool_execution_failed 事件后终止,本应只影响那一个工具调用的故障升级为整轮失败;此前(变更前)同样的场景只会生成 per-tool 的 tool_not_found 错误事件,agent 可以继续。并行模式下还会连带取消正在执行的健康工具调用并丢失其结果。

修正方向: 在 _find_unresolved_tool(以及 find_tool 内的回退)外面包一层 try/except,把 MCP 列表失败的异常降级为返回 None(走原有的 tool_not_found 错误事件);或让 _build_snapshot 在缓存为空时对单个失败的 namespace 记日志并跳过该 namespace,而不是整轮 re-raise。注意同时修改 find_tool 与 __invoke_tools 两个调用点。

严重

trpc_agent_sdk/tools/tool_search/_toolset.py:522-527

问题: _build_snapshot 对每个 MCP 工具集在每次调用时都无条件执行实时 box.toolset.get_tools(ctx) 网络列表,_dynamic_cache 仅在失败时兜底而没有任何正向缓存;快照在单轮内被反复重建(process_llm_request 1 次、execute_tools_async 1 次、find_tool 每次工具调用 1 次、未知名称再触发一次 get_unresolved_tool 重建)。同时对任一 namespace 的列表失败,只要该 namespace 缓存为空就整体 re-raise,拖垮其余健康 namespace。

触发条件: 配置 N 个 MCP 工具集、一轮包含 M 个工具调用的典型 agent turn:turn 内工具列表调用约 2+N×M 次(N=2、M=5 时约 12 次),每次都是一次与 MCP server 的完整 initialize/create_session/tools.list 往返;若某一 server 在进程启动初期不可达(缓存为空),get_tools 直接抛错,process_llm_request 阶段即失败。

实际影响: 每个 turn 增加数百毫秒到数秒的串行 MCP RPC 开销(MCP 往返 100ms 时单服务器约 +0.51.1s);单台 MCP server 故障会让整个 agent(包括 preset/本地工具)无法出任何工具,且每次工具调用都重复这一失败——可用性和性能双重退化,与 plan 中「按需搜索」的轻量目标相悖。

修正方向: 给 _build_snapshot 增加基于 fingerprint 的短 TTL 正向缓存(同一 namespace 在 turn/秒级窗口内复用上次成功列表),并让列表失败时以缓存为空为单位降级为「跳过该 namespace + 警告」,而不是整轮 raise;同时将 find_tool 与 get_unresolved_tool 的快照重建合并为每 turn 一次。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:336-343

问题: _CallTool._run_async_impl 对 is_progress_streaming 的工具直接 async for 消费 entry.tool.run_streaming(...) 并把全部 chunk 聚合成一个 {"chunks": [...]} dict 返回,绕过了框架的 _execute_progress_streaming_tool 路径:缺参校验(get_mandatory_args、TOOL_CALL_ARGUMENT_ERRORS 注入、_get_argument_error_response)和 ToolCallbackFilter/skip_summarization/set_tool_var 等钩子全部不执行,参数缺失时 func(**{}) 的 TypeError/AIInvalidArgumentError 原样抛出。

触发条件: DISPATCH 模式下 MCP 侧暴露 StreamingProgressTool;LLM 先 tool_search 加载,再以 call_tool 调用且参数不完整(例如必需参数拼写错误或缺省)。

实际影响: 用户看不到任何进度事件、整个流被内存缓冲(大结果可造成显著内存占用);缺参时返回的是未处理异常(被上游包装成 generic 错误)而不是框架标准的 invalid_tool_arguments 反馈,LLM 无从自纠;agent 的 before/after_tool_callback、计时等过滤钩子对这类工具静默失效。

修正方向: _CallTool 缺参时应先行校验并返回与 _execute_progress_streaming_tool 一致的错误响应结构(而非直接调用 run_streaming);或者对 progress-streaming 工具在 DISPATCH 面板中也走框架的 streaming 事件路径(execute_tools_async 的 streaming 桶),保留 partial events 语义。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:793-800

问题: _positive_int 把 0 与负数都归一化为 0,且 max_results 默认值用 if requested_max else ... 区分「未提供」与「显式 0」:显式 max_results:0 与未提供完全无法区分,会被静默当成 5(options.max_results)执行加载;此外 tool_names 分支完全不应用任何上限(cap 只作用于 query/namespace 分支),一次调用可装载任意多工具。

触发条件: 模型调用 tool_search(max_results=0)(例如想拒绝加载或仅探测);或调用 tool_search(tool_names=[...]) 传入远超预期数量的名称。

实际影响: max_results=0 时 5 个工具被嵌入、搜索并写入会话态(discovered),产生调用方明确想避免的 embedding 费用与状态副作用;tool_names 大列表绕过 cap 后一次灌入大量工具,膨胀每轮上下文与声明校验成本。

修正方向: 用哨兵值(如特殊常量或 None)区分「未提供」与显式 0(显式 0 应短路返回空结果);并对 tool_names 分支也应用 min(len, 上限) 截断(已在语义搜索/namespace 分支使用的 _MAX_REQUEST_RESULTS 或 options.max_results)。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:483-494

问题: 同一基础工具被多次访问时,_tool_fingerprint 每次对包装后工具重建声明并计算 sha256;LongRunningFunctionTool._get_declaration 是「先 super()._get_declaration() 再原地追加 long-running 说明」的变异函数(_long_running_tool.py:52-67),当服务端声明变化导致拼接后的描述出现差异时,指纹不匹配即触发冲突检查;且该冲突检查位于 try/except 的 else 分支(缓存写入前),此处抛出的 ValueError 不受「保留最后快照」的失败回退保护。

触发条件: MCP 服务器返回 LongRunningFunctionTool;声明内容在不同列表间发生合法变化(例如服务端补上了与框架拼接说明重复的长任务提示,或顺序变化)。

实际影响: _normalize_dynamic_tools 抛 ValueError,_build_snapshot 在 else 分支中让该异常逃逸(缓存兜底只覆盖 try 内的列表失败),整个 get_tools/tool_search 失败,工具搜索功能中断;严格模式下同一 namespace 服务端重复列出同一工具(名字相同、声明一致但包装名一致)亦可能触发。

修正方向: 指纹在包装前对原始工具计算并缓存(按 namespace+name 维度),避免每次重建;把 _normalize_dynamic_tools 纳入 try 的失败兜底范围,冲突时降级为保留旧条目并告警而非抛错;对确定性声明(如 model_dump_json 键序)做规范化。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:91-99

问题: _RenamedTool.__init__ 只把 filters=list(original.filters)(已解析实例列表)传给 BaseTool,丢弃了 original.filters_name(命名 filter 的注册名),包装后 filters_name 变成 _init_filters 重新生成的解析实例名;此外框架的 FunctionTool.run_async 会执行 context.event_actions 的 set_tool_var(TOOLS_TOOL_VAR 变量注入),而 _CallTool._run_async_impl 走 run_async 正常路径时 tool.var 是从包装对象取值的——若原工具配置了命名 filter(如 tool_usage_tracker),下游 get_filter('tool_usage_tracker') 在包装对象上会失败。

触发条件: MCP 工具集内配置了 filters_name=['tool_usage_tracker'] 之类的命名 filter,经 tool_search 加载后以 mcp__... 名调用。

实际影响: 命名 filter 静默丢失,工具调用不再走用户配置的钩子(埋点、校验、用量统计失效),但无任何告警;这是把动态工具包装到 MCP 命名空间时引入的隐式行为变化。

修正方向: 在 _RenamedTool.__init__ 中同时透传 filters_name=original.filters_name(保留注册名语义,让 _init_filters 正确解析),并对 tool.var 这类包装敏感属性做转发或显式文档说明。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:574-580

问题: 快照的 canonical_names 用 name.lower() 做键,_add_entry 只拒绝精确同名;因此存在 getWeather(preset)与另一 namespace 的 getweather(或 get_weather)时,后写入者覆盖前者的 canonical 键,前一个工具在 _entry_by_name(tool_names、精确关键词匹配)下变得不可达。

触发条件: 目录中包含仅大小写/分隔符不同的两个工具名(MCP 服务端、preset、deferred 工具彼此组合时极易出现)。

实际影响: LLM 按目录中展示的 getWeather 调用时被解析到错误工具的条目,返回的声明、权限、执行结果都与展示名不符,甚至加载了未授权的工具条目(权限过滤按解析后的条目执行);同名大小写变体静默吞掉一个工具。

修正方向: _add_entry 阶段即检测 canonical 键冲突并告警/重命名(或配置即报错),而不是让映射静默覆盖;或在 _entry_by_name 命中 canonical 后校验其精确名与查询一致。

中等

trpc_agent_sdk/tools/tool_search/_toolset.py:557-558

问题: MCP tools/list 返回同一工具名但声明内容发生变化(例如普通 FunctionTool 升级为 LongRunningFunctionTool,或描述更新)时,快照记录在 changed 集合中并触发 await self._semantic_index.forget(forget_names),但旧文档的新增向量并未被删除;同时 _dynamic_tool_names 只增不减(update 两个集合),已移除工具名永久保留。

触发条件: 长跑进程中 MCP 服务器热更新工具(版本化名称 report_6c2d… 轮换、或多个 namespace 共用工具名互相竞争写入)。

实际影响: 语义检索持续返回已不存在/已过期的工具(旧向量与当前 fingerprint 不匹配,每次搜索都被 verifier 剔除但代价是重复 embedding);_dynamic_tool_names 无界增长,长期运行占用内存持续上升,且新工具名一旦与旧名冲突会被 verifier 拒绝。

修正方向: forget 语义扩展为同步清理向量存储中的同名条目(ToolSearchVectorStore 协议增加按名字删除),并将 _dynamic_tool_names 改为按 fingerprint cache 当前存活集合重算(每次快照后 filter 掉已移除名字),而不是仅 update。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:1117-1131

问题: _QUERY_SEPARATOR 是 [^\w\-+]+(re.UNICODE),_tokenize 后术语中 + 前缀保留在词项内(例如 +get-forecast 成为 +get-forecast),随后 required 分支做 _split_compound(按 [_-] 切分)得到 ['+get','forecast'],_has_term 对 +get 恒为 False,导致带 + 的必选术语约束永远不成立。

触发条件: LLM 或调用方使用 + 前缀表达必选语义(常见搜索语法),如 tool_search(queries=['+get-forecast'])。

实际影响: 所有必选术语的候选都被 required and not all(...) 过滤掉,返回空结果;即使目录中存在 get_forecast 工具也搜不到,静默丢失功能。

修正方向: 在 _tokenize(或 required 提取处)剥离 + 前缀后再做 compound 切分(+ 不应进入词项本体),或 _split_compound 对带前缀词项先去除 +。

较低

trpc_agent_sdk/tools/tool_search/_toolset.py:855-879

问题: _search_queries 的语义搜索结果经由 _format_result 生成的返回 dict 中包含顶层 namespace 键,而 _ToolSearchTool._run_async_impl 把该 dict 原样作为函数调用结果返回;在 NATIVE/DISPATCH 管道中,FunctionCall.args 若包含 LLM 从返回里抄来的 namespace 字段(LLM 经常会回填),convert_pydantic_args/参数校验会把未知的参数名当作真正的输入参数对待,产生参数校验失败。

触发条件: 模型调用带 namespace 的 tool_search 后,把返回 JSON 中的字段(尤其 namespace)抄进后续 call_tool/直接调用的参数里——多轮 agent 中常见。

实际影响: 本应成功的工具调用因多余参数报参数校验错误(或 strict 模式下被拒),LLM 需要额外一轮自纠,用户看到不必要的失败重试。

修正方向: 从 _format_result 返回结构中移除 namespace(或将结果键全部调整为 _ 前缀的元数据键),并在 _CallTool/直接调用时用白名单方式过滤掉元数据键,避免与工具入参混淆。

较低

trpc_agent_sdk/agents/core/_tools_processor.py:348-363

问题: 通过 getattr(configured, 'get_unresolved_tool', None) 鸭子类型探测钩子时,若第三方 BaseToolSet 恰巧定义了同名但签名/语义不同的方法(例如用于别的用途、参数不同或内部抛异常),_find_unresolved_tool 直接 await 调用且无任何 try/except,异常会穿透出 __invoke_tools(在 try 之外)和 asyncio.gather。

触发条件: 与遵守 ToolSetABC 抽象契约(get_tools/close/initialize/add_tools)之外的自定义工具集协作,且其方法恰好叫 get_unresolved_tool。

实际影响: 本应只影响单个未知工具的问题升级为整个批次失败并终止 agent turn;ToolSetABC 未声明该抽象方法也意味着实现契约不明确,误用无法在编译期被发现。

修正方向: 把 get_unresolved_tool 提升到 ToolSetABC/版本化协议中作为可选钩子(带文档与签名约束),并在 _find_unresolved_tool 内对调用加 try/except 降级为 None(等价于未提供钩子)。

较低

trpc_agent_sdk/tools/tool_search/_semantic.py:66-76

问题: 恶意或损坏的 embedder 返回含 nan/inf 的向量时,_cosine_similarity 的零范数保护不覆盖 NaN(nan == 0 为 False),得分成为 nan;排序密钥 (-nan, name) 与正常分数比较结果恒为 False,tuple 按名称决胜,导致这个垃圾向量进入 Top-K。

触发条件: 自定义 embedder 输出 NaN(数值异常、模型损坏或输入超界)且恰好作用于某个文档。

实际影响: 语义搜索把完全不相关的工具排在真实匹配之上返回给 LLM,工具选择被静默污染;相似度 nan 参与 best_rank 合并时也可能覆盖其他查询的正确名次。

修正方向: 在 _cosine_similarity 或排序前过滤非有限值(math.isfinite 检查,NaN/Inf 条目直接剔除),保证排序列永远为有限数。

Comment on lines 138 to 141
tool = await self._find_tool(tool_call, resolved_tools)
if tool is None:
tool = await self._find_unresolved_tool(tool_call, context)
result_event = None

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.

问题: 新加的 _find_unresolved_tool 回退调用位于 __invoke_tools 的 try/except 之外(第 140 行),而 ToolSearchToolSet.get_unresolved_tool 内部通过 _build_snapshot 对每个 MCP 工具集执行实时 tools/list,当列表失败且 _dynamic_cache 中没有任何历史快照时,_toolset.py:527 的裸 raise 会直接穿透整个工具调用批次,取代此前针对未知工具的每次调用 tool_not_found 错误事件。

触发条件: NATIVE 模式下配置 MCP 工具集且尚未成功列出过工具(缓存为空),MCP 服务器不可达或握手失败;LLM 发起一个不在已解析列表中的工具调用(agent 循环中常见的名称缺失事件),进入 _find_unresolved_tool → _build_snapshot → box.toolset.get_tools 抛异常 → 缓存为空 → re-raise。顺序模式经 find_tool(:327-330)同样可达;并行模式中该异常逃出 asyncio.gather(只捕获 RunLimitException),同时取消同一批次的其他工具任务。

实际影响: 整个 agent turn 被 _llm_agent.py:739 的 except Exception 捕获并产生单个 tool_execution_failed 事件后终止,本应只影响那一个工具调用的故障升级为整轮失败;此前(变更前)同样的场景只会生成 per-tool 的 tool_not_found 错误事件,agent 可以继续。并行模式下还会连带取消正在执行的健康工具调用并丢失其结果。

修正方向: 在 _find_unresolved_tool(以及 find_tool 内的回退)外面包一层 try/except,把 MCP 列表失败的异常降级为返回 None(走原有的 tool_not_found 错误事件);或让 _build_snapshot 在缓存为空时对单个失败的 namespace 记日志并跳过该 namespace,而不是整轮 re-raise。注意同时修改 find_tool 与 __invoke_tools 两个调用点。

Comment on lines +522 to +527
dynamic = await box.toolset.get_tools(ctx)
except Exception: # pylint: disable=broad-except
with self._dynamic_cache_lock:
wrapped = self._dynamic_cache.get(cache_key, ())
if not wrapped:
raise

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.

问题: _build_snapshot 对每个 MCP 工具集在每次调用时都无条件执行实时 box.toolset.get_tools(ctx) 网络列表,_dynamic_cache 仅在失败时兜底而没有任何正向缓存;快照在单轮内被反复重建(process_llm_request 1 次、execute_tools_async 1 次、find_tool 每次工具调用 1 次、未知名称再触发一次 get_unresolved_tool 重建)。同时对任一 namespace 的列表失败,只要该 namespace 缓存为空就整体 re-raise,拖垮其余健康 namespace。

触发条件: 配置 N 个 MCP 工具集、一轮包含 M 个工具调用的典型 agent turn:turn 内工具列表调用约 2+N×M 次(N=2、M=5 时约 12 次),每次都是一次与 MCP server 的完整 initialize/create_session/tools.list 往返;若某一 server 在进程启动初期不可达(缓存为空),get_tools 直接抛错,process_llm_request 阶段即失败。

实际影响: 每个 turn 增加数百毫秒到数秒的串行 MCP RPC 开销(MCP 往返 100ms 时单服务器约 +0.51.1s);单台 MCP server 故障会让整个 agent(包括 preset/本地工具)无法出任何工具,且每次工具调用都重复这一失败——可用性和性能双重退化,与 plan 中「按需搜索」的轻量目标相悖。

修正方向: 给 _build_snapshot 增加基于 fingerprint 的短 TTL 正向缓存(同一 namespace 在 turn/秒级窗口内复用上次成功列表),并让列表失败时以缓存为空为单位降级为「跳过该 namespace + 警告」,而不是整轮 raise;同时将 find_tool 与 get_unresolved_tool 的快照重建合并为每 turn 一次。

Comment on lines +336 to +343
if entry.tool.is_progress_streaming:
chunks: list[Any] = []
async for value in entry.tool.run_streaming(
tool_context=tool_context,
args=params,
):
chunks.append(value)
return {"chunks": chunks}

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.

问题: _CallTool._run_async_impl 对 is_progress_streaming 的工具直接 async for 消费 entry.tool.run_streaming(...) 并把全部 chunk 聚合成一个 {"chunks": [...]} dict 返回,绕过了框架的 _execute_progress_streaming_tool 路径:缺参校验(get_mandatory_args、TOOL_CALL_ARGUMENT_ERRORS 注入、_get_argument_error_response)和 ToolCallbackFilter/skip_summarization/set_tool_var 等钩子全部不执行,参数缺失时 func(**{}) 的 TypeError/AIInvalidArgumentError 原样抛出。

触发条件: DISPATCH 模式下 MCP 侧暴露 StreamingProgressTool;LLM 先 tool_search 加载,再以 call_tool 调用且参数不完整(例如必需参数拼写错误或缺省)。

实际影响: 用户看不到任何进度事件、整个流被内存缓冲(大结果可造成显著内存占用);缺参时返回的是未处理异常(被上游包装成 generic 错误)而不是框架标准的 invalid_tool_arguments 反馈,LLM 无从自纠;agent 的 before/after_tool_callback、计时等过滤钩子对这类工具静默失效。

修正方向: _CallTool 缺参时应先行校验并返回与 _execute_progress_streaming_tool 一致的错误响应结构(而非直接调用 run_streaming);或者对 progress-streaming 工具在 DISPATCH 面板中也走框架的 streaming 事件路径(execute_tools_async 的 streaming 桶),保留 partial events 语义。

Comment on lines +793 to +800
requested_max = _positive_int(args.get("max_results"))
max_results = min(requested_max, _MAX_REQUEST_RESULTS) if requested_max else self.options.max_results

if tool_names:
selected = _select_by_name(snapshot, tool_names, allowed)
overflow: list[str] = []
else:
error = _validate_namespace(snapshot, namespace)

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.

问题: _positive_int 把 0 与负数都归一化为 0,且 max_results 默认值用 if requested_max else ... 区分「未提供」与「显式 0」:显式 max_results:0 与未提供完全无法区分,会被静默当成 5(options.max_results)执行加载;此外 tool_names 分支完全不应用任何上限(cap 只作用于 query/namespace 分支),一次调用可装载任意多工具。

触发条件: 模型调用 tool_search(max_results=0)(例如想拒绝加载或仅探测);或调用 tool_search(tool_names=[...]) 传入远超预期数量的名称。

实际影响: max_results=0 时 5 个工具被嵌入、搜索并写入会话态(discovered),产生调用方明确想避免的 embedding 费用与状态副作用;tool_names 大列表绕过 cap 后一次灌入大量工具,膨胀每轮上下文与声明校验成本。

修正方向: 用哨兵值(如特殊常量或 None)区分「未提供」与显式 0(显式 0 应短路返回空结果);并对 tool_names 分支也应用 min(len, 上限) 截断(已在语义搜索/namespace 分支使用的 _MAX_REQUEST_RESULTS 或 options.max_results)。

Comment on lines +483 to +494
_add_entry(entries, tool, namespace=None, deferred=False)

if self._deferred_tools:
namespace_tools[_DEFAULT_NAMESPACE] = set()
order.append(_DEFAULT_NAMESPACE)
for tool in self._deferred_tools:
entry = _add_entry(
entries,
tool,
namespace=_DEFAULT_NAMESPACE,
deferred=True,
)

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.

问题: 同一基础工具被多次访问时,_tool_fingerprint 每次对包装后工具重建声明并计算 sha256;LongRunningFunctionTool._get_declaration 是「先 super()._get_declaration() 再原地追加 long-running 说明」的变异函数(_long_running_tool.py:52-67),当服务端声明变化导致拼接后的描述出现差异时,指纹不匹配即触发冲突检查;且该冲突检查位于 try/except 的 else 分支(缓存写入前),此处抛出的 ValueError 不受「保留最后快照」的失败回退保护。

触发条件: MCP 服务器返回 LongRunningFunctionTool;声明内容在不同列表间发生合法变化(例如服务端补上了与框架拼接说明重复的长任务提示,或顺序变化)。

实际影响: _normalize_dynamic_tools 抛 ValueError,_build_snapshot 在 else 分支中让该异常逃逸(缓存兜底只覆盖 try 内的列表失败),整个 get_tools/tool_search 失败,工具搜索功能中断;严格模式下同一 namespace 服务端重复列出同一工具(名字相同、声明一致但包装名一致)亦可能触发。

修正方向: 指纹在包装前对原始工具计算并缓存(按 namespace+name 维度),避免每次重建;把 _normalize_dynamic_tools 纳入 try 的失败兜底范围,冲突时降级为保留旧条目并告警而非抛错;对确定性声明(如 model_dump_json 键序)做规范化。

Comment on lines +91 to +99
"""Expose a dynamic tool under an MCP-style globally unique name."""

def __init__(self, original: BaseTool, name: str) -> None:
BaseTool.__init__(
self,
name=name,
description=original.description,
filters=list(original.filters),
)

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.

问题: _RenamedTool.__init__ 只把 filters=list(original.filters)(已解析实例列表)传给 BaseTool,丢弃了 original.filters_name(命名 filter 的注册名),包装后 filters_name 变成 _init_filters 重新生成的解析实例名;此外框架的 FunctionTool.run_async 会执行 context.event_actions 的 set_tool_var(TOOLS_TOOL_VAR 变量注入),而 _CallTool._run_async_impl 走 run_async 正常路径时 tool.var 是从包装对象取值的——若原工具配置了命名 filter(如 tool_usage_tracker),下游 get_filter('tool_usage_tracker') 在包装对象上会失败。

触发条件: MCP 工具集内配置了 filters_name=['tool_usage_tracker'] 之类的命名 filter,经 tool_search 加载后以 mcp__... 名调用。

实际影响: 命名 filter 静默丢失,工具调用不再走用户配置的钩子(埋点、校验、用量统计失效),但无任何告警;这是把动态工具包装到 MCP 命名空间时引入的隐式行为变化。

修正方向: 在 _RenamedTool.__init__ 中同时透传 filters_name=original.filters_name(保留注册名语义,让 _init_filters 正确解析),并对 tool.var 这类包装敏感属性做转发或显式文档说明。

Comment on lines +574 to +580
canonical_names={name.lower(): name
for name in entries},
deferred_names=frozenset(name for name, entry in entries.items() if entry.deferred),
namespace_tools={
name: frozenset(tools)
for name, tools in namespace_tools.items()
},

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.

问题: 快照的 canonical_names 用 name.lower() 做键,_add_entry 只拒绝精确同名;因此存在 getWeather(preset)与另一 namespace 的 getweather(或 get_weather)时,后写入者覆盖前者的 canonical 键,前一个工具在 _entry_by_name(tool_names、精确关键词匹配)下变得不可达。

触发条件: 目录中包含仅大小写/分隔符不同的两个工具名(MCP 服务端、preset、deferred 工具彼此组合时极易出现)。

实际影响: LLM 按目录中展示的 getWeather 调用时被解析到错误工具的条目,返回的声明、权限、执行结果都与展示名不符,甚至加载了未授权的工具条目(权限过滤按解析后的条目执行);同名大小写变体静默吞掉一个工具。

修正方向: _add_entry 阶段即检测 canonical 键冲突并告警/重命名(或配置即报错),而不是让映射静默覆盖;或在 _entry_by_name 命中 canonical 后校验其精确名与查询一致。

Comment on lines +557 to +558
else:
with self._dynamic_cache_lock:

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.

问题: MCP tools/list 返回同一工具名但声明内容发生变化(例如普通 FunctionTool 升级为 LongRunningFunctionTool,或描述更新)时,快照记录在 changed 集合中并触发 await self._semantic_index.forget(forget_names),但旧文档的新增向量并未被删除;同时 _dynamic_tool_names 只增不减(update 两个集合),已移除工具名永久保留。

触发条件: 长跑进程中 MCP 服务器热更新工具(版本化名称 report_6c2d… 轮换、或多个 namespace 共用工具名互相竞争写入)。

实际影响: 语义检索持续返回已不存在/已过期的工具(旧向量与当前 fingerprint 不匹配,每次搜索都被 verifier 剔除但代价是重复 embedding);_dynamic_tool_names 无界增长,长期运行占用内存持续上升,且新工具名一旦与旧名冲突会被 verifier 拒绝。

修正方向: forget 语义扩展为同步清理向量存储中的同名条目(ToolSearchVectorStore 协议增加按名字删除),并将 _dynamic_tool_names 改为按 fingerprint cache 当前存活集合重算(每次快照后 filter 掉已移除名字),而不是仅 update。

Comment on lines +1117 to +1131
for name in candidates:
entry = snapshot.entries[name]
metadata = _search_metadata(entry)
if required and not all(
all(_has_term(metadata, part) for part in _split_compound(term)) for term in required):
continue
score = _score_metadata(metadata, scoring_terms)
if score > 0:
score += namespace_bias.get(entry.namespace or "", 0)
best[name] = max(best.get(name, 0), score)
return sorted(
best,
key=lambda name: (
-best[name],
len(_search_metadata(snapshot.entries[name])["parts"]),

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.

问题: _QUERY_SEPARATOR 是 [^\w\-+]+(re.UNICODE),_tokenize 后术语中 + 前缀保留在词项内(例如 +get-forecast 成为 +get-forecast),随后 required 分支做 _split_compound(按 [_-] 切分)得到 ['+get','forecast'],_has_term 对 +get 恒为 False,导致带 + 的必选术语约束永远不成立。

触发条件: LLM 或调用方使用 + 前缀表达必选语义(常见搜索语法),如 tool_search(queries=['+get-forecast'])。

实际影响: 所有必选术语的候选都被 required and not all(...) 过滤掉,返回空结果;即使目录中存在 get_forecast 工具也搜不到,静默丢失功能。

修正方向: 在 _tokenize(或 required 提取处)剥离 + 前缀后再做 compound 切分(+ 不应进入词项本体),或 _split_compound 对带前缀词项先去除 +。

Comment on lines +855 to +879
snapshot,
namespace,
queries,
allowed,
)
return _split_results(ranked, max_results)

candidates = _candidate_names(
snapshot,
namespace,
allowed,
)
documents = [_document_for(snapshot.entries[name]) for name in candidates]
best_rank: dict[str, int] = {}
usage = ToolSearchEmbeddingUsage()
try:
for query in queries:
ranked = await self._semantic_index.search(
query,
documents,
len(documents),
verifier=self._semantic_candidate_is_current,
usage=usage,
)
for rank, name in enumerate(ranked):

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.

问题: _search_queries 的语义搜索结果经由 _format_result 生成的返回 dict 中包含顶层 namespace 键,而 _ToolSearchTool._run_async_impl 把该 dict 原样作为函数调用结果返回;在 NATIVE/DISPATCH 管道中,FunctionCall.args 若包含 LLM 从返回里抄来的 namespace 字段(LLM 经常会回填),convert_pydantic_args/参数校验会把未知的参数名当作真正的输入参数对待,产生参数校验失败。

触发条件: 模型调用带 namespace 的 tool_search 后,把返回 JSON 中的字段(尤其 namespace)抄进后续 call_tool/直接调用的参数里——多轮 agent 中常见。

实际影响: 本应成功的工具调用因多余参数报参数校验错误(或 strict 模式下被拒),LLM 需要额外一轮自纠,用户看到不必要的失败重试。

修正方向: 从 _format_result 返回结构中移除 namespace(或将结果键全部调整为 _ 前缀的元数据键),并在 _CallTool/直接调用时用白名单方式过滤掉元数据键,避免与工具入参混淆。

Comment on lines +348 to +363
async def _find_unresolved_tool(
self,
tool_call: FunctionCall,
context: InvocationContext,
) -> Optional[BaseTool]:
"""Ask dynamic toolsets for a declaration-free fallback tool."""
for configured in self.tools:
if not isinstance(configured, BaseToolSet):
continue
get_unresolved_tool = getattr(configured, "get_unresolved_tool", None)
if get_unresolved_tool is None:
continue
fallback = await get_unresolved_tool(tool_call.name, context)
if fallback is not None:
return fallback
return None

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.

问题: 通过 getattr(configured, 'get_unresolved_tool', None) 鸭子类型探测钩子时,若第三方 BaseToolSet 恰巧定义了同名但签名/语义不同的方法(例如用于别的用途、参数不同或内部抛异常),_find_unresolved_tool 直接 await 调用且无任何 try/except,异常会穿透出 __invoke_tools(在 try 之外)和 asyncio.gather。

触发条件: 与遵守 ToolSetABC 抽象契约(get_tools/close/initialize/add_tools)之外的自定义工具集协作,且其方法恰好叫 get_unresolved_tool。

实际影响: 本应只影响单个未知工具的问题升级为整个批次失败并终止 agent turn;ToolSetABC 未声明该抽象方法也意味着实现契约不明确,误用无法在编译期被发现。

修正方向: 把 get_unresolved_tool 提升到 ToolSetABC/版本化协议中作为可选钩子(带文档与签名约束),并在 _find_unresolved_tool 内对调用加 try/except 降级为 None(等价于未提供钩子)。

Comment on lines +66 to +76
limit: int,
) -> list[str]:
allowed = set(candidate_names)
with self._lock:
scored = [(
_cosine_similarity(query_vector, vector),
name,
) for name, vector in self._vectors.items() if name in allowed]
scored.sort(key=lambda item: (-item[0], item[1]))
return [name for _, name in scored[:limit]]

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.

问题: 恶意或损坏的 embedder 返回含 nan/inf 的向量时,_cosine_similarity 的零范数保护不覆盖 NaN(nan == 0 为 False),得分成为 nan;排序密钥 (-nan, name) 与正常分数比较结果恒为 False,tuple 按名称决胜,导致这个垃圾向量进入 Top-K。

触发条件: 自定义 embedder 输出 NaN(数值异常、模型损坏或输入超界)且恰好作用于某个文档。

实际影响: 语义搜索把完全不相关的工具排在真实匹配之上返回给 LLM,工具选择被静默污染;相似度 nan 参与 best_rank 合并时也可能覆盖其他查询的正确名次。

修正方向: 在 _cosine_similarity 或排序前过滤非有限值(math.isfinite 检查,NaN/Inf 条目直接剔除),保证排序列永远为有限数。

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.

3 participants