feat(goal): pause a Goal after three autonomous turns that make no progress - #11239
Conversation
…ogress An autonomous Goal had two bounds and neither noticed one going in circles. The token budget is the only thing `queueContinuation` stops for, 30M tokens by default, and the checkpoint stall streak measures the opposite problem: it fires when a Goal produces too much evidence to catalogue, and a Goal producing none never advances it at all. So a model that answers each turn with a paragraph of status and calls no tools continued until the budget ran out, with nothing on the record that could ever end the loop. A Goal now pauses after three consecutive autonomous turns that recorded no evidence-bearing tool result and proposed no terminal state. `get_goal` and `update_goal` results do not count -- they carry `provenance: 'goal_runtime'`, and a turn that only re-reads its own state is the clearest form of the idling in question. Only the runtime's own continuations are measured: a turn carrying the user's text is the user steering and restarts the streak, and so does a user turn reserved while the third quiet turn was still running. The wind-down hand-off is exempt from both halves. The streak lives on the record as `noProgressTurns`, exactly like `checkpointStalls`, so a restart cannot launder it, and edit, replace, and every resume clear it -- resuming is the user asking for another run at the objective, not for its last turn. The count comes from a new optional `takeGoalTurnToolResults` on the renamed `GoalTurnLedger`, fed by the recorder where tool results are already stamped with the Goal permit. Optional is load-bearing: a ledger that cannot answer switches the bound off rather than reporting every turn as idle, and the bound reads the count measured on this turn rather than the one on the record for the same reason. The stop is a pause, not `blocked` or `usage_limited`: nothing was spent and there is no evidence to cite, the Goal simply stopped producing. Following the pause-reasons design, no new `GoalStateCause` is introduced and no host changes -- every surface already renders a paused Goal's `lastReason`.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ — every required heading is filled in, and the Chinese translation is a real paragraph-for-paragraph rendering rather than a summary. Problem: observed, not theoretical, and the two load-bearing claims check out against the tree. Direction: aligned, and better corroborated than the description claims. Beyond the Codex and Claude Code comparisons in the design doc, claude-code's own CHANGELOG carries Size: core paths are touched ( I could name every downstream consumer, which is the bar for a core change here. The Approach: the scope feels right, and I could not find a materially simpler path. My own first instinct was to count tool calls from the runtime's turn state instead of plumbing a new accessor through the recorder — but the runtime never sees tool results, the recorder already stamps them with the Goal permit, and it already keeps a per-turn spend entry with identical consume-once semantics. Extending that ledger is the smaller change, not the larger one. The reuse discipline is good throughout: Two small things worth a thought, neither blocking. The rename is technically a drive-by, but it is genuinely motivated (the interface now carries what a turn spent and what it produced) and contained to 12 lines, so splitting it out would cost more than it saves. And the committed design doc lists The fail-open choices are the part I looked hardest at, and they are the right way round: an absent or throwing ledger answers Risk: no elevated risk signals — nothing in the diff matches the revert-correlated paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需小节都填写了，中文部分是逐段对应的真实翻译，不是摘要。 问题： 是已观测到的问题，不是理论性加固，而且两个关键论断在代码树里都核对得上。 方向： 对齐，而且佐证比 PR 描述里写的更强。除了设计文档提到的 Codex 与 Claude Code 对照，claude-code 自己的 CHANGELOG 里就有 规模： 触及核心路径（ 我能点名每一个下游消费者，这是核心改动的门槛。 方案： 范围合理，我没有找到明显更简单的路径。我自己的第一反应是从 runtime 的轮次状态里数工具调用，而不是给记录器新加一个访问器——但 runtime 根本看不到工具结果，记录器本来就带着 Goal permit 打标，而且已经维护着一个语义完全相同的"按轮次记账、消费一次"的开销条目。扩展这个 ledger 是更小的改动，不是更大的。整体的复用做得很好： 两点小的想法，都不构成阻塞。重命名严格说属于顺手改动，但它确有理由（这个接口现在同时承载一轮花了多少和产出了什么），而且只有 12 行，拆出去的成本高于收益。另外，提交进仓库的设计文档在产物清单里列了 我看得最仔细的是几处 fail-open 的取舍，方向是对的：ledger 缺失或抛错时返回 风险： 无升级风险信号 —— diff 里没有匹配到与 revert 相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewNo correctness blockers. The mechanism is wired end to end, and the parts I expected to be loose are not. What I checked rather than assumed:
One suggestion — the skipped-checkpoint guard is not pinned by any test.
The guard is correct and worth keeping for more than the wasted verifier call: with an attempt present, Minor, same theme: sequenceDiagram
participant P1 as Host
participant P2 as GoalRuntime finishTurn
participant P3 as ChatRecordingService
participant P4 as goal-reducer
participant P5 as Journal
participant P6 as Subscribers
P3->>P3: recordToolResult stamps the permit, skips goal_runtime
P1->>P2: finishTurn with the turn permit
P2->>P3: takeGoalTurnToolResults for the turn
P3-->>P2: a count, or nothing when it cannot answer
P2->>P2: unmeasured or wind-down leaves the streak alone
P2->>P2: undelivered, any tool result, or a proposal resets it to zero
P2->>P2: otherwise the persisted streak advances by one
P2->>P4: reduceGoalTurnFinished carrying the streak
P4-->>P2: the next Goal record
P2->>P5: journal turn_finished
alt streak reached three and no user turn is waiting
P2->>P5: journal pause with the shared reason
P2->>P6: broadcast pause, no checkpoint and no continuation
else
P2->>P6: broadcast turn_finished
P2->>P2: queueContinuation
end
TestingThis is an unattended CI run, so the evidence below is the PR's own CI, read through the API — no PR code was built, run, or checked out here. Nothing was driven locally and there is no tmux capture to paste; real-scenario testing is N/A on this path. No check on the reviewed commit has failed. The two that matter most for this diff — the unit suite and lint/typecheck on Linux — were still running when this comment was posted, so they are listed as pending rather than guessed at; the finalize job rewrites the table in place once they settle. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The author reports 390 passing tests across Not verified: that a live model answering only in prose actually gets paused after three turns, and that Sandboxed verification would settle this: 中文说明代码审查没有正确性阻塞项。这套机制是端到端接通了的，而我原以为会松的地方并不松。 我实际核对过、而不是想当然的部分：
一条建议——跳过 checkpoint 的那道保护没有任何测试钉住。
这道保护是对的，而且它的价值不止于省掉一次 verifier 调用：一旦有 attempt， 一个小点，同一主题： （时序图见上，中文不重复：记录器在记录工具结果时按 permit 计数并排除 goal_runtime；finishTurn 取走本轮计数；测不到或交接轮则不动计数，未送达/有工具结果/有提案则清零，否则加一；交给 reducer 得到下一条记录并落盘 turn_finished；若计数达到 3 且没有用户轮在等，就落盘 pause、广播 pause，既不发 checkpoint 也不发续跑；否则照旧广播 turn_finished 并排队续跑。） 测试这是一次无人值守的 CI 运行，所以下面的证据是通过 API 读到的、本 PR 自己的 CI——这里没有构建、运行或 checkout 任何 PR 代码。本地没有驱动过任何东西，也没有 tmux 输出可以贴；这条路径上真实场景测试为 N/A。 被审查的 commit 上没有任何 check 失败。对这个 diff 最关键的两项——Linux 上的单测与 lint/typecheck——在本评论发出时仍在运行，所以如实列为 pending，不做猜测；它们结束后 finalize 任务会就地改写这张表。 作者报告 未验证：真实模型只输出文字时是否确实在三轮后 pause，以及 沙箱验证可以把这件事定下来： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid across every stage; the only things I'd change are non-blocking, and I'd merge this once CI lands. Stepping back: this is what a well-scoped core change looks like. I wrote down my own approach before reading the diff — a per-turn counter fed by the recorder, persisted on the record, paused rather than blocked, failing open when the count is unavailable — and the PR arrived at the same design, then went further on the two details I would have got wrong. It tests the count measured on the finishing turn rather than the count sitting on the record, so a streak restored from disk cannot stop a Goal whose ledger cannot see the turn that would have relieved it. And it distinguishes "the ledger cannot answer" from "the turn produced nothing", which is the whole difference between a bound that protects the user and one that eats working Goals. The question I cared most about was whether anything could make this fail unsafe — a Goal stopped while it was genuinely working. There is exactly one way: a host that records tool results without threading the Goal permit, whose turns would then all read as idle. I checked all four. They all go through the shared provenance helper, and it excludes Choosing What keeps this at 4 rather than 5 is small and I've named it in the review above: the guard that skips the checkpoint on the stopping turn is never actually exercised, because the new harness configures neither On the behavioural claim: I could not watch a live model get paused, and neither could the author — the interactive walkthrough is explicitly not run on this branch. What I have instead is eleven assertions against the real runtime and reducer, plus a static trace of the wiring from every host through the recorder to If I picked this up in six months I would thank the author: the design doc explains every exemption and why the failure direction was chosen, the constants and reason strings sit with their siblings, and the code reads like the file it lives in. The unit suite and lint were still running when I wrote this, so I'm not approving in this run — approval deferred until CI lands green on 中文说明Confidence: 4/5 —— 每个阶段都站得住；我想改的地方都不构成阻塞，CI 一绿我就会合。 退一步看：这是一个范围收得很好的核心改动该有的样子。在读 diff 之前我先写下了自己的方案——由记录器喂给它的按轮计数、持久化在记录上、pause 而不是 blocked、拿不到计数时 fail-open——而这个 PR 落到了同一个设计上，并且在我会做错的两个细节上走得更远。它判断的是本轮实测到的计数，而不是记录上读到的计数，所以从磁盘恢复出来的连停计数，不会停掉一个 ledger 根本看不到"本可以缓解它的那一轮"的 Goal。它也区分了"ledger 答不上来"和"这一轮什么都没产出"，而这两者的区别，正是一道保护用户的闸与一道吃掉正在干活的 Goal 的闸之间的全部差别。 我最在意的问题是：有没有什么会让它 fail-unsafe——一个 Goal 在真正干活的时候被停掉。恰好只有一种途径：某个 host 在记录工具结果时没有把 Goal permit 带上，那么它的每一轮都会被读成空转。我把四个 host 全查了。它们都走共享的 provenance 助手，而且该助手一致地排除了 选 让它停在 4 而不是 5 的东西很小，我在上面的审查里已经点名：停机那一轮跳过 checkpoint 的那道保护实际上从来没有被执行过，因为新的 harness 既没有配置 关于行为层面的论断：我没能看着一个真实模型被 pause，作者也没能——交互式走查明确没有在本分支上跑过。我手上代替它的是十一条针对真实 runtime 与 reducer 的断言，外加一条从每个 host 经记录器到 如果六个月后由我来维护这份代码，我会感谢作者：设计文档解释了每一处豁免、以及为什么选这个失败方向；常量与原因字符串都放在它们的同类旁边；代码读起来就像它所在的那个文件。 我写这段的时候单测与 lint 还在跑，所以本次运行不发批准——批准推迟到 CI 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
A ledger that returns something other than a finite count was read as zero, which spends one of the three turns on a measurement that never happened. It now says the same thing as a ledger that throws: the bound is off for that turn. Also pin that the turn which spends the streak takes no checkpoint -- the guard was in the code with nothing holding it there -- and point the design doc's verification list at the pull request's evidence rather than at an ignored path.
|
Took both suggestions from the review, in 16d65d1:
On the last run's red checks: 中文说明评审里两条建议都采纳了，在 16d65d1：
关于上一次运行里红的检查： |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 non-finite ledger answer mapped to idle instead of unmeasured — packages/core/src/goals/goal-runtime.ts:385 — already reported (comment 5564540945)
- R1-8 checkpoint-skip guard not pinned by the tests the diff adds — packages/core/src/goals/goal-runtime.ts:1641 — already reported (comment 5564540945)
Test Plan (not a blocker): src/goals/goal-reducer.test.ts — no such file or directory; src/goals/goal-runtime.test.ts — no such file or directory; src/services/chatRecordingService.test.ts — no such file or directory; Tests 390 passed — this review observed 1959, 298, 1818, 509, 6362 passed.
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
Test Plan（非阻断）：src/goals/goal-reducer.test.ts — no such file or directory; src/goals/goal-runtime.test.ts — no such file or directory; src/services/chatRecordingService.test.ts — no such file or directory; Tests 390 passed — this review observed 1959, 298, 1818, 509, 6362 passed。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
… budget, and a stalled checkpoint Three of the bound's neighbours outrank it, and the pause write opened a window where a fourth was lost. A user turn reserved while the pause record was being appended was discarded: `beginTurn` is synchronous and does not queue, the guard read the reservation once before the await, and the later `queuedTurnKey` assignment erased it. The guard now re-reads the reservation after the write and declines the pause when one appeared, the way `stopForSpentBudget` re-validates after its own write. The journal may then hold a `pause` record the runtime never adopted; that is the shape the budget stop already leaves, and the conservative side to land on -- a restart recovers a paused Goal with its reason, and resume is the whole remedy. The bound also stands down on the turn where a better-fitting limit coincides with it. A spent token budget reaches the continuation gate, which grants the wind-down hand-off and stops as `usage_limited`/`token_budget` instead of an idle pause with no `limitKind`. A Goal carrying a checkpoint stall streak lets its checkpoint run, so the stall breaker stops it with the evidence-catalog reason instead of a pause whose remedy would resume it into the same overflowing window. The paused snapshot is now built by the same `settledSnapshot` helper as the usage-limited one, so the settled shape lives in one place. Tests: a reservation landing inside the pause write is served; a restored streak at the limit is not spent by a turn the ledger cannot measure; the stall breaker wins three quiet overflowing turns; the budget wins the crossing turn. Each goes red under the corresponding mutation.
The reason is runtime-emitted and headless-reachable, so it must not point at a slash command a process that has already exited cannot run; and it said "no tool results" about a Goal whose transcript shows three turns of `get_goal`/`update_goal` results, which are recorded but do not count. It now says "nothing to judge", offers resume, and offers edit only together with resume -- editing a paused Goal leaves it paused. The user doc says the same and names the bookkeeping reads that do not count. The constant joins the headless-register test, which now also rejects any `/goal ` mention, and the shared-constant validation loop.
The replay projected every `pause` record as the user typing `/goal pause`. The no-progress bound is the first `pause` the runtime writes with nobody at the keyboard, so a replayed session attributed the autonomous stop to the person who was away. A `pause` now replays as a user message only when its reason says the user typed it (`GOAL_PAUSE_REASON_COMMAND`, exported on the goal wire) or when the record predates pause reasons; every other pause replays as the paused card alone, which carries the reason. `resume` and `clear` are unchanged.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: test-efficacy probe — the probe runner executed against an unbuilt tree outside the PR worktree, so no whole-file revert, deletion mutant or per-hunk probe ran (harnessValidated: null): whether the new no-progress tests fail when the behaviour is removed is unmeasured, not clean.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Test Plan (not a blocker): src/goals/goal-reducer.test.ts — no such file or directory; src/goals/goal-runtime.test.ts — no such file or directory; src/services/chatRecordingService.test.ts — no such file or directory; Tests 390 passed — this review observed 23604, 1959, 28862, 298, 1818, 504, 6362 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：test-efficacy probe — the probe runner executed against an unbuilt tree outside the PR worktree, so no whole-file revert, deletion mutant or per-hunk probe ran (harnessValidated: null): whether the new no-progress tests fail when the behaviour is removed is unmeasured, not clean.
未审查：反向审计——评审时间预算不足，未能开始第 6 轮。
Test Plan（非阻断）：src/goals/goal-reducer.test.ts — no such file or directory; src/goals/goal-runtime.test.ts — no such file or directory; src/services/chatRecordingService.test.ts — no such file or directory; Tests 390 passed — this review observed 23604, 1959, 28862, 298, 1818, 504, 6362 passed。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
|
Description corrected against the head commit — four things had gone stale as the fixes landed. The "no host changes" claim was wrong. The test plan numbers were from the first commit. Now 165 runtime cases and 1,027 across the five files, with The Evidence was rerun rather than re-quoted. The The rerun is more interesting than the original, so it replaces it rather than joining it: The model called nothing but And the description no longer points at Build note for anyone reproducing: 中文说明按 head 提交把描述里四处已经过期的内容改掉了。 「不改任何 host」这句是错的。 测试计划里的数字还是第一个提交时的。 现在是 165 个 runtime 用例、五个文件共 1,027 个；命令里补上了 Evidence 是重跑的，不是重新引用的。 旧那段里的 重跑的结果比原来那次更有意思，所以是替换而不是并列（表格见英文部分）。模型除了 描述里也不再指向 复现提示： |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Approved at head 4639aa01e2.
Scope of what I signed. A Goal that spends three consecutive autonomous turns producing nothing to judge now pauses instead of burning turns forever, with the bound yielding to anything that outranks it. 17 threads, every one answered and resolved; the two blocking rows on the page are anchored four commits back at 16d65d1112.
The Critical both reviewers raised (their R1-1, same defect) is closed, and I read the fix rather than the reply. The reservation race was real: nextTurnKey was captured before the await on the pause journal write, beginTurn is synchronous and does not pass through enqueue, so a keystroke landing inside that window reserved a turn that the subsequent queuedTurnKey = verifying ? nextTurnKey : undefined then erased with a provably-stale local. At this head: goal-runtime.ts:1678 reads the key, :1684 guards the bound on it, and after the write :1709-1710 re-reads and abandons the pause if anything arrived, so the commit at :1745 and the promotion branch at :1748 both work off the fresh value. serves a user turn reserved while the pause was being written covers exactly the interleaving that was reported, with the reservation created during the append. I also checked the shape the reply conceded — a pause record that lost the race stays in the journal uncommitted — and agree it is the conservative side: a restart recovers a paused Goal with its reason, and /goal resume is the whole remedy. It is the same residue stopForSpentBudget already leaves, and both are written down at :1704-1708.
The newest commit's reordering needed a follow-up check, and it passes. :1658 now builds the checkpoint attempt unconditionally and :1733 drops it only when the pause actually fires, which is the right fix for "a Goal drowning in prose evidence hits the bound first and never checkpoints" — the steering path keeps its checkpoint. But that also means the turn_finished record at :1661-1676 can carry a checkpointPending for an attempt the live runtime just discarded. I chased that through recovery: goal-persistence.ts:53-60 scans from the end and takes the last goal_state payload, so the pause record written immediately after supersedes it and nothing dangles on restart; and when the pause write itself throws (the catch at :1692, pinned by shows the no-progress stop even when the settle write fails), the earlier record's pending checkpoint is precisely what you want restored. That is the failure mode this shape usually has, so it is the first thing I looked for.
Precedence and fail-open directions are the part I'd have asked for unprompted. Three named tests pin that the bound loses to a waiting reservation (:5948), to the checkpoint stall breaker on an overflowing window (:6091), and to a token budget spent on the same turn (:6146) — the last two close the "the new bound jumps the queue in front of the gates that already existed" complaint, including the one about the spent-budget gate living only inside queueContinuation(). Uncountable ledger answers stay unmeasured rather than counted as quiet: three separate tests for "cannot count", "throws", "answers with something that is not a count" leave the bound off, which is the correct bias for a breaker that pauses a user's Goal.
Also verified in code, not in replies. The ACP replay no longer attributes a runtime pause to the user — transcript-replay.ts emits /goal pause only for a typed command or a record predating reasons, with the card carrying lastReason either way (this was the sharpest of the findings: a fabricated user message in a resumed transcript). GOAL_PAUSE_REASON_NO_PROGRESS now reads "recorded nothing to judge and no proposal", which is what the implementation counts, replacing the earlier "no tool results" that the get_goal/update_goal exclusion contradicted. The duplicated snapshot builders are collapsed into settledSnapshot (goal-runtime.ts:430, :458-461). The CLI host maps the new status to the right broadcast (use-llm-stream.ts: cause: status === 'paused' ? 'pause' : status), and restarts the streak on a turn the user drove / exempts the wind-down hand-off / clears the streak when the user resumes pin the three ways the streak must not accumulate.
CI facts. At this head Test (ubuntu-latest) passed in 23m50s, Lint & Static in 16m13s, Integration Tests (no-AK) in 8m36s, and the Desktop Shell and meta lanes are green; review-pr was still running and nothing has reported red. I did not re-run the core suite locally — that lane is the one that executes the new ~600 lines of goal-runtime.test.ts, and it is green. One PENDING review row from qqqys sits on this head, so a human pass may still be landing; my vote is on the content. The page's CHANGES_REQUESTED is two stale rows anchored at 16d65d1112, and @qwen-code /triage is what clears them.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Independent verification at head 4639aa01e — the one Critical is closed, and I read the fix rather than the replies
No findings from me. This is a confirmation pass, because AGENTS.md treats an approval as a claim that has to be checked against the code as it stands, and both blocking reviews on the page are anchored four commits back at 16d65d111.
R1-1 (raised independently by @wenshao and by ci-bot, same defect) — closed
The reservation race was real as reported: nextTurnKey captured before the await on the pause journal write, beginTurn synchronous and not routed through enqueue, so a keystroke inside that window reserved a turn that the later queuedTurnKey = verifying ? nextTurnKey : undefined then erased with a provably stale local.
At this head, in goal-runtime.ts:
:1678let nextTurnKey = queuedTurnKey;—let, notconst, which is the whole point.:1684if (noProgressLimitReached && !nextTurnKey)— the bound yields to a turn reserved before it.:1686-1691theawait options.journal.recordGoalState(...)pause write — the race window.:1692-1698catch {}with the reasoning inline, so a lost settle write cannot strand an "active" Goal that nothing will continue.:1709nextTurnKey = queuedTurnKey;— re-read after the await, mirroringstopForSpentBudget's post-write re-validation.:1710if (nextTurnKey) noProgressSnapshot = undefined;— the reservation wins and the pause is abandoned, not merely postponed.:1745and:1748both consume the refreshed value, so neither the queue write nor the promotion branch can act on the stale capture.
The reordering in the newest commits needed a second check, and it holds
:1658-1660 builds nextCheckpoint gated only on !activeProposal, i.e. independently of the no-progress bound, and :1661-1676 journals turn_finished with checkpointPending when it exists. :1733 then does checkpointAttempt = noProgressSnapshot ? undefined : nextCheckpoint; — the attempt is dropped only when the pause actually fires. So when :1710 abandons the pause, noProgressSnapshot is undefined and the steering turn keeps its checkpoint, which is the behaviour R1-7 asked for.
That shape leaves a turn_finished record carrying a checkpointPending for an attempt the live runtime discarded. I chased the recovery path myself rather than accepting the reply: goal-persistence.ts:48-62, selectGoalRecoveryFromRecords, iterates for (let index = records.length - 1; index >= 0; index -= 1), skips non-goal_state records, and returns on the first parseable V2 payload. Last goal_state wins, so the pause record written immediately after supersedes the dangling checkpointPending and nothing dangles on restart. The same reading covers the conceded residue of a pause record the runtime never adopted: recovery yields a paused Goal with its reason, and resume is the whole remedy.
R1-17 / R1-3 (user-facing accuracy) — addressed
GOAL_PAUSE_REASON_NO_PROGRESS at goal-protocol.ts:420-421 now reads "Three Goal turns in a row recorded nothing to judge and no proposal", not "no tool results" — which matches the counting rule that deliberately excludes get_goal/update_goal via provenance: 'goal_runtime'. The reason is also host-neutral, so the headless case reads correctly.
One item I did not verify
R1-16 — whether the CLI's detached tool-continuation settle knows that finishTurn can now produce paused. I could not locate that file within budget, so I am certifying nothing about it. It is a Suggestion, it is resolved with an author reply, and the runtime side does carry the shared shape (goal-runtime.ts:406 status: 'paused' | 'usage_limited'), so this is a gap in my pass rather than a finding.
CI at this head
Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, OpenTUI no-flicker gate and TUI parity snapshots (ink vs opentui) are all success. One review-pr is still in_progress.
What is actually holding this PR
Not the code. reviewDecision is CHANGES_REQUESTED because of two reviews anchored at 16d65d111 — @wenshao's pullrequestreview-5131979023 and ci-bot's pullrequestreview-5132835914 — and neither has been dismissed by the commits since. The Critical both of them raised is the one I verified closed above.
Comment rather than approve: this was invoked as a review, and @wenshao's request-changes is a human reviewer's to withdraw, not mine to dismiss. On the evidence I read at this head I have no objection to approval; the ci-bot review is the stale one and a fresh review-pr round is already running.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. Approved.
Tier: Deep — persisted format change (noProgressTurns field), async state machine with a race condition path, hard to reverse.
Scope: goal-{protocol,reducer,runtime}.ts, chatRecordingService.ts, acp-bridge/transcript-replay.ts, use-llm-stream.ts, config.ts, all associated test files. NOT reviewed: docs/ (design doc and user guide) — content only, no code impact.
What I checked
Class 1 (contract asymmetry)
GoalTurnLedger.takeGoalTurnToolResultsoptional → ledger absence switches bound off rather than marking every turn idle. Conservative direction is correct: "nothing measured" ≠ "nothing happened."ChatRecordingService.takeGoalTurnToolResultsreturns0for an unknownturnId; runtime treats0as "no results, increment streak" andundefinedas "can't answer, disable bound." Mapping is correct.- Single-entry pattern (new
turnIdresets the count) mirrors existinggoalTurnTokens. Sequential Goal turns make this safe in practice.
Class 2 (API/compatibility)
GoalTurnTokenLedger → GoalTurnLedger,tokenLedger → ledger: diff confirms onlyconfig.tsandgoal-runtime.test.tsupdated; author states nothing external referenced either name.GOAL_PAUSE_REASON_COMMANDnewly exported ongoal-wire.ts— additive, callers gain a stable constant.- No new
GoalStateCausevalue: keeps change out of state parsers, persistence format, legacy projection.
Class 3 (error handling)
- Failed pause journal write: swallowed, in-memory snapshot shows stop anyway, continuation blocked. Covered by "shows the no-progress stop even when the settle write fails."
- Race (
beginTurnduring journal write): post-fix code re-reads livequeuedTurnKeyafter the await and abandons the pause when a reservation appeared, mirroringstopForSpentBudget's post-write re-validation. Covered by "serves a user turn reserved while the pause was being written."
Class 10 (stated intent vs code)
use-llm-stream.tsadds'paused'to terminal-status check — matches PR description's "one host branch did need widening." Test parameterized to covercompleteandpaused.transcript-replay.ts: returnsundefinedfor all non-GOAL_PAUSE_REASON_COMMANDpauses — matches design doc. Broader than just the new reason; also corrects pre-existing misattribution of runtime-written pauses (SESSION_DISPOSED,HEADLESS_RUN_ENDED, etc.). Intentional.noProgressTurns: zero spelled as no field, matchingcheckpointStalls. Parser rejects negative and fractional counts.
Cross-check
R1-1 (Critical, goal-runtime.ts:1684) — filed by both wenshao and qwen-code-ci-bot: user-turn reservation during pause journal write was dropped because nextTurnKey was captured before the await and not re-read after. Author fixed in commit 16d65d1; current head 4639aa01 contains the fix (let nextTurnKey re-read after the write + "serves a user turn reserved while the pause was being written" test). Confirmed addressed.
Remaining suggestions from both reviewers (R1-2 through R1-17: wording, minor structural notes, single-entry pattern documentation) — none are blockers.
Not covered
- Mutation probe (no working tree): 17 runtime test cases look non-vacuous from static analysis — they check specific state transitions (
status: 'paused',noProgressTurnscount,lastReasontext) that would fail with the bound code reverted. PR also includes an end-to-end live model run as execution evidence. - macOS / Windows: author acknowledges
⚠️ ; no platform-specific code in the diff.
Reviewed with AI assistance.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approving at head 4639aa01e. Evidence is in pullrequestreview-5138243426; restating what the vote rests on.
The one Critical on this PR (R1-1, raised independently by @wenshao and ci-bot — the reservation race where a user turn reserved during the pause journal write was erased by a stale local) is closed, and I read the fix at head rather than the replies: goal-runtime.ts:1678 declares let nextTurnKey, :1684 guards the bound on it, :1709 re-reads it after the await, :1710 abandons the pause when a reservation arrived, and :1745/:1748 both consume the refreshed value.
The reordering in the newest commits also holds: :1658-1660 builds the checkpoint attempt independently of the bound and :1733 drops it only when the pause actually fires, so an abandoned pause leaves the steering turn with its checkpoint. The turn_finished record can still carry a checkpointPending for an attempt the live runtime discarded; I chased that through recovery myself — goal-persistence.ts:48-62 scans records backwards and returns the first parseable goal_state, so the pause record written immediately after supersedes it and nothing dangles on restart.
GOAL_PAUSE_REASON_NO_PROGRESS (goal-protocol.ts:420-421) now reads "recorded nothing to judge and no proposal", matching the counting rule that excludes get_goal/update_goal.
CI at this head: Test (ubuntu-latest), Lint & Static, Integration Tests (no-AK), web-shell E2E Smoke, OpenTUI no-flicker gate, TUI parity snapshots all success; one review-pr still running.
One gap in my pass, stated so it is on the record rather than implied as covered: I did not verify R1-16 (whether the CLI's detached tool-continuation settle handles paused). I could not locate that file within budget. It is a Suggestion, resolved with an author reply, and the runtime side carries the shared status: 'paused' | 'usage_limited' shape at :406 — so I am not certifying it, just not blocking on it.
|
Released in v0.23.1. |
6 participants
What this PR does
A Goal now stops itself when its autonomous turns stop producing anything to judge. After three consecutive runtime-driven turns that recorded no evidence-bearing tool result and proposed no terminal state, the Goal pauses and says so, and the user resumes it or edits the objective.
Only the runtime's own continuations are measured, identified by the same host delivery mark that already decides whether an objective announcement was delivered. A turn carrying the user's own text is the user steering, and it restarts the streak; so does a user turn reserved while the third quiet turn was still running, which is served rather than stopped in front of. The wind-down hand-off turn is exempt from both halves, because it is asked to hand off rather than to work.
get_goalandupdate_goalresults do not count as progress: they carryprovenance: 'goal_runtime', and a turn that only re-reads its own state is the clearest form of the idling this bound exists to notice.The streak is persisted on the Goal record the same way the checkpoint stall streak is, so a daemon restart or a session resume cannot launder it, and zero is spelled as no field. Edit, replace, and every resume clear it, including the resume of a Goal this bound stopped: resuming is the user asking for another run at the objective, and starting that run three-quarters of the way to the bound would end it after a single quiet turn.
The count comes from a new optional accessor on the turn ledger, fed by the chat recorder where tool results are already stamped with the Goal permit. Optional is load-bearing rather than cosmetic: a ledger that cannot answer, or that throws, switches the bound off for the session instead of reporting every turn as idle. "Nothing measured" is not "nothing happened", and the failure mode of getting that backwards is stopping a Goal that was working. For the same reason the bound tests the count measured on the finishing turn rather than the count read off the record.
The stop is a pause rather than a blocked or usage-limited state.
blockedis a verdict the verifier reaches on cited evidence, and this bound has no evidence to cite;usage_limitedsays an allowance was spent, and nothing was. Following the pause-reasons design already in the tree, no new Goal state cause is introduced, and the stop renders through the paused-Goal path every surface already has: the/goalcard, the ACP goal state update, and the headless goal state event all carry the reason with no per-host work, and in the TUI the runtime's broadcast arrives as the goal-state stream event the transcript already renders. One host branch did need widening. The interactive path that renders a terminal card right after a turn ended by a turn-terminating tool listedcomplete,blockedandusage_limitedonly, so a Goal this bound paused at the end of anupdate_goalturn finished silently there; it now includespaused, mapped to thepausecause the card is keyed to. The reason is host-neutral ("Three Goal turns in a row recorded nothing to judge and no proposal. Resume the Goal to try again, or edit its objective into one the model can act on and then resume it."), since a headless process has exited by the time anyone reads it.The bound yields to the limits that describe the Goal better on the turn where they coincide with it: a spent token budget still reaches the continuation gate, which grants the wind-down hand-off and stops as
usage_limited/token_budget; a Goal carrying a checkpoint stall streak lets its checkpoint run, so the stall breaker stops it with the evidence-catalog reason instead of a pause whose remedy would resume it into the same overflowing window. A user turn reserved while the pause record is being written is served rather than stopped in front of, exactly as one reserved earlier; the journal may then hold apauserecord the runtime never adopted, the same shape the budget stop already leaves when its post-write re-validation fails.One host-side projection changes: the ACP transcript replay used to replay every
pauserecord as the user typing/goal pause. This is the firstpausethe runtime writes with nobody at the keyboard, so a pause now replays as a user message only when its reason says the user typed it, or when the record predates pause reasons; every other pause replays as the paused card alone, which carries the reason.Why it's needed
An autonomous Goal had two bounds and neither one noticed a Goal going in circles.
The token budget is the only thing the continuation gate stops for, and it defaults to 30,000,000 tokens. The checkpoint stall streak measures the opposite problem: it fires when a Goal produces too much evidence to catalogue, and a Goal producing none never advances it at all — there is an existing test asserting exactly that, "keeps the stall streak when a turn records no evidence at all". So a model that answers each turn with a paragraph of status and calls no tools kept being continued until the budget ran out, with nothing on the record that could ever end the loop, while the user watched the turn count and the spend climb.
Both comparable runtimes bound this. Codex stops a Goal after three consecutive turns whose only tool activity was failed execs. Claude Code caps consecutive Stop-hook blocks at eight.
The threshold here is a constant rather than a setting, and three matches the checkpoint stall bound and the blocked-audit streak already in this file family. A configurable
goals.*family belongs with the separate work on turn and time budgets.Reviewer Test Plan
How to verify
The behaviour is runtime-side and fully covered by unit tests against the real runtime, reducer, and recorder. The interesting cases are the ones where the bound must not fire.
The seventeen runtime cases are grouped under "no-progress bound". Worth reading rather than just running: a ledger that cannot count, one that throws, and one that answers with something that is not a count all leave the bound switched off; a restored streak is only spent by a turn the ledger could actually measure; a waiting user turn outranks the bound, including one reserved while the pause record was being written; a spent token budget and an overflowing evidence window each stop the Goal with their own reason instead; the turn that spends the streak runs no checkpoint; and a failed settle write still shows the stop rather than stranding an active Goal nothing will continue.
To exercise it interactively, set an objective the model can only answer in prose and cannot complete on its own, for example: report your status in one sentence each turn without running any tool, and treat the objective as complete only after the user types a given word. Three autonomous turns later the Goal pauses with the new reason, and
/goal resumegives it the whole allowance again rather than its last turn.Evidence (Before & After)
No new UI. The paused card and
/goaloutput are the existing ones that already render every other pause reason, so there is nothing visually new to screenshot. The evidence is the Goal state stream, run headless against a live model with the same objective on both sides.Before — released 0.22.3. The Goal never stops. Killed at a five-minute wall clock, not by anything in the runtime:
After — this branch, rerun on the head commit. The same objective stops itself and the run exits
result success,is_error: false:This run is more informative than a clean three-and-stop, so it is the one reported. The model called no tool but
update_goalon seven turns, and every one of those restarted the streak — a terminal proposal is something to judge, even when the verifier rejects it. Turns 12 to 14 are the first three consecutive turns that proposed nothing, and the bound fires on the third. So the bound is a floor under turns that produce nothing at all, not under a model that keeps proposing; that shape is still bounded only by the token budget, and belongs with the separate work on turn and time budgets.A Goal that is working is untouched. Same branch, an objective the model can only satisfy by reading files:
The objective, the steps and the raw figures above are all in this description; the working notes live outside the repo, so there is nothing further to open.
Tested on
Environment (optional)
Linux,
npm run buildthenscripts/start.js, live model, headlessstream-jsonoutput. Baseline run against the globally installed 0.22.3.Risk & Scope
/goal resumerestores the full allowance./goal resumeand the mid-Goal user message were exercised in unit tests but not in the live headless runs above, which end when the Goal stops. Token accounting scope, budget display, and configurable turn or time budgets are separate work.GoalTurnTokenLedgertoGoalTurnLedgerand its option key fromtokenLedgertoledger, now that it carries both what a turn spent and what it produced; nothing outside the runtime referenced either name.Linked Issues
Part of #4228.
中文说明
这个 PR 做了什么
现在，当一个 Goal 的自主轮不再产出任何可供判定的东西时，它会自己停下来。连续三个由运行时驱动的轮次既没有记录可作证据的工具结果、也没有提出终态提案，Goal 就会 pause 并说明原因，由用户决定是恢复还是修改目标。
只有运行时自己发起的续跑轮会被计入，判据是 host 的送达标记——也就是现在已经用来判断目标变更通知是否送达的那个信号。承载用户自己文字的轮次是用户在把方向，它会让计数从头开始；在第三个空转轮还在跑时被用户预约的轮次也一样，它会被正常服务，而不是被挡在停机前面。预算 wind-down 的交接轮对两边都豁免，因为它被要求的是交接而不是干活。
get_goal与update_goal的结果不算进展：它们的 provenance 是goal_runtime，而一个只反复读自己状态的轮次，正是这道闸要拦的空转最典型的样子。计数像 checkpoint 连停计数一样持久化在 Goal 记录上，这样守护进程重启或会话恢复都洗不掉它，并且 0 以"没有这个字段"来表达。edit、replace 和每一次 resume 都会清零，包括对被这道闸停下的 Goal 的 resume：恢复是用户要求对这个目标再跑一轮，如果从离上限只差四分之一的地方起跑，一个空转轮就会再次停机。
计数来自轮次 ledger 上新增的一个可选方法，由聊天记录器在记录工具结果时喂给它——那些结果本来就已经打上了 Goal permit。"可选"在这里是有承重作用的，不是装饰：一个答不上来或者抛错的 ledger 会让这道闸在整个会话里关闭，而不是把每一轮都报成空转。"没测到"不等于"什么都没发生"，把这两者搞反的后果是停掉一个正在干活的 Goal。出于同样的理由，这道闸判断的是本轮实测到的计数，而不是记录上读到的计数。
这次停机是 pause，不是 blocked 也不是 usage_limited。blocked 是 verifier 基于被引用的证据做出的判定，而这道闸恰恰没有证据可引；usage_limited 表示某项额度被花光了，而这里什么都没花掉。沿用树上已有的 pause 原因设计，本 PR 不新增 GoalStateCause，停机走的是各界面本来就有的 paused Goal 渲染路径：
/goal卡片、ACP 的 goal state 更新、headless 的 goal state 事件都能直接带上原因，无需逐个 host 改；TUI 这边运行时的广播会以 goal-state 流事件到达，而那条路本来就会渲染。有一处 host 分支确实需要放宽：交互路径上「由终止回合的工具结束的那一轮」结束后渲染终态卡片的判断只列了complete、blocked、usage_limited，所以被这道闸在update_goal轮末停掉的 Goal 在那里是无声的；现在把paused也纳入，并映射成卡片所依据的pausecause。原因文案是宿主中立的（"Three Goal turns in a row recorded nothing to judge and no proposal. Resume the Goal to try again, or edit its objective into one the model can act on and then resume it."），因为 headless 进程在有人读到它之前就已经退出了。在同一轮里与其他限制重合时，这道闸让位给更能描述该 Goal 的那道：花光 token 预算的轮次仍会走到续跑闸，由它授予 wind-down 交接轮并以
usage_limited/token_budget停机；记录上带 checkpoint 连停计数的 Goal 会让 checkpoint 先跑，由熔断以证据目录的原因停机，而不是给出一个 resume 后会重新进入同一个溢出窗口的 pause。在 pause 记录落盘期间被预约的用户轮同样会被服务而不是被挡在停机前面；此时 journal 里可能留下一条运行时未采纳的pause记录，这和预算停机在写入后复验失败时留下的形状一致。有一处 host 侧的投影随之改动：ACP transcript 回放原本把每条
pause记录都回放成用户敲了/goal pause。这是运行时第一次在无人在场时写pause，所以现在只有原因表明是用户敲的（或记录早于 pause 原因）时才回放成用户消息；其他 pause 只回放带原因的 paused 卡片。为什么需要
自主 Goal 原本有两道闸，但没有一道能发现 Goal 在原地打转。
续跑闸唯一会为之停下的是 token 预算，默认 30,000,000。checkpoint 连停计数量的是相反的问题：它在 Goal 产出的证据多到装不下时才触发，而一个完全不产出证据的 Goal 根本推不动它——树上就有一个用例断言了这一点，"keeps the stall streak when a turn records no evidence at all"。于是一个每轮只回一段状态、不调任何工具的模型会一直被续跑到预算耗尽，记录上没有任何东西能结束这个循环，而用户只能看着轮数和花费一起往上爬。
两个对照实现都设了这道闸。Codex 在连续三轮只有失败的 exec、没有成功工具调用时把 Goal 置为 Blocked。Claude Code 把连续的 Stop-hook 拦截次数上限设为 8。
这里的阈值是常量而不是设置项，3 与本文件家族里已有的 checkpoint 连停上限和 blocked 审计连续数一致。可配置的
goals.*设置族属于轮数与时间预算那一项单独的工作。评审验证方式
如何验证
这个行为在运行时侧，由针对真实 runtime、reducer 和 recorder 的单测完整覆盖。真正值得看的是那些这道闸不该触发的用例。
17 个 runtime 用例归在 "no-progress bound" 下。值得读而不只是跑：数不了工具结果的 ledger、会抛错的 ledger、以及返回值不是一个计数的 ledger，都会让这道闸保持关闭；被恢复出来的计数只会被 ledger 真正测量得到的那一轮花掉；等待中的用户轮优先于这道闸，包括在 pause 记录写入过程中才登记的那一个；token 预算花光和证据窗口溢出各自以自己的原因停掉 Goal；花掉计数的那一轮不跑 checkpoint；落盘失败时仍然显示停机，而不是留下一个没人会去续跑的 active Goal。
要交互式地体验，可以设一个模型只能用文字回答、且自己无法完成的目标，例如：每轮用一句话报告状态、不许调用任何工具，并且只有当用户输入某个指定词时才算完成。三个自主轮之后 Goal 就会带着新原因 pause，而
/goal resume会把整份额度还给它，而不是只还最后一轮。证据（Before & After）
没有新 UI。paused 卡片和
/goal输出都是现成的、已经在渲染其它所有 pause 原因的那些，没有新的视觉内容可以截图。证据是 Goal 状态流，两边用同一个目标、对真实模型跑 headless。改动前 —— 已发布的 0.22.3。 Goal 永远不停。是被五分钟的墙钟掐掉的，不是被运行时里的任何东西停下的：
改动后 —— 本分支，在 head 提交上重跑。 同一个目标自己停了下来，进程以
result success、is_error: false退出：这次运行比"干净地三轮就停"更有信息量，所以报告的是它。模型除了
update_goal之外没调用任何工具，而那七轮每一轮都让计数重新开始——终态提案是"可判断的东西"，哪怕 verifier 把它驳回。第 12 到 14 轮是头三个连续什么都没提案的轮次，这道闸在第三轮触发。所以它是"完全没有产出的轮次"的下限，不是"一直在提案的模型"的下限；后者目前仍然只被 token 预算兜底，属于轮数与时间预算那部分单独的工作。正在干活的 Goal 不受影响。 同一分支，一个模型只能靠读文件才能满足的目标：
目标、步骤和上面这些原始数字都在本描述里；工作笔记不在仓库内，没有别的东西需要打开。
测试平台
环境（可选）
Linux，
npm run build后用scripts/start.js，真实模型，headlessstream-json输出。基线用全局安装的 0.22.3 跑。风险与范围
/goal resume会还回完整额度。/goal resume与 Goal 进行中插入用户消息这两条路径有单测覆盖，但没有在上面的真实 headless 运行里走一遍——那些运行在 Goal 停机时就结束了。token 记账口径、预算展示，以及可配置的轮数或时间预算都是单独的工作。GoalTurnTokenLedger更名为GoalTurnLedger，选项键从tokenLedger更名为ledger，因为它现在同时承载一轮花了多少和产出了什么；运行时之外没有任何地方引用过这两个名字。关联 Issue
属于 #4228 的一部分。