feat(autofix): deny-by-default footprint gate and positional window censuses - #9156
Conversation
…uses, review-loop backlog Follow-up to #8981/#8996, closing the structural causes behind their review-round non-convergence: - Deny-by-default footprint: every file a round touches maps to an AREA (declared workspace, else top-level directory, else the root file itself); areas outside the PR's own footprint are surfaced in a gate-authored advisory, or rejected retryably once the repo variable QWEN_AUTOFIX_FOOTPRINT_ENFORCE is staged to 'reject'. The enumerated class gate keeps rejecting regardless — this inverts the default for everything it cannot enumerate (a denylist is not a boundary). - The three window censuses (PRIOR_TIMEOUTS, WIN_HEADS, PRIOR_HEADS) attribute comments positionally over their own scan-parsed eval markers instead of whole-body win= substrings: a neutralized marker quoted in a handoff excerpt, or any future marker embedding win=, can no longer double-attribute a comment (decoy fixture included; the census fixture's non-numeric round= placeholder is corrected). - BITE_ENFORCE's reply arm inherits the thread root's CHANGES_REQUESTED membership, not just its body tag. - Backlog tests: the bite restore-failure crash contract (verdict-less exit with the rejection document, driven by a ref-deleting runner), merge-base-anchored footprint compares under an advanced main (afterPr fixture hook), and the shrink+bite advisory append order. - SKILL: cap each round's implemented batch (~8 findings, Critical first, defer the rest via comment-replies) — nine review rounds of evidence that oversized fix batches breed fix-of-fix defects — and document the footprint gate.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Re-run on
Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 本次为
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRead the full diff at
Two non-blocking nits, both cosmetic: the footprint test still provisions a TestingUnattended CI run — PR code is never executed in this environment; the evidence is the PR's own CI on the reviewed commit, quoted below. Two notes on what that evidence is worth here: the replay-style suite executes the real extracted gate blocks against fixture repos (not mocks), so a green run is direct evidence for the gate behavior, not just type-and-lint hygiene — and every R1/R2 fix above landed with its own discriminating fixture. The PR's Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The unit suite was still running on 中文说明代码审查在
两个不阻塞的外观小问题:足迹测试仍在为初版 resolver 设计遗留的 测试无人值守 CI 运行——本环境不执行 PR 代码;证据是被审 commit 上 PR 自身 CI,见上表。两点说明:回放式套件对 fixture 仓库执行的是真实抽取的门代码块(非 mock),绿色结果就是门行为的直接证据——且上述每项 R1/R2 修复都自带判别性 fixture。PR 描述中的本地测试数字是作者报告,下表 CI 才是权威信号。无用户可见产品面变化,故无真实场景 TUI 运行,也不附沙箱验证行——行为声明(advisory 触发、reject 触发、单射区域、last-wins 归属)正是回放测试所执行的内容,批准锚定在该次 CI 转绿(见 Stage 3 标记)而非本快照。 被审 commit 上单元套件在审查时仍在运行;finalize 任务会在 CI 结束后改写上方表格。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the R1/R2 rounds landed exactly as this series is supposed to work: every finding closed with a discriminating test pin and no scope creep; what remains is two cosmetic nits, and CI on the new head was still running at review time. Stepping back: I evaluated the fix delta on its own merit, and it holds up. The first cut of the footprint gate had three subtle problems that are easy to miss in shell — a resolver-prefix match that fused sibling nested workspaces, a lossy charset map under which two distinct areas could collapse into one comparison key (hiding an expansion is the exact failure mode this gate exists to prevent), and producer failure silently shrinking one side of the comparison into a verdict. All three are now fixed in the structurally right way: workspace globs compiled from the ref's own manifest with longest-ancestor matching, injective Every change in the diff still maps to a stated goal; the two fix commits carried nothing else. The standing reservations are minor and named in the review: a leftover resolver stub in one test fixture, and no fixture pin for the 中文说明置信度:4/5 —— R1/R2 两轮修复正是该系列应有的工作方式:每条发现都以判别性测试钉住关闭,无范围扩散;剩余仅两处外观小问题,且审查时新 head 上的 CI 仍在运行。 退一步看:我按自身价值评估了修复增量,站得住。足迹门的初版有三个在 shell 中容易漏掉的细微问题:resolver 前缀匹配会把兄弟嵌套 workspace 合并;有损字符集映射下两个不同区域可能坍缩为同一比较键(隐藏扩张正是此门要防止的失效模式);生产者失败会静默清空比较的一侧从而制造判定。三者现均以结构上正确的方式修复:从 ref 自身 manifest 编译 workspace 通配符并按最长祖先匹配、单射的 diff 中每处改动仍对应一个所述目标;两个修复 commit 未夹带其他内容。保留意见较小且已在评审中点名:一个测试 fixture 中遗留的 resolver 桩,以及 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未探索到全部深度(达到工具调用预算):"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| printf -- '- %s\n' "${OUT_AREAS//[^A-Za-z0-9._\/ -]/?}" | ||
| echo 'Review the expansion deliberately; the footprint gate is in advisory mode. · 本轮改动了 PR 足迹之外的区域(门自动测量,非 agent 文本),当前足迹门为 advisory 模式,请有意识地审阅该扩张。' | ||
| } >> "${WORKDIR}/gate-advisories.md" | ||
| echo "🧭 footprint expansion (advisory): $(tr '\n' ' ' <<< "${OUT_AREAS}")" | tee -a "${GATE_LOG}" |
There was a problem hiding this comment.
[Suggestion] R1-5: This third output sink writes OUT_AREAS to GATE_LOG raw, while the two sibling sinks this hunk adds (the reject-path GATE_LOG write at ~650 and the gate-advisories.md write at ~657) both apply the ${OUT_AREAS//[^A-Za-z0-9._\/ -]/?} charset — the protective idiom is borrowed two-thirds of the way. — Failure scenario: area names are path-derived and attacker-influenced; if the advisory fires and a later same-run check rejects, reject_fix tails ~3300 bytes of GATE_LOG into gate-rejection.md inside a four-backtick fence that is posted to the PR thread — an area name carrying four backticks closes the fence early and renders attacker-chosen markdown under the bot's identity. Marker forgery does not survive (the embed-site sed neutralizes <!--), so the cost is bounded to bot-identity markdown injection. Probe-verified: this line emits backticks raw while both siblings sanitize.
| echo "🧭 footprint expansion (advisory): $(tr '\n' ' ' <<< "${OUT_AREAS}")" | tee -a "${GATE_LOG}" | |
| echo "🧭 footprint expansion (advisory): $(tr '\n' ' ' <<< "${OUT_AREAS//[^A-Za-z0-9._\/ -]/?}")" | tee -a "${GATE_LOG}" |
中文说明
[Suggestion] R1-5:第三个输出槽把 OUT_AREAS 原样写入 GATE_LOG,而同一 hunk 新增的另外两个兄弟槽(reject 路径约 650 行的 GATE_LOG 写入、约 657 行的 gate-advisories.md 写入)都应用了 ${OUT_AREAS//[^A-Za-z0-9._\/ -]/?} 字符集——防护惯用法只借了三分之二。— 失败场景:区域名派生自路径、受攻击者影响;若 advisory 触发且同一次运行中后续检查拒绝,reject_fix 会把 GATE_LOG 末尾约 3300 字节嵌入 gate-rejection.md 的四反引号围栏内并发布到 PR 线程——带四个反引号的区域名会提前闭合围栏,让攻击者选定的 markdown 以机器人身份渲染。标记伪造无法存活(内嵌点的 sed 会中和 <!--),因此代价限于机器人身份的 markdown 注入。探针验证:本行原样输出反引号,而两个兄弟槽都做了消毒。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
Queued per the round's batch cap: the GATE_LOG sink gets the same charset treatment with the per-line bullet rework.
🤖 via Claude Code · model claude-fable-5
| [[ "${FOOTPRINT_ENFORCE}" == 'reject' ]] || FOOTPRINT_ENFORCE='advisory' | ||
| ROUND_FILES_Z="$(mktemp)" | ||
| PR_FILES_Z="$(mktemp)" | ||
| git diff --name-only -z --no-renames "${ROUND_RANGE}" 2> /dev/null | not_merge_freight > "${ROUND_FILES_Z}" || true |
There was a problem hiding this comment.
[Suggestion] R1-13: The round-side merge-freight exclusion is untested — no footprint fixture contains a freight file, so replacing not_merge_freight with cat in this pipe leaves every assertion green (mutation verified). — Failure scenario: on conflict-resolution rounds (SKILL directs merging origin/<base>), the round diff carries main's drifted files; not_merge_freight is what keeps them from counting as footprint expansion. If the pipe regresses, such a round fires a spurious advisory — or, once QWEN_AUTOFIX_FOOTPRINT_ENFORCE is staged to reject, a retryable rejection instructing the agent to revert files it never authored but merged in from main. Probe: an end-to-end conflict-round fixture under the mutation fires the advisory for a file the agent never authored; the pristine pipe stays silent. Suggested fix: add a fixture via the new afterPr hook — main drifts a file in an area the PR never touched, the round carries the freight file; assert no advisory in advisory mode and no rejection in enforce: 'reject' mode.
中文说明
[Suggestion] R1-13:轮侧合并货运排除未被测试——没有足迹 fixture 包含货运文件,因此把该管道中的 not_merge_freight 替换为 cat,所有断言仍为绿色(已突变验证)。— 失败场景:冲突解决轮次(SKILL 指示合并 origin/<base>)中,轮 diff 携带 main 漂移的文件;not_merge_freight 正是阻止它们被计为足迹扩张的机制。若该管道退化,这类轮次会触发误报 advisory——或者在 QWEN_AUTOFIX_FOOTPRINT_ENFORCE 分级到 reject 后,触发可重试拒绝,指示 agent 回退它从未撰写、只是从 main 合并进来的文件。探针:突变下的端到端冲突轮 fixture 会为 agent 从未撰写的文件触发 advisory;原始管道保持静默。建议修复:用新的 afterPr 钩子新增 fixture——main 在 PR 从未触碰的区域漂移一个文件,轮次携带该货运文件;断言 advisory 模式下无 advisory、enforce: 'reject' 模式下无拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
Queued per the round's batch cap: a freight-file footprint fixture (conflict-resolution round shape).
🤖 via Claude Code · model claude-fable-5
| 'ROUND_RANGE="origin/feat...feat"', | ||
| 'PR_RANGE="origin/main...origin/feat"', |
There was a problem hiding this comment.
[Suggestion] R1-16: The gate's merge-base anchoring (PR_RANGE="origin/main...origin/${BRANCH}", script ~477) has no regression fixture of its own: every footprint fixture leaves main unmoved, so three-dot ≡ two-dot, and the new afterPr hook is used only by the class-gate test, whose rejection fires before the footprint block runs. Probe: with afterPr advancing main into a new top-level area, three-dot ≠ two-dot — the two-dot form over-grants the drifted area. — Failure scenario: a future rewrite of script line 477 to a two-dot direct diff over-grants every area main touched after the branch point, silently disabling reject-mode enforcement for expansions into those areas — and all footprint tests stay green. The author added the afterPr hook and a merge-base fixture for the class gate in this same diff; the footprint gate, where the anchoring is the gate's entire correctness story, did not get the equivalent. Suggested fix: add a footprint fixture using afterPr — main drifts into a new top-level area after the branch point, the round touches that area; assert the advisory/rejection still fires.
中文说明
[Suggestion] R1-16:门的 merge-base 锚定(PR_RANGE="origin/main...origin/${BRANCH}",脚本约 477 行)没有属于自己的回归 fixture:所有足迹 fixture 都不移动 main,因此三点 ≡ 两点;新的 afterPr 钩子只被类门测试使用,而类门的拒绝在足迹块运行之前就已触发。探针:用 afterPr 把 main 推进到一个新的顶层区域后,三点 ≠ 两点——两点形式会把漂移区域过度授权。— 失败场景:未来把脚本 477 行改写为两点直接 diff,会把分支点之后 main 触碰过的每个区域都过度授权,静默禁用对这些区域扩张的 reject 模式执法——而所有足迹测试保持绿色。作者在同一个 diff 里为类门添加了 afterPr 钩子和 merge-base fixture;足迹门——其锚定是门的全部正确性所在——却没有得到等价物。建议修复:新增使用 afterPr 的足迹 fixture——分支点后 main 漂移进新的顶层区域,轮次触碰该区域;断言 advisory/拒绝仍会触发。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
Queued per the round's batch cap: an afterPr footprint fixture proving three-dot vs two-dot divergence.
🤖 via Claude Code · model claude-fable-5
| # surfaces rather than hides). | ||
| list_areas() { | ||
| local wss f matched w | ||
| wss="$(tr '\0' '\n' < "${1}" | bash "${RUNNER_TEMP}/resolve-owning-packages.sh" 2> /dev/null || true)" |
There was a problem hiding this comment.
[Critical] R1-18: The new footprint gate computes its entire area mapping from ${RUNNER_TEMP}/resolve-owning-packages.sh (this new consumer), but that file is never digest-pinned: staging records digests only for resanitize-git-config.sh and the runner (workflow ~3993-3994), and both verify steps re-verify only the runner before executing. The host-side branch build/test the gate itself runs (script ~709+) executes between staging and the second gate pass with full write access to RUNNER_TEMP — the overwrite threat the verify step's own comment documents and defends against only for the pinned scripts. — Failure scenario: a tampered resolver emitting coarse prefixes (e.g. packages for any input under packages/) maps BOTH sides of the comparison to the same area → comm -23 empty → the gate passes silently in advisory AND reject mode, defeating the staged rollout's end-state with no observable signal. Probe-verified flip (the clean resolver rejects; the tampered resolver survives both modes with no advisory). The sandboxed agent shell cannot reach the path — the confirmed vector is the host-side branch build/test inside the gate. Suggested fix: record the resolver's digest at staging and sha256sum -c it next to VERIFY_RUNNER_SHA256 before both gate passes.
中文说明
[Critical] R1-18:新的足迹门从 ${RUNNER_TEMP}/resolve-owning-packages.sh(本 diff 的新消费者)计算其全部区域映射,但该文件从未被摘要钉扎:staging 只为 resanitize-git-config.sh 与 runner 记录摘要(workflow 约 3993-3994),两个 verify 步骤在执行前也只复核 runner。门自身运行的宿主侧分支 build/test(脚本约 709+)在 staging 与第二次门通过之间执行,对 RUNNER_TEMP 有完整写权限——verify 步骤自己的注释记载了这一运行中覆盖威胁,却只为被钉扎的脚本做了防护。— 失败场景:被篡改的 resolver 输出粗糙前缀(例如对 packages/ 下任何输入都输出 packages)时,比较两侧映射到同一区域 → comm -23 为空 → 门在 advisory 与 reject 模式下都静默通过,分级推进的终态在无观测信号的情况下失效。探针验证翻转(干净 resolver 拒绝;篡改后的 resolver 在两种模式下都无 advisory 地通过)。沙箱内的 agent shell 无法触及该路径——已确认的向量是门内部的宿主侧分支 build/test。建议修复:在 staging 记录 resolver 的摘要,并在两次门通过前与 VERIFY_RUNNER_SHA256 一并 sha256sum -c。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
Partially addressed in 3bce044: the footprint path no longer consumes the staged resolver at all (ref-anchored globs), which removes this consumer from the unpinned surface. Digest-pinning the resolver for its remaining consumers (bite, class scans) is queued with the follow-up backlog.
🤖 via Claude Code · model claude-fable-5
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its workflow suite did not run locally.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its workflow suite did not run locally。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— Qwen Code via Qwen Code /review (v0.21.11)
- Advisory lifecycle: one reset at gate start, every writer appends — the footprint advisory no longer dies to the shrink section's rm or its truncating write. - Footprint membership is REF-ANCHORED: areas derive from the pre-round root manifest's workspaces globs (longest ancestor wins, nested workspaces correct), so a round cannot redefine its own boundary and the on-disk resolver is out of this path entirely; non-workspace paths under packages/ keep two segments so sibling projects stay distinct areas; emitted areas are newline-sanitized against phantom footprint grants. - The enforcement knob rides step-level env at both verify gates — $GITHUB_ENV writes from earlier steps cannot downgrade 'reject'. - TESTSIDE's critical() mirrors cr_attached (root and self), keeping enforcement and demotion on one comment set. - Census ownership is LAST-WINS over scan-parsed markers (a stray quoted-or-appended marker cannot double-attribute), the replay decoy is now genuinely discriminating (old whole-body → 0, new → 1), and the growth-gate comment stops citing retired whole-body matchers. Queued per the batch cap: per-line advisory bullets and the third sink charset, discriminating fixtures at the two remaining census sites, the reply-arm bite fixture, freight and merge-base footprint fixtures, and digest-pinning the staged resolver for its remaining consumers.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget (~25 calls). One environmental note: directory-rooted grep_search over the worktree intermittently under-rep…; "PR #9156 (QwenLM/qwen-code) inverts the autofix…": none — completed all planned checks. Used about 22 tool calls.; "PR #9156 (QwenLM/qwen-code) inverts the autofix…": none significant. I have not run the full 172-test file (I ran the three directly-touched tests). I can note this as disclosure: "Full suite of scripts/tests/qw…; "PR #9156 (QwenLM/qwen-code) inverts the autofix…": none — all checks I started reached a conclusion within budget.; "PR #9156 (QwenLM/qwen-code) inverts the autofix…": none significant. I did not exhaustively trace whether a fork-head PR named 'main' reaches the review-address job end-to-end (that's the uncertainty behind find…, and 1 more.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"You are review agent reverse-audit — Reverse audit agent…":none — all planned checks completed within budget (~25 calls). One environmental note: directory-rooted grep_search over the worktree intermittently under-rep…;"PR #9156 (QwenLM/qwen-code) inverts the autofix…":none — completed all planned checks. Used about 22 tool calls.;"PR #9156 (QwenLM/qwen-code) inverts the autofix…":none significant. I have not run the full 172-test file (I ran the three directly-touched tests). I can note this as disclosure: "Full suite of scripts/tests/qw…;"PR #9156 (QwenLM/qwen-code) inverts the autofix…":none — all checks I started reached a conclusion within budget.;"PR #9156 (QwenLM/qwen-code) inverts the autofix…":none significant. I did not exhaustively trace whether a fork-head PR named 'main' reaches the review-address job end-to-end (that's the uncertainty behind find…,另有 1 条。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| | select(([ ((.body // "") | scan("<!-- autofix-eval ts=[^ ]+ acted=[^ ]+ round=[0-9]+(?: win=([^ ]+))? -->")) ] | map(.[0] // "none")) as $wins | ||
| | ($wins | length) > 0 and (($wins | last) == $key)) |
There was a problem hiding this comment.
[Suggestion] This diff pastes the same scan+last-wins expression into three census sites (here, ~5968 WIN_HEADS, ~6441 PRIOR_HEADS), minting new textual variants of the eval-marker regex alongside the three pre-existing full-capture copies (~3029, ~4379, ~4703) — the marker grammar written at ~5869/5901/6570 now has six scan consumers in two variants. — Failure scenario: this PR itself demonstrates the tax (changing attribution required three byte-identical synchronized edits in one file); a future marker change (e.g. a new field before -->) that updates one variant but not the other makes the censuses and the watermark/round tracking silently disagree about which comments parse — rounds mis-attributed to windows, breaker counts off, no error surfaced. Suggested fix: reuse the existing full-capture regex at the three new sites (map(.[3] // "none") over the 4-group scan) so all six consumers share one grammar.
中文说明
[Suggestion] 本 diff 把同一份 scan+last-wins 表达式粘贴进三个 census 位点(此处、约 5968 WIN_HEADS、约 6441 PRIOR_HEADS),在既有三份全捕获副本(约 3029、4379、4703)之外又造出 eval 标记 regex 的新文本变体——约 5869/5901/6570 写入的标记语法现在共有六个扫描消费者、两种变体。— 失败场景:本 PR 本身已展示该成本(改归属方式需要在一个文件里做三处字节一致的同步修改);未来任何标记变更(例如在 --> 前新增字段)若只更新其中一种变体,census 与水位/轮次追踪就会对哪些评论可解析产生静默分歧——轮次被错误归属到窗口、breaker 计数偏差,且不冒任何错误。建议修复:三个新位点复用既有全捕获 regex(在 4 组 scan 上取 map(.[3] // "none")),让六个消费者共享同一语法。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
Queued as its own change per the batch cap: hoisting the eval-marker grammar into one constant consumed by all six scan/parse sites touches six delicate jq programs and deserves an isolated diff; noted on the backlog with the resolver digest-pinning.
🤖 via Claude Code · model claude-fable-5
- list_areas reads and translates the ref's workspaces globs ONCE per invocation and matches ancestors in-bash (was ~21ms git+jq+sed per file×ancestor call), emits printf %q keys — line-safe AND injective, so distinct areas can never collapse into one comparison key — and both render arms print one bullet per area from those keys. - Producer failures are a STATE: a failed round- or PR-side diff (orphan history, transient git error) skips the footprint check loudly instead of shrinking one side into a verdict. - The workflow-level FOOTPRINT_ENFORCE env is gone (the step-level pins are the only consumers and outrank it — dead config removed); the two step wirings are count-pinned. - Fixtures: nested-workspace membership discriminates against the packages/ two-segment fallback (sibling nested workspaces stay distinct areas), and the advisory-lifecycle discriminator proves an earlier section's advisory survives the shrink section. Queued: consolidating the six eval-marker regex variants behind one grammar constant (touches six jq programs; its own change).
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 115 passed · 0 failed · 115 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:115 通过 · 0 失败 · 115 总计 Verification reportPR #9156 Deep Verification — deny-by-default footprint gate, positional window censusesVerdict: 中文摘要结论: A/B 结论:
Findings(均非阻塞):两个 PR 新引入的守卫( 未覆盖:逐 commit 归属(浅克隆仅可达 head 一个 commit);完整 Central claim + A/BCentral claim: every file an autofix round touches maps to an AREA (declared The block under test was extracted verbatim from
36 scripted assertions, all passed — witness: Secondary claim — positional censuses. All three census jq programs
Witness: Findings (none blocking)F1 — Suggestion (completeness): two of the PR's own guards survive suite mutation.
Both survivors are coverage gaps, not defects — the behavior at HEAD is F2 — Note (behavior boundary): malformed/legacy markers no longer attribute. F3 — Note: the BITE reply-arm change ships without its fixture. The reply F4 — Environmental note: the working tree's Not covered
MethodologyEnvironment: the CI verify container ( Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 8 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): "This PR inverts the autofix review-round protection to…": None. Completed all checks..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 8 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未探索到全部深度(达到工具调用预算):"This PR inverts the autofix review-round protection to…":None. Completed all checks.。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
| if was_workspace_dir "${ref}" "${d}"; then | ||
| a="${d}" | ||
| break | ||
| fi |
There was a problem hiding this comment.
[Suggestion] list_areas calls was_workspace_dir once per (file × ancestor directory), and each call spawns git show + jq + sed subprocesses against the same ref. There is no caching of the manifest parse.
Failure scenario: A round touching 50 files and a PR with 200 files, each ~3 directories deep, triggers about 750 identical git show + jq subprocess spawns. All produce the same output. In GitHub Actions, 750 short-lived pipelines vs. 1 produces measurable wall-clock waste for the footprint check.
Suggested fix: Cache the workspace manifest: read git show "${ref}:package.json" | jq -r '.workspaces[]?' once at the top of list_areas, store the globs in an array, and iterate over the cached array per directory.
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
| [[ "${FOOTPRINT_ENFORCE}" == 'reject' ]] || FOOTPRINT_ENFORCE='advisory' | ||
| ROUND_FILES_Z="$(mktemp)" | ||
| PR_FILES_Z="$(mktemp)" | ||
| git diff --name-only -z --no-renames "${ROUND_RANGE}" 2> /dev/null | not_merge_freight > "${ROUND_FILES_Z}" || true |
There was a problem hiding this comment.
[Suggestion] The || true on the round-side git diff pipeline means a failure (e.g. stale tracking ref after a force-push) silently produces an empty ROUND_FILES_Z. The list_areas call on an empty file produces no output, so OUT_AREAS is empty, and the footprint check passes without flagging.
Failure scenario: A race condition where the branch's remote tracking ref (origin/${BRANCH}) is stale or missing at the moment git diff runs. Not exploitable by a PR contributor (the tracking ref is created by the workflow's own git fetch), but the failure is silent.
Suggested fix: When the round-side git diff produces no output and also has a non-zero exit code, write a 'footprint measurement unavailable' advisory rather than silently passing. Or remove the || true and let the pipeline fail loud.
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
| ROUND_FILES_Z="$(mktemp)" | ||
| PR_FILES_Z="$(mktemp)" | ||
| git diff --name-only -z --no-renames "${ROUND_RANGE}" 2> /dev/null | not_merge_freight > "${ROUND_FILES_Z}" || true | ||
| git diff --name-only -z --no-renames "${PR_RANGE}" 2> /dev/null > "${PR_FILES_Z}" || true |
There was a problem hiding this comment.
[Suggestion] The PR-side git diff pipeline has the identical || true pattern as the round-side (finding f7). If the PR range is invalid, PR_FILES_Z is silently empty, and comm -23 reports every round area as an expansion.
Failure scenario: A transient failure of git diff --name-only -z ${PR_RANGE} produces an empty PR file list. The footprint check then compares the round's areas against an empty PR footprint, flagging all round areas as expansions. Because || true swallows the exit code, the gate has no way to distinguish 'no PR footprint available' from 'PR footprint is genuinely empty'.
Suggested fix: When the PR-side git diff fails with a non-zero exit, write a 'footprint measurement unavailable' advisory rather than silently producing a false-positive expansion report.
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
| const crossWorkspace = { | ||
| base: ({ write }) => { | ||
| write('packages/cli/src/a.ts', 'a\n'); | ||
| write('packages/core/src/b.ts', 'b\n'); | ||
| }, |
There was a problem hiding this comment.
[Suggestion] The list_areas function has three code paths (workspace detection via was_workspace_dir, packages/*/* fallback, top-level dir / root file), but the footprint test fixtures never write a package.json with workspace declarations, so the workspace-detection path is never exercised.
Failure scenario: If was_workspace_dir returns false for a legitimate workspace (e.g., from a malformed glob in a ref's package.json), list_areas falls back to the coarser packages/*/* or top-level-dir mapping. For a non-packages/ workspace like apps/backend, the fallback maps to apps (the top-level dir). Conservative rather than wrong, but untested.
Suggested fix: Add a fixture that declares workspaces in the root package.json (e.g., "workspaces": ["packages/*"]), and verify that a file inside the declared workspace resolves to the workspace path, not the packages/* fallback.
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
|
Released in v0.21.13. |




What this PR does
Inverts the review-round protection model to deny-by-default: every file a round touches is mapped to an area — its declared workspace (resolver-backed, nested included), else its top-level directory, else the root file itself — and any area outside the PR's own footprint is surfaced in a gate-authored advisory, escalating to a retryable rejection once the repo variable
QWEN_AUTOFIX_FOOTPRINT_ENFORCEis staged fromadvisory(default) toreject. The enumerated sensitive-class gate from #8996 keeps rejecting regardless of the switch. Also: the three window censuses attribute comments positionally over their own scan-parsed eval markers instead of whole-bodywin=substrings;BITE_ENFORCE's reply arm inherits the thread root's CHANGES_REQUESTED membership; SKILL caps each round's implemented batch (~8 findings, Critical first); and four queued backlog tests land (bite restore-crash contract, merge-base footprint under an advanced main, advisory append order, census decoy).Why it's needed
#8996's nine review rounds demonstrated two structural problems this PR closes. First, a denylist-style protected-surface taxonomy generates unbounded "you missed surface X" findings — the enumeration can never complete because the repo's execution graph keeps growing; deny-by-default retires that finding class by construction while keeping consequences staged (advisory first, evidence before enforcement). Second, whole-body
win=census attribution could double-attribute a comment carrying two window keys or a quoted, neutralized marker in a handoff excerpt — probe-shown against PRIOR_TIMEOUTS/WIN_HEADS/PRIOR_HEADS in #8981's review. The SKILL batch cap encodes the measured process lesson: 8–16 fixes per round bred fix-of-fix Criticals across both parent PRs.Reviewer Test Plan
How to verify
npm run test:scripts(54 files, 1163 passed / 13 skipped) or the single file vianpx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow(172). New behavioral replays: the footprint gate runs the real extracted block against fixture repos (cross-workspace expansion → advisory listing the area;rejectmode → retryable rejection; in-footprint and same-top-dir rounds → silent; root files as their own areas; garbage enforcement value degrades to advisory); the census replay adds a quoted-neutralized-marker decoy that old whole-body matching mis-attributed (old → '0', new → '1'); the bite suite adds a ref-deleting runner that proves the verdict-less crash contract; the footprint-class suite adds an advanced-main fixture proving merge-base anchoring; the advisory suite proves shrink+bite advisories co-exist in one report.Evidence (Before & After)
N/A — CI workflow/gate logic; behavior exercised by the replays above. Before: surfaces outside the enumerated classes were silently free for any round. After: expansions outside the PR footprint are machine-surfaced (or rejected under staged enforcement).
Tested on
Risk & Scope
rejectgated behind a repo variable after observation. Deleted workspaces degrade to their top-level segment (mismatch surfaces rather than hides).Linked Issues
Follow-up to #8981 and #8996 (the census hazard and the enumeration-gap class were reviewed there and queued to this change).
中文说明
本 PR 做了什么
把评审轮次的保护模型反转为 deny-by-default:轮次触碰的每个文件映射到一个区域——其声明的 workspace(经 resolver,含嵌套),否则顶层目录,否则根文件本身——PR 自身足迹之外的区域以门亲笔 advisory 上浮,当仓库变量
QWEN_AUTOFIX_FOOTPRINT_ENFORCE从默认advisory调到reject时升级为可重试拒绝。#8996 的枚举类门不受开关影响照常拒绝。另含:三个窗口 census 改为按各自 scan 解析的 eval 标记做位置化归属(不再整体匹配win=子串);BITE_ENFORCE的回复臂继承线程根的 CHANGES_REQUESTED 成员资格;SKILL 给每轮实现批量设上限(约 8 条,Critical 优先);四项排队的 backlog 测试落地(bite 恢复失败 crash 契约、main 推进下的 merge-base 足迹比较、advisory 追加顺序、census 诱饵)。为什么需要
#8996 的九轮评审暴露了本 PR 关闭的两个结构性问题。其一,denylist 式保护面分类法会产生无界的"你漏了面 X"finding——仓库执行图在增长,枚举永不完备;deny-by-default 按构造退役该 finding 类,同时后果分级(先 advisory、有证据再执法)。其二,整体
win=匹配可能把携带两个窗口键、或引用了已中和标记的评论重复归属——已在 #8981 评审中对三个 census 逐一 probe 证实。SKILL 批量上限编码的是实测流程教训:每轮 8–16 条修复在两个父 PR 中都催生了"修复的修复"级 Critical。评审验证方案
如何验证
npm run test:scripts(54 文件,1163 通过/13 跳过)或单文件npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow(172)。新增行为回放:足迹门以真实抽取代码块驱动 fixture 仓库(跨 workspace 扩张 → advisory 列出区域;reject模式 → 可重试拒绝;足迹内与同顶层目录 → 静默;根文件各为独立区域;非法执法值退化为 advisory);census 回放加入"引用已中和标记"诱饵,旧整体匹配误归属(旧 → '0',新 → '1');bite 套件用删 ref 的 runner 证明无判定 crash 契约;足迹类套件用推进 main 的 fixture 证明 merge-base 锚定;advisory 套件证明收缩与 bite 两条 advisory 并存。证据(Before & After)
N/A —— CI workflow/gate 逻辑,行为由上述回放覆盖。Before:枚举类之外的面对任何轮次静默放行。After:PR 足迹外的扩张被机器上浮(或在分级执法下拒绝)。
测试平台
风险与范围
reject需观察后经仓库变量启用。删除的 workspace 退化为顶层段(不匹配会上浮而非隐藏)。关联 Issue
#8981 与 #8996 的 follow-up(census 隐患与枚举缺口类在彼处评审并排队至本改动)。