From f833897d40f7c4becf6f006a8da878b5cb58a6ef Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Sun, 25 Oct 2026 20:16:06 +0800 Subject: [PATCH] =?UTF-8?q?fix(agentcli):=20=E4=B8=8D=E5=86=8D=E6=8A=8A?= =?UTF-8?q?=E8=87=AA=E9=80=A0=E7=8A=B6=E6=80=81=E5=BD=93=E6=96=B0=E4=BA=8B?= =?UTF-8?q?=E4=BB=B6=E6=B3=A8=E5=85=A5=20=E2=80=94=E2=80=94=20=E6=AF=8F?= =?UTF-8?q?=E6=9D=A1=E9=83=BD=E7=83=A7=E4=B8=80=E6=95=B4=E8=BD=AE=E6=8E=A8?= =?UTF-8?q?=E7=90=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 问题（WSL2 真机诊断报告问题 4，影响最大）：开终端注入「已启动」、 超时注入「已超时关闭」，每条都唤醒一整轮 LLM 推理。实测后果是 agent 反复向用户解释"终端怎么又没了"，真正要做的活一件没做。 这属于本插件**自身逻辑矛盾**，不是内核缺陷： 1. 「已启动」是与 terminal_create 返回值的重复告知。返回值已含 id/command/timeout/rows/cols/notify_mode，信息完整。 2. 「已超时关闭」在创建时已由 timeout 字段告知，agent 本可推导； 且注入后紧接着 removeSession，agent 去 terminal_list 查必然是 0 个， 只能盲目排查——而这是插件自己删的。 3. 同一函数里 IsExpired() 语义相反：r.err 分支把它列为**必须静默**， 而超时分支却在注入。 4. 本插件自己的 notify=exit/interval/buffer 参数本就定义了"什么该通知"。 两处无条件注入是不经该配置的旁路，与自身设计相矛盾。 顺带：r.err 不再以 %v 原文进 prompt（原来注入 `read /dev/ptmx: input/output error`，把 agent 引去查 devpts 挂载， 好几轮推理花在这上面）。改为给**结论**「读取结束」，原始错误只进日志。 判据（plugin_test.go）： · 新增 2 条**反向判据** TestNoStartupNotificationInjected / TestNoExpiredNotificationInjected —— 没有它们，后人把注入加回来时 其它测试依然全绿，缺陷会静默复发 · 原 7 处用「等已启动通知」做时序同步的测试改为 waitReadLoopReady （轮询 mockTerm.readCalls()，不靠固定 sleep 也不依赖业务通知） ★ 保留不动：进程自己结束/读取错误的通知（agent 无法从工具返回推导）， 由 watch 配置控制。判据 TestProcessExit_StillNotifies 钉住这一点。 --- internal/plugins/agentcli/plugin.go | 47 +++++-- internal/plugins/agentcli/plugin_test.go | 169 +++++++++++++++++++---- 2 files changed, 183 insertions(+), 33 deletions(-) diff --git a/internal/plugins/agentcli/plugin.go b/internal/plugins/agentcli/plugin.go index 06dedd56..a2556224 100644 --- a/internal/plugins/agentcli/plugin.go +++ b/internal/plugins/agentcli/plugin.go @@ -995,11 +995,19 @@ func (p *Plugin) readLoop(t *TerminalSession, s *sdk.PluginSDK) { defer emitTermState(s, t, false) defer flushTermStream(s, t) - // 立即发送首次"终端已启动"通知，让 agent 感知存在。 - // 用 NoMemory：这是状态提示，不是对话内容。不关掉的话每开一个终端都会 - // 在记忆里留下一条"[终端 X 已启动]"，把真实内容挤掉。 - s.InjectTextOpts("agentcli", "agentcli", fmt.Sprintf("[终端 %s 已启动]", t.id), - sdk.InjectOptions{NoMemory: true}) + // ★ 不注入「终端已启动」通知（2026-10-25 移除）。 + // + // 理由：terminal_create 的**返回值本身**已经告知 agent 终端已建立， + // 且带 id/command/timeout/rows/cols/notify_mode —— 信息完整。 + // 再用「用户输入」的形式注入一遍，等于同一条信息告知两次， + // 而每次注入都会唤醒**一整轮 LLM 推理**。 + // + // 实测代价（WSL2 真机）：开一个终端 → 一轮推理；关一个 → + // 又一轮。agent 反复向用户解释"终端怎么又没了"，真正要做的活一件没做。 + // + // 设计依据（本插件自己的 notify 参数）：什么该通知，本来就由 agent + // 在创建时通过 notify=exit/interval/buffer 指定。无条件旁路注入 + // 与本插件自身的设计相矛盾。 now := time.Now() t.mu.Lock() t.lastNotify = now @@ -1023,9 +1031,18 @@ func (p *Plugin) readLoop(t *TerminalSession, s *sdk.PluginSDK) { } if t.IsExpired() { - log.Printf("[agentcli] terminal %s expired after %v", t.id, t.timeout) - s.InjectTextOpts("agentcli", "agentcli", fmt.Sprintf("[终端 %s 已超时关闭（%s）]", t.id, t.timeout), - sdk.InjectOptions{NoMemory: true}) + // ★ 只记日志，**不注入**（2026-10-25 修正）。 + // + // 两条理由： + // ① 超时时长已在创建时由 terminal_create 的 timeout 字段告知， + // agent 完全能推导"它 5 分钟后会自己关"——再通知没有新信息。 + // ② 注入后紧接着 removeSession，agent 去 terminal_list 查必然是 + // 0 个，于是只能一遍遍排查"终端为什么消失"，而这是插件自己删的。 + // + // 注意与下方 r.err 分支的一致性：那里已经把「超时清理」列为 + // **必须静默**；同一函数内同一个 IsExpired() 不能一处静默一处注入。 + log.Printf("[agentcli] terminal %s expired after %v (silent: timeout already reported at create)", + t.id, t.timeout) p.removeSession(t.id) return } @@ -1096,7 +1113,19 @@ func (p *Plugin) readLoop(t *TerminalSession, s *sdk.PluginSDK) { if p.isClosing(t) || t.IsExpired() { return // 主动关闭 / 超时清理：不注入 } - s.InjectTextOpts("agentcli", "agentcli", fmt.Sprintf("[终端 %s 读取结束: %v]", t.id, r.err), + // ★ 不把 r.err 原文灌进 prompt（2026-10-25 修正）。 + // + // 原来注入的是 `[终端 X 读取结束: read /dev/ptmx: input/output error]`。 + // agent 会把它当成需要排查的技术问题，花好几轮去查 + // /dev/ptmx 是否存在、devpts 挂载是否正常——虽然结论是对的 + // （EIO 是 pty 主端关闭后 read 的正常返回），但这根本不该 + // 成为 agent 的活，而且每一轮查询都在烧上下文。 + // + // 改为：给**结论**（进程已结束）而不是**机制**（什么 errno）。 + // 原始错误只进日志，需要时人工看得到。 + log.Printf("[agentcli] terminal %s read ended: %v", t.id, r.err) + s.InjectTextOpts("agentcli", "agentcli", + fmt.Sprintf("[终端 %s 读取结束]", t.id), sdk.InjectOptions{NoMemory: true}) return } diff --git a/internal/plugins/agentcli/plugin_test.go b/internal/plugins/agentcli/plugin_test.go index 41ea7a3a..c47b95ba 100644 --- a/internal/plugins/agentcli/plugin_test.go +++ b/internal/plugins/agentcli/plugin_test.go @@ -480,6 +480,16 @@ type mockTerm struct { data chan []byte running bool err error + // reads 记录 Read 被调用次数，供 waitReadLoopReady 判断 readLoop 已起来。 + // 不用固定 sleep：机器慢时 sleep 不够、快时白等。 + reads int +} + +// readCalls 返回 Read 被调用次数（并发安全）。 +func (m *mockTerm) readCalls() int { + m.mu.Lock() + defer m.mu.Unlock() + return m.reads } func newMockTerm() *mockTerm { @@ -487,6 +497,9 @@ func newMockTerm() *mockTerm { } func (m *mockTerm) Read(buf []byte) (int, error) { + m.mu.Lock() + m.reads++ + m.mu.Unlock() for { m.mu.Lock() err := m.err @@ -517,6 +530,7 @@ func (m *mockTerm) Running() bool { defer m.mu.Unlock() return m.running } + // Kill 模拟真实 PTY 的行为：把一份 **buffer 里的输出 + EIO** 交给 Read。 // // 两点必须还原，否则测不出线上的回声： @@ -570,6 +584,26 @@ func startReadLoop(p *Plugin, s *sdk.PluginSDK, t *TerminalSession) { go p.readLoop(t, s) } +// waitReadLoopReady 等 readLoop 真正进入工作状态。 +// +// ★ 为什么需要它：原测试用「已启动」注入当同步点（等那条通知出现再继续）。 +// 但 2026-10-25 起该注入已移除（它是"同一条信息告知两次"，每条都会唤醒 +// 一整轮推理）。同步点因此需要换成不依赖注入的方式。 +// +// 这里用「mockTerm 的 Read 至少被调用过一次」作为信号：readLoop 的第一件事 +// 就是 Read，比固定 sleep 可靠，也不引入对业务通知的依赖。 +func waitReadLoopReady(t *testing.T, term *mockTerm) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if term.readCalls() > 0 { + return + } + time.Sleep(2 * time.Millisecond) + } + t.Fatal("readLoop 未在 2s 内进入读取状态") +} + func waitInjected(c *injectCapture, substr string, timeout time.Duration) bool { deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { @@ -602,9 +636,9 @@ func TestReadLoopNotifyThrottle(t *testing.T) { ts := newTestSession(term) startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, term) // 持续以 100B/50ms(=2KB/s) 吐进度 3 秒 stop := make(chan struct{}) @@ -666,9 +700,9 @@ func TestReadLoopNotifyOnExit(t *testing.T) { ts := newTestSession(term) startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, term) term.setRunning(false) gotExit := waitInjected(capture, "进程已退出", 2*time.Second) @@ -698,9 +732,9 @@ func TestReadLoopNotifyOnReadError(t *testing.T) { ts := newTestSession(term) startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, term) term.setErr(fmt.Errorf("read timeout")) if !waitInjected(capture, "读取结束", 3*time.Second) { @@ -740,13 +774,13 @@ func TestClose_NoEchoNotificationsAfterClose(t *testing.T) { }) sdkInst.SetIOInjector(capture) - ts := newTestSession(newMockTerm()) + mterm := newMockTerm() + ts := newTestSession(mterm) startReadLoop(p, sdkInst, ts) - // 等"已启动"到达：这是**正常**通知（终端确实刚创建）。 - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, mterm) before := len(capture.snapshot()) // 主动关闭：模拟 close 工具路径（关 stopCh + 杀进程 + 释放 PTY）。 @@ -794,9 +828,9 @@ func TestProcessExit_StillNotifies(t *testing.T) { ts := newTestSession(term) startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, term) // 进程自己退出（**不是** Close）：mockTerm 返回 process exited。 term.mu.Lock() @@ -835,9 +869,7 @@ func TestClose_NoEcho_Repeated(t *testing.T) { term := newMockTerm() ts := newTestSession(term) startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + waitReadLoopReady(t, term) before := len(capture.snapshot()) // ★ 关键：先在 PTY 缓冲里**留下未读数据**，再关闭。 @@ -891,16 +923,17 @@ func TestClose_RemovesSessionFromTable(t *testing.T) { }) sdkInst.SetIOInjector(capture) - ts := newTestSession(newMockTerm()) + mterm2 := newMockTerm() + ts := newTestSession(mterm2) ts.id = "term_leak" p.mu.Lock() p.sessions = map[string]*TerminalSession{"term_leak": ts} p.mu.Unlock() startReadLoop(p, sdkInst, ts) - if !waitInjected(capture, "已启动", 2*time.Second) { - t.Fatal("expected startup notification") - } + // 等 readLoop 真正起来（不再依赖"已启动"注入：那已被移除， + // 理由见 plugin.go 中"不注入终端已启动通知"的说明） + waitReadLoopReady(t, mterm2) ts.Close() // 内部会 <-t.done，等 readLoop 走完 @@ -952,3 +985,91 @@ func TestCleanup_DoesNotNotifyForClosedTerminal(t *testing.T) { } _ = capture } + +// ── 2026-10-25 新增：禁止「自造状态回声」的判据 ────────────────── +// +// 背景（真机事故）：agent 通过 terminal_create 建终端后，插件注入 +// 「[终端 X 已启动]」；超时后又注入「[终端 X 已超时关闭]」。 +// 这两条都是**插件自己造成、且 agent 已能从工具返回值推导**的状态， +// 而每条注入都会唤醒一整轮 LLM 推理。实测后果：agent 反复向用户 +// 解释"终端怎么又没了"，真正要做的活一件没做。 +// +// 下面两条是**反向判据**：断言这些注入**不再出现**。没有它们， +// 后人（或我自己）很容易为了"让 agent 感知"把注入加回来，而那时 +// 上面那些 waitReadLoopReady 的测试依然全绿 —— 缺陷会静默复发。 + +// 判据一：建终端不得产生「已启动」注入（terminal_create 返回值已足够）。 +func TestNoStartupNotificationInjected(t *testing.T) { + p := New("agentcli") + p.notifyBytes = 1 << 20 // 不触发缓冲通知，隔离变量 + + capture := &injectCapture{} + sdkInst := sdk.New("agentcli", sdk.SDKConfig{ + RegTool: newToolCapture().RegisterTool, + RegStage: func(sdk.Stage, sdk.StageHandler) {}, + RegAPI: func(string) error { return nil }, + Settings: sdk.NewSettings("agentcli", nil), + }) + sdkInst.SetIOInjector(capture) + + term := newMockTerm() + ts := newTestSession(term) + startReadLoop(p, sdkInst, ts) + waitReadLoopReady(t, term) + + // 给足时间让任何潜在注入发生 + time.Sleep(150 * time.Millisecond) + + for _, text := range capture.snapshot() { + if strings.Contains(text, "已启动") { + t.Fatalf("不得注入「已启动」（terminal_create 返回值已告知）: %q", text) + } + } + + close(ts.stopCh) + <-ts.done +} + +// 判据二：超时关闭不得注入（创建时 timeout 已告知）。 +func TestNoExpiredNotificationInjected(t *testing.T) { + p := New("agentcli") + p.notifyBytes = 1 << 20 + + capture := &injectCapture{} + sdkInst := sdk.New("agentcli", sdk.SDKConfig{ + RegTool: newToolCapture().RegisterTool, + RegStage: func(sdk.Stage, sdk.StageHandler) {}, + RegAPI: func(string) error { return nil }, + Settings: sdk.NewSettings("agentcli", nil), + }) + sdkInst.SetIOInjector(capture) + + term := newMockTerm() + ts := newTestSession(term) + // 设为已超时：readLoop 下一轮循环就会走 IsExpired 分支 + ts.mu.Lock() + ts.timeout = time.Nanosecond + ts.createdAt = time.Now().Add(-time.Hour) + ts.mu.Unlock() + + startReadLoop(p, sdkInst, ts) + waitReadLoopReady(t, term) + + // 等 removeSession 真的发生（说明走了超时分支） + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + p.mu.Lock() + _, still := p.sessions[ts.id] + p.mu.Unlock() + if !still { + break + } + time.Sleep(2 * time.Millisecond) + } + + for _, text := range capture.snapshot() { + if strings.Contains(text, "已超时关闭") { + t.Fatalf("不得注入「已超时关闭」（创建时 timeout 已告知）: %q", text) + } + } +}