Skip to content

feat(goal): stop a Goal whose checkpoints stall three times in a row - #9975

Merged
wenshao merged 6 commits into
QwenLM:mainfrom
qqqys:goal/c4-compaction-breaker
Aug 25, 2026
Merged

feat(goal): stop a Goal whose checkpoints stall three times in a row#9975
wenshao merged 6 commits into
QwenLM:mainfrom
qqqys:goal/c4-compaction-breaker

Conversation

@qqqys

@qqqys qqqys commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a breaker for the one compaction loop #9835 left unbounded. A checkpoint is counted as stalled when it comes back holding the maximum number of claims while the evidence window it compacted was already truncated — compaction has two levers, folding evidence into claims and moving the cursor past what was folded, and that combination means the first lever is exhausted (the next checkpoint can only merge) and the second is not keeping up (eligible evidence was left behind uncatalogued). A busy turn that truncates while the claims still have room is not a stall, and a full claim list on a quiet Goal is not either; both levers have to be pinned at once.

The streak is persisted on the Goal record as checkpointStalls (absent means zero), so a daemon restart or session resume cannot launder it. Any check that finds room resets it — a checkpoint on an un-truncated window, or a check that needs no checkpoint at all — and so do edit and replace. After three consecutive stalls the Goal is settled as usage_limited with the existing limitKind: 'evidence_catalog' and a new reason that says what stalled, how many times, and what to do (narrow the objective via edit/replace before resuming). It goes through the same failure path as the other checkpoint bounds; no new GoalLimitKind, no branch in goalLimitKindForReason (new records always persist limitKind), so nothing crosses the wire and the pre-C3 Web Shell drift guard is untouched.

Why it's needed

Before #9835 a truncated window stopped the Goal; after it, the window compacts. That is right, but it removed the only terminator on the "compaction runs but gives no relief" path: a Goal whose evidence rate outruns the catalog now pays a checkpoint verifier call — a model call — every single turn, loses evidence every turn, and never converges on its own. CC bounds the analogous thrash with per-query-chain counters (3 and 3) that reset on every user turn; ours has to survive restarts, hence the record field. The threshold of three matches: one stalled checkpoint is a busy turn, two is a pattern, three is the loop.

Reviewer Test Plan

How to verify

  • cd packages/core && npx vitest run src/goals/ — 404 tests, 16 files (8 new). The runtime cases drive real windows through the checkpoint check: 101 records overflow the raw-entry budget (truncated), 60 compact without overflowing, 10 stay below threshold; the verifier fake answers with 32 claims.
  • Mutation probes run during development, each on goal-runtime + goal-reducer + goal-checkpoint (192 tests): remove the increment → 3 fail; remove the reset on an effective checkpoint → 1; remove the reset on a quiet check → 1; threshold >=> → 1; drop the key from parseGoalRecord → 1; never restore it → 1; predicate ignores truncation → 2; predicate ignores the claim cap → 1; edit stops resetting → 1. Every other test green in every run.
  • npx tsc --noEmit in packages/core: identical error count with the diff stashed (35, all pre-existing dependency skew outside src/goals/, 0 in goals). prettier + eslint clean on the seven changed files.

Evidence (Before & After)

N/A (runtime bound; the stop renders through the existing usage-limited surfaces).

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

N/A (unit tests only).

Risk & Scope

  • Main risk or tradeoff: a legitimately evidence-heavy Goal whose claims are full and whose every turn overflows the window will be stopped after three such turns. That is the intended reading — the catalog is provably not keeping up with that Goal and its evidence is already being lost each turn — and the reason tells the user to narrow or split the objective. The stopping checkpoint is discarded rather than written, since a restarted window begins from a fresh cursor anyway.
  • Not validated / out of scope: fix(goal): resume an evidence-limited Goal from a fresh window #9840 (resume of an evidence-limited Goal restarts the window) is unmerged on this base; once it lands, that resume path should also clear checkpointStalls, exactly as edit does here — a one-line follow-up. No UI, SDK, or CLI change.
  • Breaking changes / migration notes: none. Records without the field restore as zero; a persisted 0 restores as no field.

Linked Issues

中文说明

这个 PR 做了什么

#9835 留下的那个无界压缩循环加一个熔断。当一次 checkpoint 返回时已持有最大数量的 claim,而它所压缩的证据窗口本身已经被截断时,记为一次停滞——压缩有两个杠杆:把证据折叠进 claim,以及把游标移过已折叠的部分;这种组合意味着第一个杠杆已耗尽(下一次 checkpoint 只能合并),第二个杠杆也跟不上(合格证据被遗留在目录之外)。忙碌轮次在 claim 仍有空间时截断不算停滞,安静 Goal 的满 claim 列表也不算;两个杠杆必须同时卡死。

连续次数以 checkpointStalls 持久化在 Goal 记录上(缺省即零),daemon 重启或会话恢复都无法洗掉它。任何发现有空间的检查都会重置它——未截断窗口上的 checkpoint,或根本不需要 checkpoint 的检查——edit 与 replace 也会重置。连续三次停滞后,Goal 以现有的 limitKind: 'evidence_catalog' 落为 usage_limited,并带上一条新的 reason,说明什么停滞了、几次、该做什么(resume 前通过 edit/replace 收窄目标)。它走与其他 checkpoint 边界相同的失败路径;不新增 GoalLimitKind,不在 goalLimitKindForReason 加分支(新记录总是持久化 limitKind),因此没有任何东西跨越线上传输,C3 之前的 Web Shell drift guard 也不受影响。

为什么需要

#9835 之前,截断的窗口会停止 Goal;之后则会压缩。这是对的,但它移除了「压缩运行却无缓解」路径上唯一的终止器:证据产出速度超过目录容量的 Goal 现在每一轮都要付一次 checkpoint verifier 调用——一次模型调用——每轮丢证据,靠自己永远收敛不了。CC 用按查询链的计数器(3 和 3)约束类似的抖动,但那些计数器在每个用户轮次都会清零;我们的必须能撑过重启,因此放在记录字段上。阈值 3 与之一致:一次停滞是忙碌轮次,两次是模式,三次就是循环。

评审验证计划

如何验证

  • cd packages/core && npx vitest run src/goals/——404 个测试,16 个文件(新增 8 个)。runtime 用例驱动真实窗口经过 checkpoint 检查:101 条记录溢出原始条目预算(截断),60 条压缩但不溢出,10 条低于阈值;verifier 桩返回 32 条 claim。
  • 开发期间的变异检验,每个在 goal-runtime + goal-reducer + goal-checkpoint(192 个测试)上运行:去掉递增 → 3 个失败;去掉有效 checkpoint 上的重置 → 1;去掉安静检查上的重置 → 1;阈值 >=> → 1;从 parseGoalRecord 去掉字段 → 1;永不恢复它 → 1;谓词忽略截断 → 2;谓词忽略 claim 上限 → 1;edit 不再重置 → 1。每次运行其余测试全绿。
  • packages/corenpx tsc --noEmit:stash 掉 diff 后错误数相同(35,全是 src/goals/ 之外的已有依赖偏差,goals 内为 0)。七个改动文件 prettier + eslint 干净。

证据(前后对比)

N/A(运行时边界;停止状态通过现有的 usage-limited 界面渲染)。

已测试平台

Linux ✅;macOS / Windows ⚠️(CI 覆盖)。

环境(可选)

N/A(仅单元测试)。

风险与范围

  • 主要风险或权衡:一个证据量确实很大、claim 已满且每轮都溢出窗口的合法 Goal,会在三轮后被停止。这正是预期的解读——目录已被证明跟不上这个 Goal,而且它的证据每轮都在丢失——reason 会告诉用户收窄或拆分目标。触发停止的那次 checkpoint 被丢弃而不是写入,因为重启的窗口无论如何都从新游标开始。
  • 未验证/范围外:fix(goal): resume an evidence-limited Goal from a fresh window #9840(证据受限 Goal 的 resume 重开窗口)在此基线上尚未合入;它落地后,该 resume 路径也应清除 checkpointStalls,与此处 edit 的做法一致——一行的后续改动。无 UI、SDK 或 CLI 变更。
  • 破坏性变更/迁移说明:无。没有该字段的记录恢复为零;持久化的 0 恢复为无字段。

关联 Issue

QwenLM#9835 made a truncated evidence window compact instead of stopping the
Goal. That removed the only terminator on the path where compaction
runs and gives no relief: a Goal whose evidence rate outruns the
catalog pays a checkpoint verifier call every turn, loses evidence every
turn, and never converges on its own.

A checkpoint is counted as stalled when it comes back holding the
maximum number of claims while the window it compacted was already
truncated. Compaction has two levers, folding evidence into claims and
moving the cursor past what was folded; that combination means the
first is exhausted (the next checkpoint can only merge) and the second
is not keeping up (eligible evidence was left behind). A busy turn that
truncates with room in the claims is not a stall, and a full claim list
on a quiet Goal is not either.

The streak is persisted as `GoalRecord.checkpointStalls` (absent means
zero) so a restart or resume cannot launder it. Any check that finds
room resets it, and so do edit and replace. After three consecutive
stalls the Goal settles as `usage_limited` with the existing
`limitKind: 'evidence_catalog'` and a reason naming what stalled, how
many times, and what to do. No new limit kind, no branch in
`goalLimitKindForReason`, nothing crosses the wire.

Mutation probes (goal-runtime + goal-reducer + goal-checkpoint, 192
tests): no increment -> 3 fail; no reset on an effective checkpoint ->
1; no reset on a quiet check -> 1; threshold >= to > -> 1; parse drops
the key -> 1; parse never restores it -> 1; predicate ignores
truncation -> 2; predicate ignores the claim cap -> 1; edit stops
resetting -> 1. Every other test green in every run.
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qqqys

