fix(autofix): mutation-probe new guards before a round commits - #9578
Conversation
Provenance analysis of six multi-round takeover PRs found roughly a third of post-initial review findings were introduced by the immediately preceding fix round, and the dominant shape was guards and branches added with no test of their own: the deterministic gate re-runs only the tests that exist, so an unwitnessed guard passes every gate and its hole resurfaces as a new finding in a later round. Require the agent to prove each newly added guard or branch kills a test (mutation probe) before committing, and record the probe in the round summary.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The motivation is a provenance analysis of six merged multi-round autofix/review PRs — I spot-checked all six (#9297, #9321, #9327, #9342, #9345, #9216) and they exist and are merged. The claimed pattern — roughly a third of post-initial-review findings introduced by the immediately preceding fix round, dominated by guards/branches added without their own test — is measured churn in this repo's own pipeline. The mechanism is real by construction: the deterministic gate re-runs only the tests that exist, so an unwitnessed guard passes every gate by definition. Direction: aligned. This is an internal autofix-workflow quality rule from the collaborator who maintains that workflow (all six cited PRs are the author's). It touches no auth/sandbox/model-selection/telemetry/public-contract surface. No CHANGELOG direction signal applies — internal tooling. Size: not applicable — no core paths ( Approach: minimal and consistent with existing conventions. The new bullet lands in the GitHub Actions Rules section directly after the existing verification-commands rule, and the contract-test pin ( Risk: no elevated risk signals — neither changed file matches the revert-history high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 已观测到,而非理论性问题。动机是对六个已合入的多轮 autofix/review PR 的逐 finding 溯源分析——我抽查了全部六个(#9297、#9321、#9327、#9342、#9345、#9216),均真实存在且已合入。所声称的模式——首轮 review 之后新出现的发现约三分之一由上一轮修复自身引入,且主要形态是新增 guard/分支没有自己的测试——是本仓库自身流水线中实测的返工。其机制从构造上就成立:确定性 gate 只重跑已存在的测试,因此没有 witness 的 guard 按定义能通过所有 gate。 方向: 对齐。这是维护该 autofix 工作流的 collaborator 提交的内部工作流质量规则(六个被引用的 PR 均为作者本人)。不触及 auth/sandbox/模型选择/telemetry/公共契约面。CHANGELOG 方向信号不适用——纯内部工具。 规模: 不适用——未触及核心路径( 方案: 最小化且符合既有惯例。新规则落在 GitHub Actions Rules 一节、紧跟现有验证命令规则之后;契约测试 pin( 风险: 无升级风险信号——两个改动文件均未命中 revert 历史高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewClean. Two files, +11/−0, exactly what the description promises. My independent take on the problem would have landed on this same shape: a prompt-level rule plus a contract-test pin. The mechanical alternative — making the deterministic gate itself run mutation passes over every new guard — would need to identify "new guard" programmatically (not decidable in general) and would add cost to every round; that's over-engineering at this point. The rule text is well placed (GitHub Actions Rules, directly after the verification-commands rule it extends), and its premise matches the surrounding text: the gate re-runs only the commands/tests that exist, so an unwitnessed guard genuinely cannot be caught by it. The pin is sound: No correctness, security, or convention findings. TestingThis comment carries the PR's own CI signal fetched via the GitHub API; nothing was built or run locally (unattended triage never executes PR code). Real-scenario tmux testing: N/A — nothing user-visible, and this is an unattended CI run. At fetch time there are no failures. The only meaningful pending check is One note on the author's test plan: the report that two other tests in this file ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 No sandboxed-lane line here: the change is agent-prompt text plus a contract pin. Whether the rule actually reduces later-round churn is a behavioural question the PR itself says can only be measured after landing — there is no runtime code path for an A/B run to settle, and the pinning claim is exactly what the CI suite above substantiates. 中文说明代码审查干净。两个文件,+11/−0,与描述完全一致。 我对该问题的独立方案也会落在同样的形态上:提示词规则 + 契约测试 pin。机械化的替代方案——让确定性 gate 自身对每个新 guard 跑 mutation pass——需要在程序上识别"新 guard"(一般情况下不可判定),且每轮都要额外付出成本;现阶段属于过度设计。规则文本位置恰当(GitHub Actions Rules 一节、紧跟其所扩展的验证命令规则之后),其前提与上下文一致:gate 只重跑已存在的命令/测试,因此没有 witness 的 guard 确实无法被它捕获。 pin 是可靠的: 无正确性、安全性或规范问题。 测试本评论携带的是通过 GitHub API 获取的 PR 自身 CI 信号;未在本地构建或运行任何内容(无人值守 triage 从不执行 PR 代码)。真实场景 tmux 测试:N/A——无用户可见变化,且本次为无人值守 CI 运行。 获取时无任何失败。唯一有意义的待决检查是 关于作者测试计划的一点说明:同文件中另外两个用例( 不附加沙箱验证行:本改动是 agent 提示词文本 + 契约 pin。规则是否真能减少后续轮次返工,是 PR 自己也说只能落地后度量的行为性问题——没有可供 A/B 验证的运行时路径,而 pin 是否钉住恰由上述 CI 套件证实。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid, minimal, and data-motivated; the only reservations are inherent ones (agent-enforced rule, effectiveness measurable only post-landing), and CI is still finishing. This one is easy to be in favor of. The motivation is measured, not vibes — I checked the six cited takeover PRs and they're all real, merged, multi-round efforts, and the mechanism the rule attacks is real by construction: a gate that only re-runs existing tests structurally cannot catch a guard nobody tests. The fix is exactly as big as it needs to be: one bullet in the right section, one pin in the existing contract-test style, nothing else. If I'm maintaining this in six months, the rule reads clearly and reverting it is one bullet. The honest reservations, neither blocking: the rule is enforced by the agent's compliance like every other bullet in that section (the pin keeps the clause from being silently deleted, but nothing mechanically forces the probe), and whether it actually cuts later-round churn can only be measured after it lands — the PR says so itself. Those are properties of the problem, not defects of this PR. The heavier mechanical enforcement would cost more than it saves right now. The 中文说明这个 PR 很容易让人支持。动机来自实测而非感觉——我核查了引用的六个 takeover PR,均真实存在、已合入且都是多轮工作;规则所针对的机制从构造上就成立:只重跑已有测试的 gate,结构上就不可能捕获没人测试的 guard。修复的体量恰到好处:正确小节里的一条规则、既有契约测试风格的一个 pin,别无其他。六个月后维护它时,规则表述清晰,回退也只是删一条 bullet。 两个诚实的保留意见,均非阻塞:该规则与本节其他条目一样依赖 agent 自觉执行(pin 防止条款被静默删除,但没有机械手段强制做探测);规则能否真正减少后续轮次返工只能落地后度量——PR 自己也这么说。这是问题本身的属性,不是本 PR 的缺陷;更重的机械化强制目前得不偿失。
— 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.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The original pin stopped at the preposition "before", so a future edit that kept the intro phrase but gutted the remove/confirm-FAIL/ restore procedure would leave the contract test green. Pin both halves of the mandate. Mutation-verified: gutting the mandate in SKILL.md now fails the skill contract test.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The procedure pins cover what the probe does, but not what to do
when it exposes an unwitnessed guard: dropping the remediation
sentence ("write a test that pins it (or drop the guard)") left the
whole suite green while the rule told the agent to probe without a
verdict path. Pin the remediation clause too; mutation-verified.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 6b": full vitest run of scripts/tests/qwen-autofix-workflow.test.js (no node_modules in worktree or main checkout; npm ci skipped as disproportionate).
中文说明
未发现问题。LGTM!✅
未探索到全部深度(达到工具调用预算):"agent 6b":full vitest run of scripts/tests/qwen-autofix-workflow.test.js (no node_modules in worktree or main checkout; npm ci skipped as disproportionate)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. CI green. The new autofix-skill rule closes the unwitnessed-guard hole: every guard/branch/behavior a round commits must have a test witness, proven by a mutation probe (negate the guard, confirm the focused tests fail, then restore to green), else the guard is dropped or pinned. The workflow test pins the new guidance verbatim.
|
Released in v0.21.15. |
… guards Merges origin/main (2172721) into the Aone dedup PR. One conflict: - SKILL.test.ts: main added three revert-guard tests (fix-witness mandate, fix-induced disposition, census contract — from #9578/#9596) plus an older self-PR-only variant of the Aone presubmit guard (#9616). Kept main's three new tests and this branch's fuller "runs comment-status and presubmit on Aone targets — backed, not skipped" guard (which supersedes the self-PR-only variant, since #9627 backed both subcommands); dropped the older variant. Verified: npm run build green, tsc clean on packages/cli + packages/core, SKILL.test.ts 30/30, and 532 review tests pass (presubmit GitHub+Aone, comment-status, aone/aone-client platform, submit-aone, cleanup, test-plan).
What this PR does
Adds a new hard rule to the autofix skill's GitHub Actions rules: every guard, branch, or behavior a round's commits add must have its own witness in the tests the same round commits, verified by a mutation probe before committing. The agent temporarily removes or negates the new guard or branch, re-runs the focused tests that should catch it, confirms they fail, restores the change, and re-runs to green. If the suite stays green with the guard deleted, the guard has no coverage, and the agent must write a test that pins it (or drop the guard) rather than shipping it. Each probe and its result must be recorded in the round summary alongside the verification commands. The workflow's contract test pins the new clause so it cannot be silently removed later.
Why it's needed
A provenance analysis of six multi-round takeover PRs (#9297, #9321, #9327, #9342, #9345, #9216) traced every post-initial review finding back to the commit that introduced its commented code. Roughly a third of the 146 findings that first appeared after the initial review were introduced by the immediately preceding fix round (over half when the largest, outlier PR is excluded). The dominant shape was guards and branches added with no test of their own: the deterministic verification gate re-runs only the tests that exist, so an unwitnessed guard passes every gate, ships green, and resurfaces as a brand-new finding in a later round. Requiring a mutation probe before committing converts that later-round churn into same-round work the agent can do immediately.
Reviewer Test Plan
How to verify
Run the workflow contract test and confirm it passes and pins the new clause:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t "keeps agent decision logic in the project autofix skill". Confirm the new rule sits in the GitHub Actions Rules section of the autofix skill, right after the existing verification-command rule, and reads as described above. Note: two tests in the same file (keeps the green path intact,bite check: rejects a round whose changed tests pass on the pre-round tree) fail on this machine even at a clean checkout (exit 127, missing gate-harness dependency) — that failure is pre-existing and unrelated to this change.Evidence (Before & After)
N/A — agent-prompt text plus a contract-test pin; no user-visible behavior.
Tested on
Environment (optional)
Unit tests only (
npx vitestwith the scripts test config).Risk & Scope
Linked Issues
None.
中文说明
本 PR 做了什么
在 autofix skill 的 GitHub Actions 规则中新增一条硬性规则:每轮 commit 新增的 guard、分支或行为,必须在同一轮提交的测试中拥有自己的 witness,并在提交前通过 mutation probe 验证。agent 需要临时删除或取反新增的 guard/分支,重跑应捕获它的 focused 测试并确认测试失败,然后恢复改动并重跑至通过。若删除 guard 后测试仍然全绿,说明该 guard 没有覆盖,agent 必须补一个能钉住它的测试(或删掉该 guard),不允许带病提交。每次探测及其结果都要与验证命令一起记录在轮次总结中。workflow 契约测试同时对该条款加了 pin,防止其日后被静默删除。
为什么需要
对六个多轮 takeover PR(#9297、#9321、#9327、#9342、#9345、#9216)做了逐 finding 溯源分析,将每个首轮之后的 review 发现回溯到引入其评论代码的 commit。首轮之后新出现的 146 个发现中,约三分之一由上一轮修复自身引入(剔除体量异常的离群 PR 后超过一半)。最主要的形态是新增的 guard/分支没有自己的测试:确定性验证 gate 只重跑已存在的测试,因此没有 witness 的 guard 会通过所有 gate、全绿合入,并在之后的轮次作为全新发现重新冒出来。要求提交前做 mutation probe,可以把这类后续轮次的返工转化为本轮就能完成的工作。
评审者测试计划
如何验证
运行 workflow 契约测试并确认其通过且钉住了新条款:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t "keeps agent decision logic in the project autofix skill"。确认新规则位于 autofix skill 的 GitHub Actions Rules 一节、紧跟现有验证命令规则之后,内容与上文描述一致。注意:同一测试文件中的两个用例(keeps the green path intact、bite check: rejects a round whose changed tests pass on the pre-round tree)在本机即使干净检出也会失败(exit 127,缺少 gate harness 依赖)——该失败为既有环境问题,与本改动无关。证据(改动前后对比)
N/A —— agent 提示词文本 + 契约测试 pin,无用户可见行为。
测试平台
环境(可选)
仅单元测试(scripts 测试配置下的
npx vitest)。风险与范围
关联 Issue
无。