feat(goal): tell the model the objective changed, once, when it changed - #10013
Conversation
The continuation prompt ended every turn with "the objective in that data block is the current one and supersedes any earlier Goal objective in this conversation, including one you already started working on." That sentence does two jobs, and only one of them is true every turn. The standing half is a guard: objective-shaped text reaches the model from places the runtime does not control -- earlier turns, tool output, file contents -- so the data block has to be asserted as the only objective on every turn. That half stays. The other half warns that the objective changed. It went out on turn 2 and turn 40 of Goals whose objective never moved, so by the time an edit finally landed the warning had been identical noise for the whole run and carried nothing. It is now a separate line sent only on the first continuation after the objective actually changed. The runtime decides by remembering the (goalId, revision) pair whose objective it last handed to a host: a different pair means an edit bumped the revision or a replace minted a new Goal over the old one, and no previous pair means this Goal's first continuation, which supersedes nothing. Keying on the revision alone would miss replace, whose new Goal starts at revision 1 like a fresh create. A start the host refused puts the announcement back, so a retried continuation still carries the notice. Only continuations announce an objective -- a user turn carries the user's own text -- so the state is per-continuation and held in memory: losing it across a restart costs one prompt line, while the objective itself travels in the data block every turn and get_goal stays authoritative. Mutation probes: dropping the roll-back on a refused start fails the redelivery test (113 green); firing whenever any previous pair exists fails 3; keying on the revision alone fails the replace test (113 green).
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 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 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
Re-run on the current head after the autofix review rounds.
Moving on to code review. 🔍 中文说明在 autofix 处理完评审意见后，对当前 head 重新运行。
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal before reading the diff: track the last objective the model actually received, fire a one-shot notice when the next continuation differs, and commit the announcement on real delivery rather than acceptance — because every host resolves
What I verified by reading the code against the worktree:
No critical findings. One suggestion: the PR description is stale relative to the implementation — it still describes the earlier The one piece that changed since the previous review round is the delivery-mark lifecycle — worth the picture: sequenceDiagram
participant P1 as Goal runtime
participant P2 as Host (TUI, headless, ACP)
participant P3 as Model
P1->>P1: flushContinuation compares against the last delivered objective
P1->>P2: startGoalTurn, accepted with the flag attached
P2->>P3: continuation prompt sent
P2->>P1: markTurnDelivered with the turn key
alt turn settles after delivery
P1->>P1: commit the announcement, notice never fires again
else dropped before delivery
P1->>P1: discard the announcement, notice stays owed to the replacement
end
CI test evidenceFetched once for the reviewed commit via the check-runs API — no polling. There are no red checks on this head: all three
Sandboxed verification would settle the remaining behavioural claim: Not verified: the model-facing effect of the reworded prompt — that is a prompt-engineering claim no CI here can measure; the golden tests pin both renderings byte-for-byte and the runtime tests pin the flag mechanics instead. TUI capture: N/A — the change has no terminal surface, and CI-path runs never execute PR code. 中文说明代码审查读 diff 之前我独立给出的方案是：记住模型上一次真正收到的 objective，下一次续跑不同时发一次性通知，并以真实投递而非接受为准来提交宣告——因为所有 host 都在入队时就让
对照 worktree 逐点核实：
无严重问题。一条建议：PR 描述相对实现已过时——仍在描述早先的 (goalId, revision) 组合设计、"五个运行时用例"（现在是 17 + 2 个渲染器用例），以及针对旧键控的变异检验。代码自身的文档块把真实设计讲得很清楚；建议合入前把描述更新一致，免得记录失真。 自上轮评审以来变化的部分是投递确认生命周期，值得图示（见正文时序图）：运行时在 flush 时与上次投递的 objective 比较并附标志，host 发送提示词后以轮次键确认投递；结算时已投递则提交宣告、通知不再发出，未投递则丢弃宣告、通知留给替补。 CI 测试证据对受审提交经 check-runs API 一次性拉取，未轮询。该 head 上没有红色检查：三个 沙箱验证可以定论剩余的行为主张： 未验证：改写后提示词对模型行为的影响——那是提示词工程主张，这里的 CI 无法度量；黄金测试逐字节固定两种渲染，运行时测试固定标志机制。TUI 抓取：N/A——改动没有终端界面，且 CI 路径从不执行 PR 代码。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid across every stage; the one nit is the stale PR description, not the code. Stepping back: this is what a well-run review loop looks like from the other side. The previous round raised the hard question — acceptance is not delivery — and the answer wasn't a patch but a small, coherent redesign: content keying so no-op edits stay silent, and Two things I'm doing after posting this, transparently: approving pinned to the reviewed commit, and dismissing this bot's two earlier Reservations, for the record: the PR body still describes the pre-review design (revision-pair keying, five runtime cases) and should be refreshed before merge — suggestion only. And the prompt-engineering claim itself (that a one-shot notice reduces drift better than a standing line) is not something any test here can prove; if a maintainer wants A/B evidence on the built artifact before merge, 中文说明置信度：4/5——各阶段均扎实；唯一的小问题是 PR 描述过时，与代码无关。 退一步看：这是评审循环从另一侧看应有的样子。上一轮提出了那个尖锐的问题——"接受"不等于"投递"——而回答不是打补丁，是一次小而自洽的重新设计：按内容键控使无实际变化的 edit 保持安静；每个 host 在真实发送点调用 发出本评论后我会做两件事，明示如下：批准并固定到受审提交；撤销本机器人早先的两条"需要修改"评审——它们针对的是已被取代的提交，其反馈已逐线程处理并 resolve，若继续保留，PR 将被一个已不存在的状态挡住。 保留意见，记录在案：PR 描述仍在讲评审前的设计（revision 组合键控、五个运行时用例），建议合入前刷新——仅为建议，不阻塞。另外，提示词工程主张本身（一次性通知比常驻行更能减少漂移）不是这里的任何测试能证明的；如果维护者希望合入前拿到构建产物上的 A/B 证据，Stage 2 已点名 — Qwen Code · qwen3.8-max 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. ✅
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: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Round 2 review — COMMENT
Reviewed at: 6a39f6ddfb936d2658b7870d55a92887fa8bf324 (unchanged since round 1)
Gap disclosed: Git fetch (getaddrinfo failure on Windows) blocked worktree creation, so no build, test, or reverse audit ran. Findings are based on the diff read from gh pr diff --repo QwenLM/qwen-code 10013.
Previous round-1 findings — verification
R1-1 (Critical) — host refusal rollback
The PR adds announcedObjective = previouslyAnnounced inside handleStartFailure, which covers the synchronous throw path. The async rejection path would need the existing .then(undefined, handleStartFailure) chain to exercise the same rollback — I cannot see the surrounding code to confirm that chain is present because the worktree was not created. For the portion that is visible, the rollback is race-safe via isCurrentPermit(startedPermit). The inference from the redelivers when the host never took the prompt test (which passes startGoalTurn rejection via failures.push(new Error(...))) is that the async path does route through handleStartFailure — the test passes, so the plumbing works. Marking Probably Addressed pending a reviewer confirming the full .then(undefined, handleStartFailure) site.
R1-2 (Suggestion) — announcedObjective never reset on completion/clear 🔴 STILL OPEN
announcedObjective is set on every startGoalTurn and only rolled back inside handleStartFailure. It is never reset when a Goal completes or is cleared. The consequence:
create Goal A→ continuation →announcedObjective=(A, 1)complete Goal A→create Goal B→ first continuation of B →objectiveUpdated: true(becauseA.goalId !== B.goalId)
The model was not "working on" Goal A at that point — A was completed. Telling it "the objective changed since your last turn — stop work that only served the previous objective" is misleading; the model should get the normal prompt without the change notice. The same issue applies to clear followed by create.
The test suite has no case for create → complete → create → continuation (or clear), so this gap is untested. The fix would be resetting announcedObjective = undefined in the complete and clear dispatch handlers.
R1-3 (Suggestion) — keys on (goalId, revision) pair, not content 🔊 DESIGN CHOICE, still open
The PR author's rationale is clear and consistent: revision alone misses replace (whose new Goal starts at revision 1), and the replace test proves the pair-based key works. The niche case of editing an objective to the same text (content unchanged, revision bumped) is a false positive — the notice fires but nothing meaningful changed. This is a minor edge case unlikely to occur in practice, so it is a Suggestion rather than a Critical.
R1-4 (Suggestion) — no test for host threading hooks 🔴 STILL OPEN
The 7 new tests cover the runtime (5 cases) and the renderer (2 golden-prompt cases), but not the full propagation through the three host implementations (TUI useMessageQueue → useGeminiStream, headless nonInteractiveCli, ACP Session). The type system catches mis-keyed spreads, but there is no behavioral test verifying that the objectiveUpdated flag actually reaches the wire on the first continuation after an edit. The useGeminiStream.test.tsx change only updates the hardcoded prompt copy — it does not test the flag propagation.
New findings (round 2)
R2-1 (Suggestion) — restart action not covered
The dispatch handler for restart is not shown in the diff (it was outside the changed range). If restart creates a new Goal at revision 1 with a new goalId, the first continuation would correctly fire the notice. If restart reuses the same goalId, the notice might not fire when it should. The test suite covers edit, replace, pause, resume, and create, but not restart. Recommend adding a test case for restart or documenting that it is out of scope.
R2-2 (Suggestion) — abandon action not covered
Same concern as restart. If abandon clears the Goal state, announcedObjective is stale for the next Goal. Recommend adding a test case.
Summary
| ID | Severity | Title | Status |
|---|---|---|---|
| R1-1 | Critical | Host refusal rollback | Probably Addressed |
| R1-2 | Suggestion | Reset on completion/clear | Still open |
| R1-3 | Suggestion | Pair key vs content key | Design choice, open |
| R1-4 | Suggestion | No host plumbing test | Still open |
| R2-1 | Suggestion | restart not covered |
New |
| R2-2 | Suggestion | abandon not covered |
New |
Remediation focus: R1-2 (reset announcedObjective on complete/clear) and R1-4 (a behavioral test through one host) would close the Suggestion-level gaps.
— Qwen Code via /review (v0.22.0+)
…enLM#10013) Address review findings on the objective-updated notice: - Commit the announcement when a continuation reaches the model, not when a host merely accepts it: every host resolves startGoalTurn at enqueue time, and a turn dropped before delivery (TUI Escape, ACP cancel, headless preempt) left the one-shot notice lost for good, while a turn dropped after acceptance but before the model saw it made a replace fire the notice for an objective the model never received. Hosts now confirm delivery through markTurnDelivered; undelivered turns leave their notice owed to the replacement continuation. - Key the notice on objective content instead of the (goalId, revision) pair so a no-op or trim-equivalent edit no longer asserts a change that did not happen. - Reset the announcement when a Goal ends verifier-accepted or is cleared, so a fresh Goal's first continuation never claims it replaces completed work. - Add flag witnesses for the four host-side threading hops and for each delivery-confirmation call site.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #10013Commit: All four inline findings are resolved in code. Each claimed defect was first reproduced with a failing test on the pristine commit ( Findings[rv:5019395176] Review body — "Partially reviewed — gaps disclosed" (CHANGES_REQUESTED)Informational: the review body carries no findings of its own; its actionable content is the four inline findings below, all addressed. The disclosed gaps (integration tests skipped in CI for the PR build, reverse audit stopped by the review time budget) are workflow-level review disclosures, not defect claims. [rc:3853394241] [Critical] Rollback only covers host refusal, not accepted-but-undelivered drops — ResolvedReproduced on the pristine commit with two failing tests (drop arm: Root cause:
Witnesses: Two pre-existing tests pinned the old semantics and were updated with it: the replace witness now delivers the superseded Goal's turn first (a replace over a Goal the model never saw is exactly the misfire being fixed), and the host-refusal redelivery witness finishes the create turn first (the notice is only owed once the prior objective was handed over). [rc:3853394260] [Suggestion]
|
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Round summary — no code changesEvery actionable point in the round-2 review was verified against the current PR Key context: the round-2 review states it was reviewed at Finding dispositions
Verification
中文说明本轮总结 — 无代码变更第 2 轮评审中的每个可执行要点都已在当前 PR 头（ 关键背景：第 2 轮评审明确说明其评审对象是 各发现的处理
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
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: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 1c": none — though I note the reachability window of the reported finding (queue-drain vs direct UserQuery admission timing in AppContainer.tsx ) is the part I coul…; "agent 3c": none — no check was cut short..
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 1c"：none — though I note the reachability window of the reported finding (queue-drain vs direct UserQuery admission timing in AppContainer.tsx ) is the part I coul…；"agent 3c"：none — no check was cut short.。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 AutoFix ran out of time before finishing (timeout (2700000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (2700000ms)）（第 2/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32953779216 🧠 Handled by Qwen Code · model/模型 |
…pdated # Conflicts: # packages/cli/src/acp-integration/session/Session.test.ts # packages/cli/src/acp-integration/session/Session.ts # packages/cli/src/nonInteractiveCli.test.ts # packages/cli/src/nonInteractiveCli.ts # packages/cli/src/ui/hooks/useGeminiStream.test.tsx # packages/cli/src/ui/hooks/useGeminiStream.ts # packages/cli/src/ui/hooks/useMessageQueue.test.ts # packages/cli/src/ui/hooks/useMessageQueue.ts # packages/core/src/goals/goal-continuation-prompt.test.ts # packages/core/src/goals/goal-continuation-prompt.ts # packages/core/src/goals/goal-runtime.ts
… saw finishTurn committed the in-flight continuation's announcement unconditionally, but finishing a turn proves the permit was used, not that the continuation prompt was sent under it. Two host paths claim a queued continuation's permit and send other text instead: a Cron, Notification or Teammate submission drained ahead of the TUI's own drain, and a direct user query admitted through the same slot. Neither renders the continuation nor marks it delivered, so the turn finished with the notice unsent and the runtime recorded it as delivered. It now commits on the host's delivery mark, exactly as releaseTurn already did. The accept-time reset of the announcement was firing for every accepted terminal proposal, including blocked, which is resumable: a Goal edited while blocked and then resumed sent no notice for a real change, while pause -> edit -> resume did. Only complete resets it; a blocked Goal keeps the objective the model holds, as usage_limited does. Tests: the notice suite now marks delivery before finishing, as real hosts do; new cases pin finishing without the prompt, a delivery mark with a stale turn key, blocked -> edit -> resume, and complete -> clear -> create. Two headless cases drive the real runtime through both continuation promotion sites so the delivery mark there is covered. Mutation probes: unconditional commit -> 1 fail (143 green); mark ignoring the turn key -> 1 (143); reset on every terminal -> 1 (143); each headless promotion mark deleted -> exactly its test (132/133).
|
变异检验:无条件提交 → 恰好挂 1(143 绿);标记忽略 key → 1(143);所有终态都重置 → 1(143);两个晋升点标记分别删除 → 各恰好挂自己的用例(132 绿)。goals 468/468;CLI 四套件 2063 + 133;goal 相关 tsc 0 错;prettier/eslint 干净。 |
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: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.ts:655 — [review] delivery-mark wrapper written three times and already divergedpackages/core/src/goals/goal-runtime.ts:247 — [review] announcedObjective docstring contradicts finishTurn's settle-on-delivery semanticspackages/cli/src/ui/hooks/useGeminiStream.test.tsx:537 — [probe] mark-before-send ordering not asserted at three test sites
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round report — PR #10013 (no code changes this round)This round made no code changes. All five unresolved inline findings were already resolved in code by commit Per-finding verification
VerificationEvery command below was run at HEAD
Not addressed this round (deliberately)
中文说明Autofix 轮次报告 — PR #10013（本轮无代码改动）本轮没有做任何代码改动。五条未解决的 inline findings 已经由提交 逐条验证
验证以下所有命令均在 HEAD
本轮有意不处理的内容
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
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: 32 passed · 0 failed · 32 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：32 通过 · 0 失败 · 32 总计 抖动门： Verification reportPR #10013 Deep Verification —
|
| cell | build | old change-implication line | one-shot notice line | standing guard (reworded) | markTurnDelivered API |
result |
|---|---|---|---|---|---|---|
| control | base fc0e8276 (worktree rebuild) |
9/9 every turn | 0/9 (does not exist) | 0/9 | absent | behaved exactly as predicted — assertion passed |
| head | 2263d48a (CI build) |
0/9 | 3/9 — T4 (post edit #1), T7 (post edit #2), T8 (redelivery after T7 dropped) | 9/9 | present | all 8 head assertions passed |
Flip shape: every-turn change wording 9/9 → 0/9; actionable change notice 0 → exactly the turns where the objective changed and the model had not seen it, including the owed redelivery after an accepted turn was dropped before delivery. The T4 prompt tail on head matches the PR's claimed "after" text byte-for-byte. Witness: 01-ab-cells-head-vs-base.png; raw logs logs/ab-head.log, logs/ab-base.log.
Base-side control note: the base worktree's packages/core had no node_modules, so external resolution (ajv) fell through to the root's ajv@6. Both arms were equalized by symlinking the identical nested packages/core/node_modules (realpath /__w/qwen-code/qwen-code/packages/core/node_modules/ajv) into the base tree; the PR touches no package.json/lockfile, so the shared root install is a clean control.
Mutation matrix — the PR's new tests are not vacuous
Unmutated control green first: goal-runtime.test.ts + goal-continuation-prompt.test.ts = 156/156. Then one mutant per guard, same command; every revert restored and the tree re-verified clean (git status --short empty). Witness: 02-mutation-matrix-core.png.
| mutant | guard under test | outcome |
|---|---|---|
M1 finishTurn commits announcement unconditionally |
commit-on-delivery (finish path) | killed — exactly 1 fail: keeps the notice owed when a turn finishes under the permit without the prompt |
| M2a fire whenever any previous announcement exists | content keying | killed — 11 fail |
| M2b revision-only keying (store+compare revision) | content keying vs rejected design | killed — 3 fail: fires after a replace…, stays off for edits that leave the objective text unchanged, does not leak a refused turn's announcement… |
| M3 drop refused-start roll-back | settleCurrentTurnAnnouncement(false) in handleStartFailure |
survived 156/156 → adjudicated below |
| M4 delivery mark ignores turn key | stale-key guard in markTurnDelivered |
killed — exactly 1 fail: ignores a delivery mark carrying a stale turn key |
| M5 reset announcement on every terminal proposal | complete-only reset |
killed — exactly 1 fail: fires after an edit made while the Goal was blocked |
M5b remove the complete reset entirely |
same guard, other direction | killed — exactly 1 fail: stays off for a Goal that replaces a verifier-accepted one |
M6 renderer reverted to base behavior (if (false), old wording) |
both new prompt tests + golden prompts | killed — 4 fail (2 golden, 2 new), all on expected-vs-actual prompt text |
| M7 delete one headless promotion delivery mark (CLI) | headless markGoalTurnDelivered call site |
killed — exactly 1 fail of 134: marks a follow-up continuation delivered after a tool-terminated segment (132 passed, 1 pre-existing skip) |
M8 releaseTurn commits announcement unconditionally |
commit-on-delivery (release path) | killed — 3 fail (the drop-before-delivery trio) |
M3 survivor adjudication — redundant defence, not dead code. m3-sequencer.mjs constructs the one interleaving the clause can decide: a host that marks a turn delivered and then refuses it, so the next dispatch's settle(currentTurnDelivered) would cash the stale mark. Measured (03-m3-sequencer-clause-decides.png): clause present → refused announcement discarded, edit back to the held text stays quiet (objectiveUpdated=false); clause removed → refused announcement committed, notice re-fires (objectiveUpdated=true). The clause therefore decides an outcome — it is not dead code — but no shipped host can produce the interleaving (TUI marks on drain/submit of a queued turn, ACP marks at modelStarted, headless marks at promotion; all strictly after a successful startGoalTurn accept, and a refused turn is never queued). Classification: redundant defence, correct as it stands — with M1/M8 holding the finish/release settle sites and M4 the key guard, the set closes the hazard from every reachable direction.
Positive control: 9 mutants turned tests red through the identical runner command that observed the M3 survival, so "survived" here cannot mean "the suite never ran".
Targeted gates (re-measured at head)
| gate | result |
|---|---|
packages/core npx vitest run src/goals/ |
16 files, 468 passed / 0 failed (PR body said 404 — drift from two merges of main, see Corrections) |
packages/cli named suites (useMessageQueue.test.ts, useGeminiStream.test.tsx, nonInteractiveCli.test.ts, src/acp-integration) |
42 files, 2197 passed / 1 skipped / 0 failed (110.6 s) |
packages/core tsc --noEmit |
clean (exit 0) |
packages/cli tsc --noEmit |
clean (exit 0) |
eslint on all 12 changed files |
clean; gate proven live (planted unused variable was reported, then removed) |
prettier --check on all 12 changed files |
clean |
residual old-line grep (including one you already started working on) |
0 source occurrences at head |
Corrections
- Commit-message mutation claim is stale for the final code (description correction, no code change requested). The head commit body says "dropping the roll-back on a refused start fails the redelivery test (113 green)". At the final head that mutation survives 156/156: after the series moved the announcement commit from accept-time to delivery-time, the refused-start roll-back became defence-in-depth (see M3 adjudication). The redelivery property itself is fully pinned — it just moved onto the commit-on-delivery design (M1/M8 kill it). Nothing in the diff is wrong; only the narrated probe no longer matches the shipped code.
- Test-count drift. PR body cites 404 core goal tests and 40 + 233 + 130 + 1739 CLI tests; at head the same commands yield 468 and 2197 + 1 skipped. The branch merged
maintwice after the description was written; everything is green.
Findings
- (nit) Redundant-defence clause carries a stale commit-message justification.
settleCurrentTurnAnnouncement(false)inhandleStartFailureis kept and correct, but the commit message still claims removing it fails a test it no longer fails. No action needed on the code; a maintainer reading the history should know the clause is belt-and-braces, not load-bearing. - (nit) Description numbers drifted. As above; both counts re-measured green.
No blocking findings. In particular, the three consequence classes worth ruling out were ruled out: the notice can not fire for a change that did not happen (M2a/M2b/content-keying cells, incl. no-op and trim edits and edit-back-to-held-text), can not be lost for a change that did happen (redelivery cells T8, M1/M8, stale-key M4, blocked→edit→resume M5), and can not leak into user turns (covered by suite + traced: promoted user turns never receive a matching delivery mark).
Not covered
- Per-commit attribution. Checkout is depth 2; only the merge commit, base tip, and PR head exist locally (
git rev-parse --is-shallow-repository= true;git rev-list HEAD^1..HEAD^2= 1 vs 7 commits in the metadata snapshot). The aggregateHEAD^1..HEADdiff is what was verified; the intermediate-commit mutation numbers quoted in commit bodies were not individually re-run. - Trial merge into current
main. No GitHub token in this sandbox; the merge-ref base tip (fc0e8276) already postdates the snapshot'sbaseRefOid(5a883243…) because the branch mergedmaintwice. Whethermainmoved further since is not observable here. - Live TUI/ACP/headless runs against a real model. Host wiring verified via the mock-free unit/integration suites and code tracing, not a live Goal session; the daemon-restart tradeoff (in-memory announcement lost) is documented in the PR and was not exercised — its stated cost is one missing prompt line while the objective still travels in the data block.
- Repo-wide lint/test gates. Only the changed files (eslint/prettier) and the affected workspace suites were run, per scope.
- Windows/macOS behavior (Linux container only).
Methodology
Environment: CI verify container (node:22-bookworm), repo at refs/pull/10013/merge depth 2; npm ci + npm run build pre-done at HEAD. A/B: base side rebuilt as git worktree add tmp/base-tree HEAD^1 and compiled with tsc -p … --noCheck into the worktree's own dist/; harnesses import dist modules by absolute file URL and assert realpath containment at load, so no head code can leak into the base cell (the only shared dependency, nested node_modules/ajv, is the identical realpath on both arms). Scenario and expectations are encoded in ab-harness.mjs (arm-specific; a base cell failing as predicted counts as a passed assertion). Mutations were applied to head source with the edit tool, run through npx vitest run <the two test files>, and reverted with git checkout -- (tree verified clean after each); the M3 adjudication patched a copy of the built dist instead of source, restoring from backup afterwards. Gates are the exact commands cited above; eslint liveness was proven by planting and removing a violation. Raw logs: logs/ab-head.log, logs/ab-base.log, logs/goal-runtime.js.dist.bak; harnesses: ab-harness.mjs, m3-sequencer.mjs; evidence images: evidence/01-ab-cells-head-vs-base.png, evidence/02-mutation-matrix-core.png, evidence/03-m3-sequencer-clause-decides.png.
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/nonInteractiveCli.test.ts: (cd packages/cli) npx --no-install vitest run ./src/nonInteractiveCli.test.ts
file packages/cli/src/ui/hooks/useGeminiStream.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/hooks/useGeminiStream.test.tsx
file packages/cli/src/ui/hooks/useMessageQueue.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/hooks/useMessageQueue.test.ts
file packages/core/src/goals/goal-continuation-prompt.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-continuation-prompt.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/cli/src/acp-integration/session/Session.test.ts: PPPPP
packages/cli/src/nonInteractiveCli.test.ts: PPPPP
packages/cli/src/ui/hooks/useGeminiStream.test.tsx: PPPP
packages/cli/src/ui/hooks/useMessageQueue.test.ts: PPPP
packages/core/src/goals/goal-continuation-prompt.test.ts: PPPP
packages/core/src/goals/goal-runtime.test.ts: PPPP
verdict: timeout
summary: only 4 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/hooks/useGeminiStream.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/hooks/useMessageQueue.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-continuation-prompt.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/hooks/useGeminiStream.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/hooks/useMessageQueue.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-continuation-prompt.test.ts: P (exit 0)
round 2 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 3 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/hooks/useGeminiStream.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/hooks/useMessageQueue.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-continuation-prompt.test.ts: P (exit 0)
round 3 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 4 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 4 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 4 · packages/cli/src/ui/hooks/useGeminiStream.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/hooks/useMessageQueue.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-continuation-prompt.test.ts: P (exit 0)
round 4 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 5 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 5 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅
Superseded: this review stood on an earlier commit whose feedback was addressed and resolved; the current head was fully re-reviewed and approved pinned to its SHA. Dismissing own stale request-changes on triage re-run.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round report — PR #10013 (no code changes)This round made no code changes: the feedback batch contains nothing actionable. Feedback triage
Window state
VerificationNo code changed this round, so no build/typecheck/lint/test commands were required or run; only read-only inspection of git state, checks, and feedback was performed. The current head 中文说明Autofix 轮次报告 — PR #10013（本轮无代码改动）本轮未做任何代码改动：本批反馈中没有需要处理的内容。 反馈分类
窗口状态
验证本轮没有代码改动，因此不需要也没有运行任何 build/typecheck/lint/测试命令；仅对 git 状态、检查与反馈做了只读检查。当前 head Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Maintainer verification — built and run locally against the real stackI built both arms from source and drove the change through all three Goal hosts — a real Verdict: the PR does what it says, and every load-bearing line is load-bearing. Every externally observable claim in the description reproduced on the real stack. Four one-line mutations of the shipped bundle each break exactly the behaviour their line exists to protect, and three more source-level mutations reproduce the commit messages' own probe counts to the test. I found no correctness problem. Two minor notes and an honest coverage gap are at the bottom. Arms
Net diff BASE→HEAD is exactly the 12 files in the PR (+930/−8). Environment: macOS 26.6.2 (darwin 25.6.0), Node v24.18.1, isolated 1. The line itself, on the wire
Same 8 scenarios, 39 continuation prompts per arm:
A separate 31-turn headless run of an unchanged Goal on HEAD: 31 continuation prompts, one distinct standing line, 0 notices. No drift over a long run. 2. Scenario matrix — real
|
| # | what the operator did | objective really changed? | HEAD notices | reading |
|---|---|---|---|---|
| A | nothing, 4 turns | no | 0 / 4 | steady Goals are silent |
| B | /goal edit → different text |
yes | 1 / 5 | fires on the first continuation after the edit (rev 2), quiet after |
| C | /goal edit → same text, padded |
no (revision 1→2) | 0 / 5 | keyed on content: a no-op edit bumps the revision but says nothing |
| D | /goal <new> → replace, new Goal |
yes (revision back to 1) | 1 / 5 | keying on revision alone would have missed this |
| E | pause → edit → resume | yes | 1 / 5 | fires once after resume |
| F | pause → resume, no edit | no | 0 / 5 | pause/resume alone announces nothing |
| G | clear → create a new Goal | new work item | 0 / 5 | a fresh Goal does not claim it replaces completed work |
| H | edit, then a direct user query | yes | delivered (see note) | the ACP host renders the continuation and appends the user text into one request, so the notice went out inside that request |
BASE is 0 in every row for the trivial reason that the line does not exist there.
3. What a user actually sees
Real TUI, real keystrokes (/goal … then /goal edit …), captured through node-pty + xterm.js. The fake model's reply echoes what the prompt it just received actually carried, so the screenshot is evidence rather than decoration.
BASE — the standing line claims a change on every single turn, including turn 6 where a change actually happened:
HEAD — turns 1–5 are change-free, turn 6 (the first after /goal edit) carries the notice, turns 7–8 go quiet again:
The headless host was driven the same way (-p "/goal …"): HEAD emits the reworded standing line, BASE the old one, neither emits a spurious notice.
4. Mutation probes on the shipped bundle
One-line edits to dist/chunks/*.js, then the same five scenarios re-run through the real daemon. Cells are notices / continuation prompts.
| mutant | one-line change | what broke |
|---|---|---|
| M1 | objectiveUpdated = announced !== undefined (drop the content compare) |
the noise the PR removes comes straight back: 3/4 on the steady Goal, 4/5 everywhere else |
| M2 | delete markTurnDelivered() from Session.ts |
the notice never fires at all (0/5 on B and D) — acceptance really is not delivery |
| M3 | clear no longer forgets the announcement |
a false notice on the first turn of the newly created Goal (G: 1/5) |
| M4 | key on (goalId, revision) instead of objective text |
false notice on the no-op edit (C: 1/5) and the replace notice is lost (D: 0/5) — both halves of the PR's argument for content-keying, in one probe |
Every mutant is caught by exactly the scenarios its line is responsible for, and by no others.
5. The commit messages' own mutation claims, reproduced
Source-level mutations against packages/core goal-runtime.test.ts (144 tests):
| mutation | result | killed by |
|---|---|---|
unconditional commit in finishTurn |
1 failed / 143 passed | keeps the notice owed when a turn finishes under the permit without the prompt |
| delivery mark ignores the turn key | 1 failed / 143 passed | ignores a delivery mark carrying a stale turn key |
reset on every terminal, not just complete |
1 failed / 143 passed | fires after an edit made while the Goal was blocked |
| drop the content compare | 11 failed / 133 passed | the whole notice suite |
| key on the revision alone | 3 failed / 141 passed | replace · refused-turn leak · no-op edit |
The three "1 failed / 143 green" figures match the second and third commit messages exactly.
6. Tests, lint, merge
packages/core→npx vitest run src/goals/— 16 files, 468 tests, all pass.packages/cli→npx vitest run src/ui/hooks/useMessageQueue.test.ts src/ui/hooks/useGeminiStream.test.tsx src/nonInteractiveCli.test.ts src/acp-integration— 42 files, 2196 passed, 1 skipped.eslintandprettier --checkclean on all 12 changed files.npm run build+npm run bundleclean on both arms.- The old sentence
including one you already started working onno longer appears anywhere in the tree, and both new lines live only in the renderer plus its tests — no second copy to drift, and no docs reference the prompt text. - The shipped bundle confirms the flag is threaded through all three hosts:
objectiveUpdatedandmarkTurnDeliveredare present in the ACP, headless and TUI chunks; both are absent from the BASE bundle.
Notes
-
The Reviewer Test Plan is stale. It says
packages/core … 404 tests, 16 files; the actual count on head is 468 across 16 files, because the two follow-up commits added tests. Worth refreshing the numbers before merge — nothing else in the plan is wrong. -
Pre-existing, not introduced here, and mildly in this PR's favour. When
/goal editlands while a continuation is already in flight, the daemon/ACP host preempts and the replacement request carries two Goal data blocks inside one user message — the stale one and the new one — followed by the standing line and (on HEAD) the notice. My matrix produced 7 such requests per arm, identical count on BASE and HEAD, so this is existing host behaviour. It does argue for the rewording though:supersedes any **other** Goal objective text in this conversationcovers a stale block sitting in the same message, whichany **earlier** Goal objective in this conversationarguably did not.
What I could not verify end to end
- Scenario H, the specific race the third commit fixes — a queued continuation's permit claimed by text that is not the continuation prompt. Both natural attempts ended with the prompt delivered anyway: the ACP host bundles the continuation with the user text into one request, and the TUI queues a message typed during a running Goal turn rather than admitting it through the slot. Consistent with that, the bundle mutant that reverts this fix (
settleCurrentTurnAnnouncement(true)infinishTurn) is behaviourally identical to HEAD across all 8 of my scenarios — and is killed by exactly the one unit test above. So the fix is covered, just not by my black-box matrix. - Ordering of the notice relative to
verifierFeedback/windDown— code order plus the golden-prompt tests only; my fake model never proposed a terminal state, so no verifier feedback was ever rendered. - The documented in-memory trade-off (announcement lost across a daemon restart) was not exercised; it is deliberate and documented at the declaration.
LGTM from my side — I'd merge this after the test-count line in the body is refreshed.
中文说明
维护者本地验证 —— 在真实栈上构建并运行
我从源码构建了两条腿，并让改动跑通了全部三个 Goal host：真实的 qwen serve daemon（即 ACP Session host）、真实的交互式 TUI、以及真实的 headless nonInteractiveCli；模型侧换成一个假 OpenAI 供应商，它逐字节记录每一条渲染出来的 Goal 续跑提示词。总共记录了 329 条续跑提示词。
结论：这个 PR 做到了它声称的事，而且每一行关键代码都确实是承重的。 描述里所有可从外部观察的断言都在真实栈上复现；对已构建产物做的四个单行变异，各自恰好打破它对应那行代码所保护的行为；另外三个源码级变异逐条复现了 commit message 里自己写的探针计数。没有发现正确性问题。文末有两条小提示和一处诚实的覆盖缺口。
两条腿
| 腿 | commit | 说明 |
|---|---|---|
| HEAD | 2263d48a |
本 PR |
| BASE | 5a883243 |
本 PR 与 main 的 merge-base |
BASE→HEAD 的净差异恰好就是 PR 里那 12 个文件（+930/−8）。对今天的 main（2bd0ff92）做 git merge-tree 无冲突，且自 PR base 以来 main 新增的 18 个 commit 没有任何一个碰过本 PR 改动的文件——所以「base 落后」并没有藏着语义冲突。
环境：macOS 26.6.2（darwin 25.6.0）、Node v24.18.1，每条腿独立 QWEN_HOME，假供应商跑在 loopback，qwen serve --no-web --workspace <fixture> --token …。
1. 这一行本身（线上真实字节）
同样的 8 个场景，每条腿 39 条续跑提示词：
- BASE 在 39/39 上都发
…supersedes any earlier Goal objective in this conversation, including one you already started working on.——包括其中 19 条来自「objective 自始至终没变过」的四个场景。这正是 PR 描述里说的「第 2 轮和第 40 轮一模一样的警告」。 - HEAD 在 39/39 上发削减后的常驻护栏，另外只在恰好 3 条上追加变更通知——正是 objective 真的刚变过的那三轮。
另跑了一次 31 轮、objective 从未变动的 headless：31 条提示词、一种常驻文案、0 条通知。长跑不漂移。
2. 场景矩阵 —— 真实 qwen serve daemon，ACP Session host
每一行都是独立的 daemon 会话，通过 HTTP 驱动（控制动作走 POST /session/:id/goal，/goal <objective> 走 POST /session/:id/prompt）。
| # | 操作者做了什么 | objective 真的变了吗 | HEAD 通知数 | 读法 |
|---|---|---|---|---|
| A | 什么都不做，跑 4 轮 | 否 | 0 / 4 | 稳定的 Goal 保持安静 |
| B | /goal edit → 换成不同文本 |
是 | 1 / 5 | edit 后第一条续跑（rev 2）发一次，之后安静 |
| C | /goal edit → 同一文本（前后加空格） |
否（revision 1→2） | 0 / 5 | 按内容判定：空编辑抬高了 revision，但没给模型任何新东西 |
| D | /goal <新目标> → replace，新 Goal |
是（revision 回到 1） | 1 / 5 | 只看 revision 会漏掉这一条 |
| E | pause → edit → resume | 是 | 1 / 5 | resume 后发一次 |
| F | pause → resume，不编辑 | 否 | 0 / 5 | 单纯 pause/resume 不宣告任何变更 |
| G | clear → 新建一个 Goal | 新工作项 | 0 / 5 | 全新 Goal 不会声称自己取代了已完成的工作 |
| H | edit 之后紧接一条用户直发消息 | 是 | 已投递（见下） | ACP host 会把续跑提示词与用户文本渲染进同一条请求，通知随该请求发出 |
BASE 每一行都是 0，原因很简单：那条线在 BASE 上根本不存在。
3. 用户实际看到的样子
真实 TUI、真实按键（先 /goal …，再 /goal edit …），通过 node-pty + xterm.js 抓图。假模型的回复会回显「它刚收到的这条提示词里究竟带了什么」，所以截图本身就是证据而不是配图。
- BASE：常驻那一行在每一轮都声称发生了变更——包括真的发生了变更的第 6 轮，措辞与前五轮一字不差。
- HEAD：第 1–5 轮不含变更措辞；第 6 轮（
/goal edit之后的第一条）带上通知；第 7–8 轮重新安静。
headless host 用同样方式驱动（-p "/goal …"）：HEAD 发新措辞的常驻护栏，BASE 发旧的，两边都没有误发通知。
4. 对已构建产物做的变异探针
直接改 dist/chunks/*.js 的单行锚点，再用真实 daemon 重跑同样五个场景。单元格是「通知数 / 续跑提示词数」。
| 变异 | 单行改动 | 打破了什么 |
|---|---|---|
| M1 | objectiveUpdated = announced !== undefined（去掉内容比较） |
PR 要消除的噪声原样回来：稳定 Goal 3/4，其余场景 4/5 |
| M2 | 删掉 Session.ts 里的 markTurnDelivered() |
通知完全不再发出（B、D 均 0/5）——「被 host 接受」确实不等于「已投递」 |
| M3 | clear 不再清空宣告状态 |
新建 Goal 的第一条续跑出现误报通知（G：1/5） |
| M4 | 改回按 (goalId, revision) 判定 |
空编辑误报（C：1/5），同时 replace 的通知丢失（D：0/5）——一个探针同时证明了 PR 选择内容判定的两半理由 |
每个变异都恰好被它对应那行代码负责的场景抓住，且不会误伤其他场景。
5. 复现 commit message 里自己写的变异声明
源码级变异 + packages/core 的 goal-runtime.test.ts（144 个用例）：
| 变异 | 结果 | 被哪个用例杀死 |
|---|---|---|
finishTurn 无条件提交 |
1 失败 / 143 通过 | keeps the notice owed when a turn finishes under the permit without the prompt |
| 投递标记忽略 turn key | 1 失败 / 143 通过 | ignores a delivery mark carrying a stale turn key |
每个终态都重置（而非只有 complete） |
1 失败 / 143 通过 | fires after an edit made while the Goal was blocked |
| 去掉内容比较 | 11 失败 / 133 通过 | 整个通知用例组 |
| 只看 revision | 3 失败 / 141 通过 | replace · 被拒轮次泄漏 · 空编辑 |
三条「1 失败 / 143 绿」与第二、第三个 commit message 写的完全一致。
6. 测试、Lint、合并
packages/core→npx vitest run src/goals/——16 个文件、468 个用例全过。packages/cli→npx vitest run src/ui/hooks/useMessageQueue.test.ts src/ui/hooks/useGeminiStream.test.tsx src/nonInteractiveCli.test.ts src/acp-integration——42 个文件、2196 通过、1 跳过。- 12 个改动文件的
eslint与prettier --check全干净；两条腿的npm run build+npm run bundle均成功。 - 旧句子
including one you already started working on在整棵树里已不存在；两条新文案只活在渲染器与其测试里——没有第二份副本会漂移，文档也没有引用提示词原文。 - 已构建产物证明标志确实穿过了全部三个 host：ACP、headless、TUI 三个 chunk 里都有
objectiveUpdated与markTurnDelivered；BASE 产物里两者都不存在。
提示
-
Reviewer Test Plan 的数字过期了。 里面写
packages/core … 404 tests, 16 files，而 head 上的实际数字是 468（16 个文件不变），因为后两个 commit 又加了用例。合并前顺手刷新一下即可，计划里其余内容没有问题。 -
既有行为，不是本 PR 引入的，而且反而对本 PR 有利。 当
/goal edit恰好落在一条在途续跑上时，daemon/ACP host 会抢占，替换后的那条请求会在同一条 user 消息里携带两个 Goal 数据块——过期的那个和新的那个——后面才是常驻护栏（HEAD 上还有通知）。我的矩阵在两条腿上各产生 7 条这样的请求，数量完全一致，说明是既有行为。不过这恰好支持了这次改写：supersedes any **other** Goal objective text in this conversation能覆盖「过期块就在同一条消息里」的情况，而旧措辞any **earlier** Goal objective in this conversation严格讲覆盖不到。
我没能端到端验证的部分
- 场景 H，也就是第三个 commit 修的那个具体竞态——排队中的续跑 permit 被「不是续跑提示词」的其他文本占用。两次自然尝试最后都还是把提示词投递出去了：ACP host 会把续跑提示词和用户文本打包进同一条请求；而 TUI 在 Goal 轮次运行中会把用户输入排队，而不是从那个槽位放行。与此一致的是，把这个修复回退掉的产物变异（
finishTurn里改成settleCurrentTurnAnnouncement(true)）在我全部 8 个场景上与 HEAD 行为完全相同——但被上面那条单元用例精确杀死。所以这个修复是有覆盖的，只是我的黑盒矩阵覆盖不到。 - 通知相对
verifierFeedback/windDown的顺序——只有代码顺序与黄金提示词测试；我的假模型从不提出终态，因此从未渲染出 verifier feedback。 - 文档中写明的内存态取舍（宣告状态随 daemon 重启丢失）未做实测；这是刻意取舍且已在声明处注明。
我这边 LGTM —— 建议把正文里的用例数刷新后合入。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No changes needed this roundThe only feedback newer than the last evaluation is @wenshao's maintainer-verification comment — an LGTM with two minor notes. Neither note requires (or permits) a code change. Point-by-point disposition1. The LGTM verdict and verification matrix — no action. The comment confirms every load-bearing claim of the PR reproduced on the real stack and reports no correctness problem. Nothing to address. 2. Note 1 — stale test count in the Reviewer Test Plan (PR body). Verified as real, but the fix is a PR-body edit on GitHub, which this loop cannot perform: an address-review round can only land code commits, and this agent holds no GitHub write credentials (the workflow owns all GitHub writes and has no body-update channel for review rounds). So I am surfacing it here with the verified numbers instead of editing anything:
Leaving this to the maintainer, matching their stated intent: "I'd merge this after the test-count line in the body is refreshed." 3. Note 2 — two Goal data blocks in one message when State
中文说明本轮无需改动上次评估之后新增的反馈只有 @wenshao 的维护者验证评论——一条 LGTM，附带两条小提示。两条提示都不需要（也不允许）代码改动。 逐点处置1. LGTM 结论与验证矩阵——无需处理。该评论确认本 PR 的每个关键断言都在真实栈上复现，且未发现正确性问题。没有需要处理的内容。 2. 提示 1——Reviewer Test Plan（PR 正文）中的用例数过期。 已核实属实，但修复方式是编辑 GitHub 上的 PR 正文，本循环无法执行：address-review 轮次只能推送代码 commit，而本 agent 没有任何 GitHub 写权限（所有 GitHub 写操作由工作流负责，且评审轮次没有更新 PR 正文的通道）。因此这里不做任何编辑，只附上已核实的数字予以呈报：
交给维护者处理，与其表述的意图一致："I'd merge this after the test-count line in the body is refreshed."（正文里的用例数刷新后我就合入。） 3. 提示 2—— 状态
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
6 participants
What this PR does
Splits the last line of the Goal continuation prompt into the half that is true every turn and the half that is true once.
The line today reads: "The objective in that data block is the current one and supersedes any earlier Goal objective in this conversation, including one you already started working on." The standing half is a guard — objective-shaped text reaches the model from places the runtime does not control (earlier turns, tool output, file contents), so the data block has to be asserted as the only objective on every turn. That half stays, minus the change-implication.
The other half warns that the objective changed. It went out identically on turn 2 and turn 40 of Goals whose objective never moved, so by the time an edit finally landed, the warning had been noise for the whole run. It becomes a separate line, sent only on the first continuation after the objective actually changed, and it now says something actionable: stop work that only served the previous objective.
The runtime decides by remembering the
(goalId, revision)pair whose objective it last handed to a host. A different pair means an edit bumped the revision or a replace minted a new Goal over the old one; no previous pair means this Goal's first continuation, which supersedes nothing. Keying on the revision alone would miss replace, whose new Goal starts at revision 1 exactly like a fresh create. A start the host refused puts the announcement back, so a retried continuation still carries the notice. The flag rides the host boundary the same wayverifierFeedbackalready does, through all three hosts.Why it's needed
A warning that fires on every turn regardless of whether anything happened is not a warning.
/goal editis the one moment in a Goal's life when the model is holding an objective that is now wrong — it may have a plan, partial output, and tool state built for it — and that is exactly the moment the current prompt says nothing different from the previous forty. This is the objective-drift line of work (#9581 collected the three hosts' prompts into one renderer, #9834 converged their contract); this PR is the part that makes the drift signal legible when drift actually occurs.Only continuations announce an objective — a user turn carries the user's own text — so the state is per-continuation and held in memory rather than on the record. Losing it across a daemon restart costs one prompt line, while the objective itself travels in the data block on every turn and
get_goalstays authoritative.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/goals/— 404 tests, 16 files. New: two prompt-rendering cases (the notice appends only when flagged, and sits above verifier feedback so it is read before feedback about a turn taken under the old objective), and five runtime cases covering the one-shot semantics end to end: never for an unchanged Goal (including its first continuation), once after edit then quiet, once after replace, never across pause/resume, and redelivery when the host refuses the start.cd packages/cli && npx vitest run src/ui/hooks/useMessageQueue.test.ts src/ui/hooks/useGeminiStream.test.tsx src/nonInteractiveCli.test.ts src/acp-integration— 40 + 233 + 130 + 1739 pass.useGeminiStream.test.tsxheld the only hardcoded copy of the old line; it now asserts the reworded standing guard.npx tsc --noEmitclean for goal code in bothpackages/coreandpackages/cli. prettier + eslint clean on the nine changed files.Evidence (Before & After)
Prompt tail on an ordinary continuation, before and after — one line shorter, and no longer claims a change happened:
First continuation after
/goal edit, after this PR (the standing guard, then the notice):Tested on
Environment (optional)
N/A (unit tests only).
Risk & Scope
get_goalreturns or how objectives are stored.objectiveUpdatedis optional on the host boundary; a host that ignores it renders exactly the standing prompt.Linked Issues
中文说明
这个 PR 做了什么
把 Goal 续跑提示词的最后一行拆成「每轮都成立的那一半」和「只成立一次的那一半」。
现在这行是:「数据块中的 objective 是当前的,并取代本对话中任何更早的 Goal objective,包括你已经开始处理的那个。」 常驻的那一半是护栏——形似 objective 的文本会从运行时无法控制的地方到达模型(更早的轮次、工具输出、文件内容),所以必须每轮都声明数据块是唯一的 objective。这一半保留,去掉其中暗示「发生了变更」的措辞。
另一半警告 objective 变了。它在 objective 从未变动的 Goal 的第 2 轮和第 40 轮上一模一样地发出,所以等到真的发生一次编辑时,这条警告已经当了整轮运行的噪声。现在它成为独立的一行,只在 objective 真正变更后的第一次续跑发出,并且给出可执行的指令:停止只服务于旧 objective 的工作。
运行时的判断依据是记住「上一次交给 host 的那个 objective 所属的
(goalId, revision)组合」。组合不同意味着 edit 抬高了 revision,或者 replace 在旧 Goal 之上铸造了新 Goal;没有上一个组合则说明这是该 Goal 的首次续跑,不取代任何东西。只看 revision 会漏掉 replace——它的新 Goal 和全新创建一样从 revision 1 开始。被 host 拒绝的启动会把宣告状态放回去,因此重试的续跑仍然带着这条通知。该标志沿用verifierFeedback已有的方式穿过 host 边界,三个 host 一致。为什么需要
一条无论是否发生变化都每轮发出的警告,不是警告。
/goal edit是 Goal 生命中唯一一个「模型手里的 objective 已经作废」的时刻——它可能已经有了针对旧 objective 的计划、部分产出和工具状态——而恰恰在这个时刻,当前的提示词与之前四十轮说的一模一样。这是 objective 漂移这条线的收尾(#9581 把三个 host 的提示词收进单一渲染器,#9834 收敛了它们的契约),本 PR 让漂移信号在漂移真正发生时可被识别。只有续跑会宣告 objective——用户轮次带的是用户自己的文本——所以该状态按续跑维护并保存在内存中,而不是记录字段上。跨 daemon 重启丢失它的代价是少发一行提示,而 objective 本身每轮都在数据块里,
get_goal依然是权威。评审验证计划
如何验证
cd packages/core && npx vitest run src/goals/——404 个测试,16 个文件。新增:两个提示词渲染用例(仅在置位时追加该行;该行位于 verifier feedback 之上,因为反馈针对的是在旧 objective 下进行的那一轮,必须先读到变更通知),以及五个 runtime 用例端到端覆盖一次性语义:objective 未变的 Goal 永不发出(含首次续跑)、edit 后发一次随即安静、replace 后发一次、pause/resume 全程不发、host 拒绝启动时重新投递。cd packages/cli && npx vitest run src/ui/hooks/useMessageQueue.test.ts src/ui/hooks/useGeminiStream.test.tsx src/nonInteractiveCli.test.ts src/acp-integration——40 + 233 + 130 + 1739 全过。useGeminiStream.test.tsx是旧文案唯一的硬编码副本,现已改为断言重新措辞后的常驻护栏。packages/core与packages/cli的npx tsc --noEmit在 goal 相关代码上干净;九个改动文件 prettier + eslint 干净。证据(前后对比)
普通续跑的提示词末尾,前后对比——少一行,且不再声称发生了变更:
/goal edit之后的第一次续跑(常驻护栏,随后是通知):已测试平台
Linux ✅;macOS / Windows⚠️ (CI 覆盖)。
环境(可选)
N/A(仅单元测试)。
风险与范围
get_goal的返回内容,也不改变 objective 的存储方式。objectiveUpdated在 host 边界上是可选的;忽略它的 host 渲染出的就是常驻提示词。关联 Issue