fix(review): run verifier probes in a private scratch worktree (#9207) - #9221
fix(review): run verifier probes in a private scratch worktree (#9207)#9221wenshao wants to merge 2 commits into
Conversation
Step 4's verifier is the review's one writing agent: it writes a probe, runs it, applies the one-line fix its flip-check needs, and restores. All of that landed in the shared review worktree — the tree `working_dir` pins every other agent to — and the pipelined loop launches a round's verifiers alongside the NEXT round's reverse auditors, so those writes are live exactly while the auditors read. "Leave the tree as you found it", which the brief has always said and verifiers do obey, cannot close that window: the exposure is *during* the probe. Measured on a live run, a round-5 auditor read a probe's mutant plus a leftover probe test and came within a step of filing a Critical against code no commit contains; it recovered only by improvising `git show HEAD:`, a fallback no brief mentions. Three parts, because isolation alone is a guarantee one regression away from being false: - `qwen review scratch-tree` gives each verifier shard a throwaway sibling worktree at the commit under review, with the review worktree's node_modules linked in so a unit harness starts without an install. Every call hands back a pristine tree (a previous finding's mutant surviving into the next probe would be a wrong verdict carrying a deterministic source tag); the label is the shard's record key, because the shards of one round run concurrently; and a tree it cannot create makes the probe inconclusive rather than falling back to the shared worktree. This is the isolation the test-efficacy probe has had since #6832 and the A/B's base tree has, extended to the last step that writes. - Every code-reading brief — dimension agents, chunk agents, reverse auditors and the verifier itself — now carries the rule that auditor had to invent: the worktree is shared, code that is not in the diff and not in the commit is not a finding, and anything surprising is judged against `git show HEAD:`. - `agent-prompt` reads the tree once per call, and every wave of agents is built by it immediately before launch: residue is named in each brief it builds and warned about on stderr, so a contaminated tree is announced to the agents about to read it instead of being discovered as a phantom Critical. `exposeDependencies` moves to `lib/worktree.ts` beside the other disposable-tree machinery, and now farms each workspace member's own node_modules as well as the root's. Measured on this repo, a tree with 1560 root packages linked still could not resolve `@testing-library/react` for a UI probe, because npm could not hoist that copy out of `packages/cli` — which also silently cost the efficacy probe the same class of test. `cleanup` sweeps the scratch family by prefix; the label half is the shard's key and cannot be reconstructed by the sweeper.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ — all sections present, bilingual, honest N/A on the before/after evidence since there is no user-visible change. Problem: observed, not theoretical. Issue #9207 documents a concrete incident — reviewing PR #9118 at Direction: aligned. This is the review pipeline's own correctness, and it extends a pattern the pipeline already uses twice — the efficacy probe's Size: 874 production lines, all in Approach: the scope feels right. My independent read of the problem lands on the same shape: per-verifier-shard worktrees keyed by the shard's record key (a shared scratch tree would just move the race one level down), isolation failure → inconclusive probe rather than fallback to the shared tree, the reader-side Risk: no high-risk path matches (checked against the revert-correlated path list); no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各节齐全、双语，无可视变化的前后对比证据如实标注 N/A。 问题：已观测到，而非理论推测。 Issue #9207 记录了具体事件——在 方向：对齐。这是审查流水线自身的正确性，且延续了流水线已有的两次同款模式——efficacy 探针的 规模：874 行生产代码，全部位于 方案：范围合理。我对该问题的独立推演得到同样的形状：按验证员分片的 record key 命名的独立工作树（共用一棵 scratch tree 只是把竞态下移一层）、隔离失败时探针判 inconclusive 而非回退共享树、把 风险：未命中高风险路径（已对照 revert 相关路径清单）；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI started from an independent proposal — per-verifier-shard sibling worktrees at the PR head, keyed by the shard's record key, inconclusive-on-failure instead of fallback, plus a reader-side No critical blockers found. The spots most likely to break were checked against the base tree and hold up:
Two cosmetic edges I'm not blocking on: How the write path changessequenceDiagram
participant P1 as Orchestrator agent-prompt
participant P2 as Verifier shard
participant P3 as Scratch tree per shard
participant P4 as Shared review worktree
participant P5 as Reverse auditors next round
P1->>P4: read residue once per wave
P1->>P2: brief with scratch-tree command and shard label
P2->>P3: create or reset to pristine
P2->>P3: probe, mutant, candidate fix
P3-->>P2: verdict, or inconclusive if isolation failed
P5->>P4: reads only the commit
Files changed (18 of 18 shown)
Testing evidence — the PR's own CI (unattended run: no PR code executed here)Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The Ubuntu unit suite was still running at review time (it takes ~30 minutes; not polling — the finalize workflow updates this table once CI settles). The macOS and Windows test legs and the no-sandbox integration leg report skipped per the repo's CI configuration, and security checks are green. The author reports one failing unit test ( Sandboxed verification would settle the remaining behavioural gap: 中文说明代码审查我从一个独立方案出发——按验证员分片、以 record key 命名的兄弟工作树，隔离失败判 inconclusive 而非回退，加上读取侧的 未发现关键阻塞问题。 最容易出错的几处都对照基线核实过：
两个不阻塞的小边角： 测试证据无人值守 CI 运行：此处不执行 PR 代码，证据为 PR 自身 CI（表格见上）。审查时 Ubuntu 单测仍在运行（约 30 分钟，不轮询，finalize 工作流会在 CI 落定后更新表格）；macOS/Windows 测试腿与无沙箱集成腿按仓库 CI 配置显示 skipped；安全检查已通过。作者报告唯一失败的单测（ 沙箱验证可补上剩余的行为缺口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — observed race, clean pattern-conforming implementation, thorough real-repo tests; the last point is withheld only because CI hasn't settled and the live pipelined run wasn't driven end-to-end. Stepping back: this is what a good fix in this part of the codebase looks like. The problem is real and documented — #9207 records the exact run where an auditor nearly filed a Critical against probe residue — and the PR resists the tempting half-fix. "Tell verifiers to restore faster" or "tell auditors to be careful" would each leave the window open; instead it removes the write from the shared tree entirely, then hardens the readers anyway, because a guarantee is one regression away from being false. That second half is the part that convinced me the author was thinking about the next failure, not just this one. The implementation matches my independent proposal and stays inside the pattern the pipeline already established twice ( Non-blocking reservations, all disclosed by the author: Windows and Linux weren't exercised locally (the junction/symlink path is the same one the Verdict: approve. The Ubuntu suite is still in flight on 中文说明置信度：4/5 —— 问题已观测、实现符合既有模式、针对真实 git 仓库的测试充分；扣掉的一分仅因 CI 尚未落定、且未驱动一次完整的流水线实跑。 总体看：这是这部分代码库中一个高质量修复的样子。问题真实且有据可查——#9207 记录了审计员差一点把探针残留报成 Critical 的那次运行——而 PR 抵住了"半成品修复"的诱惑。"让验证员更快还原"或"让审计员小心一点"都关不上窗口；本 PR 把写入彻底移出共享树，同时又加固读取方——因为一个保证距离失效永远只差一次回归。后半部分让我相信作者想的是下一次失败，而不只是这一次。 实现与我的独立方案一致，且停留在流水线已确立两次的模式内（ 非阻塞保留项（作者均已披露）：Windows 与 Linux 未在本地跑过（junction/symlink 走的是 结论：approve。Ubuntu 套件仍在该提交上运行，批准推迟到 CI 在该提交上全绿——finalize 任务会在全部转绿时代为提交绑定该 commit 的批准；若有检查转红或 head 移动，将在状态评论中标记。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): lib/stale-bundle.test.ts — no such file or directory; lib/worktree.test.ts — no such file or directory; 647 passed — this review observed 20170, 19986, 1432, 1555, 494 passed; 1 passed — this review observed 20170, 19986, 1432, 1555, 494 passed.
中文说明
Test Plan（非阻断）：lib/stale-bundle.test.ts — no such file or directory; lib/worktree.test.ts — no such file or directory; 647 passed — this review observed 20170, 19986, 1432, 1555, 494 passed; 1 passed — this review observed 20170, 19986, 1432, 1555, 494 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| 'without an install.', | ||
| '', | ||
| '```bash', | ||
| `"\${QWEN_CODE_CLI:-qwen}" review scratch-tree --worktree ${resolve(wt)} \\`, |
There was a problem hiding this comment.
[Critical] The welded scratch-tree command interpolates the --worktree path UNQUOTED into a shell command — every probe silently dies on checkouts whose path contains a space or apostrophe.
The command welded into every verifier brief sanitizes --label to be shell-inert (scratchLabel), but interpolates --worktree ${resolve(wt)} bare — shellQuotePath is already imported in this file (:87) and is used for a path in the same position at :1412. Reported independently by four reverse auditors.
Failure scenario: any checkout under a path with a space or apostrophe (~/Documents/John's Projects/… — lib/shell-quote.ts's docstring calls this "an ordinary macOS workspace") → word-split or unterminated-quote command → scratch-tree fails for every verify shard → every probe falls back to the reading-based verdict and the low-confidence floor: the isolation this PR exists to provide is silently dead on that machine class.
Witness (probe against real buildRoleBrief, repo root /home/foo/My Projects/qwen-code):
welded command executed verbatim in bash:
ARGV[3]: /home/foo/My
ARGV[4]: Projects/qwen-code/.qwen/tmp/review-pr-6766 ← stray positional
runScratchTree({worktree:'/home/foo/My'}) →
{"available": false, "note": "the review worktree /home/foo/My does not exist"}
apostrophe root → bash: unexpected EOF while looking for matching `'` (exit 2)
single-quoting the path passes it as one argv intact (flip)
Suggested fix: --worktree ${shellQuotePath(resolve(wt))}; update the new test's --worktree expectation to the quoted form. (The pre-existing build-test/test-efficacy weldings at ~:1489/:1512 share the latent pattern; only this new weld is in scope here.)
中文说明
严重性：Critical
焊入每份验证员 brief 的 scratch-tree 命令将 --worktree ${resolve(wt)} 不加引号地插入 shell 命令——旁边的 --label 专门做了 shell 安全化（scratchLabel），而 shellQuotePath 已在本文件导入（:87），且 :1412 的同类位置已在使用。四位反向审计员独立报告了同一问题。
失败场景： 仓库检出路径含空格或撇号（如 ~/Documents/John's Projects/…，lib/shell-quote.ts 注释称之为"普通 macOS 工作区"）→ 命令被词分割或出现未闭合引号 → 每个验证分片的 scratch-tree 调用都失败 → 所有探针退化为基于阅读的低置信度判定：本 PR 要提供的隔离能力在这类机器上静默失效。
证据： 用真实 buildRoleBrief + 仓库根 /home/foo/My Projects/qwen-code 探针：bash 逐字执行焊接命令 → argv 被拆为 /home/foo/My 与游离的 Projects/...；runScratchTree({worktree:'/home/foo/My'}) 返回 available: false；撇号路径直接 unexpected EOF（exit 2）；给路径加引号后 argv 完整（翻转成立）。
修复建议： 改用 --worktree ${shellQuotePath(resolve(wt))}，并把新测试的 --worktree 期望改为带引号形式。（build-test/test-efficacy 的既有焊接点存在同样隐患，但只有本新增焊接点属于本 diff 范围。）
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const tree = scratchWorktreePath(worktree, args.label); | ||
| if (existsSync(tree) && resetScratchTree(tree, headSha)) { |
There was a problem hiding this comment.
[Critical] The reuse gate checks existsSync(tree) only — when a bare (unregistered) directory stands at the scratch path, the reset's git commands escape via git's upward discovery and FORCE-RESET THE PARENT CHECKOUT, then report success.
The scratch sibling sits inside the parent checkout's working tree (verified live with git worktree list), and the PR head SHA resolves in the parent's shared object store. A bare directory at this path is the leftover of a crashed git worktree add — the exact class this file's header comment says the sweep exists to clear — or of a cleanup whose git worktree remove --force succeeded but whose rmSync failed (EPERM/EBUSY; worktreeReleaseResult in lib/git.ts acknowledges that outcome). The pre-sweep cannot save it: discardWorktree runs only after this gate fails.
Failure scenario: checkout --force --detach <sha> from a bare-directory cwd discards tracked-file local changes repo-wide in the parent checkout and detaches its HEAD onto the PR SHA; rev-parse HEAD then equals the SHA, so resetScratchTree returns true → available: true, reused: true pointing the verifier at a directory that is not a worktree. The user's uncommitted tracked work is silently lost. The existing test 'clears a leftover directory a crashed run left at the path' passes under this escape (the test repo happens to sit at the same SHA) — it never asserts reused or registration.
Witness (probe, real runScratchTree, git 2.47.3):
PR as-shipped:
report.reused: true | scratch has .git: false
parent branch: refs/heads/main → DETACHED
parent a.ts: 'LOCAL UNCOMMITTED WORK' discarded ('committed content' after)
with reuse additionally gated on existsSync(resolve(tree, '.git')):
report.reused: false | scratch has .git: true
parent stays on refs/heads/main, uncommitted change intact (flip)
| const tree = scratchWorktreePath(worktree, args.label); | |
| if (existsSync(tree) && resetScratchTree(tree, headSha)) { | |
| const tree = scratchWorktreePath(worktree, args.label); | |
| if (existsSync(tree) && existsSync(resolve(tree, '.git')) && resetScratchTree(tree, headSha)) { |
中文说明
严重性：Critical
复用分支只检查 existsSync(tree) —— 当 scratch 路径上是一个未注册的裸目录时，重置命令会经 git 的向上发现逃逸到外层仓库，强制重置父检出，然后报告成功。scratch 兄弟树位于父检出工作树内部（已用 git worktree list 实测确认），PR head SHA 可在父仓库对象库中解析。裸目录的来源：崩溃的 git worktree add 残留（文件头注释明确说清扫机制就是为这类残留而设），或清理时 git worktree remove --force 成功但 rmSync 失败（EPERM/EBUSY；lib/git.ts 的 worktreeReleaseResult 承认该状态）。预清扫救不了它：discardWorktree 只在此门失败后才执行。
失败场景： 在裸目录 cwd 下执行 checkout --force --detach <sha> 会在父检出中全仓库丢弃已跟踪文件的本地修改并将其 HEAD 分离到 PR SHA；随后 rev-parse HEAD 等于该 SHA，resetScratchTree 返回 true → available: true, reused: true，把验证员指向一个并非工作树的目录。用户未提交的跟踪修改被静默丢失。现有测试"clears a leftover directory…"在该逃逸下也能通过（测试仓库恰好处于同一 SHA），它从未断言 reused 或注册状态。
证据： 真实 runScratchTree 探针（git 2.47.3）：原代码 → reused: true、scratch 无 .git、父分支 main → DETACHED、父检出未提交修改被丢弃；加上 .git 存在性门后 → reused: false、scratch 有 .git、父检出不受影响（翻转成立）。
修复建议： 复用门改为要求注册状态（如 existsSync(resolve(tree, '.git')) 或出现在 git worktree list），否则落入既有的 discard+add 路径。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| - The worktree is automatically cleaned up after the review completes | ||
| - If a review is interrupted (Ctrl+C, crash), the next `/review` of the same PR automatically cleans up the stale worktree before starting fresh | ||
| - Review reports and cache are saved to the main project directory (not the worktree) | ||
| - Steps that **modify** code to measure something — the test-efficacy probe's mutants, and a verifier's probe of a specific finding — each run in their own throwaway worktree beside it (`…-probe`, `…-scratch-<agent>`), so the tree every other agent is reading always holds exactly the commit under review. All of them are swept along with the worktree at the end of the review. |
There was a problem hiding this comment.
[Suggestion] "the tree every other agent is reading always holds exactly the commit under review" is an unconditional isolation guarantee that the same diff's agent-facing text contradicts.
worktreeEvidenceBlock (added by this PR) tells every reader "a stray uncommitted change here is still possible", and DESIGN.md in the same diff calls the isolation "a guarantee … one regression away from being false". The user-facing page presents as an invariant what the code declines to guarantee; an operator reads the tripwire as belt-and-braces when it is the load-bearing detection for a real, recurring contamination.
Failure scenario: concrete divergence mechanisms exist in this very diff (gitignored residue surviving the reuse reset; farm-symlink writes propagating into the shared worktree — see the findings on resetScratchTree and farmNodeModules). A user or operator relying on this page believes contamination cannot happen.
Suggested fix: soften to the actual contract, e.g. "…so a write by one agent is not normally visible to the others reading the shared tree; the briefs additionally name any paths that differ from the commit at build time."
中文说明
严重性：Suggestion
"其他所有 agent 读取的树始终恰好是被审提交"是一个无条件的隔离保证，而同一 diff 中面向 agent 的文本与之矛盾：worktreeEvidenceBlock 明确写着"这里仍可能出现游离的未提交修改"，DESIGN.md 也称该隔离是"距失效仅一次回归的保证"。用户文档把代码并不保证的东西写成不变量，读者会把绊线机制当成双保险，而它实际上是针对真实、反复发生的污染的主要检测手段。
失败场景： 本 diff 自身就包含具体的分歧机制（gitignore 残留在复用时幸存；farm 符号链接写入传播到共享工作树）。依赖本页的用户/运维会认为污染不可能发生。
修复建议： 改为实际契约，例如"……因此某个 agent 的写入通常不会被读取共享树的其他 agent 看到；brief 还会在构建时列出与提交不一致的路径。"
— qwen3.8-max via Qwen Code /review (v0.21.12)
| import { | ||
| exposeDependencies, | ||
| worktreeCreateFailureDetail, | ||
| } from './lib/worktree.js'; |
There was a problem hiding this comment.
[Suggestion] The diff re-homes exposeDependencies to lib/worktree.js but leaves the entire describe('exposeDependencies') block — including this PR's NEW workspace-member farming test — in test-efficacy.test.ts; lib/worktree.test.ts exists and does not cover it. Tests are no longer collocated with the source they test.
Failure scenario (concrete cost): a future change to exposeDependencies/farmNodeModules in lib/worktree.ts gets edited against the colocated lib/worktree.test.ts, which contains no farm coverage; the tests pinning the new workspace-member behavior live in a file named for the function's former owner, so the coverage is harder to find and easier to leave stale.
Rule: AGENTS.md § Code conventions — "Tests: Collocated with source (file.test.ts next to file.ts)".
Suggested fix: move the exposeDependencies (and worktreeCreateFailureDetail) describe blocks to lib/worktree.test.ts in the same change that moved the functions (the block is self-contained, using only mkdtemp fixtures).
中文说明
严重性：Suggestion
diff 把 exposeDependencies 移到 lib/worktree.js（测试 import 也随之移动），但整个 describe('exposeDependencies') 块——包括本 PR 新增的 workspace 成员 farming 测试——仍留在 test-efficacy.test.ts；lib/worktree.test.ts 已存在却不覆盖它。测试不再与被测源码同置。
失败场景（具体代价）： 未来修改 lib/worktree.ts 中的 exposeDependencies/farmNodeModules 时会面对同置的 lib/worktree.test.ts，而那里没有任何 farm 覆盖；钉住新行为的测试待在以函数旧宿主命名的文件里，更难被发现、更容易过期。
规则：AGENTS.md § Code conventions —— "Tests: Collocated with source"。
修复建议： 在移动函数的同一改动中，把 exposeDependencies（与 worktreeCreateFailureDetail）describe 块移入 lib/worktree.test.ts（该块自包含，仅用 mkdtemp 夹具）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (brief.reviewsCode) { | ||
| parts.push(...worktreeEvidenceBlock(report, opts.residue)); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The shared-worktree reader rule and residue naming reach only buildRoleBrief and buildChunkAgentPrompt; the --whole-diff launch class (Agent 8 diff-specialists) reads the same shared worktree but gets neither.
buildWholeDiffBlock (:741) never includes :2804-2806) bypasses worktreeEvidenceBlock, and the args.wholeDiff branch (buildLaunch/residue entirely, though residue is computed one scope up at :2609. Agent 8 is a code-reading agent pinned to the PR worktree by working_dir like every other workflow agent, launched 0–2 per high-effort review when one domain dominates; SKILL.md builds it via exactly agent-prompt … --whole-diff.
Failure scenario: if the shared tree carries residue at the moment an Agent 8 block is built — the exact #9207 shape this PR responds to — that agent reads the mutant/probe file as the PR's own code with no git show HEAD: rule and no named dirty paths, and can file a phantom Critical. The stderr tripwire does fire on the --whole-diff call, but only the orchestrator sees stderr — the agent never does.
Witness (probe): real agent-prompt --whole-diff against a plan whose worktree was dirty — stderr warning fired naming the dirty paths, while the recorded whole-diff prompt contained 'SHARED review worktree': false, 'not clean': false, residue paths: false, git show HEAD: false.
Suggested fix: append ...worktreeEvidenceBlock(report, residue) in buildWholeDiffBlock (pass residue in or compute via worktreeResidueOf(report)), mirroring the chunk-brief call site.
中文说明
严重性：Suggestion
共享工作树读取规则与残留命名只进入 buildRoleBrief 和 buildChunkAgentPrompt；--whole-diff 启动类（Agent 8 领域专家）读取同一棵共享工作树，却两者都拿不到。buildWholeDiffBlock（:741）从不含 :2804-2806）完全绕过 worktreeEvidenceBlock，args.wholeDiff 分支（buildLaunch/residue——尽管 residue 就在上一层 :2609 计算。Agent 8 与其他所有工作流 agent 一样被 working_dir 钉在 PR 工作树上，高强度审查中按领域集中度发射 0–2 个；SKILL.md 正是用 agent-prompt … --whole-diff 构建它。
失败场景： 若构建 Agent 8 块时共享树带有残留——正是本 PR 响应的 #9207 形态——该 agent 会把突变/探针文件当作 PR 自身代码读取，既无 git show HEAD: 规则也无脏路径命名，可能提交幻影 Critical。stderr 绊线在 --whole-diff 调用时确实触发，但只有编排者能看到 stderr，agent 看不到。
证据： 探针：对带脏 worktree 的计划执行真实 --whole-diff 调用——stderr 告警触发并点名脏路径，而记录在案的 whole-diff 提示词中 'SHARED review worktree'：false、'not clean'：false、残留路径：false、git show HEAD：false。
修复建议： 在 buildWholeDiffBlock 中追加 ...worktreeEvidenceBlock(report, residue)（传入 residue 或内部用 worktreeResidueOf(report) 计算），与 chunk brief 调用点对称。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const r = spawnSync( | ||
| 'git', | ||
| ['status', '--porcelain', '--untracked-files=normal'], |
There was a problem hiding this comment.
[Suggestion] --untracked-files=normal collapses directory-shaped contamination to a single trailing-slash dir/ entry, which the same PR's restore/evidence commands cannot act on and the new tests never pin.
Failure scenario (probe-verified, git 2.47): an agent writes probe files into a NEW directory in the shared review worktree (e.g. fixtures/probe.test.ts — the contamination class this feature exists to catch). Porcelain emits ?? probe_dir/, so the residue list is ['probe_dir/']; the scratch-tree note then advises git checkout -- probe_dir/ → error: pathspec 'probe_dir/' did not match any file(s) known to git (exit 1), and the reader brief's mandated git show HEAD:probe_dir/ → fatal: path 'probe_dir/' exists on disk, but not in 'HEAD' (exit 128). Both mechanical recoveries this diff ships fail on the entry the probe itself produces; the reader is left to improvise. Distinct from the git show HEAD: and index-vs-HEAD findings: fixing those does not un-collapse this entry. worktree.test.ts pins only file-shaped residue at the repo root.
Suggested fix: use --untracked-files=all so residue entries are file-level — the same probe shows it emits ?? probe_dir/probe.test.ts, individually actionable (the cap already bounds list growth; pair this with the maxBuffer fix below, since -uall grows output). If normal is kept deliberately, branch the restore advice for entries ending in / and pin the directory case in a test.
中文说明
严重性：Suggestion
--untracked-files=normal 把目录形态的污染折叠为单个带尾斜杠的 dir/ 条目，同一 PR 提供的恢复/取证命令对它都无法作用，新测试也从未钉住该形态。
失败场景（探针验证，git 2.47）： agent 在共享 review worktree 的新目录里写探针文件（如 fixtures/probe.test.ts——本功能要捕捉的污染类别）。porcelain 输出 ?? probe_dir/，残留列表为 ['probe_dir/']；scratch-tree 附注建议 git checkout -- probe_dir/ → error: pathspec 'probe_dir/' did not match any file(s) known to git（exit 1），读取方 brief 强制的 git show HEAD:probe_dir/ → fatal: path 'probe_dir/' exists on disk, but not in 'HEAD'（exit 128）。本 diff 提供的两种机械恢复手段在探针自身产生的条目上都失败，读者只能临场发挥。与 git show HEAD: 及 index-vs-HEAD 两条 finding 不同：修复它们并不能展开这个条目。worktree.test.ts 只钉住仓库根部的文件形态残留。
修复建议： 使用 --untracked-files=all 使残留条目为文件级——同一探针显示它输出 ?? probe_dir/probe.test.ts，可逐条处置（上限已约束列表增长；与下方 maxBuffer 修复配套，因为 -uall 增大输出）。若刻意保留 normal，则为以 / 结尾的条目分岔恢复建议，并在测试中钉住目录情形。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| { cwd, encoding: 'utf8' }, | ||
| ); | ||
| if (r.error || r.status !== 0 || typeof r.stdout !== 'string') return []; |
There was a problem hiding this comment.
[Suggestion] This spawnSync runs with the default 1MB maxBuffer; residue larger than that kills the spawn (ENOBUFS, status: null) and the function returns [] — the contamination check silently reports clean, and the slice(0, cap) defence is downstream of the buffer that dies first.
Failure scenario (probe-verified): a shared review worktree whose porcelain output exceeds 1MB — roughly 26k individually dirty paths at ~40 bytes/line (fewer when core.quotepath octal-expands non-ASCII names), e.g. a bulk codemod/rename sweep or thousands of probe/fixture files dumped individually into tracked directories. Probe with the exact option shape of this code, over a tree with 30k individually-untracked ~44-byte-line paths (~1.3MB porcelain): PR code → error code: ENOBUFS | status: null | worktreeResidue result: []; with maxBuffer: 64MB → error: null | status: 0 | result length: 60000 (flip). scratch-tree then omits the NOT-clean warning and agent-prompt builds the residue classes on an empty list — auditors read residue as the PR's own code, the #9207 incident recurring in the overload case the cap's own doc comment anticipates. Corroborated in-repo: git.ts's gitRaw docstring states execFileSync "inherits … the 1 MB maxBuffer default, so any diff past ~1 MB dies with ENOBUFS". (Build output in a new directory does not reach this — it collapses per the finding above; the trigger is individually-listed paths.)
| { cwd, encoding: 'utf8' }, | |
| ); | |
| if (r.error || r.status !== 0 || typeof r.stdout !== 'string') return []; | |
| { cwd, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }, | |
| ); | |
| if (r.error || r.status !== 0 || typeof r.stdout !== 'string') return []; |
中文说明
严重性：Suggestion
此 spawnSync 使用默认 1MB maxBuffer；超过该大小的残留会杀死 spawn（ENOBUFS，status: null），函数返回 []——污染检查静默报告干净，而 slice(0, cap) 防线位于先死的缓冲区下游。
失败场景（探针验证）： porcelain 输出超过 1MB 的共享 review worktree——约 2.6 万条独立脏路径（每行 ~40 字节；core.quotepath 八进制展开非 ASCII 名称时更少），例如批量 codemod/重命名扫描，或数千个探针/夹具文件逐个倒入被跟踪目录。用与代码完全相同的选项形状、在含 3 万个独立未跟踪路径（~1.3MB porcelain）的树上探针：PR 代码 → error code: ENOBUFS | status: null | worktreeResidue result: []；maxBuffer: 64MB 时 → error: null | status: 0 | result length: 60000（翻转成立）。随后 scratch-tree 省略 NOT-clean 告警，agent-prompt 以空列表构建残留类别——审计员把残留当作 PR 自身代码读取，#9207 事故在 cap 文档注释早已预见的过载情形中复现。仓库内佐证：git.ts 的 gitRaw 注释写明 execFileSync "继承 1MB maxBuffer 默认值，超过 ~1MB 的 diff 会 ENOBUFS 而死"。（新目录中的构建产物不会触达——按上一条 finding 被折叠；触发条件是逐条列出的路径。）
修复建议： 显式传入 maxBuffer（如 32MB）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| return arrow === -1 ? rest : rest.slice(arrow + 4); | ||
| }) |
There was a problem hiding this comment.
[Suggestion] The residue cap truncates silently, and both consumers present the capped array as the COMPLETE dirty set — the scratch-tree note ("WARNING: the shared review worktree is NOT clean — … Restore it …") and worktreeEvidenceBlock ("These paths differ from the commit under review: …") — with no "and N more" disclosure. The sharedTreeResidue JSON field is likewise indistinguishable from a complete list.
Failure scenario (probe-verified): 13 dirty paths in a fixture repo → real worktreeResidue returned 12; f9.ts silently missing from the list every consumer presents as complete. A verifier restores exactly the 12 named paths and believes the tree is clean; the 13th stays in the tree concurrent agents read — the exact contamination the tripwire exists to surface — and no reader is told to distrust it. The cap test (worktree.test.ts "caps the list") pins the truncation itself, so nothing today flags the missing disclosure.
Suggested fix: carry the total past the cap and disclose it where the cap bites — e.g. append … and ${all.length - cap} more not listed to the returned list, or surface the count to the two renderers and let them say "…and N more".
中文说明
严重性：Suggestion
残留上限静默截断，而两个消费者都把截断后的数组当作完整的脏集合呈现——scratch-tree 附注（"WARNING: the shared review worktree is NOT clean — … Restore it …"）与 worktreeEvidenceBlock（"These paths differ from the commit under review: …"）——没有"另有 N 条"的披露。sharedTreeResidue JSON 字段同样无法与完整列表区分。
失败场景（探针验证）： 夹具仓库 13 条脏路径 → 真实 worktreeResidue 返回 12；f9.ts 从每个消费者当作完整呈现的列表中静默消失。验证员恰好恢复被点名的 12 条后认为树已干净；第 13 条留在并发 agent 读取的树里——正是绊线要暴露的污染——而没有任何读取方被告知应当怀疑。上限测试（worktree.test.ts "caps the list"）钉住的是截断本身，因此今天没有任何东西标记缺失的披露。
修复建议： 把超出上限的总数带出去并在截断处披露——例如返回列表追加 … and N more not listed，或把总数交给两个渲染器输出"另有 N 条"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // the farm it already has. | ||
| } | ||
| return done; |
There was a problem hiding this comment.
[Suggestion] { linked: 0, failed: 0 } conflates "farm already present" with "source had nothing linkable", and the fresh-build path renders the first reading — "Its node_modules was already in place." (scratch-tree.ts:272) — when nothing was linked at all.
Failure scenario (probe-verified): farmDependencies (scratch-tree.ts:250) guards only on existsSync(worktree/node_modules) — existence, not content. With the review worktree's node_modules in the partial-install shape (exists, holds only the file .package-lock.json — what a killed npm install leaves): fresh path → reused=false, dependencies={linked:0,failed:0}, note "…Its node_modules was already in place.…", scratch node_modules created with 0 entries; the reuse path repeats the same. exposeDependencies fresh-builds an empty farm and returns {0,0}; the verifier trusts the note, runs the harness, and hits the mysterious vitest: not found the null-branch note was written to prevent. The state self-perpetuates: the freshly created empty target now passes existsSync(target), so every later reuse call early-returns the same {0,0} and the same misleading note. No test distinguishes the two {0,0} causes.
Witness / flip: patched copy (farmDependencies returns null when a farm this call newly created linked nothing) → note "The review worktree has no node_modules…" on both paths.
Suggested fix: distinguish "target farm pre-existed" from "source had nothing linkable" — e.g. return null for a fresh-built empty farm, or pass the already-available reused flag into dependencyNote and emit "already in place" only on the reuse branch.
中文说明
严重性：Suggestion
{ linked: 0, failed: 0 } 把"farm 已存在"与"源头没有任何可链接内容"混为一谈，而新建路径会渲染第一种解读——"Its node_modules was already in place."（scratch-tree.ts:272）——即便什么都没链上。
失败场景（探针验证）： farmDependencies 只以 existsSync(worktree/node_modules) 为守卫——看存在性，不看内容。review worktree 的 node_modules 处于部分安装形态（存在但只有文件 .package-lock.json——npm 安装被杀后的残留）时：新建路径 → reused=false、dependencies={linked:0,failed:0}、附注 "…Its node_modules was already in place.…"，scratch 的 node_modules 被创建但含 0 个条目；复用路径重复同样结果。exposeDependencies 新建了一个空 farm 并返回 {0,0}；验证员相信附注、运行 harness，撞上 null 分支附注本来要防止的神秘 vitest: not found。状态自我固化：新建的空目录随后通过 existsSync(target)，后续每次复用调用都提前返回同样的 {0,0} 与同样的误导附注。没有测试区分两种 {0,0} 成因。
证据/翻转： 修补副本（本次调用新建的 farm 若什么也没链上则返回 null）→ 两条路径都输出 "The review worktree has no node_modules…"。
修复建议： 区分"目标 farm 预先存在"与"源头无可链接内容"——例如新建的空 farm 返回 null，或把已有的 reused 标志传入 dependencyNote，仅在复用分支输出 "already in place"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect( | ||
| scratchWorktreePath(worktreePath(7), 'verify--round-1--x').startsWith( | ||
| scratchWorktreePrefix(worktreePath(7)), | ||
| ), | ||
| ).toBe(true); |
There was a problem hiding this comment.
[Suggestion] This test — which claims to pin the creator↔sweeper prefix contract — is a tautology: scratchWorktreePath is DEFINED as scratchWorktreePrefix(worktree) + scratchLabel(label), so startsWith holds by construction for any value of the prefix. It can catch the prefix getting narrower, never the dangerous direction: the prefix getting broader while remaining a substring of the path.
Failure scenario (probe-verified): a future edit broadening scratchWorktreePrefix — e.g. returning resolve(worktree) without the -scratch- infix — leaves this assertion true, so it ships green. But cleanup.ts's scratchWorktreesOf feeds that prefix to entries.filter((path) => path.startsWith(prefix)) over the real temp dir and deletes every match: with prefix .../review-pr-7, PR 7's cleanup matches review-pr-70, review-pr-70-base, review-pr-70-scratch-* ('review-pr-70'.startsWith('review-pr-7') is true) and deletes a concurrent or subsequent PR-70 review's worktrees mid-run. Probe: baseline paths.test.ts 12/12 green; mutated arm (prefix without infix) → still 12/12 green while cleanup's exact sweep shape matched 5 of 5 sibling entries including another PR's live worktrees; restored infix → exactly 1 match. The trailing - in SCRATCH_INFIX is load-bearing and nothing pins it.
Suggested fix: pin the exact infix — expect(scratchWorktreePrefix(worktreePath(7))).toBe(${resolve(worktreePath(7))}-scratch-) — and/or assert non-collision: expect(scratchWorktreePath(worktreePath(70), 'x').startsWith(scratchWorktreePrefix(worktreePath(7)))).toBe(false).
中文说明
严重性：Suggestion
这个声称钉住"创建方↔扫除方前缀契约"的测试是重言式：scratchWorktreePath 的定义就是 scratchWorktreePrefix(worktree) + scratchLabel(label)，因此对任何前缀值 startsWith 都按构造成立。它只能捕获前缀变窄，捕获不到危险方向：前缀变宽但仍是路径子串。
失败场景（探针验证）： 未来把 scratchWorktreePrefix 改宽——例如返回 resolve(worktree) 而不带 -scratch- 中缀——该断言依然为真，全绿通过。但 cleanup.ts 的 scratchWorktreesOf 会把该前缀喂给 entries.filter((path) => path.startsWith(prefix)) 并删除所有匹配：前缀为 .../review-pr-7 时，PR 7 的清理会命中 review-pr-70、review-pr-70-base、review-pr-70-scratch-*（'review-pr-70'.startsWith('review-pr-7') 为真），在运行中删除并发或后续 PR-70 审查的工作树。探针：基线 paths.test.ts 12/12 全绿；突变分支（去掉中缀的前缀）→ 依然 12/12 全绿，而 cleanup 的同形扫除匹配了 5/5 个兄弟条目（含另一 PR 的活跃工作树）；恢复中缀 → 恰好 1 个匹配。SCRATCH_INFIX 末尾的 - 是承重字符，却没有任何测试钉住它。
修复建议： 钉住精确中缀——expect(scratchWorktreePrefix(worktreePath(7))).toBe(${resolve(worktreePath(7))}-scratch-)——和/或断言不碰撞：expect(scratchWorktreePath(worktreePath(70), 'x').startsWith(scratchWorktreePrefix(worktreePath(7)))).toBe(false)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…honesty gaps
Both Criticals were real, and both had the same shape: a claim the code made
that one machine class or one leftover state falsified.
- The welded `scratch-tree` command interpolated `--worktree` unquoted, so on
any checkout under a path with a space or an apostrophe every shard's
isolation was silently unavailable and every probe fell back to a reading.
Quoted with `shellQuotePath`, like every other path this file prints into a
command.
- The reuse gate checked only that the scratch path EXISTS. A bare directory
there — the leftover of a crashed `worktree add`, or of a cleanup whose
`rmSync` failed — has no `.git`, so git walked up and ran
`checkout --force --detach` against the user's own checkout: their
uncommitted work discarded, their HEAD detached onto the PR's commit, and
`rev-parse HEAD` then returning the sha that made the reset report success.
Gated on the tree being a registered worktree, with the regression pinned by
a test that fails on the un-gated code.
The rest are the same defect class at lower stakes — something stated more
broadly than it holds:
- The residue probe hand-parsed porcelain's rendered form, so a path with a
space, a non-ASCII byte, or a literal ` -> ` came back as a name matching
nothing on disk, `--untracked-files=normal` collapsed a whole probe directory
to one unactionable `dir/` entry, the 1 MB `maxBuffer` default answered the
dirtiest trees with "clean", and the cap truncated silently while both
renderers presented the list as complete. Now `-z --untracked-files=all` with
a 64 MB buffer, and a `{paths, total}` result both renderers disclose.
- `git checkout -- <path>` restores from the INDEX, so the advised recovery
left staged residue in the tree; it now says `git checkout HEAD --`.
- `git show HEAD:<path>` cannot produce an untracked path — the prototypical
residue — so the rule now says what that answer means instead of handing the
reader a command that exits 128.
- Agent 8's `--whole-diff` block reads the same shared worktree and got neither
the rule nor the residue paths; it is built outside `buildLaunch`, which is
exactly how it was missed.
- The stderr warning claimed every agent had been told; the block is gated on
`reviewsCode`, and Agent 7 — which builds and tests that tree — is not.
- A label that flattens to no path-safe character fell back to a shared
`agent` tree, `git clean -fd` left a nested repo standing while the report
said the tree was pristine, `dependencies: null` said "no node_modules"
about a farm that had failed to link, `{0,0}` read as "already in place" for
a `node_modules` holding nothing linkable, and `--out` was validated after
the tree and its farm already existed.
- The docs stated the isolation unconditionally: local-diff and file-path
reviews have no worktree and no scratch tree, and SKILL.md/DESIGN.md and the
user page now say so.
Tests moved with the code (`exposeDependencies` and `worktreeCreateFailureDetail`
now live beside `lib/worktree.ts`), and the plumbing that was pinned at both
ends but not in the middle — that a verify shard's recorded brief carries ITS
record key as the scratch label — is pinned by a test that fails when the key
is dropped.
|
Thanks — this was a good review, and both Criticals were real. Everything below is fixed in Criticals
Suggestions — all taken:
Two I want to answer rather than silently close:
@qwen-code /review 中文说明感谢审查——两个 Critical 都成立。以下全部已在 Critical
Suggestion —— 全部采纳：
有两条我想正面回应而不是默默关掉：
|
What this PR does
Step 4's verifier is the review's one writing agent: it writes a probe, runs it, applies the one-line fix its flip-check needs, and restores. All of that used to land in the shared review worktree — the tree
working_dirpins every other agent to — and the pipelined reverse-audit loop launches a round's verifiers alongside the next round's auditors, so those writes are live exactly while the auditors read. This PR gives each verifier shard a worktree of its own, tells every code-reading agent what a shared tree means, and adds a tripwire for the case where something writes into the shared tree anyway.A tree of its own. New subcommand
qwen review scratch-tree --worktree <wt> --label <the shard's record key>stands up a throwaway sibling worktree at the commit under review, with the review worktree'snode_moduleslinked in so a unit harness starts without an install. Three properties make it more than a directory: every call hands back a pristine tree (a previous finding's mutant surviving into the next probe would be a wrong verdict carrying a deterministic[probe]source tag); the label is the shard's record key, because the shards of one round run concurrently and a shared scratch tree would be the same race one level down; and a tree it cannot create makes the probe inconclusive rather than falling back to the shared worktree. The verifier brief now sends every probe, mutant and candidate fix there, with the command welded in the way Agent 7'sbuild-testinvocation is.cleanupsweeps the family by prefix. This is the isolation the test-efficacy probe has had since #6832 and the A/B base tree already had, extended to the last step that writes.A rule for the readers. Every code-reading brief — dimension agents, chunk agents, reverse auditors, and the verifier itself — now carries the rule the live auditor had to invent for itself: the worktree is shared, code that is not in the diff and not in the commit is not a finding, and anything surprising is judged against
git show HEAD:<path>.A tripwire.
agent-promptbuilds every wave of agents immediately before it launches, so it reads the tree once per call: when it finds paths the commit does not have, it names them inside each brief it builds and warns on stderr.scratch-treereports the same thing assharedTreeResidue, at the moment a verifier asks for its tree.One adjacent fix the verification turned up:
exposeDependencies(moved tolib/worktree.ts, beside the other disposable-tree machinery, now that two commands need it) also farms each workspace member's ownnode_modules, not just the root's. Measured on this repo, a scratch tree with 1 560 root packages linked still could not resolve@testing-library/reactfor a UI probe, because npm cannot hoist that copy out ofpackages/cli— which silently cost the existing test-efficacy probe the same class of test.Why it's needed
"Leave the tree as you found it" is what the verifier brief has always said, and verifiers do obey it — the tree is clean by the time they return. It closes the wrong window: the exposure is during the probe, and the pipelined design (
Verification rides alongside the next round) is what makes that window structural rather than unlucky.It has already fired. Reviewing PR #9118 at
662d98f1fa, a round-5 reverse auditor readcompose-review.tscarrying a live probe's mutant plus a leftover__probe__.test.ts, and came within a step of filing a Critical against code no commit contains. It recovered by improvising a fallback no brief mentions — taking its evidence fromgit show HEAD:instead of the working tree — and two other agents in the same run independently reported the contamination. The final review was not corrupted, but only because three concurrent agents happened to notice and one happened to improvise correctly. A pipeline whose correctness depends on an agent inventing the right fallback mid-round is correct by luck.Reviewer Test Plan
How to verify
Unit.
cd packages/cli && npx vitest run src/commands/review→ 2 647 passed, 1 failed. The one failure islib/stale-bundle.test.ts > the SKILL.md quotes and the live notices are prefixes of each other, which fails identically on unmodifiedorigin/main(verified by stashing this branch's changes and re-running) — pre-existing, unrelated to this PR. New coverage:scratch-tree.test.ts(13 cases against a real git repo — sibling placement, per-label isolation, pristine-on-reuse, leftover-directory sweep, crafted-label traversal, residue reporting, dependency farm, "the shared worktree is untouched by everything it does"),lib/worktree.test.ts(6 cases for the residue probe, including that gitignored build output is not residue), plus additions topaths.test.ts,cleanup.test.ts,agent-prompt.test.tsandtest-efficacy.test.ts.Real stack, not just unit. Using this repository itself as a stand-in review worktree:
Then, in that scratch tree: wrote
__probe__.test.ts, rannpx vitest run …/__probe__.test.ts→ 1 passed (a probe really can run there, no install); mutatedpaths.tsin the scratch tree → the probe flipped (expected '/a/b-MUTANT' to be '/a/b-probe'); checked the shared tree →grep -c MUTANT= 0,__probe__.test.tsabsent,git status --porcelainshowing only my own edits. Calledscratch-treeagain →reused: true, mutant reverted, probe file gone, the 1 161-entry dependency farm preserved. The same run is what caught the nested-node_modulesgap:DiffDialog.test.tsxfailed withFailed to resolve import "@testing-library/react"before that fix and passes after it.Reviewers who want to reproduce the isolation claim without the driver: run a review to Step 4 and check that
<worktree>-scratch-*appears whilegit -C <worktree> status --porcelainstays empty.Evidence (Before & After)
N/A — no user-visible or TUI change. The observable outputs are the command reports and test runs quoted above.
Tested on
Environment (optional)
macOS (darwin arm64), Node v22/24, vitest 3.2.4; the real-stack run drove the subcommand through yargs against a real git worktree of this repo.
Risk & Scope
git worktree addsucceeding. That is deliberate — when isolation fails the probe isinconclusiveand the finding keeps its reading-based verdict and low-confidence floor, because the alternative (falling back to the shared tree) is the defect this PR removes. Second,agent-promptnow runs onegit status --porcelainper invocation in worktree mode (once per call, not per agent; silent on a clean tree).-probetree has used since Run the test-efficacy probe in a disposable worktree instead of mutating the shared review tree #6832, and the dependency farm is unchanged apart from the nested-member loop. A full end-to-end/reviewrun through Steps 3-5 was not driven; the concurrency window is reproduced structurally (isolation asserted directly against the shared tree) rather than by racing two live agents. The one failing unit test (stale-bundle.test.ts) is pre-existing onmain.scratch-treeis a new subcommand; the brief and cleanup changes are internal to/review.Note for the diff reader:
SKILL.mdshows ~16 extra changed lines in the role table that are pure Prettier re-padding — one cell in the Agent 7 row grew, so the whole table re-aligned.git diff -wreduces the file to 5 real lines plus the new Step 4 paragraph.Linked Issues
Closes #9207. Extends #6832 (which isolated the deterministic efficacy probe; this does the same for the verifier's hand-written one).
中文说明
这个 PR 做了什么
Step 4 的验证员是整个审查里唯一会写文件的 agent：它写探针、跑探针、施加 flip 自检所需的那一行修复、再还原。这些写入过去全部落在共享的 review worktree 上——也就是
working_dir把其他所有 agent 都钉住的那棵树——而流水线化的反向审计循环会把某一轮的验证员与下一轮的审计员一起发出，于是这些写入恰好在审计员读树期间是活的。本 PR 给每个验证员分片一棵自己的工作树，向所有读代码的 agent 说明"共享树"意味着什么，并为"仍然有人写进共享树"这一情况加了一道绊线。一棵自己的树。 新增子命令
qwen review scratch-tree --worktree <wt> --label <该分片的 record key>，在被审提交上建起一棵用后即弃的兄弟工作树，并链入 review worktree 的node_modules，使单测框架无需安装即可启动。有三个性质让它不只是一个目录：每次调用都交还一棵 pristine 的树（上一个 finding 的突变若活到下一个探针，就是一个带着确定性[probe]来源标签的错误判定）；label 取分片的 record key，因为同一轮的分片是并发的，共用一棵 scratch tree 只是把同样的竞态下移一层；建不出树时探针判为 inconclusive，绝不回退到共享树。验证员 brief 现在把所有探针、突变、候选修复都指向那里，命令连同路径一起焊进 brief——与 Agent 7 的build-test调用同样的做法。cleanup按前缀清扫这一族树。这正是 test-efficacy 探针自 #6832 起、A/B base tree 一直拥有的隔离，现在扩展到最后一个会写的步骤。给读取方的一条规则。 所有读代码的 brief——维度 agent、分块 agent、反向审计员，以及验证员自己——现在都带上了那位实战审计员当初只能临场想出来的规则：工作树是共享的，既不在 diff 里也不在提交里的代码不是 finding，任何令人意外的地方都要用
git show HEAD:<path>核对后再判断。一道绊线。
agent-prompt会在每一波 agent 发射前构建它们，因此它每次调用读一次树：发现提交中不存在的路径时，把这些路径写进它构建的每一份 brief，并在 stderr 告警。scratch-tree在验证员索要自己那棵树的时刻，也以sharedTreeResidue报告同一件事。验证过程顺带修的一处相邻问题：
exposeDependencies（移到lib/worktree.ts，与其他一次性工作树机制放在一起，因为现在有两个命令需要它）现在也会 farm 各 workspace 成员自己的node_modules，而不只是根目录的。在本仓库实测：链了 1 560 个根依赖的 scratch tree 仍然解析不到@testing-library/react，因为 npm 无法把那份副本从packages/cli提升出来——同一盲区也一直在悄悄让既有的 test-efficacy 探针漏掉这一类测试。为什么需要
"树保持原样离开"是验证员 brief 一直写着的规则，验证员也确实遵守——它们返回时树是干净的。但这条规则关的是错的窗口：暴露发生在探针在树上的那段时间，而流水线设计（
Verification rides alongside the next round）使这个窗口成为结构性的，而非偶发。而且它已经发生过。在
662d98f1fa上审查 PR #9118 时，第 5 轮的反向审计员读到的compose-review.ts带着一个活探针的突变体，旁边还有遗留的__probe__.test.ts，它差一步就把这些不属于任何提交的代码报成 Critical。它靠一个 brief 从未提及的临场办法恢复——以git show HEAD:而非工作树取证——同一轮还有另外两个 agent 各自报告了污染。最终审查未被污染，但仅仅因为三个并发 agent 恰好察觉，且其中一个恰好应对正确。一条正确性依赖于 agent 在半途自行发明正确兜底方案的流水线，是靠运气正确的。审查者验证方案
如何验证
单测。
cd packages/cli && npx vitest run src/commands/review→ 2 647 通过，1 失败。唯一失败的是lib/stale-bundle.test.ts > the SKILL.md quotes and the live notices are prefixes of each other，在未修改的origin/main上同样失败（已通过 stash 本分支改动后复跑确认）——预存在，与本 PR 无关。新增覆盖：scratch-tree.test.ts（13 个用例，全部针对真实 git 仓库——兄弟位置、按 label 隔离、复用时 pristine、清理崩溃残留目录、构造 label 的路径穿越、残留报告、依赖 farm，以及"它做的一切都不碰共享工作树"），lib/worktree.test.ts（6 个用例覆盖残留探测，包括被 gitignore 的构建产物不算残留），以及paths.test.ts、cleanup.test.ts、agent-prompt.test.ts、test-efficacy.test.ts的增补。真实栈，不只是单测。 拿本仓库自身当作 review worktree：
随后在那棵 scratch tree 里：写入
__probe__.test.ts，npx vitest run …/__probe__.test.ts→ 1 passed（探针确实能在那里跑起来，且无需安装）；在 scratch tree 里改paths.ts→ 探针翻转（expected '/a/b-MUTANT' to be '/a/b-probe'）；回看共享树 →grep -c MUTANT= 0，__probe__.test.ts不存在，git status --porcelain只有我自己的改动。再次调用scratch-tree→reused: true，突变已还原、探针文件已清除，而 1 161 项的依赖 farm 保留。也正是这次运行暴露了嵌套node_modules的缺口：DiffDialog.test.tsx在该修复前报Failed to resolve import "@testing-library/react"，修复后通过。不想用 driver 复现隔离结论的审查者：跑一次审查到 Step 4，确认
<worktree>-scratch-*出现的同时git -C <worktree> status --porcelain始终为空。证据（前后对比）
N/A —— 无用户可见/TUI 变化。可观测的输出就是上面引用的命令报告与测试运行结果。
测试环境
运行环境（可选）
macOS（darwin arm64）、Node v22/24、vitest 3.2.4；真实栈运行通过 yargs 驱动该子命令，作用于本仓库的真实 git 工作树。
风险与范围
git worktree add成功。这是刻意为之——隔离失败时探针为inconclusive，finding 保留基于阅读的判定与低置信度下限，因为另一种选择（回退到共享树）正是本 PR 要消除的缺陷。其次，worktree 模式下agent-prompt每次调用会执行一次git status --porcelain（每次调用一次，不是每个 agent 一次；树干净时无任何输出）。-probe树自 Run the test-efficacy probe in a disposable worktree instead of mutating the shared review tree #6832 起就在用的同一条代码路径，依赖 farm 除新增的嵌套成员循环外未变。未驱动一次贯穿 Step 3-5 的完整/review实跑；并发窗口是以结构方式复现（直接针对共享树断言隔离），而非让两个活 agent 真正竞争。唯一失败的单测（stale-bundle.test.ts）在main上即已存在。scratch-tree是新增子命令；brief 与 cleanup 的改动都在/review内部。给读 diff 的人一句提示：
SKILL.md里角色表有约 16 行的额外变化，是纯粹的 Prettier 重新对齐——Agent 7 那行的一个单元格变长，整张表随之重排。git diff -w可把该文件缩减为 5 行真实改动加上新增的 Step 4 段落。关联 Issue
Closes #9207。延续 #6832（该 PR 隔离了确定性的 efficacy 探针；本 PR 对验证员手写的探针做同样的事）。