fix(review): correct the round-cap marker lifecycle and stale cap docs - #8850
Conversation
Follow-up to #8773. The round-cap stop marker shared budget-stop.json with the time-budget stop but did not share its lifecycle rules, and the huge-diff cap left several docs describing the old 5-round-only world. Marker lifecycle: - First refusal wins. Both writers now skip when a same-run marker already exists, so a time-budget stop followed by a retry the cap refuses no longer overwrites the marker and posts two contradictory stop disclosures. - A CONVERGED exit clears any same-run marker. A round-cap refusal followed by a converged round (the convergence check runs before the cap gate) left a stale marker that capped a legitimately-converged verdict; the converged branch now unlinks it. - coverage.ts's reverseByDesign exemption is now cause-aware. It suppressed the not-built gap for any marker, but a round-cap stop's fix (rebuild --round 1) is admitted — so a round-cap marker keeps the gap and its rebuild remediation. Docs and tests: - Drop the stale "retirement.ts re-exports it" and "the retirement scheduler" reader claims in budget.ts; note the huge-diff 3-round cap in DESIGN.md's LLM-call-budget paragraph and table. - Mirror the time-budget stop's bounded-tail protocol into the ROUND CAP message and SKILL.md bullet (verify only via --role verify, bound the wait at the compose floor, no fresh re-verification pass). - Pin the spelled cap number in the retirement note, and pin that a converged past-cap round exits 5 rather than refusing at the cap.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao Stopping at the template gate — the description's substance is solid (each R4 finding gets a named fix, and the mutation-tested note is exactly the kind of evidence reviewers want), but the headings don't match .github/pull_request_template.md, so this is a formatting pass rather than a rework.
- Missing required headings: the body uses
## Marker lifecycle (the real bugs),## Docs & tail protocol, and## Tests; the template asks for## What this PR does,## Why it's needed,## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on),## Risk & Scope, and## Linked Issues. See the template. - Everything is already written — it just needs re-flowing: the intro paragraph plus the two change sections map to
## What this PR does; the motivation (the round-cap marker sharesbudget-stop.jsonwith the time-budget stop but not its lifecycle rules; the R4-1 … R4-11 findings) is the## Why it's neededcontent; the## Testssection becomes### How to verify, ideally with the actual suite commands so a reviewer can re-run them. - The
### Tested onOS table is missing: the review machinery runs jobs on all three CI OSes, and the marker files land in real workspaces, so it's worth stating where the 616-suite run happened. ## Risk & Scopeis missing: worth stating the tradeoffs explicitly — e.g. what a CONVERGED exit clearing a same-run marker means if a later legitimate budget stop happens in the same run, and why first-refusal-wins is safe when the first stop was the wrong one to keep.## Linked Issuesis missing: #8773 is referenced in prose; a dedicated section keeps the cross-linking clean.
The bilingual <details> section is already there ✓. Once the body follows the template, re-run the gate with @qwen-code /triage.
中文说明
@wenshao 这个 PR 停在模板关卡 —— 描述内容本身很扎实（每条 R4 发现都有点名的修复，变异测试的说明正是评审者想看到的证据），但标题结构与 .github/pull_request_template.md 不符，所以这一轮只是补格式，不是返工。
- 缺少必需标题：正文目前用
## Marker lifecycle (the real bugs)、## Docs & tail protocol、## Tests；模板要求## What this PR does、## Why it's needed、## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)、### Tested on）、## Risk & Scope、## Linked Issues。见模板。 - 内容都已具备，只需归位：开头段落加两个变更小节对应
## What this PR does；动机部分（round-cap 标记与时间预算停止共用budget-stop.json却没有共用其生命周期规则；R4-1 … R4-11 的发现）就是## Why it's needed的内容；## Tests部分可作为### How to verify，最好附上实际的测试命令，方便评审者复跑。 - 缺少
### Tested on操作系统表格：评审机制在 CI 的三个操作系统上都会跑任务，且标记文件会落在真实工作区里，值得说明 616 个测试是在哪个环境跑的。 - 缺少
## Risk & Scope：值得明确写出权衡 —— 例如 CONVERGED 退出清除同 run 标记后，若同一 run 后续又出现合理的时间预算停止会怎样；以及当首个停止并非应该保留的那个时，first-refusal-wins 为何仍然安全。 - 缺少
## Linked Issues：#8773 在正文中以文字形式引用；单独一节可以让交叉链接更清晰。
双语 <details> 部分已有 ✓。正文按模板调整后，用 @qwen-code /triage 重新触发关卡即可。
— Qwen Code · qwen3.8-max
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
@qwen-code /triage Re-flowed the description to — Claude Code · Opus 4.8 |
|
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: 38 passed · 0 failed · 38 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：38 通过 · 0 失败 · 38 总计 Verification reportPR 8850 verification — fix(review): correct the round-cap marker lifecycle and stale cap docsVerdict: 中文摘要
Central claim and A/BCentral claim: the round-cap stop marker now shares the time-budget marker's lifecycle discipline — first refusal wins, CONVERGED clears a stale same-run marker, and the coverage exemption is cause-aware — so the three R4 correctness bugs (contradictory double disclosure, stale marker capping a converged APPROVE, round-cap suppressing the not-built gap) are gone. Harness: the PR's own new tests, copied unmodified into a scratch worktree at
Witnesses: Mutation matrix (on head; every restore verified
|
| # | Mutant | Suite | Result |
|---|---|---|---|
| M1 | delete first-refusal guard in writeRoundCapStop |
deadline.test.ts | KILLED — only A1's test, on expected 'round-cap' to be undefined |
| M2 | delete first-refusal guard in writeBudgetStop |
deadline.test.ts | KILLED — only A2's test, on expected undefined to be 'round-cap' |
| M3 | delete clearBudgetStop(planPath) in refuseConverged |
agent-prompt -t "CONVERGED exit clears" |
KILLED — expected { cause: 'round-cap', … } to be null |
| M4 | revert coverage exemption to cause-blind | compose-review -t "does NOT suppress" |
KILLED — expected '' to contain 'reverse audit:' |
| M5 | positive control: reword BUDGET_STOP_PHRASE |
deadline.test.ts | KILLED — pre-existing round-trips, entry text and all goes red (harness can make old tests fail) |
| M6 | hoist the cap gate above the convergence check | agent-prompt -t "converged past-cap|CONVERGED exit clears" |
KILLED — both, on expected 4 to be 5 |
| M7 | hardcode 5-round cap leaves in the note |
agent-prompt -t "retires with a final certificate" |
KILLED — expected … to contain '3-round cap leaves' |
7/7 killed, no survivors; M1/M2 each kill exactly one of the paired tests, so attribution is per-guard, not per-file. Witness: 02-mutation-matrix-all-killed.png.
Reviewer Test Plan walkthrough
| Step | Result |
|---|---|
6 cli suites from packages/cli |
ran verbatim: Test Files 6 passed, Tests 616 passed — matches the claimed count exactly (logs/head-cli-suites.log) |
core SKILL.test.ts |
5 passed — matches claim |
| "each fix confirmed against a mutation of its own fix" | reproduced independently: M1–M4 kill exactly the claimed tests on the intended assertions |
eslint --max-warnings 0 + typecheck clean |
reproduced scoped: lint on the 7 changed files exits 0 (liveness-proven — a planted unused-any file was caught with 2 errors before deletion); typecheck -w packages/cli -w packages/core exits 0 |
Corrections
None — this is a first round; no prior review described the code inaccurately.
Findings
No blocking findings. Three nit-level observations (completeness reporting, not merge conditions):
- The mirrored bounded-tail protocol text is unpinned. The ROUND CAP message now carries the verify-only/compose-floor protocol (agent-prompt.ts:1889-1896) and SKILL.md mirrors it; tests pin only
ROUND CAP,round cap is 3, and the marker shape. A reword that drops the protocol would ship green. Same exposure as the pre-existing time-budget message, so parity rather than regression. - The
--chunkconverged site is wired but not test-driven. Both sites sharerefuseConverged(planPath)(M3 kills via the--all-chunkssite); the second site'sargs.planthreading is compile-checked but no test drives a converged--chunkbuild after a cap refusal. - First-refusal guard inherits the run-epoch fence's boundary. The guard reads
readBudgetStop, which discards markers older than the plan mtime − 2 s. If the plan file were rewritten mid-loop, the guard would admit a second write and the double-disclosure shape could recur. The complete plan-writer set is the Step 1 captures plusrepo-context(scripted check B2), and SKILL.md placesrepo-contextin the setup batch before--roster— so within the skill's own flow the fence settles before any round can be refused; the edge requires an orchestrator re-capturing mid-loop, which the fence already defines as a new run.
Not covered
- No live orchestrator end-to-end run; the harness drives the real
runAgentPrompt/composeReview/verificationGapscode paths through vitest (the CLI's own test seam), which reproduces the lifecycle cause, not a model-driven multi-round session. - macOS/Windows not exercised (same as the PR; marker I/O is plain
budget-stop.jsonreads/writes). - Repo-wide
npm run lintand full-workspace typecheck not run; gates were scoped to the changed files and the two touched workspaces (both liveness-verified where applicable). - The wording of the ROUND CAP protocol and the SKILL.md bullet are not asserted by any test (finding 1) — verified by reading, not by harness.
Methodology
CI verify container (node:22-bookworm, merge-ref checkout at depth 2, npm ci + build pre-run). Base arm: git worktree add tmp/base-tree HEAD^1 (5855553), per-package node_modules symlinked (lockfile untouched by the PR), head test files copied in unmodified; vitest aliases resolve @qwen-code/* to each worktree's own source, and the base arm's failures are the provenance proof. Mutants applied as exact-string edits, each followed by git checkout -- and a clean-git status assertion. Boundary/doc claims scripted in doc-boundary-checks.mjs (12 PASS, base side read via git show HEAD^1:). Raw logs in logs/ (ab-base-arm*.log, head-cli-suites.log, head-skill-test.log, mutant-M1..M7.log, lint/typecheck logs); harnesses mutation-run.sh, evidence-summary.mjs, doc-boundary-checks.mjs are rerunnable from this directory. Captures rendered by scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Template gate resolved — the PR body now follows .github/pull_request_template.md end to end (all required headings, the OS table, Risk & Scope, Linked Issues, and the bilingual section). Re-running the full triage per the author's @qwen-code /triage request.
模板关卡已解决——PR 正文现已完整遵循 .github/pull_request_template.md（所有必需标题、操作系统表格、风险与范围、关联 Issue、双语部分均在）。应作者的 @qwen-code /triage 请求重新执行完整关卡。
|
Re-run after the autofix loop's six rounds — two substantive commits landed since my last pass (
Moving on to code review. 🔍 中文说明autofix 循环六轮之后的重跑——距我上次通过又落了两个实质提交（
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers on the new head. My independent proposal for this problem remains the three moves the PR started with — a first-write-wins guard in both writers, a clear at the converged exit, a cause-aware read in
No convention violations — ESM named imports, no Two suggestion-level findings from the Testing evidence — the PR's own CI on the reviewed commit
Every check-run on the reviewed commit is completed — zero failures, nothing pending. The macOS/Windows unit jobs and integration tests are gated to Sandboxed verification would settle the one claim static review cannot: Real-scenario testing: N/A — nothing user-visible (internal 中文说明代码审查新 head 上无阻塞问题。我对这个问题的独立方案仍然是 PR 最初的三个动作——两个写入者各加一条首次写入胜出守卫、收敛退出处清除标记、
无规约违规——ESM 具名导入、无
测试证据——被审提交上的 PR 自身 CI被审提交上的全部 check-run 均已完成——零失败、无进行中（表格见英文版）。macOS/Windows 单测与集成测试按设计仅对 沙箱验证可以了结静态审查无法了结的一项主张： 真实场景测试：N/A——无用户可见变化（内部 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean re-run: the lifecycle fixes are minimal and correct, the autofix rounds strengthened exactly the pins the review rounds probed, CI is green and complete on the reviewed commit, and the two open items are suggestion-level follow-ups, not blockers. Stepping back: my independent proposal and the PR are still the same three moves, so no simpler path was missed — and the growth since my last pass is the review loop working as designed, not scope creep. The relay-recall sentence closes a channel the converged clear itself opens; the bounded-tail mirroring removes a real footgun on exactly the huge-diff runs the cap targets; the new tests replay the failure sequences (convergence outranking the cap, cause-blind clear, cross-run fence, both refusal orders, the failing-unlink path) instead of asserting the happy path. Every line in the diff maps to a named finding from #8773's post-merge review or the follow-up rounds. The two deferred suggestions stay on the record for the maintainer: R4-1's residual double-disclosure on a doubly-off-script orchestrator (verdict cap unaffected; the Risk section owns the tradeoff — worth deciding whether the marker-aware refusal or the either-phrase splice is wanted as a follow-up), and R3-2's one-line SKILL.md pin. Neither changes what this PR is: three one-line lifecycle corrections and their proofs, reviewed to a green suite by the repo's own machinery. The prior approval was dismissed by the autofix pushes — re-approving now, pinned to the reviewed commit; 中文说明置信度：5/5 —— 重跑全部干净：生命周期修复最小且正确，autofix 各轮恰好加固了评审轮次探测出的钉子，被审提交上的 CI 全绿且已完成，剩余两条均为建议级跟进项，不构成阻塞。 退一步看：我的独立方案与 PR 依然是同样三个动作，没有遗漏更简的路径——上次通过之后的增长是评审循环按设计运转的结果，不是范围蔓延。召回句关闭的正是收敛清除自身打开的通道；有界收尾镜像移除的是上限针对的巨型 diff 运行中真实存在的陷阱；新测试回放的是失败序列（收敛优先于上限、cause 无关的清除、跨 run 围栏、两种拒绝顺序、失败的 unlink 路径），而不是断言顺利路径。diff 中每一行都能对应到 #8773 合入后评审或后续轮次点名的发现。 两条被延后的建议留档给维护者：R4-1 在编排器两次越轨时的残留双披露（裁决封顶不受影响；Risk 部分已认领该权衡——值得决定后续是采用感知标记的拒绝，还是双短语剔除），以及 R3-2 的一行 SKILL.md 钉子。二者都不改变这个 PR 的本质：三处一行级生命周期修正及其证明，由仓库自己的机制评审至绿色套件。先前的批准已被 autofix 推送撤销——现在重新批准，钉在被审提交上； — 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. ✅
|
@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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8850This round's feedback contained no reviews, no inline comments, and no failed 1. The bounded-tail protocol text is unpinned — implementedThe ROUND CAP refusal message now carries the verify-only / compose-floor
2. The
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more.
中文说明
已审查——无阻断问题。 建议见行内评论。 Test Plan（非阻断）：src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| function refuseConverged(planPath: string): void { | ||
| clearBudgetStop(planPath); |
There was a problem hiding this comment.
[Suggestion] The cause-blind clear on a CONVERGED exit is only tested against round-cap markers — both new clear tests produce their marker via a cap-3 round-4 refusal, so a future cause-conditional clear (if (readBudgetStop(p)?.cause === 'round-cap') clearBudgetStop(p)) would pass the whole suite. — Failure scenario: under cap 5, an even round 4 refused by the time gate writes a time-budget marker, then odd round 5 converges; probe-verified, the mutation survives all 222 agent-prompt.test.ts tests, and the stale time-budget marker then caps a legitimately converged verdict at COMMENT with a false "stopped before round 4 by the review time budget" disclosure.
Fix: add a sibling test beside the two existing clear tests — same retirement history under cap 5 with a near deadline so even round 4 is refused at the time gate (assert the time-budget marker is written), then run odd round 5 and assert exit 5, CONVERGED on stderr, and readBudgetStop(plan) null.
中文说明
[建议] CONVERGED 退出时的无差别清标记目前只有 round-cap 标记的测试覆盖——两个新的清标记测试都通过 cap-3 第 4 轮拒绝来产生标记,因此未来若改成按 cause 条件清标记(if (readBudgetStop(p)?.cause === 'round-cap') clearBudgetStop(p)),整个测试套件仍会全部通过。失败场景:在 cap 5 下,偶数第 4 轮被时间门拒绝、写入 time-budget 标记,随后奇数第 5 轮收敛;经探针验证,该变异下 agent-prompt.test.ts 全部 222 个测试仍然通过,残留的 time-budget 标记会把合法收敛的裁决封顶为 COMMENT,并附带一条不实的 "stopped before round 4 by the review time budget" 披露。修复:在现有两个清标记测试旁补一个同族测试——同样的退休历史、cap 5、临近 deadline,使偶数第 4 轮被时间门拒绝(断言写入了 time-budget 标记),再运行奇数第 5 轮,断言 exit 5、stderr 出现 CONVERGED、readBudgetStop(plan) 为 null。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // First refusal wins: a same-run marker already on disk (its run-epoch | ||
| // fence in `readBudgetStop` excludes previous runs') is left untouched, | ||
| // so a time-budget stop followed by a retry that the cap then refuses | ||
| // does not post two contradictory stop disclosures. | ||
| if (readBudgetStop(planPath) !== null) return; |
There was a problem hiding this comment.
[Suggestion] The first-refusal-wins guard here has no cross-run test: the pre-existing fence test ('a marker from before the plan capture is a previous run') pins this case only for writeBudgetStop, and the two new same-run tests cannot tell existsSync from readBudgetStop() !== null. — Failure scenario: run 1 hits the round cap and is killed before cleanup, leaving a round-cap marker; run 2 re-captures the plan and runs past the cap again. Probe-verified: a regression of this guard to a raw existsSync check passes all 42 deadline.test.ts tests, but makes run 2's writeRoundCapStop a no-op — compose-review then neither caps the verdict nor prints the stop disclosure, and a cap-stopped audit posts uncapped.
Fix: mirror the existing fence test for writeRoundCapStop in deadline.test.ts — write a stale (fenced-out) round-cap marker, assert readBudgetStop is null, then writeRoundCapStop fresh and assert cause === 'round-cap'.
中文说明
[建议] 这里的首次拒绝胜出守卫缺少跨 run 测试:已有的围栏测试('a marker from before the plan capture is a previous run')只为 writeBudgetStop 钉住了该场景,而两个新的同 run 测试无法区分 existsSync 与 readBudgetStop() !== null。失败场景:run 1 触及轮数上限并在清理前被杀死,留下一个 round-cap 标记;run 2 重新捕获 plan 后再次超限。经探针验证:把该守卫退化为裸 existsSync 检查,deadline.test.ts 全部 42 个测试仍会通过,但 run 2 的 writeRoundCapStop 会变成空操作——compose-review 既不会封顶裁决、也不会输出停止披露,一个实际停在上限的审计会以未封顶的状态发布。修复:在 deadline.test.ts 中为 writeRoundCapStop 补一个与现有围栏测试同族的用例——先写入一个过期的(被围栏排除的)round-cap 标记,断言 readBudgetStop 为 null,再执行一次新的 writeRoundCapStop,断言 cause === 'round-cap'。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31389561905 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 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.
Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): Change summary: PR #8850 corrects the lifecycle of the ro...: (none — all checks I started were completed within budget).. Test Plan (not a blocker): src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more.
中文说明
已审查。 建议见行内评论。 未探索到全部深度（达到工具调用预算）：Change summary: PR #8850 corrects the lifecycle of the ro...：(none — all checks I started were completed within budget).。 Test Plan（非阻断）：src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| * Clears any same-run stop marker first: a converged exit can follow an | ||
| * over-cap round the gate already refused (round 4 refused under cap 3, |
There was a problem hiding this comment.
[Suggestion] The CONVERGED-exit clear closes the marker channel but leaves the relay channel open: nothing tells the orchestrator to drop the round-cap unreviewedDimensions entry the earlier refusal explicitly instructed it to add, and compose-review's phrase-dedup splice (compose-review.ts:423-443) runs only while a marker is on disk. — Failure scenario: cap 3; round 4 is refused at the cap and stderr instructs "still add reverse audit — did not converge within the reverse-audit round cap of 3 to unreviewedDimensions"; the orchestrator records the entry (compliant), then asks for round 5 anyway, which exits CONVERGED. The marker is cleared, but the CONVERGED message only says "no unreviewedDimensions entry is owed" — it never says to remove one already added (no removal instruction exists anywhere, including SKILL.md's CONVERGED bullet). At Step 6 the stale relayed entry survives the missing splice, caps the legitimately-converged verdict at COMMENT via unreviewed-dimension (compose-review.ts:862-873), and the public body of an audit that converged says "did not converge within the round cap". Pre-PR this was masked because the surviving marker caused the relayed copy to be spliced out — the clear is what makes this false cap newly reachable. Suggested fix: extend the CONVERGED stderr (and the matching SKILL.md bullet) with e.g. "if an earlier round-cap or budget refusal told you to add its stop entry to unreviewedDimensions, remove it now — this convergence supersedes it"; alternatively have compose-review splice both stop phrases whenever the schedule converged.
中文说明
[建议] CONVERGED 退出时的清标记只关闭了标记通道， relay 通道仍然敞着：没有任何提示让编排器丢弃此前拒绝消息明确要求它添加的 round-cap unreviewedDimensions 条目，而 compose-review 的短语去重拼接（compose-review.ts:423-443）只在标记文件存在时才运行。——故障场景：cap 3；第 4 轮在 cap 处被拒，stderr 指示「仍要把 reverse audit — did not converge within the reverse-audit round cap of 3 加入 unreviewedDimensions」；编排器照做记下该条目（合规行为），然后照样请求第 5 轮，第 5 轮以 CONVERGED 退出。标记被清掉，但 CONVERGED 消息只说「不欠 unreviewedDimensions 条目」——从不说要移除已添加的条目（包括 SKILL.md 的 CONVERGED 条目在内，任何地方都没有移除指示）。到 Step 6 时，残留的 relay 条目因缺少拼接去重而保留下来，通过 unreviewed-dimension（compose-review.ts:862-873）把合法收敛的裁决封顶为 COMMENT，一份已收敛审计的公开正文却写着「未在轮数上限内收敛」。PR 之前这一问题被掩盖，因为存活的标记会把 relay 副本拼接掉——正是清标记让这一错误封顶变得可触达。建议修复：在 CONVERGED stderr（以及对应的 SKILL.md 条目）中补充类似「如果此前某次 round-cap 或 budget 拒绝让你把停止条目加入了 unreviewedDimensions，现在移除它——本次收敛取代那次拒绝」的语句；或者让 compose-review 在调度收敛时无条件拼接两种停止短语。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| `little time remains; when the deadline is within that floor, stop ` + | ||
| `waiting on any verifier batch still out and compose with the tags ` + |
There was a problem hiding this comment.
[Suggestion] The bounded-tail protocol this PR mirrors into the ROUND CAP message is duplicated in reverseAuditBudgetMessage (lib/deadline.ts:631-660) and the two copies already diverge: a grep across packages/cli/src for the wait-bound clause ("stop waiting on any verifier batch still out") and "invent a fresh re-verification pass" matches exactly one location — this message. The time-budget twin carries neither, even though SKILL.md's budget-stop bullet (line 675) documents both rules for the budget-stop tail and the round-cap bullet this PR rewrites claims parity ("under the same bounded tail as the budget stop below"). Both copies are independently test-pinned (deadline.test.ts:473-477; agent-prompt.test.ts:3479-3486) and neither pins the divergent clauses, so green tests lock the drift in. — Failure scenario: a time-budget stop (the common CI stop mode — the one the measured #8687 kill-before-compose incident ran) hands the orchestrator a stderr message that says verify-then-compose but never says to stop waiting on an already-admitted verifier batch once the deadline enters the compose floor; a verifier admitted above the floor keeps running past it, the orchestrator waits, the wall hits before compose, and confirmed findings never post — the exact harm the wait-bound rule exists to prevent. Suggested fix: extend reverseAuditBudgetMessage's tail with the same two clauses and pin them in its deadline.test.ts sibling test, or hoist the shared tail-protocol sentences into one constant used by both refusals.
中文说明
[建议] 本 PR 镜像进 ROUND CAP 消息的有界收尾协议，在 reverseAuditBudgetMessage（lib/deadline.ts:631-660）里有一份副本，且两份已经分叉：在 packages/cli/src 全量 grep 等待上限子句（"stop waiting on any verifier batch still out"）与 "invent a fresh re-verification pass"，只命中一处——就是这条消息。时间预算的孪生消息两条都没有，而 SKILL.md 的 budget-stop 条目（第 675 行）明确要求 budget-stop 收尾同时遵守这两条规则，本 PR 重写的 round-cap 条目又声称两者对等（"under the same bounded tail as the budget stop below"）。两份副本各自被测试钉住（deadline.test.ts:473-477；agent-prompt.test.ts:3479-3486），且都没有钉分叉的子句，于是绿色测试把漂移锁死了。——故障场景：时间预算停止（CI 中最常见的停止方式——被实测记录的 #8687「compose 前被杀」事故就发生在这条路径）交给编排器的 stderr 消息只说 verify-then-compose，却没有说当 deadline 进入 compose 预留区后要停止等待已放行的验证器批次；一个在预留线之上放行的验证器继续运行越过预留线，编排器一直等，时间墙在 compose 之前落下，已确认的发现一条都没发出去——这正是等待上限规则要防止的危害。建议修复：给 reverseAuditBudgetMessage 的尾部补上同样的两条子句，并在它的 deadline.test.ts 孪生测试里钉住；或把共享的收尾协议语句提取为一个常量，供两处拒绝消息共用。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 4/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31397491013 🧠 Handled by Qwen Code · model/模型 |
…n both refusal tails Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8850All four inline findings from the automated reviewer were verified against the Findings and dispositions
VerificationCommands actually run this round (all passed):
中文说明Autofix 审查轮次 — PR #8850自动审查器的四条行内发现均已在代码中核实，并在一次提交中解决（ 发现与处理
验证本轮实际执行的命令（全部通过）：
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.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change summary: PR #8850 corrects the lifecycle of the ro...: I did not execute the new/changed vitest suites ( agent-prompt.test.ts , deadline.test.ts , compose-review.test.ts , SKILL.test.ts ) — verification was by re…; Change summary: PR #8850 corrects the lifecycle of the ro...: I did not trace scheduleReverseAuditRound 's internals in retirement.ts end-to-end (transcript parsing, cold-check cadence) beyond confirming it ignores the ….
Test Plan (not a blocker): src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：Change summary: PR #8850 corrects the lifecycle of the ro...：I did not execute the new/changed vitest suites ( agent-prompt.test.ts , deadline.test.ts , compose-review.test.ts , SKILL.test.ts ) — verification was by re…；Change summary: PR #8850 corrects the lifecycle of the ro...：I did not trace scheduleReverseAuditRound 's internals in retirement.ts end-to-end (transcript parsing, cold-check cadence) beyond confirming it ignores the …。
Test Plan（非阻断）：src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 3 more。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(msg).toContain('agent-prompt --role verify'); | ||
| expect(msg).toContain('never a hand-rolled agent'); | ||
| expect(msg).toContain('compose floor'); | ||
| expect(msg).toContain('Do NOT re-verify findings already'); |
There was a problem hiding this comment.
[Suggestion] The round-cap refusal's two new bounded-tail clauses are pinned only against reverseAuditBudgetMessage (deadline.test.ts:481-482), not at this round-cap refusal site — the deadline.test.ts comment this diff adds claims "both pin both", but the round-cap side of that symmetry does not exist. — Failure scenario: probe-verified mutation — deleting both stop waiting on any verifier batch still out and invent a fresh re-verification pass from the round-cap refusal in agent-prompt.ts alone ships green (agent-prompt.test.ts 13 passed, deadline.test.ts 43 passed, SKILL.test.ts 7 passed), silently loosening the bounded-tail termination contract on exactly the huge-diff cap-stopped runs.
| expect(msg).toContain('agent-prompt --role verify'); | |
| expect(msg).toContain('never a hand-rolled agent'); | |
| expect(msg).toContain('compose floor'); | |
| expect(msg).toContain('Do NOT re-verify findings already'); | |
| expect(msg).toContain('agent-prompt --role verify'); | |
| expect(msg).toContain('never a hand-rolled agent'); | |
| expect(msg).toContain('compose floor'); | |
| expect(msg).toContain('Do NOT re-verify findings already'); | |
| expect(msg).toContain('stop waiting on any verifier batch still out'); | |
| expect(msg).toContain('invent a fresh re-verification pass'); |
中文说明
[建议] round-cap 拒绝消息新增的两条有界收尾子句目前只钉在了 reverseAuditBudgetMessage（deadline.test.ts:481-482）上，没有钉在这个 round-cap 拒绝消息的位置——本 diff 在 deadline.test.ts 新增的注释声称 "both pin both"，但那个对称的 round-cap 一侧并不存在。— 失败场景：经探针验证的变异——仅从 agent-prompt.ts 的 round-cap 拒绝消息中删除 stop waiting on any verifier batch still out 与 invent a fresh re-verification pass 两条子句，整个套件仍为绿色（agent-prompt.test.ts 13 通过、deadline.test.ts 43 通过、SKILL.test.ts 7 通过），恰好在大 diff 触发上限停止的运行时悄悄放松了有界收尾的终止契约。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const body = skillBody(); | ||
| expect(body).toContain('`agent-prompt --role verify` **only**'); | ||
| expect(body).toContain('no fresh re-verification pass'); |
There was a problem hiding this comment.
[Suggestion] The wait-bound clause this PR adds to the round-cap bullet in SKILL.md (stop waiting on any verifier batch still out and compose with the tags in hand) is pinned nowhere at that site — this new pin test covers only the verify-only and no-fresh-pass clauses, and the clause's only pin anywhere is deadline.test.ts:481, which pins the CLI budget message, a different artifact the SKILL.md edit cannot reach. — Failure scenario: probe-verified mutation at HEAD — deleting the wait-bound clause from SKILL.md's round-cap bullet ships green (SKILL.test.ts 7 passed); the orchestrator-facing instruction for a huge-diff cap stop then silently loses the wait-bound rule — the exact PR #8687 killed-before-compose failure class the bounded tail exists to prevent — on exactly the runs the bullet itself says "This matters most on".
| const body = skillBody(); | |
| expect(body).toContain('`agent-prompt --role verify` **only**'); | |
| expect(body).toContain('no fresh re-verification pass'); | |
| const body = skillBody(); | |
| expect(body).toContain('`agent-prompt --role verify` **only**'); | |
| expect(body).toContain('no fresh re-verification pass'); | |
| expect(body).toContain('stop waiting on any verifier batch still out'); |
中文说明
[建议] 本 PR 在 SKILL.md 的 round-cap 条目中加入的等待上限子句（stop waiting on any verifier batch still out and compose with the tags in hand）在该位置没有任何测试钉子——这个新增的钉住测试只覆盖了 verify-only 与 no-fresh-pass 两条子句，而该子句在全仓库唯一的钉子是 deadline.test.ts:481，那里钉的是 CLI 预算消息，是 SKILL.md 编辑无法触及的另一份产物。— 失败场景：在 HEAD 上经探针验证的变异——从 SKILL.md 的 round-cap 条目中删除等待上限子句，套件仍为绿色（SKILL.test.ts 7 通过）；大 diff 触发上限停止时面向编排器的指示就会悄悄丢失等待上限规则——正是有界收尾机制要防止的 PR #8687 "compose 前被杀"那一类失败——而且恰好发生在该条目自己写着 "This matters most on" 的那些运行上。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 AutoFix found no actionable work this round — no changes were made. This PR is in Critical-only mode (more than five change-producing review rounds), so this round only evaluates Critical findings, failed checks, and base conflicts:
The following non-Critical items were deferred by the round budget and intentionally left untouched — no code changes, no thread resolutions, no replies. They remain open for human follow-up:
中文说明🤖 AutoFix 本轮没有可处理的工作 —— 未做任何改动。 本 PR 已进入仅处理 Critical 的模式（产生改动的评审轮次已超过 5 轮），因此本轮只评估 Critical 级别的问题、失败的检查和与基线分支的冲突：
以下非 Critical 条目因轮次预算被延后，本轮有意保持原样 —— 不改代码、不解决讨论串、不回复评论，留待人工跟进：
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Test Plan (not a blocker): src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 2 more.
中文说明
已审查——无阻断问题。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 Test Plan（非阻断）：src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 2 more。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| * legitimately converged. Missing file and unlink errors are swallowed — | ||
| * the file was the thing to be rid of. | ||
| */ | ||
| export function clearBudgetStop(planPath: string): void { |
There was a problem hiding this comment.
[Suggestion] clearBudgetStop has no direct unit test in deadline.test.ts — Failure scenario: A refactor that moves the clearBudgetStop call out of refuseConverged into a caller that never reaches it would pass the test suite if the marker-clearing assertion's path is untouched. The catch block's error handling is never exercised.
| export function clearBudgetStop(planPath: string): void { | |
| // Add unit tests for clearBudgetStop in deadline.test.ts |
中文说明
clearBudgetStop 在 deadline.test.ts 中没有直接的单元测试——失败场景：如果重构将 clearBudgetStop 调用移出 refuseConverged 到某个永远不会到达的调用者，测试套件仍会通过，因为标记清除的断言路径未被触及。catch 块的错误处理也从未被测试。建议在 deadline.test.ts 中添加直接调用 clearBudgetStop 的单元测试，验证标记被清除，以及无标记时的无操作路径。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| expect(msg).toContain('agent-prompt --role verify'); | ||
| expect(msg).toContain('never a hand-rolled agent'); | ||
| expect(msg).toContain('compose floor'); | ||
| expect(msg).toContain('Do NOT re-verify findings already'); |
There was a problem hiding this comment.
[Suggestion] Round-cap refusal message's wait-bound clauses not pinned — Failure scenario: A reword that drops either clause from the round-cap refusal message would pass the test suite, silently loosening the termination contract for the cap gate.
| expect(msg).toContain('agent-prompt --role verify'); | |
| expect(msg).toContain('never a hand-rolled agent'); | |
| expect(msg).toContain('compose floor'); | |
| expect(msg).toContain('Do NOT re-verify findings already'); | |
| expect(msg).toContain('stop waiting on any verifier batch still out'); | |
| expect(msg).toContain('invent a fresh re-verification pass'); |
中文说明
round-cap 拒绝消息的 wait-bound 从句未被测试固定——失败场景：删除 round-cap 拒绝消息中任何一个从句的改写将不会导致测试失败，从而静默地放宽了 cap 门的终止契约。建议在 agent-prompt.test.ts 中为 round-cap 拒绝消息添加 stop waiting on any verifier batch still out 和 invent a fresh re-verification pass 的断言，与 deadline.test.ts 中的模式一致。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Autofix review-response summary — PR #8850 (round after 2026-08-10T17:45:53Z)Two actionable inline suggestions from the automated reviewer; both implemented. No Critical findings, no failed checks, no conflict ( Feedback points and dispositions
Also noted, no action owed: the 2 Suggestion-level findings the reviewer could not anchor to a changed line were dropped by the reviewer itself, and the reviewer's Test Plan file-not-found entries are a path-relativity artifact (the paths resolve under ChangesCommit
Nothing was removed or shrunk this round — both findings were missing-test gaps, so the minimal resolution is purely additive test pins. VerificationCommands actually run (repo root unless noted):
中文说明Autofix 评审响应总结 — PR #8850（2026-08-10T17:45:53Z 之后的这一轮）自动评审器给出 2 条可操作的行内建议，均已实现。无 Critical 发现，无失败检查，无冲突（ 反馈点及处理
另外说明两点，无需处理：评审器自身已丢弃其无法锚定到改动行的 2 条建议级发现；评审器 Test Plan 中的文件未找到条目是路径相对性问题（这些路径在 变更内容提交
本轮没有删除或收缩任何代码——两条发现都是缺测试的缺口，因此最小解决方式是纯增量地补上测试固定。 验证实际运行的命令（未注明者均在仓库根目录执行）：
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 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.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): This PR corrects the lifecycle of the review round-cap st...: none — all checks I intended completed within budget.; This PR corrects the lifecycle of the review round-cap st...: none — all checks above completed within budget..
Test Plan (not a blocker): src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 2 more.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：This PR corrects the lifecycle of the review round-cap st...：none — all checks I intended completed within budget.；This PR corrects the lifecycle of the review round-cap st...：none — all checks above completed within budget.。
Test Plan（非阻断）：src/commands/review/agent-prompt.test.ts — no such file or directory; src/commands/review/lib/deadline.test.ts — no such file or directory; src/commands/review/lib/budget.test.ts — no such file or directory; src/commands/review/lib/retirement.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // so a time-budget stop followed by a retry that the cap then refuses | ||
| // does not post two contradictory stop disclosures. |
There was a problem hiding this comment.
[Suggestion] R4-1: The first-refusal-wins guard hardens the marker channel but not the stderr instruction channel: a same-run double refusal can still post two contradictory stop disclosures, and this comment overclaims what the guard prevents. The ROUND CAP message also prints "A marker has been recorded" although the write no-oped — the marker on disk is the other cause's. — Failure scenario: cap-3 huge-diff run: the time gate refuses round 3 (time-budget marker written, budget entry relayed); the orchestrator retries round 4; the cap gate fires first (4 > 3), writeRoundCapStop no-ops via this guard, but admitReverseAuditRound still prints the ROUND CAP message instructing the orchestrator to relay reverse audit — did not converge within the reverse-audit round cap of 3. At Step 6 compose-review splices only entries containing the retained marker's phrase ('review time budget'), so the relayed round-cap entry survives → the posted review names two contradictory stop causes. Probe-verified end to end on the PR's own scaffolding. The verdict cap is unaffected (either cause caps identically); the trigger needs two off-script orchestrator moves — exactly the retry-after-refusal class this guard was built for. The mirror order (cap marker first, then a time refusal) is the same defect class.
Suggested fix (spans agent-prompt.ts / compose-review.ts, so no one-click suggestion):
// Option (a) — make the refusal marker-aware in admitReverseAuditRound (agent-prompt.ts):
const existing = readBudgetStop(planPath);
// when existing !== null, emit a short "a stop is already recorded — relay no new
// entry" line (or re-emit existing.entry) instead of the newly-refused gate's entry.
// Option (b) — in compose-review.ts, splice EITHER stop phrase when a marker is present:
const phrases = [ROUND_CAP_PHRASE, BUDGET_STOP_PHRASE];
// splice relayed unreviewed entries matching phrases.some((ph) => entry.includes(ph))中文说明
[建议] R4-1：首次拒绝胜出的守卫只加固了标记通道，没有加固 stderr 指示通道：同一 run 内的双重拒绝仍然可能发出两条互相矛盾的停止披露，而这条注释夸大了守卫实际防止的范围。此外 ROUND CAP 消息在写入实际为空操作（no-op）时仍会打印 "A marker has been recorded"——而磁盘上的标记其实是另一种 cause 的。——故障场景：cap 为 3 的巨型 diff 运行：时间门拒绝第 3 轮（写入时间预算标记、转发预算条目）；编排器重试第 4 轮；上限门先触发（4 > 3），writeRoundCapStop 经此守卫变为空操作，但 admitReverseAuditRound 仍会打印 ROUND CAP 消息，指示编排器转发 reverse audit — did not converge within the reverse-audit round cap of 3。到 Step 6 时，compose-review 只剔除包含保留标记短语（'review time budget'）的条目，因此被转发的 round-cap 条目会保留下来 → 发布的评审里出现两个互相矛盾的停止原因。已在本 PR 自身的测试脚手架上端到端探针验证。裁决封顶不受影响（两种 cause 的封顶完全相同）；触发需要编排器两次越轨操作——这正是此守卫要防御的"重试-拒绝"类别。镜像顺序（先有 cap 标记、随后时间拒绝）属于同一缺陷类别。
建议修复（跨 agent-prompt.ts / compose-review.ts，故不用一键 suggestion）：让拒绝逻辑感知已有标记（存在同 run 标记时改发"停止已记录、无需再转发条目"的简短指示，或复述已记录标记自身的条目）；或在 compose-review 中只要标记存在就按两种停止短语中的任意一种剔除转发条目。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // compose-floor contract; a revert of the bullet's protocol hunk must | ||
| // fail a test, not slip through. | ||
| const body = skillBody(); | ||
| expect(body).toContain('`agent-prompt --role verify` **only**'); |
There was a problem hiding this comment.
[Suggestion] R3-2: Previous-round finding R3-2 still stands: the wait-bound clause this PR adds to the round-cap bullet in SKILL.md (stop waiting on any verifier batch still out and compose with the tags in hand) is pinned nowhere at that site — this new test pins only the --role verify only rule and the no fresh re-verification pass clause. — Failure scenario: a reword that drops the wait-bound clause from the round-cap bullet passes the whole suite (probe: removing the clause leaves SKILL.test.ts 7/7 green, while mangling a pinned clause fails this test), silently loosening the termination contract in the document orchestrators read. The CLI-message pins in agent-prompt.test.ts / deadline.test.ts pin a different artifact.
| expect(body).toContain('`agent-prompt --role verify` **only**'); | |
| expect(body).toContain('`agent-prompt --role verify` **only**'); | |
| expect(body).toContain('stop waiting on any verifier batch still out'); |
中文说明
[建议] R3-2：上一轮发现 R3-2 仍然存在：本 PR 为 SKILL.md round-cap 条目新增的等待上限子句（stop waiting on any verifier batch still out and compose with the tags in hand）在该位置没有任何测试固定——这个新测试只固定了 --role verify only 规则和 no fresh re-verification pass 子句。——故障场景：一次把等待上限子句从 round-cap 条目中删掉的改写，可以让整个测试套件照常通过（探针：删除该子句后 SKILL.test.ts 仍为 7/7 全绿；而破坏已固定的子句则会使本测试失败），从而在编排器所读取的文档中悄然放松终止契约。agent-prompt.test.ts / deadline.test.ts 中的 CLI 消息固定钉的是另一份产物，覆盖不到此处。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no action taken (PR #8850)This round had no actionable feedback, so no changes were made:
No commits were created; the branch head remains at 中文说明Autofix 轮次总结 — 未采取任何操作（PR #8850）本轮没有可执行的反馈，因此未做任何改动：
未创建任何提交；分支头部仍为 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 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: 59 passed · 0 failed · 59 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：59 通过 · 0 失败 · 59 总计 Verification reportPR 8850 verification (round 2) — fix(review): correct the round-cap marker lifecycle and stale cap docsVerdict: Previous-finding status
中文 — 判定：✅ 通过 · 可合入（agent 判定）第二轮沙箱验证（隔离、无凭证容器）。上一轮验证的是 head
Central claim and A/BCentral claim (unchanged from round 1, extended): the round-cap stop marker now shares the time-budget marker's lifecycle discipline — first refusal wins, a CONVERGED exit clears any same-run marker and recalls the relayed stop entry the refusal told the orchestrator to add, and the coverage exemption is cause-aware — so the three R4 correctness bugs are gone and the converged-after-refusal run reads as a clean convergence everywhere. Harness: the PR's own new/extended test files, copied unmodified into a scratch worktree at
Witnesses: Mutation matrix (on head; every restore verified
|
| # | Mutant (exact-string edit) | Suite | Result |
|---|---|---|---|
| M1 | delete first-refusal guard in writeRoundCapStop |
deadline | KILLED — 1 red: A1 only |
| M2 | delete first-refusal guard in writeBudgetStop |
deadline | KILLED — 1 red: A2 only |
| M3 | delete clearBudgetStop(planPath) in refuseConverged |
agent-prompt | KILLED — 3 reds: all three clear tests |
| M4 | cause-conditional clear (if (…cause === 'round-cap')) |
agent-prompt | KILLED — 1 red: the time-budget clear test only |
| M5 | --chunk site threads args.plan + '.bak' |
agent-prompt | KILLED — 1 red: the --chunk test only |
| M6 | revert coverage exemption to cause-blind | compose-review | KILLED — 1 red: C1 only |
| M7 | delete the recall sentence from the CONVERGED message | agent-prompt | KILLED — 1 red: the round-cap clear test (its remove it now assertion) |
| M8 | revert the ROUND CAP message tail to base wording | agent-prompt | KILLED — 1 red: the cap-message test |
| M9a | delete the wait-bound clause from the budget message | deadline | KILLED — 1 red: the budget-message test |
| M9b | delete the no-fresh-pass clause from the budget message | deadline | KILLED — 1 red: the budget-message test |
| M10 | existsSync guard instead of the fence-aware readBudgetStop guard |
deadline | KILLED — 1 red: the fence-mirror test |
| M11a | SKILL.md: revert the relay-removal sentence | SKILL.test | KILLED — 1 red: the relay-removal pin |
| M11b | SKILL.md: revert the round-cap bounded-tail hunk | SKILL.test | KILLED — 1 red: the bounded-tail pin |
| M12 | positive control: reword BUDGET_STOP_PHRASE |
deadline | KILLED — 2 reds: pre-existing round-trips, entry text and all + A1 (both pin the entry literal) |
14/14 killed, no survivors. Attribution controls scripted in evidence-summary.mjs: M1 spares A2, M2 spares A1, M4 spares the round-cap clear test, M5 spares both --all-chunks clear tests — so the cause-conditional and per-site mutants prove the new tests pin which clear and which site, not just that some clear exists. Witness: 02-mutation-matrix-14-of-14-killed.png.
The new fix, mechanistically: why the recall sentence is load-bearing
Commit 93ff7caa's claim — "once the marker is gone, the compose-review splice that dedups [the relayed entry] no longer runs; only this instruction removes it" — was checked against the real code, then driven: compose-review.ts runs its dedup splice and pushes the structural disclosure only inside if (stop !== null), so a cleared marker disables both. The mock-free relay-splice-harness.mjs drives the real composeReview through tsx from each worktree's source (all imports cli-local; occurrence counts are the oracle because the disclosure renders the same text as the relay):
| Cell | head 52590f5 |
base 8c90697 (A/A) |
|---|---|---|
| R1 no marker + 1 relay → entry survives, verdict capped COMMENT | PASS | PASS |
| R1b no marker + 2 distinct relays → both survive | PASS | PASS |
| R2 marker + 1 relay → relay spliced, disclosure renders once | PASS | PASS |
| R2b marker + 2 distinct relays → both spliced | PASS | PASS |
Both arms identical: the marker-keyed splice is pre-existing behavior; the PR's clearBudgetStop is what creates the state where the relay outlives the stop — and the CONVERGED message + SKILL.md instruction are the only removal channel. Witness: 04-relay-splice-both-arms.png. (First-round note: identical relays dedup upstream of the splice on both arms — a harness-design artifact caught and corrected with distinct-text controls, not a code behavior.)
Reviewer Test Plan walkthrough
| Step | Result |
|---|---|
6 cli suites from packages/cli |
ran verbatim: Test Files 6 passed, Tests 646 passed — command correct; the plan's claimed count (616) is stale, see finding 1 |
core SKILL.test.ts |
7 passed — claimed count (5) likewise stale |
| "each fix confirmed against a mutation of its own fix" | reproduced independently and extended: M1–M4 as claimed, plus M5/M7/M8/M9a/M9b/M10/M11 for the delta's new guards and pins |
eslint --max-warnings 0 + typecheck clean |
reproduced scoped: lint on the 8 changed TS files exits 0 (liveness-proven — a planted any/unused-var file was caught with 2 errors before deletion); npm run typecheck -w packages/cli -w packages/core exits 0 |
Corrections
None — no earlier review round described the code inaccurately.
Findings
No blocking findings. One new nit plus two carried design boundaries:
- The Reviewer Test Plan's counts are stale. The plan says
616 passed(cli) and5 passed(SKILL); the final head runs646and7— the plan text matches the PR's first commit, not the head that adds 30 tests. The commands themselves are correct and everything passes; a reviewer following the plan should expect the larger numbers. - The recall is an instruction, not an enforcement (confirmed design boundary). The relay-splice cells show that if an orchestrator ignores the
remove it nowinstruction, a legitimately-converged run posts its own superseded stop entry and caps itself at COMMENT — conservative, never a false Approve, and the marker channel (the dangerous one) is mechanically closed. SKILL.md mirrors the instruction at the CONVERGED bullet (pinned by M11a). Recorded so the dependency is named, not discovered. - F3 residual (carried). The first-refusal guard reads through
readBudgetStop's run-epoch fence (now pinned for both writers — M10's existsSync mutant dies on the new mirror test). A plan re-captured mid-loop would still re-admit a second write; within the skill's flow the plan is only written in the setup batch, and the fence defines such a rewrite as a new run. Stands as round 1 assessed it.
Not covered
- No live orchestrator end-to-end run; the harnesses drive the real
runAgentPrompt/composeReview/verificationGapscode paths through vitest (the CLI's own test seam) and the realcomposeReviewthroughtsx. This reproduces the lifecycle causes and the splice mechanics, not a model-driven multi-round session. - Per-commit attribution: the depth-2 merge-ref checkout makes only
HEAD^2reachable (git rev-list HEAD^1..HEAD^2= 1 commit vs the snapshot's 6 — the shallow boundary returns a plausible 1 rather than erroring, so the mismatch was checked explicitly). The aggregateHEAD^1..HEADdiff is what was verified. - macOS/Windows not exercised (same as the PR; marker I/O is plain
budget-stop.jsonreads/writes). - Repo-wide
npm run lintnot run; the lint gate was scoped to the 8 changed files with a liveness probe. - The round-cap bullet's prose beyond the pinned clauses (the "This matters most on exactly the huge diffs…" rationale sentence) is unpinned by design — SKILL.md pins target the contractual clauses, not rationale text.
Methodology
CI verify container (node:22-bookworm, merge-ref checkout at depth 2, npm ci + build pre-run). Base arm: git worktree add tmp/base-tree HEAD^1 (8c90697), per-package node_modules symlinked (zero @qwen-code entries in them; vitest aliases resolve workspace imports to each worktree's own source, and the base arm's 11 exact failures are the provenance proof), head test files copied in unmodified; a symbol-stripped deadline variant was prepared but not needed (the unmodified file's runtime not a function failure is the base cell). Mutants applied as exact-string edits by mutate.mjs (occurrence-count verified per pattern), each run through mutation-run.sh followed by git checkout -- and a clean-git status assertion. relay-splice-harness.mjs imports the real composeReview/writeRoundCapStop from each worktree via tsx. All 59 assertions are re-executable from evidence-summary.mjs, which parses the raw logs in logs/ (ab-base-*.log, head-cli-suites.log, head-skill-test.log, mutant-M1..M12.log, relay-splice-{head,base}.log, doc-boundary-checks.log, eslint/typecheck logs) and emits assertions.json. Doc/boundary facts scripted in doc-boundary-checks.mjs (18 PASS, base side read via git show HEAD^1:). Captures rendered by scripts/verify-capture.mjs.
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. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action neededThis round found no actionable feedback for PR #8850:
Critical-only mode is active after five change-producing rounds. The one deferred item listed in the round input (a non-Critical PR comment from the automated reviewer) is an audit record for human follow-up, not work for this round, so it was intentionally left untouched — no code change, no thread resolution, and no reply for it. No commits were made; the branch head is unchanged. 中文说明Autofix 审查轮次 — 无需处理本轮未发现针对 PR #8850 的可处理反馈：
在经历五个产生改动的轮次后，当前已进入仅处理 Critical 的模式。本轮输入中列出的那条延后条目（来自自动审查器的一条非 Critical PR 评论）只是留待人工跟进的审计记录，不属于本轮的工作范围，因此有意保持原样 —— 未对其修改代码、未解决相关话题，也未回复。 本轮未产生任何提交；分支头部保持不变。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The three lifecycle bugs from #8773's post-merge auto-review are fixed correctly and pinned by tests: (1) first-refusal-wins in both writers (a time-budget stop followed by a cap-refused retry keeps the time-budget cause, and vice versa) with the run-epoch fence still letting a fresh run write over a previous run's marker; (2) refuseConverged now clears any same-run marker at both convergence sites, so a stale round-cap marker can no longer cap a legitimately converged verdict, and the converged stderr also tells the orchestrator to drop the relayed stop entry from unreviewedDimensions since the dedup splice no longer runs once the marker is gone; (3) coverage.ts's reverseByDesign exemption is cause-aware (stop.cause !== 'round-cap'), so a round-cap stop still owes the not-built gap and its rebuild remediation — exactly right, since the cap gate's monotone-refusal premise fails for the rebuild. The round-cap refusal message now carries the same gated-verifier discipline as the time-budget one (verify only via --role verify, stop waiting inside the compose floor, no fresh re-verification pass), and the stale retirement.ts/retirement-scheduler doc claims are corrected. Tests pin first-refusal both directions, idempotent never-throwing clearBudgetStop, previous-run fencing, and the cause-aware coverage behavior; CI green on head. Nothing blocks merge.
|
Released in v0.21.10. |
|
I found two blocking lifecycle issues in the current head:
Changed-file ESLint passes. |
5 participants
What this PR does
Corrects the lifecycle of the round-cap stop marker introduced in #8773 and the docs that #8773's huge-diff cap left stale, addressing the 11 findings (R4-1 … R4-11) its post-merge auto-review raised. The round-cap marker shares
budget-stop.jsonwith the time-budget stop but did not share its lifecycle rules: (1) first refusal now wins — bothwriteRoundCapStopandwriteBudgetStopskip the write when a same-run marker already exists, so a time-budget stop followed by a retry the cap refuses no longer overwrites the marker last-write-wins and posts two contradictory stop disclosures; (2) a CONVERGED exit now clears any same-run marker via the newclearBudgetStop, called fromrefuseConvergedat both convergence sites, so a stale round-cap marker from a refused over-cap round can no longer cap a legitimately-converged APPROVE to COMMENT; (3)coverage.ts'sreverseByDesignexemption is now cause-aware (stop.cause !== 'round-cap'), so a round-cap marker no longer suppresses thenot-builtreverse-audit gap and its rebuild remediation. On the docs side it drops the stale "retirement.tsre-exports it" and "the retirement scheduler" reader claims inbudget.ts, notes the huge-diff 3-round cap inDESIGN.md's LLM-call-budget paragraph and table, and mirrors the time-budget stop's bounded-tail protocol into theROUND CAP:message and the SKILL.md round-cap bullet.Why it's needed
#8773's post-merge auto-review flagged three real correctness bugs and eight doc/test gaps. The lifecycle bugs are the substance: because the round-cap marker and the time-budget marker share one file with no lifecycle discipline, a retry-after-refusal sequence (time gate refuses round 3, the orchestrator retries round 4, the cap gate refuses it) overwrote the marker and posted a body carrying two contradictory stop disclosures, one structurally false; a round-cap refusal followed by a converged round (the convergence check runs before the cap gate) left a stale marker that capped a verdict the audit legitimately converged; and
coverage.tsread the marker cause-blind, suppressing the not-built gap and its rebuild remediation for a round-cap stop whose fix (--round 1) is actually admitted. The doc drift (retirement.tsno longer re-exports the constant or reads the cap; the LLM-call-budget section still assumed a 5-round-only world; the round-cap stop lacked the bounded-tail protocol the time-budget stop carries) would mislead the next maintainer touching this code.Reviewer Test Plan
How to verify
Run the review test suites and the SKILL gate — all pass, with the three correctness fixes and two test pins added:
Each of the three correctness fixes was confirmed against a mutation of its own fix (negative control): removing the first-refusal-wins guard fails the new
deadline.test.tscases; removing theclearBudgetStopcall inrefuseConvergedfails the new CONVERGED-clears-marker test; makingcoverage.tscause-blind again fails the new round-cap-does-not-suppress test.eslint --max-warnings 0and the review-file typecheck are clean.Evidence (Before & After)
N/A — non–user-visible (internal review-tooling correctness + docs; no TUI surface changes).
Tested on
Environment (optional)
Unit tests only (vitest); no runtime app launch.
Risk & Scope
budget-stop.jsonreads/writes with no platform-specific paths)./reviewskill's CLI subcommands; no public API, flag, or plan-schema change.Linked Issues
Follow-up to #8773 (addresses its post-merge review findings R4-1 … R4-11). No closing keyword — #8773 is already merged.
中文说明
这个 PR 做了什么
修正 #8773 引入的 round-cap 停止标记的生命周期,以及 #8773 巨型 diff 缩减上限遗留的过时文档,处理其合入后自动评审提出的 11 条发现(R4-1 … R4-11)。round-cap 标记与时间预算停止共用
budget-stop.json,却没有共用其生命周期规则:(1)首次拒绝胜出——writeRoundCapStop与writeBudgetStop在同一 run 已存在标记时跳过写入,于是"时间预算停止之后重试、被上限拒绝"不再以后写覆盖改写标记、发布两条互相矛盾的停止披露;(2)CONVERGED 退出通过新增的clearBudgetStop清除同 run 标记(在refuseConverged的两个收敛调用点调用),于是被拒超上限轮遗留的残留标记不再把合法收敛的 APPROVE 降为 COMMENT;(3)coverage.ts的reverseByDesign豁免改为区分 cause(stop.cause !== 'round-cap'),于是 round-cap 标记不再压制not-built反向审计缺口及其重建修复。文档方面:删除budget.ts中过时的 "retirement.tsre-exports it" 与 "the retirement scheduler" 读者表述,在DESIGN.md的 LLM 调用预算段落与表格中标注巨型 diff 的 3 轮上限,并把时间预算停止的有界收尾协议镜像进ROUND CAP:消息与 SKILL.md 的 round-cap 条目。为什么需要
#8773 合入后的自动评审指出了三个真正的正确性缺陷和八处文档/测试缺口。生命周期缺陷是核心:由于 round-cap 标记与时间预算标记共用同一文件却没有生命周期约束,"重试-拒绝"序列(时间门拒绝第 3 轮,编排器重试第 4 轮,上限门拒绝它)会覆盖标记、发布一条带有两条互相矛盾停止披露(其中一条结构上是错的)的正文;被拒的 round-cap 之后跟一个收敛轮(收敛检查先于上限门执行)会留下残留标记,把审计合法收敛的裁决封顶;而
coverage.ts不看 cause 读标记,会为一个"其修复(--round 1)实际会被放行"的 round-cap 停止压制 not-built 缺口及其重建修复。文档漂移(retirement.ts已不再 re-export 该常量、也不再读取 cap;LLM 调用预算段落仍假设"只有 5 轮"的世界;round-cap 停止缺少时间预算停止所带的有界收尾协议)会误导下一位改动这段代码的维护者。评审测试计划
如何验证
运行 review 测试套件与 SKILL 门——全部通过,并新增三个正确性修复与两个测试钉子:
三个正确性修复各自都经过对其自身修复的反向变异验证(负对照):去掉首次拒绝胜出的守卫会使新增的
deadline.test.ts用例失败;去掉refuseConverged中的clearBudgetStop调用会使新增的"CONVERGED 清标记"测试失败;把coverage.ts改回不看 cause 会使新增的"round-cap 不压制缺口"测试失败。eslint --max-warnings 0与 review 文件的 typecheck 均干净。证据(前后对比)
N/A——非用户可见(内部 review 工具的正确性 + 文档;无 TUI 界面变更)。
测试环境
运行环境(可选)
仅单元测试(vitest);未启动运行时应用。
风险与范围
budget-stop.json的普通读写,无平台相关路径)。/reviewskill 的 CLI 子命令内部;无公共 API、flag 或 plan schema 变更。关联 Issue
#8773 的后续(处理其合入后评审发现 R4-1 … R4-11)。无自动关闭关键字——#8773 已合入。