qqqys commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 25, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Re-run at the new head — the autofix rounds landed three things since the last pass: the resume reset (#9840 merged into the base), the error-class plumbing for unusable verifier results, and the orphaned-comment fix. Re-gating from scratch against f45ae6f3a1.

Template: complete, including the bilingual section. ✓

Problem: real, and now demonstrated rather than argued. After #9835 a truncated window compacts instead of stopping, and claims pinned at the cap while the window keeps overflowing leaves no exit: every turn pays a checkpoint verifier model call and still loses evidence, forever. The maintainer's independent A/B harness in this thread reproduces it live on the merge-base build — 19 checkpoint calls and still active — while this PR's build stops at the third stall. So the problem-existence question has direct runtime evidence, not just structural reasoning.

Direction: aligned. This is a core Goal-runtime bound in the same family as the existing catalog-exhausted and request-too-large stops, reusing their settle path and their resume semantics. The reference runtime bounds the analogous thrash the same way.

Size: all nine files are in packages/core/src/goals/ — 260 production lines (goal-runtime.ts 155, goal-checkpoint-verifier.ts 45, goal-checkpoint.ts 29, goal-protocol.ts 19, goal-reducer.ts 12) vs 519 test lines. feat type, below every threshold; nothing escalated.

Approach: still minimal. One pure predicate, one persisted record field, counter plumbing at the existing checkpoint settle points, reuse of the usage_limited / evidence_catalog failure path — no new limit kind, no wire change. The three additions since the last pass each answer a specific review finding or a disclosed follow-up; nothing drive-by.

Risk: no elevated signals — none of the changed files matches the revert-correlated paths.

Moving on to code review. 🔍

中文说明

在新 head 上重跑——自上次分诊后,autofix 轮次落地了三件事:resume 重置(#9840 已合入基线)、不可用 verifier 结果的错误类型接线、孤立注释修复。在 f45ae6f3a1 上重新走门禁。

**模板:**完整,含中文对照 ✓

**问题:**真实存在,且现在是被演示出来的而非仅靠推理。#9835 之后截断窗口会压缩而非停止;claim 顶到上限而窗口持续溢出时没有出口:每轮都付一次 checkpoint verifier 模型调用,同时还在丢证据,永不收敛。维护者在本帖中的独立 A/B 验证已在 merge-base 构建上实时复现——19 次 checkpoint 调用后仍然 active——而本 PR 的构建在第三次停滞时停止。问题存在性因此有直接的运行时证据,而不只是结构推理。

**方向:**对齐。这是 Goal 运行时边界,与既有的 catalog 耗尽、请求过大两个停止同族,复用它们的落定路径与 resume 语义。参考运行时对同类抖动的约束方式一致。

**规模:**九个文件全部位于 packages/core/src/goals/——260 行生产代码(goal-runtime.ts 155、goal-checkpoint-verifier.ts 45、goal-checkpoint.ts 29、goal-protocol.ts 19、goal-reducer.ts 12),519 行测试。feat 类型,低于所有阈值;无需升级。

**方案:**仍然最小化。一个纯谓词、一个持久化记录字段、在既有 checkpoint 落定点做计数接线、复用 usage_limited / evidence_catalog 失败路径——不新增 limit kind,不改线上传输。上次评审以来的三处新增各自对应一个具体的评审发现或已披露的后续项,没有顺手改动。

**风险:**无升级信号——改动文件均未命中与 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at f45ae6f3a1247642d7a9ed8e02dc6f008b541b7b · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Code review (re-run at f45ae6f3a1)

My independent proposal for this problem (persisted streak counter; stall = truncated window and claims pinned at the cap; reset on any check that finds room plus edit/replace; settle through the existing usage_limited/evidence_catalog path) is what the PR does. The new head also lands the two criticals the review loop raised, and I verified both fixes in the code, not just in the diff narrative:

  • Streak reset firing where it shouldn't (review round 1). finishCheckpointCheck now takes an outcome'room' resets, 'stalled' increments, default 'inconclusive' carries the streak. I walked every call site: 'room' only where the check needed no checkpoint, 'stalled' only for an unusable verifier result on an already-truncated window, and the empty-turn bookkeeping close keeps the streak (pinned by the records no evidence at all test). A transient verifier failure no longer launders the count — it proved nothing about the window, so it counts nothing.
  • Error class made load-bearing (review round 3). Unusable results are counted by instanceof InvalidGoalCheckpointError, so the parse moved out of runSideQuery's validate hook — which I confirmed in sideQuery.ts re-wraps hook failures into plain Errors, silently dropping the class. Every parse-level rejection now throws InvalidGoalCheckpointError, materializeGoalEvidenceCheckpoint already threw it for every unusable shape (empty claims, malformed claim, unknown source, byte cap), and a new test pins the class across three rejection shapes. validateGoalCheckpointVerifierText has no remaining consumers after its removal — grep-verified.
  • The orphaned JSDoc nit is fixedtakeTurnTokens owns its comment again; withCheckpointStalls follows the function body.

New since the last pass: the #9840 follow-up landed. The evidence-limited resume clears the streak (same reset edit performs, since it restarts the window), a paused resume keeps it (same window, same truth), and both are pinned by reducer tests. I checked the reducer structure directly: edit and the evidence-limited resume branch pass checkpointStalls: undefined into transitionGoal, the paused resume spreads only { status: 'active' }, so the field survives exactly where it should.

The rest of the machinery holds as before: the count happens after materialization, so a retryable failure cannot advance the streak; the stopping checkpoint is discarded, and settle persists the streak alongside limitKind: 'evidence_catalog' — safe because the resume gate reads limitKind as the field of record (verified in the reducer and in web-shell's goalGate.ts); persistence is strict (isNonNegativeInteger, -1/1.5 reject the snapshot, a persisted 0 restores as no field); and checkpointStalls appears nowhere outside packages/core/src/goals/, matching the SDK's existing narrower projection that already omits evidenceCheckpoint and tokensUsed — nothing crosses the wire.

Testing evidence

Per triage rules this run does not build or execute PR code. CI on the reviewed commit, fetched via the API: every check green, no failures — Test (ubuntu-latest, Node 22.x) (the unit suite) included. Test (macos-latest) / Test (windows-latest) are skipped by the repo's CI profile gating, not failures. The suite pins the change by design: the stall tests drive real windows through the real buildGoalEvidenceCheckpointWindow (101 records overflow, 60 compact, 10 stay quiet) and fail without the breaker.

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The central claim is behavioural, and it is substantiated beyond the suite: the maintainer's independent harness in this thread ran an A/B on the real compiled artifacts — the merge-base build pays 19+ checkpoint calls and stays active forever, this build settles usage_limited at the third stall (3 calls, 17 s); the streak survives a real process restart; a 31-claim control arm is never counted; and a mutant that degrades the error class to a plain Error brings the unbounded loop back, proving the class plumbing load-bearing. A sandboxed @qwen-code /verify run is already in flight on this head and will post its own report; the claim does not rest on the author's word.

Not verified here: this run executes no PR code (triage rules); live behaviour rests on the maintainer's in-thread harness and the pending /verify report. The author's mutation-probe numbers are their own claim, but the maintainer's independent 12-mutant sweep (all killed) covers the same ground.

中文说明

代码审查(在 f45ae6f3a1 上重跑):我独立写下的方案(持久化连续停滞计数;停滞 = 窗口截断且 claim 顶到上限;任何发现有空间的检查加 edit/replace 重置;经由现有 usage_limited/evidence_catalog 路径落定)与 PR 一致。新 head 还落地了评审循环提出的两个 Critical,我逐一在代码中核实了修复本身,而非只看 diff 叙述:

1)连续计数不该重置的路径(第 1 轮)。 finishCheckpointCheck 现在带 outcome 参数——'room' 重置、'stalled' 递增、缺省 'inconclusive' 保留计数。我走查了所有调用点:'room' 只出现在无需 checkpoint 的检查;'stalled' 只出现在已截断窗口上的不可用结果;空轮次的簿记关闭保留计数(有测试钉住)。verifier 瞬时失败不再洗掉计数——它对窗口什么都没证明,所以什么都不计。

2)错误类型成为承重墙(第 3 轮)。 不可用结果按 instanceof InvalidGoalCheckpointError 计数,因此解析移出了 runSideQueryvalidate 钩子——我在 sideQuery.ts 中确认该钩子会把失败重新包成普通 Error,悄悄丢掉这个类。现在所有解析层拒绝都抛 InvalidGoalCheckpointErrormaterializeGoalEvidenceCheckpoint 本就对所有不可用形状(空 claims、畸形 claim、未知引用源、字节上限)抛该类;新增测试在三种拒绝形状上钉住了类型。validateGoalCheckpointVerifierText 删除后已无任何引用(grep 核实)。

3)孤立 JSDoc 小瑕疵已修——takeTurnTokens 重新持有自己的注释。

上次评审以来的新增:#9840 的后续项已落地。证据受限的 resume 清除计数(与 edit 相同的重置,因为它重启窗口);暂停态 resume 保留计数(同一窗口、同一事实);两者都有 reducer 测试钉住。我直接核对了 reducer 结构:edit 与证据受限 resume 分支向 transitionGoal 传入 checkpointStalls: undefined,暂停态 resume 只展开 { status: 'active' },字段恰在该保留处保留。

其余机制依旧成立:计数发生在 materialization 之后,可重试失败不会推进计数;触发停止的 checkpoint 被丢弃,落定时连 limitKind: 'evidence_catalog' 一起持久化——安全,因为 resume 门以 limitKind 为准(已在 reducer 与 web-shell 的 goalGate.ts 中核实);持久化严格(只接受非负整数,-1/1.5 整体拒绝,0 恢复为无字段);checkpointStallspackages/core/src/goals/ 之外零出现,与 SDK 既有的更窄投影一致(本就省略 evidenceCheckpoint/tokensUsed)——没有任何东西过线。

测试证据:按分诊规则本次运行不构建或执行 PR 代码。被审提交上的 CI(经 API 拉取)全绿、无失败——含单测套件 Test (ubuntu-latest, Node 22.x);macOS/Windows 的 Test 检查为仓库 CI profile 门控下的跳过,不是失败。套件在设计上钉住了改动:停滞测试用真实的 buildGoalEvidenceCheckpointWindow 驱动真实窗口(101 条溢出、60 条压缩、10 条安静),没有熔断则失败。

核心主张是行为性的,且已被套件之外的证据支撑:维护者在本帖中的独立验证环境跑在真实编译产物上做 A/B——merge-base 构建支付 19+ 次 checkpoint 调用且永远 active,本构建在第三次停滞落定为 usage_limited(3 次调用、17 秒);计数扛过真实进程重启;31 条 claim 的对照臂从不计数;把错误类型变异为普通 Error 后无界循环回来了,证明错误类型接线是承重墙。沙箱 @qwen-code /verify 已在该 head 上运行中,将自行发布报告;该主张不依赖作者自述。此处未验证:本次运行不执行任何 PR 代码(分诊规则),实时行为依据维护者帖内验证与待发布的 /verify 报告;作者的变异探针数字是其自述,但维护者独立的 12 个变异体清扫(全部被杀)覆盖了同一范围。

Qwen Code · qwen3.8-max

Reviewed at f45ae6f3a1247642d7a9ed8e02dc6f008b541b7b · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — every stage clean on my own audit at the new head: both review-loop criticals fixed in the code (not just claimed), CI green, wire surface untouched, and the maintainer's independent runtime verification found no defect.

Stepping back: the problem is no longer just argued — the maintainer's in-thread A/B shows the merge-base build paying checkpoint calls forever while this build stops at the third stall, and the streak survives a real restart. My independent proposal and the PR converged; looking for a simpler route turned up nothing — an in-memory counter would be laundered by a restart, and rate-limiting the verifier would cap the token spend but keep losing evidence. The two criticals the review loop raised are fixed in the code itself: the reset now fires only where a check actually proved room, and the unusable-result count keys on an error class that the parse deliberately keeps out of runSideQuery's hook — with a mutant run proving the class is load-bearing, not decorative.

