feat(review): add the prose-execution audit and the counter-frame audit - #10221
feat(review): add the prose-execution audit and the counter-frame audit#10221wenshao wants to merge 4 commits into
Conversation
Two roster lenses from the #9655 post-mortem (#9707, proposals 3 and 4): - `prose-exec`: when the diff touches an instruction file, one agent EXECUTES the changed instructions in a disposable copy instead of reading them, and files the divergence between the executed outcome and what the prose promises. Its copy is a STANDALONE clone (`scratch-tree --standalone`): a `.git` of its own sharing only the object store, so a config, hook or ref write a recipe step makes dies with the tree and never reaches the user's repository through a linked worktree's shared common dir. - `6d`: the counter-frame audit — reads the PR context once for the author's nominated topics (its exclusion list) and the motivating incident (replayed step by step), whole-diff in both topologies, gated on the PR identity like Agent 0. Replaces #9717.
|
|
|
Thanks for the PR! Template: complete ✓ — all required headings present, bilingual body included. Problem: observed, not theoretical. This implements proposals 3 and 4 of the #9655 post-mortem (#9707, still open): one blocking defect escaped four Direction: aligned. This is the review pipeline that audits this repo's own PRs, and its shape was decided in the post-mortem issue, not invented here. The one genuinely new mechanism — executing changed prompt prose instead of reading it — is exactly the gap #9655 demonstrated (a fix recipe whose verbatim execution yields Size: core paths touched — Approach: this is the interesting part, and it's a subtraction. The replaced PR #9717 (closed) grew an 866-line fail-closed screen around a shared linked worktree over nine review rounds without converging; this version deletes the screen entirely by giving the prose-exec audit a standalone Risk: no revert-history high-risk paths matched. The elevated-attention item is self-inflicted by design — the PR introduces an agent that executes PR-authored text — so the scratch-tree isolation is where review depth goes. Moving on to code review. 🔍 中文说明感谢贡献！ **模板：**完整 ✓ —— 所有必需小节齐全，包含双语正文。 **问题：**已观测到，非理论性。本 PR 实现 #9655 复盘（#9707，仍 open）的提案 3、4：一个阻塞缺陷逃过四轮 **方向：**对齐。这是审查本仓库 PR 的 review 流水线，其形态由复盘 issue 决定，而非本 PR 新造。唯一真正的新机制——对变更的提示词文本"执行"而非"阅读"——正是 #9655 暴露的缺口（逐字执行会得到 **规模：**触及核心路径—— **方案：**这是本 PR 的关键，而且是做减法。被替代的 #9717（已关闭）在共享 linked worktree 上堆出 866 行 fail-closed 筛查，九轮评审仍未收敛；本版直接删除筛查，让提示词执行审计使用一个仅共享对象库的独立 **风险：**未命中 revert 历史高风险路径。需要重点关注的点是本 PR 设计上自带的——它引入了一个执行 PR 作者文本的 agent——所以 scratch-tree 隔离是审查深度的去向。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
Code reviewMy independent baseline for this problem (written before reading the diff): two roster lenses — a counter-frame reviewer gated on PR identity + effort so it can't launch into reviews that have no frame to counter, and a prose-execution lens triggered by a built-in path predicate over instruction files; for the execution lens, the scratch environment must be a fresh standalone clone (own What I verified against the code (static read; no PR code executed):
One non-blocking observation: 6d's weld re-validates the identity more thinly than Agent 0's (no No Criticals, no blockers. Testing evidence — the PR's own CI (review code is never executed here)The unit suite ( Sandboxed verification would settle the one claim a green suite can't fully prove on its own: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查：独立基线（读 diff 前写下）与 PR 方案一致——两个 roster 视角：按 PR 身份 + effort 门控的反框架审计（6d），以及由内置路径谓词触发的提示词执行审计（prose-exec）；执行环境用独立克隆（自己的 已对照代码核实（静态阅读，未执行 PR 代码）：
一条不阻塞的观察：6d 焊点对身份值的复核比 Agent 0 薄（无 无 Critical，无阻塞项。 测试证据：单元套件（ 沙箱验证可为绿色套件无法独自闭环的一点补证： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean static review with no blockers; the two reservations are the unit suite still running on the reviewed commit, and the new briefs' live-model efficacy being unmeasured by design. This one earns its merge. The motivation is a measured incident, not a theory: a blocking defect escaped four The implementation is the honest version of the change. Against #9717's +2595 lines this is +1340/−68 — the 866-line shared-worktree screen is gone because a tree that shares nothing needs no screen, and nine rounds of evidence showed the screen didn't converge. Every edit maps to the stated goal: the two roster lenses, their gates and welds, the Approval deferred until CI lands green on 中文说明置信度：4/5 —— 静态审查干净、无阻塞项；两个保留意见：被审提交上的单元套件仍在运行，以及两个新 brief 在真实模型上的效果按设计未经实测。 这个 PR 配得上合并。动机是实测事故而非假设：一个阻塞缺陷逃过四轮 实现是这个改动的诚实版本。相对 #9717 的 +2595 行，这里是 +1340/−68——866 行的共享 worktree 筛查被删除，因为什么都不共享的树不需要筛查，且九轮证据表明筛查不收敛。每一处改动都对应既定目标：两个 roster 视角、它们的门控与焊点、所依赖的 批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Verification report (head
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): review/SKILL.test.ts — no such file or directory; 5147 passed — this review observed 25036, 21951, 1737, 1664, 605, 4320, 638 passed; 48 passed — this review observed 25036, 21951, 1737, 1664, 605, 4320, 638 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：review/SKILL.test.ts — no such file or directory; 5147 passed — this review observed 25036, 21951, 1737, 1664, 605, 4320, 638 passed; 48 passed — this review observed 25036, 21951, 1737, 1664, 605, 4320, 638 passed。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (redirectedAncestor(dirname(resolve(tree)), dirname(common))) { | ||
| return unavailable( | ||
| `an ancestor of ${tree} is a symlink — a standalone tree is removed ` + |
There was a problem hiding this comment.
[Suggestion] The standalone branch's two early refusals (this ancestor-symlink one and the common-dir resolution failure just below) return unavailable() — which answers sharedTreeResidue: [] and sharedTreeUnmeasured: 'the command refused before it measured the shared worktree' — but the residue was already measured above (the worktreeResidue(...) call runs before the tree path is derived). The report discards a measured contamination list and states as unmeasured a measurement that did happen. The clone-build-failure catch below explicitly avoids unavailable() for this exact reason ("Not unavailable(): the residue was already measured") and passes the measurement through. Concretely: if the shared worktree is dirty (probe residue — the exact state sharedTreeResidue exists to surface) and the ancestor walk refuses, the caller never receives the WARNING naming the contaminated paths or the restore-by-shape guidance — at the one moment the filesystem is showing signs of tampering. Return the same shape the clone-failure catch uses (standalone: true, sharedTreeResidue, sharedTreeResidueTotal: residue.total, sharedTreeUnmeasured: residue.unmeasured, refusal note + residueNote), or move the ancestor walk above the residue measurement so unavailable()'s claim becomes true. If you take the first shape, add a test that dirties the shared worktree before triggering the refusal and asserts sharedTreeResidue names the planted path — it must go red when the fix is reverted.
中文说明
独立克隆分支的两个早期拒绝（此处的祖先符号链接拒绝，以及紧随其后的 common-dir 解析失败）返回 unavailable()——即回答 sharedTreeResidue: [] 和 sharedTreeUnmeasured: 'the command refused before it measured the shared worktree'——但残留此前已经测量（worktreeResidue(...) 调用先于树路径推导执行）。报告丢弃了已测得的污染列表，并把做过的测量说成未做。下方克隆构建失败的 catch 明确为此避开 unavailable()（"Not unavailable(): the residue was already measured"）并透传测量结果。具体地：若共享 worktree 有残留（探针残留——正是 sharedTreeResidue 要暴露的状态）且祖先目录检查触发拒绝，调用方收不到点名污染路径的 WARNING 与按形态恢复的指引——恰在文件系统显现异常的时刻。建议改用与克隆失败 catch 相同的返回形态（standalone: true、sharedTreeResidue、sharedTreeResidueTotal: residue.total、sharedTreeUnmeasured: residue.unmeasured、拒绝说明 + residueNote），或把祖先目录检查移到残留测量之前使 unavailable() 的声明成立。若采用第一种形态，请补一个测试：先弄脏共享 worktree 再触发拒绝，断言 sharedTreeResidue 点名植入的路径——还原修复时该测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // add site owes. A recorded, EMPTY list is a different statement — a | ||
| // diff that touched nothing owes nothing. |
There was a problem hiding this comment.
[Suggestion] This added comment declares a recorded empty files[] owes no prose-exec audit, but the assertion three lines below (expect(keys({ ...PR, files: [] })).toContain('prose-exec')) and the new hasPromptFiles implementation do exactly the opposite — hasPromptFiles treats a recorded [] identically to an absent/garbage one (if (files.length === 0) return true;), matching the fail-safe convention of hasDeletions (also pinned true for files: []). The same commit thus carries a comment, an assertion, and an implementation that disagree about files: []. A future maintainer touching the empty-list handling reads the comment as the pinned intent and flips hasPromptFiles to return false for a recorded [], breaking the version-skew fail-safe the surrounding sentences exist for and diverging from the pinned hasDeletions convention — and this test then blocks them, with nothing in the commit to say which artifact is the contract. Rewrite the sentence to state the shipped behaviour, e.g. "a recorded EMPTY list gets the same fail-safe answer as an absent one — the audit runs and returns a documented empty scope."
中文说明
新增注释声称记录在案的空 files[] 不欠 prose-exec 审计，但三行之下的断言（expect(keys({ ...PR, files: [] })).toContain('prose-exec')）与新的 hasPromptFiles 实现恰好相反——hasPromptFiles 对记录在案的 [] 与缺失/垃圾值一视同仁（if (files.length === 0) return true;），与 hasDeletions 的兜底约定一致（同样对 files: [] 钉住为 true）。同一提交因此携带了对 files: [] 语义互相矛盾的注释、断言与实现。未来维护者处理空列表逻辑时按注释意图把 hasPromptFiles 改成对记录在案的 [] 返回 false，就会破坏周边句子所服务的版本偏差兜底，并与已钉住的 hasDeletions 约定分叉——此时本测试还会拦住他，而提交中没有任何东西说明哪个才是契约。请把该句改写为描述实际行为，例如"记录在案的空列表与缺失获得相同的兜底答案——审计运行并返回文档化的空范围"。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
R1-2: the contradicting files: [] comment in roster.test.ts — Valid finding. Deferred to the next round only because this round is capped at ~8 implemented findings (it prioritized the one behavior fix and the gate/witness pins); it is not declined and will be addressed next round.
中文说明
R1-2：roster.test.ts 中与实现矛盾的 files: [] 注释——有效发现。仅因本轮实现上限约 8 条而推迟到下一轮（本轮优先处理唯一的行为修复与门控/见证钉句）；并非拒绝，将在下一轮处理。
| expect( | ||
| existsSync(join(elsewhere, basename(scratchWorktreePath(worktree, 'x')))), | ||
| ).toBe(false); |
There was a problem hiding this comment.
[Suggestion] This residue assertion checks a leaf name derived from label 'x', but the run it witnesses uses the default label ('prose-exec--round-1--abc123') — a path this command can never create for that run, so the assertion is vacuous for the leak it was written to witness. scratchWorktreePath(worktree, label) returns ${resolve(worktree)}-scratch-${scratchLabel(label)}, so this run could only ever create …-scratch-prose-exec--round-1--abc123 under the symlinked .qwen/tmp; the assertion instead checks elsewhere/…-scratch-x. If the redirectedAncestor refusal regressed so the clone were created through the link, the tree would land at the run's actual label-derived path and this line would still pass. (The refusal itself remains pinned via r.available/r.note, so nothing ships broken — the cost is a witness that misrepresents what it covers.) Use the run's own label: existsSync(join(elsewhere, basename(scratchWorktreePath(worktree, 'prose-exec--round-1--abc123')))).
中文说明
此残留断言检查由标签 'x' 派生的叶子名，但它所见证的那次运行使用默认标签（'prose-exec--round-1--abc123'）——该命令在这次运行中永远不会创建这个路径，所以断言对其要见证的泄漏是空洞的。scratchWorktreePath(worktree, label) 返回 ${resolve(worktree)}-scratch-${scratchLabel(label)}，因此这次运行只可能在被符号链接的 .qwen/tmp 下创建 …-scratch-prose-exec--round-1--abc123；断言却检查 elsewhere/…-scratch-x。若 redirectedAncestor 拒绝回归、克隆经符号链接创建，树会落在真实标签派生路径而此断言仍然通过。（拒绝本身仍由 r.available/r.note 钉住，不会带着破损上线——代价是见证与实际覆盖不符。）请改用本次运行自己的标签：existsSync(join(elsewhere, basename(scratchWorktreePath(worktree, 'prose-exec--round-1--abc123'))))。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const executor = role === '7' || role === 'prose-exec'; | ||
| parts.push(...tail(executor ? undefined : opts.rules, brief.output)); |
There was a problem hiding this comment.
[Suggestion] The prose-exec half of this rules-exclusion condition has no test: the only exclusion test ("carries the project rules into every reviewing role — and NOT into Agent 7") pins role '7' alone, and an exhaustive grep confirms no test anywhere passes rules to a prose-exec brief build. The roster path hands the same --rules to every role it builds, so the mutant role === '7' || role === 'prose-exec' → role === '7' (a likely "simplification" in a future edit) ships green — and on any review that loaded project rules every prose-exec brief silently carries the reviewer's rules, steering the executor audit that the adjacent comment and SKILL.md explicitly exclude. Extend the exclusion test beside the existing '7' assertions: expect(buildRoleBrief({ ...PLAN, prNumber: '1', ownerRepo: 'a/b', worktreePath: 'w' }, 'prose-exec', { rules: 'No \any`.' })).not.toContain('No `any`.'). That assertion must go red when executoris reverted torole === '7'`.
中文说明
此规则排除条件的 prose-exec 一半没有测试：唯一的排除测试（"carries the project rules into every reviewing role — and NOT into Agent 7"）只钉住角色 '7'，穷举 grep 确认没有任何测试向 prose-exec brief 构建传入 rules。roster 路径会把同一份 --rules 交给它构建的每个角色，因此变异 role === '7' || role === 'prose-exec' → role === '7'（未来编辑中很可能出现的"简化"）将绿灯通过——任何加载了项目规则的评审都会把评审规则悄悄注入每个 prose-exec brief，引导本应保持规则空白的执行器审计。请在现有 '7' 断言旁补充排除测试：expect(buildRoleBrief({ ...PLAN, prNumber: '1', ownerRepo: 'a/b', worktreePath: 'w' }, 'prose-exec', { rules: 'No \any`.' })).not.toContain('No `any`.')。把 executor还原为role === '7'` 时该断言必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| function countersFrame(plan: RosterPlan): boolean { | ||
| return ( | ||
| plan.effort !== 'medium' && |
There was a problem hiding this comment.
[Suggestion] Medium now also drops the counter-frame audit (6d) via this countersFrame gate, but the medium-drop enumeration is stale in three places: the user-facing --effort help string in parse-args.ts (EFFORT_OPTION.describe, read via fetch-pr/capture-local/plan-diff), the RosterPlan.effort doc comment in this file, and the comment at effort.ts:17 — all still enumerate only the personas (6a/6b/6c) and the specialists (1d/1e). SKILL.md's medium bullet WAS updated in this PR to name the 6d skip; these three now contradict it. An operator choosing medium vs high off --help reads a stale reduction list, and a maintainer extending the effort tiers reads the RosterPlan.effort doc — written as the authoritative statement of what medium drops — and concludes 6d survives medium, contradicting the gate added here. The option's own doc comment states the contract this breaks: "its describe names what medium drops from the roster, so a roster change edits one string". Add the counter-frame audit (6d) to all three enumerations.
中文说明
medium 现在通过此 countersFrame 门控同时跳过反框架审计（6d），但 medium 删减列表在三处过期：面向用户的 --effort 帮助文本（parse-args.ts 的 EFFORT_OPTION.describe，经 fetch-pr/capture-local/plan-diff 读取）、本文件的 RosterPlan.effort 文档注释、以及 effort.ts:17 的注释——三处仍只列出人格（6a/6b/6c）与专项（1d/1e）。SKILL.md 的 medium 条目在本 PR 中已更新点名跳过 6d；这三处现在与之矛盾。操作者据 --help 在 medium 与 high 之间选择时读到过期列表；维护者扩展 effort 层级时读 RosterPlan.effort 注释——它被写成 medium 删减内容的权威陈述——会得出 6d 在 medium 存活的错误结论，与此处新增门控矛盾。该选项自身的文档注释写明了被打破的契约："its describe names what medium drops from the roster, so a roster change edits one string"。请在三处列表中都加入反框架审计（6d）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| @@ -475,9 +475,9 @@ Agent 2 (Security) — WHIFF (returned "No issues found." with no evidence | |||
|
|
|||
| A check you perform silently is a check you skip, and this one has been skipped (measured; DESIGN.md — The six-second Agent 0). The roll-call is what makes that impossible to miss — you cannot write the artifact line for an agent that named no artifact, and a `WHIFF` line you have written is a `WHIFF` you must then act on (relaunch once; on a second bare return, record the dimension in `unreviewedDimensions`, which forbids the Approve). | |||
|
|
|||
| **The whole-diff agents have no receipt, so this is the only check they get: an agent that returns near-instantly with almost no output did not do its job, and its silence is indistinguishable from "found nothing".** This is not hypothetical (measured; DESIGN.md — The eleven-second invariant agent). Apply the check to **every agent that owes no receipt** — in 3B, the whole-diff agents (Agent 0, **1b**, 1c, Agent 7, the invariant agents, the test-coverage matrix, Agent 8); in 3A, **all of them**, since no 3A agent emits a receipt (Agents 0, 1a, 1b, 1c, 1d, 2, 3a, 3b, 3c, 4, 5, 6a, 6b, 6c, 7, and 1e and Agent 8 if launched). A whiffing 3A dimension agent is exactly as invisible as a whiffing invariant agent, and the same one-line fix applies. For each such agent, sanity-check that its return is substantive: it names the specific fields/callers/lines it walked, or it explicitly says "No issues found" **after** describing what it examined. For **Agent 7** the evidence is the build/test **commands it ran and their outcomes** — a Build & Test return that names no command whiffed even if it says "build passed", and after its second whiff record `build-and-test` in `unreviewedDimensions` like any other dimension: a zero-finding run whose deterministic verification never actually ran must not certify on its silence. A legitimately empty scope also passes — Agent 0 on a feature PR with no linked issue returns "No issues found — scope empty" plus the evidence it checked (empty `closingIssuesReferences`, no referenced issue, not a bugfix — plus, when the description narrates a motivating incident, the replay's outcome: the step the replay saw change, or, when it narrates none, an explicit statement of that; a replay that found NO step changed arrives as a Critical **finding**, never inside this receipt), and that is a complete answer, not a whiff; do not relaunch it. What fails the check is a bare "No issues found" with no evidence of any walk or scope determination, or a response conspicuously shorter and faster than its peers — relaunch that one agent before Step 4, **once**. The relaunch is capped at one attempt per agent: if the second return is also bare, do not spin — take it, and record that agent's dimension in an **`unreviewedDimensions`** list. (The finding format tells every agent to return `No issues found — <what you examined>`; an agent that ignores that twice is not going to comply on the third ask.) A silent whole-diff agent is the Step-3A/3B equivalent of a chunk with no receipt — **and it is treated like one**: `unreviewedDimensions` is carried into Step 6's "Not reviewed" section, it **forbids an Approve** (a dimension nobody reviewed cannot be certified clean, exactly as an uncoverable chunk cannot), and Step 7 serializes it in the review body (compose-review's `unreviewedDimensions` input), named alongside any uncoverable chunks. A run that silently drops Security or the cross-chunk removed-behavior audit and then posts LGTM is the failure this whole check exists to prevent; noting the gap in the terminal and approving anyway would only move it. | |||
| **The whole-diff agents have no receipt, so this is the only check they get: an agent that returns near-instantly with almost no output did not do its job, and its silence is indistinguishable from "found nothing".** This is not hypothetical (measured; DESIGN.md — The eleven-second invariant agent). Apply the check to **every agent that owes no receipt** — in 3B, the whole-diff agents (Agent 0, **1b**, 1c, Agent 7, the invariant agents, the test-coverage matrix, the counter-frame audit 6d, `prose-exec` when owed, Agent 8); in 3A, **all of them**, since no 3A agent emits a receipt (Agents 0, 1a, 1b, 1c, 1d, 1e, 2, 3a, 3b, 3c, 4, 5, 6a, 6b, 6c, 6d, 7, `prose-exec` when owed, and Agent 8 if launched). A whiffing 3A dimension agent is exactly as invisible as a whiffing invariant agent, and the same one-line fix applies. For each such agent, sanity-check that its return is substantive: it names the specific fields/callers/lines it walked, or it explicitly says "No issues found" **after** describing what it examined. For **Agent 7** the evidence is the build/test **commands it ran and their outcomes** — a Build & Test return that names no command whiffed even if it says "build passed", and after its second whiff record `build-and-test` in `unreviewedDimensions` like any other dimension: a zero-finding run whose deterministic verification never actually ran must not certify on its silence. A legitimately empty scope also passes — Agent 0 on a feature PR with no linked issue returns "No issues found — scope empty" plus the evidence it checked (empty `closingIssuesReferences`, no referenced issue, not a bugfix — plus, when the description narrates a motivating incident, the replay's outcome: the step the replay saw change, or, when it narrates none, an explicit statement of that; a replay that found NO step changed arrives as a Critical **finding**, never inside this receipt), and that is a complete answer, not a whiff; do not relaunch it. What fails the check is a bare "No issues found" with no evidence of any walk or scope determination, or a response conspicuously shorter and faster than its peers — relaunch that one agent before Step 4, **once**. The relaunch is capped at one attempt per agent: if the second return is also bare, do not spin — take it, and record that agent's dimension in an **`unreviewedDimensions`** list. (The finding format tells every agent to return `No issues found — <what you examined>`; an agent that ignores that twice is not going to comply on the third ask.) A silent whole-diff agent is the Step-3A/3B equivalent of a chunk with no receipt — **and it is treated like one**: `unreviewedDimensions` is carried into Step 6's "Not reviewed" section, it **forbids an Approve** (a dimension nobody reviewed cannot be certified clean, exactly as an uncoverable chunk cannot), and Step 7 serializes it in the review body (compose-review's `unreviewedDimensions` input), named alongside any uncoverable chunks. A run that silently drops Security or the cross-chunk removed-behavior audit and then posts LGTM is the failure this whole check exists to prevent; noting the gap in the terminal and approving anyway would only move it. | |||
There was a problem hiding this comment.
[Suggestion] The whiff check now names prose-exec as an agent to sanity-check, but the paragraph's executor-evidence rule — "For Agent 7 the evidence is the build/test commands it ran … a return that names no command whiffed even if it says 'build passed'" — is not extended to it; grep confirms no other clause does. So a whiffing prose-exec reads its brief and the diff, executes nothing, and returns "No issues found — walked the changed recipe steps in hunks 3–5"; the general substantiveness rule passes it, and the run certifies the instruction prose with zero executions — the measured #9655 failure shape shipping green. This PR models prose-exec as Agent 7's executor twin everywhere else (budgetExempt "for Agent 7's reason", the build-boundary weld role === '7' || role === 'prose-exec', the rules exclusion "prose-exec sits on Agent 7's side of that line") — only the whiff check breaks the pairing. Extend the sentence to both executors, e.g. "For Agent 7 and prose-exec the evidence is the commands they ran and their outcomes — … a prose-exec return that names no executed step whiffed even if it says the prose is consistent (reading is exactly the evidence this role exists to distrust)".
中文说明
虚检（whiff check）现在把 prose-exec 列为需要健全性检查的 agent，但段中的执行器证据规则——"对 Agent 7，证据是它运行过的构建/测试命令……不点名任何命令的返回即使说'构建通过'也是虚检"——没有扩展给它；grep 确认没有其他句子扩展了。于是虚检的 prose-exec 读了 brief 与 diff、什么都没执行，返回"No issues found — walked the changed recipe steps in hunks 3–5"；通用实质性规则放行，整轮评审在零执行的情况下为指令文本背书——正是 #9655 实测失败形态绿灯通过。本 PR 在其他所有位置都把 prose-exec 建模为 Agent 7 的执行器孪生（budgetExempt "因 Agent 7 的原因"、构建边界焊点 role === '7' || role === 'prose-exec'、规则排除 "prose-exec 与 Agent 7 同侧"）——只有虚检破坏了这一配对。请把该句扩展到两个执行器，例如"对 Agent 7 与 prose-exec，证据是它们运行过的命令及结果——……prose-exec 若不点名任何已执行步骤，即使声称文本一致也是虚检（阅读恰是此角色生来不信的证据）"。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
R1-18: SKILL.md whiff-check executor-evidence rule not extended to prose-exec — Valid finding. Deferred to the next round only because this round is capped at ~8 implemented findings (it prioritized the one behavior fix and the gate/witness pins); it is not declined and will be addressed next round.
中文说明
R1-18：SKILL.md 虚检的执行器证据规则未扩展到 prose-exec——有效发现。仅因本轮实现上限约 8 条而推迟到下一轮（本轮优先处理唯一的行为修复与门控/见证钉句）；并非拒绝，将在下一轮处理。
| * True when the tree is a standalone clone (`--standalone`) rather than a | ||
| * linked worktree: its `.git` is its own and nothing written under it | ||
| * reaches the user's repository. Disclosed so a reader of the report knows |
There was a problem hiding this comment.
[Suggestion] This unconditional guarantee — "nothing written under it reaches the user's repository" — is falsified by the node_modules farm this same PR wires into the standalone tree (farmDependencies(tree, worktree, { rebuild: true }) in the standalone branch): exposeDependencies's own doc states the links are read-write and point OUT of the disposable tree into the review worktree, which sits inside the user's repository — and the prose-exec brief's "never write THROUGH a link" cannot bind the PR-authored recipe code this shape exists to contain. A recipe step that writes into an existing dependency directory — npm rebuild (lifecycle scripts run with cwd at the link target), or a package that writes into its own directory at install/runtime — follows node_modules/<pkg> and lands in the review worktree's node_modules: invisible to the residue probe (gitignored), re-linked into every later scratch tree, and read by Agent 7's build/test — a cross-agent contamination channel surviving rounds inside the review, while this report field certifies no write can leave the tree. Every other isolation claim in this file is deliberately git-scoped ("config, hooks or refs"; "nothing you do through its git"); this one alone is not. Scope the doc to git state ("nothing written into its git state reaches the user's repository") to match the file's other claims — and consider enforcing the farm contract for this untrusted-writer shape (read-only links or copies) rather than declaring it. If you add enforcement, a test that writes through a linked package inside the clone and asserts the review worktree's copy is untouched must go red when read-write links are restored.
中文说明
此无条件保证——"写入其下的任何内容都不会到达用户仓库"——被本 PR 接入独立树的 node_modules 农场所证伪（独立分支中的 farmDependencies(tree, worktree, { rebuild: true })）：exposeDependencies 的文档明确这些链接是读写且指向一次性树之外、位于用户仓库之内的评审 worktree——而 prose-exec brief 的"绝不透过链接写入"约束不了本形态要容纳的、由 PR 作者编写的 recipe 代码。向已有依赖目录写入的 recipe 步骤——npm rebuild（生命周期脚本以链接目标为 cwd 运行）、或安装/运行时写入自身目录的包——会沿 node_modules/<pkg> 落在评审 worktree 的 node_modules：残留探针不可见（被 gitignore）、每次后续 scratch 树都会重新链入、并被 Agent 7 的构建/测试读取——一条在评审内部跨轮存活的跨 agent 污染通道，而本报告字段却声称任何写入都无法离开此树。本文件其他所有隔离声明都刻意限定在 git 范围（"config、hooks 或 refs"；"通过其 git 所做的一切"）；唯独此处不是。请把文档限定到 git 状态（"写入其 git 状态的任何内容都不会到达用户仓库"）以与文件其他声明一致——并考虑为这种不可信写入者形态强制执行农场契约（只读链接或拷贝）而非仅作声明。若增加强制，请补一个测试：透过克隆内链接包写入并断言评审 worktree 的副本未被动——恢复读写链接时它必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| 'installing INTO your copy is fine (the next call re-links it), but ' + | ||
| 'never write THROUGH a link (`npm rebuild`, a package writing into its ' + | ||
| 'own directory) — that lands in the shared tree every other agent is ' + | ||
| 'reading.', |
There was a problem hiding this comment.
[Suggestion] The prose-exec weld omits the monorepo workspace-resolution caveat the verifier weld carries one block above ("One limit of the scratch tree … in a monorepo that means a workspace package (@scope/pkg) resolves to the review worktree's built copy, not to your scratch tree's source"), even though farmDependencies links the same farm into the standalone tree and its doc states workspace packages "resolve through it to code the PR head produced" (links point OUT of the scratch tree). A monorepo PR that changes workspace package A and an instruction recipe that builds or modifies A in the copy and then runs something in dependent package B (npm run build -w packages/a && npm test -w packages/b) executes B's import of @scope/a through the linked farm — against the review worktree's built A, not the copy the recipe just built. The run executes the environment's artifact, not the recipe's output, and prose-exec files the resulting failure (or a false success) as a prose divergence — the exact misattribution this weld's own comment exists to prevent ("a hand-rolled copy without the farm fails builds for environment reasons the agent would misfile as prose divergence"). Port the verifier weld's paragraph into this block, phrased for execution: a workspace package resolves to the review worktree's built copy, so a step that builds/modifies package A and runs in package B will not see the change — attribute that to the harness, not the prose. Pin it in the prose-exec weld test beside the existing --standalone pins; deleting the sentence must turn that assertion red.
中文说明
prose-exec weld 漏掉了上方一个块处 verifier weld 携带的 monorepo 工作区解析告警（"One limit of the scratch tree … in a monorepo that means a workspace package (@scope/pkg) resolves to the review worktree's built copy, not to your scratch tree's source"），尽管 farmDependencies 把同一个农场链入独立树、且其文档写明工作区包"透过它解析到 PR head 产出的代码"（链接指向 scratch 树之外）。某 monorepo PR 修改了工作区包 A，指令 recipe 在副本中构建或修改 A 后再运行依赖包 B（npm run build -w packages/a && npm test -w packages/b）——B 对 @scope/a 的导入会透过链接农场解析到评审 worktree 构建的 A，而不是 recipe 刚构建的副本。运行执行的是环境产物而非 recipe 输出，prose-exec 会把由此产生的失败（或假成功）记为提示词分歧——正是本 weld 注释要防止的错误归因（"没有农场的手搓副本会因环境原因构建失败，代理会误记为提示词分歧"）。请把 verifier weld 的那段移植到本块，改为执行口径：工作区包解析到评审 worktree 构建的副本，因此构建/修改包 A 后在包 B 运行的步骤看不到该变更——把它归因于测试设施而非提示词。在 prose-exec weld 测试中现有 --standalone 钉句旁补钉；删除该句时断言必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
R1-20: the prose-exec weld's missing monorepo workspace-resolution caveat — Valid finding. Deferred to the next round only because this round is capped at ~8 implemented findings (it prioritized the one behavior fix and the gate/witness pins); it is not declined and will be addressed next round.
中文说明
R1-20：prose-exec weld 漏掉的 monorepo 工作区解析告警——有效发现。仅因本轮实现上限约 8 条而推迟到下一轮（本轮优先处理唯一的行为修复与门控/见证钉句）；并非拒绝，将在下一轮处理。
| - **medium** — **balanced**: the high pipeline with its most expensive passes removed. It runs the parallel review agents (Step 3A/3B) over a **reduced dimension set** — issue fidelity (Agent 0, PR targets only), correctness (Agents 1a/1b/1c), **security (Agent 2)**, quality (Agents 3a/3b/3c), performance (Agent 4), **test coverage (Agent 5)**, and **build & test (Agent 7)** — followed by a **single verification pass** (Step 4). It loads and enforces project rules (Step 2) and runs `comment-status` like high. It **skips** the adversarial-persona agents (6a/6b/6c), the language-pitfall and wrapper/proxy specialists (Agents 1d/1e), the diff-specialist finders (Agent 8), the **reverse audit** (Step 5), the incremental cache, and PR posting (`--comment` still forces high). Findings are **verified** (Step 4 ran — they are not "unverified" the way low's are), but without the reverse-audit second pass. Reach for it when high is too slow/expensive but a real bug-catching review is still needed: it keeps the two things that reliably catch bugs cheaply — the finder fan-out and `build-test` (which mechanically catches compile/test failures) — and drops the depth passes with the lowest marginal yield. Measured against high on the same PR it lands at roughly **one-third to one-half** the time and tokens. It reliably catches mechanical defects (compile errors, failing tests) and obvious correctness bugs, but is **not an exhaustive correctness audit** — a subtle Critical that only the reverse audit or the adversarial personas would surface can slip; for a security-sensitive or pre-release review, use `--effort high`. | ||
| - **high** — the full pipeline: parallel review agents (Step 3A/3B — the full dimension set including security, test-coverage, the language-pitfall and wrapper/proxy specialists 1d/1e, the adversarial personas 6a/6b/6c, and Agent 8), verification (Step 4), iterative reverse audit (Step 5), PR submission (Step 7), incremental cache (Step 8). | ||
| - **medium** — **balanced**: the high pipeline with its most expensive passes removed. It runs the parallel review agents (Step 3A/3B) over a **reduced dimension set** — issue fidelity (Agent 0, PR targets only), correctness (Agents 1a/1b/1c), **security (Agent 2)**, quality (Agents 3a/3b/3c), performance (Agent 4), **test coverage (Agent 5)**, and **build & test (Agent 7)** — plus the prose-execution audit (`prose-exec`) when the diff touches an instruction file, which is not effort-gated — followed by a **single verification pass** (Step 4). It loads and enforces project rules (Step 2) and runs `comment-status` like high. It **skips** the adversarial-persona agents (6a/6b/6c), the counter-frame audit (6d), the language-pitfall and wrapper/proxy specialists (Agents 1d/1e), the diff-specialist finders (Agent 8), the **reverse audit** (Step 5), the incremental cache, and PR posting (`--comment` still forces high). Findings are **verified** (Step 4 ran — they are not "unverified" the way low's are), but without the reverse-audit second pass. Reach for it when high is too slow/expensive but a real bug-catching review is still needed: it keeps the two things that reliably catch bugs cheaply — the finder fan-out and `build-test` (which mechanically catches compile/test failures) — and drops the depth passes with the lowest marginal yield. Measured against high on the same PR it lands at roughly **one-third to one-half** the time and tokens. It reliably catches mechanical defects (compile errors, failing tests) and obvious correctness bugs, but is **not an exhaustive correctness audit** — a subtle Critical that only the reverse audit or the adversarial personas would surface can slip; for a security-sensitive or pre-release review, use `--effort high`. | ||
| - **high** — the full pipeline: parallel review agents (Step 3A/3B — the full dimension set including security, test-coverage, the language-pitfall and wrapper/proxy specialists 1d/1e, the adversarial personas 6a/6b/6c, the counter-frame audit 6d, and Agent 8), verification (Step 4), iterative reverse audit (Step 5), PR submission (Step 7), incremental cache (Step 8). |
There was a problem hiding this comment.
[Suggestion] This high bullet lists the counter-frame audit 6d as an unconditional member of the full dimension set, but this same PR's roster gates 6d on PR identity (countersFrame = effort gate AND isPositivePrNumber(plan.prNumber) AND ownerRepo in roster.ts), so no non-PR review ever runs it — and every other 6d site this PR touches carries the qualifier (Step 3A "run only when the review target is a PR", the 3B roles list "6d (PR reviews, high effort)", the medium tip "the counter-frame audit (PR reviews)"). A local-diff or file-path review at high effort has no prNumber in its plan; an orchestrator that launches 6d on this bullet's authority reaches agent-prompt --role 6d, which throws "needs a plan with prNumber and ownerRepo", halting the launch; one that follows the roster gets a verdict without 6d, so this description contradicts the roster verdict the skill says to use verbatim. The sibling medium bullet shows the intended fix shape, qualifying Agent 0 with "(PR targets only)". Qualify the enumeration: "the adversarial personas 6a/6b/6c, the counter-frame audit 6d (PR targets only), and Agent 8".
中文说明
此 high 条目把反框架审计 6d 列为完整维度集的无条件成员，但本 PR 的 roster 将 6d 门控在 PR 身份上（roster.ts 的 countersFrame = effort 门 AND isPositivePrNumber(plan.prNumber) AND ownerRepo），因此非 PR 评审从不运行它——而本 PR 触及的其他每个 6d 位置都带限定（Step 3A "仅当评审目标是 PR 时运行"、3B 角色列表 "6d (PR reviews, high effort)"、medium 提示 "the counter-frame audit (PR reviews)"）。high effort 的本地/文件评审计划没有 prNumber；按本条权威启动 6d 的编排器会碰到 agent-prompt --role 6d 抛出 "needs a plan with prNumber and ownerRepo" 而中止；跟随 roster 的编排器则得到没有 6d 的结论，使本描述与 skill 要求逐字使用的 roster 结论矛盾。兄弟 medium 条目展示了预期的修复形态：给 Agent 0 加 "(PR targets only)" 限定。请给枚举加限定："the adversarial personas 6a/6b/6c, the counter-frame audit 6d (PR targets only), and Agent 8"。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
R1-21: SKILL.md high bullet lists 6d without the '(PR targets only)' qualifier — Valid finding. Deferred to the next round only because this round is capped at ~8 implemented findings (it prioritized the one behavior fix and the gate/witness pins); it is not declined and will be addressed next round.
中文说明
R1-21：SKILL.md high 条目未给 6d 加 '(PR targets only)' 限定——有效发现。仅因本轮实现上限约 8 条而推迟到下一轮（本轮优先处理唯一的行为修复与门控/见证钉句）；并非拒绝，将在下一轮处理。
| @@ -239,7 +239,7 @@ Read from it: | |||
| - `diffLines`, `diffChars`, and `srcDiffLines` / `testDiffLines` / `docsDiffLines` / `generatedDiffLines` | |||
| - `chunks[]` — contiguous, non-overlapping line ranges tiling the whole diff. Each entry has `id`, `startLine`, `endLine` (1-based, inclusive), `lines`, `chars`, an `oversized` flag, and `files[]` naming the source files and new-side line ranges it covers. A chunk with `oversized: true` may exceed what one `read_file` call returns. | |||
| - `files[]` — per-file `kind` (`source` / `test` / `generated`), `hunks[]` new-side ranges (Step 7 validates comment anchors against these), `addedRanges[]` and `diffRange` (present only on `heavy` files — the exact lines the PR wrote, and where that file's own diff lives, so an invariant agent can see what was deleted), change counts, and the `heavy` flag | |||
| - `budget` — how much walking the **size-elastic** parts of this run owe, sized from `srcDiffLines` except that an all-non-source diff (docs, lockfiles) counts its total lines at an eighth rate, so the size these tiers read is `effective = max(srcDiffLines, floor(diffLines / 8))`; recorded here rather than passed as a flag so every reader sees one number. `inlineAngles` and `sweep` scope Step 3C's low pass; `specialistCap` is the Agent 8 ceiling (**0** below 80 source lines — "one domain dominates the diff" is a judgement, and a judgement made about forty lines finds a dominant domain every time, because forty lines are usually all one thing — **and 0 again for a huge diff (effective ≥ 3000)**, where an Agent 8 whole-diff pass on top of the base fan-out is the marginal cost that tips a review too big to finish into posting nothing); `verifyShard` is Step 4's findings-per-verifier; `reverseAuditRounds` is the reverse-audit loop's round cap, **one value per topology**: **10** on a Step 3A diff, **5** on a Step 3B one, **3 for a huge diff** (effective ≥ 3000 lines) — but the huge reduction applies **only when the run has a deadline** (`QWEN_REVIEW_DEADLINE_EPOCH`); without a clock a huge diff is just a large 3B diff and gets 5. One number cannot price all three, because what is being capped is a _round_ and a round costs one auditor on 3A, one auditor per non-retired chunk on 3B, and ~90 minutes on a 4,000-line PR — where five rounds (450 min) alone exceed the six-hour ceiling before the fan-out and tail are counted, and the 6-hour timeouts that posted nothing were 4,000-5,300-line PRs (measured; DESIGN.md — The six-hour timeouts). Ten on 3A because the marginal round there is a single agent against a whole review of 19-30 calls: five was the 3B arithmetic applied where it does not hold, and it stopped loops that were still confirming Criticals to save ~5 calls. Three when huge is not a claim that a huge diff converges sooner — it plainly does not, and on recall it deserves more rounds than a small one, not fewer; it is a claim that five ~90-minute rounds do not fit a six-hour ceiling, and a review killed mid-flight posts nothing at all. Where there is no ceiling the premise is absent and so is the reduction. Three is one audit round above the convergence floor of two — the all-dry rounds-1-and-2 shape converges under any cap of two or more, since the convergence check runs before the cap gate; the extra round buys hot chunks one more pass. An operator may LOWER the tier for every review through the `review.reverseAuditRounds` setting (honoured from the User, System and SystemDefaults scopes — never from the repository's own `.qwen/settings.json`; a value below 3, or above the tier, is ignored rather than clamped, so it leaves the tier alone) — the capture command resolves it into this field, so you read one number here either way and never learn that a setting was involved; it can never RAISE a tier. The `agent-prompt` builder enforces the cap itself (a `ROUND CAP:` refusal, exit 4, that writes a marker `compose-review` caps on — same contract as the deadline gate below), so you never count rounds yourself. `agentToolBudget` is the base rate of the soft tool-call ceiling `agent-prompt` bakes into every finder and auditor brief — not the verifier's, not Agent 7's, and not Agent 0's, whose mandatory work scales with the linked issues rather than the diff. The ceiling is per **launch**: a scoped agent (a chunk, a heavy file) gets an allowance derived from its own territory — never above the plan's recorded allowance, which is clamped into the budget's own band in both directions, so the plan stays the one number every launch answers to — and every launch's assigned reads ride on top of the allowance rather than inside it, so a huge diff's mandatory chunk reads can never exhaust the exploration a whole-diff role owes — because a wave's wall clock is its slowest agent and the slowest agent is reliably one that kept exploring past any recall gain: the same 14-agent fan-out has measured 11.7 and 41 minutes on comparable diffs, the difference being individual agents spending 40-100 calls walking the tree (measured; DESIGN.md — The forty-one minute wave). The ceiling is soft and the briefs restate the recall rule beside it: at the budget an agent stops **exploring**, never reporting — findings in hand are filed, and each stopped check is disclosed on its own line in the fixed form `Budget gap: <the check>`, which `check-coverage` parses out of the transcripts (its report's `budgetGaps`) — see Step 3D for the ruling each gap is owed. **It never scales a dimension away** — which agents a review owes is the roster's answer and the roster reads `effort`, so a size input cannot become a back door into shrinking coverage. Nothing here is yours to override: a budget the caller can inflate is a budget that gets inflated. **A plan with no `budget` field** (written by an older CLI — the version-skew this skill has already measured once) falls back to the pre-budget flat behaviour: walk all six angles, run the sweep, cap Agent 8 at 2, shard verification at 8. Those four err toward more coverage, never less. The round cap is the one exception and is worth naming rather than lumping in: **in a run that has a deadline**, a field-less **huge** plan reads 3 where the flat fallback read 5 — deliberately _less_, because that tier is a finishability ruling and the reviews it exists for are the ones that ran six hours and posted nothing. Without a deadline it reads 5, the same as the flat fallback. | |||
| - `budget` — how much walking the **size-elastic** parts of this run owe, sized from `srcDiffLines` except that an all-non-source diff (docs, lockfiles) counts its total lines at an eighth rate, so the size these tiers read is `effective = max(srcDiffLines, floor(diffLines / 8))`; recorded here rather than passed as a flag so every reader sees one number. `inlineAngles` and `sweep` scope Step 3C's low pass; `specialistCap` is the Agent 8 ceiling (**0** below 80 source lines — "one domain dominates the diff" is a judgement, and a judgement made about forty lines finds a dominant domain every time, because forty lines are usually all one thing — **and 0 again for a huge diff (effective ≥ 3000)**, where an Agent 8 whole-diff pass on top of the base fan-out is the marginal cost that tips a review too big to finish into posting nothing); `verifyShard` is Step 4's findings-per-verifier; `reverseAuditRounds` is the reverse-audit loop's round cap, **one value per topology**: **10** on a Step 3A diff, **5** on a Step 3B one, **3 for a huge diff** (effective ≥ 3000 lines) — but the huge reduction applies **only when the run has a deadline** (`QWEN_REVIEW_DEADLINE_EPOCH`); without a clock a huge diff is just a large 3B diff and gets 5. One number cannot price all three, because what is being capped is a _round_ and a round costs one auditor on 3A, one auditor per non-retired chunk on 3B, and ~90 minutes on a 4,000-line PR — where five rounds (450 min) alone exceed the six-hour ceiling before the fan-out and tail are counted, and the 6-hour timeouts that posted nothing were 4,000-5,300-line PRs (measured; DESIGN.md — The six-hour timeouts). Ten on 3A because the marginal round there is a single agent against a whole review of 20-31 calls: five was the 3B arithmetic applied where it does not hold, and it stopped loops that were still confirming Criticals to save ~5 calls. Three when huge is not a claim that a huge diff converges sooner — it plainly does not, and on recall it deserves more rounds than a small one, not fewer; it is a claim that five ~90-minute rounds do not fit a six-hour ceiling, and a review killed mid-flight posts nothing at all. Where there is no ceiling the premise is absent and so is the reduction. Three is one audit round above the convergence floor of two — the all-dry rounds-1-and-2 shape converges under any cap of two or more, since the convergence check runs before the cap gate; the extra round buys hot chunks one more pass. An operator may LOWER the tier for every review through the `review.reverseAuditRounds` setting (honoured from the User, System and SystemDefaults scopes — never from the repository's own `.qwen/settings.json`; a value below 3, or above the tier, is ignored rather than clamped, so it leaves the tier alone) — the capture command resolves it into this field, so you read one number here either way and never learn that a setting was involved; it can never RAISE a tier. The `agent-prompt` builder enforces the cap itself (a `ROUND CAP:` refusal, exit 4, that writes a marker `compose-review` caps on — same contract as the deadline gate below), so you never count rounds yourself. `agentToolBudget` is the base rate of the soft tool-call ceiling `agent-prompt` bakes into every finder and auditor brief — not the verifier's, not Agent 7's, and not Agent 0's (whose mandatory work scales with the linked issues rather than the diff), not the counter-frame audit 6d's (its mandated PR-context read is discussion-sized) and not the prose-execution audit's (its work is recipe-sized) — five exemptions, the set `agent-prompt` computes from the briefs' own `budgetExempt`. The ceiling is per **launch**: a scoped agent (a chunk, a heavy file) gets an allowance derived from its own territory — never above the plan's recorded allowance, which is clamped into the budget's own band in both directions, so the plan stays the one number every launch answers to — and every launch's assigned reads ride on top of the allowance rather than inside it, so a huge diff's mandatory chunk reads can never exhaust the exploration a whole-diff role owes — because a wave's wall clock is its slowest agent and the slowest agent is reliably one that kept exploring past any recall gain: the same 14-agent fan-out has measured 11.7 and 41 minutes on comparable diffs, the difference being individual agents spending 40-100 calls walking the tree (measured; DESIGN.md — The forty-one minute wave). The ceiling is soft and the briefs restate the recall rule beside it: at the budget an agent stops **exploring**, never reporting — findings in hand are filed, and each stopped check is disclosed on its own line in the fixed form `Budget gap: <the check>`, which `check-coverage` parses out of the transcripts (its report's `budgetGaps`) — see Step 3D for the ruling each gap is owed. **It never scales a dimension away** — which agents a review owes is the roster's answer and the roster reads `effort`, so a size input cannot become a back door into shrinking coverage. Nothing here is yours to override: a budget the caller can inflate is a budget that gets inflated. **A plan with no `budget` field** (written by an older CLI — the version-skew this skill has already measured once) falls back to the pre-budget flat behaviour: walk all six angles, run the sweep, cap Agent 8 at 2, shard verification at 8. Those four err toward more coverage, never less. The round cap is the one exception and is worth naming rather than lumping in: **in a run that has a deadline**, a field-less **huge** plan reads 3 where the flat fallback read 5 — deliberately _less_, because that tier is a finishability ruling and the reviews it exists for are the ones that ran six hours and posted nothing. Without a deadline it reads 5, the same as the flat fallback. | |||
There was a problem hiding this comment.
[Suggestion] The roster re-count this PR writes into SKILL.md (16→17 same-repo agents, 19-30→20-31 calls) leaves the sibling Token Efficiency table in docs/users/features/code-review.md stating the pre-PR numbers — verified stale at HEAD: "Review agents (Step 3): 16 (+0-2)", "(15 without it)", cross-repo "(14)", "Total ~19-30 (~17-29)". A reader sizing cost or capacity from that user-facing page plans on 16 parallel agents and a ~19-30-call total while the shipped roster runs 17 (+0-2) at ~20-31 — plus a conditional 18th agent (prose-exec) on instruction-file diffs the table never mentions; cross-repo lightweight with PR identity is now 15 agents, not 14. The local/file row survives only by coincidence (still 15, but now because both Agent 0 AND 6d are skipped, not just Agent 0 as it states). The two pages of the same feature disagree about the cost of the identical review, and any future re-numbering of one site will have to rediscover this one. Update the docs table with the same re-count: Review agents 17 (+0-2), 16 without 1e, cross-repo 15 when the plan carries the PR identity (14 when pr-context failed), local/file 15 (skipping Agent 0 and the counter-frame audit), Total ~20-31, and note the +1 conditional prose-exec on instruction-file diffs.
中文说明
本 PR 写入 SKILL.md 的 roster 重数（同仓库 16→17 个 agent、19-30→20-31 次调用）让 docs/users/features/code-review.md 中的同款 Token 效率表停留在 PR 前数字——已在 HEAD 验证过期："Review agents (Step 3): 16 (+0-2)"、"(15 without it)"、跨仓库 "(14)"、"Total ~19-30 (~17-29)"。按该用户页面估算成本或容量的读者会按 16 个并行 agent、约 19-30 次调用规划，而出货的 roster 运行 17（+0-2）个、约 20-31 次——且指令文件 diff 上还有表格从未提及的条件性第 18 个 agent（prose-exec）；带 PR 身份的跨仓库轻量模式现在是 15 个而非 14。local/file 行只是碰巧仍对（仍是 15，但现在是因为同时跳过 Agent 0 与 6d，而非其所述只跳 Agent 0）。同一功能的两份页面对同一评审的成本说法不一，未来任一处再次重数都要重新发现这一处。请用同样的重数更新文档表格：Review agents 17（+0-2）、不含 1e 为 16、计划带 PR 身份时跨仓库 15（pr-context 失败时 14）、local/file 15（跳过 Agent 0 与反框架审计）、Total ~20-31，并注明指令文件 diff 上 +1 的条件性 prose-exec。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
R1-22: docs/users/features/code-review.md token-efficiency table re-count — Valid finding. Deferred to the next round only because this round is capped at ~8 implemented findings (it prioritized the one behavior fix and the gate/witness pins); it is not declined and will be addressed next round.
中文说明
R1-22：docs/users/features/code-review.md token 效率表的重数——有效发现。仅因本轮实现上限约 8 条而推迟到下一轮（本轮优先处理唯一的行为修复与门控/见证钉句）；并非拒绝，将在下一轮处理。
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summary — addressing the round-1 review on PR #10221The automated review returned 22 Suggestion findings (no Criticals, no failed checks, no base conflict). This round implements 8 of them and defers the remaining 14 to the next round under the per-round batch cap, so each fix gets a real mutation-tested witness instead of a shallow bulk edit. All changes stay inside the PR's existing footprint ( Addressed this round
Deferred to the next round (batch cap)These are all valid; they are deferred only because the round is capped at ~8 implemented findings, not declined: [R1-2] (contradicting Conflict notes
VerificationCommands actually run (from the repository root unless noted), and their results:
中文说明本轮总结 — 处理 PR #10221 的第 1 轮评审自动评审返回 22 条 Suggestion 发现（无 Critical、无失败检查、无与 base 的冲突）。本轮实现其中 8 条，其余 14 条按每轮批次上限推迟到下一轮，以保证每个修复都有真正经过变异测试的见证，而不是浅层的批量改动。所有改动都在本 PR 已有的足迹内（ 本轮已处理
推迟到下一轮（受批次上限约束）以下均为有效发现，仅因本轮上限约 8 条而推迟，并非拒绝：[R1-2]（ 冲突说明
验证实际运行的命令（除非注明，均在仓库根目录执行）及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
14 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 files:[] comment/assertion contradiction — already reported (comment 3869215573)
- R1-6 'Before the first step runs' timing-clause pin — already reported (comment 3869215596)
- R1-7 lifecycle-script + credential-read clause pins — already reported (comment 3869215600)
- R1-8 quoted-step severity-contract pin — already reported (comment 3869215603)
- R1-9 6d weld guard-shape pins — already reported (comment 3869215606)
- R1-10 6d cannot-read-context branch pin — already reported (comment 3869215609)
- R1-14 SKILL.md 3B base-cost enumeration missing prose-exec — already reported (comment 3869215632)
- R1-15 SKILL.md reserved-shapes enumeration missing .qwen/review-rules.md — already reported (comment 3869215638)
- R1-16 SKILL.md roles-list prose-exec parenthetical too narrow — already reported (comment 3869215644)
- R1-17 SKILL.md prose-exec table row understates the trigger — already reported (comment 3869215651)
- R1-18 SKILL.md whiff-check executor-evidence rule — already reported (comment 3869215656)
- R1-20 prose-exec weld missing monorepo workspace-resolution caveat — already reported (comment 3869215669)
- R1-21 SKILL.md high bullet lists 6d without the PR-only qualifier — already reported (comment 3869215676)
- R1-22 docs/users/features/code-review.md token-efficiency table re-count — already reported (comment 3869215679)
Test Plan (not a blocker): review/SKILL.test.ts — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/agent-prompt.test.ts:2966 — [probe] prose-exec boundary pin covers only Required configurations
中文说明
本轮确认的 14 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
Test Plan（非阻断）：review/SKILL.test.ts — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| git(tree, 'checkout', '--quiet', '--force', '--detach', headSha); | ||
| git(tree, 'remote', 'remove', 'origin'); |
There was a problem hiding this comment.
[Critical] R2-1: buildStandaloneTree hardcodes git remote remove origin, but the clone above it honours the user's global clone.defaultRemoteName — sanitizedGitEnv strips GIT_CONFIG_GLOBAL yet keeps HOME, and this function's own doc says the user's global config still applies to these commands. When that setting names another remote (stock git ≥ 2.37), the clone and the detached checkout succeed, then git remote remove origin dies with No such remote: 'origin', and the catch reports a tree it already built as un-buildable — so every --standalone call refuses and prose-exec answers not-executed on every single one: the audit is permanently dropped for such users, not because isolation failed but because a cosmetic rename failed after isolation had already succeeded. The suite cannot see this: isolateHostGitConfig() gives every test an empty global config.
Witness (probe, git 2.43.0):
PR code: available=false note='git remote remove origin failed: error: No such remote: origin' tree on disk, remotes=[upstream]
+ '--origin','origin': available=true remotes=[]
Pin the remote name at clone time — add '--origin', 'origin' to the clone argv above (or read git remote and remove whatever is listed). Fix witness: in the runScratchTree --standalone describe, set clone.defaultRemoteName=upstream in the isolated global config after isolateHostGitConfig(), run, and assert available === true with no remotes left; removing --origin origin then makes the run refuse and the assertion red.
中文说明
[Critical] R2-1：buildStandaloneTree 硬编码 git remote remove origin，但上面的克隆会尊重用户全局的 clone.defaultRemoteName——sanitizedGitEnv 会剥掉 GIT_CONFIG_GLOBAL 但保留 HOME，而本函数自己的注释也说明用户全局配置对这些命令仍然生效。当该配置指向其他远程名（git ≥ 2.37 的标准配置）时，克隆与 detached checkout 成功，随后 git remote remove origin 以 No such remote: 'origin' 失败，catch 把一个已经建好的树报告为无法建立——于是每次 --standalone 调用都拒绝、prose-exec 每次都回答 not-executed：这类用户的审计被永久丢弃，原因不是隔离失败，而是隔离已经成功之后一个表面性的重命名失败。测试套件看不到这一点：isolateHostGitConfig() 给每个测试的都是空的全局配置。
见证（探针，git 2.43.0）：PR 代码 → available=false，note 为 'git remote remove origin failed: error: No such remote: origin'，磁盘残留树且 remotes=[upstream]；为克隆加上 '--origin','origin' 后 → available=true、remotes=[]。
修复：在克隆时钉住远程名——给上面的克隆 argv 加上 '--origin', 'origin'（或读取 git remote 输出、移除实际列出的远程）。修复见证：在 runScratchTree --standalone describe 中，isolateHostGitConfig() 之后在隔离的全局配置里设置 clone.defaultRemoteName=upstream，运行并断言 available === true 且无残留远程；移除 --origin origin 后运行会拒绝、断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // `worktree add` both execute configured content filters. A standalone | ||
| // clone's checkout reads the CLONE's config, which holds none of these, so | ||
| // the screen has nothing to say there (see `buildStandaloneTree`). | ||
| const filters = args.standalone ? [] : localFilterCommands(worktree); |
There was a problem hiding this comment.
[Critical] R2-2: The standalone screen-skip assumes the clone's checkout is the only filter-executing step, but the same invocation still runs worktreeResidue's git status --porcelain --untracked-files=all -z in the SHARED review worktree, and git status executes repo-local filter.*.clean when re-reading stat-stale files — the exact execution the screen exists to refuse. Plant filter.evil.clean in the common config, select it via attributes, and touch one tracked file to stale its stat cache (the documented two-write plant — any same-user process can do all three; a verifier probe in a linked scratch tree is one such): the next --standalone call then executes the payload while answering residue: [] — certifying clean the same tree it just executed in. Before this diff every mode refused on planted filters before this status ran; worktreeResidue guards core.fsmonitor for exactly this reason while the filter path stays open. The pinned filter test plants a SMUDGE filter on a stat-fresh tree — the one combination git status cannot execute.
Witness (probe, git 2.43.0):
linked control: available=false marker absent note names filter.evil.clean
standalone (PR): available=true residue=[] marker created
screen restored: available=false marker absent
| const filters = args.standalone ? [] : localFilterCommands(worktree); | |
| const filters = localFilterCommands(worktree); |
This restores the pre-diff screen; also adjust the refusal text to say the shared-worktree measurement — not just checkouts — would execute the filters, and flip the standalone arm of the existing filter test to expect the refusal. Fix witness: plant filter.evil.clean plus selecting attributes, touch the selected tracked file, then runScratchTree({…, standalone: true}) must answer available: false naming the filter; removing the restored screen makes it answer available: true — red.
中文说明
[Critical] R2-2：standalone 的筛查跳过假定克隆的 checkout 是唯一执行 filter 的步骤，但同一次调用仍会在共享的 review worktree 里运行 worktreeResidue 的 git status --porcelain --untracked-files=all -z，而 git status 在重读 stat 过期的文件时会执行仓库本地的 filter.*.clean——正是筛查存在所要拒绝的执行。在 common config 植入 filter.evil.clean、用 attributes 选中它、再 touch 一个被跟踪文件使其 stat 缓存过期（文档化的两处写入植入——任何同用户进程都能做到这三步，linked scratch tree 里的 verifier 探针就是其一）：下一次 --standalone 调用就会执行载荷、同时回答 residue: []——在它刚刚执行过载荷的同一棵树上认证其干净。此 diff 之前，所有形态都会在这个 status 运行之前拒绝植入的 filter；worktreeResidue 正是出于同样原因防着 core.fsmonitor，filter 路径却敞着。已钉桩的 filter 测试植入的是 SMUDGE filter 且树是 stat 新鲜的——恰是 git status 无法执行的组合。
见证（探针，git 2.43.0）：linked 对照 available=false、无标记、note 点名 filter.evil.clean；PR 代码的 standalone available=true、residue=[]、标记被创建；恢复 diff 前的筛查后 available=false、无标记。
修复如上（恢复 diff 前的筛查）；同时把拒绝文案改为"共享 worktree 的测量——而不只是 checkout——会执行 filter"，并把现有 filter 测试的 standalone 分支翻转为预期拒绝。修复见证：植入 filter.evil.clean 与选中的 attributes、touch 选中的被跟踪文件，随后 runScratchTree({…, standalone: true}) 必须回答 available: false 并点名该 filter；移除恢复的筛查后它会回答 available: true——变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| * knows which of the two contracts the note is stating. (The dependency | ||
| * farm's links point back at the review worktree; writes THROUGH them are | ||
| * the exception the dependency note discloses.) |
There was a problem hiding this comment.
[Suggestion] R1-19: (fix-induced) The round-2 fix for R1-19 rescoped the falsified guarantee to git state — the right direction — but the added parenthetical claims "writes THROUGH them are the exception the dependency note discloses", and dependencyNote() (scratch-tree.ts:848-874) discloses no such thing: its branches render only "install in the SCRATCH tree…" and "N dependencies linked in". The actual write-through warning exists only in the prose-exec prompt weld (agent-prompt.ts:1820), both standalone report notes (lines 720-723, 809) still state unqualified that "the review worktree stays read-only", and sharedTreeResidue excludes the pipeline footprint — so a write through a farm link is never measured either. Anyone auditing the standalone contract — or a future caller of --standalone outside the prose-exec brief — reads this JSDoc and believes the note carries the warning; a recipe step writing through a farm link (patching a dependency, npm rebuild, a postinstall — no knowledge of any other path needed) then mutates the shared worktree every other agent is reading while the report handed to the executor certified it read-only.
Put the disclosure where the doc says it is — add to dependencyNote()'s linked-case text that writes through a link land in the review worktree; or, if the disclosure deliberately lives in the brief, correct the field doc to point there:
| * knows which of the two contracts the note is stating. (The dependency | |
| * farm's links point back at the review worktree; writes THROUGH them are | |
| * the exception the dependency note discloses.) | |
| * knows which of the two contracts the note is stating. (The dependency | |
| * farm's links point back at the review worktree; writes THROUGH them are | |
| * the exception the prose-exec brief's "farm is borrowed" paragraph states.) |
Fix witness: with a farm standing in the review worktree, assert the standalone report's note discloses that writes through the linked dependencies land in the review worktree; deleting the added text makes the test red.
中文说明
[Suggestion] R1-19：（fix-induced，由 R1-19 的修复引入）本轮对 R1-19 的修复把被证伪的保证收窄到 git 状态——方向正确——但新增的括注声称"穿透它们的写入是依赖备注所披露的例外"，而 dependencyNote()（scratch-tree.ts:848-874）根本没有这类披露：它的分支只渲染"在 SCRATCH 树里安装……"和"已链入 N 个依赖"。真正的穿透写警告只存在于 prose-exec 的提示词焊点（agent-prompt.ts:1820），两条 standalone 报告备注（720-723、809 行）仍无条件地声称"review worktree 保持只读"，且 sharedTreeResidue 排除流水线足迹——因此穿透农场链接的写入也不会被测量。任何审计 standalone 契约的人——或 prose-exec brief 之外未来调用 --standalone 的使用方——读到这段 JSDoc 都会以为备注已带警告；随后一个穿透农场链接写入的 recipe 步骤（给依赖打补丁、npm rebuild、postinstall——不需要知道任何其他路径）就会在其他所有 agent 正在读取的共享 worktree 里造成改动，而交给执行者的报告却认证它只读。
把披露放到文档所说的位置——在 dependencyNote() 的 linked 分支文案中说明穿透链接的写入会落在 review worktree；若披露有意放在 brief 里，则把字段注释改为指向那里（如上 suggestion）。
修复见证：在 review worktree 有农场的情况下，断言 standalone 报告的 note 披露穿透链入依赖的写入会落在 review worktree；删除新增文案后测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Pin the clone's remote name with `--origin origin` so a user's global `clone.defaultRemoteName` cannot wedge the standalone build after the isolation already succeeded. Restore the content-filter screen for the standalone shape — the shared-worktree residue measurement runs a `git status` that executes clean filters on stat-stale files even though the clone's own checkout cannot — and state that in the refusal. Disclose in the dependency note that writes through a farm link land in the review worktree, where the `standalone` field doc says the disclosure lives.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round 3 summary — PR #10221Commit: Feedback points and dispositions[rc:3871840058] R2-1 — Critical — [rc:3871840069] R2-2 — Critical — the standalone filter-screen skip let the shared-worktree residue measurement execute planted [rc:3871840099] R1-19 — Suggestion — the [rv:5040944062] Review body — no separate action. The 14 listed Suggestion-level findings are confirmed duplicates of round-1 items already addressed in Conflict notesNone — VerificationCommands actually run, in order:
中文说明第 3 轮总结 — PR #10221提交： 反馈点及处置[rc:3871840058] R2-1 — Critical — [rc:3871840069] R2-2 — Critical — standalone 的 filter 筛查跳过使共享 worktree 的残留下测量得以执行植入的 [rc:3871840099] R1-19 — Suggestion — [rv:5040944062] 评审主体 — 无需单独处理。 其中列出的 14 条 Suggestion 级发现被确认为第 1 轮条目的重复，已在 冲突说明无 — 验证实际执行的命令（按顺序）：
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.
Test Plan (not a blocker): review/SKILL.test.ts — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/scratch-tree.ts:697 — [review] raw path interpolation in two refusal notespackages/cli/src/commands/review/scratch-tree.test.ts:1096 — [review] standalone describe duplicates the first describe's fixture verbatim
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/cli/src/commands/review/scratch-tree.ts (findings in rounds 1, 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
Test Plan（非阻断）：review/SKILL.test.ts — no such file or directory。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/scratch-tree.ts（第 1、2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // commands (see worktreeResidue). A standalone clone's checkout reads the | ||
| // CLONE's config, which holds none of these — but the shared measurement | ||
| // it still runs can execute them, so the screen covers both shapes. | ||
| const filters = localFilterCommands(worktree); |
There was a problem hiding this comment.
[Critical] R3-1: This round's fix makes the filter screen unconditional and load-bearing for EVERY shape — the standalone shape it introduces has no other protection against the shared-worktree measurement, and the rewritten refusal claims completeness ("until then no scratch tree is safe to create or reset") — but the scan still queries each config file with git config --file <file> --get-regexp, which defaults --includes OFF, so [include] path = … / [includeIf] directives inside the scanned files are never resolved. The measurement the screen authorizes — worktreeResidue's git status — reads MERGED config (includes resolved) and executes filter.*.clean on stat-stale tracked files. A planter with write access to the common dir — the exact actor this screen's docstring names; a verifier probe in a linked scratch tree is one such — writes [include] path = <file> into the common config and puts filter.evil.clean = <cmd> in <file>, with selection via committed .gitattributes: localFilterCommands sees no filter key, certifies the repository, and the measurement executes the payload while answering sharedTreeResidue: [] — certifying clean the same tree it just executed in, in both shapes.
Witness (probe, vitest in scratch tree, production call shape, git 2.43):
PR code, linked shape -> {"available":true,"pwnedExists":true,"sharedTreeResidue":[]}
PR code, standalone shape -> {"available":true,"standalone":true,"pwnedExists":true}
direct-key control -> {"available":false,"pwnedExists":false} (probe discriminates)
scan + '--includes' -> both arms flip to {"available":false,"pwnedExists":false}
The includeIf.gitdir: arm was verified separately: it resolves in merged config from a linked worktree while the per-file scan misses it. The scan's documented rationale (--file keeps the user's GLOBAL filters, e.g. git-lfs, out of scope) does not answer for include directives INSIDE the scanned local files; the fix below preserves that property — global config is still never scanned.
Fix: in localFilterCommands (scratch-tree.ts:211), fail closed on the redirect itself — alongside the existing --get-regexp '^filter\..*\.(smudge|clean)$', also match ^include\. / ^includeIf\. keys in each scanned file and return them as a refusal (an include directive in a local config file redirects the very surface being screened; --includes alone would still leave includeIf.gitdir: conditions context-dependent), and adjust the refusal text accordingly. Verified in scratch tree: adding --includes to the scan makes both shapes refuse before measurement while global config stays unscanned.
Fix witness: a test in both shapes that appends [include] path=<file> to the repo config plus a filter file, with selecting attributes and a stat-stale tracked file, asserting available: false naming the include and no payload file created — red against HEAD as it stands (the probe shows the screen passes and the measurement executes the payload); removing the new include screen makes it red again.
中文说明
[Critical] R3-1：本轮修复让 filter 筛查无条件运行、对所有形态承重生——新增的 standalone 形态对共享 worktree 测量只有这一层保护，改写后的拒绝文案还宣称了完备性（"until then no scratch tree is safe to create or reset"）——但筛查仍用 git config --file <file> --get-regexp 逐文件查询，该形态默认不解析 include，于是被扫描文件里的 [include] path = … / [includeIf] 指令指向的过滤器筛查完全看不见。而筛查所授权的测量——worktreeResidue 的 git status——读的是合并后配置（会解析 include），并会对 stat 过期的被跟踪文件执行 filter.*.clean。对 common dir 有写权限的植入者（正是本筛查 docstring 点名的角色；linked scratch tree 里的 verifier 探针就是其一）把 [include] path = <file> 写进 common config、把 filter.evil.clean = <cmd> 放进 <file>、再用已提交的 .gitattributes 选中它：localFilterCommands 看不到任何 filter 键、认证仓库干净，测量却在回答 sharedTreeResidue: [] 的同时执行了载荷——在它刚刚执行过载荷的同一棵树上认证其干净，两种形态皆然。
见证（探针，scratch tree 内 vitest，生产调用形态，git 2.43）：PR 代码 linked 形态 available=true、载荷文件被创建、sharedTreeResidue=[]；standalone 形态同样 available=true、载荷被创建；直接键对照 available=false、无载荷（探针可区分）；给扫描加 --includes 后两形态都翻转为 available=false、无载荷。includeIf.gitdir: 一支单独验证：从 linked worktree 出发合并配置能解析、逐文件扫描漏掉。扫描选用 --file 的既有理由（让用户全局过滤器如 git-lfs 不进范围）不能为被扫描本地文件内部的 include 指令背书；下面的修复保留该性质——全局配置依旧不被扫描。
修复：在 localFilterCommands（scratch-tree.ts:211）内对重定向本身 fail-closed——在现有 --get-regexp '^filter\..*\.(smudge|clean)$' 之外，同时匹配每个被扫描文件中的 ^include\. / ^includeIf\. 键并将其作为拒绝返回（本地配置文件里的 include 指令重定向的正是被筛查的这个面；只加 --includes 仍会让 includeIf.gitdir: 条件依赖上下文），并相应调整拒绝文案。已在 scratch tree 验证：给扫描加 --includes 后两种形态都会在测量前拒绝，且全局配置仍不被扫描。
修复见证：两种形态各加一个测试——向仓库配置追加 [include] path=<file> 与过滤器文件、配选中 attributes 与 stat 过期文件，断言 available: false 点名 include 且无载荷文件——对当前 HEAD 为红（探针已证筛查放行、测量执行载荷）；移除新增的 include 筛查后再次变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
4 participants
What this PR does
Adds the two remaining lenses from the #9655 post-mortem (#9707, proposals 3 and 4) to the review roster. Replaces #9717 — same two lenses, same briefs and roster gates (every one of that PR's 74 review threads is mapped to where it landed in a comment there); what changed is where the prose-execution audit's writes go, and that change is a subtraction: 13 files, +1340/−68 against #9717's +2595/−120.
prose-exec, the prose-execution audit. When the diff touches an instruction file — aSKILL.md, a root guidance file (AGENTS.md/CLAUDE.md/QWEN.md/GEMINI.md,copilot-instructions.md), an agent or slash-command definition under.claude/or.qwen/, aprompts/file,.qwen/review-rules.md, or a prompt/brief-named source — one agent EXECUTES the changed instructions instead of reading them: it stands up the smallest honest scenario, follows the instructions literally with no charity on ambiguity, and files the divergence between the executed outcome and what the prose promises, with the run's output as the witness. It runs in both topologies and at every effort, gated only on having a tree; the trigger is a built-in path predicate (thehasExecutableScriptprecedent) rather than a committed manifest rule, so it ships with the CLI instead of fail-closing every review until the new CLI is deployed, and the role joinsREPOSITORY_CONTEXT_ROLESso a manifest can require it back where the predicate misses. The recipe text is untrusted input: egress, credential reads, writes outside the copy and PR-committed symlinks that resolve outside it are quoted as findings, never executed; installs run with--ignore-scriptswherever the recipe's goal survives it.6d, the counter-frame audit. The reviewer the author's narrative cannot steer. It reads the PR context once, for exactly two extractions: the description's nominated topics, which become its EXCLUSION list, and the motivating incident, which it replays step by step the day after the merge — no step differing is a Critical. It carries the personas' effort gate (skipped at medium) but is whole-diff in BOTH topologies, and — like Agent 0 — is gated on the PR identity: the brief builder welds the context pointer and throws on a plan without it; a context file that cannot be read degrades to an unperformable return that still opens its assigned diff ranges.qwen review scratch-tree --standalonestands the tree up withgit clone --shared --no-checkout --template=and a detached checkout at the reviewed head: a.gitof its own, sharing only the object store through an alternates pointer, no template hooks,originremoved, rebuilt from scratch on every call. Agit config, hook or ref write a recipe step makes lands in that tree and dies with it — there is no path from it to the user's repository. The verifier's linked scratch worktree (/review: verification probes mutate the shared worktree while reverse auditors read it #9207) is untouched: the flag is opt-in and only prose-exec's weld passes it.Why the copy changed shape: in #9717 prose-exec shared the verifier's LINKED worktree, whose
.gitpoints into the user's repository. Because the agent executes PR-authored text, nine review rounds grew an 866-line fail-closed screen over that shared common dir — an allowlist of git config keys certified inert, an executable-hooks scan, submodule-gitdir and nested-worktree enumeration, FIFO gates, timeouts — and every fix produced a sibling finding, for three structural reasons the last rounds themselves named: the surface is git-defined and grows across versions (gc.recentObjectsHook,<helper>::dispatch,core.worktree); the screen's own inputs — the worktree admincommondir/gitdirfiles, the registered-worktree list — live on the very surface an attacker writes, so its decoy gate was circular; and it refused the repo-local state the pipeline's own CI checkout writes (core.hooksPath=.husky/_,includeIf.gitdir:credential includes), so the isolation could never stand up where it was deployed. A tree with nothing shared needs no screen, so the screen is gone (scratch-tree.tsand its tests are back tomain's shape plus the standalone path).Also folded in from #9717's last round:
hasPromptFilesnow fails open on a plan with nofiles[](a plan an older CLI wrote), likehasDeletions(R20-4); the brief's symlink rule is scoped to COMMITTED symlinks, naming the dependency farm'snode_moduleslinks as the sanctioned read path (R20-8); the--rulestail is excluded for prose-exec as it is for Agent 7 (D20-2); a step quoted instead of run because it falls in a never-execute class is rated Critical regardless of the rest of the recipe (D20-3); and the SKILL.md narration is synced (Sixteen agents,where the run owes it).Why it's needed
The measured motive is PR #9655, whose one blocking defect escaped four
/reviewrounds, a three-stage triage pass, and an autofix takeover, and was found by a human eleven minutes after the final automated LGTM. Issue #9707's post-mortem traced that to two structural gaps the brief edits in #9708 cannot close: attention capture by the author's own framing (every one of the four rounds' twenty-five findings landed inside the four decisions the PR body nominated), and prose reviewed by reading when its defects only fall out of execution (#9655 produced two of these in one PR: capture guidance that authorised a misattribution, and a fix recipe whose verbatim execution yieldscaptured: null).The standalone clone is needed because the alternative provably does not converge: #9717's screen was patched in rounds 12 through 20 and the twentieth round still found three new entrances plus a circular gate, while the pipeline's own runners were being refused. Isolation by construction — nothing shared — is the only shape where a new git config key cannot be a new finding.
Reviewer Test Plan
How to verify
scratch-tree.test.tskeepsmain's 46 cases unchanged and adds 11 underrunScratchTree --standalone, against a real git repo: the clone's.gitis a directory whoseobjects/info/alternatesnames the repository's object store, with no hooks dir, no remote and no entry ingit worktree list; acore.editor, an executablepre-commitand a commit planted from inside the tree leave the user's config, hooks dir andrev-list --alluntouched; a repository whose repo-local config defines a smudge filter is REFUSED by the linked shape and served by the standalone one without the filter running; every call rebuilds (probe file, mutated source, planted config andnode_modulesresidue all gone); a linked worktree an earlier call left at the same path is unregistered rather thanrmSync'd over; a symlink at the path is replaced without being followed; a symlinked ancestor is refused; a clone that cannot be made is reported with the residue still measured; the farm links in; the review worktree's residue is still measured; and--standaloneparses through the real yargs builder into the fieldrunScratchTreereads.Mutation matrix on the standalone path, each mutant run against the standalone describe (11 tests): drop
--template=→ 1 failed; dropremote remove origin→ 1 failed; alwaysrmSync(skipdiscardWorktreefor a linked leftover) → 1 failed; drop the ancestor-symlink guard → 1 failed; run the filter screen in standalone mode → 1 failed. Source restored byte-identical afterwards (cmp).Live witness of the clone semantics (git 2.55.0, isolated
HOME/GIT_CONFIG_GLOBAL, a repo carryingfilter.evil.smudge,core.editor=evil-editorand an executablepost-checkouthook, with a linked review worktree beside it):The 74 review threads on #9717 were re-verified against that branch's head (
fcf19bfa9d) before this branch was cut: every finding outside the scratch-tree screen is fixed there and carried here verbatim; the screen findings are superseded by the removal; the two still open at that head (R20-4, R20-8) are fixed here. The per-thread table is on #9717.Evidence (Before & After)
N/A — roster and prompt text plus an opt-in flag on a review subcommand; no user-visible or TUI surface.
Tested on
Environment (optional)
Local npm workspaces, Node v24.18.1, git 2.55.0; vitest via
npx vitest runinsidepackages/cli/packages/core.Risk & Scope
--sharedclones depend on the source object store for the life of the tree, which is one review; the reviewed head stays reachable from the review worktree's HEAD throughout.main's shape and its shared common dir — that is /review: verification probes mutate the shared worktree while reverse auditors read it #9207's design and not this PR's to change. The pr-context temp-and-rename write from feat(review): add the prose-execution audit and the counter-frame audit #9717's deferred list (D20-4) was tried and dropped: 22 handler-level tests mockwriteFileSyncby path, so it is its own change. A.qwen/review-context.jsonrule requiringprose-execfor this repository's own prompt files is a follow-up after this CLI ships.scratch-treegains an opt-in--standaloneflag and astandalonefield in its report; prompt-file diffs and high-effort reviews launch more agents than before;check-coveragerequires the new roles exactly where the roster computes them.Linked Issues
Refs #9707 — implements proposals 3 and 4; with #9708, completes the issue's four proposals. Replaces #9717.
中文说明
本 PR 做了什么
把 #9655 复盘（#9707 提案 3、4）剩下的两个审查视角加入 review roster。替代 #9717——同样的两个视角、同样的 brief 与 roster 门控（那个 PR 的 74 条评审线程逐条映射到了各自的去向，见其上的评论）；变的是提示词执行审计的写入去哪儿，而且这个改动是做减法：13 个文件 +1340/−68，对比 #9717 的 +2595/−120。
prose-exec，提示词执行审计。 当 diff 触及指令文件——SKILL.md、根目录指导文件（AGENTS.md/CLAUDE.md/QWEN.md/GEMINI.md、copilot-instructions.md）、.claude/或.qwen/下的 agent 或 slash-command 定义、prompts/目录下的文件、.qwen/review-rules.md、或以 prompt/brief 命名的源码——由一个 agent 去执行变更后的指令而不是阅读：搭起指令所针对的最小诚实场景，按字面逐步执行、对歧义不做善意解读，并以运行输出为证据，报告执行结果与文字承诺之间的偏差。它在两种拓扑、所有 effort 下都运行，只要求有工作树；触发条件是内置的路径谓词（沿用hasExecutableScript的先例）而非提交进仓库的 manifest 规则，这样它随 CLI 一起发布，而不会在新 CLI 部署前让每次评审 fail-close；该角色同时加入REPOSITORY_CONTEXT_ROLES，谓词漏掉的仓库可通过 manifest 要求它回来。要执行的文本是不可信输入：网络外发、读取凭据、写到副本之外、以及 PR 提交的解析到副本外的符号链接，一律只引用为发现、绝不执行；安装只要不影响 recipe 目标就带--ignore-scripts。6d，反框架审计。 作者叙事无法引导的审查者。它只读一次 PR 上下文，做恰好两个提取：描述里提名的话题——这是它的排除清单——以及叙述中的诱因事故，它逐步重放"合并次日事故原样重现"，若没有任何一步的结果不同即为 Critical。它沿用人格 agent 的 effort 门（medium 跳过），但在两种拓扑下都是全 diff 的；并且像 Agent 0 一样按 PR 身份门控：brief 构建器焊入上下文指针，计划缺身份则抛错；上下文文件读不到时降级为"无法执行"的返回，但仍打开被分派的 diff 范围。qwen review scratch-tree --standalone用git clone --shared --no-checkout --template=加一次在被审 head 上的 detached checkout 把树搭起来：自己的.git，只通过 alternates 指针共享对象库，没有模板 hooks，origin被移除，每次调用从头重建。recipe 步骤写下的git config、hook 或 ref 落在这棵树里并随之消亡——从它到用户仓库没有任何路径。verifier 的 linked scratch worktree（/review: verification probes mutate the shared worktree while reverse auditors read it #9207）原样不动：该 flag 是可选的，只有 prose-exec 的 weld 传它。副本形态为何改变：在 #9717 里 prose-exec 与 verifier 共用 linked worktree，其
.git指向用户自己的仓库。由于这个 agent 执行 PR 作者写的文本，九轮评审在那个共享 common dir 上堆出了一个 866 行的 fail-closed 筛查——被认证为惰性的 git config 键白名单、可执行 hooks 扫描、submodule gitdir 与嵌套 worktree 枚举、FIFO 门、超时——而每一次修复都生出一个兄弟发现，原因是最后几轮自己点出的三条结构性事实：筛查面由 git 定义且随版本增长（gc.recentObjectsHook、<helper>::分派、core.worktree）；筛查自身的输入——worktree 管理文件commondir/gitdir、已注册 worktree 列表——就躺在攻击者可写的那个面上，所以其诱饵门是循环论证；它还拒绝流水线自己的 CI checkout 写下的仓库本地状态（core.hooksPath=.husky/_、includeIf.gitdir:凭据 include），于是隔离在其部署环境里根本立不起来。一棵什么都不共享的树不需要筛查，所以筛查删掉了（scratch-tree.ts及其测试回到main的形态，只加独立克隆路径）。同时收进 #9717 最后一轮的几项：
hasPromptFiles在计划没有files[]（旧 CLI 写的计划）时像hasDeletions一样 fail-open（R20-4）；brief 的符号链接规则限定为已提交的符号链接，并把依赖农场的node_modules链接点名为许可的读取路径（R20-8）；--rules尾部像 Agent 7 一样对 prose-exec 排除（D20-2）；因落入禁止执行类而只引用未运行的步骤，不论 recipe 其余部分如何一律评 Critical（D20-3）；SKILL.md 叙述同步（Sixteen agents、where the run owes it）。为什么需要
实测动机是 PR #9655：其唯一的阻塞缺陷逃过四轮
/review、三阶段 triage 和一次 autofix takeover，在最终自动 LGTM 后十一分钟被人类发现。Issue #9707 的复盘把它归因于 #9708 的 brief 文本修改无法关闭的两个结构性缺口：被作者自身叙事俘获的注意力（四轮二十五条发现全部落在 PR 正文提名的四个决策之内），以及只有执行才会暴露缺陷、却只被阅读的提示词（#9655 一个 PR 里出了两处：授权了错误归因的抓取指引，以及逐字执行会得到captured: null的修复 recipe）。独立克隆之所以必要，是因为另一条路被证明不收敛：#9717 的筛查从第 12 轮补到第 20 轮，第二十轮仍找出三个新入口加一个循环门，同时流水线自己的 runner 还在被拒绝。构造上的隔离——什么都不共享——是唯一让"新的 git config 键"不可能成为"新的发现"的形态。
审查者测试计划
如何验证
scratch-tree.test.ts保留main的 46 个用例不变，在runScratchTree --standalone下新增 11 个，针对真实 git 仓库：克隆的.git是目录，其objects/info/alternates指向仓库对象库，没有 hooks 目录、没有 remote、不在git worktree list里；从树内植入的core.editor、可执行pre-commit和一次 commit 不触碰用户的 config、hooks 目录与rev-list --all；仓库本地 config 定义了 smudge filter 的仓库会被 linked 形态拒绝、被 standalone 形态正常服务且 filter 不运行；每次调用都重建（探针文件、被改的源码、植入的 config 和node_modules残留全部消失）；早先调用留在同一路径的 linked worktree 会被注销而不是被rmSync盖掉；路径上的符号链接被替换而不被跟随；祖先目录是符号链接则拒绝；克隆失败时照常报告且残留已测量；农场正常链入；review worktree 的残留仍被测量；--standalone经真实 yargs builder 解析到runScratchTree读取的字段。standalone 路径的变异矩阵，每个变异体只跑 standalone 的 describe（11 个用例）：去掉
--template=→ 1 失败；去掉remote remove origin→ 1 失败；一律rmSync（linked 残留不走discardWorktree）→ 1 失败；去掉祖先符号链接守卫 → 1 失败；standalone 模式也跑 filter 筛查 → 1 失败。事后源码cmp逐字节还原。克隆语义的活体见证（git 2.55.0，隔离的
HOME/GIT_CONFIG_GLOBAL，仓库带filter.evil.smudge、core.editor=evil-editor和可执行post-checkouthook，旁边有 linked review worktree）：见上文英文块中的实测输出——克隆 config 只有git clone自己写的六个键；在克隆内git config core.editor PLANTED后用户仓库仍是evil-editor；hook 与 PWNED 标记均未出现；从 worktree 对克隆执行git worktree remove --force报"不是 working tree"（exit 128）；克隆内 commit 不改变用户仓库的rev-list --all计数。#9717 的 74 条评审线程在切出本分支前已按该分支 head（
fcf19bfa9d）逐条复核：scratch-tree 筛查之外的每条发现都已在那边修好并原样带到这里；筛查类发现由删除取代；该 head 仍未修的两条（R20-4、R20-8）在这里修好。逐线程表在 #9717 上。证据（Before & After）
N/A —— roster 与 prompt 文本，外加一个 review 子命令的可选 flag；无用户可见或 TUI 表面。
测试平台
macOS ✅；Windows / Linux⚠️ （CI 覆盖）。
环境（可选）
本地 npm workspaces，Node v24.18.1，git 2.55.0；在
packages/cli/packages/core内用npx vitest run。风险与范围
--shared克隆在树的生命期内依赖源对象库，即一次评审；被审 head 在此期间始终可由 review worktree 的 HEAD 到达。main的形态和它的共享 common dir——那是 /review: verification probes mutate the shared worktree while reverse auditors read it #9207 的设计，不归本 PR 改。feat(review): add the prose-execution audit and the counter-frame audit #9717 deferred 清单里的 pr-context temp-and-rename 写入（D20-4）尝试后放弃：22 个 handler 级测试按路径 mockwriteFileSync，应单独成 PR。为本仓库自己的提示词文件在.qwen/review-context.json提交要求prose-exec的规则，是本 CLI 发布之后的 follow-up。scratch-tree新增可选--standaloneflag 与报告中的standalone字段；提示词文件 diff 与 high-effort 评审会比以前多启动 agent；check-coverage恰好在 roster 计算出的位置要求新角色。关联 Issues
Refs #9707 —— 实现提案 3 与 4；连同 #9708 完成该 issue 的全部四条提案。替代 #9717。