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 0000000000..2762d3a453 --- /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。 | diff --git a/docs/findings/dag-batch-findings.md b/docs/findings/dag-batch-findings.md new file mode 100644 index 0000000000..5321aa242f --- /dev/null +++ b/docs/findings/dag-batch-findings.md @@ -0,0 +1,87 @@ +# DAG 批次 Findings Register + +- 验收 primary source：`docs/audit-dag-memory-goal-2026-08-18.md`（DAG 章节） +- 分支：`fix/dag-batch` → PR `dev` +- 收敛判据：连续两轮独立审阅（Spec 镜 + Standards 镜）零 findings + 模块门禁全绿 +- 规格：`workflows/audit-fix-loop.md` +- 触发：用户指示在 GOAL run（PR #334）之后立即开工，不等合入 + +## 审计缺陷切片（输入项，非审阅 finding） + +| ID | 严重性 | 切片顺序 | 状态 | 提交 | +|---|---|---|---|---| +| DAG-01 + DAG-02 | High | A（P0，审计明确要求一并修） | 完成（红-绿-变异×3 通过） | 71ab1bdf6 | +| DAG-03 | Medium | B (P1) | 完成（红-绿-变异×2 通过） | 1c4f1ad7a | +| DAG-04 | Medium | C (P1，#316 机制部分；触发源不追查，按审计记录缺口） | 完成（红-绿-变异×2 通过） | db626d4ba | + +## 切片设计要点（实现后回填） + +- **A（DAG-01+02）**： + - 运行时：spawnReady 条件求值前对字符串依赖输出做 `parseJsonOption` 归一化（与 replan-verdict 门同源）；非 JSON 回退原串（整串等值可用、字段路径仍 false、数值比较仍 loudly-fail）。 + - authoring：`checkpointGateDiagnostics` 追加「被门控 checkpoint 必须声明 output_schema」（authoring 期错误；运行时路径不要求——runtime-created 图按 CONTEXT.md 有意豁免 authoring 校验，`requireOutputSchema:false`）。 + - 门禁接线：`validatePostCompile` 的 checkpoint 门不再随 `structural:false` 对 replan/extend 关闭（fragment 内对生效）；`replanStructuralDiagnostics` 对 merged 图补跑 checkpoint 门（覆盖 fragment 挂到既有 checkpoint 的场景），**豁免持久图中已终态的 checkpoint**（裁决已交付，加波/重开是受 sanction 的模式——reopenDenial 加性重开的既有语义）。 + - 波及适配 2 个既有 harness（blanket `report_to_parent:true` 的 rev-view / stale-nodefailed 形状按门禁语义补 condition）；dag-wake-integration 的加波/重开场景经终态豁免自然兼容，无需改动。 +- **B（DAG-03）**：pause 终态失败 fail-closed（恒 hold），`Effect.catch` → `Effect.catchCause`（hasInterrupts 再抛）折叠 defect；logWarning → logError（含 durableStatus）。 +- **C（DAG-04）**：publisher 外层 catchCause 依 F1 模式 `hasInterrupts` 再抛；`disposeAllInstancesAndEmitGlobalDisposed` 加 10s 有界超时（`timeoutOption`——超时即放弃且不产生错误，保住 HttpApi dispose endpoint 的 `never` 错误通道），去掉 uninterruptible 包裹；Disposed 事件在超时/吞错后仍必落地；真实处置失败在非 swallow 路径仍传播。 + - 中断测试注入方式：scope-disposal 杀 fiber 的 cause 实测为 Die 而非 Interrupt（已实证），改用 `Effect.failCause(Cause.interrupt(0))` 在 store 边界直接注入 interrupt cause（goal e2e 既有模式），精确命中被修复的 catchCause 判别线。 + +## 模块门禁 +- 切片级：每切片 dag 目标测试簇绿 + 包内 typecheck 绿 + 变异翻红验证（见上表） +- 全量套件：进行中 + +## 审阅轮次 + +（每轮审阅结果记账于此；全部关闭后才具备发 PR 资格） + +### Round 1 +- Spec 镜：**PASS**，3 条 Low INFO；Standards 镜：**PASS**，6 条 findings（F1-F6）。处置： + - F1 + INFO-2（Medium）：ADR-0003 与新 enforcement 矛盾。→ **已同步**：Decision/Consequences/Deferred 改写为「validatePostCompile 全动作 + replanStructuralDiagnostics merged 图（终态豁免）+ create 刻意不动 + output_schema authoring 义务」，Deferred 首项标记 resolved。 + - F2 + INFO-1（Medium，需主裁决）：fail-closed 保持会被后续 NodeCompleted/NodeSkipped/stepped 的 durable-row re-sync 解除；且 Replanned 处理器从不重同步 paused（hold 也会闷死 corrective 派发）。用户裁决「解决所有已知问题」。→ **已实装**：`WorkflowEntry.vetoHold`——门设置、两处 re-sync 点（node 终态序言 + refreshControlFlags）尊重保持、三个父控制事件（Replanned/Resumed/Stepped）释放并重同步（Replanned 补上从未有过的 flag 重同步）；新增 2 条红绿测试（re-sync 存活 + replan 释放）+ 双向变异验证。 + - F3（Low）：global-lifecycle.test.ts 未用 `Option` 导入。→ 已删。 + - F4（Low）：loop.ts「normalization above」方向失准。→ 已改为指向 NodeCompleted 处理器。 + - F5（Low）：终态豁免措辞「delivered its verdict」对 failed/aborted/skipped 不真。→ 三处改为「settled and immutable」（CONTEXT.md/validation.ts×2）。 + - F6（Low, informational）：无 uninterruptible 的取舍记录。→ 无需动作（reviewer 确认 trade 正确）。 + - INFO-3（Low）：register 误记「3 个 harness 适配」。→ 已更正为 2（wake-integration 经终态豁免免改）。 +- 结论：非干净轮。修复后进入 Round 2。 + +### Round 2 +- Spec 镜：**PASS**，3 条 INFO；Standards 镜：**PASS**，1 条 INFO。处置： + - R2-1（INFO）：vetoHold 注释宣称「replan/resume/step 均可释放」不精确——hold 态持久行是 running，resume 对 running 是非法迁移（InvalidTransitionError），resume 释放仅在 stepping/pending 态可达。→ 已改写字段注释：replan/step 任何 hold 态可达；resume 仅 stepping/pending；control(replan) 正是 verdict 所求的处置路径。 + - R2-2（INFO）：hold 为进程内状态，重启后从持久行重建（审计 DAG-03 的范围就是进程内 fail-open）。→ 已在字段注释记录该边界。 + - R2-3（INFO）：timeoutOption 的切断发生在第一个可中断点——uninterruptible finalizer 区域内的 wedge 可越过上限（硬切断需 Effect.disconnect，刻意不取，与 exerciser 的 Promise.race 同残差）。→ 已在 global-lifecycle 注释精确化。 + - Standards INFO-1：dag-loop-guards.test.ts 未用的 `probe` 变量。→ 已删。 +- 结论：非干净轮（4 INFO）。修复后进入 Round 3。 + +### Round 3 +- Spec 镜：**PASS，no findings**（干净轮候选）。 +- Standards 镜：**PASS**，1 条 INFO：R3-1——vetoHold 注释的 resume 枚举「stepping/pending」不完整：hold 之后父层仍可先持久 pause 再 resume（该路径释放有效），且 pending 对已启动工作流不可达。→ 已改为「resume only when the durable row is not running (paused/stepping)」并说明直至持久 pause 落地。 +- 结论：非干净轮。修复后进入 Round 4。 + +### Round 4 +- Spec 镜：**PASS，no findings**。 +- Standards 镜：**PASS**，1 条 INFO：R4-1——ungated 诊断的 hint 把「declare output_schema」列为独立替代项，但单独声明 schema 不能解除 ungated 错误（须与 gating 条件组合）。→ 已改写为「Gate … with condition … and declare output_schema …」组合句式。 +- 结论：非干净轮。修复后进入 Round 5。 + +### Round 5 +- Spec 镜：**PASS，no findings**；Standards 镜：**PASS，no findings**（R4 hint 修复逐条复核为真）。 +- 结论：**干净轮 1/2**。 + +### Round 6 +- Spec 镜：**PASS，no findings**（四缺陷 + F2 + 文档同步独立复核；附 process note：PR 前回填 R5 结果——本条即回填）。 +- Standards 镜：**PASS，no findings**（Effect v4 API 对照 effect-smol 源码逐一验证；注释真实性对照 transition table/replan.ts/spawn.ts 复核）。 +- 结论：**干净轮 2/2**。连续两轮零 findings → **DAG 模块收敛**。 + +## 收敛结论 + +R1（Spec 3 Low + Standards 6）→ R2（Spec 3 + Standards 1）→ R3（Spec 0 + Standards 1）→ R4（Spec 0 + Standards 1）→ **R5+R6 连续两轮双镜零 findings**。全部 findings 关闭，findings 衰减轨迹清晰（Medium 实装 → Low 措辞 → 零）。 + +## 模块门禁（终态） + +- 切片级：dag 测试簇绿（579/579 含新增 19 条回归）+ 包内 `bun typecheck` 绿 + 每切片变异翻红验证。 +- 全量套件：4157 tests / 342 files，仅 2 失败均为 GOAL run 期间已在干净基线 detached 复跑证实的 darwin 环境既有失败（help-snapshots、project-copy；pty 本轮通过），与本批无因果；DAG/GOAL 相关零失败。 + +## 交付 + +- 分支：`fix/dag-batch`（基于 origin/dev 1d087ffe9） +- 提交链：43fd72bbd（audit 文档）→ 71ab1bdf6（DAG-01/02）→ 1c4f1ad7a（DAG-03）→ db626d4ba（DAG-04）→ 337814648 / ff1f8dab5 / 85df04311 / 5128f0958（审阅轮修复） +- PR → dev（Typecheck 门禁），合入由用户授权执行 diff --git a/docs/findings/goal-batch-findings.md b/docs/findings/goal-batch-findings.md new file mode 100644 index 0000000000..3863b86a4e --- /dev/null +++ b/docs/findings/goal-batch-findings.md @@ -0,0 +1,89 @@ +# 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) | 完成（红-绿-变异通过） | 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 均通过 +- 全量测试套件（`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 +- 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 +- 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 +- 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 +- 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 资格。 + +## 交付 + +- **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 复跑证实非本批引入）。 +- 已知本地环境既有失败（与本批无关，已在干净基线 detached 复跑证实）：全量测试中 project-copy / help-snapshots / pty 三项（darwin 环境/计时性）。根 turbo typecheck 曾一次命中 `@opencode-ai/app` 的瞬时缓存失败，随后（pre-push 钩子）29/29 全绿自愈。 + +## 下一 run + +DAG 批次（DAG-01..04）：事件触发 = 本 PR 合入 dev 后从新基线切 `fix/dag-batch`。 diff --git a/docs/findings/memory-batch-findings.md b/docs/findings/memory-batch-findings.md new file mode 100644 index 0000000000..426b1beb01 --- /dev/null +++ b/docs/findings/memory-batch-findings.md @@ -0,0 +1,79 @@ +# MEMORY 批次（验收遗留）Findings Register + +- 验收 primary source：`docs/audit-dag-memory-goal-2026-08-18.md`（MEMORY 章节）+ 产品→代码验收（f1c2c8c33→11cfafe9c）Spec 轴三项遗留 +- 分支：`fix/memory-fence-scope` → PR `dev` +- 收敛判据：连续两轮独立审阅（Spec 镜 + Standards 镜）零 findings + 模块门禁全绿 +- 规格：`workflows/audit-fix-loop.md` + +## 处置项 + +| ID | 来源 | 处置 | 状态 | 提交 | +|---|---|---|---|---| +| MEM-01 后半（P1） | 审计 + 验收 (a)2 | prepareUnsafe `shouldMatch` 分支的 select matcher 移出 fence/lock：matcher 无锁跑，markMatched 经 `applyUpdate`（fence+lock 只包提交） | 完成（红-绿-变异通过） | 70291fbe4 | +| MEM-02（P2） | 审计 + 验收 (a)1 | `search` 的 identity fence 缩到 markMatched 提交；同查询合并从「持锁阻塞后来者」改为 turn 内 per-(session,turn,key) in-flight coalescing（进程内 Deferred），语义等价（后来者 reused:true、不耗 slot；失败/中断/retired 时唤醒降级而非挂起）且不再跨模型调用持 fence/lock | 完成（红-绿-变异通过；旧 coalescing 回归保持绿） | 70291fbe4 + f08548d6d + ef8d6b8d1 | +| MEM-03（P3） | 验收 (a)3 | 处置记录：已被 PR #333 的 MEM-01 重构结构性抵消——维护恒为后台（kickMaintenance），失败折入其 catchCause，维护前渲染是已声明设计（CONTEXT.md「render the pre-maintenance snapshot」）。旧的「维护前快照渲染注入」窗口随 inline maintain 一起消失。本行即处置记录。 | 已记录 | — | + +## 设计要点 + +- `applyUpdate` 泛型化：`Update` 透传结果，select 的 markMatched 提交经它返回 matched topics（供 render）。 +- `select` 拆两半：`match()`（无 fence/lock）+ markMatched 提交（`applyUpdate`）。 +- `search`：短临界区（进程内 lock）只做缓存 re-read/limit/queryCount++ 与 in-flight 登记；matcher 在任何 fence/lock 之外；同 key 后来者 await in-flight Deferred（进程内合并，语义与旧「锁内阻塞」等价）；提交走 `applyUpdate`。 +- `prepareUnsafe` shouldMatch 分支：matcher 出 fence/lock；identity retired（applyUpdate 返回 None）时仍 clearSession（fail-closed 不变）。 +- CONTEXT.md 最后一条 invariant 改写：fence 只包提交；同查询合并显式声明为进程内 in-flight coalescing。 +- 锁序不变量：全程不出现 lock 内嵌 fence（KeyedMutex 不可重入 + 与 checkpoint 的 fence>lock 序相反会死锁）。 + +## 模块门禁 +- 未开始 + +## 审阅轮次 + +### Round 1 +- Spec 镜：**BLOCKING**（R-1 P1 + R-2/R-3 INFO）；Standards 镜：**BLOCKING**（F1 P1 与 R-1 同源 + F2-F5 INFO）。处置： + - R-1/F1（P1）：in-flight 泄漏——失败/中断/retired 路径 deferred 永不完成 → coalesced awaiter 永久挂起、(session,key) 进程级 wedge。→ **已修**：runner 分支 `Effect.onExit`（对齐 kickMaintenance 槽位纪律）——每个退出路径先 `releaseIfOwner` 再把真实 Exit 打包进 deferred（deferred 永不失败，Exit 载荷即全部消息）；awaiter 按 Exit 分支：失败→`failed`、interrupt→failCause 传播、retired→`unavailable`。新增红绿测试「a failed first query never wedges the session key or its coalesced awaiter」（有界等待断言无挂起、无 wedge）+ 变异验证（去掉 onExit → 新测试与旧 coalescing 测试双红）。 + - R-2（INFO）：dereg→缓存写入窗口内第三个同查询会多耗一次调用。→ 已修：缓存写入提前到 `Effect.tap`（releaseIfOwner 之前），窗口闭合。 + - R-3（INFO）：等价声明只覆盖 happy path。→ CONTEXT.md 措辞已含失败路径降级语义。 + - F2（INFO）：CONTEXT.md「outside every fence/lock」对短注册临界区不真。→ 已改为「outside every fence + SHORT project-lock critical section (registration)」。 + - F3/F4/F5（INFO）：裸块、`!` 断言、命名不一致。→ 已修（裸块展开、get-then-check、统一 `selected`）。 +- 结论：修复后进入 Round 2。 + +### Round 2 +- Spec 镜：**BLOCKING**（Issue 1 P1 + Issue 2 INFO）；Standards 镜：**BLOCKING**（Issue 1 P1 同源 + 2/3/4 INFO）。处置： + - Issue 1（P1）：onExit 括号只覆盖 select 管道——注册后的 `store.readTopics` 挂起（生产为 flock 磁盘 IO）期间被中断/失败会在括号附着前 unwind → 泄漏条目 + wedge。→ **已修**：整个尾部（readTopics + select 管道）包进同一 `Effect.gen(...).pipe(tap, onExit, exit)` 括号；mock 的 `readTopics` 加 `parkReads` 挂起钩子；新增红绿测试「an interrupted topics read releases the in-flight entry and never wedges the key」+ 变异验证（readTopics 挪出括号 → 翻红）。 + - Spec Issue 2（INFO）：in-flight key 无 turn 分量 → 新轮次的同文查询可能骑上一轮的 deferred，attached 但自身缓存不填充。→ **已修**：key 加入 turn origin（messageID）——合并严格 turn 内，跨轮重跑。 + - Standards 2（INFO）：awaiter 注释的 interrupt 机制描述不准（failCause 重抛在 awaiter，failed 映射在 search wrapper）。→ 已改写。 + - Standards 3（INFO）：测试注释宣称 interrupted 覆盖但原先无此测试。→ R2 新增的中断测试已补齐该覆盖。 + - Standards 4（INFO）：`undefined as void` 多余 cast。→ 已删。 +- 结论：修复后进入 Round 3。 + +### Round 3 +- Spec 镜：**PASS**，1 条 INFO；Standards 镜：**PASS**，2 条 INFO。处置： + - R3-1（INFO）：CONTEXT.md 与 searchUnsafe 注释仍写 per-(session,key)，key 已 turn-scoped。→ 两处已改为 per-(session,turn,key)。 + - R3-2（INFO）：awaiter 注释「shares the cancellation」不符合 v4 语义（failCause 重抛由 wrapper catchCause 吸收，awaiter 仍以 failed 完成）。→ 已改写为准确机制。 + - register「待提交」歧义。→ 已改为实际提交哈希。 +- 结论：非干净轮（INFO）。修复后进入 Round 4。 + +### Round 4 +- Spec 镜：**PASS，no findings**（干净轮候选）。 +- Standards 镜：**PASS**，2 条 INFO：R4-1 awaiter 注释把非中断失败的映射错归 wrapper catchCause（实际直接返回 failed）；R4-2 测试注释「flock'd disk I/O」不准（readTopics 无 flock，是 async fs）。→ 均已改写。 +- 结论：非干净轮。修复后进入 Round 5。 + +### Round 5 +- Spec 镜：**PASS，no findings**；Standards 镜：**PASS，no findings**（R4 两处措辞独立复核为准确）。 +- 结论：**干净轮 2/2**。连续两轮零 findings → **MEMORY 验收遗留批次收敛**。 + +## 收敛结论 + +R1（双镜 BLOCKING：in-flight 泄漏 P1）→ R2（双镜 BLOCKING：括号窗口 P1 + turn 作用域）→ R3（双 PASS，3 INFO 措辞）→ R4（Spec 干净 + Standards 2 INFO 措辞）→ **R4+R5 连续两轮零 findings**。findings 轨迹：两轮 P1 并发缺陷（实装修复+回归测试）→ 纯措辞 → 零。 + +## 模块门禁（终态） + +- memory 测试簇 95/95 绿（新增 5 条回归：fence-free ×2、failed-wedge、interrupted-read-wedge、原 coalescing 保持） +- `bun typecheck` + pre-push turbo 29/29 绿 +- 全量套件 4164 tests：仅 2 失败为历次批次已在干净基线证实的 darwin 环境既有失败（help-snapshots、project-copy），与本批无因果 +- 变异验证：fence 回置翻红 ×2（search/prepare）、onExit 移除翻红 ×2（wedge + coalescing）、readTopics 出括号翻红 ×1 + +## 交付 + +- 分支：`fix/memory-fence-scope`（基于 origin/dev 11cfafe9c） +- 提交链：70291fbe4（MEM-01/02 主体）→ f08548d6d（R1 exit-safe）→ ef8d6b8d1（R2 整尾括号 + turn key）→ 9094abcaa/04385af79/0c1919465（记账与措辞） +- PR → dev（Typecheck 门禁） diff --git a/packages/opencode/src/dag/CONTEXT.md b/packages/opencode/src/dag/CONTEXT.md index 38ac3de76a..87f180fa95 100644 --- a/packages/opencode/src/dag/CONTEXT.md +++ b/packages/opencode/src/dag/CONTEXT.md @@ -32,7 +32,7 @@ Workflow Orchestration turns one user objective into one durable DAG. Its model- - Model-facing graph actions expose only `spec_path`; graph fields live in YAML so provider tool-call serialization cannot turn a nested graph into a string. - Legacy YAML may be adapted at the file boundary without making legacy fields valid inline input. - Runtime Admission and Workflow Authoring Check have separate names, state, and responsibilities. -- Dependents of a reporting checkpoint must be gated on its output; authoring rejects ungated shapes at start/validate (enforcement point: authoring boundary only, runtime create deliberately unchanged). +- Dependents of a reporting checkpoint must be gated on its output; authoring rejects ungated shapes at start/validate AND at replan/extend fragment actions, and the runtime replan/extend mutation seam re-checks the merged graph (exempting checkpoints already terminal in the durable graph — they are settled and immutable, the spawn-before-verdict race is past; runtime create remains deliberately unchanged). A gated checkpoint must declare `output_schema` (authoring obligation). ## Boundaries diff --git a/packages/opencode/src/dag/blocks.ts b/packages/opencode/src/dag/blocks.ts index c63cd33f7b..42be90b106 100644 --- a/packages/opencode/src/dag/blocks.ts +++ b/packages/opencode/src/dag/blocks.ts @@ -327,6 +327,14 @@ function node(input: { outputSchema?: Record }): NodeConfig { const instruction = input.instruction?.trim() ? "Block-specific instruction:\n{{instruction}}" : "" + // issue #323: a reporting checkpoint adjudicates a direction, so its + // prompt must demand adversarial independent verification. The production + // incident: a gate confirmed parent-supplied "defect evidence" that was a + // production no-op because it re-read the parent's narrative instead of + // the source. Upstream claims are hypotheses, not facts. + const adversarial = input.reportToParent + ? "As a reporting checkpoint you adjudicate this direction: treat upstream and parent-supplied claims as hypotheses to verify, never facts to confirm. Independently read the relevant source before endorsing, spot-check at least three of the most load-bearing claims (name the count you actually inspected), cite what you actually inspected, and replan or reject the direction when a load-bearing claim does not survive inspection." + : "" return { id: input.id, name: input.name, @@ -339,6 +347,7 @@ function node(input: { "Workflow objective:\n{{objective}}", instruction, input.contract, + adversarial, "Use dependency outputs as evidence and return a concise artifact that downstream blocks can consume. Do not ask the user questions from this child session.", ] .filter(Boolean) diff --git a/packages/opencode/src/dag/dag.ts b/packages/opencode/src/dag/dag.ts index 28185c5c3c..75263d79b7 100644 --- a/packages/opencode/src/dag/dag.ts +++ b/packages/opencode/src/dag/dag.ts @@ -583,6 +583,9 @@ export const layer = Layer.effect( addCount: plan.add.length, merged: wfConfig ? computeMergedConfig(wfConfig, normalizedFragment, plan) : { nodes: normalizedFragment.nodes }, config: { mode: wfConfig?.mode, max_total_nodes: wfConfig?.max_total_nodes }, + terminalNodeIds: new Set( + nodes.filter((n) => isNodeTerminalStatus(n.status as NodeStatus)).map((n) => n.id), + ), }) const replanErrors = DagValidation.sortLegacyStructural(replanDiagnostics.filter((d) => d.severity === "error")) for (const warning of replanDiagnostics.filter((d) => d.severity === "warning")) { diff --git a/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md b/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md index 41b1e50b6d..e006873489 100644 --- a/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md +++ b/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md @@ -27,24 +27,44 @@ checkpoint must be a reporting leaf, or the node must drop `report_to_parent`. `node_defaults.report_to_parent` is honored: a node inheriting the default reports the same way. -Enforcement lives in `checkpointGateDiagnostics`, wired only into -`validatePostCompile`'s structural branch — the authoring start/validate path. -Every ungated dependent emits one error-severity `dag.invalid` diagnostic in -both `portable` and `environment` profiles, so `start` and `validate` reject -the shape before any durable graph exists. - -Enforcement is authoring-only by design. `Dag.create` and the replan/extend -fragment paths stay untouched: the verdict vocabulary is open, the ACCEPT path -must not wait for the parent, and runtime enforcement would change the -semantics of every existing graph, including issue #294's wake-chain and -reopen-extend behavior. +Enforcement lives in `checkpointGateDiagnostics`. It is wired into +`validatePostCompile` for every action — `start`/`validate` AND the +replan/extend fragment authoring paths (DAG-02, 2026-08-18: pre-fix it hid +behind the `structural === start-only` branch, so a fragment could attach an +ungated dependent to a reporting checkpoint and the engine spawned it before +the parent read the verdict) — and into `replanStructuralDiagnostics`, which +re-checks the MERGED graph at the runtime replan/extend mutation seam, so a +fragment dependent on an EXISTING checkpoint cannot escape either. Every +ungated dependent emits one error-severity `dag.invalid` diagnostic in both +`portable` and `environment` profiles. + +Two deliberate carve-outs: + +- Checkpoints already terminal in the durable graph are exempt at the runtime + merged-graph check: they are settled and immutable, the + spawn-before-verdict race is past, and the sanctioned additive/reopen waves + (`node("repair", ["checkpoint"])` after a completed reporting leaf) keep + working. +- A gated checkpoint must additionally declare `output_schema` (DAG-01, + authoring obligation): without a schema the child may complete with prose + that resolves no fields, leaving the gate permanently false and silently + skipping the gated subtree. The runtime path does not impose the schema + obligation (`requireOutputSchema: false`) because runtime-created graphs + deliberately bypass authoring validation. + +`Dag.create` itself stays untouched by design: the verdict vocabulary is +open, the ACCEPT path must not wait for the parent, and runtime create-level +enforcement would change the semantics of every existing graph, including +issue #294's wake-chain and reopen-extend behavior. ## Consequences -- Ungated reporting checkpoints fail fast at start/validate with a diagnostic - naming the checkpoint, the dependent, and the three legal fixes. +- Ungated reporting checkpoints fail fast at start/validate — and since + DAG-02 also at replan/extend fragment authoring and at the runtime + replan/extend mutation seam — with a diagnostic naming the checkpoint, the + dependent, and the legal fixes. - Runtime create, wake chains, and reopen-extend semantics are unchanged; - trusted internal callers retain full runtime flexibility. + trusted internal callers retain full runtime flexibility at create time. - Saved and curated workflows were audited: 14 curated block workflows are unaffected; only `ultra-flow-route.yaml` and `release-route.yaml` trip the new check and are tracked in opencode-dag-config#14. @@ -62,9 +82,8 @@ reopen-extend behavior. ## Deferred -- Replan/extend fragments are not checkpoint-gate-checked (coverage gap; no - date). Runtime flexibility was prioritized; fragment authoring remains - advisory. +- ~~Replan/extend fragments are not checkpoint-gate-checked~~ — resolved + 2026-08-18 (DAG-02): fragment authoring and the runtime merged-graph seam + both enforce the gate, with the terminal-checkpoint carve-out above. - Deprecation of advisory wake chains (no date): `report_to_parent` without - gated dependents stays legal but is a smell worth revisiting once fragment - coverage exists. + gated dependents stays legal but is a smell worth revisiting. diff --git a/packages/opencode/src/dag/runtime/loop.ts b/packages/opencode/src/dag/runtime/loop.ts index 26d1a7ae8b..e2c3f316bc 100644 --- a/packages/opencode/src/dag/runtime/loop.ts +++ b/packages/opencode/src/dag/runtime/loop.ts @@ -57,6 +57,18 @@ interface WorkflowEntry { config: WorkflowConfig | undefined fibers: Map> watchers: Map> + /** DAG-03/F2: a replan-verdict veto whose durable pause could not be + * persisted. While set, the in-memory paused flag survives the durable-row + * re-syncs performed by node terminal events and refreshControlFlags, so + * no stimulus spawns the vetoed direction before the parent acts. Released + * by the parent's explicit control events — replan and step are reachable + * from every hold state; resume only when the durable row is not running + * (paused/stepping — a held row reads "running" and resume is an invalid + * transition there until a durable pause lands; control(replan) is the + * disposition the verdict asked for). Process-local: a restart while the + * durable pause never landed rebuilds the flags from the durable row (the + * audit's DAG-03 scope was the in-process fail-open, which this closes). */ + vetoHold: boolean } const serviceLayer = Layer.effect( @@ -142,7 +154,25 @@ const serviceLayer = Layer.effect( const outputs: Record = {} for (const dep of node.dependsOn) { const depNode = nodesSnapshot.find((n) => n.id === dep) - if (depNode) outputs[dep] = { output: depNode.output } + if (!depNode) continue + // DAG-01: a schema-less checkpoint completes with a raw + // string output. Normalize JSON strings before condition + // evaluation so `.output.` gates read the parsed + // structure instead of resolving undefined — pre-fix the + // equality gate was permanently false, every gated dependent + // was skipped as condition_false, and checkCompletion still + // reported the workflow COMPLETED (silent half-graph loss). + // Mirrors the replan-verdict gate's parseJsonOption + // normalization in the NodeCompleted handler below (issue + // #322). Non-JSON strings fall back to the raw value: + // whole-output equality still works, field paths stay + // undefined (documented condition_false), and numeric + // comparisons keep their loud failure. + outputs[dep] = { + output: typeof depNode.output === "string" + ? Option.getOrElse(parseJsonOption(depNode.output), () => depNode.output) + : depNode.output, + } } const condResult = evaluateCondition(nodeConfig.condition, outputs) if (!condResult.ok) { @@ -439,7 +469,7 @@ const serviceLayer = Layer.effect( const isStepping = wf.status === "stepping" if (isPaused) runtime.setPaused(true) if (isStepping) runtime.setStepMode(true) - const entry: WorkflowEntry = { runtime, semaphore, evalLock: Semaphore.makeUnsafe(1), parentSessionID: wf.sessionId, config, fibers: new Map(), watchers: new Map() } + const entry: WorkflowEntry = { runtime, semaphore, evalLock: Semaphore.makeUnsafe(1), parentSessionID: wf.sessionId, config, fibers: new Map(), watchers: new Map(), vetoHold: false } // P2-E deletion-race re-check: the SessionV1.Event.Deleted sweep // only removes entries already published into `runtimes`. If the // FK cascade deleted this workflow's row while reconciliation ran, @@ -568,7 +598,7 @@ const serviceLayer = Layer.effect( const maxConcurrency = Math.max(1, config?.max_concurrency ?? Dag.DEFAULT_WORKFLOW_CONFIG.maxConcurrency) const runtime = new WorkflowRuntime(toSchedulingNodes(nodes), maxConcurrency) const semaphore = Semaphore.makeUnsafe(maxConcurrency) - const entry: WorkflowEntry = { runtime, semaphore, evalLock: Semaphore.makeUnsafe(1), parentSessionID: wf.sessionId, config, fibers: new Map(), watchers: new Map() } + const entry: WorkflowEntry = { runtime, semaphore, evalLock: Semaphore.makeUnsafe(1), parentSessionID: wf.sessionId, config, fibers: new Map(), watchers: new Map(), vetoHold: false } // P2-E deletion-race re-check (same window as recoverWorkflow): // the Deleted sweep only removes entries already in `runtimes`, // and getNodes above is an awaited yield a deletion can slip @@ -651,7 +681,9 @@ const serviceLayer = Layer.effect( yield* Effect.logDebug("DagLoop dropped stale node terminal event", { dagID, nodeID, expected, dbStatus: node?.status ?? "missing" }) } const workflow = yield* store.getWorkflow(dagID) - entry.runtime.setPaused(workflow?.status === "paused") + // F2: honor the veto hold — a durable "running" row must + // not lift the fail-closed pause a verdict gate set. + entry.runtime.setPaused(workflow?.status === "paused" || entry.vetoHold) entry.runtime.setStepMode(workflow?.status === "stepping") // Guard against stale events: a node already cancelled // (markUnsatisfied) or already satisfied must not be flipped @@ -679,20 +711,42 @@ const serviceLayer = Layer.effect( // corrective nodes — a paused workflow resumes as part // of replan (workflow tool) so corrections can run. const paused = yield* Effect.gen(function* () { - // Pause can fail transiently (e.g. the workflow lock is - // held by a concurrent long replan); retry once before - // falling back to the durable status, so the workflow - // is never silently stranded. + // DAG-03: the checkpoint VETOED this direction — the + // pause must fail CLOSED. Pause can fail transiently + // (e.g. the workflow lock is held by a concurrent + // long replan); retry once, and if it still cannot + // be persisted, HOLD the in-memory pause anyway. + // Pre-fix this returned `wf?.status === "paused"` — + // fail-OPEN: it explicitly un-paused the runtime, so + // the next stimulus calling spawnReady (a NodeFailed + // handler, a step, a resume) spawned the vetoed + // direction with no gate, no pause, no diagnostic. + // catchCause (not catch): a DEFECT from dag.pause + // must fold into the same path — pre-fix it escaped + // to guarded() and dropped this whole handler, so + // the pause was never even attempted and the gate's + // own warning was lost. Interrupts (scope disposal) + // still propagate. const attemptPause = dag.pause(dagID).pipe( Effect.map(() => true), - Effect.catch(() => Effect.succeed(false)), + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) ? Effect.failCause(cause) : 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" + if (wf?.status !== "paused") { + // F2: record the hold so the durable-row re-syncs + // below (node terminal prologues, refreshControlFlags) + // cannot lift it until the parent acts. + entry.vetoHold = true + yield* Effect.logError( + "DagLoop pause on replan verdict failed — holding in-memory pause (fail-closed)", + { dagID, nodeID, durableStatus: wf?.status ?? "missing" }, + ) + } + return true }) entry.runtime.setPaused(paused) yield* Effect.logWarning("DagLoop paused workflow after gate verdict: replan", { dagID, nodeID }) @@ -820,7 +874,8 @@ const serviceLayer = Layer.effect( const refreshControlFlags = Effect.fnUntraced(function* (dagID: string, entry: WorkflowEntry) { const workflow = yield* store.getWorkflow(dagID) if (!workflow || isWorkflowTerminalStatus(workflow.status as never)) return undefined - entry.runtime.setPaused(workflow.status === "paused") + // F2: honor the veto hold — see WorkflowEntry.vetoHold. + entry.runtime.setPaused(workflow.status === "paused" || entry.vetoHold) entry.runtime.setStepMode(workflow.status === "stepping") return workflow }) @@ -847,6 +902,9 @@ const serviceLayer = Layer.effect( if (!entry) return yield* entry.evalLock.withPermits(1)( Effect.gen(function* () { + // F2 (DAG-03): resume/step is the parent's explicit control — + // release the fail-closed veto hold before the flag re-sync. + entry.vetoHold = false const workflow = yield* refreshControlFlags(dagID, entry) if (workflow?.status !== "stepping") return // Dag.step validated "no in-flight node" on a DB snapshot @@ -873,6 +931,9 @@ const serviceLayer = Layer.effect( if (!entry) return yield* entry.evalLock.withPermits(1)( Effect.gen(function* () { + // F2 (DAG-03): resume/step is the parent's explicit control — + // release the fail-closed veto hold before the flag re-sync. + entry.vetoHold = false const workflow = yield* refreshControlFlags(dagID, entry) if (workflow?.status === "running") yield* spawnReady(dagID) // A workflow can be resumed with every node already settled @@ -900,6 +961,15 @@ const serviceLayer = Layer.effect( yield* entry.evalLock.withPermits(1)( Effect.gen(function* () { const wf = yield* store.getWorkflow(dagID).pipe(Effect.orDie) + // F2 (DAG-03): a replan is the parent's explicit disposition + // of the verdict — release the fail-closed veto hold and + // re-sync the control flags from the durable row (this + // handler never refreshed them, so a hold set by the verdict + // gate would otherwise silence the trailing spawnReady + // forever and the corrective nodes would never run). + entry.vetoHold = false + entry.runtime.setPaused(wf?.status === "paused") + entry.runtime.setStepMode(wf?.status === "stepping") const oldConfig = entry.config if (wf) entry.config = parseWorkflowConfig(wf.config) // Rev-view (v1.0.15 Train A): THE aggregation filter point. diff --git a/packages/opencode/src/dag/runtime/summary-publisher.ts b/packages/opencode/src/dag/runtime/summary-publisher.ts index 618d375762..7afd0fa1db 100644 --- a/packages/opencode/src/dag/runtime/summary-publisher.ts +++ b/packages/opencode/src/dag/runtime/summary-publisher.ts @@ -160,8 +160,16 @@ export const layer = Layer.effect( ) return Effect.void const dagID = data.dagID return schedulePublishByDag(dagID, evt.location.workspaceID).pipe( + // DAG-04 (#316): interrupt causes are the normal disposal path — + // the inner coalescer deliberately rethrows them (a scoped + // shutdown mid-publish must unwind, not masquerade as a failure). + // Swallowing them here logged a spurious "failed to publish" on + // every normal shutdown; rethrow per F1 discipline (same shape as + // spawn.ts / loop.ts). Real failures still warn. Effect.catchCause((cause) => - Effect.logWarning("DagSummaryPublisher: failed to publish summaries", { dagID, cause }), + Cause.hasInterrupts(cause) + ? Effect.failCause(cause) + : Effect.logWarning("DagSummaryPublisher: failed to publish summaries", { dagID, cause }), ), Effect.forkIn(scope), Effect.asVoid, diff --git a/packages/opencode/src/dag/runtime/supervision-sweep.ts b/packages/opencode/src/dag/runtime/supervision-sweep.ts new file mode 100644 index 0000000000..fbfbb0ad60 --- /dev/null +++ b/packages/opencode/src/dag/runtime/supervision-sweep.ts @@ -0,0 +1,259 @@ +// SPDX-FileCopyrightText: 2026 LeXwDeX +// SPDX-License-Identifier: AGPL-3.0-or-later + +export * as DagSupervisionSweep from "./supervision-sweep" + +import { Cause, Context, Effect, Fiber, Layer, Scope } from "effect" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { Database } from "@opencode-ai/core/database/database" +import { DagStore } from "@opencode-ai/core/dag/store" +import { WorkflowNodeTable } from "@opencode-ai/core/dag/sql" +import { and, eq, sql } from "drizzle-orm" +import { Dag, parseWorkflowConfig } from "@/dag/dag" +import { SessionPrompt } from "@/session/prompt" + +/** + * Host-level deadline-supervision sweep — the fallback retry for the + * production incident (2026-08-18, dag_fe5feabfcae607fqVdRh47lN1B): + * + * A per-directory instance teardown (lifecycle cleanup, directory switch, + * config change) silently reaps every fiber forked into its scope — the + * DagLoop subscriptions, the spawn execution fiber, AND the deadline + * watcher — while the durable node row stays `running`. Because the host + * process keeps running, nothing ever re-arms supervision: the node rots in + * `running` past its deadline with `timeout_extensions` frozen for hours + * (7.5h observed). Re-init/crash recovery CAN settle such rows, but only + * when something re-triggers the instance — and in a live host nothing + * does. + * + * This sweep is deliberately NOT forked into any per-directory + * InstanceState scope: its repeating fiber is forked into the LAYER scope + * at construction and lives for the process lifetime. It must therefore + * never depend on ambient per-instance context (InstanceRef) — its fiber's + * context is the layer-build context, which has none. Ownership is decided + * from the durable rows alone: this process's Database owns every workflow + * row it can read. Same-host races (a live watcher, the DagLoop) are + * serialized by the workflow's in-process lock; a second host sharing the + * DB is handled by the durable guardNode status read plus the projector's + * conditional UPDATE (only the first NodeFailed folds a non-terminal row) — + * double settles collapse to one. + * + * False-positive safety: a LIVE watcher escalates on + * escalateIntervalMs == max(1s, timeout_ms ?? 10min) and nodeTimeoutEscalated + * does NOT move deadline_ms — so a live overdue node legitimately shows a + * flat timeout_extensions for up to one full escalate interval (10 minutes + * on the default config). The freeze window is therefore expressed in + * ticks: a node is only declared dead once its counter has stayed flat for + * frozenTicksNeeded(escalateIntervalMs) consecutive sweep ticks — the + * default 10-minute cadence needs 11 ticks (≈11 minutes), so a live watcher + * always moves the counter well inside the window, while a dead one (the + * incident shape: 7.5h frozen) is settled in bounded time. + */ + +export interface Interface { + /** Re-arm the periodic sweep fiber (idempotent; production layers fork it at construction). */ + readonly init: () => Effect.Effect + /** One scan pass. Exported for deterministic tests: loop it frozenTicksNeeded times to simulate a dead watcher. */ + readonly sweepOnce: () => Effect.Effect +} + +export class Service extends Context.Service()("@opencode/DagSupervisionSweep") {} + +export const SWEEP_INTERVAL = "60 seconds" +const SWEEP_INTERVAL_MS = 60_000 + +/** + * Flat ticks before declaring supervision dead, derived from the node's own + * escalation cadence: a LIVE watcher escalates every + * escalateIntervalMs == max(1s, timeout_ms ?? 10min) and never moves + * deadline_ms, so its counter can legitimately stay flat for up to one full + * interval. Requiring ceil(interval / sweep interval) + 1 consecutive flat + * ticks means a live watcher — at ANY configured timeout, including the + * doc-recommended 30-minute verifier timeouts — always moves the counter + * inside the window, while a dead one (the incident shape: hours frozen) is + * settled in bounded time. The config lookup happens once a node has been + * overdue-flat for at least one tick, so healthy graphs pay nothing. + */ +export const frozenTicksNeeded = (escalateIntervalMs: number) => + Math.ceil(Math.max(escalateIntervalMs, 1_000) / SWEEP_INTERVAL_MS) + 1 + +/** + * The node's escalation cadence derived from a persisted config row. Pure — + * exported for unit tests. parseWorkflowConfig is the repo's defensive + * parser: malformed JSON or shape-divergent rows return undefined instead of + * throwing, and every degrade path lands on the DEFAULT cadence (the widest + * guaranteed-safe window) — a single corrupt row must never defect the sweep. + */ +export const escalateIntervalFromConfig = (raw: string | undefined, nodeId: string) => { + const node = raw === undefined ? undefined : parseWorkflowConfig(raw)?.nodes.find((n) => n.id === nodeId) + const timeoutMs = node?.worker_config?.timeout_ms + return Math.max(1_000, typeof timeoutMs === "number" ? timeoutMs : Dag.DEFAULT_WORKFLOW_CONFIG.nodeTimeoutMs) +} + +const serviceLayer = Layer.effect( + Service, + Effect.gen(function* () { + const { db } = yield* Database.Service + const store = yield* DagStore.Service + const dag = yield* Dag.Service + const promptSvc = yield* SessionPrompt.Service + const scope = yield* Scope.Scope + + // nodeKey -> {extensions, flatTicks}: the counter value last observed and + // how many consecutive sweep ticks it has stayed flat while the node was + // running and overdue. Reset on any counter movement, terminal status, or + // disappearance from the query. + const flatStreak = new Map() + + // The node's escalation cadence, from the workflow's persisted config — + // the same source spawn.ts derived the watcher's escalateIntervalMs from. + // Only nodes already flat for a tick pay this lookup; a store read + // failure degrades to the DEFAULT cadence (the widest guaranteed-safe + // window). + const escalateIntervalFor = Effect.fnUntraced(function* (workflowId: string, nodeId: string) { + const wf = yield* store.getWorkflow(workflowId).pipe( + Effect.catchCause((cause) => (Cause.hasInterrupts(cause) ? Effect.interrupt : Effect.succeed(undefined))), + ) + return escalateIntervalFromConfig(wf?.config, nodeId) + }) + + const sweepOnce = Effect.fn("DagSupervisionSweep.sweepOnce")(function* () { + const rows = yield* db + .select({ + workflowId: WorkflowNodeTable.workflow_id, + nodeId: WorkflowNodeTable.id, + childSessionId: WorkflowNodeTable.child_session_id, + extensions: WorkflowNodeTable.timeout_extensions, + }) + .from(WorkflowNodeTable) + .where( + and( + eq(WorkflowNodeTable.status, "running"), + sql`${WorkflowNodeTable.deadline_ms} IS NOT NULL AND ${WorkflowNodeTable.deadline_ms} <= ${Date.now()}`, + ), + ) + .all() + .pipe( + // A store defect must not kill the sweep — the same silent-death + // class this service exists to eliminate. Degrade to an empty + // pass and retry next tick (mirror of spawn.ts's R13 hardening). + // Interrupts (scope disposal) still propagate — the repo's + // background-loop discipline. + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.gen(function* () { + yield* Effect.logWarning("DagSupervisionSweep store query failed — skipping tick", { cause }) + return [] + }), + ), + ) + + const observed = new Map() + for (const row of rows) { + const key = `${row.workflowId}\0${row.nodeId}` + const prior = flatStreak.get(key) + const flatTicks = prior && prior.extensions === row.extensions ? prior.flatTicks + 1 : 0 + observed.set(key, { extensions: row.extensions, flatTicks }) + // Only nodes already flat for a tick pay the config lookup. + if (flatTicks < 1) continue + const escalateIntervalMs = yield* escalateIntervalFor(row.workflowId, row.nodeId) + if (flatTicks < frozenTicksNeeded(escalateIntervalMs)) continue + // Frozen across the full window: cancel the (possibly dead) child and + // settle the node. Same-host races (a live watcher) are serialized by + // the workflow's in-process lock; another host's sweep is collapsed + // by the durable terminal-status guard — either way at most one + // settle lands. + if (row.childSessionId) { + // Best-effort cancel, recovered at CAUSE level: the sweep's layer + // context has no ambient InstanceRef, so a real SessionPrompt.cancel + // dies at InstanceState.context ("InstanceRef not provided") — and + // cancel's channel is E=never, where Effect.ignore recovers nothing. + // In the incident shape (instance disposed) the child fiber died + // with the scope, so a skipped cancel is also the correct outcome; + // the durable settle below is the source of truth either way. + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- the durable column is typed string|null; the cancel seam brands SessionID. + yield* promptSvc.cancel(row.childSessionId as never).pipe( + Effect.catchCause((cause) => (Cause.hasInterrupts(cause) ? Effect.interrupt : Effect.void)), + ) + } + const settled = yield* dag + .nodeFailed( + row.workflowId, + row.nodeId, + `deadline supervision lost (no escalation progress across ${flatTicks} sweep ticks, escalate cadence ${escalateIntervalMs}ms) — swept, extensions ${row.extensions}`, + "timeout", + ) + .pipe( + Effect.as(true), + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.gen(function* () { + yield* Effect.logWarning("DagSupervisionSweep nodeFailed failed — retrying next tick", { + dagID: row.workflowId, + nodeID: row.nodeId, + cause, + }) + return false + }), + ), + ) + // On a failed settle keep the streak so the next tick retries + // immediately instead of deferring by a full freeze window. + if (!settled) continue + yield* Effect.logWarning("DagSupervisionSweep settled a node with dead deadline supervision", { + dagID: row.workflowId, + nodeID: row.nodeId, + extensions: row.extensions, + }) + observed.delete(key) + } + // Retain only what is still overdue-running so settled/restarted nodes + // do not accumulate. + flatStreak.clear() + for (const [key, streak] of observed) flatStreak.set(key, streak) + }) + + let sweepFiber: Fiber.Fiber | undefined + + const init = Effect.fn("DagSupervisionSweep.init")(function* () { + if (sweepFiber) return + sweepFiber = yield* Effect.gen(function* () { + for (;;) { + yield* Effect.sleep(SWEEP_INTERVAL) + yield* sweepOnce().pipe( + // Per-tick guard: any residual defect inside a tick degrades to + // a logged skip — the loop itself must outlive every failure. + // Interrupts (scope disposal) still exit the loop. + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.interrupt + : Effect.logWarning("DagSupervisionSweep tick failed — retrying next interval", { cause }), + ), + ) + } + }).pipe(Effect.forkIn(scope)) + }) + + // AGENTS.md background-loop convention: fork at construction so the sweep + // survives without any caller remembering to init it. + yield* init() + + return Service.of({ init, sweepOnce }) + }), +) + +/** The bare effect layer — bring your own Database/Dag/SessionPrompt. Tests compose this against their mocks; production uses `defaultLayer`. */ +export const layerWithoutDeps = serviceLayer + +export const layer = serviceLayer.pipe( + Layer.provide(Database.defaultLayer), + Layer.provide(DagStore.defaultLayer), + Layer.provide(Dag.defaultLayer), + Layer.provide(SessionPrompt.defaultLayer), +) + +export const defaultLayer = layer + +export const node = LayerNode.make(serviceLayer, [Database.node, DagStore.node, Dag.node, SessionPrompt.node]) diff --git a/packages/opencode/src/dag/validation.ts b/packages/opencode/src/dag/validation.ts index c92fae8847..d61882afb9 100644 --- a/packages/opencode/src/dag/validation.ts +++ b/packages/opencode/src/dag/validation.ts @@ -580,17 +580,35 @@ function conditionDiagnostics(nodes: readonly NodeConfig[]): Diagnostic[] { * checkpoint completes, so the checkpoint verdict can never act first. * Block-compiled graphs gate dependents on the verdict (issue #294 * REJECT-checkpoint shape); hand-built node graphs must do the same or keep - * the checkpoint as a reporting leaf. */ + * the checkpoint as a reporting leaf. + * + * Options: + * - `exemptCheckpointIds` (DAG-02 runtime path): a checkpoint already + * terminal in the durable graph is settled and immutable (terminal nodes + * never re-run) — the ordering race the gate protects is in the past, so + * additive waves / reopens may attach dependents without a condition. + * Authoring never exempts: nothing is terminal there yet. + * - `requireOutputSchema` (default true; the runtime replan path passes + * false): DAG-01's "gated checkpoints must declare output_schema" is an + * AUTHORING obligation — runtime-created graphs deliberately bypass + * authoring validation (CONTEXT.md), so the runtime gate polices the + * ordering race only, not the schema declaration. */ export function checkpointGateDiagnostics( nodes: readonly NodeConfig[], defaults?: { readonly report_to_parent?: boolean }, + options?: { + readonly exemptCheckpointIds?: ReadonlySet + readonly requireOutputSchema?: boolean + }, ): Diagnostic[] { const reportsToParent = (node: NodeConfig) => node.report_to_parent ?? defaults?.report_to_parent ?? DEFAULT_WORKFLOW_CONFIG.reportToParent + const requireOutputSchema = options?.requireOutputSchema ?? true return nodes.flatMap((checkpoint) => { if (!reportsToParent(checkpoint)) return [] - return nodes - .filter((dependent) => dependent.depends_on.includes(checkpoint.id)) + if (options?.exemptCheckpointIds?.has(checkpoint.id)) return [] + const dependents = nodes.filter((dependent) => dependent.depends_on.includes(checkpoint.id)) + const ungated = dependents .filter((dependent) => conditionReference(dependent.condition) !== checkpoint.id) .map((dependent) => diagnostic({ @@ -600,10 +618,34 @@ export function checkpointGateDiagnostics( `reporting checkpoint "${checkpoint.id}" has dependent "${dependent.id}" that is not gated on its output` + ` — the engine spawns "${dependent.id}" as soon as "${checkpoint.id}" completes, so the checkpoint verdict cannot be acted on first`, hint: - `Gate "${dependent.id}" with condition: "${checkpoint.id}.output. == ..." (e.g. on its verdict),` + `Gate "${dependent.id}" with condition: "${checkpoint.id}.output. == ..." (e.g. on its verdict) and declare output_schema on "${checkpoint.id}" so the gate reads a schema-validated verdict,` + ` keep "${checkpoint.id}" a reporting leaf, or set report_to_parent: false on "${checkpoint.id}" if downstream must run unconditionally`, }), ) + // DAG-01: a checkpoint whose output a gate reads must declare + // output_schema. Without a schema the child may complete with a raw + // string; the runtime normalizes JSON strings, but a prose reply + // resolves no fields, so the `.output.` gate is permanently + // false and the gated subtree is silently skipped while the workflow + // still reports COMPLETED. Make the unsatisfiable gate an authoring + // error instead of a runtime trap. + const gatedDependents = dependents.some((dependent) => conditionReference(dependent.condition) === checkpoint.id) + const schemaRequired = + gatedDependents && requireOutputSchema && checkpoint.output_schema === undefined + ? [ + diagnostic({ + code: DIAGNOSTIC_CODES.dagInvalid, + path: `nodes[${checkpoint.id}].output_schema`, + message: + `reporting checkpoint "${checkpoint.id}" is gated on its output but declares no output_schema` + + ` — without a schema the child may complete with prose that resolves no fields, leaving the gate permanently false and silently skipping the gated subtree`, + hint: + `Declare output_schema on "${checkpoint.id}" (e.g. the verdict shape),` + + ` keep "${checkpoint.id}" a reporting leaf, or set report_to_parent: false if downstream must run unconditionally`, + }), + ] + : [] + return [...ungated, ...schemaRequired] }) } @@ -771,8 +813,18 @@ export interface ReplanStructuralInput { existingNodeCount: number /** New node ids being added by this replan (toward the lifetime ceiling). */ addCount: number - /** Merged config (existing + fragment) for review-lifecycle validation. */ - merged: { name?: string; mode?: ExecutionMode; nodes: readonly NodeConfig[] } + /** Merged config (existing + fragment) for review-lifecycle and + * checkpoint-gate validation. */ + merged: { + name?: string + mode?: ExecutionMode + nodes: readonly NodeConfig[] + node_defaults?: { readonly report_to_parent?: boolean } + } + /** Durable nodes already terminal before this replan. Their reporting + * verdicts are delivered — the checkpoint gate exempts them so additive + * waves/reopens can attach dependents without a condition (DAG-02). */ + terminalNodeIds?: ReadonlySet config: { mode?: ExecutionMode; max_total_nodes?: number } } @@ -798,6 +850,19 @@ export function replanStructuralDiagnostics(input: ReplanStructuralInput): Diagn ...tagLegacyClass(review.warnings, 5), ...(duplicates.length === 0 ? topologyDiagnostics(input.rerunNodes) : []), ...tagLegacyClass(outputSchemaKeywordDiagnostics(input.rerunNodes), 8), + // DAG-02: the checkpoint gate must police the MERGED graph too — a + // fragment can attach a new dependent to an EXISTING reporting + // checkpoint, which the fragment-scoped authoring check cannot see. + // Pre-fix replan/extend skipped this gate entirely, so the dependent + // was spawned the moment the checkpoint completed, before the parent + // could read the verdict. Checkpoints already terminal in the durable + // graph are exempt: they are settled and immutable, the race is past. The + // output_schema obligation is authoring-only (requireOutputSchema:false) + // — runtime-created graphs deliberately bypass authoring validation. + ...checkpointGateDiagnostics(input.merged.nodes, input.merged.node_defaults, { + exemptCheckpointIds: input.terminalNodeIds, + requireOutputSchema: false, + }), ]) } @@ -971,17 +1036,21 @@ export function validatePostCompile(input: { structural?: boolean }): Effect.Effect { return Effect.gen(function* () { - const diagnostics = - input.structural === false + const diagnostics = [ + ...(input.structural === false ? [] - : [ - ...structuralDiagnostics({ - nodes: input.nodes, - mode: input.config.mode, - max_total_nodes: input.config.max_total_nodes, - }), - ...checkpointGateDiagnostics(input.nodes, input.config.node_defaults), - ] + : structuralDiagnostics({ + nodes: input.nodes, + mode: input.config.mode, + max_total_nodes: input.config.max_total_nodes, + })), + // DAG-02: the checkpoint gate is NOT a whole-graph structural check — + // fragment actions (replan/extend) must satisfy it exactly like start. + // Pre-fix it was skipped together with `structural`, so a replan could + // attach an ungated dependent to a reporting checkpoint and the engine + // spawned it the moment the checkpoint completed. + ...checkpointGateDiagnostics(input.nodes, input.config.node_defaults), + ] if (input.profile === "portable") diagnostics.push(...nonportablePromptDiagnostics(input.nodes)) if (input.profile === "environment") { diagnostics.push( diff --git a/packages/opencode/src/effect/app-runtime.ts b/packages/opencode/src/effect/app-runtime.ts index d9edca954c..435b173794 100644 --- a/packages/opencode/src/effect/app-runtime.ts +++ b/packages/opencode/src/effect/app-runtime.ts @@ -60,6 +60,7 @@ import { Dag } from "@/dag/dag" import { DagStore } from "@opencode-ai/core/dag/store" import { DagLoop } from "@/dag/runtime/loop" import { DagSummaryPublisher } from "@/dag/runtime/summary-publisher" +import { DagSupervisionSweep } from "@/dag/runtime/supervision-sweep" import { Memory } from "@/memory/memory" export const AppLayer = Layer.mergeAll( @@ -132,6 +133,11 @@ export const AppLayer = Layer.mergeAll( Layer.provideMerge(GoalLoop.defaultLayer), Layer.provideMerge(DagLoop.defaultLayer), Layer.provideMerge(DagSummaryPublisher.defaultLayer), + // Host-level deadline-supervision sweep (production incident 2026-08-18): + // forks its repeating fiber at construction into the LAYER scope — unlike + // DagLoop it must NOT die with a per-directory instance teardown, or a + // `running` node with dead supervision would rot forever. + Layer.provideMerge(DagSupervisionSweep.defaultLayer), Layer.provideMerge(SettingsHook.defaultLayer), ) diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index 1fd6c07f96..ea46ddf37a 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< | { @@ -161,6 +163,13 @@ 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 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. */ @@ -203,9 +212,13 @@ 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 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) { @@ -243,6 +256,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 @@ -250,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 @@ -259,13 +280,30 @@ const serviceLayer = Layer.effect( yield* automation.unregister(sessionID, { kind: "goal", id: paused.goal_id ?? "legacy" }).pipe( Effect.ignore, ) - } else { + turnDriven.delete(sessionID) + } 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 may resurrect on next idle", - { sessionID, cause: lastCause ? Cause.pretty(lastCause) : "unknown" }, + "goal pause on cancel failed after retries — goal stays active and turn-driven; a repeat ESC retries the pause", + { 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) } - turnDriven.delete(sessionID) return paused }) @@ -730,7 +768,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。" @@ -746,7 +794,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/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index f6a55ecd70..fc8b8c704f 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* () { @@ -208,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, ) @@ -217,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() @@ -247,10 +262,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 +289,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,139 +353,156 @@ 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) 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. - 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 } - // 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. @@ -657,6 +703,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. @@ -664,15 +713,59 @@ 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", { - 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), + }), + ), + ) + } + 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) => + Cause.hasInterrupts(cause) + ? Effect.void + : Effect.logWarning("goal startup scan retry failed for session", { + sessionID, + cause: Cause.pretty(cause), + }), ), ) } diff --git a/packages/opencode/src/memory/CONTEXT.md b/packages/opencode/src/memory/CONTEXT.md index cc95f18b7b..140467d01f 100644 --- a/packages/opencode/src/memory/CONTEXT.md +++ b/packages/opencode/src/memory/CONTEXT.md @@ -51,6 +51,8 @@ The domain runs on the existing seams; the elaborate `ProjectMemoryAuthority` re - Worktree `remove`/`reset` reconcile legacy memory fail-closed against the **complete** directory snapshot (primary + every registered sandbox) and always invalidate the admission cache before rescanning; they never trust a cached clean result. - Legacy files are re-read and compared immediately before deletion; content that changed after the scan is preserved and surfaced as a conflict. - Every writer of a MEMORY config file serializes on the file's cross-process lock; byte-atomicity is not undermined by whole-document last-writer-wins. +- Maintenance model calls never run under the identity fence or the project lock: prepare and checkpoint render the pre-maintenance snapshot, then kick maintenance in the background, gated on identity liveness so a retired identity never starts a job (one job in flight per project, the commit-only write back under the fence). +- Matcher model calls never run under the identity fence either (issue #324 acceptance): search, prepare, and checkpoint run their matcher outside every fence, and only the markMatched commit acquires the fence (`applyUpdate`). Search keeps a SHORT project-lock critical section (stale/cache/limit check + in-flight registration); concurrent identical queries within one turn coalesce through a process-local per-(session,turn,key) in-flight Deferred — the second caller awaits the first caller's exit (reused, no extra query slot) and degrades to `failed`/`unavailable` when the first call fails or its identity retires, instead of blocking a lock across the model call. ## Boundaries diff --git a/packages/opencode/src/memory/memory.ts b/packages/opencode/src/memory/memory.ts index 4a33e01c77..89b2f99da2 100644 --- a/packages/opencode/src/memory/memory.ts +++ b/packages/opencode/src/memory/memory.ts @@ -3,7 +3,7 @@ export * as Memory from "./memory" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { ProjectV2 } from "@opencode-ai/core/project" import { SessionV1 } from "@opencode-ai/core/v1/session" -import { Context, Effect, Layer, Option, Ref, Schema, Scope, Semaphore } from "effect" +import { Cause, Context, Deferred, Effect, Exit, Layer, Option, Ref, Schema, Scope, Semaphore } from "effect" import { stringify } from "yaml" import { Config } from "@/config/config" import { Provider } from "@/provider/provider" @@ -93,7 +93,23 @@ export const layer: Layer.Layer< const state = yield* InstanceState.make(() => Effect.succeed({ sessions: new Map() })) const scope = yield* Scope.Scope const maintenanceInFlight = yield* Ref.make(new Set()) - + // MEM-02: per-(session,turn,key) in-flight matcher registrations — the + // turn origin (messageID) in the key keeps coalescing turn-scoped, so a + // new turn's identical query re-runs instead of riding a previous turn's + // result that could never populate its cache. Replaces the old "hold the + // fence/lock across the model call so the second caller blocks and + // re-reads the cache" coalescing: the second identical query now awaits + // the first one's Deferred instead — same observable semantics (reused: + // true, no extra query slot) without a model call under the fence/lock. + // The runner brackets EVERYTHING after registration in an exit guard + // (onExit), so a failed or interrupted first call wakes its coalesced + // awaiter (degraded to "failed") instead of parking it forever. + // Process-local by design (the fence is the cross-process seam, and it + // now covers only the markMatched commit). + type MatchRun = { count: number; rendered: string[] } + const matchInFlight = yield* Ref.make( + new Map>>(), + ) const availableModels = Effect.fn("Memory.availableModels")(function* () { const providers = yield* provider.list() return new Set( @@ -283,7 +299,12 @@ export const layer: Layer.Layer< // 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. - const applyUpdate = (projectID: ProjectV2.ID, update: (topics: MemorySchema.Topic[]) => MemoryStore.Update) => + // The update callback's result is passed through, so a commit like + // markMatched can hand the caller the post-commit topics to render. + const applyUpdate = ( + projectID: ProjectV2.ID, + update: (topics: MemorySchema.Topic[]) => MemoryStore.Update, + ) => fence.withLiveIdentity( projectID, lock.withProject(projectID)(store.updateTopics(projectID, update)), @@ -328,27 +349,9 @@ export const layer: Layer.Layer< return decoded.value.actions }) - const maintain = Effect.fn("Memory.maintain")(function* (input: { - model: Provider.Model - config: MemorySchema.Config - topics: MemorySchema.Topic[] - messages: SessionV1.WithParts[] - projectID: Project.Info["id"] - }) { - const actions = yield* proposeMaintenance(input) - if (!actions) return input.topics - return yield* store - .updateTopics(input.projectID, (topics) => ({ - applied: MemoryStore.applyActions({ - topics, - actions, - topicLimit: input.config.topic_limit, - }), - result: undefined, - })) - .pipe(Effect.map((updated) => updated.topics)) - }) - + // MEM-01/02: the matcher model call runs OUTSIDE the fence/lock; only the + // markMatched commit acquires them (applyUpdate). The matched topics come + // back from the commit for rendering. const select = Effect.fn("Memory.select")(function* (input: { model: Provider.Model config: MemorySchema.Config @@ -357,11 +360,20 @@ export const layer: Layer.Layer< projectID: Project.Info["id"] }) { const topicIDs = yield* match(input) - const matched = yield* store.updateTopics(input.projectID, (topics) => ({ - applied: MemoryStore.markMatched(topics, topicIDs), - result: undefined, - })) - const byID = new Map(matched.topics.map((topic) => [topic.id, topic])) + const committed = yield* applyUpdate(input.projectID, (topics) => { + // Re-filter against the post-read topics: the matcher filtered on the + // snapshot it saw; a topic deleted since then must not resurrect. + const live = new Set(topics.map((topic) => topic.id)) + return { + applied: MemoryStore.markMatched(topics, topicIDs.filter((id) => live.has(id))), + result: undefined, + } + }) + // Identity retired between the model call and the commit: nothing was + // written; there is no matched set to render. + if (Option.isNone(committed)) return undefined + const matched = committed.value.topics + const byID = new Map(matched.map((topic) => [topic.id, topic])) const selected = topicIDs.flatMap((id) => { const topic = byID.get(id) return topic ? [topic] : [] @@ -373,12 +385,13 @@ export const layer: Layer.Layer< // matcher and maintenance model run OUTSIDE the fence/lock, and only the // topic commit acquires them (applyUpdate), so a long reasoning call // cannot wedge the lock, leak it on interruption, or block the caller. - const backgroundMaintain = Effect.fn("Memory.backgroundMaintain")(function* (input: { + type MaintenanceInput = { model: Provider.Model config: MemorySchema.Config messages: SessionV1.WithParts[] projectID: ProjectV2.ID - }) { + } + const backgroundMaintain = Effect.fn("Memory.backgroundMaintain")(function* (input: MaintenanceInput) { const topics = yield* store.readTopics(input.projectID) const actions = yield* proposeMaintenance({ model: input.model, @@ -405,37 +418,40 @@ export const layer: Layer.Layer< return next }) - const kickMaintenance = Effect.fn("Memory.kickMaintenance")(function* (input: { - model: Provider.Model - config: MemorySchema.Config - messages: SessionV1.WithParts[] - projectID: ProjectV2.ID - }) { + const kickMaintenance = Effect.fn("Memory.kickMaintenance")(function* (input: MaintenanceInput) { const job = backgroundMaintain(input).pipe( Effect.catchCause((cause) => Effect.logWarning("background MEMORY maintenance failed", { cause })), Effect.ensuring(releaseMaintenanceSlot(input.projectID)), ) - // Reserve and fork atomically: an interruption between the two would - // leak the in-flight slot and silently skip every later maintenance for - // this process; a fork into a closing scope must hand the slot back. - yield* Effect.uninterruptible( - Effect.gen(function* () { - const reserved = yield* Ref.modify(maintenanceInFlight, (set) => - set.has(input.projectID) - ? ([false, set] as const) - : ([true, new Set(set).add(input.projectID)] as const), - ) - if (!reserved) return - yield* job.pipe( - Effect.forkIn(scope), - Effect.catchCause((cause) => - Effect.gen(function* () { - yield* releaseMaintenanceSlot(input.projectID) - yield* Effect.logWarning("background MEMORY maintenance fork failed", { cause }) - }), - ), - ) - }), + // The single definition of the kickoff rule: the identity fence gates + // the fork, so a retired identity never burns a maintenance model call. + // Callers must NOT already hold the fence (it is not reentrant) and must + // treat None as "identity retired" — dropping their cached session state + // is the whole cost, because the commit inside applyUpdate is fenced too. + return yield* fence.withLiveIdentity( + input.projectID, + // Reserve and fork atomically: an interruption between the two would + // leak the in-flight slot and silently skip every later maintenance for + // this process; a fork into a closing scope must hand the slot back. + Effect.uninterruptible( + Effect.gen(function* () { + const reserved = yield* Ref.modify(maintenanceInFlight, (set) => + set.has(input.projectID) + ? ([false, set] as const) + : ([true, new Set(set).add(input.projectID)] as const), + ) + if (!reserved) return + yield* job.pipe( + Effect.forkIn(scope), + Effect.catchCause((cause) => + Effect.gen(function* () { + yield* releaseMaintenanceSlot(input.projectID) + yield* Effect.logWarning("background MEMORY maintenance fork failed", { cause }) + }), + ), + ) + }), + ), ) }) @@ -470,50 +486,47 @@ export const layer: Layer.Layer< session.firstTurnAttempted = true if (!due && !shouldMatch) return - // Cross-process identity guard (see checkpointUnsafe): MemoryIdentityFence - // re-checks identity liveness under the identity lock before writing. - 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({ - model: current.model, - config: current.loaded.config, - topics, - messages: input.messages, - projectID: current.project.id, - }).pipe( - Effect.catchCause((cause) => - Effect.gen(function* () { - yield* Effect.logWarning("periodic MEMORY maintenance failed", { cause }) - return topics - }), - ), - ) - : topics - const rendered = shouldMatch - ? (yield* select({ - model: current.model, - config: current.loaded.config, - topics: maintained, - text: user.text, - projectID: current.project.id, - })).rendered - : (data.sessions.get(input.sessionID)?.turn.rendered ?? []) - const entry = data.sessions.get(input.sessionID) - if (entry?.turn.messageID !== user.info.id) return - entry.turn = { ...entry.turn, completedTurns: turns, rendered } - }), - ) - }), - ) - if (Option.isNone(live)) { + const maintenance = { + model: current.model, + config: current.loaded.config, + messages: input.messages, + projectID: current.project.id, + } + + if (!shouldMatch) { + // Due-only turns reuse the cached injection: no project lock, no store + // read, and the cadence bookkeeping is a process-local map write. The + // kick carries the identity gate (see kickMaintenance). + const entry = data.sessions.get(input.sessionID) + if (entry?.turn.messageID === user.info.id) entry.turn = { ...entry.turn, completedTurns: turns } + if (Option.isNone(yield* kickMaintenance(maintenance))) yield* clearSession(input.sessionID) + return + } + + // MEM-01: the first-turn matcher runs OUTSIDE the fence/lock; only its + // markMatched commit acquires them (inside select → applyUpdate). An + // identity retired mid-call surfaces as select === undefined — fail + // closed by dropping the cached session state. Due maintenance is + // kicked AFTERwards, so a long reasoning call never holds the fence: + // this turn renders the pre-maintenance topics and the committed update + // surfaces on a later prepare. + const selected = yield* select({ + model: current.model, + config: current.loaded.config, + topics: yield* store.readTopics(current.project.id), + text: user.text, + projectID: current.project.id, + }) + if (!selected) { yield* clearSession(input.sessionID) return } + const entry = data.sessions.get(input.sessionID) + if (entry?.turn.messageID === user.info.id) { + entry.turn = { ...entry.turn, completedTurns: turns, rendered: selected.rendered } + } + if (!due) return + if (Option.isNone(yield* kickMaintenance(maintenance))) yield* clearSession(input.sessionID) }) const prepare: Interface["prepare"] = Effect.fn("Memory.prepare")((input) => @@ -574,48 +587,123 @@ export const layer: Layer.Layer< } const origin = user.info.id - // 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, + // MEM-02 (issue #324 acceptance): the matcher model call runs OUTSIDE + // the fence/lock. Concurrent identical queries coalesce through the + // per-(session,turn,key) in-flight Deferred instead of lock-blocking: + // the second caller re-checks the cache under a SHORT project-lock + // critical section (stale/cache/limit check + registration), awaits + // the first caller's result, and reports reused without spending + // another model call or query slot. Only the markMatched commit + // (inside select → applyUpdate) acquires the fence. + const inFlightKey = `${input.sessionID}\0${origin}\0${key}` + const deferred = yield* Deferred.make>() + + const releaseIfOwner = Effect.fnUntraced(function* () { + yield* Ref.update(matchInFlight, (map) => { + if (map.get(inFlightKey) !== deferred) return map + const next = new Map(map) + next.delete(inFlightKey) + return next + }) + }) + + const outcome = yield* lock.withProject(current.project.id)( Effect.gen(function* () { - return yield* lock.withProject(current.project.id)( - Effect.gen(function* () { - const activeTurn = data.sessions.get(input.sessionID)?.turn - if (activeTurn?.messageID !== origin) return { status: "stale" as const } - const repeated = activeTurn.queries.get(key) - if (repeated) { - activeTurn.rendered = repeated.rendered - return repeated.count > 0 - ? { status: "attached" as const, count: repeated.count, reused: true } - : { status: "empty" as const, reused: true } - } - if (activeTurn.queryCount >= 2) return { status: "limit" as const } - activeTurn.queryCount++ - const topics = yield* store.readTopics(current.project.id) - const selected = yield* select({ - model: current.model, - config: current.loaded.config, - topics, - text: query, - projectID: current.project.id, - }) - const latest = data.sessions.get(input.sessionID)?.turn - if (latest?.messageID !== origin) return { status: "stale" as const } - latest.queries.set(key, selected) - latest.rendered = selected.rendered - return selected.count > 0 - ? { status: "attached" as const, count: selected.count, reused: false } - : { status: "empty" as const, reused: false } - }), - ) + const activeTurn = data.sessions.get(input.sessionID)?.turn + if (activeTurn?.messageID !== origin) return { status: "stale" as const } + const repeated = activeTurn.queries.get(key) + if (repeated) { + activeTurn.rendered = repeated.rendered + return repeated.count > 0 + ? { status: "attached" as const, count: repeated.count, reused: true } + : { status: "empty" as const, reused: true } + } + if (activeTurn.queryCount >= 2) return { status: "limit" as const } + const running = yield* Ref.modify(matchInFlight, (map) => { + const existing = map.get(inFlightKey) + if (existing) return [existing, map] as const + return [deferred, new Map(map).set(inFlightKey, deferred)] as const + }) + if (running !== deferred) return { kind: "await-first" as const, first: running } + activeTurn.queryCount++ + return { kind: "run" as const } }), ) - if (Option.isNone(live)) { - yield* clearSession(input.sessionID) - return { status: "unavailable" as const } + if ("kind" in outcome) { + if (outcome.kind === "await-first") { + // The awaiter rides the runner's exit: the runner's exit bracket + // packs every outcome — success, failure, interrupt, retired — + // into the deferred payload, so the await always wakes. Failures + // surface as this caller's "failed" (returned directly here); + // the runner's interrupt cause is re-raised via failCause so the + // wrapper's log carries it — the awaiter itself still completes + // with "failed". Never a permanent park. + const first = yield* Deferred.await(outcome.first) + if (Exit.isFailure(first)) { + if (Cause.hasInterrupts(first.cause)) return yield* Effect.failCause(first.cause) + return { status: "failed" as const } + } + const selected = first.value + if (!selected) return { status: "unavailable" as const } + const latest = data.sessions.get(input.sessionID)?.turn + if (latest?.messageID !== origin) return { status: "stale" as const } + return selected.count > 0 + ? { status: "attached" as const, count: selected.count, reused: true } + : { status: "empty" as const, reused: true } + } + // This caller owns the matcher run. EVERYTHING after registration — + // the topics read and the select pipeline (model call, fenced + // markMatched commit) — runs inside one exit bracket: onExit fires + // on success, failure, interrupt, and identity-retired alike, + // deregistering the map entry and packing the real exit into the + // deferred so a coalesced awaiter wakes instead of parking forever + // (kickMaintenance's slot discipline, applied from the moment the + // entry exists — an interrupt during the topics read would otherwise + // unwind before the bracket attaches and wedge the (turn,key) + // forever). The deferred itself never fails — the Exit payload is + // the whole message. + const runExit = yield* Effect.gen(function* () { + return yield* select({ + model: current.model, + config: current.loaded.config, + topics: yield* store.readTopics(current.project.id), + text: query, + projectID: current.project.id, + }) + }).pipe( + Effect.tap((selected) => { + // Publish the cache entry BEFORE the in-flight deregistration in + // onExit: a third identical caller entering between the two + // would otherwise miss both the cache and the in-flight entry + // and burn a second model call + query slot where the old + // lock-blocking design guaranteed reuse. Stale-origin runs skip + // the write; the stale check below still governs the response. + if (!selected) return Effect.void + const latest = data.sessions.get(input.sessionID)?.turn + if (latest?.messageID !== origin) return Effect.void + latest.queries.set(key, selected) + latest.rendered = selected.rendered + return Effect.void + }), + Effect.onExit((exit) => releaseIfOwner().pipe(Effect.andThen(Deferred.succeed(deferred, exit)))), + Effect.exit, + ) + if (Exit.isFailure(runExit)) return yield* Effect.failCause(runExit.cause) + const selected = runExit.value + // Identity retired between model call and commit — fail closed. The + // deferred already carries the same (succeeded-undefined) exit, so a + // coalesced awaiter degrades to "unavailable" rather than hanging. + if (!selected) { + yield* clearSession(input.sessionID) + return { status: "unavailable" as const } + } + const latest = data.sessions.get(input.sessionID)?.turn + if (latest?.messageID !== origin) return { status: "stale" as const } + return selected.count > 0 + ? { status: "attached" as const, count: selected.count, reused: false } + : { status: "empty" as const, reused: false } } - return live.value + return outcome }) const search: Interface["search"] = Effect.fn("Memory.search")((input) => @@ -639,36 +727,33 @@ export const layer: Layer.Layer< return [] } const user = latestRealUser(input.messages) - const live = yield* fence.withLiveIdentity( - current.project.id, - lock.withProject(current.project.id)( - Effect.gen(function* () { - const topics = yield* store.readTopics(current.project.id) - return (yield* select({ - model: current.model, - config: current.loaded.config, - topics, - text: user?.text ?? "", - projectID: current.project.id, - })).rendered - }), - ), - ) - if (Option.isNone(live)) { + // MEM-01: the render matcher runs OUTSIDE the fence/lock; its + // markMatched commit is fenced inside select → applyUpdate. An identity + // retired mid-call (select === undefined) fails closed to an empty + // render, same as the retired-fence outcome before. + const selected = yield* select({ + model: current.model, + config: current.loaded.config, + topics: yield* store.readTopics(current.project.id), + text: user?.text ?? "", + projectID: current.project.id, + }) + if (!selected) { yield* clearSession(input.sessionID) return [] } - // Maintenance runs in the background AFTER the identity fence: compaction - // must not wait on a long reasoning call, a retired identity never burns - // model calls, and the injection above rendered the pre-maintenance - // topics. At most one job per project is in flight. - yield* kickMaintenance({ + // Maintenance is kicked AFTER the identity fence releases: compaction must + // not wait on a long reasoning call, and the injection above rendered the + // pre-maintenance topics. The kick carries the identity gate and the + // one-job-per-project reservation (see kickMaintenance). + const kicked = yield* kickMaintenance({ model: current.model, config: current.loaded.config, messages: input.messages, projectID: current.project.id, }) - return live.value + if (Option.isNone(kicked)) yield* clearSession(input.sessionID) + return selected.rendered }) const checkpoint: Interface["checkpoint"] = Effect.fn("Memory.checkpoint")((input) => diff --git a/packages/opencode/src/server/global-lifecycle.ts b/packages/opencode/src/server/global-lifecycle.ts index 12b7687bfe..c6c47ba9e5 100644 --- a/packages/opencode/src/server/global-lifecycle.ts +++ b/packages/opencode/src/server/global-lifecycle.ts @@ -1,6 +1,6 @@ import { GlobalBus } from "@/bus/global" import { InstanceStore } from "@/project/instance-store" -import { Effect } from "effect" +import { Effect, Option } from "effect" import { Event } from "./event" export const emitGlobalDisposed = Effect.sync(() => @@ -13,15 +13,34 @@ export const emitGlobalDisposed = Effect.sync(() => }), ) +// DAG-04 (#316): bounded disposal, mirroring the httpapi exerciser's cleanup +// guard (test/server/httpapi-exercise/runner.ts `bounded`, 10s). A wedged +// instance disposal must not hang global shutdown forever; after the timeout +// we abandon the in-flight disposal and move on — the same trade the +// exerciser makes ("resource may leak" beats "never terminates"). +// timeoutOption represents the timeout as Option.none (never as an error), so +// a genuine disposal failure still propagates for callers that do not swallow, +// while a hang is always cut off and the Disposed event below always lands. +// The cut lands at the disposal's first interruptible point: a wedge inside +// an uninterruptible finalizer region can outlast the cap (a hard sever would +// need Effect.disconnect, deliberately not taken — same residual the +// exerciser's Promise.race guard carries). +const DISPOSE_ALL_TIMEOUT = "10 seconds" + export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn("Server.disposeAllInstancesAndEmitGlobalDisposed")( function* (options?: { swallowErrors?: boolean }) { const store = yield* InstanceStore.Service - 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) + const disposeAttempt = options?.swallowErrors + ? store.disposeAll().pipe( + Effect.catchCause((cause) => Effect.logWarning("global disposal failed", { cause })), + ) + : store.disposeAll() + const outcome = yield* disposeAttempt.pipe(Effect.timeoutOption(DISPOSE_ALL_TIMEOUT)) + if (Option.isNone(outcome)) + yield* Effect.logWarning("global disposal timed out — abandoning in-flight disposal", { + timeout: DISPOSE_ALL_TIMEOUT, + }) + yield* emitGlobalDisposed }, ) diff --git a/packages/opencode/test/dag/blocks.test.ts b/packages/opencode/test/dag/blocks.test.ts index ba51154e58..a36184d272 100644 --- a/packages/opencode/test/dag/blocks.test.ts +++ b/packages/opencode/test/dag/blocks.test.ts @@ -44,6 +44,56 @@ describe("workflow blocks", () => { ]) }) + // issue #323: a reporting checkpoint adjudicates a direction — its prompt + // must demand adversarial independent verification, not self-confirmation + // of upstream claims. The production incident: cp-after-exploration + // confirmed a parent-supplied "defect" that was a production no-op because + // the gate re-read the parent's narrative instead of reading the source. + it("compiles reporting checkpoints with an adversarial verification clause", () => { + const nodes = DagBlocks.compileWorkflowBlocks({ + objective: "Ship the feature", + blocks: [ + { id: "cp-after-exploration", kind: "explore", report_to_parent: true }, + { id: "stage", kind: "coding", depends_on: ["cp-after-exploration"] }, + ], + }) + const checkpoint = nodes.find((node) => node.id === "cp-after-exploration") + expect(checkpoint?.report_to_parent).toBe(true) + // Upstream claims are hypotheses to verify, not facts to confirm. + expect(checkpoint?.prompt_template.inline).toContain("hypotheses to verify") + // Independent source inspection is mandatory for load-bearing claims. + expect(checkpoint?.prompt_template.inline).toContain("Independently read the relevant source") + // A quantified spot-check floor on the most load-bearing claims. + expect(checkpoint?.prompt_template.inline).toContain("at least three") + // A claim that fails inspection must fail the direction, not pass it. + expect(checkpoint?.prompt_template.inline).toContain("replan or reject") + }) + + it("keeps the adversarial clause off non-reporting nodes", () => { + const nodes = DagBlocks.compileWorkflowBlocks({ + objective: "Ship the feature", + blocks: [ + { id: "map", kind: "explore" }, + { id: "stage", kind: "coding", depends_on: ["map"] }, + ], + }) + // Only adjudicating checkpoints pay the adversarial cost; evidence + // producers (a plain explore) and executors (coding) do not. + for (const node of nodes) { + expect(node.report_to_parent).toBe(false) + expect(node.prompt_template.inline).not.toContain("hypotheses to verify") + } + }) + + it("gives default-reporting synthesize nodes the adversarial clause", () => { + const nodes = DagBlocks.compileWorkflowBlocks({ + objective: "Ship the feature", + blocks: [{ id: "closing", kind: "synthesize" }], + }) + expect(nodes[0]?.report_to_parent).toBe(true) + expect(nodes[0]?.prompt_template.inline).toContain("hypotheses to verify") + }) + it("composes configured design delivery capabilities without new lifecycle kinds", () => { const nodes = DagBlocks.compileWorkflowBlocks({ objective: "Design, implement, and review project-owned memory", diff --git a/packages/opencode/test/dag/dag-checkpoint-gate.test.ts b/packages/opencode/test/dag/dag-checkpoint-gate.test.ts index 18fcebecf8..80a8291be7 100644 --- a/packages/opencode/test/dag/dag-checkpoint-gate.test.ts +++ b/packages/opencode/test/dag/dag-checkpoint-gate.test.ts @@ -137,3 +137,108 @@ it.effect("flags a condition that gates a different dependency than the checkpoi expect(result.errors.some((d) => d.message.includes('"cp-design-decision"') && d.message.includes('"stage-development"'))).toBe(true) }), ) + +// DAG-01 (authoring half): a checkpoint whose output a condition reads must +// declare output_schema. Without it the child completes with a raw string; +// even with the runtime's JSON normalization a prose reply resolves no +// fields, so the `.output.` gate would be permanently false and the +// gated subtree silently skipped while the workflow reports COMPLETED. +it.effect("rejects a gated reporting checkpoint without output_schema", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "schemaless-gate", + nodes: [ + { ...checkpoint("cp-decision"), output_schema: undefined }, + stage("stage-next", ["cp-decision"], 'cp-decision.output.verdict == "continue"'), + ], + }), + ) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp-decision"') && d.message.includes("output_schema"))).toBe(true) + }), +) + +it.effect("accepts a gated reporting checkpoint that declares output_schema", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "schema-gate", + nodes: [ + checkpoint("cp-decision"), + stage("stage-next", ["cp-decision"], 'cp-decision.output.verdict == "continue"'), + ], + }), + ) + expect(result.errors.filter((d) => d.message.includes("output_schema"))).toEqual([]) + }), +) + +it.effect("does not require output_schema on a reporting leaf checkpoint", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "schemaless-leaf", + nodes: [{ ...checkpoint("cp-final"), depends_on: [], output_schema: undefined }], + }), + ) + expect(result.errors.filter((d) => d.message.includes("output_schema"))).toEqual([]) + }), +) + +// DAG-02: pre-fix authoring closed ALL structural diagnostics for non-start +// actions (`structural: input.action === "start"`), so a replan/extend could +// attach an ungated dependent to a reporting checkpoint and the engine would +// spawn it the moment the checkpoint completes — the checkpoint gate must +// apply to fragment actions too. +function validateReplan(fragmentGraph: Record) { + return WorkflowAuthoring.make().prepare({ + action: "replan", + source: { + kind: "inline", + value: { fragment: fragmentGraph }, + source: "", + }, + }) +} + +function validateExtend(value: unknown) { + return WorkflowAuthoring.make().prepare({ + action: "extend", + source: { kind: "inline", value, source: "" }, + }) +} + +it.effect("rejects a replan fragment whose dependent is not gated on the fragment's reporting checkpoint", () => + Effect.gen(function* () { + const result = yield* validateReplan({ + name: "replan-ungated", + nodes: [checkpoint("cp-review"), stage("stage-fix", ["cp-review"])], + }) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp-review"') && d.message.includes('"stage-fix"'))).toBe(true) + }), +) + +it.effect("rejects an extend fragment whose dependent is not gated on its reporting checkpoint", () => + Effect.gen(function* () { + const result = yield* validateExtend({ + nodes: [checkpoint("cp-review"), stage("stage-fix", ["cp-review"])], + }) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp-review"') && d.message.includes('"stage-fix"'))).toBe(true) + }), +) + +it.effect("accepts a replan fragment that gates its dependent on the checkpoint output", () => + Effect.gen(function* () { + const result = yield* validateReplan({ + name: "replan-gated", + nodes: [ + checkpoint("cp-review"), + stage("stage-fix", ["cp-review"], 'cp-review.output.verdict == "continue"'), + ], + }) + expect(result.errors.filter((d) => d.message.includes("not gated"))).toEqual([]) + }), +) diff --git a/packages/opencode/test/dag/dag-loop-guards.test.ts b/packages/opencode/test/dag/dag-loop-guards.test.ts index fce8c31d39..ab44d6f3b7 100644 --- a/packages/opencode/test/dag/dag-loop-guards.test.ts +++ b/packages/opencode/test/dag/dag-loop-guards.test.ts @@ -84,6 +84,8 @@ function guardLayer(input: { readonly cancels: string[] /** Injected one-shot defects for DagStore.getWorkflow (P1 survival test). */ readonly failGetWorkflow?: { remaining: number } + /** Injected Dag.pause failures (typed or defect) for the DAG-03 gate test. */ + readonly failPause?: { remaining: number; defect?: boolean } }) { const database = Database.layerFromPath(":memory:") const events = EventV2.layer.pipe(Layer.provide(database)) @@ -113,10 +115,31 @@ function guardLayer(input: { Layer.provide(events), Layer.provide(database), ) - const dag = Dag.layer.pipe( + const realDag = Dag.layer.pipe( Layer.provide(bridge), Layer.provide(store), ) + const dag = input.failPause + ? Layer.effect( + Dag.Service, + Effect.gen(function* () { + const real = yield* Dag.Service + return Dag.Service.of({ + ...real, + pause: (id) => + Effect.suspend(() => { + if (input.failPause!.remaining > 0) { + input.failPause!.remaining-- + return input.failPause!.defect + ? Effect.die(new Error("injected pause defect")) + : Effect.fail(new Error("injected pause failure")) + } + return real.pause(id) + }), + }) + }), + ).pipe(Layer.provide(realDag)) + : realDag const base = Layer.mergeAll(database, events, bridge, store, projector, dag, status) const childTitles = new Map() const created: string[] = [] @@ -176,6 +199,7 @@ function runGuardTest( /** Project the current instance belongs to. */ readonly instanceProject: string readonly failGetWorkflow?: { remaining: number } + readonly failPause?: { remaining: number; defect?: boolean } }, test: (services: { readonly dag: Dag.Interface @@ -214,7 +238,7 @@ function runGuardTest( yield* loop.init() return yield* test({ dag, loop, store, childPrompts, cancels }) }).pipe( - Effect.provide(guardLayer({ childPrompts, cancels, failGetWorkflow: options.failGetWorkflow })), + Effect.provide(guardLayer({ childPrompts, cancels, failGetWorkflow: options.failGetWorkflow, failPause: options.failPause })), Effect.provideService(InstanceRef, { directory: process.cwd(), worktree: process.cwd(), @@ -486,3 +510,287 @@ describe("DagLoop replan verdict gate (issue #322)", () => { ) }) }) + +// DAG-01 (runtime half): a schema-less reporting checkpoint completes with a +// RAW STRING output. Pre-fix the condition evaluator resolved +// `gate.output.` on that string to undefined, so an equality gate was +// permanently false: every gated dependent skipped (condition_false), the +// orphan cascade terminalized the subtree, and checkCompletion marked the +// workflow COMPLETED with skipReviewGate — half the graph never ran, with +// no error anywhere. The fix normalizes string outputs through the same +// parseJsonOption the replan-verdict gate already uses. +describe("DagLoop equality gates on schema-less string outputs (DAG-01)", () => { + it("evaluates a .output. condition against a JSON-string checkpoint output", async () => { + await Effect.runPromise( + runGuardTest({ instanceProject: "project-1" }, ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + projectID: "project-1", + sessionID: "ses_project-1", + title: "String equality gate", + config: { + name: "string-equality-gate", + nodes: [ + node({ id: "gate", name: "gate", required: true, report_to_parent: true }), + node({ + id: "downstream", + name: "downstream", + required: false, + depends_on: ["gate"], + condition: 'gate.output.verdict == "continue"', + }), + ], + }, + }) + const gateChild = yield* takeWithin(childPrompts, "gate node did not start") + expect(gateChild.title).toBe("gate") + yield* dag.nodeCompleted(dagID, "gate", JSON.stringify({ verdict: "continue", findings: "confirmed" })) + // The report_to_parent wake and the downstream spawn can land in + // either order; accept the downstream prompt whichever comes second. + const first = yield* takeWithin(childPrompts, "no prompt after continue verdict — gate evaluated false on the string output") + const downstreamChild = first.title === "downstream" + ? first + : yield* takeWithin(childPrompts, "downstream was silently skipped — string output never normalized (DAG-01)") + expect(downstreamChild.title).toBe("downstream") + expect((yield* store.getNode(dagID, "downstream"))?.status).not.toBe("skipped") + yield* Deferred.succeed(downstreamChild.release, "done") + }), + ), + ) + }) + + it("keeps a non-JSON string output gate false without the subtree silently vanishing", async () => { + await Effect.runPromise( + runGuardTest({ instanceProject: "project-1" }, ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + projectID: "project-1", + sessionID: "ses_project-1", + title: "Prose gate", + config: { + name: "prose-gate", + nodes: [ + node({ id: "gate", name: "gate", required: true, report_to_parent: true }), + node({ + id: "downstream", + name: "downstream", + required: false, + depends_on: ["gate"], + condition: 'gate.output.verdict == "continue"', + }), + ], + }, + }) + const gateChild = yield* takeWithin(childPrompts, "gate node did not start") + expect(gateChild.title).toBe("gate") + // Prose (non-JSON) output: normalization falls back to the raw + // string, the field path resolves undefined, the equality gate is + // false — the documented skip, not a crash. + yield* dag.nodeCompleted(dagID, "gate", "All good, shipping it.") + yield* pollWithTimeout( + Effect.gen(function* () { + const downstream = yield* store.getNode(dagID, "downstream") + return downstream?.status === "skipped" ? (true as const) : undefined + }), + "prose-output gate did not settle to condition_false", + ) + }), + ), + ) + }) +}) + +// DAG-02 (runtime half): the checkpoint gate must also police the MERGED +// graph at replan/extend — a fragment may attach a new dependent to an +// existing reporting checkpoint, which the fragment-scoped authoring check +// cannot see. Pre-fix replanStructuralDiagnostics never ran +// checkpointGateDiagnostics, so the engine spawned the dependent the moment +// the checkpoint completed, before the parent could read the verdict. +describe("Dag.replan merged-graph checkpoint gate (DAG-02)", () => { + it("rejects a replan fragment that attaches an ungated dependent to an existing reporting checkpoint", async () => { + await Effect.runPromise( + runGuardTest({ instanceProject: "project-1" }, ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + projectID: "project-1", + sessionID: "ses_project-1", + title: "Merged gate", + config: { + name: "merged-gate", + nodes: [ + node({ id: "gate", name: "gate", required: true, report_to_parent: true, output_schema: { type: "object" } }), + node({ + id: "downstream", + name: "downstream", + required: false, + depends_on: ["gate"], + condition: 'gate.output.verdict == "continue"', + }), + ], + }, + }) + const gateChild = yield* takeWithin(childPrompts, "gate node did not start") + expect(gateChild.title).toBe("gate") + // Fragment adds a dependent on the existing checkpoint WITHOUT a + // condition — the merged graph must reject it. + const attempt = yield* dag + .replan(dagID, { nodes: [node({ id: "late", name: "late", depends_on: ["gate"] })] }) + .pipe( + Effect.match({ + onFailure: (error) => ({ ok: false as const, message: String(error) }), + onSuccess: () => ({ ok: true as const, message: "" }), + }), + ) + expect(attempt.ok).toBe(false) + expect(attempt.message.includes('"gate"') && attempt.message.includes('"late"')).toBe(true) + expect((yield* store.getNode(dagID, "late"))).toBeUndefined() + yield* Deferred.succeed(gateChild.release, "done") + }), + ), + ) + }) +}) + +// DAG-03: the replan-verdict gate must FAIL CLOSED. The checkpoint vetoed +// the direction; if the durable pause cannot be persisted (both attempts +// fail/defect and the row still reads non-paused), the in-memory scheduler +// must still HOLD — pre-fix it returned `wf?.status === "paused"` +// (fail-OPEN) and explicitly un-paused the runtime, so the very next +// stimulus that calls spawnReady (here: a NodeFailed handler) spawned the +// vetoed dependent. Defects from dag.pause must also fold into the retry +// path: pre-fix `Effect.catch` only covered the error channel, a defect +// escaped to guarded() and dropped the whole NodeCompleted handler (no +// pause, no gate log). +describe("DagLoop replan verdict gate fail-closed (DAG-03)", () => { + function vetoedGateGraph(title: string, name: string) { + return { + projectID: "project-1", + sessionID: "ses_project-1", + title, + config: { + name, + nodes: [ + node({ id: "gate", name: "gate", required: true, report_to_parent: true, output_schema: { type: "object" } }), + node({ id: "downstream", name: "downstream", required: false, depends_on: ["gate"] }), + node({ id: "probe", name: "probe", required: false }), + ], + }, + } + } + + // Create the graph; gate and probe are both ready at boot, so take both + // prompts and index them by title (order is racy). + function takeBootPrompts(childPrompts: Queue.Queue) { + return Effect.gen(function* () { + const first = yield* takeWithin(childPrompts, "first boot prompt did not arrive") + const second = yield* takeWithin(childPrompts, "second boot prompt did not arrive") + const byTitle = new Map([[first.title, first], [second.title, second]]) + const gate = byTitle.get("gate") + const probe = byTitle.get("probe") + if (!gate || !probe) return yield* Effect.fail(new Error(`expected gate+probe, got ${first.title}/${second.title}`)) + return { gate, probe } + }) + } + + it("holds the in-memory pause when the durable pause exhausts its retries", async () => { + await Effect.runPromise( + runGuardTest( + { instanceProject: "project-1", failPause: { remaining: 99 } }, + ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create(vetoedGateGraph("Fail-closed gate", "fail-closed-gate")) + const { gate } = yield* takeBootPrompts(childPrompts) + expect(gate.title).toBe("gate") + yield* dag.nodeCompleted(dagID, "gate", { verdict: "replan", findings: "vetoed" }) + // The durable pause never landed + expect((yield* store.getWorkflow(dagID))?.status).toBe("running") + // Post-veto stimulus on an unrelated node. + yield* dag.nodeFailed(dagID, "probe", "probe exploded", "exec_failed") + yield* Effect.sleep("300 millis") + // Fail-closed: the vetoed dependent was NOT spawned by the + // post-veto stimulus. + expect((yield* store.getNode(dagID, "downstream"))?.status).toBe("pending") + }), + ), + ) + }) + + it("holds the in-memory pause when the pause attempts defect", async () => { + await Effect.runPromise( + runGuardTest( + { instanceProject: "project-1", failPause: { remaining: 99, defect: true } }, + ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create(vetoedGateGraph("Defect gate", "defect-gate")) + const { gate } = yield* takeBootPrompts(childPrompts) + expect(gate.title).toBe("gate") + yield* dag.nodeCompleted(dagID, "gate", { verdict: "replan", findings: "vetoed" }) + expect((yield* store.getWorkflow(dagID))?.status).toBe("running") + yield* dag.nodeFailed(dagID, "probe", "probe exploded", "exec_failed") + yield* Effect.sleep("300 millis") + expect((yield* store.getNode(dagID, "downstream"))?.status).toBe("pending") + }), + ), + ) + }) + + // Review F2 (R1): the hold must SURVIVE the durable-row re-sync that the + // next node terminal event performs, and it must be RELEASED by an explicit + // parent control action (replan/resume/step) — otherwise the fail-closed + // hold was lifted by any subsequent NodeCompleted/NodeSkipped/stepped + // stimulus and spawnReady ran on the vetoed direction before the parent + // ever adjudicated the verdict. + it("the veto hold survives a terminal-event flag re-sync (DAG-03 / F2)", async () => { + await Effect.runPromise( + runGuardTest( + { instanceProject: "project-1", failPause: { remaining: 99 } }, + ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create(vetoedGateGraph("Resync hold", "resync-hold")) + const { gate } = yield* takeBootPrompts(childPrompts) + expect(gate.title).toBe("gate") + yield* dag.nodeCompleted(dagID, "gate", { verdict: "replan", findings: "vetoed" }) + expect((yield* store.getWorkflow(dagID))?.status).toBe("running") + // Completing the unrelated probe lands in the NodeCompleted + // handler, whose prologue re-syncs paused from the DURABLE row + // ("running") and then calls spawnReady — the re-sync must not + // lift the veto hold. + yield* dag.nodeCompleted(dagID, "probe", "probe done") + yield* Effect.sleep("300 millis") + expect((yield* store.getNode(dagID, "downstream"))?.status).toBe("pending") + }), + ), + ) + }) + + it("a parent replan releases the hold and the corrective path spawns (DAG-03 / F2)", async () => { + await Effect.runPromise( + runGuardTest( + { instanceProject: "project-1", failPause: { remaining: 99 } }, + ({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create(vetoedGateGraph("Replan release", "replan-release")) + const { gate } = yield* takeBootPrompts(childPrompts) + expect(gate.title).toBe("gate") + yield* dag.nodeCompleted(dagID, "gate", { verdict: "replan", findings: "vetoed" }) + expect((yield* store.getWorkflow(dagID))?.status).toBe("running") + // Parent disposition: replan adds a corrective node off the + // (terminal) checkpoint — exempt from the merged checkpoint gate. + yield* dag.replan(dagID, { nodes: [node({ id: "corrective", name: "corrective", depends_on: ["gate"] })] }) + // The WorkflowReplanned handler releases the hold and re-syncs + // flags from the durable row, so the corrective node spawns. The + // wake prompt may interleave; drain until the corrective prompt. + const corrective = yield* Effect.gen(function* () { + for (let i = 0; i < 4; i++) { + const next = yield* takeWithin(childPrompts, `prompt ${i} after replan never arrived`) + if (next.title === "corrective") return next + } + return yield* Effect.fail(new Error("corrective node did not spawn after the releasing replan")) + }) + expect(corrective.title).toBe("corrective") + yield* Deferred.succeed(corrective.release, "fixed") + }), + ), + ) + }) +}) diff --git a/packages/opencode/test/dag/dag-node-supervision.test.ts b/packages/opencode/test/dag/dag-node-supervision.test.ts new file mode 100644 index 0000000000..a12681bd0a --- /dev/null +++ b/packages/opencode/test/dag/dag-node-supervision.test.ts @@ -0,0 +1,473 @@ +// oxlint-disable typescript-eslint/no-unsafe-type-assertion -- The incident +// harness deliberately mirrors dag-loop-guards.test.ts: mocked service layers +// and seeded row fixtures use `as never` type shims (mock objects implement +// only the interface slice the scenario exercises). The shims are type-only; +// converting them would fork the template's shape without changing behavior. +// oxlint-disable eslint/no-unused-vars -- gate objects are taken for their +// readiness side effect (takeWithin), not their value. +import { describe, expect, it } from "bun:test" +import { Deferred, Effect, Layer, Option, Queue } from "effect" +import type { SessionV1 } from "@opencode-ai/core/v1/session" +import { Database } from "@opencode-ai/core/database/database" +import { DagProjector } from "@opencode-ai/core/dag/projector" +import { DagStore } from "@opencode-ai/core/dag/store" +import { EventV2 } from "@opencode-ai/core/event" +import { ProjectTable } from "@opencode-ai/core/project/sql" +import { SessionTable } from "@opencode-ai/core/session/sql" +import { Agent } from "@/agent/agent" +import { Dag, type NodeConfig } from "@/dag/dag" +import { DagLoop } from "@/dag/runtime/loop" +import { InstanceRef } from "@/effect/instance-ref" +import { disposeInstance } from "@/effect/instance-registry" +import { DagSupervisionSweep } from "@/dag/runtime/supervision-sweep" +import { EventV2Bridge } from "@/event-v2-bridge" +import { SessionPrompt } from "@/session/prompt" +import { MessageID } from "@/session/schema" +import { Session } from "@/session/session" +import { SessionStatus } from "@/session/status" +import { pollWithTimeout } from "../lib/effect" +import { withIdleAdmission } from "../lib/session-prompt" + +// Production-incident harness (2026-08-18, dag_fe5feabfcae607fqVdRh47lN1B): +// a coding node's child session LLM stream died silently mid-turn; the node +// stayed `running` past its deadline for 7.5+ hours with escalation_pending=0 +// and timeout_extensions=0 — the deadline watcher never fired while the host +// process stayed alive. This harness reproduces the supervision shape at +// 2-second deadlines and asserts the invariant the incident violated: +// +// A running node past its deadline must leave `running` (escalate or fail) +// within a bounded window — no matter HOW the surrounding fibers die. +// +// Modes cover the candidate death paths: +// stream-hang — the child prompt never resolves (incident shape) +// dispose-instance — the per-directory instance scope closes mid-run +// healthy — control: the watcher fires normally + +interface PromptGate { + readonly title: string + readonly release: Deferred.Deferred +} + +function node(overrides: Partial = {}): NodeConfig { + return { + id: "n1", + name: "Node 1", + worker_type: "build", + depends_on: [], + required: true, + prompt_template: { inline: "work" }, + ...overrides, + } +} + +function takeWithin(queue: Queue.Queue, message: string) { + return Queue.take(queue).pipe( + Effect.timeoutOption("2 seconds"), + Effect.flatMap(Option.match({ onNone: () => Effect.fail(new Error(message)), onSome: Effect.succeed })), + ) +} + +function reply(sessionID: string, text: string): SessionV1.WithParts { + return { + info: { + id: MessageID.ascending(), + sessionID, + role: "assistant", + time: { created: Date.now() }, + }, + parts: [{ type: "text", text }], + } as never +} + +function supervisionLayer(input: { + readonly childPrompts: Queue.Queue + readonly cancels: string[] + readonly cancelDefect?: boolean +}) { + const database = Database.layerFromPath(":memory:") + const events = EventV2.layer.pipe(Layer.provide(database)) + const bridge = EventV2Bridge.layer.pipe(Layer.provide(events)) + const store = DagStore.layer.pipe(Layer.provide(database)) + const status = SessionStatus.layer.pipe(Layer.provide(bridge)) + const projector = DagProjector.layer.pipe(Layer.provide(events), Layer.provide(database)) + const dag = Dag.layer.pipe(Layer.provide(bridge), Layer.provide(store)) + const base = Layer.mergeAll(database, events, bridge, store, projector, dag, status) + const childTitles = new Map() + const created: string[] = [] + const session = Layer.mock(Session.Service, { + get: () => Effect.succeed({ id: "ses_parent", permission: [], agent: "build" } as never), + create: (value) => + Effect.sync(() => { + const id = `ses_child_${created.length + 1}` + created.push(id) + childTitles.set(id, (value?.title ?? id).replace(" (DAG node)", "")) + return { id } as never + }), + messages: () => Effect.succeed([]), + }) + const deliver = Effect.fn("test.SessionPrompt.deliver")(function* (value: SessionPrompt.PromptInput) { + const sessionID = value.sessionID as string + const release = yield* Deferred.make() + yield* Queue.offer(input.childPrompts, { + title: childTitles.get(sessionID) ?? sessionID, + release, + }) + return reply(sessionID, yield* Deferred.await(release)) + }) + const prompt = Layer.mock( + SessionPrompt.Service, + withIdleAdmission({ + // cancelDefect simulates the production sweep context: the layer-scoped + // fiber has no ambient InstanceRef, so a REAL SessionPrompt.cancel dies + // at InstanceState.context with exactly this defect. + cancel: input.cancelDefect + ? () => Effect.die(new Error("InstanceRef not provided")) + : (sessionID: string) => + Effect.sync(() => { + input.cancels.push(sessionID) + }), + prompt: (value: SessionPrompt.PromptInput) => deliver(value), + promptIfIdle: (value: SessionPrompt.PromptInput) => deliver(value).pipe(Effect.map(Option.some)), + }), + ) + const agent = Layer.mock(Agent.Service, { + get: () => + Effect.succeed({ + name: "build", + mode: "all", + permission: [], + options: {}, + description: "", + prompt: "", + model: { providerID: "test" as never, modelID: "test-model" as never }, + tools: {}, + hooks: {}, + }), + }) + const loop = DagLoop.layer.pipe(Layer.provide(base), Layer.provide(session), Layer.provide(prompt), Layer.provide(agent)) + const sweep = DagSupervisionSweep.layerWithoutDeps.pipe(Layer.provide(base), Layer.provide(prompt)) + return Layer.merge(Layer.merge(base, loop), sweep) +} + +interface SupervisionServices { + readonly dag: Dag.Interface + readonly loop: DagLoop.Interface + readonly store: DagStore.Interface + readonly sweep: import("@/dag/runtime/supervision-sweep").Interface + readonly childPrompts: Queue.Queue + readonly cancels: string[] + readonly database: Database.Interface +} +function runSupervisionTest( + options: { readonly instanceProject: string; readonly cancelDefect?: boolean }, + test: (services: SupervisionServices) => Effect.Effect, +) { + return Effect.gen(function* () { + const childPrompts = yield* Queue.unbounded() + const cancels: string[] = [] + return yield* Effect.gen(function* () { + const dag = yield* Dag.Service + const loop = yield* DagLoop.Service + const store = yield* DagStore.Service + const sweep = yield* DagSupervisionSweep.Service + const database = yield* Database.Service + for (const project of ["project-1", "project-2"]) { + yield* database.db + .insert(ProjectTable) + .values({ id: project as never, worktree: process.cwd() as never, sandboxes: [] }) + .run() + .pipe(Effect.orDie) + yield* database.db + .insert(SessionTable) + .values({ + id: `ses_${project}` as never, + project_id: project as never, + slug: project, + directory: process.cwd() as never, + title: `Parent of ${project}`, + version: "test", + }) + .run() + .pipe(Effect.orDie) + } + yield* loop.init() + return yield* test({ dag, loop, store, sweep, childPrompts, cancels, database }) + }).pipe( + Effect.provide(supervisionLayer({ childPrompts, cancels, cancelDefect: options.cancelDefect })), + Effect.provideService(InstanceRef, { + directory: process.cwd(), + worktree: process.cwd(), + project: { id: options.instanceProject }, + } as never), + Effect.scoped, + ) + }) +} + +// Shared graph: one coding node with a 2-second worker timeout. nodeTimeoutMs +// default would make the deadline 10 minutes — unusable for a test. The cap +// is pinned to 1 escalation so cap enforcement lands inside the test window. +const incidentGraph = { + projectID: "project-1", + sessionID: "ses_project-1", + title: "incident", + config: { + name: "incident", + max_timeout_extensions: 1, + nodes: [node({ id: "worker", name: "worker", worker_config: { timeout_ms: 2_000 } })], + }, +} + +// The incident invariant, as a poll predicate: the node must leave `running` +// (any terminal status, or escalated-but-running counts as progress only if +// extensions climb — the incident had BOTH frozen at zero, so we assert on +// status change OR timeout_extensions > 0). +const supervisionProgress = (store: DagStore.Interface, dagID: string, nodeID: string) => + Effect.gen(function* () { + const row = yield* store.getNode(dagID, nodeID) + if (!row) return undefined + if (row.status !== "running") return row + if (row.timeoutExtensions > 0) return row + return undefined + }) + +// bun's default per-test timeout is 5s; several bodies here need 8-40s at a +// 2s deadline — run this file with --timeout 30000 (the CI suite default). +describe("DAG node supervision — deadline enforcement (production incident)", () => { + it("healthy: a node past its deadline gets escalated by the watcher", async () => { + await Effect.runPromise( + runSupervisionTest({ instanceProject: "project-1" }, ({ dag, store, childPrompts, cancels }) => + Effect.gen(function* () { + const dagID = yield* dag.create(incidentGraph) + const child = yield* takeWithin(childPrompts, "worker did not start") + // Leave the prompt unresolved past the 2s deadline: the watcher + // must escalate (timeout_extensions climbs), then exhaust the cap + // and force-cancel the child. + yield* pollWithTimeout( + supervisionProgress(store, dagID, "worker"), + "watcher never escalated a node past its deadline (healthy control)", + "8 seconds", + ) + // Cap enforcement: this graph pins max_timeout_extensions to 1 — + // after one escalation the watcher cancels the child and fails the + // node. + yield* pollWithTimeout( + Effect.gen(function* () { + const row = yield* store.getNode(dagID, "worker") + return row?.status === "failed" ? row : undefined + }), + "watcher never cap-enforced (cancel + nodeFailed(timeout))", + "30 seconds", + ) + expect(cancels.length).toBeGreaterThan(0) + }), + ), + ) + }) + + it("stream-hang: the incident shape — prompt never resolves, node still must not rot in running", async () => { + await Effect.runPromise( + runSupervisionTest({ instanceProject: "project-1" }, ({ dag, store, childPrompts }: SupervisionServices ) => + Effect.gen(function* () { + const dagID = yield* dag.create(incidentGraph) + const child = yield* takeWithin(childPrompts, "worker did not start") + // Incident shape: the LLM stream died — the prompt gate is never + // released and never errors. Supervision must still progress. + void child + yield* pollWithTimeout( + supervisionProgress(store, dagID, "worker"), + "node rotted in running past its deadline with zero supervision progress (incident)", + "8 seconds", + ) + }), + ), + ) + }) + + // H5 (instance-scope harvest): closing the per-directory instance state + // mid-run interrupts every fiber forked into its scope — the DagLoop + // subscriptions, the spawn execution fiber, AND the deadline watcher — + // without touching the durable row. The production signature (a node stuck + // in running with escalation frozen at zero for hours while the host kept + // logging) is only reachable if supervision dies silently this way. + it("dispose-instance: instance teardown mid-run freezes durable supervision (incident mechanism)", async () => { + await Effect.runPromise( + runSupervisionTest({ instanceProject: "project-1" }, ({ dag, store, sweep, childPrompts }: SupervisionServices) => + Effect.gen(function* () { + const dagID = yield* dag.create(incidentGraph) + const child = yield* takeWithin(childPrompts, "worker did not start") + void child + // Wait for the first escalation so we know supervision was live. + yield* pollWithTimeout( + supervisionProgress(store, dagID, "worker"), + "watcher never escalated before dispose", + "8 seconds", + ) + const extensionsAtDispose = (yield* store.getNode(dagID, "worker"))?.timeoutExtensions ?? 0 + // Dispose the instance (the production candidate: lifecycle/ + // directory cleanup) — silently reaps every in-scope fiber. + yield* Effect.promise(() => disposeInstance(process.cwd())) + // Give any surviving supervision ample time to escalate again. + yield* Effect.sleep("4 seconds") + const row = yield* store.getNode(dagID, "worker") + // The frozen-supervision signature: still running, extensions + // frozen at the dispose-time value, no cap enforcement. + expect(row?.status).toBe("running") + expect(row?.timeoutExtensions).toBe(extensionsAtDispose) + + // The fallback-retry contract (production fix): the HOST-LEVEL + // supervision sweep — whose fiber lives in the layer scope and + // survives the instance teardown — settles the frozen node once + // its counter has stayed flat for frozenTicksNeeded(2s) = 2 ticks + // (dead supervision; a live 2s-cadence watcher always moves the + // counter inside the window). + const needed = DagSupervisionSweep.frozenTicksNeeded(2_000) + for (let tick = 0; tick <= needed; tick++) { + yield* sweep.sweepOnce() + yield* Effect.sleep("50 millis") + } + const swept = yield* pollWithTimeout( + Effect.gen(function* () { + const settled = yield* store.getNode(dagID, "worker") + return settled && settled.status !== "running" ? settled : undefined + }), + "host-level sweep never settled the node with dead supervision (fallback retry)", + "5 seconds", + ) + expect(swept?.errorClass).toBe("timeout") + }), + ), + ) + }) + + // Review R4 issue 1 (P0): the sweep's layer-scoped fiber has no ambient + // InstanceRef, so a real SessionPrompt.cancel DIES at + // InstanceState.context — and cancel's channel is E=never, where + // Effect.ignore recovers nothing. A cause-level recovery on that seam is + // what keeps the durable settle reachable in production. Simulated here by + // mocking cancel to the exact production defect. + it("cancel-defect: a dying cancel seam (production: no ambient InstanceRef) never blocks the settle", async () => { + await Effect.runPromise( + runSupervisionTest({ instanceProject: "project-1", cancelDefect: true }, ({ dag, store, sweep, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create(incidentGraph) + const child = yield* takeWithin(childPrompts, "worker did not start") + void child + yield* pollWithTimeout( + supervisionProgress(store, dagID, "worker"), + "watcher never escalated before dispose", + "8 seconds", + ) + yield* Effect.promise(() => disposeInstance(process.cwd())) + yield* Effect.sleep("4 seconds") + // Deadline passed, watcher dead: every settle-attempt pass hits the + // dying cancel seam first. Pre-fix, the defect aborts sweepOnce + // before nodeFailed; post-fix the settle still lands. + const needed = DagSupervisionSweep.frozenTicksNeeded(2_000) + for (let tick = 0; tick <= needed; tick++) { + yield* sweep.sweepOnce() + yield* Effect.sleep("50 millis") + } + const swept = yield* pollWithTimeout( + Effect.gen(function* () { + const settled = yield* store.getNode(dagID, "worker") + return settled && settled.status !== "running" ? settled : undefined + }), + "sweep never settled past a dying cancel seam", + "5 seconds", + ) + expect(swept?.errorClass).toBe("timeout") + expect(swept?.errorReason).toContain("swept") + }), + ), + ) + }) + + // Review R1 issue 2 (false-positive kill): a LIVE watcher on a node whose + // escalation cadence spans multiple sweep intervals must never be swept — + // the counter legitimately stays flat between escalations. The graph keeps + // the default cap (20) so the watcher's ladder is the intended path; the + // sweep passes run alongside a live watcher for well over the freeze + // window, and the settle that eventually lands must carry the WATCHER's + // own cap reason, never the sweep's. + it("freeze window: a live watcher is never swept — only its own cap enforcement ends the node", async () => { + await Effect.runPromise( + runSupervisionTest({ instanceProject: "project-1" }, ({ dag, store, sweep, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + ...incidentGraph, + config: { ...incidentGraph.config, max_timeout_extensions: 3 }, + }) + const child = yield* takeWithin(childPrompts, "worker did not start") + void child + // Supervision alive: the watcher escalates on its 2s cadence. + yield* pollWithTimeout( + supervisionProgress(store, dagID, "worker"), + "watcher never escalated before the streak test", + "8 seconds", + ) + const needed = DagSupervisionSweep.frozenTicksNeeded(2_000) + // Sweep passes at the PRODUCTION cadence relationship: each pass is + // spaced just past the node's 2s escalate interval, so the live + // watcher moves the counter between every pass and the streak can + // never reach `needed`. (Spacing the passes closer than the + // escalate interval would defeat the window's math — a live + // watcher's counter is legitimately flat for up to one full + // interval.) + for (let tick = 0; tick < needed + 2; tick++) { + yield* Effect.sleep("2.3 seconds") + yield* sweep.sweepOnce() + } + const row = yield* store.getNode(dagID, "worker") + // Either still running (ladder ongoing) or terminalized by the + // watcher's OWN cap — never by the sweep. + if (row?.status === "failed") { + expect(row?.errorReason).toContain("timeout extensions exhausted") + } else { + expect(row?.status).toBe("running") + } + }), + ), + ) + }) +}) + +describe("DagSupervisionSweep cadence derivation (pure)", () => { + it("derives the cadence from the node's persisted timeout_ms", () => { + const config = JSON.stringify({ nodes: [{ id: "worker", depends_on: [], worker_config: { timeout_ms: 30_000 } }] }) + expect(DagSupervisionSweep.escalateIntervalFromConfig(config, "worker")).toBe(30_000) + }) + + it("floors sub-second timeouts to the watcher's 1s minimum", () => { + const config = JSON.stringify({ nodes: [{ id: "worker", depends_on: [], worker_config: { timeout_ms: 10 } }] }) + expect(DagSupervisionSweep.escalateIntervalFromConfig(config, "worker")).toBe(1_000) + }) + + it("degrades to the DEFAULT cadence on absent row, malformed JSON, shape-divergent rows, or missing node", () => { + const expected = Dag.DEFAULT_WORKFLOW_CONFIG.nodeTimeoutMs + // No workflow row at all. + expect(DagSupervisionSweep.escalateIntervalFromConfig(undefined, "worker")).toBe(expected) + // Corrupt JSON string, and JSON whose root is not a record. + expect(DagSupervisionSweep.escalateIntervalFromConfig("{not json", "worker")).toBe(expected) + expect(DagSupervisionSweep.escalateIntervalFromConfig("null", "worker")).toBe(expected) + // Shape-divergent rows: nodes not an array / null entries. + expect(DagSupervisionSweep.escalateIntervalFromConfig(JSON.stringify({ nodes: null }), "worker")).toBe(expected) + expect(DagSupervisionSweep.escalateIntervalFromConfig(JSON.stringify({ nodes: [null] }), "worker")).toBe(expected) + // Node absent from the config, or present without worker_config.timeout_ms. + const other = JSON.stringify({ nodes: [{ id: "other", depends_on: [], worker_config: { timeout_ms: 30_000 } }] }) + expect(DagSupervisionSweep.escalateIntervalFromConfig(other, "worker")).toBe(expected) + const bare = JSON.stringify({ nodes: [{ id: "worker", depends_on: [] }] }) + expect(DagSupervisionSweep.escalateIntervalFromConfig(bare, "worker")).toBe(expected) + }) + + it("freeze window boundaries: one interval of flat plus one tick, at every configured timeout", () => { + expect(DagSupervisionSweep.frozenTicksNeeded(1)).toBe(2) + expect(DagSupervisionSweep.frozenTicksNeeded(60_000)).toBe(2) + expect(DagSupervisionSweep.frozenTicksNeeded(60_001)).toBe(3) + // The default 10-minute cadence and the doc-recommended 30-minute + // verifier timeout — a live watcher at each cadence always moves the + // counter inside the window. + expect(DagSupervisionSweep.frozenTicksNeeded(Dag.DEFAULT_WORKFLOW_CONFIG.nodeTimeoutMs)).toBe(11) + expect(DagSupervisionSweep.frozenTicksNeeded(1_800_000)).toBe(31) + }) +}) diff --git a/packages/opencode/test/dag/dag-replan-stale-nodefailed.test.ts b/packages/opencode/test/dag/dag-replan-stale-nodefailed.test.ts index 6c9de51b77..c952761549 100644 --- a/packages/opencode/test/dag/dag-replan-stale-nodefailed.test.ts +++ b/packages/opencode/test/dag/dag-replan-stale-nodefailed.test.ts @@ -397,9 +397,11 @@ describe("DagLoop replan vs stale NodeFailed", () => { expect(gateB.title).toBe("b") // Restart b mid-flight, rewiring its dependency from a → c (new node). + // DAG-02: c is a fresh reporting checkpoint, so the rewired dependent + // must gate on its output (the merged checkpoint check at replan). const plan = yield* dag.replan(dagID, { nodes: [ - { ...node("b", ["c"]), restart: true }, + { ...node("b", ["c"]), restart: true, condition: 'c.output == "done"' }, node("c"), ], }) diff --git a/packages/opencode/test/dag/dag-rev-view.test.ts b/packages/opencode/test/dag/dag-rev-view.test.ts index 34867d29ff..97be991771 100644 --- a/packages/opencode/test/dag/dag-rev-view.test.ts +++ b/packages/opencode/test/dag/dag-rev-view.test.ts @@ -306,8 +306,15 @@ describe("Train A rev-view (durable data untouched, view = current revision only // Bypass C: new suffix E→G→H off B; D (pending) is dropped by the // fragment and cancels; C (terminal failed, absent from fragment) is // the replaced segment the view must hide. + // DAG-02: E/G are fresh reporting checkpoints, so their new + // dependents gate on their outputs (the merged checkpoint check); + // the E-on-B edge is exempt because B already completed. const plan = yield* dag.replan(dagID, { - nodes: [node("e", ["b"]), node("g", ["e"]), node("h", ["g"])], + nodes: [ + node("e", ["b"]), + { ...node("g", ["e"]), condition: 'e.output == "e done"' }, + { ...node("h", ["g"]), condition: 'g.output == "g done"' }, + ], }) expect(plan.cancel).toEqual(["d"]) expect(plan.add.sort()).toEqual(["e", "g", "h"]) diff --git a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts index 15da4955fa..1353cb87ff 100644 --- a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts +++ b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts @@ -1,7 +1,8 @@ import { describe, expect } from "bun:test" -import { DateTime, Deferred, Effect, Layer } from "effect" +import { Cause, DateTime, Deferred, Effect, Layer } from "effect" import { DagStore, type WorkflowRow, type WorkflowSummary } from "@opencode-ai/core/dag/store" import { DagEvent } from "@opencode-ai/schema/dag-event" +import { logLines } from "effect/testing/TestConsole" import { EventV2Bridge } from "@/event-v2-bridge" import { DagSummaryPublisher } from "@/dag/runtime/summary-publisher" import { GlobalBus } from "@/bus/global" @@ -19,6 +20,9 @@ interface SummaryEmission { interface StoreControl { failures: number failuresAfterGate: number + /** DAG-04: fail the summary read with an interrupt cause (the shape a + * scoped disposal delivers mid-publish). */ + interruptRead?: boolean readGate?: { started: Deferred.Deferred release: Deferred.Deferred @@ -91,6 +95,11 @@ function runtime(state: StoreControl, bus: EventControl) { getWorkflowSummaries: (sessionID) => Effect.gen(function* () { state.reads.set(sessionID, (state.reads.get(sessionID) ?? 0) + 1) + if (state.interruptRead) { + // The defined fiber id matters: Cause.interruptors() only collects + // defined ids (the F1 shape pinned by the goal e2e interrupt tests). + return yield* Effect.failCause(Cause.interrupt(0)) + } if (state.failures > 0) { state.failures -= 1 throw new Error("simulated summary read failure") @@ -509,3 +518,30 @@ describe("DagSummaryPublisher behavior", () => { ).pipe(Effect.provide(runtime(state, bus))) }) }) + +// DAG-04 (#316): the coalescer deliberately rethrows interrupt causes (a +// scoped disposal mid-publish must unwind, not be mistaken for a failure). +// The outer listener boundary must preserve that: pre-fix its catchCause +// swallowed the interrupt and logged a spurious "failed to publish +// summaries" on every normal shutdown. F1 discipline, same as spawn.ts. +describe("DagSummaryPublisher interrupt discipline (DAG-04)", () => { + it.instance("an interrupt cause from the read path is rethrown, not reported as a publish failure", () => { + const state = control() + const bus = {} satisfies EventControl + state.interruptRead = true + state.sessions.set("dag-interrupt", "ses-interrupt") + state.summaries.set("ses-interrupt", [summary("dag-interrupt", 1)]) + + return Effect.gen(function* () { + yield* (yield* DagSummaryPublisher.Service).init() + yield* publishNodeEvents(bus, "dag-interrupt", 1) + // Wait for the coalesce window to run the (interrupted) read. + yield* pollWithTimeout( + Effect.sync(() => (state.reads.get("ses-interrupt") === 1 ? true : undefined)), + "interrupted summary read never ran", + ) + yield* Effect.sleep("150 millis") + expect(JSON.stringify(yield* logLines)).not.toContain("failed to publish summaries") + }).pipe(Effect.provide(runtime(state, bus))) + }) +}) diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index 0ba2cf7eb9..5635ac8dad 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 @@ -1377,7 +1377,23 @@ 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. + // (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)), @@ -1731,7 +1747,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 +1781,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,28 +1802,33 @@ 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") }), ) }) -// 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 +1914,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 +1930,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/packages/opencode/test/goal/goal.test.ts b/packages/opencode/test/goal/goal.test.ts index ef65f9babc..876070ee07 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) + }), + ) }) // --------------------------------------------------------------------------- diff --git a/packages/opencode/test/goal/turn-scope.test.ts b/packages/opencode/test/goal/turn-scope.test.ts index 99dde397d5..184b0dd53c 100644 --- a/packages/opencode/test/goal/turn-scope.test.ts +++ b/packages/opencode/test/goal/turn-scope.test.ts @@ -1,11 +1,15 @@ 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 { logLines } from "effect/testing/TestConsole" import { testEffect } from "../lib/effect" // GOAL-TURN-SCOPE regression tests: the turn-provenance mark (kick / @@ -18,10 +22,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 +107,72 @@ 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) + }), + ) + + // 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.pause(sid, "auto-paused") + yield* goal.markTurnDriven(sid) + + 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") + expect(JSON.stringify(yield* logLines)).not.toContain("failed after retries") + }), + ) + it.live("terminal transitions clear the mark (markDone)", () => Effect.gen(function* () { const goal = yield* Goal.Service diff --git a/packages/opencode/test/memory/memory.test.ts b/packages/opencode/test/memory/memory.test.ts index 2f0a9e4b3c..9b711d3897 100644 --- a/packages/opencode/test/memory/memory.test.ts +++ b/packages/opencode/test/memory/memory.test.ts @@ -29,7 +29,7 @@ import { MCP } from "@/mcp" import { Skill } from "@/skill" import { SystemPrompt } from "@/session/system" import { tmpdirScoped } from "../fixture/fixture" -import { pollWithTimeout, testEffect } from "../lib/effect" +import { awaitWithTimeout, pollWithTimeout, testEffect } from "../lib/effect" import { ProviderTest } from "../fake/provider" const config = { @@ -317,6 +317,9 @@ function recallFixture() { config: MemorySchema.Config projectInitialized: number matcher?: (query: string) => Effect.Effect + maintenanceHook?: () => Effect.Effect + /** Parks the runner's pre-select topics read (interrupt-window probe). */ + parkReads?: { started: Deferred.Deferred; release: Deferred.Deferred } } = { queries: [], reads: 0, @@ -367,6 +370,7 @@ function recallFixture() { return { topic_ids: query.includes("架构") ? [state.topics[0]?.id] : [] } } state.maintenance++ + if (state.maintenanceHook) return yield* state.maintenanceHook() return { actions: [{ type: "no_change" }] } }), }), @@ -374,8 +378,12 @@ function recallFixture() { MemoryLock.defaultLayer, Layer.mock(MemoryStore.Service, { readTopics: () => - Effect.sync(() => { + Effect.gen(function* () { state.reads++ + if (state.parkReads) { + yield* Deferred.succeed(state.parkReads.started, undefined) + yield* Deferred.await(state.parkReads.release) + } return state.topics }), updateTopics: (_projectID, update) => @@ -410,6 +418,8 @@ function recallFixture() { state.config = config state.projectInitialized = 1 state.matcher = undefined + state.maintenanceHook = undefined + state.parkReads = undefined }, it: testEffect(layer), systemIt: testEffect(systemLayer), @@ -1268,13 +1278,70 @@ describe("memory turn-scoped retrieval", () => { ] yield* memory.prepare({ sessionID, messages }) - expect(recall.state.maintenance).toBe(1) + // Maintenance runs in the background after the render fence (issue + // #324): polling stands in for the old synchronous completion. + yield* pollWithTimeout( + Effect.sync(() => (recall.state.maintenance === 1 ? true : undefined)), + "due maintenance never ran in the background", + ) expect(recall.state.queries).not.toContain("再次讨论架构边界") expect(yield* memory.context(sessionID)).toEqual([]) }), { git: true }, ) + recall.it.instance( + "keeps the fence and project lock free while background maintenance streams", + () => + Effect.gen(function* () { + recall.reset() + recall.state.config = { ...config, turn_interval: 1 } + const memory = yield* Memory.Service + const sessionID = SessionID.make("ses_memory_maintenance_lock_free") + const firstID = MessageID.ascending() + const first = user(firstID, sessionID, "继续之前确认的架构边界") + + yield* memory.prepare({ sessionID, messages: [first] }) + const messages = [ + first, + { + info: assistant(firstID, sessionID, ProviderV2.ID.make("test"), ModelV2.ID.make("test-model"), "end_turn"), + parts: [], + }, + user(MessageID.ascending(), sessionID, "第二次架构讨论"), + ] + + const release = yield* Deferred.make() + recall.state.maintenanceHook = () => + Effect.gen(function* () { + yield* Deferred.await(release) + return { actions: [{ type: "no_change" }] } + }) + + const pending = yield* memory.prepare({ sessionID, messages }).pipe(Effect.forkChild) + yield* pollWithTimeout( + Effect.sync(() => (recall.state.maintenance >= 1 ? true : undefined)), + "due maintenance never reached the model call", + ) + + // The maintenance model call is in flight. Because prepare kicked it + // outside the fence (issue #324), a concurrent checkpoint — whose + // render select needs the same identity fence and project lock — is + // not starved; under the old inline shape it would wait on the fence + // until the streaming call finished. + const rendered = yield* awaitWithTimeout( + memory.checkpoint({ sessionID, messages }), + "checkpoint starved by background maintenance", + ) + expect(rendered.length).toBeGreaterThan(0) + + yield* Deferred.succeed(release, undefined) + yield* Fiber.join(pending) + expect(recall.state.maintenance).toBe(1) + }), + { git: true }, + ) + recall.it.instance( "discards a query that completes after a new real user turn starts", () => @@ -1322,6 +1389,179 @@ describe("memory turn-scoped retrieval", () => { }), { git: true }, ) + + // MEM-02 follow-up (acceptance): the cross-process identity fence must not + // be held across the SEARCH matcher model call — only the markMatched + // commit is fenced. While the matcher is parked mid-call, a concurrent + // checkpoint (whose render select needs the same identity fence) must not + // starve. Under the old shape the fence wrapped the whole lock block, so + // the checkpoint waited on the streaming matcher. + recall.it.instance( + "keeps the identity fence free while the search matcher streams", + () => + Effect.gen(function* () { + recall.reset() + const started = yield* Deferred.make() + const release = yield* Deferred.make() + // Only the SLOW search query parks; the checkpoint's own render match + // must sail through — that is the assertion. + recall.state.matcher = (query) => + query === "慢架构查询" + ? Effect.gen(function* () { + yield* Deferred.succeed(started, undefined) + yield* Deferred.await(release) + return { topic_ids: [recall.state.topics[0]?.id ?? ""] } + }) + : Effect.succeed({ topic_ids: [recall.state.topics[0]?.id ?? ""] }) + const memory = yield* Memory.Service + const sessionID = SessionID.make("ses_memory_search_fence_free") + const messages = [ + user(MessageID.ascending(), sessionID, "先处理当前问题"), + user(MessageID.ascending(), sessionID, "召回相关历史"), + ] + + const pending = yield* memory.search({ sessionID, messages, query: "慢架构查询" }).pipe(Effect.forkChild) + yield* Deferred.await(started) + + const rendered = yield* awaitWithTimeout( + memory.checkpoint({ sessionID, messages }), + "checkpoint starved by the search matcher — fence held across the model call (MEM-02)", + ) + expect(rendered.length).toBeGreaterThan(0) + + yield* Deferred.succeed(release, undefined) + expect(yield* Fiber.join(pending)).toEqual({ status: "attached", count: 1, reused: false }) + }), + { git: true }, + ) + + // MEM-01 follow-up (acceptance): same discipline for the prepare + // first-turn (shouldMatch) branch — the bounded matcher moved out of the + // fence/lock, so a parked first-turn matcher cannot starve the fence. + recall.it.instance( + "keeps the identity fence free while the first-turn prepare matcher streams", + () => + Effect.gen(function* () { + recall.reset() + const started = yield* Deferred.make() + const release = yield* Deferred.make() + // Only the FIRST matcher call (the first-turn prepare) parks; the + // concurrent checkpoint's render match — same text, so query-based + // discrimination is impossible — must sail through on its own call. + let matcherCalls = 0 + recall.state.matcher = () => + Effect.gen(function* () { + matcherCalls++ + if (matcherCalls > 1) return { topic_ids: [recall.state.topics[0]?.id ?? ""] } + yield* Deferred.succeed(started, undefined) + yield* Deferred.await(release) + return { topic_ids: [recall.state.topics[0]?.id ?? ""] } + }) + const memory = yield* Memory.Service + const sessionID = SessionID.make("ses_memory_prepare_fence_free") + const messages = [user(MessageID.ascending(), sessionID, "首次真实用户输入关于架构")] + + const pending = yield* memory.prepare({ sessionID, messages }).pipe(Effect.forkChild) + yield* Deferred.await(started) + + const rendered = yield* awaitWithTimeout( + memory.checkpoint({ sessionID, messages }), + "checkpoint starved by the first-turn prepare matcher — fence held across the model call (MEM-01)", + ) + expect(rendered.length).toBeGreaterThan(0) + + yield* Deferred.succeed(release, undefined) + yield* Fiber.join(pending) + // Both the first-turn prepare and the concurrent checkpoint render + // matched the same text; each recorded exactly its own call. + expect(recall.state.queries).toEqual(["首次真实用户输入关于架构", "首次真实用户输入关于架构"]) + }), + { git: true }, + ) + + // Review R-1: the runner must complete the in-flight deferred on EVERY + // exit. A failed first matcher call must not wedge the (session,key): a + // coalesced awaiter wakes (degraded) and a later identical query re-runs + // the matcher instead of parking on a leaked in-flight entry. + recall.it.instance( + "a failed first query never wedges the session key or its coalesced awaiter", + () => + Effect.gen(function* () { + recall.reset() + const started = yield* Deferred.make() + let matcherCalls = 0 + recall.state.matcher = () => + Effect.gen(function* () { + matcherCalls++ + if (matcherCalls === 1) { + yield* Deferred.succeed(started, undefined) + throw new Error("matcher exploded") + } + return { topic_ids: [recall.state.topics[0]?.id ?? ""] } + }) + const memory = yield* Memory.Service + const sessionID = SessionID.make("ses_memory_failed_first") + const messages = [ + user(MessageID.ascending(), sessionID, "先处理当前问题"), + user(MessageID.ascending(), sessionID, "召回相关历史"), + ] + + const failing = yield* memory.search({ sessionID, messages, query: "易碎查询" }).pipe(Effect.forkChild) + yield* Deferred.await(started) + // Concurrent identical query — either it coalesced onto the failing + // run (wakes degraded to "failed") or it raced past the in-flight + // window and re-runs (attached). Both are non-hang outcomes; a + // parked-forever awaiter or a wedged key fails the bounded waits. + const coalesced = yield* memory.search({ sessionID, messages, query: "易碎查询" }).pipe(Effect.forkChild) + const failingResult = yield* awaitWithTimeout(Fiber.join(failing), "failing query hung") + const coalescedResult = yield* awaitWithTimeout(Fiber.join(coalesced), "coalesced awaiter hung on the failing run") + expect(failingResult).toEqual({ status: "failed" }) + expect(["failed", "attached", "empty"]).toContain(coalescedResult.status) + // The (session,key) is NOT wedged: the next identical query resolves + // within the bounded window (fresh run or cached reuse from the + // coalesced fiber's successful re-run — either proves liveness). + expect(yield* awaitWithTimeout(memory.search({ sessionID, messages, query: "易碎查询" }), "later identical query wedged on the leaked in-flight entry")).toMatchObject({ + status: "attached", + }) + }), + { git: true }, + ) + + // Review R2 issue 1: the exit bracket must start at REGISTRATION, not at + // the select pipeline — an interrupt during the runner's pre-select topics + // read (a real async fs suspension in production) previously unwound + // before onExit attached, leaking the in-flight entry and wedging the + // (turn,key) forever. + recall.it.instance( + "an interrupted topics read releases the in-flight entry and never wedges the key", + () => + Effect.gen(function* () { + recall.reset() + const memory = yield* Memory.Service + const sessionID = SessionID.make("ses_memory_interrupted_read") + const messages = [ + user(MessageID.ascending(), sessionID, "先处理当前问题"), + user(MessageID.ascending(), sessionID, "召回相关历史"), + ] + const started = yield* Deferred.make() + const release = yield* Deferred.make() + recall.state.parkReads = { started, release } + + const runner = yield* memory.search({ sessionID, messages, query: "可中断查询" }).pipe(Effect.forkChild) + yield* Deferred.await(started) + yield* Fiber.interrupt(runner).pipe(Effect.ignore) + + // The entry must be released despite the interrupt landing before + // the select pipeline attached its bracket: a later identical query + // resolves within the bounded window instead of parking forever + // (empty: the query text matches no topic — liveness is the point). + yield* Deferred.succeed(release, undefined) + expect(yield* awaitWithTimeout(memory.search({ sessionID, messages, query: "可中断查询" }), "later identical query wedged on the entry leaked by the interrupted read")).toMatchObject({ + status: "empty", + }) + }), + { git: true }, + ) }) describe("memory project config Git exclusions", () => { diff --git a/packages/opencode/test/server/global-lifecycle.test.ts b/packages/opencode/test/server/global-lifecycle.test.ts new file mode 100644 index 0000000000..415c494fa0 --- /dev/null +++ b/packages/opencode/test/server/global-lifecycle.test.ts @@ -0,0 +1,99 @@ +import { describe, expect } from "bun:test" +import { Effect, Exit, Fiber, Layer } from "effect" +import * as TestClock from "effect/testing/TestClock" +import { logLines } from "effect/testing/TestConsole" +import { InstanceStore } from "@/project/instance-store" +import { GlobalLifecycle } from "@/server/global-lifecycle" +import { GlobalBus } from "@/bus/global" +import { it } from "../lib/effect" + +function collectDisposed() { + const events: string[] = [] + const handler = (event: { payload?: { type?: string } }) => { + if (event.payload?.type === "global.disposed") events.push(event.payload.type) + } + GlobalBus.on("event", handler) + return { events, stop: () => GlobalBus.off("event", handler) } +} + +const wedgedStoreLayer = Layer.mock(InstanceStore.Service, { + disposeAll: () => Effect.never, +}) + +// DAG-04 (#316): the production shutdown disposal was uninterruptible AND +// had no timeout — a wedged instance could hang the whole path forever (the +// httpapi exerciser already guards its cleanup steps with a 10s bounded +// guard; production had no equivalent). The dispose step is now bounded; a +// HANG is always abandoned (timeout → Option.none, never an error — the +// HttpApi dispose endpoint's error contract stays untouched), the Disposed +// event always lands, and genuine disposal failures still propagate for +// callers that do not swallow. +describe("GlobalLifecycle bounded disposal (DAG-04)", () => { + it.effect("a wedged disposeAll is abandoned at the bounded timeout and the Disposed event still lands (swallow)", () => + Effect.acquireUseRelease( + Effect.sync(collectDisposed), + (collector) => + Effect.gen(function* () { + const fiber = yield* GlobalLifecycle.disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }).pipe( + Effect.forkScoped, + ) + yield* TestClock.adjust("11 seconds") + yield* Fiber.await(fiber) + expect(collector.events).toEqual(["global.disposed"]) + expect(JSON.stringify(yield* logLines)).toContain("global disposal timed out") + }).pipe(Effect.provide(wedgedStoreLayer)), + (collector) => Effect.sync(collector.stop), + ), + ) + + it.effect("a wedged disposeAll never hangs the non-swallow caller either", () => + Effect.acquireUseRelease( + Effect.sync(collectDisposed), + (collector) => + Effect.gen(function* () { + const fiber = yield* GlobalLifecycle.disposeAllInstancesAndEmitGlobalDisposed().pipe(Effect.forkScoped) + yield* TestClock.adjust("11 seconds") + const exit = yield* Fiber.await(fiber) + // A hang is abandonment, not failure — the caller completes and the + // Disposed event lands (pre-fix this path hung forever). + expect(Exit.isSuccess(exit)).toBe(true) + expect(collector.events).toEqual(["global.disposed"]) + }).pipe(Effect.provide(wedgedStoreLayer)), + (collector) => Effect.sync(collector.stop), + ), + ) + + it.effect("a genuine disposeAll failure still propagates when not swallowing", () => + Effect.gen(function* () { + const failing = Layer.mock(InstanceStore.Service, { + disposeAll: () => Effect.die(new Error("injected disposal defect")), + }) + const fiber = yield* GlobalLifecycle.disposeAllInstancesAndEmitGlobalDisposed().pipe( + Effect.provide(failing), + Effect.forkScoped, + ) + const exit = yield* Fiber.await(fiber) + expect(Exit.isFailure(exit)).toBe(true) + }), + ) + + it.effect("a healthy disposeAll completes without touching the timeout", () => + Effect.acquireUseRelease( + Effect.sync(collectDisposed), + (collector) => + Effect.gen(function* () { + let disposed = 0 + const healthy = Layer.mock(InstanceStore.Service, { + disposeAll: () => + Effect.sync(() => { + disposed += 1 + }), + }) + yield* GlobalLifecycle.disposeAllInstancesAndEmitGlobalDisposed().pipe(Effect.provide(healthy)) + expect(disposed).toBe(1) + expect(collector.events).toEqual(["global.disposed"]) + }), + (collector) => Effect.sync(collector.stop), + ), + ) +}) diff --git a/workflows/audit-fix-loop.md b/workflows/audit-fix-loop.md new file mode 100644 index 0000000000..f52e11e88d --- /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 运行链接已附。至此循环终止。