feat(review): statement-level mutation probes in test-efficacy - #8020
Conversation
The revert probe is all-or-nothing: it reverts whole production files, so a
suite that gates six of a diff's behaviours goes red and the probe says
"gated" — even when the seventh behaviour, a one-line safety statement, has no
test at all. Dogfooded on a live PR: deleting a single `reminders.clear()`
inside the not-continued branch left the full 471-test suite green, and that
line carried the PR's headline safety property (an abandoned task's todos must
not bleed into an unrelated new prompt). A human reviewer found it with a
hand-rolled mutation probe; the command could not.
Add the probe kind the human ran, deterministically. Candidates are added
lines from the committed head (never the dirty worktree) whose trimmed
statement calls a safety verb — .clear(), .delete(), .reset(), .abort(),
.removeListener(), .unref() — or reassigns state to empty ([] / new
Map()/Set()), and that are removable as a whole: single complete expression
statements, brace-balanced via a string/comment-aware scanner, previous
significant line ending ;, { or } (which rejects fluent tails, continuations,
and the brace-less-if silent-rebind trap), outside template literals and block
comments. Selection is conservative by design: a false negative costs
nothing, a false positive burns a suite run.
Each mutant (capped at 8, files with collocated new tests first) deletes its
one line in the existing probe worktree and re-runs the affected suites
through the existing vitest-json classifier: red = killed (the line is
guarded), green = SURVIVED — the invariant it enforces ships unprotected —
filed as a finding in the unreachable/inert register so the Agent-7 pipeline
picks it up without any skill change. Compile/load failure = inconclusive,
never a finding. Mutants run only after a cleanly green baseline, inside the
command's existing deadline budget (each run must leave room for the revert
probe); candidates that no longer fit are counted, not silently dropped.
|
Thanks for the PR! Template: the body uses its own headings ("What", "Why", "How", "Tests") rather than the template's exact sections, but the content covers the same ground — what, why, how, and test evidence are all present. Passing. ✓ Problem: this is an observed gap, not theoretical. The PR describes a concrete dogfood finding: deleting a single Direction: aligned. This extends the existing Size: production logic is ~857 lines (836 in Approach: the scope feels right for what it's solving. The scanner ( Risk: no elevated risk signals — none of the changed files match the high-risk path patterns. Moving on to code review. 🔍 中文说明感谢贡献! 模板:PR body 使用了自己的标题("What"、"Why"、"How"、"Tests"),而非模板的精确章节,但内容覆盖了相同的部分——做了什么、为什么、怎么做、测试证据都有。通过。✓ 问题:这是一个已观测到的缺口,不是理论性的。PR 描述了一个具体的 dogfood 发现:删掉一行 方向:对齐。这在现有 规模:生产逻辑约 857 行( 方案:范围对于所解决的问题感觉合适。扫描器( 风险:无升级风险信号——变更文件均未匹配高风险路径模式。 进入代码审查。🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (the revert probe is all-or-nothing — a file with six tested behaviours and one untested safety statement reports "gated"), I would: parse the diff for added lines in source files, filter for cleanup/reset statements ( Comparison: the PR matches this approach and extends it where correctness demands. The scanner ( Findings: no critical blockers. The code follows project conventions (ESM, strict TypeScript, collocated tests, kebab-case files). Two observations, neither blocking:
The test coverage is thorough: 12+ unit tests for selection rules (each edge case pinned individually), 7 integration tests (survivor, killed, baseline-red skip, per-file gate, budget skip, cap skip, selection failure disclosure, mismatch guard), and agent-prompt brief pins. The integration tests use real git repos with fake vitest runners — the right level of fidelity. TestingFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Ubuntu unit tests passed. macOS/Windows tests and integration tests were skipped (the PR touches only the review command's probe machinery, no platform-specific code). The web-shell E2E smoke test is still running — unrelated to this PR's changes. Real-scenario testing: N/A — the changes are to the review command's internal probe machinery (invoked programmatically by Agent 7 during PR review), not a user-visible TUI surface. 中文说明代码审查独立方案: 给定问题(revert 探针是全有或全无的——一个有六个已测试行为和一个未测试安全语句的文件报告 "gated"),我会:解析 diff 找到源文件中的新增行,过滤清理/重置语句( 对比: PR 匹配了这个方案,并在正确性要求的地方做了扩展。扫描器( 发现: 无关键阻塞项。代码遵循项目约定(ESM、严格 TypeScript、并置测试、kebab-case 文件名)。两个观察,均不阻塞:
测试覆盖全面:12+ 个选择规则单测(每个边界情况单独固定)、7 个集成测试(幸存者、被杀、基线红跳过、逐文件门控、预算跳过、上限跳过、选择失败披露、不匹配守卫),以及 agent-prompt brief 固定。集成测试使用真实 git 仓库和假 vitest 运行器——正确的保真度级别。 测试Ubuntu 单测通过。macOS/Windows 测试和集成测试被跳过(PR 仅触及 review 命令的探针机制,无平台特定代码)。web-shell E2E 冒烟测试仍在运行——与本 PR 变更无关。 真实场景测试:N/A——变更是 review 命令的内部探针机制(由 Agent 7 在 PR 审查期间程序化调用),不是用户可见的 TUI 界面。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review, thorough tests, well-motivated feature; the Stage 0 maintainer-awareness flag (500+ production lines with a core-path touch) caps the score as policy, not doubt. Going back to my independent proposal: the PR matches it and extends it exactly where correctness demands. The scanner is the most complex piece, but every branch exists because a real edge case (nested templates, regex in interpolations, backslash-continued strings) would otherwise produce a false finding or a wasted suite run. Each one is pinned by a regression test. I don't see a materially simpler path that preserves the correctness guarantees. The problem is real and demonstrated — a 471-test suite green after deleting the PR's headline safety statement. The solution is well-scoped: high-precision selection (false negatives are free, false positives burn a run), a hard cap of 8 mutants, a budget that reserves the revert probe's slot, and per-file baseline gating so an unrelated quarantined suite doesn't take the whole phase down. The If I had to maintain this in six months, the comments explain why (not what), the tests pin every edge case individually, and the architecture (select → baseline → mutate → classify → report) is straightforward to follow. I'd thank the author. The only reservation is the policy cap: the PR touches Deferring to a maintainer for the final call. 中文说明置信度:3/5 —— 干净的审查、全面的测试、动机充分的功能;Stage 0 维护者知悉标记(500+ 生产行且有核心路径触及)按政策限制了分数,而非表示怀疑。 回到我的独立方案:PR 匹配了它,并在正确性要求的地方做了精确扩展。扫描器是最复杂的部分,但每个分支的存在都是因为真实的边界情况(嵌套模板、插值中的正则、反斜杠续行字符串)否则会产生错误发现或浪费一次套件运行。每个都有回归测试固定。我没有看到一条在保持正确性保证的同时显著更简的路径。 问题是真实且有演示的——删掉 PR 的核心安全语句后 471 个测试全绿。方案范围合理:高精度选择(漏选零成本,误选烧一次运行)、硬上限 8 个变异、为 revert 探针预留槽位的预算、逐文件基线门控使无关的隔离套件不会拖垮整个阶段。 如果六个月后我要维护这个,注释解释了_为什么_(不是做什么),测试单独固定了每个边界情况,架构(选择 → 基线 → 变异 → 分类 → 报告)易于跟随。我会感谢作者。 唯一的保留是政策限制:PR 触及 转交维护者做最终决定。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
doudouOUC
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.7-max via Qwen Code /review
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. |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action requiredThe only feedback this round is a COMMENTED review (downgraded from Approve) noting that CI checks were failing at the time of review. It contains no code-level findings, no inline comments, and no requested changes. The "Failed checks" and "Still-red checks" sections are both empty, so there are no currently-failing checks to address. Local verification confirms the branch is healthy:
No code changes were made this round. 中文说明无需操作本轮唯一的反馈是一条评论性质的审查(从批准降级为评论),指出审查时 CI 检查处于失败状态。该审查不包含任何代码层面的发现、行内评论或修改请求。"失败检查"和"持续失败检查"两个部分均为空,因此当前没有需要处理的失败检查。 本地验证确认分支状态健康:
本轮未做任何代码变更。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
Mutant selection ran its end-anchored checks on raw trimmed lines, so a
trailing comment hid a statement's real end (dropping genuine candidates
like `reminders.clear(); // why`) and a safety verb inside a string faked
one (a wasted suite run plus a misleading survivor). Route the
`SAFETY_VERB_RE`, `endsWith(';')`, and predecessor `/[;{}]$/` checks
through a shared `codeOnly()` that strips comments and blanks literal
contents first.
Also guard the template-state escape skip against a backslash-continued
line, mirroring the single/double-quote branch: swallowing that newline
dropped a per-line literal flag and shifted every later line's verdict.
Adds unit coverage for all three selection fixes and an integration test
for the baseline-not-green skip branch.
…8020) The fake vitest bin was committed by `git add -A` and checked out into the probe worktree as the stale passing copy, so installFailingVitest's overwrite in the main worktree was never seen by npx in the probe tree. The baseline read green and the baseline-not-green skip test failed.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedAll three inline suggestions from the automated reviewer remain resolved in code (commit 1. Baseline-not-green skip branch has no test (rc:3673554132)Decision: implemented (previous commit), test fix (this commit). The integration test 2. Selection checks run on raw lines — comments/strings fool them (rc:3673554138)Decision: implemented (previous commit, unchanged). The 3. Template-state escape skip lacks a newline guard (rc:3673554143)Decision: implemented (previous commit, unchanged). The template branch in Conflict notes
Verification
中文说明已处理的审查反馈自动审查机器人提出的三条行内建议仍在代码中保持已解决状态(提交 1. 基线非全绿的跳过分支没有测试 (rc:3673554132)决定:已实现(上一次提交),测试修复(本次提交)。 集成测试 2. 选择检查在原始行上运行——注释/字符串会欺骗它 (rc:3673554138)决定:已实现(上一次提交,未更改)。 3. template 状态的转义跳过缺少换行守卫 (rc:3673554143)决定:已实现(上一次提交,未更改)。 冲突说明
验证
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/模型 |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
Local verification — real
|
| Check | Result |
|---|---|
test-efficacy.test.ts + agent-prompt.test.ts |
192 passed |
test-efficacy.integration.test.ts (real git) |
6 passed |
Full src/commands/review/ suite |
1019 passed / 38 files |
eslint --max-warnings 0 (5 changed files) |
clean |
prettier --check (5 changed files) |
clean |
tsc --noEmit (review dir) |
clean |
| Budget path, end-to-end | skippedForBudget: 2 reported on stdout; revert probe still ran |
| Shared worktree after every run | byte-identical; probe tree discarded |
One thing I could not close. I tried a full-scale dogfood on merged PR #7862 (adds this.descriptionCache.clear();). The mutant phase was skipped: my donor node_modules predates that PR and lacks the @qwen-code/channel-gitlab workspace link, so channel-registry.test.ts failed to resolve its import and the baseline wasn't green. That's my environment, not the PR — but note it landed on Finding 2's exact shape on the first real PR I pointed it at.
中文说明
本地验证 —— 真实 git、真实 vitest、隔离 worktree
在 head f488b090 上验证,使用与共享检出隔离的 scratchpad worktree。以下所有结论均来自实际运行,而非阅读代码。
结论:可以合并。 该探针确实实现了 PR 所声称的功能,且理由正确 —— 我不仅复现了驱动本 PR 的缺口,更重要的是证明了探针能够区分"有缺口"与"无缺口"。三条不阻断合并的发现,均属报告精度问题而非正确性缺陷,其中两条是一行修复。
1. 在真实 runner 下复现核心论断
PR 自带的集成测试安装了一个假 vitest,它把每个文件都报告为通过。就其锁定的性质(探针在哪运行、留下什么)而言这是恰当的选择 —— 但这也意味着该套件中的变异只可能"存活"。因此我补上了缺失的对照:相同场景,真实 vitest,两个臂仅有一个变量不同 —— 并置测试的函数体。
同一份源码、同一条语句、同一行 15。空洞测试 → survived → 产生发现;有效测试 → killed → 无发现。探针能区分二者。
值得停下来看的是页脚:两个臂中,既有的 revert 探针都返回了 verdict: "gated" —— "回退源码后有 1 处断言失败 —— 该测试捕获了此变更"。这就是 dogfood 缺口的微缩版、按需复现:在核心安全语句零覆盖的那个臂上,revert 探针给出了虚假的安心感。新探针是唯一能分辨两者的东西。
2. 选择精度 —— 18/18 对抗用例
每个 fixture 的每一行都作为"新增行"提交,因此选择逻辑是唯一的过滤器。18 条全部成立,其中 8 种形态一旦误选后果严重:无花括号的 if/else(删除会静默地重绑定下一条语句)、多行模板字面量内部、块注释内部、字符串内的动词、类字段初始化、const x = []、return map.delete(k)、链式调用尾部。召回用例同样成立 —— 行尾注释不会遮蔽语句结尾,反斜杠续行字符串的行号偏移陷阱也已处理。
3. 以其人之道 —— 变异 14 处防护
10 个被杀死,4 个存活。四个存活者我都追查到底,而非直接上报:
- G6(括号配平)确实是承重的 ——
register(() => { cache.clear();只有在移除它之后才会被选中 —— 但其失效模式是浪费一次套件运行并以inconclusive收场,绝不会产生误报发现。这与模块自述的权衡一致("误选烧一次套件运行")。 - G14 + G11 是真正的一对。 同一文件中两个变异,均有守护。完整实现:
killed, killed。去掉逐变异恢复后,第二个变异的行号已偏移 —— 而错行守护捕获了它:inconclusive —— "probe 树在该行与所选语句不匹配"。恢复失败会诚实降级。这正是第三种结局的纪律在测试未模拟的故障下依然成立。 - G12(
.d.ts)是防御性的,没有可达的触发路径。
一处真实的覆盖缺口: classifyMutantRun → 'killed' 有单测覆盖,但没有任何测试驱动端到端的 killed 路径 —— 删除一行、真实套件变红、不产生发现 —— 因为假 runner 无法失败。我的 ARM B 正是这个测试。值得移植进来:再加一个"当被变异行缺失时就失败"的假 bin 即可,无需真实 runner。
4. 它会真的触发吗?在真实历史上重放
在 main 最近 54 个 merge commit 上运行该命令自身的选择管线(parseAddedLines → selectMutants)—— 真实 diff、真实 head blob。9/54(17%)产生 ≥1 个候选;共 37 个候选。 这不是一个会闲置的探针。
发现
1 · 中 —— 上限静默丢弃候选(test-efficacy.ts:421,报告在 :1146)
selectMutants 在 MAX_MUTANTS = 8 处截断,但只有预算循环会报告自己的跳过数。被上限丢弃的候选无处体现,于是"8 个已探测、"survived": 0"会被读成"该 diff 的安全语句都有覆盖"。在同一份 54 commit 重放中,9 个产生候选的 commit 里有 3 个(33%)触顶:#6930 丢弃 19 中的 11、#7891 丢弃 24 中的 16、#7812 丢弃 16 中的 8。PR 自述的纪律 —— "放不下的候选计入 skippedForBudget,不静默丢弃" —— 恰恰没有应用到上限上。修复:加一个 skippedForCap 计数;selectMutants 本就知道截断前的长度。
2 · 中 —— 一个无关的全 skip 套件会禁用整个变异探针(test-efficacy.ts:1028)
baseline.perFile.some((r) => r.verdict !== 'inert') 让所有变异都取决于每一个 probe 文件都被收集且全绿。一个被隔离、整体 it.skip 的套件会归为 inconclusive —— 不是红、也不是失败 —— 却会把整个探针一起拖下水。单变量 A/B:同一 diff 加上一个无关的 it.skip 文件,probed: 1, survived: 1 变成 probed: 0,幸存者发现随之丢失。它确实通过 mutants.note 披露了,这保持了诚实,但该注记对一个"无任何失败"的套件说的是*"未干净全绿"*。修复:改为逐文件门控(只对基线全绿的 probe 文件跑变异),或至少收窄措辞。
3 · 低 —— "ships unprotected" 过度断言(test-efficacy.ts:1136)
变异只针对该 diff 自身的测试文件运行。消息正确陈述了这个范围 —— "该 diff 中没有测试在它被移除时失败" —— 随后却由此过度推论:"因此它所维护的不变量在无保护状态下发布"。已复现:某 PR 的并置测试是空洞的,但其不变量由一个未被触碰的既有测试覆盖(对照:该测试对变异体是 1 失败 / 0 通过),仍然产生了幸存者发现。Agent 7 会将其作为预确认发现归档,因此这个过度断言会作为"事实"传达给作者。一行修复:止于有范围限定的表述 —— "…请确认已有测试覆盖它,或补一个"。
门禁
| 检查项 | 结果 |
|---|---|
test-efficacy.test.ts + agent-prompt.test.ts |
192 通过 |
test-efficacy.integration.test.ts(真实 git) |
6 通过 |
src/commands/review/ 全量套件 |
1019 通过 / 38 文件 |
eslint --max-warnings 0(5 个变更文件) |
干净 |
prettier --check(5 个变更文件) |
干净 |
tsc --noEmit(review 目录) |
干净 |
| 预算路径端到端 | stdout 报告 skippedForBudget: 2;revert 探针仍然运行 |
| 每次运行后的共享 worktree | 字节级一致;probe 树已清理 |
一件我没能闭环的事。 我尝试在已合并的 PR #7862(新增 this.descriptionCache.clear();)上做全量 dogfood。变异阶段被跳过:我的 donor node_modules 早于该 PR,缺少 @qwen-code/channel-gitlab 这个 workspace 链接,导致 channel-registry.test.ts 无法解析导入、基线不绿。这是我的环境问题,不是 PR 的问题 —— 但值得注意的是,我指向的第一个真实 PR 就正好落在了发现 2 的形态上。
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
Address review feedback on the test-efficacy mutation probe:
- Count candidates the MAX_MUTANTS cap drops in `skippedForCap` instead of
silently losing them, so a capped `survived: 0` cannot read as "every
safety statement is covered" (mirrors the existing `skippedForBudget`).
- Gate the mutant phase per probe file: run each mutant against the files
that are green in the unmutated baseline, so one unrelated quarantined
(all-skip) suite — `inconclusive`, not red — no longer disables the whole
probe.
- Scope the `mutant-survived` finding to the diff's own tests ("confirm an
existing test covers it, or add one") rather than asserting the invariant
ships unprotected, which an untouched pre-existing test may still cover.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8020Addressed all three numbered findings from @wenshao's review. All are report-precision fixes to the test-efficacy mutation probe; none change what the probe runs, only how it counts and describes its results. One commit: Feedback triageFinding 1 · MEDIUM — the cap silently drops candidates → addressed. Finding 2 · MEDIUM — one unrelated all-skipped suite disables the whole probe → addressed (per-file gate, the finding's primary recommendation). Finding 3 · LOW — "ships unprotected" over-claims → addressed. Coverage-gap suggestion (section 3) — an end-to-end "killed" test → deferred, recorded here so it is not dropped. Failed check — Conflict notes
Verification
中文说明Autofix 审查轮次 — PR #8020已处理 @wenshao 审查中的全部三条编号发现。三者均为 test-efficacy 变异探针的报告精度修复;都不改变探针运行什么,只改变它如何计数与描述结果。单次提交: 反馈分诊发现 1 · 中 —— 上限静默丢弃候选 → 已处理。 发现 2 · 中 —— 一个无关的全 skip 套件禁用整个探针 → 已处理(采用该发现的首选方案:逐文件门控)。 发现 3 · 低 —— "ships unprotected" 过度断言 → 已处理。 覆盖缺口建议(第 3 节)—— 端到端 "killed" 测试 → 延期,记录于此以免遗漏。 失败检查 —— 冲突说明
验证
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/模型 |
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed — PR #8020Feedback points[rc:3676497069] Budget-skip path lacks integration test — ImplementedFinding: The budget-skip path ( Decision: Implemented. The coverage gap is real and the fix is small. Change: Added one integration test (
Files changed: Verification
中文说明审查反馈已处理 — PR #8020反馈要点[rc:3676497069] 预算跳过路径缺少集成测试 — 已实现发现: 预算跳过路径( 决定: 已实现。覆盖缺口真实存在,且修复量很小。 变更: 新增一个集成测试( 使用 mock 变更文件: 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedAll six suggestions from the automated reviewer are implemented. rc:3680844013 — Cap-skip disclosure integration testDecision: Implement. Added an integration test mirroring the budget-skip test: scaffolds a file with 9 added safety statements (MAX_MUTANTS is 8), runs the handler end-to-end, and asserts rc:3680844016 — Numeric accounting for baseline-skipped candidatesDecision: Implement. Added a rc:3680844023 — Set/WeakMap/WeakSet coverage in safety-verb fixtureDecision: Implement. Added a separate rc:3680844024 — Template interpolation string/nested-template trackingDecision: Implement. Added string skipping and nested-template state tracking (
Both triggers have dedicated unit tests. The existing nested-template tests continue to pass. rc:3680844026 — insideClassBody backward walk vs balanced bracesDecision: Implement. Changed the backward walk to break only on lines with a semicolon or net-unbalanced braces (via rc:3680844030 — Multi-statement line rejectionDecision: Implement. Added a one-line guard in Verification
中文说明已处理的审查反馈自动审查者的六条建议全部实现。 rc:3680844013 — cap 跳过披露集成测试决定: 实现。 新增仿照 budget 跳过测试的集成测试:构造含 9 条新增安全语句的文件(MAX_MUTANTS 为 8),端到端运行 handler,断言 rc:3680844016 — 基线跳过候选的数值核算决定: 实现。 新增 rc:3680844023 — 安全动词 fixture 中 Set/WeakMap/WeakSet 覆盖决定: 实现。 新增独立的 rc:3680844024 — 模板插值内字符串/嵌套模板跟踪决定: 实现。 在
两个触发均有专门单测。现有嵌套模板测试继续通过。 rc:3680844026 — insideClassBody 反向遍历 vs 平衡括号决定: 实现。 将反向遍历的中断条件从"含 rc:3680844030 — 多语句行拒绝决定: 实现。 在 验证
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/模型 |
Review: statement-level mutation probes in
|
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedAll six findings from @wenshao's review triaged and addressed. Finding 1 — Agent-7 brief discloses two of four zero-result paths (Suggestion) ✅ ImplementedThe brief now names all three skip counters ( Finding 2 — Diff parsing exposed to the invoking user's git config (Suggestion) ✅ ImplementedThe Finding 3 — Regex literal disables selection for the rest of the file (Nit) ✅ Implemented (cheap guard)
Finding 4 — Budget test asserts on Date.now() call count (Nit) ⏭️ Declined (follow-up)Threading Finding 5 —
|
Local verification at
|
| revision | baseline killed at | revert killed at | total wallclock |
|---|---|---|---|
e0e7fb9ad (no clamp) |
t = 301 s | t = 600 s | 600.39 s — at Agent 7's ceiling |
8fa3b65b7 (clamped) |
t = 241 s | t = 540 s | 540.50 s — 60 s of headroom |
241 s = startedAt + 540 − 300, i.e. the revert probe's whole slot reserved up front — measured from the sleep child's own age at the moment it was replaced, not derived from the source. The clamped run still wrote a report: both mutants inconclusive with spawnSync npx ETIMEDOUT, findings: []. An unresponsive runner degrades to a non-verdict, never to a finding. runProbeSuite and the deadline arithmetic are byte-identical between 809f342ab and 8fa3b65b7, so this measurement carries to the current head.
4. Isolation and determinism
| invariant | result |
|---|---|
| shared review worktree after a full run | byte-identical (sha256 over every tracked file) |
probe worktree …-probe |
discarded; git worktree list clean |
| main fixture tree | clean, no stray refs |
| two identical runs | byte-identical report JSON |
5. Static gates at 8fa3b65b7
vitest over the three touched suites: 217 passed. eslint --max-warnings 0 on the five changed files: clean. tsc --noEmit on packages/cli: no errors attributable to the changed files.
Finding — MEDIUM, not a blocker: the newest commit drops real candidates
8fa3b65b7 correctly fixes the nested-template false positive I hit at 809f342ab (my reproducer now yields 0 mutants, 0 findings — confirmed by re-running it). But the same commit also added a quote-skip inside interpolations, and a regex literal is not a string.
// packages/cli/src/utils/standalone-update.ts:584
return `'${p.replace(/'/g, "'\\''")}'`;The scanner enters the interpolation, reads the first / of /'/g as ordinary code, then sees ' and skips forward to the next ' — which lives inside the "'\\''" string. From there its brace depth is wrong and it never leaves template.
I ran HEAD's selector and the same selector minus the two quote-skip branches over every backtick-bearing source file in this repo (2566 of 4352):
| file | lines wrongly flagged as template text | candidate dropped |
|---|---|---|
packages/cli/src/utils/standalone-update.ts |
585–734 (150 lines) | 696: child.unref(); |
packages/core/src/utils/secure-browser-launcher.ts |
144–340 (197 lines) | 244: child.unref(); |
Two genuine safety statements in this repo's own source become invisible to the probe, and ~350 lines of two files go with them. Nothing is reported — the run just returns fewer candidates. (I hit the identical bug drafting my own fix; the 2566-file scan is what caught it.)
The commit's own test does not gate the branch that causes this. Running the three scanner tests against three different sources:
| source under test | quoted-} test |
nested-} test |
single-line test |
|---|---|---|---|
809f342ab (before both new branches) |
PASS | FAIL | PASS |
8fa3b65b7 (HEAD) |
PASS | PASS | PASS |
8fa3b65b7 minus the quote-skip branches |
PASS | PASS | PASS |
does not let a } in nested-template text close the outer interpolation genuinely gates the nestedDepth change — it fails without it. does not let a quoted } inside an interpolation close the template passes on all three: it gates nothing. Deleting the branch it was written for is a surviving mutant of this PR's own diff — which is, pleasingly, precisely the class of gap this PR exists to surface.
Verified correction: delete the two else if (ch === '"' || ch === "'") skip branches in scanFileLines's template state, keep nestedDepth. Results: the two dropped candidates are restored (0 disagreements with the pre-regression scanner across 2566 files), the nested-template false positive is still rejected, all three template controls plus the regex control behave correctly, and test-efficacy.test.ts + test-efficacy.integration.test.ts are 71 passed. Handling }-in-a-string inside an interpolation correctly needs regex-literal awareness; not skipping quotes at all is what the corpus says is safe today.
Not a merge blocker — versus main this is still strictly new capability, and a false negative costs less than a false positive by this PR's own design rule. But it is a regression the final commit introduced, it is untested in either direction, and the fix is a deletion.
Reproduction harness
- Isolated
git worktreeof the PR head,node_moduleshardlinked from the main checkout (same filesystem —cp -alfails cross-device). - Fixture repo:
packages/*npm workspaces, own git history, base commit + "PR" commit, real vitest 3.2.4. The review worktree is created inside the fixture at.qwen/tmp/review-wtso the probe tree (…-probe, a sibling) still resolvesnode_modulesby walking up — matching how the command is invoked for real. - The command is driven through its real yargs
CommandModule, not by calling internals. - Budget harness:
PATH-shadowednpxthatexec sleep 100000. Note it must not shadow thenpxthat launches the driver itself — invoke the driver vianode_modules/.bin/tsx. - Counterfactuals swap only
packages/cli/src/commands/review/test-efficacy.tsbetween runs; the fixture, base, and suite are untouched.
中文说明
在 8fa3b65b7 上的本地验证 —— 真实命令、真实 git、真实 vitest
这是我第三次验证本 PR(此前为 f488b0901 与 e0e7fb9ad)。以下全部结论来自在与共享检出隔离的 scratchpad worktree 中实际运行代码,而非阅读代码。验证期间 head 变动了两次;除非表格另有说明,所有数字均已在 8fa3b65b7 上重新测量。
结论:可以合并,但建议先落一个修复。 探针确实做到了 PR 所声称的事,而且是出于正确的原因 —— 我既复现了动机缺口,也证明了探针能区分缺口存在与不存在。自我上次评审以来的三个提交各自都是承重的,每一个我都给出了反事实证据。一处发现:最新提交 8fa3b65b7 修复了我在 809f342ab 上发现的误报,但同时引入了一个针对本仓库自身源码的静默漏报。修正方式是删除两个分支,我已验证。
1. 核心主张,在真实运行下复现
一个带 npm workspaces 的 fixture 仓库,有真实的 base 提交与真实的"PR"提交,以及真实的 vitest 套件。新增五行,其中三行是给选择器设的陷阱;一行安全语句被覆盖,一行未被覆盖。
revert 探针对一个包含零覆盖行的文件返回 gated("这些测试能抓住此变更")。变异探针对第 7 行返回 killed、第 28 行返回 survived,恰好产出一条发现。这正是 PR 描述中的"全有或全无"盲区 —— 是端到端观测到的,而不是论证出来的。
在 merge-base(00fe2034)上,同一 fixture 产出 0 条发现,且完全没有 mutants 区。这是新增能力,不是改名。
2. 我上次评审之后的提交是否承重?
同一 fixture、同一 base、同一套件;两次运行之间只替换 test-efficacy.ts。
e0e7fb9ad(我上轮认可的版本)漏掉两个真候选、并报出一个假候选:它从不选中第 7 行(类中首个方法的首条语句),从不选中第 28 行(其扫描器在两行前的嵌套反引号处失步),却把第 18 行 —— 一个类字段 —— 报告为幸存的"安全语句"。逐个移除构造即可定位成因:去掉嵌套反引号,第 28 行就回来了;去掉括号不平衡的注释,模板文本反而被选中。d117b0485 与 d5caee227 各修复了一处独立且可复现的错选。
3. 预算钳制,在真实常量下测量
不缩放、不 mock:在 PATH 上放一个永不响应的假 npx,使命令启动的每次 vitest 运行都挂起。其余全部是真实命令。
| 版本 | baseline 被杀时刻 | revert 被杀时刻 | 总墙钟时间 |
|---|---|---|---|
e0e7fb9ad(无钳制) |
t = 301 s | t = 600 s | 600.39 s —— 正好顶到 Agent 7 的上限 |
8fa3b65b7(已钳制) |
t = 241 s | t = 540 s | 540.50 s —— 留出 60 s 余量 |
241 s = startedAt + 540 − 300,即 revert 探针的整个时段被预先预留 —— 该数字取自 sleep 子进程被替换那一刻自身的存活时长,是测量值而非从源码推导。被钳制的那次运行仍然写出了报告:两个变异均为 inconclusive,detail 为 spawnSync npx ETIMEDOUT,findings: []。运行器无响应时降级为非裁决,绝不降级为发现。runProbeSuite 与 deadline 算术在 809f342ab 与 8fa3b65b7 之间逐字节一致,因此该测量对当前 head 同样成立。
4. 隔离性与确定性
| 不变量 | 结果 |
|---|---|
| 完整运行后的共享评审 worktree | 逐字节一致(对每个受版本控制文件做 sha256) |
probe worktree …-probe |
已清理;git worktree list 干净 |
| fixture 主树 | 干净,无残留引用 |
| 两次相同运行 | 报告 JSON 逐字节一致 |
5. 8fa3b65b7 上的静态门禁
三个受影响套件的 vitest:217 通过。五个变更文件的 eslint --max-warnings 0:干净。packages/cli 的 tsc --noEmit:无可归因于变更文件的错误。
发现 —— MEDIUM,非阻塞:最新提交丢弃了真实候选
8fa3b65b7 正确修复了我在 809f342ab 上命中的嵌套模板误报(重跑我的复现用例现在得到 0 变异、0 发现 —— 已确认)。但同一提交还在插值内部加入了引号跳过,而正则字面量并不是字符串:
// packages/cli/src/utils/standalone-update.ts:584
return `'${p.replace(/'/g, "'\\''")}'`;扫描器进入插值后,把 /'/g 的第一个 / 当作普通代码,随后看到 ' 便向前跳到下一个 ' —— 而它位于 "'\\''" 这个字符串内部。此后其括号深度即告错误,并且再也没能离开 template 状态。
我用 HEAD 的选择器与"去掉这两个引号跳过分支"的同一选择器,跑遍了本仓库每一个含反引号的源文件(4352 个中的 2566 个):
| 文件 | 被错误标记为模板文本的行 | 被丢弃的候选 |
|---|---|---|
packages/cli/src/utils/standalone-update.ts |
585–734(150 行) | 696: child.unref(); |
packages/core/src/utils/secure-browser-launcher.ts |
144–340(197 行) | 244: child.unref(); |
本仓库源码中两条真实的安全语句对探针变为不可见,连带约 350 行代码一并失明。且无任何提示 —— 运行只是返回更少的候选。(我在起草自己的修复时命中了完全相同的 bug;正是这次 2566 文件扫描把它抓了出来。)
该提交自己的测试并不守护导致此问题的分支。 把三个扫描器测试分别跑在三份不同源码上:
| 被测源码 | quoted-} 用例 |
nested-} 用例 |
单行用例 |
|---|---|---|---|
809f342ab(两个新分支之前) |
PASS | FAIL | PASS |
8fa3b65b7(HEAD) |
PASS | PASS | PASS |
8fa3b65b7 去掉引号跳过分支 |
PASS | PASS | PASS |
does not let a } in nested-template text close the outer interpolation 确实守护了 nestedDepth 改动 —— 缺了它就会失败。而 does not let a quoted } inside an interpolation close the template 在三份源码上全部通过:它什么也没守护。删掉它所对应的那个分支,正是本 PR 自身 diff 上的一个幸存变异 —— 而这恰好就是本 PR 存在的意义所在。
已验证的修正: 删除 scanFileLines 模板状态中的两个 else if (ch === '"' || ch === "'") 跳过分支,保留 nestedDepth。结果:两个被丢弃的候选恢复(在 2566 个文件上与回归前扫描器 0 分歧),嵌套模板误报仍被拒绝,三个模板对照用例加正则对照用例行为均正确,test-efficacy.test.ts + test-efficacy.integration.test.ts 71 通过。要正确处理插值内字符串中的 } 需要正则字面量感知;就目前语料而言,完全不跳过引号才是安全的做法。
这不是合并阻塞项 —— 相对 main,本 PR 仍然是纯粹的新增能力,而且按本 PR 自己的设计准则,漏报的代价低于误报。但它是最后一个提交引入的回归,两个方向都没有测试覆盖,而修复只是一次删除。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8020Addressed the one finding from @wenshao's local-verification review (a MEDIUM, Feedback point → decisionFinding (MEDIUM, correctness): the newest commit's quote-skip inside Decision: Required — verified, then fixed minimally (a deletion).
Net change: 18 insertions, 32 deletions across two files — a pure deletion in Conflict notesNone — VerificationCommands actually run this round (working dir
Committed as 中文说明Autofix 评审轮次 — PR #8020处理了 @wenshao 本地验证评审中的唯一发现(一个由最新提交引入的 MEDIUM、非阻塞回归)。本轮没有行内评审评论;无 base 冲突( 反馈点 → 决定发现(MEDIUM,正确性):最新提交在插值内部的引号跳过会静默丢弃真实候选。 决定:Required —— 先验证,再以最小改动(一次删除)修复。
净变更:跨两个文件 18 行新增、32 行删除——扫描器中的一次纯删除,外加一段准确的注释和一个更强的测试。没有新增防御、配置或叙述。 冲突说明无 —— 验证本轮实际运行的命令(vitest 的工作目录为
已提交为 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/模型 |
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — conservative mutation probe design where every error direction is rejection, never false findings. Budget arithmetic is explicit and disclosed. Three-outcome discipline (killed/survived/inconclusive) correctly maintained throughout. Worktree isolation is sound. Test coverage at 1.5:1 ratio with real-git integration tests that read mutated source.
Non-blocking suggestions:
- Run
scanLineDelimitersoncodeLines[idx]instead of raw line (unterminated block comment causes false rejection) - Document the 2-level nested template tracking limit in
scanFileLinesdoc comment - Cross-reference or extract the two independent template-scanning implementations (
skipTemplateOnLine/scanFileLines) - Add
--before probe file paths inspawnSyncargs
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. |
…iles; harden and pin the remaining probe paths The nested-template fix that landed as a counter cannot represent a nested template INSIDE a nested interpolation: at two levels the deep template's text `}` is charged against the wrong frame, the scan desyncs, and the file either admits template text as a mutant or derail-drops its real candidates. Replace the counter with a stack — one frame per open template, `}` closes only the top interpolation, a backtick closes only the innermost template — and derive the end state from the stack. The two-level trigger is pinned by a test written red-first against the counter. Derailed files are now disclosed, not silently dropped: selectMutants returns them, and the note composer stacks the derail note with the red-baseline note instead of clobbering. The hostile-git-config path gets its missing test (repo diff.srcPrefix/dstPrefix, diff.external, core.quotePath with a non-ASCII path — fails with the pinned flags removed). The budget test drops its Date.now call-count coupling for an injected clock threaded through runTestEfficacy/runProbeSuite. The mutation-phase catch gets an end-to-end test (ENOBUFS mid-phase → all candidates inconclusive, revert probe still runs, report still written).
yiliang114
left a comment
There was a problem hiding this comment.
Re-review after 93e5dd2: the stack-based template scanner fully resolves the 2-level nesting limit (P2-2 fixed). End-anchored checks now use code-stripped lines (P2-1 downgraded to P3, false-rejection only). New tests cover hostile git config, budget exhaustion, cap enforcement, scan derailment, and mismatch guard. All remaining items are P3 suggestions. LGTM.
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 93e5dd2 (first pass done at a7b07df; the head moved mid-review, so this covers the stack-scanner commit too). The selection scanner survives every trap I fed it against the actual code: safety verbs inside template literals and block comments are excluded via the inLiteral/codeLines split; the brace-less-if silent-rebind trap is rejected from both sides (single-line via NON_STATEMENT_START_RE, two-line via the previous-significant-line [;{}]$ check); fluent tails, argument continuations, and multi-statement lines are refused; and a derailed scan (regex holding a backtick) now drops the file's candidates AND discloses it in mutants.derailed instead of silently returning nothing. I read the new stack-based scanFileLines in full: one frame per open template with -1 for text vs brace-depth for an open interpolation, } charges only the top frame, a backtick closes only the innermost template, and endState derives from the stack — the two-level nesting case the old counter desynced on is structurally impossible now, and the trigger test was written red-first against the counter. Execution isolation holds: runOneMutant restores the original content in a finally (no compounding mutants) and guards against line drift by comparing statement text before deleting; the integration test pins the shared worktree byte-identical (porcelain + HEAD SHA + literal content). Budget math is consistent in ms: 540s total, mutants reserve the full 300s revert-probe slot, per-run pricing = measured baseline + 15s margin via the injected clock (no more Date.now call-count coupling), and every skip path is disclosed (skippedForBudget/skippedForCap/skippedForBaseline, notes now stack instead of clobbering). Verdicts cannot inflate efficacy: unparseable JSON, zero collected assertions, and all-skipped files collapse to inconclusive; killed requires a failed assertion; the mid-phase crash path (ENOBUFS) marks remaining candidates inconclusive and still runs the revert probe — covered end-to-end. mutant-survived reuses the findings shape and the Agent-7 brief names the kind plus all mutants counters (test-pinned, all read sites grepped). Selection reads blobs from git show HEAD:path, never the worktree; hostile git config (diff.external, srcPrefix, quotePath) is neutralised with pinned flags and now has its own test. House style clean: no any, no cross-package relative imports, kebab-case, collocated tests. Ran all three touched suites locally at this HEAD: 224/224 passed. CI green. No findings.
|
Released in v0.21.2. |







Part of #7981 — a new probe kind motivated by a live dogfood gap the existing probes cannot close.
What
qwen review test-efficacygains statement-level mutation probes: deterministic single-line deletion mutants over the diff's added safety statements, run through the existing probe-worktree + vitest-json machinery. A mutant whose suite stays green is a finding: the invariant that line enforces ships unprotected.Why
The revert probe is all-or-nothing: it reverts whole production files, so a suite that gates six of a diff's behaviours goes red and the probe says "gated" — even when the seventh behaviour, a one-line safety statement, has no test at all. Dogfooded on a live PR: deleting a single
reminders.clear()left the full 471-test suite green, and that line carried the PR's headline safety property (an abandoned task's todos must not bleed into an unrelated new prompt). A human reviewer found it with a hand-rolled mutation probe; the command could not.How
.clear(),.delete(,.reset(,.abort(,.removeListener(,.unref(— or reassigns state to empty (= [],= new Map()/Set()), and that are removable as a whole: complete single expression statements, brace-balanced via a string/comment-aware scanner, previous significant line ending;/{/}(rejects fluent tails, continuations, and the brace-less-ifsilent-rebind trap), outside template literals and block comments. A false negative costs nothing; a false positive burns a suite run.classifyProbeRun: any red = killed (guarded); green = survived (finding); compile/load failure = inconclusive, never a finding. Mutants run only after a cleanly green baseline, inside the command's existing deadline budget — each run must leave room for the revert probe; candidates that no longer fit are counted inskippedForBudget, not silently dropped.mutantssection (probed[]with per-mutant verdicts, counts, budget note). Survivors also land infindings[]askind: "mutant-survived"in the unreachable/inert register, so the existing Agent-7 pipeline files them with no skill change; the Agent-7 brief names the new kind and the 600s tool timeout the command budgets under.Tests
12 new unit tests (selection rules incl. the continuation-line scanner regression the self-audit caught — verified failing against the pre-fix scanner), one real-git integration test (PR adds
state.clear()+ a vacuous test → survivor reported, shared tree byte-identical, probe tree discarded), agent-prompt brief pins. 194/194 green across the touched suites; eslint--max-warnings 0and tsc clean for changed files.中文说明
关联 #7981——由一次真实 dogfood 缺口驱动的新探针类型,现有探针无法覆盖。
做了什么
qwen review test-efficacy新增语句级变异探针:对 diff 新增的安全语句做确定性的单行删除变异,复用现有 probe worktree + vitest-json 机制运行。套件保持绿色的变异即为发现:该行所维护的不变量在无保护状态下发布。为什么
revert 探针是全有或全无的:它回退整个生产文件,只要套件守护了 diff 六个行为中的任意一个就会变红、探针便报 "gated"——即使第七个行为(一行安全语句)完全无测试。真实 PR 实测:删掉一行
reminders.clear(),471 个测试依然全绿,而这行承载着该 PR 的核心安全性质(被放弃任务的 todo 不得泄入无关的新 prompt)。人工评审者用手工变异探针找到了它;命令找不到。怎么做
.clear()/.delete(/.reset(/.abort(/.removeListener(/.unref()或将状态重置为空(= []、= new Map()/Set()),且可整体删除:完整的单表达式语句、经字符串/注释感知扫描器验证括号自平衡、上一个有效行以;/{/}结尾(排除链式尾部、续行、无花括号if的静默重绑陷阱)、不在模板字面量或块注释内。漏选零成本;误选烧一次套件运行。classifyProbeRun重跑受影响套件:变红 = killed(有守护);保持绿 = survived(发现);编译/加载失败 = inconclusive,绝不算发现。变异仅在基线干净全绿后运行,受命令现有 deadline 预算约束——每次运行须为 revert 探针留出余量;放不下的候选计入skippedForBudget,不静默丢弃。mutants区(逐变异裁决、计数、预算注记)。幸存者同时以kind: "mutant-survived"进入findings[],沿用 unreachable/inert 的措辞口径,现有 Agent-7 管道无需 skill 改动即可归档;Agent-7 brief 注明新类型与命令自我预算所依赖的 600s 工具超时。测试
12 个新单测(选择规则,含自审计捉到的续行扫描器回归——已验证对修复前扫描器失败)、1 个真实 git 集成测试(PR 添加
state.clear()+ 一个空洞测试 → 幸存者被报告、共享树字节级不变、probe 树被清理)、agent-prompt brief 固定。涉及套件 194/194 全绿;变更文件 eslint--max-warnings 0与 tsc 干净。