feat(review): add Maven multi-module verification - #8777
Conversation
`review build-test` combined three responsibilities in one module: reading the review plan, deciding which toolchain can be verified deterministically, and implementing npm workspace installation, affected-package selection, dependency widening, build execution, test execution, and reporting. Move the npm implementation behind an internal `ReviewToolchainAdapter` contract. `build-test.ts` keeps CLI routing, plan reading, output trimming, env shaping, and the spawn boundary; `lib/npm-toolchain.ts` owns npm detection and the verification algorithm; `lib/disk.ts` holds the shared free-disk floors. This is a move, not a rewrite: 95% of the lines removed from build-test.ts reappear verbatim in the new files. The CLI arguments, the BuildTestReport JSON shape, and every npm behaviour are unchanged, and the existing build-test suite is the compatibility oracle for that. Selection requires exactly one applicable adapter and fails closed to the `unsupported` handoff otherwise, so a second toolchain lands as a registration rather than another branch in this file.
|
Thanks for the PR! Template looks good ✓ — all sections present ( Problem: real and observed. Direction: aligned. #8776 establishes the toolchain adapter boundary; this registers the Maven adapter on it — the natural next step. The design note ("Maven owns the reactor" — read Size: no core-module paths touched — all changes live in Approach: the scope matches the stated goal — ownership walk, wrapper preference, one scoped Risk: no elevated risk signals — nothing matches the revert-correlated paths; all changes are inside the review subsystem. Note this PR stacks on #8776 (still open), so merge order matters. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各节齐全（ 问题： 真实且已被观测到。 方向： 对齐。#8776 建立了 toolchain adapter 边界，本 PR 在其上注册 Maven adapter —— 自然的下一步。设计说明（"reactor 由 Maven 说了算" —— 读回 规模： 未触及核心模块路径 —— 全部改动在 方案： 范围与目标相称 —— 归属向上查找、wrapper 优先、从 reactor 根执行一次带 风险： 无升级风险信号 —— 未命中 revert 相关路径；全部改动在 review 子系统内。注意本 PR 堆叠在 #8776（仍 open）之上，合并顺序有依赖。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI wrote my independent proposal before reading the diff: root detection with fail-closed ambiguity, nearest-ancestor-POM ownership that skips fixture trees, wrapper preference, one scoped What I verified closely:
No blockers found. Two observations, neither blocking:
Files changed (16 of 16 shown)
Testing evidenceThis is an unattended CI run, so PR code is neither built nor executed here; the evidence section carries the PR's own CI signal — and for this commit, there is none. The PR description reports Not verified: all runtime behaviour (command construction, classification, evidence collection, test-plan settlement). The suite is static-readable and the 160 new tests name the right behaviours, but nothing has executed them on this commit.
Sandboxed verification would settle this: 中文说明代码审查读 diff 前我先写下自己的独立方案：根目录识别 + 歧义 fail-closed、跳过 fixture 树的最近祖先 POM 归属、wrapper 优先、从 reactor 根执行一次 scoped 重点核对过：
未发现阻塞项。两点非阻塞观察：
测试证据无人值守 CI 运行不会构建或执行 PR 代码。本 commit 上没有任何 CI 证据： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean static review held back by an evidence gap, not by the code: nothing has executed this commit because CI doesn't fire on the feature-branch base. Stepping back: this is what the split of #8394 was for. #8776 establishes the adapter boundary, and this PR is a fully-formed first citizen on it — ownership, scoping, evidence, classification, and every downstream consumer updated together, with a design doc recording the one decision that matters most (Maven owns the reactor; the adapter reads Maven's answer instead of re-modeling it). My independent proposal for this problem was materially shallower — no fail-never handling, no evidence caps, no The reason I'm not approving is procedural, not technical. This commit has zero CI: ⏸️ Deferring to @wenshao — not for a judgement call on the code, but to land the evidence. Two clean paths:
Either way this should convert to an approval quickly; the review itself found nothing to fix. 中文说明信心：3/5 —— 静态审查干净，卡住审批的是证据缺口，不是代码：CI 不对 feature 分支 base 触发，本 commit 没有任何执行记录。 退一步看：这正是 #8394 拆分的意义。#8776 建立 adapter 边界，本 PR 是其上第一个完整公民 —— 归属、scope、证据、分类与所有下游消费方一起更新，设计文档记录了最关键的决定（reactor 由 Maven 说了算，adapter 读回 Maven 的答案而不是重新建模）。我对这个问题的独立方案明显更浅 —— 没有 fail-never 处理、没有证据上限、没有声明结算中的 不批准的原因是程序性的，不是技术性的。本 commit 的 CI 为零： ⏸️ 转交 @wenshao —— 不需要对代码做判断，只需补上证据。两条路：
无论走哪条路都应很快转为批准；审查本身没有发现需要修改的问题。 — Qwen Code · qwen3.8-max Reviewed at |
`review build-test` produced deterministic evidence for npm projects but handed Maven projects back to an agent for ad hoc command selection. That made Java verification inconsistent and provided no reliable module ownership, timeout classification, or fresh test-report evidence. Register a Maven adapter on the toolchain boundary. It maps changed files to the nearest ancestor Maven project, prefers the root Maven Wrapper, runs one scoped `test` or build-only `test-compile` from the reactor root with `-am` upstream expansion, and emits module-qualified Surefire/Failsafe evidence limited to reports this invocation created or updated. Maven, not this adapter, is the authority on the reactor. Ownership is a nearest-ancestor `pom.xml` walk; whether that project is active under the current profiles, JDK, and `<modules>` inheritance is answered by Maven rejecting the `-pl` selector, before anything is compiled. A changed POM runs the reactor rather than a guessed inheritance closure. Driven by alibaba/fastjson2 and alibaba/druid, where nested, profile-activated, and standalone POMs make a text-level reactor model unsafe. The same principle applies to the command line: a recorded Maven run carries its lifecycle, module set, and `-am` flag as `CommandResult.maven`, straight from the values the adapter rendered the command from. `test-plan` settles a Test Plan claim against those facts and parses only the claim, which is the one side that is genuinely free text. Downstream: base-tree accepts successful Maven build-only results, test-plan settles Maven command and Surefire count claims, Agent 7 gains a Maven branch, and Maven failures do not enter the npm-only test-delta rerun path. Fails closed for mixed npm+Maven roots and for module directories a `-pl` selector cannot express. Does not claim Maven effective-model or CI-matrix parity, and does not implement Gradle, JaCoCo, or Maven-aware base-side test-delta.
74d65ab to
95e5ba6
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Test Plan (not a blocker): ./mvnw — no such file or directory.
中文说明
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
Test Plan（非阻断）：./mvnw — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.8)
# Conflicts: # docs/design/review-toolchain-adapters.md # packages/cli/src/commands/review/build-test.test.ts # packages/cli/src/commands/review/build-test.ts # packages/cli/src/commands/review/lib/disk.ts # packages/cli/src/commands/review/lib/npm-toolchain.ts # packages/cli/src/commands/review/lib/toolchain.ts
wenshao
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
wenshao
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
|
🤖 AutoFix crashed before it could evaluate the feedback (attempt 1/100) — it will retry on the next scan. AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why). Run log: https://github.com/QwenLM/qwen-code/actions/runs/31464549314 🧠 Handled by Qwen Code · model/模型 |
Address all 31 findings from the qwen3.8-max review: - Ownership: a src/-nested POM that would collapse to the root project now fails closed to reactor-wide (a real <module>src/core</module> would otherwise go untested under a green -pl . verdict) - Selector gate: reject leading '-' (commons-cli re-reads the value as an option) and '!' (Maven exclusion) module dirs, widening to the full reactor like the existing ,/:/% rejections - Verdict: a skip-tests marker with zero reports is a swallowed failure, not a pass; a wrapper distribution-download failure is classified infrastructure; classification runs on SGR-stripped output so colored Maven logs cannot launder failures - Evidence: sweep truncation (scan cap, fan-out bound) and parse rejections (oversized/unreadable/zero-suite reports) count as unknown evidence and fail closed like the fresh-report cap; the sweep streams entries instead of materializing unbounded Dirent arrays, and never follows a symlinked report dir - Propagation: CommandResult.evidenceCapped records the refused-to- certify outcome; test-plan excludes it from finished(), observedTestCounts, and ranFailed like infrastructure - Claim side: failureInsideClaim also mines surviving failing [maven-test-report] lines (case-cap truncation can erase every per-case line of the claimed module); attached -pl=/--projects= quoted selectors are consumed like the space form; claim -pl modules normalize a leading ./; coordinate selectors stay unsettleable instead of cross-matching module dir names; deploy/ site/plugin-goal work in a claim forces the phase-reduction disclosure; a header-only failing report emits a fallback [maven-test-failure] line; clean rollup lines carry the per-report clamped passed totals the parser expects - Agent 7 brief no longer forbids the sanctioned unsupported fallback it instructs five lines later; design doc and PR description corrected where they drifted from the code
wenshao
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
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. |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no code changes (failed checks diagnosed as a self-hosted runner infrastructure outage)This round ran in Critical-only mode: no actionable review comments, inline comments, or issue-level comments were present. The only actionable items were the two failed checks — Root cause: the
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm:
- [Critical] maven-toolchain.ts — 32 round-2..6 blocker threads whose bodies were truncated in the context file and could not be individually re-traced this round: 3757694132, 3757694138, 3757694146, 3757694153, 3757694160, 3757694166, 3757694172, 3757694179, 3757694183, 3757694192, 3757694201, 3757694207, 3757694215, 3757694226, 3760539291, 3760539296, 3760539303, 3760539343, 3760539347, 3760539382, 3762610237, 3762610281, 3762610295, 3764177565, 3764177620, 3764177622, 3764177682, 3767483207, 3767483213, 3767483217, 3767483225, 3767483248
- [Critical] test-plan.ts — 22 round-2..6 blocker threads whose bodies were truncated in the context file and could not be individually re-traced this round: 3757694238, 3757694247, 3757694252, 3757694261, 3757694270, 3760539332, 3760539361, 3760539368, 3760539375, 3762610240, 3762610264, 3762610287, 3762610302, 3762610308, 3762610312, 3762610315, 3764177561, 3764177665, 3764177668, 3764177691, 3767483260, 3767483269
- [Critical] base-tree.ts — round-5 blocker 3762610292 (npm-applicability empty-string-glob twin) body truncated; not re-traced this round
- [Critical] maven-toolchain.test.ts — round-5 blocker 3764177649 (three win32-unguarded wrapper tests) body truncated; note the win32 paths were also not exercised by this review's Linux-only run
Not reviewed: reverse audit — stopped at the 3-round cap (huge-diff budget) without converging: all three rounds reported new findings.
Not reviewed: build-and-test — 'Test (macos-latest, Node 22.x)' and 'Test (windows-latest, Node 22.x)' were skipped in CI and the packages/cli suite ran on Linux only, so this diff's win32-specific paths (wrapper selection, launch-death wordings, runner vocabulary) were not exercised on those platforms.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — finished within the tool budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: did not walk mavenToolchainAdapter.run() 's warm-up classification path end to end (it lives outside my chunk, in maven-toolchain.ts); I verified via the test …; Change summary: PR #8777 (QwenLM/qwen-code) adds a Maven ...: none — all checks in my dimension completed (tool usage well under the ceiling)., and 9 more.
Test Plan (not a blocker): ./mvnw — no such file or directory; 2477 tests passed — this review observed 19714 passed.
中文说明
未决，请确认：共 4 条（原文未翻译，列表见上方英文部分）。
未审查：reverse audit — stopped at the 3-round cap (huge-diff budget) without converging: all three rounds reported new findings。
未审查：build-and-test — 'Test (macos-latest, Node 22.x)' and 'Test (windows-latest, Node 22.x)' were skipped in CI and the packages/cli suite ran on Linux only, so this diff's win32-specific paths (wrapper selection, launch-death wordings, runner vocabulary) were not exercised on those platforms。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — finished within the tool budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：did not walk mavenToolchainAdapter.run() 's warm-up classification path end to end (it lives outside my chunk, in maven-toolchain.ts); I verified via the test …；Change summary: PR #8777 (QwenLM/qwen-code) adds a Maven ...：none — all checks in my dimension completed (tool usage well under the ceiling).，另有 9 条。
Test Plan（非阻断）：./mvnw — no such file or directory; 2477 tests passed — this review observed 19714 passed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const exempt = | ||
| !comment && | ||
| !contentSinceOpen && | ||
| (innermost === 'system-out' || innermost === 'system-err'); |
There was a problem hiding this comment.
[Critical] R7-1: The system-out/system-err CDATA exemption bypasses the swallowing-shape probe entirely. A CDATA opened immediately after the stream open tag whose first ]]> lands only after swallowing </system-out></testcase></testsuite> plus a whole second suite deletes that suite's header and failure evidence. — Failure scenario (probe-confirmed end-to-end through the adapter): a PR-controlled fresh report whose green suite opens <system-out><![CDATA[ and first closes after swallowing a second suite (tests="1" failures="1" with <failure>boom</failure>) parses as tests=2, failures=0 → ok:true, a failing run certified green. The comment-twin shape is rejected fail-closed; no downstream gate (stdout cross-check, summaryTotals, evidenceCapped) catches it. Suggested fix: narrow the exemption instead of waiving the probe — still reject when the interior contains a close of the stream element itself (e.g. if (exempt && new RegExp('</\\s*' + innermost, 'i').test(interior)) return null;), failing closed into the disclosed rejected gap.
中文说明
针对 system-out/system-err 的 CDATA 豁免完全绕过了吞噬形态探针。紧跟流元素开始标签打开、且首个 ]]> 落在吞掉 </system-out></testcase></testsuite> 乃至整个第二个 testsuite 之后的 CDATA，会删除该 suite 的头部与失败证据。失败场景（已通过 adapter 端到端探针确认）：PR 可控的新报告中，绿色 suite 打开 <system-out><![CDATA[，首次闭合发生在吞掉第二个 suite（tests="1" failures="1" 且含 <failure>boom</failure>）之后 → 解析为 tests=2, failures=0 → ok:true，一次失败的运行被认证为绿色。注释（comment）同构形态会被 fail-closed 拒绝；下游任何闸门（stdout 交叉检查、summaryTotals、evidenceCapped）都拦不住它。建议修复：收窄豁免而非取消探针——当内部包含流元素自身的闭合标签时仍然拒绝（如 if (exempt && new RegExp('</\\s*' + innermost, 'i').test(interior)) return null;），fail-closed 进入已披露的 rejected 缺口。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if (char !== '<') continue; | ||
| if (isTestcaseCloseAt(xml, i)) { | ||
| let end = i; | ||
| while (xml[end] !== '>') end += 1; | ||
| return { start: i, end: end + 1 }; | ||
| } | ||
| inTag = true; |
There was a problem hiding this comment.
[Critical] R7-9: An unterminated <![CDATA[ section is kept verbatim by stripOpaqueSections (the end === -1 break path, whose docstring claims it "fails closed"), and findTestcaseClose then scans that opaque text as markup — a planted </testcase> inside it cuts the testcase body before its <failure>/<error> evidence, so a failing report parses green. — Failure scenario (probe-confirmed): a fresh report with <![CDATA[ x > </testcase> (never terminated) before a live <failure> body → ok:true, "Maven test passed with fresh reports: 1 tests, 0 failures"; the terminated twin is rejected fail-closed. PR-writable reports are this file's stated threat model. Suggested fix: fail closed on unterminated sections (return null), or run the swallowing-shape probe over the unterminated tail before keeping it.
中文说明
未闭合的 <![CDATA[ 段会被 stripOpaqueSections 原样保留（end === -1 的 break 路径，其文档注释声称"fail closed"），随后 findTestcaseClose 把这段不透明文本当作标记扫描——其中植入的 </testcase> 会在 <failure>/<error> 证据之前截断 testcase 正文，使失败报告解析为绿色。失败场景（探针确认）：新报告在真实 <failure> 正文之前含有 <![CDATA[ x > </testcase>（永不闭合）→ ok:true、"Maven test passed with fresh reports: 1 tests, 0 failures"；带闭合的孪生形态则被 fail-closed 拒绝。PR 可写报告正是本文件声明的威胁模型。建议修复：对未闭合段 fail-closed（返回 null），或在保留前对未闭合尾部运行吞噬形态探针。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| (isDependencyFailure(result.output) && !dependencyInputsChanged)) && | ||
| !framingUntrusted) || |
There was a problem hiding this comment.
[Critical] R7-10: At failing exits the acquisition arms scan the WHOLE output while the source-failure suppression reads only the pre-test-phase prelude — dependency wording echoed by a passing upstream test launders a genuine later compile failure into an infrastructure result. — Failure scenario (probe-confirmed through the real adapter): -pl changed-mod -am test, exit 1, an upstream test echoing [ERROR] Could not resolve dependencies plus the changed module's genuine COMPILATION ERROR after the first [INFO] Running marker → infrastructure:true, note "This is infrastructure evidence, not a source finding"; the control without the echo → source finding. The echoed line is the only difference between the arms. A fix scoped to one predicate must cover the sibling disk arm too (see the twin comment at the isDiskFailureLine site).
中文说明
在退出码非 0 时，获取类分支扫描整个输出，而源码失败抑制只读测试阶段之前的前奏——上游 passing 测试回显的依赖类措辞会把其后真实的编译失败洗白成基础设施结果。失败场景（已通过真实 adapter 探针确认）：-pl changed-mod -am test，退出码 1，上游测试回显 [ERROR] Could not resolve dependencies，随后被改动模块出现真实 COMPILATION ERROR（位于首个 [INFO] Running 标记之后）→ infrastructure:true，note 为 "This is infrastructure evidence, not a source finding"；去掉回显行的对照 → 源码失败。回显行是两个分支间唯一的差异。修复若只针对某个谓词，必须同时覆盖同族的 disk 分支（见 isDiskFailureLine 处的孪生评论）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| /^(?:curl: \(\d+\)|wget: )/.test(line), | ||
| ) || lines.some(isDiskFailureLine) |
There was a problem hiding this comment.
[Critical] R7-10 (disk arm): The disk-full component of isLaunchFailure scans the whole output — including test stdout echoed after Maven framing — and feeds acquisitionFailure at failing exits: the same laundering mechanism as the dependency arm (twin comment there). — Failure scenario (probe-confirmed): on a -pl changed -am run, an upstream module's passing test prints [ERROR] … No space left on device in its own stdout; the changed module then fails to compile (no Surefire XML, upstream reports green, compile error after the prelude cut) → infrastructure:true: a genuine PR compile failure laundered into an environmental result. Suggested fix: at failing exits, run the disk scan over preTestPhaseOutput like the source-failure suppression, or gate it behind the same carve-out exceptions as the wording arms.
中文说明
isLaunchFailure 的磁盘满分支扫描整个输出——包括 Maven 框架之后回显的测试 stdout——并在退出码非 0 时喂给 acquisitionFailure：与依赖分支（见该处孪生评论）相同的洗白机制。失败场景（探针确认）：-pl changed -am 运行中，上游模块的 passing 测试在自己的 stdout 打印 [ERROR] … No space left on device；随后被改动模块编译失败（无 Surefire XML、上游报告绿色、编译错误位于前奏切割点之后）→ infrastructure:true：PR 自身的编译失败被洗白成环境结果。建议修复：退出码非 0 时让 disk 扫描只跑 preTestPhaseOutput（与源码失败抑制一致），或将其置于与措辞分支相同的豁免例外之后。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| rejected && | ||
| !( | ||
| result.exitCode === 0 && |
There was a problem hiding this comment.
[Critical] R7-11: The selector-rejection gate discards an exit-0 run that carries fresh FAILING Surefire reports to unsupportedReport whenever forged rejection wording also matches — the keep-exception demands !freshFailures, exactly backwards. — Failure scenario (probe-confirmed): exit-0 run with fresh failing XML whose test stdout forges [ERROR] Could not find the selected project in the reactor: core → toolchain:unsupported, ok:true, zero commands recorded, note "this run verified nothing"; the captured genuine failures never surface, and the agent brief forbids replacing adapter-unsupported results with ad-hoc runs. Absent fail-never, a genuine rejection fail-fasts non-zero before any test runs — so exit 0 + any fresh reports + rejection wording is always forgery. Suggested fix: drop !freshFailures from the keep-exception (the green twin already survives; the failing twin must too).
中文说明
当伪造的拒绝措辞同时匹配时，选择器拒绝闸门会把携带新的失败 Surefire 报告的退出码 0 运行丢弃进 unsupportedReport——保留例外要求 !freshFailures，方向恰好反了。失败场景（探针确认）：退出码 0、有新失败 XML、测试 stdout 伪造 [ERROR] Could not find the selected project in the reactor: core 的运行 → toolchain:unsupported、ok:true、零命令记录、note "this run verified nothing"；被捕获的真实失败永不可见，且 agent brief 禁止用临时命令替代 adapter 的 unsupported 结果。在没有 fail-never 时，真实拒绝会在任何测试运行前以非 0 快速失败——因此退出码 0 + 任何新报告 + 拒绝措辞必然是伪造。建议修复：从保留例外中去掉 !freshFailures（绿色孪生已经保留，失败孪生也必须保留）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| '^(?:npm|npx|yarn|pnpm|bun|make|node|go|cargo|python3?|pytest)\\b', | ||
| ); | ||
|
|
||
| const MAVEN_RUNNER_RE = new RegExp(`^(?:${MAVEN_RUNNER_SOURCE})(?=\\s|$)`, 'i'); |
There was a problem hiding this comment.
[Suggestion] R7-32: Case-insensitive runner extraction (MAVEN_RUNNER_RE, 'i') promises settlement for Windows-style capitalization, but the settlement grammar's phase vocabulary (MAVEN_PHASE_RE, no 'i') is case-sensitive — so any claim with a non-lowercase phase is extracted and then can never settle: the exact outcome the case-insensitive extraction was added to prevent. — Concrete cost (probe-confirmed): ./mvnw test and MVNW test → reproduces, but MVNW TEST and ./mvnw TEST → unchecked with the misstated note "the Maven runs it made had a different scope or phase" — scope and phase matched, only casing differed. BARE_MAVEN_LIFECYCLE_RE IS built with 'i' while MAVEN_PHASE_RE is not — the two halves of the same grammar disagree. Suggested fix: lowercase Maven claim tokens once at the top of ruleCommand, or give MAVEN_PHASE_RE/MAVEN_UNRUN_WORK_RE the i flag.
中文说明
大小写不敏感的 runner 提取（MAVEN_RUNNER_RE，'i'）承诺为 Windows 风格大写提供结算，但结算语法的阶段词汇表（MAVEN_PHASE_RE，无 'i'）区分大小写——于是任何阶段非小写的 claim 被提取后永远无法结算：恰是引入大小写不敏感提取所要防止的结果。具体代价（探针确认）：./mvnw test 与 MVNW test → reproduces，但 MVNW TEST 与 ./mvnw TEST → unchecked，附错误 note "the Maven runs it made had a different scope or phase"——作用域与阶段都匹配，只有大小写不同。BARE_MAVEN_LIFECYCLE_RE 带 'i' 而 MAVEN_PHASE_RE 不带——同一语法的两半互相矛盾。建议修复：在 ruleCommand 顶部一次性小写化 Maven claim 令牌，或给 MAVEN_PHASE_RE/MAVEN_UNRUN_WORK_RE 加 i 标志。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred this round: valid grammar inconsistency (case-insensitive runner extraction vs case-sensitive MAVEN_PHASE_RE — MVNW TEST extracts but can never settle). One consistent lowercase-at-the-gate fix; out of the Critical-only scope, tracked for the follow-up.
中文说明
本轮推迟：语法不一致确实存在（大小写不敏感的 runner 提取 vs 大小写敏感的 MAVEN_PHASE_RE——MVNW TEST 能提取却永远无法 settle）。在入口统一小写化即可修复；超出本轮 Critical-only 范围，留待后续处理。
| if (PATH_RE.test(t) && isPathClaim(t)) { | ||
| // The bare-runner guard above applies to argument tokens too: `./mvnw` | ||
| // as a command's runner token is the runner, not a path claim. | ||
| if (PATH_RE.test(t) && isPathClaim(t) && !MAVEN_RUNNER_RE.test(t)) { |
There was a problem hiding this comment.
[Suggestion] R7-33: The bare-runner guard in extractClaims' argument-token loop applies to the token stream of EVERY span, including prose spans that are not commands — so a ./mvnw-style file reference there loses its path claim. The guard's own comment ("as a command's runner token") names a condition the code never checks. — Concrete cost (probe-confirmed): wrappers ./mvnw here (a non-command span) → [] — the ./mvnw file reference is dropped; standalone `./mvnw` spans work as designed. If the diff deletes or never adds the wrapper, wrapper-existence claims inside prose spans produce zero claims and no contradicted note — silent under-extraction of exactly the claims the span-level guard was written to preserve. Suggested fix: gate the loop's !MAVEN_RUNNER_RE.test(t) on the span actually being a command (or exclude only the first token of command spans).
中文说明
extractClaims 参数令牌循环中的裸 runner 守卫作用于每个 span 的令牌流——包括不是命令的叙述性 span——于是其中的 ./mvnw 式文件引用会丢失其路径 claim。守卫自己的注释（"as a command's runner token"）说出了一个代码从未检查的条件。具体代价（探针确认）：wrappers ./mvnw here（非命令 span）→ []——./mvnw 文件引用被丢弃；独立的 `./mvnw` span 按设计工作。若 diff 删除或从未添加 wrapper，叙述性 span 中的 wrapper 存在性 claim 产生零 claim、也没有 contradicted note——恰是 span 级守卫本要保留的 claim 被静默漏提。建议修复：把循环中的 !MAVEN_RUNNER_RE.test(t) 限定在该 span 确实是命令时（或只排除命令 span 的第一个令牌）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred this round: valid extraction gap (the bare-runner guard applies to prose-span token streams too, dropping ./mvnw file references in non-command spans). Out of the Critical-only scope; tracked for the follow-up.
中文说明
本轮推迟：提取缺口确实存在（裸 runner 守卫也作用于散文片段的 token 流，导致非命令片段中的 ./mvnw 文件引用被丢弃）。超出本轮 Critical-only 范围；留待后续处理。
| @@ -55,8 +55,9 @@ import { | |||
| const ANSI_SGR_RE = /\x1b\[[0-9;]*m/g; | |||
There was a problem hiding this comment.
[Suggestion] R7-46: This PR consolidated the SGR-strip regex into maven-toolchain.ts' exported ANSI_SGR_RE (deleting build-test's private copy, with a comment that sharing makes both sides "strip the same bytes: same answer") but left this third, private copy unconsolidated — three lines above a comment block this PR edits. — Concrete cost: failingFilesOf re-parses the same recorded npm-command bytes with this private copy that the primary pipeline stripped with the shared constant; a future change to the canonical regex — exactly the kind this PR performs — updates one copy and not the other, and test-delta's net-new adjudication silently diverges from the verdict it is supposed to diff against. A drift trap planted in a verdict-relevant byte-level predicate by an incomplete consolidation. Suggested fix: delete the private constant (and its eslint-disable) and import { ANSI_SGR_RE } from './lib/maven-toolchain.js', matching build-test.ts.
中文说明
本 PR 把 SGR 剥离正则统一到 maven-toolchain.ts 导出的 ANSI_SGR_RE（删除了 build-test 的私有副本，并注明共享使两侧"剥离相同字节：相同答案"），却留下了这第三个私有副本未统一——就在本 PR 编辑的注释块上方三行。具体代价：failingFilesOf 用这个私有副本重新解析同一些已记录的 npm 命令字节，而主流水线用的是共享常量剥离；未来对规范正则的改动——正是本 PR 做的那种——只更新其中一份，test-delta 的 net-new 裁决就会与它本应对比的裁决悄悄偏离。一个由不完整统一在裁决相关的字节级谓词中埋下的漂移陷阱。建议修复：删除私有常量（及其 eslint-disable），import { ANSI_SGR_RE } from './lib/maven-toolchain.js'，与 build-test.ts 一致。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred this round: valid incomplete consolidation (test-delta keeps a private ANSI_SGR_RE copy while the PR consolidated the other two). Three-line fix but it is verdict-relevant byte-level behavior; lands as its own focused change in the follow-up.
中文说明
本轮推迟：合并不完整确实存在（本 PR 合并了另外两处，test-delta 仍保留私有 ANSI_SGR_RE 副本）。虽是三行修复，但涉及判定相关的字节级行为；留待后续作为独立聚焦改动处理。
| while ((match = interiorClose.exec(interior)) !== null) { | ||
| const name = match[1].toLowerCase(); | ||
| if ((openCounts.get(name) ?? 0) > 0) { |
There was a problem hiding this comment.
[Suggestion] R7-47: The swallowing-shape close probe consults openCounts before pairing interior closes against earlier interior opens, so the doc-comment's explicitly sanctioned "self-contained phantom markup, whose open/close pairs both sit inside the comment" is still rejected whenever the phantom's element name collides with a genuinely open element. — Concrete cost (probe-confirmed): a green report with the self-contained phantom comment AFTER the closed suite parses ok:true (comparator alive), while the identical comment INSIDE the open suite yields ok:false + evidenceCapped ("could not be parsed") — a genuinely green run reads as uncertified evidence, contradicting the function's own documented contract. Direction is fail-closed (never greenwash), hence Suggestion. Suggested fix: pair interior closes against preceding interior opens of the same name first (a per-section count offset), consulting openCounts only for closes that remain unpaired.
中文说明
吞噬形态闭合探针在与更早的内部打开配对之前先查询 openCounts，于是文档注释明确允许的"自包含幽灵标记（开/闭对都在注释内部）"在幽灵元素名与真实打开的元素冲突时仍被拒绝。具体代价（探针确认）：自包含幽灵注释位于已闭合 suite 之后的绿色报告解析为 ok:true（比较器存活），而相同注释位于打开的 suite 内部时却得到 ok:false + evidenceCapped（"could not be parsed"）——真正绿色的运行被读作不确定证据，与函数自己的文档契约矛盾。方向为 fail-closed（永不漂绿），故为 Suggestion。建议修复：先让内部闭合与同名的更早内部打开配对（按节计数偏移），只对仍未配对的闭合查询 openCounts。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred for the older non-exempt probe this round; note the NEW exempt-branch sequence probe added this round for R7-1 already pairs interior closes against earlier interior opens before consulting the open-at-marker counts (the exact pairing this finding asks for). Applying the same re-order to the non-exempt probe is tracked for the follow-up.
中文说明
本轮对较旧的非豁免探针推迟；注意本轮为 R7-1 新增的豁免分支顺序探针已先做"内部闭合与更早的内部开标签配对"再查标记处打开计数（正是该发现要求的配对）。把同样的顺序调整应用到非豁免探针留待后续处理。
| const mavenCommand = | ||
| MAVEN_RUNNER_RE.test(span) && span.split(/\s+/).length > 1; | ||
| if (RUNNER_RE.test(span) || mavenCommand) { | ||
| push('command', span); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R7-49: Bare backslash wrapper spellings (.\mvnw, .\mvnw.cmd, ..\mvnw) extract no claim at all — neither command nor path — even though the added comment block explicitly models .\mvnw and the bare-runner gate exists to route phase-less runner tokens to path verification. — Concrete cost (probe-verified): MAVEN_RUNNER_RE matches .\mvnw, the span is single-token so it is not a command, and PATH_RE (forward-slash-only class) rejects the backslash spelling → the span matches no branch; forward-slash twins (./mvnw, ../../mvnw.cmd) parse as path claims (pinned at test-plan.test.ts:1287). Trigger: a Windows author writes "added .\mvnw.cmd" and the PR forgets to commit the wrapper file — ./mvnw.cmd would yield a contradicted path note, but .\mvnw.cmd escapes claim verification entirely. Suggested fix: when MAVEN_RUNNER_RE.test(span) and the span is a single token, push it as a path claim directly (teaching the existence check to accept backslash separators).
中文说明
裸反斜杠 wrapper 拼写（.\mvnw、.\mvnw.cmd、..\mvnw）完全不提取任何 claim——既非命令也非路径——尽管新增注释块明确建模了 .\mvnw，且裸 runner 闸门的存在就是为了把无阶段 runner 令牌路由到路径验证。具体代价（探针验证）：MAVEN_RUNNER_RE 匹配 .\mvnw，span 为单令牌故不是命令，而 PATH_RE（仅正斜杠字符类）拒绝反斜杠拼写 → 该 span 不匹配任何分支；正斜杠孪生（./mvnw、../../mvnw.cmd）解析为路径 claim（test-plan.test.ts:1287 已 pin）。触发：Windows 作者写 "added .\mvnw.cmd" 而 PR 忘记提交 wrapper 文件——./mvnw.cmd 会给出 contradicted 路径 note，但 .\mvnw.cmd 完全逃脱 claim 验证。建议修复：当 MAVEN_RUNNER_RE.test(span) 且 span 为单令牌时直接推入路径 claim（并让存在性检查接受反斜杠分隔符）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred this round: valid extraction gap (bare backslash wrapper spellings .\mvnw/.\mvnw.cmd extract no claim — neither command nor path — despite the comment block modeling them; forward-slash twins work). Out of the Critical-only scope; tracked for the follow-up.
中文说明
本轮推迟：提取缺口确实存在（裸反斜杠 wrapper 写法 .\mvnw/.\mvnw.cmd 提取不出任何声明——既非命令也非路径——尽管注释块明确建模了它们；正斜杠孪生形态正常）。超出本轮 Critical-only 范围；留待后续处理。
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 7/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31658644409 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Fail closed on the remaining greenwash shapes in the Maven report parser (swallowing exempt CDATA, unterminated sections, testcase nesting in quoted attributes), stop echoed infrastructure wording and forged selector rejections from laundering or hiding genuine failures, tighten never-ran evidence for diff-modified wrappers, and close the Test Plan holes where capped/exit-0 failure evidence settled nothing or contradicted its build-test verdict.
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Autofix round 8 — PR #8777 (address-review)Scope note: this round ran under the budget warning (a previous round ran Critical findings — fixed (15)
Critical findings — escalated for a maintainer decision (1)
Suggestions — deferred (20)R7-2, R7-3, R7-4, R7-5, R7-5-twin, R7-6, R7-8, R7-18, R7-19, R7-20, R7-21, Re-asserted round-2..6 blocker threads (56, bodies truncated in the reviewer's context)These were individually un-retraceable by the reviewer this round. From the Conflict
Verification
中文说明Autofix 第 8 轮 — PR #8777（address-review）范围说明： 本轮在预算警告下运行（上一轮耗尽时间），因此只处理阻塞子集 Critical 发现 — 已修复（15 个）
Critical 发现 — 升级给维护者决策（1 个）
Suggestion — 推迟（20 个）R7-2、R7-3、R7-4、R7-5、R7-5 孪生、R7-6、R7-8、R7-18、R7-19、R7-20、R7-21、 被重新提出的第 2..6 轮阻塞线程（56 个，正文在评审者上下文中被截断）评审者本轮无法逐条重新追踪。通过工作流自带的评论存档（本地可读取完整正文） 冲突
验证
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; only the Linux matrix ran, and this diff carries win32-specific logic (selector wrap, case-insensitive-FS paths).
Not explored to full depth (tool budget reached): chunk 3: could not execute agent-prompt.test.ts / base-tree.test.ts — the worktree and parent checkout have no node_modules (vitest unresolvable); assertions were …; "You are review agent reverse-audit — Reverse audit agent…": did not run the unit suite to observe the live verdict text for an unknownWork uppercase claim (mechanism verified by reading instead).; chunk 22: none — all checks I started were completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget., and 10 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Test Plan (not a blocker): ./mvnw — no such file or directory; 2477 tests passed — this review observed 19722 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; only the Linux matrix ran, and this diff carries win32-specific logic (selector wrap, case-insensitive-FS paths)。
未探索到全部深度（达到工具调用预算）：chunk 3：could not execute agent-prompt.test.ts / base-tree.test.ts — the worktree and parent checkout have no node_modules (vitest unresolvable); assertions were …；"You are review agent reverse-audit — Reverse audit agent…"：did not run the unit suite to observe the live verdict text for an unknownWork uppercase claim (mechanism verified by reading instead).；chunk 22：none — all checks I started were completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.，另有 10 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
Test Plan（非阻断）：./mvnw — no such file or directory; 2477 tests passed — this review observed 19722 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| Command results carry five optional classification flags consumed by | ||
| `test-plan`: |
There was a problem hiding this comment.
[Suggestion] R8-22: The Report-semantics enumeration says "five optional classification flags" but this PR adds a sixth, swallowedReports — whose entire contract is "consumers must read this as a failed run, never as a pass". It is consumed by both test-plan (settlement-blocking filter) and test-delta (failed-run filter); the Open-questions list even names rescueOverflow (correctly absent here — test-plan never reads it directly) yet omits swallowedReports. — Failure scenario: a follow-up adapter author (the Gradle phase this doc sketches) enumerating settlement-blocking flags from this spec implements five, misses swallowedReports, and exit-0-over-failing-reports runs read as all-clear in the new consumer. Fix: add a CommandResult.swallowedReports bullet and change "five" to "six".
中文说明
报告语义部分写的是「五个可选分类标志」，但本 PR 新增了第六个 swallowedReports——它的完整契约是「消费方必须将其视为失败运行，绝不能视为通过」。test-plan（阻断结算的过滤器）和 test-delta（失败运行过滤器）都在消费它；Open-questions 列表甚至列出了 rescueOverflow（它不出现在此处是正确的——test-plan 从不直接读取），却遗漏了 swallowedReports。后果：后续适配器（如文档规划的 Gradle 阶段）按此规范实现时只会实现五个标志，漏掉 swallowedReports，导致 exit-0 但报告有失败的运行被当作全部通过。建议补充该标志说明并把「五」改为「六」。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // the guard's conjunct then makes npm applicable and flips selection | ||
| // to the ambiguous handoff, turning this test red. |
There was a problem hiding this comment.
[Suggestion] R8-23: This mutation-pin comment is false: the PR's own concession fallback (if (report.toolchain === 'unsupported') return mavenToolchainAdapter.run(runArgs), build-test.ts:524) converts the "ambiguous handoff" right back into the same Maven sentinel report. — Failure scenario: mutation-verified — dropping the !hasUnmodeledWorkspaceGlob(globs) conjunct from npmToolchainAdapter.applies() keeps this test AND the zero-package sibling (:576) green (full build-test 93 + npm-toolchain 7 suites green): applies() flips true → ambiguous → npm runs → run-side gate concedes unsupported → fallback returns the same sentinel. The applies()-side guard is pinned by no test; if it is removed while the fallback is ever reverted, the original defect returns with nothing red. Fix: pin applies() directly (the pattern npm-toolchain.test.ts uses) and correct the comment.
中文说明
这条「变异必杀」注释是错误的：本 PR 自己新增的让步回退（build-test.ts:524）会把「歧义移交」直接转换回相同的 Maven 哨兵报告。变异验证：从 npmToolchainAdapter.applies() 中删除 !hasUnmodeledWorkspaceGlob(globs) 后，本测试与 :576 的零包兄弟测试仍然全绿（build-test 93 + npm-toolchain 7 全部通过）。也就是说 applies() 侧的守卫没有任何测试固定；若将来该守卫被删除且回退又被还原，原缺陷将在无测试报警的情况下回归。建议直接断言 applies() 返回值（npm-toolchain.test.ts 的既有写法）并修正注释。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| * The command exited 0 over fresh failing Surefire/Failsafe reports (a | ||
| * `testFailureIgnore`-style setting swallowed them). None of the other | ||
| * flags fire for this shape — they all key on the ABSENCE of fresh |
There was a problem hiding this comment.
[Suggestion] R8-25: The swallowedReports docstring claims "None of the other flags fire for this shape — they all key on the ABSENCE of fresh failing reports", but evidenceCapped can co-occur — probe-verified: exit 0 + fresh failing report + oversized (parser-rejected) report → both flags; also with a -l log-file config. The producer comment (maven-toolchain.ts:2324) states the opposite itself: flags are set independently. — Concrete cost: three locations overclaim exclusivity (here, maven-toolchain.ts:2124 "carries NONE of the classification flags", maven-toolchain.test.ts:677); no behavioral bug today (consumers OR all flags), but it is a demonstrably false contract a future consumer could rely on. Fix: state that unread-evidence flags (evidenceCapped) can coincide.
中文说明
swallowedReports 的文档字符串声称「此形态下其他标志都不会触发——它们都取决于'不存在'新的失败报告」，但 evidenceCapped 可以同时出现——探针验证：exit 0 + 新的失败报告 + 超大（被解析器拒绝的）报告 → 两个标志同时为真；-l 日志配置下同样如此。生产方注释（maven-toolchain.ts:2324）自己也说标志是独立设置的。共三处过度声明（此处、maven-toolchain.ts:2124、maven-toolchain.test.ts:677）。当前无行为缺陷（消费方对所有标志取或），但这是一份可被证伪的契约。建议改为说明「未读证据类标志（evidenceCapped）可能同时出现」。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (report.toolchain === 'unsupported') { | ||
| return mavenToolchainAdapter.run(runArgs); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R8-8: The mixed-root Maven fallback fires only when npm concedes toolchain: 'unsupported'; a vacuous npm run never attempts the Maven adapter. — Failure scenario (probe-verified, flips with the fix): polyglot root with modeled workspaces resolving a real package + root pom.xml, diff touching only core/src/Main.java → npm applies, affectedWorkspaces resolves to [], npm returns toolchain: 'npm', ok:true, ZERO commands executed → the === 'unsupported' check fails and Maven is never run. The exact polyglot+Maven-only shape this PR exists to verify gets zero automated Maven verification — only the disclosure note. The cold-yarn concession case gets the Maven fallback for the same "nothing ran" rationale. Fix: also fall back when npm executed nothing (empty install/build/test), keeping the mixed note only when npm actually ran.
中文说明
混合根回退只在 npm 让步（toolchain: 'unsupported'）时触发；npm「空跑」时永远不会尝试 Maven 适配器。探针验证：多语言根（建模的 workspaces 能解析出真实包 + 根 pom.xml），diff 只改 core/src/Main.java → npm 适用但 affectedWorkspaces 为空，返回 toolchain: 'npm'、ok:true、零命令 → === 'unsupported' 不成立，Maven 永远不运行。这正是本 PR 要验证的「多语言仓库 + 纯 Maven 改动」形态，却得不到任何自动化 Maven 验证，只有一句说明文字；而冷 yarn 让步场景基于同样的「什么都没跑」理由却能得到 Maven 回退。建议：npm 未执行任何命令时也回退到 Maven。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } else if (match[3] !== '/') { | ||
| if (closesSurrounding) return null; |
There was a problem hiding this comment.
[Critical] R8-11: The exempt stream-CDATA swallowing probe (added this round for R7-1) skips self-closing interior opens (else if (match[3] !== '/')), so a self-closing failing suite swallowed by an exempt CDATA deletes its header and the report parses green. — Failure scenario (probe-verified, flips when self-closing opens reject like opens): report <testsuite…><testcase…><system-out><![CDATA[</system-out></testcase></testsuite><testsuite tests="1" failures="1"…/>]]></system-out></testcase></testsuite> → closesSurrounding is set by the surrounding closes, but the self-closing <testsuite failures="1"/> token falls through both branches; after stripping, the swallowed suite's header is gone → ok:true, rollup tests=1, failures=0. A self-closing <testsuite> is verdict markup this very parser reads (header counts include self-closing suites), so swallowing it deletes failure evidence — a shape the rationale does not answer. Two independent traces (round-1 chunk 13 + the R7-1 ledger re-check's self-closing residual) converged here.
中文说明
本轮为 R7-1 新增的「豁免流 CDATA 吞没探针」会跳过自闭合的内部开标签（match[3] !== '/' 分支），因此被豁免 CDATA 吞没的自闭合失败套件会丢失其头部，报告按绿色解析。探针验证：上述构造 → closesSurrounding 被置位，但自闭合的 <testsuite failures="1"/> 两个分支都不命中；剥离后该失败套件头部消失 → ok:true、汇总 tests=1, failures=0。自闭合 <testsuite> 正是本解析器读取的裁决标记（头部计数包含自闭合套件），吞没它等于删除失败证据。两条独立追踪（一轮 chunk 13 与 R7-1 复核的自闭合残留）汇聚于此。建议：自闭合开标签按普通开标签一样拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| try { | ||
| parsed = parseShellQuote(danced, literalEnv, { escape: '\u0000' }); | ||
| } catch { |
There was a problem hiding this comment.
[Critical] R8-12: shellTokens inherits shell-quote's non-POSIX comment rule — an unquoted # ANYWHERE starts a comment, dropping the rest of the token AND every following token, whereas a real shell starts a comment only when # begins a word. — Failure scenario (empirically verified against installed shell-quote 1.9.0 + end-to-end probe through runTestPlan, flips when mid-word # routes to the conservative fallback): shellTokens('mvn test#install') → ['mvn','test'] and shellTokens('mvn test -pl core#1') → ['mvn','test','-pl','core'] (bash passes the single words whole, and real Maven dies on "Unknown lifecycle phase" / rejects the selector — zero work). Both claims rule reproduces against recorded green runs: over-settlement of dead commands, and core#1 settles on module core's evidence. Quoted spellings are safe and real trailing comments still drop correctly; only mid-word unquoted # mis-splits. The docstring's "the same word-splitting a shell performs" is false for this case. Fix: substitute a sentinel for non-word-start # outside quotes (mirroring the '\'' dance) or route such claims to the whitespace-split fallback so unknownWork fires.
中文说明
shellTokens 继承了 shell-quote 非 POSIX 的注释规则：任意位置的未引用 # 都被当作注释起点，丢弃该 token 剩余部分及其后所有 token；而真实 shell 只在 # 位于词首时才开始注释。对 shell-quote 1.9.0 的实测 + runTestPlan 端到端探针验证（把词中 # 路由到保守回退后翻转）：mvn test#install → ['mvn','test']、mvn test -pl core#1 → ['mvn','test','-pl','core']（bash 传递的是完整单词，真实 Maven 会报 Unknown lifecycle phase/拒绝选择器，零工作），两个断言却都判 reproduces——死命令被过度结算，core#1 用模块 core 的证据结算。建议对引号外非词首的 # 使用哨兵替换或路由到空白切分回退。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| matches.find( | ||
| (c) => c.evidenceCapped === true && cappedDefinitiveRuling(c) !== null, | ||
| ) ?? |
There was a problem hiding this comment.
[Critical] R8-1: The capped-run ranking slot admits infrastructure-classified runs — no !c.infrastructure guard — so cappedDefinitiveRuling's non-zero-exit arm settles a claim contradicted off an environmental failure. — Failure scenario (probe-verified end-to-end through adapter + runTestPlan; flips with the one-line fix): a -pl core -am test run exits 1 on dependency resolution classified infrastructure while also carrying evidenceCapped (e.g. .mvn/maven.config -l build.log or a rejected fresh report — the producer sets the flags independently, maven-toolchain.ts:2322-2331) → the claim rules contradicted ("the non-zero exit is definitive"), although finished() excludes infrastructure runs, the CommandResult.infrastructure contract says test-plan "must not settle a Test Plan claim against" such results, and the cascade's environmental arm is never reached because if (ran) returns first. The exit-0 infra+capped test (~:3271) does not pin this non-zero-exit shape. Fix:
| matches.find( | |
| (c) => c.evidenceCapped === true && cappedDefinitiveRuling(c) !== null, | |
| ) ?? | |
| (c) => | |
| c.evidenceCapped === true && | |
| !c.infrastructure && | |
| cappedDefinitiveRuling(c) !== null, |
中文说明
封顶运行排序槽会接纳被分类为 infrastructure 的运行——缺少 !c.infrastructure 守卫——cappedDefinitiveRuling 的非零退出码分支会用环境失败把断言结算为 contradicted。端到端探针验证（一行修复后翻转）：依赖解析失败 exit 1 且同时 evidenceCapped（如 -l build.log 或被拒绝的新报告；生产方独立设置两个标志）→ 断言被判 contradicted，尽管 finished() 排除 infrastructure 运行、CommandResult.infrastructure 契约明确禁止用此类结果结算、且级联的环境分支因 if (ran) 提前返回而永远到不了。建议按 suggestion 补上 !c.infrastructure。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // The definitive shapes — a non-zero exit, or exit-0 failure evidence | ||
| // the cap cannot defeat — rule through the shared helper, the same | ||
| // ruling the ranking above hands them when a sibling matches. | ||
| // `neverRan` is deliberately NOT definitive here: the states that |
There was a problem hiding this comment.
[Critical] (comment 3762610287, re-checked this round: still stands): capped+neverRan is deliberately excluded from the definitive set, so the contradicted→unchecked verdict flip remains reachable via before.truncated. — Failure scenario: fresh.truncated inherits before.truncated (maven-toolchain.ts:1065), and the pre-run snapshot runs BEFORE the wrapper launches — a truncated PRE-run sweep (chmod-000 dir, >20k dirs) proves nothing about whether Maven started. A stub wrapper (exit 0, no framed output, zero summaries) then sets neverRan, so evidenceCapped && neverRan is reachable: the capped arm returns unchecked while the uncapped twin rules contradicted ("exit 0, but Maven never started") — the verdict flips "just because the cap also fired", the exact flip this blocker forbade. The codified rationale (cap states prove the toolchain DID start) is false for the pre-run-sweep source of the cap. Fix: track which source fired the cap; when only before.truncated fired, admit neverRan as definitive like the uncapped path.
中文说明
老 blocker（comment 3762610287）复核结论：仍然存在。封顶 + neverRan 被有意排除在决定性集合之外，因此经由 before.truncated 的 contradicted→unchecked 翻转仍可达：fresh.truncated 继承 before.truncated，而预运行快照在包装器启动之前执行——预运行扫描截断（chmod-000 目录、超过 2 万目录）并不能证明 Maven 是否启动；桩包装器随后触发 neverRan，封顶分支返回 unchecked，而未封顶孪生判 contradicted——裁决仅因「上限同时触发」而翻转。建议记录上限的触发来源：仅 before.truncated 触发时，按未封路径接纳 neverRan 的决定性。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const definitive = cappedDefinitiveRuling(capped); | ||
| if (definitive) return definitive; |
There was a problem hiding this comment.
[Suggestion] R8-6: Dead branch — the Maven cascade runs only when ran is undefined, which requires ranking slot 3 (lines 1607-1610) to have found nothing in the same matches; cappedDefinitiveRuling is pure (reads only claim-scoped state and immutable result fields), so const definitive = cappedDefinitiveRuling(capped); if (definitive) return definitive; can never fire. — Concrete cost: a maintainer changing cap semantics audits a branch that cannot execute (the capped arm can only produce the unchecked return below it); the adjacent comment ("the same ruling the ranking above hands them when a sibling matches") mis-describes the control flow — slot 3 admits a lone capped run too. Behavior is identical by construction, so no probe can distinguish; fix is readability-only: drop the two lines and adjust the comment.
中文说明
死分支：Maven 级联只在 ran 为 undefined 时执行，而这要求排序槽 3 在同一个 matches 里没找到任何东西；cappedDefinitiveRuling 是纯函数，因此级联里的 if (definitive) return definitive; 永远不会触发。代价是误导性：维护者审计一个不可能执行的分支，且相邻注释对控制流的描述有误（槽 3 也会接纳孤立的封顶运行）。行为上完全等价，修复仅为可读性：删除这两行并修正注释。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const failed = (report.test ?? []).filter( | ||
| (t) => !t.timedOut && t.exitCode !== 0, | ||
| (t) => | ||
| !t.timedOut && |
There was a problem hiding this comment.
[Critical] R8-15: The widened failed filter's blanket !t.timedOut exclusion misses the adapter's interrupted-with-captured-evidence family — all five ok:false flags it reads are gated on exitCode === 0 (maven-toolchain.ts:2130/:2207/:2250) while the run timed out, so a deadline-killed run carrying captured test failures reads as nothing-to-attribute. — Failure scenario (probe-verified end-to-end through the real adapter + runTestDelta; flips when timedOut runs carrying [maven-test-failure] markers join the disclosure): deadline kill after failing Surefire reports were written → build-test report ok:false, note "treat those as test failures, not as a pass or as purely environmental", output carrying [maven-test-failure] markers, none of the five flags set → runTestDelta returns "no PR-side test command failed — there is nothing to attribute, and the base run would measure nothing" — the opposite of build-test's verdict, produced via the timedOut exclusion instead of exit codes. The adapter's own pinned shape ("keeps fresh failing reports as test evidence when the run times out") and the Agent-7 brief's exception clause both state the contract the filter violates. Fix: admit timedOut runs carrying failure evidence, e.g. (t.timedOut === true && /^\[maven-test-failure\] /m.test(t.output ?? '')) as an additional disjunct.
中文说明
加宽后的 failed 过滤器用 !t.timedOut 一刀切排除，漏掉了「被中断但已捕获证据」的形态：它读取的五个 ok:false 标志都以 exitCode === 0 为前提，而超时运行的退出码为空，因此携带已捕获测试失败的超时运行会被当作「无可归因」。端到端探针验证（加入「携带失败标记的超时运行」分支后翻转）：截止时间杀死运行、失败 Surefire 报告已写入 → build-test 报告 ok:false 且说明「把这些当作测试失败」，但 runTestDelta 返回「没有 PR 侧测试命令失败——没有可归因的内容」——与 build-test 的裁决完全相反。适配器自己固定的形态与 Agent-7 简报的例外条款都陈述了该过滤器违反的契约。建议把携带失败证据的超时运行纳入披露分支。
— qwen3.8-max via Qwen Code /review (v0.21.11)
wenshao
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
|
Maintainer call: freezing the autofix loop on this PR — the approach, not the patches, is what needs to change. Eight rounds of "close the Nth-round fail-open gaps" and the reviewer is still finding real release-direction holes (R8-11 self-closing CDATA, R8-19 substring So: stop patching XML corners. Close the class instead. Class-closing refactor
What this dissolves
Landing it without another eight rounds
The rule this goes into the toolchain-adapter design doc as
Same lesson as #9020 — read what the tool authoritatively reports, don't enumerate the ways its output can lie — applied to build results. 中文说明维护者决定:冻结本 PR 的 autofix 循环——要改的是方法,不是补丁。 八轮"close the Nth-round fail-open gaps",reviewer 仍在找真实的放行漏洞(R8-11 自闭合 CDATA、R8-19 子串 所以:别再补 XML 角落,去关掉整类。 关类重构
落地:另开 refactor PR,本分支循环冻结;把现有 4565 行测试当 oracle,在同一批测试下换实现(同 markdown-it 换法),测试会瘦身。 写进 toolchain-adapter 设计文档的规则:判定不可信工具输出时,优先用权威结构化信号(退出码 + 机器可读报告),用真解析器解、解析失败即 fail-closed,绿必须有正向结构证据,不得不解析文法时用一处 spec 引用 tokenizer、歧义即 fail-closed。同 #9020 一课,搬到 build 结果上。 |
|
冻结与关类重构已落地：
合入顺序：本 PR 先行，#9099 随后。 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 9/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31696039130 🧠 Handled by Qwen Code · model/模型 |
|
关闭：由 #9099 取代（superseded）。 按维护者决定（冻结评论），本分支的补丁循环在 round 8（
本 PR 上的八轮 review 历史保留在此（关闭态可查），它驱动了 #9099 的最终形态。 |
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
…QwenLM#9104) QwenLM#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR QwenLM#8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
4 participants
What this PR does
Registers a Maven adapter on the toolchain boundary.
review build-testnow recognizes Maven roots, maps changed files to the owning module, prefers the root Maven Wrapper, runs one scopedtest(or build-onlytest-compile) from the reactor root with-amupstream expansion, and emits module-qualified Surefire/Failsafe evidence limited to reports the invocation itself created or updated.Downstream consumers follow:
test-plansettles Maven command and Surefire count claims, Agent 7 gains a Maven branch, and Maven failures do not enter the npm-onlytest-deltarerun path.base-treeskips Maven merge bases in this release (its A/B consumer is npm-only, so a Maven base build would be cost without a consumer); the gate lifts when Maven delta attribution exists.Why it's needed
The build-and-test path produced deterministic evidence for npm projects but handed Maven projects back to an agent for ad hoc command selection. Module ownership, command choice, result parsing, timeout classification, and failure attribution were all agent decisions, which is the weakest possible foundation for a review verdict.
Driven by active use in
alibaba/fastjson2andalibaba/druid: both are real multi-module reactors where core changes must run through the root wrapper, and where nested, profile-activated, and standalone POMs make a naive model unsafe.Design note: Maven owns the reactor
This adapter does not model the Maven reactor. It reads Maven's answer back:
pom.xmlwalk, skipping directories beneath asrc/tree (a POM there is maven-invoker or archetype test data).<modules>inheritance — is answered by Maven rejecting the-plselector (Could not find the selected project in the reactor), which it does before compiling anything. That rejection becomes theunsupportedhandoff.An earlier revision parsed the POMs directly — literal
<modules>recursion, CDATA and comment handling,<parent>relativePathresolution with the artifactId match Maven itself applies, named and deleted parent files, and a closure over all of it. That is a second, weaker model of exactly what the next command evaluates for real, and it was weakest on the shapes that motivated the work: profile-activated modules in Druid and Flink, where a text-level parse cannot evaluate activation and has to fail closed. Removing it deleted ~670 lines of adapter source and ~1360 lines of its tests, and moved the profile-activation answer from an approximation to Maven's own.docs/design/review-toolchain-adapters.mdrecords the decision.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/— expect all files to pass (66 files, 2477 tests here)../mvnw --batch-mode --no-transfer-progress -pl core -am test, while a changed POM,.mvn/**, or the wrapper runs the full reactor and build-only mode usestest-compile.-plrejection and check it returnstoolchain: "unsupported"naming the project, rather than a build failure attributed to the diff (reports unsupported when Maven rejects the selected project).,,:, or%cannot reach a-plselector and widens the run to the full reactor with a disclosure. These are directory names read off disk, so this gate is the only thing between them and ashell: truecommand line../mvnw testclaim to be settled by the scoped command actually run, but does not equate claims carrying different-Pprofiles or-plscopes.Evidence
The five cases pinning the new behaviour were checked by mutation: breaking
SELECTOR_REJECTED_RE, theshellSelectorcharacter gate, the POM-widens-scope branch, or the recursive report sweep each turns its test red, and restoring each turns it green again.packages/clireview suite: 66 files passed, 2477 tests passed, 6 skipped. Prettier and ESLint clean.tsc --noEmitreports no errors undersrc/commands/review/; the errors it does report are pre-existing and confined toserve/andacp-integration/(an unbuiltpackages/acp-bridge/distin a fresh worktree).Adapter behaviour is exercised through the injectable executor, so no test spawns Maven or touches the network.
Tested on
Risk & Scope
src/fixture tree,.mvn/**, the settings or repository locations.mvn/maven.confignames, or the executed wrapper). Over-attributing costs a visible failure carrying Maven's own output; under-attributing would ship the diff's own breakage as someone else's outage, so the carve-outs are deliberately narrow.verifyexecution, JaCoCo, mutation testing, Maven-aware base-sidetest-delta, and mixed-toolchain aggregation. Downstream (-amd) expansion is deliberately not used: it selects the whole reactor on exactly the repositories this targets, and a run that spends its whole deadline proves nothing — downstream coverage stays with the project's CI matrix.BuildTestReport.toolchainadds the"maven"discriminant, andCommandResultgains optionalinfrastructureandswallowedFailureflags. Existing npm behaviour and report fields are unchanged; roots where both npm and Maven apply fail closed instead of choosing one implicitly.Linked Issues
Replaces #8394, which is being split into #8776 and this PR.
中文说明
此 PR 的改动
在 toolchain 边界上注册 Maven adapter。
review build-test现在可以识别 Maven 根目录，把变更文件映射到所属模块，优先使用根 Maven Wrapper，从 reactor 根目录执行一条带-am上游扩展的 scopedtest（或 build-onlytest-compile），并输出仅限本次执行新建或更新的、带模块路径的 Surefire/Failsafe 证据。下游随之跟进：
test-plan可结算 Maven 命令与 Surefire 计数声明，Agent 7 增加 Maven 分支，Maven 失败不会进入仅支持 npm 的test-delta重跑路径。base-tree本版本跳过 Maven merge base（其 A/B 消费方仅支持 npm，构建 Maven base 有成本无消费方）；待 Maven delta 归因存在后放开该闸门。为什么需要
原有的构建与测试路径能为 npm 项目产出确定性证据，但 Maven 项目只能交还给 agent 临时选择命令。模块归属、命令选择、结果解析、超时分类、失败归因全部是 agent 的判断 —— 这是 review 结论所能依赖的最薄弱的基础。
由
alibaba/fastjson2和alibaba/druid的实际使用驱动：两者都是真实的多模块 reactor，core 变更必须通过根 wrapper 验证，而嵌套模块、profile 激活模块和独立 POM 使得朴素建模并不安全。设计说明：reactor 由 Maven 说了算
本 adapter 不为 Maven reactor 建模，而是读回 Maven 的答案：
pom.xml目录，跳过src/树下的目录（那里的 POM 是 maven-invoker 或 archetype 测试数据）。<modules>继承下是否属于 reactor，由 Maven 拒绝-pl选择器来回答（Could not find the selected project in the reactor），而且它在编译任何东西之前就返回。这个拒绝会转成unsupported交接。早先的版本直接解析 POM —— 字面
<modules>递归、CDATA 与注释处理、按 Maven 自身的 artifactId 匹配来解析<parent>的relativePath、命名及被删除的 parent 文件，以及在其上的闭包计算。那是对下一条命令真实求值内容的第二套更弱的模型，而且恰恰在驱动这项工作的场景上最弱：Druid 和 Flink 中 profile 激活的模块，文本层解析无法求值激活条件，只能 fail closed。移除它删掉了约 670 行 adapter 源码和约 1360 行对应测试，并把 profile 激活的答案从近似换成了 Maven 自己的。docs/design/review-toolchain-adapters.md记录了这个决定。Reviewer Test Plan
验证方式
cd packages/cli && npx vitest run src/commands/review/，预期全部通过（此处为 66 个文件、2477 个测试）。./mvnw --batch-mode --no-transfer-progress -pl core -am test；而 POM、.mvn/**或 wrapper 变更运行完整 reactor，build-only 模式使用test-compile。-pl拒绝，检查它返回toolchain: "unsupported"并指明项目，而不是把失败归因给 diff（reports unsupported when Maven rejects the selected project）。,、:或%时不能进入-pl选择器，运行会扩大到完整 reactor 并加以说明。这些是从磁盘读到的目录名，此闸门是它们与shell: true命令行之间唯一的屏障。./mvnw test声明可以由实际执行的 scoped 命令结算，但带不同-Pprofile 或-plscope 的声明不会被视为同一次执行。证据
钉住新行为的 5 个用例经过变异验证：破坏
SELECTOR_REJECTED_RE、shellSelector的字符闸门、POM 扩大范围的分支，或递归报告遍历，各自对应的测试变红，恢复后变绿。packages/clireview 套件：66 个文件通过，2477 个测试通过，6 个跳过。Prettier 与 ESLint 通过。tsc --noEmit在src/commands/review/下无错误；其报告的错误均为既有问题且集中在serve/与acp-integration/（全新 worktree 中未构建的packages/acp-bridge/dist）。adapter 行为通过可注入的 executor 验证，没有任何测试会拉起 Maven 或触网。
风险与范围
src/fixture 树之外的任何 POM、.mvn/**、.mvn/maven.config指向的 settings 或仓库位置、以及实际执行的 wrapper）。多归因的代价是一条带 Maven 原始输出的可见失败；少归因则会把 diff 自己的破坏当成别人的故障，因此这些豁免被刻意收窄。verify执行、JaCoCo、变异测试、Maven 感知的 base 侧test-delta，以及多 toolchain 聚合。下游（-amd）扩展被刻意排除：在本工作针对的这些仓库上它会选中整个 reactor，而一次耗尽全部 deadline 的运行什么也证明不了 —— 下游覆盖仍由项目 CI 矩阵负责。BuildTestReport.toolchain增加"maven"判别值，CommandResult增加可选的infrastructure与swallowedFailure标志。现有 npm 行为与报告字段不变；npm 与 Maven 同时适用的根目录会 fail closed，而不是隐式二选一。