feat(review): add the persistently-critical convergence advisory (land-with-residual-risk) - #9526
Conversation
The severity floor converges a healthy loop — Suggestions stop posting and the volume falls to the Criticals, then to zero as those get fixed. But a loop whose Criticals never clear — the security-sensitive PR under adversarial review — posts Criticals every round forever: the floor engages, the Suggestions stop, and the volume flatlines at the Critical count instead of falling. Nothing before this said so. This adds the shape detector and its ONE recommendation: - lib/convergence.ts — `convergenceAssessment` computes one fact from the carried telemetry (Criticals stood in the previous round's work-list AND stand again this round, with the two-round posting window present and not shrinking) and, when it fires, returns the `land-with-residual-risk` recommendation. Pure data, never authority: no threshold, no blocking, no merge/close — every input degrades OPEN, so absence is fail-safe, never a suppressed finding. - compose-review wires it: `prevLedgerFacts` now recovers the previous work-list's Critical presence beside the round and volume; the assessment surfaces on three surfaces — a structured `convergence` field on the composed JSON, a rank-1 non-capping body disclosure, and a terminal CONVERGENCE line — each advisory-only and self-disclaiming, with a blank residual-risk inventory scaffold (attack surface · attacker-dependency · blast radius) for the maintainer's risk-acceptance decision. The exit the floor cannot provide: when the loop is provably stuck on Criticals, the tool names the maintainer's decision (merge, carrying the residual risk) instead of opening another round. Advisory only — it never blocks this review. Closes the convergence-exit gap in #9278; evidence and design in #9410.
|
Re-run gate against
Moving on to code review. 🔍 中文说明针对
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (re-run, head
|
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | success |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | success |
| Desktop Shell (ubuntu-22.04) | success |
| Desktop Shell (windows-2022) | success |
| Post Coverage Comment (ubuntu-latest, 22.x) | success |
| Classify PR | success |
| Dependency CVE audit | success |
| Secret scan (TruffleHog) | success |
| Test (macos-latest, Node 22.x) | skipped (merge-queue-only by CI policy) |
| Test (windows-latest, Node 22.x) | skipped (merge-queue-only by CI policy) |
| Integration Tests (CLI, No Sandbox) | skipped (merge-queue-only by CI policy) |
Sandboxed verification would settle the remaining claim: @qwen-code /verify — whether the new dedup and caveat tests actually pin the change (suite red with the dedup removed or the caveat unwired) is not observable from a green run alone; the live multi-round shape itself is out of reach of every lane and settles only by landing.
中文说明
代码审查(re-run,head ad56909)
无阻断项。上次之后只有一个提交——第 11 轮修复——我是对着 head 源码读的,不是只读提交说明:
- R11-2(gate Critical 复利)修在了正确的层。
withoutGateReposts匹配的是 gate 行的 LOCATOR——行首的`路径`:行号 CODE(取第一个破折号之前、反引号归一、携带的 id 经台账自己的LEDGER_ID_READBACK剥掉)——而不是整串,因此换了措辞的重发依然能去重(有测试钉住),另一行不同的发现不受影响(同样钉住)。保留的是 gate 的副本:它由绑定本 diff 哈希的报告再生成,而模型的副本是不受信散文、[lint]并不豁免其验证。去重发生在 body 组合器捕获modelBodyCriticals之前,被丢弃的重发不会继续拉动未验证阻断项封顶(封顶有测试钉住);标记工作清单经同一函数应用同一规则。body 组合器里的第二次 gate 调用已并入第一次、复用。 - R11-1(截断读取)是披露而非门禁——方向正确。
prevTruncated不是合取项:已在 head 上核对,合取式未变(同样六条守卫、同样 degrade-open)。变的是段落能声称什么——两条从前一轮清单「缺席」读出的结论(「没有 Suggestion,故 floor 在执法」「积压没有减少」)在字节预算截短清单时偏向触发,渲染出的建议现在双语、正文与终端行一致地如实说明(同一份渲染)。旧的一刀切「每个输入都 degrade open」注释换成了明说的例外。产物校验器把缺省读作「未披露」,字段出现之前写下的产物仍可保存(有测试钉住)。 - main 的合并(
6fa3e76)共存干净。 它把 churn/非收敛机制带进同一文件;PR 对 main 的差集仍恰为八个文件,两个特性不触碰彼此的谓词。合并解算还修掉了上次点名的四处陈旧梯度注释之一——Bi.trim接口 doc 现在是建议 rank 0 / 披露 rank 2 / 观察 rank 3。
两项非阻断:
- 迁移(relocated)通道不在去重范围内,而 body 的去重实际上覆盖了它——两侧不对称。
composeReviewBody的去重发生在迁移 Critical 加入bodyCriticals之后,因此 locator 撞上 gate 发现的迁移条目会从正文被丢弃,而标记一侧只对模型自有清单去重、保留迁移条目。碰撞条件很窄(被延迟的 lint Critical 在同一轮又被 gate 再生成),发现本身不丢(gate 副本会发布),建议一侧的偏差是 fail-safe(抬高的prevCriticals偏向抑制),且下一轮的去重会接住重发的成对条目——但这一轮里正文与标记对该条目说法不一,且去重前取的relocatedCount尾部算术可能把一条模型自有 Critical 误划进迁移桶、影响验证封顶。值得跟进:要么让标记侧的迁移通道也走同一语句,要么让迁移条目豁免 body 的去重——由 maintainer 定夺,不构成合入阻断。 - 上次点名的四处陈旧梯度注释还剩三处(合并只修掉了
Bi.trimdoc):save-artifact.ts 的PersistedVerdictdoc 仍写「convergence是 rank 0……而本字段是 rank 2」(与 head 相反——观察 rank 3、建议 rank 0);同文件校验器注释仍写「rank 2 在未审查披露之前让位」(建议现在是 rank 0);compose-review.test.ts 的 plan-gate 披露测试仍写其块「均带trim: 3」与goesWithRank3(现在是 trim 2)。仅注释、零运行时影响。
CI 测试证据(经 API 读取该提交自身的 CI)
该 head 上零失败、零未决的 pull_request 工作流运行:两个 PR CI 工作流(Qwen Code CI、Security Checks)均绿,PR 门禁 Test (ubuntu-latest, Node 22.x) 与安全检查同为 success。macOS/Windows 单测与 CLI 集成任务是 skipped 而非失败——ci.yml 把它们限定在 merge_group(已核对工作流定义)。沙盒验证可以闭合剩余主张:@qwen-code /verify——新增的去重与限定测试是否真的钉住改动(移除去重或解除限定后套件应红)无法仅从绿运行看出;跨轮实况形态超出所有通道能力,只有合入后才能落定。
— Qwen Code · qwen3.8-max
Reviewed at ad569092bee52a6c6d978637414b91a68baeaf29 · re-run with @qwen-code /triage
|
Confidence: 4/5 — clean review, no blockers; the only items are the one narrow relocated-channel asymmetry and the three stale rank comments, all named in Stage 2 and all fixups, not doubts. Stepping back: this commit is the loop's own round-11 findings, fixed reproduction-first, and reading it against the head source confirms both fixes land at the right layer. The compounding gets a locator-keyed dedup that keeps the provenance-tracked gate copy — regenerated from a report bound to the diff's hash, so the model's re-post is structurally redundant, and keeping it instead would have pulled the unverified-blocker cap on a linter-proven blocker. The truncated reading gets a disclosure rather than a gate, which is the right call: restoring the completeness gate would silence the advisory on exactly the deep-work-list rounds it exists for, and the two absence-derived readings are the two that lean toward firing, so the paragraph now says so — in both languages, body and terminal alike. My independent read of the problem matches the approach; I did not find a simpler path it missed. On the two mechanical reasons prior runs deferred rather than approved, and this one should say so plainly rather than silently change course:
The non-blocking items stay on the record for a follow-up, none gating this merge: the relocated-channel dedup asymmetry (narrow, fails safe, self-heals next round — a maintainer's call on whether to unify the two statements) and the three stale rank comments (comments only). They are the kind of thing that would be a fixup commit, not a reason to hold a correct, well-tested change. There is a point in favor of landing worth saying plainly, same as the last pass: this PR's own thread is a live specimen of the persistently-critical shape it detects — eleven-plus rounds, Criticals standing and re-standing, the autofix side flatlining into CI at the reviewed head: every 中文说明置信度:4/5——干净的审查、无阻断项;唯一的遗留是一条很窄的迁移通道不对称与三处陈旧梯度注释,均已在 Stage 2 点名,属于修复项而非怀疑。 退一步看:本提交正是循环自己第 11 轮的发现,按先复现后修复的方式处理;对着 head 源码核对,两处修复都落在正确的层。复利问题用按 locator 归键的去重解决,保留有出处可溯的 gate 副本——它由绑定 diff 哈希的报告再生成,模型的重发在结构上冗余,若反而保留它,会让一个已被 linter 证明的阻断项持续拉动未验证封顶。截断读取用披露而非门禁解决,这也是正确选择:恢复完整性门禁恰好会在建议存在的深工作清单轮次把它噤声,而那两条从「缺席」读出的结论正是偏向触发的两条,段落现在双语、正文与终端一致地如实说明。我对问题的独立判断与该方案一致,没有找到它遗漏的更简路径。 关于此前几轮「缓批而非批准」的两个机械原因,本次明说而非悄悄改向:
非阻断项留档待后续跟进,均不构成本次合入门槛:迁移通道去重不对称(很窄、fail-safe、下一轮自愈——是否统一两处语句由 maintainer 定夺)与三处陈旧梯度注释(仅注释)。这类问题属于修复提交的范畴,不是扣住一个正确且测试充分改动的理由。 有一点值得直说,与上次相同:这个 PR 自己的线程就是它所检测形态的活体样本——十一轮以上、Critical 反复未决、autofix 一侧躺平成 被审查 head 上的 CI:每个 — Qwen Code · qwen3.8-max Reviewed at |
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-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. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
…t, gated on floor engagement (#9526) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Round 1 — address-review for PR #9526Commit: Critical
Suggestions addressed
Deferred to the next round (recorded, not dropped)
No conflicts to resolve ( VerificationCommands actually run this round (repo root unless noted):
中文说明第 1 轮 — PR #9526 的 address-review提交: Critical
已处理的 Suggestion
延后到下一轮(已记录,不丢弃)
无冲突需要解决( 验证本轮实际运行的命令(未注明者均在仓库根目录):
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
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": run the new vitest suites ( packages/cli/src/commands/review/lib/convergence.test.ts , the four new tests in compose-review.test.ts ) — neither the worktree no…; "agent 5": running the new tests to confirm they pass — the review worktree has no node_modules and a full npm ci + build exceeds this review's tool/time budget; all cov….
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:1690 — [probe] Gate-produced Criticals are invisible to both halves of the persistently-critical conjunction (advisory-only blast radius; deferred by the code-age rule — anchored on lines u…packages/cli/src/commands/review/compose-review.ts:4487 — [probe] Terminal line prints "CONVERGENCE: Convergence: …" — the label twice (deferred by the code-age rule — anchored on lines unchanged since the round-1 head)packages/cli/src/commands/review/compose-review.ts:4487 — [probe] Multi-line CONVERGENCE record: prefix on line 1 only, self-disclaimer on the last physical line (deferred by the code-age rule — anchored on lines unchanged since the round-1…
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1a":run the new vitest suites ( packages/cli/src/commands/review/lib/convergence.test.ts , the four new tests in compose-review.test.ts ) — neither the worktree no…;"agent 5":running the new tests to confirm they pass — the review worktree has no node_modules and a full npm ci + build exceeds this review's tool/time budget; all cov…。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
… rank (#9526) The advisory shared trim rank 1 with the deferral display, but every rank-1 disclosure surface names "the deferred-findings list" — a fired zero-deferral round whose body overflowed posted a trim notice asserting a deferral list that never existed while the dropped advisory went unnamed. The advisory now holds its own rank (and RANK_NAMES entry), yielding after the deferral display and before the not-reviewed disclosures. Adds the overflow fixture that pins the yield and the relocated-arm firing fixture that pins the third thisCriticals term, and corrects the prevLedgerFacts threat docstring: under `auto` the floor-engagement conjunct is forgeable via the carried round, so the only unforgeable conjunct is this round's own standing Critical.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review-feedback round — PR #9526All four inline findings from the round-2 automated review are addressed; all Dispositions[rc:3819546879] R1-3 — the advisory shares trim rank 1 with the deferral [rc:3819546883] R1-8 — the relocated-Criticals term of [rc:3819546889] R1-9 — the advisory's body-budget yield (trim rank, rung 2) [rc:3819546896] the docstring claims floor engagement is unforgeable → So under the default Changed files
Verification
中文说明Autofix 评审反馈轮次 — PR #9526第 2 轮自动评审的 4 条行内发现全部已处理,且全部在一个提交中于代码层面解决 各项处置[rc:3819546879] R1-3 — 建议块与延后清单共享 trim rank 1,而所有 rank-1 [rc:3819546883] R1-8 — [rc:3819546889] R1-9 — 建议块在正文预算下的让位行为(trim rank、rung 2)无测试 [rc:3819546896] docstring 声称 floor 生效不可伪造 → 已解决(更正 docstring); 即默认 变更文件
验证
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
There was a problem hiding this comment.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:1696 — [probe] Advisory engagement arms (critical floor, contextUnavailable) have no advisory-side oraclepackages/cli/src/commands/review/compose-review.ts:1370 — [probe] hadCritical reader has no negative oracle at the production boundary (sev-S-only work-list)packages/cli/src/commands/review/compose-review.test.ts:2806 — [probe] COMMENT-branch advisory fixture never asserts its eventpackages/cli/src/commands/review/compose-review.test.ts:2797 — [probe] Missing-volume fail-open arm unpinned at the production boundarypackages/cli/src/commands/review/lib/convergence.ts:159 — [review] Advisory text overclaims identity persistence of previous-round Criticalspackages/cli/src/commands/review/compose-review.test.ts:2817 — [probe] No rising-volume firing fixture at the compose boundarypackages/cli/src/commands/review/save-artifact.ts:41 — [probe] convergence Omit exclusion has no test oraclepackages/cli/src/commands/review/compose-review.test.ts:2800 — [probe] Post-floor-enforcement volume source unpinned at the handler boundary
中文说明
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…e signal (#9526) The persistently-critical signal read `thisCriticals` before the gate pushed its Criticals into `bodyCriticals`, and the ledger work-list feeding the next round's persistence half omitted them too. A loop whose standing blocker is the deterministic [lint] gate — the exact shape the signal exists to name — held the whole conjunction semantically while the advisory stayed silent: the count was taken before the array was complete, and the gate-only round recorded no sev 'C' for its successor to recover. The assessment now runs after the relocated and gate pushes and reads the completed array with the same semantics as the verdict's own `c` (the explicit relocated term drops with the push that already carries it), and the gate's Criticals join the marker work-list. Adds the handler fixture arming the gate end to end — advisory fires, marker records sev 'C' — and pins both branches of the trim notice's copy-location conditional, which had no oracle on either side.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #9526Both actionable inline findings are resolved in code (commit Feedback points and dispositions
Changes
Conflict notesNone — VerificationCommands actually run this round (integration tests not run: the touched behavior is exercised through the real exported
中文说明本轮总结 — PR #9526两条可操作的行内发现均已在代码中解决(提交 反馈点与处置
变更内容
冲突说明无 —— 验证本轮实际执行的命令(未运行集成测试:所触及的行为通过真实导出的
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
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2734 — [probe] noteTrimmedRanks tail clause misstates copy location when ranks 2+3 drop togetherpackages/cli/src/commands/review/save-artifact.ts:56 — [probe] Omit justification is false when the body budget trims the advisory — no durable copy survivespackages/cli/src/commands/review/lib/convergence.test.ts:118 — [probe] zh Critical-count interpolation is clause-ambiguous and unpinnedpackages/cli/src/commands/review/compose-review.test.ts:2860 — [probe] shrinking-volume boundary pinned only at delta 2; delta-1 mutant ships greenpackages/cli/src/commands/review/compose-review.test.ts:2815 — [probe] thisCriticals wiring exercised only at value 1; constant mutant false-fires on the just-converged roundpackages/cli/src/commands/review/lib/convergence.test.ts:133 — [probe] zh residual-risk scaffold clause has no oracle
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 6 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round: no action requiredCritical-only mode is active for this counting window: the growth brake engaged on the test-line budget (net source 108 / test 506 lines vs budgets of 400/400; 0 prior rounds were over budget). After the deterministic filter, this round's actionable sections contain no work:
The two items listed under Deferred non-Critical feedback (an automated-reviewer review, and inline finding Result: no code changes and no commit this round; the branch stays at its current head. 中文说明Autofix 轮次:无需处理本计数窗口已启用仅处理 Critical 模式:增长刹车由测试行数预算触发(净增长源码 108 / 测试 506 行,预算为 400/400;此前没有轮次超出预算)。经过确定性过滤后,本轮可执行区域中没有任何待处理工作:
延迟的非 Critical 反馈中列出的两个条目(一次自动评审器的评审,以及位于 结果:本轮不做任何代码改动,也不产生任何提交;分支保持在当前 head。 Deferred non-Critical feedbackCritical-only mode is active: the PR's diff grew src 108 / test 506 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 净增长已达 源码 108 / 测试 506 行(预算 400/400)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。纯增长触发期间维护者反馈照常流动(按作者的批次预算仅在完成 5 个产生改动的轮次后生效)。(评论 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: 4129 passed · 0 failed · 4129 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:4129 通过 · 0 失败 · 4129 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9526 Deep Verification — persistently-critical convergence advisoryVerdict: Assertion accounting: 4071 review-dir vitest tests at head (92 files; 4 pre-existing skips not counted) + 50 mock-free A/B harness assertions + 6 mutation-matrix assertions + 2 gates (typecheck, ESLint on touched files). Raw logs in 中文摘要结论:
Central claim + A/BCentral claim: when the carried telemetry proves the loop is stuck on Criticals (previous round's work-list held a Critical AND this round stands behind one AND the severity floor is engaged AND the two-round posting window is present and not shrinking), the real compose command surfaces the Secondary claims: (1) the final commit's fix — the script-lint gate's Criticals count in the signal and join the marker work-list, so a gate-only loop fires and its successor recovers the persistence half; (2) the advisory holds its own trim rank (2), yielding after the deferral display (1) and before the not-reviewed disclosures (3), with the trim notice naming it correctly. Harness:
50/50 assertions passed ( One oracle note: the terminal Mutation matrix (no survivors; positive controls landed in the mutated files)
Unmutated control is green: 4071 passed / 4 skipped on the restored tree (identical to the pre-mutation run); tree verified byte-identical to HEAD after each restore ( Both branches of the trim notice's copy-location conditional are pinned by the PR's own tests, which ran green in the suite: the rank-3-only drop asserts Advisory-only auditProduction read sites of the assessment, by census: the body block ( CorrectionsDescriptions in the PR body that do not match the final head (corrections to the description, not requests to change code):
Findings1. (Suggestion) The save-artifact acceptance path for a fired composed JSON is unpinned. Not covered
MethodologyRan in the CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
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 takenThis round ran in Critical-only mode because the diff-growth brake engaged: the PR grew by 108 source / 506 test net lines beyond this counting window's baseline (budgets: 400/400; 1 prior round was already over budget). In this mode only Critical findings, failed checks, and maintainer requests are actionable. After triaging the workflow-prepared feedback, there was nothing actionable this round:
The single item listed under No conflict resolution was requested ( Result: no commits were made; the PR head remains at 中文说明Autofix 审查轮次:未采取任何操作本轮运行于 仅处理 Critical 模式,原因是 diff 增长刹车被触发:本计数窗口内,PR 净增长已达源码 108 行 / 测试 506 行,超出基线(预算:400/400;此前已有 1 轮超出预算)。该模式下只有 Critical 级别的发现、失败的检查以及维护者的请求才是可处理的。 对工作流准备好的反馈进行分类后,本轮没有任何可处理的事项:
本轮也未请求解决冲突( 结果: 未产生任何提交;PR 的 head 仍为 Deferred non-Critical feedbackCritical-only mode is active: the PR's diff grew src 108 / test 506 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 净增长已达 源码 108 / 测试 506 行(预算 400/400)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。纯增长触发期间维护者反馈照常流动(按作者的批次预算仅在完成 5 个产生改动的轮次后生效)。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix deferred this item to a human under instruction (round 4/100) — the agent's handoff note below names the decision and the options. The loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: Handoff — PR #9526 needs a maintainer's decisionWhy I am stoppingThe growth brake has engaged and the PR is not converging. In this counting What the PR isPR #9526 (issue #9410) adds a persistently-critical convergence advisory to Shape of the change: a new module Why it is not converging — and a collision I found this roundTwo reinforcing causes:
中文说明🤖 AutoFix 已按指示将此项移交人工处理(第 4/100 轮)—— 下方 agent 的 handoff 说明列出了待决决策与各选项。循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32431061529 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/convergence.test.ts:1225 — [probe] describe('convergenceAssessment') closes one test early, stranding four conjunct testspackages/cli/src/commands/review/save-artifact.test.ts:595 — [probe] bad-shape fixtures keep dead posted/prevPosted keys; negative-count case throws on missing freshpackages/cli/src/commands/review/save-artifact.ts:65 — [review] JSDoc says convergence sheds before everything; the rank -1 health note sheds firstpackages/cli/src/commands/review/compose-review.ts:2935 — [probe] missing-fresh degrade-open arm has no e2e oracle; a ?? 0 mutant ships greenpackages/cli/src/commands/review/compose-review.test.ts:3117 — [probe] advisory floorEngaged wiring's contextUnavailable argument has no oraclepackages/cli/src/commands/review/lib/convergence.ts:899 — [review] module header still attributes the window to the posted/prevPosted pair after the fresh renamepackages/cli/src/commands/review/compose-review.test.ts:3178 — [probe] prevHadCritical oracle fixture is confounded with the new prevPostedSuggestion conjunctpackages/cli/src/commands/review/compose-review.test.ts:3722 — [review] fixture comment names a + relocatedCriticals.length term that does not exist in the sumpackages/cli/src/commands/review/lib/convergence.ts:911 — [review] prevHadCritical JSDoc misdescribes undefined — wiring folds recovered-zero-Critical into itpackages/cli/src/commands/review/lib/convergence.ts:1135 — [probe] advisory asserts identity ('stand again') but the conjuncts prove only presencepackages/cli/src/commands/review/compose-review.test.ts:3323 — [review] posture fixture's narrative claims a Suggestion-posting predecessor its own data does not containpackages/core/src/skills/bundled/review/SKILL.md:928 — [review] trim-order fix leaves line 1348's 'first section trimmed' claim contradicting it, test pin included
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 12 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
#9526) Correct, and reproduced through the real composer before changing anything. Recovery adopts the highest-round marker whoever posted it. Where that marker was NOT merged over this account's own findings, this account's entries are in no work list at all — the state `openCriticals` already refuses to infer across, one screen up in the same function. Every prev-round fact this signal reads comes off that list, and it read it unconditionally: pure-foreign {foreign:true, merged:false} -> FIRES own list {foreign:false} -> FIRES merged {foreign:true, merged:true} -> FIRES An own round-6 marker that was a clean LGTM (empty findings, fresh 0, floor stamped `c`), a foreign same-round marker carrying Criticals and no Suggestions winning recovery, and one Critical drafted this round were enough to publish "Criticals stood in the previous round's work-list and stand again this round — land-with-residual-risk" over this account's own LGTM. All three list-derived facts are withheld on that state, not just `prevHadCritical`: it alone silences the assessment today, but leaving the other two reading a stranger's list is a hole waiting for the next edit to re-open. `prevPostedSuggestion` in particular reads ABSENCE, and a stranger's Critical-only list is exactly the shape that reads as "the floor enforced". Merged foreign lists are deliberately NOT withheld: the union keeps this account's own certified entries under their own ids, which is the part that makes the list speak for this account again — the same distinction `openCriticals` draws. The test drives all three arms and asserts them as one table, so the fix is pinned in both directions: a mutant disabling the gate fires on the stranger, and a mutant widening it to any `foreign` marker silences the merged arm. Both turn the suite red, as does un-gating `prevHadCritical` alone. Not changed, and recorded rather than left implicit: a TRUNCATED work-list still reads as this account's. Truncation shortens our own list, which is a different thing from a stranger's, and the direction it errs in is already documented on `prevPostedSuggestion` and the backlog veto. Requiring completeness would silence the advisory on precisely the deep-work-list rounds it exists for. packages/cli: 4433 pass / 1 skipped. Typecheck, ESLint and Prettier clean.
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed diff-only — the PR’s existing discussion could not be fetched, so this is not an approval and not a no-blockers claim.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\fbe0140e-9ca7-4911-955e-885d58527a47 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\fbe0140e-9ca7-4911-955e-885d58527a47'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\fbe0140e-9ca7-4911-955e-885d58527a47 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\fbe0140e-9ca7-4911-955e-885d58527a47'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
— qwen-code-review via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
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.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2944 — [review] thisCriticals re-derives the Critical count instead of reusing openCriticals/cpackages/cli/src/commands/review/compose-review.ts:3781 — [probe] trim 2→3 renumbering of deferredBlock/testPlanBlock has no overflow oraclepackages/cli/src/commands/review/compose-review.ts:2924 — [probe] truncation degrade-direction comment is false for two conjuncts; the truncated-predecessor arm is untested and ungatedpackages/cli/src/commands/review/compose-review.test.ts:8843 — [probe] #9526 overflow fixture actually drops [-1, 2, 3], never the pure [2, 3] shape it claims to pinpackages/cli/src/commands/review/lib/convergence.test.ts:1248 — [review] two tests pin production-unreachable fact combinations and name the opposite of production behaviorpackages/core/src/skills/bundled/review/SKILL.test.ts:459 — [review] 'All four ranks' comment miscounts the five-rank ladder the test pinspackages/cli/src/commands/review/compose-review.ts:2963 — [probe] 'Suggestion presence means the floor was not running' is falsified by the deterministic-tag exclusion
中文说明
仅完成部分审查,审查缺口已披露。
未审查: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。
收敛姿态下延后(第 10 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
@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: 4579 passed · 0 failed · 4579 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:4579 通过 · 0 失败 · 4579 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9526 Deep Verification (follow-up round 3) — persistently-critical convergence advisoryVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)结论:
Previous-finding statusThis is a follow-up round. The previous report verified head
Central claim + A/BCentral claim (re-tested at the new head): when the carried telemetry proves the loop is stuck on Criticals — previous round's work-list held a Critical AND this round stands behind one AND the floor is engaged on both rounds (proven, not stamped) AND the two-round FRESH window is present and not shrinking AND the standing backlog is not observably shrinking — the real compose command surfaces Delta attribution: the base tip ( Harness:
90/90 assertions passed ( CorrectionsDescriptions in the PR body that do not match the final head (corrections to the description, not requests to change code). Carried from previous rounds and stale again — the body was not updated for the five new commits. Gate witness:
Mutation matrix (8 mutants + 2 positive controls — 9/10 rows killed, 1 classified; witness
|
| # | mutation (scratch, restored) | result |
|---|---|---|
| M1 | window reverted to posting TOTALS (fresh: postedFresh → total; prevFresh → prev.posted) — 59f6489 claim |
KILLED, 2 red: stays silent while the FRESH rate is falling under re-posts (#9526) → expected [ Array(1) ] to have a length of +0 but got 1 (advisory fired where silence expected), plus fires at zero fresh when the backlog HOLDS (totals ≠ fresh counts) |
| M2 | delete the backlog-shrinkage veto — 59f6489 claim | KILLED, 2 red: the clearing-backlog wiring fixture (length of +0 but got 1) + unit suppresses when the standing backlog is SHRINKING |
| M3 | disable the pure-foreign refusal (pureForeignPrev = false) — 4ced96c claim |
KILLED, exactly 1 red: the three-arm fixture, arm: 'pure-foreign' moved (fires where silence expected) — sibling arms untouched, attribution exact |
| M4 | delete the prevPostedSuggestion guard (trust the stamp) — edfd5fa claim |
KILLED, 2 red: the fold-stamp wiring fixture + unit suppresses when the predecessor still posted a Suggestion — the stamp lied |
| M5 | delete the prevFloor posture-change guard — bd857f0 claim |
KILLED, 2 red: stays silent on the round the floor ENGAGES on (#9526) + unit suppresses when the previous round posted under a DIFFERENT floor |
| M6 | tighten the veto to suppress on UNKNOWN predecessor — 59f6489 claim | KILLED, 1 red: unit abstains on the backlog when the previous count is unknown → expected null not to be null |
| M7 | tighten the floor guard to reject UNRECORDED floors — bd857f0 claim, the pin's other direction | KILLED, 7 red: the six wiring firing fixtures (each predecessor records no floor; the advisory suppressed where it was expected) + unit still evaluates when the previous floor was never recorded → expected null not to be null — both directions of the guard pinned, as the commit claims |
| M8 | relax the zero-Critical guard (<= 0 → < 0) — layered-guard probe |
SURVIVED — classified below (Findings 2); not a defect |
| C1 | positive control in convergence.ts: LAND_WITH_RESIDUAL_RISK constant changed |
KILLED, 8+ red incl. the wiring fixtures and saveReviewArtifact > carries the residual-risk advisory into the artifact (#9526) — proves the runner fails suites where the M2/M4/M5/M6 mutants land |
| C2 | positive control in compose-review.ts: rank-2 trim name mangled |
KILLED by the intended overflow tests → expected '⚠️ This body was trimmed…' to contain 'the persistently-critical convergence…' |
All six kill-claims in the three fix-commit messages reproduced. Note on the first C1 attempt: relaxing the zero-Critical guard was first tried as the positive control and SURVIVED — not because the runner is dead (C1/C2 prove otherwise), but because FIRE carries prevCriticals: 2, and with thisCriticals overridden to 0 the backlog veto (0 < 2) catches the case the relaxed guard lets through. That is layered guards hiding each other; the relaxed probe is retained as M8 and classified in Findings.
Advisory-only audit (re-run at the new head)
Production read sites of the assessment, by census (grep over packages/*/src, tests excluded): compose-review.ts (body block, JSON-field spread, terminal record) and save-artifact.ts (validator/persistence). ReviewArtifactV1 is referenced only inside save-artifact.ts — no code consumes the persisted residualRisk, so the durable copy remains a record, not an input. The assessment feeds nothing that moves the event or caps the verdict: every firing cell asserts head/base event agreement (and cappedBy identity on zero-fresh-holds), and all 14 cells agree. The advisory prose's "the standing Critical backlog is not shrinking" is backed wherever it can fire: prevCriticals is defined whenever the signal can reach the veto (undefined only with no recovered predecessor or a pure-foreign list — both already silent via prevHadCritical), and prevHadCritical === true forces prevCriticals ≥ 1, so a fired advisory always measured thisCriticals >= prevCriticals ≥ 1.
Findings
1. (Suggestion, carried — stands) The forgeability threat note still did not survive. Commit 7389a10's message says it "corrects the prevLedgerFacts threat docstring: under auto the floor-engagement conjunct is forgeable via the carried round, so the only unforgeable conjunct is this round's own standing Critical." At head 4ced96c0 no advisory-surface comment carries it (grep: the only forgeable hit is the pre-existing model-identity note at compose-review.ts:5087). The behavior it describes is exactly what the code does: floorEngaged keys on criticalFloorInEffect(input.severityFloor, contextUnavailable, prevRound), and under auto the carried — stranger-influenceable — round number can satisfy that one conjunct; a false fire still needs every other conjunct including this round's OWN standing Critical, and 4ced96c0 additionally removed a stranger's unmerged marker as a source for prevHadCritical. The note's absence remains documentation drift against a commit message, not a code defect. (The sibling half of the threat — the forgeable c stamp — DID get its comment this round: the prevPostedSuggestion wiring comment and docstring state the reporting-fold hole explicitly.)
2. (Completeness reporting, not a merge condition) The surviving M8 mutant is redundant defence, not a coverage gap. Deleting the thisCriticals <= 0 guard changes no test because the backlog veto subsumes it on every shape the wiring can produce: with thisCriticals = 0 and a recovered non-foreign predecessor, prevCriticals ≥ 1 (the list holds the Critical prevHadCritical saw), so 0 < prevCriticals vetoes; and prevCriticals === undefined only where prevHadCritical is already undefined (unrecovered or pure-foreign), which returns null one line earlier. The guard is correct exactly as it stands — it is the cheap first line of a two-line defence. If the author wants it pinned independently, the fixture is { ...FIRE, thisCriticals: 0, prevCriticals: undefined }, which no current test constructs; nothing in this finding blocks the merge.
No new behavioral findings this round: the five delta claims were each exercised through the real command in both directions (fires/silent), the #9623 merge's coexistence and 5-rank ladder hold (RANK_NAMES −1…3 match the skill prose, whose two assertions pass on both arms), and the persistence surface re-proved after the field rename.
Not covered
- Per-commit attribution. Depth-2 checkout (
git rev-parse --is-shallow-repository= true);git rev-list HEAD^1..HEAD^2reports 1 commit where the metadata snapshot lists 12 — the shallow boundary makes the five intermediate commits unreachable. The aggregateHEAD^1..HEADdiff is what was verified; the delta is nonetheless attributable to the five commit messages, whose specific claims (five shapes, both-direction pins, matrix kills) were each re-measured above. - Upstream trigger chain. The harness reproduces the wire shape: carried-ledger side files exactly as
pr-contextwrites them, including theforeign/mergedflags that decide the new refusal. Recovery of markers from real posted GitHub comments was traced in code, not driven end to end — no GitHub token exists in this sandbox by design. The pure-foreign shape itself is the finding's own witness fixture, not a captured production artifact. - Gate-only cross-round chain cell (previous round's
gate-next-from-head/base): not re-run as an A/B cell — the gate path is unchanged by the five delta commits; at head the wiring fixturecounts the script-lint gate's standing Critical — advisory and work-list (#9526)passes in the baseline and goes red under M7, which is head-side evidence; the base arm is trivially advisory-blind. - Overflow/trim behavior in the A/B. The 5-rank ladder and the rank-3-keyed tail clause are pinned by the vitest overflow fixtures plus control mutant C2 (2 red on the rank-name change), not by a dedicated A/B cell — base has no advisory, so the cross-arm comparison there reduces to the absence already proven by the silence cells.
- Repo-wide gates. Only the affected surfaces ran: review-dir suite on both arms (97 files each), core
SKILL.test.tson both arms,npm run typecheck(repo-wide by construction, clean), ESLint on the seven touched code files (clean, liveness probe caught + clean-after-restore). Other workspaces' suites were not run — the diff touches nothing outsidepackages/cli/src/commands/review/and the bundled skill page. - Both arms drove TypeScript source via tsx (symmetric runner). The head's compiled
dist/was separately confirmed to contain the wiring (RESIDUAL-RISK×3 indist/src/commands/review/compose-review.js, the recommendation constant inlib/convergence.js,residualRisk×10 insave-artifact.js). - Flakiness gate runs in the workflow proper, as in previous rounds; not duplicated here.
Methodology
Ran in the CI verify container (node:22-bookworm, Node v22.23.2) on the merge-ref checkout. Targeted gates first: the full src/commands/review/ vitest dir at head and (for delta attribution) at the base worktree, core SKILL.test.ts on both arms, repo npm run typecheck, ESLint on the touched files with a planted-violation liveness probe. The A/B drove the real composeReviewCommand.handler and saveReviewArtifact from both trees through one shared tsx cell runner — each cell in its own temp dir with real plan/ledger/comments files, clean HOME, empty GH_TOKEN; the base worktree at HEAD^1 reused the head dependency tree (the PR's only non-cli change is the bundled SKILL page and its test; internal-link realpath asserted, cli-suite aliasing keeps each arm on its own core source). Mutants were applied one at a time as exact single-occurrence string replacements against the three advisory suites, each kill checked for the intended expected-versus-actual mismatch, restored via git checkout HEAD -- with git status --porcelain verified clean after each row. Evidence captures used scripts/verify-capture.mjs. Raw logs: logs/ (per-suite logs, harness log, matrix log, ab-assertions.json); harness sources: harness/ab-harness.mjs, harness/run-cell.mts, harness/mutation-matrix.mjs; per-cell results under harness/cells/.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/commands/review/compose-review.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/compose-review.test.ts
file packages/cli/src/commands/review/lib/convergence.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/convergence.test.ts
file packages/cli/src/commands/review/save-artifact.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/save-artifact.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/compose-review.test.ts: PPPPP
packages/cli/src/commands/review/lib/convergence.test.ts: PPPPP
packages/cli/src/commands/review/save-artifact.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
main renumbered the trim ladder: the convergence observation moved from rank
0 to rank 3 — LAST, on the arithmetic its own block now spells out (shed
second it could pay for at most 4% of an overflow, so the author lost the
only sentence about the shape of the loop AND the disclosures anyway) — and
the not-reviewed disclosures took rank 2. Rank 0 was left empty.
This branch's advisory takes that empty rank, and the placement follows
main's own argument rather than being wedged in. The ladder now orders by
what a dropped block costs its reader:
-1 mechanism-health note operator has the `HEALTH:` line
0 residual-risk advisory maintainer has `RESIDUAL-RISK:` AND the
composed JSON, which the artifact persists
1 deferred-findings list one copy, in the findings artifact
2 not-reviewed disclosures no copy but the terminal report
3 convergence observation the author's only sentence about the loop
Two things the textual merge got wrong and tests did not catch:
- `RANK_NAMES` came out with a DUPLICATE `3:` key — this branch's disclosures
and main's observation collided on the same literal and the later one
silently won, so a dropped disclosure section would have been named "the
convergence observation". No conflict marker; found by reading the merged
map. The five ranks are now spelled out once each.
- The tail clause still keyed on rank 3. Under main's numbering that is the
observation, which HAS a durable copy; the rank with nothing behind it is
now 2. Keyed wrong it would have told the operator the trimmed set was
backed up on exactly the drop that took the disclosures. Re-keyed on 2,
and the docblock now says which fact the key stands on rather than naming
a number.
The disclosure-family blocks return to rank 2 with main, the advisory takes
rank 0, and three enumerations follow: the ladder docstring, the
`noteTrimmedRanks` docblock, and the bundled skill's trim-order prose with
its assertion.
Mutation matrix re-run after the merge. New ladder: the advisory sharing
rank 1 with the deferral list, sharing rank 2 with the disclosures, and the
tail clause keying on the advisory again each turn the suite red. Carried
over: reverting the window to totals, deleting the backlog veto, deleting
the posture guard, deleting the enforcement-evidence guard, disabling the
pure-foreign gate, and dropping `residualRisk` from the artifact all still
turn it red.
packages/cli 4533 pass / 1 skipped; packages/core skills 379 pass; typecheck,
ESLint and Prettier clean on both.
|
@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: 4756 passed · 0 failed · 4756 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:4756 通过 · 0 失败 · 4756 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9526 Deep Verification (follow-up round 4) — persistently-critical convergence advisory after the ladder re-rank mergeVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)结论:
Previous-finding statusThis is a follow-up round. The previous report verified head
Central claim + A/BCentral claim (re-tested at the new head): when the carried telemetry proves the loop is stuck on Criticals — previous round's work-list held a Critical AND this round stands behind one AND the floor is engaged on both rounds (proven, not stamped) AND the two-round FRESH window is present and not shrinking AND the standing backlog is not observably shrinking — the real compose command surfaces Delta attribution: the base tip ( Harness:
86/86 assertions passed both runs ( CorrectionsDescriptions that do not match the final head (corrections to the description, not requests to change code). Gate witness:
Findings1. (Suggestion, new this round) The re-rank missed three rank enumerations — one of them introduced by the merge itself. The merge message claims "three enumerations follow: the ladder docstring, the
The code is right — the A/B overflow cells and mutants L1–L4 pin the actual order — so this is documentation drift on comments that explicitly describe policy, at the severity the same class carried in previous rounds. Minimal fix: renumber the three comments to the merged ladder (advisory 0, deferral 1, disclosures 2, observation 3) and say "five ranks" in the test comment; the prose in the first docblock also needs its before/after clauses swapped, not just renumbered. 2. (Suggestion, carried — stands) The forgeability threat note still did not survive. Commit 3. (Completeness reporting, not a merge condition) The surviving M8 mutant remains redundant defence, re-measured. Relaxing No new behavioral findings this round: the merge's three ladder claims were each exercised through the real command (A/B overflow cells) and through mutants (L1–L4, all killed with the intended expected-versus-actual mismatches), and all 14 semantics/persist cells re-passed on both arms. Mutation matrix (14 mutants + baseline — 13/14 killed, 1 classified; witness
|
| # | mutation (scratch, restored) | result |
|---|---|---|
| M1 | window reverted to posting TOTALS (fresh: postedFresh → postedInline; prevFresh → prev.posted) |
KILLED, 2 red: stays silent while the FRESH rate is falling under re-posts (#9526) + fires at zero fresh when the backlog HOLDS |
| M2 | delete the backlog-shrinkage veto | KILLED, 2 red: the clearing-backlog wiring fixture + unit suppresses when the standing backlog is SHRINKING |
| M3 | disable the pure-foreign refusal (pureForeignPrev = false) |
KILLED, 1 red: will not read a PURE-FOREIGN work-list as this account's history (#9526) |
| M4 | delete the prevPostedSuggestion guard (trust the stamp) |
KILLED, 2 red: the fold-stamp wiring fixture + unit suppresses when the predecessor still posted a Suggestion |
| M5 | delete the prevFloor posture-change guard |
KILLED, 2 red: stays silent on the round the floor ENGAGES on (#9526) + unit suppresses when the previous round posted under a DIFFERENT floor |
| M6 | tighten the veto to suppress on UNKNOWN predecessor | KILLED, 1 red: unit abstains on the backlog when the previous count is unknown |
| M7 | tighten the floor guard to reject UNRECORDED floors | KILLED, 7 red: the six wiring firing fixtures + unit still evaluates when the previous floor was never recorded — both directions pinned |
| M8 | relax the zero-Critical guard (<= 0 → < 0) — layered-guard probe |
SURVIVED — redundant defence (Findings 3) |
| L1 | advisory shares rank 1 with the deferral list (trim: 0 → 1) — merge claim |
KILLED, 2 red: names the trimmed advisory for itself — never a deferral list that does not exist (#9410) + warns for the disclosures when the advisory went with them (#9526) |
| L2 | advisory shares rank 2 with the disclosures (trim: 0 → 2) — merge claim |
KILLED, same 2 red |
| L3 | tail clause re-keyed on the advisory rank (includes(2) → includes(0)) — merge claim |
KILLED, 2 red: points at the findings artifact only when the deferral list is what went + the trimmed-advisory fixture |
| L4 | advisory rank name mangled to the observation (the duplicate-key collision shape the merge message describes) | KILLED, same 2 red as L1 |
| C1 | positive control in convergence.ts: LAND_WITH_RESIDUAL_RISK constant changed |
KILLED, 8 red incl. saveReviewArtifact > carries the residual-risk advisory into the artifact (#9526) |
| C2 | positive control in compose-review.ts: rank-2 trim name mangled |
KILLED, 5 red incl. trims the not-reviewed disclosures only after the deferral display |
All three kill-claims in the merge commit message reproduced (L1/L2/L3). Positive controls landed one in each mutated file.
Advisory-only audit (re-run at the new head)
Production read sites of the assessment, by census (grep over packages/*/src, tests excluded): compose-review.ts (body block, JSON-field spreads, terminal record) and save-artifact.ts (validator/persistence) — unchanged set. ReviewArtifactV1 is referenced only inside save-artifact.ts, so no code consumes the persisted residualRisk: the durable copy remains a record, not an input. Every firing cell asserts head/base event agreement (and cappedBy identity on zero-fresh-holds); all 16 cells agree. The advisory prose's "the standing Critical backlog is not shrinking" remains backed wherever it can fire: prevHadCritical === true forces prevCriticals ≥ 1, so a fired advisory always measured thisCriticals >= prevCriticals ≥ 1.
Not covered
- Per-commit attribution. Depth-2 checkout (
git rev-parse --is-shallow-repository= true);git rev-list HEAD^1..HEAD^2reports 1 commit where the metadata snapshot lists 13 — the shallow boundary makes the intermediate commits unreachable. The aggregateHEAD^1..HEADdiff (8 files) is what was verified; this round's delta is a single merge commit whose resolution claims were each re-measured above. - Upstream trigger chain. The harness reproduces the wire shape: carried-ledger side files exactly as
pr-contextwrites them, including theforeign/mergedflags. Recovery of markers from real posted GitHub comments was traced in code, not driven end to end — no GitHub token exists in this sandbox by design. - Overflow sizing is synthetic. Blockers are
'B'.repeat(n)sized by binary search against the real budget; real model-written bodies overflow with different content mixes. The ORDER assertions (which ranks drop, what the notice names, what survives) do not depend on the content, but the exact threshold does. - Repo-wide gates. Only the affected surfaces ran: review-dir suite on both arms (98 files each), core
SKILL.test.tson both arms,npm run typecheck(repo-wide by construction, clean), ESLint on the seven touched code files (clean, liveness probe caught 2/2 planted errors and was removed clean). Other workspaces' suites were not run — the effective diff touches nothing outsidepackages/cli/src/commands/review/and the bundled skill page. Main-side changes that rode in with the merge (web-shell, sdk, workflows) are base content for this PR, not verified here. - Both arms drove TypeScript source via tsx (symmetric runner). The head's compiled
dist/was separately confirmed to contain the wiring (RESIDUAL-RISK×3 indist/src/commands/review/compose-review.js). - Flakiness gate runs in the workflow proper, as in previous rounds; the A/B harness itself was run twice with identical 86/86 results.
- The observation-side handling of foreign lists (
diagnoseConvergencereads prev facts ungated bypureForeignPrev) is feat(review): tell the author why a review loop is not settling #9461's surface, pre-existing and out of this PR's diff; the advisory side is gated, and theprevHadCriticalguard runs before any ungated field is read, so advisory silence on pure-foreign inputs is structural (cellpure-foreign+ mutant M3).
Methodology
Ran in the CI verify container (node:22-bookworm, Node v22.23.2) on the merge-ref checkout. Targeted gates first: the full src/commands/review/ vitest dir at head (twice) and at the base worktree, core SKILL.test.ts on both arms, repo npm run typecheck, ESLint on the touched files with a planted-violation liveness probe. The base worktree at HEAD^1 reused the head dependency tree: auxiliary workspace dist/ dirs and the package-local node_modules of core/cli (which hold non-hoisted deps like ajv) were symlinked from head — none of those packages is touched by the PR (effective diff is 8 files); packages/cli/src/generated/git-commit.ts was copied (build prerequisite only); internal-link realpath asserted (node_modules/@qwen-code/qwen-code-core → head packages/core, whose only PR change is the bundled skill page, not consumed by the driven path), and cli's vitest aliases core to each tree's OWN source. The A/B drove the real composeReviewCommand.handler and saveReviewArtifact from both trees through one shared tsx cell runner — each cell in its own temp dir with real plan/ledger/comments files, clean HOME, empty GH_TOKEN; overflow cells binary-searched the minimal trimming blocker per shape on the head arm and re-ran the base arm at the same size. Mutants were applied one at a time as exact single-occurrence string replacements against the three advisory suites (baseline 588 green), each kill checked for the intended expected-versus-actual mismatch, restored via git checkout HEAD -- with git status --porcelain verified clean after each row. Evidence captures used scripts/verify-capture.mjs. Raw logs: logs/ (per-suite logs, junit files, harness logs, ab-assertions.json, per-mutant logs, matrix-rows.json); harness sources: harness/ab-harness.mjs, harness/run-cell.mts, harness/gen-cells.mjs, harness/mutation-matrix.mjs; per-cell results under harness/cells/.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/commands/review/compose-review.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/compose-review.test.ts
file packages/cli/src/commands/review/lib/convergence.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/convergence.test.ts
file packages/cli/src/commands/review/save-artifact.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/save-artifact.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/compose-review.test.ts: PPPPP
packages/cli/src/commands/review/lib/convergence.test.ts: PPPPP
packages/cli/src/commands/review/save-artifact.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not explored to full depth (tool budget reached): chunk 2: running the two new compose-review.test.ts fixtures (worktree has no node_modules and core dist is unbuilt; full npm ci + build was disproportionate — oracles….
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.test.ts:2939 — [review] 'never caps the verdict' pinned only by two name filters on cappedBypackages/cli/src/commands/review/compose-review.ts:4575 (+12 locations) — [review] class: ~12 trim-ladder comments/docs contradict the shipped ladder (advisory=0, disclosures=2, observation=3)packages/cli/src/commands/review/lib/convergence.test.ts:1142 — [probe] doubly-determined 'this round posts no Critical' test — guard-deletion mutant ships greenpackages/cli/src/commands/review/save-artifact.test.ts:596 — [probe] wrong-shape case 4 uses stale posted/prevPosted keys — negative-count rejection untestedpackages/cli/src/commands/review/compose-review.ts:2969 — [probe] deterministic-tagged Suggestion left inline by enforcement suppresses the advisory permanentlypackages/cli/src/commands/review/compose-review.test.ts:3224 — [review] 'previous work-list held no Critical' fixture doubly determined; empty-list shape uncoveredpackages/cli/src/commands/review/lib/convergence.test.ts:1188 — [review] backlog veto's GROWING direction has no fixture — a !== mutant ships greenpackages/cli/src/commands/review/lib/convergence.test.ts:1298 — [review] advisory markdown scaffold pipes/separator pinned by no testpackages/cli/src/commands/review/lib/convergence.ts:1134 — [review] 'stand again this round' asserts identity the count-only facts cannot establishpackages/cli/src/commands/review/compose-review.test.ts:3841 — [probe] fixture comment records a nonexistent mutant (+ relocatedCriticals.length)packages/cli/src/commands/review/lib/convergence.test.ts:1251 — [review] 'still evaluates when unreadable' pins an unreachable tuple; unreadable list actually suppressespackages/cli/src/commands/review/lib/convergence.test.ts:1195 — [review] abstention test pins an unreachable tuple — truncation yields an undercount, never undefinedpackages/cli/src/commands/review/lib/convergence.test.ts:1114 — [probe] criticals field's source slot unpinned — a prevCriticals-reading mutant ships greenpackages/cli/src/commands/review/compose-review.ts:887 — [review] 'posting window' docstring drift — the gate is the FRESH window (3 sites + parenthetical)packages/cli/src/commands/review/lib/convergence.test.ts:1161 — [review] three degrade-open arms pin producer-impossible inputs with reachability claims
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未探索到全部深度(达到工具调用预算):chunk 2:running the two new compose-review.test.ts fixtures (worktree has no node_modules and core dist is unbuilt; full npm ci + build was disproportionate — oracles…。
收敛姿态下延后(第 11 轮,非阻断)——已记录,本轮不要求修改:共 15 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
main landed #9759, a comments-only pass that names the axis wherever a comment says "rank N", because `trim` and `keep` now both use small integers for opposite orderings — `trim: 3` sheds LAST, `keep: 3` is spent FIRST. Resolutions: - `noteTrimmedRanks` docblock: #9759 edited the sentence this branch had already replaced wholesale. This branch's content stays and adopts the discipline — every number in it now says `trim rank`. - `lib/convergence.test.ts`: main added an `isFreshDraft` describe (#9674's fix-induced marking) where this branch's fixtures sit, and both sides were cut mid-`it` sharing one terminator. main's block goes first, where main put it; this branch's follows. Two comments of this branch's own were STALE from last merge's renumber, and neither is reachable by a test — found by reading, in exactly the class #9759 exists to prevent: - the artifact-pointer note still said "Ranks 2 and 3 can drop alone — rank 3 does on any run with disclosures", written when disclosures were rank 3; they are trim rank 2 now and the advisory is trim rank 0. - `Bi.trim`'s docstring still described the pre-renumber order (deferral list 1 -> advisory 2 -> disclosures 3). It now states the current order and why it runs that way, and says outright that `keep` above it is a different axis. One of main's own comments became ambiguous because of this branch, so it is qualified rather than left: the observation's "Shed second (it was rank 0)" is historically true, and rank 0 is now the residual-risk advisory's slot. `isFreshDraft` gaining the fix-induced arm changes what `postedFresh` counts, which is the number this branch's window runs on. That is the intended direction — a NEW defect wearing an old id is first-time work — and both ends of the window are computed the same way in successive rounds, so the comparison stays like-for-like. Mutation matrix re-run after the merge: the advisory sharing trim rank 1, the tail clause keying on the advisory, reverting the window to totals, deleting the backlog veto, deleting the enforcement-evidence guard, and disabling the pure-foreign gate each still turn the suite red. packages/cli 4692 pass / 1 skipped; packages/core skills 384 pass; typecheck, ESLint and Prettier clean on both.
…d reading (#9526) Round 11's two Criticals. Both reproduced through the real composer before anything was changed. R11-2 — a standing gate Critical entered the posting set twice, and the pair compounded. This is a regression from this branch's own commit d72287c: once the gate's `[lint]` Criticals are in the carried work-list, SKILL Step 6's still-standing rule tells the model to re-post the entry under its original id while compose re-derives the same Critical from the report. `buildLedger` keys by claimed id and the regenerated copy claims none, so it minted a second id beside the carried one: ROUND1 work-list [R1-1] blocker rendered once ROUND2 work-list [R1-1, R2-1] rendered twice ROUND3 work-list [R1-1, R2-1, R3-1] rendered three times FLIP (revert the gate spread): round-1 work-list [], rendered once `withoutGateReposts` drops the re-post, keeping the GATE's copy rather than the model's. That direction is load-bearing: `[lint]` is not in `DETERMINISTIC_TAG_RE` (`[build]`/`[test]`/`[probe]` only), so the model's copy counts toward `criticalsNeedingVerify` — a linter-proven blocker was pulling the unverified-blocker cap on every re-post round, and the probe shows that cap disappearing with the fix. For the same reason the dedup runs BEFORE `modelBodyCriticals` is captured: dropping the re-post from the body alone left provenance still counting it, so the first draft of this fix fixed the rendering and kept the cap. Matched on the gate line's LOCATOR (the `` `path`:line CODE `` it opens with, backticks normalised), not the whole string: a re-post is model prose that carries the entry forward without reproducing the message byte for byte, and an exact-match rule stopped deduping the moment the wording drifted. The carried id is stripped through the ledger's own `LEDGER_ID_READBACK`. The body composer's gate call is now the only one on that path, so this also removes one half of R4-1's double invocation. R11-1 — the residual-risk facts are read off a work-list that may be known-truncated, without the completeness gate `openCriticals` applies. The completeness gate is NOT restored, and that is the same call as round 8: a whole-list requirement would silence the advisory on exactly the deep-work-list rounds it exists for, which are the rounds the byte budget shortens. What was wrong is what the code SAID about it. The block comment claimed "every input degrades open to no assessment"; two of these inputs do not. "No Suggestion, so the floor was enforcing" and "the backlog is not shrinking" are read off ABSENCE, and a shortened list can only lose entries, so both lean toward firing. `prevTruncated` now rides the facts and the assessment — deciding nothing — and the paragraph discloses, in both languages, that those two readings came off an incomplete list. The sibling diagnosis in the same module qualifies its own recurrence reading on the same fact; this follows that precedent rather than inventing one. The block comment states the exception instead of the blanket claim. Mutation matrix: never rendering the caveat, wiring `prevTruncated` to a constant, disabling the gate dedup, reverting the dedup to exact-match, and removing it from the marker work-list each turn the suite red — alongside the carried set (window on totals, pure-foreign gate, enforcement-evidence guard). packages/cli: 4697 pass / 1 skipped. Typecheck, ESLint and Prettier clean.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not explored to full depth (tool budget reached): chunk 6: full vitest suite run + tsc typecheck of convergence.test.ts (worktree has no node_modules and a monorepo install would exceed this review's budget; mitigated b….
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.test.ts:9274 (+6 locations) — [review] Stale comments/identifiers contradict the shipped trim ladder (2→3 renumber fiction, inverted save-artifact ladder, nonexistent sum term)packages/cli/src/commands/review/lib/convergence.test.ts:1284 — [probe] describe('convergenceAssessment') closes early — a -t filtered run silently skips five tests incl. the #9526 regressionpackages/cli/src/commands/review/lib/convergence.ts:1191 — [probe] Advisory asserts Criticals 'stand again' (identity) — the conjunction proves only presence; fires over a cleared predecessor blockerpackages/cli/src/commands/review/save-artifact.ts:433 — [probe] prevTruncated present-wrong-shape coerced to false instead of refused — unlike every sibling fieldpackages/cli/src/commands/review/save-artifact.test.ts:593 — [probe] wrong-shape fixtures use dead posted/prevPosted keys — negative fresh/prevFresh rejection unpinned (mutant ships green)packages/cli/src/commands/review/compose-review.ts:2092 (+2 locations) — [probe] class: nonConvergence churn Critical never reaches the work-list — silent advisory on NC-only rounds; false 'not shrinking' fire on consecutive churn roundspackages/cli/src/commands/review/compose-review.ts:2091 — [probe] backlog veto straddles the marker format boundary — pre-PR prev lists miss gate entries; undisclosed false fire on the first post-upgrade roundpackages/cli/src/commands/review/compose-review.test.ts:3229 — [probe] 'previous work-list held no Critical' fixture overdetermined — the bare-true mutant its comment names ships greenpackages/cli/src/commands/review/lib/convergence.test.ts:1339 — [probe] zh truncation caveat's disclosure clause unpinned while en pins both halves — zh-clause mutant ships greenpackages/cli/src/commands/review/lib/convergence.test.ts:1198 — [probe] thisCriticals <= 0 gate unpinned — veto co-suppresses the fixture; gate-deletion mutant ships suite-greenpackages/cli/src/commands/review/lib/convergence.test.ts:1139 (+3 locations) — [review] class: 'every input degrades OPEN / WITH ONE EXCEPTION' blanket claims falsified — prevFloor absence leans toward firing; three abstain tests contradict…packages/cli/src/commands/review/lib/convergence.ts:1197 — [review] truncation caveat names only the byte-budget cause — admission-refusal (rejected > 0) rounds publish a false causal claimpackages/cli/src/commands/review/compose-review.test.ts:6238 — [probe] withoutGateReposts locator axes unpinned — both single-axis coarsening mutants ship 496/496 greenpackages/cli/src/commands/review/compose-review.ts:3296 — [review] thisCriticals re-derives the count already named openCriticals — the drift class the file's header exists to preventpackages/cli/src/commands/review/lib/convergence.test.ts:1336 — [probe] truncated-predecessor test pins only the caveat strings — advisory-shortening mutant ships 624-passed greenpackages/cli/src/commands/review/compose-review.test.ts:2986 — [probe] 'loop IS converging' fixture pins a nonexistent posting-total arm — silence is the fresh window; fossil narrativepackages/cli/src/commands/review/compose-review.test.ts:9158 — [probe] combined-drop fixture lacks sha — the health note drops as a third section while comments claim two; toContain masks itpackages/cli/src/commands/review/lib/convergence.test.ts:1365 — [review] 'Residual risk:' / '残余风险:' lead-in — load-bearing per the docblock — asserted by zero tests in either languagepackages/cli/src/commands/review/compose-review.ts:5007 — [probe] withoutGateReposts inserted between scriptLintGate's docstring and the function — the gate's provenance invariant now documents the wrong functionpackages/cli/src/commands/review/lib/convergence.ts:925 — [review] module header narrates a posted/prevPosted window — the shipped conjunct reads fresh/prevFresh (the totals design the fresh docblock rejects)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未探索到全部深度(达到工具调用预算):chunk 6:full vitest suite run + tsc typecheck of convergence.test.ts (worktree has no node_modules and a monorepo install would exceed this review's budget; mitigated b…。
收敛姿态下延后(第 12 轮,非阻断)——已记录,本轮不要求修改:共 20 条(原文未翻译,列表见上方英文部分)。
机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@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: 350 passed · 0 failed · 350 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:350 通过 · 0 失败 · 350 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9526 Deep Verification (follow-up round 5) — R11 fixes: gate-Critical dedup and truncated-list caveatVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)结论:
Previous-finding statusThis is a follow-up round (the previous report verified head
Central claim + A/BCentral claim (re-tested at the new head, three arms): the advisory fires on the persistently-critical conjunction and is advisory-only; the two R11 mechanisms hold —
Arm suites (review dir, identical vitest): head 4694 passed | 4 skipped (4698) vs base 4653 passed | 4 skipped (4657) — +41 passing, +0 failing, +0 skipped; the 4 skips are the identical environmental set on both arms. Core Harness: The per-cell witness ( Reading the ladder: the prefix arm is the defect the round-11 review reported — one blocker gains a second work-list entry and a second body rendering every round, and its model re-post (a The 86→319: assertion totals. Overflow cells (real budget, sized blockers)Blockers are
Merge-message claim probed: the
|
| # | mutation (scratch, restored) | result |
|---|---|---|
| M1 | window reverted to posting TOTALS (fresh: postedFresh → postedInline; prevFresh → prev.posted) |
KILLED, 2 red: stays silent while the FRESH rate is falling under re-posts (#9526) + fires at zero fresh when the backlog HOLDS |
| M2 | delete the backlog-shrinkage veto | KILLED, 2 red: the clearing-backlog wiring fixture + unit suppresses when the standing backlog is SHRINKING |
| M3 | disable the pure-foreign refusal (pureForeignPrev = false) |
KILLED, 1 red: will not read a PURE-FOREIGN work-list as this account's history (#9526) |
| M4 | delete the prevPostedSuggestion guard (trust the stamp) |
KILLED, 2 red: the fold-stamp wiring fixture + unit suppresses when the predecessor still posted a Suggestion |
| M5 | delete the prevFloor posture-change guard |
KILLED, 2 red: stays silent on the round the floor ENGAGES on (#9526) + unit suppresses when the previous round posted under a DIFFERENT floor |
| M6 | tighten the veto to suppress on UNKNOWN predecessor | KILLED, 1 red: unit abstains on the backlog when the previous count is unknown |
| M7 | tighten the floor guard to reject UNRECORDED floors | KILLED, 7 red: the six wiring firing fixtures + unit still evaluates when the previous floor was never recorded — both directions pinned |
| M8 | relax the zero-Critical guard (<= 0 → < 0) — layered-guard probe |
SURVIVED — redundant defence (Findings 4) |
| L1 | advisory shares trim rank 1 with the deferral list (trim: 0 → 1) |
KILLED, 2 red: names the trimmed advisory for itself — never a deferral list that does not exist (#9410) + warns for the disclosures when the advisory went with them (#9526) |
| L2 | advisory shares trim rank 2 with the disclosures (trim: 0 → 2) |
KILLED, same 2 red |
| L3 | tail clause re-keyed on the advisory rank (includes(2) → includes(0)) |
KILLED, 2 red: points at the findings artifact only when the deferral list is what went + the trimmed-advisory fixture |
| L4 | advisory rank name mangled to the observation (duplicate-key collision shape) | KILLED, same 2 red as L1 |
| N1 | R11-2: disable the body-composer gate dedup (withoutGateReposts → identity) |
KILLED, 2 red: does not compound the work-list or the body across rounds + keeps the deterministic copy, so a proven blocker pulls no verify cap |
| N2 | R11-2: revert dedup to whole-string match (locator → full claim) | KILLED, 1 red: matches the locator, not the model's wording |
| N3 | R11-2: remove the dedup from the MARKER work-list only | KILLED, 1 red: does not compound the work-list or the body across rounds — the marker side alone restores compounding |
| N4 | R11-1: never render the caveat (en+zh conditionals → false) |
KILLED, 2 red: the truncated wiring fixture + unit fires on a truncated predecessor, and says the reading came off one |
| N5 | R11-1: wire prevTruncated to a constant TRUE |
SURVIVED — coverage gap (Findings 1); the harness trunc-whole cell is the fixture that kills it |
| N6 | R11-1: validator prevTruncated constant TRUE (breaks absence semantics) |
KILLED, 1 red: saveReviewArtifact carries the residual-risk advisory into the artifact (#9526) |
| C1 | positive control in convergence.ts: LAND_WITH_RESIDUAL_RISK constant renamed |
KILLED, 8 red incl. all wiring firing fixtures |
| C2 | positive control in compose-review.ts: rank-2 trim name mangled |
KILLED, 5 red incl. trims the not-reviewed disclosures only after the deferral display |
| C3 | positive control in save-artifact.ts: shape-check string mangled |
KILLED, 1 red: carries the residual-risk advisory into the artifact (#9526) |
All three kill-claims in the fix-commit message reproduced (the dedup disabling, the exact-match revert, and the marker-list removal each turn the suite red), with the one exception corrected under Corrections and Findings 1: the wiring-to-a-constant claim holds only for the FALSE constant. Every surviving mutant has a positive control in the same file proving the runner can make that file's tests fail (C1–C3 all killed), and the unmutated baseline is green.
Advisory-only audit (re-run at the new head)
Production read sites of the assessment, by census (grep over packages/*/src, tests excluded): compose-review.ts (body block, composed-JSON field, terminal RESIDUAL-RISK: record at line 5618) and save-artifact.ts (validator/persistence) — unchanged set, nothing else. ReviewArtifactV1 is referenced only inside save-artifact.ts, so no code consumes the persisted residualRisk: the durable copy remains a record, not an input. All 12 event-carrying cells (every handler-mode cell except fold-stamp, which asserts body content only) assert head/prefix/base event agreement and identical cappedBy (assertEventsAgree, all passed); the fired cells add residualRisk to the JSON and a line to the terminal without touching either. The advisory prose's "the standing Critical backlog is not shrinking" remains backed wherever it can fire: prevHadCritical === true forces prevCriticals ≥ 1, so a fired advisory always measured thisCriticals >= prevCriticals ≥ 1 — and when the list it was read off is known-shortened, the paragraph now says so (R11-1).
The R11-2 cap oracle closes the other direction: a gate finding is deterministic by provenance (scriptLintGate ran the linter against a report bound to this diff's hash), so keeping the gate's copy and dropping the model's re-post BEFORE provenance is taken means a linter-proven blocker no longer pulls the criticals-unverified cap — measured in the gate-cap cell on all three arms (head: cap absent; prefix and base: cap armed).
Not covered
- Per-commit attribution. Depth-2 checkout (
git rev-parse --is-shallow-repository= true);git rev-list --count HEAD^1..HEAD^2reports1where the metadata snapshot lists 15 commits — the shallow boundary makes the intermediate commits unreachable as graph nodes (their objects are present, which is what enabled the6fa3e76epre-fix arm, butrev-listcannot walk them). The aggregateHEAD^1..HEADdiff (8 files) is what was verified; this round's delta commits (6fa3e76e,ad569092be) were each exercised through their own cells and mutants. - Upstream trigger chain. The harness reproduces the wire shape: carried-ledger side files exactly as
pr-contextwrites them (includingforeign/merged/dropped), script-lint reports bound to a real diff hash. Recovery of markers from real posted GitHub comments was traced in code, not driven end to end — no GitHub token exists in this sandbox by design. - Overflow sizing is synthetic. Blockers are
'B'.repeat(n); the ORDER assertions (which ranks drop, what the notice names, what survives, the tail clause) do not depend on content, but the exact thresholds do. The rank-1-alone window does not exist for the deferral-only shape in this scan (its small rank-2 disclosure always co-sheds); the combined-drop contrast was used instead and says which. - Repo-wide gates. Only the affected surfaces ran: review-dir suite on head and base arms, core
SKILL.test.tson both arms, ESLint on the six touched cli files (liveness probe caught the planted violation and was removed clean),npm run typecheckclean repo-wide (all 11 workspaces incl. cli and core, exit 0, zero errors). Other workspaces' suites were not run — the effective diff touches nothing outsidepackages/cli/src/commands/review/and the bundled skill page. Main-side changes riding in the6fa3e76emerge are base content for this PR, not verified here. - Both arms drove TypeScript source via tsx (symmetric runner). The head's compiled
dist/was separately confirmed to contain the wiring (RESIDUAL-RISK×3,withoutGateReposts×3,prevTruncatedin all three dist files). - The observation-side handling of truncated lists (
diagnoseConvergencequalifies its own recurrence reading on the sametruncatedfact — the precedent R11-1 follows) is feat(review): tell the author why a review loop is not settling #9461's surface; verified only insofar as thetrunc-caveatprefix arm shows it present before this commit. - Flakiness gate runs in the workflow proper; the A/B harness ran three full iterations, and the final one's 319 assertions are the reported set.
Methodology
Ran in the CI verify container (node:22-bookworm, Node v22.23.2) on the merge-ref checkout (HEAD = merge commit, HEAD^1 = base tip 7f2c4416, HEAD^2 = verified head ad569092be). Three arms: the head tree, a git worktree at HEAD^1 (base), and — because R11-2 fixes a regression the PR's own d72287cc introduced — a third worktree at the pre-fix merge 6fa3e76e (its objects are present at the shallow boundary though rev-list cannot walk them). Both auxiliary trees reused the head dependency tree: root and package-local node_modules symlinked, workspace dist/ dirs symlinked for packages the PR does not touch, packages/cli/src/generated/git-commit.ts copied; control purity asserted by realpath (node_modules/@qwen-code/qwen-code-core → head packages/core, whose only PR change is the bundled skill page — the sole core import on the driven path is atomicWriteFileSync, save cells only), and each tree's vitest aliases core to its OWN source. Targeted gates first (review-dir suite on both arms, core SKILL.test.ts on both arms, ESLint with a planted-violation liveness probe, typecheck). The A/B harness (harness/run-cell.mts, driven by harness/ab-harness.mjs) ran 20 cells × 3 arms through the real composeReviewCommand.handler / composeReview / scriptLintGate / parseLedger / saveReviewArtifact via tsx — each cell in its own temp dir with real plan/ledger/comments/report files, clean HOME, empty GH_TOKEN, no mocks of PR code; overflow cells sized their blockers by stepped scan on head and replayed the identical size on prefix and base. The mutation matrix (harness/mutation-matrix.mjs) applied 19 single-point string substitutions (each verified unique in-file before running), ran the 3-file scoped suite per mutant, restored via git checkout HEAD --, and verified git status --porcelain clean after every row (all rows restoredClean=true). Raw per-cell logs live under harness/cells/<arm>/<cell>/ (result.json, stdout.log, stderr.log); harness console logs under logs/.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/commands/review/compose-review.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/compose-review.test.ts
file packages/cli/src/commands/review/lib/convergence.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/convergence.test.ts
file packages/cli/src/commands/review/save-artifact.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/save-artifact.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/compose-review.test.ts: PPPPP
packages/cli/src/commands/review/lib/convergence.test.ts: PPPPP
packages/cli/src/commands/review/save-artifact.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/convergence.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Re-review at ad569092 — the four Criticals raised since my last pass are fixed; approving
I reviewed this substantively at 59f6489e9d (the fresh-window + backlog-veto round) and found it sound. So rather than re-derive the whole diff, I took the delta since then: four Criticals were raised and closed in that span, two of them in this head commit. I checked each against the code, plus re-confirmed the one property the whole design rests on now that the diff has grown.
The four Criticals since 59f6489e
R8-1 — the posture-change guard paired two different floor readings. Fixed, and the direction is better than either remedy offered. Rather than restamping the marker from the enforcement reading — which would have relocated the same cross-reading defect into the sibling diagnosis — the guard now asks for positive evidence: if (prevPostedSuggestion === true) return null, with the reasoning in place beside it ("a c STAMP is not proof the floor enforced… A Suggestion in the predecessor's work-list is the proof, and it says the floor did not"). A stamp that can be folded from a missing floor is no longer load-bearing.
Pure-foreign predecessor work-list. Fixed, and gated wider than the report asked: prevHadCritical, prevPostedSuggestion, prevCriticals and prevTruncated all carry !pureForeignPrev. Taking the wide gate is right — prevPostedSuggestion reads absence, and a stranger's Critical-only list is exactly the shape that reads as "the floor was enforcing".
R11-1 — facts read off a possibly-truncated list, in the firing direction. Resolved by disclosure rather than suppression, and I think that is the correct call for this signal: a completeness gate would silence the advisory on precisely the deep-work-list rounds the byte budget shortens, which are the rounds it exists for. What matters is that the claim was corrected too. The block comment no longer asserts a blanket "every input degrades open" — it now reads "WITH ONE EXCEPTION, stated here because a blanket claim is the kind of false record this module polices" and names the two absence-derived readings. prevTruncated rides as a non-conjunct and the paragraph qualifies itself in both languages, naming the two readings explicitly rather than gesturing at uncertainty:
— though the previous round's work list was truncated to fit the marker, so "the backlog is not shrinking" and "the floor was enforcing" are both read off a list known to be incomplete
R11-2 — a gate Critical compounding across rounds. Fixed, and the two load-bearing details hold. withoutGateReposts matches on the gate line's locator rather than the whole string, which is the right key: a re-post is model prose that carries the entry forward without reproducing the message byte-for-byte, so an exact-match rule would stop deduping the moment wording drifted. And the ordering is genuinely as claimed — I read it rather than trusting the note:
const gate = input.planPath ? scriptLintGate(input.planPath) : …;
const ownAfterGateDedup = withoutGateReposts(bodyCriticals, gate.criticals);
bodyCriticals.length = 0;
bodyCriticals.push(...ownAfterGateDedup);
const modelBodyCriticals = [...bodyCriticals]; // captured AFTER the dedupThat is what stops the dropped re-post from still pulling the unverified-blocker cap through criticalsNeedingVerify — [lint] is not in DETERMINISTIC_TAG_RE, so fixing only the rendering would have left a linter-proven blocker capping the verdict every round.
Re-confirmed at the grown diff
Advisory-only still holds. Every cappedBy.push site sits above the line where residualRisk is computed, and all eleven references to it are rendering, the returned result, the terminal line, or persistence — nothing in the event or cap path reads it. That was the safety premise of my earlier pass and it survives the additions.
One direction changed since I last looked, and it is justified. residualRisk is now persisted rather than omitted from PersistedVerdict. The rationale is recorded where it belongs: rank 2 sheds before the not-reviewed disclosures, so the rounds that fire this are the long rounds whose body is most likely to drop it, leaving the durable record as the only place the facts survive. The validator earns it — object(), shape pinned, recommendation pinned to the single issued code, all three counts through volumeOf, and prevTruncated reading absence as "not disclosed" so an older artifact is not refused. That also answers the minor observation I left open last round about volumeOf covering all three counts: the comment now states why that boundary uses the ledger's own reader.
House style is clean; CI is green; MERGEABLE.
One nit, non-blocking
compose-review.ts:2596 carries the trailing comment // input's, captured before the gate, two lines below a block comment that says "The gate runs BEFORE that capture." Both are describing something true — the capture excludes the gate's own criticals, and it follows the gate call — but read together they contradict each other at exactly the seam R11-2 was about. Worth a word change while it is fresh; this file's recent history is mostly comments that stopped matching their code.
Where I land
Approving. Every Critical raised since my last substantive pass is closed against the code, the two that were resolved by argument rather than by suppression corrected their own claims instead of leaving an absolute I could not verify, and the advisory-only invariant still holds.
Not verified by me this round: I did not re-run the suites here (no node_modules in this worktree — CI is green, and I ran 562 of them on the earlier head). My evidence is the four fixes traced at this head plus the cap/event path re-walked.
|
Released in v0.22.2. |














What this PR does
This adds a convergence-exit advisory to the review compose step: when the carried telemetry proves the review loop is stuck on Criticals — Criticals stood in the previous round's work-list and stand again this round, while the two-round window of FIRST-TIME findings is present and not falling, and the standing Critical backlog is not shrinking — compose-review surfaces a single recommendation,
land-with-residual-risk, telling the operator the loop is unlikely to self-converge via the severity floor and that the exit is a maintainer risk-acceptance decision. The advisory appears on three surfaces — a structured field on the composed JSON, a low-retention non-capping disclosure in the review body, and a terminalRESIDUAL-RISK:line beside the existingVOLUME:telemetry — each carrying a blank three-column residual-risk inventory scaffold (attack surface · attacker-dependency · blast radius) for the maintainer to complete, and each explicitly advisory-only: it never moves the event, never caps the verdict, never blocks the post.Merged with #9461, which landed the loop-settling observation in the same module: it answers "is this loop settling, and if not, why", and its handling advice ends at a posture the operator can still change — including dropping the round to a Critical-only floor. This advisory picks up where that advice has already been taken and the loop still does not settle. Both render, so this side is named for its exit throughout rather than for the word they share: the composed field is
residualRisk(the observation keepsconvergence), the terminal line isRESIDUAL-RISK:beside the observation'sCONVERGENCE:, and the paragraph leads with "Residual risk:" rather than "Convergence:". One name over two different paragraphs is a body — and a terminal record — that no reader and no parser can split apart. The floor-engagement conjunct reads #9461'scriticalFloorInEffect(the strict enforcement reading, since the claim is about Suggestions having actually left the posting set), and the persistence half reads the same recovered work-list the observation clusters over, so the two features cannot disagree about what the previous round held.The window measures FIRST-TIME findings rather than the posting total, and for the reason #9461's observation measures its own trend the same way: Step 6 re-posts every still-standing ledger Critical under its original id, so the total only ever rises and a loop whose new findings are collapsing still posts flat-or-rising totals. The standing Critical backlog rides beside it as a veto, because the fresh window alone is blind to a reviewer finding nothing new for two rounds while the author clears blockers — fresh 0 against fresh 0, which "not falling" would read as stuck.
Why it's needed
The severity floor converges a healthy loop: from the floor onward, Suggestions stop posting and the volume falls to the Criticals, then to zero as those get fixed. But a security-sensitive PR under adversarial review never reaches that state — its Criticals never clear, the floor engages, the Suggestions stop, and the posting volume flatlines at the Critical count instead of falling. Every convergence exit that exists today (
land-and-defer,fix-pipeline,re-anchor) assumes the loop can reach a Critical-free state, so none of them names this shape, and the loop just keeps opening rounds. This PR supplies the missing exit: a pure-facts detector plus the one recommendation that fits, so the operator is told to land with the residual risk instead of reviewing again. Evidence that this shape is real and that the floor does not close it: PR 9226 ran twelve rounds, Critical-only from round six yet finding new Criticals every round, and ended only when a maintainer froze the loop and merged.Reviewer Test Plan
How to verify
The signal is pure and unit-tested; the wiring is integration-tested through the real compose command.
cd packages/cli && npx vitest run src/commands/review/lib/convergence.test.ts— nine tests pin the full firing conjunction and each degraded arm individually (prior round unrecovered, prior work-list clean, no Criticals this round, a missing volume, a shrinking volume, and the flat-zero edge).cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts -t '#9410'— two tests drive the real compose command over a carried-ledger fixture: the persistently-critical fixture must surface the advisory on all three surfaces (JSON field, body disclosure, terminal line), and a shrinking-volume fixture must stay completely silent.cd packages/cli && npx vitest run src/commands/review/— 4297 pass, 1 skipped;npm run typecheckand the ESLint run on the touched files are clean.Evidence (Before & After)
N/A — non-UI change (compose-review telemetry/advisory). Behavior is covered by the unit and integration tests above; the persistently-critical shape requires a multi-round carried ledger and is not practical to reproduce manually.
Tested on
Environment
Unit and integration tests via
npx viteston macOS (npm run devtoolchain); CI covers Windows and Linux.Risk & Scope
residualRiskfield, no disclosure, noRESIDUAL-RISK:line).Linked Issues
Closes #9410. Refs #9278 (this fills the persistently-critical exit in that design's recommendation menu; the other menu items remain open there).
中文说明
这个 PR 做了什么
给评审 compose 步骤新增一个收敛出口建议:当携带的遥测证明评审循环卡在 Critical 上——上一轮工作清单里有 Critical、本轮依然有,、两轮「首次发现」窗口存在且未下降、且未决 Critical 积压没有减少——compose-review 会给出唯一建议
land-with-residual-risk,告诉操作者这个循环不太可能靠 severity floor 自行收敛,出口是 maintainer 的风险接受决定。建议出现在三个面上——composed JSON 的结构化字段、评审正文里一个低保留的非封顶披露、以及紧挨现有VOLUME:遥测的终端RESIDUAL-RISK:行——每一面都附带一份空白的三栏残余风险清单脚手架(攻击面 · 攻击者依赖性 · blast radius)供 maintainer 填写,并且都明确声明仅为建议:绝不移动事件、绝不封顶裁决、绝不阻断发布。与 #9461 合并:那个 PR 在同一模块落了循环收敛观察——回答"这个循环在收敛吗,不收敛的话为什么",它的处理建议止步于操作者还能改的姿态,包括把本轮降到 Critical-only floor。本 PR 的建议接在"那条建议已经被采纳、循环依然不收敛"之后。两者都会渲染,所以本侧全程按它的出口命名,而不是按两者共用的那个词:composed 字段是
residualRisk(观察保留convergence),终端行是RESIDUAL-RISK:,与观察的CONVERGENCE:并列,段落引导词是"残余风险:"而非"收敛情况:"。一个名字盖住两个不同段落,正文和终端记录都会变成读者与解析器都拆不开的东西。floor 咬合这一合取项读 #9461 的criticalFloorInEffect(严格的执法读法——因为该主张是关于 Suggestion 确实已被移出发布集),持久性那一半读的是观察用来聚类的同一份恢复工作清单,所以两个特性不会对"上一轮持有什么"产生分歧。窗口度量的是首次发现而非发布总量,理由与 #9461 的观察度量自身趋势时相同:Step 6 会把每条仍未决的台账 Critical 以原 id 重发,因此总量只增不减——一个新发现正在塌缩的循环,其总量依然持平甚至上涨。未决 Critical 积压作为否决项并列其旁,因为仅靠 fresh 窗口看不见「连续两轮没有新发现、而作者正在清理阻断项」这一形态:它是 fresh 0 对 fresh 0,被「未下降」读成了卡死。
为什么需要
severity floor 能让健康循环收敛:floor 生效后 Suggestion 不再发布,音量降到 Critical,再随着 Critical 被修复降到零。但安全敏感 PR 在对抗式评审下永远到不了那个状态——它的 Critical 清不完,floor 咬合后 Suggestion 停了,发布音量却在 Critical 数量上躺平而不是下降。今天存在的每个收敛出口(
land-and-defer、fix-pipeline、re-anchor)都假设循环能到达"无 Critical"态,所以没有一个能命名这个形状,循环就只能一直开新轮。本 PR 补上缺失的出口:一个纯事实检测器 + 唯一匹配的建议,让操作者被告知"带着残余风险合入"而不是再开一轮评审。这个形状真实存在、floor 无法关闭它的证据:PR 9226 跑了十二轮,从第六轮起 Critical-only 却每轮都发现新 Critical,最终只有一位 maintainer 冻结循环并合入才结束。评审者测试计划
如何验证
信号是纯函数、有单测;接线通过真实 compose 命令做了集成测试。
cd packages/cli && npx vitest run src/commands/review/lib/convergence.test.ts——九个测试钉住完整触发合取,并逐个钉住每个降级臂(上轮未恢复、上轮工作清单无 Critical、本轮无 Critical、音量缺失、音量收缩、以及 flat-zero 边界)。cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts -t '#9410'——两个测试用携带台账的 fixture 驱动真实 compose 命令:persistently-critical fixture 必须在三个面都披露建议(JSON 字段、正文披露、终端行),音量收缩的 fixture 必须完全静默。cd packages/cli && npx vitest run src/commands/review/——4297 通过、1 跳过;npm run typecheck与对改动文件的 ESLint 均干净。证据(前后对比)
N/A——非 UI 改动(compose-review 遥测/建议)。行为由上述单测与集成测试覆盖;persistently-critical 形态需要多轮携带台账,手工复现不现实。
测试环境
环境(可选)
macOS 上经
npx vitest的单测与集成测试(npm run dev工具链);CI 覆盖 Windows 与 Linux。风险与范围
residualRisk字段、无披露、无RESIDUAL-RISK:行)。关联 issue
Closes #9410。Refs #9278(本 PR 补上该设计建议菜单中的 persistently-critical 出口;菜单其余项仍在该 issue 下开放)。