fix(review): gate decided stops on a composed re-rule verdict - #10310
Conversation
A decided stop (unchanged-since-last-round, clean-tree, scope-emptied) completed with event: null, so `qwen review run --fail-on request-changes` exited 0 while the round's own output rendered open Criticals as standing — a user who committed without fixing a blocker left a permanently passing gate (#9659's accepted R8-1/R13-3 residual; the blocker-dating chain that tried to answer this from file bytes produced successor Criticals every regime it grew and was deleted). The judgement goes where judgement lives, and the arithmetic goes where arithmetic lives. Step 1's stop branches now compose a real verdict whenever the cache ledger holds open Criticals: - On the two incremental stops the dispositions are DEDUCED, not judged: a byte-identical state means every open finding still stands verbatim, and scope-emptied's supersededPaths split rules the rest — the exact structural-accuracy argument the R13-3 escalation recorded. - On clean-tree (no anchor certifies what moved) the orchestrator re-rules each finding against HEAD under Step 6's discipline. compose-review gains a stopReRule input, granted fail-closed and machine-checked: the plan must carry the capture's own nothingToReview field (no full round's plan does), a published run id must match the runId-fenced stop sidecar, and the dispositions must equal the ledger's open-Critical set BOTH ways — an omitted row cannot drop a blocker, an invented one cannot resurrect it, a still-stands must post its body re-assertion and a fixed/superseded must not. Granted, the round is exempt from the agent-transcript floors (no agents ran; nothing can satisfy them) and a cleared round composes a Comment, never an Approve — nothing new was reviewed. The composed artifact rides the same name a full round writes, so run.ts gates it with zero new plumbing. Rewritten from scratch against current main; supersedes the pre-round-12 draft on fix/review-stop-composed-verdict, whose assumptions the following twelve review rounds invalidated. Closes #9908. Every refusal is pinned by a test; the completeness gate, the full-round forgery guard, and the Approve downgrade are mutation-checked red.
github-actions
Bot
added
the
review/self-reported
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @wenshao. The problem is concrete — a decided stop round completing with event: null lets an open Critical sail through qwen review run --fail-on request-changes, and #9908 records it as the accepted residual of the blocker-dating deletion. Before code review, one gate item: the PR body doesn't follow the PR template. Instead of the required sections it is structured as Problem / Design / Verification, so all of these are missing:
## What this PR does## Why it's needed## Reviewer Test Plan— including### How to verify,### Evidence (Before & After), and the### Tested onOS matrix## Risk & Scope## Linked Issues(theCloses #9908reference currently sits inside the Problem prose)
Most of the substance is already in the body — Problem covers What/Why, Verification maps to the Reviewer Test Plan, and the supersede note fits Risk & Scope — so this should be a restructure rather than new work. The two pieces that genuinely need adding are the Evidence section — a before/after for the gate miss (a decided-stop round with an open Critical exiting 0 under --fail-on request-changes) — and the Tested-on matrix. Once the body follows the template, re-run with @qwen-code /triage and we'll pick up from the code review.
中文说明
感谢这个 PR，@wenshao。问题很具体——决定性 stop 轮次以 event: null 完成，会让 open Critical 直接穿过 qwen review run --fail-on request-changes，#9908 已把这一点记录为删除 blocker-dating 链后接受的残余。但在进入代码审查之前有一个门禁项：PR 正文没有遵循 PR 模板。当前正文写成了 Problem / Design / Verification 的形式，以下必填章节全部缺失：
## What this PR does## Why it's needed## Reviewer Test Plan—— 包括### How to verify、### Evidence (Before & After)和### Tested on操作系统矩阵## Risk & Scope## Linked Issues（Closes #9908的引用目前写在 Problem 正文里）
正文里其实已经包含了大部分实质内容——Problem 对应 What/Why，Verification 对应 Reviewer Test Plan，supersede 说明适合放进 Risk & Scope——所以这应该是一次重组，而不是新增工作。真正需要补充的是两处：Evidence 小节——gate 失效的 before/after 证据（决定性 stop 轮次中存在 open Critical 时 --fail-on request-changes 退 0）——以及 Tested-on 矩阵。正文按模板调整后，用 @qwen-code /triage 重新触发，我们会从代码审查继续。
— Qwen Code · qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — all diff pages read in full; the one check I could not finish (ledger admission of softened-round findings) is named in the finding's uncertainty clause …; "agent reverse-audit (round 8)": mutant run for the duplicate-disposition guard (survival established statically instead: stopReRule appears nowhere in the suite outside the four added call s….
Not reviewed: reverse audit — stopped before round 10 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — all diff pages read in full; the one check I could not finish (ledger admission of softened-round findings) is named in the finding's uncertainty clause …；"agent reverse-audit (round 8)"：mutant run for the duplicate-disposition guard (survival established statically instead: stopReRule appears nowhere in the suite outside the four added call s…。
未审查：反向审计——评审时间预算不足，未能开始第 10 轮。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
11 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 leftover FENCE-PROBE debug instrumentation — already reported (comment 3875873359)
- R1-5 orphaned planNamesPr doc comment — already reported (comment 3875873364)
- R1-6 divergent duplicate of run.ts readStopSidecar — already reported (comment 3875873369)
- R1-7 dispositions[].evidence dead field — already reported (comment 3875873375)
- R1-8 stop disclosure rendered under the 'Not linted' prefix — already reported (comment 3875873381)
- R1-9 cleared-stop body opener certifies a review that never ran — already reported (comment 3875873387)
- R1-10 verdictLine dangling colon on the stop demotion — already reported (comment 3875873395)
- R1-11 stale run.ts comment contradicting the new gate — already reported (comment 3875873399)
- R1-12 clean-tree bullet keeps the contradictory legacy carve-out — already reported (comment 3875873401)
- R1-13 MIT license text dropped for a still-bundled package — already reported (comment 3875873405)
- R1-14 four untested refusal branches of the new gate — already reported (comment 3875873412)
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:277 — [probe] openLedgerCriticalIds returns [] (not null) for a parseable non-ledger cache — fail-open against its own contractpackages/cli/src/commands/review/compose-review.ts:243 — [probe] the runId fence never binds the stamped sidecar to the plan being re-ruledpackages/cli/src/commands/review/run.ts:752 — [review] the stop path fails open when the sidecar exists but the orchestrator never composedpackages/cli/src/commands/review/compose-review.test.ts:15578 — [probe] obfuscated 'REQUEEST_CHANGES'.replace('EE','E') event literal in the headline test
[Critical] R1-2: Still stands — the code at this anchor is byte-identical to round 1, so this blocker is re-posted under its original id (relocated from inline to the body: its original thread at compose-review.ts:3413 is a location overlap this round's presubmit deterministically drops). The body↔disposition cross-check is unsound in two ways: it reads the raw input.bodyCriticals (not the final rendered set), and it matches ledger ids by substring against the joined blob. (a) splitDeferralChannel relocates severity:'Critical' deferral entries into the local bodyCriticals copy (~line 3090) and ingest applies collapse/footer-strip after the grant IIFE runs, so a Critical riding deferredSuggestions titled 'R1-1: …' is invisible to the check — the grant fires and the rendered body posts a blocker carrying an id ruled fixed (and launders a brand-new Critical past the verify-floor exemption). (b) Substring matching means a prefix collision (R1-1 ⊂ R1-10) or a sibling id merely mentioned inside another entry's prose satisfies includes(id): with ledger open R1-1 + R1-10, ruling R1-1 fixed / R1-10 still-stands and re-asserting only R1-10 throws 'R1-1 … still carries its id' on a fully compliant composition (unsatisfiable every retry); omitting a still-stands id's own entry while a sibling's prose mentions it silently drops a standing blocker from the posted body. Witness: [probe, this round, live against the compiled code at this commit] ledger {R1-1 open, R1-10 open}, dispositions R1-1: fixed / R1-10: still-stands, body ['R1-10: …'] → THREW 'R1-1 is ruled fixed yet a body Critical still carries its id' (a valid re-rule can never compose); the mirror case (R1-1 still-stands, own re-assertion omitted) composed REQUEST_CHANGES without throwing. Fix: run the check over the final local array (below the relocation push and gate-repost dedup) and bind ids per entry — derive each entry's own leading R- token into a Set (cf. lib/ledger.ts LEDGER_ID_READBACK), then require still-stands ids ∈ set and fixed/superseded ids ∉ set; add tests where a still-stands entry whose verbatim text names a fixed sibling id composes, a still-stands id present only inside another entry's prose throws, and a deferred Critical titled with an id ruled fixed throws, and confirm reverting to substring matching turns them red.
中文说明
仍然成立——该锚点处的代码与第 1 轮逐字节相同,因此以原 id 重张此 blocker(因 presubmit 与 3413 行原线程位置重叠被确定性丢弃,从行内移入正文)。body↔disposition 交叉检查在两个方面不成立:它读取的是原始input.bodyCriticals(而非最终渲染集合),并且用子串方式在拼接后的整块文本里匹配台账 id。(a) splitDeferralChannel 会把 severity:'Critical' 的延期条目搬入本地 bodyCriticals 副本(约 3090 行),且 ingest 在这个 IIFE 之后才做折叠/去页脚处理,所以挂在 deferredSuggestions 里、标题为 'R1-1: …' 的 Critical 对该检查不可见——授权成立后,渲染出的正文会发布一条携带被判 fixed 的 id 的 blocker(还把一个全新 Critical 洗过 verify-floor 豁免)。(b) 对拼接文本做子串匹配意味着前缀冲突(R1-1 ⊂ R1-10)、或仅在另一条目正文中提到的兄弟 id 都能满足 includes(id)——于是当台账有 open R1-1+R1-10、判 R1-1 fixed/R1-10 still-stands 且只重张 R1-10 时,会对一个完全合规的组稿抛出 'R1-1 … still carries its id'(每次重试都无解);而省略某 still-stands id 自身条目、仅让兄弟条目正文提到它,就能把一条仍站立的 blocker 从发布正文里悄悄丢掉。证据:[探针,本轮,在本提交的编译产物上实时运行] 台账 {R1-1 open, R1-10 open}、dispositions R1-1: fixed / R1-10: still-stands、body ['R1-10: …'] → 抛出 'R1-1 is ruled fixed yet a body Critical still carries its id'(合规重裁永远无法组稿);镜像用例(R1-1 still-stands、省略其自身重张)未抛错即组出 REQUEST_CHANGES。修复:把检查放到最终本地数组上(在搬移 push 与 gate-repost 去重之后),并按条目绑定 id——取每个条目自身的起始 R- token 构成 Set(参见 lib/ledger.ts 的 LEDGER_ID_READBACK),再要求 still-stands 的 id ∈ 集合、fixed/superseded 的 id ∉ 集合;新增测试:正文逐字文本提到已修复兄弟 id 的 still-stands 条目必须能组稿、仅存在于他人正文中的 still-stands id 必须抛错、标题携带被判 fixed 的 id 的延期 Critical 必须抛错,并确认回退到子串匹配会使这些测试变红。 中文说明
本轮确认的 11 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
[Critical] R1-2: Still stands — the code at this anchor is byte-identical to round 1, so this blocker is re-posted under its original id (relocated from inline to the body: its original thread at compose-review.ts:3413 is a location overlap this round's presubmit deterministically drops). The body↔disposition cross-check is unsound in two ways: it reads the raw input.bodyCriticals (not the final rendered set), and it matches ledger ids by substring against the joined blob. (a) splitDeferralChannel relocates severity:'Critical' deferral entries into the local bodyCriticals copy (~line 3090) and ingest applies collapse/footer-strip after the grant IIFE runs, so a Critical riding deferredSuggestions titled 'R1-1: …' is invisible to the check — the grant fires and the rendered body posts a blocker carrying an id ruled fixed (and launders a brand-new Critical past the verify-floor exemption). (b) Substring matching means a prefix collision (R1-1 ⊂ R1-10) or a sibling id merely mentioned inside another entry's prose satisfies includes(id): with ledger open R1-1 + R1-10, ruling R1-1 fixed / R1-10 still-stands and re-asserting only R1-10 throws 'R1-1 … still carries its id' on a fully compliant composition (unsatisfiable every retry); omitting a still-stands id's own entry while a sibling's prose mentions it silently drops a standing blocker from the posted body. Witness: [probe, this round, live against the compiled code at this commit] ledger {R1-1 open, R1-10 open}, dispositions R1-1: fixed / R1-10: still-stands, body ['R1-10: …'] → THREW 'R1-1 is ruled fixed yet a body Critical still carries its id' (a valid re-rule can never compose); the mirror case (R1-1 still-stands, own re-assertion omitted) composed REQUEST_CHANGES without throwing. Fix: run the check over the final local array (below the relocation push and gate-repost dedup) and bind ids per entry — derive each entry's own leading R- token into a Set (cf. lib/ledger.ts LEDGER_ID_READBACK), then require still-stands ids ∈ set and fixed/superseded ids ∉ set; add tests where a still-stands entry whose verbatim text names a fixed sibling id composes, a still-stands id present only inside another entry's prose throws, and a deferred Critical titled with an id ruled fixed throws, and confirm reverting to substring matching turns them red.
中文说明
仍然成立——该锚点处的代码与第 1 轮逐字节相同,因此以原 id 重张此 blocker(因 presubmit 与 3413 行原线程位置重叠被确定性丢弃,从行内移入正文)。body↔disposition 交叉检查在两个方面不成立:它读取的是原始input.bodyCriticals(而非最终渲染集合),并且用子串方式在拼接后的整块文本里匹配台账 id。(a) splitDeferralChannel 会把 severity:'Critical' 的延期条目搬入本地 bodyCriticals 副本(约 3090 行),且 ingest 在这个 IIFE 之后才做折叠/去页脚处理,所以挂在 deferredSuggestions 里、标题为 'R1-1: …' 的 Critical 对该检查不可见——授权成立后,渲染出的正文会发布一条携带被判 fixed 的 id 的 blocker(还把一个全新 Critical 洗过 verify-floor 豁免)。(b) 对拼接文本做子串匹配意味着前缀冲突(R1-1 ⊂ R1-10)、或仅在另一条目正文中提到的兄弟 id 都能满足 includes(id)——于是当台账有 open R1-1+R1-10、判 R1-1 fixed/R1-10 still-stands 且只重张 R1-10 时,会对一个完全合规的组稿抛出 'R1-1 … still carries its id'(每次重试都无解);而省略某 still-stands id 自身条目、仅让兄弟条目正文提到它,就能把一条仍站立的 blocker 从发布正文里悄悄丢掉。证据:[探针,本轮,在本提交的编译产物上实时运行] 台账 {R1-1 open, R1-10 open}、dispositions R1-1: fixed / R1-10: still-stands、body ['R1-10: …'] → 抛出 'R1-1 is ruled fixed yet a body Critical still carries its id'(合规重裁永远无法组稿);镜像用例(R1-1 still-stands、省略其自身重张)未抛错即组出 REQUEST_CHANGES。修复:把检查放到最终本地数组上(在搬移 push 与 gate-repost 去重之后),并按条目绑定 id——取每个条目自身的起始 R- token 构成 Set(参见 lib/ledger.ts 的 LEDGER_ID_READBACK),再要求 still-stands 的 id ∈ 集合、fixed/superseded 的 id ∉ 集合;新增测试:正文逐字文本提到已修复兄弟 id 的 still-stands 条目必须能组稿、仅存在于他人正文中的 still-stands id 必须抛错、标题携带被判 fixed 的 id 的延期 Critical 必须抛错,并确认回退到子串匹配会使这些测试变红。 — qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 FENCE-PROBE debug instrumentation (re-confirmed this round at Suggestion via probe; the Critical escalation was rejected) — already reported (comment 3875873359)
- R1-5 orphaned planNamesPr doc comment — already reported (comment 3875873364)
- R1-6 divergent duplicate of run.ts readStopSidecar — already reported (comment 3875873369)
- R1-7 dispositions[].evidence dead field — already reported (comment 3875873375)
- R1-8 stop disclosure rendered under the 'Not linted' prefix — already reported (comment 3875873381)
- R1-12 clean-tree bullet contradiction (re-confirmed at Suggestion, low confidence; the Critical escalation was rejected) — already reported (comment 3875873401)
- R1-14 four untested refusal branches of the new gate (fence negative branch, superseded disposition, unreadable-cache refusal) — already reported (comment 3875873412)
- openLedgerCriticalIds returns [] (not null) for a parseable non-ledger cache — already recorded as round-2 deferral D2-1 (review 5048654441)
- stop path fails open when the sidecar exists but the orchestrator never composed (re-confirmed at Suggestion, low confidence; the Critical claim was capped — nondeterministic trigger, pre-existing hole this PR narrows) — already recorded as…
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/run.ts:757 — [review] stop-round rulings never persisted to the cache ledger — cleared blockers re-adjudicated on every clean-tree stoppackages/cli/src/commands/review/compose-review.ts:3368 — [probe] grant's baseline ledger is model-selectable via planPath — bind the capture's cachePath into the sidecar (defense-in-depth)
[Critical] R1-2: Still stands — the branch only merged main since round 2; the code is byte-identical, so this blocker is re-posted under its original id (relocated from inline to the body: its original thread at compose-review.ts:3413 is a location overlap this round's presubmit deterministically drops). The body↔disposition cross-check is unsound in two ways: it reads the raw input.bodyCriticals (not the final rendered set), and it matches ledger ids by substring against the joined blob. (a) splitDeferralChannel relocates severity:'Critical' deferral entries into the local bodyCriticals copy and ingest applies collapse/footer-strip after the grant IIFE runs, so a Critical riding deferredSuggestions titled 'R1-1: …' is invisible to the check — the grant fires and the rendered body posts a blocker carrying an id ruled fixed (and launders a brand-new Critical past the verify-floor exemption). (b) Substring matching means a prefix collision (R1-1 ⊂ R1-10) or a sibling id merely mentioned inside another entry's prose satisfies includes(id): with ledger open R1-1 + R1-10, ruling R1-1 fixed / R1-10 still-stands and re-asserting only R1-10 throws 'R1-1 … still carries its id' on a fully compliant composition (unsatisfiable every retry); omitting a still-stands id's own entry while a sibling's prose mentions it silently drops a standing blocker from the posted body. Witness: [probe, round 2, live against the compiled code; code byte-identical at this commit; re-detected by 8 agents this round] ledger {R1-1 open, R1-10 open}, dispositions R1-1: fixed / R1-10: still-stands, body ['R1-10: …'] → THREW 'R1-1 is ruled fixed yet a body Critical still carries its id' (a valid re-rule can never compose); the mirror case (R1-1 still-stands, own re-assertion omitted) composed REQUEST_CHANGES without throwing. Fix: run the check over the final local array (below the relocation push and gate-repost dedup) and bind ids per entry — derive each entry's own leading R- token into a Set (cf. lib/ledger.ts LEDGER_ID_READBACK), then require still-stands ids ∈ set and fixed/superseded ids ∉ set; add tests where a still-stands entry whose verbatim text names a fixed sibling id composes, a still-stands id present only inside another entry's prose throws, and a deferred Critical titled with an id ruled fixed throws, and confirm reverting to substring matching turns them red. 中文说明:仍然成立——第 2 轮之后分支只合并了 main,代码逐字节相同,因此以原 id 重张此 blocker(因 presubmit 与 3413 行原线程位置重叠被确定性丢弃,从行内移入正文)。body↔disposition 交叉检查在两个方面不成立:它读取的是原始 input.bodyCriticals(而非最终渲染集合),并且用子串方式在拼接后的整块文本里匹配台账 id。(a) splitDeferralChannel 会把 severity:'Critical' 的延期条目搬入本地 bodyCriticals 副本,且 ingest 在这个 IIFE 之后才做折叠/去页脚处理,所以挂在 deferredSuggestions 里、标题为 'R1-1: …' 的 Critical 对该检查不可见——授权成立后,渲染出的正文会发布一条携带被判 fixed 的 id 的 blocker(还把一个全新 Critical 洗过 verify-floor 豁免)。(b) 子串匹配意味着前缀冲突(R1-1 ⊂ R1-10)或仅在另一条目正文中提到的兄弟 id 都能满足 includes(id)——合规重裁永远无法组稿,而省略仍站立 blocker 的自身重张却能悄悄把它从发布正文里丢掉。修复:把检查放到最终本地数组上,并按条目绑定 id(取每个条目自身的起始 R- token 构成 Set),再要求 still-stands 的 id ∈ 集合、fixed/superseded 的 id ∉ 集合,并新增对应测试、确认回退到子串匹配会使测试变红。
中文说明
本轮确认的 9 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
[Critical] R1-2: Still stands — the branch only merged main since round 2; the code is byte-identical, so this blocker is re-posted under its original id (relocated from inline to the body: its original thread at compose-review.ts:3413 is a location overlap this round's presubmit deterministically drops). The body↔disposition cross-check is unsound in two ways: it reads the raw input.bodyCriticals (not the final rendered set), and it matches ledger ids by substring against the joined blob. (a) splitDeferralChannel relocates severity:'Critical' deferral entries into the local bodyCriticals copy and ingest applies collapse/footer-strip after the grant IIFE runs, so a Critical riding deferredSuggestions titled 'R1-1: …' is invisible to the check — the grant fires and the rendered body posts a blocker carrying an id ruled fixed (and launders a brand-new Critical past the verify-floor exemption). (b) Substring matching means a prefix collision (R1-1 ⊂ R1-10) or a sibling id merely mentioned inside another entry's prose satisfies includes(id): with ledger open R1-1 + R1-10, ruling R1-1 fixed / R1-10 still-stands and re-asserting only R1-10 throws 'R1-1 … still carries its id' on a fully compliant composition (unsatisfiable every retry); omitting a still-stands id's own entry while a sibling's prose mentions it silently drops a standing blocker from the posted body. Witness: [probe, round 2, live against the compiled code; code byte-identical at this commit; re-detected by 8 agents this round] ledger {R1-1 open, R1-10 open}, dispositions R1-1: fixed / R1-10: still-stands, body ['R1-10: …'] → THREW 'R1-1 is ruled fixed yet a body Critical still carries its id' (a valid re-rule can never compose); the mirror case (R1-1 still-stands, own re-assertion omitted) composed REQUEST_CHANGES without throwing. Fix: run the check over the final local array (below the relocation push and gate-repost dedup) and bind ids per entry — derive each entry's own leading R- token into a Set (cf. lib/ledger.ts LEDGER_ID_READBACK), then require still-stands ids ∈ set and fixed/superseded ids ∉ set; add tests where a still-stands entry whose verbatim text names a fixed sibling id composes, a still-stands id present only inside another entry's prose throws, and a deferred Critical titled with an id ruled fixed throws, and confirm reverting to substring matching turns them red. 中文说明:仍然成立——第 2 轮之后分支只合并了 main,代码逐字节相同,因此以原 id 重张此 blocker(因 presubmit 与 3413 行原线程位置重叠被确定性丢弃,从行内移入正文)。body↔disposition 交叉检查在两个方面不成立:它读取的是原始 input.bodyCriticals(而非最终渲染集合),并且用子串方式在拼接后的整块文本里匹配台账 id。(a) splitDeferralChannel 会把 severity:'Critical' 的延期条目搬入本地 bodyCriticals 副本,且 ingest 在这个 IIFE 之后才做折叠/去页脚处理,所以挂在 deferredSuggestions 里、标题为 'R1-1: …' 的 Critical 对该检查不可见——授权成立后,渲染出的正文会发布一条携带被判 fixed 的 id 的 blocker(还把一个全新 Critical 洗过 verify-floor 豁免)。(b) 子串匹配意味着前缀冲突(R1-1 ⊂ R1-10)或仅在另一条目正文中提到的兄弟 id 都能满足 includes(id)——合规重裁永远无法组稿,而省略仍站立 blocker 的自身重张却能悄悄把它从发布正文里丢掉。修复:把检查放到最终本地数组上,并按条目绑定 id(取每个条目自身的起始 R- token 构成 Set),再要求 still-stands 的 id ∈ 集合、fixed/superseded 的 id ∉ 集合,并新增对应测试、确认回退到子串匹配会使测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…sed-verdict-v2 # Conflicts: # packages/cli/src/commands/review/compose-review.test.ts
…10310) Address the round-3 review findings on the stop re-rule gate: - Enforce per-reason ruling constraints in the stopReRule grant: the capture's nothingToReview reason certifies what could have moved, so unchanged-since-last-round licences only still-stands, scope-emptied still-stands/superseded, clean-tree all three, and an unknown reason fails closed. - Move the body-disposition cross-check off the raw input onto the final local body set (past the deferral channel's relocation push and the gate-repost dedup) and bind ids per entry through the claim head's own leading token — the substring scan over the joined blob misfired on prefix collisions (R1-1 inside R1-10) and prose mentions, and never saw relocated Criticals. - Require every body Critical on a granted stop to carry exactly one still-stands-ruled ledger id (entry count equal to the still-stands count) and refuse inline Criticals on a stop re-rule, closing the verify-floor exemption's invented-entry and --comments-file holes. - Drop the FENCE-PROBE stderr instrumentation, the dispositions[] .evidence dead field, and the contradictory clean-tree carve-out sentence in SKILL Step 1; scope the run.ts exit-contract comments to the capture stops and disclose the PR-stop residual (up-to-date and empty-diff stops write only the sidecar, so a gate-only PR re-run exits 0 over the PR cache's open Criticals), pinned by a run test. - Restore the NOTICES.txt json-schema-traverse entry the offline regeneration replaced with placeholders (the CI freshness check's acceptance witness).
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10310This round merged Conflict resolution (
|
| Finding | Disposition | Change |
|---|---|---|
| R1-1 — grant never validates dispositions against the certified stop reason (rc:3875873344, rc:3878565522, rc:3880796103) | Fixed | planCarriesDecidedStop became decidedStopReason (returns the reason instead of discarding it) and the grant now enforces per-reason ruling constraints: unchanged-since-last-round → only still-stands; scope-emptied → still-stands/superseded; clean-tree → all three; unknown reason → refused fail-closed. The contradictory test 'a re-rule that cleared every blocker COMMENTS, never approves' was repointed at a clean-tree plan as requested. Reproduced first: the new throw-expecting tests were red on the pre-fix code (the old shape composed COMMENT over a certified-standing blocker). |
| R1-2 — body↔disposition cross-check unsound: raw input + substring match (rc:3875873347, re-posted in review bodies rounds 2–3) | Fixed | The check moved out of the grant IIFE onto the FINAL local body set — past the deferral channel's relocation push and the gate-repost dedup — and binds ids PER ENTRY through the claim head's own leading token (the same readClaim/head-slot readback the ledger builder applies; relocated entries bind through their typed title). Reproduced first: with ledger open R1-1+R1-10, ruling R1-1 fixed/R1-10 still-stands and re-asserting only R1-10 threw on the pre-fix code (unsatisfiable every retry) and now composes REQUEST_CHANGES; the mirror shape (a still-stands id present only inside another entry's prose) composed silently before and now throws; a deferred Critical titled with an id ruled fixed (invisible to the old check via relocation) now throws. |
| R1-3 — verify-floor exemption admits invented entries and inline Criticals (rc:3875873354, rc:3878565529, rc:3880796108) | Fixed | Every body Critical on a granted stop must boundary-carry exactly one still-stands-ruled ledger id (entry without an id, or with an id no ruling covers → refused; entry count must equal the still-stands count, so duplicate re-assertions are refused too), and criticalsInline > 0 is refused on a granted stop round. Reproduced first: the invented-entry and criticalsInline: 1 shapes composed REQUEST_CHANGES on the pre-fix code; both throw now. |
| R3-1 — NOTICES.txt placeholder for a still-bundled package fails the CI freshness check (rc:3880796119; escalated from R1-13 rc:3875873405) | Fixed | Regenerated with npm run generate:notices --workspace=qwen-code-vscode-ide-companion after syncing dependencies: the json-schema-traverse@1.0.0 repository URL and full MIT text are restored (23 insertions / 2 deletions), and the regenerated file is byte-identical to origin/main's — the failed offline-regeneration hunk is fully reverted, so the 'Check VS Code companion notices are up-to-date' step's regeneration matches the committed file. This also addresses the red Test (ubuntu-latest Node 22.x) check, which failed with Error: NOTICES.txt is out of date. on the reviewed commit. |
| R3-2 — exit-contract comment claims every stop with open Criticals composes, but PR-target stops never compose (rc:3880796127) | Fixed via the reviewer's disclosure option | Verified in code: the PR stops (up-to-date, empty-diff) write only the stop sidecar and consume no plan, so the stopReRule grant is unreachable there and a gate-only PR re-run exits 0 over the PR cache's open Criticals (completed = composed ?? stop, event = null → exit 0; pinned by a new run test reproducing exactly that shape). Extending the composed contract to PR stops needs a new grant form plus SKILL orchestration changes — beyond this round's bound — so this round implements the finding's second accepted option: both run.ts contract comments are scoped to the three capture stops and disclose the PR-stop residual in the code itself, and the stale universal claim in run.test.ts was scoped the same way (R3-3). The residual pin test keeps the disclosure honest. |
Suggestions — seven resolved this round
- R1-4 (rc:3875873359): both
FENCE-PROBEstderr writes deleted; the fail-closedreturn falsein the readdir catch stays. - R1-5 (rc:3875873364): the orphaned 'Does this plan name a pull request?' doc comment restored to
planNamesPr. - R1-7 (rc:3875873375): the dead
dispositions[].evidencefield deleted (no read site anywhere). - R1-11 (rc:3875873399): the stale run.ts comment block contradicting the new gate rewritten to the new contract (a stop whose ledger holds nothing open composes none; a stop with open Criticals composes one via Step 1's re-rule).
- R1-12 (rc:3875873401): the contradictory pre-PR carve-out sentence deleted from the SKILL clean-tree bullet; its still-useful half (open Suggestions are re-listed and block nothing) folded into the judged re-rule prescription.
- R1-14 (rc:3875873412): one test per untested refusal branch — stale-runId sidecar refused;
supersededgrants without a body entry and refuses with one; missing/empty and nonexistentcachePathrefused; duplicate disposition refused. - R3-3 (rc:3880796132): the test comment opener scoped to the subcase the test exercises.
Suggestions — deferred to the next round (batch bound)
Implemented 12 findings this round (5 Critical + 7 Suggestion) under the ~8-per-round bound the Criticals forced; the remainder are deferred explicitly, not dropped:
- R1-6 (rc:3875873369) — dedup
stopSidecarFenced's sidecar read against run.tsreadStopSidecar. - R1-8 (rc:3875873381) — render the stop disclosure through its own block instead of the lint-specific
gateDisclosedprefix. - R1-9 (rc:3875873387) — gate the COMMENT opener on the grant (cleared stops open 'Reviewed — no blockers.' today).
- R1-10 (rc:3875873395) — give the stop demotion a nameable
verdictLinereason instead of the dangling colon.
Each has a reply on its own thread with the reason; all four are independent of this round's changes.
Not addressable in this environment
- rv:5042584510 (triage stage 1a): the PR body needs restructuring to the PR template (
What this PR does/Why it's needed/Reviewer Test Planwith Before/After evidence and Tested-on matrix /Risk & Scope/Linked Issues). This requires editing the PR body on GitHub, which this workflow cannot do (no GitHub write access); flagged for the maintainer or a workflow path that owns PR-body edits. The substance already exists in the body per the triage comment, so it is a restructure plus a before/after evidence block for the gate miss. - rv:5045675051: partial-review disclosure (tool/time budget) — informational, carries no actionable finding.
Verification
Commands actually run this round, in order:
- Reproduction (red phase):
cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts -t "decided-stop re-rule"— 9 new defect tests FAILED on the pre-fix code as expected (R1-1 ×3, R1-2 ×3, R1-3 ×3); 4 branch-witness tests green pre-fix as expected. cd packages/cli && npx vitest run src/commands/review/run.test.ts -t "stop"— 8 passed, including the new PR-stop residual pin (exit 0 with sidecar only) reproducing R3-2's claimed shape.- Mutation probes (6): per-reason constraint removed → 2 witnesses red; unknown-reason refusal disabled → 1 red; criticalsInline refusal removed → 1 red; per-entry binding reverted to the old substring check → 3 red; count-equality guard removed → 1 red; coverage loop removed → 2 red; file restored after each probe; full suite green after restore.
npm run build— passed (after fixing one strict-null narrowing error the first build surfaced in the new check).npm run typecheck— passed.npm run lint— passed.npx prettier --checkon the five touched source files — all formatted.cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts— 641 passed.cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/run.test.ts— 701 passed.cd packages/cli && npx vitest run src/commands/review/— 113 files, 5534 passed, 17 skipped.cd packages/cli && npx vitest run src/commands/review.test.ts— 5 passed.cd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts— 56 passed.npm run generate:notices --workspace=qwen-code-vscode-ide-companion— regenerated;diffvsorigin/main's NOTICES.txt: whole file byte-identical (the placeholder hunk fully reverted).- No integration-test run: no integration harness exercises the stop re-rule path (checked
integration-tests/forstopReRule/review runreferences — none); the touched behavior is covered by the package unit suites above.
中文说明
Autofix 审查轮次 — PR #10310
本轮先合并了 origin/main（一处冲突，见下），随后处理未决的审查发现：全部 5 条 Critical 与 7 条 Suggestion 已在本轮以代码解决；另有 4 条 Suggestion 按每轮批次上限明确延后到下一轮。
冲突解决（--conflict true）
origin/main 已越过本分支上次合并基线（四个 review/web-shell/CI 提交）。合并仅在 packages/cli/src/commands/review/compose-review.test.ts 出现一处内容冲突：双方在文件同一位置各自追加了新的 describe 块（本分支的 decided-stop re-rule 套件 vs main 的 Critical deferral by axes at the critical floor (#10291) 套件）。解决方式为完整保留两个块——它们是主题互不重叠的独立测试套件；git 抽出的公共收尾括号已分别归还两侧。
Critical 发现 — 全部解决
| 发现 | 处置 | 变更 |
|---|---|---|
| R1-1 —— 授权从未把 dispositions 与 capture 认证的 stop 原因做交叉校验（rc:3875873344、rc:3878565522、rc:3880796103） | 已修复 | planCarriesDecidedStop 改为 decidedStopReason（返回原因而非丢弃），授权内按 reason 强制 ruling 约束：unchanged-since-last-round → 只允许 still-stands；scope-emptied → still-stands/superseded；clean-tree → 三者皆可；未知 reason → fail closed 拒绝。矛盾形状的测试 'a re-rule that cleared every blocker COMMENTS, never approves' 已按要求改指向 clean-tree plan。先复现：新增的断言抛错测试在修复前为红（旧形状会让认证仍站立的 blocker 组出 COMMENT）。 |
| R1-2 —— body↔disposition 交叉检查不成立：读原始输入 + 子串匹配（rc:3875873347，第 2–3 轮以正文重张） | 已修复 | 该检查从授权 IIFE 移到最终本地 body 集合上执行——位于延期通道搬移 push 与 gate 重发去重之后——并按条目绑定 id：走 claim head 自身的起始 token（与 ledger 构建器所用的 readClaim/head-slot 读回一致；搬移条目走其带类型的 title）。先复现：台账 open R1-1+R1-10、判 R1-1 fixed/R1-10 still-stands 且只重张 R1-10 时，修复前抛错（每次重试都无解），现在组出 REQUEST_CHANGES；镜像形状（still-stands id 只出现在另一条目正文中）修复前静默组稿、现在抛错；标题携带被判 fixed 的 id 的延期 Critical（旧检查因搬移而不可见）现在抛错。 |
| R1-3 —— verify-floor 豁免放行捏造条目与行内 Critical（rc:3875873354、rc:3878565529、rc:3880796108） | 已修复 | 已授权 stop 上的每条 body Critical 必须以边界方式恰好携带一个 still-stands 裁决的台账 id（无 id 或 id 不在裁决内的条目 → 拒绝；条目数必须等于 still-stands 数，因此重复重张也被拒绝），且已授权 stop 轮中 criticalsInline > 0 一律拒绝。先复现：捏造条目与 criticalsInline: 1 两种形状在修复前都能组出 REQUEST_CHANGES，现在均抛错。 |
| R3-1 —— NOTICES.txt 对仍在打包的包留下占位符，确定性使 CI 新鲜度检查失败（rc:3880796119；由 R1-13 rc:3875873405 升级） | 已修复 | 在同步依赖后运行 npm run generate:notices --workspace=qwen-code-vscode-ide-companion 重新生成：json-schema-traverse@1.0.0 的仓库 URL 与完整 MIT 文本已恢复（23 行插入 / 2 行删除），且重生成文件与 origin/main 逐字节相同——失败的离线重生成 hunk 被完全还原，'Check VS Code companion notices are up-to-date' 步骤的重生成将与提交文件一致。这同时修复了红色的 Test (ubuntu-latest Node 22.x) 检查——它在被审提交上以 Error: NOTICES.txt is out of date. 失败。 |
| R3-2 —— 退出契约注释声称每个有 open Critical 的 stop 都会 compose，但 PR 目标的 stop 从不 compose（rc:3880796127） | 按审阅者给出的披露选项修复 | 代码核实：PR stop（up-to-date、empty-diff）只写 stop sidecar、不消费 plan，因此 stopReRule 授权在那里构造上不可达，仅门控的 PR 重跑会对 PR 缓存中仍 open 的 Critical 退 0（completed = composed ?? stop、event = null → 退 0；新增的 run 测试精确复现了该形状）。把 composed 契约扩展到 PR stop 需要新的授权形式加 SKILL 编排改动——超出本轮边界——因此本轮采用该发现接受的第二个选项：run.ts 两处契约注释均限定为三个 capture stop，并在代码中披露 PR stop 残余；run.test.ts 中过时的全称断言同步限定（R3-3）。残余钉桩测试保证披露与行为一致。 |
Suggestion — 本轮解决七条
- R1-4（rc:3875873359）：删除两处
FENCE-PROBEstderr 写入；readdir catch 中 fail-closed 的return false保留。 - R1-5（rc:3875873364）：把孤立的 'Does this plan name a pull request?' 文档注释还原给
planNamesPr。 - R1-7（rc:3875873375）：删除死字段
dispositions[].evidence（全仓库无读取点）。 - R1-11（rc:3875873399）：把与新门矛盾的 run.ts 陈旧注释块改写为新契约（台账无 open 的 stop 完成时无 composed 裁决；有 open Critical 的 stop 经 Step 1 重裁产出一个）。
- R1-12（rc:3875873401）：删除 SKILL clean-tree 条目中与 PR 前保留的矛盾豁免句；其中仍有用的一半（open Suggestions 重新列出、不阻断）并入判断式重裁处方。
- R1-14（rc:3875873412）：为四个未测拒绝分支各加一个测试——过期 runId sidecar 拒绝；
superseded无 body 放行、带 body 拒绝；缺失/空与不存在的cachePath拒绝；重复 disposition 拒绝。 - R3-3（rc:3880796132）：把测试注释开头句限定为该测试实际覆盖的子情形。
Suggestion — 延后到下一轮（批次上限）
本轮在 Critical 占满 ~8 条/轮上限的情况下实现了 12 条发现（5 Critical + 7 Suggestion）；其余明确延后、不丢失：
- R1-6（rc:3875873369）—— 把
stopSidecarFenced的 sidecar 读取与 run.ts 的readStopSidecar去重。 - R1-8（rc:3875873381）—— stop 披露改用独立块渲染，不再复用 lint 专用的
gateDisclosed前缀。 - R1-9（rc:3875873387）—— 把 COMMENT 开场白也置于授权门控之下（已清空的 stop 目前以 'Reviewed — no blockers.' 开场）。
- R1-10（rc:3875873395）—— 给 stop 降级一个可命名的
verdictLine原因，消除悬空冒号。
每条的线程都有说明延后原因的回复；四条均与本轮改动相互独立。
本环境无法处理
- rv:5042584510（triage stage 1a）：PR 正文需要按 PR 模板重组（
What this PR does/Why it's needed/Reviewer Test Plan（含 Before/After 证据与 Tested-on 矩阵）/Risk & Scope/Linked Issues）。这需要编辑 GitHub 上的 PR 正文，本工作流无此权限（无 GitHub 写权限）；已标记给维护者或拥有 PR 正文编辑能力的工作流路径处理。按 triage 评论，正文实质内容已齐备，因此是一次重组外加 gate 失效的 before/after 证据。 - rv:5045675051：部分审查披露（工具/时间预算）——信息性，无可执行发现。
验证
本轮实际执行的命令（按顺序）：
- 复现（红阶段）：
cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts -t "decided-stop re-rule"—— 9 条新缺陷测试在修复前按预期失败（R1-1 ×3、R1-2 ×3、R1-3 ×3）；4 条分支见证测试修复前按预期为绿。 cd packages/cli && npx vitest run src/commands/review/run.test.ts -t "stop"—— 8 条通过，含新增的 PR stop 残余钉桩（仅 sidecar 时退 0），复现 R3-2 所述形状。- 突变探针（6 个）：移除按 reason 约束 → 2 条见证变红；关闭未知 reason 拒绝 → 1 条变红；移除 criticalsInline 拒绝 → 1 条变红；把按条目绑定回退为旧子串检查 → 3 条变红；移除计数相等守卫 → 1 条变红；移除覆盖循环 → 2 条变红；每次探针后恢复原文件，恢复后全套件转绿。
npm run build—— 通过（首次构建暴露新检查中的一处严格空值收窄错误，已修复）。npm run typecheck—— 通过。npm run lint—— 通过。- 对 5 个改动的源文件运行
npx prettier --check—— 全部符合格式。 cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts—— 641 条通过。cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/run.test.ts—— 701 条通过。cd packages/cli && npx vitest run src/commands/review/—— 113 个文件，5534 条通过，17 条跳过。cd packages/cli && npx vitest run src/commands/review.test.ts—— 5 条通过。cd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts—— 56 条通过。npm run generate:notices --workspace=qwen-code-vscode-ide-companion—— 重生成；与origin/main的 NOTICES.txtdiff比对：整个文件逐字节相同（占位符 hunk 完全还原）。- 未运行集成测试：集成测试没有任何覆盖 stop 重裁路径的用例（已在
integration-tests/检索stopReRule/review run—— 无）；改动的行为由上述包内单测套件覆盖。
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 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/模型 qwen3.8-max
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- Obfuscated 'REQUEEST_CHANGES'.replace('EE','E') event literal in the headline re-rule test — already recorded as round-2 deferral D2-4 (review 5048654441)
- Stop disclosure rendered under the 'Not linted (tool limitation)' prefix via gateDisclosed — already reported as R1-8 (comment 3875873381)
- verdictLine dangling colon on the stop APPROVE→COMMENT demotion — already reported as R1-10 (comment 3875873395)
- Stop path exits 0 when the sidecar exists but no verdict composed (a refused or failed re-rule) — already recorded as round-2 deferral D2-3, re-recorded in round 3 (review 5051329062)
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 4, not a blocker) — recorded, not requested in this round:
packages/core/src/skills/bundled/review/SKILL.test.ts:1657 — [probe] the clean-tree compose directive is pinned by no assertion — deleting the clause keeps all 56 tests green (deferred under the code-age rule: anchor unchanged since round 3…packages/cli/src/commands/review/compose-review.ts:3631 — [probe] stopReRule: null crashes with an opaque TypeError instead of the designed refusal (deferred under the code-age rule: anchor unchanged since round 3)
Convergence: round 4 posted 7 inline comment(s), 7 of them reported for the first time; the previous round posted 5 (3 new). Findings keep coming back to the same files: packages/cli/src/commands/review/compose-review.ts (findings in round 1; 5 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 7 条行内评论，其中 7 条是首次提出；上一轮发布了 5 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/compose-review.ts（第 1 轮已出过发现，本轮又有 5 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
…ions by content (#10310) Address the round-4 review findings on the stop re-rule gate: - The baseline read failed open on malformed cache ledgers — the last fail-open in an otherwise fail-closed gate. A present-but-non-array findings field read as an empty ledger, and schema-violating entries (drifted severity, missing status, null rows) were silently skipped; both shapes shrink the completeness baseline below the ledger's real open set, so the grant issued over Criticals it could not enumerate. Re-validate every entry of the model-written cache and refuse an unreadable baseline, exactly like every sibling path in the function. - Bind each still-stands body Critical by CONTENT, on both the ingested and the relocated leg: the claim title read back from the entry must equal the title the ledger recorded under that id, so a brand-new claim can no longer wear a verified id's exemption. A re-assertion the ledger recorded no title for keeps its id binding but loses the verify-floor exemption — its Critical rides the regular floor and the verdict softens to disclosed-but-not-blocking. - Index the ruling table with Object.hasOwn: a prototype-chain stop reason (__proto__, constructor) resolved through the table's prototype past the undefined guard and crashed with an opaque TypeError instead of the designed unknown-reason refusal. - Witnesses for the load-bearing paths that had none: a stop round with two still-standing Criticals (the per-entry binding loop only ever ran at N=1), a leading invisible-residue re-assertion (the strip before the id readback), the licence table's superseded-refusal cell under unchanged-since-last-round, and the per-reason licence loop over more than one disposition — each verified red against its mutation.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round 4 address summary — PR #10310Resolved all 7 inline findings from round 4 (1 Critical, 6 Suggestions) in one batch. No conflict resolution was needed ( Feedback points and decisions
Review body [rv:5053943902] listed four already-reported findings (D2-4, R1-8, R1-10, D2-3) and two convergence-posture deferrals; none of those is new work this round, so nothing in it required code changes. ChangesR4-1 (Critical). R4-2. R4-3. The ruling-table lookup uses R4-4. Added a positive grant test with two open Criticals, both ruled R4-5. Added a grant test whose body entry opens with a ZWSP ( R4-6. Added the refusal test for a R4-7. Added a Test-side adjustment. The Mutation probes (each new guard's witness)Every guard/branch this commit adds was probed: the mutation was applied, the focused suite rerun, the killing test recorded, and the guard restored.
All 13 mutants were killed; the restored state reran green (31 passed). Verification
Commit: 中文说明第 4 轮处理总结 — PR #10310第 4 轮的 7 条行内发现（1 条 Critical、6 条 Suggestion）已在同一批次中全部解决。无需冲突处理（ 反馈点与决定
审查正文 [rv:5053943902] 列出了 4 条已报告过的发现（D2-4、R1-8、R1-10、D2-3）以及 2 条收敛姿态延后项；它们都不是本轮的新工作，因此正文本身不需要任何代码改动。 改动内容R4-1（Critical）。 R4-2。 R4-3。 裁决表查找改用 R4-4。 新增正向授权用例：两条 open Critical、均判 R4-5。 新增授权用例：body 条目以 ZWSP 开头（ R4-6。 新增 R4-7。 新增 测试侧调整。 突变探针（每个新守卫的见证）本提交新增的每个守卫/分支都做了探针：套用突变、重跑聚焦套件、记录击杀用例、恢复守卫。
13 个突变体全部被击杀；恢复后的状态重跑为绿（31 通过）。 验证
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-6 stopSidecarFenced duplicating run.ts readStopSidecar — already reported (comment 3875873369)
- Obfuscated 'REQUEEST_CHANGES'.replace('EE','E') event literal in the headline re-rule test — already recorded as round-2 deferral D2-4 (review 5048654441)
- R1-8 stop disclosure rendered under the 'Not linted (tool limitation)' prefix via gateDisclosed — already reported (comment 3875873381)
- R1-9 cleared-stop body opener certifying a review that never ran — already reported (comment 3875873387)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6c": none — though I did not execute the test suite to confirm the new tests pass at HEAD (read-only audit; no tool-budget pressure, I simply did not run them)..
4 Suggestion(s) were drafted inline past the resolved critical posting floor — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s); the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 5, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:3849 — [review] R5-3: A Critical the deferral channel keeps DEFERRED (instead of relocated) never reaches the body↔disposition cross-check — only bodyCriticals and relocatedEntries are …packages/cli/src/commands/review/compose-review.ts:3850 — [review] R4-5: (fix-induced) The relocated leg applies readClaim(entry.title) bare, omitting the LEADING_INVISIBLE_RE strip the own-body leg applies three lines above. A still-st…packages/cli/src/commands/review/compose-review.ts:318 — [review] R5-4: The legacy-cache branch of openLedgerCriticalEntries — findings key absent, empty ledger, grant — has no test; the adjacent non-array branch is tested ('findings-no…packages/cli/src/commands/review/compose-review.ts:3734 — [review] R5-5: Duplicate ids in the model-written ledger collapse silently in the set-based completeness check ( new Set(ledger.map((e) => e.id)) ), and ledgerTitles.set is last-wi…packages/cli/src/commands/review/compose-review.ts:3664 — [probe] stopReRule: null throws a raw TypeError instead of the designed refusal (deferred under the code-age rule: anchor unchanged since round 4)packages/cli/src/commands/review/compose-review.ts:3717 — [probe] both stopReRule input-shape guards have zero test coverage (deferred under the code-age rule: guards predate the round-4 anchor)packages/cli/src/commands/review/compose-review.ts:3751 — [probe] scope-emptied admits superseded without consulting the plan's supersededPaths (deferred under the code-age rule: anchor unchanged since round 4)packages/cli/src/commands/review/compose-review.ts:1174 — [test] the stopReRule interface-field hunk ships with no vitest gate (deferred under the code-age rule: anchor unchanged since round 4)packages/cli/src/commands/review/compose-review.ts:3943 — [test] the granted-stop coverage branch ships with no test gate (deferred under the code-age rule: anchor unchanged since round 4)
Convergence: round 5 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 7 (7 new). Findings keep coming back to the same files: packages/cli/src/commands/review/compose-review.ts (findings in round 4; 4 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 6c"：none — though I did not execute the test suite to confirm the new tests pass at HEAD (read-only audit; no tool-budget pressure, I simply did not run them).。
4 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论——发布下限因首次发现速率连续 2 轮未下降而提前生效；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 5 轮，非阻断）——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 7 条（其中 7 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/compose-review.ts（第 4 轮已出过发现，本轮又有 4 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 8/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 8/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33388791397 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
The decided-stop grant re-read the plan and the cache once per consumer:
the fence hashed the cache in one readFileSync and the ledger enumeration
read it again in another, so nothing bound the bytes the fence certified
to the bytes the grant enumerated (R10-2's settled race). The grant now
takes ONE snapshot of both files and every check is a projection of it.
Hardening rounds 6-12 folded into the same snapshot pass:
- R8-1: a plan carrying the capture's nothingToReview decision composed
WITHOUT stopReRule walked the regular floors into a non-blocking
artifact that run.ts read as completion. A decided stop now composes
only through its re-rule; composing one without stopReRule refuses.
- R9-1: openLedgerCriticalEntries accepted ANY string for status and
filtered on === 'open', so a drifted row ('oppn') silently left the
baseline. The status vocabulary is enum-checked; a drift is an
unreadable baseline, never a skipped row.
- R9-2: the critical floor's reroute moved drafted Criticals into the
deferral channel before the grant read criticalsInline, and the moved
entries posted without reaching the body<->disposition bind. A granted
stop now refuses any Critical riding either deferral leg.
- R9-3: scope-emptied licensed 'superseded' as a class without reading
the deduction's input. Each superseded disposition is machine-checked
against the plan's incremental.scope.supersededPaths via the ledger
row's cited file; a mismatch or a file-less row refuses.
- R7-1: the relocated leg kept an unvouched re-assertion's
deterministic-source credit, defeating the unverified softening the
own-leg correction enforces. Unvouched relocated entries now lose it.
- R11-3: the relocated leg bound only the first line of a multi-line
title, letting a matching line 1 smuggle new claims in its tail. The
leg now binds the COLLAPSED title (plus the leading-invisible strip),
symmetric with the own leg's ingest.
- R11-2: input.presubmit's downgrade arms stayed reachable on a granted
stop, so a model-written downgradeRequestChanges moved a
certified-standing blocker to COMMENT. No presubmit runs on a stop
round; both arms are now closed there.
- R12-1: with no published run id the fence was skipped outright,
leaving every interactive grant gated only by model-supplied inputs.
The sidecar now binds regardless - only the run-id equality is waived
when no id is published - and a null-parsing sidecar or a null
stopReRule gets the designed refusal instead of a bare TypeError.
Each guard is pinned by a test that goes red when the guard is removed
(mutation-checked for R7-1, R11-2, R12-1 in-session).
The stop DECISION reads the --cache-resolved ledger (cachePathEarly), but the stop stamp and the plan's published cachePath always named the canonical .qwen/review-cache path. Whenever the two differed (a file-form --cache outside the canonical dir), the fence faithfully verified a baseline the stop never saw: an ENOENT null hash over a nonexistent canonical file, an empty grant baseline, and an exit 0 over the open Critical the stop had just consumed (R7-2, carrying its directory-form twin R10-1). One resolved value now feeds the decision, the stamp, the hash, and the plan's cachePath. A capture that decides NO stop also unlinks the target's stale stop sidecar (the retirement half of R10-4): an earlier round's sidecar at this stable name stayed fence-valid after a later capture proved the tree moved, and a hand-written stop plan could ride it. Absent is the truthful state.
…exit Two gate-consumer holes in review run: - R10-5: readComposed accepted any file carrying a string event - no provenance at all, only the mtime window - while the sibling stop sidecar was already runId-fenced. compose-review now echoes the parent's published run id into the composed artifact, and readComposed requires the stamp: an artifact this run's compose did not write reads as no verdict, never as this round's exit code. - R8-2: the sidecar-alone completion (up-to-date / empty-diff) was keyed on the reason STRING alone. capture-local never writes those reasons, so a local/file sidecar wearing one was a forged or drifted stamp that completed the round with no composed artifact - exit 0 over whatever the local cache held open. The exemption is now keyed on the PR target class beside the reason.
The rewritten unchanged-since-last-round bullet keyed its two compose
branches on 'open Criticals exist' and 'no open findings', leaving a
Suggestions-only ledger in NEITHER branch: the model stopped without
composing, run.ts read a decided stop with no composed artifact, and the
round exited 1 ('Review did not complete') on every unchanged re-run - a
standing wedge with nothing open to fix (R11-1). The branch now keys on
'no open Criticals', symmetric with the scope-emptied and clean-tree
siblings, and SKILL.test.ts pins the keying in both directions.
…clause Three recurring nits from the review rounds' deferral lists: - The decided-stop round-kind disclosure rode gateDisclosed, whose only renderer wraps every entry in 'Not linted (tool limitation, not a blocker)' - a round kind is not a linting gap. It now renders through its own bilingual block on both events a granted stop can produce. - The stop demotion is the one APPROVE->COMMENT mover with an empty cappedBy and no presubmit downgrade, and verdictLine joined the empty reason list into 'an Approve was NOT available: ' - a dangling colon over nothing. The empty-reasons case now names the stop re-rule. - run.ts's stop-read comment still described the pre-#9908 contract ('a stop whose ledger holds nothing open composes none'), contradicting the exit contract four lines below it. Rewritten to match.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- null/non-object sidecar shape guard has no fixture (compose-review.test.ts:16124) — already reported in round 6 (review 5057155855), re-recorded rounds 7 and 11 (U11-2)
- findingsHash fence null-stamp 'file appearing since' direction untested (compose-review.test.ts:16485) — already recorded as a round-7 deferral (review 5058058758), re-recorded rounds 9-10
- duplicate open-Critical ids collapse silently in the completeness check (compose-review.ts:480) — already recorded as round-5 deferral R5-5 (review 5056355996), re-recorded rounds 9-10 as RA-M
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Test Plan (not a blocker): src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory; 5686 tests green — this review observed 1869, 1741, 503 passed.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/run.ts:836 — [review] stamp-fence rejection still reported as 'could not be parsed' in the no-verdict prosepackages/cli/src/commands/review/run.ts:783 — [review] PR-stop exemption conjuncts unpinned: forged-reason PR sidecar and empty-diff disjunct mutations ship green (empty-diff half re-records round-7 deferral F3)packages/core/src/skills/bundled/review/SKILL.test.ts:60 — [review] 'composes EVERY decided stop' pins no string unique to the nothing-open compose sentences (SKILL.md:118/121)packages/cli/src/commands/review/compose-review.ts:7455 — [review] composed-artifact runId stamp write side has no handler testpackages/cli/src/commands/review/compose-review.test.ts:16413 — [review] reason-departure fence pinned one direction only (plan-wider case never fixtured)packages/cli/src/commands/review/compose-review.ts:4970 — [review] APPROVE-arm presubmit-downgrade guard (!stopReRuleGranted) pinned by no testpackages/cli/src/commands/review/compose-review.ts:6587 — [review] COMMENT-side stopRoundBlock disclosure unpinned (cleared-stop body loses 'no review agents ran')packages/core/src/skills/bundled/review/SKILL.test.ts:1721 — [review] clean-tree open-Criticals compose directive + evidence clause (SKILL.md:121) pinned by no assertionpackages/core/src/skills/bundled/review/SKILL.test.ts:1743 — [review] negative revert-guard mis-aimed at a casing that never existed ('When' vs 'Only when')
Convergence: round 13 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 15 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/review/compose-review.ts (findings in rounds 6, 7, 8, 9, 10, 11, 12; 3 more now); packages/cli/src/commands/review/capture-local.ts (findings in rounds 7, 10; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
Test Plan（非阻断）：src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory; 5686 tests green — this review observed 1869, 1741, 503 passed。
收敛姿态下延后（第 13 轮，非阻断）——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 13 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 15 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/compose-review.ts（第 6、7、8、9、10、11、12 轮已出过发现，本轮又有 3 条）；packages/cli/src/commands/review/capture-local.ts（第 7、10 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
Round-13 findings, all in the decided-stop path: - R13-1: the stop stamp hashed a SECOND disk read of the ledger, so an edit landing in the decision->stamp window was baked into the stamp and invisible to the compose fence. capture-local now reads the --cache ledger's bytes ONCE - the decision parses that buffer and the stamp hashes it (raw bytes, malformed JSON included); only the no---cache canonical path still reads the disk at stamp time, where the decision consulted no ledger. Spy-witnessed: a mutated second read no longer moves the stamp. - R13-2: the superseded deduction validated the plan's supersededPaths - model-editable state the fence never bound. The capture now stamps the split into the sidecar on every scope-emptied stop (interactive shape included) and the fence refuses a plan whose split departs from the stamp, failing closed on stamp absence for that reason. - R13-3 (interim; the structural close is #10654): a granted interactive (no-run-id) sidecar is consumed once the full grant passes - nothing else ever reads it, and left on disk it re-licensed the same plan on a later, moved tree. A refusal leaves it for the corrected retry; the gated sidecar stays for the parent, whose runId fence already refuses cross-run replays. - R1-8 (re-escalation): the cleared stop's COMMENT opened 'Reviewed - no blockers.' two paragraphs above its own 'no review agents ran this round' disclosure. Granted stops take their own opener ahead of the certifying chain: 'Re-rule of standing findings - no new review ran.' Also pinned from the deferral backlog: a sidecar parsing to JSON null gets the designed refusal, and a cache file APPEARING after a null-hash stamp is refused as a moved ledger.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/review/compose-review.ts:4169 — [probe] Critical [fails-closed] [new-surface] 交互式 stop sidecar 在授权后、四个后续状态形状验证之前被消费——修正重试被栅栏永久拒绝packages/cli/src/commands/review/compose-review.ts:7518 — [probe] 组稿工件 runId 回写（readComposed 栅栏的写半侧）无测试证人——变异删除后 822/822 仍绿packages/cli/src/commands/review/compose-review.ts:5016 — [probe] presubmit downgradeApprove 臂的 !stopReRuleGranted 守卫无测试证人（孪生臂有）packages/cli/src/commands/review/capture-local.ts:1197 — [probe] 无 --cache 时对已存在规范台账的盖章分支未被测试——no-op 变异绿色上线并使该类重裁被栅栏拒绝packages/cli/src/commands/review/capture-local.ts:1244 — [probe] 决策块前抛错的 capture 不执行陈旧 sidecar unlink——留下栅栏有效盖章，树移动后可被手写计划骑行packages/cli/src/commands/review/compose-review.ts:3898 — [probe] cannotTellCriticals 是 stop 轮唯一未关闭的 Critical 级通道——伪造条目在重张轮渲染并加上限packages/cli/src/commands/review/compose-review.ts:6554 — [probe] contextUnavailable 分支漏渲染 stop 开头句——清空 stop 以 'Reviewed diff-only' 开头packages/cli/src/commands/review/compose-review.test.ts:15952 — [review] dispositions 数组守卫与逐条 id/ruling 守卫两个失败关闭检查全树零测试packages/cli/src/commands/review/compose-review.ts:4642 — [probe] stop 轮拒绝行内 Critical 却接受行内 Suggestions——'无审查运行' 与 'Suggestions are inline' 并列packages/cli/src/commands/review/compose-review.test.ts:16451 — [probe] 消费测试的重试臂对消费顺序失明——套件在有缺陷与修复顺序下都 55/55（R14-1 的缺证人半侧）packages/cli/src/commands/review/capture-local.ts:1190 — [probe] --cache 点名但缺失文件的 null 盖章臂无证人——变异绿色上线并使该形状失败关闭地活锁packages/cli/src/commands/review/compose-review.test.ts:16476 — [probe] readStopSnapshot 的不可读（非-ENOENT）臂无证人——三元折叠变异绿色上线并在不可读缓存上授权
Convergence: round 14 posted 4 inline comment(s), 3 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/cli/src/commands/review/compose-review.ts (findings in round 13; 3 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
Test Plan（非阻断）：src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 12 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 14 轮发布了 4 条行内评论，其中 3 条是首次提出；上一轮发布了 4 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/compose-review.ts（第 13 轮已出过发现，本轮又有 3 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
…rule Round-14 findings: - R14-11: openLedgerCriticalEntries re-validated every per-entry shape fail-closed but never rejected duplicate ids, so two open Critical rows under ONE id collapsed the grant's set-based completeness check and the last-wins title/file maps into a single disposition - the real blocker left the verdict lineage through its filler twin (and under scope-emptied the twin's file could blanket-supersede it). A repeated id is now the same unreadable-baseline refusal as any other shape violation, symmetric with the disposition-side duplicate refusal. - R14-7: a granted stop re-rule accepted the model-written convergence census - the fresh <= reported cross-check was satisfied by the carried-id re-assertions the grant itself proves are NOT fresh - and could mint the non-convergence blocker over a round that measured nothing, publishing a false 'newly identified' claim and advancing the churn streak. A stop re-rule is now the third unmeasurable state beside round-0 and context-unavailable: the census reads as null and the streak carries rather than resets. Both guards mutation-checked red-first.
There was a problem hiding this comment.
Pull request overview
This PR fixes a gap in qwen review run --fail-on request-changes where “decided stop” rounds (e.g. unchanged-since-last-round, scope-emptied, clean-tree) could previously finish with event: null, allowing CI gating to pass despite still-open Criticals in the cache ledger. It introduces a decided-stop “re-rule” path that composes a real verdict via a new stopReRule input to compose-review, and hardens the stop-path fencing (run-id stamping, sidecar binding, and cache hash consistency) so stop verdicts are fail-closed and attributable to the current run.
Changes:
- Add a decided-stop re-rule mechanism (
stopReRule) tocompose-reviewthat machine-checks completeness vs. the cache ledger’s open Critical set and enforces per-stop-reason ruling constraints. - Require run-id stamping on composed verdict artifacts and fence
run.tsconsumption to same-run artifacts; treat decided stops without a composed verdict as incomplete (exit 1), while keeping the known PR-stop residual behavior. - Harden capture/compose consistency: single-read cache snapshotting for stop decisions + stamping, sidecar binding fields (reason/cachePath/findingsHash/supersededPaths), and cleanup of stale stop sidecars.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/skills/bundled/review/SKILL.test.ts | Adds assertions ensuring the bundled skill’s Step-1 decided-stop instructions require composing a verdict (including the “nothing-open” branch). |
| packages/core/src/skills/bundled/review/SKILL.md | Updates Step-1 stop handling so all three decided stops compose a verdict via compose-review using stopReRule (REQUEST_CHANGES when blockers still stand; COMMENT when cleared). |
| packages/cli/src/commands/review/run.ts | Gates completion primarily on a run-id-stamped composed verdict for decided capture-stops; restricts sidecar-only completion to PR stop reasons only. |
| packages/cli/src/commands/review/run.test.ts | Updates fixtures to stamp composed artifacts with the spawned run id; adds coverage for the new decided-stop gating/exit semantics and unstamped-artifact refusal. |
| packages/cli/src/commands/review/lib/local-anchor.ts | Splits cache parsing into readLocalCacheFromBytes to support single-read decision/stamp flows without TOCTOU windows. |
| packages/cli/src/commands/review/compose-review.ts | Implements stopReRule contract + fail-closed machine checks (ledger completeness, per-reason licenses, sidecar/run-id fences, content binding) and stamps composed artifacts with run id. |
| packages/cli/src/commands/review/compose-review.test.ts | Adds an extensive test suite for the decided-stop re-rule grant, including refusal matrices, fencing, and content binding. |
| packages/cli/src/commands/review/capture-local.ts | Ensures single-read cache bytes for stop decisions + stamps, publishes consistent resolved cachePath, stamps findings hash and superseded split, and unlinks stale sidecars on non-stop captures. |
| packages/cli/src/commands/review/capture-local.incremental.test.ts | Adds regression tests for stop-sidecar binding fields (cachePath/findingsHash/supersededPaths) and single-read hashing behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm:
- [Critical] R13-4 (compose-review.ts:393): write-surface provenance class — mechanism present at HEAD (the fence binds drift but authenticates no provenance); this round's probe re-demonstrated it through a plan-nominated foreign sidecar (folded into t…
- [Critical] R14-1 (compose-review.ts:471): missing cache admitted as an EMPTY re-rule baseline — mechanism present at HEAD, but the claimed harm conflicts with the verify-floor doctrine (an unverified Critical deliberately never enters the ledger as a …
Not explored to full depth (tool budget reached): "agent 1c": none — the checks above completed within budget..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Test Plan (not a blocker): src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/review/compose-review.ts:4187 — [review] Critical [fails-closed] [new-surface] D15-1 interactive stop sidecar consumed before four late shape validations — corrected retry burns the grantpackages/cli/src/commands/review/compose-review.ts:7531 — [review] D15-9 artifact runId stamp writer half has no testpackages/cli/src/commands/review/compose-review.ts:3900 — [review] D15-10 dispositions arrayness/entry-vocabulary guards untestedpackages/cli/src/commands/review/compose-review.ts:5034 — [review] D15-11 Approve-branch presubmit guard on granted stops unwitnessedpackages/cli/src/commands/review/compose-review.test.ts:15886 — [review] D15-12 floors-skipped guarantee pinned by one cap name onlypackages/cli/src/commands/review/compose-review.test.ts:16538 — [review] D15-13 vanished-cache direction of the hash bind unwitnessedpackages/cli/src/commands/review/compose-review.ts:3917 — [review] D15-14 grant leaves the Suggestion surface ungated on stop rounds
中文说明
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1c"：none — the checks above completed within budget.。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
Test Plan（非阻断）：src/commands/review/compose-review.test.ts — no such file or directory; src/commands/review/run.test.ts — no such file or directory; src/commands/review/capture-local.incremental.test.ts — no such file or directory; src/skills/bundled/review/SKILL.test.ts — no such file or directory。
收敛姿态下延后（第 15 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 7 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No code changes this round — every actionable Critical in the feedback is already resolved at HEAD ( Triage of the standing CriticalsFixed in code (re-verified at HEAD this round). The following lineages were fixed by earlier commits, and this round re-verified each fix is present in the code and pinned by the green suites listed below: the unvouched-relocated deterministic credit (R7-1, Escalated — carried by #10654. The write-surface provenance class (R6-1/R10-4/R12-1/R13-3/R13-4) is the maintainer-acknowledged standing class: Deferred — carried by #10673. The missing-cache-as-empty-baseline finding (R14-1/R14-19) is not a regression of this PR (the pre-PR flow exited 0 on the same shape with no gate at all) and its claimed harm conflicts with the verify-floor doctrine; the genuine gap — a withholding round leaves no durable record of its unruled items — is a persistence-layer design question tracked in #10673. Refuted. R10-3 was rejected by the round-12 verifier as documented, tested design (the #7166 doctrine); no code change backs that disposition and the record stands in the review body. CI stateRound 15 downgraded Approve→Comment citing failing VerificationCommands actually run this round at HEAD
中文说明本轮没有代码改动——反馈中所有可执行的 Critical 都已在 HEAD（ 未决 Critical 的分流已在代码中修复（本轮在 HEAD 复核）。 以下各谱系由更早的提交修复，本轮复核了每一处修复确实存在于代码中、且被下列绿色套件钉住：无凭证 relocated 确定性信用（R7-1， 已升级——由 #10654 承接。 写入面出处（provenance）类发现（R6-1/R10-4/R12-1/R13-3/R13-4）是 maintainer 确认的常设类： 已延后——由 #10673 承接。 缺缓存被当作空基线的发现（R14-1/R14-19）不是本 PR 的回归（PR 之前的流程对同一形状直接退 0、根本没有任何门），其声称的危害也与 verify-floor 原则冲突；真正的缺口——扣留轮没有为其未裁条目留下持久记录——是持久层设计问题，由 #10673 跟踪。 已被证伪。 R10-3 已被第 12 轮验证者以"有文档、有测试的设计"（#7166 原则）拒绝；该处置没有对应的代码改动，记录保留在评审正文中。 CI 状态第 15 轮以 验证本轮在 HEAD
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 56 selected review thread(s). · 已关闭全部选中的 56 条评审线程。 🧠 Handled by Qwen Code · model/模型 |
|
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: 1747 passed · 0 failed · 1747 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：1747 通过 · 0 失败 · 1747 总计 抖动门： Verification reportPR #10310 deep verification — gate decided stops on a composed re-rule verdictVerdict: Assertion totals: 41 A/B harness checks (13 cells × exit-code/completed/event/stderr) + 885 head gate tests (826 CLI + 59 core) + 814 base gate tests (689 CLI + 56 core + 69 incremental — delta attribution) + 4 mutation-red observations + 1 restore check + 2 control checks (realpath / core-TS closure). 中文摘要
Central claim and A/B proofCentral claim: How the A/B was driven.
Witness: The composed artifact cell 1 produced carries the full contract: Secondary claim 1 (fail-closed direction): proven by cells 3, 5, 7, 9, 12 — every malformed/refused shape exits 1, and each base control cell (2, 6, 8) shows base admitted the same shape with exit 0. Secondary claim 2 (cleared ⇒ Comment, never Approve): proven by cells 4 and 13 at the process level and by mutation M3 below (removing the one-line downgrade flips the event to APPROVE and the verdictLine to CorrectionsNone needed — no inaccurate earlier-round descriptions were in scope (first verification round; no FindingsNo blocking findings. Observations, in severity order:
Not covered
MethodologyEnvironment: CI merge-ref checkout ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the gating chain end to end — LGTM, the hole is closed fail-closed:
- Exit contract (
run.ts):readComposednow requires the artifact to be stamped with THIS run's id (stale/forged/concurrent artifacts are null), and a decided capture stop with no composed verdict reads as "did not complete" (exit 1) instead of the oldevent: nullsilent exit 0. The sidecar-alone exit-0 exemption is correctly scoped to PR targets — gated ontargetClass.kind === 'pr'beside the reason string, so a local sidecar wearingup-to-datecan't slip through. - Grant integrity (
compose-review.ts): oneStopSnapshotread per plan+cache with hash and ledger enumeration as projections of the same buffer — the alternating-writer TOCTOU from the measured probe is structurally gone. Per-reason ruling licences match the capture's certification exactly (unchanged→still-stands only,scope-emptied→+superseded,clean-tree→full set, unknown reasons licence nothing), and the sidecar fence binds run-id/reason/cachePath/findingsHash even for interactive rounds that previously skipped it entirely. - SKILL Step 1: all three stop branches compose before stopping, including the nothing-open case (no-event Comment), which is what makes "no artifact ⇒ refused" a sound fail-closed signal.
clean-treere-rules are judged, incremental stops deduced — consistent with the licence table. - Disclosed residual (PR-target stops exiting 0 over the PR cache) is documented in code and body with architectural follow-up in #10654.
CI green on this head (Ubuntu tests, integration, Desktop Shell, E2E smoke, CVE, TruffleHog); 0 unresolved threads — the CHANGES_REQUESTED stack is the ci-bot's iterative review history across superseded commits.
|
Thanks for the PR! Template looks good ✓ Problem: observed and documented, not theoretical — a decided stop completes with Direction: aligned — this is the repo's own review gate passing over a blocker it just declared standing, exactly the false-pass class the gate exists to prevent. No CHANGELOG reference, but the area is core review infrastructure. Size: cross-package ( Approach: the core shape is right, and it matches what I'd propose independently: a decided stop composes a real verdict through the same composed-artifact path a full round writes, so Risk: no high-risk path matches — no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测、有记录，不是理论性问题——决定性 stop 以 方向：对齐——这是仓库自己的评审门在自己刚宣布 blocker 仍站立时放行，正是这个门要防的误放行类别。 规模：跨包（ 方案：核心形态正确，也与我独立会提的方案一致：决定性 stop 经由与完整轮相同的组稿产物路径组出真实裁决， 风险：无高风险路径命中——无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRead the full production diff (capture-local, compose-review, local-anchor, run.ts, SKILL.md) against the base. No critical blockers found. The grant reads as designed: one plan/cache snapshot for the whole grant, sidecar fence binding reason + cachePath + findingsHash (+ the capture-certified supersededPaths split for scope-emptied), completeness checked both ways against the ledger's open-Critical set, content-bound re-assertions, and every malformation failing closed with a designed refusal rather than degrading to the regular floors. Three things I checked independently rather than taking on the diff's word:
Reuse is clean — On the two round-15 points left "unresolved, please confirm": my read is that both are disclosed residuals/doctrine, not defects of this diff — flagging for maintainer confirmation since the round left them open. R13-4 (write-surface provenance) is the #10654 architectural class, disclosed in the PR body; this diff carries the interim hardening (interactive sidecar consumed on grant, run-id fences) and stays fail-closed across runs. R14-1 (missing cache as an EMPTY baseline) is consistent: an incremental stop cannot fire without a ledger existing at decision time, a clean-tree stop over a never-written ledger recorded nothing, drift between decision and compose is hash-bound in both directions (both tested), and an out-of-band ledger deletion loses history equally on a full round — pre-existing, not regressed here. Testing evidence — the PR's own CI (never run locally)This is an unattended run, so no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit, read via the API. The two checks round 15 saw red (
Not verified: the end-to-end shape (seed a high-effort local review with an open Critical, commit without fixing, re-run 中文说明代码审查对照 base 通读了全部生产代码 diff。未发现阻断性问题。grant 与设计一致：整个授权只做一次 plan/cache 快照；sidecar 围栏绑定 reason + cachePath + findingsHash（scope-emptied 另绑定 capture 认证的 supersededPaths 划分）；与台账 open-Critical 集双向完整性校验；正文按内容绑定；一切畸形状态都以设计好的拒绝失败关闭，而不是降级走常规 floors。 三处我没有轻信 diff、独立核实过： 复用干净： 关于第 15 轮留下的两个"未决请确认"：我的判断是两者都属于已披露残余/原则问题，而非本 diff 的缺陷——R13-4（写面来源类）是 #10654 的架构类问题，PR 正文已披露，本 diff 携带过渡加固并保持跨运行失败关闭；R14-1（缺失 cache 作为空基线）是自洽的——增量 stop 在决策时台账必然存在，决策与组稿之间的漂移双向哈希绑定（均有测试），台账被带外删除在完整轮次同样丢历史，属既有属性而非本 PR 回归。因评审轮留下未确认，仍提请维护者确认。 测试证据无人值守运行，未构建或执行任何 PR 代码；以上证据为被审提交上 PR 自身 CI 经 API 读取。第 15 轮看到红色的两项检查（Test ubuntu、Post Coverage Comment）在同一 SHA 上已转绿——降级原因已消除。macOS/Windows 单测与 CLI 沙箱集成腿在 pull_request 事件下按设计跳过（仅 merge-group 执行）。未验证：端到端形态（种入 open Critical、不修直接提交、重跑 --fail-on → 退 3）——单测在 CI 中钉住了退出码契约，但本次没有独立的行为复跑。沙箱验证可以定论： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage on the reviewed SHA; the missing fifth is honest hesitation at the complexity this gate path now carries, not a defect I can name. Stepping back: the problem is real and documented, the fix matches the minimal mechanism I'd propose independently (a decided stop composes a real verdict through the existing composed-artifact path; The reservations, named because they're real even though none blocks:
CI is settled (no pending 中文说明置信度：4/5 —— 在被审提交上各阶段均干净；扣掉的一分是对这条门禁路径如今承载的复杂度的诚实迟疑，不是我能指出的缺陷。 退一步看：问题真实且有记录；修复与我独立会提的最小机制一致（决定性 stop 经由既有组稿产物路径组出真实裁决，run.ts 零新增管道门控，无产物退 1、绝不静默退 0）；核心之外的每一层加固都回应本 PR 评审轮中的一次实测探针，并各有测试钉住。套件不是"有无 diff 都绿"——旧的"未门控 stop 放行"测试已被门控契约测试替换。该 SHA 上 CI 全绿，包括第 15 轮看到红色的两项。 保留意见如下，虽不阻断但都真实存在：
CI 已落定（该 SHA 无未完成的 pull_request 运行），审批护栏未触发（同仓、fix 类型），故本次运行按被审提交钉住审批。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ — the non-blocking notes are in my Stage 3 comment above.
What this PR does
Makes
qwen review run --fail-on request-changesactually gate the three decided stops (unchanged-since-last-round,clean-tree,scope-emptied). A decided stop now composes a real verdict through a newstopReRuleinput oncompose-review: when the cache ledger holds open Criticals, the orchestrator's dispositions (deduced on the two incremental stops, judged on clean-tree) are machine-checked fail-closed — completeness both ways against the ledger's open-Critical set, per-reason ruling licences, content-bound body re-assertions under the original ids — and a standing blocker composes REQUEST_CHANGES exactly like a full round's. A cleared round composes a Comment, never an Approve. The composed artifact rides the sameqwen-review-<target>-composed.jsona full round writes, sorun.tsgates it with no new plumbing; a decided stop with no composed artifact reads as "did not complete" (exit 1), never as a clean exit 0.The review rounds on this PR then hardened the grant end to end: the whole grant reads ONE plan/cache snapshot (no TOCTOU between the fence's hash and the ledger enumeration), the capture-stamped stop sidecar binds on every compose — run-id-fenced under the gate, still content-bound (reason / cachePath / findingsHash) interactively — the stamp names the
--cache-resolved ledger the stop decision actually read, stale sidecars are retired by the next non-stop capture, the composed artifact itself is run-id-stamped andreadComposedrequires the stamp, the sidecar-alone exit-0 exemption is scoped to PR targets, ledgerstatusstrings are enum-checked, deferral-channel Criticals and presubmit downgrades are refused/closed on granted stops, and a decided-stop plan composed withoutstopReRuleis refused outright.Why it's needed
A decided stop used to complete with
event: null, soqwen review run --fail-on request-changesexited 0 while the round's own output rendered open Criticals as still standing — a user who commits without fixing a blocker leaves a permanently passing CI gate. This is #9659's accepted residual (R8-1/R13-3): the blocker-dating chain that tried to answer "does the blocker still stand?" from file bytes produced successor Criticals in every regime it grew and was deleted; the recorded ruling deferred the real gate to this change.Reviewer Test Plan
How to verify
From
packages/cli:npx vitest run src/commands/review/compose-review.test.ts src/commands/review/run.test.ts src/commands/review/capture-local.incremental.test.ts— the decided-stop describe covers the grant's refusal matrix (forged full-round flag, missing/invented/duplicate dispositions, per-reason licence cells, content-departure binds on both legs, the sidecar fence with and without a published run id, ledger schema/status drifts, supersededPaths deduction, deferral-channel Criticals, presubmit downgrade closure), andrun.test.tspins the exit contract (standing re-rule → exit 3; nothing-open stop → exit 0; refused re-rule / unstamped artifact / local sidecar wearing a PR reason → exit 1).packages/core:npx vitest run src/skills/bundled/review/SKILL.test.tspins the Step-1 directives, including the Criticals-keyed nothing-open branch.End-to-end shape: seed a local review at high effort so the cache ledger records an open Critical, commit without fixing it, and re-run
qwen review run --fail-on request-changeson the unchanged tree — the stop round composes the re-rule verdict and the command exits 3; fix the blocker and the clean-tree round's judgedfixeddisposition composes a Comment and exits 0.Evidence (Before & After)
Before (base): a decided stop completes with
event: nullandqwen review run --fail-on request-changesexits 0 over a ledger whose open Critical the round itself renders as still standing (pinned then only by the absence of any gating test). After:run.test.ts— 'gates a stop round through its COMPOSED verdict — the #9908 path' asserts the same shape exits 3 withevent: REQUEST_CHANGES; 'exits 1 when a decided stop never composed a verdict — the refused re-rule' pins the fail-closed direction. Full local run:packages/clireview suite 116 files / 5686 tests green; SKILL guards 59 green; the load-bearing guards were mutation-checked red-first in review (details in the round summaries and resolved threads).Tested on
Environment (optional)
Unit suites only (vitest from
packages/cli/packages/core); no sandbox.Risk & Scope
up-to-date/empty-diff) still complete on the sidecar alone and exit 0 over whatever the PR cache holds open — a disclosed residual, now at least scoped to PR targets only. The fence's trust anchor necessarily lives inside the model session's write surface in the current topology; the architectural closure (parent-held anchor) is tracked in review run: the decided-stop fence's trust anchor lives inside the model session's write surface #10654.compose-reviewoutput JSON gains an optionalrunIdfield, and stop-state consumers must supplystopReRulefor decided-stop plans (the bundled SKILL already does).Linked Issues
Closes #9908. Refs #9659, #10654.
Supersedes the pre-round-12 draft on
fix/review-stop-composed-verdict— rewritten from scratch against current main, since rounds 12–24 of #9659 invalidated that draft's assumptions.中文说明
这个 PR 做了什么
让
qwen review run --fail-on request-changes真正对三种决定性 stop(unchanged-since-last-round、clean-tree、scope-emptied)起门禁作用。决定性 stop 现在通过compose-review新增的stopReRule输入组出真实裁决:当 cache 台账存在 open Critical 时,编排者的 dispositions(两个增量 stop 为演绎、clean-tree 为判断)被 fail-closed 机器校验——与台账 open-Critical 集双向完整性、按 stop 原因的裁定许可、正文重张按原 id 做内容绑定——站立的 blocker 组出与完整轮次相同的 REQUEST_CHANGES;清空的轮次组 Comment,绝不 Approve。组稿产物使用与完整轮相同的qwen-review-<target>-composed.json,run.ts零新增管道即完成门控;有决定性 stop 而无组稿产物按"未完成"(退 1)处理,绝不静默退 0。本 PR 的评审轮次随后对 grant 做了端到端加固:整个 grant 只做一次 plan/cache 快照读取(fence 哈希与台账枚举之间无 TOCTOU),capture 盖章的 stop sidecar 在每次 compose 都绑定——门控下有 runId 围栏,交互式下仍绑定 reason / cachePath / findingsHash——stamp 指向 stop 决策实际读取的
--cache解析文件,后续非 stop 捕获会清理陈旧 sidecar,组稿产物自身盖 runId 且readComposed强制校验,sidecar 单独完成豁免限定 PR 目标类,台账status枚举校验,granted stop 上拒绝 deferral 通道 Critical 并关闭 presubmit downgrade,携带决定性 stop 的 plan 不带stopReRule直接拒绝。为什么需要
决定性 stop 以
event: null完成,qwen review run --fail-on request-changes因此退 0,而该轮自己的输出正把 open Critical 渲染为 still-standing——不修 blocker 直接提交的用户会留下一个永久放行的 CI 门。这是 #9659 的接受残余(R8-1/R13-3):试图从文件字节回答"blocker 是否仍站立"的 blocker-dating 链每加一种机制就产出下一代 Critical,最终被删除;记录在案的裁决把真正的门指向本变更。评审验证方案
如何验证
在
packages/cli下:npx vitest run src/commands/review/compose-review.test.ts src/commands/review/run.test.ts src/commands/review/capture-local.incremental.test.ts——decided-stop describe 覆盖 grant 的拒绝矩阵(伪造完整轮标志、缺行/多行/重复 disposition、按原因许可格、双腿内容偏离绑定、有无 runId 的 sidecar 围栏、台账 schema/status 漂移、supersededPaths 演绎、deferral 通道 Critical、presubmit downgrade 关闭),run.test.ts钉住退出码契约(站立重裁 → 退 3;nothing-open stop → 退 0;被拒重裁 / 未盖章产物 / 本地 sidecar 冒 PR 原因 → 退 1)。packages/core下:npx vitest run src/skills/bundled/review/SKILL.test.ts钉住 Step-1 指令,含按 Critical 键控的 nothing-open 分支。端到端形态:高档位跑一次本地评审使 cache 台账记录一个 open Critical,不修直接提交,在未变化的树上重跑
qwen review run --fail-on request-changes——stop 轮组出重裁裁决、命令退 3;修复 blocker 后 clean-tree 轮的判断性fixeddisposition 组 Comment、退 0。证据(前后对比)
之前(base):决定性 stop 以
event: null完成,qwen review run --fail-on request-changes在台账 open Critical 仍站立时退 0。之后:run.test.ts的 'gates a stop round through its COMPOSED verdict — the #9908 path' 断言同一形态退 3 且event: REQUEST_CHANGES;'exits 1 when a decided stop never composed a verdict' 钉住 fail-closed 方向。本地全量:packages/clireview 套件 116 文件 / 5686 测试全绿;SKILL guards 59 全绿;关键守卫在评审中先验红(细节见各轮总结与已 resolve 的 threads)。测试平台
环境(可选)
仅单测(在
packages/cli/packages/core下 vitest);无沙箱。风险与范围
up-to-date/empty-diff)仍仅凭 sidecar 完成并对 PR cache 的 open 项退 0——已披露的残余,现至少限定在 PR 目标类。当前拓扑下围栏的信任锚必然位于模型会话的写面之内;架构级闭合(parent 持锚)在 review run: the decided-stop fence's trust anchor lives inside the model session's write surface #10654 跟踪。compose-review输出 JSON 新增可选runId字段;决定性 stop 的 plan 组稿必须携带stopReRule(bundled SKILL 已照做)。关联 Issue
Closes #9908。Refs #9659、#10654。
取代
fix/review-stop-composed-verdict上 round-12 之前的旧稿——基于当前 main 重写,旧稿假设已被 #9659 第 12–24 轮推翻。