Residuals, carried from the maintainer's verification and confirmed by my read, none blocking: the does not count an unusable result while the window has room test names a state a real append-only transcript leaves on its own next turn (the guard buys one turn in practice; the end state is defensible); and the strict snapshot parser means old code reading a new record skips it — pre-existing in this family (#9165 paid the same cost), and an argument for ignoring unknown keys before the next GoalRecord field lands. The disclosed tradeoff stands as designed: a verifier that habitually fills the claim list stops a high-volume Goal quickly — observed at three turns — and the reason text tells the user to narrow the objective.

The two standing CHANGES_REQUESTED reviews from the bot were raised on superseded heads against findings the autofix rounds have since addressed; this pass re-reviewed the current head end to end and supersedes that stance. A sandboxed /verify run is still in flight and will post its report — green or red, it lands in the thread; nothing here rests on it.

Approving, pinned to the reviewed commit. ✅

中文说明

回顾:问题不再只是推理——维护者的帖内 A/B 显示 merge-base 构建永远支付 checkpoint 调用,而本构建在第三次停滞时停止,且计数扛过真实重启。我的独立方案与 PR 收敛;寻找更简路径无果——内存计数会被重启洗掉,给 verifier 限流能省调用却仍丢证据。评审循环提出的两个 Critical 都在代码里修好了(而非只是声称):重置只在检查真正证明了有空间时触发;不可用结果的计数以一个刻意绕开 runSideQuery 钩子的错误类型为键——变异运行证明该类型是承重墙而非装饰。

