fix(review): pin the verified git identity across the residue probe (#9557) - #9742
Conversation
…9557) `worktreeResidue` verified a tree's identity once and then ran five commands that each re-discovered the repository from `cwd`, through the same `.git` file the check had read — a file writable by anything running as this user. Two halves, and they close different windows: - **The gate now requires the gitfile's admin entry to point BACK at this tree.** A repository whose `core.worktree` names this path answers `--show-toplevel` with this path, so the old gate saw itself while every command after it measured the plant's index — which already holds the contamination as committed content, so `status` comes back clean for a tree with a mutant in it. A planted standalone repo has no admin entry to round-trip; `scratch-tree` gates its own reset on exactly this. - **The verified `--git-dir`/`--work-tree` are pinned onto every later spawn**, so a swap that lands AFTER the gate cannot redirect them either. The reason recorded against doing this — that an explicit pin changes what `ls-files` resolves, so it needs its own round — was measured and is wrong. Across a standalone checkout, a linked worktree, a superproject with an initialised submodule, and a worktree reached through a symlinked ancestor, all five commands return **byte-identical** output pinned and unpinned. What was right about it is that it needed measuring, which is what this is. The backpointer half is pinned by a test that goes red without it. The pin half is not: it closes a check-then-use window the suite cannot open deterministically (the swap has to land between two spawns). Its evidence is a shell repro — with the identity captured, then the gitfile swapped, discovery reports a clean tree while the pinned commands still report ` M a.ts` and the untracked probe file. Saying that rather than implying a pin that is not there.
github-actions
Bot
added
the
review/self-reported
|
Re-run at the new head Template ✓ (substance over headings: "Why it is worth doing" + "Why a new PR" carry what the template's "Why it's needed" asks for; the Reviewer Test Plan and bilingual sections are all there). Problem: observed and measured, not theoretical. Linked P2 security issue #9557 documents the check-then-use window, and the PR body carries a before/after repro — identity captured, gitfile swapped, discovery certifies the mutant clean while the pinned path still names it. The prior verify round independently reproduced the attack shapes against real git. Direction: aligned. This is the tripwire every review wave is built from, and the fix is exactly what #9557 prescribes: resolve the identity once, pin it onto every later spawn, fail closed on any gap. Size: not core paths ( Approach: the scope grew since the body was written — the body still says "same single commit, confined to worktree.ts", while the branch now carries 10 commits across 6 files. Every addition I can see serves the stated goal (the sha-256 validator exists so the pin works on SHA-256 repos; the scratch-tree pre-validation exists so the welded sha actually anchors), and the last commit removes an interim helper rather than adding sprawl. The body is stale, the diff is coherent — noted for the record, not a blocker. Risk: no high-risk-path matches. One structural note: the final commit drops the containment gate entirely (after Moving on to code review. 🔍 中文说明在新 head 模板 ✓（实质优先于标题："Why it is worth doing" + "Why a new PR" 承载了模板 "Why it's needed" 所要求的内容；Reviewer Test Plan 与双语部分齐全）。 问题：已观测、已实测，非理论性问题。关联的 P2 安全 issue #9557 记录了 check-then-use 窗口，PR 正文带 before/after 复现——身份被捕获后掉包 gitfile，发现路径把突变体认证为干净，而固定路径仍能点名。上一轮沙箱验证也对真实 git 独立复现了各攻击形态。 方向：对齐。这是每一波审查所依赖的绊线，修法正是 #9557 开出的方子：解析一次身份、固定到其后每个 spawn、任何缺口失败即关闭。 规模：非核心路径。生产 422 行（worktree.ts 299、agent-prompt.ts 64、scratch-tree.ts 59）对测试 856 行——测试代码比本身说明了这是什么类型的 PR。 方案：范围相对正文写作时已扩大——正文仍写"同一提交、限于 worktree.ts"，分支实际已有 10 个提交、6 个文件。我看到的每处新增都服务于既定目标（sha-256 校验是为了让固定在 SHA-256 仓库上可用；scratch-tree 前置校验是为了让焊接的 sha 真正起到锚定作用），且最后一个提交是删除一个中间态辅助函数而非继续膨胀。正文过期、diff 自洽——记录在案，不作阻断。 风险：无高风险路径命中。一点结构性说明：最后一个提交整体移除了包含门（ 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed at My independent proposal for #9557 before reading the diff is the issue's own prescription — resolve the admin dir once, pin On the two items the deferral named:
Non-blocking, already on the record from review round 10 and not re-litigated here: the pin itself has no automated witness (disclosed in the PR body — its evidence is the manual repro), the sha pin could peel CI evidenceThe PR's own CI at this head, fetched once via the API. The Windows/macOS unit jobs and the integration job show skipped — that is by design in this repo (
The author's "4 504 passed" figure is their claim on macOS; the evidence here is the ubuntu CI run above, which is green. Sandboxed verification would settle the one thing this suite structurally cannot: whether any false-refusal shape remains for symlinked-host spellings — the fixtures realpath their roots, so host-level links are invisible to the suite even with the new synthetic-symlink test. A fresh verify run is already in flight on this head (triggered by this 中文说明在 读 diff 之前我对 #9557 的独立方案就是 issue 自己开出的方子——解析一次 admin 目录、把 关于移交点名的两项：F1——机制随门移除，且这次套件钉住了修复：新的健康形态测试用合成符号链接构造、不依赖宿主 非阻断、第 10 轮已记录在案、此处不再展开：固定本身无自动化见证（PR 正文已披露）、固定可再剥 CI 证据：本 head 上 PR 自己的 CI。Windows/macOS 单元与集成任务显示 skipped——本仓库 沙箱验证能补上套件结构性看不见的一角：符号链接宿主拼写下是否还残留误拒形态——夹具均对根做 realpath，宿主层链接对套件不可见。新一轮验证已在本 head 上运行（由本次 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the security change is proven and the F1 defect is eliminated with a regression witness that cannot pass for the old suite's reason; two reservations named below, neither blocking the code. Stepping back: the approach I would have proposed before reading the diff — resolve once, pin every spawn, fail closed — is exactly what shipped, so there was no simpler path it missed. The three post-deferral commits are the right kind of response to a deferral: F1 got a structural fix (the containment gate that produced it is gone, its security work redistributed to the pin, the unconditional no-record refusal, and the root-walking symlink walk) plus a synthetic-symlink regression test, rather than a patch to the symptom. The interim The two reservations, plainly:
Approving on the code and the CI as they stand. The earlier approval of 中文说明置信度：4/5 —— 安全改动已被证明，F1 缺陷已连同"不可能以旧套件看不见的原因通过"的回归见证一并消除；下面点名两条保留意见，均不阻断代码本身。 退一步看：读 diff 之前我会提出的方案——解析一次、固定每个 spawn、失败即关闭——与落地的完全一致，不存在它错过的更简路径。移交以来的三个提交是对移交的正确回应方式：F1 得到结构性修复（产生它的包含门被移除，其安全性重新分配到固定、无记录即一律拒绝、走根目录的符号链接行走），外加一个合成符号链接回归测试，而不是对症状打补丁。中间态的 两条保留意见，直说：
按代码与 CI 的现状批准。对 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
One change before this can merge — the full reasoning is in my Stage 2 comment above.
Drop the new localFilterCommands export from this PR (or rewire scratch-tree.ts onto the shared export and update the description). As submitted it is a ~60-line near-duplicate of the private function scratch-tree already carries, nothing calls it, the description never mentions it, and its one improvement (the process filter key) lands in the dead copy while the live screen keeps its gap.
The gate and the pin themselves are solid — the backpointer check is the right reading-side counterpart of scratch-tree's, all six post-gate spawns are pinned, everything fails closed to unmeasured, and the test asserts the genuine reading before the refusal. Once the dead duplicate is out, this is ready.
Reviewed at cd727ad97659a765a0c9ec7831ddc8dd56a5f289.
— Qwen Code · qwen3.8-max
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.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Test Plan (not a blocker): 504 passed — this review observed 23137 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 6 轮。
Test Plan（非阻断）：504 passed — this review observed 23137 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // which is a different and much vaguer thing than what was found. | ||
| let pointsBack = false; | ||
| try { | ||
| const backpointer = readFileSync(join(gitDir, 'gitdir'), 'utf8').trim(); |
There was a problem hiding this comment.
[Critical] R1-1: The new backpointer round-trip is forgeable. The gate proves only that the gitfile's admin entry says this tree is its worktree, but a same-user attacker controls both halves of the pair — the .git gitfile and the repository it names — and can write the backpointer themselves. A planted repository shaped like a linked worktree passes the gate, and the pinned --git-dir/--work-tree spawns then measure the plant's index: a tree carrying a mutant certifies clean to every agent wave this probe feeds. We executed the attack end-to-end against this code — contaminate the tree, commit the same contamination into a forge repo, hand-craft its admin entry (gitdir naming the victim's .git, plus commondir, HEAD, copied index), and swap tree/.git to a gitfile naming that entry:
GENUINE: {"paths":["a.ts","__probe__.test.ts"],"total":2}
PINNED STATUS RAW: "" <- the diff's pinned command measures the plant's index
FORGED-ADMIN VERDICT: {"paths":[],"total":0} <- certified clean, no unmeasured, mutant on disk
BROKEN-BACKPOINTER: unmeasured "does not point back" <- flip arm
The verdict flips purely on the content of <admin>/gitdir — one file the planter writes; "a planted standalone repo has no admin entry to round-trip" holds only for a plant that does not forge it (four small writes). Fix direction: bind identity to something the plant cannot write — callers already know the expected PR head sha (fetch-pr records it), and a forge that carries the contamination cannot reproduce it — or require the admin dir to match the path git worktree add recorded when the tree was created.
中文说明
新的 backpointer 往返校验可以伪造。这道门只证明 gitfile 的 admin 条目"声称"本树是它的 worktree，但同用户攻击者控制着这对关系的两端——.git gitfile 与它所指向的仓库——因此可以自己写出 backpointer。一个构造成链接 worktree 形态的植入仓库能通过这道门，随后被固定的 --git-dir/--work-tree 命令测量的是植入仓库的索引：带着突变体的树会被认证为干净，并传给这条探针所服务的每一波 agent。我们对这份代码完整执行了该攻击：污染工作树，把同样的污染提交进一个伪造仓库，手工构造它的 admin 条目（gitdir 指向受害树的 .git，外加 commondir、HEAD、复制的 index），再把 tree/.git 掉包为指向该条目的 gitfile：
GENUINE: {"paths":["a.ts","__probe__.test.ts"],"total":2}
PINNED STATUS RAW: "" <- 本 diff 固定的命令测量的是植入仓库的索引
FORGED-ADMIN VERDICT: {"paths":[],"total":0} <- 认证为干净，无 unmeasured，突变体仍在磁盘上
BROKEN-BACKPOINTER: unmeasured "does not point back" <- 翻转对照
裁决完全取决于 <admin>/gitdir 的内容——一个植入者可以写入的文件；"植入的独立仓库没有可往返的 admin 条目"只对不伪造该条目的植入成立（四次小写入即可伪造）。修复方向：把身份绑定到植入者写不出来的东西上——调用方已经知道期望的 PR head sha（fetch-pr 有记录），而携带污染的伪造仓库无法复现它——或者要求 admin 目录与 git worktree add 创建该树时记录的路径一致。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| top.status === 0 && | ||
| typeof top.stdout === 'string' && | ||
| realpathSync(top.stdout.trim()) === realpathSync(cwd); | ||
| realpathSync(toplevel) === realpathSync(cwd); |
There was a problem hiding this comment.
[Critical] R1-2: Symlink-redirect bypass — this gate omits the redirectedAncestor check every sibling identity gate applies. A symlink planted at the worktree leaf or any ancestor below the checkout (e.g. .qwen/tmp) redirects cwd into attacker territory holding a completely genuine git init + git worktree add pair with the contamination COMMITTED — no forged admin entry needed. spawnSync chdirs through the link, so --show-toplevel answers the physical forge path and the self-equality on this line holds (both sides resolve through the same link); the gitfile→admin→gitdir round trip is real git state, so pointsBack passes; the pin then freezes the forge's identity and all five measurement spawns answer clean:
PRE-FIX: baseline {"paths":["__probe__.test.ts","a.ts"],"total":2}
ancestorRedirect {"paths":[],"total":0}
leafRedirect {"paths":[],"total":0}
POST-FIX: both shapes -> unmeasured "the path resolves through a symlink — …"
(bounded redirectedAncestor walk + leaf lstatSync; 57/57 tests green)
redirectedAncestor is exported from this very file (line 122) and applied by every sibling identity gate — scratch-tree.ts:309 (whose comment documents exactly this shape), cleanup.ts, test-efficacy.ts, and releaseWorktree in lib/git.ts; this probe is the one identity-checking consumer without it. Suggested fix: before trusting the round trip, walk the ancestors of cwd bounded at the common dir — redirectedAncestor(dirname(resolve(cwd)), dirname(commonDir)) — plus an lstatSync(cwd).isSymbolicLink() leaf check, returning the UNMEASURED refusal when either fires.
中文说明
符号链接重定向绕过——这道门缺少所有同类身份门都会做的 redirectedAncestor 检查。在 worktree 叶子目录或检出之下任一祖先目录（例如 .qwen/tmp）植入符号链接，即可把 cwd 重定向到攻击者领地——那里放着一对完全真实的 git init + git worktree add，且污染已被提交——无需伪造任何 admin 条目。spawnSync 会穿过链接 chdir，因此 --show-toplevel 回答的是伪造仓库的物理路径，本行的自相等比较成立（两侧都经同一链接解析）；gitfile→admin→gitdir 往返是真实的 git 状态，pointsBack 通过；随后固定（pin）冻结的是伪造仓库的身份，五条测量命令全部回答干净：
修复前: 基线 {"paths":["__probe__.test.ts","a.ts"],"total":2}
祖先重定向 {"paths":[],"total":0}
叶子重定向 {"paths":[],"total":0}
修复后: 两种形态 -> unmeasured "the path resolves through a symlink — …"
（有界的 redirectedAncestor 行走 + 叶子 lstatSync；57/57 测试全绿）
redirectedAncestor 就导出自本文件（第 122 行），且所有同类身份门都在用——scratch-tree.ts:309（其注释正是描述的这种形态）、cleanup.ts、test-efficacy.ts，以及 lib/git.ts 的 releaseWorktree；本探针是唯一没有这道守卫的身份检查消费者。建议修复：在信任往返校验之前，从 cwd 向上在 common dir 处截止地行走祖先——redirectedAncestor(dirname(resolve(cwd)), dirname(commonDir))——外加 lstatSync(cwd).isSymbolicLink() 叶子检查，任一命中即返回 UNMEASURED 拒绝。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| * filter the user set deliberately, and cannot be safely wiped, so a hit is a | ||
| * refusal upstream, not a cleanup here. | ||
| */ | ||
| export function localFilterCommands(worktree: string): string[] { |
There was a problem hiding this comment.
[Suggestion] R1-3: This hardened filter screen ships as a dead export. It adds the process branch — which its own comment calls "the third executable key … enumerating two of three is how the first cut of this screen read as complete" — but nothing anywhere imports it (repo-wide grep at HEAD: this export, one comment mention, and scratch-tree.ts's private definition + call). The one live consumer, the pre-checkout refusal at scratch-tree.ts:458, still calls scratch-tree.ts's own private copy, whose regex matches only smudge|clean; the two bodies are byte-identical except export and |process. So a filter.evil.process planting is still unscreened on the live path and executes at the next scratch-tree reset/rebuild checkout, while the improved copy runs nowhere and the two now diverge on the security-relevant regex. The process gap in scratch-tree.ts predates this commit; the defect this diff introduces is shipping the fix dead — in a PR whose body says "Same single commit … nothing else" and "Risk & Scope: One new refusal path … and an argument prefix on five commands". Either wire it in (import it in scratch-tree.ts, delete the private copy) or drop it from this PR.
中文说明
这份加强后的过滤器筛查以死导出的形式交付。它加入了 process 分支——其注释称其为"第三个可执行键……只枚举三个中的两个，正是第一版筛查看似完整的原因"——但全仓库没有任何地方导入它（在 HEAD 全仓 grep：仅此导出、一处注释提及，以及 scratch-tree.ts 的私有定义与调用）。唯一的实际消费者——scratch-tree.ts:458 的 checkout 前拒绝——仍调用 scratch-tree.ts 自己的私有副本，其正则只匹配 smudge|clean；两个函数体除 export 与 |process 外逐字节相同。因此 filter.evil.process 植入在真实路径上仍然不被筛查，并会在下一次 scratch-tree 重置/重建的 checkout 时执行，而改进版副本在任何地方都不运行，两份实现从此在安全相关的正则上分叉。scratch-tree.ts 中的 process 缺口在本提交之前已存在；本 diff 引入的缺陷是把这个修复以死代码的形式交付——而 PR 正文写着"同一个提交……没有别的东西"与"风险与范围：一条新拒绝路径……以及五条命令上的参数前缀"。要么把它接上（在 scratch-tree.ts 中导入它、删除私有副本），要么从本 PR 中移除。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| 'config', | ||
| '--file', | ||
| file, | ||
| '--get-regexp', |
There was a problem hiding this comment.
[Suggestion] R1-4: This screen reads each candidate config with git config --file <file> --get-regexp, which does not follow include/includeIf directives — while the checkout the screen authorizes does. Measured with real git: a config carrying [include] path = evil.cfg (evil.cfg defining filter.evil.smudge CMD) yields no match (exit 1) from this query, and the checkout executes the smudge; an includeIf "gitdir:…" plant executes too while staying invisible even to git config --local without --includes. Adding --includes to this query makes both plants visible (fix demonstrated). If R1-3 resolves as wire-it-in this hole becomes live — fix them together; if the export is dropped it is moot for this PR (the unchanged live copy in scratch-tree.ts shares the divergence either way).
| '--get-regexp', | |
| '--get-regexp', | |
| '--includes', |
中文说明
该筛查用 git config --file <file> --get-regexp 读取每个候选配置，但该写法不会跟随 include/includeIf 指令——而筛查所放行的 checkout 会跟随。用真实 git 实测：携带 [include] path = evil.cfg（evil.cfg 定义 filter.evil.smudge CMD）的配置，在此查询下无匹配（exit 1），而 checkout 会执行该 smudge；includeIf "gitdir:…" 植入同样会执行，且在不加 --includes 时连 git config --local 也看不到它。给此查询加上 --includes 后两种植入均可见（修复已验证）。若 R1-3 以"接入"方式解决，这个洞就会变活——请一并修复；若删除该导出，则对本 PR 无影响（scratch-tree.ts 中未改动的活副本无论哪种处理都共享同样的分歧）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the follow-up queue (verified, not declined). The claim reproduces with real git: git config --file <f> --get-regexp exits 1 on an [include]-planted filter.evil.smudge that --includes surfaces, so the screen is blind to exactly the plant the checkout executes. This round dropped the dead localFilterCommands export from this PR per the review's request, so the PR no longer carries any filter-screen code — the hole lives in scratch-tree.ts's pre-existing live private copy, which is outside this PR's footprint. The --includes fix is recorded in the follow-up queue together with the process key and the tests this copy still owes, so they land as one coherent change.
中文说明
延迟到后续队列（已验证，非拒绝）。该论断用真实 git 复现成立：git config --file <f> --get-regexp 对经 [include] 植入的 filter.evil.smudge 返回 exit 1，而加上 --includes 即可见——筛查对 checkout 恰恰会执行的那种植入是盲的。本轮按评审要求把死导出 localFilterCommands 从本 PR 移除，因此本 PR 不再携带任何过滤器筛查代码——漏洞在 scratch-tree.ts 既有的活私有副本里，超出本 PR 足迹。--includes 修复已与process 键以及该副本仍欠的测试一并记入后续队列，作为一次完整的改动落地。
| // Its own try: a plant has no `gitdir` file to read, and letting that | ||
| // ENOENT fall into the outer catch reported it as "not a git worktree", | ||
| // which is a different and much vaguer thing than what was found. | ||
| let pointsBack = false; |
There was a problem hiding this comment.
[Suggestion] R1-5: The admin-entry round-trip is now spelled three times in two files — this block, the pre-existing adminDirOf() in the same file (~line 358), and the near-verbatim copy in resetScratchTree (scratch-tree.ts ~lines 322-338) — and the spellings diverge: adminDirOf normalises with samePath() (realpath of the parent, last component kept literal) while this block and the scratch-tree block use full realpathSync on both sides. This is security-gate logic in three spellings: a future path-normalisation fix must land in three independent places, and missing one leaves one gate silently disagreeing with the others about which tree an admin entry belongs to. Extract the round-trip into one helper (or reuse adminDirOf, whose returned admin dir after realpathSync also supplies the anchor pin) and call it from both gates.
中文说明
admin 条目往返校验现在在两个文件里被写了三遍——本块、同文件中既有的 adminDirOf()（约第 358 行），以及 resetScratchTree 中近乎逐字的副本（scratch-tree.ts 约 322-338 行）——且写法的归一化并不一致：adminDirOf 用 samePath()（对父目录取 realpath，末级组件保持字面），而本块与 scratch-tree 的块在两侧都用完整 realpathSync。这是安全门逻辑的三种写法：未来任何路径归一化修复都必须落在三处独立的位置，漏掉一处就会让某道门与其他门在"admin 条目属于哪棵树"上悄悄分歧。把往返校验提取为一个辅助函数（或复用 adminDirOf——其返回值经 realpathSync 后还能同时提供锚定参数），并在两道门中调用它。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the follow-up queue. The three divergent spellings are verified, and the cross-file dedup is the right end state — but it requires editing scratch-tree.ts, which this PR has never touched, so it is recorded for the follow-up that already owns that file (the filter-copy cluster above). One divergence is deliberate and worth pinning now so that dedup does not flatten it: adminDirOf's samePath spelling normalises the PARENT and keeps the leaf literal because the tree is often already deleted when cleanup asks, while the residue gate compares live trees with full realpathSync. Measured: rewriting adminDirOf onto the gate's full-realpath comparison makes a symlinked tree path resolve to the VICTIM's admin dir and hand it to discardWorktree for deletion — a cleanup regression, not a neutral refactor.
中文说明
延迟到后续队列。三处分歧写法已验证，跨文件去重是正确的最终形态——但它需要修改本 PR 从未触碰过的 scratch-tree.ts，因此记入已经负责该文件的后续工作（上方的过滤器副本簇）。其中一处分歧是刻意的，现在钉住以免去重时把它抹平：adminDirOf 的 samePath 写法对父目录归一化、末级保持字面，因为清理来询问时树往往已被删除；而残留探针对活着的树用完整 realpathSync 比较。实测：把 adminDirOf 改写成门里的完整 realpath 比较，会让符号链接树路径解析到受害者的 admin 目录，并把它交给 discardWorktree 删除——是清理回归，不是中性重构。
| // superproject with an initialised submodule and a worktree reached | ||
| // through a symlinked ancestor, all five commands below return | ||
| // byte-identical output pinned and unpinned. | ||
| anchor = [ |
There was a problem hiding this comment.
[Suggestion] R1-6: The Closes linkage to issue 9557 auto-closes it at one third of its named scope. The issue body names three helpers lacking the pin — scratch-tree.ts, worktreeResidue and base-tree.ts — and prescribes pinning "on every subsequent spawn". This PR pins only worktreeResidue: scratch-tree.ts's destructive reset (checkout --force / clean -ffdx) still re-discovers through cwd after a gate whose own comment admits the remaining window, and base-tree.ts's gitOut/git have neither gate nor pin (verified: 0 changed lines in both files at this commit). On merge, GitHub closes the tracker while the check-then-use pattern the issue was filed to remove persists in two live pipeline commands. Either extend the pin to the helpers the issue names, or change the linkage (e.g. "Part of issue 9557") and file follow-ups for the two helpers before merge.
中文说明
对 issue 9557 的 Closes 关联会在其点名范围的三分之一处自动关闭该 issue。issue 正文点名了三个缺少固定的辅助模块——scratch-tree.ts、worktreeResidue 与 base-tree.ts——并要求"在其后每一个 spawn 上"固定身份。本 PR 只固定了 worktreeResidue：scratch-tree.ts 的破坏性重置（checkout --force / clean -ffdx）仍在一道连自身注释都承认还有残余窗口的门之后经 cwd 重新发现仓库，而 base-tree.ts 的 gitOut/git 既无门也无固定（已验证：本提交中这两个文件改动行数均为 0）。合并时，GitHub 会关闭这个跟踪 issue，而该 issue 所要消除的 check-then-use 模式仍存在于两条活着的流水线命令中。要么把固定扩展到 issue 点名的辅助模块，要么把关联改为"Part of issue 9557"，并在合并前为这两个辅助模块建立后续 issue。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Verified and deferred — with one open question for a maintainer. Confirmed at this commit: scratch-tree.ts and base-tree.ts have 0 changed lines, so issue 9557 would close at one third of its named scope. Extending the pin to both helpers is a multi-file change (gates, pins, tests, and for scratch-tree a non-circular sha source, which its caller can only get through new CLI plumbing) beyond this PR's declared single-commit scope — it is recorded in the follow-up queue so it is not lost at merge. The remaining half is not the bot's to do: should the PR body's Closes #9557 be changed to Part of #9557 before merge so the tracker stays open for scratch-tree.ts and base-tree.ts? A maintainer (or the workflow on instruction) needs to edit the body; please advise here.
中文说明
已验证并延迟——另有一个留给维护者的开放问题。本提交确认：scratch-tree.ts 与 base-tree.ts 改动行数均为 0，issue 9557 会在其点名范围的三分之一处被关闭。把固定扩展到这两个辅助模块是多文件改动（门、固定、测试，以及 scratch-tree 需要一个非循环的 sha 来源——其调用方只能通过新的 CLI 参数获得），超出本 PR 声明的单一提交范围——已记入后续队列，合并时不会丢失。剩下一半不是机器人能做的：合并前是否应把 PR 正文的 Closes #9557 改为 Part of #9557，以便跟踪 issue 对 scratch-tree.ts 与 base-tree.ts 保持开启？需要维护者（或工作流按指示）编辑正文，请在此线程给出意见。
| // `process` beside the pair: it is the third executable key (a | ||
| // long-running filter git speaks a protocol to), and enumerating two | ||
| // of three is how the first cut of this screen read as complete. | ||
| '^filter\\..*\\.(smudge|clean|process)$', |
There was a problem hiding this comment.
[Suggestion] R1-7: This ~58 lines of new security-screen logic has zero tests — including none pinning the process key the function's own comment says is the reason this version exists, and none pinning the key parse. With no test, the one-line mutation back to the two-key smudge|clean regex ships green — verified: scratch-tree.test.ts exercises only the private two-key copy, and only via smudge keys, and no caller exercises this export. If it is wired in later (see R1-3), a filter.pwn.process planting goes unreported with no regression net. Add a collocated real-git test: plant each of filter.pwn.smudge, filter.pwn.clean and filter.pwn.process — one per candidate location (common config, the tree's config.worktree, a sibling worktree's config.worktree) — assert all three keys come back, plus a negative case (filters defined only in global config are not reported).
中文说明
这约 58 行新的安全筛查逻辑没有任何测试——既没有钉住 process 键（按该函数自己的注释，这正是此版本存在的理由），也没有钉住键名解析。没有测试时，把正则退回两键 smudge|clean 的单行突变即可全绿通过——已验证：scratch-tree.test.ts 只测私有的两键副本，且只经由 smudge 键；也没有任何调用方执行到本导出。若日后将其接入（见 R1-3），filter.pwn.process 植入将不被报告且无回归保护。请添加同目录的真实 git 测试：分别植入 filter.pwn.smudge、filter.pwn.clean、filter.pwn.process——各放一个候选位置（common config、本树的 config.worktree、兄弟 worktree 的 config.worktree）——断言三个键全部被报告，外加一个负例（只定义在全局配置中的过滤器不应被报告）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the follow-up queue with R1-4. Verified: after this round dropped the dead export, this PR owns no filter-screen code at all — the only live copy is scratch-tree.ts's private two-key function, which predates this commit and is exercised by no test (scratch-tree.test.ts reaches it only through smudge keys). The prescribed tests — filter.pwn.smudge/clean/process planted one per candidate location (common config, the tree's config.worktree, a sibling's config.worktree) plus the global-config negative — belong with that copy's --includes + process fix, recorded as one follow-up so the screen and its regression net land together.
中文说明
与 R1-4 一并延迟到后续队列。已验证：本轮移除死导出后，本 PR 完全不再拥有过滤器筛查代码——唯一的活副本是 scratch-tree.ts 的私有两键函数，它早于本提交且没有任何测试覆盖（scratch-tree.test.ts 只经由 smudge 键触及它）。规定的测试——在三个候选位置（common config、本树的 config.worktree、兄弟 worktree 的 config.worktree）各植入一个 filter.pwn.smudge/clean/process，外加大局配置负例——应与该副本的 --includes + process 修复一起，作为同一个后续工作落地，让筛查与它的回归保护网同时交付。
| paths: [], | ||
| total: 0, | ||
| unmeasured: | ||
| 'the .git gitfile names an admin entry that does not point back ' + |
There was a problem hiding this comment.
[Suggestion] R1-8: This UNMEASURED message describes one shape but is emitted for two. Shape A: the gitfile names a real admin entry whose backpointer names a different tree — the message is accurate. Shape B — the inner catch above, which the PR's own headline test constructs: the gitfile names a standalone repo answering via core.worktree, which has NO admin entry at all; there is no gitdir file to "not point back". Both shapes reach this same return, byte-identical (measured: constructed both in one run — shape B has no admin entry yet receives the shape-A message), and whoever triages the refusal — surfaced via scratch-tree.ts's NOTE as "git status failed: " — hunts for a <repo>/.git/gitdir file that does not exist. Record which sub-shape fired and emit a distinct reason for the catch branch, e.g. "the .git gitfile names a repository with no admin entry for this tree — a standalone repository answering for this path".
中文说明
这条 UNMEASURED 消息描述的是一种形态，却在两种形态下都会发出。形态 A：gitfile 指向一个真实的 admin 条目，其 backpointer 指向另一棵树——消息准确。形态 B——即上方内层 catch、也是本 PR 头条测试所构造的形态：gitfile 指向一个通过 core.worktree 为本路径作答的独立仓库，它根本没有 admin 条目；不存在"没有指回来"的 gitdir 文件。两种形态到达同一个返回，消息逐字节相同（实测：一次运行中构造两种形态——形态 B 没有 admin 条目，却收到了形态 A 的消息），而排查该拒绝的人——经 scratch-tree.ts 的 NOTE 以 "git status failed: " 呈现——会去寻找一个并不存在的 <repo>/.git/gitdir 文件。请记录命中的是哪种子形态，并为 catch 分支发出不同的原因，例如 "the .git gitfile names a repository with no admin entry for this tree — a standalone repository answering for this path"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| let pointsBack = false; | ||
| try { | ||
| const backpointer = readFileSync(join(gitDir, 'gitdir'), 'utf8').trim(); | ||
| pointsBack = |
There was a problem hiding this comment.
[Suggestion] R1-9: The mismatch branch of this comparison — the exact shape the new UNMEASURED message describes (a real admin entry whose backpointer names a different tree) — has zero test coverage. The PR's new test forges a standalone repo with no admin entry, so readFileSync throws, the inner catch fires, and this comparison is never evaluated by any test. A future edit that corrupts only the mismatch evaluation ships green — measured: a mutant removing realpathSync from the comparison passes worktree.test.ts 54/54 and scratch-tree.test.ts 41/41, while the same mutant flips a borrowed-sibling-admin-entry probe from unmeasured to {"paths":["a.ts"],"total":1} (the gate passes and measures the sibling's index against this tree). Add a test: create a second linked worktree of the fixture repo, rewrite tree/.git to gitdir: <common>/worktrees/<sibling-id>, and assert worktreeResidue(tree).unmeasured contains 'does not point back'.
中文说明
此比较的不匹配分支——正是新 UNMEASURED 消息所描述的形态（真实存在的 admin 条目，其 backpointer 指向另一棵树）——没有任何测试覆盖。本 PR 的新测试伪造的是一个没有 admin 条目的独立仓库，于是 readFileSync 抛错、内层 catch 触发，该比较从未被任何测试求值。未来任何只破坏不匹配求值的改动都会全绿通过——实测：把比较中的 realpathSync 移除的突变体，worktree.test.ts 54/54 与 scratch-tree.test.ts 41/41 全部通过；而同一突变体让"借用兄弟 worktree admin 条目"的探针从 unmeasured 翻转为 {"paths":["a.ts"],"total":1}（门通过，并把兄弟的索引当作本树来测量）。请补一个测试：为夹具仓库创建第二个链接 worktree，把 tree/.git 改写为 gitdir: <common>/worktrees/<sibling-id>，并断言 worktreeResidue(tree).unmeasured 包含 'does not point back'。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback round — PR #9742Commit: Both Critical findings were reproduced end-to-end against the pre-fix code DispositionsImplemented
Deferred to the follow-up queue (
No action
Changes
Verification
中文说明评审反馈处理 — PR #9742提交： 两条 Critical 发现都在写任何修复之前先在修复前代码上完整复现（伪造 admin 条目与两种符号链接重定向形态各自把突变体认证为干净：污染仍在磁盘上时返回 处理结论已实现
延迟到后续队列（
无需处理
变更内容
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 1a": none — the vitest suite itself was not run (no node_modules in this worktree), but every behavioral assumption it encodes was verified against real git instead.; "agent 5": executing cd packages/cli && npx vitest run src/commands/review/lib/worktree.test.ts to confirm the six new tests pass — the review worktree has no node_modu….
Test Plan (not a blocker): 504 passed — this review observed 23139 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:767 — [review] R1-5 admin-entry round-trip spelled three times in two files — still stands, deferred to the follow-up queuepackages/cli/src/commands/review/lib/worktree.ts:800 — [review] R1-6 Closes linkage to issue 9557 auto-closes at one third of its named scope — still stands, deferred with an open maintainer question
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 1a"：none — the vitest suite itself was not run (no node_modules in this worktree), but every behavioral assumption it encodes was verified against real git instead.；"agent 5"：executing cd packages/cli && npx vitest run src/commands/review/lib/worktree.test.ts to confirm the six new tests pass — the review worktree has no node_modu…。
Test Plan（非阻断）：504 passed — this review observed 23139 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const sha = report.fetchedSha; | ||
| return worktreeResidue( | ||
| resolve(wt), | ||
| 12, | ||
| typeof sha === 'string' && /^[0-9a-f]{40}$/i.test(sha) ? sha : undefined, |
There was a problem hiding this comment.
[Critical] R2-1: The anti-forge anchor reaches the probe through a channel inside the attacker's own write boundary. fetchedSha is re-read from the plan JSON on disk at every agent-prompt invocation — including every wave after the reviewed code has executed (the PR's build/test runs mid-review as this user) — and this guard degrades to undefined when the field is absent or malformed. A same-user writer who rewrites .qwen/tmp/qwen-review-pr-<n>-fetch.json defeats the sha pin without reproducing it: deleting the field skips the pin entirely and round 1's forged admin entry (whose backpointer round-trips) certifies clean again, while setting the field to the forge's own HEAD sha passes the pin outright. In both corners the pinned status measures the forge's clean index and the contaminated tree certifies clean to every agent brief. Executed against this commit: a forged admin entry beside a full-mirror forge (contamination committed, mutant on disk) answers {"paths":[],"total":0} — certified clean — to the sha-less call and unmeasured "…not the fetched PR head…" to the pinned call; the plan rewrite itself is a plain same-user file write, the same write class the issue's threat model names. The comment above — "the one identity element a forge planted at the worktree cannot reproduce" — is what makes this dangerous: the forge never needs to reproduce the sha when it can rewrite where the probe reads it. Fail closed when the record is absent or malformed instead of degrading: every worktree-mode plan fetch-pr writes carries fetchedSha, so absence means tampering or corruption — return unmeasured from worktreeResidueOf rather than passing undefined. Longer term, the identity anchor must not be re-read from attacker-writable storage after reviewed code has executed; until then the comment should claim "raises the plant's cost", not "cannot reproduce".
中文说明
反伪造锚（fetchedSha）是经由攻击者可写边界内的通道送达探针的。fetchedSha 在每次 agent-prompt 调用时都从磁盘上的计划 JSON 重新读取——包括审查代码已执行之后的每一波（本 PR 的构建/测试会在审查中途以本用户身份运行）——而字段缺失或格式错误时，此处的守卫会退化为 undefined。同用户写入者只要改写 .qwen/tmp/qwen-review-pr-<n>-fetch.json，就无需复现 sha 即可击败 sha 固定：删除该字段会完全跳过固定，第 1 轮那个 backpointer 可往返的伪造 admin 条目将再次被认证为干净；把该字段设为伪造仓库自己的 HEAD sha 则直接通过固定。两种角落下，固定的 status 测量的都是伪造仓库的干净索引，带污染的树会被认证为干净并写入每一份 agent 简报。已在本提交上执行验证：伪造 admin 条目 + 全量镜像伪造仓库（污染已提交、突变体在磁盘上），对无 sha 调用回答 {"paths":[],"total":0}（认证为干净），对固定调用回答 unmeasured "…not the fetched PR head…"；计划文件改写本身就是一次普通的同用户文件写入，正是 issue 威胁模型所命名的写入类别。上方注释——"植入工作树的伪造仓库无法复现的唯一身份要素"——正是危险所在：伪造仓库能改写探针读取记录的位置时，根本不需要复现 sha。记录缺失或格式错误时应改为失败关闭而不是退化：fetch-pr 写入的每个 worktree 模式计划都带有 fetchedSha，缺失即意味着篡改或损坏——worktreeResidueOf 应返回 unmeasured，而不是传 undefined。长期来看，审查代码执行之后，身份锚不应再从攻击者可写的存储中重新读取；在此之前，注释应声明"提高植入成本"，而不是"无法复现"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const redirected = lstatSync(cwd).isSymbolicLink() | ||
| ? resolve(cwd) | ||
| : redirectedAncestor(dirname(resolve(cwd)), dirname(commonDir)); |
There was a problem hiding this comment.
[Critical] R2-2: Symlink-redirect bypass — the walk's stop boundary comes from the verified repository's own planter-writable metadata. Plant a symlink at an ancestor of the tree below the checkout (here .qwen) and lay the redirect territory out so the resolved tree sits directly under the forge's common-dir root: the discovery spawn reports --git-common-dir = outside/tmp/common.git, the walk starts at <repo>/.qwen/tmp — whose realpath equals the boundary's realpath — and the stop test fires on iteration 1 without ever lstat'ing .qwen. A completely genuine git init + git worktree add pair in the territory then passes the round trip, and the sha-less production caller (scratch-tree.ts:474) certifies the forge clean — suppressing the "shared review worktree is NOT clean" warning scratch-tree prints to every verifier shard. Executed against this commit: NO-SHA: {"paths":[],"total":0} (forge certified clean) vs WITH-SHA: unmeasured "…not the fetched PR head…" (the pin still refuses); a contrast run with a non-bare forge (boundary one level deeper) catches the same .qwen link on iteration 2 — the evasion turns exactly on the boundary landing at the walk start's realpath. Round 1's R1-2 promised refusal of "a symlink planted at the worktree leaf or any ancestor below the checkout (e.g. .qwen/tmp)" — .qwen is such an ancestor. Require resolve(stopAt) to be a literal path-prefix of resolve(dir) before honoring the stop test — in a genuine review worktree the common dir's parent is a literal ancestor of the tree, in this bypass it is not — or lstat every component between cwd and the boundary before the boundary's identity, which comes from the thing being verified, is allowed to end the walk.
中文说明
符号链接重定向绕过——行走的停止边界来自被验证仓库自身的、植入者可写的元数据。在检出之下、树的某个祖先处（此处为 .qwen）植入符号链接，并把重定向领地布置成"解析后的树恰好位于伪造仓库 common-dir 根目录之下"：发现命令会报告 --git-common-dir = outside/tmp/common.git，行走从 <repo>/.qwen/tmp 开始——其 realpath 恰等于边界的 realpath——停止测试在第 1 轮迭代即命中，.qwen 本身从未被 lstat。领地中一对完全真实的 git init + git worktree add 随之通过往返校验，无 sha 的生产调用方（scratch-tree.ts:474）会将伪造仓库认证为干净——从而压掉 scratch-tree 向每个验证分片打印的 "shared review worktree is NOT clean" 警告。已在本提交上执行：NO-SHA: {"paths":[],"total":0}（伪造仓库被认证为干净）对比 WITH-SHA: unmeasured "…not the fetched PR head…"（固定仍然拒绝）；对照实验使用非 bare 伪造仓库（边界深一层）时，同一个 .qwen 链接在第 2 轮迭代即被捕获——绕过恰好取决于边界落在行走起点的 realpath 上。第 1 轮的 R1-2 承诺拒绝"植入在工作树叶子或检出之下任一祖先（例如 .qwen/tmp）的符号链接"——.qwen 正是这样一个祖先。在信任停止测试之前，要求 resolve(stopAt) 是 resolve(dir) 的字面路径前缀——在真实审查工作树中，common dir 的父目录是树的字面祖先，在本绕过中则不是——或者在允许"来自被验证对象自身的边界身份"终止行走之前，对 cwd 与边界之间的每一个分量做 lstat。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // link and agrees with itself: `--show-toplevel` answers the physical | ||
| // forge path and the self-equality above holds. The walk is the one | ||
| // every sibling identity gate applies, bounded at the repository the | ||
| // common dir belongs to — above that is the user's own layout, and |
There was a problem hiding this comment.
[Suggestion] R2-3: The walk bound dirname(commonDir) is only an ancestor of the tree in the healthy shape; in the planted/forged shapes this same block refuses, it is not — so redirectedAncestor's stop test never matches and the walk climbs to /, contrary to this comment and to every sibling call site (scratch-tree.ts:309, test-efficacy.ts:1577 — both bound at a repository the walked tree is structurally under). Concrete consequences: (1) on a macOS developer machine, where tmpdir() is /var/folders/… — a symlink — the two new tests in this diff fail: the swapped-gitfile test asserts toContain('no admin entry') and the forged-admin test asserts toContain('not the fetched PR head'), but the unbounded walk returns /var first and both get 'resolves through a symlink' instead (CI stays green: Linux /tmp is real and the macOS merge-queue job pins TMPDIR to the real RUNNER_TEMP, which masks this); (2) in production those same shapes are still refused — fail-closed is preserved — but the refusal names a system link far above the tree instead of the missing admin entry, sending whoever triages it hunting in the wrong place. Executed: real spelling unmeasured "…no admin entry…" vs link spelling unmeasured "the path resolves through a symlink (…)"; forged-admin variant "not the fetched PR head" vs "resolves through a symlink". Only walk when the bound is actually an ancestor (e.g. a resolve(cwd).startsWith(resolve(bound) + sep) guard), otherwise fall through to the admin-entry checks that own those shapes — and adjust this comment, which currently describes a guarantee the code does not provide.
中文说明
行走边界 dirname(commonDir) 只在健康形态下才是树的祖先；在本块所拒绝的植入/伪造形态下它并不是祖先——于是 redirectedAncestor 的停止测试永不命中，行走一路爬到 /，与本注释及所有同类调用点相悖（scratch-tree.ts:309、test-efficacy.ts:1577——两者都以"被行走的树在结构上位于其下"的仓库为界）。具体后果：（1）在 macOS 开发机上，tmpdir() 是 /var/folders/…——一个符号链接——本 diff 的两个新测试会失败：掉包 gitfile 测试断言 toContain('no admin entry')、伪造 admin 测试断言 toContain('not the fetched PR head')，但无界行走先撞到 /var，两者都得到 'resolves through a symlink'（CI 保持绿色：Linux 的 /tmp 是真实目录，macOS 合并队列任务把 TMPDIR 固定为真实的 RUNNER_TEMP，掩盖了这一点）；（2）生产中同样形态仍会被拒绝——失败关闭得以保持——但拒绝理由点名的是树上方很远的系统链接，而不是缺失的 admin 条目，把排查者引向错误方向。已执行验证：真实拼写 unmeasured "…no admin entry…" 对比链接拼写 unmeasured "the path resolves through a symlink (…)"；伪造 admin 变体 "not the fetched PR head" 对比 "resolves through a symlink"。仅在边界确实是祖先时才行走（例如 resolve(cwd).startsWith(resolve(bound) + sep) 守卫），否则落入拥有这些形态的 admin 条目检查——并修正本注释，它目前描述的保证代码并不提供。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if ( | ||
| realpathSync(dirname(resolve(gitDir, backpointer))) !== | ||
| realpathSync(cwd) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R2-4: A dangling admin-entry backpointer — the gitdir file names a nonexistent path (crash mid-worktree add, a cleanup gone wrong, a sloppy forge) — throws ENOENT out of realpathSync into the outer catch, producing the "not a git worktree" refusal: the exact misdiagnosis the inner try/catch above was written to avoid. git rev-parse still exits 0 in that state (verified with real git), so isWorktree is true and the comparison runs; the throw resets isWorktree = false in the outer catch and the probe reports "the path is not a git worktree (repository discovery walks up into the enclosing checkout)" for a path that IS a worktree with an existing admin entry — the triager six months from now hunts a missing-gitfile / walk-up problem that is not there. Still fail-closed; only the diagnosis is wrong. The sibling gates keep the same comparison inside their own handling (scratch-tree.ts try → false → rebuild; test-efficacy.ts:1563-1569 try → message). Give the comparison its own try and treat an unresolvable backpointer as "does not point back", reusing that refusal. Witness: R24 rev-parse status: 0 with --show-toplevel answering the real tree path; R24 residue: {"paths":[],"total":0,"unmeasured":"the path is not a git worktree (repository discovery walks up into the enclosing checkout)"}.
| if ( | |
| realpathSync(dirname(resolve(gitDir, backpointer))) !== | |
| realpathSync(cwd) | |
| ) { | |
| let pointsBack = false; | |
| try { | |
| pointsBack = | |
| realpathSync(dirname(resolve(gitDir, backpointer))) === | |
| realpathSync(cwd); | |
| } catch { | |
| // A backpointer that does not resolve does not point back at this tree. | |
| } | |
| if (!pointsBack) { |
中文说明
悬空的 admin 条目 backpointer——gitdir 文件指向一个不存在的路径（worktree add 中途崩溃、清理出错、粗糙的伪造）——会让 realpathSync 抛出 ENOENT 落进外层 catch，产生 "not a git worktree" 拒绝：正是上方内层 try/catch 特意要避免的误诊。该状态下 git rev-parse 仍以 0 退出（已用真实 git 验证），因此 isWorktree 为 true、比较会执行；抛错使外层 catch 把 isWorktree 重置为 false，探针会对一个确实是工作树、admin 条目也确实存在的路径报告 "the path is not a git worktree (repository discovery walks up into the enclosing checkout)"——六个月后的排查者会去找一个并不存在的 gitfile 缺失 / 向上发现问题。仍是失败关闭；只是诊断错了。同类门把同样的比较放在自己的处理之内（scratch-tree.ts try → false → 重建；test-efficacy.ts:1563-1569 try → 消息）。请给该比较单独套一层 try，把无法解析的 backpointer 当作 "does not point back"，复用那条拒绝。见证：R24 rev-parse status: 0，--show-toplevel 回答真实树路径；R24 residue: {"paths":[],"total":0,"unmeasured":"the path is not a git worktree (repository discovery walks up into the enclosing checkout)"}。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // forge planted at the worktree cannot reproduce, and with it the probe | ||
| // refuses a forged admin entry (see worktreeResidue). | ||
| const sha = report.fetchedSha; | ||
| return worktreeResidue( |
There was a problem hiding this comment.
[Suggestion] R2-5: The new production wiring that hands the fetched sha to the residue probe has zero test coverage — no test anywhere supplies fetchedSha in a plan report (grep of agent-prompt.test.ts: no matches). The tripwire integration test in agent-prompt.test.ts writes a plan JSON without fetchedSha, so it exercises only the no-record path; the new worktree.test.ts tests pass the sha straight to worktreeResidue, bypassing this seam. A one-line regression at exactly this seam — dropping the third argument, or reading a renamed field — silently strips the forge-anchor from the only production call site, re-opening the forged-admin certification this PR closes, while every test in the PR stays green (the R2-1 probe shows what that re-opens). Extend the tripwire test to write fetchedSha: <fixture HEAD sha> into the plan JSON and assert the brief still reports the residue (a healthy sha must not become a refusal of its own), and/or write a wrong sha and assert the UNMEASURED sentence reaches the brief.
中文说明
把已获取 sha 移交给残留探针的这段新生产接线完全没有测试覆盖——全部测试中没有任何一处在计划报告里提供 fetchedSha（grep agent-prompt.test.ts：无匹配）。agent-prompt.test.ts 里的绊线集成测试写入的计划 JSON 不含 fetchedSha，因此只覆盖"无记录"路径；worktree.test.ts 的新测试把 sha 直接传给 worktreeResidue，绕过了这条接缝。恰好在这条接缝上的一行回归——丢掉第三个实参、或读取改名后的字段——会悄无声息地剥掉唯一生产调用点上的反伪造锚，重新打开本 PR 所关闭的伪造 admin 认证，而 PR 中的所有测试仍然全绿（R2-1 的探针展示了重新打开的是什么）。请扩展绊线测试：向计划 JSON 写入 fetchedSha: <夹具 HEAD sha> 并断言简报仍报告残留（健康 sha 不应自身成为一种拒绝），以及/或者写入错误 sha 并断言 UNMEASURED 语句进入简报。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if ( | ||
| got === null || | ||
| got.toLowerCase() !== expectedHeadSha.toLowerCase() | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R2-6: Two branches of the new sha pin have no witness. (a) The case-fold: the caller's guard /^[0-9a-f]{40}$/i admits an uppercase sha, but every fixture passes lowercase, so the mutant dropping the second toLowerCase() survives the full 60-test suite and would refuse a healthy worktree as "not the fetched PR head" the moment a plan carries an uppercase sha — every agent brief's residue block becomes a false UNMEASURED. (b) The unreadable-HEAD arm (got === null) is never exercised: deleting the got === null || guard throws on null.toLowerCase(), falls into the outer catch, and answers with the generic "not a git worktree" refusal instead of the specific reason this arm's comment says exists (reachable via a forged admin entry over a commit-less repo). Both mutants were run: suite green, probe flips — baseline uppercase-sha run {"paths":["__probe__.test.ts"],"total":1} vs mutant unmeasured "…checked out at 12f3847…, not the fetched PR head 12F3847F…"; null-arm mutant flips "could not read its own HEAD" to "not a git worktree". Add expect(worktreeResidue(tree, 12, expected.toUpperCase()).paths.sort()).toEqual([...]) to the happy arm of the forge test, and one forged-entry-over-unborn-repo case asserting unmeasured contains 'could not read its own HEAD'.
中文说明
新 sha 固定有两个分支没有见证。（a）大小写折叠：调用方守卫 /^[0-9a-f]{40}$/i 接受大写 sha，但所有夹具都传小写，因此"去掉第二个 toLowerCase()"的突变体能在完整 60 测试套件下存活；一旦计划携带大写 sha，它会拒绝健康工作树为 "not the fetched PR head"——每份 agent 简报的残留块都会变成假的 UNMEASURED。（b）不可读 HEAD 分支（got === null）从未被演练：删除 got === null || 守卫会在 null.toLowerCase() 上抛错、落进外层 catch，回答泛化的 "not a git worktree" 拒绝，而不是该分支注释声明存在的专用理由（可经"无提交仓库上的伪造 admin 条目"到达）。两个突变体都已运行：套件全绿、探针翻转——大写 sha 基线 {"paths":["__probe__.test.ts"],"total":1} 对比突变体 unmeasured "…checked out at 12f3847…, not the fetched PR head 12F3847F…"；null 分支突变体把 "could not read its own HEAD" 翻转为 "not a git worktree"。请在伪造测试的健康分支加入 expect(worktreeResidue(tree, 12, expected.toUpperCase()).paths.sort()).toEqual([...])，并新增一个"未诞生仓库上的伪造条目"用例，断言 unmeasured 包含 'could not read its own HEAD'。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| expect(worktreeResidue(tree, 12, expected).paths.sort()).toEqual([ | ||
| '__probe__.test.ts', | ||
| 'a.ts', | ||
| ]); | ||
|
|
||
| const forge = join(repo, 'forge'); | ||
| mkdirSync(forge); | ||
| const fgit = (...args: string[]) => |
There was a problem hiding this comment.
[Suggestion] R2-7: The fgit wrapper and the forge-repo setup block are pasted four times in this diff (fgit at lines ~206, 264, 364, 422; the init + contamination writes + add -A + commit setup immediately after each, plus the "genuine measurement first" prelude in each test). Each copy is ~17 lines of attack-fixture construction. When fixture isolation has to change — this suite already carries that history: isolateHostGitConfig and --template= exist because ambient host git config broke fixture commits — the change must be made in four places, and a missed or drifted copy silently changes what the attack fixture produces: a probe testing a different shape than it claims, in a suite whose subject is identity-gate bypasses. Extract a local builder — e.g. forgeAt(dir, opts?) running the init/contamination-commit sequence (optionally a worktree add) and returning the fgit runner — and have the four tests call it; the per-test divergence then shrinks to the genuinely different part (the core.worktree config, the forged admin-entry writes, the symlink plant).
中文说明
fgit 包装器与伪造仓库的搭建块在本 diff 中被粘贴了四次（fgit 位于约 206、264、364、422 行；每处紧随 init + 污染写入 + add -A + commit 的搭建，外加每个测试的"先做真实测量"前奏）。每份副本约 17 行攻击夹具搭建。当夹具隔离需要变更时——本套件已有这段历史：isolateHostGitConfig 与 --template= 的存在，正是因为宿主环境 git 配置曾破坏夹具提交——变更必须在四处分别做出，任何一处遗漏或漂移都会悄然改变攻击夹具所产生的形态：在一份以"身份门绕过"为主题的套件里，探针测试的将与其声称的形态不符。请抽取一个本地构建器——例如 forgeAt(dir, opts?)，执行 init/污染提交序列（可选 worktree add）并返回 fgit 运行器——让四个测试调用它；每个测试的差异于是收缩到真正不同的部分（core.worktree 配置、伪造 admin 条目写入、符号链接植入）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round under the ~8-finding batch bound: this round implemented the ten findings that share the identity-gate fix cluster (they cannot be shipped coherently apart). The restructure already removed two of the four fgit copies — the gitfile-swap and forged-admin fixtures no longer build an external forge, they reuse the real repository — leaving two copies (the two redirect tests), which makes the extraction smaller and safer to land on its own.
中文说明
按每轮约 8 条发现的批次上限延后到下一轮：本轮实现了共享同一身份门修复簇的 10 条发现（它们无法被拆开独立交付）。本轮的重构已消除了四份 fgit 拷贝中的两份——gitfile 掉包与伪造 admin 条目夹具不再搭建外部伪造仓库，而是直接复用真实仓库——仅剩两份拷贝（两个重定向测试），抽取因此更小、也更安全，适合单独提交。
| resolve(wt), | ||
| 12, |
There was a problem hiding this comment.
[Suggestion] R2-8: The bare 12 restates worktreeResidue's default cap solely to reach the new third positional parameter — an unlabeled duplicate of a default owned by another module; only worktree.test.ts:164-165 pins that default today. This is the only production caller that passes the sha anchor, so if a maintainer later changes the default cap in worktree.ts, this call site silently keeps 12, and every agent brief's residue list diverges from every other caller's report with no signal anywhere. Export a named constant from worktree.ts (e.g. export const RESIDUE_PATH_CAP = 12) used both as the default and here, or give worktreeResidue an options object ({ cap?, expectedHeadSha? }) so callers never restate a default to reach a later parameter.
| resolve(wt), | |
| 12, | |
| resolve(wt), | |
| RESIDUE_PATH_CAP, |
中文说明
裸写的 12 只是为了够到新的第三个位置参数而复述 worktreeResidue 的默认 cap——一个对别的模块所拥有的默认值的无标签复制；目前只有 worktree.test.ts:164-165 钉住该默认值。这是唯一传递 sha 锚的生产调用点，因此若维护者日后修改 worktree.ts 的默认 cap，本调用点会悄悄保留 12，所有 agent 简报的残留列表将与其他调用方的报告分叉，且没有任何信号。请从 worktree.ts 导出一个命名常量（例如 export const RESIDUE_PATH_CAP = 12），默认值与这里都使用它；或给 worktreeResidue 一个选项对象（{ cap?, expectedHeadSha? }），让调用方永远不必为够到更靠后的参数而复述默认值。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| anchor = [ | ||
| `--git-dir=${realpathSync(gitDir)}`, | ||
| `--work-tree=${realpathSync(toplevel)}`, | ||
| ]; |
There was a problem hiding this comment.
[Critical] R2-9: The pin freezes names, not identity — every pinned spawn re-reads or re-resolves what those names point to, so a same-user writer active in the pin→measurement window spoofs every measurement. The comment above claims the pin "closes the window between this gate and the commands after it"; two channels executed against this function falsify the claim. Channel 1 (git-dir side): rewrite the pinned admin entry's commondir, HEAD and index after the pin, before status — every "pinned" command reads the forge, and a contaminated tree certifies clean 10/10 with a deterministic trigger (a watcher on the pin's own file opens; measured window ~8 ms). Channel 2 (work-tree side): mv <tree> <tree>.bak && ln -s <clean-copy> <tree> in the same window — every measurement re-resolves --work-tree through the link and scans the clean copy while --git-dir (outside the swapped directory) stays genuine, so the sha pin and the index reads pass: certified clean 10/10, plus a committed-gitlink variant 5/5 (readdirSync follows the link). Control runs report the residue 10/10 both ways; the contamination survives its own clean certification and re-detects after swap-back — this is the window the PR body's "a swap landing after the gate cannot redirect them either" claims closed. Witness: control {"paths":["a.ts","__probe__.test.ts"],"total":2} (10/10) vs attack {"paths":[],"total":0} (10/10 on both channels). The class is not closable channel-by-channel: concealing worktree files during the window evades any identity check, and a trailing re-check is evaded by a double-swap. Structural close: measure a snapshot of the tree taken at gate time, or run the probe under a sandbox boundary (the class is tracked in issue 9556, and it is the direction issue 9557 itself points at). Interim, in priority order: (1) correct this comment and the PR claim — the pin closes post-gate identity-redirect (gitfile swaps) and pre-planted shapes, not in-window state presentation by an active writer; (2) add trailing re-verification after the last spawn — re-run the anchored rev-parse HEAD and re-check the cwd path resolution (lstat / bounded walk / realpath against the pinned toplevel), refusing on divergence; patched into a scratch copy it flipped both channels to UNMEASURED (10/10 and 5/5), which raises the attack's cost — but present it as cost-raising, since a double-swap still evades it. Note: --git-common-dir=<path> is not a real git CLI flag, so that fix shape is unavailable.
中文说明
固定（pin）冻结的是名字，而不是身份——每一条固定的 spawn 都会重新读取或重新解析这些名字所指向的对象，因此在"固定→测量"窗口内活跃的同用户写入者可以欺骗所有测量。上方注释声称 pin"关闭了本门与后续命令之间的窗口"；针对本函数执行的两条通道证伪了该说法。通道 1（git-dir 侧）：在固定之后、status 之前改写被固定 admin 条目的 commondir、HEAD、index——所有"被固定"的命令读取的都是伪造仓库，带污染的树 10/10 被认证为干净，且触发是确定性的（监视固定自身打开文件的 watcher；实测窗口约 8ms）。通道 2（work-tree 侧）：在同一窗口内执行 mv <tree> <tree>.bak && ln -s <clean-copy> <tree>——所有测量都会透过链接重新解析 --work-tree 并扫描干净副本，而 --git-dir（位于被掉包目录之外）保持真实，于是 sha 固定与索引读取全部通过：10/10 认证为干净，另有已提交 gitlink 变体 5/5（readdirSync 会跟随链接）。两种形态下对照运行 10/10 均报告残留；污染在自己的"干净认证"之后依然存在，掉包还原后残留被重新检出——这正是 PR 正文"掉包即使落在门之后也无法重定向它们"所声称关闭的窗口。见证：对照 {"paths":["a.ts","__probe__.test.ts"],"total":2}（10/10）对比攻击 {"paths":[],"total":0}（两通道均 10/10）。该类无法逐通道关闭：窗口内隐藏工作树文件可以绕过任何身份检查，而尾部复查可被双重掉包绕过。结构性关闭：测量门时快照的树，或让探针运行在沙箱边界之内（该类别记录于 issue 9556，也正是 issue 9557 本身所指的方向）。过渡措施按优先级：（1）修正本注释与 PR 声明——pin 关闭的是门后的身份重定向（gitfile 掉包）与预先植入形态，而不是活跃写入者在窗口内的状态呈现；（2）在最后一条 spawn 之后追加再验证——重跑固定的 rev-parse HEAD 并复查 cwd 路径解析（lstat / 有界行走 / 与固定 toplevel 的 realpath 比对），不一致即拒绝；把该补丁打进临时副本后两通道均翻转为 UNMEASURED（10/10 与 5/5），这提高了攻击成本——但请如实表述为提高成本，因为双重掉包仍可绕过。注意：--git-common-dir=<path> 不是真实的 git CLI 参数，该修复形态不可用。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
The overclaim is fixed in code: the round-trip and sha-pin comment blocks no longer say the pin "closes the window" — they now say it freezes NAMES, that a writer active between the pin and the measurement can still rewrite the pinned admin entry's HEAD/index/commondir or swap the tree path itself, and that this raises the attack's cost rather than closing it, with the structural close (a gate-time snapshot or a sandbox boundary) named as #9556. The trailing re-verification is NOT implemented this round: worktreeResidue is fully synchronous (spawnSync end to end), so no deterministic vitest witness can exercise the pin→measurement window, and this repository's own rule is that a guard ships with a witness or not at all — an unwitnessed race guard would read as protection the code does not provide. The in-window active-writer class is the sandbox-tracked class of issue 9556; if a maintainer wants the cost-raising re-check despite the missing witness, say so on this thread and it comes back as a follow-up.
中文说明
过度声明已在代码中修正：round-trip 与 sha 固定的注释块不再声称 pin"关闭了窗口"——现在明确写明它冻结的是名字（NAMES），活跃在"固定→测量"窗口内的写入者仍可改写被固定 admin 条目的 HEAD/index/commondir，或掉包树路径本身；这只是提高攻击成本，并非关闭该类攻击，结构性关闭（门时快照或沙箱边界）指向 #9556。尾部再验证本轮未实现：worktreeResidue 全程同步（全部 spawnSync），vitest 中不存在能演练"固定→测量"窗口的确定性见证，而本仓库自身的规则是守卫要么带见证、要么不上——一个没有见证的竞态守卫会被误读为代码并不提供的保护。窗口内活跃写入者这一类别正是 issue 9556 所跟踪的沙箱类别；若维护者在缺少见证的情况下仍希望加上这个提高成本的复查，请在本帖说明，它将作为后续跟进项处理。
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32604037975 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI — the platform where R3-1's new-test failure fires was not exercised by CI or locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:715 — [review] the walk bound dirname(commonDir) is not an ancestor when the review runs from the user's own linked worktree — false unmeasured for a healthy tree (probe-confirmed)packages/cli/src/commands/review/lib/worktree.ts:757 — [review] the identity-gate sequence is hand-duplicated from scratch-tree.ts's reset gate (a third copy lives in test-efficacy.ts)packages/cli/src/commands/review/agent-prompt.ts:1337 — [review] the 40-hex-only fetchedSha regex diverges from sibling validators that admit 64-hex SHA-256 ids — on a SHA-256 repo the anchor is silently withheldpackages/cli/src/commands/review/lib/worktree.ts:844 — [review] the ...anchor spread is pinned by zero tests — mutants deleting it survive 59/59 and the gate-to-measurement window is deterministically openable via a git shim (probe)
Convergence: round 3 posted 12 inline comment(s), 3 of them reported for the first time; the previous round posted 9 (9 new). Findings keep coming back to the same files: packages/cli/src/commands/review/lib/worktree.ts (findings in round 2; 3 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.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI — the platform where R3-1's new-test failure fires was not exercised by CI or locally。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 12 条行内评论，其中 3 条是首次提出；上一轮发布了 9 条（其中 9 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/lib/worktree.ts（第 2 轮已出过发现，本轮又有 3 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| return worktreeResidue( | ||
| resolve(wt), | ||
| 12, | ||
| typeof sha === 'string' && /^[0-9a-f]{40}$/i.test(sha) ? sha : undefined, |
There was a problem hiding this comment.
[Critical] R2-1: The anti-forge anchor reaches the probe through a channel inside the attacker's own write boundary. fetchedSha is re-read from the plan JSON on disk at every agent-prompt invocation — including every wave after the reviewed code has executed (the PR's build/test runs mid-review as this user) — and this guard degrades to undefined when the field is absent or malformed. A same-user writer who rewrites .qwen/tmp/qwen-review-pr-<n>-fetch.json defeats the sha pin without reproducing it: deleting the field skips the pin entirely and round 1's forged admin entry (whose backpointer round-trips) certifies clean again, while setting the field to the forge's own HEAD sha passes the pin outright. In both corners the pinned status measures the forge's clean index and the contaminated tree certifies clean to every agent brief. Executed against this code: a forged admin entry beside a full-mirror forge (contamination committed, mutant on disk) answers {"paths":[],"total":0} — certified clean — to the sha-less call and unmeasured "…not the fetched PR head…" to the pinned call; the plan rewrite itself is a plain same-user file write, the same write class the issue's threat model names. Fail closed when the record is absent or malformed instead of degrading: every worktree-mode plan fetch-pr writes carries fetchedSha, so absence means tampering or corruption — return unmeasured from worktreeResidueOf rather than passing undefined. Longer term, the identity anchor must not be re-read from attacker-writable storage after reviewed code has executed; until then the comment above should claim "raises the plant's cost", not "cannot reproduce". Re-checked this round: the code is unchanged since round 2; the defect stands.
中文说明
反伪造锚（fetchedSha）是经由攻击者可写边界内的通道送达探针的。fetchedSha 在每次 agent-prompt 调用时都从磁盘上的计划 JSON 重新读取——包括审查代码已执行之后的每一波（本 PR 的构建/测试会在审查中途以本用户身份运行）——而字段缺失或格式错误时，此处的守卫会退化为 undefined。同用户写入者只要改写 .qwen/tmp/qwen-review-pr-<n>-fetch.json，就无需复现 sha 即可击败 sha 固定：删除该字段会完全跳过固定，第 1 轮那个 backpointer 可往返的伪造 admin 条目将再次被认证为干净；把该字段设为伪造仓库自己的 HEAD sha 则直接通过固定。两种角落下，固定的 status 测量的都是伪造仓库的干净索引，带污染的树会被认证为干净并写入每一份 agent 简报。已对本提交的代码执行验证：伪造 admin 条目 + 全量镜像伪造仓库（污染已提交、突变体在磁盘上），对无 sha 调用回答 {"paths":[],"total":0}（认证为干净），对固定调用回答 unmeasured "…not the fetched PR head…"；计划文件改写本身就是一次普通的同用户文件写入，正是 issue 威胁模型所命名的写入类别。记录缺失或格式错误时应改为失败关闭而不是退化：fetch-pr 写入的每个 worktree 模式计划都带有 fetchedSha，缺失即意味着篡改或损坏——worktreeResidueOf 应返回 unmeasured，而不是传 undefined。长期来看，审查代码执行之后，身份锚不应再从攻击者可写的存储中重新读取；在此之前，上方注释应声明"提高植入成本"，而不是"无法复现"。本轮复查：代码自第 2 轮以来未变，缺陷仍然成立。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // certified a mutant clean before the walk. | ||
| const redirected = lstatSync(cwd).isSymbolicLink() | ||
| ? resolve(cwd) | ||
| : redirectedAncestor(dirname(resolve(cwd)), dirname(commonDir)); |
There was a problem hiding this comment.
[Critical] R2-2: Symlink-redirect bypass — the walk's stop boundary comes from the verified repository's own planter-writable metadata. Plant a symlink at an ancestor of the tree below the checkout (here .qwen) and lay the redirect territory out so the resolved tree sits directly under the forge's common-dir root: the discovery spawn reports --git-common-dir = outside/tmp/common.git, the walk starts at <repo>/.qwen/tmp — whose realpath equals the boundary's realpath — and the stop test fires on iteration 1 without ever lstat'ing .qwen. A completely genuine git init + git worktree add pair in the territory then passes the round trip, and the sha-less production caller (scratch-tree.ts:474) certifies the forge clean — suppressing the "shared review worktree is NOT clean" warning scratch-tree prints to every verifier shard. Re-executed against this commit in the current round: NO-SHA: {"paths":[],"total":0} (forge certified clean) vs WITH-SHA: unmeasured "…not the fetched PR head…" (the pin still refuses); a contrast run with a non-bare forge (boundary one level deeper) catches the same .qwen link on iteration 2 — the evasion turns exactly on the boundary landing at the walk start's realpath. Round 1's R1-2 promised refusal of "a symlink planted at the worktree leaf or any ancestor below the checkout (e.g. .qwen/tmp)" — .qwen is such an ancestor. Require resolve(stopAt) to be a literal path-prefix of resolve(dir) before honoring the stop test — in a genuine review worktree the common dir's parent is a literal ancestor of the tree, in this bypass it is not — or lstat every component between cwd and the boundary before the boundary's identity, which comes from the thing being verified, is allowed to end the walk.
中文说明
符号链接重定向绕过——行走的停止边界来自被验证仓库自身的、植入者可写的元数据。在检出之下、树的某个祖先处（此处为 .qwen）植入符号链接，并把重定向领地布置成"解析后的树恰好位于伪造仓库 common-dir 根目录之下"：发现命令会报告 --git-common-dir = outside/tmp/common.git，行走从 <repo>/.qwen/tmp 开始——其 realpath 恰等于边界的 realpath——停止测试在第 1 轮迭代即命中，.qwen 本身从未被 lstat。领地中一对完全真实的 git init + git worktree add 随之通过往返校验，无 sha 的生产调用方（scratch-tree.ts:474）会将伪造仓库认证为干净——从而压掉 scratch-tree 向每个验证分片打印的 "shared review worktree is NOT clean" 警告。本轮已在当前提交上重新执行：NO-SHA: {"paths":[],"total":0}（伪造仓库被认证为干净）对比 WITH-SHA: unmeasured "…not the fetched PR head…"（固定仍然拒绝）；对照实验使用非 bare 伪造仓库（边界深一层）时，同一个 .qwen 链接在第 2 轮迭代即被捕获——绕过恰好取决于边界落在行走起点的 realpath 上。第 1 轮的 R1-2 承诺拒绝"植入在工作树叶子或检出之下任一祖先（例如 .qwen/tmp）的符号链接"——.qwen 正是这样一个祖先。在信任停止测试之前，要求 resolve(stopAt) 是 resolve(dir) 的字面路径前缀——在真实审查工作树中，common dir 的父目录是树的字面祖先，在本绕过中则不是——或者在允许"来自被验证对象自身的边界身份"终止行走之前，对 cwd 与边界之间的每一个分量做 lstat。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| anchor = [ | ||
| `--git-dir=${realpathSync(gitDir)}`, | ||
| `--work-tree=${realpathSync(toplevel)}`, | ||
| ]; |
There was a problem hiding this comment.
[Critical] R2-9: The pin freezes names, not identity — every pinned spawn re-reads or re-resolves what those names point to, so a same-user writer active in the pin→measurement window spoofs every measurement. The comment above claims the pin "closes the window between this gate and the commands after it"; two channels executed against this function falsify the claim. Channel 1 (git-dir side): rewrite the pinned admin entry's commondir, HEAD and index after the pin, before status — every "pinned" command reads the forge, and a contaminated tree certifies clean 10/10 with a deterministic trigger (a watcher on the pin's own file opens; measured window ~8 ms). Channel 2 (work-tree side): mv <tree> <tree>.bak && ln -s <clean-copy> <tree> in the same window — every measurement re-resolves --work-tree through the link and scans the clean copy while --git-dir (outside the swapped directory) stays genuine, so the sha pin and the index reads pass: certified clean 10/10, plus a committed-gitlink variant 5/5. Control runs report the residue 10/10 both ways; the contamination survives its own clean certification and re-detects after swap-back — this is the window the PR body's "a swap landing after the gate cannot redirect them either" claims closed. Witness: control {"paths":["a.ts","__probe__.test.ts"],"total":2} (10/10) vs attack {"paths":[],"total":0} (10/10 on both channels). The class is not closable channel-by-channel: concealing worktree files during the window evades any identity check, and a trailing re-check is evaded by a double-swap. Structural close: measure a snapshot of the tree taken at gate time, or run the probe under a sandbox boundary (the class is tracked in issue 9556, and it is the direction issue 9557 itself points at). Interim, in priority order: (1) correct this comment and the PR claim — the pin closes post-gate identity-redirect (gitfile swaps) and pre-planted shapes, not in-window state presentation by an active writer; (2) add trailing re-verification after the last spawn — re-run the anchored rev-parse HEAD and re-check the cwd path resolution (lstat / bounded walk / realpath against the pinned toplevel), refusing on divergence; patched into a scratch copy it flipped both channels to UNMEASURED (10/10 and 5/5), which raises the attack's cost — but present it as cost-raising, since a double-swap still evades it. Note: --git-common-dir=<path> is not a real git CLI flag, so that fix shape is unavailable. Re-checked this round: the code is unchanged since round 2; the defect stands.
中文说明
固定（pin）冻结的是名字，而不是身份——每一条固定的 spawn 都会重新读取或重新解析这些名字所指向的对象，因此在"固定→测量"窗口内活跃的同用户写入者可以欺骗所有测量。上方注释声称 pin"关闭了本门与后续命令之间的窗口"；针对本函数执行的两条通道证伪了该说法。通道 1（git-dir 侧）：在固定之后、status 之前改写被固定 admin 条目的 commondir、HEAD、index——所有"被固定"的命令读取的都是伪造仓库，带污染的树 10/10 被认证为干净，且触发是确定性的（监视固定自身打开文件的 watcher；实测窗口约 8ms）。通道 2（work-tree 侧）：在同一窗口内执行 mv <tree> <tree>.bak && ln -s <clean-copy> <tree>——所有测量都会透过链接重新解析 --work-tree 并扫描干净副本，而 --git-dir（位于被掉包目录之外）保持真实，于是 sha 固定与索引读取全部通过：10/10 认证为干净，另有已提交 gitlink 变体 5/5。两种形态下对照运行 10/10 均报告残留；污染在自己的"干净认证"之后依然存在，掉包还原后残留被重新检出——这正是 PR 正文"掉包即使落在门之后也无法重定向它们"所声称关闭的窗口。见证：对照 {"paths":["a.ts","__probe__.test.ts"],"total":2}（10/10）对比攻击 {"paths":[],"total":0}（两通道均 10/10）。该类无法逐通道关闭：窗口内隐藏工作树文件可以绕过任何身份检查，而尾部复查可被双重掉包绕过。结构性关闭：测量门时快照的树，或让探针运行在沙箱边界之内（该类别记录于 issue 9556，也正是 issue 9557 本身所指的方向）。过渡措施按优先级：（1）修正本注释与 PR 声明——pin 关闭的是门后的身份重定向（gitfile 掉包）与预先植入形态，而不是活跃写入者在窗口内的状态呈现；（2）在最后一条 spawn 之后追加再验证——重跑固定的 rev-parse HEAD 并复查 cwd 路径解析（lstat / 有界行走 / 与固定 toplevel 的 realpath 比对），不一致即拒绝；把该补丁打进临时副本后两通道均翻转为 UNMEASURED（10/10 与 5/5），这提高了攻击成本——但请如实表述为提高成本，因为双重掉包仍可绕过它。注意：--git-common-dir=<path> 不是真实的 git CLI 参数，该修复形态不可用。本轮复查：代码自第 2 轮以来未变，缺陷仍然成立。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Same disposition as the round-2 thread: the overclaim is fixed in code (the comments now state the pin freezes names, raises the attack's cost, and does not close the in-window class — the structural close is the sandbox boundary, #9556). The trailing re-verification is not implemented this round: the probe is fully synchronous, so no deterministic vitest witness can exercise the window, and an unwitnessed guard violates this repository's mutation-witness rule. If a maintainer wants the cost-raising re-check anyway, say so here and it comes back as a follow-up.
中文说明
与第 2 轮线程的处置相同：过度声明已在代码中修正（注释现在写明 pin 冻结的是名字、只提高攻击成本、并不关闭窗口内类别——结构性关闭是沙箱边界，见 #9556）。尾部再验证本轮未实现：探针全程同步，vitest 中无法确定性演练该窗口，无见证的守卫违反本仓库的突变见证规则。若维护者仍希望加上这个提高成本的复查，请在此说明，将作为后续跟进项处理。
| // its HEAD. Measured: through the forge the sha-less call certifies a | ||
| // mutant clean; pinned to the fetched sha, the same tree answers | ||
| // unmeasured. | ||
| if (expectedHeadSha !== undefined) { |
There was a problem hiding this comment.
[Critical] R1-1: The forged-admin-entry attack still certifies a plant clean for sha-less callers. The backpointer round-trip proves only that the gitfile's admin entry SAYS this tree is its worktree; a same-user attacker controls both halves of the pair — the .git gitfile and the repository it names — and can write the backpointer themselves. The sha pin added in round 2 refuses the forge on the sha-carrying flow (agent-prompt.ts) — except via open R2-1's plan rewrite — but the pin is conditional on this line, and the production caller scratch-tree.ts:474 invokes worktreeResidue WITHOUT a sha: for it a planted repository shaped like a linked worktree passes the gate and the pinned spawns measure the plant's index, certifying a contaminated tree clean and suppressing scratch-tree's "shared review worktree is NOT clean" warning. Executed in round 1 against this shape: GENUINE {"paths":["a.ts","__probe__.test.ts"],"total":2}; FORGED-ADMIN VERDICT: {"paths":[],"total":0} — certified clean, no unmeasured, mutant on disk; BROKEN-BACKPOINTER: unmeasured "does not point back" (flip arm). Fix direction: until every production caller carries a trusted sha record, treat a sha-less probe as unable to certify identity against a forged admin entry — refuse (unmeasured) rather than measure — or give scratch-tree.ts its own trusted head record to pin against (the tree's own HEAD cannot serve: it self-certifies).
中文说明
伪造 admin 条目的攻击对于不带 sha 的调用方仍然能把植入仓库认证为干净。backpointer 往返校验只证明 gitfile 的 admin 条目"声称"本树是它的工作树；同用户攻击者控制着这对关系的两端——.git gitfile 与它所指向的仓库——因此可以自己写出 backpointer。第 2 轮加入的 sha 固定在带 sha 的流程（agent-prompt.ts）中能拒绝伪造仓库（经 R2-1 的计划改写除外），但该固定以本行为条件，而生产调用方 scratch-tree.ts:474 调用 worktreeResidue 时不带 sha：对它而言，一个构造成链接工作树形态的植入仓库能通过这道门，随后被固定的各条命令测量的是植入仓库的索引，带污染的树被认证为干净，同时压掉 scratch-tree 的 "shared review worktree is NOT clean" 警告。第 1 轮已对该形态执行验证：真实读数 {"paths":["a.ts","__probe__.test.ts"],"total":2}；伪造 admin 条目裁决 {"paths":[],"total":0}——认证为干净，无 unmeasured，突变体仍在磁盘上；破坏 backpointer 对照：unmeasured "does not point back"（翻转对照）。修复方向：在每个生产调用方都携带可信 sha 记录之前，把无 sha 的探针视为无法对伪造 admin 条目做身份认证——拒绝（unmeasured）而不是测量——或者让 scratch-tree.ts 用自己独立的可信 head 记录来固定（树自身的 HEAD 不能充当：它会自我认证）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| top.status === 0 && | ||
| typeof top.stdout === 'string' && | ||
| realpathSync(top.stdout.trim()) === realpathSync(cwd); | ||
| realpathSync(toplevel) === realpathSync(cwd); |
There was a problem hiding this comment.
[Critical] R1-2: The symlink-redirect class is still open — the walk added in round 2 is bypassed by R2-2's boundary shape. A symlink planted at the worktree leaf or any ancestor below the checkout redirects cwd into attacker territory holding a completely genuine git init + git worktree add pair with the contamination COMMITTED — no forged admin entry needed; --show-toplevel answers the physical forge path and the self-equality on this line holds (both sides resolve through the same link), so the gate measures the plant. Round 2 added the redirectedAncestor walk in response, closing the leaf/ancestor inputs in the healthy layout — but open R2-2 demonstrates a shape within this finding's own promised scope (".qwen is an ancestor below the checkout") that still passes: the walk's stop test fires at iteration 1 on the attacker-steered boundary, before the planted component is ever lstat'd. Executed evidence — round 1: PRE-FIX baseline {"paths":["__probe__.test.ts","a.ts"],"total":2}, ancestorRedirect {"paths":[],"total":0}, leafRedirect {"paths":[],"total":0}; round 3 re-execution (R2-2): the walk-boundary shape still answers NO-SHA {"paths":[],"total":0}. Fixing R2-2's boundary handling (literal path-prefix stop test, or lstat every component between cwd and the boundary before the boundary may end the walk) closes the remaining entrance of this family.
中文说明
符号链接重定向类别仍然开放——第 2 轮加入的行走被 R2-2 的边界形态绕过。在工作树叶子或检出之下任一祖先植入符号链接，即可把 cwd 重定向到攻击者领地——那里放着一对完全真实的 git init + git worktree add，且污染已被提交——无需伪造任何 admin 条目；--show-toplevel 回答伪造仓库的物理路径，本行的自相等比较成立（两侧都经同一链接解析），于是门测量的是植入仓库。第 2 轮加入了 redirectedAncestor 行走作为回应，在健康布局下关闭了叶子/祖先植入——但仍未关闭的 R2-2 证明了一种落在本发现承诺范围之内的形态（".qwen 是检出之下的一个祖先"）仍可通过：行走的停止测试在第 1 轮迭代就在攻击者引导的边界上命中，被植入的分量从未被 lstat。执行证据——第 1 轮：修复前基线 {"paths":["__probe__.test.ts","a.ts"],"total":2}，祖先重定向 {"paths":[],"total":0}，叶子重定向 {"paths":[],"total":0}；第 3 轮重新执行（R2-2）：边界形态对无 sha 调用仍回答 {"paths":[],"total":0}。修复 R2-2 的边界处理（字面路径前缀停止测试，或在允许边界终止行走之前对 cwd 与边界之间的每个分量做 lstat）即可关闭本族的剩余入口。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| }; | ||
| } | ||
| if ( | ||
| realpathSync(dirname(resolve(gitDir, backpointer))) !== |
There was a problem hiding this comment.
[Suggestion] R2-4: A dangling admin-entry backpointer — the gitdir file names a nonexistent path (crash mid-worktree add, a cleanup gone wrong, a sloppy forge) — throws ENOENT out of realpathSync into the outer catch, producing the "not a git worktree" refusal: the exact misdiagnosis the inner try/catch above was written to avoid. git rev-parse still exits 0 in that state (verified with real git), so isWorktree is true and the comparison runs; the throw resets isWorktree = false in the outer catch and the probe reports "the path is not a git worktree (repository discovery walks up into the enclosing checkout)" for a path that IS a worktree with an existing admin entry — the triager six months from now hunts a missing-gitfile / walk-up problem that is not there. Still fail-closed; only the diagnosis is wrong. Witness: R24 rev-parse status: 0 with --show-toplevel answering the real tree path; R24 residue: {"paths":[],"total":0,"unmeasured":"the path is not a git worktree (repository discovery walks up into the enclosing checkout)"}. Give the comparison its own try and treat an unresolvable backpointer as "does not point back", reusing that refusal:
let pointsBack = false;
try {
pointsBack =
realpathSync(dirname(resolve(gitDir, backpointer))) ===
realpathSync(cwd);
} catch {
// A backpointer that does not resolve does not point back at this tree.
}
if (!pointsBack) {中文说明
悬空的 admin 条目 backpointer——gitdir 文件指向一个不存在的路径（worktree add 中途崩溃、清理出错、粗糙的伪造）——会让 realpathSync 抛出 ENOENT 落进外层 catch，产生 "not a git worktree" 拒绝：正是上方内层 try/catch 特意要避免的误诊。该状态下 git rev-parse 仍以 0 退出（已用真实 git 验证），因此 isWorktree 为 true、比较会执行；抛错使外层 catch 把 isWorktree 重置为 false，探针会对一个确实是工作树、admin 条目也确实存在的路径报告 "the path is not a git worktree (repository discovery walks up into the enclosing checkout)"——六个月后的排查者会去找一个并不存在的 gitfile 缺失 / 向上发现问题。仍是失败关闭；只是诊断错了。见证：R24 rev-parse status: 0，--show-toplevel 回答真实树路径；R24 residue: {"paths":[],"total":0,"unmeasured":"the path is not a git worktree (repository discovery walks up into the enclosing checkout)"}。请给该比较单独套一层 try，把无法解析的 backpointer 当作 "does not point back"，复用那条拒绝（代码见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // forge planted at the worktree cannot reproduce, and with it the probe | ||
| // refuses a forged admin entry (see worktreeResidue). | ||
| const sha = report.fetchedSha; | ||
| return worktreeResidue( |
There was a problem hiding this comment.
[Suggestion] R2-5: The new production wiring that hands the fetched sha to the residue probe has zero test coverage — no test anywhere supplies fetchedSha in a plan report (grep of agent-prompt.test.ts: no matches, re-verified this round). The tripwire integration this PR exists to harden is exercised only through the sha-less path. A regression that breaks the sha handover (field renamed, validation tightened, call reordered) ships green: no test observes the pinned path from the production entry point, so the probe silently degrades to its unpinned gates — exactly the hole this PR closes. Add a worktreeResidueOf-level test: a plan carrying a 40-hex fetchedSha reaches worktreeResidue with the sha (assert the pin refuses a forge at a different head), and a malformed/absent field degrades per the intended contract.
中文说明
把已获取 sha 交给残留探针的新生产接线完全没有测试覆盖——任何测试都没有在计划报告中提供 fetchedSha（grep agent-prompt.test.ts：无匹配，本轮已重新验证）。本 PR 要加固的绊线集成，只经由无 sha 路径被测试。任何破坏 sha 交接的回归（字段改名、校验收紧、调用顺序变动）都会绿灯通过：没有测试从生产入口观察固定路径，探针会静默退化到未固定的门——正是本 PR 要堵的洞。请补一个 worktreeResidueOf 层级的测试：携带 40 位十六进制 fetchedSha 的计划能把 sha 送达 worktreeResidue（断言固定会拒绝处于不同 head 的伪造仓库），缺失或格式错误的字段按约定退化。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| : head.stdout.trim(); | ||
| if ( | ||
| got === null || | ||
| got.toLowerCase() !== expectedHeadSha.toLowerCase() |
There was a problem hiding this comment.
[Suggestion] R2-6: Two branches of the new sha pin have no witness. (a) The case-fold: the caller's guard /^[0-9a-f]{40}$/i admits an uppercase sha, but every fixture passes lowercase, so a mutant dropping the second toLowerCase() (the comparison side) survives. (b) The got === null arm: no fixture makes the pinned rev-parse fail after the gate passed, so the distinct "the pinned identity could not read its own HEAD" refusal is untested. A future edit that breaks the case-fold or removes the null arm's distinct refusal ships green — the pin silently degrades or misreports on exactly the inputs the arms exist for. Pass one fixture an uppercase fetchedSha and assert parity, and one fixture a pinned identity whose HEAD cannot be read, asserting the "could not read its own HEAD" reason.
中文说明
新 sha 固定有两个分支没有见证。(a) 大小写折叠：调用方的守卫 /^[0-9a-f]{40}$/i 接受大写 sha，但所有夹具都传小写，因此删掉第二个 toLowerCase()（比较侧）的突变体会存活。(b) got === null 分支：没有任何夹具能让固定后的 rev-parse 在门通过之后失败，因此独立的 "the pinned identity could not read its own HEAD" 拒绝未被测试。未来任何破坏大小写折叠、或删去 null 分支独立理由的修改都会绿灯通过——固定会在恰恰是这两个分支所针对的输入上静默退化或误报。请给一个夹具传大写 fetchedSha 并断言等价行为，再给一个夹具构造"固定身份无法读取自身 HEAD"的状态，断言 "could not read its own HEAD" 理由。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| '--no-verify', | ||
| ); | ||
| const admin = join(forge, '.git', 'worktrees', 'evil'); |
There was a problem hiding this comment.
[Suggestion] R2-7: The fgit wrapper and the forge-repo setup block are pasted four times in this diff (fgit at lines ~206, 264, 364, 422 — this anchor sits in the second copy; the init + contamination writes + add -A + commit setup follows each, and the copies vary only in their tails: one adds core.worktree, two add worktree add). The shared core is cleanly factorable. The next time the forge recipe must change — an additional -c flag a newer git requires, a different init invocation, an extra setup step — the change has to be applied in four places, and a copy missed or edited differently makes one attack-shape test exercise a different fixture than its siblings while all four still pass. Add a local helper in the describe scope, e.g. function forgeAt(dir: string, commitMsg: string) { mkdirSync(dir); const fgit = (...args: string[]) => execFileSync('git', ['-c', 'user.email=t@t.t', '-c', 'user.name=t', '-c', 'commit.gpgsign=false', ...args], { cwd: dir, encoding: 'utf8' }); fgit('init', '-q', '-b', 'main', '--template=', '.'); writeFileSync(join(dir, 'a.ts'), 'export const x = 2; // MUTANT\n'); writeFileSync(join(dir, '__probe__.test.ts'), 'probe'); fgit('add', '-A'); fgit('commit', '-qm', commitMsg, '--no-verify'); return fgit; } and let each test append its own tail.
中文说明
fgit 包装器与伪造仓库的搭建块在本 diff 中被粘贴了四次（fgit 位于约 206、264、364、422 行——本锚点在第二份拷贝内；每份之后跟随 init + 污染写入 + add -A + commit 搭建，各拷贝仅在尾部不同：一份加 core.worktree，两份加 worktree add）。共享核心可以干净地提取。下次伪造配方必须变更时——新 git 要求的额外 -c 参数、不同的 init 调用、新增的搭建步骤——改动必须在四处分别应用，任何一份漏改或改法不同，都会让某个攻击形态测试使用与兄弟测试不同的夹具，而四份测试却全部通过。请在 describe 作用域内加一个本地辅助函数（示例代码见上方英文部分），让每个测试各自追加自己的尾部。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round (batch bound), same as the round-2 thread. Progress this round: two of the four fgit copies are gone — the gitfile-swap fixture now points the gitfile at the enclosing repository's own git dir, and the forged-admin fixture hand-writes its entry inside the real repository's .git/worktrees/, so neither builds an external forge any more. The remaining two copies live in the redirect tests; the helper extraction is smaller now and lands next round.
中文说明
与第 2 轮线程相同，按批次上限延后到下一轮。本轮已有进展：四份 fgit 拷贝中已消除两份——gitfile 掉包夹具现在把 gitfile 指向外层仓库自身的 git 目录，伪造 admin 条目夹具直接在真实仓库的 .git/worktrees/ 内手写条目，两者都不再搭建外部伪造仓库。剩余两份拷贝位于重定向测试中；抽取现在更小，将在下一轮提交。
| resolve(wt), | ||
| 12, |
There was a problem hiding this comment.
[Suggestion] R2-8: The bare 12 restates worktreeResidue's default cap solely to reach the new third positional parameter — an unlabeled duplicate of a default owned by another module; only worktree.test.ts pins that default, and this call site has no coverage at all (see R2-5). If worktreeResidue's default cap changes, this call site keeps passing the stale literal and the two silently disagree, with no diagnostic. Make the cap impossible to misstate from here: accept an options object ({ cap?, expectedHeadSha? }) or a thin wrapper worktreeResiduePinned(path, sha) that owns the default, so this call site names no number it does not own.
中文说明
裸写的 12 只是为了够到新的第三个位置参数而复述 worktreeResidue 的默认 cap——一个由其他模块拥有的默认值的无标签副本；只有 worktree.test.ts 钉住了该默认值，而本调用点完全没有覆盖（见 R2-5）。若 worktreeResidue 的默认 cap 变更，本调用点会继续传递过期的字面量，两者静默不一致且无任何诊断。请让此处无法误传 cap：改用选项对象（{ cap?, expectedHeadSha? }），或一个拥有默认值的薄包装 worktreeResiduePinned(path, sha)，使本调用点不再写出任何不属于它自己的数字。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…ndaries (#9742) The residue probe's identity gate answered CLEAN for shapes it cannot vouch for. Four closes, all fail-closed: - A repository answering for a path it does not contain — the common dir no literal ancestor of the tree path — is refused before the symlink walk. That is the shape a forge uses to steer the walk's own stop boundary so it fires before the planted link is lstat'd, and the walk's bound escaping to the filesystem root misdiagnosed refusals (and reddened the suite on hosts whose tmpdir resolves through a symlink). - A clean measurement no fetched-head record anchored is refused: a forged pair answers clean too, so an unanchored empty status certifies whichever index the gitfile names. Dirty reports still name paths — a forge answers clean, never dirty. - The agent-prompt wiring fails closed when the plan's fetchedSha is absent or malformed instead of degrading to an unpinned probe: every worktree-mode fetch writes the field. - A dangling admin-entry backpointer is "does not point back", not the outer catch's "not a git worktree". Comments now say what the pin actually closes (post-gate identity redirect, pre-planted shapes) and what it does not (in-window state presentation by an active writer — cost-raising, not closure; the structural close is the sandbox boundary, #9556). Witnessed: every guard flips its focused tests when removed (mutation probes), including new ones for the case-fold, the unreadable-HEAD arm, the sha handover seam, and the sha-less production caller. Fixtures realpath their roots so the suite holds under any tmpdir spelling.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round 4 summary — PR #9742 (review residue probe identity gates)This round closed the remaining certification holes in the residue probe's Resolved in code
Deferred / declined
Notes
VerificationCommands run and their results:
中文说明第 4 轮总结 — PR #9742（审查残留探针身份门）本轮关闭了残留探针身份门上剩余的认证漏洞。21 条行内评论共对应 12 条独立发现：本轮在代码中解决 10 条（17 个评论 id）；1 条 Critical（R2-9）部分解决，其余部分以证据拒绝并保持线程开放；1 条 Suggestion（R2-7，夹具去重）按批次上限延后至下一轮。单个提交： 已在代码中解决
延后 / 拒绝
备注
验证实际执行的命令及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Convergence: round 4 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 12 (3 new). Findings keep coming back to the same files: packages/cli/src/commands/review/lib/worktree.ts (findings in rounds 2, 3; 3 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
收敛情况：第 4 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 12 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/lib/worktree.ts（第 2、3 轮已出过发现，本轮又有 3 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // distinguishes this tree from a forged pair whose index already holds the | ||
| // contamination as committed content — so an empty measurement no record | ||
| // anchored is refused rather than certified. | ||
| if (paths.length === 0 && expectedHeadSha === undefined) { |
There was a problem hiding this comment.
[Suggestion] The sha-less production caller (scratch-tree.ts:474) has no channel to bring the fetched-sha record, so this new no-record clean refusal fires on every healthy run — the pipeline's tampering signal now fires on 100% of clean runs. sharedTreeUnmeasured is set on every call and the note is appended to every verifier report, so a run where the note fires for genuine cause becomes indistinguishable from healthy noise — the signal this PR exists to protect is flattened by the PR's own blast radius, and consumers documented to read sharedTreeUnmeasured as "not clean" (scratch-tree.ts:110) can no longer tell a suspicious tree from a healthy caller without a record. The record exists one hop away: fetch-pr writes fetchedSha into the plan (fetch-pr.ts:1583) and siblings already read it from there (compose-review.ts:1954, build-test.ts:414). Probe on a healthy clean fixture: AVAILABLE: true, SHARED_RESIDUE: [], UNMEASURED: "the caller brought no record of the commit this tree must hold…" on 100% of clean runs; the pre-change CLI emitted no note on the same tree. Give scratch-tree the anchor — a --fetched-sha option (or a --plan path) threaded to worktreeResidue(worktree, RESIDUE_PATH_CAP, fetchedSha) at scratch-tree.ts:474 — or, if always-refuse is the intended steady state, say so in the PR thread so the permanent note is a decision, not a surprise.
中文说明
无 sha 的生产调用方（scratch-tree.ts:474）没有任何通道携带 fetched-sha 记录，因此这条新的"无记录即拒绝干净"会在每一次健康运行中触发——流水线的篡改告警信号现在在 100% 的干净运行上都会响起。每次调用都会设置 sharedTreeUnmeasured，并且每个验证器报告都会附上这条说明——于是真正因故触发的运行与正常噪音变得无法区分：本 PR 要保护的信号被本 PR 自己的影响范围抹平了；被文档标注为按"不干净"读取 sharedTreeUnmeasured 的消费方（scratch-tree.ts:110）也无法再区分"可疑的树"与"调用方恰好没有记录的健康的树"。该记录其实只隔一跳：fetch-pr 会把 fetchedSha 写入计划文件（fetch-pr.ts:1583），同类命令已在从那里读取（compose-review.ts:1954、build-test.ts:414）。对健康干净夹具的探针：100% 的干净运行得到 AVAILABLE: true、SHARED_RESIDUE: []、UNMEASURED: "the caller brought no record of the commit this tree must hold…"；改动前的 CLI 在同一棵干净共享树上不会产生任何说明。请给 scratch-tree 传入锚：新增 --fetched-sha 选项（或 --plan 路径），接到 scratch-tree.ts:474 的 worktreeResidue(worktree, RESIDUE_PATH_CAP, fetchedSha)——或者，如果"永远拒绝"就是预期的稳态，请在 PR 线程中说明，让这个常驻说明成为一个决定，而不是意外。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| paths: [], | ||
| total: 0, | ||
| unmeasured: | ||
| 'the caller brought no record of the commit this tree must hold, ' + |
There was a problem hiding this comment.
[Suggestion] Both renderers of residue.unmeasured (agent-prompt.ts:1410, scratch-tree.ts:476) still hard-code the "(git status failed: …)" framing, but this diff makes the majority of unmeasured reasons ones where git status did not fail — it never ran (caller sha absent/malformed, pin mismatch, pinned HEAD unreadable, the gate refusals) or it ran and answered CLEAN (this no-record tail refusal). On a healthy, genuinely clean shared review worktree, scratch-tree always hits the no-record tail refusal, so the note reads "could not be measured (git status failed: the caller brought no record of the commit this tree must hold…)" — self-contradictory in one sentence; probe-confirmed on a healthy clean tree. An agent or operator told "git status failed" debugs the git environment (running git status, which succeeds) instead of looking at the plan file or the missing identity record — the prefix actively misdirects triage of exactly the refusals this round adds. Neutralise the prefix in both renderers (the only two consumers of unmeasured), e.g. **Whether it is clean could not be measured** (reason: …) at agent-prompt.ts:1410 and the same at scratch-tree.ts:476, keeping the two renderings in sync.
中文说明
residue.unmeasured 的两个渲染器（agent-prompt.ts:1410、scratch-tree.ts:476）仍然硬编码 "(git status failed: …)" 这一框架，但本 diff 使大多数 unmeasured 原因对应的场景里 git status 并没有失败——它要么根本没有运行（调用方 sha 缺失/格式错误、固定不一致、固定的 HEAD 不可读、各身份门的拒绝），要么运行了并且回答干净（即这条无记录的末尾拒绝）。对一个健康且真正干净的共享审查工作树，scratch-tree 总是命中无记录的末尾拒绝，于是说明读作 "could not be measured (git status failed: the caller brought no record of the commit this tree must hold…)"——同一句话内自相矛盾；探针已在健康干净树上观察到这条说明。被告知 "git status failed" 的 agent 或值班运维会去调试 git 环境（执行 git status，而它是成功的），而不是去查看计划文件或缺失的身份记录——这个前缀对本轮新增的拒绝而言会主动误导排查方向。请把两个渲染位置的前缀改为中性标签（它们是 unmeasured 仅有的两个消费方），例如 agent-prompt.ts:1410 改为 **Whether it is clean could not be measured** (reason: …)，scratch-tree.ts:476 同步修改。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // self-equality above holds. The walk is the one every sibling | ||
| // identity gate applies. Measured: both shapes certified a mutant | ||
| // clean before the walk. | ||
| const redirected = redirectedAncestor(dirname(spelled), bound); |
There was a problem hiding this comment.
[Suggestion] The ancestor-symlink walk arm has no test that reaches it — both new symlink tests refuse at earlier gates (the boundary check, whose own comment says "Refused before the symlink walk even runs", and the leaf lstat), so a mutation deleting this branch ships green; probe-confirmed: deleting the arm alone leaves worktree.test.ts 60/60, scratch-tree.test.ts 41/41 and agent-prompt.test.ts 290/290 passing. Round 3's ancestor-redirect test exercised exactly this arm; this round's containment gate moved the refusal earlier and silently dropped the coverage. The branch is reachable: an intermediate symlink (…/.qwen/tmp) whose target carries a gitfile naming the REAL repo's admin entry passes the leaf check, the self-equality and the literal-ancestor boundary, leaving only the walk to refuse. Dropping this arm alone cannot flip a verdict to certifying contamination (the sha pin and the no-record tail still catch every construct found), so this is lost coverage of a defence-in-depth gate, not a shippable incorrect behaviour. Add a test: with the genuine fixture measured first, move the worktree contents to an outside directory, plant a symlink at dirname(tree) pointing there, and leave the moved tree's .git file naming its original admin entry in repo; assert worktreeResidue(tree) returns paths: [] and unmeasured containing 'resolves through a symlink'. That shape passes the leaf, self-equality and boundary gates, so the new assertion can only be satisfied by the walk.
中文说明
祖先符号链接行走分支没有任何能到达它的测试——两个新的符号链接测试都在更早的门被拒绝（边界检查，其自身注释写着 "Refused before the symlink walk even runs"；以及叶子 lstat），因此删除该分支的突变体可以绿灯通过；探针已确认：仅删除该分支，worktree.test.ts 仍 60/60、scratch-tree.test.ts 41/41、agent-prompt.test.ts 290/290 全部通过。第 3 轮的祖先重定向测试恰好验证过这个分支；本轮插入的包含检查门把拒绝提前了，这条覆盖被悄悄丢掉。该分支是可达的：在中间层植入符号链接（…/.qwen/tmp），其目标携带一个指向真实仓库 admin 条目的 gitfile——它能通过叶子检查、自相等检查与字面祖先边界检查，只剩行走本身能拒绝它。仅删除该分支不会把裁决翻转为"认证污染干净"（sha 固定与无记录末尾拒绝仍能捕获所构造的全部形态），因此这是一条纵深防御门丢失的测试覆盖，而不是可上线的错误行为。请新增测试：先对真实夹具完成一次测量，然后把工作树内容移到外部目录，在 dirname(tree) 处植入指向那里的符号链接，并让被移动树的 .git 文件继续指向 repo 中原来的 admin 条目；断言 worktreeResidue(tree) 返回 paths: [] 且 unmeasured 包含 'resolves through a symlink'。该形态能通过叶子、自相等与边界各门，因此新断言只能由行走分支满足。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…d reasons (#9742) Three closes on the round-4 findings, all inside the residue probe's own footprint: - The sha-less production caller flattened the tampering signal the probe exists to protect: the no-record refusal fired on 100% of clean runs, so a note that fires always is a note nobody reads. scratch-tree now takes --fetched-sha, agent-prompt welds the plan's record into the verifier's command, and a healthy shared tree measures clean again — a forged pair is refused at the pin with a distinguishable reason instead. - The unmeasured renderers blamed `git status` for refusals where it never failed (the gates run before it; the no-record tail runs after it answered CLEAN), sending triagers to debug a git environment with nothing wrong. All three renderings now name a reason. - The ancestor-symlink walk had no witness: both sibling shapes refuse at earlier gates, and deleting the arm shipped green (measured). A shape that passes the leaf, the self-equality and the boundary gate leaves only the walk to refuse it.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9742Commit: All three inline findings were verified against the current code before any change and are resolved in code; every new guard has its own mutation-probe witness. The review body's convergence observation (round 4, no open Critical) was read; it carries no actionable item of its own, and its "same root cause" note matches what this round does: the three findings were three surfaces of one gap — the probe's new identity gates had no anchored production caller, mislabelled framing, and one unwitnessed arm. Feedback points and dispositions[rc:3837595930] scratch-tree's sha-less call flattens the tampering signal — RESOLVED (implemented option a). Verified by the PR's own pre-existing test ( [rc:3837595932] "(git status failed: …)" framing misdirects triage — RESOLVED. Verified by reading all rendering sites: the majority of unmeasured reasons are refusals where [rc:3837595935] the ancestor-symlink walk arm has no witness — RESOLVED. Reproduced the reviewer's probe first: deleting the arm left all 391 tests green (worktree 60, scratch-tree 41, agent-prompt 290). Added the exact shape the finding prescribes: measure the genuine fixture clean, move the worktree outside, rewrite its gitfile to name the original admin entry absolutely, plant a symlink at Mutation probes (each new guard's witness)
Verification
中文说明轮次总结 — PR #9742提交： 三条行内发现均在改动前针对当前代码完成验证，并已在代码中解决；每一个新增守卫都有独立的突变探针见证。审查正文中的收敛性观察（第 4 轮，无未决 Critical）已阅读；其本身没有可执行事项，其中"同一根因"的判断与本轮工作一致：三条发现是同一缺口的三个表面——探针新增的身份门没有一个已锚定的生产调用方、拒绝说明的措辞误导排查、以及一条没有测试见证的分支。 反馈点与处置[rc:3837595930] scratch-tree 无 sha 调用抹平了篡改信号 — 已解决（实现方案 a）。 由本 PR 已有的测试（ [rc:3837595932] "(git status failed: …)" 措辞误导排查 — 已解决。 通过阅读全部渲染位置验证：大多数 unmeasured 原因对应的场景中 [rc:3837595935] 祖先符号链接行走分支没有见证 — 已解决。 先复现了审查者的探针：删除该分支后全部 391 个测试仍为绿色（worktree 60、scratch-tree 41、agent-prompt 290）。按发现给出的形态新增测试：先对真实夹具测得干净，把工作树移到外部目录，将其 gitfile 改写为以绝对路径指向原仓库中的 admin 条目，再在 突变探针（每个新守卫的见证）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 1b": none — all checks above ran to completion..
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.test.ts:466 — [review] forge-territory fixture pasted verbatim into two tests (deferred — anchored on code unchanged since round 4)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 1b"：none — all checks above ran to completion.。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| .option('fetched-sha', { | ||
| type: 'string', |
There was a problem hiding this comment.
[Suggestion] R5-1: The new --fetched-sha CLI option has no test through the yargs boundary — every fetchedSha test calls runScratchTree directly, and the handler tests only ever pass {worktree, label, out}. The handler reads args.fetchedSha off argv as unknown as ScratchTreeArgs, relying on yargs' camel-case expansion of --fetched-sha, and the only production delivery of the sha is this flag, welded into the verifier's command by buildRoleBrief. If the option key and the interface field ever drift (a rename on either side, or a global camel-case-expansion disable), every real invocation arrives with fetchedSha === undefined, so every shared-tree probe answers "brought no record" — the anchor silently never engages again and the suite stays green. This repo already shipped exactly this bug class (--build-test in test-plan) and pinned it with "CLI option contract" tests; scratch-tree got the new flag without one. Add the same contract test as test-delta.test.ts:519-580: parse the flags through scratchTreeCommand.builder, feed the parsed object to runScratchTree, and assert sharedTreeUnmeasured is undefined (and red with a wrong sha).
const parsed = (scratchTreeCommand.builder as (y: Argv) => Argv)(
yargs([]),
).parseSync([
'--worktree', worktree,
'--label', 'verify--round-1--cli',
'--fetched-sha', headSha,
]);
expect(
runScratchTree(parsed as unknown as ScratchTreeArgs).sharedTreeUnmeasured,
).toBeUndefined();中文说明
[建议] 新增的 --fetched-sha CLI 选项没有经过 yargs 边界的测试——所有 fetchedSha 测试都直接调用 runScratchTree，handler 测试也只传 {worktree, label, out}。handler 通过 argv as unknown as ScratchTreeArgs 读取 args.fetchedSha，依赖 yargs 对 --fetched-sha 的驼峰展开；而 sha 唯一的生产送达通道就是这个标志（由 buildRoleBrief 焊接进 verifier 的命令）。如果选项键与接口字段发生漂移（任一侧改名，或全局关闭驼峰展开），每次真实调用都会带着 fetchedSha === undefined 到达，每个共享树探针都回答 "brought no record"——锚点悄无声息地永不再生效，而测试套件仍是绿的。本仓库已经出过完全相同的 bug 类别（test-plan 的 --build-test），并用 "CLI option contract" 测试将其钉住；scratch-tree 新增了这个标志却没有相应测试。建议补上与 test-delta.test.ts:519-580 相同的契约测试：让 scratchTreeCommand.builder 解析这些标志，把解析结果传给 runScratchTree，断言 sharedTreeUnmeasured 为 undefined（传入错误 sha 时应为红）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| ).toContain(`--fetched-sha ${sha}`); | ||
| expect(p).not.toContain('--fetched-sha'); |
There was a problem hiding this comment.
[Suggestion] R5-2: The weld test pins the flag's presence but not the line continuation that keeps it inside the command — deleting the continuation in --label ${label}${sha === undefined ? '' : ' \\'} emits a two-statement snippet: the verifier's first command runs review scratch-tree … --label … unpinned and the --fetched-sha line becomes a standalone command-not-found. toContain('--fetched-sha …') still passes, so every verify shard would silently measure unanchored with the suite green. The sibling mutation — appending \\ unconditionally — leaves a dangling continuation before the closing fence in the no-record case, likewise invisible to not.toContain('--fetched-sha'). Verified by probe against this commit: the continuation-deleting mutant passes this test (Tests 1 passed | 289 skipped), and a continuation-pinning assertion fails against it (AssertionError … to contain '--label verify--round-2--deadbeef1234…'). Pin the joined fragment instead of the bare flag.
| ).toContain(`--fetched-sha ${sha}`); | |
| expect(p).not.toContain('--fetched-sha'); | |
| ).toContain( | |
| `--label verify--round-2--deadbeef1234 \\ | |
| --fetched-sha ${sha}`, | |
| ); | |
| expect(p).not.toMatch(/--label verify--round-2--deadbeef1234 \\/); |
中文说明
[建议] 焊接测试只钉住了标志的存在，没有钉住让它留在同一条命令内部的那条行续接符——删除 --label ${label}${sha === undefined ? '' : ' \\'} 中的续接符会产出一段两个语句的片段：verifier 的第一条命令以未固定身份运行 review scratch-tree … --label …，而 --fetched-sha 那行变成一个独立的 command-not-found。toContain('--fetched-sha …') 仍然通过，于是每个验证分片都会悄无声息地以无锚方式测量，而套件仍是绿的。兄弟突变——无条件追加 \\——会在无记录分支的闭合围栏前留下悬空续行符，同样对 not.toContain('--fetched-sha') 不可见。已对本提交用探针验证：删除续接符的突变体能通过本测试（Tests 1 passed | 289 skipped），而钉住续接符的断言能令其失败（AssertionError … to contain '--label verify--round-2--deadbeef1234…'）。请钉住拼接后的完整片段，而不是裸标志。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6c": running worktree.test.ts under vitest to confirm green — the shared worktree has no node_modules / dist , and install + monorepo build was out of proportion ….
Test Plan (not a blocker): 504 passed — this review observed 23292 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 6c"：running worktree.test.ts under vitest to confirm green — the shared worktree has no node_modules / dist , and install + monorepo build was out of proportion …。
Test Plan（非阻断）：504 passed — this review observed 23292 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 60 passed · 2 failed · 62 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：60 通过 · 2 失败 · 62 总计 抖动门：✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9742 — deep verification (follow-up round)Verdict: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：60 通过 · 2 失败 · 62 总计 中文摘要
Previous-finding status at the new headThe prior round verified
Prior-round corrections (stale PR body) also stand — see Corrections. Central claim and A/BCentral claim: the residue probe's identity gate fails closed on forged/redirected git identity (and pins the verified identity across every spawn), while healthy trees still measure — clean with the fetched-sha anchor, dirty without it. Harness:
Witnesses: Pin equivalence ( Suite gate: Corrections to the descriptionAll three corrections from the prior round stand; re-verified at the new head:
FindingsF1 — boundary gate mixes spelled and physical paths: false refusal of healthy trees under symlinked-ancestor spellings (medium, fail-closed) — STANDS, fix now measuredUnchanged gate at the new head ( Blast radius is unchanged: every worktree-mode consumer ( Measured candidate fix (new this round) — apply to a scratch copy, drive through the same harnesses, restore ( - const spelled = resolve(cwd);
+ const spelled = realpathSync(resolve(cwd));Three measured results:
Cost the author must price: with F2 — guard census at the new head: leaf lstat redundant defence, walk load-bearing for pinned callers (low, completeness) — STANDSMutation matrix ( Classification re-measured:
The new commit's shared fixture feeds exactly two tests, and both are pinned by the matrix: the ancestor-redirect test dies on M4 ( F3 — wrong sha on a dirty tree answers unmeasured, not residue (informational) — STANDSRe-measured cell C8: the sha gate runs before measurement, so a wrong-sha call on a dirty tree returns Not covered
MethodologyEnvironment: the lane's own Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9742One commit this round: Feedback dispositions[ic:5385812484] F1 (medium) — FIXED. The boundary gate compared [ic:5385812484] F2 (low) — DECLINED. The finding's own census concludes the leaf lstat is redundant defence that is "correct as-is; a cheap first door", not a coverage gap, and upgrades the walk to load-bearing for pinned callers — which the harness witnesses (05) confirm and the suite's own witnesses already pin. No defect, no change; removing the leaf check would only shrink defence the analysis says is correct. [ic:5385812484] F3 (informational) — DECLINED. Explicitly "Note only". The sha gate answering [rv:5002464387] — NO ACTION. Partial-review disclosure with an empty findings ledger. Its non-blocking note concerns the PR description's stale test count and first-commit scope wording; the PR body is owned by the workflow (this bot performs no GitHub writes), so there is nothing to change in code. Changes
Conflict notesNone — Verification
中文说明Autofix 本轮总结 — PR #9742本轮一个提交： 反馈处理[ic:5385812484] F1（中）— 已修复。 边界门把 [ic:5385812484] F2（低）— 拒绝。 该发现的自身清点已得出结论：叶子 lstat 是"现状正确、便宜的第一道门"的冗余纵深防御，不是允许认证的覆盖缺口；并把 walk 升级为带 pin 调用方的承重件 —— 这由 harness 见证（05）确认，且套件自身的见证已经钉住。没有缺陷，不做改动；删除叶子检查只会削减该分析认定为正确的防御。 [ic:5385812484] F3（信息级）— 拒绝。 明确标注"仅提示"。在错 sha 的脏树上，sha 门禁回答 [rv:5002464387] — 无需处理。 部分审查披露，发现清单为空。其非阻断提示针对 PR 描述中过时的测试数量与首个提交的范围措辞；PR 正文由工作流维护（本机器人不做任何 GitHub 写操作），代码侧无可改动。 变更
冲突说明无 —— 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:1165 — [probe] no-record refusal masks the three blind-set/index-bit witness tests
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const spelled = resolve(cwd); | ||
| const bound = dirname(commonDir); | ||
| let contained = spelled.startsWith(bound + sep); |
There was a problem hiding this comment.
[Critical] The containment gate string-compares paths from two sources without canonicalising separators: bound = dirname(commonDir) keeps git's forward-slash rendering while spelled and sep follow Node's platform spelling. On Windows, resolve(cwd) and realpathSync produce backslash paths (C:\repo\...) and git prints --git-common-dir with forward slashes (C:/repo/.git), so bound + sep is C:/repo\, which no backslash path starts with — neither the literal test nor the realpath fallback (which reuses the same bound + sep) can ever match. On Windows the gate refuses every tree, healthy or not: the sha pin this PR adds is never reached, every scratch-tree report and agent brief permanently says the shared tree could not be measured, and the un-skipped clean-certification test at worktree.test.ts:114 fails in the merge-queue test_windows job (.github/workflows/ci.yml:875), the only Windows lane. The rest of this file routes both sides of its cross-source comparisons through Node (samePath at ~383, the realpath'd self-equality) — this gate is the lone separator-mixing comparison.
Witness — probe running the exact shipped expressions through Node's real path.win32 implementations:
AS SHIPPED:
spelled = "C:\repo\.qwen\tmp\review-wt"
bound = "C:/repo"
bound + sep = "C:/repo\"
contained = false
CANONICALIZED:
boundReal = "C:\repo"
contained = true
POSIX control: contained = true
(An actual Windows git and the real test_windows job were not exercisable from the Linux review host; slash-form output is Git for Windows' documented behavior.)
The fix spans the fallback below, so a full block rather than a one-click suggestion — canonicalize both sides through Node before comparing:
const spelled = resolve(cwd);
const bound = dirname(commonDir);
let contained = spelled.startsWith(bound + sep);
if (!contained) {
try {
contained = realpathSync(spelled).startsWith(realpathSync(bound) + sep);
} catch {
// Unresolvable: the literal test is the whole containment test.
}
}realpathSync accepts git's forward-slash spelling and re-emits the platform spelling, so both sides agree on Windows; on POSIX this also covers the mirror case where the tree arrives physically spelled while bound carries a link in its spelling.
中文说明
该包含门在比较来自两个不同来源的路径时没有统一分隔符：bound = dirname(commonDir) 保留了 git 渲染的正斜杠，而 spelled 与 sep 遵循 Node 的平台拼写。在 Windows 上，resolve(cwd) 与 realpathSync 产生反斜杠路径（C:\repo\...），而 git 以正斜杠输出 --git-common-dir（C:/repo/.git），于是 bound + sep 变成 C:/repo\，任何反斜杠路径都不可能以其开头——字面测试与 realpath 兜底（复用了同一个 bound + sep）都不可能匹配。在 Windows 上该门会拒绝每一棵树，无论健康与否：本 PR 新增的 sha 固定永远无法到达，每一份 scratch-tree 报告与 agent 简报都会永久声称共享树"无法测量"，且 worktree.test.ts:114 处未跳过的干净认证测试会在合并队列的 test_windows 作业（.github/workflows/ci.yml:875）中失败——那是唯一的 Windows 通道。本文件其余跨来源比较的两侧都经由 Node 规范化（约 383 行的 samePath、realpath 后的自相等比较）——该门是唯一混用分隔符的比较。
见证——用 Node 真实的 path.win32 实现逐字运行 shipped 表达式的探针（输出见上方英文部分代码块）。
（真实的 Windows git 与 test_windows 作业无法在 Linux 审查主机上执行；正斜杠输出是 Git for Windows 的文档化行为。）
修复跨越下方的兜底分支，因此给出完整代码块而非一键建议：比较前先用 Node 规范化两侧（代码见上方英文部分）。realpathSync 接受 git 的正斜杠拼写并按平台拼写重新输出，于是 Windows 上两侧一致；在 POSIX 上还覆盖了"树以物理拼写到达、而 bound 拼写中带链接"的镜像情形。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #9742Feedback points and dispositions[rc:3838873454] Critical — containment gate compares two path spellings without canonicalising separators — FIXEDReproduced before changing anything: a probe ( That is the reported defect exactly: Fix (commit Witness and mutation probe: the new test ( The walk below the gate remains intact: [rv:5002723903] CHANGES_REQUESTED — partially reviewed, gaps disclosed — substance resolvedIts single finding (R8-1) is the Critical above — resolved. The "Not reviewed" lines are coverage disclosures (the windows-latest and macos-latest build-and-test jobs were skipped in CI and the reviewer did not run them locally), not actionable code findings; this round's fix addresses the exact risk the Windows lane covers, and the existing un-skipped clean-certification test exercises it whenever the [ic:5386319676] Sandboxed verification report (ran at the earlier head 28bad8a) — no further code action
ConflictsNone ( Verification
中文说明已处理的评审反馈 — PR #9742反馈点与处置[rc:3838873454] Critical — 包含门在比较两份路径拼写时未规范化分隔符 — 已修复改动前先复现：探针（ 这正是所报告的缺陷： 修复（提交 见证与突变探针：新测试（ 门下端的 walk 保持原样： [rv:5002723903] CHANGES_REQUESTED — 部分审查、缺口已披露 — 实质内容已解决其唯一发现（R8-1）即上方的 Critical——已解决。"Not reviewed" 各条是覆盖缺口披露（windows-latest 与 macos-latest 的 build-and-test 作业在 CI 中被跳过，审查者也未在本地运行），不是可执行的代码发现；本轮修复正对着 Windows 通道所覆盖的风险，且现有未跳过的干净认证测试会在 [ic:5386319676] 沙箱验证报告（运行于较早的 head 28bad8a）— 无需进一步代码改动
冲突无（ 验证
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/模型 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — chunk 3's final cap-round auditor failed twice; chunks 1, 2, and 4 have final dry certificates.
Not reviewed: verification — the repository stat-cache bypass candidate at worktree.ts:936 could not be independently verified after two verifier failures and is withheld as low-confidence.
Not reviewed: build-and-test — packages/cli full suite exited 1 in unchanged test files and the coverage writer raised ENOENT for coverage/.tmp/coverage-149.json; base-side rerun timed out, so the failures were not attributable.
Not reviewed: required configuration — verification ran on Node 24.18.1; the repository-required Node 22 configuration was not exercised locally.
Not reviewed: platform coverage — CI skipped the macOS and Windows Node 22 test jobs and the no-sandbox CLI integration job.
Not reviewed: test plan — the claimed '504 passed' count was unchecked because this review did not produce a comparable passing suite count.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:866 — [review] Add a deterministic post-gate substitution mutation witness for each anchored subprocess; current pre-gate forge tests do not prove every status/ls-files/check-ignore/index cal…
Convergence: round 9 posted 5 inline comment(s), 5 of them reported for the first time. Findings keep coming back to the same files: packages/cli/src/commands/review/lib/worktree.ts (findings in round 8; 3 more now). (Evidence: the previous round was recovered from a marker this account did not post, so those rounds may not be this account's own.) 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.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R8-1 still stands at reviewed commit 59f6b88b: packages/cli/src/commands/review/lib/worktree.ts:753 compares a Node-spelled tree path with Git's commonDir spelling without canonicalizing bound. On Git for Windows, forward-slash Git output mixed with Node's backslash sep makes healthy worktrees fail containment before the SHA pin runs. Canonicalize both sides through Node, including realpathSync(bound) in the physical fallback, and add a Windows-path regression witness. (中文：[严重] R8-1 在被审提交 59f6b88b 仍成立：该包含检查混用 Git 与 Node 的路径拼写；Git for Windows 的正斜杠输出与 Node 反斜杠分隔符组合后会误拒健康工作树。请统一规范化两侧并添加 Windows 路径回归见证。)
[Critical] The PR advanced after this review's single permitted restart: this review is bound to 59f6b88bf7d502a83db1c232997cd65716fe6dce, while presubmit observed live HEAD c80763e48cb367ab4ba354311a4aa79d839607ae (+1 commit touching worktree.ts and worktree.test.ts). The findings and anchors below were verified against the reviewed SHA, not the newer commit. (中文：本轮唯一允许的 restart 之后 PR 再次前进；本审查绑定 59f6b88b，presubmit 观察到 live HEAD c80763e4，新增提交触及 worktree.ts 与测试。以下发现和锚点针对被审 SHA，而非新提交。)
中文说明
未审查：reverse audit — chunk 3's final cap-round auditor failed twice; chunks 1, 2, and 4 have final dry certificates。
未审查：verification — the repository stat-cache bypass candidate at worktree.ts:936 could not be independently verified after two verifier failures and is withheld as low-confidence。
未审查：build-and-test — packages/cli full suite exited 1 in unchanged test files and the coverage writer raised ENOENT for coverage/.tmp/coverage-149.json; base-side rerun timed out, so the failures were not attributable。
未审查：required configuration — verification ran on Node 24.18.1; the repository-required Node 22 configuration was not exercised locally。
未审查：platform coverage — CI skipped the macOS and Windows Node 22 test jobs and the no-sandbox CLI integration job。
未审查：test plan — the claimed '504 passed' count was unchecked because this review did not produce a comparable passing suite count。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 9 轮发布了 5 条行内评论，其中 5 条是首次提出。发现反复回到同一批文件：packages/cli/src/commands/review/lib/worktree.ts（第 8 轮已出过发现，本轮又有 3 条）。（证据说明：上一轮的数据来自并非本账号发布的标记，上述轮次可能不属于本账号。）一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R8-1 still stands at reviewed commit 59f6b88b: packages/cli/src/commands/review/lib/worktree.ts:753 compares a Node-spelled tree path with Git's commonDir spelling without canonicalizing bound. On Git for Windows, forward-slash Git output mixed with Node's backslash sep makes healthy worktrees fail containment before the SHA pin runs. Canonicalize both sides through Node, including realpathSync(bound) in the physical fallback, and add a Windows-path regression witness. (中文：[严重] R8-1 在被审提交 59f6b88b 仍成立：该包含检查混用 Git 与 Node 的路径拼写；Git for Windows 的正斜杠输出与 Node 反斜杠分隔符组合后会误拒健康工作树。请统一规范化两侧并添加 Windows 路径回归见证。)
[Critical] The PR advanced after this review's single permitted restart: this review is bound to 59f6b88bf7d502a83db1c232997cd65716fe6dce, while presubmit observed live HEAD c80763e48cb367ab4ba354311a4aa79d839607ae (+1 commit touching worktree.ts and worktree.test.ts). The findings and anchors below were verified against the reviewed SHA, not the newer commit. (中文：本轮唯一允许的 restart 之后 PR 再次前进；本审查绑定 59f6b88b，presubmit 观察到 live HEAD c80763e4，新增提交触及 worktree.ts 与测试。以下发现和锚点针对被审 SHA，而非新提交。)
— gpt-5.4 via Qwen Code /review (v0.22.0)
| */ | ||
| function fetchedShaOf(report: PlanReport): string | undefined { | ||
| const sha = report.fetchedSha; | ||
| return typeof sha === 'string' && /^[0-9a-f]{40}$/i.test(sha) |
There was a problem hiding this comment.
[Critical] R9-3: This validator rejects valid 64-character Git SHA-256 object IDs. In a SHA-256 repository, fetch-pr records a 64-hex commit, but fetchedShaOf() returns undefined; worktree prompts then fail closed as though the trusted record were missing, and the generated scratch-tree command omits --fetched-sha. A focused probe returned false for a valid 64-hex ID and true after accepting both full Git object-ID lengths. Please use the shared full-object-ID predicate (40 or 64 hex) and add a SHA-256 plan test proving the value reaches worktreeResidue and the scratch-tree command.
中文说明
[严重] R9-3：这里会拒绝合法的 64 字符 Git SHA-256 对象 ID。在 SHA-256 仓库中，fetch-pr 会记录 64 位十六进制提交，但 fetchedShaOf() 返回 undefined；工作树 prompt 随后会像可信记录缺失一样失败关闭，生成的 scratch-tree 命令也会漏掉 --fetched-sha。定向探针显示合法 64 位 ID 当前返回 false，接受两种完整 Git 对象 ID 长度后返回 true。请使用共享的完整对象 ID 判定（40 或 64 位十六进制），并添加 SHA-256 plan 测试，证明该值会到达 worktreeResidue 和 scratch-tree 命令。
— gpt-5.4 via Qwen Code /review (v0.22.0)
| let isWorktree = false; | ||
| let anchor: string[] = []; | ||
| try { | ||
| const [toplevel, gitDir, commonDir] = (top.stdout ?? '').trim().split('\n'); |
There was a problem hiding this comment.
[Critical] R9-1: These are three arbitrary filesystem paths, so newline-delimited parsing is not valid on POSIX. A healthy linked worktree below a directory containing \n makes this split produce extra records, misassigns gitDir/commonDir, and reports the checkout as not a worktree. A real linked-worktree probe under a newline-containing directory reproduced the refusal. Please query the three values independently or consume an authoritative NUL-delimited form, removing only Git's terminal record delimiter; the fix witness should be a POSIX-only newline-path fixture that returns clean residue.
中文说明
[严重] R9-1：这里读取的是三个任意文件系统路径，因此在 POSIX 上不能使用换行符分隔解析。健康的 linked worktree 若位于名称含 \n 的目录下，这次 split 会产生额外记录，错误分配 gitDir/commonDir，并把该检出报告为非工作树。真实 linked-worktree 探针已在含换行目录下复现。请分别查询三个值，或使用权威的 NUL 分隔形式，只移除 Git 的末尾记录分隔符；修复见证应是 POSIX-only 的换行路径夹具并返回 clean residue。
— gpt-5.4 via Qwen Code /review (v0.22.0)
| // and the literal-only test refused every healthy tree on a host whose | ||
| // checkout spelling carries a link above the repository. | ||
| const spelled = resolve(cwd); | ||
| const bound = dirname(commonDir); |
There was a problem hiding this comment.
[Critical] R9-5: Using dirname(commonDir) as a mandatory containment ancestor rejects valid repositories created with --separate-git-dir. In that supported Git layout the common directory intentionally lives outside the checkout; a linked review worktree can have a valid gitfile backpointer and matching expected HEAD, yet both containment checks fail and the residue probe always returns unmeasured. A real git init --separate-git-dir fixture reproduced this false refusal. Please derive the symlink-walk boundary from the checkout/worktree layout rather than requiring the common-dir parent to contain it, or explicitly validate this Git layout; the regression fixture should return clean residue.
中文说明
[严重] R9-5：把 dirname(commonDir) 强制当作包含祖先，会拒绝使用 --separate-git-dir 创建的合法仓库。在这种受 Git 支持的布局中，common directory 本来就位于检出目录之外；linked review worktree 即使拥有有效 gitfile backpointer 和匹配的 expected HEAD，两次包含检查仍会失败，residue 探针永久返回 unmeasured。真实 git init --separate-git-dir 夹具已复现该误拒。请根据 checkout/worktree 布局推导 symlink-walk 边界，而不是要求 common-dir 父目录包含它，或显式验证并支持该 Git 布局；回归夹具应返回 clean residue。
— gpt-5.4 via Qwen Code /review (v0.22.0)
| // distinguishes this tree from a forged pair whose index already holds the | ||
| // contamination as committed content — so an empty measurement no record | ||
| // anchored is refused rather than certified. | ||
| if (paths.length === 0 && expectedHeadSha === undefined) { |
There was a problem hiding this comment.
[Critical] R9-4: The SHA-less identity refusal only runs when the measured path list is empty, so one dirty decoy bypasses it. A forged linked-worktree pair can commit the mutant, leave an unrelated dirty-decoy.txt untracked, and return only that decoy as measured residue while the committed contamination remains invisible; the probe reproduced this result without unmeasured. Please refuse whenever expectedHeadSha is absent regardless of paths.length, preserving paths and total for diagnostics. The fix witness should combine a committed mutant with one dirty decoy and require both the retained path and an unmeasured verdict.
中文说明
[严重] R9-4：无 SHA 的身份拒绝只在测得路径列表为空时执行，因此一个脏的诱饵路径即可绕过。伪造的 linked-worktree 对可以提交突变体，再留下无关的未跟踪 dirty-decoy.txt；函数只返回该诱饵为已测 residue，而已提交污染不可见，探针已复现且没有 unmeasured。请在 expectedHeadSha 缺失时无条件拒绝，不受 paths.length 影响，同时保留 paths 和 total 供诊断。修复见证应同时包含已提交突变体与一个脏诱饵，并要求保留路径且返回 unmeasured。
— gpt-5.4 via Qwen Code /review (v0.22.0)
| // fetched sha, when the caller brought it, is the probe's identity anchor: | ||
| // with it a healthy tree measures clean, and a forged pair is refused at | ||
| // the pin (see worktreeResidue). | ||
| const residue = worktreeResidue(worktree, RESIDUE_PATH_CAP, args.fetchedSha); |
There was a problem hiding this comment.
[Critical] R9-2: A fetched-SHA mismatch is only copied into sharedTreeUnmeasured; scratch-tree still proceeds with reset/creation and returns available: true. If the shared tree is at B while the plan records reviewed commit A, verifier probes can therefore run in an available scratch tree at code other than the reviewed head. A real scratch-tree probe reproduced availability despite the mismatch. Please return unavailable before any reset or creation when the fetched SHA is malformed, unreadable, or mismatched, and use the validated fetched SHA as the checkout target. The fix witness should assert available === false, no path, and no scratch worktree created.
中文说明
[严重] R9-2：fetched-SHA 不匹配只会被复制到 sharedTreeUnmeasured；scratch-tree 仍继续 reset/创建并返回 available: true。如果共享树位于 B，而 plan 记录的是被审提交 A，verifier 探针就可能在“可用”的 scratch tree 中审查不同于 reviewed head 的代码。真实 scratch-tree 探针已复现不匹配时仍可用。请在 fetched SHA 格式错误、不可读或不匹配时，于任何 reset/创建前返回 unavailable，并以已验证的 fetched SHA 作为检出目标。修复见证应断言 available === false、没有 path、且未创建 scratch worktree。
— gpt-5.4 via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally — a platform where this diff's path-separator and symlinked-tmpdir behavior fires.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally — a platform where this diff's path-separator behavior fires.
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 — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.test.ts:920 — [probe] unpinned blind-set/index-bit witness tests absorbed by the new no-record refusalpackages/cli/src/commands/review/lib/worktree.ts:882 — [probe] the --git-dir/--work-tree pin has no automated witness; dropping it ships greenpackages/cli/src/commands/review/lib/worktree.test.ts:631 — [probe] uninitialized-submodule witness fixture never contains the gitlink it claims to witnesspackages/cli/src/commands/review/lib/worktree.ts:171 (+2 locations) — [probe] both containment arms' '+ sep' guard ships green when dropped — no witness either arm
Convergence: round 9 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/review/lib/worktree.ts (findings in round 8; 4 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally — a platform where this diff's path-separator and symlinked-tmpdir behavior fires。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally — a platform where this diff's path-separator behavior fires。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 9 轮发布了 6 条行内评论，其中 6 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/lib/worktree.ts（第 8 轮已出过发现，本轮又有 4 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const spelled = resolve(cwd); | ||
| const bound = dirname(commonDir); | ||
| if (!containedUnderCheckout(spelled, bound)) { |
There was a problem hiding this comment.
[Critical] R9-1: The containment gate computes its bound as dirname(commonDir) and refuses any tree not under it — but a perfectly healthy review worktree created under a checkout that is itself a linked worktree (or a --separate-git-dir clone) is refused as "a shape no healthy review worktree takes" even though the round trip and the sha pin verify its identity completely. Re-confirms the still-standing blocker in the adjacent thread (comment 3839147505, the --separate-git-dir shape), re-probed at this head.
A review run from a sibling linked worktree — git -C main worktree add ../sib, review tree created at <sib>/.qwen/tmp/review-wt exactly as fetch-pr does — or from a --separate-git-dir checkout: --git-common-dir answers the main checkout's git dir, dirname(...) is no ancestor of the tree path, and the probe answers unmeasured on every run. fetch-pr creates the review worktree from the process cwd with no main-checkout requirement, so every round on such a host renders "Whether it is clean could not be measured" into every agent brief plus a stderr warning, attaches the NOTE to every verifier, and the contamination tripwire this PR hardens never answers there.
Witness (probe executed against this commit):
BASELINE HEAD:
sibling-linked-worktree => {"paths":[],"total":0,"unmeasured":"the repository answering for this
path does not contain it — its common dir's parent (/tmp/.../main) is no ancestor of the tree
path (/tmp/.../sib/.qwen/tmp/review-wt), a shape no healthy review worktree takes, ..."}
separate-git-dir => identical refusal
GATE DISABLED (mutant):
both layouts => {"paths":[],"total":0}
Disabling only the containment gate flips both layouts to clean — the symlink walk, backpointer round trip, sha pin, and measurement all accept these trees, so the refusal is a false positive.
Suggested fix: when the bound does not contain the tree, do not refuse on that alone — walk spelled's ancestors for symlinks bounded at the filesystem root (spelled arrives physically resolved, so a healthy spelling contains no links), and refuse only when one is found. The gate cannot simply be deleted (the redirect-forge tests rely on it), and re-bounding at the main worktree root does not fix the sibling-linked-worktree case — for a standard repo the main worktree root IS dirname(commonDir).
中文说明
[严重] R9-1：包含门把边界计算为 dirname(commonDir) 并拒绝任何不在其下的树——但在"检出自本身就是 linked worktree"（或 --separate-git-dir 克隆）之下创建的完全健康的审查工作树，会被判为"健康审查工作树不会呈现的形态"而遭拒绝，尽管往返校验与 sha 固定已完整验证其身份。本条在该 head 上重新确认相邻线程（评论 3839147505，--separate-git-dir 形态）中仍未解决的阻断，并重新执行了探针。
从兄弟 linked worktree（git -C main worktree add ../sib，审查树按 fetch-pr 的方式创建于 <sib>/.qwen/tmp/review-wt）或 --separate-git-dir 检出运行审查时：--git-common-dir 回答主检出的 git 目录，dirname(...) 不是树路径的祖先，探针每轮都回答 unmeasured。fetch-pr 从进程 cwd 创建审查工作树、没有主检出要求，因此这类宿主上每一轮都会把 "Whether it is clean could not be measured" 写入每一份 agent 简报并附 stderr 警告、给每个验证器附上 NOTE，本 PR 所加固的污染绊线在该布局下永远无法作答。
见证（对本提交执行的探针）：仅禁用包含门即把两种布局翻转为干净——符号链接行走、backpointer 往返、sha 固定与测量都接受这些树，拒绝是误报（输出见英文部分）。
修复建议：边界不包含树时不要仅凭此拒绝——以文件系统根为界行走 spelled 的祖先查找符号链接（spelled 以物理解析形式到达，健康拼写不含链接），仅在发现链接时拒绝。注意：该门不能直接删除（重定向伪造测试依赖它），把边界改为主工作树根也无法修复兄弟 linked worktree 场景——标准仓库的主工作树根就是 dirname(commonDir)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (spelled.startsWith(bound + sep)) return true; | ||
| try { | ||
| return realpathSync(spelled).startsWith(realpathSync(bound) + sep); |
There was a problem hiding this comment.
[Critical] R9-2: containedUnderCheckout builds its containment prefix as bound + sep, which at the filesystem/drive root produces // (POSIX) or C:\\ (Windows) — a prefix no real path starts with. So when the bound IS the root (the git common dir sits directly under /), the gate refuses every tree even though the root contains all of them.
Trigger: any repository whose git common dir is a single path component under the filesystem root — a bare repo at /R.git (rootful CI container, mirror layout) or a --separate-git-dir=/repo.git clone. From the review worktree, git rev-parse --git-common-dir answers the bare dir itself (no .git suffix — verified against real git), so the caller's bound = dirname('/R.git') is /; both arms then test startsWith('//'), which no real path satisfies, and the gate refuses with "its common dir's parent (/) is no ancestor of the tree path" — a false reason and a false refusal. "Whether it is clean could not be measured" is rendered into every agent brief and every verifier NOTE for the life of that layout, and the contamination tripwire never answers for it. Distinct from R9-1 (where the bound is not an ancestor at all) and survives R9-1's fix options (a bare repo has no main worktree root to re-bound at).
Witness (executed against this commit):
PR: containedUnderCheckout('/tmp/.../some/tree', '/') = false
sanity arm: containedUnderCheckout(tree, '/tmp/...') = true
root-safe-prefix mutant: containedUnderCheckout(tree, '/') = true; full suite still 63/63
(The sandbox cannot write /, so a literal root-level bare repo could not be instantiated end-to-end; the claim was settled at the function level plus real git's answer shape.)
Suggested fix: make the prefix root-safe in both arms, e.g. const prefix = (b: string) => (b.endsWith(sep) ? b : b + sep); used in both startsWith tests; add a witness in 'canonicalises the bound': expect(containedUnderCheckout(wt, parse(wt).root)).toBe(true) (parse(...).root for portability to Windows drive roots).
中文说明
[严重] R9-2：containedUnderCheckout 以 bound + sep 构造包含前缀，在文件系统/盘符根处会得到 //（POSIX）或 C:\\（Windows）——没有任何真实路径以该前缀开头。当边界就是根（git common dir 直接位于 / 之下）时，该门会拒绝每一棵树，尽管根包含所有路径。
触发：git common dir 是文件系统根下单个路径分量的仓库——位于 /R.git 的 bare 仓库（rootful CI 容器、镜像布局）或 --separate-git-dir=/repo.git 克隆。从审查工作树执行 git rev-parse --git-common-dir 会回答 bare 目录本身（无 .git 后缀——已用真实 git 验证），于是 bound = dirname('/R.git') 为 /；两个分支都测试 startsWith('//')，没有真实路径满足，门以 "its common dir's parent (/) is no ancestor of the tree path" 拒绝——错误的理由、错误的拒绝。该布局存续期间，"Whether it is clean could not be measured" 会写入每一份 agent 简报与每个验证器 NOTE，污染绊线永远无法作答。与 R9-1 不同（那里边界根本不是祖先），且不受 R9-1 修复方案覆盖（bare 仓库没有可供重定界的主工作树根）。
见证（对本提交执行）：containedUnderCheckout(tree, '/') 在 PR 代码上为 false、在根安全前缀突变体上为 true，套件保持 63/63（输出见英文部分）。沙箱无法写入 /，故以函数级执行 + 真实 git 的回答形态定案。
修复建议：两个分支都使用根安全前缀，例如 const prefix = (b: string) => (b.endsWith(sep) ? b : b + sep);；并在 'canonicalises the bound' 中加入见证：expect(containedUnderCheckout(wt, parse(wt).root)).toBe(true)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| */ | ||
| function fetchedShaOf(report: PlanReport): string | undefined { | ||
| const sha = report.fetchedSha; | ||
| return typeof sha === 'string' && /^[0-9a-f]{40}$/i.test(sha) |
There was a problem hiding this comment.
[Critical] R9-3: This validator accepts only exact 40-hex shas, rejecting valid 64-character Git SHA-256 object IDs, while the pipeline's own sha-shape contract admits them — COMMIT_SHA_RE = /^[0-9a-f]{40,64}$/ in pr-context.ts (whose test comment says the breadth exists for SHA-256 heads) and (?:[0-9a-f]{40}|[0-9a-f]{64}) in repo-context.ts. fetch-pr records git rev-parse verbatim and the pin comparison is length-agnostic; only this regex drops the 64-hex shape. Re-confirms the still-standing blocker in comment 3839147502, re-verified at this head with an end-to-end SHA-256 fixture.
In a SHA-256 repository, fetch-pr records a 64-hex commit but this function returns undefined, so every worktree-mode round (a) fails closed as though the trusted record were missing — accusing the healthy plan of tampering/corruption on every run — and (b) renders the scratch-tree command without --fetched-sha, so every verifier's shared-tree probe measures unpinned and refuses any clean verdict with 'brought no record'. A permanently firing false-tamper note and total loss of the pin hardening for that repo class. GitHub does not serve SHA-256 repositories today, but the pipeline's contract admits the shape and the failure is deterministic given such a plan.
Witness (end-to-end A/B on git 2.43, through the real agentPromptCommand.handler / buildRoleBrief):
BASE (SHA-1 head): verifyBriefHasFetchedShaFlag=true, no warning
PR (SHA-256 head): readerBriefHasUnmeasuredNote=true, verifyBriefHasFetchedShaFlag=false,
stderr "...absence means tampering or corruption..."
direct worktreeResidue(wt, 12, sha64) = {"paths":[],"total":0} <- only the regex drops the shape
Suggested fix: use the shared full-object-ID predicate — /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i — and add a SHA-256 plan test proving the value reaches worktreeResidue and the scratch-tree command.
中文说明
[严重] R9-3：该验证器只接受恰好 40 位的十六进制 sha，拒绝了合法的 64 字符 Git SHA-256 对象 ID，而流水线自身的 sha 形态契约接受该形态（pr-context.ts 的 COMMIT_SHA_RE = /^[0-9a-f]{40,64}$/——其测试注释明确该宽度正是为 SHA-256 head 而设——以及 repo-context.ts 的 (?:[0-9a-f]{40}|[0-9a-f]{64})）。fetch-pr 原样记录 git rev-parse 输出，固定比较与长度无关；只有这个正则丢弃 64 位形态。本条在该 head 上重新确认评论 3839147502 中仍未解决的阻断，并以端到端 SHA-256 夹具重新验证。
在 SHA-256 仓库中，fetch-pr 记录 64 位提交而本函数返回 undefined：每一轮 worktree 模式审查（a）像可信记录缺失一样失败关闭——每轮都指控健康计划被篡改/损坏；（b）生成的 scratch-tree 命令漏掉 --fetched-sha，每个验证器的共享树探针无固定地测量、并以 'brought no record' 拒绝任何干净裁决。永久误报的篡改提示 + 该类仓库完全失去固定加固。GitHub 目前不提供 SHA-256 仓库，但流水线契约接受该形态，且给定这样的 plan 时失败是确定性的。
见证（git 2.43 上经真实 agentPromptCommand.handler / buildRoleBrief 的端到端 A/B，输出见英文部分）：BASE（SHA-1 head）携带 --fetched-sha 且无警告；PR（SHA-256 head）丢失该旗标并触发篡改警告，而直接以 64 位 sha 调用 worktreeResidue 测得干净——只有正则丢弃该形态。
修复建议：使用共享的完整对象 ID 判定——/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i——并添加 SHA-256 plan 测试，证明该值到达 worktreeResidue 与 scratch-tree 命令。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| let isWorktree = false; | ||
| let anchor: string[] = []; | ||
| try { | ||
| const [toplevel, gitDir, commonDir] = (top.stdout ?? '').trim().split('\n'); |
There was a problem hiding this comment.
[Critical] R9-4: This parses three arbitrary filesystem paths (--show-toplevel, --git-dir, --git-common-dir) out of one newline-delimited stdout — and newline-delimited parsing is not valid for paths on POSIX. A healthy linked worktree below a directory whose name contains \n makes the split produce extra records, misassigns gitDir/commonDir, and reports a genuine checkout as not a worktree. Re-confirms the still-standing blocker in comment 3839147503, re-probed at this head.
Witness (probe executed against the built module at this commit):
worktreeResidue(wt, 12, <own HEAD>) =
{"paths":[],"total":0,"unmeasured":"the path is not a git worktree (repository discovery walks up into the enclosing checkout)"}
discovery spawn exits 0; its stdout is three paths / five newline-split lines:
/tmp/.../dir\nwith-newline/repo/.qwen/tmp/review-wt
/tmp/.../dir\nwith-newline/repo/.git/worktrees/review-wt
/tmp/.../dir\nwith-newline/repo/.git
The tree IS a genuine git worktree add worktree; the refusal is a misparse introduced by this diff's multi-option discovery spawn.
Suggested fix: query the three values independently (one rev-parse per value), or consume an authoritative NUL-delimited form, removing only Git's terminal record delimiter; the fix witness should be a POSIX-only newline-path fixture that returns clean residue.
中文说明
[严重] R9-4：这里从一个换行分隔的 stdout 中解析三个任意文件系统路径（--show-toplevel、--git-dir、--git-common-dir），而对路径而言换行分隔解析在 POSIX 上不成立。健康 linked worktree 若位于名称含 \n 的目录下，split 会产生额外记录、错误分配 gitDir/commonDir，并把真实检出报告为非工作树。本条在该 head 上重新确认评论 3839147503 中仍未解决的阻断，并重新执行了探针。
见证（对本提交构建产物执行的探针）：真实 git worktree add 工作树回答 unmeasured: "the path is not a git worktree..."；发现命令以 0 退出，其 stdout 是三个路径、按换行切分却得到五条记录（输出见英文部分）。误解析由本 diff 的多选项发现 spawn 引入。
修复建议：分别查询三个值（每个值一次 rev-parse），或使用权威的 NUL 分隔形式、只移除 Git 的末尾记录分隔符；修复见证应为 POSIX-only 的换行路径夹具并返回干净 residue。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // distinguishes this tree from a forged pair whose index already holds the | ||
| // contamination as committed content — so an empty measurement no record | ||
| // anchored is refused rather than certified. | ||
| if (paths.length === 0 && expectedHeadSha === undefined) { |
There was a problem hiding this comment.
[Critical] R9-5: The sha-less identity refusal only runs when the measured path list is empty, so one dirty decoy bypasses it. A forged linked-worktree pair can commit the contamination and leave an unrelated untracked decoy: the probe returns only the decoy as measured residue with no unmeasured verdict, and the committed contamination stays invisible. Re-confirms the still-standing blocker in comment 3839147508, re-probed at this head.
Witness (executed against the built module at this commit) — a worktree with the contamination committed and an untracked dirty-decoy.txt:
worktreeResidue(wt, 12) = {"paths":["dirty-decoy.txt"],"total":1} <- no unmeasured
A sha-less caller — the shape every tampered/record-less plan produces, since agent-prompt fails closed without a sha and scratch-tree's --fetched-sha is only welded in when the plan carries one — sees an ordinary dirty reading here and never learns the identity was unanchored; the committed mutant is by construction absent from any residue list.
Suggested fix: refuse whenever expectedHeadSha is absent regardless of paths.length, preserving paths and total for diagnostics; the fix witness should combine a committed mutant with one dirty decoy and require both the retained path and an unmeasured verdict.
中文说明
[严重] R9-5：无 SHA 的身份拒绝只在测得路径列表为空时执行，因此一个脏诱饵即可绕过。伪造的 linked-worktree 对可以把污染提交进索引、再留下一个无关的未跟踪诱饵：探针只把诱饵作为已测 residue 返回、没有 unmeasured 裁决，已提交的污染保持不可见。本条在该 head 上重新确认评论 3839147508 中仍未解决的阻断，并重新执行了探针。
见证（对本提交构建产物执行）：污染已提交 + 未跟踪 dirty-decoy.txt 的工作树，worktreeResidue(wt, 12) = {"paths":["dirty-decoy.txt"],"total":1}——没有 unmeasured。无 SHA 调用方（计划缺失/被篡改时的形态：agent-prompt 在无 sha 时失败关闭，scratch-tree 的 --fetched-sha 仅在计划携带时才被焊接进命令）会在此看到普通脏读数、无从得知身份未锚定；已提交的突变体按构造不会出现在任何 residue 列表中。
修复建议：expectedHeadSha 缺失时无条件拒绝，不受 paths.length 影响，同时保留 paths 与 total 供诊断；修复见证应同时包含已提交突变体与一个脏诱饵，并要求保留路径且返回 unmeasured。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // fetched sha, when the caller brought it, is the probe's identity anchor: | ||
| // with it a healthy tree measures clean, and a forged pair is refused at | ||
| // the pin (see worktreeResidue). | ||
| const residue = worktreeResidue(worktree, RESIDUE_PATH_CAP, args.fetchedSha); |
There was a problem hiding this comment.
[Critical] R9-6: A fetched-SHA mismatch — indeed any pin refusal — is only copied into sharedTreeUnmeasured and the residue NOTE; scratch-tree still proceeds with reset/creation and returns available: true. If the shared tree is at B while the plan records reviewed commit A, verifier probes can run in an available scratch tree at code other than the reviewed head, with the mismatch disclosed only inside a NOTE string. Re-confirms the still-standing blocker in comment 3839147512, mechanism traced at this head.
Witness: not run — the mechanism is fully traced in the code at the reviewed head: between this call and the two available: true returns (the reuse path and the fresh path) there is no branch that inspects residue.unmeasured or the fetched sha; the reset/creation paths run unconditionally on the tree's own headSha. The original thread's probe reports reproducing availability despite the mismatch.
Suggested fix: return unavailable before any reset or creation when the fetched SHA is malformed, unreadable, or mismatched, and use the validated fetched SHA as the checkout target; the fix witness should assert available === false, no path, and no scratch worktree created.
中文说明
[严重] R9-6：fetched-SHA 不匹配（以及任何固定拒绝）只会被复制到 sharedTreeUnmeasured 与 residue NOTE——scratch-tree 仍继续 reset/创建并返回 available: true。若共享树位于 B 而 plan 记录被审提交 A，验证器探针会在"可用"的 scratch tree 中运行于非被审 head 的代码，而不匹配仅以 NOTE 字符串披露。本条在该 head 上重新确认评论 3839147512 中仍未解决的阻断。
见证：未运行——机制已在被审 head 的代码中完整追踪：从本调用到两处 available: true 返回（复用路径与新建路径）之间，没有任何分支检查 residue.unmeasured 或 fetched sha；reset/创建路径无条件地以该树自身的 headSha 执行。原线程的探针报告已复现不匹配时仍可用。
修复建议：在 fetched SHA 格式错误、不可读或不匹配时，于任何 reset/创建前返回 unavailable，并以已验证的 fetched SHA 作为检出目标；修复见证应断言 available === false、没有 path、且未创建 scratch worktree。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #9742 (round 10)All eleven inline findings (six distinct defects, each confirmed by both reviewers) are resolved in commit R9-3 — the validator rejects valid 64-hex SHA-256 object IDs (rc:3839147502, rc:3839280077)Fixed. R9-1/R9-4 — newline-delimited parsing of three filesystem paths (rc:3839147503, rc:3839280079)Fixed. Discovery now asks one R9-5/R9-1 — the containment gate rejects healthy layouts (rc:3839147505, rc:3839280074)Fixed at the root cause. The gate computed On the reviewers' note that the gate cannot simply be deleted because the redirect-forge tests rely on it: verified, and the reliance is the reason string only. With the refusal gone, the forge-territory fixture is still refused — by the walk itself ( R9-4/R9-5 — the SHA-less refusal only ran on an empty measurement (rc:3839147508, rc:3839280082)Fixed. R9-2/R9-6 — scratch-tree proceeds despite a fetched-SHA mismatch (rc:3839147512, rc:3839280083)Fixed. R9-2 — the root-bound prefix bug (rc:3839280076)Fixed by the removal above. The R8-1 (review body, rv:5002969413)Already addressed by the unreviewed commit the review names ( Deferred under the convergence postureThe round-9 deferred item (post-gate substitution mutation witnesses for the anchored subprocesses, worktree.ts:866) was recorded as not requested this round and is untouched. VerificationCommands actually run (this round, after all fixes):
Mutation probes (guard removed ⇒ witness must fail, then restored ⇒ green):
中文说明已处理的审查反馈 — PR #9742（第 10 轮）全部 11 条行内发现（6 个不同缺陷，均由两位审查者各自确认）已在提交 R9-3 — 校验器拒绝合法的 64 位 SHA-256 对象 ID（rc:3839147502, rc:3839280077）已修复。 R9-1/R9-4 — 对三个文件系统路径的换行分隔解析（rc:3839147503, rc:3839280079）已修复。 发现过程现在对每个值单独执行一次 R9-5/R9-1 — 包含门拒绝健康布局（rc:3839147505, rc:3839280074）已从根因修复。 该门计算 关于审查者所说"该门不能简单删除，因为 redirect-forge 测试依赖它"：已核实，这种依赖仅是理由字符串层面的。拒绝移除后，forge-territory 夹具依然被拒——由行走本身拒绝（ R9-4/R9-5 — 无 SHA 拒绝只在测量为空时执行（rc:3839147508, rc:3839280082）已修复。 R9-2/R9-6 — scratch-tree 在 fetched-SHA 不匹配时仍继续（rc:3839147512, rc:3839280083）已修复。 R9-2 — 根边界前缀缺陷（rc:3839280076）由上述移除一并修复。 在 POSIX 根产生 R8-1（审查正文，rv:5002969413）已由审查点名的未审查提交处理（ 收敛姿态下延后的项目第 9 轮延后项（锚定子进程的 post-gate 替换突变见证，worktree.ts:866）已记录为本轮不要求，未做改动。 验证实际执行的命令（本轮，所有修复之后）：
突变探针（移除守卫 ⇒ 见证必须失败，随后恢复 ⇒ 绿色）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally — this diff's symlinked-tmpdir and path behavior fires on that platform.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally — this diff's path-separator behavior fires on that platform.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:856 — [review] the --git-dir/--work-tree pin has no automated witness; dropping it ships greenpackages/cli/src/commands/review/lib/worktree.ts:771 — [review] walk stop bound steerable via realpath equality — added comment overstates the invariantpackages/cli/src/commands/review/agent-prompt.ts:1364 — [review] no-record early return drops the measured residue paths both renderers would namepackages/cli/src/commands/review/lib/worktree.test.ts:848 — [review] unpinned blind-set/index-bit witness tests absorbed by the new no-record refusalpackages/cli/src/commands/review/lib/worktree.ts:874 — [review] sha pin admits a dangling raw sha in HEAD; peel HEAD^{commit} to close the holepackages/cli/src/commands/review/scratch-tree.ts:517 — [review] note ternary swallows measured residue paths whenever unmeasured is also set
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally — this diff's symlinked-tmpdir and path behavior fires on that platform。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally — this diff's path-separator behavior fires on that platform。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix — no action required (round on PR #9742)No code change this round. The only new feedback is the automated reviewer's round-10 "partially reviewed — gaps disclosed" note. It posts zero new findings, and its six prior findings are explicitly deferred under the round-10 convergence posture ("recorded, not requested in this round"). Nothing in this round is actionable, and nothing is Critical. What was classifiedSkipped-CI disclosures (not findings). The reviewer flags three suites it could not cover: Six deferred findings — re-verified, kept deferred. I did not defer these on authority alone; I re-checked the load-bearing one against the code and a live probe:
None of these alters a clean/dirty verdict or opens a certification path, so under round-10 convergence (land Critical fixes only after ~5 rounds, defer the rest with a visible record) they stay deferred rather than growing the diff. Checks. No failed checks and no still-red checks; ubuntu unit tests passed, the rest were skipped. No base-conflict resolution was requested ( No commit was made and the working tree is unchanged. 中文说明Autofix — 无需处理（PR #9742 本轮）本轮不做任何代码改动。唯一的新反馈是自动审查器第 10 轮的“部分审查——已披露缺口”说明。它没有提出任何新发现，其先前的 6 条发现也已按第 10 轮收敛姿态被显式延后（“已记录，本轮不要求处理”）。本轮没有任何可执行项，也没有任何 Critical 级问题。 分类情况被跳过的 CI 披露（并非发现）。 审查器标注了它无法覆盖的三个套件： 6 条被延后的发现——已复核，维持延后。 我并非仅凭权威就延后它们；我对其中承重的一条结合代码和实际探测做了复核：
以上均不改变干净/脏的判定，也不打开任何认证通路，因此按第 10 轮收敛姿态（约 5 轮之后仅落地 Critical 修复、其余以可见记录延后）维持延后，以免扩大 diff。 检查。 没有失败的检查，也没有持续变红的检查；ubuntu 单元测试通过，其余被跳过。未要求解决与 base 的冲突（ 未进行任何提交，工作区保持不变。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 90 passed · 1 failed · 91 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：90 通过 · 1 失败 · 91 总计 抖动门：✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9742 — deep verification (follow-up round 2)Verdict: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：90 通过 · 1 失败 · 91 总计 中文摘要
Previous-finding status at the new headThe prior round verified
Prior-round corrections also stand, re-verified — see Corrections. Central claim and A/BCentral claim: the residue probe fails closed on forged/redirected git identity (and pins the verified identity across every spawn), while healthy trees — including the shapes the old containment gate refused — still measure: clean with the fetched-sha anchor, named-but-unverdicted without it. Harness: Table 1 — A/B cells (witnesses:
Two harness repairs vs the first run are recorded for honesty (both mine, not the PR's): the no-record tail keeps dirty paths alongside Pin equivalence ( Suite gate / Reviewer Test Plan: the plan's exact command Drift probe ( Corrections to the description
FindingsF4 — walk-to-root refuses a healthy sibling-bound layout spelled through a link, where the identical spelling is admitted with an ancestor bound (low, fail-closed) — NEWMeasured cell C11: a review worktree under a checkout that is itself a linked worktree (the layout the new test "measures a review worktree under a checkout that is itself a linked worktree" admits) is refused as Mechanism: where Two things for the reviewer: (a) the asymmetry — C9 admits, C11 refuses the same spelling — follows from where the stop test can fire, and is worth a sentence in the commit's tradeoff list; (b) the refusal message ("every identity check resolves through it … the commands below would measure wherever it points") overstates the risk for the pinned commands, which anchor realpath'd identities after the gate; the walk runs before the anchor exists, so the text describes the unpinned counterfactual. Blast radius: every worktree-mode consumer on hosts where the checkout's common dir is a sibling (linked-worktree checkouts, F2 census at the new head (carried, re-measured) — 18 guards pinned, one redundant defenceMutation matrix ( The one expected survivor, M7b — folding the PIN's read of git's own HEAD output ( A second, deliberately-probed survivor: M16 — delete the anchor assignment entirely ( One harness correction, recorded: my first M7 mutant deleted the wrong side (the record-side fold that the uppercase test exercises), survived, and was replaced by the two finer rows above — "a surviving mutation needs a finer mutation before it becomes a finding". The positive control is the matrix itself: 18 rows in the same three files all turn tests red, so the suites are live and collected. F3 — wrong sha on a dirty tree answers unmeasured, not residue (informational) — STANDSRe-measured cell C8: the sha gate runs before measurement; a wrong-sha call on a dirty tree returns Not covered
MethodologyEnvironment: the lane's own Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action taken (PR #9742)Triage summaryThis round's prepared feedback contains no actionable items:
Diff growth this window remains within budget (source 48/400 lines, test 320/400 lines; 0 prior rounds over budget), and no growth audit was required. OutcomeNo code changes and no commits this round. The branch head is unchanged at 中文说明Autofix 审查轮次:未采取行动(PR #9742)分诊摘要本轮准备好的反馈中没有任何可处理的项目:
本窗口的 diff 增长仍在预算之内(源码 48/400 行,测试 320/400 行;此前没有超预算的轮次),也不需要进行增长审计。 结果本轮没有代码变更,也没有提交。分支头部保持不变,仍为 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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.
LGTM, looks ready to ship. ✅ — two non-blocking reservations are named in the Stage 3 comment above (the Closes #9557 scope, and the in-flight verify verdict).
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
I found one merge-readiness issue; I did not find a new code correctness or security defect in the exact head 6bdbf17a6281196c63e41bba6fd955e660addaee.
The PR description no longer describes the change being reviewed:
- “Same single commit … nothing else” is now 10 commits across 6 files (
+1221/-57). - “Confined to
worktree.ts” is no longer true: production behavior also changesagent-prompt.tsandscratch-tree.ts. - The Reviewer Test Plan / Evidence still describes the original proof and says the pin is not covered by a test, while the current head adds substantial regression coverage for the SHA delivery, identity pin, supported Git layouts, and scratch-tree refusal paths.
- The existing thread about
Closes #9557also needs a final disposition becausebase-tree.tsremains outside this diff.
Please refresh the English and Chinese What/Why, Reviewer Test Plan, Evidence, Risk & Scope, and Linked Issues sections so they match the current head. I am withholding approval until the review record accurately states what will merge.
Verification performed locally on Node 22.22.3:
- CLI build and typecheck passed.
- ESLint, Prettier check, and
git diff --checkpassed for the changed files. agent-prompt.test.ts: 291 passed.worktree.test.ts: 65 passed, 1 skipped.scratch-tree.test.ts: 46 passed.
The current Ubuntu CI suite is green; the Windows, macOS, and CLI no-sandbox integration jobs are skipped by this PR workflow. The latest sandbox verification job is still in progress at the time of this review.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Review feedback round — PR #9742 — no code changes this roundHead: Feedback and disposition[rv:5003818778] (@doudouOUC): one merge-readiness issue — the PR description no longer describes the change being reviewed (the body still says a single commit confined to Disposition — no code change; the refreshed description and the Linked-Issues disposition are delivered below. Nothing in the feedback claims a code defect, and re-checking the head against it found nothing to fix in the tree. Both requested changes are GitHub-side PR metadata that this loop cannot write: the address-review path has no PR-body-edit mechanism and the agent holds no GitHub credentials (the same limitation named in the round-1 reply on R1-6). Rather than invent a code change, this round delivers the exact text and the disposition for a maintainer to apply:
Ready-to-apply PR descriptionTitle (unchanged): Body: What this PR doesThe review pipeline's residue probe measures whether a review worktree is clean — which paths the tree carries that its HEAD commit does not — by running git against the tree. Before this change every one of those spawns re-discovered the repository through the tree's Three further shapes fail closed on the same principle. A path reached through a symlink — at the leaf or at any ancestor — is refused, because a link redirects every check into territory holding a completely genuine worktree pair with the contamination committed. The ancestor walk that finds one stops at the repository the common dir belongs to when that contains the tree; where it does not — a review worktree under a checkout that is itself a linked worktree, a The record is welded through the two callers. Aggregate at the current head: 10 commits across 6 files (+1221/−57) — three production files ( Why it's neededIssue #9557 (P2 security) documents the check-then-use window with a repro: identity captured, gitfile swapped, and the discovery path certifies the mutant clean while the pinned path still names it. A contaminated review worktree answering "clean" feeds contaminated code to every review wave built on top of that measurement. The fix is the prescription in #9557 itself: resolve the identity once, pin it onto every later spawn, fail closed on any gap. Reviewer Test PlanHow to verify
Evidence (Before & After)Non-UI change. The deep-verification A/B at this head, against real git fixtures in an isolated container: the base build certifies the forged-admin-entry, swapped-gitfile, leaf-symlink, and intermediate-ancestor-symlink shapes CLEAN (the bug); the head build refuses each with a distinguishing reason ( Tested on
Environment (optional)Unit/integration vitest runs only. Linux evidence: this round's run (Node v22.23.2), the maintainer's local verification (Node 22.22.3), and the Ubuntu CI suite at this head are all green. The Windows/macOS unit jobs and the CLI no-sandbox integration job are gated on Risk & Scope
Linked IssuesPart of #9557 — deliberately not 中文说明（以上 PR 正文的中文版本）本 PR 做了什么审查流水线的残差探针通过向工作树运行 git 来测量审查 worktree 是否干净——即树上有哪些路径是 HEAD 提交所没有的。改动之前，探针的每一次 git 调用都经由树的 另有三种形态按同一原则失败即关闭。经符号链接到达的路径——无论叶子还是任一祖先——一律拒绝，因为链接会把所有检查改道到一块持有完整真实 worktree 对、且污染已被提交进去的领地。查找链接的祖先行走，当 common dir 所属仓库包含该树时停在该仓库；当不包含时——主 checkout 本身是 linked worktree 的审查树、 记录经由两个调用方焊接进来。 当前 head 的合计：10 个提交、6 个文件（+1221/−57）——3 个生产文件（ 为什么需要Issue #9557（P2 安全）记录了 check-then-use 窗口并附复现：身份被捕获、gitfile 被掉包后，发现路径把突变体认证为干净，而固定路径仍能点名。一个被污染的审查 worktree 若回答"干净"，会把被污染的代码喂给建立在该测量之上的每一波评审。修法正是 #9557 开出的方子：解析一次身份、固定到其后每个 spawn、任何缺口失败即关闭。 评审者测试计划如何验证
证据（前后对比）非 UI 改动。本 head 上的深度验证 A/B，在隔离容器内对真实 git 夹具执行：base 构建把伪造管理条目、掉包 gitfile、叶子符号链接、中间祖先符号链接四种形态认证为 CLEAN（即漏洞本身）；head 构建以可区分的理由逐一拒判（ 测试环境
环境（可选）仅单元/集成 vitest 运行。Linux 证据：本轮运行（Node v22.23.2）、维护者本地验证（Node 22.22.3）与本 head 上的 Ubuntu CI 套件全绿。Windows/macOS 单元任务与 CLI 无沙箱集成任务在 风险与范围
关联 IssuePart of #9557——有意不使用 VerificationCommands run this round (Linux, Node v22.23.2, head
中文说明评审反馈轮 — PR #9742 — 本轮无代码改动Head： 反馈与处置[rv:5003818778]（@doudouOUC）：一项合并就绪问题——PR 描述不再描述正在评审的改动（正文仍写"单一提交、限于 worktree.ts"，而 head 已有 10 个提交、6 个文件、+1221/−57，生产行为还改动了 处置——无代码改动；下面直接交付刷新后的描述与 Linked Issues 的处置。 反馈未主张任何代码缺陷，对照 head 复核也未发现树内有任何可修之处。两项请求都是 GitHub 侧的 PR 元数据，本循环无法写入：address-review 路径没有编辑 PR 正文的机制，agent 也不持有任何 GitHub 凭据（与第 1 轮对 R1-6 的回复所述同一限制）。因此本轮不制造代码改动，而是交付可供维护者直接应用的准确文本与处置：
验证本轮实际运行的命令（Linux，Node v22.23.2，head
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Verified every R9 Critical against head 6bdbf17 — the unresolved threads were probed at earlier heads; the final commit lands the fixes:
- Newline-delimited parsing is gone:
--show-toplevel/--git-dir/--git-common-dirare now three independentdiscover()invocations, each stripping only git's terminal newline — a POSIX name carrying\ncan no longer split into extra records (R9-1). - scratch-tree returns unavailable BEFORE any reset/creation when the fetched SHA is malformed (not 40/64 hex) or mismatches the shared tree's HEAD, case-folded on both sides; the content-filter refusal on top closes the config-driven-filter execution vector at checkout (R9-2).
- The fetched-SHA validator accepts both full Git object-ID lengths,
{40,64}hex, so SHA-256 repositories no longer fail closed on a valid 64-char record (R9-3). - The sha-less identity refusal is now unconditional (
expectedHeadSha === undefined→ unmeasured regardless ofpaths.length), preserving paths/total for diagnostics — the committed-mutant-plus-dirty-decoy shape can no longer certify clean (R9-4). dirname(commonDir)is no longer a mandatory containment ancestor: when the bound is not an ancestor of the tree path the symlink walk continues to the filesystem root instead of refusing, and--separate-git-dir/ linked-checkout / links-above-root layouts are held by the round-trip plus the sha pin — all three pinned by dedicated fixtures (R9-5).
The code is explicit about the residual window it does NOT close (a same-user writer swapping the pinned admin entry's contents inside the pin-to-measure window raises the attack's cost but needs a gate-time snapshot or a sandbox boundary, tracked as #9556) — disclosed, not hidden.
CI at approval time: 15 checks passing, none failing. The R9 threads still show unresolved because no automated round has re-probed this head.
|
Follow-up after reviewing the updated sandbox report and AutoFix response:
A generated comment/artifact does not update the merge record shown at the top of the PR. Please apply that replacement body (or an equivalent accurate version) to the PR itself and use |
|
Released in v0.22.2. |
5 participants
What this PR does
Replaces #9572 at its original size. Same single commit, rebased onto current
main, nothing else.worktreeResidue— the tripwire every wave of review agents is built from — verified a tree's identity once and then ran five commands that each re-discovered the repository fromcwd, through the same.gitfile the check had read. That file is writable by anything running as this user, so the gate and the measurement were a check-then-use pair.Two halves, closing different windows:
core.worktreenames this path answers--show-toplevelwith this path — so the old gate saw itself, while every command after it measured the plant's index, which already holds the contamination as committed content. A planted standalone repo has no admin entry to round-trip.scratch-treegates its own reset on exactly this check; the reading side did not have it.--git-dir/--work-treeare pinned onto every later spawn, so a swap landing after the gate cannot redirect them either.Why a new PR
#9572 was this commit plus four more that grew it from 2 files / +204 to 8 files / +2 602, reaching into
agent-prompt.ts,fetch-pr.tsandscratch-tree.ts. Opening a replacement rather than force-pushing keeps that history intact for anyone who wants to read what was tried.Why it is worth doing
The recorded reason for not doing this (#9221, round 17) was that an explicit pin changes what
ls-filesresolves, so it needs its own round of measurement. The "own round" part was right. The rest was wrong, and this is the measurement:across all five commands the probe runs (
status --porcelain -uall -z,ls-files --others -z,ls-files -s -z,ls-files -v -z,check-ignore -z -v --stdin).And it does something. With the identity captured, then the gitfile swapped at a repository whose index already holds the mutant:
Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review→ 4 504 passed, 1 skipped, 0 failed.New:
worktreeResiduereportsunmeasurednaming "does not point back" for a tree whose.gitwas swapped at a repo that answers for this path. The fixture asserts the genuine reading first, so it cannot pass by being unmeasurable for some other reason.Evidence (Before & After)
Tested on
macOS 26.6 (Darwin 25.6.0), git 2.51, Node 24,
packages/clivitest 3.2.4.Risk & Scope
Confined to
packages/cli/src/commands/review/lib/worktree.ts. One new refusal path (a gitfile whose admin entry does not round-trip isunmeasured, notclean), and an argument prefix on five commands that was measured to change nothing.The shape it refuses is one a healthy pipeline never produces:
fetch-prbuilds review worktrees withgit worktree add, which always writes the round-tripping admin entry.Linked Issues
Closes #9557. Replaces #9572. Follow-up from #9221; the class it belongs to is #9556.
中文说明
这个 PR 做了什么
以原始尺寸替代 #9572。同一个提交，重新基于当前
main，没有别的东西。worktreeResidue——每一波审查 agent 构建时都会用到的那条绊线——只校验一次树的身份，随后五条命令各自从cwd重新发现仓库，经由校验时读过的那同一个.git文件。该文件对任何以此用户身份运行的东西都可写，因此"门"与"测量"构成了一对 check-then-use。两半，各自关掉不同的窗口：门现在要求 gitfile 所指的 admin 条目反向指回本树（把
core.worktree指向本路径的仓库会让--show-toplevel回答本路径，于是旧的门看到的是它自己，而其后每条命令测量的是植入仓库的索引；植入的独立仓库根本没有可往返的 admin 条目——scratch-tree对自己的重置正是用这道检查把关，读取侧此前没有）；以及把校验通过的--git-dir/--work-tree固定到其后每一个 spawn，使门之后才落地的掉包同样无法重定向。为什么新开 PR
#9572 是这个提交外加另外四个，把它从 2 文件 / +204 撑到 8 文件 / +2 602，伸进了
agent-prompt.ts、fetch-pr.ts、scratch-tree.ts。开替代 PR 而不是 force-push，是为了让那段历史对想读的人完整保留。为什么值得做
此前"不做"的记录理由（#9221 第 17 轮）是：显式固定会改变
ls-files的解析对象，因此需要单独一轮测量。"需要单独一轮"是对的，其余是错的——上方表格就是那次测量：四种形态、五条命令，固定与不固定逐字节相同。而且它确实有用：在身份已被捕获之后把 gitfile 掉包到一个索引里已含突变体的仓库，发现路径报告空（突变体被认证为干净），固定路径仍然报告
M a.ts与未跟踪的探针文件（见上方输出）。审查者验证方案
如何验证
cd packages/cli && npx vitest run src/commands/review→ 4 504 通过、1 跳过、0 失败。新增：对于
.git被掉包到"能为本路径作答"的仓库的树，worktreeResidue返回unmeasured并点名 "does not point back"。夹具先断言真实读数，因此它不会因为"碰巧不可测量"而通过。证据（Before & After）
测试环境
macOS 26.6（Darwin 25.6.0）、git 2.51、Node 24、
packages/clivitest 3.2.4。风险与范围
限于
packages/cli/src/commands/review/lib/worktree.ts。新增一条拒绝路径（admin 条目不能往返的 gitfile 判为unmeasured而非clean），以及五条命令上一段经测量证明零影响的参数前缀。它拒绝的形态是健康流水线永远不会产生的：
fetch-pr用git worktree add创建 review 工作树，而该命令总会写入可往返的 admin 条目。关联 Issue
Closes #9557。替代 #9572。来自 #9221 的后续；所属类别是 #9556。