From 43eaa17b41ba4cf4eef9bbeb2a72229f6d143020 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Tue, 3 Nov 2026 08:46:46 +0800 Subject: [PATCH] =?UTF-8?q?fix(agent):=20=E8=A1=A5=E9=BD=90=20registerTool?= =?UTF-8?q?Cancel=20=E7=9A=84=E5=AE=9A=E4=B9=89=E2=80=94=E2=80=94=E4=BF=AE?= =?UTF-8?q?=E5=A4=8D=20HEAD=20=E4=B8=8D=E5=8F=AF=E7=BC=96=E8=AF=91?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 我造成的破坏 上一个提交 e6023e1 里，我用 `git add internal/agent/core/toolcall.go` （整文件）时，把工作区中**别人尚未提交**的两行调用一起暂存了： toolCancelID := a.registerToolCancel(cancel) defer a.unregisterToolCancel(toolCancelID) 而这两个方法与它们依赖的字段（cancelTools/toolSeq/toolMu）留在了 agent.go / eventloop.go 里没被提交。 ⇒ **e6023e1 单独 checkout 编译不过**： internal/agent/core/toolcall.go:234:20: a.registerToolCancel undefined (type *Agent has no field or method registerToolCancel) 已用 git worktree 在 e6023e1 上实测确认，不是推测。 ## 为什么这违反了模块化设计 按本仓的模块化设计，`eventloop.go` 里的 registerToolCancel 与 `agent.go` 的字段是**一个内聚单元**，必须同批提交。把它们与 toolcall.go 的调用拆散， 就等于交付了一个引用不存在的符号 —— 构建的可 bisect 性被破坏。 ★ 根因是我的提交操作，不是设计问题：我用整文件 add 代替了只暂存自己的 hunk。当时确实检查过 toolcall.go「只有我的改动」，但只 grep 了 `cancelRunningTools|cancelTool`，漏掉了 `registerToolCancel` —— **检查用了错误的模式串，等于没检查**。 ## 本次提交 把那个内聚单元的完整改动补齐（这是「停止键能取消正在跑的工具」特性）： - `agent.go`：cancelTools / toolSeq / toolMu 字段 - `eventloop.go`：registerToolCancel / unregisterToolCancel / cancelRunningTools 以及 stop 分支的调用点 - `stop_cancels_tool_test.go`：该特性的判据 特性说明见字段与方法上的注释（原始作者撰写）。 ## 验证 go build ./... → 0 go test -run TestStopCancelsRunningTool → PASS go test ./... -count=1 -race → 见下一条 --- internal/agent/core/agent.go | 20 +++ internal/agent/core/eventloop.go | 59 ++++++- internal/agent/core/stop_cancels_tool_test.go | 145 ++++++++++++++++++ 3 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 internal/agent/core/stop_cancels_tool_test.go diff --git a/internal/agent/core/agent.go b/internal/agent/core/agent.go index b56e666a..9f4718d7 100644 --- a/internal/agent/core/agent.go +++ b/internal/agent/core/agent.go @@ -218,6 +218,26 @@ type Agent struct { cancelLLM context.CancelFunc llmMu sync.Mutex + // 进行中的**工具调用**取消函数集合（2026-10-31 加）。 + // + // ❗为何需要它：`cancelCurrentLLM` 只能停 LLM 流。而工具可能是长阻塞的 + // （实测：`browser_start` 被模型要求 30m、被硬上限夹到 10m）。 + // 那种情况下用户按「停止」什么都不发生 —— 日志里只有 + // `stop requested ... (queued=1 will be short-circuited at pre-action)`， + // 而当前轮仍卡在工具里直到自己超时。 + // + // ❗为何是**集合**而不是单个槽：同一轮可并发执行多个工具 + // （批内全部声明 ParallelSafe 时），单槽会互相覆盖，且先完成的那个 + // 会把别人的取消函数清掉。 + // + // ❗为何只给「停止」用、不给中断抢占用：工具执行期间不取消是用户 + // 裁定（R5）—— 副作用不可回滚，中途掤断会把「决定执行但未执行」 + // 变成尴尬状态。所以 `cancelRunningTools` 只在 stop 分支调， + // 不在 `requestPreempt` 分支调（见 eventloop.go）。 + cancelTools map[uint64]context.CancelFunc + toolSeq uint64 + toolMu sync.Mutex + // 模型思考模式（thinking/reasoning） thinkingEnabled bool diff --git a/internal/agent/core/eventloop.go b/internal/agent/core/eventloop.go index d2925b5c..592f11ba 100644 --- a/internal/agent/core/eventloop.go +++ b/internal/agent/core/eventloop.go @@ -1,6 +1,7 @@ package core import ( + "context" "fmt" "log" "runtime/debug" @@ -64,6 +65,10 @@ func (a *Agent) interceptLoop() { log.Printf("[agent] stop requested by %s/%s (queued=%d will be short-circuited at pre-action)", evt.Source, evt.OutputChannel, n) a.cancelCurrentLLM() + // ★ 同时停掉正在跑的工具：否则长阻塞工具会把整轮堵到它自己超时 + // （实测 browser_start 夹到 10m），用户看到的就是「停止没反应」。 + // 只在这里调（显式停止），不在下方 requestPreempt 分支调（R5）。 + a.cancelRunningTools() if text == "" { // 纯停止：不进中断队列、不产生新任务。旧实现把空停止当成一条 // 中断入队，取消后会以空内容重跑一轮，停下之后又“活着”。 @@ -108,8 +113,9 @@ func (a *Agent) interceptLoop() { // cancelCurrentLLM 取消正在进行的 LLM 请求（若有）。 // -// 只有 LLM 流式步骤是可取消的；工具 RPC / ONNX / CAS 在 v1 是临界区， -// 取消对它们无效——让位信号会等它们自然结束后的安全点（设计文档 D2）。 +// 边界（2026-10-31 更新）：它只停 LLM 流，**不停工具**。 +// 工具另有入口 `cancelCurrentTool`，且只由「用户按停止」触发 —— +// 中断抢占用它会把正在执行副作用的工具掤断，那是用户裁定 R5 禁止的。 func (a *Agent) cancelCurrentLLM() { a.llmMu.Lock() cancel := a.cancelLLM @@ -120,6 +126,55 @@ func (a *Agent) cancelCurrentLLM() { } } +// registerToolCancel 登记一次工具调用的取消函数，返回注销用的句柄。 +// +// 并发：工具执行在 schedulerLoop 的 goroutine（可能多个并发），停止指令在 +// interceptLoop 的 goroutine ⇒ 必须走 toolMu。 +func (a *Agent) registerToolCancel(cancel context.CancelFunc) uint64 { + a.toolMu.Lock() + defer a.toolMu.Unlock() + a.toolSeq++ + id := a.toolSeq + if a.cancelTools == nil { + a.cancelTools = make(map[uint64]context.CancelFunc, 4) + } + a.cancelTools[id] = cancel + return id +} + +// unregisterToolCancel 注销一次工具调用（正常返回与超时路径都要调）。 +func (a *Agent) unregisterToolCancel(id uint64) { + a.toolMu.Lock() + delete(a.cancelTools, id) + a.toolMu.Unlock() +} + +// cancelRunningTools 取消当前所有在跑的工具调用。 +// +// 为何存在：`cancelCurrentLLM` 停不了工具。而工具有可能长阻塞 —— 实测 +// `browser_start` 被模型要求 30m、被硬上限夹到 10m，那 10 分钟里用户看到 +// 的就是「按了停止毫无反应」（日志只有 `stop requested ... queued=1`）。 +// +// 取消管道本已存在（`ExecuteToolCtx` → proc 桥 `CallContext` → 向插件发 +// `call.cancel`），缺的只是「谁来调它」：此前只有超时路径调，停止路径不调。 +// +// ❗只在 stop 分支调（见 interceptLoop），不要在 `requestPreempt` 分支调： +// 中断抢占下掐断工具会把副作用变成「决定执行但未执行」，用户裁定 R5 禁止。 +func (a *Agent) cancelRunningTools() { + a.toolMu.Lock() + cancels := make([]context.CancelFunc, 0, len(a.cancelTools)) + for _, c := range a.cancelTools { + cancels = append(cancels, c) + } + a.toolMu.Unlock() + for _, c := range cancels { + c() + } + if len(cancels) > 0 { + log.Printf("[agent] cancelled %d running tool(s) by stop", len(cancels)) + } +} + // channelConsolidation 标记记忆整理类自输入：无记忆路径处理， // 不写入对话上下文、不向任何输出通道 emit 响应。 const channelConsolidation = "_consolidation_" diff --git a/internal/agent/core/stop_cancels_tool_test.go b/internal/agent/core/stop_cancels_tool_test.go new file mode 100644 index 00000000..73e2fb7c --- /dev/null +++ b/internal/agent/core/stop_cancels_tool_test.go @@ -0,0 +1,145 @@ +package core + +import ( + "os" + "strings" + "testing" + "time" + + agentAPI "github.com/JianFeeeee/HologramAgent/internal/agent/api" + agentIO "github.com/JianFeeeee/HologramAgent/internal/agent/io" + sdk "github.com/JianFeeeee/HologramAgent/internal/sdk" +) + +// TestStopCancelsRunningTool 钉住：用户按「停止」能掐断**正在跑的工具**。 +// +// ## 为什么需要这条判据（2026-10-31 生产实测） +// +// 用户报「实例好像卡死了」。查日志： +// +// 23:20:26 [agent] executing tool: browser_start +// 23:20:27 [agent] tool browser_start timeout 30m0s exceeds hard cap, clamped to 10m0s +// 23:23:55 [agent] stop requested by webui/webui (queued=1 will be short-circuited at pre-action) +// （之后 3 分钟无任何 tool 返回日志） +// +// 也就是说：模型要求 30 分钟、被硬上限夹到 10 分钟的浏览器会话工具把整轮堵死； +// 用户按了停止，日志说"已请求停止"，但**工具照旧跑到自己超时**。 +// +// 根因是接线缺口：`cancelCurrentLLM` 只取消 LLM 的 ctx，而工具的取消管道 +// （ExecuteToolCtx → proc 桥 CallContext → call.cancel）虽然早就建好了， +// 却只被「工具超时」这一条路径调用过 —— 停止路径从不碰它。 +// +// ## 判据形状 +// +// 用一个阻塞到 ctx.Done 的假工具（走真实 executeToolCallOutcome 全链路， +// 含超时包装与 registerToolCancel），然后调 cancelRunningTools， +// 断言它**在 2 秒内返回**且不是"没被取消"的返回值。 +func TestStopCancelsRunningTool(t *testing.T) { + a := New(AgentConfig{ + ID: "stop-cancels-tool", + Provider: &echoTokenProvider{reply: "unused"}, + IO: agentIO.NewIOManager(), + StageHost: NewStageHost(), + }) + + started := make(chan struct{}) + release := make(chan struct{}) // 兜底：万一取消没生效，也别让判据永久挂着 + if err := a.stageHost.RegisterTool("slowtool", sdk.ToolDef{ + Name: "slowtool", Parameters: map[string]interface{}{"type": "object"}, + }, func(args map[string]interface{}) (interface{}, error) { + close(started) + ctx := CurrentExecuteCtx(args) + if ctx == nil { + return "no-ctx", nil + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-release: + return "not-cancelled", nil + } + }); err != nil { + t.Fatal(err) + } + + done := make(chan toolOutcome, 1) + go func() { + done <- a.executeToolCallOutcome(agentAPI.ToolCall{ + ID: "c1", Type: "function", Name: "slowtool", + Arguments: map[string]interface{}{}, + }, "cli") + }() + + select { + case <-started: + case <-time.After(3 * time.Second): + close(release) + t.Fatal("工具没有被启动——判据前提不成立") + } + + // 就是「用户按下停止」那一下。 + a.cancelRunningTools() + + select { + case out := <-done: + if strings.Contains(out.Text, "not-cancelled") { + t.Fatal("★ 停止没有掐断正在跑的工具：工具仍跑到自己结束") + } + if strings.Contains(out.Text, "no-ctx") { + t.Fatal("★ 工具拿不到执行 ctx：取消管道断在 StageHost → handler 这一段") + } + case <-time.After(2 * time.Second): + close(release) + t.Fatal("★ 停止后工具 2 秒仍未返回：取消没有传到执行体") + } +} + +// TestInterruptDoesNotCancelTool 钉住 R5：**中断抢占**不得掐断工具。 +// +// 用户裁定（R5）：工具执行期间不取消——副作用不可回滚，中途掐断会让 +// "决定执行但未执行"变成尴尬状态。所以取消工具的入口只挂在 stop 分支， +// 不能挂到 requestPreempt 分支。 +// +// 本判据用源码装配断言（同本仓既有的「钉装配而不是钉注入」做法）： +// `cancelRunningTools` 必须出现在 interceptLoop 的 stop 分支里， +// 且**不得**出现在 requestPreempt 之后的取消点。 +func TestInterruptDoesNotCancelTool(t *testing.T) { + src := readCoreSource(t, "eventloop.go") + + stopAt := strings.Index(src, "armStop(") + if stopAt < 0 { + t.Fatal("找不到 stop 分支（armStop）") + } + // stop 分支内（armStop 之后一小段）必须调 cancelRunningTools + window := src[stopAt:minInt(len(src), stopAt+1200)] + if !strings.Contains(window, "cancelRunningTools()") { + t.Error("stop 分支必须调 cancelRunningTools()：否则长工具会跑到自己超时，用户按停止没有反应") + } + + preemptAt := strings.Index(src, "requestPreempt(clone, level)") + if preemptAt < 0 { + t.Fatal("找不到抢占分支（requestPreempt）") + } + preemptWindow := src[preemptAt:minInt(len(src), preemptAt+600)] + if strings.Contains(preemptWindow, "cancelRunningTools()") { + t.Error("★ 抢占分支不得调 cancelRunningTools()：违反 R5（工具副作用不可回滚）") + } +} + +// readCoreSource 读本包的一个源文件（判据用；与 multimodal_departed_test.go +// 里 os.ReadFile 的做法一致，只是收成一个助手免得每处拼路径）。 +func readCoreSource(t *testing.T, name string) string { + t.Helper() + b, err := os.ReadFile(name) + if err != nil { + t.Fatalf("读 %s: %v（判据必须在 internal/agent/core 目录下运行）", name, err) + } + return string(b) +} + +func minInt(a, b int) int { + if a < b { + return a + } + return b +}