残余项(来自维护者验证并经我复核,均不阻塞):「窗口有空间时不计数不可用结果」这个测试命名的状态,真实的只追加 transcript 在下一轮就会自行离开(该防护实际上只买一轮;终态可辩护);严格的快照解析意味着旧代码读到新记录会跳过——这一族既有如此(#9165 付过同样代价),也是下一个 GoalRecord 字段落地前让解析器忽略未知键的理由。已披露的取舍按设计成立:习惯性返回满额 claim 列表的 verifier 会很快停掉高吞吐 Goal——实测三轮——reason 文本告诉用户收窄目标。

bot 现存的两个 CHANGES_REQUESTED 评审都针对已被取代的 head 上的发现,autofix 轮次已处理;本次重新端到端审查了当前 head 并取代该立场。沙箱 /verify 仍在运行,将自行发布报告——无论绿红都会出现在帖中;本结论不依赖它。

批准,钉在被审提交上。✅

Qwen Code · qwen3.8-max

Reviewed at f45ae6f3a1247642d7a9ed8e02dc6f008b541b7b · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

Comment thread packages/core/src/goals/goal-runtime.ts Outdated
Comment thread packages/core/src/goals/goal-runtime.ts Outdated
@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 3 finishedview run. See this round's report below.

中文说明

AutoFix 第 3 轮已完成 —— 查看运行。本轮报告见下方。

…nLM#9975)

finishCheckpointCheck reset checkpointStalls on all three call sites, but
only the room branch proved the window had relief. A transient verifier
failure or an empty turn now preserves the streak, so intermittent
checkpoint-verifier errors cannot launder the count and keep the stall
breaker from firing. Also moves withCheckpointStalls out from between
takeTurnTokens and its JSDoc.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Round summary (PR #9975, review rv:5014777655)

Two findings from the automated reviewer's CHANGES_REQUESTED review; both are resolved in code this round. No conflict resolution was requested (--conflict false) and no merge was performed.

Feedback points and decisions

[rc:3849420322] Critical — streak reset fires on paths that never proved room

Decision: Resolved in code. The claim is checkable and was reproduced before any fix: finishCheckpointCheck unconditionally reset checkpointStalls to 0, and two of its three call sites — the transient/malformed checkpoint-verifier catch and the current_turn_not_tail empty-turn catch — never established that the window had room. A Goal at the compaction ceiling whose verifier intermittently throws would therefore oscillate stall (+1) → error (reset to 0) and never reach GOAL_CHECKPOINT_STALL_LIMIT, so the breaker this PR installs could never fire.

Reproduction: the new test keeps the stall streak through a transient checkpoint verifier failure (stall → stall → transient verifier error → stall) was run against the unmodified commit and failed exactly at the post-error assertion — the streak had been reset and the goal stayed active instead of stopping.

Fix (minimal, as the finding prescribes): finishCheckpointCheck now takes a relieved flag that defaults to false; only the !window.shouldCheckpoint room branch passes true. Both catch paths keep the existing streak (snapshot.goal.checkpointStalls ?? 0). This makes the code match the checkpointStalls field doc in goal-protocol.ts ("Reset by any checkpoint check that finds room, and by edit or replace"), which needed no change. Two new tests pin both catch paths:

  • keeps the stall streak through a transient checkpoint verifier failure — the streak survives the error turn and the breaker stops the Goal (usage_limited, GOAL_CHECKPOINT_STALLED_REASON, checkpointStalls: 3) on the next stalled checkpoint.
  • keeps the stall streak when a turn records no evidence at all — an empty turn (lineage tail not at the current permit → current_turn_not_tail) keeps the streak and the goal stays active.

[rc:3849420327] Suggestion — withCheckpointStalls splits takeTurnTokens from its JSDoc

Decision: Resolved in code. Verified by inspection against the pre-PR tree: the token-ledger JSDoc documented takeTurnTokens, and this PR's insertion placed withCheckpointStalls between the doc block and the function, misattributing the contract. The helper is moved below takeTurnTokens so the comment stays with the function it documents. No extra doc comment was added to the helper — its body (checkpointStalls > 0 ? { ...rest, checkpointStalls } : rest) already spells out the zero-as-absence behavior, and the persistence side carries its own note in goal-reducer.ts.

Mutation probes

  • Probe 1 (new guard): replaced the relieved ? 0 : preserved expression with a constant 0 (pre-fix behavior) → both new tests FAILED (streak laundered, breaker never fires); restored → green.
  • Probe 2 (room-branch argument): dropped the true argument from the room branch → the existing resets the stall streak when a check needs no checkpoint at all test FAILED; restored → green. (The sibling reset test resets through recordCheckpoint, so it is unaffected — the room branch is witnessed by the one test.)

Files changed

  • packages/core/src/goals/goal-runtime.tsrelieved gate in finishCheckpointCheck; helper moved below takeTurnTokens.
  • packages/core/src/goals/goal-runtime.test.ts — two regression tests.

Commit: fix(goal): preserve the stall streak when a check proves no room (#9975)

Verification

  • Reproduction pre-fix: npx vitest run src/goals/goal-runtime.test.ts -t "keeps the stall streak" — both new tests FAILED on the unmodified commit (expected)
  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx vitest run src/goals/ (packages/core, touched) — 16 files, 406 tests passed
  • Mutation probes 1 and 2 — failed as expected when mutated, green after restore (see above)
  • npx prettier --check on the two changed files — passed
中文说明

本轮总结(PR #9975,评审 rv:5014777655)

自动化评审器发出的 CHANGES_REQUESTED 评审中共有两条发现;本轮均已在代码中解决。未要求处理冲突(--conflict false),也未执行任何合并。

反馈点与处理决定

[rc:3849420322] Critical — 连续停滞计数在从未证明"窗口有空间"的路径上被重置

处理决定:已在代码中解决。 该指控可核查,且在修复之前已完成复现:finishCheckpointCheck 无条件地把 checkpointStalls 重置为 0,而它的三个调用点中有两个——瞬态/畸形 checkpoint verifier 的 catch 分支、以及 current_turn_not_tail 空轮次的 catch 分支——从未证明窗口有空间。因此,一个处于压缩上限、其 verifier 间歇性抛错的 Goal 会在"停滞(+1)→ 出错(清零)"之间反复震荡,永远到不了 GOAL_CHECKPOINT_STALL_LIMIT,本 PR 安装的熔断器也就永远无法触发。

复现:新增测试 keeps the stall streak through a transient checkpoint verifier failure(停滞 → 停滞 → 瞬态 verifier 错误 → 停滞)先在未修改的提交上运行,恰好在错误轮之后的断言处失败——计数被清零,Goal 保持 active 而没有停止。

修复(按发现给出的最小方案):finishCheckpointCheck 现在接受一个默认为 falserelieved 参数;只有 !window.shouldCheckpoint 的有空间分支传 true。两条 catch 路径保留已有计数(snapshot.goal.checkpointStalls ?? 0)。这使代码与 goal-protocol.tscheckpointStalls 字段的文档("由任何发现空间的 checkpoint 检查重置,以及由 edit 或 replace 重置")保持一致,该文档无需修改。两个新测试钉住两条 catch 路径:

  • keeps the stall streak through a transient checkpoint verifier failure —— 计数在错误轮后幸存,下一次停滞的 checkpoint 使熔断器停止 Goal(usage_limitedGOAL_CHECKPOINT_STALLED_REASONcheckpointStalls: 3)。
  • keeps the stall streak when a turn records no evidence at all —— 空轮次(谱系尾部不在当前 permit 处 → current_turn_not_tail)保留计数,Goal 保持 active。

[rc:3849420327] Suggestion —— withCheckpointStallstakeTurnTokens 与其 JSDoc 拆开了

处理决定:已在代码中解决。 对照 PR 之前的代码树核实:token 账本的 JSDoc 描述的是 takeTurnTokens,而本 PR 的插入把 withCheckpointStalls 放在了文档块与函数之间,导致契约被挂错。现将该 helper 移到 takeTurnTokens 之下,让注释继续挂在它所描述的函数上。没有给 helper 额外添加文档注释——它的函数体(checkpointStalls > 0 ? { ...rest, checkpointStalls } : rest)已经说明了"零以缺省字段表示"的行为,持久化一侧在 goal-reducer.ts 中也已有自己的说明。

变异探针

  • 探针 1(新守卫):把 relieved ? 0 : 保留原值 表达式替换为常量 0(即修复前行为)→ 两个新测试均失败(计数被洗掉、熔断器从不触发);恢复后 → 全绿。
  • 探针 2(有空间分支的参数):去掉有空间分支的 true 参数 → 现有测试 resets the stall streak when a check needs no checkpoint at all 失败;恢复后 → 全绿。(另一个重置测试经由 recordCheckpoint 重置,因此不受影响——有空间分支由该测试钉住。)

变更文件

  • packages/core/src/goals/goal-runtime.ts —— finishCheckpointCheckrelieved 门控;helper 移到 takeTurnTokens 之下。
  • packages/core/src/goals/goal-runtime.test.ts —— 两个回归测试。

提交:fix(goal): preserve the stall streak when a check proves no room (#9975)

验证

  • 修复前复现:npx vitest run src/goals/goal-runtime.test.ts -t "keeps the stall streak" —— 两个新测试在未修改的提交上均失败(符合预期)
  • npm run build —— 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • npx vitest run src/goals/(packages/core,本次触及的包)—— 16 个文件、406 个测试全部通过
  • 变异探针 1 与 2 —— 变异时按预期失败,恢复后全绿(见上文)
  • 对两个变更文件运行 npx prettier --check —— 通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/core/src/goals/goal-runtime.ts:989 — [probe] sub-32-claim checkpoints on truncated windows reset the streak — breaker disarmed
中文说明

已审查——无阻断问题。 建议见行内评论。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread packages/core/src/goals/goal-runtime.test.ts Outdated
Comment thread packages/core/src/goals/goal-runtime.ts Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #9975 (round 3)

Addressed both inline findings; no failed checks and no base conflict to resolve (--conflict false, no merge performed). The finding deferred by the reviewer itself (goal-runtime.ts:989, sub-32-claim checkpoints on truncated windows) is recorded in the review body as not requested this round and was left untouched; it lives on a different path (recordCheckpoint with a materialized, non-stalled checkpoint) and this round's change does not interact with it.

Findings and dispositions

1. rc:3850510684 — a persistently malformed verifier result keeps the Goal active forever (Suggestion, checkable defect) — FIXED

Reproduced before changing anything: added a focused test driving three consecutive overflowing windows (101 records/turn) against a verifier that always answers {claims: []} (fails materializeGoalEvidenceCheckpoint). On the pre-fix commit the Goal stayed active with no checkpointStalls ever counted — the streak was merely preserved on every errored check, so the breaker installed by this PR could never fire for that failure mode, while its input-too-large sibling does stop the Goal.

Fix (first of the two options the finding proposed — counting, distinguishing "verifier returned an unusable result" from "check never ran"):

  • A check whose result fails materialization while the window is truncated now counts toward GOAL_CHECKPOINT_STALL_LIMIT exactly like a stalled checkpoint: finishCheckpointCheck gained a three-way outcome ('room' resets the streak, 'stalled' increments it and stops the Goal at the limit via the existing settleCheckpointFailure settle, 'inconclusive' — the default — preserves it).
  • The window.truncated condition keeps the count honest: an unusable result while the window has room loses no evidence and is retried for free on the next turn (this also preserves the committed round-1 behavior pinned by skips a checkpoint that changes source proof semantics), while an unusable result on an overflowing window is a compaction that produced nothing.
  • Transient verifier-call rejections (e.g. a provider failure) still settle as inconclusive bookkeeping, keeping the committed keeps the stall streak through a transient checkpoint verifier failure test green. The current_turn_not_tail close keeps preserving, as the finding required.
  • GOAL_CHECKPOINT_STALLED_REASON and the checkpointStalls field doc were generalized so the persisted stop reason stays accurate for both shapes (the old text claimed "each checkpoint already held the maximum number of claims", which is false for a result that never materialized).

2. rc:3850510656 — hand-pasted checkpoint-record fixture duplicates runCheckpointTurn (Suggestion) — FIXED

Replaced the 11-line hand-pasted fixture in keeps the stall streak when a turn records no evidence at all with a runCheckpointTurn(runtime, host, setRecords, records, 0, 'quiet') call, as suggested (verified: with count: 0 the helper emits exactly the same record chain; verifierEvidenceWindow(...).slice(1) is empty). One deviation from the literal suggestion: the trailing records = reassignment was dropped because nothing reads the variable afterwards and ESLint (no-unused-vars) rejects a dead reassignment; the bare call is the same simplification.

Mutation probes (witness verification)

  • Probe 1: deleted the new InvalidGoalCheckpointError && window.truncated branch → stops a Goal whose verifier keeps returning unusable checkpoint results FAILED (Goal stayed active) — restored → green.
  • Probe 2: dropped the window.truncated && condition → does not count an unusable result while the window has room FAILED (Goal stopped usage_limited despite room) — restored → green.

Both new guards are witnessed by tests committed in this round; the reproduction test also fails on the pre-round commit, as required for a defect fix.

Changed files

  • packages/core/src/goals/goal-runtime.ts — three-way streak handling in finishCheckpointCheck, malformed-result branch in the inner runCheckpoint catch, InvalidGoalCheckpointError import.
  • packages/core/src/goals/goal-protocol.ts — generalized GOAL_CHECKPOINT_STALLED_REASON; refreshed checkpointStalls doc.
  • packages/core/src/goals/goal-runtime.test.ts — two new tests (stop-on-persistent-unusable-results; no-count-with-room), fixture dedup in the flagged test.

Commit: 740f540cf2 fix(goal): count unusable checkpoint results toward the stall limit (#9975) (additive, on the PR head branch).

Verification

  • cd packages/core && npx vitest run src/goals/goal-runtime.test.ts -t 'unusable' on the pre-fix commit — reproduced the defect: 1 failed (Goal stayed active, no checkpointStalls), 1 passed
  • cd packages/core && npx vitest run src/goals/goal-runtime.test.ts after the fix — 116 passed
  • cd packages/core && npx vitest run src/goals/ — 16 files, 408 tests passed
  • Mutation probe 1 (guard removed) — stop test failed as expected; restored — green
  • Mutation probe 2 (truncated condition removed) — room test failed as expected; restored — green
  • npm run build — passed
  • npm run typecheck — passed (re-run after the final test-file edit)
  • npm run lint — first run flagged one unused-variable error introduced by the fixture replacement; fixed by dropping the dead reassignment; re-run passed
  • npx prettier --check on the three touched files — passed
  • Integration tests — not run: the touched behavior is exercised by the unit suite, not only through the bundled CLI or integration harness
  • npm run generate:settings-schema — not required: no settings source changed
中文说明

Autofix 审查轮次 — PR #9975(第 3 轮)

处理了两条行内发现;没有失败的检查,也没有需要解决的与 base 分支的冲突(--conflict false,未执行任何合并)。审查者自己延后的那条发现(goal-runtime.ts:989,截断窗口上少于 32 条 claim 的 checkpoint)在审查正文中被记录为「本轮不要求修改」,因此未做改动;它位于另一条路径上(recordCheckpoint 处理已物化且未停滞的 checkpoint),本轮改动与其没有交集。

发现与处置

1. rc:3850510684 — 持续返回畸形结果的 verifier 会让 Goal 永远保持 active(Suggestion,可检验的缺陷)— 已修复

在改动任何代码之前先复现:新增一个聚焦测试,用持续溢出的窗口(每轮 101 条记录)驱动连续三轮检查,verifier 始终返回 {claims: []}(无法通过 materializeGoalEvidenceCheckpoint)。在修复前的提交上,Goal 一直保持 active,checkpointStalls 从未被计数——每次出错的检查都只是保留(不清零、不递增)streak,因此本 PR 安装的熔断器在该失败模式下永远不会触发,而它的同类(输入过大)却会停止 Goal。

修复方式(采用该发现提出的两个选项中的第一个——计数,并区分「verifier 返回了不可用结果」与「检查从未运行」):

  • 当结果物化失败且窗口已被截断时,该检查现在与停滞的 checkpoint 一样计入 GOAL_CHECKPOINT_STALL_LIMIT:finishCheckpointCheck 新增三态 outcome('room' 清零 streak,'stalled' 递增 streak 并在达到上限时通过既有的 settleCheckpointFailure 收尾停止 Goal,'inconclusive'——默认值——仅保留)。
  • window.truncated 条件保证计数的语义准确:窗口尚有余量时的不可用结果没有丢失任何证据,下一轮可以免费重试(这也保留了第 1 轮提交中由 skips a checkpoint that changes source proof semantics 固化下来的行为);而溢出窗口上的不可用结果则是一次什么也没产出的压缩。
  • 瞬时的 verifier 调用失败(如 provider 故障)仍然以仅做簿记的方式收尾,保持已提交的 keeps the stall streak through a transient checkpoint verifier failure 测试为绿。current_turn_not_tail 收尾继续保持仅保留,符合该发现的要求。
  • GOAL_CHECKPOINT_STALLED_REASONcheckpointStalls 字段文档被泛化,使持久化的停止原因在两种形态下都准确(旧文案声称「每个 checkpoint 都已持有最大数量的 claim」,对从未物化成功的结果来说是不成立的)。

2. rc:3850510656 — 手工粘贴的 checkpoint 记录夹具与 runCheckpointTurn 重复(Suggestion)— 已修复

keeps the stall streak when a turn records no evidence at all 中 11 行手工粘贴的夹具替换为建议中的 runCheckpointTurn(runtime, host, setRecords, records, 0, 'quiet') 调用(已验证:count: 0 时该辅助函数生成的记录链完全相同;verifierEvidenceWindow(...).slice(1) 为空)。与建议原文有一处偏差:末尾的 records = 重新赋值被去掉了,因为此后没有任何代码读取该变量,而 ESLint(no-unused-vars)会拒绝无用的重新赋值;裸调用本身就是同样的简化。

变异探针(见证验证)

  • 探针 1:删除新增的 InvalidGoalCheckpointError && window.truncated 分支 → stops a Goal whose verifier keeps returning unusable checkpoint results 失败(Goal 保持 active)——恢复后变绿。
  • 探针 2:去掉 window.truncated && 条件 → does not count an unusable result while the window has room 失败(窗口有余量时 Goal 却被停止为 usage_limited)——恢复后变绿。

本轮新增的两个守卫都由本轮提交的测试见证;复现测试在轮次前的提交上也会失败,符合缺陷修复的要求。

变更文件

  • packages/core/src/goals/goal-runtime.tsfinishCheckpointCheck 的三态 streak 处理、runCheckpoint 内层 catch 中的畸形结果分支、InvalidGoalCheckpointError 导入。
  • packages/core/src/goals/goal-protocol.ts — 泛化 GOAL_CHECKPOINT_STALLED_REASON;更新 checkpointStalls 文档。
  • packages/core/src/goals/goal-runtime.test.ts — 两个新测试(持续不可用结果时停止;有余量时不计数)、被标记测试中的夹具去重。

提交:`740f540cf2 fix(goal): count unusable checkpoint results toward the stall limit (#9975)``(追加式,位于 PR head 分支)。

验证

  • 在修复前的提交上运行 cd packages/core && npx vitest run src/goals/goal-runtime.test.ts -t 'unusable' — 复现缺陷:1 失败(Goal 保持 active,无 checkpointStalls),1 通过
  • 修复后运行 cd packages/core && npx vitest run src/goals/goal-runtime.test.ts — 116 通过
  • cd packages/core && npx vitest run src/goals/ — 16 个文件,408 个测试全部通过
  • 变异探针 1(删除守卫)— 停止测试按预期失败;恢复后变绿
  • 变异探针 2(删除截断条件)— 余量测试按预期失败;恢复后变绿
  • npm run build — 通过
  • npm run typecheck — 通过(在最后一次测试文件编辑后重新运行)
  • npm run lint — 首次运行发现夹具替换引入的一个未使用变量错误;通过去掉无用的重新赋值修复;重新运行通过
  • 对三个变更文件运行 npx prettier --check — 通过
  • 集成测试 — 未运行:本次触及的行为由单元测试覆盖,并非只能通过打包后的 CLI 或集成测试框架验证
  • npm run generate:settings-schema — 不需要:未改动任何 settings 源

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/goals/goal-runtime.ts (findings in round 2; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:packages/core/src/goals/goal-runtime.ts(第 2 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread packages/core/src/goals/goal-runtime.ts
Comment thread packages/core/src/goals/goal-runtime.ts Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下:

Round summary

Two inline findings, both resolved in code with one commit; one review-body observation that required no code change. The Critical and the Suggestion shared a root-cause area (the stall-breaker machinery added by this PR), so both were batched into this round per the convergence observation's advice.

Findings and dispositions

[rc:3851765994] [Critical] Parse-level unusable verifier results never reach the stall breaker — RESOLVED

Reproduced before changing anything: a new witness test driving createGoalCheckpointVerifier (the production wiring, config.ts) with a model answering {"claims": []} failed on the pre-fix code with expected Error: Goal checkpoint verifier returned … to be an instance of InvalidGoalCheckpointError, proving the defect on the exact code the finding cites.

Root cause confirmed by code reading: parse-level rejections (parseGoalCheckpointVerifierText / parseClaim) threw plain Errors, and the validate hook path was even worse — runSideQuery re-wraps hook failures into new Error(customError) (sideQuery.ts), losing any class. The runtime breaker (runCheckpoint's catch) keys the "unusable result while the window overflows" branch on instanceof InvalidGoalCheckpointError, so a Goal whose verifier persistently returns empty/malformed claims settled every check as transient bookkeeping and the breaker never fired — exactly the loop this PR exists to terminate.

Minimal root-cause fix (the finding's first suggested shape, classify at the source rather than broadening the runtime catch):

  • parseGoalCheckpointVerifierText and parseClaim now throw InvalidGoalCheckpointError (same messages) for invalid JSON, empty/oversized/non-exact claim lists, and malformed claims.
  • Dropped the validate: validateGoalCheckpointVerifierText hook from the runSideQuery call and deleted the now-unreferenced validateGoalCheckpointVerifierText wrapper: parsing already runs on the returned text immediately after the query, where the class survives. This is net-subtractive. A comment at the call site records why parsing must stay out of a validate hook so the class is not silently lost again.
  • Transient failures (provider errors, timeouts, aborts) still settle as bookkeeping with the streak preserved; only unusable results while the window overflows count. The existing runtime tests pin all three paths.

On the finding's note that the existing runtime test stops a Goal whose verifier keeps returning unusable checkpoint results "masks the gap": the test stays valid as the witness for materialize-level unusable results (unknown source refs, proof-kind changes, byte cap — which only materializeGoalEvidenceCheckpoint can produce in production). The previously untested parse-level path is now pinned by the new goal-checkpoint-verifier.test.ts witness; the two tests compose into the full production chain (verifier rejects with the class → runtime counts the class toward the stall limit).

[rc:3851766006] [Suggestion] Stall-limit stop contract duplicated in two siblings — RESOLVED

Implemented the suggested extraction: settleIfCheckpointStalled(attempt, goal, checkpointStalls): Promise<boolean> now owns the >= GOAL_CHECKPOINT_STALL_LIMIT test, the settleCheckpointFailure(..., GOAL_CHECKPOINT_STALLED_REASON, 'evidence_catalog') call, and the persist-the-streak-with-the-stop shape; finishCheckpointCheck and recordCheckpoint both call it. The pre-existing explanatory comment moved onto the helper; recordCheckpoint keeps only the note specific to it (a stopped Goal discards the checkpoint it would have written). Behavior-preserving: all 116 runtime tests pass unchanged, and the mutation probe below witnesses the helper.

[rv:5017501066] CHANGES_REQUESTED review body — observation only, no code action

The review explicitly states it is an observation. Its advice (triage the shared root cause before fixing instances, batch the fixes) is what this round did: both findings trace to the stall-breaker machinery, and both landed in one verified batch.

Mutation probes (both restored to green afterwards)

  1. Reverted one fixed throw site (invalid claims) back to a plain Error → the new witness test FAILED (1 failed | 5 passed) → restored.
  2. Negated the helper threshold (<<=, i.e. never settle) → three runtime stall tests FAILED: stops a Goal after three consecutive stalled checkpoints (recordCheckpoint site), stops a Goal whose verifier keeps returning unusable checkpoint results (finishCheckpointCheck site), keeps the stall streak through a transient checkpoint verifier failure (final-stop turn) → restored.

Conflict notes

--conflict false: no merge performed, branch stayed on its own head.

Verification

  • npx vitest run src/goals/goal-checkpoint-verifier.test.ts (pre-fix, reproduction) — 1 failed (the new witness test, as required for a defect claim), 5 passed
  • npx vitest run src/goals/ (focused, packages/core) — 16 files, 409 passed
  • Mutation probe 1 (revert throw site) — witness test failed as required; restored, re-ran to green
  • Mutation probe 2 (negate helper threshold) — 3 runtime tests failed as required; restored, re-ran to green
  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx prettier --write on the three changed files — applied; no further drift
  • npx vitest run src/goals/ re-run on the committed tree — 16 files, 409 passed
中文说明

本轮摘要

两条行内发现,均已在代码中解决,合入一个提交;一条评审正文观察,无需代码改动。Critical 与 Suggestion 同属一个根因区域(本 PR 新增的停滞熔断机制),因此按照收敛观察的建议,在本轮中打包处理。

发现与处置

[rc:3851765994] [Critical] 解析层面的不可用 verifier 结果永远到不了停滞熔断器 —— 已解决

先复现再改动:新增的见证测试驱动 createGoalCheckpointVerifier(生产装配,见 config.ts),让模型回答 {"claims": []};该测试在修复前的代码上以 expected Error: Goal checkpoint verifier returned … to be an instance of InvalidGoalCheckpointError 失败,在该发现所指的确切代码上证明了缺陷。

根因经代码阅读确认:解析层面的拒绝(parseGoalCheckpointVerifierText / parseClaim)抛的是普通 Error,而 validate 钩子路径更糟——runSideQuery 会把钩子失败重新包装成 new Error(customError)(sideQuery.ts),丢掉任何错误类。运行时熔断器(runCheckpoint 的 catch)以 instanceof InvalidGoalCheckpointError 作为「窗口溢出时结果不可用」分支的键,因此一个 verifier 持续返回空或畸形 claims 的 Goal,每次检查都按瞬态记账收尾,熔断器永远不触发——这正是本 PR 要终止的循环。

最小根因修复(采用该发现建议的第一种形态:在源头分类,而不是放宽运行时 catch):

  • parseGoalCheckpointVerifierTextparseClaim 现在对坏 JSON、空/超限/非精确 claims 列表、畸形 claim 抛 InvalidGoalCheckpointError(消息不变)。
  • runSideQuery 调用中移除 validate: validateGoalCheckpointVerifierText 钩子,并删除已无引用的 validateGoalCheckpointVerifierText 包装函数:解析本来就会在查询返回后立即对文本执行,在那里错误类得以保留。这是净删减。调用处留了一条注释,说明解析为何必须留在 validate 钩子之外,以免该类再次被悄悄丢失。
  • 瞬态失败(provider 错误、超时、中止)仍按记账收尾并保留计数;只有窗口溢出时的不可用结果才计数。现有运行时测试钉住了全部三条路径。

关于该发现提到现有运行时测试 stops a Goal whose verifier keeps returning unusable checkpoint results「掩盖了缺口」:该测试仍然有效,它是 materialize 层不可用结果(未知来源引用、proof 类型变化、字节上限——生产中只有 materializeGoalEvidenceCheckpoint 会产生)的见证。此前未被测试的解析层路径现在由新增的 goal-checkpoint-verifier.test.ts 见证测试钉住;两个测试拼成完整的生产链路(verifier 以该类拒绝 → 运行时把该类计入停滞上限)。

[rc:3851766006] [Suggestion] 停滞上限停止契约在两个兄弟分支中重复 —— 已解决

按建议实施抽取:settleIfCheckpointStalled(attempt, goal, checkpointStalls): Promise<boolean> 现在统一持有 >= GOAL_CHECKPOINT_STALL_LIMIT 判断、settleCheckpointFailure(..., GOAL_CHECKPOINT_STALLED_REASON, 'evidence_catalog') 调用、以及「把计数随停止一起持久化」的形态;finishCheckpointCheckrecordCheckpoint 均调用它。原有解释性注释移到该 helper 上;recordCheckpoint 只保留其特有的说明(被停止的 Goal 丢弃本要写入的 checkpoint)。行为保持不变:116 个运行时测试全部原样通过,下方的变异探针见证了该 helper。

[rv:5017501066] CHANGES_REQUESTED 评审正文 —— 仅为观察,无代码动作

该评审明确声明仅为观察。其建议(先定位共享根因再修实例、批量修复)正是本轮的做法:两条发现都追溯到停滞熔断机制,并在同一个经过验证的批次中落地。

变异探针(事后均已恢复为绿)

  1. 把其中一个已修复的 throw 点(invalid claims)改回普通 Error → 新见证测试失败(1 failed | 5 passed)→ 恢复。
  2. 取反 helper 阈值(<<=,即永不 settle)→ 3 个运行时停滞测试失败:stops a Goal after three consecutive stalled checkpoints(recordCheckpoint 分支)、stops a Goal whose verifier keeps returning unusable checkpoint results(finishCheckpointCheck 分支)、keeps the stall streak through a transient checkpoint verifier failure(最后一轮触发停止)→ 恢复。

冲突说明

--conflict false:未执行合并,分支保持在自身 head 上。

验证

  • npx vitest run src/goals/goal-checkpoint-verifier.test.ts(修复前,复现)— 1 失败(新见证测试,缺陷主张所必需),5 通过
  • npx vitest run src/goals/(聚焦,packages/core)— 16 个文件,409 通过
  • 变异探针 1(回退 throw 点)— 见证测试按要求失败;恢复后重跑为绿
  • 变异探针 2(取反 helper 阈值)— 3 个运行时测试按要求失败;恢复后重跑为绿
  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • 对 3 个改动文件执行 npx prettier --write — 已应用,无进一步漂移
  • 在提交后的树上重跑 npx vitest run src/goals/ — 16 个文件,409 通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

qqqys added 2 commits August 25, 2026 19:02
… window

QwenLM#9840 landed after this branch opened: an evidence-limited Goal now
resumes by repointing the cursor and dropping the checkpoint, which is
a different evidence window from the one the streak was counted
against. Carrying the count across it spends the new window's
allowance on the old window's failures -- a Goal resumed at two stalls
would stop again after a single stalled checkpoint.

A resume that does NOT restart the window (paused, blocked) keeps the
streak: that Goal comes back to the same window, so what it learned
about that window is still true.

Mutation probe: removing the reset fails exactly the new resume test
(75 others green).
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

f45ae6f3a1 补上 PR 正文 Risk 段里留的那条 follow-up:#9840 已合入 main,证据受限的 Goal 现在通过重指游标、丢弃 checkpoint 来恢复——那是与 streak 计数所针对的不同的证据窗口。跨过它继续累计,等于用新窗口的额度偿还旧窗口的失败:一个在 2 次 stall 时被恢复的 Goal,只要再来一次 stalled checkpoint 就会立刻再停。

不重启窗口的 resume(paused / blocked)则保留 streak——那个 Goal 回到的是同一个窗口,它对该窗口的判断依然成立。两种情形各有一个测试。

变异检验:去掉这次清零,恰好挂新增的 resume 测试(其余 75 绿)。全量 src/goals/ 412/412 绿,goals 内 tsc 0 错,prettier/eslint 干净。

本轮 6 条线程已全部逐条回复并 resolve——其中 5 条由 autofix 环路的三个 commit 修复(含两条 Critical:catch 路径洗掉 streak、生产 verifier 抛的是普通 Error 导致 stall 分支打不着),我核对了当前代码确认每条都真的落地了。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:

  • packages/core/src/goals/goal-protocol.ts:131 — [probe] no runtime-level restore test for the checkpointStalls anti-laundering claim
  • packages/core/src/goals/goal-protocol.ts:25 — [review] stalled-reason prose hard-codes "three" instead of deriving it from the limit constant
中文说明

已审查——无阻断问题。 建议见行内评论。

收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.0)

);
});

it('surfaces unusable model output as InvalidGoalCheckpointError', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This new class-pin test covers three of the four rejection sites the diff converts to InvalidGoalCheckpointError, but not parseClaim's shape rejection (invalid proofKind, duplicate sourceRefs, wrong field types) — that site is asserted only by a message match in the pre-existing test, which passes for any Error subclass. A probe confirmed the gap: reverting just that throw site to a plain Error leaves the whole goals suite green (412/412), while a class-pinned probe for the duplicate-sourceRefs shape fails against that mutant and passes on restored code. In production the stall breaker dispatches on error instanceof InvalidGoalCheckpointError && window.truncated, so a verifier reply with duplicated sourceRefs arriving while the evidence window overflows would settle as inconclusive instead of stalled: the streak never advances for that class of unusable output, and the Goal keeps paying a checkpoint call per turn while shedding uncatalogued evidence — the loop this breaker exists to stop. Add one more reply to the list so every converted throw site is class-pinned:

JSON.stringify({
  claims: [
    { proofKind: 'external_fact', claim: 'c', sourceRefs: ['r-1', 'r-1'] },
  ],
}),
中文说明

这个新的错误类固定测试覆盖了本 diff 转换为 InvalidGoalCheckpointError 的四个抛出点中的三个,但没有覆盖 parseClaim 的形状拒绝(非法 proofKind、重复 sourceRefs、字段类型错误)——该抛出点只在既有测试中通过错误消息匹配来断言,而消息匹配对任何 Error 子类都通过。探针确认了这个缺口:仅把该抛出点还原为普通 Error,整个 goals 测试套件仍然全绿(412/412);而对重复 sourceRefs 形状做错误类固定的探针在该变异体上失败、在还原后的代码上通过。生产环境中熔断器以 error instanceof InvalidGoalCheckpointError && window.truncated 分派,因此当证据窗口溢出时,一份含重复 sourceRefs 的 verifier 回复会被归为 inconclusive 而非 stalled:这类不可用输出永远不会推进计数,Goal 会每轮继续支付一次 checkpoint 调用并持续丢失未编目的证据——正是本熔断器要终止的循环。向列表再加一条回复,使所有被转换的抛出点都固定错误类:

JSON.stringify({
  claims: [
    { proofKind: 'external_fact', claim: 'c', sourceRefs: ['r-1', 'r-1'] },
  ],
}),

— qwen3.8-max via Qwen Code /review (v0.22.0)

@wenshao

wenshao commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Independent runtime verification (maintainer review)

I built a local harness for this PR and ran it in two layers against real compiled artifacts, not just the PR's vitest suite. Both new stop paths reproduce end-to-end, the false-positive guards hold, and the persistence claim survives a real process restart. No correctness defect found — I'm happy to merge this. Two low-severity observations are at the bottom.

Arms. AFTER = PR head f45ae6f3a1. BEFORE = ef18a73885, the merge-base with origin/main (the head carries a merge commit, so HEAD^ is the wrong baseline). git diff ef18a73885...f45ae6f3a1 --stat = 9 files, +720/−59, byte-identical to what GitHub shows for this PR.

Harness (how the numbers below were produced)

Layer 1 — compiled runtime, file-backed transcript. packages/core built into dist in both worktrees; a standalone driver imports the real createGoalRuntime from dist/src/goals/goal-runtime.js. The journal and evidence source are backed by a real append-only JSONL file on disk (recordGoalState appends, readActiveTranscriptChain re-reads the file), so the transcript can only grow — the same shape ChatRecordingService produces in production. Only the turn host and the checkpoint verifier are scripted.

Layer 2 — the real CLI. packages/cli built from the PR worktree and driven in a tmux pty against a mock OpenAI server on 127.0.0.1:8975. Each Goal turn executes 110 real run_shell_command calls (2 rounds × 55), producing 229 transcript records per turn; the checkpoint verifier is a genuine HTTP side-query whose reply I control. Arms differ only by which packages/core the workspace symlink points at (@qwen-code/qwen-code-core → AFTER vs BEFORE), so the CLI half is byte-identical across arms.

Truncation was verified, not assumed. A temporary probe compiled into dist logged the window at the decision point:

[PROBE] window.truncated=true shouldCheckpoint=true evidence=58 previousClaims=0  newClaims=32 stalled=true  priorStalls=0
[PROBE] window.truncated=true shouldCheckpoint=true evidence=41 previousClaims=32 newClaims=32 stalled=true  priorStalls=1
[PROBE] window.truncated=true shouldCheckpoint=true evidence=41 previousClaims=32 newClaims=32 stalled=true  priorStalls=2

The probe was removed before every measured arm below.

1. The loop this PR bounds is real, and the breaker closes it

Same objective, same mock, same CLI — only the core differs:

BEFORE (ef18a73885) AFTER (PR head)
Goal state after 3 turns active usage_limited / evidence_catalog
Checkpoint verifier calls (model calls) 19 and still climbing 3, then none
Wall clock when I stopped watching still ◎ /goal active (2m) stopped at 17.2 s
Transcript records written 4359 462
Evidence per turn: recorded → catalogued 110 → 41 110 → 41

BEFORE paid a model call and lost ~⅔ of the turn's evidence every turn, forever. AFTER stops on the third stalled checkpoint with the new reason, and mints no further continuation.

AFTER stops

BEFORE runs forever

The journal agrees, and shows the streak persisted on the record rather than held in memory:

create        status=active         stalls=-  claims=0
turn_finished status=active         stalls=-  claims=0
checkpoint    status=active         stalls=1  claims=32
turn_finished status=active         stalls=1  claims=32
checkpoint    status=active         stalls=2  claims=32
turn_finished status=active         stalls=2  claims=32
usage_limited status=usage_limited  stalls=3  claims=32  limitKind=evidence_catalog

The stopping checkpoint is discarded exactly as described: the final record still carries the previous checkpoint's 32 claims and the previous cursor.

2. The streak survives a real process restart

Killed the CLI mid-streak (checkpointStalls: 2 on disk), relaunched with --continue against the same session file. The resumed process made exactly one checkpoint verifier call, and that single stalled checkpoint took the Goal from 2 → 3 and stopped it. The count was carried, not re-earned.

restart

3. Unusable verifier output counts — over the wire, and the error class is load-bearing

With the mock returning {"claims": []}, the failure travels the real path (runSideQueryparseGoalCheckpointVerifierTextInvalidGoalCheckpointError) and the Goal stops after 3 turns, cursor never advancing.

Then I mutated the compiled verifier — new InvalidGoalCheckpointError(...)new Error(...), i.e. the pre-#9975 shape that runSideQuery's validate hook would also have produced — and re-ran the identical arm: the stall stops being counted and the unbounded loop comes back (10 checkpoint calls, still ◎ /goal active). Commit 4 is not cosmetic.

mutant

4. False-positive guards hold

Control arm on the PR build: identical evidence pressure (window truncated every turn) but the verifier returns 31 claims instead of 32 → never counted, Goal still running after 10 checkpoints. One claim below the cap is the whole difference.

control

5. Full behaviour matrix (Layer 1, compiled runtime + real JSONL)

Scenario Result Verdict
Truncated window + 32 claims, ×3 stalls 1 → 2 → 3, usage_limited / evidence_catalog, no 4th turn
Same on BEFORE 12 turns, 12 model calls, still active ✅ (bug reproduced)
Truncated window + 31 claims 6 turns, never counted ✅ busy turn is not a stall
Full claims, window with room stall 1 then reset to 0 on the next effective checkpoint ✅ quiet Goal is not a stall
Check that needs no checkpoint resets the streak
Verifier throws (503) mid-streak streak held at 1 across 3 failures, then 2 → 3 → stop ✅ transient errors cannot launder
Unusable result + truncated window stalls 1 → 2 → 3 → stop
Restart (2 processes, same JSONL) streak restored as 2, next stall stops it
resume of the evidence-limited Goal streak cleared, checkpoint dropped, cursor repointed ✅ (commit 5)
resume of a paused Goal streak kept at 2, next stall stops it ✅ same window, same truth
edit streak cleared

resume is only ever dispatched from /goal resume — there is no automatic resume in prod code, so the reset cannot be self-triggered into a loop.

6. Tests, mutation, typecheck

  • src/goals suite on the PR head: 377 passing / 15 files. (goal-tools.test.ts fails on both arms in my checkout — @modelcontextprotocol/client is missing from my local node_modules; environment, not this PR.) BEFORE: 362.
  • Mutation sweep — 12 mutants, all killed, zero collateral failures (205 tests in the 4 changed test files):
Mutant Tests failed
drop the increment in recordCheckpoint 5
reset on every check (ignore outcome) 3
!shouldCheckpoint'inconclusive' instead of 'room' 1
threshold >=> 3
predicate ignores window.truncated 2
predicate ignores the claim cap 1
parseGoalRecord drops the key from the allow-list 1
parseGoalRecord never restores it 1
runtime stops counting unusable results 1
edit stops resetting 1
evidence-limited resume stops resetting 1
verifier throws plain Error instead of InvalidGoalCheckpointError 1
  • tsc --noEmit in packages/core: 37 errors on BEFORE, 37 on AFTER, identical file distribution, 0 in src/goals/. (prettier/eslint locally flag the same files on both arms — local version drift, not a signal; Test (ubuntu-latest) is green on f45ae6f3a1.)
  • validateGoalCheckpointVerifierText has no remaining references anywhere in the repo after its removal.
  • Wire claim checked: the SDK's GoalRecord (packages/sdk-typescript/src/daemon/types.ts) is a narrower structural projection that already omits evidenceCheckpoint; checkpointStalls follows the same pattern, and Web Shell's canResumeGoal already returns true for usage_limited + evidence_catalog, so a stalled Goal keeps its Resume button. ✅

Observations (neither blocks merge)

O1 — does not count an unusable result while the window has room asserts a state a real session cannot reach. An unusable checkpoint never advances the cursor, so on an append-only transcript the window grows every turn and truncates on its own. Driving the real runtime against a real JSONL with 85 evidence records per turn and {"claims": []} every time:

turn 1 (room)       stalls=-   status=active
turn 2 (truncated)  stalls=1   status=active
turn 3 (truncated)  stalls=2   status=active
turn 4 (truncated)  stalls=3   status=usage_limited / evidence_catalog

The PR's helper takes the !checkpoint branch on every turn of that test, which replaces the record list with a fresh window rooted at the unchanged cursor — the transcript shrinks, which it can never do in production. So the guard buys exactly one turn in real life rather than the open-ended exemption the test name implies. The end state (stop) is defensible; the test just doesn't guard the property it names, and the emitted reason says "the evidence window overflowed every time" when the first check didn't. Worth either a comment on the test or growing the fixture across turns.

O2 — a downgrade silently rewinds the Goal. The pre-PR parseGoalRecord uses a strict hasOnlyKeys, so every record carrying the new key is skipped by recovery:

record WITHOUT checkpointStalls    BEFORE=accepted   AFTER=accepted
record WITH  checkpointStalls: 2   BEFORE=REJECTED   AFTER=accepted (checkpointStalls=2)

End to end: I ran the PR build to checkpointStalls: 2 (turnCount 3, a 32-claim checkpoint), then opened the same session with the merge-base core and --continue. The Goal restored from the last record without the key — turnCount 3 → 1, evidence cursor back to the create-time cursor, the whole 32-claim checkpoint dropped — and carried on with no warning. This is not new to this PR (adding limitKind in #9165 paid the same cost) and cannot be fixed here, but "Breaking changes / migration notes: none" is only true in the new→old-record direction, and it's an argument for making that parser ignore unknown keys before the next GoalRecord field lands.

Note on the risk you already flagged: in my canonical arm the Goal died 17 seconds and 3 turns in. Models do tend to fill an allowed list to its maximum, so a verifier that habitually returns 32 claims will stop any high-volume Goal quickly. That is the documented tradeoff and the reason text does tell the user what to do — flagging the observed magnitude, not disputing the design.

中文版报告

独立运行时验证(维护者复核)

我为这个 PR 搭了一套本地验证环境,分两层跑在真实编译产物上,而不只是重跑 PR 自带的 vitest。两条新的停止路径都能端到端复现,误报防护成立,持久化的说法也扛过了真实的进程重启。没有发现正确性缺陷,我认为可以合入。 底部有两条低优先级观察。

对照臂。 AFTER = PR head f45ae6f3a1BEFORE = ef18a73885,即与 origin/main 的 merge-base(head 上有一个 merge commit,所以 HEAD^ 不是正确基线)。git diff ef18a73885...f45ae6f3a1 --stat = 9 个文件,+720/−59,与 GitHub 显示的完全一致。

验证环境。 第一层:两个 worktree 各自把 packages/core 编译成 dist,驱动程序直接 import 真实的 createGoalRuntime;journal 与 evidence source 由磁盘上真实的只追加 JSONL 文件支撑(recordGoalState 追加,readActiveTranscriptChain 重新读文件),因此 transcript 只增不减——与生产中 ChatRecordingService 的形状一致。第二层:从 PR worktree 构建真实 CLI,在 tmux pty 中对着 127.0.0.1:8975 的 mock OpenAI 服务运行;每个 Goal 轮次真实执行 110 次 run_shell_command(2 轮 × 55),每轮写入 229 条 transcript 记录;checkpoint verifier 是真正的 HTTP side-query。两臂@qwen-code/qwen-code-core 这个 workspace 符号链接指向哪个 core,CLI 部分逐字节相同。窗口是否 truncated 是用临时编入 dist 的探针实测的,探针在所有正式测量臂之前已移除。

1. 这个 PR 要收敛的循环真实存在,熔断确实把它关上了。 同一目标、同一 mock、同一 CLI,只换 core:BEFORE 跑 3 轮后仍是 active19 次 checkpoint verifier(模型)调用且还在增长,2 分钟后仍显示 ◎ /goal active,写入 4359 条记录;AFTER 在第 3 次停滞 checkpoint 上落为 usage_limited / evidence_catalog,共 3 次模型调用,17.2 秒停止,不再产生续轮。两臂每轮都是 110 条证据记录只有 41 条进入目录——BEFORE 每轮都在付一次模型调用并丢掉约三分之二的证据,且永不收敛。journal 显示 stalls 1 → 2 → 3 持久化在记录上;触发停止的那次 checkpoint 确实被丢弃(最终记录仍带着上一次 checkpoint 的 32 条 claim 与旧游标)。

2. 连续计数扛过真实重启。checkpointStalls: 2 落盘时杀掉 CLI,用 --continue 对同一 session 文件重启:恢复后的进程只发起了一次 checkpoint verifier 调用,这一次停滞就把 2 → 3 并停止了 Goal。计数是被带过来的,不是重新累积的。

3. 不可用的 verifier 输出会计数,且错误类型是承重的。 mock 返回 {"claims": []} 时,失败沿真实路径(runSideQueryparseGoalCheckpointVerifierTextInvalidGoalCheckpointError)传播,Goal 在 3 轮后停止,游标始终未推进。随后我把编译后的 verifier 变异为 new Error(...)(即 #9975 之前、validate hook 也会产生的形状)再跑同一场景:停滞不再被计数,无界循环回来了(10 次 checkpoint 调用,仍 ◎ /goal active)。commit 4 不是修饰性改动。

4. 误报防护成立。 对照臂:证据压力完全相同(每轮窗口都截断),但 verifier 返回 31 条 claim 而非 32 → 从不计数,10 次 checkpoint 后 Goal 仍在跑。差别就在于比上限少一条。

5. 完整行为矩阵(第一层): 截断窗口 + 32 claim ×3 → 停止;BEFORE 同场景 12 轮仍 active;截断 + 31 claim → 6 轮不计数;满 claim 但窗口有空间 → 计数归零;无需 checkpoint 的检查 → 归零;verifier 503 → 连续计数保持不变,之后 2 → 3 停止;不可用结果 + 截断窗口 → 3 次后停止;重启(两个进程、同一 JSONL)→ 恢复为 2,下一次停滞即停止;证据受限 Goal 的 resume → 清零并重开窗口;paused Goal 的 resume → 保留 2;edit → 清零。全部符合预期。生产代码中 resume 只来自 /goal resume,没有自动 resume,因此重置不会被自我触发成循环。

6. 测试、变异与类型检查。 PR head 上 src/goals 套件 377 passing / 15 文件goal-tools.test.ts两臂都失败——我本地 node_modules@modelcontextprotocol/client,是环境问题不是 PR 问题);BEFORE 为 362。变异检验 12 个变异体全部被杀死、无误伤(4 个改动测试文件共 205 个测试),覆盖递增、重置、阈值、谓词两个条件、解析允许列表与还原、不可用结果计数、edit/resume 重置、以及 verifier 错误类型。tsc --noEmit:BEFORE 37 个错误、AFTER 37 个,文件分布完全一致,src/goals/ 内为 0(本地 prettier/eslint 在两臂标记相同文件,是本地版本漂移,不构成信号;Test (ubuntu-latest)f45ae6f3a1 上是绿的)。validateGoalCheckpointVerifierText 删除后全仓无残留引用。线上契约方面:SDK 的 GoalRecord 本就是省略了 evidenceCheckpoint 的更窄结构投影,checkpointStalls 沿用同一模式;Web Shell 的 canResumeGoalusage_limited + evidence_catalog 已返回 true,所以停滞的 Goal 仍保留 Resume 按钮。

观察 1(不阻塞)——「窗口有空间时不计入不可用结果」这条测试断言的是真实会话到不了的状态。 不可用的 checkpoint 不会推进游标,因此在只增不减的 transcript 上窗口每轮都在长,很快自己就截断了。用真实 JSONL 驱动真实运行时(每轮 85 条证据、每次都返回 {"claims": []}):第 1 轮不计数,第 2/3 轮计到 1/2,第 4 轮停止为 usage_limited。PR 的测试辅助函数在每一轮都走 !checkpoint 分支,那条分支会用一个以旧游标为根的新窗口替换整个记录列表——transcript 变短了,而生产中不可能。所以这条豁免在现实中只多买一轮。最终停止本身是合理的,只是该测试没有守住它名字所声称的性质,而且给出的 reason 说「每次窗口都溢出」,但第一次并没有。建议给测试加注释,或让 fixture 跨轮增长。

观察 2(不阻塞)——降级会静默回退 Goal。 PR 之前的 parseGoalRecord 使用严格的 hasOnlyKeys,因此带有新字段的记录会在恢复时被整条跳过:带 checkpointStalls 的记录 BEFORE 判定为 REJECTED、AFTER 为 accepted。端到端验证:用 PR 构建跑到 checkpointStalls: 2(turnCount 3、一份 32 条 claim 的 checkpoint),再用 merge-base 的 core 加 --continue 打开同一 session——Goal 从最后一条不含该字段的记录恢复:turnCount 3 → 1、证据游标退回到创建时的游标、整份 32 条 claim 的 checkpoint 被丢弃,并且没有任何提示就继续跑下去。这不是本 PR 引入的(#9165limitKind 时付过同样代价),也无法在本 PR 内修复;但「Breaking changes / migration notes: none」只在「新记录缺字段 → 旧行为」这个方向上成立,同时这也是在下一个 GoalRecord 字段落地之前,把该解析器改为忽略未知字段的理由。

关于你已经标注的风险: 在我的标准场景里,Goal 在 17 秒、3 轮内就被停掉了。模型确实倾向于把允许的列表填满,因此一个习惯性返回 32 条 claim 的 verifier 会很快停掉任何高证据量的 Goal。这正是你写明的权衡,reason 文案也告诉了用户该怎么做——我只是把观察到的量级摆出来,不是反对这个设计。

@wenshao

wenshao commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge August 25, 2026 13:21
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 91 passed · 0 failed · 91 total

Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:91 通过 · 0 失败 · 91 总计

抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR 9975 — feat(goal): stop a Goal whose checkpoints stall three times in a row

Verdict: merge-ready — 91/91 scripted assertions passed (0 unexpected failures), verified head f45ae6f3a1247642d7a9ed8e02dc6f008b541b7b (merge of the PR head into base tip 42b4c09ceb).

中文摘要
  • 结论:merge-ready。 91 条脚本化断言全部通过,0 个非预期失败。
  • A/B 结论(核心主张成立):在 head 上,真实 goal runtime 被真实证据窗口驱动——连续 3 次停滞 checkpoint(窗口截断 + claim 满 32)后 Goal 以 usage_limited / evidence_catalog / 停滞 reason 停止,verifier 恰好计费 3 次,停止那次 checkpoint 被丢弃、streak 逐步持久化在 journal 记录上;同一场景在 base 构建上永不停止(跑满 6 轮仍 active,verifier 每轮照付,第 7 个 continuation 已发出)——即 PR 要熔断的无界压缩循环。不可用 verifier 结果(空 claims)在截断窗口下同样计数(head 3 轮停止 / base 5 轮仍 active)。
  • 次级语义全部验证:room checkpoint 与 edit/replace/证据受限 resume 重置 streak;paused/blocked resume、瞬时 verifier 失败、空轮次保留 streak;checkpointStalls 解析往返(0 拼为无字段,负数/小数/字符串拒绝)。11 个变异全部杀死,控制行 199/199 绿;goals 全量套件 16 文件 412 测试全绿;tsc --noEmit 在 head 为 0 错误(gate 已用植入错误证明为活门)。
  • Findings(均不阻塞):① 描述中「fix(goal): resume an evidence-limited Goal from a fresh window #9840 的后续 reset 未做」与代码不符——最后一个 commit 已实现并被我杀死验证,描述欠更新;② 描述中的测试数(404/192)与 tsc 基线(35 错误)相对合入 head 已过期(实际 412/199、0 错误),不影响结论。
  • 未覆盖:逐 commit 归因(浅克隆仅可达聚合 diff)、真实模型调用的 checkpoint verifier E2E、Web Shell UI 渲染、未触碰的 goalJudge/goal-tools/goalHook 路径。

Central claim and A/B proof

Central claim: a Goal whose checkpoints stall three times in a row (window truncated and claim list at the 32-cap, or a verifier result that could not be folded while the window overflowed) is settled usage_limited with limitKind: 'evidence_catalog' and the stall reason; the base build has no such terminator and pays a verifier call every turn forever.

Both arms drive the real compiled runtime (createGoalRuntime from each tree's dist/) through real evidence windows (101 records/turn overflow the 100-entry raw budget → truncated; 60 compact without overflow; 10 below threshold) with an injected journal as the wire — every persisted GoalStateRecord payload is the oracle. Base-side control identity asserted before trusting it: readlink -f of both dists points into its own tree, and the base dist contains 0 occurrences of checkpointStalls/isGoalCheckpointStalled (head: 15).

Cell Scenario Oracle HEAD BASE
1 6 stalled turns (101 rec, 32 claims) status, streak, verifier calls, journal stops at turn 3: usage_limited, evidence_catalog, stall reason, checkpointStalls: 3; verifier billed 3; 2 checkpoints materialized, stopping one discarded (settle carries the inherited checkpoint id); streak 1→2 persisted on the records never stops: 6 turns ran, still active, verifier billed 6, 6 checkpoints written, 7th continuation minted
2 unusable result (claims: []) while truncated same stops at 3, 0 checkpoints ever materialized 5 turns ran, still active, verifier billed every turn
3 busy turn: truncated, 31 claims streak absent active, 4 compactions, no streak (unchanged path)
4 stall, stall, room checkpoint, stall streak resets reset to absent, restarts at 1
5 stall, stall, transient verifier failure, stall streak preserved held at 2 through the failure, then stops at 3 (verifier billed 4)
6 stall, then a turn with no goal-owned records streak preserved held at 1
7 unusable result while window has room streak absent active, retried every turn

Witnesses: 01-ab-head-breaker-fires-at-3.png (head, 40/40) and 02-ab-base-unbounded-compaction-loop.png (base, 10/10). The pair is the load-bearing proof: identical scenario, one build stops at 3, the other keeps billing the verifier.

Secondary cells (head, compiled reducer/parser)

05-reducer-parse-cells-head.png, 25/25: edit and replace clear the streak; resume of an evidence-limited Goal (evidence_catalog and checkpoint_request) restarts the window and the streak; paused/blocked resume keeps both streak and cursor; goalLimitKindForReason has no branch for the stall reason and a legacy-shape record (reason present, limitKind absent) resumes without restarting the window — bounding exactly what "new records always persist limitKind" is load-bearing for; parse roundtrip restores 2, spells 0 as no field, and rejects −1, 1.5, "2", and Infinity.

Mutation matrix (vacuity proof of the new tests)

Control (no mutation): 199/199 green on the PR's probe set (runtime + reducer + checkpoint). Every mutant killed; each failure quoted in logs/mut-*-verbose.log is the intended behavioural mismatch (e.g. M2 leaves the Goal active/running with checkpointStalls: 3 instead of settling; M4's resume keeps checkpointStalls: 2). Witness: 03-mutation-matrix-all-killed.png.

Mutant Guard PR-claimed fails Observed
M0 control green 199/199
M1 recordCheckpoint increment 3 5
M2 threshold < vs <= 1 3
M3 room-check reset 1 1
M4 resume-window reset (reducer) 1 1
M5a parseGoalRecord allowed-keys 1 1
M5b parse restore spread 1 1
M6 predicate ignores truncation 2 2
M7 predicate ignores claim cap 1 1
M8 unusable-result branch (runtime) 1
M9 verifier parse error class (4-file suite) 1
M10 edit reset 1 1

No survivors, so nothing to adjudicate. M1/M2 exceed the PR's counts only upward: commits 2–6 of the PR added tests pinning the same guards after the description's probe numbers were written (see Corrections).

Corrections (description vs. code at the merged head)

  1. The fix(goal): resume an evidence-limited Goal from a fresh window #9840 follow-up is already in this PR. The description's Risk & Scope says "fix(goal): resume an evidence-limited Goal from a fresh window #9840 … is unmerged on this base; once it lands, that resume path should also clear checkpointStalls — a one-line follow-up." At the merged head, fix(goal): resume an evidence-limited Goal from a fresh window #9840 has landed (the reducer's evidence-limited resume restarts the window), and the PR's final commit (f45ae6f3) implements exactly that reset. Verified load-bearing: mutant M4 kills precisely the resume test, and reducer cell R3 shows the resumed Goal active with the streak, checkpoint, and limitKind cleared. A reviewer reading the description would believe a follow-up is still owed; it is not.
  2. Stale counts in the description. "404 tests, 16 files" → 412 tests at the merged head (04-goals-suite-gate-412-pass.png); "192 tests" probe set → 199; "35 pre-existing tsc errors" → tsc --noEmit in packages/core is 0 errors at head (gate proven live by planting a type error, which was caught; base shows one pre-existing @lydell/node-pty declaration-skew error outside src/goals/). All deltas are the PR's own later commits plus a newer base; none change the conclusions.

Findings (non-blocking)

  1. "Nothing crosses the wire" is true in the client-visible sense, not literally. checkpointStalls does travel inside the serialized goal snapshot on the daemon→client wire. What holds is the PR's actual point: no new GoalLimitKind member (protocol and the SDK's duplicated union both remain 'evidence_catalog' | 'checkpoint_request'), goalLimitKindForReason untouched, and every client parser whitelists fields — packages/webui's mapper silently drops unknown keys (and its own test pins dropping an unknown limitKind), so no client change is required and the streak is invisible to clients by construction. Noted so a future reader does not read the description as "the field is never serialized."
  2. A legacy-shape record with the stall reason but no limitKind would resume without a window restart (reducer cell R5). Such a record cannot be produced by this code (every settle persists limitKind alongside the reason), so this is a documentation of the load-bearing invariant, not a defect.

Not covered

  • Per-commit attribution. The checkout is shallow (merge-ref, depth 2); git rev-list HEAD^1..HEAD^2 reports 1 while the metadata lists 6 commits, so only the aggregate HEAD^1..HEAD diff was exercised.
  • Real-model checkpoint verifier E2E. The harness injects the verifier at its seam; createGoalCheckpointVerifier's model path is covered by the PR's unit tests (mocked client) and my M9 mutant (error-class survival), not by live model calls.
  • Web Shell UI rendering of the new usage_limited reason (no UI change in the PR; mapper evidence is static source reading plus the webui package's own pinned tests).
  • Untouched surfaces: goalJudge, goal-tools, goalHook, and the repo-wide suite/lint beyond packages/core goals + typecheck.
  • Interactive /goal UX and daemon restart-resume of a mid-streak Goal at the process level (the persistence roundtrip is covered at the record level by R6 and the journal payloads).

Methodology

Environment: CI verify container (node:22), merge-ref checkout; npm ci/npm run build pre-existing at head. Base control: git worktree add tmp/base-tree HEAD^1, packages/core rebuilt there (tsc --build, assets step skipped — irrelevant to the harness), packages/core/node_modules symlinked in (lockfile untouched by the PR, so dependencies are identical); control identity asserted by realpath + grep. Harnesses (ab-stall.mjs, reducer-cells.mjs, mutations.mjs in the artifact dir) drive compiled dist/ with injected journal/evidence-source/verifier/host seams — no stub of the code under test; raw logs in logs/. Assertion tally: A/B head 40 + A/B base 10 + reducer cells 25 + matrix 12 (1 control + 11 kills) + gates 4 = 91. One intermediate harness iteration mis-fed the verifier lambda (returning the function instead of its result); that run inadvertently exercised the unusable-result door, which the breaker also counts — the fixed harness is what the tallies count.

Flakiness gate log

rounds=5 files=4 skipped=0
file packages/core/src/goals/goal-checkpoint-verifier.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-checkpoint-verifier.test.ts
file packages/core/src/goals/goal-checkpoint.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-checkpoint.test.ts
file packages/core/src/goals/goal-reducer.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-reducer.test.ts
file packages/core/src/goals/goal-runtime.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-runtime.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/goals/goal-checkpoint-verifier.test.ts: PPPPP
  packages/core/src/goals/goal-checkpoint.test.ts: PPPPP
  packages/core/src/goals/goal-reducer.test.ts: PPPPP
  packages/core/src/goals/goal-runtime.test.ts: PPPPP

verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/goals/goal-checkpoint-verifier.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-checkpoint.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-checkpoint-verifier.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-checkpoint.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-checkpoint-verifier.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-checkpoint.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-checkpoint-verifier.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-checkpoint.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 5 · packages/core/src/goals/goal-checkpoint-verifier.test.ts: P (exit 0)
round 5 · packages/core/src/goals/goal-checkpoint.test.ts: P (exit 0)
round 5 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 5 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)

Evidence images

01-ab-head-breaker-fires-at-3

02-ab-base-unbounded-compaction-loop

03-mutation-matrix-all-killed

04-goals-suite-gate-412-pass

05-reducer-parse-cells-head

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Aug 25, 2026
Merged via the queue into QwenLM:main with commit 463809c Aug 25, 2026
236 of 246 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.22.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants