Files
Agent-System-Dev/.task/reviews/2026-10-22-agent-stack-b2-independent-review.md
T

16 KiB
Raw Blame History

独立代码 Review:feature/agent-stack-audit-remediation-b2(U8 / U9 / U11a-c)

  • 单元:agent-stack-b2-independent-review
  • 评审对象:Dev 仓 f23f5bb0...ea099a5f(本 checkout 无 origin remote,f23f5bb0 = R0/#682 合并点,即分支切出时 main 的 tip;git merge-base 确认为唯一基线)跨 pin 组件:
    • deps/agent-native 2fc66be..cff6935
    • deps/agent-squad c7121bd..9edb6ba
    • modules/backend f084d10..9b40595
    • Dev 级:.task/feature-agent-stack-audit-remediation-b2.md + 五个 gitlink
  • 方法:逐 diff 静态评审(spec / 回归 / 范围 / 测试四个透镜)+ 局部聚焦测试;产出 findings only,不改代码。
  • pin-restore 认定:646a2191 恢复批次一 tip(command fe7987d、intent 765a7e7、native ac91b84、squad 0f1daa8、backend b614180),批次二增量基于其上。native 范围内 afbb567/5c0d2d8、squad 范围内 aa15a5d..0f1daa8、backend 范围内 f084d10..b614180 均为已合入批次一(#685)的恢复,按 packet 预定视为 intended,不做重复评审。
  • 评审环境限制(诚实出口):本 worktree 仅材料化了 Dev 级组件;modules/backend/deps/*、deps/agent-native/deps/*、deps/agent-squad/deps/* 及 adapters/{sqlite,native} 子模块目录为空,且本单元禁止 submodule 操作,因此 backend 与上述包无法本地编译执行。相关验证以卡片记录的组合 make test(0b46569d 组合 PASS,exit=0)为执行证据,本地未跑部分标 NOT_RUN。

Findings

Blocker

无。

Major

无。

Minor

  1. modules/backend/internal/runtime/squad_events.go:80(onEvent 持锁发射):squadEventLog.onEvent 在 l.mu 临界区内调用 l.rc.Emit(...)(text_delta/tool start/end/usage 各分支)。锁内做 I/O 意味着慢 sink 会串行化所有后续帧并延迟收口 toolParts()。当前 sink 均为同步非阻塞,且「发射与部件同临界区」保证到达序一致(注释已说明),故不阻塞;建议后续把 Emit 摘要构造移出临界区或注明 sink 契约。属可维护性耦合,非正确性缺陷。
  2. modules/backend/internal/runtime/llm_direct.go:755-763(queued 在锁外读写,隐式不变量):tool_execution_start 分支 len(st.queued) 读取与 takeQueued 变更均未持 toolMu,仅 inflight 插入加锁(:764-766);takeInflight 注释要求「调用方须持有 toolMu」,takeQueued 无对应要求。安全性当前成立:native serializeHostCallbacks 串行化全部 OnEvent,且只有 onEvent 触碰 queued——但该不变量是隐式的,7b8a34d 的加锁改造未把它写进注释。此代码为批次一/既有(不在本 diff 修改行内),-race 全包绿;记一致性提示,不阻塞。
  3. deps/agent-squad/squad/engine.go:998-1013(gcFinish)/ squad/review.go:329-343(runReviewDriver):落盘失败后内存态与结果不一致:终态 save 失败时函数返回 RunFailed,但内存 st.Meta.Status 仍是刚写入的 "completed"。结果对象主导后续流程,且 save 失败通常伴随存储故障,实际影响小;建议失败分支回写内存状态或在注释中说明取舍。
  4. 任务卡收据 SHA 与最终 lineage 不一致(追溯性小疵):卡片实施收据表引用 aa81b50/7d0630a(u8)与 25397e3(u9),这三个提交不在 9edb6ba 的祖先链上。核对 git diff aa81b50 cf83d30 与 git diff 25397e3 12a53b0 均为空——即最终提交与收据候选逐字节相同,只是 transport 提交被重建。内容无偏差,建议收据表补注最终 SHA。
  5. deps/agent-squad/adapters/sqlite/queue.go:256(并发 claim 下的瞬时错误暴露):容量预读失败直接 return error;WAL + busy_timeout(5000)(adapters/sqlite/db.go:20)下高强度并发 claim 理论上可出现 SQLITE_BUSY 上抛为一次 Claim 失败。按 U9「读失败=拒绝、绝不静默放行」的语义这是正确取舍,调用方已有重试语义(runtime 轮询),仅记录为行为注记。

Notes(不构成缺陷)

  1. pin restore 核对无误:646a2191 恢复的五个 gitlink 与批次一 tip 一致;a0b8d12b/0b46569d 仅推进 native/squad/backend 至批次二 tip;Dev 级 diff = 任务卡 + 5 gitlink,无其他文件(git diff f23f5bb0...HEAD --stat 共 6 文件)。
  2. 收据候选与最终提交同树:见 Minor 4,无内容偏差。
  3. U8 的 migrate() 在每次 Save/Load 都执行(PRAGMA table_info + 版本读),频度低(每次运行态保存一次),开销可接受。
  4. U11c noteRecovered(squad_events.go:126-131)以「同名工具再次发起或成功」作为失败恢复启发式,注释已声明「失败部件仍留在过程部件里」;并行同名工具下极端顺序可能提前清除失败标记,仅影响收尾兜底文案,不影响部件与事件的真实状态。

分单元评审(spec 透镜:是否挡住 finding 的触发)

U8 — SQLite 持久化 Mailbox/Handoffs/Board + 身份安全解码 + 错误可见

  • 触发(F11):SQLite 运行重启丢失 mailbox/handoffs/board;相同 JSON 文本列互相覆盖;checkpoint/终态保存失败被吞、照发完成事件。
  • 挡住触发:
    • 持久化:adapters/sqlite/runstore.go:159-176(runStateColumnsOf 含 mailbox/handoffs/board)+ :178-198(SaveRun INSERT 三列,ON CONFLICT 同步更新);schema v2 增量迁移 runstore.go:28、:60-77、:104-124(ALTER TABLE 补列,旧行读 NULL→空默认,并发迁移复核)。
    • 身份安全解码:runstore.go:212-262 LoadRun 改为按列位置 Scan + 具名解码切片(dec.col 逐列报错),废除 map[string]any 文本键——同文本两列不再互相覆盖;runstore.go:252 NULL/空串跳过。
    • 错误可见:checkpoint flush 幂等化 squad/runstore.go:146-149、:198-202(flushOnce,出口与 defer 兜底只报一次);squad/workflow.go:58-83 出口 flush 失败经 persistenceFailure 折算可区分失败且幂等(:1148-1152 去重);workflow.go:996-1005(finalize 终态 save 失败→RunFailed,EvRunCompleted 不再发出)、:1006-1012(manifest 写失败同);边界快照失败带标记 workflow.go:185-188;engine 侧 engine.go:976-985(manualPass)、:1001-1016(cancelRun 可见但保持取消语义)、groupchat.go:342-348、review.go:329-335。失败经外层 finalizeEvent(engine.go:1479-1491)落到 EvRunFailed,完成事件严格晚于成功落盘。
    • FileStore 侧 steering 持久化对称:squad/runstate.go:100-103(persist 写 steering.json)+ loadRunState 的 st.loadSteering()。
  • 结论:覆盖全部三个触发面,无缺口。

U9 — Claim 原子执行 agent + runtime 双重在途上限,无静默计数错误放行

  • 触发(F12):仅按 agent 计数,同一 runtime 多 agent 可超 runtime 上限;计数错误被 _= 吞掉、静默放行。
  • 挡住触发:adapters/sqlite/queue.go:197-203 claimSQL 单条条件 UPDATE 同时内嵌两个守卫计数(agent 上限 + runtime 总量:t.runtime_id=? 或 agent 绑定该 runtime),SQLite 单语句原子性 + 写锁使并发 claim 无法越过任一上限;:248-258 预读失败显式 return error(拒绝而非放行);:229-234 旧的双 cap 取 min 预检保留为 only-skip 优化,UPDATE 内重数(RowsAffected==0 落空则尝试下一候选,:266-270);守卫计数读失败即语句失败(测试直接 DROP TABLE agent 验证)。计数口径无重复计数(OR 单行只计一次)、无遗漏(本 runtime 已租的 t.runtime_id 任务计入)。
  • 结论:触发被原子守卫挡住,无静默放行路径。

U11a — exec.ProcessEvent 携带 CallID/Status/ErrorReason(含 cancel/deny)

  • 触发(F14 源侧):过程事件无身份与真实结局,消费方按工具名配对、cancel/error 一律显示成功。
  • 挡住触发:
    • 字段落地:deps/agent-native/exec/backend.go:59-64(call_id/status/error_reason 全 omitempty);runtime/loop.go:46-51(runtime.Event 同名字段)。
    • 发射填充:runtime/loop.go:715-719 emitCancelledTool(跳过的调用也发 start+end,Status=canceled);:722-730 emitToolEnd 统一投影,error/denied/canceled 附 ErrorReason;:838 deny→denied;:844-847、:855-858 BeforeTool/commitToolIntent 失败→error;:884-893 执行后 ctx.Err() 优先于工具自报结局(取消不得报 ok)、res.IsError→error,否则 ok。
    • 版本兼容:新字段 omitempty,legacy 事件字节不变(exec/process_event_fidelity_test.go TestProcessEventLegacyOmitempty 断言)。
  • 结论:三类触发(配对、cancel 伪 ok、error 伪 ok)均在发射点挡住。

U11b — squad port/bridge/taps 透传字段 + 帧到达序保持

  • 触发(F14 squad 侧):事件端口丢 CallID/Status/ErrorReason;探针路径把并发工具帧按调用重排。
  • 挡住触发:
    • 端口:squad/port.go:78-92(ProcessEvent 三字段 + omitempty + ToolStatus* 常量域);squad/progress.go:145-160 processEventData 三键非空才输出(legacy 投影字节不变,测试钉住);调用点 squad/workflow.go:432、squad/leader.go:36。
    • 桥:adapters/native/register.go:188-206 一帧一回调同步转发;:269-306 反射按字段名探测(native 未跟进字段时零值透传、跟进后免改即生效,optionalStringField 带嵌套歧义防护 :299-311)。
    • 帧序(repair 9edb6ba):register.go:314-337 eventTap 以 mu 串行扇出——「一帧→全部下游」为整体,杜绝「组合根侧 sink 已收帧、请求侧被后帧抢先」造成的两侧顺序分叉;fanoutWindow 测试把该窗口钉成确定性失败(register_event_test.go:316-373)。
  • 结论:字段透传与到达序两个触发均被挡住,且有 legacy passthrough 用例。

U11c — backend squad_runner/hostbridge 按 CallID 配对、真实 Status/ErrorReason 映射、共享工件态上锁

  • 触发(F14 backend 侧):CallID 用工具名顶替、Status 硬编码 ok、并行回调共享工件表竞态。
  • 挡住触发:
    • 配对:modules/backend/internal/runtime/squad_events.go:16-22 toolEventID(CallID 优先,legacy 回退工具名保持展示不变);:96-105 start/end 以 pending[id] 配对。
    • 状态映射::31-53 toolEventOutcome——非空 Status 原样透传(ok/error/denied/canceled 不新增不改写);空 Status 且无 ErrorReason → legacy "ok"+Detail(字节一致);空 Status 但有 ErrorReason → error(有失败信号不报 ok);非 ok 摘要取 ErrorReason 原文。
    • 收尾一致性::133-137 未恢复失败记录、:163-176 finalReply 只在宿主兜底「已完成。」时替换为如实失败行,模型文本不代写。
    • 工件表:internal/runtime/hostbridge.go:29-48 artifactList 加 mu + snapshot() 拷贝,squad_runner.go 终局改用 produced.snapshot()。
    • -race 修复(7b8a34d):llm_direct.go:603-610 writeSlot 把工具 goroutine 对槽位的回写全部收进 toolMu;:636-655 slotFor 空 ID 改为认领未认领槽位(claimed 标志,防并行直调撞槽);:786-799 onEvent 在锁内快照槽位字段后再组装部件/发事件(解锁后读即竞态——原失败模式已消除)。
  • 结论:配对、状态映射、并发三面均在触发点修复;legacy 空字段行为逐字节保留(squad_events.go:36-39 + TestSquadEventsLegacyFramesUnchanged)。

回归透镜

  • legacy 空字段行为:native(omitempty,exec/backend.go:59-64)、squad port(port.go:78-92 + progress.go:150-154 三键全空输出与旧投影逐字节一致,测试 squad/process_event_test.go:129-152)、backend(toolEventOutcome 空 Status+空 ErrorReason → 旧 "ok"+Detail(160),toolEventID 空回退工具名)——三层 legacy 路径均有字节一致断言。
  • 旧 schema 升级:v1 库(无新列、无版本表)→ 建表/补列/记版本,行数据不丢;runstore_test.go:425-513 用真实 legacy DDL+行数据验证(含升级后写入协作态、旧列不被 clobber)。
  • 既有语义保留:U9 租约 5 分钟、reclaim、RowsAffected==0 换下一候选;U8 async/on_exit 出口语义保留(对照测试 checkpoint_test.go:318-338);U11c 的 usage 转发、text/reasoning delta、textIdx 行为不变;eventTap 仍为同步回调(不排队不缓存)。
  • 取消语义:cancelRun(engine.go:1001-1016)保存失败仅补可见错误与 out.Error,仍返回 RunCancelled——不改变取消本身的结局。

范围透镜

  • Dev 级:仅任务卡 + 5 gitlink,与声明一致。
  • native cff6935:7 文件全部落在 U11a 声明面(exec/、runtime/loop.go、adapters/native 转换 + 测试);其余范围 diff 均属批次一恢复提交。
  • squad cf83d30/12a53b0/676a1ed/9edb6ba:文件均在 U8/U9/U11b 声明面;deps/agent-intent/deps/agent-native gitlink 指向批次一恢复 tip(cdd51c8),无越界。
  • backend f8e9e9f..9b40595:4 提交均为 U11c + repair + pin;大 diff 中的 intentgate/route/server/cmdbus 等改动全部来自 f084d10..b614180 的批次一恢复,与卡片「pin-restore 预期」一致。
  • 未发现超出声明单元的代码变更。

测试透镜(是否断言触发/并发/故障注入而非 happy path)

  • U9:queue_test.go 覆盖双上限交叉(runtime 跨 agent :72、agent 跨 runtime :119)、12 worker 真并发不越界 :144、释放后重取 :207、计数失败三态(库不可读/agent 表缺失/守卫计数直击 :248-329,后者直接执行 claimSQL 验证语句级失败且任务保持 queued)。
  • U8:runstore_test.go 往返 :64、同文条目不坍缩 :385、legacy 升级 :425、跨重启协作态 :514、save 失败 3 mode × 2 fault 矩阵 :686(断言无 run_completed + "persistence failure" 可见);checkpoint_test.go:248/279 三档 flush/终态失败注入(faultStore),并断言事件序最后一枚终态事件必为 run_failed、交付物文件仍在(业务动作不回滚);:318 对照组防「压根没写」空过。
  • U11a:loop_event_fidelity_test.go 同名工具异 CallID :27、error/denied/canceled 状态 :95/:122/:164、批量取消跳过帧必为 canceled 非 ok :243。
  • U11b:register_event_test.go legacy 透传 :27、反射探测 :55、并发身份保持 :236、到达序(桥与 tap 双 sink 单序,fanoutWindow 确定性复现竞态窗口 :306/:373)。
  • U11c:squad_events_test.go CallID 配对 :74、真实状态 :108、legacy 不变 :167、64 路并发回调零丢失/零重复 :206、并发共享工件表 :267、收尾文本与工具结局一致 :300。
  • 测试质量总体:故障注入为真实注入(faultStore/faultingStore/DROP TABLE/关库),并发用例为真 goroutine,且普遍带 legacy 对照组——满足「断言触发而非 happy path」。

本地聚焦测试(GOWORK=off,逐条一行)

  • deps/agent-native:go test ./exec/ ./runtime/session/ ./runtime/tools/ → ok(3 包全绿,tools 11.4s)。
  • deps/agent-native:go test ./runtime/ 与 ./adapters/native/ → NOT_RUN(缺嵌套 deps/error-code-system 材料,本单元禁止 submodule 操作;见卡片组合验证)。
  • deps/agent-squad:go test ./squad/ ./adapters/sqlite/ ./adapters/native/ → NOT_RUN(同上,嵌套 deps 未材料化;见卡片组合验证)。
  • modules/backend:go build ./internal/runtime/ → NOT_RUN(backend 自身嵌套 deps 为空目录,无法解析 replace;执行证据取卡片记录的组合 make test @ 组合验证 PASS(exit=0))。
  • 说明:组合验证收据(卡片:全量 make test @ 0b46569d)覆盖上述 NOT_RUN 包在精确 pin 上的执行;本评审的独立复核以静态逐行为主。

结论

五个声明单元(U8、U9、U11a、U11b、U11c)及两处修复(帧序、-race)均在触发点完成修复,legacy/升级兼容有测试钉住,范围与声明一致,无 blocker/major;5 条 minor 均为非阻塞的一致性/追溯性提示,可作为后续独立增量。

REVIEW VERDICT: APPROVE