fix(review): fix silent reverse-audit retirement failures and keep non-converged evidence - #9213
Conversation
…erged evidence (QwenLM#9206) A round-5 reverse audit (PR QwenLM#9118, 12 chunks) never retired a chunk: four territories returned substantive dry receipts in BOTH rounds 1 and 2, yet rounds 3-5 rebuilt all 12 auditors, no retirement note appeared, and nothing anywhere said which certification condition refused the receipts. Step 9 cleanup then deleted the prompt-record directory of the non-converged run, so the failure could never be diagnosed. Two root causes, reproduced end to end (issue-9206-repro.test.ts): - The dry-receipt separator class admitted dashes and colons only, but honest receipts arrive separated by sentence punctuation too — "No new issues were found. Re-walked …" and "未发现新问题，重新走查了…" both reproduce the never-retire loop byte for byte. The clause after the separator is the part that proves the walk; the separator only has to show it exists. Widen the class to period, comma and semicolon in either width. The substance floor, the tool-call bars and the territory bar are unchanged — the bare stock sentence still reads unknown, and a yield still outranks any receipt. - Every certification refusal landed in the same silent unknown, and the schedule's catch swallowed every exception without a word. classifyReturn now reports the first bar that fell; the schedule carries one diagnostic line per twice-audited chunk that neither retired nor yielded, and the builder prints them on stderr (stdout stays the deliverable). The catch names itself: a round whose transcripts cannot be read audits every chunk AND says so. And the evidence half: cleanup swept the record directory of a non-converged run unconditionally — the round-cap marker that proves the non-convergence sat inside the very directory deleted. A record directory holding a same-run stop marker is now kept, with a note naming it; a converged run clears its marker, so its history sweeps as before.
github-actions
Bot
added
the
review/self-reported
E2E test report (reproduction + post-fix verification)Both phases ran headless, no model calls — the deterministic Reproduction (unfixed code, STATUS: REPRODUCED). Simulated the reported shape: 4 chunks, rounds 1–2 where two chunks return substantive dry receipts (verbatim delivery, successful tool calls, baked-territory diff reads) while two file findings. With an English receipt separated by a period ( Verification (fixed code, STATUS: VERIFIED) — against the rebuilt bundle, confirmed to contain the fix strings before use:
Regression: repro suite 1/5 → 5/5; retirement 53 ✓, cleanup 29 ✓, agent-prompt 236 ✓; full review dir 2538/2539 (the single failure, |
|
Re-run after the takeover rounds — this pass reviews Template looks good ✓ Problem: Observed bug with full forensics. #9206 reports a real run (review of PR #9118, 12 chunks) where four twice-dry chunks never retired across rounds 3–5, no Direction: Aligned. This repairs the retirement mechanism CHANGELOG entry #8498 shipped ("retiring dry chunks … for large pull requests") — the optimization exists exactly for the budgeted large-diff runs where it stopped firing, and the evidence destruction made the failure undiagnosable. Fixing both halves (observable retirement failures + preserved evidence) is the minimum that makes the mechanism trustworthy. Size: Not applicable — all 11 files live in Approach: The scope is larger than the issue's three asks (widen the separator class, name the failed certification bar, keep the record directory), but every increment traces to a leak shape the review rounds found and pinned — the line-spanning Risk: No high-risk paths matched (Stage 1e). The touched surface — the dry-receipt classifier and the reverse-audit scheduler — is fail-safe by construction: a classifier error costs an extra audit, never a skipped one. Moving on to code review. 🔍 中文说明接手轮次后的重新运行——本次审查 模板完整 ✓ 问题：已观测到的 bug，取证完整。#9206 报告了一次真实运行（审查 PR #9118，12 chunks）：4 个连续两轮干回执的 chunk 在第 3–5 轮始终未退役，任何 方向：对齐。这修复的是 CHANGELOG 中 #8498 引入的退役机制（"为大 PR 退役干 chunk……"）——该优化恰恰为预算型大 diff 运行而生，却在这类运行上失效，且证据销毁使失败无法诊断。两半都修（退役失败可观测 + 证据保留），才能让该机制可信。 规模：不适用——全部 11 个文件都在 方案：范围大于 issue 的三项诉求（放宽分隔符类、点名失败的认证条件、保留 record 目录），但每一处增量都能追溯到审查轮次发现并钉住的泄漏形状——跨行 风险：未命中高风险路径（Stage 1e）。所触表面——干回执分类器与反向审计调度器——构造上即失败安全：分类器出错只会多花一次审计，绝不会跳过审计。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal for #9206 was narrower: widen the separator class in the dry-receipt regex, return a failure reason from the classifier and print it on stderr, keep the record directory when a stop marker is present. The PR does all three, then goes further — and the further parts are earned, not speculative: the anchored, line-scoped receipt form (clause ends at the line, every whitespace element line-bound) closes a real leak in the old What I checked specifically:
No critical findings. The standing Suggestions from the review ledger (negation-vocabulary residue, strip fence-awareness, test-label hygiene) are collected in #9259 — none blocks this change. Test evidence — the PR's own CI on the reviewed commit, fetched via API (per policy this gate does not run PR code). No check on this head failed: 16 checks green, the rest conditional skips plus
The three skipped test jobs are pre-existing CI routing, not this PR's doing — the same checks read Sandboxed verification would settle the behavioural remainder: Real-scenario testing: N/A on this CI path — nothing user-visible in a TUI sense; the change lives in the review orchestrator's stderr diagnostics, brief text, and cleanup output. 中文说明代码审查读 diff 之前，我对 #9206 的独立方案更窄：放宽干回执正则的分隔符类、让分类器返回失败原因并打到 stderr、存在停止标记时保留 record 目录。PR 做到了这三点，并走得更远——而多出的部分是有据的，不是推测：锚定、行作用域的回执形式（从句止于行尾、所有空白元素行绑定）堵住了旧 具体核对：
无 Critical 发现。审查台账遗留的 Suggestion（否定词表残留、剥离的围栏感知、测试标签卫生）已汇总至 #9259——均不阻断本变更。 测试证据——被审查提交上 PR 自己的 CI，经 API 获取（按策略，本 gate 不运行 PR 代码）。该提交无任何失败检查：16 项绿色，其余为条件性跳过与读作 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean re-run on the round-8 head; the two residual reservations are non-blocking (the cross-platform test matrix skipped per the repo's standing routing, and the deferred Suggestions now living in #9259). Stepping back: my independent proposal was a ~250-line version of this fix, and the PR's extra production scope is not creep — each increment closes a leak shape the eight review rounds actually found and pinned (line-spanning match, fused layer label, hedge-in-clause, case-folded parrot), with the failure direction uniformly toward audit. The issue's three asks are all met: the separator widening retires the period / full-width-comma receipts #9206 reproduced, twice-audited chunks that fail certification now name the failed bar on stderr round by round, and a non-converged run's record directory survives cleanup. The code will read honestly in six months — the classifier's doc comments are dense, but that is this module's established style, and they carry the constraints (the polarity guard, the failure direction) a future editor must not break. Every change in the diff is load-bearing for the stated goal; the rest went to #9259 instead of bloating this PR. CI is green on the reviewed head with no 中文说明置信度：4/5——第 8 轮 head 上的干净复审；剩余两点保留均为非阻断（跨平台测试矩阵按仓库既有路由跳过；延期 Suggestion 已收在 #9259）。 退一步看：我的独立方案约 250 行，PR 多出的生产范围不是膨胀——每一处增量都关闭了八轮审查真实发现并钉住的泄漏形状（跨行匹配、融合 layer 标签、从句内 hedge、大小写折叠复读），失败方向一致倒向审计。issue 三项诉求全部满足：分隔符放宽使 #9206 复现的句号/全角逗号回执得以退役；两次审计却未通过认证的 chunk 现在逐轮在 stderr 点名失败条件；未收敛运行的 record 目录在清理后保留。代码六个月后依然可读——分类器注释密集，但这是该模块既有风格，承载未来修改者不可破坏的约束（极性守卫、失败方向）。diff 中每处改动都对所述目标不可或缺，其余进了 #9259 而非膨胀本 PR。 审查 head 上 CI 全绿且无 pending 的 — 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 /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-code-dev-bot
added
the
autofix/takeover
…wenLM#9213) The widened receipt separator admitted clauses that contradict the no-issues phrase ("…found, but I could not open the files") and let a quoted phrase open a clause out of its own negation, retiring chunks on their auditor's admission that nothing was checked. Judge polarity on every separator path and admit sentence-punctuation separators only when the phrase leads the return. Cleanup's retention keyed on the run-epoch-fenced marker reader, so a previous run's preserved evidence was swept by the next run — and clearBudgetStop unlinked any marker, including a previous run's that retention had just kept. Retention now reads the marker unfenced and also keeps record directories carrying files older than the plan's own capture (a killed run leaves no marker); convergence clears only its own run's marker. The per-chunk build path now prints its chunk's certification-failure diagnostics like the round builder does.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #9213Round 1 feedback: 2 Critical + 7 Suggestion findings. Eight findings are Every defect claim was reproduced against the pre-round code first: the Critical findingsR1-1 — widened separator admits self-contradicting receipts as
|
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.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…wenLM#9213) The per-chunk gate stamped the round on its first chunk build and gated the certification diagnostics behind that stamp, so chunks 2..N of a one-auditor-at-a-time round re-audited in the exact silence the note exists to end. Print the chunk's diagnostic on every build of the round; only the convergence and budget rulings stay gated on admission. The polarity guard enumerated contrast words over unbounded prose, and prose has no last hedge — though/Yet/unfortunately/只是 all retired chunks on clauses admitting nothing was checked, while a hedge in the phrase's filler never reached the clause-only test at all. Invert the burden: a clause carrying a negation or incapacity marker contradicts the phrase whatever its length, and a contrast word without one contradicts nothing — an innocuous "but I re-verified" retires again, and the guard's remaining gaps land on the audit side. The anchored receipt matcher now leads (the unanchored one truncated clauses at nested phrase occurrences), the judged text is re-trimmed after the budget-gap strip, and cleanup keeps a record directory whose plan a previous cleanup already swept and skips unstatable entries one by one instead of letting one veto the previous-run evidence.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #9213Round 3 addressed 9 of the 15 round-2 findings — both Criticals first, per Critical
Suggestions resolved
Deferred to the next round (replies posted on each thread)
Diff growthNet this round: source +109 / test +274 lines (all within the PR's VerificationCommands actually run this round (results):
中文说明审查轮次总结 — PR #9213第 3 轮处理了第 2 轮 15 项发现中的 9 项——按每轮批次上限（约 8 项）优先处理 Critical
已解决的建议
延迟到下一轮（已在各自线程回复）
差异增长本轮净增：源码 +109 / 测试 +274 行（全部位于本 PR 已有足迹内—— 验证本轮实际执行的命令（结果）：
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.
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: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…otes (QwenLM#9213) The polarity guard read only the match's own prefix and the clause, so a hedge BEFORE the phrase on the same line was invisible and retired the chunk (`I could not check everything, but no new issues — re-walked…`), and its marker list missed the incapacity/omission families the leak probes executed — unable/failed/skipped/unchecked/skimmed, zh bare-不 and 跳过. Scope the guard to the LINE the receipt matched on — the match plus any same-line text before the phrase, quoted spans exempted — and name the executed marker families. Prose on any other line neither certifies nor contradicts: scanning the whole return would refuse an honest receipt beside an innocent "not re-reporting it" paragraph. The substance floor now measures the phrase-STRIPPED clause (greedy with the phrase's filler), so an echoed phrase cannot lend the floor its length, and the comment's direction claim is corrected — a marker the list misses inside the line still fails toward RETIREMENT, stated rather than papered over. The parrot refusal compared case-sensitively, but the example clause starts lowercase only because it continues the model receipt mid-sentence; the widened sentence-punctuation separators let a parroting auditor open it as a NEW sentence and capitalize it. Compare in any casing — no honest clause contains the model clause verbatim in any casing. The three new informational stderr writes — both schedule catch NOTEs and the uncertified-chunks note — used the throwing writeStderrLine on the CONTINUING build path. A headless retry with stderr redirected or closed (the very QwenLM#9206 shape) made the write throw out of a catch with no outer guard, abandoning the round that must audit every chunk. Use the Safe writer, matching writeFindingsFile.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Address summary — PR #9213 review roundAll three actionable Critical findings are resolved in code. Each was Findings and decisions
Verification
中文说明处理摘要 — PR #9213 审查轮三个可执行的 Critical 发现均已在代码中解决。每一条都是先在未修改的代码树上用失败测试复现，再做最小修复，且新测试只在修复后通过。十条非 Critical 的 Suggestion 发现被本轮的"仅处理 Critical"刹车排除（本 PR 的测试 diff 已超出计数窗口预算），保持开放、留待人工跟进——本轮不为它们改代码、不动线程、不写回复。 发现与决定
验证
Deferred non-Critical feedbackCritical-only mode is active: the PR's diff grew src 203 / test 613 net lines beyond this counting window's baseline (budgets: 400/400). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback continues to flow unaffected during a growth-only engagement (the per-author batch budget applies only after 5 change-producing rounds). ( 中文说明已进入仅处理 Critical 的模式：本计数窗口内 diff 净增长已达 源码 203 / 测试 613 行（预算 400/400）。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。纯增长触发期间维护者反馈照常流动（按作者的批次预算仅在完成 5 个产生改动的轮次后生效）。（评论 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.
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: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — no check was cut short..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — no check was cut short.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…ipt form (QwenLM#9213) The polarity guard enumerated hedges over unbounded auditor prose, and every hedge the list missed failed toward RETIREMENT — the direction the module header declares impossible. Round 5 executes the structural fix the round-4 Critical named: one machine-parseable receipt form, and everything else read as unknown. The form: the phrase LEADS the return (anchor; the lead CONTRAST list is deleted), the receipt line stands ALONE (Budget gap: and Layer walked: lines stripped with audit-layers' own matcher; any other prose before or after reads unknown, named for the side it fell at), and the clause NAMES THE WALK (a verb from the form's vocabulary, or a named object — misses fail toward audit, the opposite direction of a marker miss). Within the form the marker test is repaired, not extended: the strip uses the phrase core without the greedy tail that swallowed its own listed markers, and the quoted-span exemption that blanked a self-admission is deleted. Zero words added to the vocabulary. Also closes R4-1: the clause capture ran to the END of the return, so prose after the receipt line contradicted the phrase while identical prose before it passed. The capture stops at the line; both sides now read unknown, symmetrically named. The reverse-audit brief mandates the form so compliant dry returns stay one line and keep retiring.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #9213 (round 5, Critical-only)Critical-only mode was active for this round (the counting window measured Feedback points and decisions[Critical] R2-2 — the polarity guard is an enumeration over unbounded prose (rc:3790561111) — FIXEDThe finding executed four NEW entrance families at the round-4 commit that The form, now mandated by the reverse-audit brief and enforced by the
Subtractive accounting: CONTRAST_RE, the quoted-span exemption, the [Critical] R4-1 — polarity domain contradicts the line-scope claim (rc:3790561114) — FIXEDThe clause capture ran
One honest note: the finding's flip witness ( Deferred Suggestions (R4-3, R4-4, R4-5, R4-6, R4-7, R4-8, R4-9, R4-10, R4-11, R3-6) — NOT TOUCHEDCritical-only mode excludes them from this round's work; they remain open Conflict resolution (
|
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: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 3)": none — wait, per the brief I must not write that line when nothing was cut short; withdrawing it. Nothing was cut short: the full chunk was read un-truncated, t….
Test Plan (not a blocker): 323 tests green — this review observed 20319 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 3)"：none — wait, per the brief I must not write that line when nothing was cut short; withdrawing it. Nothing was cut short: the full chunk was read un-truncated, t…。
Test Plan（非阻断）：323 tests green — this review observed 20319 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Blocked: PR #9213 needs a maintainer decision — the review loop is not convergingThe blockerThe workflow's deterministic growth brake has been over budget across rounds and The decision neededHow to land PR #9213 without continuing unbounded review-driven growth. Options
Run log: https://github.com/QwenLM/qwen-code/actions/runs/31926744017 🧠 Handled by Qwen Code · model/模型 |
|
Round 8 (takeover): Critical-only fixes, per the post-round-5 landing policy. Fixed in
Both fixes are pinned by new tests in Verified already-addressed at HEAD and resolved: R2-1 (per-chunk diagnostics de-gated from the round stamp), the Deferred: all open Suggestions are collected in #9259 so nothing is silently dropped. 中文说明第 8 轮（接手）：按超过 5 轮后的落地策略，只修 Critical。
|
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. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only.
Test Plan (not a blocker): 323 tests green — this review observed 20367 passed.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; the unit suite ran on Linux only。
Test Plan（非阻断）：323 tests green — this review observed 20367 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@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: 3027 passed · 0 failed · 3027 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：3027 通过 · 0 失败 · 3027 总计 Verification reportPR #9213 deep verificationVerdict: 中文摘要
Central claim + A/BCentral claim: sentence-punctuation-separated dry receipts (EN period, full-width comma — the exact shapes #9206 reported) classify
head 14/14, base 13/13 (every base cell behaved as the expected-broken prediction; the base wiring control proves the base mechanism itself works). Secondary claim 1 — observability & evidence retention.
head 12/12, base 8/8. The per-chunk/per-round diagnostic path is additionally proven by the bare-phrase cell above (NOTE names both chunks and the bar per round) and by Secondary claim 2 — the widened form admits nothing it should not.
Reviewer Test Plan, per step: (1) regression suites — executed, see Gates (453/453 on the named files; the plan's "323" is stale, see Corrections). (2) invariants — canonical dash retires from round 3 ✓ (A/B + probes); bare Corrections
FindingsF1 (Suggestion, pre-existing, non-blocking): Measured minimal fix (apply in a scratch copy, not required for this PR)Pre-filter on the characters the regexes require — a path match is impossible without function namesAnObject(clause: string): boolean {
if (!clause.includes('/') && !clause.includes('`')) return false;
return ( /* the three existing regexes, unchanged */ );
}Measured in F2 (Suggestion, pre-existing, non-blocking): the anchored receipt matcher is quadratic on a long space run between phrase and clause. Not covered
MethodologyEnvironment: 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. ✅
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: PR #9213 — autofix round stopped: needs a maintainer's decision (PR is not converging)The workflow's deterministic growth brake has been over budget across rounds and the diff is still not shrinking: this counting window measures source 319 / test 1022 net lines vs budgets 400/400, with 3 prior rounds already over budget. The overage is entirely in test lines. The findings themselves are driving the growth, so the loop cannot converge by patching — this round applied NO code changes and hands the decision to a maintainer. The decisionHow to close out PR #9213 (the #9206 fix: dry receipts retiring chunks that were never examined):
Current state (verified at HEAD
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — approving per maintainer request. Note: web-shell E2E smoke and review-pr checks were still in progress at approval time (18c6b65).
|
Released in v0.21.13. |
…n-converged evidence (QwenLM#9213) * fix(review): name reverse-audit certification failures, keep non-converged evidence (QwenLM#9206) A round-5 reverse audit (PR QwenLM#9118, 12 chunks) never retired a chunk: four territories returned substantive dry receipts in BOTH rounds 1 and 2, yet rounds 3-5 rebuilt all 12 auditors, no retirement note appeared, and nothing anywhere said which certification condition refused the receipts. Step 9 cleanup then deleted the prompt-record directory of the non-converged run, so the failure could never be diagnosed. Two root causes, reproduced end to end (issue-9206-repro.test.ts): - The dry-receipt separator class admitted dashes and colons only, but honest receipts arrive separated by sentence punctuation too — "No new issues were found. Re-walked …" and "未发现新问题，重新走查了…" both reproduce the never-retire loop byte for byte. The clause after the separator is the part that proves the walk; the separator only has to show it exists. Widen the class to period, comma and semicolon in either width. The substance floor, the tool-call bars and the territory bar are unchanged — the bare stock sentence still reads unknown, and a yield still outranks any receipt. - Every certification refusal landed in the same silent unknown, and the schedule's catch swallowed every exception without a word. classifyReturn now reports the first bar that fell; the schedule carries one diagnostic line per twice-audited chunk that neither retired nor yielded, and the builder prints them on stderr (stdout stays the deliverable). The catch names itself: a round whose transcripts cannot be read audits every chunk AND says so. And the evidence half: cleanup swept the record directory of a non-converged run unconditionally — the round-cap marker that proves the non-convergence sat inside the very directory deleted. A record directory holding a same-run stop marker is now kept, with a note naming it; a converged run clears its marker, so its history sweeps as before. * fix(review): refuse hedged receipts, keep cross-run audit evidence (QwenLM#9213) The widened receipt separator admitted clauses that contradict the no-issues phrase ("…found, but I could not open the files") and let a quoted phrase open a clause out of its own negation, retiring chunks on their auditor's admission that nothing was checked. Judge polarity on every separator path and admit sentence-punctuation separators only when the phrase leads the return. Cleanup's retention keyed on the run-epoch-fenced marker reader, so a previous run's preserved evidence was swept by the next run — and clearBudgetStop unlinked any marker, including a previous run's that retention had just kept. Retention now reads the marker unfenced and also keeps record directories carrying files older than the plan's own capture (a killed run leaves no marker); convergence clears only its own run's marker. The per-chunk build path now prints its chunk's certification-failure diagnostics like the round builder does. * fix(review): diagnose every chunk build, invert the polarity burden (QwenLM#9213) The per-chunk gate stamped the round on its first chunk build and gated the certification diagnostics behind that stamp, so chunks 2..N of a one-auditor-at-a-time round re-audited in the exact silence the note exists to end. Print the chunk's diagnostic on every build of the round; only the convergence and budget rulings stay gated on admission. The polarity guard enumerated contrast words over unbounded prose, and prose has no last hedge — though/Yet/unfortunately/只是 all retired chunks on clauses admitting nothing was checked, while a hedge in the phrase's filler never reached the clause-only test at all. Invert the burden: a clause carrying a negation or incapacity marker contradicts the phrase whatever its length, and a contrast word without one contradicts nothing — an innocuous "but I re-verified" retires again, and the guard's remaining gaps land on the audit side. The anchored receipt matcher now leads (the unanchored one truncated clauses at nested phrase occurrences), the judged text is re-trimmed after the budget-gap strip, and cleanup keeps a record directory whose plan a previous cleanup already swept and skips unstatable entries one by one instead of letting one veto the previous-run evidence. * fix(review): line-scope the polarity guard, case-blind parrot, safe notes (QwenLM#9213) The polarity guard read only the match's own prefix and the clause, so a hedge BEFORE the phrase on the same line was invisible and retired the chunk (`I could not check everything, but no new issues — re-walked…`), and its marker list missed the incapacity/omission families the leak probes executed — unable/failed/skipped/unchecked/skimmed, zh bare-不 and 跳过. Scope the guard to the LINE the receipt matched on — the match plus any same-line text before the phrase, quoted spans exempted — and name the executed marker families. Prose on any other line neither certifies nor contradicts: scanning the whole return would refuse an honest receipt beside an innocent "not re-reporting it" paragraph. The substance floor now measures the phrase-STRIPPED clause (greedy with the phrase's filler), so an echoed phrase cannot lend the floor its length, and the comment's direction claim is corrected — a marker the list misses inside the line still fails toward RETIREMENT, stated rather than papered over. The parrot refusal compared case-sensitively, but the example clause starts lowercase only because it continues the model receipt mid-sentence; the widened sentence-punctuation separators let a parroting auditor open it as a NEW sentence and capitalize it. Compare in any casing — no honest clause contains the model clause verbatim in any casing. The three new informational stderr writes — both schedule catch NOTEs and the uncertified-chunks note — used the throwing writeStderrLine on the CONTINUING build path. A headless retry with stderr redirected or closed (the very QwenLM#9206 shape) made the write throw out of a catch with no outer guard, abandoning the round that must audit every chunk. Use the Safe writer, matching writeFindingsFile. * fix(review): close the polarity guard's unbounded class with one receipt form (QwenLM#9213) The polarity guard enumerated hedges over unbounded auditor prose, and every hedge the list missed failed toward RETIREMENT — the direction the module header declares impossible. Round 5 executes the structural fix the round-4 Critical named: one machine-parseable receipt form, and everything else read as unknown. The form: the phrase LEADS the return (anchor; the lead CONTRAST list is deleted), the receipt line stands ALONE (Budget gap: and Layer walked: lines stripped with audit-layers' own matcher; any other prose before or after reads unknown, named for the side it fell at), and the clause NAMES THE WALK (a verb from the form's vocabulary, or a named object — misses fail toward audit, the opposite direction of a marker miss). Within the form the marker test is repaired, not extended: the strip uses the phrase core without the greedy tail that swallowed its own listed markers, and the quoted-span exemption that blanked a self-admission is deleted. Zero words added to the vocabulary. Also closes R4-1: the clause capture ran to the END of the return, so prose after the receipt line contradicted the phrase while identical prose before it passed. The capture stops at the line; both sides now read unknown, symmetrically named. The reverse-audit brief mandates the form so compliant dry returns stay one line and keep retiring. * fix(review): line-bind the dry-receipt matcher and cut fused layer labels (QwenLM#9213) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
…t breach (QwenLM#9262) * feat(autofix): audit the approach instead of stopping on growth-budget breach A growth-budget breach no longer escalates to a maintainer handoff that stops the takeover. The breach now makes the round a growth-audit round: the agent audits the PR's approach on two axes — KISS (name a simpler alternative or prove each piece load-bearing) and minimal change (every hunk traces to the problem, an accepted finding, or a failing check) — and records a machine-readable verdict that the verification gate requires. sound re-arms the counting window at the current size and the loop keeps solving; drift simplifies first, then continues; conflict is the only growth path to a human, parked idempotently until a trusted human responds. The old divergence ladder (over budget for N rounds and not shrinking → stop) terminated takeovers whose remaining work could still fit: the growth it punished was protocol-mandated pinned tests (QwenLM#9213 stalled at round 5 with two small Criticals left). A size signal now triggers a judgment, never a stop. Design: docs/design/autofix-growth-audit.md * fix(autofix): update the artifact-list pin for the growth-audit.json upload entry * fix(autofix): surface conflict verdicts past the failure.md exits and strip verdict forgery channels (QwenLM#9262) * fix(autofix): harden the growth-audit verdict pipeline and park wake set (QwenLM#9262) * fix(autofix): close the verdict-pipeline forgeries and loop-generated wake entrances (QwenLM#9262) * fix(ci): drop the retired divergence rationale records (af-046/af-047) from qwen-autofix.md --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Fourth round on #9678, and one item is a gap the third round's own fix left open. `typeNote` was hoisted out of the worktree gate but still printed only by `runRoster`. Step 4's verify shards and Step 5's audit rounds are built by the other two emission paths, and those are both the most numerous agents a high-effort review launches and the ones furthest from SKILL.md's statement of the rule — an omitted `subagent_type` there resolves to `general-purpose` at full cost, silently. It is now a module constant carried by all three. The single-block path carries it on stderr, not stdout. Its stdout IS the block the orchestrator pastes verbatim, and the delivery check compares that against the record, so appending to stdout made every launch differ from its record — five existing tests caught it, which is the guard working. stderr is the channel this command already uses for operator-facing lines, and via the ...Safe writer, since #9213 pins that a broken stderr must not refuse a build. The SKILL.md paragraph is cut from 816 characters to 457. The skill's own convention keeps narrative in DESIGN.md because SKILL.md is loaded on every orchestrator turn — quoting four numbers already verbatim in DESIGN.md was the opposite of this change's thesis. It also named the tool set as "read, grep, glob, shell, write, edit", four labels matching no registered name, while the next sentence asked the orchestrator to judge what falls outside that set; it now names the registered tools and a test pins the prose against the registry. The `subagent_type` assertion is a set, not an ordered array. Pinning count and order froze the document's shape, so restating the rule at Steps 4 and 5 — strictly more correct — would have turned it red.
* perf(review): give review agents their own subagent type
Review dimension agents were launched as `general-purpose`, which is the
only builtin subagent that declares no `tools` list. That sends it down
`AgentCore.prepareTools`' inherit-everything branch —
`getFunctionDeclarations({ includeDeferred: true })` — so every agent was
handed all 51 tool schemas, deferred ones included, and re-declared them
on every turn.
Measured with a recording endpoint on a 6-file / 115-line diff, driving
one real dimension agent through its four turns: 21,178 prompt tokens of
tool declarations per turn, of which the 35 `computer_use__*` schemas
alone were 11,011. A review names 13-14 such agents.
`review-agent` declares the six tools a dimension actually uses, which
takes the `getFunctionDeclarationsFiltered` branch that `Explore` and
`statusline-setup` already use. Same fixture, same launch prompt,
nothing else changed: 3,447 tokens per turn, and one agent's delivered
prompt fell from 139,013 to 55,733 (-59.9%) — about 1.08M tokens across
one roster.
The alternative — applying the deferral that trims the orchestrator to
subagents too — was measured at 84,537 and rejected: deferral does not
go below the ~14 core tools, and `revealDeferredTool` writes to the
registry the parent session shares, so one subagent's discovery would
rewrite the orchestrator's declarations and void its prompt-cache prefix.
* fix(review): correct the review-agent docs and pin its system prompt
Addresses the review on QwenLM#9678.
The `REVIEW_BUILTIN_SUBAGENT_TYPE` docstring described a fallback that
does not exist. `AgentTool.execute` substitutes the default only when
`subagent_type` is omitted, and `loadSubagent` ends at `getBuiltinAgent`
with no default, so an unknown non-empty type fails loudly with
`Subagent "<name>" not found` rather than silently reverting. It also
claimed the literal is the only input to the branch choice; resolution
runs session > project > user > extension > builtin and builtin names
are not reserved, so a user-authored `review-agent` shadows this entry —
deliberate, since that is how any builtin is customised, but worth
documenting where the cost is explained. The same wrong mechanism was
repeated in a SKILL.test.ts comment.
The system prompt had no content assertions: blanking it left every
test green while every dimension agent would have launched with no
instructions. Its contract lines are now pinned the way the sibling
builtins pin theirs.
The stale `coverage.ts` comment ("`agentName` is `general-purpose` for
all of them") is updated — it is now the launched type, so it is not a
value to match on.
Web Shell has per-type display labels for every other builtin; adds the
en/zh pair so review agents do not render as a raw kebab-case id.
DESIGN.md now carries the per-turn record the totals decompose from, and
states the second-order share as 12,476 of 83,280 (15%) with its
arithmetic: the skills catalogue lives in the first user message, which
is re-sent every turn, so its 3,119-token saving is charged four times.
The figure was right; the prose put a per-turn number next to a
four-turn ratio and invited the reading that it was not.
* fix(review): make the review-agent prompt role-neutral and its tests bite
Second round on QwenLM#9678.
The systemPrompt was written for a diff-reading dimension, but the same
type now serves every role the review launches, and it is
`systemInstruction` — it outranks the brief that arrives as a user turn.
Agent 7 reads no diff at all (`readsDiff: false`), `verify` rules on a
findings file, and `reverse-audit` exists to look outside what the first
pass covered, so a frame naming "your diff ranges" and bounding scope to
them contradicted three required roles. A blanket "silence is better
than noise" was worse: the finder briefs carry RECALL, whose whole point
is that a withheld half-believed candidate is unrecoverable, and the
verifier brief withholds RECALL deliberately — a confidence bar from
above breaks both halves. The prompt is now role-neutral and sends the
agent to its brief; the two restraint lines `general-purpose` carried
about a shared tree are restored. Its "absolute paths everywhere" note
also contradicted SKILL.md, which tells worktree-mode agents NOT to
prefix paths for `read_file`/`grep_search`; it now says what it means
about cwd and shell.
The `subagent_type` reminder sat inside the worktree-only `paramNote`,
so the three modes with no worktree were told nothing — and an omitted
type resolves to `general-purpose`, which is the entire cost this change
removes. It is now unconditional, with the non-worktree branch covered.
Three assertions had no teeth: `getBuiltinAgent` returns `null` and
`toBeDefined()` accepts it, `?.tools ?? []` satisfies every
`not.toContain` trivially, and `tools: ['*']` takes the
inherit-everything branch that the "declares a list" filter let through.
A renamed entry failed one test before and fails three now. The SKILL.md
guard was negative-only, so a reworded "Each is a general-purpose
subagent" passed; the set of `subagent_type` literals is now asserted
positively.
The Agent tool splices every type's description into its declaration in
every request of every session, so the description is one clause.
Documented rather than fixed: `agent`, `web_fetch` and MCP tools are
real losses against the inherited surface, not free savings; and a
user-authored `.qwen/agents/review-agent.md` shadows this builtin, which
also traps the file behind `deleteSubagent`'s builtin-name check.
Re-measured end to end on the rebuilt bundle: 55,669 delivered against
139,013 (-60.0%).
* fix(review): scope the cwd rule and correct the tool-search rationale
Third round on QwenLM#9678. One of these is a regression the previous round
introduced.
The rewritten prompt said "Never `cd`", which is broader than the rule
it was porting: SKILL.md forbids `cd` into the pinned working directory,
not everywhere. The Step 4 verifier is sent to its own scratch tree and
told to work there, and that tree is a SIBLING of the review worktree
(`<worktree>-scratch-<label>`), so `run_shell_command(directory:)` fails
the workspace check and `cd` is the only remaining route. A blanket ban
left the verifier running probes in the shared worktree — the QwenLM#9207
contamination the scratch tree exists to prevent — or demoting them. The
rule is scoped now, and a test pins the scoping in both directions.
The stated reason for excluding TOOL_SEARCH was wrong, and it was
load-bearing: it was also the argument for rejecting the deferral
alternative in DESIGN.md and in the PR description. A subagent does not
share the parent's registry — `rebuildToolRegistryOnOverride` builds one
per launch and rebinds `getToolRegistry` on the override config — so a
reveal cannot reach the orchestrator's declarations or its cache prefix.
The exclusion still stands on a closed list and on the schema costing
357 tokens/turn, more than two of the tools kept. The DESIGN.md
rejection now rests on the measurement and the blast radius, and records
the argument that did not hold.
Documented, not fixed, both pre-existing and both first surfaced by a
type with a restricted list: `coreToolScheduler`'s skill-activation
reminder gates on the registry rather than on the declared list, so it
is unconditionally true and announces a tool the agent does not have;
and `buildMcpServerInstructionsReminder` has no gate at all beside two
reminders that do. Each changes what every subagent receives, so each
belongs in its own change. The capability the second-order saving buys —
review parts can no longer invoke a project skill — is now named beside
the number rather than left implied.
Re-measured on the rebuilt bundle: 55,789 delivered against 139,013
(-59.9%).
* fix(review): state the launch type on every emission path
Fourth round on QwenLM#9678, and one item is a gap the third round's own fix
left open.
`typeNote` was hoisted out of the worktree gate but still printed only
by `runRoster`. Step 4's verify shards and Step 5's audit rounds are
built by the other two emission paths, and those are both the most
numerous agents a high-effort review launches and the ones furthest from
SKILL.md's statement of the rule — an omitted `subagent_type` there
resolves to `general-purpose` at full cost, silently. It is now a module
constant carried by all three.
The single-block path carries it on stderr, not stdout. Its stdout IS
the block the orchestrator pastes verbatim, and the delivery check
compares that against the record, so appending to stdout made every
launch differ from its record — five existing tests caught it, which is
the guard working. stderr is the channel this command already uses for
operator-facing lines, and via the ...Safe writer, since QwenLM#9213 pins that
a broken stderr must not refuse a build.
The SKILL.md paragraph is cut from 816 characters to 457. The skill's
own convention keeps narrative in DESIGN.md because SKILL.md is loaded
on every orchestrator turn — quoting four numbers already verbatim in
DESIGN.md was the opposite of this change's thesis. It also named the
tool set as "read, grep, glob, shell, write, edit", four labels matching
no registered name, while the next sentence asked the orchestrator to
judge what falls outside that set; it now names the registered tools and
a test pins the prose against the registry.
The `subagent_type` assertion is a set, not an ordered array. Pinning
count and order froze the document's shape, so restating the rule at
Steps 4 and 5 — strictly more correct — would have turned it red.
* test(review): give the review-agent pins teeth, and log the monitor loss
Fifth round on QwenLM#9678.
Four assertions did not hold what they claimed. The tools array was
pinned with `toEqual`, so alphabetising a list whose order carries no
semantics turned the suite red while changing nothing; it is a set plus
a length now. The SKILL.md tool-name pin ran one way only — registry
names must appear in the body — so shrinking the registry left the skill
advertising a capability the agent no longer has, green; it is now set
equality against the sentence itself. The stderr assertion joined every
accumulated mock call in the file and passed whether or not its own
invocation emitted anything, because the enclosing beforeEach clears
only stdout. And the third emission path — the reverse-audit round
header — was asserted nowhere, so dropping its note shipped green while
every Step 5 auditor launched untyped. Each is mutation-verified against
the case that used to pass.
`run_in_background: false` is now pinned too. Dropping it defaulted
every review agent to a background launch, whose findings never return
inline — the review stalls in Step 4 with nothing to aggregate, and no
test was red.
MONITOR joins the deliberately-absent ledger. It is the one removal the
agent is actively pointed at: `shell.ts` answers a blocked foreground
sleep with "use the Monitor tool", and it is not in the subagent
exclusion set, so a `general-purpose` review agent had it. Recorded with
the measurement that qualifies it — across both arms of a real A/B
review, neither the guidance nor the tool ever fired.
* docs(review): record how to re-run the tool-surface measurement
Sixth round on QwenLM#9678. The figures are load-bearing and quoted in five
places, but a reader had no way to re-derive them — only to re-check the
arithmetic against a table.
No script is committed, because none is needed: both halves of the
measurement run on commands this repository already ships. DESIGN.md now
carries the two-arm build, the per-turn capture, the run-level ledger
read-back, and the environment facts that are part of the result — the
51-tool arm is the product default (35 of them `computer_use__*`, on by
default), not a local quirk.
Two traps are named because both are easy to hit and silent. A second
run of the same PR is an incremental re-review, so an arm sharing a
working tree with the other reuses its findings and the comparison
measures nothing; and `review mock-provider` truncates its record at
8 KB, which is smaller than one tool block, so its log cannot be the
source for per-turn token counts.
Deliberately not a `###` heading: SKILL.test.ts requires every incident
heading under that section to carry a SKILL.md pointer, and a procedure
note is not an incident the orchestrator needs pointed at.
* fix(review): drop the stderr launch note, and stop asserting the brief is a file
Seventh round on QwenLM#9678. Both items are regressions this PR introduced.
The launch note on the single-block path was moved to stderr last round
because stdout is the block the orchestrator pastes verbatim. stderr is
not a second channel: `ShellExecutionService` returns
`stdout + separator + stderr` as one string and `ShellToolInvocation`
hands that back, so the note arrived inside the very text the caller is
told to copy. It failed the same recorded-prompt equality as stdout
would, except invisibly — the five tests that catch the stdout version
see nothing — and removing it by hand is the edit the delivery gate
forbids, so those launches could enter drift/relaunch repair. The note
now has no channel on that path, and the code says why so it is not
re-added. The rule still reaches those launches: SKILL.md states it for
every `agent` call, and the two paths whose note sits OUTSIDE the ─────
blocks — the roster header and the audit-round header — still carry it.
The role-neutral prompt asserted that the assignment is a brief on disk.
Agent 8 is the reachable exception: `buildWholeDiffBlock` deliberately
writes no brief and SKILL.md appends its domain brief inline, so a
specialist launched that way was told from `systemInstruction` — which
outranks its own launch prompt — to read a file that does not exist. It
is optional and outside `requiredAgents`, so a generic diff walk would
have passed coverage in its place. The instruction is conditional now:
read the brief when the launch names one, otherwise the inline
assignment is the brief.
Both are mutation-verified against the shape that used to pass.
* docs(review): re-measure after the Agent 8 prompt fix
The conditional-brief wording is 27 tokens/turn longer than the sentence
it replaced, so the delivered figure moved: 55,789 to 55,897, and the
gap it is measured against from 83,224 to 83,116. Every citation in the
tree is realigned, including the per-turn table's system-prompt column.
Correctness bought the difference, and it is worth it — the previous
wording sent Agent 8 after a brief that does not exist. Recorded because
a figure quoted in five places is only useful while it matches what the
built bundle actually delivers, and this one is re-measured from that
bundle rather than adjusted on paper.
* docs(review): revert the test file too in the A/B recipe
The re-run recipe's baseline-arm revert list named the review sources but
not `agent-prompt.test.ts`, which imports `REVIEW_BUILTIN_SUBAGENT_TYPE`.
Reverting core removes that export, so `build:packages` fails with
TS2724 — and because the recipe chains with `&&`, the bundle step is
skipped and whatever `dist/` was there before is copied as the baseline
arm.
Reproduced rather than reasoned about: the recipe as written exits 1 on
TS2724, and adding the test file to the list exits 0. That failure is
also what happened when this measurement was first taken; the bundle was
re-run by hand afterwards, which is why the arm was still correct and
the broken recipe went unnoticed.
The note says the general rule, since the specific file will drift:
reverting the sources is not enough, anything that references them has
to go back too.
* test(review): pin the agent-type labels, and fix a comment the delta outran
Both from the round-3 sandboxed verification.
The web-shell `agentType.review-agent` keys were consumed at a live read
site and pinned by nothing. `localizeAgentTypeName` falls back to the raw
id, so a missing key is invisible — the badge renders kebab-case beside
siblings showing "Explore" and "Status Line Setup" and no test fails. The
badge is also newly visible for review agents: both surfaces elide the
type prefix only for the default type, so giving the review its own type
turned a never-rendered label into one shown on every row.
The test goes through `getTranslator`, the lookup the component uses,
because that is what makes the nastiest miss catchable: it falls back to
the English table, so a key added to `en` and forgotten in `zh-CN` still
returns something other than the raw id. Comparing the two locales'
output is what catches it — mutation-verified both ways (zh key removed:
1 red; both removed: 3 red).
The other item is a comment this PR's own previous round outran: the test
that now asserts the single-block path emits the note on NO channel still
opened by saying it emits on stderr, and its name was a beat behind too.
Behaviour and assertions were correct; the prose contradicted them, which
is exactly what would mislead the next reader about what the test pins.
4 participants
What this PR does
Makes the reverse-audit loop's retirement failures observable, and fixes the receipt shape that broke retirement on a real run. Three behaviour changes: (1) the dry-receipt parser now accepts sentence punctuation — period, comma, semicolon, in either width — as the separator between the all-clear phrase and the clause that proves the walk; dashes and colons were the only separators before. (2) A chunk audited twice that neither retired nor proved hot now gets a stderr diagnostic naming the certification condition that failed, round by round; and the schedule's fallback names itself when it cannot read the audit history instead of silently auditing everything. (3) Post-review cleanup keeps the prompt-record directory of a run whose loop stopped without converging — the stop marker proves the non-convergence — instead of deleting the only evidence that could diagnose it. A converged run clears its marker, so its history sweeps as before.
Why it's needed
Fixes the run reported in #9206: four chunks returned substantive dry receipts in both rounds 1 and 2, yet rounds 3, 4 and 5 rebuilt auditors for all 12 chunks, no
retirement:note appeared, nothing said which condition refused the receipts, and Step 9 cleanup then deleted the record directory — forensics impossible. Reproduced end to end: receipts separated by a period (No new issues were found. Re-walked …) or a full-width comma (未发现新问题，重新走查了…) failed the dry classification, and the refusal was indistinguishable from "nothing was certifiable" — the loop ran to its round cap paying full auditor cost on territories it had already certified.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/issue-9206-repro.test.ts src/commands/review/lib/retirement.test.ts src/commands/review/cleanup.test.ts src/commands/review/agent-prompt.test.ts— 323 tests green. The repro file encodes the issue's expectation as a disjunction (a twice-dry chunk must produce EITHER a retirement note OR a certification diagnostic); 4 of its 5 tests failed before this change, all 5 pass now.No issues found.still does NOT retire (the clause substance floor rejects the empty clause even through the widened separator class); a return that files a finding still outranks any receipt; cleanup of a run with no stop marker still deletes everything.Evidence (Before & After)
Before, round 3 with twice-dry chunks whose receipts used a period/full-width comma:
4 auditors required this round — one per chunk.and nothing else — byte-identical across rounds 3–5; cleanup printedRemoved temp file: …-fetch-prompts, destroying the certification history.After: rounds 3 and 5 print
2 auditors required this roundwithretirement: chunk N — retired: dry in rounds 1 and 2, next cold check round 4(round 4 is the designed alternating cold check, which re-retires); uncertified receipts produce a stderr NOTE naming the failed bar per chunk and round (e.g.chunk 1 — round 1: receipt clause not substantive; round 2: receipt clause not substantive) plus a pointer to the record directory; a cap-hit run's cleanup printsKept …-fetch-prompts: this review's reverse audit stopped without converging (stop marker on disk) — …and leaves the record directory intact.Tested on
Environment (optional)
Unit tests via vitest; E2E verification drove the bundled CLI (
node dist/cli.js review agent-prompt/cleanup) with synthetic harness-shaped transcripts — no model calls involved.Risk & Scope
stale-bundle.test.ts, a SKILL.md quote-parity fixture) also fails on unmodified HEAD and involves none of the changed files.Linked Issues
Fixes #9206
中文说明
本 PR 做了什么
让反向审计循环的退役（retirement）失败可观测，并修复了导致真实运行中退役失效的 receipt 形状。三个行为变更：(1) dry-receipt 解析器现在接受句子标点——句号、逗号、分号（半角与全角均可）——作为"无问题"短语与证明走查过程的从句之间的分隔符；此前只接受破折号与冒号。(2) 一个被审计两次、既未退役也未证明有问题的 chunk，现在会在 stderr 输出诊断，逐轮指出是哪个认证条件失败；当调度器无法读取审计历史时，其回退路径也会自报原因，而不是静默地审计一切。(3) 评审结束后的清理会保留"循环未收敛即停止"的运行的 prompt-record 目录（停止标记即未收敛的证明），不再删除唯一可用于诊断的证据。已收敛的运行会清除自己的标记，其历史记录照旧被清理。
为什么需要
修复 #9206 报告的运行：四个 chunk 在第 1、2 轮都返回了实质性的 dry receipt，但第 3、4、5 轮仍为全部 12 个 chunk 重建审计 agent，没有出现任何
retirement:注解，也没有任何输出说明是哪个条件拒绝了这些 receipt，Step 9 清理随后删除了 record 目录——事后无法取证。已端到端复现：以句号分隔（No new issues were found. Re-walked …）或全角逗号分隔（未发现新问题，重新走查了…）的 receipt 无法通过 dry 分类，而这种拒绝与"没有任何可认证的 chunk"完全无法区分——循环一路跑到轮数上限，在早已认证完成的 territory 上支付全部审计成本。审阅者测试计划
如何验证
cd packages/cli && npx vitest run src/commands/review/issue-9206-repro.test.ts src/commands/review/lib/retirement.test.ts src/commands/review/cleanup.test.ts src/commands/review/agent-prompt.test.ts——323 个测试全绿。repro 文件把 issue 的期望编码为析取式（两次 dry 的 chunk 必须产生退役注解或认证诊断之一）；改动前 5 个测试中 4 个失败，现在 5 个全部通过。No issues found.仍然不会退役（从句实质性下限拒绝空从句，即使分隔符类已放宽）；提交 finding 的返回仍然压过任何 receipt；没有停止标记的运行的清理仍然删除一切。证据（前后对比）
改动前：对 receipt 使用句号/全角逗号的两次 dry chunk，第 3 轮输出
4 auditors required this round — one per chunk.，再无其他——第 3–5 轮逐字节相同；清理输出Removed temp file: …-fetch-prompts，销毁认证历史。改动后：第 3、5 轮输出
2 auditors required this round与retirement: chunk N — retired: dry in rounds 1 and 2, next cold check round 4（第 4 轮是设计中的交替冷检查，冷检查后重新退役）；未通过认证的 receipt 会在 stderr 输出 NOTE，逐 chunk、逐轮指出失败的门槛（如chunk 1 — round 1: receipt clause not substantive; round 2: receipt clause not substantive），并附 record 目录路径；触发轮数上限的运行的清理输出Kept …-fetch-prompts: this review's reverse audit stopped without converging (stop marker on disk) — …，record 目录原样保留。测试环境
环境（可选）
单元测试通过 vitest；E2E 验证以合成的 harness 形状 transcript 驱动打包后的 CLI（
node dist/cli.js review agent-prompt/cleanup）——不涉及模型调用。风险与范围
stale-bundle.test.ts，SKILL.md 引文一致性 fixture）在未改动的 HEAD 上同样失败，与本 PR 改动的文件无关。关联 Issue
Fixes #9206