fix(goal): stamp the wind-down hand-off only when its turn was delivered - #10260
Conversation
QwenLM#10132 marked the record's `windDownTurnId` whenever the turn holding the wind-down permit finished -- reading "the permit was used" as "the user got the hand-off". QwenLM#10013 established why that inference is wrong for the objective-updated notice: a system message or a direct user query can claim a queued continuation's permit and send its own text under it, so the turn finishes with the prompt never reaching the model. Hosts therefore mark delivery at the real send site, and only a delivered turn commits what it carried. The hand-off now follows the same rule. `finishTurn` stamps the marker only when the wind-down turn was marked delivered; an undelivered one leaves the record clean, so the next `queueContinuation` grants the hand-off again instead of settling `usage_limited` on a hand-off the user never received (which a resume would not have repaired either, since the marker is cleared only by a re-arm). The in-memory permit marker is released either way; it belongs to the permit, not the outcome. The wind-down tests that finish the hand-off turn now mark it delivered first, so they keep meaning "the model saw the hand-off". Two new cases pin the split: finished-but-undelivered leaves no marker and re-mints the hand-off; finished-and-delivered stamps it and stops. Mutation probe: making the stamp unconditional again fails exactly the undelivered case (145 others 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，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The linked issue #10150 (filed out of the #10013 review round) documents the exact mechanism: a system message ( Direction: aligned. This is correctness bookkeeping inside the already-shipped goal runtime, applying the delivery rule #10013 established. Upstream CHANGELOG shows active investment in Size: core path touched ( Approach: minimal and exactly on point. One production hunk: capture Risk: no elevated risk signals — none of the changed files match the revert-correlated path list. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，不是理论问题。关联 issue #10150（来自 #10013 评审轮次）记录了确切机制：系统消息（ 方向：对齐。这是已上线 goal 运行时内部的正确性记账，应用 #10013 确立的送达规则。上游 CHANGELOG 显示对 规模：触及核心路径（ 方案：最小且切中要害。一个生产 hunk：在宣告结算重置送达标记之前捕获 风险：无升级风险信号——改动文件均未命中回滚关联路径列表。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: the stamp means "the user got the hand-off", so it must be gated on the same delivery flag #10013 introduced — capture What I verified against the code at the reviewed commit:
No correctness, security, or convention issues found. Comment style matches the file's existing why-comments. No AGENTS.md violations. TestingEvidence carried: this PR's own CI, read through the API (unattended run — I don't execute PR code). The The two new tests pin the change by structure, not by author's word: the undelivered case asserts the record carries no Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查读 diff 之前我的独立思路：盖章的含义是「用户已收到交接」，因此应当用 #10013 引入的送达标记来设闸——在宣告结算重置送达标记之前先捕获 在受审提交上核实过的内容：
未发现正确性、安全性或约定问题。注释风格与文件中现有的 why-注释一致。无 AGENTS.md 违规。 测试本次携带的证据：该 PR 自身的 CI，通过 API 读取（无人值守运行——不执行 PR 代码）。受审提交上的 两个新测试从结构上钉住了改动，而非仅凭作者口径：未送达用例断言记录上没有 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; approval is deferred only until the Linux unit lane lands green on the reviewed commit. What I'm actually thinking after the full pass: this is the delivery rule from #10013 applied a second time, and it's settling into a real principle in this runtime — the record may only say what the host's delivery mark can prove. The PR follows that principle exactly and goes no further: 13 production lines, no scaffolding, no drive-bys. My independent proposal (gate the stamp on the delivery flag captured before the settle resets it; release the permit marker either way) is what the diff does, and I couldn't construct a materially simpler path — clearing the marker earlier (e.g. in The tests are the right shape. The undelivered case cannot pass on pre-change code: the old unconditional stamp settles the Goal The one residual risk is the one the PR itself names: a future host that forgets to mark delivery would re-grant the hand-off every window instead of stopping. All three current hosts mark at their real send sites (verified in tree), and re-offering is the better failure mode — it's loud, where the bug was silent. CI on the reviewed commit is still running (the Linux unit lane), so no approval is posted in this run — approval is deferred until CI lands green on the reviewed commit, and the finalize job will act on it. 中文说明置信度：5/5 —— 各阶段均干净；仅因受审提交上的 Linux 单元车道尚未落绿而暂缓批准。 完整过一遍之后我真实的想法：这是 #10013 送达规则的第二次应用，正在沉淀为该运行时的真实原则——记录只能陈述 host 送达标记能够证明的事实。PR 恰好遵循这一原则且毫无超出：13 行生产代码，无脚手架，无顺手改动。我的独立方案（以结算重置前捕获的送达标记为盖章设闸；无论如何都释放 permit 标记）与 diff 一致，而且我想不出更简的路径——更早清除标记（如在 测试形态正确。未送达用例在旧代码上不可能通过：旧的无条件盖章会在两轮后把 Goal 落为 唯一残留风险正是 PR 自述的那条：未来某个忘记标记送达的 host 会导致交接每个窗口重复授予而非一次即停。现有三个 host 都在真实发送点标记（已在树中核实），且重复提供是更好的失败模式——它是显性的，而这个 bug 是静默的。 受审提交上的 CI 仍在运行（Linux 单元车道），本次运行不发布批准——批准推迟到 CI 在该提交上落绿，finalize 任务将据此执行。 — 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.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": executing npx vitest run src/goals/goal-runtime.test.ts in packages/core — the worktree has no installed/built node_modules and install-plus-build exceeds the…; "agent 3b": none — no check was cut short..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：executing npx vitest run src/goals/goal-runtime.test.ts in packages/core — the worktree has no installed/built node_modules and install-plus-build exceeds the…；"agent 3b"：none — no check was cut short.。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // else's text leaves no marker, and the next continuation grants | ||
| // the hand-off again instead of stopping cold. | ||
| const heldWindDown = windDownTurnId === permit.turnId; | ||
| const finishedWindDown = heldWindDown && delivered; |
There was a problem hiding this comment.
[Suggestion] This change tightens the wind-down stamp rule — finishedWindDown = heldWindDown && delivered — but leaves three earlier comments stating the old "finished ⇒ stamps" rule: the in-memory field doc at goal-runtime.ts:300-304 ("only the turn that actually finishes stamps the record"), the budget-gate comment at goal-runtime.ts:608-610 ("The record marks the hand-off that finished" — its case list omits the new granted-finished-but-undelivered state), and the GoalTurnFinishedTransition.windDownTurnId contract at goal-reducer.ts:49 ("Set when the finishing turn was the spend window's wind-down hand-off"). goal-protocol.ts:175-183 already documents the delivered rule, so the module now contradicts itself. A maintainer extending the settle/release/restore paths — or adding a caller of reduceGoalTurnFinished — reads one of these comments first, concludes that finishing a wind-down turn alone stamps the record, and stamps an undelivered hand-off through a new path, silently reintroducing the #10150 defect: a Goal stopped at usage_limited with the record claiming a hand-off the user never received. Reword all three to the delivered-and-finished rule, for example:
// goal-runtime.ts (field doc)
* In memory only: a wind-down the host dropped undelivered must be minted
* again, and only a wind-down turn that finishes delivered stamps the
* record; one finished under someone else's text leaves the hand-off owed.
// goal-runtime.ts (budget gate)
// A spent window buys one hand-off before it stops. The record marks
// the hand-off that was delivered and finished; until then -- never
// granted, dropped before the model saw it, or finished under another
// text -- grant it.
// goal-reducer.ts (transition contract)
/** Set when the finishing turn was the spend window's wind-down hand-off and was delivered to the model. */中文说明
此变更收紧了收尾盖章规则——finishedWindDown = heldWindDown && delivered——但留下了三处仍在陈述旧规则（「结束即盖章」）的注释：goal-runtime.ts:300-304 的内存字段文档（"only the turn that actually finishes stamps the record"）、goal-runtime.ts:608-610 的预算闸门注释（"The record marks the hand-off that finished"——其情形列表缺少新增的「已授予、已结束但未送达」状态）、以及 goal-reducer.ts:49 的 GoalTurnFinishedTransition.windDownTurnId 契约（"Set when the finishing turn was the spend window's wind-down hand-off"）。goal-protocol.ts:175-183 已记载送达规则，模块内部因此自相矛盾。未来扩展 settle/release/restore 路径（或新增 reduceGoalTurnFinished 调用方）的维护者会先读到这些注释，误以为仅结束收尾轮即可盖章，从而在新路径上为未送达的交接盖章，悄悄重现 #10150 缺陷：Goal 停在 usage_limited，记录却声称用户已收到交接，而用户从未收到。建议将三处注释都改为「已送达且结束」规则，例如：
// goal-runtime.ts（字段文档）
* In memory only: a wind-down the host dropped undelivered must be minted
* again, and only a wind-down turn that finishes delivered stamps the
* record; one finished under someone else's text leaves the hand-off owed.
// goal-runtime.ts（预算闸门）
// A spent window buys one hand-off before it stops. The record marks
// the hand-off that was delivered and finished; until then -- never
// granted, dropped before the model saw it, or finished under another
// text -- grant it.
// goal-reducer.ts（transition 契约）
/** Set when the finishing turn was the spend window's wind-down hand-off and was delivered to the model. */— qwen3.8-max via Qwen Code /review (v0.22.2)
…wenLM#10260) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #10260 (round 2)Feedback point
Decision: Implemented. Verified before editing by reading all three cited sites: the in-memory Changes (comment-only, 2 files, +9/−4):
No mutation probe: this round adds no guard, branch, or behavior — comments only. No deferred findings, no escalations. Verification
中文说明处理评审总结 — PR #10260（第 2 轮）反馈点
决定：已实现。 编辑前逐一核实了三处被引用的位置： 改动（仅注释，2 个文件，+9/−4）：
无需变异探针：本轮未新增任何守卫、分支或行为——仅注释改动。无延迟处理的发现，无升级给维护者的问题。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/goals/goal-runtime.ts:1534 — [probe] the heldWindDown && stamp guard has no witness test — the finishedWindDown = delivered mutation survives the whole suite
中文说明
无阻断问题。LGTM！✅
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
4 participants
What this PR does
Stamps
GoalRecord.windDownTurnId— the marker that says "the user received the budget hand-off" — only when the wind-down turn was actually delivered to the model, using the delivery mark hosts already set (markTurnDelivered) and the rule #10013 established for the objective-updated notice.finishTurnreadscurrentTurnDeliveredbefore the announcement settle resets it; a wind-down permit that finished undelivered leaves the record clean, so the nextqueueContinuationgrants the hand-off again instead of settlingusage_limited. The in-memory permit marker is released either way, since it belongs to the permit rather than to the outcome.Why it's needed
#10132 inferred "the hand-off was delivered" from "the turn holding the wind-down permit finished". #10013 showed why that inference fails: a system message (Cron/Notification/Teammate via
claimSystemGoalTurn) or a direct user query (/btwviaclaimDirectUserAdmission) can claim a queued continuation's permit and send its own text under it, skipping both the continuation render and the delivery mark. If the claimed permit is the wind-down permit, the record was stamped, the next continuation stopped the Goal on a hand-off the user never got, and a resume did not repair it — the marker is cleared only by a re-arm. Filed as #10150.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/goals/— 478 pass (17 files).goal-runtime.test.tsgains two cases:grants the hand-off again when its turn finished without being delivered(finish the wind-down turn with no delivery mark → nowindDownTurnIdon the record or in the journal, Goal stillactive, the next continuation iswindDown: trueagain) andstops after the hand-off once a delivered wind-down turn finishes(mark delivered, finish → marker stamped,usage_limited, no third turn). The three existing wind-down tests that finish the hand-off turn now mark it delivered first, so they keep meaning "the model saw the hand-off".finishedWindDown = heldWindDown) fails exactly the undelivered case, 145 others green.cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts src/ui/hooks/useGeminiStream.test.tsx src/acp-integration— 2151 pass; the host suites that exercise wind-down are unchanged, since hosts already mark delivery at their send sites.tsc --noEmit: no goal-code errors inpackages/core; prettier + eslint clean on the two changed files.Evidence (Before & After)
N/A (runtime bookkeeping; the visible surfaces are unchanged — what changes is whether the hand-off is re-offered when the first attempt never reached the model).
Tested on
Environment (optional)
N/A (unit tests only).
Risk & Scope
markTurnDeliveredwould now re-grant the hand-off each window instead of stopping after one; all three hosts mark delivery at their real send site as of feat(goal): tell the model the objective changed, once, when it changed #10013, and the runtime tests were updated to say so explicitly rather than rely on the old unconditional stamp.Linked Issues
中文说明
这个 PR 做了什么
GoalRecord.windDownTurnId——表示「用户已收到预算收尾交接」的标记——现在只在收尾轮真正送达模型时才盖章,复用 host 已在真实发送点设置的送达标记(markTurnDelivered)以及 #10013 为「objective 已变更」通知确立的规则。finishTurn在宣告结算重置之前读取currentTurnDelivered;未送达就结束的收尾 permit 不在记录上留标记,于是下一次queueContinuation会再次授予交接,而不是落为usage_limited。内存中的 permit 标记无论如何都会释放,它属于 permit 而非结果。为什么需要
#10132 以「持有收尾 permit 的轮次结束了」推断「交接已送达」。#10013 说明了这个推断为何不成立:系统消息(经
claimSystemGoalTurn的 Cron/Notification/Teammate)或直接用户查询(经claimDirectUserAdmission的/btw)可以认领排队中续跑的 permit 并在其下发送自己的文本,跳过续跑渲染与送达标记。若被认领的正是收尾 permit,记录会被盖章,下一次续跑会在一份用户从未收到的交接上停止 Goal,resume 也无法修复——标记只在重新武装时清除。已记录为 #10150。评审验证计划
如何验证
cd packages/core && npx vitest run src/goals/——478 通过(17 个文件)。goal-runtime.test.ts新增两个用例:grants the hand-off again when its turn finished without being delivered(未标记送达就结束收尾轮 → 记录与日志均无windDownTurnId,Goal 仍为active,下一次续跑再次为windDown: true)和stops after the hand-off once a delivered wind-down turn finishes(标记送达后结束 → 盖章、usage_limited、无第三轮)。原有三个结束收尾轮的测试现在先标记送达,使其含义保持为「模型看到了交接」。finishedWindDown = heldWindDown),恰好挂未送达用例,其余 145 绿。cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts src/ui/hooks/useGeminiStream.test.tsx src/acp-integration——2151 通过;涉及收尾的 host 套件不变,因为 host 已在发送点标记送达。tsc --noEmit:packages/core中 goal 代码无错误;两个改动文件 prettier + eslint 干净。证据(前后对比)
N/A(运行时账本;可见界面不变,改变的是首次交接未到达模型时是否重新提供)。
已测试平台
Linux ✅;macOS / Windows⚠️ (CI 覆盖)。
环境(可选)
N/A(仅单元测试)。
风险与范围
markTurnDelivered,收尾轮会在每个窗口重复授予而不是一次即停;自 feat(goal): tell the model the objective changed, once, when it changed #10013 起三个 host 都在真实发送点标记送达,运行时测试也已改为显式表达这一点,而不再依赖旧的无条件盖章。关联 Issue