fix: consume injected tool call results in tool loop runner - #9673
fix: consume injected tool call results in tool loop runner#9673Rail1bc wants to merge 4 commits into
Conversation
Plugins can inject fake assistant(tool_calls) → tool(result) pairs via req.append_tool_calls_result(); reset() now appends them after the current user message so the block belongs to the current turn, matching how providers assemble the text_chat payload. Export the segment/result types from astrbot.api.provider and make _save_to_history skip any _no_save message regardless of role, so temp-marked fake pairs never persist. Co-Authored-By: deepseek-v4-flash <deepseek-ai@claude-code-best.win>
|
@whatevertogo review it |
whatevertogo
left a comment
There was a problem hiding this comment.
我是 whatevertogo 的替身。
审查会话:d87ecbed-3927-429e-96ee-00a094e29ebf
触发评论:5288467560
Head SHA:43913ba1252c19c491d0160a7ea7ccf1502b7643
已发布 8 条行内审查评论。最终总结将作为单独评论发布。
- 最高风险:P2 partial mark_as_temp 会在持久化历史中留下悬空 tool 消息,导致下一轮 provider API 拒绝
- 暂无法定位的发现:0
| if message.role in ["assistant", "user"] and message._no_save: | ||
| # _no_save 语义与角色无关:临时注入的消息(含伪造工具调用对的 | ||
| # tool 消息)一律不落库,避免历史中残留悬空的 tool 消息。 | ||
| if message._no_save: |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P2][Advisory][high 置信度] partial mark_as_temp 会在持久化历史中留下悬空 tool 消息,导致下一轮 provider API 拒绝
类别:Correctness
证据:RIGHT 470 if message._no_save: continue 替代了旧的 role in ["assistant", "user"] 检查。旧代码不对 tool 角色检查 _no_save,所以即使插件设置了 _no_save=True,tool 消息也总是被持久化——这恰好避免了悬空 assistant(tool_calls) 的情况。新代码正确过滤 tool 消息,但如果 mark_as_temp() 未成对使用,就会产生悬空消息。OpenAI API 要求 "messages with tool_calls must be followed by tool messages",Anthropic 要求 tool_use 后跟 tool_result,Gemini 要求 function_call 后跟 function_response。_save_to_history 和 mark_as_temp() 都没有验证配对一致性。
问题:PR 将 _save_to_history 的过滤条件从 role in ["assistant", "user"] and _no_save 改为纯 _no_save,使 tool 角色消息现在也会被 _no_save 过滤。这引入了一个新的失败模式:如果插件只对注入对的一个消息调用 mark_as_temp()(例如标记了 ToolCallMessageSegment.mark_as_temp() 但忘记标记 AssistantMessageSegment),_save_to_history 会过滤掉 tool 消息但保留 assistant(tool_calls) 消息,在持久化历史中留下悬空的 assistant(tool_calls)。下一轮加载该历史并发送给 provider 时,OpenAI API 要求 assistant(tool_calls) 后必须跟随对应的 tool 消息,会直接拒绝请求;Anthropic/Gemini 有相同的 tool_use/tool_result 配对约束。
项目上下文:mark_as_temp() 是本 PR 新增并经 astrbot.api.provider 导出的公共 API,插件开发者是主要使用者。仓库 AGENTS.md 强调 KISS 和 first principles——最小正确实现应包含对自身引入的不变式(成对 temp)的验证或防护。
影响:插件作者忘记成对标记时,当前轮运行正常,但下一轮所有 provider API 调用都会因消息结构非法而被拒绝。错误出现在与触发错误不同的轮次,极难调试。考虑到该 API 面向 1000+ 插件生态,误用概率不低。
修复建议:在 _save_to_history 的过滤循环中增加悬空检测:如果保留的 assistant(tool_calls) 消息后紧邻的不是对应 tool 消息(被 _no_save 过滤掉了),记录 warning 日志并跳过该 assistant 消息。或者在 ToolCallsResult.to_openai_messages_model() 中验证 tool_calls_info._no_save 与所有 tool_calls_result[i]._no_save 一致,不一致时抛出 ValueError。
| def mark_as_temp(self) -> "Message": | ||
| """Mark this message as provider-facing only, not persisted. | ||
|
|
||
| 临时注入(如伪造工具调用对)应成对标记:assistant 与 tool 消息都调用 |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][high 置信度] mark_as_temp() docstring 使用中文且缺少 Google 格式 Returns 段
类别:Tests/API Contract
证据:astrbot/core/agent/message.py RIGHT 218-225,docstring 正文 临时注入(如伪造工具调用对)应成对标记... 为中文;方法返回 self(用于链式调用)但无 Returns: 段。对比同文件 ContentPart.mark_as_temp()(line 68-71)使用简洁英文单行 docstring。
问题:mark_as_temp() 的 docstring 正文为中文,且缺少 Google 格式的 Returns: 段。AGENTS.md 规则 #5 要求 "Use English for all comments and logs",规则 "Mandatory Google-Style Docstrings" 要求所有 docstring 严格使用 Google 格式。该方法是新增的公共 API(经 astrbot.api.provider 导出供插件使用),文档质量直接影响插件开发者。
项目上下文:AGENTS.md 将英文注释和 Google 格式 docstring 列为基本开发要求(Basic rule #5 + Mandatory Google-Style Docstrings 小节)。
影响:公共 API 的中文 docstring 会在 IDE hover / 自动文档生成中展示中文,与项目全英文策略不一致;缺失 Returns: 降低了类型可读性。
修复建议:将 docstring 改为英文并补充 Returns: 段,例如:"""Mark this message as provider-facing only, not persisted.\n\nInjected pairs (e.g. fake tool calls) should mark both the\nassistant and tool messages so no dangling tool message\nremains in history.\n\nReturns:\n Self, for method chaining.\n"""。
| skipped_initial_system = True | ||
| continue | ||
| if message.role in ["assistant", "user"] and message._no_save: | ||
| # _no_save 语义与角色无关:临时注入的消息(含伪造工具调用对的 |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][high 置信度] _save_to_history 注释使用中文,违反 AGENTS.md 英文注释规则
类别:Tests/API Contract
证据:internal.py RIGHT 468-469,注释 # _no_save 语义与角色无关:临时注入的消息(含伪造工具调用对的 # tool 消息)一律不落库... 为中文。
问题:_save_to_history 中新增的代码注释为中文,AGENTS.md 规则 #5 要求 "Use English for all comments and logs"。
项目上下文:AGENTS.md Basic rule #5 明确要求所有注释使用英文。同 PR 中 tool_loop_agent_runner.py 的对应注释(RIGHT 318-323)已正确使用英文。
影响:仓库注释语言不统一,后续维护者可能难以保持一致性。
修复建议:改为英文,例如:# _no_save is role-agnostic: temp-injected messages (including the tool message in a fake tool-call pair) are never persisted to avoid dangling tool messages in history.。
| _checkpoint_after: CheckpointData | None = PrivateAttr(default=None) | ||
|
|
||
| def mark_as_temp(self) -> "Message": | ||
| """Mark this message as provider-facing only, not persisted. |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][high 置信度] mark_as_temp() docstring 为中文且缺少 Google 格式 Returns 段
类别:Tests/API Contract
证据:RIGHT 219-223 docstring 正文 临时注入(如伪造工具调用对)应成对标记... 为中文;方法返回 self(用于链式调用如 AssistantMessageSegment(...).mark_as_temp())但无 Returns: 描述。同模块 ContentPart.mark_as_temp()(line 68-69)使用简洁英文单行 docstring,本方法未保持一致。
问题:新增的 mark_as_temp() 是公共 API(经 astrbot.api.provider 导出供插件使用),但其 docstring 正文使用中文,且缺少 Google 格式的 Returns: 段。
项目上下文:AGENTS.md Basic rule #5 要求 "Use English for all comments and logs";"Mandatory Google-Style Docstrings" 要求所有 docstring 严格使用 Google 格式(Args:、Returns:、Raises:)。
影响:公共 API 的中文 docstring 会在 IDE hover / 自动文档中展示中文,与项目全英文策略不一致;缺失 Returns: 降低链式调用场景的类型可读性。
修复建议:改为英文并补充 Returns: 段,例如:
def mark_as_temp(self) -> "Message":
"""Mark this message as provider-facing only, not persisted.
Injected pairs (e.g. fake tool-call results) should mark both the
assistant and tool messages to avoid dangling tool messages in history.
Returns:
Self, for method chaining.
"""
self._no_save = True
return self| _no_save: bool = PrivateAttr(default=False) | ||
| _checkpoint_after: CheckpointData | None = PrivateAttr(default=None) | ||
|
|
||
| def mark_as_temp(self) -> "Message": |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][medium 置信度] mark_as_temp() 返回类型标注为 Message 而非子类类型,与 ContentPart 同名方法不一致
类别:Tests/API Contract
证据:RIGHT 218 def mark_as_temp(self) -> "Message":。同模块 ContentPart.mark_as_temp()(line 68)使用 TypeVar 模式 def mark_as_temp(self: ContentPartT) -> ContentPartT 正确保留子类类型。ToolCallsResult.tool_calls_info: AssistantMessageSegment(entities.py line 69),文档示例传入 AssistantMessageSegment(...).mark_as_temp(),类型推断为 Message 而非 AssistantMessageSegment。
问题:Message.mark_as_temp() 返回类型标注为 "Message",但 AssistantMessageSegment/ToolCallMessageSegment 继承该方法后,调用 .mark_as_temp() 在静态类型检查下返回 Message 而非子类类型。文档示例 AssistantMessageSegment(...).mark_as_temp() 传给 ToolCallsResult.tool_calls_info(类型 AssistantMessageSegment),strict type checker 会报不兼容。
项目上下文:ContentPart 已建立 TypeVar 链式返回的先例(line 16 定义 ContentPartT,line 68 使用),Message 应保持一致。本方法是新导出的公共 API,类型正确性影响插件开发者。
影响:使用 mypy/pyright 严格模式的插件会收到类型错误;IDE 自动补全会失去子类信息。运行时无影响。
修复建议:定义 MessageT = TypeVar("MessageT", bound="Message") 并改为 def mark_as_temp(self: MessageT) -> MessageT:。
| skipped_initial_system = True | ||
| continue | ||
| if message.role in ["assistant", "user"] and message._no_save: | ||
| # _no_save 语义与角色无关:临时注入的消息(含伪造工具调用对的 |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][high 置信度] _save_to_history 新增注释使用中文,违反 AGENTS.md 英文注释规则
类别:Tests/API Contract
证据:RIGHT 468-469 # _no_save 语义与角色无关:临时注入的消息(含伪造工具调用对的 # tool 消息)一律不落库... 为中文。对比 tool_loop_agent_runner.py RIGHT 318-323 的英文注释 # Plugin-injected tool call results (on_llm_request → ...)。
问题:_save_to_history 中新增的代码注释为中文,AGENTS.md rule #5 要求所有注释使用英文。同 PR 的 tool_loop_agent_runner.py 对应注释(RIGHT 318-323)已正确使用英文,两处风格不一致。
项目上下文:AGENTS.md Basic rule #5 明要求 "Use English for all comments and logs"。
影响:仓库注释语言不统一,后续维护者可能难以保持一致性。
修复建议:改为英文,例如:# _no_save is role-agnostic: temp-injected messages (including the tool message in a fake tool-call pair) are never persisted to avoid dangling tool messages in history.。
| payload 组装路径(真实 provider,仅对 SDK create 的入口 _query 打桩捕获)。 | ||
| 顺序:history → 当前 user → assistant(tool_calls) → tool(result)。 | ||
| """ | ||
| from astrbot.core.agent.tool import FunctionTool, ToolSet |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][high 置信度] 测试函数内存在与模块级重复的 import
类别:Tests/API Contract
证据:RIGHT 1634 from astrbot.core.agent.tool import FunctionTool, ToolSet 与 RIGHT 1635 from astrbot.core.provider.entities import ToolCallsResult 已分别在模块级导入(本文件 RIGHT 25 from astrbot.core.agent.tool import FunctionTool, ToolSet 和 RIGHT 29-34 from astrbot.core.provider.entities import ... ToolCallsResult)。仅 ProviderOpenAIOfficial(RIGHT 1636)是合理的 lazy import。
问题:test_runner_with_openai_provider_preserves_injected_tool_calls_order 在函数内重新导入了已在模块级导入的符号。
项目上下文:项目无明确的 lazy import 约定;既有测试(如 test_reset_appends_injected_tool_calls_result_after_user)直接使用模块级导入的 ToolCallsResult/AssistantMessageSegment。
影响:代码冗余,后续维护者可能困惑为何同一符号在不同测试中导入方式不同。
修复建议:删除 RIGHT 1634-1635 两行冗余 import,仅保留 from astrbot.core.provider.sources.openai_source import ProviderOpenAIOfficial。
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_reset_appends_injected_tool_calls_result_after_user( |
There was a problem hiding this comment.
我是 whatevertogo 的替身。
[P3][Advisory][medium 置信度] list 形式的多个 ToolCallsResult 注入路径未被测试覆盖
类别:Tests/API Contract
证据:RIGHT 1587 传入 tool_calls_result=ToolCallsResult(...)(单对象)。文档推荐使用的 req.append_tool_calls_result()(entities.py append_tool_calls_result)在第二次调用时会把单对象转换为 list,所以 list 路径在实际使用中会被触及——例如插件注入两次记忆召回。
问题:runner 的 reset()(shard 1 RIGHT 324-331)有一个 isinstance(request.tool_calls_result, list) 分支用于处理多个 ToolCallsResult,但三个新增测试都只传入单个 ToolCallsResult 对象,list 分支未被覆盖。
项目上下文:PR 声明的核心卖点之一是支持插件通用注入,多次 append_tool_calls_result() 是预期用法。
影响:如果 list 分支有 off-by-one 或顺序错误(如多个注入对的追加顺序),现有测试无法捕获。
修复建议:增加一个测试,构造 request.tool_calls_result = [ToolCallsResult(...), ToolCallsResult(...)](两个不同的 tool_call_id),断言 run_context.messages 中两组 assistant→tool 对按列表顺序追加且 ID 各自正确。
|
我是 whatevertogo 的替身。 审查会话: 行内评论已发布(ID 4933200049,查看审查)。以下是我的总结。 代码审查范围本次审查覆盖了当前 PR diff,并按 Correctness、Security、Reliability/Performance、Tests/API Contract 四个角度检查。 验证
发现[P2][Advisory][high] partial mark_as_temp 会在持久化历史中留下悬空 tool 消息,导致下一轮 provider API 拒绝
[P3][Advisory][high] mark_as_temp() docstring 使用中文且缺少 Google 格式 Returns 段
[P3][Advisory][high] _save_to_history 注释使用中文,违反 AGENTS.md 英文注释规则
[P3][Advisory][high] mark_as_temp() docstring 为中文且缺少 Google 格式 Returns 段
[P3][Advisory][medium] mark_as_temp() 返回类型标注为 Message 而非子类类型,与 ContentPart 同名方法不一致
[P3][Advisory][high] _save_to_history 新增注释使用中文,违反 AGENTS.md 英文注释规则
[P3][Advisory][high] 测试函数内存在与模块级重复的 import
[P3][Advisory][medium] list 形式的多个 ToolCallsResult 注入路径未被测试覆盖
设计提醒
低置信度观察
合并评估总体:需要处理已发布的 inline findings 做了什么完成覆盖优先审查并发布 8 条行内评论;未能定位到 diff 行的发现数为 0。 下一步建议
剩余风险
覆盖范围
|
Partial mark_as_temp() on an injected pair would persist a dangling assistant(tool_calls) message that providers reject on the next round. Validate _no_save consistency in ToolCallsResult.to_openai_messages_model() so mismatched pairs fail fast at injection time. Also switch Message. mark_as_temp() to a TypeVar return type (matching ContentPart) and move comments to English per AGENTS.md. Co-Authored-By: deepseek-v4-flash <deepseek-ai@claude-code-best.win>
|
@whatevertogo 讲解一下pr内容,每个文件添加了那些东西? |
|
多个插件之间的顺序可以复用现有 filter.on_llm_request(priority=...):priority 越高,handler 越早执行,其 append 的结果也越靠前。建议在文档中明确这一点,并增加一个覆盖高低 priority 注入顺序的测试。只是建议哈 |
Lock in that an injected assistant(tool_calls) + tool(result) pair converts correctly through the three non-OpenAI-format providers: Anthropic (tool_use/tool_result), Gemini (functionCall/functionResponse), and OpenAI Responses (function_call/function_call_output). Co-Authored-By: deepseek-v4-flash <deepseek-ai@claude-code-best.win>
|
我是 whatevertogo 的替身。 审查会话: 已经看完全部 diff。以下是这个 PR 的完整讲解。 PR 总体目标这个 PR 让插件能够向 LLM 注入伪造的工具调用结果——即插件可以"欺骗" LLM,让它认为自己调用了某个工具并拿到了结果,而不需要真正等待 LLM 自己发起工具调用。典型用途是 LivingMemory 插件把检索到的长期记忆包装成一次"记忆召回"工具调用。 核心做法是复用已有的 逐文件讲解1.
|
| 测试 | 覆盖内容 |
|---|---|
test_reset_appends_injected_tool_calls_result_after_user |
单个注入对的追加顺序 + _no_save 标记经 reset() 后保留 |
test_reset_appends_multiple_injected_tool_calls_results_in_order |
list 形式多个注入对按列表顺序追加(两个不同 tool_call_id) |
test_injected_tool_calls_result_partial_temp_mark_raises |
只标记一半时抛 ValueError(成对一致性校验) |
test_runner_with_openai_provider_preserves_injected_tool_calls_order |
端到端验证:真实 ProviderOpenAIOfficial,mock _query 捕获 payload,断言消息顺序 |
test_temp_injected_tool_pair_not_persisted(checkpoint 文件) |
temp 对不落库 |
与上一版的关键变化
这个 head SHA(1306655b)相比之前审查的版本(43913ba),作者修复了之前审查提出的多个问题:
- P2(悬空消息):在
entities.py的to_openai_messages_model()增加了成对一致性校验 + 文档增加ValueError提示 - P3(中文 docstring):
mark_as_temp()已改为英文 Google 格式 - P3(中文注释):
_save_to_history注释改为英文 - P3(返回类型):引入
MessageTTypeVar 替代硬编码"Message" - P3(list 测试缺口):新增
test_reset_appends_multiple_injected_tool_calls_results_in_order - P3(冗余 import):测试函数内的重复 import 已清理
测试从 3 个增加到 5 个,覆盖度和断言强度都有提升。
Higher on_llm_request priority handlers run earlier, so their appended tool_calls_result appears earlier in the final context. Add a dispatch-level test through call_event_hook asserting high-priority injection precedes low-priority, and document the ordering guarantee. Co-Authored-By: deepseek-v4-flash <deepseek-ai@claude-code-best.win>
|
look good 偷 me, @Soulter 粉毛大人看看呢 |
|
我是 whatevertogo 的替身。 PR 审查失败,未继续重试。 触发评论: 可以在本 PR 评论 |
Motivation / 动机
通过复用现有的
ProviderRequest.tool_calls_result,允许插件注入伪造的assistant(tool_calls) → tool(result)消息对,强制 LLM 认为自己已经调用了某个工具并拿到了结果,从而:
这是一种通用手法,已在 LivingMemory 插件中长期记忆注入中使用,未来可能被更多插件用于各种确定性工具调用场景。
初版方案在
OpenAI_Provider内做启发式重排(从尾部弹出 user 消息,再收集 assistant(tool_calls)+tool对重排),存在两个问题:
OpenAI_Provider,Anthropic / Gemini / OpenAI Responses 等其他 provider不重排,注入顺序依然错误
本 PR 使用@whatevertogo 审查建议的机制方案:
tool_calls_result通道本就存在,且所有 provider 在组装 text_chat 载荷时都按history → user → assistant(tool_calls) → tool(result)顺序消费它。让ToolLoopAgentRunner.reset()以相同顺序把注入对绑定到会话上下文,保证:
Refs: #9451
Modifications / 改动点
astrbot/core/agent/runners/tool_loop_agent_runner.pyToolLoopAgentRunner.reset()在拼接[history, 当前用户消息]之后,消费request.tool_calls_result(单个ToolCallsResult或列表),经to_openai_messages_model()追加伪造的assistant(tool_calls) → tool(result)消息,使该块属于当前轮次,与 provider 载荷组装顺序完全一致。
astrbot/api/provider/__init__.py公开导出
AssistantMessageSegment、ToolCallMessageSegment、ToolCallsResult,插件可通过公共 API路径构造并注入伪造对。
astrbot/core/agent/message.py新增
Message.mark_as_temp():标记消息为仅 provider 可见、不落库。伪造的 assistant(tool_calls) 与 tool(result)应成对调用,避免历史中残留悬空 tool 消息。
astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py_save_to_history持久化过滤由按角色(仅 assistant/user)改为按_no_save标记,临时注入的消息(含 tool角色)一律不落库。
docs/zh/dev/star/plugin.md新增"注入工具调用结果"小节,说明
req.append_tool_calls_result()用法、顺序保证与成对.mark_as_temp()语义。不依赖具体插件实现,通用机制
不改动
req.contexts,不破坏其他on_llm_requesthandler覆盖全部 provider 路径
真实工具调用场景不受影响
This is NOT a breaking change. / 这不是一个破坏性变更。
Test Results / 测试结果
新增 3 个测试:
test_reset_appends_injected_tool_calls_result_after_user— 顺序断言[user, assistant, user, assistant(tc), tool]test_runner_with_openai_provider_preserves_injected_tool_calls_order— 走真实ProviderOpenAIOfficial端到端验证(mock
_query,无网络)test_temp_injected_tool_pair_not_persisted— 伪造对整对不落库,真实 tool 消息正常落库验证结果:
tests/test_tool_loop_agent_runner.py+tests/test_conversation_checkpoint.py:59 passedtests/unit/test_astr_main_agent.py:102 passedChecklist / 检查清单
Summary by Sourcery
Handle plugin-injected tool call results via ProviderRequest.tool_calls_result so they are bound to the current user turn and consumed consistently across providers without affecting real tool usage or other handlers.
New Features:
Enhancements:
Documentation:
Tests: