From bcad76ebf5d9f3fed263abc14e4b80990d269d9e Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 13:31:37 +0800 Subject: [PATCH 01/13] docs(audit): add three-module functional audit evidence (DAG/MEMORY/GOAL) --- docs/audit-dag-memory-goal-2026-08-18.md | 537 +++++++++++++++++++++++ 1 file changed, 537 insertions(+) create mode 100644 docs/audit-dag-memory-goal-2026-08-18.md diff --git a/docs/audit-dag-memory-goal-2026-08-18.md b/docs/audit-dag-memory-goal-2026-08-18.md new file mode 100644 index 000000000..2762d3a45 --- /dev/null +++ b/docs/audit-dag-memory-goal-2026-08-18.md @@ -0,0 +1,537 @@ +# 功能审计：DAG / MEMORY / GOAL 三模块缺陷与证据 + +审计日期：2026-08-18 +审计对象：`origin/dev` = `f1c2c8c33`（内容等同 `origin/main` = `25a711b40`，即 PR #332 发布批次之后的当前状态） +审计范围：**功能性运行时缺陷**。配置类问题（YAML 模板内容、config knob 命名/默认值、prompt 文案、文档措辞、`LeXwDeX/opencode-dag-config` 仓库内容）不在本次范围内。 + +## 方法与证据纪律 + +1. 本地 `dev` 落后 `origin/dev` 25 个提交（缺 PR #313–#332）。审计在 `origin/dev` 的 detached worktree 上进行，避免对着过期代码下结论。 +2. 该 worktree 以 `mode=fast` 重新索引为 codebase-memory 项目 `audit-dmg-20260818`（29826 nodes / 132182 edges，0 skipped）。 +3. 三个模块由三个独立 auditor 子代理并行做首轮结构化排查（图工具 + coverage 校验）。 +4. **本文档中每一条 `file:line` 引用与代码引文，均由主会话在上述 worktree 中直接读取源码复核过。** 子代理提出但复核不成立、或严重性被证据推翻的候选项已剔除或降级（见「复核中被推翻/降级的候选项」）。 +5. 测试覆盖结论来自直接读取 `packages/opencode/test/**`（`fast` 索引不含 `*.test.ts`，因此这部分不依赖图索引）。 + +## 缺陷汇总 + +| ID | 严重性 | 置信度 | 模块 | 一句话描述 | 与 tracker 关系 | +|---|---|---|---|---|---| +| DAG-01 | High | Confirmed | DAG | 无 `output_schema` 的 reporting checkpoint 上的等值门恒为 false，整棵下游子树被静默跳过且工作流报 COMPLETED | PR #331 的不完整修复 | +| DAG-02 | High | Confirmed | DAG | `replan` / `extend` 完全不跑 `checkpointGateDiagnostics`，checkpoint 门禁在每次图变更路径上失效 | #325 的不完整修复 | +| DAG-03 | Medium | Confirmed | DAG | replan 裁决门在持久化 pause 终态失败时 **fail-open**，显式把内存调度器置为未暂停 | PR #331/#327 的不完整修复 | +| DAG-04 | Medium | Confirmed（机制）| DAG | summary publisher 把 interrupt 当成功日志吞掉；生产关停路径 uninterruptible 且无超时 | Known-#316（机制补齐，触发源仍未钉死）| +| MEM-01 | High | Confirmed | MEMORY | 周期 `prepare` 在 fence+lock 下内联跑 **3 次**模型调用（比 #324 描述的更广，含首轮 match） | Known-#324 debt 2，未偿付 | +| MEM-02 | Medium | Confirmed | MEMORY | `search` 跨 matcher 模型调用持有跨进程 identity flock | Known-#324 debt 2 后半 | +| MEM-03 | Low | Confirmed | MEMORY | 周期维护失败后用**维护前**快照渲染注入，仅 logWarning | New | +| GOAL-01 | High | Confirmed | GOAL | 崩溃丢失的 continuation 使目标被持久边界门永久搁死；**测试把错误行为钉住了** | PR #289 的过度修正 | +| GOAL-02 | Medium | Confirmed | GOAL | ESC pause 重试耗尽后仍保留 lease 注册与 active 行，却无条件清掉 `turnDriven` | PR #284 的不完整修复 | +| GOAL-03 | Low | Confirmed | GOAL | judge 传输/解析失败仍消耗 `turns_used` 并盖上 `last_judged_msg` | New | +| GOAL-04 | Low | Confirmed | GOAL | 启动扫描对非 idle 会话静默跳过，无日志、无重新武装 | New | + +--- + +## DAG + +### DAG-01（High）等值条件门在字符串输出上恒为 false，静默跳过整棵子树并把工作流标为 COMPLETED + +**位置**：`packages/opencode/src/dag/runtime/eval.ts:133-147`、`packages/opencode/src/dag/runtime/loop.ts:141-156`、对照点 `packages/opencode/src/dag/runtime/loop.ts:664-672` + +**证据 1 — 路径解析在字符串上返回 `undefined`，不报错**（`eval.ts:133-147`）： + +```ts +function resolvePath(path: string, source: Record): unknown { + const parts = path.split(".") + let current: unknown = source + if (parts[0] && parts[0] in source) { + current = source[parts[0]] + parts.shift() + } + for (const part of parts) { + if (current == null) return undefined + current = (current as Record)[part] + } + return current +} +``` + +**证据 2 — 数值比较会 loudly fail，等值比较不会**（`eval.ts:56-70`）： + +```ts + if (op === ">" || op === "<" || op === ">=" || op === "<=") { + if (typeof lhs !== "number" || !Number.isFinite(lhs)) + return { ok: false, error: `condition "${condition}": left operand resolved to ${describeOperand(lhs)}, expected a finite number` } + ... + } + if (op === "==") return { ok: true, value: lhs === rhs } +``` + +`undefined === "ACCEPT"` → `false`，`{ ok: true, value: false }`，调度层走 skip 分支（`loop.ts:152-155`）： + +```ts + if (!condResult.value) { + yield* dag.nodeSkipped(dagID, nodeID, "condition_false").pipe(Effect.ignore) + continue + } +``` + +**证据 3 — 同一文件 500 行后的姊妹门做了字符串解析，本处没有**（`loop.ts:664-672`，PR #331 只补了这一处）： + +```ts + // A checkpoint output can arrive as a raw string (no + // output_schema, or a string-typed child reply); parse it + // before matching the verdict so a string-typed + // {"verdict":"replan"} cannot bypass the gate (the spin + // behind issue #322). + const gateOutput = typeof node?.output === "string" + ? Option.getOrUndefined(parseJsonOption(node.output)) + : node?.output +``` + +**证据 4 — 无 `output_schema` 的节点确实以裸字符串完成**（`spawn.ts:482-516`）：`if (input.outputSchema)` 分支走 `settleCapturedOutput`；`else` 分支 `const rawText = result.parts.findLast(...)`，最终 `dag.nodeCompleted(input.dagID, input.nodeID, rawText)`。 + +**证据 5 — authoring 主动把作者引导到这个形状**（`validation.ts:600-604`）： + +```ts + hint: + `Gate "${dependent.id}" with condition: "${checkpoint.id}.output. == ..." (e.g. on its verdict),` +``` + +`checkpointGateDiagnostics`（`validation.ts:584-609`）只检查 `conditionReference(dependent.condition) === checkpoint.id`，**从不要求该 checkpoint 声明 `output_schema`**；`conditionReferenceErrors`（`validation.ts:459-467`）同样只检查引用 id 在 `depends_on` 里。 + +**可达性**：Block 编译路径上 `verify` → `VERIFICATION_SCHEMA`、`review` 决策节点 → `GENERAL_VERDICT_SCHEMA`/`DIFF_REVIEW_SCHEMA`、`coding`/`prototype` → `IMPLEMENTATION_SCHEMA`（`blocks.ts:251,271,305-309`），**这些默认路径是安全的**。暴露面是： +- `synthesize` block：`reportToParent: block.report_to_parent ?? block.kind === "synthesize"`（默认 **true**）而 `outputSchema` 落到 `undefined`（`blocks.ts:300-309`）——一旦它有 dependents，就同时是「reporting checkpoint」且「无 schema」； +- 任何被作者显式设成 `report_to_parent: true` 的 `explore`/`plan`/`debug`/`synthesize` block（ultra-flow 的 gate checkpoint 正是这种形状，见 #323 里的 `cp-after-exploration`）； +- 全部 low-level `nodes:` 手写 checkpoint。 + +**为何是缺陷**：违反 `dag/CONTEXT.md` 不变量「Dependents of a reporting checkpoint must be gated on its output」。门存在但结构上惰性——它不是「按裁决放行」，而是**无条件否决**。与 PR #331 建立的一致性也自相矛盾：字符串归一化只补在裁决匹配上，没补在门禁真正依赖的 `evaluateCondition` 上。 + +**运行时影响**：checkpoint 通过 → 所有被门控的 dependent 以 `condition_false` 被跳过 → `spawnReady` 的 cascade 定点循环逐波发布 `NodeSkipped(orphan_cascade)`（`loop.ts:119-126`）→ `checkCompletion` 认为 `isComplete()` → `dag.complete(dagID, { skipReviewGate: true })`（`loop.ts:338`，**显式绕过 review gate**）。操作者看到的是一个状态为 **COMPLETED** 的工作流，而 checkpoint 之后的整个半图从未运行。无错误、无失败、无告警。 + +**测试覆盖**：未覆盖。`test/dag/dag-checkpoint-gate.test.ts` 全部是 authoring 层断言（`action: "start"`），没有任何用例在运行时把一个无 schema 的 checkpoint 输出喂给 `evaluateCondition`。 + +**建议修法**：`loop.ts:141-152` 在构造 `outputs` 时对字符串输出做与 `loop.ts:667` 相同的 `parseJsonOption` 归一化；并在 `checkpointGateDiagnostics` 中要求被门控引用的 checkpoint 声明 `output_schema`（否则该门在运行时不可满足），把它变成 authoring 期错误。 + +--- + +### DAG-02（High）`replan` / `extend` 跳过 `checkpointGateDiagnostics`，门禁在每次运行时图变更路径上失效 + +**位置**：`packages/opencode/src/dag/authoring.ts:136`、`packages/opencode/src/dag/validation.ts:974-984` + +**证据 1 — 非 `start` 动作整体关闭结构检查**（`authoring.ts:136`）： + +```ts + structural: input.action === "start", +``` + +动作集合恰为 `start | extend | replan`（`authoring.ts:197-215` `decodeAction`）。 + +**证据 2 — `structural === false` 把 checkpoint 门与其余结构检查一起跳过**（`validation.ts:974-984`）： + +```ts + const diagnostics = + input.structural === false + ? [] + : [ + ...structuralDiagnostics({ ... }), + ...checkpointGateDiagnostics(input.nodes, input.config.node_defaults), + ] +``` + +**证据 3 — 全仓唯一调用点**： + +``` +packages/opencode/src/dag/validation.ts:584:export function checkpointGateDiagnostics( +packages/opencode/src/dag/validation.ts:983: ...checkpointGateDiagnostics(input.nodes, input.config.node_defaults), +``` + +（其余命中只有 ADR 文档 `docs/adr/0003-reporting-checkpoint-gating.md:30`，其自述「Enforcement lives in `checkpointGateDiagnostics`, wired only into …」。） + +**为何是缺陷**：`dag.ts:570-576` 的注释声称 replan 走的是「the create/replan parity the spec requires: one authority, two entry points」，但这份 parity 恰好在 checkpoint 门上不成立。ADR-0003 把 enforcement point 限定在 authoring 边界，而 authoring 边界又对 replan/extend 自我关闭——两者叠加后，**没有任何权威**在图变更路径上施加这条不变量。而 replan 正是编排器在每个纠偏周期都要走的路径，包括 replan 裁决门自己指示 parent 去做的那次。 + +**运行时影响**：一次 replan 可以把 dependent 直接挂到 reporting checkpoint 上且不带 `condition`。引擎会在 checkpoint 完成的瞬间 spawn 该 dependent——早于 parent 读到裁决。运行时兜底网（`loop.ts:670-703`）只认字面 `verdict: "replan"`；返回 `reject` / `fail` / `needs_changes` 的 checkpoint 会让未门控的 dependent 在已被否决的方向上继续跑，无门、无暂停、无诊断。 + +**测试覆盖**：未覆盖。`dag-checkpoint-gate.test.ts` 的 7 个用例全部使用 `action: "start"`。 + +--- + +### DAG-03（Medium）replan 裁决门在 pause 终态失败时 fail-open + +**位置**：`packages/opencode/src/dag/runtime/loop.ts:681-703` + +**证据**： + +```ts + const paused = yield* Effect.gen(function* () { + const attemptPause = dag.pause(dagID).pipe( + Effect.map(() => true), + Effect.catch(() => Effect.succeed(false)), + ) + if (yield* attemptPause) return true + if (yield* attemptPause) return true + const wf = yield* store.getWorkflow(dagID).pipe(Effect.orDie) + if (wf?.status !== "paused") + yield* Effect.logWarning("DagLoop pause on replan verdict failed", { dagID, nodeID }) + return wf?.status === "paused" + }) + entry.runtime.setPaused(paused) +``` + +两次尝试都失败且持久行不是 `paused` 时，`paused === false`，第 697 行**显式把内存 runtime 置为未暂停**，唯一后果是一条 WARN。调度抑制只作用于本次事件（`loop.ts:703`）： + +```ts + if (!gateReplan && !entry.runtime.isStepMode()) yield* spawnReady(dagID) +``` + +**可达性**：`spawnReady` 会被后续任意刺激再次触发——`NodeCancelled`（`loop.ts:739` 附近）、`WorkflowStepped`（`loop.ts:787` 附近）、`WorkflowResumed`、`WorkflowReplanned`（`loop.ts:882` 附近）、`recoverWorkflow`（`loop.ts:465` 附近）；`getReadyNodes()` 只在 `this.paused` 时返回空，而该标志刚被置 false。 + +**为何是缺陷**：裁决门必须 fail-**closed**。PR #331 加固了瞬态情形（重试两次后查持久状态），但终态情形反向失败：正确动作是无论持久 pause 是否被拒都 `setPaused(true)`，代码做的恰好相反。另注：`Effect.catch` 只处理 error channel——`dag.pause` 抛出的 **defect** 会逃到 `guarded("NodeCompleted")`（`loop.ts:356-357`），整个 handler 被丢弃，pause 从未发生且连门专属的 WARN 都不会打。 + +**运行时影响**：checkpoint 返回 `verdict: "replan"`（显式否决）、持久 pause 被拒（例如工作流处于 `stepping`，或与并发控制操作竞争），工作流继续在被自己 checkpoint 否决的方向上调度。 + +**测试覆盖**：未覆盖（无用例注入持久性 pause 失败）。 + +--- + +### DAG-04（Medium，Known-#316）summary publisher 把 interrupt 当成功吞掉；生产关停 uninterruptible 且无超时 + +**位置**：`packages/opencode/src/dag/runtime/summary-publisher.ts:151-170`、`packages/opencode/src/server/global-lifecycle.ts:16-25` + +**证据 1 — listener 边界把 interrupt cause 转成成功的日志行**（`summary-publisher.ts:163-170`）： + +```ts + return schedulePublishByDag(dagID, evt.location.workspaceID).pipe( + Effect.catchCause((cause) => + Effect.logWarning("DagSummaryPublisher: failed to publish summaries", { dagID, cause }), + ), + Effect.forkIn(scope), + Effect.asVoid, + ) + }) + yield* Effect.addFinalizer(() => unsubscribe) +``` + +`coalesceLatest` 内层刻意**重新抛出** interrupt（`summary-publisher.ts:111-113`）： + +```ts + if (Exit.isFailure(outcome) && Cause.hasInterrupts(outcome.cause)) { + return yield* Effect.failCause(outcome.cause) + } +``` + +——但外层这个 `catchCause` 没有 `Cause.hasInterrupts` 再抛，作者在内层建立的取消语义在外层被抹掉。仓库内正确写法出现过三次（`spawn.ts:255-257`、`spawn.ts:545`、`loop.ts:1409-1411` 附近），此处是唯一例外。 + +**证据 2 — 生产关停路径无超时且不可中断**（`global-lifecycle.ts:17-25`）： + +```ts + yield* Effect.gen(function* () { + yield* options?.swallowErrors + ? store.disposeAll().pipe(Effect.catchCause((cause) => Effect.logWarning("global disposal failed", { cause }))) + : store.disposeAll() + yield* emitGlobalDisposed + }).pipe(Effect.uninterruptible) +``` + +exerciser 用 `bounded("disposeApps", ...)` 兜住，生产路径没有等价保护。这直接回答 #316 的验收项 3：**真实 server 关停走的是同一 dispose，且比测试路径更脆弱**。 + +**未钉死的部分（对 #316 的诚实缺口）**：本次没有定位 dispose 期间持续发 `dag.*` 事件的组件。已排除的候选：`spawnNode` teardown 在 interrupt 时不发节点事件（`spawn.ts:545` 提前返回）；publisher 自身发出的 `dag.workflow.summary.updated` 不在 `SUMMARY_TRIGGER_EVENTS` 里，无法自触发。放大机制已证明，触发源未证明。 + +**测试覆盖**：`dag-summary-publisher.test.ts` / `dag-summary-publisher-behavior.test.ts` 存在，但均未覆盖 dispose 期间的 interrupt 语义。 + +--- + +## MEMORY + +### MEM-01（High，Known-#324 debt 2）周期 `prepare` 在 fence+lock 下内联跑 3 次模型调用 + +**位置**：`packages/opencode/src/memory/memory.ts:474-505`；对照的成文规则在 `packages/opencode/src/memory/memory.ts:283-285` + +**证据 1 — 模块自己写下的锁纪律**（`memory.ts:283-285`）： + +```ts + // Serialize the identity-liveness recheck and the per-project lock around + // the store write only; the model calls that produce the update run + // outside the fence/lock so a long reasoning call cannot wedge or leak it. +``` + +**证据 2 — `prepareUnsafe` 违反它**（`memory.ts:474-505`）： + +```ts + const live = yield* fence.withLiveIdentity( + current.project.id, + Effect.gen(function* () { + yield* lock.withProject(current.project.id)( + Effect.gen(function* () { + const topics = yield* store.readTopics(current.project.id) + const maintained = due + ? yield* maintain({ ... }) + : topics + const rendered = shouldMatch + ? (yield* select({ ... })).rendered + : (data.sessions.get(input.sessionID)?.turn.rendered ?? []) +``` + +`maintain`（`memory.ts:376-397` 的模型半部 `proposeMaintenance`）发起 **2 次** `modelCalls.generate`；`select`（`memory.ts:408` 起）再发 **1 次** matcher 调用。三次模型往返全部在 `memory-identity:` 跨进程 flock + 项目内存互斥锁之内。 + +**比 #324 描述的更广**：issue 只指出 `prepareUnsafe` 的 due 分支跑 maintain。实际上 `shouldMatch` 分支的 `select` 也在锁内——即**每个会话首个真实用户轮**都会跨一次模型调用持有跨进程 identity flock，与 `turn_interval` 无关。 + +**可达性**：`SystemPrompt.memory` → `memory.prepare(...)`（`session/system.ts`）→ `prepare`（`memory.ts:519`）→ `prepareUnsafe`（`memory.ts:442`）。`Memory.node` 已在交付的 httpapi app 图中（PR #313），为生产活代码。 + +**运行时影响**：`model.ts` 已退役墙钟（见「验证为正确」），`CONNECT_TIMEOUT`/`IDLE_TIMEOUT` 各 60s 且每个 chunk 重置——这意味着一条持续流式的慢推理调用可以**任意长时间**持有该锁。等待者在 `EffectFlock` 的 5 分钟后拿到 `LockTimeoutError`：并发的 `/compact` checkpoint、`memory_search`、`/memory on|off`、worktree `remove`/`reset` 的 admission，以及 **identity upgrade**（`ProjectIdentityMigration.migrate` 用同一把 key）都会在 5 分钟僵持后失败。同时，落在 `turn_interval` 边界上的每个 prompt 都要串行等两次模型调用才能组装系统提示。 + +**修复要点**：把 `prepareUnsafe` 的 due 分支改为与 checkpoint 路径同构——复用 `kickMaintenance`/`backgroundMaintain` + `applyUpdate`（只有 commit 拿锁）；`select` 同理，只在写 `markMatched` 时拿锁。注意这会改变「周期维护同步」测试的语义（#324 已预告）。 + +--- + +### MEM-02（Medium，Known-#324 debt 2 后半）`search` 跨 matcher 模型调用持有 identity flock + +**位置**：`packages/opencode/src/memory/memory.ts:577-600` + +**证据**： + +```ts + // Cross-process identity guard (see checkpointUnsafe): MemoryIdentityFence + // re-checks identity liveness under the identity lock before matching/writing. + const live = yield* fence.withLiveIdentity( + current.project.id, + Effect.gen(function* () { + return yield* lock.withProject(current.project.id)( + Effect.gen(function* () { + ... + const topics = yield* store.readTopics(current.project.id) + const selected = yield* select({ ... }) +``` + +**为何是缺陷**：与 MEM-01 同类。即使接受「同查询合并」的刻意取舍，**跨进程 identity fence** 也不需要覆盖 matcher 调用，只需覆盖 `markMatched` 写入。代码注释只解释了 liveness recheck 的理由，**没有**声明「刻意跨模型调用持锁」——而 #324 的验收要求正是把这个取舍显式写进规格。 + +**运行时影响**：一次 `memory_search` 会在 matcher 模型调用期间阻塞 `/compact` checkpoint、`/memory` 开关、worktree `remove`/`reset` 的 admission 以及 identity upgrade，上限到 `EffectFlock` 的 5 分钟等待超时。 + +--- + +### MEM-03（Low，New）周期维护失败后用维护前快照渲染注入 + +**位置**：`packages/opencode/src/memory/memory.ts:482-495` + +**证据**： + +```ts + ? yield* maintain({ ... }).pipe( + Effect.catchCause((cause) => + Effect.gen(function* () { + yield* Effect.logWarning("periodic MEMORY maintenance failed", { cause }) + return topics + }), + ), + ) + : topics +``` + +**为何是缺陷**：`maintain` 内部已经执行过 `store.updateTopics` 提交（`memory.ts:386-395`）。若失败发生在提交之后，恢复值 `topics` 是**维护前**快照，随后 `select`/渲染（`memory.ts:496-505`）基于它工作——本轮注入的 Memory 上下文与已落盘的持久修订不一致，且只有一条 `logWarning`，不向用户暴露冲突。 + +**运行时影响**：瞬态不一致（下一次 `prepare` 自愈），不是数据丢失。严重性 Low。 + +--- + +## GOAL + +### GOAL-01（High）崩溃丢失的 continuation 使目标被持久边界门永久搁死；测试把错误行为钉住了 + +**位置**：`packages/opencode/src/goal/loop.ts:245-264`（门）、`packages/opencode/src/goal/goal.ts:746-749`（写入点）、`packages/opencode/test/goal/e2e-loop.test.ts:1836-1906`（钉错的测试） + +**证据 1 — 门的实现与自述理由**（`loop.ts:245-264`）： + +```ts + // issue #285 — durable boundary gate (scan path only). ... + // While the session window still ends on that same message, no new progress has landed — + // re-judging would inflate turns_used and dispatch a duplicate continuation. ... + if (scanResume && goalState.last_judged_msg) { + const win = yield* sessions.messages({ sessionID, limit: 20 }).pipe(...) + const lastSeen = [...win].reverse().find((m) => m.info.role === "assistant") + if (lastSeen && lastSeen.info.id === goalState.last_judged_msg) return + } +``` + +**证据 2 — 只有 continue 提交会写 `last_judged_msg`**（`goal.ts:746-749`）： + +```ts + // issue #285: record the judged boundary for the durable scan gate. + ...(judged !== undefined ? { last_judged_msg: judged } : {}), +``` + +`blocked` 分支（`goal.ts:713-721`）与 `resume`（`goal.ts:561-575`）都不写不清；`GoalState.advance`（`state.ts:62`）原样带下去。 + +**证据 3 — 测试明确把这个场景当成「应跳过」并断言不派发 continuation**（`e2e-loop.test.ts:1871-1906`）： + +```ts + // Commits one continue evaluation ahead of the (re)boot — models a process + // that crashed right after the commit, before the continuation produced an + // assistant message. + const commitPriorBoundary = (sid: SessionID) => ... + + it.instance("scan with an unchanged boundary skips re-evaluation (no inflation)", () => + ... + expect(judgeCalls).toBe(0) + expect(continuationCalls).toBe(0) + const g = yield* goal.load(sid) + expect(g?.turns_used).toBe(1) +``` + +**为何是缺陷**：门把两种状态混为一谈—— +- 「边界已判定，continuation 已完成」→ 跳过是正确的（避免 turn 膨胀 + 重复派发）； +- 「边界已判定，continuation 随进程崩溃丢失」→ 跳过是**错误的**，因为重启后不存在任何在飞的 continuation，跳过意味着没有任何东西会驱动这个目标。 + +测试同时断言了 `judgeCalls === 0`（正确：不该重判，否则 turns 膨胀）和 `continuationCalls === 0`（错误：目标被留在 `active` 且无驱动者）。正确行为应是 **跳过 judge、但仍派发 continuation**。 + +**可达性与永久性**：窗口是「continue 提交 / `resume` kick 之后、下一条 assistant 消息落库之前」的崩溃。`/goal resume` 返回 `type: "kick"`（`goal.ts:831-835`），由 prompt.ts 派发，同样落在这个窗口内。搁死是**跨重启永久的**：每次启动扫描都命中同一个门而 `return`，`last_judged_msg` 因为不再判定而永不推进。D6 zombie 守卫也救不了它——`isStaleZombie` 要求 `turns_used === 0`（`loop.ts:90-101`），而此时 `turns_used >= 1`。唯一出路是用户主动向该会话发消息（走 `scanResume=false` 的活 idle 路径）。 + +**运行时影响**：这正是 #283 / #289 想消灭的 silent-stall 类问题——目标持久停在 `active`，无驱动、无日志、无暂停原因，直到用户偶然与该会话交互。 + +**测试覆盖**：**测试钉住了错误行为**（`e2e-loop.test.ts:1889-1906`）。修复必然要改这条断言：把 `expect(continuationCalls).toBe(0)` 改为 `toBe(1)`，同时保留 `judgeCalls === 0` 与 `turns_used === 1`。 + +--- + +### GOAL-02（Medium）ESC pause 重试耗尽后仍保留 lease 注册与 active 行，却无条件清掉 `turnDriven` + +**位置**：`packages/opencode/src/goal/goal.ts:246-270` + +**证据**： + +```ts + if (paused) { + yield* automation.unregister(sessionID, { kind: "goal", id: paused.goal_id ?? "legacy" }).pipe( + Effect.ignore, + ) + } else { + yield* Effect.logError( + "goal pause on cancel failed after retries — goal may resurrect on next idle", + { sessionID, cause: lastCause ? Cause.pretty(lastCause) : "unknown" }, + ) + } + turnDriven.delete(sessionID) + return paused +``` + +**为何是缺陷**：PR #284 加的重试循环 + 大声日志是修复的正确一半。失败分支与模块内其他所有 pause 点不对称——`pauseGoal`（`loop.ts:114-119`）、`pause`（`goal.ts:534` 附近）、派发失败处理（`loop.ts:564` 附近）都把 pause 与 `automation.unregister` 成对处理。这里在耗尽后：持久行仍 `active`、lease 注册仍在，**而 `turnDriven.delete(sessionID)` 无条件执行**——进程内的 ESC 来源信息被丢掉，持久态与 lease 却仍宣称「goal 拥有该会话且处于活跃」。 + +**运行时影响**：ESC + 三次 pause 写入失败后，下一个 idle 事件重入 `afterIdle`，`status === "active"` 通过、claim 成功（注册完好）、`shouldPreempt` 返回 false（ESC 不产生用户消息，`goal.ts:240-245` 的注释已承认这点），目标复活并派发用户已显式中止的 continuation。日志让它可见，但没让它自洽；丢掉 `turnDriven` 还意味着**复活轮上的第二次 ESC 不再走 goal pause 快路径**。 + +**测试覆盖**：只钉了成功路径。`test/goal/turn-scope.test.ts:76-110` 在健康 DB 上验证 pause 与无活跃目标时的 no-op，没有用例注入持续性 DB 失败。 + +--- + +### GOAL-03（Low）judge 传输/解析失败仍消耗 turn 预算并盖上 `last_judged_msg` + +**位置**：`packages/opencode/src/goal/judge.ts:84-89`（fallback）、`packages/opencode/src/goal/goal.ts:733-749`（应用点） + +**证据**： + +```ts + Effect.catchCause(() => + Effect.succeed({ + verdict: "continue", + reason: "judge transport error (timeout or network) — counting toward pause budget", + parseFailed: true, + } satisfies JudgeResult), + ), +``` + +continue 分支随后无条件自增并记录边界： + +```ts + const turnsUsed = GoalState.nni(state.turns_used + 1) + ... + ...(judged !== undefined ? { last_judged_msg: judged } : {}), +``` + +**为何是缺陷**：fail-open 本身是成文的刻意设计（一次抖动不应停摆，由 `MAX_CONSECUTIVE_PARSE_FAILURES` 兜底），`judge.ts:70-84` 的注释解释得很清楚。真正不一致的是**预算记账**：一次 judge 从未返回裁决的轮次，仍然消耗用户 `max_turns` 的一格，并且仍然像真判过边界一样盖上 `last_judged_msg`（后者与 GOAL-01 的搁死风险叠加）。计数器在任一成功时重置（`goal.ts:693` 附近），因此在间歇性成功的不稳定 provider 下可以无限烧预算而永不触发自动暂停。 + +**运行时影响**：不可靠 judge 模型下目标预算被未评估的轮次吃掉，导致提前「预算耗尽」暂停。可通过 `/goal resume` 恢复，严重性 Low。 + +**测试覆盖**：测试把当前行为当作预期钉住（`test/goal/judge.test.ts:99-145` 断言 `parseFailed: true` + `verdict: "continue"`；`test/goal/goal.test.ts:641-710` 断言计数器爬到自动暂停）。「失败 judge 应对预算中性」这一点没有任何断言。 + +--- + +### GOAL-04（Low）启动扫描对非 idle 会话静默跳过，无日志、无重新武装 + +**位置**：`packages/opencode/src/goal/loop.ts:666-679` + +**证据**： + +```ts + const scanForActiveGoals = Effect.fnUntraced(function* (snapshot: ReadonlyArray) { + for (const sessionID of snapshot) { + const current = yield* status.get(sessionID) + if (current.type !== "idle") continue + yield* triggerEvaluation(sessionID, true).pipe( + Effect.catchCause((cause) => + Effect.logWarning("goal startup scan failed for session", { sessionID, cause: Cause.pretty(cause) }), + ), + ) + } + }) +``` + +**为何是缺陷**：注释（`loop.ts:656-659`）以「a session mid-turn is skipped and will be driven by its own turn-end idle event」为理由。这在本进程启动的轮次上成立，但扫描发生在 boot、本进程尚未启动任何轮次之时；注释自己也承认「At startup the status map is empty (get defaults to idle), so this only filters sessions that genuinely flipped busy between bootstrap and the scan」。该 `continue` 是裸跳过：无日志、无重试义务——与 lease 的 `blockedGoalClaims` 重触发机制（记录重试义务）不同。快照在 builder 期一次性捕获，没有任何路径重新武装扫描。 + +**运行时影响**：窄但真实的恢复漏洞——在扫描时刻显示 busy 的会话既不被评估也不被记录，目标持久停在 `active` 且休眠，直到无关的用户交互。因为窗口需要 boot 期恰好 busy，实际概率低，故 Low。 + +--- + +## 验证为正确的部分（本次特意检查并确认无缺陷） + +**MEMORY** +- **#324 debt 1（SSE 逐 chunk 存活判定）已真正偿付。** `model.ts:13-14` 把 `CONNECT_TIMEOUT` 与 `IDLE_TIMEOUT` 分成两个独立 60s 预算；`drainWithLiveness`（`model.ts:84-124`）在遍历 `result.fullStream` 的**每次**迭代都 `arm(input.idleTimeout)`，且先重置再判断 `part.type === "error"`，因此没有任何 chunk 种类（含 reasoning delta）被排除在看门狗重置之外。生产路径已无墙钟：`make`（`model.ts:48-67`）只在 `input.timeout !== undefined` 时套 `Effect.timeoutOrElse`，而生产 `layer` 构造 `make({ execute })` 不传 `timeout`。 +- **#328 的 json 词保证**：`requireJsonToken`（`model.ts:71-74`）在 system/prompt 都不含 `/json/i` 时追加 `JSON_HINT`，且在每次 `generate` 上生效（`model.ts:53`）。 +- **#313 的装配修复在可枚举的图上是完整的**：`Memory.node` 在 httpapi `server.ts` 的 app 图中，`Memory.defaultLayer` 在 `AppLayer`；`BootstrapLayer` 不含 Memory，但其唯一消费者 `project/bootstrap.ts` 走 `Effect.serviceOption` 并按设计 no-op。 +- 迁移「先写持久副本再消费 legacy」三阶段实现正确（`identity-migration.ts:106-184`），`sameContent` 正确忽略 controller-owned 元数据（`identity-migration.ts:52-71`）；legacy 文件删除前重读比对（`admission.ts:129-139`、`243-250`）；admission 缓存只在 `unresolved === 0` 时写入，worktree `remove`/`reset` 先 invalidate 再 `ensure` 并传完整目录快照；`writeSnapshot` 以 manifest 发布为单一提交点（`store.ts:239-268`）；strict/lenient 读分离正确；global identity 下 inert 正确；后台维护 fiber 绑定 layer scope 且槽位释放无泄漏。 + +**GOAL** +- **单事务 transition 语义正确**（`goal.ts:335-413`）：读、`decide`、写/删全在一个 `db.transaction(..., { behavior: "immediate" })` 内，外包 `Effect.uninterruptible`，事件在提交后才发布；接口上每个持久变更都走这个 seam。 +- **终态 done 正确**：`goal_outcome` 插入与 `goal_state` 删除同事务，不留中间清理义务。 +- **revision / goal_id 栅栏正确**：`matchesExpected` 在事务内的 `decide` 回调中求值，延迟裁决无法应用到被替换的目标或已 bump 的 revision。 +- **generation fence 未跨 provider 执行**：`prepareIfIdle` 返回延迟的 `AfterFence`，`handoff` 在 `activate` 后释放会话锁再返回 `result`，GoalLoop 在锁外 await；`promptIfIdle` 仍是最终 idle 守卫，Goal 从不用裸 `prompt` 驱动轮次。 +- **lease 优先级正确**：`owner()` 先返回任何 `dag` 再返回 `goal`，最后一个 DAG unregister 的 dag→非 dag 转换在 per-session 锁下原子计算。 +- **loop fiber 生命周期与订阅清理正确**：`registerFiber` 中断前任，`clearFiberIf` 按身份作用域且不中断；idle 订阅与扫描 fiber 都 `forkScoped`。 +- **judge snippet 窗口一致**：`JUDGE_RESPONSE_SNIPPET_CHARS = 4000` 与调用方 `.slice(-4000)` 及 `renderJudgeUserPrompt` 的再切片一致。 +- `/goal resume` 命令路径确实接线（`goal.ts:807-835`），返回 `kick` 由 prompt.ts 派发。 + +**DAG** +- `spawn.ts` 的 `makeDeadlineWatcher` 在各失败模式下正确（store 读重试而非终止监督、瞬态 defect 视为「无法否证所有权」、上限与升级均重试并重抛 interrupt），`Effect.ensuring` 中断 watcherFiber 无泄漏。 +- watcher 替换先中断旧 watcher 再覆写；终态 handler 在 `NodeCompleted` 与 `NodeSkipped` 上都中断。 +- 三个 adoption 入口都在首次 yield 前同步预留 `recovering`、经 `Effect.ensuring` 释放、并以原子 `store.tryClaimAdoption(dagID)` 收口。 +- 陈旧事件仲裁正确：节点终态 handler 重读持久行并丢弃状态已不匹配的事件；`refreshControlFlags` 从 DB 重建 pause/step 标志。 +- rev-view 过滤正确：所有重建输入都用 `store.getCurrentNodes`，被取代的行无法重新播种失败。 +- **有 `output_schema` 的节点若未成功调用 `submit_result` 会 fail（`verdict_fail`）而非以字符串完成**（`capture.ts:143-150` `settleCapturedOutput`），且该判定为 live 路径与崩溃恢复共用——这正是 DAG-01 未命中默认 block 路径的原因。 +- review 裁决门 fail-closed：`reviewVerdict`（`review-lifecycle.ts:323-327`）要求对象并拒绝字符串。 +- `evaluateCondition` 的数值比较在非数/非有限操作数上 loudly fail（与 DAG-01 的等值比较形成对照）。 +- wake 持久性（#326）：`loop.ts:1384-1424` 在持有 lease 时于 admit 时刻持久化 `wake_reported`，lease 丢失/generation 竞争降级为稍后重试，正确重抛 interrupt。 + +## 复核中被推翻/降级的候选项 + +- 子代理最初把 DAG-01 判为「默认 block 路径即命中」。复核 `blocks.ts:251,271,305-309` 与 `capture.ts:143-150` 后**推翻**：`verify`/`review`/`coding`/`prototype` 均声明 schema，且缺 `submit_result` 会 fail 而非以字符串完成。暴露面收窄为 `synthesize` 默认 reporting、作者显式 `report_to_parent: true` 的无 schema block、以及 low-level 手写节点。严重性仍为 High（后果是静默 COMPLETED），但可达性描述已按证据改写。 +- 子代理把 GOAL-01 描述为「blocked → resume」路径。复核后发现该路径下 tail assistant 通常已推进、门不命中；**真正的机制**是「continue 提交 / resume kick 之后、下一条 assistant 落库之前崩溃」，且 `e2e-loop.test.ts:1871-1906` 把这个场景当成「应跳过」显式钉住。结论更强而非更弱。 +- 子代理的 MEM-02（原编号）称「维护提交后失败导致渲染陈旧」置信度 Likely。复核确认代码事实成立，但影响为瞬态自愈，**降级为 Low**（本文 MEM-03）。 +- 子代理的 GOAL-02（原编号，启动扫描 busy 跳过）评 Medium。依据代码自述「启动时 status map 为空、默认 idle」，**降级为 Low**（本文 GOAL-04）。 +- 关于 `Goal.resume` 无生产调用方的初步怀疑**推翻**——是我的 `rg -r` 误用（`-r` 是替换标志）污染了输出；实际接线在 `goal.ts:807`。 + +## 局限 + +1. **未运行测试套件。** 所有并发/竞态结论来自静态阅读控制流，未做动态验证。DAG-01/02/03、MEM-01/02、GOAL-01/02 的修复都应配回归测试后再动态确认。 +2. **#316 触发源未钉死。** DAG-04 证明了放大机制与生产暴露面，但未定位 dispose 期间持续发 `dag.*` 事件的组件；未阅读 `EventV2Bridge.listen`、`InstanceStore.disposeAll`、`InstanceState` scope-close 实现。 +3. **`loop.ts`（1668 行）未逐行读完。** 已读约 62-160、300-360、374-500、543-712、725-800、1220-1290、1380-1424 等区段；`~160-300`、`~945-1107`、`~1290-1380`、`~1520-1639` 未读。这些区段内的缺陷不会被本次发现——DAG 的**否证性结论不具备穷尽性**。 +4. **DAG 模块内未审计的文件**：`blocks.ts` 的 `aggregateParallelWriters`（#299 并行 writer 聚合）、`templates/*`、`workflows.ts`、`admission.ts`、`recovery.ts`、`capture.ts` 的 `validateAgainstSchema`（cyclomatic 29 / cognitive 56，且直接在 structured-output 路径上）、`output-ref.ts`、`tool/workflow.ts` 主体、httpapi dag handlers。未验证的不变量：「一个用户目标至多一个 live DAG」、`portable` 不加载环境目录 / `environment` 验证模型可用性的分工、model-facing schema 隐藏身份字段、Runtime Admission 与 Authoring Check 的职责分离。 +5. **Effect v4 / effect-smol 语义未查证参考实现**：DAG-04 关于 scope finalizer LIFO 顺序与 `Effect.forkIn` 在关闭中 scope 上行为的推理未对照 `effect-smol` 源码。`Effect.catchCause` 捕获 interrupt cause 这一点已由代码内三处 `Cause.hasInterrupts` 显式再抛的既有写法反证成立。 +6. **索引覆盖为 best-effort。** `check_index_coverage` 对所引用路径报 `no_recorded_issue`，但按工具自身声明这不构成完整性证明；`*.test.ts` 全部不在 `fast` 索引内，测试相关结论均来自直接文件读取。 +7. **未审计 `packages/opencode/src` 之外的消费者**（TUI / desktop / CLI 各自的组合根），因此若存在 packages/opencode 之外的 Memory / Goal / Dag 消费者，本次不会发现其装配缺陷。 + +## 建议的处置顺序 + +| 优先级 | 动作 | +|---|---| +| P0 | DAG-01 + DAG-02 一并修：`loop.ts` 条件求值前做字符串归一化；`checkpointGateDiagnostics` 追加「被门控 checkpoint 必须声明 `output_schema`」；把 checkpoint 门接入 `replanStructuralDiagnostics`（或让 `structural` 不再对 replan/extend 整体关闭）。回归用例覆盖 `action: "replan"` 与运行时字符串输出两条。 | +| P0 | GOAL-01：把边界门从「抑制驱动」改为「抑制重判」——命中门时跳过 judge 但仍派发 continuation。必须同步修改 `e2e-loop.test.ts:1889-1906` 的 `continuationCalls` 断言。 | +| P1 | DAG-03：pause 终态失败时改为 `entry.runtime.setPaused(true)` fail-closed；并把 `dag.pause` 的 defect 纳入同一处理。 | +| P1 | MEM-01：`prepareUnsafe` 的 due 分支与 `shouldMatch` 分支改用 `backgroundMaintain` / `applyUpdate` 形状，仅提交拿锁。归入 #324。 | +| P1 | DAG-04：`summary-publisher.ts:166` 补 `Cause.hasInterrupts` 再抛；`global-lifecycle.ts` 的 `disposeAll` 加有界超时。归入 #316（触发源仍需独立定位）。 | +| P2 | GOAL-02：pause 耗尽时保持 `turnDriven` 或同步 unregister，使持久态、lease、进程内标记三者自洽。 | +| P2 | MEM-02：把 identity fence 缩到 `markMatched` 写入；并在规格中显式声明「同查询合并」这一取舍（#324 验收项）。 | +| P3 | MEM-03、GOAL-03、GOAL-04。 | From ed7185a0f1555b82efa0b4e4b8669b9b0d0ea20f Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 13:37:03 +0800 Subject: [PATCH 02/13] fix(goal): boundary gate suppresses re-judgment, not the drive (GOAL-01) --- docs/findings/goal-batch-findings.md | 22 ++ packages/opencode/src/goal/loop.ts | 264 ++++++++++--------- packages/opencode/test/goal/e2e-loop.test.ts | 38 ++- workflows/audit-fix-loop.md | 82 ++++++ 4 files changed, 273 insertions(+), 133 deletions(-) create mode 100644 docs/findings/goal-batch-findings.md create mode 100644 workflows/audit-fix-loop.md diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md new file mode 100644 index 000000000..9e0e9c6ea --- /dev/null +++ b/docs/findings/goal-batch-findings.md @@ -0,0 +1,22 @@ +# GOAL 批次 Findings Register + +- 验收 primary source：`docs/audit-dag-memory-goal-2026-08-18.md`（GOAL 章节） +- 分支：`fix/goal-batch` → PR `dev` +- 收敛判据：连续两轮独立审阅（Spec 镜 + Standards 镜）零 findings + 模块门禁全绿 +- 规格：`workflows/audit-fix-loop.md` + +## 审计缺陷切片（输入项，非审阅 finding） + +| ID | 严重性 | 切片顺序 | 状态 | 提交 | +|---|---|---|---|---| +| GOAL-01 | High | 1 (P0) | 进行中 | — | +| GOAL-02 | Medium | 2 (P2) | 待办 | — | +| GOAL-03 | Low | 3 | 待办 | — | +| GOAL-04 | Low | 4 | 待办 | — | + +## 审阅轮次 + +（每轮审阅结果记账于此；全部关闭后才具备发 PR 资格） + +### Round 1 +- 未开始 diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index f6a55ecd7..65b85e4b2 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -247,10 +247,24 @@ const serviceLayer = Layer.effect( // process; the goal row's last_judged_msg is the crash-surviving record // of which boundary was already judged and committed. While the session // window still ends on that same message, no new progress has landed — - // re-judging would inflate turns_used and dispatch a duplicate - // continuation. Live idle events are never gated here: every dispatched - // continuation produces a fresh assistant message, so the live path - // always judges a new boundary. + // re-judging would inflate turns_used. Live idle events are never gated + // here: every dispatched continuation produces a fresh assistant + // message, so the live path always judges a new boundary. + // + // GOAL-01: the gate suppresses RE-JUDGMENT, never the drive. The old + // behavior `return`ed here, which permanently stranded goals whose + // committed continue evaluation lost its continuation to a crash + // (process died after the commit, before the next assistant message): + // every boot scan re-hit this gate, nothing ever dispatched another + // turn, and last_judged_msg (only written by judge commits) never + // advanced. Now the gate sets suppressJudge and falls through — the + // judge call and its updateAfterJudge commit below are skipped (the + // boundary is already judged; re-judging is what would inflate + // turns_used), but the shared continuation dispatch still runs and + // restores the driver. A second crash repeats this safely: a fresh + // process starts with an empty evaluatedRevisions map and an unchanged + // last_judged_msg, so the gate fires and re-dispatches again. + let suppressJudge = false if (scanResume && goalState.last_judged_msg) { const win = yield* sessions .messages({ sessionID, limit: 20 }) @@ -260,7 +274,7 @@ const serviceLayer = Layer.effect( ), ) const lastSeen = [...win].reverse().find((m) => m.info.role === "assistant") - if (lastSeen && lastSeen.info.id === goalState.last_judged_msg) return + if (lastSeen && lastSeen.info.id === goalState.last_judged_msg) suppressJudge = true } const goalOwner = { kind: "goal" as const, id: goalState.goal_id ?? "legacy" } yield* automation.register(sessionID, goalOwner) @@ -324,123 +338,133 @@ const serviceLayer = Layer.effect( yield* promptSvc.prompt({ sessionID, noReply: true, parts: [{ type: "text", text: `⏸ 目标已暂停 — ${pauseMsg}` }] }).pipe(Effect.ignore) return } - const responseText = lastAssistant.parts - .filter((p): p is Extract<(typeof lastAssistant.parts)[number], { type: "text" }> => p.type === "text") - .map((p) => p.text) - .join("\n") - .slice(-4000) - // When the last assistant turn produced no text (pure tool calls, - // reasoning-only, or a submit_result with no prose), the goal should - // NOT silently stall — the agent is making progress via tools. Skip - // the judge (there is nothing to classify) and continue directly, - // using a synthetic "continue" verdict so the loop dispatches the - // next turn. Previously this was a bare `return` that left the goal - // permanently "active" with no continuation — the agent appeared to - // stop working on its own. - const callLLM = Option.getOrUndefined(yield* Effect.serviceOption(GoalLoopJudgeLLM)) - const verdict = responseText - ? yield* GoalJudge.run( - goalState.goal, - responseText, - goalState.subgoals ?? [], - // Judge LLM call: prefer the test-injected callable so e2e tests - // can script verdicts without Provider/network; otherwise build the - // production Provider → generateText path. - callLLM?.call ?? - ((opts) => - Effect.gen(function* () { - const defaultM = yield* provider.defaultModel() - const small = yield* provider.getSmallModel(defaultM.providerID) - const model = small ?? (yield* provider.getModel(defaultM.providerID, defaultM.modelID)) - const language = yield* provider.getLanguage(model) - const result = yield* Effect.tryPromise({ - try: (signal) => - generateText({ - model: language, - system: opts.system, - prompt: opts.user, - temperature: opts.temperature, - maxOutputTokens: opts.maxTokens, - abortSignal: signal, - }), - catch: (e) => new Error(`judge LLM call failed: ${String(e)}`), - }).pipe(Effect.timeout(`${opts.timeout} seconds`)) - if (!result) return "" - return result.text - })), - ) - : { verdict: "continue" as const, reason: "上一轮无文本输出（纯工具调用），跳过判定直接继续", parseFailed: false } - - const updateResult = Option.getOrUndefined( - yield* automation.use( - observedLease, - goal.updateAfterJudge( - sessionID, - verdict.verdict, - verdict.reason, - verdict.parseFailed, - { - goalID: goalState.goal_id ?? "legacy", - revision: goalState.revision ?? 0, - }, - lastAssistant.info.id, - ), - ), - ) - if (!updateResult) return - - // D-4: record the committed revision as evaluated-by-this-process - // (every verdict — continue, done, blocked — is a completed - // evaluation of the pre-commit state). - evaluatedRevisions.set(sessionID, updateResult.state.revision ?? 0) - - if (!updateResult.shouldContinue) { - yield* automation.unregister(sessionID, goalOwner) - evaluatedRevisions.delete(sessionID) - if (verdict.verdict === "done") { - // GOAL-FP-01-15: the done transition has already committed when this - // prompt runs (durable state leads presentation — the row is gone - // and goal.updated(done)/goal.cleared are published), so a failure - // here loses only the transcript line, never the state. Never - // swallow it silently — log it so a lost confirmation is - // diagnosable. No retry: a retried prompt could re-inject a "done" - // line after the goal was re-created. - yield* promptSvc.prompt({ - sessionID, - noReply: true, - parts: [{ type: "text", text: updateResult.message }], - }).pipe( - Effect.catchCause((cause) => - Effect.logWarning("goal done message delivery failed", { - sessionID, - cause: Cause.pretty(cause), - }), - ), - ) - } else { - // Auto-pause branch: updateAfterJudge paused the goal due to - // judge-parse-failure or budget exhaustion (verdict.verdict is - // still "continue"). Without surfacing the message here, these - // automatic pauses would be invisible to the user — updateAfterJudge - // already saved the paused state and published goal.updated, but - // nothing rendered the "⏸ 目标已暂停 — …" line into the transcript. - // Emit it as a noReply part so it shows up without spawning a new - // agent turn; the fiber then naturally terminates (no clearFiber - // needed, see updateAfterJudge). - yield* promptSvc.prompt({ - sessionID, - noReply: true, - parts: [{ type: "text", text: updateResult.message }], - }).pipe( - Effect.catchCause((cause) => - Effect.logWarning("goal pause message delivery failed", { - sessionID, - cause: Cause.pretty(cause), - }), + // GOAL-01: on a boundary-gate hit the judge call and its commit are + // skipped wholesale (see suppressJudge above) — execution falls through + // to the shared continuation dispatch below. + if (!suppressJudge) { + const responseText = lastAssistant.parts + .filter((p): p is Extract<(typeof lastAssistant.parts)[number], { type: "text" }> => p.type === "text") + .map((p) => p.text) + .join("\n") + .slice(-4000) + // When the last assistant turn produced no text (pure tool calls, + // reasoning-only, or a submit_result with no prose), the goal should + // NOT silently stall — the agent is making progress via tools. Skip + // the judge (there is nothing to classify) and continue directly, + // using a synthetic "continue" verdict so the loop dispatches the + // next turn. Previously this was a bare `return` that left the goal + // permanently "active" with no continuation — the agent appeared to + // stop working on its own. + const callLLM = Option.getOrUndefined(yield* Effect.serviceOption(GoalLoopJudgeLLM)) + const verdict = responseText + ? yield* GoalJudge.run( + goalState.goal, + responseText, + goalState.subgoals ?? [], + // Judge LLM call: prefer the test-injected callable so e2e tests + // can script verdicts without Provider/network; otherwise build the + // production Provider → generateText path. + callLLM?.call ?? + ((opts) => + Effect.gen(function* () { + const defaultM = yield* provider.defaultModel() + const small = yield* provider.getSmallModel(defaultM.providerID) + const model = small ?? (yield* provider.getModel(defaultM.providerID, defaultM.modelID)) + const language = yield* provider.getLanguage(model) + const result = yield* Effect.tryPromise({ + try: (signal) => + generateText({ + model: language, + system: opts.system, + prompt: opts.user, + temperature: opts.temperature, + maxOutputTokens: opts.maxTokens, + abortSignal: signal, + }), + catch: (e) => new Error(`judge LLM call failed: ${String(e)}`), + }).pipe(Effect.timeout(`${opts.timeout} seconds`)) + if (!result) return "" + return result.text + })), + ) + : { verdict: "continue" as const, reason: "上一轮无文本输出（纯工具调用），跳过判定直接继续", parseFailed: false } + + const updateResult = Option.getOrUndefined( + yield* automation.use( + observedLease, + goal.updateAfterJudge( + sessionID, + verdict.verdict, + verdict.reason, + verdict.parseFailed, + { + goalID: goalState.goal_id ?? "legacy", + revision: goalState.revision ?? 0, + }, + lastAssistant.info.id, ), - ) + ), + ) + if (!updateResult) return + + // D-4: record the committed revision as evaluated-by-this-process + // (every verdict — continue, done, blocked — is a completed + // evaluation of the pre-commit state). + evaluatedRevisions.set(sessionID, updateResult.state.revision ?? 0) + + if (!updateResult.shouldContinue) { + yield* automation.unregister(sessionID, goalOwner) + evaluatedRevisions.delete(sessionID) + if (verdict.verdict === "done") { + // GOAL-FP-01-15: the done transition has already committed when this + // prompt runs (durable state leads presentation — the row is gone + // and goal.updated(done)/goal.cleared are published), so a failure + // here loses only the transcript line, never the state. Never + // swallow it silently — log it so a lost confirmation is + // diagnosable. No retry: a retried prompt could re-inject a "done" + // line after the goal was re-created. + yield* promptSvc.prompt({ + sessionID, + noReply: true, + parts: [{ type: "text", text: updateResult.message }], + }).pipe( + Effect.catchCause((cause) => + Effect.logWarning("goal done message delivery failed", { + sessionID, + cause: Cause.pretty(cause), + }), + ), + ) + } else { + // Auto-pause branch: updateAfterJudge paused the goal due to + // judge-parse-failure or budget exhaustion (verdict.verdict is + // still "continue"). Without surfacing the message here, these + // automatic pauses would be invisible to the user — updateAfterJudge + // already saved the paused state and published goal.updated, but + // nothing rendered the "⏸ 目标已暂停 — …" line into the transcript. + // Emit it as a noReply part so it shows up without spawning a new + // agent turn; the fiber then naturally terminates (no clearFiber + // needed, see updateAfterJudge). + yield* promptSvc.prompt({ + sessionID, + noReply: true, + parts: [{ type: "text", text: updateResult.message }], + }).pipe( + Effect.catchCause((cause) => + Effect.logWarning("goal pause message delivery failed", { + sessionID, + cause: Cause.pretty(cause), + }), + ), + ) + } + return } - return + } else { + // GOAL-01 gate hit: mark the current revision as drive-restored so a + // duplicate scan trigger on the SAME revision is skipped by the D-4 + // gate above (at most one continuation per revision per process). + evaluatedRevisions.set(sessionID, goalState.revision ?? 0) } const currentStatus = yield* status.get(sessionID) diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index 0ba2cf7eb..290d26018 100644 --- a/packages/opencode/test/goal/e2e-loop.test.ts +++ b/packages/opencode/test/goal/e2e-loop.test.ts @@ -1794,14 +1794,18 @@ describe("GoalLoop — judge-chain defect degrades into the parse budget (GOAL-F ) }) -// issue #285 / GOAL-FP-01-21: the boot scan must not re-judge a boundary the -// crashed process already judged and committed. The process-local -// evaluatedRevisions map dies with the process, so the DURABLE gate is the -// goal row's last_judged_msg: updateAfterJudge records the judged assistant -// message ID on every continue commit, and the scan path skips evaluation -// while the session window still ends on that same message. Live idle events -// are never gated (each dispatched continuation produces a fresh assistant -// message, so the live path always sees a new boundary). +// issue #285 / GOAL-FP-01-21 / GOAL-01: the boot scan must not re-judge a +// boundary the crashed process already judged and committed, but it MUST +// still restore the drive. The process-local evaluatedRevisions map dies +// with the process, so the DURABLE gate is the goal row's last_judged_msg: +// updateAfterJudge records the judged assistant message ID on every continue +// commit, and while the session window still ends on that same message the +// scan path suppresses RE-JUDGMENT only — a plain skip stranded goals whose +// committed continuation was lost to the crash (nothing left to drive them, +// GOAL-01). On a gate hit the judge and its commit are skipped, but the +// continuation dispatch still runs. Live idle events are never gated (each +// dispatched continuation produces a fresh assistant message, so the live +// path always sees a new boundary). describe("GoalLoop — boot scan must not re-evaluate an already-judged boundary (issue #285)", () => { let judgeCalls = 0 let continuationCalls = 0 @@ -1887,7 +1891,11 @@ describe("GoalLoop — boot scan must not re-evaluate an already-judged boundary return result?.state }) - it.instance("scan with an unchanged boundary skips re-evaluation (no inflation)", () => + // GOAL-01: the gate suppresses RE-JUDGMENT, never the drive. The crashed + // continuation must be re-dispatched (the goal would otherwise sit + // permanently active with nothing driving it), while the judge call and + // the turns_used increment stay suppressed (no inflation). + it.instance("scan with an unchanged boundary skips re-judgment but still dispatches the continuation", () => Effect.gen(function* () { reset() const loop = yield* GoalLoop.Service @@ -1899,11 +1907,15 @@ describe("GoalLoop — boot scan must not re-evaluate an already-judged boundary yield* commitPriorBoundary(sid) yield* loop.init() - // Negative assertion: the scan runs in a forked fiber with no readiness - // signal on the skip path, so a bounded wait stands in for polling. - yield* Effect.sleep("300 millis") + // The gate-hit path dispatches the continuation synchronously enough to + // poll on its admission signal instead of a bounded sleep. + yield* pollWithTimeout( + Effect.sync(() => (continuationCalls >= 1 ? true : undefined)), + "gate-hit scan never dispatched the crashed continuation", + "5 seconds", + ) expect(judgeCalls).toBe(0) - expect(continuationCalls).toBe(0) + expect(continuationCalls).toBe(1) const g = yield* goal.load(sid) expect(g?.turns_used).toBe(1) }), diff --git a/workflows/audit-fix-loop.md b/workflows/audit-fix-loop.md new file mode 100644 index 000000000..f52e11e88 --- /dev/null +++ b/workflows/audit-fix-loop.md @@ -0,0 +1,82 @@ +# Workflow: audit-fix-loop（审计缺陷修复固定点循环） + +Primary source：`docs/audit-dag-memory-goal-2026-08-18.md`（验收依据的唯一 source of truth）。 +本规格取代此前 /private/tmp 下的全部 loop 文档与 findings register（已灭失）。 + +## 目的 + +以「开发切片 → 独立审阅 → 发现问题 → 修复 → loop 回审阅」的固定点循环，把审计文档中的缺陷按模块收敛到**零 findings**，每个模块分别以 PR → dev 落地。 + +## Runs + +| Run | 模块 | 分支 | 缺陷集 | 触发 | +|---|---|---|---|---| +| 1 | GOAL | `fix/goal-batch`（已建，基于 origin/dev） | GOAL-01..04 | 立即 | +| 2 | DAG | `fix/dag-batch`（Run 1 合入后从新 dev 切出） | DAG-01..04 | Run 1 PR 合入 dev（事件触发） | + +MEMORY（MEM-01..03）已在多轮循环中偿付（PR #333 合入 dev），不再重跑。 + +## 硬边界 + +- PR 只发 `dev`；禁止发 dev→main PR、禁止 release、禁止直推 `main`/`dev`。 +- 验收依据 = 审计文档缺陷条目（位置证据 + 建议修法 + 测试覆盖缺口）及其「建议处置顺序」；不扩大审计面（审计未读区段的缺陷不在本轮范围）。 +- 所有审阅发现入账 findings register，全部关闭后才具备发 PR 资格。 +- 测试不从仓库根运行；typecheck = 在 `packages/opencode` 内 `bun typecheck`。 + +## 单次 run 流程 + +### 0. 准备 +- GOAL run：先以独立 `docs(audit)` 提交把审计文档入库（两个 run 共同的验收依据必须先进 dev）。创建 `docs/findings/goal-batch-findings.md`。 +- DAG run：确认 dev 基线已含 GOAL 修复 + 审计文档，切 `fix/dag-batch`。创建 `docs/findings/dag-batch-findings.md`。 + +### 1. 切片开发（按审计「建议处置顺序」） +- GOAL run：GOAL-01（P0）→ GOAL-02（P2）→ GOAL-03 → GOAL-04 +- DAG run：DAG-01 + DAG-02（P0，审计明确要求一并修）→ DAG-03（P1）→ DAG-04（P1） + +每个切片： +1. **红**：按审计「测试覆盖」缺口先写/改回归测试，测试必须先在当前代码上失败。 +2. **绿**：按审计「建议修法」最小实现；每个缺陷一个独立提交（提交信息引用缺陷 ID）。 +3. **变异**：临时回退实现 → 第 1 步测试必须翻红 → 恢复（证明测试真的钉住了该缺陷）。 +4. **门禁**：目标测试簇 + `bun typecheck` 绿。 + +### 2. 审阅轮（固定点循环主体） +每轮并行派遣**两个互相独立、只读**的审阅子代理（不得复用开发者推理上下文，只看 diff + 审计文档 + 仓库规约）： +- **Spec 镜**：diff 逐条对照审计文档对应缺陷条目的验收要求； +- **Standards 镜**：diff 对照仓库 AGENTS.md、Effect 规则、`src/goal|dag` 的 CONTEXT.md 与测试 fixture 规约。 + +每个 finding 必须含：ID、严重度、file:line、证据引文、要求动作；写入 findings register。 +- 有 findings → 逐条修复（修复同样走红-绿门禁）→ 回到审阅。 +- **连续两轮全部审阅零 findings = 模块收敛**。 + +### 3. 模块门禁 +- `bun typecheck`（packages/opencode 内）绿； +- 全量测试套件绿（packages/opencode 内运行）； +- diff 自检：改动仅落在对应模块源码 + 测试 + docs。 + +### 4. Checkpoint（唯一人工介入点，push right） +PR 发起前交付一份决策 brief： +- diff 概览（按缺陷分列文件/行数）； +- findings register 全部条目的关闭证据； +- 全量测试 + typecheck 结果； +- PR 标题与正文草稿（conventional 格式）。 + +用户批准 → `gh pr create --base dev` → 附 PR 链接与 CI run 链接收口。 + +**条件 checkpoint（仅当发生时）**：某切片建不出红测试——审计验收失去可验证依据，暂停等待用户裁决降级或停。DAG-04 触发源不属于此类：审计文档已给出无复现时的交付边界（见下），无需人工裁决。 + +## Run-specific 设计要点（探索阶段已定案，实施者直接遵循） + +### GOAL run +- **GOAL-01**：`src/goal/loop.ts` 边界门从「抑制驱动」改为「只抑制重判」——引入 `boundaryGateHit` 标志，命中时跳过 judge + `updateAfterJudge`（不膨胀 turns_used），fall-through 到共享 continuation 派发段恢复驱动；gate-hit 分支写 `evaluatedRevisions` 做同 revision 去重。改写 `test/goal/e2e-loop.test.ts:1890-1910` 钉错的断言：judgeCalls=0、continuationCalls=1（pollWithTimeout 信号）、turns_used=1。 +- **GOAL-02**：`src/goal/goal.ts` `pauseForUserCancel` 把无条件 `turnDriven.delete` 移入成功分支；失败分支保留 turnDriven，使持久态(active)/lease(已注册)/进程内标记三者一致，再次 ESC 仍走 pause 快路径。`test/goal/turn-scope.test.ts` 用写坏 goal_state payload 注入确定性 defect 覆盖。 +- **GOAL-03**：`src/goal/goal.ts` `updateAfterJudge` continue 分支对失败 judge 预算中性：parseFailed 时不递增 turns_used、不盖 last_judged_msg；consecutive_parse_failures 计数与 MAX=3 自动暂停不变。改 GOAL-FP-01-18b e2e 测试：poll 信号换 consecutive_parse_failures>=1，turns_used 断言 0。 +- **GOAL-04**：`src/goal/loop.ts` `scanForActiveGoals` busy skip 记 logInfo + deferred 列表；主循环后同一 scan fiber 内单次有界重试（2s），仍 busy 则 logWarning 收口（会话自身 idle 事件仍是驱动者）。扩展现有 busy-session 测试经 `logLines` 断言跳过日志。 + +### DAG run +- **DAG-01+02（一并修）**：`loop.ts:141-156` 条件求值前对字符串输出做与 `loop.ts:667` 相同的 `parseJsonOption` 归一化；`checkpointGateDiagnostics` 追加「被门控引用的 checkpoint 必须声明 output_schema」；把 checkpoint 门接入 replan/extend 路径（`authoring.ts:136` 的 structural 不再对非 start 动作整体关闭结构检查）。回归用例覆盖 `action: "replan"` 与运行时字符串输出两条。 +- **DAG-03**：`loop.ts:681-703` pause 终态失败改 fail-closed `entry.runtime.setPaused(true)`；`dag.pause` 的 defect 纳入同一处理（不只 error channel）。 +- **DAG-04**：`summary-publisher.ts:151-170` 外层 catchCause 依 `spawn.ts:255-257` 既有模式补 `Cause.hasInterrupts` 再抛；`global-lifecycle.ts:16-25` 生产 disposeAll 加有界超时（参照 exerciser 的 bounded 形状）。回归测试以程序注入事件覆盖 dispose 期间 interrupt 语义。**触发源不追查**，按审计文档原样记录为已知缺口（#316 验收项 3 已由审计回答）。 + +## 完成定义 + +两个 run 各自满足：连续两轮零 findings + 模块门禁全绿 + findings register 全部关闭 + PR → dev 创建成功且 CI 运行链接已附。至此循环终止。 From 551b8f78a9d646104044189e39c3aa04884ae7aa Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 13:42:21 +0800 Subject: [PATCH 03/13] fix(goal): keep turn-driven mark when ESC pause exhausts retries (GOAL-02) --- packages/opencode/src/goal/goal.ts | 22 +++++++- .../opencode/test/goal/turn-scope.test.ts | 50 +++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index 1fd6c07f9..2171cf24f 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -161,6 +161,11 @@ export interface Interface { * seam so SessionPrompt.cancel stays free of lease plumbing. No-op (returns * undefined) when the goal is not active. Never fails: pause failures are * logged and swallowed so a cancel path can always proceed. + * + * GOAL-02: when the pause exhausts its retries, durable row and lease + * both still say "active" — the turn mark is RETAINED so the three + * authorities agree and a repeat ESC retries the pause. The mark is + * cleared only on a successfully persisted pause. */ readonly pauseForUserCancel: (sessionID: SessionID, reason: string) => Effect.Effect /** True when the session's current turn is goal-driven. */ @@ -243,6 +248,9 @@ const serviceLayer = Layer.effect( // (shouldPreempt cannot catch it: ESC adds no user message). Retry the // pause twice with a short backoff; if it still fails, log LOUDLY — the // goal may resurrect, but it will never do so invisibly. + // GOAL-02: an exhausted failure path keeps durable row, lease, and the + // turn mark consistent (all still "active/owned/driven") — see the + // failure branch below. const pauseForUserCancel = Effect.fnUntraced(function* (sessionID: SessionID, reason: string) { let paused: GoalState.Info | undefined let lastCause: Cause.Cause | undefined @@ -259,13 +267,23 @@ const serviceLayer = Layer.effect( yield* automation.unregister(sessionID, { kind: "goal", id: paused.goal_id ?? "legacy" }).pipe( Effect.ignore, ) + turnDriven.delete(sessionID) } else { + // GOAL-02: the pause could not be persisted — the durable row is + // still "active" and the lease registration is still in place, so the + // process-local mark must AGREE with both: keep it. Pre-fix it was + // deleted unconditionally, which disagreed with the durable + // authorities (goal still owns the session as active) and lost the + // ESC provenance on the resurrected turn — the user's second ESC + // would no longer route through this goal-pause fast path, because + // SessionPrompt.cancel maps ESC to a goal pause only for marked + // turns. With the mark retained, every repeat ESC retries the pause + // until the store recovers. yield* Effect.logError( - "goal pause on cancel failed after retries — goal may resurrect on next idle", + "goal pause on cancel failed after retries — goal stays active and turn-driven; a repeat ESC retries the pause", { sessionID, cause: lastCause ? Cause.pretty(lastCause) : "unknown" }, ) } - turnDriven.delete(sessionID) return paused }) diff --git a/packages/opencode/test/goal/turn-scope.test.ts b/packages/opencode/test/goal/turn-scope.test.ts index 99dde397d..5608ff293 100644 --- a/packages/opencode/test/goal/turn-scope.test.ts +++ b/packages/opencode/test/goal/turn-scope.test.ts @@ -1,10 +1,13 @@ import { describe, expect } from "bun:test" -import { Effect, Layer } from "effect" +import { Effect, Layer, Schema } from "effect" +import { eq } from "drizzle-orm" import { Goal } from "@/goal/goal" +import { GoalState } from "@/goal/state" import { GoalPrompts } from "@/goal/prompts" import { EventV2Bridge } from "@/event-v2-bridge" import { SessionStatus } from "@/session/status" import { Database } from "@opencode-ai/core/database/database" +import { GoalStateTable } from "@opencode-ai/core/goal/sql" import { SessionID } from "@/session/schema" import { testEffect } from "../lib/effect" @@ -18,10 +21,11 @@ import { testEffect } from "../lib/effect" const testLayer = Goal.layer.pipe( // provideMerge (not provide): the statusLine test body yields // SessionStatus.Service to set busy/idle — it must see the SAME instance the - // Goal service reads. + // Goal service reads. Database is merged for the GOAL-02 fault injection + // (the test body corrupts/restores the goal_state payload directly). Layer.provideMerge(EventV2Bridge.defaultLayer), Layer.provideMerge(SessionStatus.defaultLayer), - Layer.provide(Database.defaultLayer), + Layer.provideMerge(Database.defaultLayer), ) const it = testEffect(testLayer) @@ -102,6 +106,46 @@ describe("Goal turn-scope — pauseForUserCancel (ESC semantics)", () => { }), ) + // GOAL-02: when the pause cannot be persisted after all retries, the durable + // row is still "active" and the lease registration is still in place — the + // process-local turnDriven mark must AGREE with both (kept, not deleted). + // Pre-fix the mark was deleted unconditionally, which lost the ESC + // provenance: the resurrected turn's second ESC no longer routed through the + // goal pause fast path. + it.live("pause failure after retries keeps the turn mark (durable row, lease, mark agree on active)", () => + Effect.gen(function* () { + const goal = yield* Goal.Service + const { db } = yield* Database.Service + const sid = SessionID.descending() + const seeded = yield* goal.set(sid, "test goal", 5) + yield* goal.markTurnDriven(sid) + + // Deterministic pause failure: corrupt the durable row's payload so the + // transition's decode defects on every one of the three retry attempts. + yield* db + .update(GoalStateTable) + .set({ payload: "{corrupt" }) + .where(eq(GoalStateTable.session_id, sid)) + .run() + + const paused = yield* goal.pauseForUserCancel(sid, "用户中断（ESC）") + expect(paused).toBeUndefined() + expect(yield* goal.isTurnDriven(sid)).toBe(true) + + // Restore a valid active row: the pause seam works again, and the + // successful pause clears the mark exactly like the healthy path. + yield* db + .update(GoalStateTable) + .set({ payload: JSON.stringify(Schema.encodeSync(GoalState.Info)(seeded)) }) + .where(eq(GoalStateTable.session_id, sid)) + .run() + + const retried = yield* goal.pauseForUserCancel(sid, "用户中断（ESC）重试") + expect(retried?.status).toBe("paused") + expect(yield* goal.isTurnDriven(sid)).toBe(false) + }), + ) + it.live("terminal transitions clear the mark (markDone)", () => Effect.gen(function* () { const goal = yield* Goal.Service From 9fc67e8e787ab8473804af3f2cc20d76bcd0387a Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 13:46:37 +0800 Subject: [PATCH 04/13] fix(goal): failed judge is budget-neutral, does not stamp judged boundary (GOAL-03) --- packages/opencode/src/goal/goal.ts | 20 +++++-- packages/opencode/test/goal/e2e-loop.test.ts | 17 ++++-- packages/opencode/test/goal/goal.test.ts | 56 +++++++++++++++++++- 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index 2171cf24f..add33dc17 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -91,7 +91,9 @@ export interface Interface { /** issue #285: the assistant message ID judged for this evaluation. * Persisted on continue commits as the DURABLE crash-recovery gate — the * boot scan skips a window still ending on this boundary (the - * process-local evaluatedRevisions map cannot survive a crash). */ + * process-local evaluatedRevisions map cannot survive a crash). + * GOAL-03: never persisted when `parseFailed` — a judge that produced no + * verdict judged no boundary. */ judged?: string, ) => Effect.Effect< | { @@ -748,7 +750,17 @@ const serviceLayer = Layer.effect( } } - const turnsUsed = GoalState.nni(state.turns_used + 1) + // GOAL-03: a judge that never produced a verdict (transport error or + // unparseable output) evaluated no turn — budget-neutral: it must not + // consume one of the user's max_turns, and it must not stamp + // last_judged_msg (the boundary was never judged; a crash after this + // commit must re-judge the same boundary, and that re-judgment — not + // this failed attempt — may consume the budget slot). Pre-fix a flaky + // judge burned budget on unevaluated turns while intermittent + // successes kept the parse-failure counter resetting. The counter + // itself still climbs here, so MAX_CONSECUTIVE_PARSE_FAILURES + // auto-pause is unaffected. + const turnsUsed = parseFailed ? state.turns_used : GoalState.nni(state.turns_used + 1) const pauseReason = newParseFailures >= GoalPrompts.MAX_CONSECUTIVE_PARSE_FAILURES ? "judge 模型未返回有效 JSON 判定。请检查模型配置或换用更可靠的模型，然后 /goal resume。" @@ -764,7 +776,9 @@ const serviceLayer = Layer.effect( paused_reason: pauseReason, consecutive_parse_failures: GoalState.nni(newParseFailures), // issue #285: record the judged boundary for the durable scan gate. - ...(judged !== undefined ? { last_judged_msg: judged } : {}), + // GOAL-03: only a judge that actually returned a verdict judged the + // boundary (see turnsUsed above). + ...(judged !== undefined && !parseFailed ? { last_judged_msg: judged } : {}), }) return { tag: "save", diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index 290d26018..1a83bbfd1 100644 --- a/packages/opencode/test/goal/e2e-loop.test.ts +++ b/packages/opencode/test/goal/e2e-loop.test.ts @@ -1731,7 +1731,9 @@ describe("GoalLoop — NotFoundError on the post-judge reload must not stall (GO // resolution → getLanguage → generateText) can defect (config orDie, payload // decode throws); a defect escaping into the fork was the invisible 0-turn // stall class. catchCause folds it into the parseFailed budget so the loop -// commits the turn and auto-pauses after MAX_CONSECUTIVE_PARSE_FAILURES. +// commits the parse failure and auto-pauses after +// MAX_CONSECUTIVE_PARSE_FAILURES (GOAL-03: budget-neutrally — a failed judge +// consumed no turn, so turns_used and the boundary stamp are untouched). describe("GoalLoop — judge-chain defect degrades into the parse budget (GOAL-FP-01-18b)", () => { let judgeCalls = 0 const sessionMock = Layer.mock(Session.Service, { @@ -1763,7 +1765,11 @@ describe("GoalLoop — judge-chain defect degrades into the parse budget (GOAL-F ) const it = testEffect(defectLayer) - it.instance("a defecting judge commits the turn and counts a parse failure", () => + // GOAL-03: the commit still lands (the parse-failure counter advances so + // the auto-pause safety valve keeps working), but the failed judge is + // budget-neutral — it evaluated no turn, so turns_used stays 0 and the + // boundary is not stamped as judged. + it.instance("a defecting judge commits a parse failure without consuming budget", () => Effect.gen(function* () { judgeCalls = 0 const loop = yield* GoalLoop.Service @@ -1780,14 +1786,15 @@ describe("GoalLoop — judge-chain defect degrades into the parse budget (GOAL-F const committed = yield* pollWithTimeout( Effect.gen(function* () { const g = yield* goal.load(sid) - return g && g.turns_used >= 1 ? g : undefined + return g && g.consecutive_parse_failures >= 1 ? g : undefined }), - "turn never committed — the judge defect escaped the fork", + "parse failure never committed — the judge defect escaped the fork", "5 seconds", ) expect(judgeCalls).toBe(1) - expect(committed.turns_used).toBe(1) + expect(committed.turns_used).toBe(0) expect(committed.consecutive_parse_failures).toBe(1) + expect(committed.last_judged_msg).toBeUndefined() // First defect is a blip: verdict stays continue, goal keeps running. expect(committed.status).toBe("active") }), diff --git a/packages/opencode/test/goal/goal.test.ts b/packages/opencode/test/goal/goal.test.ts index ef65f9bab..876070ee0 100644 --- a/packages/opencode/test/goal/goal.test.ts +++ b/packages/opencode/test/goal/goal.test.ts @@ -460,11 +460,17 @@ describe("Goal.resume — preserves turns_used (no fresh budget), resets parse f const sessionID = SessionID.descending() const state = yield* goal.set(sessionID, "build feature X", 10) - // One continuation dispatch with a parse failure → turns_used=1, cpf=1 - yield* goal.updateAfterJudge(sessionID, "continue", "more steps", true, { + // One SUCCESSFUL continue judgment → turns_used=1 (real budget spent). + const s1 = yield* goal.updateAfterJudge(sessionID, "continue", "real verdict", false, { goalID: state.goal_id ?? "legacy", revision: state.revision ?? 0, }) + // Then one parse-failure judgment → cpf=1 while turns_used stays 1 + // (GOAL-03 budget-neutrality: a failed judge spends no turn). + yield* goal.updateAfterJudge(sessionID, "continue", "more steps", true, { + goalID: s1?.state.goal_id ?? "legacy", + revision: s1?.state.revision ?? 0, + }) const beforePause = yield* goal.load(sessionID) expect(Number(beforePause?.turns_used)).toBe(1) expect(Number(beforePause?.consecutive_parse_failures)).toBe(1) @@ -727,6 +733,52 @@ describe("Goal.updateAfterJudge — transport failures trigger auto-pause (D5)", expect(state?.status).toBe("paused") }), ) + + // GOAL-03: a judge that never produced a verdict (transport error or + // unparseable output) evaluated no turn — it must not consume one of the + // user's max_turns and must not stamp last_judged_msg (the boundary was + // never judged; a crash after this commit must re-judge the same boundary, + // and that re-judgment — not this failed attempt — may consume the budget + // slot). The parse-failure counter still climbs, so an unreliable judge + // still auto-pauses after MAX_CONSECUTIVE_PARSE_FAILURES. + it.live("a failed judge is budget-neutral: no turns_used increment, no last_judged_msg stamp", () => + Effect.gen(function* () { + const goal = yield* Goal.Service + const sessionID = SessionID.descending() + const seeded = yield* goal.set(sessionID, "build feature X", 10) + + const failed = yield* goal.updateAfterJudge( + sessionID, + "continue", + "judge transport error (timeout or network) — counting toward pause budget", + true, + { goalID: seeded.goal_id ?? "legacy", revision: seeded.revision ?? 0 }, + "msg_boundary_failed", + ) + expect(failed?.shouldContinue).toBe(true) + + const state = yield* goal.load(sessionID) + expect(Number(state?.turns_used)).toBe(0) + expect(state?.last_judged_msg).toBeUndefined() + expect(Number(state?.consecutive_parse_failures)).toBe(1) + + // A successful judge afterwards consumes exactly one budget slot and + // stamps the boundary it actually judged. + const ok = yield* goal.updateAfterJudge( + sessionID, + "continue", + "real verdict", + false, + { goalID: state?.goal_id ?? "legacy", revision: state?.revision ?? 0 }, + "msg_boundary_real", + ) + expect(ok?.shouldContinue).toBe(true) + const after = yield* goal.load(sessionID) + expect(Number(after?.turns_used)).toBe(1) + expect(after?.last_judged_msg).toBe("msg_boundary_real") + expect(Number(after?.consecutive_parse_failures)).toBe(0) + }), + ) }) // --------------------------------------------------------------------------- From f0e7278655938c27b8a0a072806a11f19100028a Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 13:51:04 +0800 Subject: [PATCH 05/13] fix(goal): startup scan busy skip is logged and retried once, never silent (GOAL-04) --- packages/opencode/src/goal/loop.ts | 50 +++++++++++++++++++- packages/opencode/test/goal/e2e-loop.test.ts | 13 +++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index 65b85e4b2..f02d94866 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -100,6 +100,13 @@ export function isStaleZombie( ) } +// GOAL-04: the startup scan's single retry window for sessions busy at scan +// time. Short by design: a genuinely running turn ends with its own idle +// event, which the (already armed) idle subscription drives — the retry only +// covers the bootstrap→scan status race and gives the skip a visible, +// bounded end instead of a silent drop. +const SCAN_BUSY_RETRY_DELAY = "2 seconds" + const serviceLayer = Layer.effect( Service, Effect.gen(function* () { @@ -681,6 +688,9 @@ const serviceLayer = Layer.effect( // will be driven by its own turn-end idle event. At startup the status // map is empty (get defaults to idle), so this only filters sessions // that genuinely flipped busy between bootstrap and the scan. + // GOAL-04: the skip is never silent — deferred sessions are logged and + // retried once after SCAN_BUSY_RETRY_DELAY, and a final busy state is + // abandoned with an explicit warning. // - Crash window (query → trigger): terminal changes are absorbed by the // active-status re-check; non-terminal changes (already evaluated in // this process) by the D-4 record gate in triggerEvaluation/afterIdle. @@ -688,9 +698,17 @@ const serviceLayer = Layer.effect( // session never kills the rest of the scan; the whole scan is forked, // so a failure can never kill init. const scanForActiveGoals = Effect.fnUntraced(function* (snapshot: ReadonlyArray) { + const deferred: SessionID[] = [] for (const sessionID of snapshot) { const current = yield* status.get(sessionID) - if (current.type !== "idle") continue + if (current.type !== "idle") { + // GOAL-04: never skip silently. Pre-fix this was a bare `continue` + // — no log, no retry obligation — leaving the goal persistently + // active but dormant with nothing to diagnose. Record the session + // for the bounded retry below. + deferred.push(sessionID) + continue + } yield* triggerEvaluation(sessionID, true).pipe( Effect.catchCause((cause) => Effect.logWarning("goal startup scan failed for session", { @@ -700,6 +718,36 @@ const serviceLayer = Layer.effect( ), ) } + if (deferred.length === 0) return + yield* Effect.logInfo("goal startup scan deferred busy sessions", { + sessions: deferred.join(","), + }) + // GOAL-04 retry obligation: one bounded re-check in this same scan + // fiber (already forked + supervised, so the sleep can never kill + // init). Sessions still busy after the window are left to their own + // turn-end idle event — the idle subscription is armed and drives them + // then; a session whose turn never emits idle is a runner defect + // outside the goal module, but the warning makes the abandonment + // visible instead of silent. + yield* Effect.sleep(SCAN_BUSY_RETRY_DELAY) + for (const sessionID of deferred) { + const current = yield* status.get(sessionID) + if (current.type !== "idle") { + yield* Effect.logWarning( + "goal startup scan gave up on busy session — its own idle event remains the driver", + { sessionID, status: current.type }, + ) + continue + } + yield* triggerEvaluation(sessionID, true).pipe( + Effect.catchCause((cause) => + Effect.logWarning("goal startup scan retry failed for session", { + sessionID, + cause: Cause.pretty(cause), + }), + ), + ) + } }) const init = Effect.fn("GoalLoop.init")(function* () { diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index 1a83bbfd1..ccd93aadd 100644 --- a/packages/opencode/test/goal/e2e-loop.test.ts +++ b/packages/opencode/test/goal/e2e-loop.test.ts @@ -1377,6 +1377,19 @@ describe("GoalLoop — startup scan resumes pre-boot active goals (GOAL-FP-01-04 expect(a?.status).toBe("active") expect(Number(a?.turns_used)).toBe(0) + // GOAL-04: the busy skip must be VISIBLE — the scan logs the deferral + // with the session id (previously a bare `continue`: no log, no retry + // obligation, a silently dormant goal). + yield* pollWithTimeout( + Effect.gen(function* () { + const logs = JSON.stringify(yield* logLines) + return logs.includes("goal startup scan deferred busy sessions") ? (true as const) : undefined + }), + "busy session skip was never logged (GOAL-04)", + "5 seconds", + ) + expect(JSON.stringify(yield* logLines)).toContain(String(sidA)) + // When the busy session finishes, its own idle event drives the goal. yield* status.set(sidA, { type: "idle" }) yield* pollWithTimeout( From 429e58815c34a29ab03384d8064d655277e09dcf Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 14:07:03 +0800 Subject: [PATCH 06/13] chore(goal): align comments with GOAL-02/04 semantics (review round 1) --- docs/findings/goal-batch-findings.md | 14 ++++++++++---- packages/opencode/src/goal/goal.ts | 9 ++++++--- packages/opencode/test/goal/e2e-loop.test.ts | 5 ++++- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index 9e0e9c6ea..0ec838d23 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -9,10 +9,16 @@ | ID | 严重性 | 切片顺序 | 状态 | 提交 | |---|---|---|---|---| -| GOAL-01 | High | 1 (P0) | 进行中 | — | -| GOAL-02 | Medium | 2 (P2) | 待办 | — | -| GOAL-03 | Low | 3 | 待办 | — | -| GOAL-04 | Low | 4 | 待办 | — | +| GOAL-01 | High | 1 (P0) | 完成（红-绿-变异通过） | ed7185a0f | +| GOAL-02 | Medium | 2 (P2) | 完成（红-绿-变异通过） | 551b8f78a | +| GOAL-03 | Low | 3 | 完成（红-绿-变异通过） | 9fc67e8e7 | +| GOAL-04 | Low | 4 | 完成（红-绿-变异通过） | f0e727865 | + +## 模块门禁 +- `bun typecheck`（tsgo --noEmit）：✅ 绿 +- goal 目标测试簇（test/goal/，107 tests）：✅ 绿 +- 全量测试套件：进行中 +- 每切片变异验证（revert 翻红 → 恢复）：✅ GOAL-01/02/03/04 均通过 ## 审阅轮次 diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index add33dc17..f0590641a 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -210,9 +210,12 @@ const serviceLayer = Layer.effect( // GOAL-TURN-SCOPE: process-local provenance of the CURRENT goal-driven // turn. Keyed by session; set at every goal dispatch (kick in prompt.ts, // continuation in loop.ts), cleared at turn end (afterIdle entry) and at - // every terminal transition (pause/clear/markDone) plus ESC-cancel. A stale - // mark is harmless: goalTurnMaxSteps re-validates against the durable goal - // row before reporting a ceiling. + // every terminal transition (pause/clear/markDone). On ESC-cancel the + // clear happens ONLY when the pause persisted — if the pause exhausts its + // retries the mark is RETAINED so it agrees with the still-active + // durable row and lease (GOAL-02). A stale mark is harmless: + // goalTurnMaxSteps re-validates against the durable goal row before + // reporting a ceiling. const turnDriven = new Set() const markTurnDriven = Effect.fnUntraced(function* (sessionID: SessionID) { diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index ccd93aadd..5635ac8da 100644 --- a/packages/opencode/test/goal/e2e-loop.test.ts +++ b/packages/opencode/test/goal/e2e-loop.test.ts @@ -1348,7 +1348,7 @@ describe("GoalLoop — startup scan resumes pre-boot active goals (GOAL-FP-01-04 }), ) - it.instance("a busy session is not force-evaluated by the scan; its own idle event drives it", () => + it.instance("a busy session is not force-evaluated by the scan; its own idle event (or the bounded scan retry) drives it", () => Effect.gen(function* () { reset() const loop = yield* GoalLoop.Service @@ -1391,6 +1391,9 @@ describe("GoalLoop — startup scan resumes pre-boot active goals (GOAL-FP-01-04 expect(JSON.stringify(yield* logLines)).toContain(String(sidA)) // When the busy session finishes, its own idle event drives the goal. + // (The GOAL-04 bounded scan retry may also re-trigger it if the flip to + // idle happens within the retry window — the revision fence keeps the + // commit exactly-once either way.) yield* status.set(sidA, { type: "idle" }) yield* pollWithTimeout( Effect.sync(() => (judgeCalls >= 2 ? true : undefined)), From 5dd5a3037063d4563b4b7be8cb230e8a3e3e447e Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 14:37:08 +0800 Subject: [PATCH 07/13] fix(goal): distinguish ESC-pause no-op from retry exhaustion; silence scan disposal interrupts (review R3) --- docs/findings/goal-batch-findings.md | 17 +++++++++- packages/opencode/src/goal/goal.ts | 31 ++++++++++++------- packages/opencode/src/goal/loop.ts | 7 ++++- .../opencode/test/goal/turn-scope.test.ts | 21 +++++++++++++ 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index 0ec838d23..b16282edc 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -17,12 +17,27 @@ ## 模块门禁 - `bun typecheck`（tsgo --noEmit）：✅ 绿 - goal 目标测试簇（test/goal/，107 tests）：✅ 绿 -- 全量测试套件：进行中 - 每切片变异验证（revert 翻红 → 恢复）：✅ GOAL-01/02/03/04 均通过 +- 全量测试套件（`bun test`，4141 tests / 341 files）：goal 相关全绿；另 3 处失败经基线复跑判定为**非本批引入**（见下）。 + - 基线 CI（`1d087ffe9`，GitHub linux）全量 **success** → 基线干净。 + - 本机（darwin）基线 detached 复跑：`project-copy`、`help-snapshots` 同样失败，`httpapi-v2-pty` 计时性 flake（隔离复跑即过）。三者均不 import `src/goal`，diff 亦不触及其依赖闭包 → 环境/时序性既有缺陷，与本批改动无因果。 ## 审阅轮次 （每轮审阅结果记账于此；全部关闭后才具备发 PR 资格） ### Round 1 +- 派遣：Spec 镜（对照审计 GOAL 章节逐条验收）+ Standards 镜（仓库规约/Effect/CONTEXT/测试纪律），只读、并行、互不复用上下文。 +- Standards 镜：**PASS，no findings**。 +- Spec 镜：**PASS**，2 项 Low findings（均已关闭）： + - F-1（Low）`src/goal/goal.ts`：GOAL-02 后 turnDriven 汇总注释仍写"ESC-cancel 即清除"，与"仅 pause 持久化成功才清除"不符。→ 已改写注释（commit 429e58815）。 + - F-2（Low）`test/goal/e2e-loop.test.ts`：GOAL-04 断言所在用例名/注释未提"有界 scan 重试也可驱动 deferred 会话"。→ 已改名 + 补注释（commit 429e58815）。 +- 结论：非干净轮。修复 F-1/F-2 后进入 Round 2。 + +### Round 2 +- Spec 镜：**PASS，no findings**（F-1/F-2 修复逐行复核通过；四缺陷验收保持满足；429e58815 仅注释/命名变更，无行为影响）。 +- Standards 镜：**PASS，no findings**。 +- 结论：第 1 个干净轮。按收敛判据需连续两轮零 findings → 进入 Round 3。 + +### Round 3 - 未开始 diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index f0590641a..60483419b 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -273,21 +273,28 @@ const serviceLayer = Layer.effect( Effect.ignore, ) turnDriven.delete(sessionID) - } else { - // GOAL-02: the pause could not be persisted — the durable row is - // still "active" and the lease registration is still in place, so the - // process-local mark must AGREE with both: keep it. Pre-fix it was - // deleted unconditionally, which disagreed with the durable - // authorities (goal still owns the session as active) and lost the - // ESC provenance on the resurrected turn — the user's second ESC - // would no longer route through this goal-pause fast path, because - // SessionPrompt.cancel maps ESC to a goal pause only for marked - // turns. With the mark retained, every repeat ESC retries the pause - // until the store recovers. + } else if (lastCause) { + // GOAL-02: genuine retry exhaustion — the pause could not be + // persisted, the durable row is still "active" and the lease + // registration is still in place, so the process-local mark must + // AGREE with both: keep it. Pre-fix it was deleted unconditionally, + // which disagreed with the durable authorities (goal still owns the + // session as active) and lost the ESC provenance on the resurrected + // turn — the user's second ESC would no longer route through this + // goal-pause fast path, because SessionPrompt.cancel maps ESC to a + // goal pause only for marked turns. With the mark retained, every + // repeat ESC retries the pause until the store recovers. yield* Effect.logError( "goal pause on cancel failed after retries — goal stays active and turn-driven; a repeat ESC retries the pause", - { sessionID, cause: lastCause ? Cause.pretty(lastCause) : "unknown" }, + { sessionID, cause: Cause.pretty(lastCause) }, ) + } else { + // Successful NO-OP: pauseAndPublish found no active goal (row absent + // or already paused/cleared — e.g. an auto-pause committed between + // the mark and this ESC). No durable authority claims the goal as + // active, so there is nothing to retain the mark for and no failure + // to report — retire the stale mark silently. + turnDriven.delete(sessionID) } return paused }) diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index f02d94866..06a11b36c 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -215,7 +215,12 @@ const serviceLayer = Layer.effect( // init. yield* scanForActiveGoals(snapshot).pipe( Effect.catchCause((cause) => - Effect.logWarning("goal startup scan failed", { cause: Cause.pretty(cause) }), + // GOAL-04: instance disposal interrupts this forked scan fiber — + // the bounded retry sleep widened that window. Same F1 discipline + // as triggerEvaluation: interrupts stay silent, real failures log. + Cause.hasInterrupts(cause) + ? Effect.void + : Effect.logWarning("goal startup scan failed", { cause: Cause.pretty(cause) }), ), Effect.forkScoped, ) diff --git a/packages/opencode/test/goal/turn-scope.test.ts b/packages/opencode/test/goal/turn-scope.test.ts index 5608ff293..9cafded3b 100644 --- a/packages/opencode/test/goal/turn-scope.test.ts +++ b/packages/opencode/test/goal/turn-scope.test.ts @@ -146,6 +146,27 @@ describe("Goal turn-scope — pauseForUserCancel (ESC semantics)", () => { }), ) + // Review R3-INFO-1: a successful NO-OP (goal already paused/cleared when ESC + // lands) must not be reported as a retry-exhaustion failure — no durable + // authority claims the goal as active, so the stale mark is retired + // silently. + it.live("cancel on an already-paused goal is a silent no-op that retires a stale mark", () => + Effect.gen(function* () { + const goal = yield* Goal.Service + const sid = SessionID.descending() + yield* goal.set(sid, "test goal", 5) + yield* goal.markTurnDriven(sid) + yield* goal.pause(sid, "auto-paused") + + const paused = yield* goal.pauseForUserCancel(sid, "用户中断（ESC）") + expect(paused).toBeUndefined() + expect(yield* goal.isTurnDriven(sid)).toBe(false) + const state = yield* goal.load(sid) + expect(state?.status).toBe("paused") + expect(state?.paused_reason).toBe("auto-paused") + }), + ) + it.live("terminal transitions clear the mark (markDone)", () => Effect.gen(function* () { const goal = yield* Goal.Service From ce87f84bdd9c2ed7261e4ec746076a14cf93c7b4 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 14:38:02 +0800 Subject: [PATCH 08/13] docs(goal): record review round-3 findings and closures --- docs/findings/goal-batch-findings.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index b16282edc..c23910fb4 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -40,4 +40,12 @@ - 结论：第 1 个干净轮。按收敛判据需连续两轮零 findings → 进入 Round 3。 ### Round 3 +- Spec 镜：**PASS，no findings**。 +- Standards 镜：**PASS**（verdict），3 条 INFO（非阻塞），处置如下： + - R3-INFO-1（goal.ts pauseForUserCancel）：成功 no-op（ESC 落在已暂停/已清除目标上，如 auto-pause 提交与 mark 之间的窗口）被误报为 retry-exhaustion ERROR。→ **已修复**（commit 5dd5a3037）：以 `lastCause` 区分三态——成功 pause（清 mark+unregister）、真实耗尽（保留 mark+ERROR）、成功 no-op（静默清除陈旧 mark）；新增回归测试 `cancel on an already-paused goal is a silent no-op that retires a stale mark`。 + - R3-INFO-2（loop.ts scan 级 catchCause）：GOAL-04 新增 2s 重试放大了 dispose 中断窗口，正常关停会被记成 "goal startup scan failed"。→ **已修复**（commit 5dd5a3037）：与同文件 triggerEvaluation 相同的 F1 纪律——`Cause.hasInterrupts` 静默，真实失败才告警。无独立红测试：dispose-期间中断无法在当前 harness 内确定性触发而不耦合 instance 内部；以同文件既有 F1 模式一致性为准。 + - R3-INFO-3（分支含 3 个非 goal 文件）：审计文档/findings register/workflow 规格随 GOAL PR 落地是 workflow 规格的设计决定（audit-fix-loop.md §0：审计文档必须先于两个 run 进 dev），**非缺陷，按设计关闭**。 +- 结论：非干净轮（Round 2 的连续干净计数重置）。修复后进入 Round 4。 + +### Round 4 - 未开始 From db44487c77f88a1ec8801ee26468d4cefe1f8f58 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 15:01:16 +0800 Subject: [PATCH 09/13] refactor(goal): address review round-4 findings (no-op classification, neutral pause msg, interrupt suppression, stronger no-op test) --- docs/findings/goal-batch-findings.md | 9 ++++++ packages/opencode/src/goal/goal.ts | 20 +++++++++---- packages/opencode/src/goal/loop.ts | 28 +++++++++++++------ .../opencode/test/goal/turn-scope.test.ts | 18 ++++++++---- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index c23910fb4..0a0ea85ec 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -48,4 +48,13 @@ - 结论：非干净轮（Round 2 的连续干净计数重置）。修复后进入 Round 4。 ### Round 4 +- Spec 镜：**PASS**，2 条 INFO；Standards 镜：**PASS**，4 条 INFO（其中 lastCause 混合结果一条与 Spec 镜重合）。处置： + - lastCause 分类按最终尝试结果（R4 共同项）：成功退出重试循环时 `lastCause = undefined`，杜绝「早期瞬态失败 + 后续成功 no-op」被误判为耗尽。→ 已修复。 + - pause 文案「judge 期间会话状态变化」在 GOAL-01 gate-hit 路径失准：改为中性「会话状态变化（X），目标已暂停」（既有测试只断言 contains「状态变化」，不受影响）。→ 已修复。 + - noop 回归测试未真正钉住（Goal.pause 本身清 mark，前置 mark 到不了 pauseForUserCancel）：重写为 pause 之后重新 markTurnDriven 造真实陈旧 mark，并断言 logLines 不含 "failed after retries"（旧代码必触发该日志 → 测试真正翻红可验证）。→ 已修复。 + - 两处注释（Interface doc + GOAL-TURN-SCOPE 块）与第三分支（no-op 静默清 mark）矛盾：已改写一致。 + - GOAL-04 重试环的 per-session catchCause 缺 interrupt 抑制（与外层 scan handler 不一致）：两处 per-session catchCause（首轮 + 重试环）均加 `Cause.hasInterrupts` F1 抑制。→ 已修复。 +- 结论：非干净轮。修复后进入 Round 5。 + +### Round 5 - 未开始 diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index 60483419b..ea46ddf37 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -167,7 +167,9 @@ export interface Interface { * GOAL-02: when the pause exhausts its retries, durable row and lease * both still say "active" — the turn mark is RETAINED so the three * authorities agree and a repeat ESC retries the pause. The mark is - * cleared only on a successfully persisted pause. + * cleared on a successfully persisted pause; a successful NO-OP (goal + * already paused/cleared when ESC lands) silently retires the stale mark + * — no durable authority claims the goal as active. */ readonly pauseForUserCancel: (sessionID: SessionID, reason: string) => Effect.Effect /** True when the session's current turn is goal-driven. */ @@ -211,11 +213,12 @@ const serviceLayer = Layer.effect( // turn. Keyed by session; set at every goal dispatch (kick in prompt.ts, // continuation in loop.ts), cleared at turn end (afterIdle entry) and at // every terminal transition (pause/clear/markDone). On ESC-cancel the - // clear happens ONLY when the pause persisted — if the pause exhausts its - // retries the mark is RETAINED so it agrees with the still-active - // durable row and lease (GOAL-02). A stale mark is harmless: - // goalTurnMaxSteps re-validates against the durable goal row before - // reporting a ceiling. + // clear happens when the pause persisted OR the cancel is a successful + // no-op (goal already inactive — nothing claims it as active); only if + // the pause exhausts its retries is the mark RETAINED so it agrees with + // the still-active durable row and lease (GOAL-02). A stale mark is + // harmless: goalTurnMaxSteps re-validates against the durable goal row + // before reporting a ceiling. const turnDriven = new Set() const markTurnDriven = Effect.fnUntraced(function* (sessionID: SessionID) { @@ -263,6 +266,11 @@ const serviceLayer = Layer.effect( const exit = yield* pauseAndPublish(sessionID, reason).pipe(Effect.exit) if (Exit.isSuccess(exit)) { paused = exit.value + // Classify by the FINAL attempt: an early transient failure followed + // by a successful outcome (e.g. a concurrent pauser lands between + // retries) is a success/no-op, not retry exhaustion — drop the + // stale cause so the branches below read the real outcome. + lastCause = undefined break } lastCause = exit.cause diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index 06a11b36c..0f685486a 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -485,7 +485,11 @@ const serviceLayer = Layer.effect( // Previously this was a bare `return` that left the goal silently // "active" with no continuation. Pause with a visible reason so the // user knows the loop was interrupted by a status change. - const pauseMsg = `judge 期间会话状态变化（${currentStatus.type}），目标已暂停` + // Neutral wording on purpose: this pause is reachable both after a + // real judge call AND via the GOAL-01 gate-hit fall-through, where + // the judge was suppressed — the user-visible reason must not claim + // a judge was running. + const pauseMsg = `会话状态变化（${currentStatus.type}），目标已暂停` yield* pauseGoal(sessionID, pauseMsg).pipe(Effect.ignore) yield* promptSvc.prompt({ sessionID, noReply: true, parts: [{ type: "text", text: `⏸ 目标已暂停 — ${pauseMsg}` }] }).pipe(Effect.ignore) return @@ -716,10 +720,14 @@ const serviceLayer = Layer.effect( } yield* triggerEvaluation(sessionID, true).pipe( Effect.catchCause((cause) => - Effect.logWarning("goal startup scan failed for session", { - sessionID, - cause: Cause.pretty(cause), - }), + // F1 discipline (same as the outer scan handler): instance + // disposal interrupts these per-session effects silently. + Cause.hasInterrupts(cause) + ? Effect.void + : Effect.logWarning("goal startup scan failed for session", { + sessionID, + cause: Cause.pretty(cause), + }), ), ) } @@ -746,10 +754,12 @@ const serviceLayer = Layer.effect( } yield* triggerEvaluation(sessionID, true).pipe( Effect.catchCause((cause) => - Effect.logWarning("goal startup scan retry failed for session", { - sessionID, - cause: Cause.pretty(cause), - }), + Cause.hasInterrupts(cause) + ? Effect.void + : Effect.logWarning("goal startup scan retry failed for session", { + sessionID, + cause: Cause.pretty(cause), + }), ), ) } diff --git a/packages/opencode/test/goal/turn-scope.test.ts b/packages/opencode/test/goal/turn-scope.test.ts index 9cafded3b..184b0dd53 100644 --- a/packages/opencode/test/goal/turn-scope.test.ts +++ b/packages/opencode/test/goal/turn-scope.test.ts @@ -9,6 +9,7 @@ import { SessionStatus } from "@/session/status" import { Database } from "@opencode-ai/core/database/database" import { GoalStateTable } from "@opencode-ai/core/goal/sql" import { SessionID } from "@/session/schema" +import { logLines } from "effect/testing/TestConsole" import { testEffect } from "../lib/effect" // GOAL-TURN-SCOPE regression tests: the turn-provenance mark (kick / @@ -146,17 +147,21 @@ describe("Goal turn-scope — pauseForUserCancel (ESC semantics)", () => { }), ) - // Review R3-INFO-1: a successful NO-OP (goal already paused/cleared when ESC - // lands) must not be reported as a retry-exhaustion failure — no durable - // authority claims the goal as active, so the stale mark is retired - // silently. - it.live("cancel on an already-paused goal is a silent no-op that retires a stale mark", () => + // Review R3-INFO-1: a successful NO-OP (goal already paused/cleared when + // ESC lands) must not be reported as a retry-exhaustion failure — no + // durable authority claims the goal as active. Recreates the genuinely + // stale mark the way an auto-pause leaves it behind (updateAfterJudge + // pauses the row WITHOUT clearing the turn mark; loop.ts clears it at the + // next afterIdle entry), then asserts pauseForUserCancel retires it + // silently. Pre-fix this window logged a false "failed after retries" + // ERROR. + it.instance("cancel on an already-paused goal is a silent no-op that retires a stale mark", () => Effect.gen(function* () { const goal = yield* Goal.Service const sid = SessionID.descending() yield* goal.set(sid, "test goal", 5) - yield* goal.markTurnDriven(sid) yield* goal.pause(sid, "auto-paused") + yield* goal.markTurnDriven(sid) const paused = yield* goal.pauseForUserCancel(sid, "用户中断（ESC）") expect(paused).toBeUndefined() @@ -164,6 +169,7 @@ describe("Goal turn-scope — pauseForUserCancel (ESC semantics)", () => { const state = yield* goal.load(sid) expect(state?.status).toBe("paused") expect(state?.paused_reason).toBe("auto-paused") + expect(JSON.stringify(yield* logLines)).not.toContain("failed after retries") }), ) From 58b56b490ff6eac175068a8c92c034265dcff923 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 15:25:12 +0800 Subject: [PATCH 10/13] docs(goal): sync D-4/status-branch/freshMsgs comments with the GOAL-01 judge-less path --- docs/findings/goal-batch-findings.md | 7 +++++++ packages/opencode/src/goal/loop.ts | 18 ++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index 0a0ea85ec..3c0d7dd2e 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -57,4 +57,11 @@ - 结论：非干净轮。修复后进入 Round 5。 ### Round 5 +- Spec 镜：**PASS，no findings**（干净轮 1/2 候补——但 Standards 非干净，计数重置）。 +- Standards 镜：**PASS**，2 条 INFO（GOAL-01 judge-less 路径后遗留的陈旧注释）： + - R5-INFO-1：D-4 evaluatedRevisions 头注释仍称「仅由成功 updateAfterJudge commit 写入」，未含 gate-hit drive-restored 写入点。→ 已改写（并自查发现同根第 3 处：freshMsgs 的 "Reload messages after judge LLM call" 一并改为两可措辞）。 + - R5-INFO-2：branch-3 首行 "Session is no longer idle after the judge call" 对 gate-hit 路径失准。→ 已改写。 +- 结论：非干净轮。进入 Round 6。 + +### Round 6 - 未开始 diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index 0f685486a..fc8b8c704 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -229,11 +229,14 @@ const serviceLayer = Layer.effect( ) // D-4 (GOAL-FP-01-04 follow-up): per-process record of which goal - // revision this process already evaluated. Written by afterIdle on every - // successful updateAfterJudge commit; consulted ONLY by the startup-scan + // revision this process already evaluated. Written by afterIdle at TWO + // sites: every successful updateAfterJudge commit, and the GOAL-01 + // boundary-gate hit (where the drive is restored WITHOUT a commit — the + // map then marks the revision as drive-restored, so a duplicate scan + // trigger on the same revision skips). Consulted ONLY by the startup-scan // path (scanResume) — the idle path must keep re-evaluating the same // revision across new turn boundaries, so the gate never applies to it. - // Lifecycle mirrors the fibers map: overwritten by every commit, deleted + // Lifecycle mirrors the fibers map: overwritten by every write, deleted // at the same terminal points where afterIdle unregisters the goal // automation. const evaluatedRevisions = new Map() @@ -481,7 +484,9 @@ const serviceLayer = Layer.effect( const currentStatus = yield* status.get(sessionID) if (currentStatus.type !== "idle") { - // Session is no longer idle after the judge call (5-30s latency). + // Session is no longer idle by the time dispatch resumes — it flipped + // during the judge call (5-30s latency), or between the gate and here + // on the GOAL-01 judge-less fall-through. // Previously this was a bare `return` that left the goal silently // "active" with no continuation. Pause with a visible reason so the // user knows the loop was interrupted by a status change. @@ -495,8 +500,9 @@ const serviceLayer = Layer.effect( return } - // Reload messages after judge LLM call — the snapshot from before judge - // may be stale if user sent messages during the 5-30s judge latency. + // Reload messages before dispatch — the pre-judge snapshot may be stale + // (the user can send messages during the 5-30s judge latency, or during + // the GOAL-01 judge-less fall-through). // Same vanished-session tolerance as the pre-judge window: NotFoundError // becomes an empty window (shouldPreempt is defensively false for it), // never a typed failure escaping the fork. From 5e6ab11fe1b6450bb532738e7585fe5be8c77e37 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 15:47:16 +0800 Subject: [PATCH 11/13] docs(goal): declare module convergence after two consecutive clean review rounds --- docs/findings/goal-batch-findings.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index 3c0d7dd2e..3626ca6a3 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -64,4 +64,15 @@ - 结论：非干净轮。进入 Round 6。 ### Round 6 -- 未开始 +- Spec 镜：**PASS，no findings**。 +- Standards 镜：**PASS，no findings**（含注释真实性、Effect 习语、测试纪律、CONTEXT.md 不变量的全量复核）。 +- 结论：**干净轮 1/2**。进入 Round 7；若再干净 → 连续两轮零 findings，模块收敛。 + +### Round 7 +- Spec 镜：**PASS，no findings**（独立复核 GOAL-01..04 修复 + 测试义务 + 验证为正确部分）。 +- Standards 镜：**PASS，no findings**（Effect 习语/风格/CONTEXT.md 不变量/测试纪律/注释真实性全量复核）。 +- 结论：**干净轮 2/2**。连续两轮零 findings → **GOAL 模块收敛**。 + +## 收敛结论 + +R1 有 2 Low → 修复；R2 干净（因 R3 有 findings 计数重置）；R3 有 3 INFO → 修复；R4 有 5 INFO → 修复；R5 有 2 INFO → 修复；**R6+R7 连续两轮双镜零 findings**。全部 findings 已关闭，模块具备发 PR 资格。 From a56ed05214165c8dee800586cc1ed6f0a660200e Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 15:58:12 +0800 Subject: [PATCH 12/13] docs(goal): record PR #334 delivery and closure of the goal run --- docs/findings/goal-batch-findings.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index 3626ca6a3..ab4d2fcc2 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -76,3 +76,14 @@ ## 收敛结论 R1 有 2 Low → 修复；R2 干净（因 R3 有 findings 计数重置）；R3 有 3 INFO → 修复；R4 有 5 INFO → 修复；R5 有 2 INFO → 修复；**R6+R7 连续两轮双镜零 findings**。全部 findings 已关闭，模块具备发 PR 资格。 + +## 交付 + +- **PR**：https://github.com/LeXwDeX/OpenCode-GraphAgent/pull/334 → `dev`（门禁 Typecheck；CI run 32113882464 进行中） +- 提交链：bcad76ebf（audit 文档）→ ed7185a0f（GOAL-01）→ 551b8f78a（GOAL-02）→ 9fc67e8e7（GOAL-03）→ f0e727865（GOAL-04）→ 429e58815 / 5dd5a3037 / ce87f84bd / db44487c7 / 58b56b490 / 5e6ab11fe（审阅轮修复与记账） +- 终态门禁：goal 测试簇 108/108 绿；`bun typecheck`（packages/opencode）绿；全量 4142 tests 除 3 项基线既有 darwin 环境性失败外全绿（已在干净基线 detached 复跑证实非本批引入）。 +- 已知本地环境既有问题（与本批无关，已证实）：根 turbo typecheck 的 `@opencode-ai/app` 子路径解析、project-copy / help-snapshots / pty 三个测试。 + +## 下一 run + +DAG 批次（DAG-01..04）：事件触发 = 本 PR 合入 dev 后从新基线切 `fix/dag-batch`。 From 799ea0529cc13def21d2ce088e2928738f4427dc Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 18 Aug 2026 15:59:48 +0800 Subject: [PATCH 13/13] docs(goal): correct turbo typecheck status (pre-push hook green) --- docs/findings/goal-batch-findings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md index ab4d2fcc2..3863b86a4 100644 --- a/docs/findings/goal-batch-findings.md +++ b/docs/findings/goal-batch-findings.md @@ -82,7 +82,7 @@ R1 有 2 Low → 修复；R2 干净（因 R3 有 findings 计数重置）；R3 - **PR**：https://github.com/LeXwDeX/OpenCode-GraphAgent/pull/334 → `dev`（门禁 Typecheck；CI run 32113882464 进行中） - 提交链：bcad76ebf（audit 文档）→ ed7185a0f（GOAL-01）→ 551b8f78a（GOAL-02）→ 9fc67e8e7（GOAL-03）→ f0e727865（GOAL-04）→ 429e58815 / 5dd5a3037 / ce87f84bd / db44487c7 / 58b56b490 / 5e6ab11fe（审阅轮修复与记账） - 终态门禁：goal 测试簇 108/108 绿；`bun typecheck`（packages/opencode）绿；全量 4142 tests 除 3 项基线既有 darwin 环境性失败外全绿（已在干净基线 detached 复跑证实非本批引入）。 -- 已知本地环境既有问题（与本批无关，已证实）：根 turbo typecheck 的 `@opencode-ai/app` 子路径解析、project-copy / help-snapshots / pty 三个测试。 +- 已知本地环境既有失败（与本批无关，已在干净基线 detached 复跑证实）：全量测试中 project-copy / help-snapshots / pty 三项（darwin 环境/计时性）。根 turbo typecheck 曾一次命中 `@opencode-ai/app` 的瞬时缓存失败，随后（pre-push 钩子）29/29 全绿自愈。 ## 下一 run