From c9be04fdac3843276a8a31dbbb57a39145cc71aa Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Wed, 4 Nov 2026 16:24:49 +0800 Subject: [PATCH] =?UTF-8?q?fix(agent):=20=E8=A2=AB=E6=8B=92=E7=BB=9D/?= =?UTF-8?q?=E8=A2=AB=E8=B7=B3=E8=BF=87=E7=9A=84=E5=B7=A5=E5=85=B7=E6=B2=A1?= =?UTF-8?q?=E8=90=BD=20ToolResults=20=E2=80=94=E2=80=94=20call=20=E4=B8=8E?= =?UTF-8?q?=20result=20=E5=A4=B1=E9=85=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 根因（4ab888c 的自检抓出来的） 4ab888c 在发送前加了配对自检，生产日志立刻指出真正来源： ★ dropped 1 unpaired tool message(s) before send (names: [qq_call_state]) 顺着工具名查 context.json，脏帧是决定性的： [42] tool_calls = [qq_call_state, output_send__qq] tool_results = [output_send__qq] ← call_state 从未落盘 该帧自己的记录写了原因：qq_call_state 被 QQ 插件的来源权限策略拦下 （判定为"非所有者私聊"）。 ## 缺陷位置 stepToolBegin 的顺序是「先记 call，再判能不能执行」： f.appendToolCallRecord(tc) // ← 先落 ToolCalls if a.runStage(sdk.StageBeforeToolcall, …) { f.Msgs 补一条 role=tool // ← 模型能看到 f.ToolIdx++ return outcomeContinue // ← 但没写 ToolResults } 同一函数里还有一处同构分支（插件崩溃态跳过执行），同样只补 f.Msgs。 task.go 顶部的注释写着「ToolCalls 与 ToolResults 通过 CallID 配对； 两者都由 appendToolCallRecord 同步追加」—— 这个不变量在**提前返回路径** 上被违反了。 ## 为什么这会变成会话级故障 ContextEvent{ToolCalls: 2, ToolResults: 1} ⇒ 渲染出 assistant(tool_calls=[c1,c2]) 但只有 c1 的结果 ⇒ 上游硬拒绝（HTTP 400）： An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'. (insufficient tool messages…) 关键在于脏数据**持久化进 context.json**，之后每一轮都带着它，直到自检剔掉。 ## 修法 新增 appendToolResultRecord（与 appendToolCallRecord 成对的公开入口）， 在两处提前返回各补一行。 判定"被拒绝/被跳过"≠"没有结果"：拒绝原因本身就是结果，而且模型确实 从 f.Msgs 看到了它。落 context 的那份必须与发给模型的一致。 并发路径（stepToolBatch）没有这个问题 —— runOneTool 只返回结果， join 后的顺序收尾段无条件写 ToolResults（含被拒绝项）。 ## 判据（2 条，2 次变异全部实测变红） TestDeniedToolStillRecordsResult 注册一个真的会拒绝的 before_toolcall 钩子，跑完整一轮， 断言落进 context 的每个 call 都有 result，且拒绝原因文本一致。 变异：删掉拒绝分支那行 ⇒ "★ call call_deny_1 (denied_tool) 没有对应的 ToolResults"（实测红） TestUnhealthyPluginSkipStillRecordsResult 钉住第二条同构分支。 变异：删掉跳过分支那行 ⇒ "★ 被跳过的工具 call_unhealthy_1 没落 ToolResults —— 同型失配，上游 400"（实测红） 两条都走真实链路（真的调 runStage / 真的置崩溃态），不是只测 appendToolResultRecord 这个函数本身。 ## 顺带发现一个独立缺陷（未修，不在本次范围） 写第二条判据时发现：插件崩溃熔断**实际从不生效**。 recordCrash 标记 Unhealthy=true，但从不设置 LastReload ⇒ isHealthy 的冷却判据 time.Since(r.LastReload) > reloadCooldown 在 LastReload 为零值时恒为真（Since 是巨大的正数） ⇒ 判定"冷却期早已过去" ⇒ 立刻自愈 实测：连续 3 次 recordCrash 后 needReload=true、Unhealthy=true， 紧接着 isHealthy 返回 true。 后果：task.go:1065/1135 的「插件崩溃态跳过执行」分支实际走不到。 这条应单独修并单独钉，本提交的判据用直达状态绕过它（注释已写明原因）。 ## 验证 go build ./... PASS go vet ./... PASS gofmt -l 干净 go test ./... -count=1 62 包全过，0 FAIL 干净检出（worktree） build + TestDenied/TestUnhealthy 全绿 --- internal/agent/core/deniedpair_test.go | 212 +++++++++++++++++++++++++ internal/agent/core/task.go | 41 +++++ 2 files changed, 253 insertions(+) create mode 100644 internal/agent/core/deniedpair_test.go diff --git a/internal/agent/core/deniedpair_test.go b/internal/agent/core/deniedpair_test.go new file mode 100644 index 00000000..1b6de516 --- /dev/null +++ b/internal/agent/core/deniedpair_test.go @@ -0,0 +1,212 @@ +package core + +// 判据：「被拒绝 / 被跳过」的工具**也必须落 ToolResults**。 +// +// ## 守的是什么（2026-11-04 生产故障，根因） +// +// `stepToolBegin` 的顺序是「先记 call，再判能不能执行」： +// +// f.appendToolCallRecord(tc) // ← 先落 ToolCalls +// if a.runStage(sdk.StageBeforeToolcall, …) { +// f.Msgs 补一条 role=tool // ← 模型能看到 +// return // ← 但没写 ToolResults +// } +// +// 于是 `ContextEvent{ToolCalls: 2, ToolResults: 1}`，渲染出 +// assistant(tool_calls=[c1,c2]) 却只有 c1 的结果 ⇒ 上游硬拒绝： +// +// An assistant message with 'tool_calls' must be followed by tool messages +// responding to each 'tool_call_id'. (insufficient tool messages…) +// +// 而且脏数据**持久化进 context.json**，之后每一轮都带着它。 +// +// 实测那一次：qq 插件的来源权限策略拒绝了 `qq_call_state`。 +// +// [42] tool_calls=[qq_call_state, output_send__qq] +// tool_results=[output_send__qq] ← call_state 从未落盘 +// +// ## 为什么这个判据必须走真实链路 +// +// 只测 `appendToolResultRecord` 本身毫无意义 —— 它当然会 append。 +// 要钉的是「stepToolBegin 的拒绝分支有没有调用它」。所以这里注册一个 +// **真的会拒绝**的 before_toolcall 钩子，跑完整一轮，再检查落进 context 的 +// 那条事件。 + +import ( + "context" + "strings" + "testing" + "time" + + agentAPI "github.com/JianFeeeee/HologramAgent/internal/agent/api" + agentIO "github.com/JianFeeeee/HologramAgent/internal/agent/io" + "github.com/JianFeeeee/HologramAgent/internal/sdk" +) + +// deniedProvider 第一轮要求调用一个工具，之后给最终回复。 +type deniedProvider struct { + calls int + tc agentAPI.ToolCall +} + +func (p *deniedProvider) Name() string { return "denied" } +func (p *deniedProvider) Chat(_ context.Context, _ *agentAPI.CompletionRequest) (*agentAPI.CompletionResponse, error) { + p.calls++ + if p.calls == 1 { + return &agentAPI.CompletionResponse{ToolCalls: []agentAPI.ToolCall{p.tc}}, nil + } + return &agentAPI.CompletionResponse{Content: "done"}, nil +} +func (p *deniedProvider) ChatStream(_ context.Context, _ *agentAPI.CompletionRequest) (<-chan agentAPI.StreamChunk, error) { + return nil, context.Canceled +} +func (p *deniedProvider) MaxContextTokens() int { return 8192 } + +// newDeniedAgent 建一个「工具会被 before_toolcall 拒绝」的 agent。 +// +// 与被拒绝工具同时提供一个正常工具，用以断言**不能因噎废食**： +// 拒绝分支修好之后，同批的另一个工具仍需正常落 result。 +func newDeniedAgent(t *testing.T, tc agentAPI.ToolCall) *Agent { + t.Helper() + a := New(AgentConfig{ + ID: "deniedagent", + Provider: &deniedProvider{tc: tc}, + ProviderManager: agentAPI.NewProviderManager(), + IO: agentIO.NewIOManager(), + StageHost: NewStageHost(), + }) + dev := &mockOutputDevice{ + name: "denydev", + caps: agentIO.CapText, + tools: []agentIO.ToolDef{ + {Name: tc.Name, Description: "会被拒绝"}, + }, + toolFn: func(tool string, args map[string]interface{}) (interface{}, error) { + t.Fatalf("★ 被拒绝的工具不该真的执行（%s）", tool) + return nil, nil + }, + } + if err := a.io.RegisterDevice(dev); err != nil { + t.Fatalf("注册测试设备: %v", err) + } + return a +} + +// TestDeniedToolStillRecordsResult 是主判据。 +// +// 变异：删掉 stepToolBegin 拒绝分支里的 f.appendToolResultRecord(...) ⇒ 本例红。 +func TestDeniedToolStillRecordsResult(t *testing.T) { + tc := agentAPI.ToolCall{ID: "call_deny_1", Name: "denied_tool", Arguments: map[string]interface{}{}} + a := newDeniedAgent(t, tc) + + // ★ 真的会拒绝的 before_toolcall 钩子（拒绝 = ctx.Response != nil）。 + a.stageHost.RegisterStage(sdk.StageBeforeToolcall, func(ctx *sdk.StageContext) error { + reason := "被来源权限策略拦下（非所有者私聊）" + ctx.Lock() + ctx.Response = &reason + ctx.Unlock() + return nil + }) + + f := a.newTaskFrame("跑一下被拒绝的工具", a.stageCtxFromInput("跑一下", "", "")) + if out := a.runTaskSteps(f); out != outcomeDone { + t.Fatalf("runTaskSteps = %v, err=%v", out, f.Err) + } + + // 前置条件：确认这个工具**确实**被记成了一次调用（否则本判据空转）。 + if len(f.ToolCalls) == 0 { + t.Fatal("★ 没有记录任何 ToolCall —— 前置条件不成立，判据会空转") + } + + answered := map[string]bool{} + for _, r := range f.ToolResults { + answered[r.CallID] = true + } + for _, c := range f.ToolCalls { + if !answered[c.CallID] { + t.Fatalf("★ call %s (%s) 没有对应的 ToolResults ——\n"+ + "ContextEvent 会以 ToolCalls=%d / ToolResults=%d 落盘，\n"+ + "渲染出 assistant(tool_calls) 却没有它的结果 ⇒ 上游 400。\n"+ + "实测同型故障：qq_call_state 被权限策略拒绝后每一轮都触发。", + c.CallID, c.Name, len(f.ToolCalls), len(f.ToolResults)) + } + } + + // 被拒绝的结果必须带上**拒绝原因**（模型看到什么，落盘就该是什么）。 + for _, r := range f.ToolResults { + if r.CallID == tc.ID && !strings.Contains(r.Output, "权限策略") { + t.Fatalf("★ 落盘的拒绝原因丢了（Output=%q）——\n"+ + "模型从 f.Msgs 看到的是原因，context 里却是别的，两者必须一致", + r.Output) + } + } +} + +// TestUnhealthyPluginSkipStillRecordsResult 钉住第二条同构分支： +// 插件处于崩溃态、工具被跳过时，同样必须落 result。 +// +// 变异：删掉该分支的 f.appendToolResultRecord(...) ⇒ 本例红。 +func TestUnhealthyPluginSkipStillRecordsResult(t *testing.T) { + tc := agentAPI.ToolCall{ID: "call_unhealthy_1", Name: "denied_tool", Arguments: map[string]interface{}{}} + a := newDeniedAgent(t, tc) + + // 把该工具所属的插件标成崩溃态。 + // + // 用 recordCrash 打到阈值（maxPluginCrashes=3）而不是直接改私有字段 —— + // 这样判据依赖的是**公开状态机**，字段改名不会让它变成"编译不过"以外的 + // 沉默失效。插件名必须与 stepToolBegin 里 resolveToolPlugin 的结果一致， + // 否则 isHealthy 永远返回 true，判据会空转。 + // 显式登记工具归属，否则 resolveToolPlugin 会把它归到 "core" + // （builtinToolOwner 的兜底），而 health tracker 是按插件名记账的。 + const pluginName = "denyplug" + if err := a.stageHost.RegisterTool(tc.Name, sdk.ToolDef{ + Name: tc.Name, Description: "x", Plugin: pluginName, + }, func(map[string]interface{}) (interface{}, error) { + t.Fatalf("★ 被跳过的工具不该执行") + return nil, nil + }); err != nil { + t.Fatalf("登记工具归属: %v", err) + } + if got := a.resolveToolPlugin(tc.Name); got != pluginName { + t.Fatalf("★ resolveToolPlugin=%q，期望 %q —— 前置条件不成立", got, pluginName) + } + // ★ 直接置 Unhealthy，不用 recordCrash 打满阈值。 + // + // 为什么不走 recordCrash：实测发现它标记 Unhealthy 之后，isHealthy **立刻** + // 判回健康 —— 因为 recordCrash 从不设置 LastReload，而 isHealthy 的冷却 + // 判据是 `time.Since(r.LastReload) > reloadCooldown`，零值的 Since 是巨大的 + // 正数 ⇒ 冷却期"早已过去" ⇒ 熔断自我解除。 + // + // 这是**另一个独立缺陷**（插件崩溃熔断实际从不生效），不在本次修复范围 + // （本次只修 tool 配对）。这里直达状态，把判据聚焦在被测分支上； + // 熔断那个缺陷应单独修并单独钉。 + // LastReload 必须设成"刚刚"，否则 isHealthy 的冷却判据 + //（time.Since(LastReload) > reloadCooldown）立刻判为已冷却 ⇒ 自愈。 + a.pluginHealth.mu.Lock() + a.pluginHealth.records[pluginName] = &pluginHealthRecord{ + Unhealthy: true, + LastReload: time.Now(), // 冷却期未过 ⇒ 熔断生效 + } + a.pluginHealth.mu.Unlock() + if a.pluginHealth.isHealthy(pluginName) { + t.Fatalf("★ 置 Unhealthy 后 %s 仍被判健康，判据前置条件不成立", pluginName) + } + + f := a.newTaskFrame("跑一下", a.stageCtxFromInput("跑一下", "", "")) + if out := a.runTaskSteps(f); out != outcomeDone { + t.Fatalf("runTaskSteps = %v, err=%v", out, f.Err) + } + if len(f.ToolCalls) == 0 { + t.Fatal("★ 没有记录 ToolCall —— 前置条件不成立") + } + answered := map[string]bool{} + for _, r := range f.ToolResults { + answered[r.CallID] = true + } + for _, c := range f.ToolCalls { + if !answered[c.CallID] { + t.Fatalf("★ 被跳过的工具 %s (%s) 没落 ToolResults —— 同型失配，上游 400", + c.CallID, c.Name) + } + } +} diff --git a/internal/agent/core/task.go b/internal/agent/core/task.go index e7bf54dd..dc75bf18 100644 --- a/internal/agent/core/task.go +++ b/internal/agent/core/task.go @@ -1115,6 +1115,10 @@ func (a *Agent) stepToolBegin(f *TaskFrame) stepOutcome { result := denialResultText(tctx, tc.Name) f.ensureBatchAssistant() f.Msgs = append(f.Msgs, agentAPI.Message{Role: "tool", ToolCallID: tc.ID, Content: result}) + // ★ 必须与 f.Msgs 同步落 context（见 appendToolResultRecord 的说明）。 + // 漏掉这行的实测后果：qq_call_state 被权限策略拒绝后，ContextEvent 里 + // 有 call 没有 result，每一轮都触发上游 400，直到发送前自检把它剔掉。 + f.appendToolResultRecord(tc.ID, tc.Name, result) a.publishEvent(events.EventToolCall, map[string]interface{}{ "tool": tc.Name, "plugin": pluginName, @@ -1133,6 +1137,8 @@ func (a *Agent) stepToolBegin(f *TaskFrame) stepOutcome { log.Printf("[agent] skip tool %s: plugin %s unhealthy", tc.Name, pluginName) f.ensureBatchAssistant() f.Msgs = append(f.Msgs, agentAPI.Message{Role: "tool", ToolCallID: tc.ID, Content: result}) + // 同上：跳过执行也要落 result，否则 call/result 失配。 + f.appendToolResultRecord(tc.ID, tc.Name, result) f.ToolIdx++ return outcomeContinue } @@ -1192,6 +1198,41 @@ func (f *TaskFrame) appendToolCallRecord(tc agentAPI.ToolCall) { }) } +// appendToolResultRecord 追加一条工具结果记录（与 appendToolCallRecord 成对）。 +// +// ★ 为什么不变量必须由**每一次**调用 path 维护（2026-11-04 生产故障） +// +// task.go 顶部的注释写着「ToolCalls 与 ToolResults 通过 CallID 配对； +// 两者都由 appendToolCallRecord 同步追加」。但"跳过执行"的三条提前返回 +// 分支只补了 f.Msgs（模型能看到），**没写 f.ToolResults**（落 context 的那份）。 +// +// 后果不是"少一条记录"这么轻： +// +// stepToolBegin appendToolCallRecord(tc) ← 记了 call +// before_toolcall 钩子拒绝 → return ← 没记 result +// ⇒ ContextEvent{ ToolCalls: 2, ToolResults: 1 } +// ⇒ 渲染出 assistant(tool_calls=[c1,c2]) 但只有 c1 的结果 +// ⇒ 上游硬拒绝（HTTP 400）：insufficient tool messages following tool_calls +// ⇒ 脏数据持久化进 context.json，之后**每一轮**都带着它 +// +// 实测（qq 插件的来源权限策略拒绝了 qq_call_state）： +// +// [42] tool_calls=[qq_call_state, output_send__qq] +// tool_results=[output_send__qq] ← call_state 的结果从未落盘 +// +// 判定"被拒绝/被跳过"≠"没有结果"：拒绝原因本身就是结果，而且模型 +// 确实看到了它（f.Msgs 里那条 tool 消息）。落 context 的必须与发给模型的一致。 +func (f *TaskFrame) appendToolResultRecord(callID, name, output string) { + if f == nil { + return + } + f.ToolResults = append(f.ToolResults, ToolResultItem{ + CallID: callID, + Name: name, + Output: output, + }) +} + // stepToolExec 执行工具。**临界区**：见设计文档 §4.3。 func (a *Agent) stepToolExec(f *TaskFrame) stepOutcome { // 执行工具前先解析本轮场景：写侧要用它给记忆自动挂场景（主动+被动两条路），