feat(review): prebuild the review worktree before any agent runs - #10423
feat(review): prebuild the review worktree before any agent runs#10423wenshao wants to merge 18 commits into
Conversation
) The worktree fetch-pr builds is a bare checkout, and an install plus the prerequisite builds does not fit inside an agent's tool budget: every probe that decided to run a test burned its budget on a doomed install and the round downgraded to a read-only audit. When the review workflow sets QWEN_REVIEW_PREBUILD=1, fetch-pr now runs Agent 7's own `build-test --install --build-only` on the worktree right after writing the plan - the same command, environment, sandbox policy and scoped closure, on the orchestrator's clock with a step-sized budget - and records the outcome in the fetch report (`dependencies`). The plan is rewritten with it before the session ledger is appended, so the run-epoch fence keys on the final write. Fail-open throughout: a prebuild that could not complete records a reason and Agent 7 installs and builds on its own path as before; without the variable the report is byte-for-byte unchanged; an empty diff skips it. Replaces the host-cache design of #10129: nothing is shared across jobs, nothing is keyed, nothing can go stale - npm's own sync check and the PR's own sources are the whole of the correctness argument.
github-actions
Bot
added
the
review/self-reported
|
|
main already sits 3706 bytes over the recorded baseline for qwen-code-pr-review.yml (allowance 4096), so the step comment shrinks to a pointer at lib/prebuild.ts, where the rationale lives anyway.
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — issue #10108 describes the bare review worktree (no Direction: aligned. This is the root-cause replacement for #10129's host-level dependency cache, whose review rounds converged on exactly this: pay the install once, outside every agent's budget, instead of building a shared, mutable, cross-job cache surface. It is internal review infrastructure — no product CHANGELOG signal applies, and none is needed. Size: 8 files, +642/−9. Production logic is 265 lines ( Approach: matches the shape of the fix I'd propose independently — reuse Agent 7's own Risk: no elevated risk signals — no high-risk-path matches. One note for reviewers: the prebuild executes the PR tree's Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，不是理论问题——issue #10108 描述了裸的评审 worktree（无 方向：对齐。这是 #10129 宿主级依赖缓存方案的根因替代——那几轮评审最终收敛到的正是这个方向：在所有 agent 预算之外只付一次安装，而不是建立一个共享、可变、跨 job 的缓存信任面。属于内部评审基础设施，不涉及产品 CHANGELOG 信号。 规模：8 个文件，+642/−9。生产逻辑 265 行（ 方案：与我独立会提出的修法一致——在编排器的时钟上复用 Agent 7 自己的 风险：无升级风险信号——未命中任何高风险路径。一点供评审者注意：prebuild 会在 agent 启动前执行 PR 树的 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers found. The implementation matches the approach this problem calls for, and I verified its load-bearing claims against the base code rather than the PR's word:
Style matches the surrounding review-lib code (dense rationale comments, colocated tests, kebab-case). No AGENTS.md violations, no over-abstraction — one focused module reusing three existing seams. Two non-blocking notes: the author's end-to-end evidence is a self-reported macOS run (the How the prebuild sits in the fetch flowsequenceDiagram
participant P1 as CI Run review step
participant P2 as fetch-pr
participant P3 as prebuild
participant P4 as build-test
participant P5 as review worktree
participant P6 as Agent 7
P1->>P2: run with QWEN_REVIEW_PREBUILD=1
P2->>P5: create worktree, capture diff
P2->>P2: write plan (fetch report)
P2->>P3: prebuildWorktree(plan, worktree)
P3->>P4: runBuildTest install, build-only, 30 min budget
P4->>P5: npm ci, then scoped build closure
P4-->>P3: report (ok, notBuilt, note)
P3-->>P2: dependencies (installed off npm marker, built)
P2->>P2: rewrite plan with dependencies
P2->>P2: session ledger records the final write
P2-->>P1: agents fan out
P6->>P5: install and build are no-ops, marker present
Files changed (8)
TestingThis is an unattended CI run, so no PR code was built or executed here — the evidence below is the PR's own CI, fetched via the API for the reviewed commit. The ubuntu unit suite and the no-AK integration tests were still in flight at fetch time; the finalize job updates the table when CI settles. The Not verified: the live prebuild on CI runners — it is unobservable before merge, and neither sandboxed lane can close that gap pre-merge ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查未发现阻塞问题。实现与这个问题所需的修法一致，关键论断均对照基线代码核实，而非采信 PR 自述：
风格与周边 review-lib 代码一致（稠密的理由注释、同置测试、kebab-case）。无 AGENTS.md 违规，无过度抽象——一个聚焦的模块复用三个既有接缝。 两点非阻塞备注：作者的端到端证据是自行报告的 macOS 运行（ 时序图与文件表见上（图中内容：CI 的 Run review 步骤带变量启动 fetch-pr；fetch-pr 建 worktree、抓 diff、写 plan，随后调用 prebuild；prebuild 以 30 分钟预算运行 build-test 的安装与定向构建闭包，把结果回传；fetch-pr 以结果重写 plan，并让会话账本记录最终写入；之后 agent 扇出，Agent 7 因标记已就位而空操作安装与构建）。 测试这是无人值守的 CI 运行，此处未构建或执行任何 PR 代码——以下证据是 PR 自己的 CI，通过 API 在受审提交上获取。抓取时 ubuntu 单测套件与 no-AK 集成测试仍在运行；finalize job 会在 CI 落定后更新表格。显示为 cancelled 的 未验证：CI runner 上的实际 prebuild——合入前不可观测，且两个沙箱通道在合入前都无法弥合该缺口（ CI 表格见上（英文部分）。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, focused change that reuses the right machinery and pins its contracts; the only gap is the live CI run nobody can observe before merge, and the design degrades to today's behavior if it disappoints. Stepping back: my independent proposal for #10108 was exactly this shape — move the install and closure build out of every agent's budget onto the orchestrator's clock, reusing Agent 7's own It also lands the lesson from #10129 correctly: five rounds on a host-level cache showed the shared mutable trust surface was the problem, and this removes the need for any cache at all — a review that runs for hours can afford four minutes of blocking prefix. Every edit in the diff serves the stated goal; the second commit is the workflow size ratchet, nothing else. What I'm not crediting: the author's end-to-end evidence is self-reported (macOS), and the claim that matters — prebuild completes within budget on the CI pool and probes start running tests before Agent 7 finishes — is only observable on the next automatic review after merge. That is named in the PR itself, the failure mode is bounded by the fail-open contract, and no sandboxed lane can exercise Verdict: approve. The ubuntu unit suite and the no-AK integration tests are still running on the reviewed commit, so approval is deferred until CI lands green on 中文说明置信度：4/5 —— 干净、聚焦的改动，复用了正确的机制并钉住了契约；唯一的缺口是合入前谁也观测不到的实际 CI 运行，而设计保证即便它不如预期也只是退化为今天的行为。 退一步看：我对 #10108 的独立修法正是这个形状——把安装与闭包构建从每个 agent 的预算里挪到编排器的时钟上，复用 Agent 7 自己的 它也正确地吸收了 #10129 的教训：五轮评审表明宿主级缓存的共享可变信任面才是问题本身，而本方案让任何缓存都不再必要——一跑数小时的评审完全付得起四分钟的阻塞前缀。diff 中每一处改动都服务于既定目标；第二个提交只是 workflow 大小棘轮，别无其他。 我不计入的：作者的端到端证据是自行报告的（macOS），而真正重要的论断——prebuild 在 CI 池上于预算内完成、probe 在 Agent 7 完成前就能跑测试——只能在合入后的下一次自动评审中观测。这一点 PR 自己就写明了，失败模式由失败开放契约兜底，且合入前没有任何沙箱通道能运行 结论：批准。受审提交上的 ubuntu 单测套件与 no-AK 集成测试仍在运行，故批准推迟到 CI 在该提交上变绿之后（见英文部分的推迟标记）。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
@qwen-code /takeover |
qwen-code-dev-bot
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 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 25882, 22118, 1794, 1690, 605, 4444, 639 passed; 56 passed — this review observed 25882, 22118, 1794, 1690, 605, 4444, 639 passed.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 25882, 22118, 1794, 1690, 605, 4444, 639 passed; 56 passed — this review observed 25882, 22118, 1794, 1690, 605, 4444, 639 passed。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| * when something hangs, and then it bounds the loss instead of the review's | ||
| * own deadline doing so. | ||
| */ | ||
| export const PREBUILD_BUDGET_S = 1800; |
There was a problem hiding this comment.
[Critical] The 1800s whole-call budget and 1200s per-command deadline sit inside fetch-pr, but the only environment that enables this feature runs the SKILL Step 1 fetch-pr command through the agent shell tool, whose default foreground timeout is 120s and whose per-call ceiling is 600s (DEFAULT_FOREGROUND_TIMEOUT_MS = 120000 and the max-timeout refusal in packages/core/src/tools/shell.ts). Nothing in this diff, in the workflow, or in the SKILL invocation welds a covering timeout onto that call — agent-prompt.ts welds timeout: 600000 onto Agent 7's build-test for exactly this reason, but nothing equivalent covers fetch-pr, and no committed settings write tools.shell.defaultTimeoutMs. On essentially every CI review with QWEN_REVIEW_PREBUILD=1 the shell tool therefore kills fetch-pr at 120s mid-npm ci (this PR measures install+build at ~4-5 minutes): the plan rewrite and the session-ledger append never happen, a partial marker-less node_modules is left behind, and the skill's "if the command fails, inform the user and stop" rule stops the review. Even a spontaneous retry at the 600000ms ceiling stays a third of PREBUILD_BUDGET_S, so the documented budget cannot exist in the only environment that enables the feature.
Witness (probe over the real sources at the reviewed commit):
PREBUILD_BUDGET_S: 1800 shell_default_ms: 120000
PREBUILD_COMMAND_TIMEOUT_S: 1200 shell_max_per_call_ms: 600000
budget_fits_default: false budget_fits_max_per_call: false
fetchpr_block_welds_timeout: false workflow_welds_defaultTimeoutMs: false
cover_exists: false → flips to true after welding a covering defaultTimeoutMs
Fix: weld the cover where the opt-in is welded — write tools.shell.defaultTimeoutMs (at least the budget) into the per-run QWEN_HOME settings in the same workflow step (even the 600s per-call ceiling cannot carry the 1800s budget), or run the prebuild as its own workflow shell step, which is what "sized to a workflow step" literally requires.
中文说明
1800 秒的整体预算与 1200 秒的单命令期限都嵌在 fetch-pr 内部，但唯一启用该功能的环境是通过 agent 的 shell 工具来运行 SKILL 第 1 步的 fetch-pr 命令的：该工具的默认前台超时是 120 秒，单次调用上限是 600 秒（packages/core/src/tools/shell.ts 中的 DEFAULT_FOREGROUND_TIMEOUT_MS = 120000 与最大超时拒绝逻辑）。本 diff、workflow、SKILL 调用中都没有任何地方为该调用接上覆盖性的超时——agent-prompt.ts 正是出于这个原因才给 Agent 7 的 build-test 接上 timeout: 600000，但 fetch-pr 没有对等的保护，也没有任何已提交的配置写入 tools.shell.defaultTimeoutMs。因此几乎每一次设置了 QWEN_REVIEW_PREBUILD=1 的 CI 评审中，shell 工具都会在 120 秒时于 npm ci 进行到一半时杀掉 fetch-pr（本 PR 实测安装+构建约 4-5 分钟）：plan 重写与会话账本追加都不会发生，留下一个没有完整性标记的半成品 node_modules，而 skill 的「命令失败则告知用户并停止」规则会让评审直接停摆。即使模型自发以 600000ms 上限重试，也只有 PREBUILD_BUDGET_S 的三分之一，所以文档承诺的预算在唯一启用该功能的环境里根本不存在。
修复：把覆盖性配置接在开关所在的同一处——在同一个 workflow 步骤里向每次运行的 QWEN_HOME 设置写入 tools.shell.defaultTimeoutMs（至少等于预算；600 秒的单次调用上限装不下 1800 秒的预算），或者把 prebuild 移成独立的 workflow shell 步骤——这正是「按 workflow 步骤的尺寸来定预算」的字面要求。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| * it so the fetch report and Agent 7 can never disagree about whether the | ||
| * tree needs an install. | ||
| */ | ||
| const NPM_COMPLETENESS_MARKER = join('node_modules', '.package-lock.json'); |
There was a problem hiding this comment.
[Suggestion] This constant re-implements the unexported installComplete() closure at npm-toolchain.ts:741-742 (existsSync(join(root, 'node_modules', '.package-lock.json'))) instead of exporting and reusing it, and nothing pins the two copies together. The gate has already changed once (commit 7c1a93477c: bare node_modules directory → the npm marker). If Agent 7's gate changes again, prebuildWorktree keeps answering off the old check, and the report's installed: true silently stops meaning "Agent 7's install is a no-op" — exactly the disagreement the doc comment above rules out — visible only as flaky agent behaviour mid-review, and caught by neither test suite (prebuild.test.ts hardcodes its own literal fixture). Export the predicate from its owner and consume it here:
// npm-toolchain.ts
export function npmInstallComplete(root: string): boolean {
return existsSync(join(root, 'node_modules', '.package-lock.json'));
}
// prebuild.ts (replacing NPM_COMPLETENESS_MARKER)
const installed = npmInstallComplete(args.worktree);If this lands, prebuild.test.ts's "reads installed off npm's marker" case exercises the shared predicate — mutating the gate in npm-toolchain.ts must then turn it red (today, mutating either side's literal leaves the other test green); please add that mutation check.
中文说明
该常量重新实现了 npm-toolchain.ts:741-742 中未导出的 installComplete() 闭包（existsSync(join(root, 'node_modules', '.package-lock.json'))），而不是将其导出复用；两份拷贝之间也没有任何机制互相钉住。这道闸已经改过一次（提交 7c1a93477c：从「存在 node_modules 目录」改为 npm 标记文件）。如果 Agent 7 的闸再改一次，prebuildWorktree 会继续按旧的检查作答，报告里的 installed: true 就会悄悄不再意味着「Agent 7 的安装阶段是空操作」——正是上方文档注释声称绝不会出现的那种分歧——而且只会表现为评审中途 agent 的飘忽行为，两个测试套件都抓不到（prebuild.test.ts 硬编码了自己的字面量 fixture）。建议从属主模块导出该谓词并在此处调用（见上方代码块）。
如果采纳，prebuild.test.ts 的「installed 读自 npm 标记」用例就会走到共享谓词上——此时对 npm-toolchain.ts 中闸的变异必须让该用例变红（今天变异任何一侧的字面量，另一侧的测试仍是绿的）；请补上这个变异检查。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // A build the budget truncated is not a compiled closure: a probe against | ||
| // packages that were never built manufactures resolution failures that | ||
| // read as defects in the diff — the same rule `base-tree` applies. | ||
| const built = |
There was a problem hiding this comment.
[Suggestion] built evaluates true for report shapes where nothing was compiled: the unsupported toolchain hand-off (unsupportedReport in npm-toolchain carries ok: true and no notBuilt) and an empty npm build closure (affected.length === 0 → ok: true, build: [] — the docs-only shape the review workflow dispatches at medium effort). The comment two lines up claims "the same rule base-tree applies", but base-tree.ts:294-313 additionally refuses on toolchain !== 'npm' and build.length === 0. A docs-only PR on CI then gets dependencies: {installed: true, built: true} over a tree with zero compiled outputs, and the SKILL.md text this PR adds tells readers that built: true means the closure is compiled and a probe can run a test before Agent 7 finishes — a probe trusting it hits missing sibling dist outputs, manufacturing exactly the resolution failures the module's own comment warns "read as defects in the diff". Align with base-tree (or, if the intent is the weaker "nothing left unbuilt" semantics, fix the comment, the docstring, and the SKILL text instead, since all three currently claim base-tree's rule):
const built =
report !== null &&
report.ok &&
report.toolchain === 'npm' &&
report.build.length > 0 &&
(report.notBuilt?.length ?? 0) === 0;If the stricter predicate lands, add prebuild.test.ts cases: run returning {...green, toolchain: 'unsupported'} must yield built: false, and an npm report with build: [] must yield built: false; removing either new condition must red the corresponding test.
中文说明
built 在某些「什么都没编译」的报告形状下会得出 true：unsupported 工具链的移交（npm-toolchain 的 unsupportedReport 带 ok: true 且没有 notBuilt），以及空的 npm 构建闭包（affected.length === 0 → ok: true, build: []——评审 workflow 会以 medium 力度派发纯文档 PR，正是这个形状）。上方两行的注释声称「与 base-tree 应用同一规则」，但 base-tree.ts:294-313 还额外拒绝了 toolchain !== 'npm' 与 build.length === 0。于是 CI 上的纯文档 PR 会得到一棵零编译产物却 dependencies: {installed: true, built: true} 的树，而本 PR 新增的 SKILL.md 文本告诉读者 built: true 意味着闭包已编译、probe 可以在 Agent 7 完成前跑测试——信任它的 probe 会撞上缺失的兄弟 dist 产物，恰好制造出本模块自己注释里警告的「读起来像 diff 里缺陷的解析失败」。建议与 base-tree 对齐（见上方代码块）；如果作者本意是较弱的「没有剩余未构建项」语义，那就改注释、docstring 和 SKILL 文本——三者目前都声称是 base-tree 的规则。
如果采纳更严的谓词，请补 prebuild.test.ts 用例：run 返回 {...green, toolchain: 'unsupported'} 时 built 必须为 false；npm 报告 build: [] 时 built 必须为 false；删掉任一新增条件都必须让相应用例变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| * agent starts and outside every agent's budget. `installed: true` means | ||
| * the tree holds a complete `node_modules` (npm's own marker, the gate | ||
| * Agent 7 reads), `built: true` that the scoped build closure compiled too, | ||
| * so a probe can run a test before Agent 7 finishes and Agent 7's install |
There was a problem hiding this comment.
[Suggestion] "Agent 7's install and build are no-ops" (this doc comment, the stderr success message below it, and the dependencies field text this PR adds to SKILL.md) only reproduces for the install half. npm-toolchain.ts's build loop has no freshness gate for existing dist, and scripts/build_package.js does rmSync('dist') + rmSync('tsconfig.tsbuildinfo') before tsc --build, so every npm run build --workspace=… is a full non-incremental recompile that deletes what the prebuild produced; Agent 7's welded first call also writes a different --out than the prebuild report, so no resume bridges the two. Every CI review therefore compiles the scoped closure twice, and the second compile lands inside Agent 7's ~570s whole-call budget — the exact budget this PR exists to relieve — competing with the test suites. At minimum, scope the claim to the install half; to actually realize the win, add a dist-freshness gate in npm-toolchain's build loop above the per-package build script (build_package.js pre-cleans, so the gate must sit above it).
中文说明
「Agent 7 的安装与构建都是空操作」（此文档注释、下方的 stderr 成功消息、以及本 PR 给 SKILL.md 新增的 dependencies 字段文本）只对安装这一半成立。npm-toolchain.ts 的构建循环没有针对已存在 dist 的新鲜度闸，而 scripts/build_package.js 在 tsc --build 之前会 rmSync('dist') + rmSync('tsconfig.tsbuildinfo')，所以每个 npm run build --workspace=… 都是一次完整的全量重编译，会把 prebuild 的产物删掉重造；Agent 7 被固定下来的首次调用写入的 --out 也与 prebuild 报告不同，因此 resume 也无法把两次调用接起来。于是每次 CI 评审都会把定向闭包编译两遍，第二遍还落在 Agent 7 约 570 秒的整体调用预算内——正是本 PR 要缓解的那个预算——与测试套件互相挤占。至少请把该论断限定到安装这一半；要真正拿到收益，需要在 npm-toolchain 的构建循环里、在按包构建脚本之上加一道 dist 新鲜度闸（build_package.js 会预先清理，所以闸必须位于它之上）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Partially addressed in a44944669c, remainder deferred. Fixed (the "at minimum" half): the overclaim is scoped to the install half everywhere it appeared — the dependencies docstring in fetch-pr.ts, the stderr success line, prebuild.ts's module docstring, and the SKILL.md field text now say Agent 7's install is a no-op and its build recompiles because the per-package build script pre-cleans dist; a SKILL.test.ts assertion pins the scoped claim and forbids the old wording. Deferred to the follow-up queue: the dist-freshness gate in npm-toolchain's build loop. The double compile is verified real (this round confirmed build_package.js pre-cleans dist/tsconfig.tsbuildinfo and the build loop has no gate), but the gate is a behavior change to build-test's shared machinery: it must not skip a compile the review relies on as its one deterministic signal (a stale-dist false green), and it must reconcile its skipped-build report shape with the corrected built predicate landed this round (which refuses build.length === 0), so it gets its own design and its own tests rather than riding this PR.
中文说明
已在 a44944669c 中部分处理，其余延期。已修复（「至少」那一半）： 过度论断已在所有出现处限定到安装这一半——fetch-pr.ts 的 dependencies 文档注释、stderr 成功行、prebuild.ts 的模块 docstring 以及 SKILL.md 的字段文本，现在都说明 Agent 7 的安装是空操作、其构建会重编译，因为按包构建脚本会预先清理 dist；SKILL.test.ts 新增断言钉住限定后的论断并禁止旧措辞。延期到后续队列： npm-toolchain 构建循环里的 dist 新鲜度闸。双份编译已核实属实（本轮确认 build_package.js 会预先清理 dist/tsconfig.tsbuildinfo，且构建循环没有闸），但该闸是对 build-test 共享机制的行为变更：它不能跳过评审赖以作为唯一确定性信号的编译（陈旧 dist 造成的误绿），还必须与本轮落地的修正后 built 谓词（拒绝 build.length === 0）协调其「跳过构建」的报告形状——因此它应有自己的设计与测试，而不是搭载本 PR。
| const took = `${Math.round(deps.durationMs / 1000)}s`; | ||
| writeStderrLine( | ||
| deps.installed && deps.built | ||
| ? `Prebuilt the worktree in ${took}: dependencies installed and the ` + |
There was a problem hiding this comment.
[Suggestion] Both branches of this stderr disclosure are untested — no test anywhere asserts a prebuild writeStderrLine message, and all four wiring tests drive the gate with a fully-green DEPS and never inspect stderr. Inverting the ternary's condition (or && → ||) leaves the whole suite green while CI logs Prebuild did not complete in 1s (installed: true, built: true…) for a fully successful prebuild — sending an operator to diagnose a nonexistent install failure — or, inverted the other way, announces success over an incomplete tree. The took rounding and the failure-branch-only note suffix are equally unpinned. Add two wiring cases in the prebuild (issue 10108) describe: one asserting producerMocks.writeStderrLine received the success line for the green DEPS, and one where prebuildWorktree returns a failing shape ({installed: false, built: false, note: 'npm ci exited 1', …}) asserting the incomplete line including the note suffix. The new assertions are the witness: inverting the ternary condition or dropping the note suffix must turn them red — please confirm by mutation.
中文说明
这条 stderr 披露的两个分支都没有测试——没有任何测试断言过 prebuild 的 writeStderrLine 消息，四个接线测试都用全绿的 DEPS 驱动门控、且从不检查 stderr。把三元条件取反（或把 && 改成 ||）整个套件仍然全绿，而 CI 日志会对一次完全成功的 prebuild 输出 Prebuild did not complete in 1s (installed: true, built: true…)——让运维去排查一个并不存在的安装失败；反方向取反则会对一棵不完整的树宣布成功。took 的四舍五入与只在失败分支出现的 note 后缀同样没有被钉住。请在 prebuild (issue 10108) describe 里补两个接线用例：一个断言绿 DEPS 时 producerMocks.writeStderrLine 收到了成功行；另一个让 prebuildWorktree 返回失败形状（{installed: false, built: false, note: 'npm ci exited 1', …}），断言含 note 后缀的未完成行。新断言本身就是验收标准：取反三元条件或去掉 note 后缀都必须让它们变红——请以变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // throwing — and absent from the report entirely when not asked for, | ||
| // so every local review reads the plan it always did. | ||
| if (prebuildRequested() && !emptyDiff) { | ||
| result.dependencies = prebuildWorktree({ |
There was a problem hiding this comment.
[Suggestion] The prebuild is a new blocking prefix of up to 30 minutes that emits zero output until it completes: runBuildTest executes commands via spawnSync with captured stdio, its only live writes sit in its CLI handler (which the programmatic path does not go through), and fetch-pr's single prebuild line is written after the call returns. If npm ci stalls (registry/proxy degradation — the cost class this module exists for) and the run is killed mid-prebuild (job cancellation, the outer GNU timeout, a host fault), the Run review log ends at the plan write with nothing distinguishing "inside npm ci/build" from any other stall, and the oncall has to locate it by reading source. Emit one stderr line before the call so the start is visible regardless of how the call ends (import PREBUILD_ENV/PREBUILD_BUDGET_S from ./lib/prebuild.js):
writeStderrLine(
`Prebuilding the worktree via build-test (${PREBUILD_ENV}=1, ` +
`budget ${PREBUILD_BUDGET_S}s)...`,
);
result.dependencies = prebuildWorktree({The companion test asserts a "Prebuilding" stderr line whose invocationCallOrder precedes the prebuildWorktree call; removing the new line must red it.
中文说明
prebuild 是一个最长 30 分钟、完成前零输出的新阻塞前缀：runBuildTest 通过 spawnSync 执行命令并捕获 stdio，它唯一的实时输出在其 CLI handler 里（程序化调用路径不经过那里），而 fetch-pr 唯一一条 prebuild 日志在调用返回之后才写。如果 npm ci 卡住（注册表/代理退化——正是本模块要兜住的那类成本），而进程在 prebuild 中途被杀（job 取消、外层 GNU timeout、宿主机故障），Run review 日志会停在 plan 写入处，无法把「正在 npm ci/build」与其他任何卡死区分开，值班人只能靠读源码定位。请在调用前先输出一条 stderr 日志，使无论调用如何结束都能看到开始时刻（从 ./lib/prebuild.js 导入 PREBUILD_ENV/PREBUILD_BUDGET_S，见上方代码块）。配套测试断言存在一条 "Prebuilding" stderr 日志，且其 invocationCallOrder 早于 prebuildWorktree 调用；删掉这行新代码必须让测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…10423) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10423One commit: Feedback points and dispositions[rc:3883870678] [Critical] The prebuild budget cannot exist in the only environment that enables it — FIXED. [rc:3883870683] [Suggestion] Duplicated npm completeness gate — FIXED. [rc:3883870690] [Suggestion] [rc:3883870698] [Suggestion] "Agent 7's install and build are no-ops" overclaims the build half — claim FIXED, freshness gate DEFERRED. [rc:3883870703] [Suggestion] Both stderr branches untested — FIXED. [rc:3883870708] [Suggestion] Up-to-30-minute blocking prefix emits zero output — FIXED. [rv:5054888801] [CHANGES_REQUESTED] review body — informational only: its own text marks everything "not a blocker" (an actionlint embedded-shell source-mapping limitation, and a passing Test Plan). The substance of the round is the six inline findings above. Verification
No conflict resolution was needed ( 中文说明Autofix 评审回合 — PR #10423一次提交： 反馈点与处理结果[rc:3883870678] [Critical] prebuild 预算在唯一启用它的环境里根本不存在 — 已修复。 [rc:3883870683] [Suggestion] npm 完整性闸的重复实现 — 已修复。 [rc:3883870690] [Suggestion] [rc:3883870698] [Suggestion] 「Agent 7 的安装与构建都是空操作」对构建这一半过度声称 — 论断已修复，新鲜度闸已延期。 [rc:3883870703] [Suggestion] stderr 两个分支都没有测试 — 已修复。 [rc:3883870708] [Suggestion] 最长 30 分钟的阻塞前缀零输出 — 已修复。 [rv:5054888801] [CHANGES_REQUESTED] 评审正文 — 仅信息性：其自身文本把一切都标为「非阻断」（actionlint 嵌入 shell 源映射的工具限制，以及通过的 Test Plan）。本轮的实质内容是上面六条行内发现。 验证
无需冲突解决（ 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.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 25884, 22117, 1794, 1690, 605, 4444, 639 passed; 56 passed — this review observed 25884, 22117, 1794, 1690, 605, 4444, 639 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-pr-review-workflow.test.js:2307 — [test] new prebuild contract tests sit outside every npm workspace — the scoped pipeline never collects them (mitigated by the root test:scripts lane; 182/182 passed when run directly)
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 25884, 22117, 1794, 1690, 605, 4444, 639 passed; 56 passed — this review observed 25884, 22117, 1794, 1690, 605, 4444, 639 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| printf '%s\n' '{"tools":{"shell":{"defaultTimeoutMs":1800000}}}' \ | ||
| > "$QWEN_HOME/settings.json" |
There was a problem hiding this comment.
[Critical] R2-1: [fails-closed] [new-surface] The cover written here is exactly PREBUILD_BUDGET_S × 1000 (1800000 ms), with zero headroom for the fetch work that precedes the prebuild inside the same fetch-pr call — PR-ref fetch, gh metadata, merge-base/base fetch, diff capture, plan write — which is strictly > 0. The shell-tool timer starts when the fetch-pr command spawns; the prebuild's 1800 s budget clock starts only later, inside the call, when runBuildTest starts. A prebuild that saturates its own budget — precisely the hang case the budget exists for — is therefore killed by the shell timeout before prebuildWorktree returns its fail-open outcome: no dependencies/note lands in the plan, no session-ledger entry is appended, and the skill's fetch-failure stop kills the review outright instead of degrading to Agent 7's own path — the exact issue #10108 shape recurring on precisely the tail the budget exists to bound. The adjacent comment's claim that the prebuild's own budget still bounds a hang from inside holds only when the cover exceeds prefix + budget; at equality it is false. Normal ~4-5 minute runs are unaffected.
Witness:
cover_ms: 1800000 budget_ms: 1800*1000 headroom_ms: 0
workflow prebuild test block: 3 passed (toBeGreaterThanOrEqual(budgetS * 1000) accepts equality at exactly 1800000)
shell.test.ts -t "foreground timeout resolution": 5 passed (per-call > settings default > built-in; the resolved default arms AbortSignal.timeout at call start)
witness: not run for the end-to-end kill — needs live gh auth/network, the full CI agent stack, and a 30-minute hang; not constructible here
Fix: give the cover explicit headroom over the budget, and update the comment's PREBUILD_BUDGET_S x 1000 statement to match:
printf '%s\n' '{"tools":{"shell":{"defaultTimeoutMs":2400000}}}' \
> "$QWEN_HOME/settings.json"The raised value must stay at least PREBUILD_BUDGET_S × 1000 — export const PREBUILD_BUDGET_S = 1800; in packages/cli/src/commands/review/lib/prebuild.ts — and the workflow test must keep reading that constant out of the source.
Fix witness: tightening covers the prebuild call with a session shell timeout carrying the budget in scripts/tests/qwen-pr-review-workflow.test.js to require headroom strictly beyond budgetS * 1000 must go red if the headroom is removed again — please remove the headroom and confirm the test reds.
中文说明
此处写入的覆盖值恰好等于 PREBUILD_BUDGET_S × 1000（1800000 ms），对同一次 fetch-pr 调用中 prebuild 之前的抓取前置工作（PR 引用抓取、gh 元数据、merge-base/基线抓取、diff 捕获、plan 写入——严格大于 0）没有任何余量。shell 工具的计时从 fetch-pr 命令启动时开始，而 prebuild 的 1800 秒预算时钟要等到调用内部 runBuildTest 启动时才开始。因此，一个耗尽自身预算的 prebuild——正是预算存在所要兜住的挂死场景——会在 prebuildWorktree 返回其失败开放结果之前被 shell 超时杀掉：plan 里没有 dependencies/note，会话账本没有追加条目，skill 的「抓取失败即停止」规则会让评审直接停摆，而不是退化到 Agent 7 自己的路径——这正是 #10108 的形态在预算本要兜住的尾部场景上重演。相邻注释里「prebuild 自身的预算仍从内部约束挂死」的说法只有在覆盖值大于「前置 + 预算」时才成立；相等时为假。正常的 4-5 分钟运行不受影响。
修复：给覆盖值留出明确的余量（例如 (PREBUILD_BUDGET_S + 600) × 1000 = 2400000），并把注释中 PREBUILD_BUDGET_S x 1000 的表述同步更新。抬高后的数值必须仍不低于 PREBUILD_BUDGET_S × 1000（常量定义于 packages/cli/src/commands/review/lib/prebuild.ts），且 workflow 测试继续从源码读取该常量。
验收测试：把 scripts/tests/qwen-pr-review-workflow.test.js 中的 covers the prebuild call with a session shell timeout carrying the budget 收紧为要求余量严格大于 budgetS * 1000；移除余量后该测试必须变红——请删除余量并确认测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # Install and build the review worktree BEFORE any agent starts, | ||
| # through Agent 7's own build-test (issue #10108). Opt-in here only; | ||
| # rationale, budget and fail-open contract: cli lib/prebuild.ts. | ||
| QWEN_REVIEW_PREBUILD: '1' |
There was a problem hiding this comment.
[Critical] R2-2: [fails-closed] [new-surface] This opt-in is welded unconditionally, but the prebuild's fixed 1800 s budget is never reconciled with the attempt budget, which an explicit @qwen-code /review --timeout=N (or dispatch input) can set as low as 6 minutes — the step rejects only N <= 5 and N > MAX_TIMEOUT_MINUTES, and an explicit value skips the size-aware default. The outer GNU timeout --kill-after=10s "${attempt_timeout}s" is the only clock (the deadline gate is a round-admission gate and never kills fetch-pr). With a 6-10 minute budget the fetch prefix (~1-2 min) plus the prebuild (measured ~4-5 min on this repo) overruns the attempt: the GNU timeout kills the whole agent mid-npm ci, OUTCOME='timeout' is explicitly never retried (only retryable is), the run posts the fallback timeout comment, and no review ever happened. Before this diff the same budget went entirely to the review; the prebuild's fail-open contract cannot hold when the enclosing attempt is killed before any agent starts. Affected band: explicit budgets under roughly 15-20 minutes (auto reviews floor at 90).
Witness:
the step accepts explicit N >= 6; TIMEOUT_EXPLICIT=true skips the size-aware default
the only outer clock is GNU timeout ... "${attempt_timeout}s"; the deadline gate is round-admission only
OUTCOME='timeout' (exit 124/137) routes to fail without retry — the retry branch fires only for 'retryable'
prefix ~1-2 min + prebuild measured ~4-5 min >= 360 s for every accepted N <= 7
witness: not run for a real --timeout=6 CI run — stubbing the full agent/skill/fetch-pr stack to a faithful clock model was beyond this shard
Fix: after EFFECTIVE_TIMEOUT_MINUTES is computed (and after the halving/flooring), when QWEN_REVIEW_PREBUILD=1 but the effective budget cannot carry PREBUILD_BUDGET_S × 1000 plus margin for the fetch prefix and the review itself, unset/blank QWEN_REVIEW_PREBUILD and skip the settings.json cover write — both already sit after that point in the script — so the run degrades to the pre-prebuild flow instead of dying in it. The step accepts explicit timeouts down to 6 minutes (Run review timeout validation), and the env weld and the cover write both sit after EFFECTIVE_TIMEOUT_MINUTES is finalized, so a gate there sees the final budget.
Fix witness: an assertion in scripts/tests/qwen-pr-review-workflow.test.js (describe review worktree prebuild (issue #10108)) that the Run review script contains a gate comparing the effective budget against the prebuild budget and clearing the opt-in must go red when the gate is removed — please remove it and confirm.
中文说明
该开关被无条件焊死，但 prebuild 固定的 1800 秒预算从未与整个尝试的预算对账：显式的 @qwen-code /review --timeout=N（或 dispatch 输入）可以把尝试预算压低到 6 分钟——步骤只拒绝 N <= 5 与 N > MAX_TIMEOUT_MINUTES，且显式值会跳过按规模定预算的默认逻辑。外层 GNU timeout 是唯一的时钟（deadline 闸门只做轮次准入，从不杀 fetch-pr）。在 6-10 分钟的预算下，抓取前置（约 1-2 分钟）加上 prebuild（本仓库实测约 4-5 分钟）会超出尝试预算：GNU timeout 在 npm ci 进行到一半时杀掉整个 agent，OUTCOME='timeout' 明确不重试（只有 retryable 会），运行发出兜底的超时评论，评审根本没有发生。本 diff 之前，同样的预算全部用于评审本身；当外层尝试在任何 agent 启动之前就被杀掉时，prebuild 的失败开放契约无从成立。受影响区间：大约 15-20 分钟以下的显式预算（自动评审有 90 分钟下限）。
修复：在 EFFECTIVE_TIMEOUT_MINUTES 计算完成（含减半/下限处理）之后，若 QWEN_REVIEW_PREBUILD=1 但有效预算装不下 PREBUILD_BUDGET_S × 1000 加上前置与评审本身的余量，则清空 QWEN_REVIEW_PREBUILD 并跳过 settings.json 覆盖写入——两者在脚本中都位于该点之后——使运行退化到 prebuild 之前的流程，而不是死在其中。
验收测试：在 scripts/tests/qwen-pr-review-workflow.test.js 的 review worktree prebuild (issue #10108) 中新增断言，要求 Run review 脚本包含一个把有效预算与 prebuild 预算比较并清空开关的闸门；移除该闸门后测试必须变红——请移除并确认。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const raw = env[PREBUILD_ENV]?.trim().toLowerCase(); | ||
| return (raw === '1' || raw === 'true') && !fileSourced(PREBUILD_ENV); |
There was a problem hiding this comment.
[Critical] R2-3: [fails-closed] [new-surface] The documented local opt-in (Set to 1 (or true) to run the prebuild) deterministically kills the review outside CI: the shell-tool cover that carries the 1800 s budget is welded only in the workflow, and locally the session default is the built-in 120000 ms with a 600000 ms per-call ceiling — both below this repo's measured ~4-5 minute prebuild cost. An operator who exports QWEN_REVIEW_PREBUILD=1 locally engages the prebuild (a real process variable, not file-sourced), the skill runs fetch-pr through the agent shell tool, and the tool kills the entire process at 120 s (or 600 s with a max per-call timeout — still under budget) mid-npm ci. The skill treats a failed fetch as fatal ('inform the user and stop'), so the review stops over a partial tree and the module's fail-open contract is void — the process is dead, nothing proceeds. The trap is repo-size dependent (repos whose install+build fits under the ceiling survive), which makes the failure look environmental when it is structural.
Witness:
shell.test.ts -t "foreground timeout resolution" — 5 passed, incl. toHaveBeenCalledWith(120_000) for the neither-param-nor-setting arm
per-call ceiling: 'Timeout cannot exceed 600000ms' (shell.ts)
PREBUILD_COMMAND_TIMEOUT_S = 1200, PREBUILD_BUDGET_S = 1800 (prebuild.ts); install alone measured ~161s on this repo (SKILL.md)
witness: not run for a live local review with the env var — needs model API access this shard does not have
Fix: make prebuildRequested (or the fetch-pr call site) decline when the session cannot carry the budget — fetch-pr already reaches operator settings in the same result assembly, so read tools.shell.defaultTimeoutMs and return false with a stderr note that the prebuild needs the CI cover when it is unset or below PREBUILD_BUDGET_S * 1000. The shell tool's precedence is per-call timeout, then tools.shell.defaultTimeoutMs, then the 120000 ms built-in (packages/core/src/tools/shell.ts), and the per-call param is hard-capped at 600000 ms there — the session default is the only knob that can carry the budget.
Fix witness: a new case in prebuild.test.ts asserting the prebuild declines when the session shell default timeout is unset or below the budget (inject the setting the way fileSourced is injected) must go red when the guard is removed — please remove it and confirm.
中文说明
文档化的本地开关（Set to 1 (or true) to run the prebuild）在 CI 之外会确定性地杀死评审：承载 1800 秒预算的 shell 工具覆盖值只焊在 workflow 里，本地会话默认是内置的 120000 ms、单次调用上限 600000 ms——都低于本仓库实测 4-5 分钟的 prebuild 成本。本地 export QWEN_REVIEW_PREBUILD=1 的运维人员会启用 prebuild（真实进程变量，非文件来源），skill 通过 agent 的 shell 工具运行 fetch-pr，工具会在 120 秒（或带上单次调用上限的 600 秒——仍低于预算）时于 npm ci 进行到一半杀掉整个进程。skill 把抓取失败视为致命错误（「告知用户并停止」），评审停在半成品树上，模块的失败开放契约失效——进程已死，无法继续。该陷阱与仓库规模相关（安装+构建能塞进上限的仓库可以幸存），使结构性失败看起来像环境问题。
修复：让 prebuildRequested（或 fetch-pr 调用点）在会话装不下预算时拒绝——fetch-pr 在同一结果组装中已经能读到运维设置，读取 tools.shell.defaultTimeoutMs，未设置或低于 PREBUILD_BUDGET_S * 1000 时返回 false 并输出一条说明 prebuild 需要 CI 覆盖的 stderr 日志。shell 工具的优先级为单次调用超时 → tools.shell.defaultTimeoutMs → 120000 ms 内置值（packages/core/src/tools/shell.ts），单次调用参数上限被硬性限定在 600000 ms——会话默认值是唯一能承载该预算的旋钮。
验收测试：在 prebuild.test.ts 新增用例，断言会话 shell 默认超时未设置或低于预算时 prebuild 被拒绝（按 fileSourced 的注入方式注入该设置）；移除该守卫后测试必须变红——请移除并确认。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const built = | ||
| report !== null && | ||
| report.ok && | ||
| report.toolchain === 'npm' && | ||
| report.build.length > 0 && | ||
| (report.notBuilt?.length ?? 0) === 0; |
There was a problem hiding this comment.
[Suggestion] R1-3: (fix-induced) The round-1 fix aligned built with base-tree's availability rule by inlining it — this predicate is now a second inline copy of the rule in base-tree.ts (the notBuilt refusal plus the !ok || toolchain !== 'npm' || build.length === 0 gate), and the code's own comment declares it 'The same rule base-tree applies'. build-test has already grown three report shapes that return ok: true without a compiled tree (unsupported hand-off, empty npm scope, notBuilt budget truncation), and each one required gate updates at every consumer: when the next shape lands (or a clause changes), a maintainer updates one site and not the other, dependencies.built says true for a tree shape base-tree declares un-runnable, and pre-Agent-7 probes are pointed at packages that were never compiled — manufacturing resolution failures that read as defects in the diff, with nothing comparing the two consumers, which disagree silently. Extract one predicate next to the report type it judges (e.g. isCompiledClosure in build-test.ts) and call it from both prebuildWorktree and base-tree's availability gates.
Witness:
witness: not run for the divergence itself (static duplication; settled by reading both sites and grepping build-test.ts exports — zero shared-predicate matches)
premise run: prebuild.test.ts 11 passed, including the three ok:true non-compiled shapes
Fix witness: the three shape cases in prebuild.test.ts ('is not built when the budget cut the closure short', '…when build-test handed the repo to the brief', '…when the npm scope had nothing to compile') plus base-tree.test.ts's notBuilt case must stay red against any regression of the extracted helper.
中文说明
第 1 轮的修复通过内联的方式把 built 与 base-tree 的可用性规则对齐——该谓词现在是 base-tree.ts 规则（notBuilt 拒绝 + !ok || toolchain !== 'npm' || build.length === 0 闸门）的第二份内联拷贝，代码自己的注释也写明「与 base-tree 应用的规则相同」。build-test 已经出现过三种 ok: true 却没有编译产物的报告形状（unsupported 移交、空 npm 构建范围、notBuilt 预算截断），每一种都要求所有消费方同步更新闸门：下一种形状出现（或某个条件变化）时，维护者只更新一处而漏掉另一处，dependencies.built 就会对 base-tree 判定为不可运行的树形状返回 true，pre-Agent-7 的 probe 会被指向从未编译的包——制造出读起来像 diff 缺陷的解析失败，而两个消费方之间没有任何比对，会悄无声息地互相矛盾。建议在报告类型旁边导出一个共享谓词（如 build-test.ts 中的 isCompiledClosure），prebuildWorktree 与 base-tree 的可用性闸门都调用它。
验收测试：prebuild.test.ts 的三个形状用例与 base-tree.test.ts 的 notBuilt 用例，必须对提取出的共享助手的任何回归保持变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # The prebuild's own budget still bounds a hang from inside, and | ||
| # the GNU timeout below bounds the whole attempt. |
There was a problem hiding this comment.
[Suggestion] R2-4: The cover welded here for fetch-pr's prebuild raises the session shell default to 30 minutes for every shell call in the CI review session, not just the one it was welded for: QWEN_REVIEW_PREBUILD: '1' is hardcoded on the Run review step, shell-tool precedence is per-call timeout → settings default → built-in, and the skill's agent invocations carry no per-call timeout, so every foreground shell call that omits one — chunk-agent probes, verifier scratch-tree test runs — now inherits 1800000 ms instead of the 120 s built-in default. A hung command (deadlocked test, process blocked on stdin, stalled network fetch) occupies the agent up to 30 minutes — one sixth of the default 180-minute review budget per hang — before the kill, with only the outer GNU timeout and review deadline above it. The weld's comment argues the cover exclusively for fetch-pr/prebuild and does not state that the session default moves for all other callers. At minimum, state the session-wide effect and its acceptance in the comment block above.
Witness:
shell.test.ts -t "foreground timeout resolution" — 'falls back to the configured default when no per-call timeout is given' (toHaveBeenCalledWith(300_000)) — 5 passed
cover literal 1800000 welded unconditionally behind the always-true CI gate (QWEN_REVIEW_PREBUILD: '1' on Run review)
中文说明
此处为 fetch-pr 的 prebuild 焊接的覆盖值会把整个 CI 评审会话的 shell 默认超时抬到 30 分钟，而不只是它要保护的那一次调用：QWEN_REVIEW_PREBUILD: '1' 在 Run review 步骤上是硬编码的，shell 工具优先级为单次调用超时 → 设置默认 → 内置默认，而 skill 的 agent 调用不带单次调用超时，所以每一个省略超时的前台 shell 调用——chunk agent 的 probe、verifier 的临时树测试运行——现在都继承 1800000 ms 而非内置的 120 s。一条挂死的命令（死锁的测试、阻塞在 stdin 的进程、卡住的网络请求）会占用 agent 最长 30 分钟——每次挂死消耗默认 180 分钟评审预算的六分之一——才被杀掉，其上只有外层 GNU timeout 与评审 deadline。焊接处的注释只为 fetch-pr/prebuild 论证该覆盖值，没有说明会话默认值对所有其他调用方也整体移动。至少请在上方注释块中写明这一会话级效果及其被接受的理由。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| expect(review.run).toContain('"$QWEN_HOME/settings.json"'); | ||
| expect(review.run).toContain(`"\${${envName}:-}"`); |
There was a problem hiding this comment.
[Suggestion] R2-5: These containment checks are the only guard over the workflow's settings.json cover block, and they cannot discriminate the gate's comparison operator. Executed mutation at the reviewed commit: changing the workflow gate if [ "${QWEN_REVIEW_PREBUILD:-}" = '1' ] to != '1' ships green — all 182 tests still pass, because the defaultTimeoutMs regex still matches 1800000 and both containment strings are still present — yet the cover is then written exactly when the prebuild is off and skipped when it is on: with QWEN_REVIEW_PREBUILD=1 set in CI, fetch-pr runs under the shell tool's 120 s default / 600 s per-call ceiling and is killed mid-npm ci (measured ~4-5 min cost) — the exact incident the block exists to prevent. The value arm is live (mutating 1800000 → 600000 reds the test), but the operator arm is not, and this file's bash-execution machinery extracts the run body starting at OUTCOME='', which sits after the cover block, so no scenario execution ever reaches it. Follow this file's own pattern of executing extracted bash: slice the cover block out of review.run (from the if [ "${QWEN_REVIEW_PREBUILD:-}" line through its fi), run it with spawnSync('bash', …) with QWEN_HOME pointed at a temp dir — once with the variable set to 1, once unset — and assert settings.json exists with defaultTimeoutMs ≥ budget × 1000 in the first case and does not exist in the second.
Witness:
MUTANT(gate !=): 182 passed (182) — inverted gate ships green
MUTANT(value 600000): 1 failed — 'expected 600000 to be greater than or equal to 1800000' (the value arm is live; the operator arm is not)
intact baseline: 3 passed in the prebuild block
Fix witness: the proposed executed-gate test itself must go red when the workflow gate is inverted or the settings write is removed — the current containment assertions do not.
中文说明
这两条包含断言是 workflow 中 settings.json 覆盖块唯一的守卫，但它们无法区分闸门的比较运算符。在受审提交上执行的变异：把 workflow 闸门 if [ "${QWEN_REVIEW_PREBUILD:-}" = '1' ] 改为 != '1' 后仍然全绿——182 个测试全部通过，因为 defaultTimeoutMs 正则仍能匹配 1800000、两条包含字符串也都还在——但覆盖块从此恰好在 prebuild 关闭时写入、开启时跳过：CI 中设置了 QWEN_REVIEW_PREBUILD=1 时，fetch-pr 会在 shell 工具的 120 秒默认/600 秒单次上限下运行，于 npm ci 进行到一半被杀（实测成本约 4-5 分钟）——正是该代码块要防止的事故。数值分支是活的（把 1800000 变异为 600000 会让测试变红），但运算符分支不是；且本文件的 bash 执行机制从 OUTCOME='' 开始提取 run 主体，而它在覆盖块之后，任何场景执行都到不了覆盖块。请沿用本文件自己「执行提取出的 bash」的模式：从 review.run 中切出覆盖块（从 if [ "${QWEN_REVIEW_PREBUILD:-}" 行到其 fi），用 spawnSync('bash', …) 执行，QWEN_HOME 指向临时目录——一次变量设为 1，一次不设置——并断言第一种情况下 settings.json 存在且 defaultTimeoutMs ≥ 预算 × 1000，第二种情况下不存在。
验收测试：所提议的可执行闸门测试本身必须在 workflow 闸门被取反或 settings 写入被移除时变红——当前的包含断言做不到。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # above the setting's interactive 600000 ceiling — the startup | ||
| # loader does not range-check, the runtime gate accepts it, and |
There was a problem hiding this comment.
[Suggestion] R2-6: The read side of this cover — the settings loader accepting defaultTimeoutMs: 1800000, a value above the schema's declared maximum: 600000 (settingsSchema.ts) — is true at this commit but pinned by no test. validateSettingValue is the only consumer of the schema maximum and is called exclusively from interactive write paths (config-command, SettingsDialog, serve workspace-settings routes), never the startup loader, so the value loads unclamped today; the workflow test asserts only the YAML write side, and the existing CLI pass-through test uses 300000, under the schema max. The day a maintainer wires schema enforcement into the startup load path — an ordinary hardening, and config.test.ts itself documents that schema validation is bypassed on load — the 1800000 is rejected or dropped, the session falls back to the built-in 120000 ms, and the shell tool kills fetch-pr mid-npm ci at two minutes on every CI review: the cover silently becomes a no-op and reproduces the exact #10108 shape it was welded to prevent, while the workflow test stays green. Add a load-path regression test next to the existing pass-through test in packages/cli/src/config/config.test.ts: load settings carrying tools.shell.defaultTimeoutMs: 1800000 and assert getShellDefaultTimeoutMs() returns 1800000 unchanged; optionally also raise the schema maximum so the declared contract matches what CI relies on.
Witness:
PROBE loader: 1 passed — getShellDefaultTimeoutMs() returned 1800000 for { tools: { shell: { defaultTimeoutMs: 1800000 } } }
PROBE schema: 1 passed — def.maximum === 600000; validateSettingValue(def, 1800000) === 'Value must be <= 600000' (600000 accepted, oracle live)
existing pass-through test uses only 300000 (under the schema max); nothing pins the cover value on the load side
Fix witness: the new assertion — a settings file carrying 1800000 must reach Config.getShellDefaultTimeoutMs() unclamped — must go red the moment load-time validation of the schema maximum is introduced.
中文说明
该覆盖值的读取侧——配置加载器接受 defaultTimeoutMs: 1800000（高于 schema 声明的 maximum: 600000，见 settingsSchema.ts）——在本提交上成立，但没有任何测试钉住。validateSettingValue 是 schema maximum 的唯一消费者，且只被交互式写入路径（config-command、SettingsDialog、serve 的 workspace-settings 路由）调用，启动加载器从不调用它，因此当前该值不被范围校验地加载；workflow 测试只断言 YAML 写入侧，既有的 CLI 透传测试用的是 300000（低于 schema 上限）。一旦有维护者把 schema 校验接入启动加载路径——这是常规加固，且 config.test.ts 自己也写明「加载时不做 schema 校验」——1800000 会被拒绝或丢弃，会话退回内置的 120000 ms，shell 工具会在每一次 CI 评审中于两分钟时在 npm ci 进行到一半杀掉 fetch-pr：覆盖值悄无声息地变成空操作，复现它本要防止的 #10108 形态，而 workflow 测试仍然是绿的。请在 packages/cli/src/config/config.test.ts 既有透传测试旁补一个加载路径回归测试：加载携带 tools.shell.defaultTimeoutMs: 1800000 的配置，断言 getShellDefaultTimeoutMs() 原样返回 1800000；也可以同时抬高 schema maximum，使声明的契约与 CI 实际依赖一致。
验收测试：新增断言——携带 1800000 的配置文件必须不被钳制地到达 Config.getShellDefaultTimeoutMs()——必须在引入加载期 schema maximum 校验的那一刻变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🔀 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 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- upToDate/low-effort prebuild gate (Agents 3b/4 rediscovery) — already recorded as D4-1 in round 4's deferred list (review 5057002626)
- double prebuild on side-file anchor-recovery re-fetch (round-3 reverse audit rediscovery) — already recorded as D3-1 in round 3's deferred list (review 5056685889)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 22204 passed; 56 passed — this review observed 22204 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-code-pr-review.yml:950 — [review] D5-1 new QWEN_REVIEW_PREBUILD switch and dependencies field have no entry in docs/users/features/code-review.md where its siblings are documented (code-age rule: anchored on code unch….github/workflows/qwen-code-pr-review.yml:1355 — [review] D5-2 prebuildRequested accepts '1'/'true' but the workflow cover gate accepts only '1' — any other documented value runs the prebuild without its timeout cover (code-age rule: anchor…
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 22204 passed; 56 passed — this review observed 22204 passed。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| # above the setting's interactive 600000 ceiling — the startup | ||
| # loader does not range-check, the runtime gate accepts it, and |
There was a problem hiding this comment.
[Suggestion] R2-6: Still stands — the read side of this cover — the settings loader accepting defaultTimeoutMs: 1800000, a value above the schema's declared maximum: 600000 (settingsSchema.ts:2717-2720) — is true at this commit but pinned by no test. validateSettingValue (which enforces the maximum, rejecting with "Value must be <= 600000") is called only from the interactive write surfaces (SettingsDialog, /config command, serve settings routes), never at startup load; the runtime gate accepts integers 0..2^31-1. The moment anyone wires the existing validator into settings loading — an ordinary hardening, all pieces already exist — the value is rejected, getShellDefaultTimeoutMs() falls back to the built-in 120000, and fetch-pr is killed 2 minutes into npm ci on every CI review, reproducing incident #10108 with no test red: the workflow test pins the YAML literal against the TS constant — neither side ever runs the loader.
Witness:
scratch-tree vitest probe through the real loadSettings / validateSettingValue / Config (3/3 assertions):
PROBE LOADED_VALUE=1800000 — startup load accepts the above-schema value, no range check, no load errors
PROBE SCHEMA_MAXIMUM=600000 — settingsSchema.ts declares maximum: 600000
PROBE WRITE_PATH_ERROR="Value must be <= 600000" — validateSettingValue rejects it (interactive surfaces only)
PROBE RUNTIME_GATE=1800000 — core Config gate accepts it, so getShellDefaultTimeoutMs() === 1800000 today
no test anywhere feeds the cover content through the settings load path (grep 1800000/1_800_000 across all tests)
Fix: give the value a contract instead of slipping past one — raise the schema maximum (settingsSchema.ts + validateSettingValue + tool param validation) to explicitly admit the workflow-sized window, or add an env knob for the shell default mirroring QWEN_CODE_API_TIMEOUT_MS (which the adjacent comment describes as outranking settings) and weld the cover through that; plus a test that feeds the cover content through the real load path and asserts the resolved value reaches the shell tool.
The fix must reconcile, not bypass, two declared bounds: maximum: 600000 for defaultTimeoutMs (packages/cli/src/config/settingsSchema.ts:2720) and "Timeout cannot exceed 600000ms (10 minutes)." (packages/core/src/tools/shell.ts:5124); the runtime gate currently admitting the value is params.shellDefaultTimeoutMs <= 2_147_483_647 (packages/core/src/config/config.ts:2801). Extend 'covers the prebuild call with a session shell timeout carrying the budget' (or add a cli config test) to load {"tools":{"shell":{"defaultTimeoutMs":1800000}}} via the actual loader and assert getShellDefaultTimeoutMs() === 1800000 — wiring validateSettingValue into the load path must then red; please confirm.
中文说明
仍然成立——该覆盖的读取侧——设置加载器接受 defaultTimeoutMs: 1800000 这一超过 schema 声明的 maximum: 600000（settingsSchema.ts:2717-2720）的值——在本提交上成立，但没有任何测试钉住。validateSettingValue（强制执行该上限，以「Value must be <= 600000」拒绝）只被交互式写入面（SettingsDialog、/config 命令、serve 设置路由）调用，启动加载从不调用它；运行时闸门接受 0..2^31-1 的整数。一旦有人把现成的校验器接进设置加载——一次普通的加固，所有部件都已存在——该值就会被拒绝，getShellDefaultTimeoutMs() 回落到内置 120000，每次 CI 评审的 fetch-pr 都会在 npm ci 进行到 2 分钟时被杀，复现事故 #10108 且没有测试变红：workflow 测试只把 YAML 字面量与 TS 常量对照——两侧都不曾运行加载器。
修复：给该值一个契约，而不是绕过契约——抬高 schema 上限（settingsSchema.ts + validateSettingValue + 工具参数校验）以显式接纳按 workflow 步骤取值的窗口；或仿照 QWEN_CODE_API_TIMEOUT_MS（相邻注释称其优先于设置）为 shell 默认值新增环境开关，并把覆盖焊在开关上；外加一个测试，把覆盖内容经由真实加载路径载入并断言解析值到达 shell 工具。
修复必须对账而非绕过两个已声明的边界：defaultTimeoutMs 的 maximum: 600000（packages/cli/src/config/settingsSchema.ts:2720）与「Timeout cannot exceed 600000ms (10 minutes).」（packages/core/src/tools/shell.ts:5124）；当前接受该值的运行时闸门是 params.shellDefaultTimeoutMs <= 2_147_483_647（packages/core/src/config/config.ts:2801）。扩展 'covers the prebuild call with a session shell timeout carrying the budget'（或新增 cli 配置测试），经由真实加载器载入 {"tools":{"shell":{"defaultTimeoutMs":1800000}}} 并断言 getShellDefaultTimeoutMs() === 1800000——之后把 validateSettingValue 接入加载路径必须使其变红；请确认。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Deferred to the next round (round bound, not declined) — this round landed all 5 Critical findings plus 3 Suggestions, at the ~8-findings-per-round cap. R2-6 is real and stays open: the cover value still reads above the schema's declared maximum: 600000 with no load-path test pinning it. It needs its own pass because every fix option lands outside this PR's footprint — raising settingsSchema.ts/validateSettingValue (plus regenerating the vscode schema artifact) or adding a core env knob mirroring QWEN_CODE_API_TIMEOUT_MS — and this round's R2-1 change (cover 1800000 → 2400000) makes the contract more urgent, not less. Note for that pass: the load-path witness the finding describes (feed the cover JSON through the real loadSettings, assert getShellDefaultTimeoutMs() resolves it) can be written alongside whichever option is chosen.
中文说明
延后到下一轮（轮次上限所致，并非拒绝）——本轮已落地全部 5 条 Critical 与 3 条 Suggestion，达到每轮约 8 条的上限。R2-6 属实且保持未解决：覆盖值仍高于 schema 声明的 maximum: 600000，且没有任何走加载路径的测试钉住它。它需要单独一轮处理，因为每个修复选项都落在本 PR 足迹之外——抬高 settingsSchema.ts/validateSettingValue（并同步重新生成 vscode schema 产物），或在核心侧仿照 QWEN_CODE_API_TIMEOUT_MS 新增环境开关——而本轮 R2-1 的改动（覆盖值 1800000 → 2400000）只让这个契约更紧迫。给那一轮的提示：本发现描述的加载路径见证（把覆盖 JSON 经真实 loadSettings 载入、断言 getShellDefaultTimeoutMs() 解析到该值）可以与所选方案一并写入。
| const built = | ||
| report !== null && | ||
| report.ok && | ||
| report.toolchain === 'npm' && |
There was a problem hiding this comment.
[Suggestion] R1-3: Still stands — the round-1 fix aligned built with base-tree's availability rule by inlining it — this predicate is now a second inline copy of the rule in base-tree.ts:294-317 (the notBuilt refusal plus the !ok || toolchain !== 'npm' || build.length === 0 refusal), and neither copy is a callable predicate — grep for reportFullyBuilt|fullyBuilt|isFullyBuilt across packages/cli returns zero matches. BuildTestReport has a documented history of growing new ok: true shapes that are not compiled trees (notBuilt and endedBeforeTests were made structural precisely because consumers could not see them in prose). When the next such shape lands, the edit goes to base-tree's gate (the consumer named in notBuilt's docstring) and misses this copy — the fetch report then says dependencies.built: true over a closure that was never compiled, and SKILL.md's new field text invites verifier probes to run against it, manufacturing the false resolution failures the prebuild exists to prevent. No divergence exists at this commit — maintainability, not a live defect.
Witness:
prebuild.ts:186-191 built = report !== null && report.ok && report.toolchain === 'npm'
&& report.build.length > 0 && (report.notBuilt?.length ?? 0) === 0
base-tree.ts:~294 if ((build.notBuilt?.length ?? 0) > 0) → unavailable
base-tree.ts:~317 if (!build.ok || build.toolchain !== 'npm' || build.build.length === 0) → unavailable
grep reportFullyBuilt|fullyBuilt|isFullyBuilt packages/cli → zero matches; no shared predicate exists
witness: not run — code-shape claim settled by the two quoted predicates and the empty helper grep
Fix: extract the predicate once — e.g. export function reportFullyBuilt(report: BuildTestReport): boolean in build-test.ts — and call it from both sites: prebuild.ts computes report !== null && reportFullyBuilt(report), and base-tree's two refusal guards become the negation of the same call.
The extracted predicate must keep both call sites agreeing with the availability gate's semantics — build-test.ts:150 (notBuilt?: string[]) docstring: consumers of this report (base-tree's availability gate) must be able to see that without parsing prose. prebuild.test.ts already pins each conjunct ('cut the closure short', 'handed the repo to the brief', 'nothing to compile') and base-tree.test.ts pins the base-tree side — after the extraction, removing any conjunct from the shared predicate must red the matching test; please remove it and confirm.
中文说明
仍然成立——第 1 轮的修复通过内联使 built 与 base-tree 的可用性规则对齐——该谓词现在是 base-tree.ts:294-317 中规则的第二份内联拷贝（notBuilt 拒绝加上 !ok || toolchain !== 'npm' || build.length === 0 拒绝），且两份拷贝都不是可调用的谓词——在 packages/cli 中 grep reportFullyBuilt|fullyBuilt|isFullyBuilt 返回零匹配。BuildTestReport 有新增「ok: true 但并非已编译树」形态的既定历史（notBuilt 与 endedBeforeTests 被结构化，正是因为使用方无法从散文中看到它们）。下一个这类形态出现时，改动会落在 base-tree 的闸门（notBuilt 文档串点名的消费者）而漏掉这份拷贝——fetch 报告会在一个从未编译的闭包上写 dependencies.built: true，而 SKILL.md 的新字段文本正邀请验证器 probe 在其上运行，制造出 prebuild 存在所要防止的假解析失败。本提交上尚不存在分歧——是可维护性问题，不是现行缺陷。
修复：把谓词抽取一次——例如在 build-test.ts 中 export function reportFullyBuilt(report: BuildTestReport): boolean——两处调用：prebuild.ts 计算 report !== null && reportFullyBuilt(report)，base-tree 的两道拒绝闸门改写为同一调用的否定。
抽取后的谓词必须让两个调用点与可用性闸门的语义保持一致——build-test.ts:150（notBuilt?: string[]）文档串：该报告的使用方（base-tree 的可用性闸门）必须无需解析散文即可看到。prebuild.test.ts 已钉住每个合取项（'cut the closure short'、'handed the repo to the brief'、'nothing to compile'），base-tree.test.ts 钉住 base-tree 侧——抽取之后，从共享谓词中移除任一合取项必须使对应用例变红；请移除并确认。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Deferred to the next round (round bound, not declined) — this round landed all 5 Critical findings plus 3 Suggestions, at the ~8-findings-per-round cap. R1-3 is verified as described (no divergence at this commit; prebuild.ts and base-tree.ts carry two inline copies of the availability rule, neither callable), and the extraction (reportFullyBuilt in build-test.ts, called from both sites) remains the intended fix. It is deferred, not declined, because it rewrites base-tree.ts's two refusal guards — a file this PR has never touched — and a maintainability refactor into a fresh footprint deserves its own round with the conjunct-removal witnesses this finding asks for (prebuild.test.ts and base-tree.test.ts red on each removed conjunct).
中文说明
延后到下一轮（轮次上限所致，并非拒绝）——本轮已落地全部 5 条 Critical 与 3 条 Suggestion，达到每轮约 8 条的上限。R1-3 经核实与描述一致（本提交上尚无分歧；prebuild.ts 与 base-tree.ts 各持一份可用性规则的内联拷贝，且都不可调用），抽取方案（在 build-test.ts 中 reportFullyBuilt，两处调用）仍是既定修复。之所以延后而非拒绝，是因为它要改写 base-tree.ts 的两道拒绝闸门——本 PR 从未触及的文件——一次进入新足迹的可维护性重构理应单独一轮完成，并附上本发现要求的合取项移除见证（每移除一个合取项，prebuild.test.ts 与 base-tree.test.ts 相应变红）。
…10423) The session-shell cover welded for the prebuild sat exactly at the budget, but its clock starts at the fetch-pr spawn while the budget clock starts only inside runBuildTest, so the hang case the budget exists for expired the cover first; the cover now carries a named headroom. An attempt budget that cannot carry the prebuild budget plus the deadline reserve plus a review margin now unsets the opt-in instead of dying mid-`npm ci`, and a local opt-in without the CI-welded cover warns and skips instead of killing fetch-pr at the 120s built-in default. The opt-in grammar narrows to the '1' the cover gate welds for, the probe-overlap invitation names the dist pre-clean window, the wiring suite is hermetic against the ambient switch, and the workflow pins parse the cover JSON, the gate literals, and the write-before-launch ordering.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round 6 summary — PR #10423Addressed 8 of 10 actionable findings (all 5 Critical, 3 of 5 Suggestions) in one commit ( Critical findings
Suggestions
Deferred to the next round (round bound, not declined)
Mutation probes (witnesses)
All mutants restored; every suite re-ran green after restoration. Verification
中文说明第 6 轮总结 — PR #10423在一个提交（ Critical 发现
Suggestion 发现
延后到下一轮（轮次上限，非拒绝）
变异探针（见证）
所有变异体均已还原；每次还原后各套件复跑均为绿色。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。 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.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- upToDate/low-effort prebuild gate miss (chunk-2 rediscovery) — already recorded as D4-1 in round 4's deferred list (review 5057002626) and disclosed again in round 5's body
- fileSourced default binding untested (round-5 chunk-3 rediscovery) — already reported as R3-4 (comments 3885423123, 3885673521)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 22412, 1795, 1699, 604, 4605, 642 passed; 56 passed — this review observed 22412, 1795, 1699, 604, 4605, 642 passed.
6 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1401 — [probe] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself w…packages/cli/src/commands/review/fetch-pr.test.ts:497 — [probe] Critical [fails-closed] [new-surface] R5-1: (fix-induced) env sanitization is describe-scoped — filtered runs of the other suites leak the ambient QWEN_REVIEW_PREBUILD and red …packages/cli/src/commands/review/lib/prebuild.ts:209 — [review] prebuildCovered reads a settings-level defaultTimeoutMs of 0 as zero milliseconds of cover, but the shell tool reads that same value as "timeout disabled" — unlimited cov…packages/cli/src/commands/review/lib/prebuild.ts:233 — [review] The production default binding — the module's central claim that the prebuild runs Agent 7's real runBuildTest — is exercised by no test: every prebuild.test.ts call inject…scripts/tests/qwen-pr-review-workflow.test.js:2469 — [review] This describe pins the gate's literals and both existing orderings ( finalized < gateIdx , coverWrite < agent ) but never pins that the budget-reconciliation gate precedes the c…packages/cli/src/commands/review/lib/prebuild.ts:195 — [review] prebuildCovered 's five tests weld only the user-scope settings in QWEN_HOME ; none creates a workspace .qwen/settings.json , so the deliberate scope restriction — a reposit…scripts/tests/qwen-pr-review-workflow.test.js:2455 — [review] This suite never pins that the refusal's unset is CONDITIONAL — inside the -lt refusal branch. Hoisting the unset one level up (always unset when the opt-in is set, still i…packages/cli/src/commands/review/lib/prebuild.test.ts:321 — [review] The "in-range integers only" admissibility gate that decides cover exists as two independent inline copies — prebuildCovered 's in prebuild.ts and the shellDefaultTime…
Convergence: round 6 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 10 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/review/lib/prebuild.ts (findings in rounds 1, 2; 1 more now); .github/workflows/qwen-code-pr-review.yml (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 22412, 1795, 1699, 604, 4605, 642 passed; 56 passed — this review observed 22412, 1795, 1699, 604, 4605, 642 passed。
6 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改；其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 8 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 6 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 10 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/lib/prebuild.ts（第 1、2 轮已出过发现，本轮又有 1 条）；.github/workflows/qwen-code-pr-review.yml（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 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 将重新运行。 |
The prebuild opt-in adds two guarded blocks to the review job — the attempt-budget reconciliation and the shell-tool cover — taking qwen-code-pr-review.yml from 173139 to 177690 bytes. That is 4551 over the recorded baseline, 455 past the 4096 allowance, so the ratchet in .github/scripts/check-workflow-size.sh fails the whole Test job. The growth is real and stays: both blocks are shell that has to run in the job's own step, next to the EFFECTIVE_TIMEOUT_MINUTES and QWEN_HOME it reads, and their comments carry the arithmetic that scripts/tests/qwen-pr-review-workflow.test.js pins against PREBUILD_BUDGET_S and PREBUILD_COVER_HEADROOM_S. At 177690 the file is 34% of GitHub's 512000-byte start-runs limit and well under the repo's 470000-byte gate, so the ceiling is not in play — only the ratchet, which is what this line is for.
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.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- gate-vs-cover-write ordering unpinned (test-matrix finding) — already recorded in round 6's deferred list (review 5058840798)
- defaultTimeoutMs: 0 read as zero cover instead of a disabled timer — already recorded in round 6's deferred list (review 5058840798)
- unset conditional-containment unpinned (hoist survives all tests) — already recorded in round 6's deferred list (review 5058840798)
- prebuild gate misses the upToDate stop — already recorded as D4-1 in round 4's deferred list (review 5057002626)
- workspace-scope guard has no pinning test — already recorded in round 6's deferred list (review 5058840798)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 22425, 1813, 1705, 482, 5221, 94 passed; 56 passed — this review observed 22425, 1813, 1705, 482, 5221, 94 passed.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1403 — [probe] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself w…packages/cli/src/commands/review/lib/prebuild.test.ts:322 — [probe] rejected-values test cannot discriminate the Number.isInteger clause — dropping it survives 16/16 and opens an above-cover rejected-value class (mutant probe)scripts/tests/qwen-pr-review-workflow.test.js:2455 — [probe] unset pin matches by substring — unset of a prefixed variable silently no-ops the reconciliation gate (mutant probe: survives 186/186)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 22425, 1813, 1705, 482, 5221, 94 passed; 56 passed — this review observed 22425, 1813, 1705, 482, 5221, 94 passed。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- prebuildCovered reads defaultTimeoutMs: 0 as zero cover instead of a disabled timer — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in round 7's body (review 5059385720)
- prebuildRequested's default fileSourced binding to the real isFileSourcedEnvKey is exercised by no test — already reported as R3-4 (comments 3885423123, 3885673521)
- prebuildCovered's workspace-scope exclusion (skipWorkspaceSettings: true) has no pinning test — already recorded in round 6's deferred list (review 5058840798)
- the workflow test never pins the budget-reconciliation gate's downstream ordering (gate-vs-cover-write ordering unpinned) — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in round 7's body (review 505938572…
- the refusal's unset QWEN_REVIEW_PREBUILD conditional-containment is unpinned (hoist survives all tests) — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in round 7's body (review 5059385720)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI, and this review's own test phase never ran: the runner was degraded (load average ~260) and the build phase exhausted its budget before reaching packages/cli (install and 11 workspace builds were green; the chunk agents ran the new prebuild, workflow-test and SKILL suites directly, all green).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1402 — [review] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself …packages/cli/src/commands/review/lib/prebuild.ts:186 — [review] prebuildCovered models only the settings layer — a per-call timeout on fetch-pr (capped at 600000ms) outranks the welded coverpackages/cli/src/commands/review/lib/prebuild.test.ts:223 — [probe] marker-vs-directory mutant survives the installed-pin — the prebuild's own installed read is unguarded
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 0, previous 0), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI, and this review's own test phase never ran: the runner was degraded (load average ~260) and the build phase exhausted its budget before reaching packages/cli (install and 11 workspace builds were green; the chunk agents ran the new prebuild, workflow-test and SKILL suites directly, all green)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 3 条（原文未翻译，列表见上方英文部分）。
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 1 条 Critical），首次发现的速率没有下降（本轮 0，上一轮 0），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 4/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 4/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33288670977 🧠 Handled by Qwen Code · model/模型 |
…10423) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #10423, round on commit 2d4a928Two Critical inline findings were open. Both are resolved in code; no finding was declined, deferred, or escalated this round. The deferred lists from review rounds 6–8 ( Feedback points and decisionsR3-1 — workflow-size ratchet fails CI (rc:3887333218) — RESOLVED (prior commit, re-verified)The claim was reproduced-by-inspection and was already fixed on this branch by commit R2-3 —
|
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- gate-vs-cover-write ordering unpinned — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in rounds 7-8
- prebuild gate round-shape blindness (upToDate stop and side-file re-run double prebuild) — already recorded as D3-1 in round 3's deferred list (review 5056685889) and as D4-1 in round 4's deferred list (review 5057002626), re-disclosed in r…
- budget gate's unset conditional-containment unpinned — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in rounds 7-8
Not reviewed: build-and-test — the test phase never ran: build-test exhausted its budget during the build phase (packages/cli build timed out at its deadline, stamped infrastructure), and Integration Tests (CLI, No Sandbox) was skipped in CI; chunk agents ran the new prebuild, workflow-test and SKILL suites directly, all green.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1401 — [review] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself …packages/cli/src/commands/review/lib/prebuild.test.ts:279 — [review] the prebuildCovered suite does not isolate the system scope — an admin-managed /etc/qwen-code/settings.json decides the welded-cover tests and can revoke the cover, pinned…scripts/tests/qwen-pr-review-workflow.test.js:2434 — [review] the cover-value assertion is lower-bound-only and accepts runtime-rejected values — a float weld passes the test but the integer gate refuses it, silently skipping the prebuild
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — the test phase never ran: build-test exhausted its budget during the build phase (packages/cli build timed out at its deadline, stamped infrastructure), and Integration Tests (CLI, No Sandbox) was skipped in CI; chunk agents ran the new prebuild, workflow-test and SKILL suites directly, all green。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R10-1 budget gate's unset conditional-containment unpinned (hoist survives all tests) — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in rounds 7-9
- R10-2 gate-vs-cover-write ordering unpinned — already recorded in round 6's deferred list (review 5058840798) and re-disclosed in rounds 7-9
- R10-3 covers() hand-copied admissibility predicate unpinned against Config's runtime gate — already recorded in round 6's deferred list (review 5058840798)
Not reviewed: build-and-test — the test phase never ran: build-test exhausted its per-call budget during the build phase (packages/cli build timed out at its clamped deadline under shared-host load, stamped infrastructure) and Integration Tests (CLI, No Sandbox) was skipped in CI; the chunk agents ran the new prebuild, fetch-pr wiring and workflow-test suites directly, all green.
Not reviewed: reverse audit — stopped at the 5-round cap without converging (round 5 still reported findings).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round; 3 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1401 — [probe] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself w…packages/cli/src/commands/review/fetch-pr.test.ts:497 — [probe] Critical [fails-closed] [new-surface] R5-1: (fix-induced) env sanitization is describe-scoped — the four sibling handler-driving suites leak the ambient QWEN_REVIEW_PREBUILD an…packages/cli/src/commands/review/fetch-pr.ts:1764 — [probe] Critical [fails-closed] [new-surface] unguarded dependencies plan rewrite kills fetch-pr on a write fault (ENOSPC) and can truncate the good planpackages/core/src/skills/bundled/review/SKILL.md:166 — [probe] probe-window rule under-covers the unsafe set (out-of-closure importers of closure members)scripts/tests/qwen-pr-review-workflow.test.js:2446 — [probe] cover-write containment inside its own gate is unpinnedpackages/core/src/skills/bundled/review/SKILL.md:166 — [probe] failed-prebuild fallback equates installed-but-unbuilt with no-prebuild for probespackages/cli/src/commands/review/lib/prebuild.test.ts:300 — [probe] prebuildCovered outer tests leak the ambient workspace-scope settings
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — the test phase never ran: build-test exhausted its per-call budget during the build phase (packages/cli build timed out at its clamped deadline under shared-host load, stamped infrastructure) and Integration Tests (CLI, No Sandbox) was skipped in CI; the chunk agents ran the new prebuild, fetch-pr wiring and workflow-test suites directly, all green。
未审查：reverse audit — stopped at the 5-round cap without converging (round 5 still reported findings)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改；其中 3 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 7 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no actionThis round found no actionable feedback on PR #10423: the review, inline-comment, and issue-level-comment sections are empty, and the round runs in Critical-only mode (the two deferred automated reviews remain open for human follow-up). No code change was warranted and none was made. Failed checks investigated
Evidence that this PR's code is not involved
The cancelled CI lane should clear on the next run once the shared runner pool is uncontended; no code change is required on this PR. Verification
中文说明Autofix 轮次：无操作本轮在 PR #10423 上没有发现可处理的反馈：评审、行内评论与 issue 级评论区均为空，且本轮处于仅处理 Critical 的模式（两条被延后的自动化评审保持开放，留待人工跟进）。无需任何代码改动，本轮也未做任何改动。 已调查的失败检查
本 PR 代码与失败无关的证据
被取消的 CI 通道应在共享运行器资源池不再争用后的下一次运行中恢复正常；本 PR 无需任何代码改动。 验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. 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. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R11-1 new prebuild contract tests sit outside every npm workspace — already recorded in round 2's deferred list (review 5056030145)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 701 passed — this review observed 1818, 1721, 481, 5163 passed; 56 passed — this review observed 1818, 1721, 481, 5163 passed.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1401 — [probe] Critical [fails-closed] [new-surface] R2-2: (fix-induced) the budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself w…
中文说明
已审查。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：701 passed — this review observed 1818, 1721, 481, 5163 passed; 56 passed — this review observed 1818, 1721, 481, 5163 passed。
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Integration Tests (no-AK, No Sandbox)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Integration Tests (no-AK, No Sandbox)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D12-4 .env-sourced refusal's default wiring untested plus the overclaiming vi.mock comment — already reported as R3-4 (comments 3885423123, 3885673521, unresolved)
- D12-5 welded cover value unwitnessed through the real core Config gate — already recorded as R10-3 in round 6's deferred list (review 5058840798), re-disclosed rounds 7-8
- D12-6 defaultTimeoutMs: 0 read as zero cover instead of a disabled timer (filed Critical, downgraded by verification) — already recorded in round 6's deferred list (review 5058840798), re-disclosed rounds 7-8
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/qwen-code-pr-review.yml:1405 — [review] Critical [fails-closed] [new-surface] R2-2: (fix-induced) budget gate runs once before the retry loop — a shrunk-window retry re-runs the prebuild under a budget the gate itself woul…scripts/tests/qwen-pr-review-workflow.test.js:2465 — [review] reserve-mirror pin matches only the gate's copy — run_review_once's QWEN_REVIEW_DEADLINE_RESERVE_SECONDS computation is pinned by no testpackages/core/src/skills/bundled/review/SKILL.test.ts:1703 — [review] SKILL doc test hardcodes the QWEN_REVIEW_PREBUILD=1 literal instead of reading PREBUILD_ENV like its sibling workflow test
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (prebuildRequested() && !emptyDiff) { | ||
| if (!prebuildCovered()) { |
There was a problem hiding this comment.
[Critical] R12-1: [certifies-falsely] [new-surface] The .env-sourced refusal cannot fire cross-process: prebuildRequested() consults the per-process file-sourced registry inside the spawned fetch-pr child, but the environment loader records provenance only for keys it writes itself (if (isEffectivelyUnset) { process.env[key] = …; dotEnvSourcedKeys.add(key); }, environment.ts:593-599). A value the agent process applied from the reviewed repository's .qwen/.env is inherited into the child through the composed env unmarked, so isFileSourcedEnvKey(PREBUILD_ENV) returns false there and prebuildRequested() returns true for a repository-supplied value. A review session whose cwd is an untrusted checkout — the exact topology this guard's own comment names — carrying a repo-shipped .qwen/.env with QWEN_REVIEW_PREBUILD=1 (not excluded by DEFAULT_EXCLUDED_ENV_VARS nor by PROJECT_ENV_HARDCODED_EXCLUSIONS) therefore opts its own review into the prebuild whenever an operator-scoped tools.shell.defaultTimeoutMs >= 2,400,000 makes prebuildCovered() pass: an unrequested blocking npm-ci + build prefix of up to 1800s decided by the repository, against this module's own documented property that what a PR can toggle about its own review is the operator's decision, not the PR's. The blast radius is bounded — the prebuild runs the same install/build Agent 7 performs minutes later, and in CI the variable is a process env, so the bypass is moot there — but the gate this diff adds is proven broken in its stated purpose.
Witness:
Two-process probe on the intact PR (agent applies the repo's .qwen/.env, child spawned with the inherited env as the shell tool composes it):
AGENT: {"envValue":"1","fileSourced":true,"prebuildRequested":false}
CHILD: {"envValue":"1","fileSourced":false,"prebuildRequested":true}
Flip arm (key added to PROJECT_ENV_HARDCODED_EXCLUSIONS):
AGENT: {"envValue":null,"fileSourced":false,"prebuildRequested":false} → CHILD: prebuildRequested:false
Fix — reject the key at load time from project files, using the pattern already in the file for operator-decision keys: add 'QWEN_REVIEW_PREBUILD' to PROJECT_ENV_HARDCODED_EXCLUSIONS in packages/cli/src/config/shared-env-keys.ts. That closes the hole at the source — the value never enters the agent's env from a project file, so nothing is inherited — while the read-time check stays as defence in depth. The fix must not violate canApplyParsedEnvKey's home-scope exemption (environment.ts:461-463) nor isFileSourcedEnvKey's documented operator routes (environment.ts:186-190): CI welds the variable as a workflow step env (.github/workflows/qwen-code-pr-review.yml:997), a real process variable that must keep working — hardcoded exclusions apply only to parsed .env files, so it is unaffected.
Acceptance test: add a loader-level test asserting a project (non-home) .qwen/.env carrying QWEN_REVIEW_PREBUILD does not reach process.env while a home-scoped .env and a pre-set process variable do — then remove the key from PROJECT_ENV_HARDCODED_EXCLUSIONS, run that test, and confirm it reds.
中文说明
.env 来源的拒绝无法跨进程生效：prebuildRequested() 在被派生的 fetch-pr 子进程里查询「按进程隔离的文件来源注册表」，但环境加载器只为自己写入的键记录来源（if (isEffectivelyUnset) { process.env[key] = …; dotEnvSourcedKeys.add(key); }，environment.ts:593-599）。agent 进程从被评审仓库的 .qwen/.env 应用进来的值，会经由组合后的环境变量不带标记地继承到子进程，于是子进程里 isFileSourcedEnvKey(PREBUILD_ENV) 返回 false，prebuildRequested() 对一个仓库提供的值返回 true。当评审会话的 cwd 位于一个不可信检出（正是这道守卫自己的注释所点名的拓扑）且仓库携带写有 QWEN_REVIEW_PREBUILD=1 的 .qwen/.env（该键既不在 DEFAULT_EXCLUDED_ENV_VARS 也不在 PROJECT_ENV_HARDCODED_EXCLUSIONS 里）时，只要操作者作用域的 tools.shell.defaultTimeoutMs >= 2,400,000 使 prebuildCovered() 通过，仓库就能让自己的评审跑起 prebuild——一段最长 1800 秒、未经请求的阻塞式 npm-ci + 构建前缀，由仓库自己决定，违背本模块自己写明的属性：「一个 PR 能为自己的评审拨动的开关，是操作者的决定，不是 PR 的」。影响范围有限——prebuild 执行的正是 Agent 7 几分钟后会跑的同一套安装/构建，且在 CI 里该变量是进程环境变量、此绕过无效——但本 diff 新增的这道守卫已被证明在其声明的用途上是坏的。
证据：
对未改动的 PR 做双进程探针（agent 应用仓库的 .qwen/.env，子进程按 shell 工具组合环境的方式继承）：
AGENT: {"envValue":"1","fileSourced":true,"prebuildRequested":false}
CHILD: {"envValue":"1","fileSourced":false,"prebuildRequested":true}
修复臂（把该键加入 PROJECT_ENV_HARDCODED_EXCLUSIONS）：
AGENT: {"envValue":null,"fileSourced":false,"prebuildRequested":false} → CHILD: prebuildRequested:false
修复——在加载期从项目文件拒绝该键，复用文件中「操作者决定类键」的既有模式：把 'QWEN_REVIEW_PREBUILD' 加入 packages/cli/src/config/shared-env-keys.ts 的 PROJECT_ENV_HARDCODED_EXCLUSIONS。这从源头堵住漏洞——该值永远不会从项目文件进入 agent 环境，也就无从继承——读取时的检查作为纵深防御保留。修复不得违反 canApplyParsedEnvKey 的 home 作用域豁免（environment.ts:461-463）与 isFileSourcedEnvKey 文档化的操作者通道（environment.ts:186-190）：CI 以 workflow 步骤 env 的形式焊入该变量（qwen-code-pr-review.yml:997），这是一个必须继续生效的真实进程变量——硬编码排除只作用于解析出的 .env 文件，因此不受影响。
验收测试：新增一个加载器层面的测试，断言携带 QWEN_REVIEW_PREBUILD 的项目级（非 home）.qwen/.env 不会进入 process.env，而 home 作用域的 .env 与预设进程变量可以；然后从 PROJECT_ENV_HARDCODED_EXCLUSIONS 中移除该键、运行该测试并确认变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Conflict: .github/workflows/.size-baseline — both sides re-recorded qwen-code-pr-review.yml. Resolved to the merged file's actual size (198946): main re-recorded 193208 for its own growth and this branch adds the prebuild env plus the session-timeout cover on top, so neither side's number fits the other's file. The ratchet passes on the merged tree (check-workflow-size.sh clean). This merge also brings c480330 (browser daemon bundle budget to 216KB), which is the fix for the Install dependencies failure that was reddening Test/no-AK/web-shell E2E on this PR's previous head.
4 participants
What this PR does
On CI, the review worktree is now installed and compiled before any agent starts. When the review workflow sets
QWEN_REVIEW_PREBUILD=1,fetch-pr— right after it writes the plan — runs Agent 7's ownbuild-test --install --build-onlyon the worktree it just created: the same command, environment, sandbox policy and scoped build closure Agent 7 would run minutes later, with one difference — it runs on the orchestrator's clock, with a budget sized to a workflow step (30 minutes) instead of a shell tool call (10). The outcome lands in the fetch report as data (dependencies:installed, read off npm's own completeness marker — the exact gatebuild-testreads;built,build-test's verdict on the closure; anote; and the path of thebuild-testreport for the run), the plan is rewritten with it, and the session ledger records the final write so the run-epoch fence keys on the right mtime. The bundled review skill documents the field and keeps its "never install by hand" rule, which on a prebuilt tree is stricter, not looser.Nothing about what runs, or as whom, or with which environment, is decided here — only when. Everything fails open: a prebuild that timed out, failed, or threw records a reason and Agent 7 installs and builds on its own path exactly as today; a run that did not ask for one (every local review) writes a plan byte-for-byte identical to before; an empty diff skips it, because the skill stops there before any agent runs. The switch is one literal on two sides, and the workflow test reads the CLI constant out of the source so a rename on either side reds a test instead of silently turning the prebuild off.
Replaces #10129. That PR fixed the same issue with a host-level dependency cache — lockfile-keyed entries on the persistent runners, link-farmed into the worktree. Five review rounds and four autofix rounds later it had grown from 1,500 to 2,836 lines and every standing Critical sat on the same two surfaces: cache entries living on a path the unsandboxed PR code can write to (seal → self-authored seal → root-own → unreadable entry → renamed cache root), and a "demand check" re-implementing
npm ci's manifest/lockfile sync in TypeScript, which the review measured as false-failing 12 of this repository's own workspace members — the cache never engaged here at all. The reviewer's own convergence note asked for the root cause rather than the next sibling; this is that. The review job runs for hours and the persistent pool installs and builds this repository in about four minutes, so there is nothing for a shared cache to buy that is worth a shared, mutable, cross-job trust surface. The closing comment on #10129 maps all 28 open Criticals to this design, cluster by cluster.Why it's needed
Issue #10108: the review worktree has no
node_modulesand no built workspacedist, and a full install plus the prerequisite builds does not fit inside an agent's tool budget. So every probe or chunk agent that decided the right evidence was "run the test" failed the same way, burned its budget on a doomed install, and the round downgraded to a read-only audit with a "Not explored to full depth (tool budget reached)" disclosure — PR #9729 rounds 13 and 15, and PR #9940's own review, which could not run the very tests that PR added. Probes settle verdicts that re-reading cannot; this blunted the strongest verification tool the skill has. The fix is to pay the install once, outside every agent's budget, with the machinery that already exists for it.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/lib/prebuild.test.ts src/commands/review/fetch-pr.test.ts— the prebuild's 9 cases (env gate incl. the.env-sourced refusal; the exactbuild-testarguments;installedread off npm's marker, notbuild-test's word;builtfalse when the budget cut the closure short; a throwingbuild-testrecorded, never raised; an unwritable report path) and 4 wiring cases infetch-pr(runs against the fresh worktree and lands in the plan; plan written first, rewritten with the outcome, and the session ledger appended AFTER the rewrite; absent and single-write when not asked for; skipped on an empty diff).npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js -t prebuild(theRun reviewstep sets the variable the CLI reads — read out ofprebuild.ts— and nothing else in the workflow does) andcd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts(the field, the switch and the no-hand-install rule are named).!emptyDiffgate → "skips the prebuild on an empty diff"; moveappendRunSessionbefore the rewrite → "ledgers the FINAL write"; ignorenotBuilt→ "cut the closure short"; rename the YAML literal → the workflow pin (expected undefined to be '1'). Files restored byte-identical (cmp) after each.fetch-pr's stderr showsPrebuilt the worktree in <N>s: dependencies installed and the scoped build closure compiled; build-test will find both in place.(orPrebuild did not complete …; build-test installs and builds on its own path as before.), the fetch report carriesdependencies, and a chunk agent's probe can run a test before Agent 7 finishes.Evidence (Before & After)
End to end from source (
node scripts/dev.js review fetch-pr 10310 QwenLM/qwen-code --out …, PR #10310 touchespackages/cli), same machine, same PR:Before (no
QWEN_REVIEW_PREBUILD): report has nodependencies;.qwen/tmp/review-pr-10310/node_modules: No such file or directory;packages/core/dist: No such file or directory — the bare tree #10108 describes.After (
QWEN_REVIEW_PREBUILD=1): stderr:Prebuilt the worktree in 390s: dependencies installed and the scoped build closure compiled; build-test will find both in place.The report carriesdependencies: { installed: true, built: true, note: "Built 19 of 25 workspaces (the 2 the diff changes, plus what they compile against). Tests were not run (build-only).", report: ".qwen/tmp/qwen-review-pr-10310-prebuild.json", durationMs: 389566 }; thebuild-testreport at.qwen/tmp/qwen-review-pr-10310-prebuild.jsonrecordstoolchain: npm,affected: [packages/cli, packages/core], a 19-workspacebuildSet(core, acp-bridge, the channels, sdk-typescript, cli, webui, web-shell, …),notBuilt: null,timedOut: [],ok: true,install.command: npm ci --no-audit --no-fund. A probe in the worktree with no install of its own:cd .qwen/tmp/review-pr-10310/packages/cli && npx vitest run src/commands/review/run.test.ts src/commands/review/compose-review.test.ts→ 701 passed, andpackages/core … SKILL.test.ts→ 56 passed — the PR's own test files, the exact shape PR #9940's review could not run.qwen review cleanup pr-10310then removed the worktree as usual.Typecheck:
packages/clitsc --noEmitreports no error in the touched files (the 168 pre-existing errors are all inacp-integration/serve/ui, from this worktree's stale coredist). ESLint and Prettier clean on every touched file;actionlinton the workflow reports only the pre-existing SC2012 note in "Clean review worktrees";yamllintclean.Tested on
Environment (optional)
macOS, Node 24.18.1 / npm 11.16.0, vitest 3.2.7; the end-to-end run used the source CLI via
scripts/dev.jsagainst the real GitHub PR and the real registry, sandbox policyoff(the default, and what CI runs).Risk & Scope
built: falseand Agent 7 rebuilds as today rather than resuming the prebuild'sbuild-testrun; hosted fallback runners get the prebuild too (nothing here is pool-specific).mainalready sits 3,706 bytes over the recorded baseline forqwen-code-pr-review.yml(allowance 4,096), so the step comment is a three-line pointer atlib/prebuild.tsand the file lands at 169,847 bytes — 125 bytes under the gate. Ifmaingrows again before this merges, the baseline entry needs a re-record, not this PR's prose.Linked Issues
Fixes #10108
Replaces #10129
中文说明
这个 PR 做了什么
在 CI 上，评审 worktree 现在会在任何 agent 启动之前完成安装与编译。当评审 workflow 设置
QWEN_REVIEW_PREBUILD=1时，fetch-pr在写完 plan 之后立刻对刚创建的 worktree 运行 Agent 7 自己的build-test --install --build-only：与 Agent 7 几分钟后要跑的完全相同的命令、环境、沙箱策略与定向构建闭包，唯一的区别是它在编排器的时钟上运行，预算按 workflow 步骤（30 分钟）而非 shell 工具调用（10 分钟）来定。结果作为数据落入 fetch 报告（dependencies：installed读自 npm 自己的完整性标记——正是build-test读取的那道闸；built是build-test对闭包的裁决；一行note；以及本次build-test报告的路径），plan 随之重写，会话账本记录的是最终那次写入，保证 run-epoch 栅栏键在正确的 mtime 上。内置评审 skill 文档化了该字段并保留「绝不手动安装」规则——在已预构建的树上这条规则只会更严，不会更松。这里不决定跑什么、以谁的身份、用什么环境——只决定何时。一切都朝今天的行为回退：prebuild 超时、失败或抛错都记录原因，Agent 7 照旧在自己的路径上安装与构建；没有要求 prebuild 的运行（所有本地评审）写出的 plan 与之前逐字节一致；空 diff 跳过，因为 skill 在那里就会停下、不会启动任何 agent。开关是横跨两侧的同一个字面量，workflow 测试从源码里读取 CLI 常量，任一侧改名都会让测试变红，而不是悄悄把 prebuild 关掉。
替代 #10129。 那个 PR 用宿主级依赖缓存修同一个 issue——持久 runner 上按锁文件键控的条目，以链接农场映进 worktree。五轮评审、四轮 autofix 之后，它从 1,500 行涨到 2,836 行，而仍然站着的每条 Critical 都落在同两个面上：缓存条目躺在未沙箱化的 PR 代码可写的路径上（封印 → 自签封印 → root 属主 → 条目不可读 → 缓存根被改名），以及一个在 TypeScript 里重写
npm ci清单/锁文件同步检查的「demand check」，评审实测它对本仓库自己的 12 个 workspace 成员全部误判——缓存在这里从未生效过。评审自己的收敛提示要求归因而不是再修一个兄弟入口；这就是归因的结果。评审 job 一跑数小时，持久池安装+构建本仓库约四分钟，共享缓存能省下的东西不值得一个共享、可变、跨 job 的信任面。#10129 的关闭评论按簇把 28 条未决 Critical 逐一映射到本设计。为什么需要
Issue #10108：评审 worktree 没有
node_modules也没有构建产物，完整安装加前置构建放不进单个 agent 的工具预算。于是每个判断「正确证据是把测试跑起来」的 probe/chunk agent 都以同样方式失败，预算烧在注定失败的安装上，轮次降级为只读审计并披露 "Not explored to full depth (tool budget reached)"——PR #9729 第 13、15 轮，以及 PR #9940 自己的评审（连该 PR 新增的测试都跑不了）。probe 能裁决重读裁决不了的问题；这钝化的恰是这个 skill 最强的验证手段。修法是用已有的机制、在所有 agent 预算之外只付一次安装。评审测试计划
如何验证
cd packages/cli && npx vitest run src/commands/review/lib/prebuild.test.ts src/commands/review/fetch-pr.test.ts——prebuild 的 9 个用例（环境门控含.env来源拒绝；传给build-test的精确参数；installed读自 npm 标记而非build-test的说法；预算截断闭包时built为 false；build-test抛错被记录而非上抛；报告路径不可写）以及fetch-pr的 4 个接线用例（对新建 worktree 运行并落入 plan；先写 plan、再以结果重写、会话账本在重写之后追加；未要求时缺席且只写一次；空 diff 跳过）。npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js -t prebuild（Run review步骤设置的正是 CLI 读取的变量——从prebuild.ts源码读出——且 workflow 中别处都不设）与cd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts（字段、开关与「不手动安装」规则都有提及）。!emptyDiff门 → "skips the prebuild on an empty diff"；把appendRunSession挪到重写之前 → "ledgers the FINAL write"；忽略notBuilt→ "cut the closure short"；改 YAML 字面量 → workflow 钉住（expected undefined to be '1'）。每次之后cmp确认文件逐字节恢复。fetch-pr的 stderr 出现Prebuilt the worktree in <N>s: dependencies installed and the scoped build closure compiled; build-test will find both in place.（或Prebuild did not complete …; build-test installs and builds on its own path as before.），fetch 报告带dependencies，chunk agent 的 probe 可以在 Agent 7 完成前跑测试。证据（Before & After）
从源码端到端（
node scripts/dev.js review fetch-pr 10310 QwenLM/qwen-code --out …，PR #10310 触及packages/cli），同一台机器、同一个 PR：Before（不设
QWEN_REVIEW_PREBUILD）：报告无dependencies；.qwen/tmp/review-pr-10310/node_modules: No such file or directory；packages/core/dist: No such file or directory——正是 #10108 描述的裸树。After（
QWEN_REVIEW_PREBUILD=1）：stderr:Prebuilt the worktree in 390s: dependencies installed and the scoped build closure compiled; build-test will find both in place.报告带dependencies: { installed: true, built: true, note: "Built 19 of 25 workspaces (the 2 the diff changes, plus what they compile against). Tests were not run (build-only).", report: ".qwen/tmp/qwen-review-pr-10310-prebuild.json", durationMs: 389566 }；.qwen/tmp/qwen-review-pr-10310-prebuild.json的build-test报告记录toolchain: npm,affected: [packages/cli, packages/core], a 19-workspacebuildSet(core, acp-bridge, the channels, sdk-typescript, cli, webui, web-shell, …),notBuilt: null,timedOut: [],ok: true,install.command: npm ci --no-audit --no-fund。在 worktree 里不做任何安装直接跑 probe：cd .qwen/tmp/review-pr-10310/packages/cli && npx vitest run src/commands/review/run.test.ts src/commands/review/compose-review.test.ts→ 701 passed, andpackages/core … SKILL.test.ts→ 56 passed — the PR's own test files, the exact shape PR #9940's review could not run。随后qwen review cleanup pr-10310照常移除 worktree。类型检查：
packages/cli的tsc --noEmit在所改文件上零错误（既有的 168 个错误全在acp-integration/serve/ui，源于本 worktree 陈旧的 coredist）。所有触及文件 ESLint 与 Prettier 干净；workflow 的actionlint只报「Clean review worktrees」里既有的 SC2012 提示；yamllint干净。测试平台
环境（可选）
macOS，Node 24.18.1 / npm 11.16.0，vitest 3.2.7；端到端运行通过
scripts/dev.js使用源码 CLI，对真实 GitHub PR 与真实 registry，沙箱策略off（默认值，也是 CI 的运行方式）。风险与范围
built: false，Agent 7 照今天那样重建而不是续跑 prebuild 的build-test；托管 fallback runner 同样获得 prebuild（这里没有任何池特有的东西）。main自身已比qwen-code-pr-review.yml的记录基线大 3,706 字节（余量 4,096），所以步骤注释压成三行、指向lib/prebuild.ts，文件落在 169,847 字节——距闸门 125 字节。若合入前main再增长，需要重记基线条目，而不是再压本 PR 的文字。关联 Issue
Fixes #10108
Replaces #10129