fix(ci): port the verify gate's remaining hardening from #8765 - #8878
Merged
Conversation
#8816's branch accidentally carried #8765's early commits, and the takeover loop evolved the gate further there (subset identity via comm -23, the retryable third arg, subset fixtures) — so #8765 closes as subsumed, and this PR ports what main still lacks: the two improvements its reviewers named for porting, plus the open round-6/7 findings that survive on main's gate. - Pre-detach short-circuit: an empty head signature (vite/esbuild/ crash — the KNOWN LIMIT class) fails closed regardless of the baseline, so decide it BEFORE paying the detach + full baseline re-run + restore. - Build-dirt guard: the A/B'd build REWRITES a tracked file (the vscode companion settings schema), and the undiscarded rewrite makes either checkout refuse — degrading a real verdict into the crash path. `git restore -- .` before both checkouts; tracked-only, and the tree was asserted clean before the deterministic checks. - Restore-failure semantics: a plain outcome=failed is an EVALUATED rejection — the watermark advances and a transient git failure strands the item as a permanent human handoff. The gate now leaves outcome unset (the gate-crashed path retries next scan) and still writes the detail document so the crash comment explains itself. - The dist-rebuilt steering note seeds the repair feedback on both retryable A/B exits — the repair agent's only warning that dist/ holds baseline-built artifacts. - The stale-base retry handoff prefixes its embedded rejection with a the-base-has-moved note, so the retry agent is not steered toward no-action by framing written before the auto-update. - The two A/B side logs joined the repair step's cleanup list. - Tests: identity-less short-circuit, tracked-dirt survival, verdict-less restore crash, long-preamble render cap, PREEXISTING clause selection through the executable report harness, and the stale-framing note pin. Mutation-tested, 5 of 5 caught: short-circuit dropped, restore guards dropped, restore-failure reverted to the evaluated rejection, dist note dropped, stale-framing note dropped.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks — this closes out a review backlog I can trace: #8765 (the A/B gate) closed as subsumed, #8816 merged with the evolved gate, and this ports the five remaining hardening items onto main. Template — complete, with a concrete reviewer test plan (suite command + expected counts, plus the mutation-testing claim). Problem — these are observed gate deficiencies, not theoretical ones. I independently verified the two that are checkable from the tree: Direction — hardening the repo's own verify gate; internal CI reliability, owned by the gate's author. Aligned, nothing to escalate. Size — small and not core: 169 lines total — 62 production (50 in the gate script, 12 in the workflow) vs 107 test lines. Stage 0 not applicable. Approach — every hunk maps 1:1 to a named gap; no drive-by changes. Verdict semantics are preserved where it matters: the empty-signature short-circuit returns the same fail-closed "charge the round" verdict, just before paying the detach + baseline re-run. The one real behavior change — restore failure becomes a verdict-less crash (retry on the next scan) instead of an evaluated rejection — is the point of gap 3, and its tradeoff (a persistent git corruption now retries each scan instead of stopping) is acknowledged in the PR body and bounded by fresh trusted checkouts. Risk — no elevated risk signals: none of the changed files match the revert-correlated path list, and the suite that pins all of this ( Moving on to code review. 🔍 中文说明感谢——这是可以追溯的评审积压收尾:#8765(A/B 门)以"已被吸收"关闭,#8816 合并了演进后的门,本 PR 把剩余五项加固移植到 main。 模板 — 完整,且有具体的评审者测试计划(套件命令 + 预期数量,以及变异测试声明)。 问题 — 这些是已观测到的门缺口,不是理论问题。我独立验证了其中两个可以从代码树核实的: 方向 — 加固仓库自身的验证门;内部 CI 可靠性,由门的作者本人维护。方向对齐,无需升级。 规模 — 小且不涉核心:共 169 行——62 行生产(门脚本 50 行、workflow 12 行)对 107 行测试。Stage 0 不适用。 方案 — 每个 hunk 与一个点名缺口一一对应;没有顺手改动。关键处的判定语义保持不变:空签名短路返回同样的 fail-closed "计入本轮"判定,只是不再白付 detach + 基线重跑。唯一真正的行为变化——restore 失败改为无判定崩溃(下次 scan 重试)而非已评估拒绝——正是缺口 3 的目的,其取舍(持续性 git 损坏会每轮重试而非停下)已在 PR 正文中说明,并由全新的可信 checkout 兜底。 风险 — 无升级风险信号:改动文件均未命中与 revert 相关的高风险路径列表;钉住这一切的套件( 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI derived my own fix for the five gaps before reading the diff, and this matches it point for point — pre-detach signature short-circuit, tracked-dirt discard around both checkouts, verdict-less crash on restore failure, the steering note, and the stale-framing invalidation. No correctness blockers found. A few things I checked closely and want to record:
One non-blocking observation: the dist-rebuild steering note fires only on the baseline-green retryable path. On the DIFFERENT-reason path (baseline also failed, mismatched signature) the tree also carries a partially baseline-built Test evidence — the PR's own CI, fetched via APIFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 No red checks so far; the decisive one is still running at review time, so the finalize pass will update the table when it settles. Two reasons the ubuntu Test job is the load-bearing signal here: Not verified: behavior in a live production autofix round — that surface is exercised by future rounds, and fixture execution of the real script is this file family's established verification method, so no extra lane is named here. 中文说明代码审查我在读 diff 之前先独立推导了五个缺口的修法,本 PR 与之逐点一致——detach 前签名短路、两次 checkout 前后的跟踪脏文件丢弃、restore 失败时不产生判定、引导注记、过期框架的失效声明。未发现正确性阻塞项。几处我仔细核对过、值得记录的点:
一个非阻塞观察:dist 重建引导注记只在"基线绿"的可重试路径上发出。在 DIFFERENT-reason 路径(基线也失败、签名不匹配)上,树里同样残留部分基线构建的 测试证据 — PR 自己的 CI,经 API 获取(见上方 CI 表格) 目前无红色检查;决定性的 ubuntu Test 作业在评审时仍在运行,finalize 流程会在其落定后更新表格。该作业是承重信号有两个原因: 未验证:真实生产 autofix 轮次中的行为——该表面由后续轮次行使,且对真实脚本的 fixture 执行是这一文件族的既定验证方式,故此处不额外点名沙箱验证通道。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, scoped port with every premise checkable from the tree verified; one non-blocking note-placement question, and CI still in flight. Stepping back: this is what a good follow-up looks like. Five named gaps, each hunk mapped to exactly one of them, 62 production lines, and tests that execute the real gate in real-git fixtures and assert absences — so they flip the moment a guard is dropped. I derived my own fix for each gap before reading the diff, and this matches it point for point. The two premises checkable from the tree (the build rewriting a tracked file; the old restore failure being an evaluated rejection by construction) are confirmed, and the remaining three are structural facts readable in the base script. The one genuine behavior change — restore failure now retries instead of handing off permanently — is gap 3's entire point, with its tradeoff acknowledged and bounded by fresh trusted checkouts. My only reservation is the one raised above: the dist-rebuild steering note doesn't cover the DIFFERENT-reason path, which also leaves a partially baseline-built Verdict: approve. The 中文说明Confidence: 4/5 — 干净、范围明确的移植,所有可从代码树核实的前提均已验证;一个非阻塞的注记放置问题,CI 仍在运行。 回头看:这是一个好的后续 PR 该有的样子。五个点名缺口,每个 hunk 恰好对应一个,62 行生产代码,测试在真实 git fixture 中执行真实的门并断言缺席——守卫一旦被移除断言即翻转。我在读 diff 之前独立推导了每个缺口的修法,本 PR 与之逐点一致。两个可从代码树核实的前提(构建改写跟踪文件;旧的 restore 失败按构造就是已评估拒绝)已确认,其余三个是读 base 脚本即可确认的结构性事实。唯一真正的行为变化——restore 失败改为重试而非永久移交——正是缺口 3 的目的,其取舍已被说明,并由全新可信 checkout 兜底。 我唯一的保留意见已在上面提出:dist 重建引导注记未覆盖 DIFFERENT-reason 路径,该路径同样残留部分基线构建的 结论:approve。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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
approved these changes
Aug 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
yiliang114
approved these changes
Aug 10, 2026
yiliang114
approved these changes
Aug 10, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. This is exactly the agreed closeout of the #8765/#8816 overlap: #8816's subset semantics (comm -23) stay untouched, and the five surviving gaps are ported correctly, each verified in the diff and pinned by new executable tests: (1) the pre-detach empty-head-signature short-circuit decides identity-less failures (vite/esbuild/crash) before paying detach+baseline+restore, retryable and fail-closed; (2) git restore -- . before both checkouts discards tracked build dirt (the regenerated vscode settings schema) so a real verdict no longer degrades into the crash path — tracked-only is safe because the tree was asserted clean pre-checks; (3) restore failure is no longer an EVALUATED rejection: outcome stays unset, the detail document is written, and the report's existing gate-crashed path retries on the next scan instead of advancing the watermark into a permanent handoff; (4) the dist-rebuilt steering note lands on both retryable exits so the repair agent rebuilds before trusting dist-consuming checks; (5) the stale-base retry comment now discloses that its verdict predates the auto-update. Evidence hygiene (both A/B side logs join the repair cleanup) and the tail-budget long-preamble cap are pinned too. CI green on head, 149/149 gate tests. With this merged, #8765 can close as subsumed. Ship it.
Collaborator
|
Released in v0.21.9. |
wenshao
added a commit
that referenced
this pull request
Aug 11, 2026
Address the two review suggestions on #8958: - The steering note existed as three byte-identical copies, and the "every retryable exit seeds the note" invariant depended on copy-paste — the exact drift this PR was patching (one exit seeded on #8765's branch, one lost in the #8878 port). Both reviewers flagged it. The string now lives in seed_dist_note(), called from all three exits. - The comm-failure exit seeded the note but, unlike its sibling retryable exits, emitted no verdict-rationale line — an oncall could not distinguish "the comparison itself failed" from "baseline is green" without re-running the A/B. It now says so. Mutation-tested: mutating the string inside the helper fails all three path assertions at once; mutating the rationale line fails the comm-exit test.
pull Bot
pushed a commit
to moqimoqidea/qwen-code
that referenced
this pull request
Aug 12, 2026
…wenLM#8958) * fix(ci): seed the dist-rebuild warning on every retryable A/B exit The baseline leg rebuilds dist/ from baseline sources, and every retryable exit of the verify gate hands that tree to the repair agent — but the "run npm run build first" steering note only reached the green-baseline rejection. The comm -23 comparison failure and different-signature exits sent the repair agent in blind, free to trust or test against stale baseline artifacts (the different-signature exit carried the note on QwenLM#8765's branch; the QwenLM#8878 port kept it on the green exit only). QwenLM#8765's post-close round-3 review flagged the comm path as Critical. Append the note on both missing exits and pin all three paths: the DIFFERENT-reason test now asserts the note, a new test stubs comm to fail and asserts the same, and the pre-existing test pins the note OUT of its document — no repair runs for that verdict. Mutation-tested, 3 of 3 caught: comm-path note dropped, different-reason note dropped, note leaked into the pre-existing document. * fix(ci): single emit point for the dist note, name the comm-failure exit Address the two review suggestions on QwenLM#8958: - The steering note existed as three byte-identical copies, and the "every retryable exit seeds the note" invariant depended on copy-paste — the exact drift this PR was patching (one exit seeded on QwenLM#8765's branch, one lost in the QwenLM#8878 port). Both reviewers flagged it. The string now lives in seed_dist_note(), called from all three exits. - The comm-failure exit seeded the note but, unlike its sibling retryable exits, emitted no verdict-rationale line — an oncall could not distinguish "the comparison itself failed" from "baseline is green" without re-running the A/B. It now says so. Mutation-tested: mutating the string inside the helper fails all three path assertions at once; mutating the rationale line fails the comm-exit test. * test(ci): pin the no-identity baseline arm of the A/B gate (QwenLM#8958) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Ports the verify gate's remaining hardening from #8765 onto main, closing out that PR's review backlog. #8765 closes as subsumed: #8816's branch accidentally carried its early commits, and the takeover loop evolved the gate further there (subset identity via
comm -23, theretryablethird arg, subset fixtures) — merging #8765 after #8816 would have regressed that evolution.Why it's needed
Five gaps survive on main's gate, each named by #8765's reviewers (rounds 6–7 plus the human review's port list):
outcome=failedadvances the watermark: a transient git failure strands the item as a permanent human handoffPlus evidence hygiene (the two A/B side logs join the repair cleanup list) and six test gaps the reviews mutation-proved blind.
Reviewer Test Plan
Expected: 149/149 (6 new). Full scripts suite: 1090 passed.
yamllint+bash -nclean.New scenarios execute the real gate in the real-git fixture: identity-less short-circuit (no
Baseline A/Bin the transcript), tracked-dirt survival (a pre-existing verdict lands instead of the crash), verdict-less restore crash (nooutcome=, detail document written), long-preamble render cap (fence intact underhead -c 3900),PREEXISTINGclause selection through the executable report harness (ahead → own-code, behind+conflict → merge-main), and the stale-framing note pin.Mutation-tested — 5 of 5 caught: short-circuit dropped, restore guards dropped, restore-failure reverted to the evaluated rejection, dist note dropped, stale-framing note dropped.
Tested on
Risk & Scope
git restore -- .discards tracked build dirt only; an untracked-file clash still (correctly) crashes verdict-less — that scenario is pinned. The remedy-prose triplication flagged in round 6 is accepted debt, noted in the thread replies.Linked Issues
Closes out #8765's review backlog (rounds 6–7 + the coordination review); companion to #8816.
中文说明
What this PR does
把 #8765 验证门尚未落地的加固移植到 main,并了结该 PR 的评审积压。#8765 以"已被吸收"关闭:#8816 的分支意外携带了它的早期提交,且托管循环在其上把门演进得更远(
comm -23子集同一性、retryable第三参、子集 fixtures)——在 #8816 之后再合 #8765 反而会回退这些演进。Why it's needed
main 的门上尚存五个缺口,均由 #8765 的评审(第 6–7 轮及人工评审的移植清单)点名:
outcome=failed推进水位线:瞬态 git 失败把条目永久搁置移交人类另有证据卫生(两个 A/B 侧日志加入修复清理列表)与评审用变异证明失明的六个测试缺口。
Reviewer Test Plan
预期 149/149(新增 6 条)。scripts 全量 1090 passed。
yamllint+bash -n干净。新场景在真实 git fixture 中执行真实的门:无同一性短路(transcript 无
Baseline A/B)、跟踪脏文件幸存(落下 pre-existing 判定而非崩溃)、无判定 restore 崩溃(无outcome=、detail 文档已写)、长 preamble 渲染上限(head -c 3900下围栏完好)、经可执行 report harness 的PREEXISTING子句选择(ahead → 自身代码,behind+冲突 → 合 main)、以及过期框架注记钉。变异测试——5 个全部被捕获:短路删除、restore 防护删除、restore 失败回退为已评估拒绝、dist 注记删除、过期框架注记删除。
Tested on
Risk & Scope
git restore -- .只丢弃跟踪文件的构建脏写;未跟踪文件冲突仍(正确地)走无判定崩溃——该场景有钉。第 6 轮指出的补救文案三处重复是接受的债务,在 thread 回复中注明。Linked Issues
了结 #8765 的评审积压(第 6–7 轮 + 协调评审);与 #8816 配套。