fix(review): repair seven pipeline defects found by live runs - #9175
Conversation
Four full reviews (PRs #9113, #9094, #9109, #9106) were run headless against qwen3.8-max and watched step by step. Every one of them reached a verdict, and every one of them also exposed a defect in the pipeline itself. Each fix below carries the measurement that found it. The incremental anchor was being withheld for a reason that says nothing about which lines were read. A dimension nobody can run — on this repo, the integration suite CI skips and the local budget cannot fit — capped the verdict, the cap withheld the anchor, and the missing anchor sent the next round over the full diff again: 119 minutes and 34M input tokens on a pull request whose code had not changed a line. The anchor now answers to the coverage evidence alone, recomputed from the harness's own transcripts, so a depth gap no longer costs a range. Two of the four runs had capped and two had not, on identical mechanical facts, which is the second reason this could not stay a judgment call. The build-and-test dimension could not finish, and no amount of reallocation would have made it. One shell call is capped at ten minutes by the tool itself; this repo needs more than that (install, the scoped builds, then a core suite at 106s and a CLI suite measured at 401s). Since the ceiling is per call, the run now continues across calls: a resumed call reuses the installed and compiled tree, runs only the suites the previous call could not reach, and merges into the same report. A suite killed on a deadline the budget had shortened is recorded as provisional rather than as a timeout, so the continuation knows to give it a full window instead of reproducing the kill. The per-command deadline is sized to the slowest measured suite, and the whole-call budget is now derived from the tool's ceiling rather than from the deadline. A machine ledger posted by another account was invisible, which turned off cross-round recovery in precisely the case it was built for: continuous integration posts as a bot and a maintainer runs as themselves. The two halves of that ledger are not the same claim — the findings are a work list every round re-rules against the code, while the reviewed-at commit decides which lines the next round skips — so the list now travels across accounts and the anchor never does. The attribution of test failures was reading a truncated record. The failing file set for the pull request side was re-parsed from a bounded report, which on a live run recovered one file out of eleven; the same set is now measured where the raw output still exists, before the bound is applied. The loss ran in the direction that matters: a file only the pull request side fails is exactly the one that should have been reported. Three smaller repairs round it out. A review launched from the bundle never learned which build it belonged to, so every helper command it shelled out to resolved whatever happened to be installed on the path — one live run died on its second command. A "nothing to disclose" answer written in Chinese was classified as a real gap and published as one, in a body whose own evidence said the opposite. And a count field surrounded by list fields was sent a list twice in four runs, so it now says what to send instead.
|
Re-run after fourteen review rounds — the gate now sees a different PR than the one it passed on 2026-08-14: the same seven defects, but the diff more than doubled as each round's answers hardened the seams. Re-checking from the top. Template: complete, with the Chinese translation ✓ Problem: observed, not theoretical — every defect carries a measurement from a live review run, and the description's before/after tables are data, not claims: the incremental anchor withheld because an unrunnable dimension was disclosed cost one re-review of an unchanged PR 119 minutes and ~34M input tokens (#9113 round 2); the CLI suite measured at 401s against a 300s per-command deadline was killed in all three runs where it was in scope; failing-file attribution recovered 1 of 11 files after output trimming; helper commands resolved to a stale global install; a Chinese "no gap" answer was published as a real gap. Direction: aligned — this repairs the repository's own review pipeline, the infrastructure every review on this repo (including this triage) runs through, and the defects were silently shaping every round's output. Size: ~2,111 production-logic lines vs ~2,809 test lines across 28 files (the first pass, at Approach: seven defects, seven repairs, and the batch hangs together — Risk: no high-risk path matches (Stage 1e). Moving on to code review. 🔍 中文说明这是第十四轮审查后的重跑——门禁现在看到的 PR 与 2026-08-14 通过时不同：还是同样七处缺陷，但每一轮的答复都在加固缝隙，diff 因此翻了一倍多。从头重新检查。 模板：完整，含中文翻译 ✓ 问题：已观测、非理论——每处缺陷都带着真实审查运行的测量值，描述中的前后对比表是数据而非口头声明：因披露了不可运行维度而被扣留的增量锚点，让一次对未改动 PR 的重审耗费 119 分钟和约 3400 万输入 token（#9113 第二轮）；CLI 套件实测 401 秒却只有 300 秒的单命令期限，凡进入范围三次均被杀；输出截断后失败文件归因 11 个只恢复出 1 个；辅助命令解析到旧的全局安装；中文的"无缺口"答复被当作真实缺口发布。 方向：对齐——修复的是本仓库自己的审查管线，即本仓库每一次审查（包括本次 triage）所依赖的基础设施，这些缺陷一直在悄悄影响每一轮的产出。 规模：28 个文件，生产逻辑约 2111 行、测试约 2809 行（首遍在 方案：七处缺陷、七处修复，且这批修复相互咬合—— 风险：无高风险路径命中（Stage 1e）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff I wrote down what I would do for each of the seven defects: gate the anchor on machine coverage evidence rather than the orchestrator's caps, continue a build-and-test run across calls with a resume flag that refuses anything half-built, recover the posted ledger from any account but never let a foreign one set the anchor, measure failing files before the output trim, stamp the running build's entry for helper commands, classify the Chinese no-gap answers, and name the expected type in the count error. Reading the diff against that baseline: every repair matches or exceeds the sketch, and the places it goes further are all where a live round found the naive version unsafe — run identity so a stale report from an interrupted round cannot be resumed, a grammar check on stored commands before they re-execute, a bounded headroom on foreign round numbers, and an anonymous-persistence guard so an identity outage cannot swap the certified work list. The properties I checked most closely, with what I found:
Non-blocking items, all flagged by the pipeline's round 14 or my own reuse check, none of them blocking:
sequenceDiagram
participant P1 as Round N review
participant P2 as Posted review body
participant P3 as recoverLedger
participant P4 as Next round
P1->>P2: embeds ledger marker with findings and reviewed-at sha
P4->>P3: recovers the latest marker from any author
P3-->>P4: own account, findings and anchor both travel
P3-->>P4: another account, findings travel and anchor is stripped
Files changed (28 of 28)
Testing — the PR's own CI, read via the API (per policy this review never runs PR code)CI on the reviewed commit is settled and green — both
What the suite cannot settle: that a second round of an unchanged PR now scopes incrementally end to end, and that a real 中文说明代码审查读 diff 之前我先写下自己对七处缺陷的修法：锚点只由机器覆盖证据把关而非编排器的封顶、build-and-test 跨调用续跑并拒绝半构建状态、从任何账号恢复已发布的账本但绝不允许外部标记设定锚点、在输出截断之前测量失败文件、为辅助命令盖上当前构建的入口、识别中文的"无缺口"答复、在计数错误里点名期望类型。对照这份基线读 diff：每处修复都与我的设想相当或更优，而所有超出之处，恰恰都是某轮实审查发现朴素版本不安全的地方——运行身份使中断轮次的陈旧报告无法被续跑、存储命令重执行前先过语法门禁、外部轮次号有 64 的头部空间上限、匿名持久化守卫确保身份故障不能换掉已认证的工作清单。 审查最仔细的几条性质及结论：
非阻断项（来自第 14 轮管线或我的复用检查，均不阻断）：
（流程图见英文部分：账本恢复在恢复缝隙处拆分信任面——同账号 findings 与锚点同行，跨账号只带 findings、剥离锚点。） 测试按政策，本审查从不运行 PR 代码；以下为经 API 读取的 PR 自身 CI。审查提交上的 CI 已落定且全绿——两个 套件无法落定的：未改动 PR 的第二轮现在是否端到端增量收口、真实 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean static review on every path I traced, CI settled green on the reviewed commit, and the pipeline's fourteenth round ended at suggestions-only; what stands between this and a 5 is one unforced-structure nit the tests cannot catch (the positional-boolean swap) and the loop-level outcome that only a sandboxed A/B can prove. Stepping back: the simpler fix I would have reached for in each spot — raise the trim cap, keep the own-account filter, derive the budget from the timeout, gate the anchor on the cap list — fails for a measured reason, and the code's comments say exactly which measurement kills it. That is still the difference between a repair and a patch, fourteen rounds in. Is every change necessary? I mapped the diff against the seven named defects and the hardening each round forced; there are no drive-by edits, and the batch's interactions (resume needs the clamped flag, the anchor needs the scope signal, the parser serves both delta sides) justify keeping it together despite the bisect granularity a batch this size costs. The anchor relaxation remains the change I scrutinized most, because it is the one that loosens a safety property. It loosens exactly the part that was prose — an unrunnable dimension's disclosure, a relayed stop entry matched to the character — and keeps every part that is evidence: unread chunks, idle or blind agents, a whiffed lens named in dimension prose, an absent machine signal read as unproven. The closed loop this PR measured at 119 minutes had no exit because the cap, the anchor, and the scope all answered to the same prose; now the anchor answers only to what the harness can show it read. My reservations, plainly: the two positional booleans at the persistence call site are the one place this diff's own discipline — validate at the seam, fail closed — was not applied to its own call shape, and the pipeline proved a swap compiles green; fix it before merge or take it as the very next commit. And the loop-level claims ride on the author's macOS measurements until the sandboxed verification lands — the lane is named in the Stage 2 comment. Verdict: approve, pinned to the reviewed commit. CI is settled ( 中文说明置信度：4/5 —— 我追踪过的每条路径静态审查干净，审查提交上的 CI 已落定全绿，管线第十四轮以"仅剩建议"收束；与 5 分之间的差距，一是测试抓不到的一处结构性瑕疵（位置布尔互换），二是只有沙箱 A/B 才能证明的环路级结果。 退一步看：我在每处缺陷上本来会选用的更简单修法——抬高截断上限、保留仅本账号过滤、让预算从期限派生、用封顶列表给锚点把关——都因某个实测原因走不通，而代码注释恰好写明了是哪个测量杀死了哪条路。十四轮之后，这仍然是"修复"与"打补丁"的区别。每处改动都必要吗？我把 diff 逐条对照七个已点名的缺陷以及每轮被迫补上的加固，没有顺手改动；这批修复之间的依赖（resume 需要 clamped 标志、锚点需要 scope 信号、解析器服务 delta 两侧）足以证明打包的正当性，尽管这个体量在回归二分时粒度偏粗。 锚点放宽依旧是我审查最仔细的改动，因为它放宽了一条安全性质。它放宽的恰恰是纯属文字的部分——不可运行维度的披露、按字符精确匹配的停止条目转述——而保留每一条属于证据的部分：未读分块、空转或未开 diff 的 agent、以维度文字出现的失手透镜、缺失的机器信号按未证明处理。本 PR 实测出 119 分钟的死循环之所以没有出口，是因为封顶、锚点与范围都听命于同一段文字；现在锚点只听命于工具能证明自己读过的东西。 我的保留意见，直说：持久化调用点的两个位置布尔，是这份 diff 唯一没有把自身纪律（在缝隙处校验、失败关闭）用在自己调用形态上的地方，且管线已证明互换实参可以编译全绿——要么合并前修掉，要么作为紧随其后的提交。另外，环路级结论在沙箱验证落定之前仍系于作者在 macOS 上的实测——通道已在 Stage 2 评论中点名。 结论：批准，并与所审提交绑定。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. |
…rt from one that finished A run that never reached its test phase — a failed install, the disk-space gate, a budget spent during the build, a deliberate build-only probe — carries neither a test scope nor a test result, so a continuation finds nothing to do and said the run had reached every suite in scope. That is the same shape this branch fixes elsewhere: prose asserting the opposite of the evidence beside it. It now says no suite ran and names the fix, and a test pins the two facts apart.
commented
Aug 14, 2026
|
Addressed the stage-2 observation in 7f73fe8. A continuation that finds nothing to do now distinguishes the two opposite facts that reach that branch. A run which ended before its test phase — a failed install, the disk-space gate, a budget spent during the build, or a deliberate build-only probe — carries neither a test scope nor a test result, and a continuation cannot manufacture one, because the scope it would run is computed by the phase that never happened. It now says that no suite ran and names the fix, instead of reporting that every suite in scope was reached. A test pins the two apart. Worth saying why this was worth a commit rather than a note: it is the same defect this branch fixes elsewhere, in English rather than Chinese — prose asserting the opposite of the evidence sitting beside it. 已按 stage-2 的观察修复（7f73fe8056）。 找不到可续跑工作时，现在会区分抵达该分支的两个相反事实。一个在测试阶段之前就结束的运行——安装失败、磁盘空间门、预算在构建阶段耗尽、或刻意的仅构建探针——既没有测试范围也没有测试结果，而续跑无法凭空造出一个，因为它要运行的范围本该由那个从未发生的阶段计算得出。现在它会说明没有任何套件运行过并指出修法，而不是报告范围内每个套件都已抵达。新增一条测试把这两个事实钉开。 值得说明为什么这值得一个提交而不是一条备注：这正是本分支在别处修复的同一个缺陷，只不过是英文版而非中文版——正文断言的内容与紧挨着它的证据相反。 |
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent 1b": none — all checks I started completed; no check was left unfinished.; "agent reverse-audit (round 1)": vitest's ❯-line wording for collection-error files (whether "error" vs "failed" could slip past the ❯ filter) — could not verify: node_modules absent in this …; chunk 6: did not run the unit tests for build-test.test.ts / pr-context.test.ts (static verification only).; chunk 6: did not execute npm-toolchain / pr-context unit tests — verification was static.; "agent reverse-audit (round 1)": did not trace run.ts / round-2 orchestrator consumption of the recovered anchor end-to-end (the per-round fencing of layerAuditGate was verified directly inst…, and 2 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent 1b"：none — all checks I started completed; no check was left unfinished.；"agent reverse-audit (round 1)"：vitest's ❯-line wording for collection-error files (whether "error" vs "failed" could slip past the ❯ filter) — could not verify: node_modules absent in this …；chunk 6：did not run the unit tests for build-test.test.ts / pr-context.test.ts (static verification only).；chunk 6：did not execute npm-toolchain / pr-context unit tests — verification was static.；"agent reverse-audit (round 1)"：did not trace run.ts / round-2 orchestrator consumption of the recovered anchor end-to-end (the per-round fencing of layerAuditGate was verified directly inst…，另有 2 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| `|\\s*[-—–、,，]\\s*(?:(?!${ZH_EXCEPTION}).){0,40}` + | ||
| `(?<!未)(?<!没)(?<!无法)${ZH_COMPLETION}` + |
There was a problem hiding this comment.
[Critical] R1-1: The Chinese placeholder classifier drops REAL budget-gap disclosures — three mechanisms, all probed against the real module:
- The negation lookbehinds only see the character adjacent to the completion word, so
没有完成passes them (the char before完成is有):Budget gap: 无 — 检查还没有完成is dropped. - The post-completion
{0,40}span blocks only但/除exception words and swallows any trailing gap clause:无 — 安全检查完成，渗透测试未进行is dropped. - Unlike the English branch it claims to mirror ("Shapes are the English branch's, narrowed the same way"), the ZH branch has no all-done head requirement, so a gap clause BEFORE the completion is swallowed too:
无 — 3 项未运行，其余完成,无 — 渗透测试失败，单元测试完成are dropped.
Failure scenario: under Chinese output language, an agent's real disclosure is classified as a no-answer placeholder; budgetGapDisclosures returns []; compose-review applies no cap and no disclosure — the round certifies depth that was never reached. The English translations of the same sentences survive the English branch. Probes: all 8 sentences dropped on the unmodified PR; reverting the ZH branch restores them; adding an all-done head fixes mechanism 3 with the existing suite still green.
Suggested fix: rewrite the ZH dash branch to mirror the English narrowing — require an all-done head before span 1 (e.g. 所有|全部|一切|均), extend the lookbehinds with (?<!没有), and restrict the post-completion span to the budget-adverbial vocabulary; add keep-tests for the sentences above.
中文说明
中文占位符分类器会丢弃真实的预算缺口披露——三种机制，均已在真实模块上实测验证：
- 否定向后断言只能看到完成词紧邻的前一个字符，因此
没有完成能通过断言（完成前一个字是有）：Budget gap: 无 — 检查还没有完成被丢弃。 - 完成词后的
{0,40}区间只排除但/除例外词，会吞掉任何尾随的缺口从句：无 — 安全检查完成，渗透测试未进行被丢弃。 - 与其声称要镜像的英文分支（"Shapes are the English branch's, narrowed the same way"）不同，中文分支没有"全部完成"头部要求，因此完成词之前的缺口从句也会被吞掉：
无 — 3 项未运行，其余完成、无 — 渗透测试失败，单元测试完成被丢弃。
失败场景：在中文输出语言下，agent 的真实披露被判定为"无内容"占位符；budgetGapDisclosures 返回 []；compose-review 不加封顶也不披露——该轮审查会为从未达到的深度背书。同样句子的英文翻译都能通过英文分支。探针验证：未改动的 PR 上 8 个句子全部被丢弃；回退中文分支后全部恢复；为机制 3 加上"全部完成"头部后现有测试套件仍然全绿。
修复建议：重写中文破折号分支以镜像英文分支的收窄——在 span 1 前要求"全部完成"头部（如 所有|全部|一切|均），为向后断言增加 (?<!没有)，并把完成词后的区间限制为预算副词词汇表；为上述句子补充保留测试。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } else if (failures.length === 0 && timedOut.length === 0) { | ||
| parts.push('Every suite in scope has now run, and everything passed.'); | ||
| } else { | ||
| parts.push('Every suite in scope has now run.'); | ||
| } |
There was a problem hiding this comment.
[Critical] R1-2: Resume completion accounting ignores still-clamped entries, and the budget-break drops unattempted retry commands from stillPending — the merged note declares "Every suite in scope has now run." while provisional results remain.
Failure scenario (probed with the exact defaults, 570s budget / 540s deadline): a report with two clamped suites resumes — the first retry passes at a full deadline (401s), the second is admitted at min(540, 169) = 169s, killed, and re-clamped; the note still says "Every suite in scope has now run." and testScope.notRun is absent. With three clamped suites the third hits the budget break; the .filter((d): d is string => d !== null) at the break discards it because retry work items carry dir: null, so it lands in neither stillPending nor notRun, and the caveat miscounts ("ran the remaining 2 command(s)" — 3 were left). An agent trusting the prose (this function's own comment: "an agent trusts the prose it is handed") skips the next resume; a suite the PR broke keeps a provisional timeout as its final verdict. No test covers re-clamp-on-resume or the break-with-retry path.
Suggested fix: after the merge, compute stillClamped = mergedTest.filter((t) => t.clamped); when non-empty, name them as still provisional in the note instead of asserting completion, and recover the dirs of unattempted retries at the break so they land in stillPending/notRun.
中文说明
续跑的完成度统计忽略了仍处于 clamped 状态的条目，且预算中断时会把未执行的 retry 命令从 stillPending 中丢掉——合并后的 note 宣称 "Every suite in scope has now run."，而临时结果仍然挂着。
失败场景（以精确默认值实测：570s 预算 / 540s 期限）：两个 clamped 套件的报告续跑——第一个 retry 以完整期限通过（401s），第二个以 min(540, 169) = 169s 被接纳、被杀、再次标记 clamped；note 仍写着 "Every suite in scope has now run." 且 testScope.notRun 缺失。三个 clamped 套件时，第三个撞上预算中断；中断处的 .filter((d): d is string => d !== null) 把它丢掉，因为 retry 工作项的 dir: null——它既不在 stillPending 也不在 notRun，caveat 计数也错（"ran the remaining 2 command(s)"——实际剩 3 个）。信任这段文字的 agent（本函数自己的注释："an agent trusts the prose it is handed"）会跳过下一次续跑；被 PR 破坏的套件最终以临时超时作为结论。没有任何测试覆盖续跑再-clamp 或带 retry 的预算中断路径。
修复建议：合并后计算 stillClamped = mergedTest.filter((t) => t.clamped)；非空时在 note 中点名它们仍是临时结果而不是宣称完成；并在预算中断处恢复未执行 retry 的目录，使其进入 stillPending/notRun。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // agent trusts the prose it is handed. The structured fields (`install`, | ||
| // `build`, `notBuilt`, `testScope`) carry everything the old note | ||
| // summarised; this one describes the run as it now stands. | ||
| note: resumedNote(previous, mergedTest, timedOut, stillPending), |
There was a problem hiding this comment.
[Critical] R1-3: The resume merge path replaces previous.note wholesale, dropping the frameInstallFailure() framing that fresh runs prepend when the install exited non-zero but left a usable tree — the exact case the framing was built for.
Failure scenario (probed): a previous report with install.exitCode: 1 + the framing directive ("report it as informational, never as a Critical, and never against this PR") + one notRun suite resumes and completes the suite; the merged note reads "Continued from a previous build-test call … Every suite in scope has now run, and everything passed." — the directive is gone while install.exitCode: 1 sits beside the silent note. resumedNote() never reads previous.install, and frameInstallFailure is unreachable from resume (runNpmToolchain returns to resumeNpmToolchain first). The fresh path applies that framing on every return precisely because, per its own comment, the structured fields alone were insufficient — an agent can now file the pre-existing install failure as an additional Critical against the PR.
Suggested fix: when previous.install && previous.install.exitCode !== 0, re-prepend the same framing in the merged note (or carry it through the merge as the early returns do via withNote).
中文说明
续跑合并路径整体替换了 previous.note，丢掉了全新运行在"安装非零退出但留下可用树"时会前置的 frameInstallFailure() 措辞——正是这套措辞被造出来要处理的场景。
失败场景（已实测）：前一份报告带 install.exitCode: 1 + 框定性指令（"report it as informational, never as a Critical, and never against this PR"）+ 一个 notRun 套件；续跑完成该套件后，合并 note 变为 "Continued from a previous build-test call … Every suite in scope has now run, and everything passed."——指令消失了，而 install.exitCode: 1 就挂在沉默的 note 旁边。resumedNote() 从不读取 previous.install，且 frameInstallFailure 在续跑路径上不可达（runNpmToolchain 会先转入 resumeNpmToolchain）。全新路径之所以在每次返回时都加上这段措辞，正是因为它自己的注释说明：仅有结构化字段是不够的——现在 agent 可能把这个既有的安装失败作为额外的 Critical 记到 PR 头上。
修复建议：当 previous.install && previous.install.exitCode !== 0 时，在合并 note 中重新前置同样的框定措辞（或像早退路径经 withNote 那样把它带过合并）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| 'write). A suite the budget cannot give a full deadline is left to ' + | ||
| 'notRun rather than started and killed, and --resume continues it in ' + |
There was a problem hiding this comment.
[Suggestion] R1-4: The rewritten --budget help text states the opposite of the shipped admission behavior: "A suite the budget cannot give a full deadline is left to notRun rather than started and killed".
Failure scenario: the implementation does the opposite — deadline = Math.min(perCommandMs, remaining) then exec unconditionally; killed suites are tagged clamped; only remaining < BUDGET_MIN_ATTEMPT_MS (15s) goes to notRun. Contradicted inside this same PR by: the evidence table ("CLI suite, first call | killed at 125s, recorded as provisional"), the test named 'marks a suite killed on a BUDGET-shortened deadline as clamped', the fresh-path comment "A suite still gets whatever remains", and the sibling --resume describe ("plus any it started with a budget-shortened deadline and killed"). An operator sizing --budget or triaging a clamped entry is misled by the CLI's own documentation about the central mechanism of this fix.
Suggested fix: reword to match the code — a suite the budget cannot give a full deadline is still attempted with what remains; a kill at that boundary is recorded as clamped (provisional, not "too slow"); only suites the budget cannot attempt at all are named notRun.
中文说明
重写后的 --budget 帮助文本与实际接纳行为相反："A suite the budget cannot give a full deadline is left to notRun rather than started and killed"（预算给不出完整期限的套件会被留到 notRun，而不是启动后被杀）。
失败场景：实现恰恰相反——deadline = Math.min(perCommandMs, remaining) 之后无条件 exec；被杀的套件标记 clamped；只有 remaining < BUDGET_MIN_ATTEMPT_MS（15s）才进入 notRun。本 PR 内部就有多处矛盾：证据表（"CLI suite, first call | killed at 125s, recorded as provisional"）、名为 'marks a suite killed on a BUDGET-shortened deadline as clamped' 的测试、全新路径注释 "A suite still gets whatever remains"、以及相邻的 --resume 描述（"plus any it started with a budget-shortened deadline and killed"）。设置 --budget 或排查 clamped 条目的使用者会被 CLI 自己的文档误导——而说的正是本修复的核心机制。
修复建议：按代码改写——预算给不出完整期限的套件仍会以剩余时间尝试；在该边界被杀记为 clamped（临时结果，不是"太慢"）；只有预算完全无法尝试的套件才列入 notRun。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(p).toContain('--resume'); | ||
| expect(p).toContain('testScope.notRun'); | ||
| expect(p).toContain('"clamped": true'); |
There was a problem hiding this comment.
[Suggestion] R1-5: The new CONTINUE test cannot catch the regression its own comment names — every assertion is satisfied by text outside the resume block it exists to protect.
Failure scenario (mutants probed): p.indexOf('--resume') finds the PROSE occurrence (agent-prompt.ts ~1300); slice(idx - 400) runs to the END of the prompt, so the plan-path assertion is satisfied by the first invocation block's --plan line regardless, and the --out path is never asserted at all. The three toContain assertions (--resume, testScope.notRun, "clamped": true) are all present in the sibling agent-briefs.ts:538 bullet this same PR adds, which buildRoleBrief emits before the role-7 blocks. Mutant 1 — delete the ENTIRE continuation block (prose + welded bash invocation): all four assertions stay green. Mutant 2 — delete every --plan /abs/tmp/plan.json line prompt-wide: the window still contains the path. The regression the comment guards ("an agent that has to re-derive them gets them wrong") ships green.
Suggested fix: isolate the resume block — find the last bash fence (or a block-specific heading such as 'If the report says work is left'), slice that block alone, and assert it contains --resume, --plan /abs/tmp/plan.json, and the --out path.
中文说明
新增的 CONTINUE 测试抓不住它自己注释所说的那个回归——所有断言都能被"续跑块之外的文本"满足。
失败场景（变异已实测）：p.indexOf('--resume') 命中的是散文里的那处（agent-prompt.ts ~1300）；slice(idx - 400) 一直延伸到提示词末尾，因此 plan 路径断言无论如何都会被第一个调用块的 --plan 行满足，而 --out 路径从未被断言。三个 toContain 断言（--resume、testScope.notRun、"clamped": true）在本 PR 自己新增的 agent-briefs.ts:538 要点里全都存在，而 buildRoleBrief 会先输出它再输出 role-7 命令块。变异 1——删掉整个续跑块（散文 + 焊接的 bash 调用）：四个断言全部仍为绿。变异 2——在整个提示词中删除每一行 --plan /abs/tmp/plan.json：窗口内仍含该路径。注释声称要防的回归（"an agent that has to re-derive them gets them wrong"）会一路绿灯地发出去。
修复建议：把续跑块单独切出来——定位最后一个 bash 代码块（或块特有的标题如 'If the report says work is left'），只切该块，并断言其中包含 --resume、--plan /abs/tmp/plan.json 和 --out 路径。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| .option('timeout', { | ||
| type: 'number', | ||
| default: 300, | ||
| default: DEFAULT_COMMAND_TIMEOUT_S, |
There was a problem hiding this comment.
[Suggestion] R1-33: The diff raises build-test's --timeout default 300→540s so the 401s packages/cli suite can finish — but test-delta's base-tree rerun of the same command still uses its own unchanged 300s default (DEFAULT_TIMEOUT_S, test-delta.ts:209; the welded invocation passes no --timeout): a deadline below the slowest suite, which this diff's own rationale calls "a guaranteed timeout".
Failure scenario (deterministic trace on code constants): a PR that breaks a test in packages/cli (real cost 401s — the exact suite the new default is sized to) now completes on the PR side at ~401s with capture-time failingFiles; test-delta admits the base rerun and kills it at 300s → baseUnusable → netNew = shared = [] → disclosed as "base-side rerun timed out — infrastructure" and falls back to the path-based judgment this PR exists to replace — also spending ~300s of test-delta's 540s total budget, shrinking the window for other failed commands. Any suite with real cost in (300s, 540s] hits the wall: the measurement this diff builds the infrastructure for is structurally unavailable for the motivating suite.
Suggested fix: give test-delta the same floor — import DEFAULT_COMMAND_TIMEOUT_S from ./lib/build-budget.js for its --timeout default (its TOTAL_BUDGET_MS already equals one such deadline), or weld --timeout onto the brief's test-delta invocation.
中文说明
diff 把 build-test 的 --timeout 默认值从 300 提到 540s，好让 401s 的 packages/cli 套件跑完——但 test-delta 对同一条命令的基线树重跑仍用它自己未改的 300s 默认值（DEFAULT_TIMEOUT_S，test-delta.ts:209；焊接的调用不传 --timeout）：期限低于最慢套件，而本 diff 自己的理由称这是"a guaranteed timeout"。
失败场景（基于代码常数的确定性推演）：某 PR 破坏了 packages/cli 里的一个测试（真实耗时 401s——正是新默认值为之校准的那个套件），PR 侧现在能在 ~401s 完成并带捕获时 failingFiles；test-delta 接纳基线重跑并在 300s 杀掉它 → baseUnusable → netNew = shared = [] → 披露为 "base-side rerun timed out — infrastructure"，回退到本 PR 存在所要取代的按路径判断——同时还消耗 test-delta 540s 总预算中的 ~300s，压缩其他失败命令的窗口。任何真实耗时在 (300s, 540s] 的套件都会撞墙：本 diff 为之建设施的测量，对作为动机的那个套件结构性不可用。
修复建议：给 test-delta 同样的下限——让其 --timeout 默认值从 ./lib/build-budget.js 导入 DEFAULT_COMMAND_TIMEOUT_S（其 TOTAL_BUDGET_MS 本就等于一个这样的期限），或在简报的 test-delta 调用上焊接 --timeout。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| 'toolchain, so it never had suites to leave unrun.', | ||
| ); | ||
| } | ||
| if (previous.notBuilt?.length) { |
There was a problem hiding this comment.
[Suggestion] R1-34: The notBuilt refusal rejects the ENTIRE continuation — including the suites of packages the previous call DID compile — discarding resumable work its own justification does not cover.
Failure scenario (probed on unmodified code, two builds under a 40s budget): the first call produces notBuilt: ['packages/b'], test: [], fully populated testScope.notRun. A --resume with a FRESH 570s budget runs nothing (calls: []), returns the 'unbuilt … Re-run build-test without --resume' note, and leaves notRun intact — which keeps the brief's resume condition ("work is left when testScope.notRun is non-empty") true, so an agent can burn all three continuations on no-op refusals. The rationale ("a suite run against packages that were never compiled") argues for refusing the UNBUILT packages' suites only; the fresh path answers the identical state with a partial run, and the report retains everything needed to separate them (buildSet excludes notBuilt; reverseDependencyClosure is already imported). The instructed full re-run re-pays install/build from the same budget and hits the same stop whenever the build genuinely exceeds it — the built packages' suites never run in scoped mode across any number of calls. Nothing false is certified; the cost is discarded coverage.
Suggested fix: instead of refusing wholesale, mirror the fresh path: exclude reverseDependencyClosure(previous.notBuilt) from pendingDirs (leaving them in notRun with the existing disclosure) and run the remaining pending suites, which were all compiled.
中文说明
notBuilt 拒绝把整个续跑都拒掉——包括上一次调用已编译完成的包的套件——丢弃了它自己的理由并不覆盖的可续跑工作。
失败场景（在未改动代码上实测，两个构建、40s 预算）：第一次调用产出 notBuilt: ['packages/b']、test: []、完整填充的 testScope.notRun。带着全新 570s 预算的 --resume 什么都不跑（calls: []），返回 'unbuilt … Re-run build-test without --resume' 备注，且 notRun 原样保留——这让简报的续跑条件（"work is left when testScope.notRun is non-empty"）一直为真，agent 可能把三次续跑全烧在空操作的拒绝上。该理由（"a suite run against packages that were never compiled"）只支持拒绝未编译包的套件；全新路径对同样状态的回答是部分运行，而且报告保留了分离两者所需的一切（buildSet 不含 notBuilt；reverseDependencyClosure 已导入）。被指示的"完整重跑"要从同一预算重新支付 install/build，只要构建真的超出预算就撞上同一个停止点——已编译包的套件在任何次数的调用中都永远不会以 scoped 模式运行。不会背书任何假结果；代价是被丢弃的覆盖。
修复建议：不要整体拒绝，镜像全新路径：把 reverseDependencyClosure(previous.notBuilt) 从 pendingDirs 中排除（留在 notRun 里并保留现有披露），运行其余待定套件——它们都已编译。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const found = reviews.length ? latestLedger(reviews, currentUser()) : null; | ||
| prevLedger = found?.ledger ?? null; | ||
| prevLedgerAuthor = found?.foreign ? found.author : null; |
There was a problem hiding this comment.
[Critical] R1-35: Cross-account recovery can run the round counter BACKWARD: a foreign marker with a lower round number but later submitted_at unconditionally overwrites the side file's higher round, and the next compose re-issues finding ids the PR already carries.
Failure scenario: the side-file write (~1035-1038) performs no round comparison; ledgerMarkerFor reads only prev.round and never writes back. Trigger: the maintainer has posted rounds 1-7 (side file round 7); a CI bot whose recovery failed transiently (the kept catch path — "offline/unauthenticated just means no ledger") posts its own Round 1 marker with a later submitted_at. The maintainer's re-run picks it (latest by submitted_at regardless of author — pinned by test 'takes the LATEST marker whoever posted it'), the side file is overwritten 7→1, the next verdict composes as "Round 2" and buildLedger stamps new findings R2-1…R2-n — ids the maintainer's actual round 2 already used for different findings — and rounds 3-7 are re-issued against live history until the counter recovers (a depth-N regression costs N rounds of colliding ids). Pre-diff, own-account-only recovery could not return a foreign marker through the success path; the interleaved bot+maintainer flow is the exact scenario this PR targets (it cites PRs #9113/#9094). The author's own adjacent guard ("Keeping the stale copy only ever advances the count") covers only the failed-recovery path, not a successful foreign recovery. Verdicts and anchors stay safe (foreign sha is stripped); the corruption is cross-round claim identity in the PR's public history — the ledger's stated payoff ("R1-2 names the same claim in every round") silently inverted.
Suggested fix: preserve round monotonicity at the write — read the existing side file and write the recovered ledger with round: Math.max(found.ledger.round, existing.round); the side file's round is consumed only for numbering, so this does not affect the rendered work list.
中文说明
跨账号恢复可能让轮次计数器倒退：一个轮次更小但 submitted_at 更晚的外部标记会无条件覆盖 side file 里更大的轮次，下一次 compose 会重新签发 PR 上已经存在的发现 id。
失败场景：side file 写入（~1035-1038）不做任何轮次比较；ledgerMarkerFor 只读 prev.round、从不写回。触发：维护者已发布第 1-7 轮（side file 轮次 7）；某 CI bot 的恢复瞬时失败（保留的 catch 路径——"offline/unauthenticated just means no ledger"），以自己的身份发布了 "Round 1" 标记且 submitted_at 更晚。维护者重跑时选中它（按 submitted_at 取最新、不论作者——测试 'takes the LATEST marker whoever posted it' 钉住的行为），side file 被从 7→1 覆盖，下一次裁决 compose 成 "Round 2"，buildLedger 为新发现打上 R2-1…R2-n——这些 id 维护者真实的第 2 轮已用于不同的发现——随后第 3-7 轮会对着活着的历史重新签发，直到计数器追回（深度 N 的倒退要花 N 轮 id 冲突）。PR 之前，仅本账号恢复在成功路径上不可能返回外部标记；bot+维护者交替正是本 PR 针对的场景（它引用了 PR #9113/#9094）。作者自己在相邻处的守卫（"Keeping the stale copy only ever advances the count"）只覆盖恢复失败路径，不覆盖成功的外部恢复。裁决与锚点仍安全（外部 sha 已被剥离）；被破坏的是 PR 公开历史中的跨轮主张同一性——账本声明的收益（"R1-2 在每一轮都指同一条主张"）被静默反转。
修复建议：在写入处保持轮次单调——读取已有 side file，以 round: Math.max(found.ledger.round, existing.round) 写入恢复的账本；side file 的轮次只被用于编号，因此这不影响渲染出的工作清单。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // A resumed call continues a report; without one there is nothing to | ||
| // continue, and silently starting a fresh run would re-install and re-build | ||
| // inside a budget the caller sized for suites alone. Fail loudly instead. | ||
| const previous = args.resume ? previousReport(args.out) : undefined; |
There was a problem hiding this comment.
[Critical] R1-37: A --resume call whose worktree root scopes no adapter destroys the report it was asked to continue: runBuildTest's early returns build a report with no reference to previous, and the handler writes it over --out — the file previousReport just consumed.
Failure scenario (probed end-to-end through the real yargs handler): a worktree dir with no package.json and an --out holding an in-flight npm report (install record, a passed suite, a clamped timeout, testScope.notRun) — after the handler returned, --out contained {"toolchain": "unsupported", "install": null, "build": [], "test": []}; a second resume against the destroyed file with a CORRECT worktree refused with "did not scope an npm toolchain" — the continuation chain is dead even after the path is fixed, and the only record of up to three calls of accumulated state is gone. Only the adapter's refusal paths preserve the input (resumeNpmToolchain spreads ...previous via every refusal); runBuildTest's own early returns skip both protections, while previousReport fails loudly on a MISSING file — this path silently clobbers a present one. Pre-diff impossible because nothing ever read --out. Trigger: a wrong/renamed worktree path, or a worktree pruned/not-yet-recreated in the multi-round recovery flow this PR targets.
Suggested fix: when previous is set, refuse instead of returning a fresh unsupported report on the no-adapter paths — throw ('build-test: --resume cannot continue this report — the worktree scopes no toolchain. The report at was left untouched.'); a throw reaches the handler's catch, which writes nothing.
中文说明
一次 --resume 调用，若其 worktree 根目录匹配不到任何适配器，会摧毁它本要续跑的报告：runBuildTest 的早退路径构建的报告不包含对 previous 的任何引用，而 handler 会把它写回 --out——正是 previousReport 刚刚读过的那个文件。
失败场景（经真实 yargs handler 端到端实测）：一个没有 package.json 的 worktree 目录 + --out 里放着一份进行中的 npm 报告（install 记录、一个已通过的套件、一个 clamped 超时、testScope.notRun）——handler 返回后，--out 里变成 {"toolchain": "unsupported", "install": null, "build": [], "test": []}；用正确的 worktree 对这份被摧毁的文件再次续跑，得到 "did not scope an npm toolchain" 的拒绝——即使路径修正，续跑链也已死掉，最多三次调用积累的状态只剩这一份记录且已丢失。只有适配器自己的拒绝路径保留输入（resumeNpmToolchain 的每个拒绝都展开 ...previous）；runBuildTest 自己的早退路径两层保护都没有，而 previousReport 对缺失文件是大声拒绝的——这条路径却静默覆盖一份存在的报告。PR 之前不可能发生，因为没有任何东西读 --out。触发：worktree 路径写错/被改名，或本 PR 针对的多轮恢复流程中 worktree 被清理/尚未重建。
修复建议：当 previous 已设置时，在无适配器路径上拒绝而不是返回一份全新的 unsupported 报告——抛错（'build-test: --resume cannot continue this report — the worktree scopes no toolchain. The report at was left untouched.'）；抛错会进入 handler 的 catch，那里什么都不写。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| prevLedger, | ||
| prevLedgerAuthor, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-40: The new prevLedgerAuthor forwarding chain (runPrContext → buildMarkdown → renderLedgerSection) has zero test pinning: no test calls buildMarkdown with a ledger at all, and prevLedgerAuthor appears only in production lines (grep: :774, :849, :1016, :1024, :1027, :1057).
Failure scenario (mutant run): deleting the second argument at the renderLedgerSection seam (:849) — all 72 tests pass; the buildMarkdown call-site seam (:1057) survives a fortiori (nothing executes runPrContext — only the yargs builder is tested). All 18 buildMarkdown call sites in tests pass ≤6 positional arguments, and the foreign-rendering test this PR adds exercises renderLedgerSection directly, bypassing both seams. After such a regression, a recovered CI-bot marker renders in the built PR context as "recovered from the marker this account's last posted review carried" with none of the "THEIR claims / no incremental anchor" wording — the exact misreading the test this PR adds says must not happen. Anchor stripping stays pinned by the latestLedger tests; impact is confined to the attribution text readers and models rely on.
Suggested fix: add one buildMarkdown-level test — pass a ledger plus a non-null author and assert the foreign attribution text reaches the built markdown; pass author null and assert the own-account wording.
中文说明
新的 prevLedgerAuthor 转发链（runPrContext → buildMarkdown → renderLedgerSection）零测试钉住：没有任何测试带着账本调用 buildMarkdown，prevLedgerAuthor 只出现在生产代码行（grep：:774、:849、:1016、:1024、:1027、:1057）。
失败场景（变异实测）：删掉 renderLedgerSection 缝隙（:849）的第二个参数——全部 72 个测试通过；buildMarkdown 调用点缝隙（:1057）更不在话下（没有任何测试执行 runPrContext——只测了 yargs builder）。测试中全部 18 处 buildMarkdown 调用都只传 ≤6 个位置参数，而本 PR 新增的外部渲染测试直接调用 renderLedgerSection，绕过了两个缝隙。此类回归之后，恢复出的 CI-bot 标记会在构建的 PR context 中渲染为 "recovered from the marker this account's last posted review carried"，没有任何 "THEIR claims / no incremental anchor" 措辞——正是本 PR 新增测试声称绝不能发生的误读。锚点剥离仍被 latestLedger 测试钉住；影响局限于读者与模型依赖的归属文案。
修复建议：补一个 buildMarkdown 层级的测试——传入账本加非空作者，断言外部归属文案出现在构建的 markdown 中；传入作者 null，断言本账号措辞。
— qwen3.8-max via Qwen Code /review (v0.21.11)
The review this branch changes was pointed at this branch, and it filed three blockers and six suggestions. All of them hold; each fix carries the probe that found it. The Chinese placeholder classifier was dropping real disclosures. It claimed to mirror the English branch's narrowing and did not: without an all-done head the span before the completion word swallowed a gap clause, the single-character negation lookbehind let "not yet finished" read as finished, and forty free characters after the completion word swallowed the clause carrying the gap. Eight real sentences were classified as no-answers, which is the direction that certifies depth nobody reached. The branch now requires the same head the English side has always required, extends the lookbehinds, and ends at the completion word with only a budget adverbial after it. A continuation was declaring victory it had not won. A retry admitted late is killed again — the ordinary outcome when an expensive suite gets the tail of a budget — and the merged note still said every suite in scope had run, while a provisional result sat in the report. Retries the budget never reached were dropped from the accounting altogether, because a retry is a command and the not-run list holds workspaces. Both now count, and the note names what is still provisional and why another continuation is worth it. A continuation also dropped the framing an install failure owes the reader. The fresh path prepends it on every return precisely because the structured field alone was judged insufficient; the merge replaced the note wholesale, so a report carrying a non-zero install arrived with nothing saying that failure is infrastructure rather than something to file against the pull request. The framing is now shared between both paths. The rest are smaller. The budget help text described an admission rule this branch had already replaced, contradicting the code, the tests, and the sibling flag's own description. The continuation test could not fail: it searched the whole prompt, so text from the first invocation block and the sibling brief satisfied every assertion, and deleting the block it guards left it green — it now isolates that block, and a mutation confirms it fails when the block goes. The report guard checked one of the three arrays the merge walks, so a partial report died on a raw type error instead of the named refusal that is the guard's whole purpose. Two prose copies of a constant this branch consolidated now interpolate it. And the comment at the ledger recovery site still described the own-account-only model this branch removes, at exactly the boundary a reader auditing the change would consult.
commented
Aug 14, 2026
|
All nine hold. Fixed in cbc1824 — each with the probe that would have caught it. R1-1 — all three mechanisms confirmed. The branch claimed to mirror the English narrowing and did not: no all-done head, a single-character negation lookbehind that R1-2 — both halves fixed. The merged note now names what is still provisional and why another continuation is worth it, computed from the merged results rather than from the loop's own bookkeeping; and unattempted retries are recovered at the budget break. They cannot enter R1-3 — correct, and the reason it matters is in the framing's own history: the fresh path prepends it on every return because the structured field alone was judged insufficient. The framing is now a shared function both paths call, with a test asserting a continuation still carries "never as a Critical, and never against this PR". R1-4 — mine. I wrote that sentence while the admission rule was "price the suite, refuse to start it", then reverted to the clamped-and-retry design and left the help text describing the abandoned one. Reworded to the shipped behaviour. R1-5 — the sharpest of the nine, because the test I wrote to guard a regression could not fail. It now isolates the continuation block by fence rather than searching the whole prompt, and asserts the plan path, the out path, and the command inside that block. Mutation-checked both ways: deleting the block turns it red, restoring it turns it green. The old assertions survived the same mutation. R1-6 — the guard now requires every array the merge walks, not just the one that names the work, and both refusal branches have tests. R1-7 — both locations interpolate the constant now. R1-8 — updated, and pointed at the seam where the split actually happens. R1-9 — no code change, and I want to be explicit about why rather than let it sit unanswered. The mutation is a TypeScript optional-property declaration; erasing it cannot change runtime behaviour, so no runtime test can fail on it, and 中文说明九条全部成立，已在 cbc1824 修复——每条都附带能抓住它的探针。 R1-1 —— 三种机制均确认。该分支声称镜像英文分支的收窄，实际并没有：没有"全部完成"头部、否定向后断言只看一个字符（ R1-2 —— 两半都修了。合并后的 note 现在点名仍处于临时状态的命令并说明再续跑一次的价值，且该判断从合并结果计算而非依赖循环自身的记账；未执行的 retry 在预算中断处被回收。它们无法进入 R1-3 —— 正确，而它之所以重要正写在这段措辞自己的历史里：全新路径在每次返回时都前置它，恰恰因为仅有结构化字段被判定为不够。该措辞现在是两条路径共用的函数，并有测试断言续跑后仍带着 "never as a Critical, and never against this PR"。 R1-4 —— 我的问题。我写那句时接纳规则还是"给套件定价、装不下就不启动"，之后改回 clamped 重试方案，却把帮助文本留在了被放弃的那版。已按实际行为改写。 R1-5 —— 九条里最锋利的一条，因为我写来防回归的测试根本不会失败。现在它按代码围栏切出续跑块，只在该块内断言 plan 路径、out 路径与命令。双向变异验证：删掉该块变红，恢复变绿；旧断言在同样变异下是绿的。 R1-6 —— 守卫现在要求合并路径会遍历的每一个数组，而不只是列出工作的那个；两个拒绝分支都有测试。 R1-7 —— 两处都改为插值常量。 R1-8 —— 已更新，并指向拆分真正发生的那个缝隙。 R1-9 —— 不改代码，但我想把理由说清楚而不是放着不答。该变异删除的是 TypeScript 可选属性声明，擦除它不可能改变运行时行为，因此没有任何运行时测试会失败，把关的是 |
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) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally.
Not explored to full depth (tool budget reached): chunk 4: running the vitest suite budget.test.ts in-place — the review worktree has no node_modules and a monorepo install exceeded the tool budget; substituted by e…; chunk 4: deep trace of coverageFromTranscripts (coverage.ts, outside my chunk) — the machine-derived-evidence claim is taken at its existing contract..
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally。
未探索到全部深度（达到工具调用预算）：chunk 4：running the vitest suite budget.test.ts in-place — the review worktree has no node_modules and a monorepo install exceeded the tool budget; substituted by e…；chunk 4：deep trace of coverageFromTranscripts (coverage.ts, outside my chunk) — the machine-derived-evidence claim is taken at its existing contract.。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| budget: args.budget, | ||
| previous, | ||
| exec: args.exec ?? run, |
There was a problem hiding this comment.
[Critical] R1-37: A --resume call whose worktree root scopes no adapter destroys the report it was asked to continue: runBuildTest's no-adapter early returns build a fresh report with no reference to previous, and the handler writes it over --out — the file previousReport just consumed (round-1 finding, still standing; probed through the real yargs handler). — Failure scenario: a worktree dir without package.json plus an in-flight npm report at --out → after the handler returns, --out contains {"toolchain":"unsupported","install":null,"build":[],"test":[]}; a second resume with a CORRECT worktree then refuses 'did not scope an npm toolchain' — the continuation chain is dead even after fixing the path, and the only record of accumulated state is gone. Trigger: a wrong/renamed --worktree, or a worktree pruned/not-yet-recreated in the multi-round recovery flow this PR targets.
Suggested fix: when previous is set, throw on the no-adapter paths instead of returning a fresh unsupported report; the throw reaches the handler's catch, which writes nothing.
中文说明
一次 --resume 调用，若其 worktree 根目录匹配不到任何适配器，会摧毁它本要续跑的报告：runBuildTest 的无适配器早退路径构建的报告不包含对 previous 的任何引用，而 handler 会把它写回 --out——正是 previousReport 刚刚读过的那个文件（第 1 轮已提出，仍然成立；已用真实 yargs handler 实测）。
失败场景：没有 package.json 的 worktree 目录 + --out 里一份进行中的 npm 报告 → handler 返回后，--out 里变成 {"toolchain":"unsupported","install":null,"build":[],"test":[]}；用正确的 worktree 再次续跑会得到 "did not scope an npm toolchain" 的拒绝——即使路径修正，续跑链也已死掉，积累的状态记录全部丢失。触发：worktree 路径写错/被改名，或本 PR 针对的多轮恢复流程中 worktree 被清理/尚未重建。
修复建议：当 previous 已设置时，在无适配器路径上抛错而不是返回一份全新的 unsupported 报告；抛错会进入 handler 的 catch，那里什么都不写。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const ZH_PLACEHOLDER = | ||
| `${ZH_TOKEN}(?:${ZH_TAIL}` + | ||
| `|\\s*[-—–、,，]\\s*${ZH_ALL_DONE}(?:(?!${ZH_EXCEPTION}).){0,40}` + | ||
| `(?<!未)(?<!没)(?<!没有)(?<!无法)(?<!尚未)${ZH_COMPLETION}` + |
There was a problem hiding this comment.
[Critical] R2-1: The new Chinese placeholder classifier's negation lookbehinds miss inability modifiers (未能/没法/不能/难以 — probed; bare 不/不曾/未曾 are the same class): each lookbehind sees only the character(s) adjacent to the completion word. — Failure scenario: probe on the real budgetGapDisclosures: Budget gap: 无 — 所有检查未能完成, 无 — 全部检查没法完成, 无 — 所有检查难以完成, 无 — 所有检查不能完成 all return [] (DROPPED) while controls behave. An agent whose checks could not run has its real disclosure classified as a no-answer placeholder — the composed review certifies full depth, the exact error the docstring's 没有 bullet says it eliminated. The new tests cover 均未完成 and 还没有完成, not this family.
Suggested fix: extend the chain with the missing modifiers (e.g. (?<!不能)(?<!未能)(?<!没法)(?<!难以)) and add keep-tests for those sentences.
中文说明
新增的中文占位符分类器，其否定向后断言漏掉了"无能"类修饰词（未能/没法/不能/难以——已实测；光杆的 不/不曾/未曾 属同类）：每个断言只能看到完成词紧邻的字符。
失败场景：对真实 budgetGapDisclosures 实测：Budget gap: 无 — 所有检查未能完成、无 — 全部检查没法完成、无 — 所有检查难以完成、无 — 所有检查不能完成 全部返回 []（被丢弃），而对照组行为正常。一个检查没能跑完的 agent，其真实披露会被判定为"无内容"占位符——合成后的审查为从未达到的深度背书，这正是 docstring 里 没有 一条声称已消除的错误。新增测试覆盖了 均未完成 和 还没有完成，没有覆盖这一族。
修复建议：为断言链补上缺失的修饰词（如 (?<!不能)(?<!未能)(?<!没法)(?<!难以)），并为这些句子补充保留测试。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const ZH_BUDGET_TAIL = | ||
| '(?:[，,、]?\\s*(?:未触及|未达到|未超出|在|不超过)[^，,。．.!！…;；]{0,20}预算[^，,。．.!！…;；]{0,10})?'; |
There was a problem hiding this comment.
[Critical] R2-2: ZH_BUDGET_TAIL's two free-character classes can cross the exception markers 但/除, violating the docstring's own invariant ('neither span may cross an exception') and diverging from the English branch (probed). — Failure scenario: probe: Budget gap: 无 — 所有检查完成，未触及预算但渗透测试未运行 and …在预算内但跳过了渗透测试 are DROPPED — the {0,10} class after 预算 consumes the unpunctuated 但… clause and the real gap is discarded, certifying depth nobody reached (the direction the docstring names as fatal). With a comma before 但 the sentence correctly survives. Flip (tempering both classes with 但/除) restores both sentences with all 52 budget tests green.
Suggested fix: exclude the exception markers from both free classes: [^，,。．.!！…;；但除]{0,20}预算[^，,。．.!！…;；但除]{0,10}.
中文说明
ZH_BUDGET_TAIL 的两个自由字符类可以跨越例外标记 但/除，违反了 docstring 自己写明的不变量（"任何区间都不得跨越例外"），也与英文分支不一致（已实测）。
失败场景：实测：Budget gap: 无 — 所有检查完成，未触及预算但渗透测试未运行 和 …在预算内但跳过了渗透测试 被丢弃——预算 之后的 {0,10} 字符类吞掉了不带标点的 但… 从句，真实的缺口被抛弃，为从未达到的深度背书（docstring 称之为致命方向）。但 前有逗号时句子能正常保留。翻转验证（给两个字符类加入 但/除 排除）后两句恢复保留，52 条 budget 测试全绿。
修复建议：把例外标记从两个自由字符类中排除：[^，,。．.!！…;；但除]{0,20}预算[^，,。．.!！…;；但除]{0,10}。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| '`testScope.notRun` is non-empty, or when any `test[]` entry has ' + | ||
| '`"clamped": true` — a suite the budget started too late and killed, which ' + |
There was a problem hiding this comment.
[Critical] R2-9: The new resume-decision rule taught to Agent 7 recognizes only two 'work is left' signals (testScope.notRun, clamped test[] entry), but the tool emits a third shape: a single-root repo whose budget stopped before its one suite — no testScope, test:[], ok:true, leftover work only in the free-form note (probed through runBuildTest). — Failure scenario: single-package repo whose install+build spends the budget below the 15s floor (shape pinned by build-test.test.ts:1517): probe shows the rule evaluates signal1=false signal2=false → the agent concludes no work is left and reports the build-and-test dimension FINISHED with zero suites ever run — the exact failure the block's own comment says it exists to prevent. --resume on this shape answers 'ended before its test phase', so the continuation path cannot repair it either; the sibling copy in agent-briefs.ts:539 has the same hole (see the duplication finding).
Suggested fix: add the third signal to the rule (when the note says suites were not run, a single-package repo carries no testScope) in BOTH copies, and fix the consumer side per the neverTested finding.
中文说明
教给 Agent 7 的新续跑判定规则只认两种"还有工作"的信号（testScope.notRun、带 clamped 的 test[] 条目），但工具还会产生第三种形状：预算在自己的唯一套件之前耗尽的单根仓库——没有 testScope、test:[]、ok:true，剩余工作只存在于自由文本 note 里（已经由 runBuildTest 实测）。
失败场景：install+build 把预算耗到 15s 下限以下的单包仓库（该形状由 build-test.test.ts:1517 钉住）：探针显示规则算出 signal1=false signal2=false → agent 判定没有剩余工作，把构建测试维度报告为"已完成"——而实际上一个套件都没跑过——正是该代码块注释声称要防止的失败。对这个形状执行 --resume 会得到 "ended before its test phase"，续跑路径同样无法补救；agent-briefs.ts:539 的孪生副本有同样的洞（见重复副本那条发现）。
修复建议：在两处副本中都给规则加上第三种信号（当 note 说明有套件未运行时；单包仓库没有 testScope），并按 neverTested 那条发现修复消费端。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const shape = parsed as { | ||
| test?: unknown; | ||
| build?: unknown; | ||
| timedOut?: unknown; | ||
| }; |
There was a problem hiding this comment.
[Critical] R2-10: previousReport's shape gate validates only test/build/timedOut, but the continuation also walks previous.testScope.workspaces and previous.testScope.notRun — a report with a truthy testScope lacking workspaces crashes --resume with a raw TypeError; a string notRun iterates characters into garbage suite commands (probed). — Failure scenario: probe on unmodified code: (a) report carrying testScope:{notRun:[…]} without workspaces → THREW 'prevScope.workspaces is not iterable' — a raw stack trace exactly where the gate's comment promises the structured refusal; (b) notRun as a string → 10 commands executed: npm test --workspace="p", "a", "c", "k"… — character-iterated garbage spending the whole-call budget and merging fabricated failures. Flip (validating testScope arrays in the gate) turned both into the structured refusal with zero commands executed.
Suggested fix: extend the gate: when testScope is present require workspaces (and, when present, notRun) to be arrays of strings; reject with the same structured message otherwise.
中文说明
previousReport 的形状门禁只校验 test/build/timedOut，但续跑还会遍历 previous.testScope.workspaces 和 previous.testScope.notRun——testScope 存在但缺少 workspaces 的报告会让 --resume 崩出裸 TypeError；notRun 是字符串时会被逐字符迭代成垃圾套件命令（已实测）。
失败场景：在未改动代码上实测：(a) 带 testScope:{notRun:[…]} 但没有 workspaces 的报告 → 抛出 'prevScope.workspaces is not iterable'——恰恰在门禁注释承诺给出结构化拒绝的位置蹦出裸栈 trace；(b) notRun 为字符串 → 执行了 10 条命令：npm test --workspace="p"、"a"、"c"、"k"……逐字符迭代的垃圾命令耗掉整次调用预算并合并进捏造的失败。翻转验证（在门禁中校验 testScope 数组）后两种输入都变成结构化拒绝且零命令执行。
修复建议：扩展门禁：testScope 存在时要求 workspaces（以及存在时的 notRun）为字符串数组；否则用同一条结构化消息拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| `${SHELL_TOOL_MAX_TIMEOUT_MS / 1000}-second ceiling is per CALL, not per run: this repo needs more than ` + | ||
| 'one call to finish its suites (install, the builds, then `packages/core` ' + | ||
| 'at 106s and `packages/cli` at 401s, before the rest). Work is left when ' + | ||
| '`testScope.notRun` is non-empty, or when any `test[]` entry has ' + |
There was a problem hiding this comment.
[Suggestion] R2-31: The resume-decision predicate is added by this PR as two separately-maintained prose copies — agent-prompt.ts:1301-1304 and lib/agent-briefs.ts:539 — with nothing tying them together; a fix to the rule (see the single-root hole finding) must land on both. — Failure scenario: both texts reach the same Agent 7 prompt (agent-prompt.ts composes the role brief from agent-briefs' BRIEFS['7'].brief). A repair editing only agent-prompt.ts leaves the agent-briefs.ts bullet — which states the predicate as a standalone rule ('Either one means the dimension is unfinished') — still teaching the agent to declare an early-stopped dimension finished. This is the exact anti-pattern build-budget.ts's own header says it exists to retire ('Exported so the briefs and the budget below quote ONE number: they were separately-maintained copies') — the numbers were centralized, but the predicate was duplicated.
Suggested fix: emit the work-left rule from one place (a shared constant/paragraph interpolated into both surfaces), or at minimum cross-reference the copies and land the single-root fix on both.
中文说明
续跑判定谓词由本 PR 以两份各自维护的散文副本加入——agent-prompt.ts:1301-1304 与 lib/agent-briefs.ts:539——没有任何东西把它们绑在一起；对该规则的修复（见单根空洞那条发现）必须同时落在两处。
失败场景：两份文本都会进入同一个 Agent 7 提示词（agent-prompt.ts 从 agent-briefs 的 BRIEFS['7'].brief 组装角色简报）。只改 agent-prompt.ts 的修复会留下 agent-briefs.ts 的条目——它把谓词作为独立规则陈述（'Either one means the dimension is unfinished'）——继续教 agent 把提前停止的维度宣布为完成。这正是 build-budget.ts 头部声称要终结的反模式（'Exported so the briefs and the budget below quote ONE number: they were separately-maintained copies'）——数字被集中了，谓词却被复制了。
修复建议：把"还有工作"规则从单一来源发出（共享常量/段落，插值到两个面），至少也要互相交叉引用并让单根修复同时落在两处。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } | ||
| }); | ||
|
|
||
| it("stamps this build's entry when NOTHING was inherited", async () => { |
There was a problem hiding this comment.
[Suggestion] R2-36: No test pins that the stamp is absolutized: every fixture sets process.argv[1] to an already-absolute path, so dropping resolve() from childEnv() leaves all 44 tests green while re-breaking the exact #9113 launch shape (mutant verified). — Failure scenario: the motivating case in childEnv's own docstring is node dist/cli.js review run <pr> where argv[1] is the RELATIVE dist/cli.js. Mutant dropping resolve(): all 44 run.test.ts tests pass (probe). The stamp would be dist/cli.js; the child review session cds into its PR worktree, and every "${QWEN_CODE_CLI:-qwen}" subcommand execs the relative path → ENOENT/exit 127 for the whole review. :- falls back only on unset/empty, so a set-but-relative value never degrades to qwen.
Suggested fix: add a fixture with a relative argv[1] (process.argv[1] = 'cli.js' after process.chdir(dir)) asserting childEnvValue() equals the absolute join(dir, 'cli.js').
中文说明
没有测试钉住 stamp 会被绝对化：每个夹具都把 process.argv[1] 设为已绝对的路径，因此从 childEnv() 中删除 resolve() 后 44 条测试仍全绿，却重新打破了 #9113 的那个启动形状（变异已验证）。
失败场景：childEnv docstring 自己的动机场景是 node dist/cli.js review run <pr>，argv[1] 是相对的 dist/cli.js。删除 resolve() 的变异：run.test.ts 全部 44 条测试通过（探针）。stamp 会是 dist/cli.js；子审查会话 cd 进它的 PR worktree，每条 "${QWEN_CODE_CLI:-qwen}" 子命令都会执行相对路径 → 整个审查 ENOENT/exit 127。:- 只在未设置/为空时回退，因此"已设置但相对"的值永远不会退化为 qwen。
修复建议：新增相对 argv[1] 的夹具（process.chdir(dir) 后 process.argv[1] = 'cli.js'），断言 childEnvValue() 等于绝对的 join(dir, 'cli.js')。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
|
||
| // The first retry finishes; the second is admitted with what is left and | ||
| // killed again, so it stays provisional. | ||
| let call = 0; |
There was a problem hiding this comment.
[Suggestion] R2-38: The 'keeps reporting work left when a retry is killed AGAIN' test cannot distinguish its named mechanism (second retry admitted on a shortened deadline and killed again) from the second retry never being admitted at all — the exec callback counts calls but records nothing (mutant verified). — Failure scenario: mutant making the resume loop budget-stop at index 1: the test still passes (probe: 'Tests 1 passed' under the mutant) — the unattempted retry lands in unattemptedRetries so stillClamped is non-empty anyway (the original clamped fixture entry survives in mergedTest), the note gets both 'still provisional' and 'Resume again' from the stillClamped clause in both branches, and the completion claim is suppressed in both. The sibling 'replaces a CLAMPED timeout' captures deadlines and pins what this test's title claims — this test is one captured array (or expect(call).toBe(2)) short.
Suggested fix: capture deadlines in the exec callback and assert expect(deadlines).toHaveLength(2) and expect(deadlines[1]).toBeLessThan(60_000) — proving the second retry ran with a budget-shortened deadline.
中文说明
"重试再次被预算杀死时继续报告剩余工作"这条测试无法区分它命名的机制（第二个重试以缩短期限被接纳并再次被杀）与第二个重试根本没被接纳——exec 回调计数但不记录任何东西（变异已验证）。
失败场景：让续跑循环在索引 1 处预算停止的变异：测试仍通过（探针：变异下 'Tests 1 passed'）——未尝试的重试落入 unattemptedRetries，stillClamped 依然非空（原始 clamped 夹具条目留在 mergedTest 中），note 在两个分支下都从 stillClamped 从句获得 'still provisional' 和 'Resume again'，完成声明在两个分支下都被压住。兄弟测试 'replaces a CLAMPED timeout' 捕获期限并钉住了本测试标题所声称的东西——本测试只差一个捕获数组（或 expect(call).toBe(2)）。
修复建议：在 exec 回调中捕获期限，断言 expect(deadlines).toHaveLength(2) 且 expect(deadlines[1]).toBeLessThan(60_000)——证明第二个重试以预算缩短的期限运行过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ok: false, | ||
| timedOut: [], | ||
| note: 'two clamped', |
There was a problem hiding this comment.
[Suggestion] R2-39: The 'counts an unattempted RETRY' fixture pairs two clamped test entries with timedOut: [] — a report shape production cannot produce (the first-call path pushes every timed-out command into results.timedOut, npm-toolchain.ts:940) — so the merge's retention of previous.timedOut entries for never-retried commands is pinned by no test (mutant verified). — Failure scenario: mutant changing the merge filter from previous.timedOut.filter((c) => !replaced.has(c)) to !retryCommands.includes(c) would drop still-provisional, never-retried commands from the merged timedOut — probe: all 89 tests pass under the mutant (in every existing fixture the two filters coincide). The under-count feeds resumedNote's 'N command(s) ran out of time' accounting and any consumer of the field, shipping a cleaner-looking report than the run warrants.
Suggested fix: make the fixture realistic — timedOut: ['npm test --workspace="packages/core"', 'npm test --workspace="packages/a"'] — and assert rep.timedOut still contains the unattempted retry's command after the merge.
中文说明
"把未尝试的重试计为剩余工作"的夹具把两个 clamped 测试条目与 timedOut: [] 配对——这是生产线产生不出的报告形状（首次调用路径会把每个超时命令推进 results.timedOut，npm-toolchain.ts:940）——因此合并对从未重试命令保留 previous.timedOut 条目这一点没有任何测试钉住（变异已验证）。
失败场景：把合并过滤从 previous.timedOut.filter((c) => !replaced.has(c)) 改为 !retryCommands.includes(c) 的变异会把仍临时、从未重试的命令从合并后的 timedOut 中丢掉——探针：全部 89 条测试在该变异下通过（每个现有夹具中两种过滤结果重合）。少报会喂给 resumedNote 的 'N command(s) ran out of time' 记账和该字段的所有消费者，上线一份比运行实际更"干净"的报告。
修复建议：让夹具符合真实形状——timedOut: ['npm test --workspace="packages/core"', 'npm test --workspace="packages/a"']——并断言合并后 rep.timedOut 仍包含未尝试重试的命令。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| testScope.caveat = testScope.caveat | ||
| ? `${testScope.caveat}; ${resumedNote}` | ||
| : resumedNote; |
There was a problem hiding this comment.
[Suggestion] R2-42: Resume only ever APPENDS to testScope.caveat: a fully completed resume chain leaves the caveat permanently present with a stale 'still to run — not run: …' segment — violating the field's own contract ('Absent means the run covers everything the diff can break', workspace-scope.ts) and tripping the brief's 'caveat present ⇒ scope may be incomplete' rule (probe-verified; flip green). — Failure scenario: this repo's own suites require --resume (install + builds + core + cli already exceed the 570s budget). Probe: a budget-stop report resumed to completion → ok:true, note 'Every suite in scope has now run, and everything passed.', notRun gone — but caveat still reads '…was spent with 2 suite(s) still to run — not run: packages/a, packages/b; a --resume call ran the remaining 2 command(s)'. The build-test brief says 'When testScope.caveat is present, the scope may be incomplete — quote the caveat', so the published review reports coverage as possibly incomplete and re-quotes suites a–b as not run although they passed. Even a clamped-only resume with no prior caveat ends with caveat present. Direction is conservative (under-claims coverage), hence Suggestion.
Suggested fix: when a resume clears all outstanding work (left === 0 && stillClamped.length === 0), retire the superseded budget-stop segment instead of appending — keep only live incompleteness reasons.
中文说明
续跑对 testScope.caveat 只做追加：一条完全跑完的续跑链会让 caveat 永远存在，且带着过期的"still to run — not run: …"片段——违反该字段自己的契约（'Absent means the run covers everything the diff can break'，workspace-scope.ts），并触发简报的"caveat 存在 ⇒ 范围可能不完整"规则（已实测；翻转为绿）。
失败场景：本仓库自己的套件就需要 --resume（install + builds + core + cli 已超过 570s 预算）。探针：预算停止的报告续跑至完成 → ok:true、note 'Every suite in scope has now run, and everything passed.'、notRun 消失——但 caveat 仍写着 '…was spent with 2 suite(s) still to run — not run: packages/a, packages/b; a --resume call ran the remaining 2 command(s)'。build-test 简报说"testScope.caveat 存在时范围可能不完整——引用 caveat 并照说"，于是发布的审查把覆盖报告为可能不完整，并把已经跑过的套件 a–b 再次引用为未运行。即便没有先前 caveat 的纯 clamped 续跑，结束时 caveat 也存在。方向是保守的（低报覆盖），故为 Suggestion。
修复建议：当续跑清空全部剩余工作（left === 0 && stillClamped.length === 0）时，废弃被取代的预算停止片段而不是追加——只保留仍然有效的不完整原因。
— qwen3.8-max via Qwen Code /review (v0.21.11)
Ten more blockers, from the pipeline reviewing this branch a second time. The sharpest one is about the change this branch makes to the anchor. Exempting an unreviewed dimension from withholding the anchor was too wide. The field carries two different claims: a dimension nobody could RUN, which says nothing about which lines were read, and a lens that whiffed — made some tool calls, opened some files, returned nothing substantive twice — which is a claim about lines that no machine detector produces. The first cut exempted both, so a twice-whiffed security pass could advance the range past the lines it never reviewed, and the fixture that pinned exactly that was deleted along the way. The exemption now rests on a fact the code can check rather than on the field's name: exactly one dimension declares that it reads no diff, and only its gaps are depth. The deleted fixture is restored, and the other direction is pinned beside it. The Chinese no-answer classifier is rewritten as a closed vocabulary. Spelling the completion clause as bounded spans that merely refuse to cross an exception word invited exactly what two review rounds then found: negations the single-character lookbehind could not see, inability modifiers, hedged completions, gap clauses swallowed on either side of the completion word, and a span that slid past a negated completion to a later affirmed one. Every one of those drops a real disclosure, which is the direction this module's own header calls the worse of the two. A closed vocabulary cannot be walked through, because there is nothing to walk: a sentence carrying a gap is built from pieces the clause does not contain, so it fails to match and is kept. All thirteen evasions are now keep-tested against it. A continuation could destroy the report it was asked to continue. When no toolchain applied at the worktree root, the early returns built a fresh report with no reference to the previous one, and the handler wrote it over the file the run had just read — a wrong or pruned worktree path would replace an in-flight report with an empty one, and the chain stayed dead after the path was fixed. A continuation now refuses instead, and the refusal leaves the file untouched. Recovery across accounts could also run the round counter backward. Rounds are an id space, so a recovered round that goes down re-issues ids the pull request already carries against different findings. A bot whose own recovery failed transiently posts a round-one marker after a round-seven one, and ordering by timestamp alone hands the next round a two. The counter only ever advances, so preferring the highest round cannot lose a newer work list. The rest: the report guard now validates the scope the merge walks, not only its arrays; the ledger's one uncapped field is capped now that the read path takes text any account can post; the build-and-test brief names the third shape of unfinished work, a single-package repo whose budget stopped before its only suite; and the continuation test builds its paths the way its neighbours do instead of spelling them for one platform.
commented
Aug 14, 2026
|
Round 2's ten blockers are answered in e0524b0. Two of them changed my mind about the shape of the fix, not just its details. R2-12 is the one that mattered most, and it was right about the deleted fixture. The exemption keyed on the field's NAME, and that field carries two different claims: a dimension nobody could run says nothing about which lines were read, while a whiffed lens says exactly that and is the only detector of itself. It now keys on a fact the code can check — exactly one role declares that it reads no diff, so only its gaps are depth, and everything else in that field withholds as before. R2-1, R2-2, R2-11 and R2-26 are one finding, and the third round on the same regex was the signal. Spelling the completion clause as bounded spans that merely refuse to cross an exception word is an open invitation: every round found another way through, and each way drops a real disclosure — the direction this module's own header calls the worse of the two. So the clause is now a closed vocabulary with no free spans at all. A sentence carrying a gap is built from pieces the clause does not contain, so it cannot match and is kept, and adding a phrase is a deliberate edit rather than a side effect of loosening a quantifier. All thirteen of your evasion sentences are keep-tested, and the six real no-answer shapes still drop. R1-37 — a continuation could destroy the report it was continuing, which is the worst failure filed against this branch. It now refuses on the no-adapter paths, and the refusal leaves the file untouched. R1-35 — correct, and it generalises: rounds are an id space, so recovery now prefers the highest round rather than the latest timestamp. The counter only ever advances, so that cannot lose a newer work list. R2-10 — the guard validates the scope the merge walks, not only the arrays beside it. R2-13 — the one uncapped field is capped now that the read path takes text any account can post. R2-37 — the block builds its paths the way its neighbours do. R2-9 — the brief now names the third shape in both copies. I did not change the tool side: recording an unrun suite for a single-package repo means giving that shape a On the ~20 suggestions from this round: I am deliberately not landing them here. The repository's own guidance says a pull request that has been through several review rounds should land Criticals and defer the rest rather than keep widening, and this diff is already four commits past its original intent. The test-strength ones (a fixture that cannot distinguish the fix from the defect, an assertion that passes in one direction only, an untested exclusion) are the ones I would take first in a follow-up, and I would rather they arrive as a change that can be reviewed on its own than as more edits to a diff under review. 中文说明第二轮的十条阻断项已在 e0524b0 回应。其中两条改变的是修法的形状，而不只是细节。 R2-12 最关键，而且它对"被删掉的 fixture"这一点说得没错。 原先的豁免以字段的名字为判据，而这个字段承载两种不同的主张：没人跑得动的维度对"哪些行被读过"不置一词，而 whiff 掉的 lens 恰恰就是关于行的主张，并且它自己是唯一的探测器。现在豁免以代码可核查的事实为判据——全仓库只有一个角色声明自己不读 diff，因此只有它的缺口属于深度，该字段里其余一切照旧扣留。 R2-1、R2-2、R2-11、R2-26 是同一条发现，而"同一个正则被审到第三轮"本身就是信号。 把完成从句写成"只拒绝跨越例外词的有界跨度"等于公开邀请：每一轮都能找到新的绕法，而每一种绕法都会丢弃真实披露——正是本模块自己的文件头判定为更严重的那个方向。所以该从句现在是闭合词表，完全没有自由跨度。带缺口的句子由该从句不包含的成分构成，因此根本无法匹配、必然被保留；而增加一种说法要靠一次刻意的编辑，而不是放宽某个量词的副作用。你给出的十三个规避句全部有保留测试，六种真实的"无内容"形态仍然被丢弃。 R1-37 —— 续跑会摧毁它正在续跑的那份报告，这是本分支收到的最严重的一条。现在无适配器路径改为拒绝，且拒绝时文件原封不动。 R1-35 —— 正确，而且可以推广：轮次是一个 id 空间，因此恢复现在优先取最高轮次而不是最新时间戳。计数器只会前进，所以这样不会丢掉更新的工作清单。 R2-10 —— 守卫改为校验合并路径真正会遍历的 scope，而不只是它旁边的数组。R2-13 —— 既然读路径现在接受任何账号发布的文本，那唯一未设上限的字段已加上限。R2-37 —— 该块现在按邻居同样的方式构造路径。 R2-9 —— 两处 brief 都补上了第三种形态。工具侧我没有改：为单包仓库记录未运行的套件意味着给这种形态一个 关于本轮约二十条建议：我刻意不在这里落地。仓库自己的规范写明，经过若干轮审查的 PR 应当只落地 Critical、其余延后，而不是继续扩大；本 diff 已经比它最初的意图多出四个提交。其中测试强度类的几条（无法区分修复与缺陷的 fixture、只在一个方向断言、未被测试的排除项）是我在后续改动里最先要处理的，而我更希望它们作为一次可独立审查的改动出现，而不是继续往一个正在被审查的 diff 上叠加编辑。 |
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 still reported findings).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally.
[Critical] R2-12: (round-2 blocker, cache channel still standing; anchor channel fixed in this head) The Step 8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false) correctly loses the marker anchor but still writes .qwen/review-cache/pr-.json with lastCommitSha — the next same-SHA run short-circuits, a later SHA scopes past lines the lens never read. The same hole reaches layerAuditGate/scriptLintGate machine-debt entries (R1-14's cache channel). The parenthetical 'the marker's anchor is withheld under exactly this net' is false at head. Witness: the PR's own test 'withholds the anchor when a dimension gap is about LINES' pins the code side (sha undefined) while grep shows dimensionGapsAreDepthOnly appears zero times in SKILL.md. (Unanchorable: the SKILL.md snippet is a mid-paragraph span the resolver cannot map; the target paragraphs are single 1500-char lines.) Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false.
[Critical] R3-53: childEnv stamps a DIRECTORY argv[1] as the CLI entry — isUnusableScriptEntry has no file-ness check, and the consumer applies the same gate, so nothing blanks the stamp; pre-PR this branch wrote '' and the :- fallback worked. Trigger: npm start runs node /packages/cli (argv[1] = the directory — the shape stale-bundle.ts documents as measured). Witness (probe through the real handler): argv[1]=a directory → stamped = the directory; QWEN_CODE_CLI=
sh -c '"${QWEN_CODE_CLI:-qwen}" review match-remote' → 'Permission denied', EXIT=126; pre-PR '' stamp → EXIT=0; flip (statSync(entry).isFile() check) re-stamps '' (reverted). Fix: require statSync(resolve(ownEntry)).isFile() in addition to the gate, write '' otherwise. (Relocated from inline: its anchor line run.ts:405 collides with an existing round-1 thread at the same line.)中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 still reported findings)。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally。
[Critical] R2-12: (round-2 blocker, cache channel still standing; anchor channel fixed in this head) The Step 8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false) correctly loses the marker anchor but still writes .qwen/review-cache/pr-.json with lastCommitSha — the next same-SHA run short-circuits, a later SHA scopes past lines the lens never read. The same hole reaches layerAuditGate/scriptLintGate machine-debt entries (R1-14's cache channel). The parenthetical 'the marker's anchor is withheld under exactly this net' is false at head. Witness: the PR's own test 'withholds the anchor when a dimension gap is about LINES' pins the code side (sha undefined) while grep shows dimensionGapsAreDepthOnly appears zero times in SKILL.md. (Unanchorable: the SKILL.md snippet is a mid-paragraph span the resolver cannot map; the target paragraphs are single 1500-char lines.) Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false.
[Critical] R3-53: childEnv stamps a DIRECTORY argv[1] as the CLI entry — isUnusableScriptEntry has no file-ness check, and the consumer applies the same gate, so nothing blanks the stamp; pre-PR this branch wrote '' and the :- fallback worked. Trigger: npm start runs node /packages/cli (argv[1] = the directory — the shape stale-bundle.ts documents as measured). Witness (probe through the real handler): argv[1]=a directory → stamped = the directory; QWEN_CODE_CLI=
sh -c '"${QWEN_CODE_CLI:-qwen}" review match-remote' → 'Permission denied', EXIT=126; pre-PR '' stamp → EXIT=0; flip (statSync(entry).isFile() check) re-stamps '' (reverted). Fix: require statSync(resolve(ownEntry)).isFile() in addition to the gate, write '' otherwise. (Relocated from inline: its anchor line run.ts:405 collides with an existing round-1 thread at the same line.)— qwen3.8-max via Qwen Code /review (v0.21.12)
| * The stamp is only written when a shell could exec it — the same test the | ||
| * consumer applies at spawn time (isUnusableScriptEntry). A stamp that fails | ||
| * that test is worse than none: `${QWEN_CODE_CLI:-qwen}` falls back on empty, | ||
| * but a set-and-unusable path dies on exit 126. |
There was a problem hiding this comment.
[Suggestion] R1-24: the retained final paragraph of childEnv's JSDoc (~378-381) still asserts the exact behavior this PR removes — "write '': empty counts as unset … the child re-stamps from its own modules" — while the code now stamps resolve(ownEntry) in exactly those cases, and the PR's own new test repudiates the rationale ("Not '': the child cannot be relied on to re-stamp"). (Round-1 ledger entry, still standing.) — Failure scenario: a maintainer debugging version skew reads the opposite of what the code does; anyone "restoring" the documented behavior reintroduces the #9113 bug (a bundle-launched child's subcommands resolve qwen off PATH into an older global install).
Fix: rewrite the paragraph to match — on a root mismatch / unresolvable inherited path, stamp this build's own entry unless a shell could not exec it (isUnusableScriptEntry), in which case write '' so :- falls back.
中文说明
R1-24（第一轮账目项，仍然成立）：childEnv 的 JSDoc 保留的最后一段（约 378-381 行）仍在断言本 PR 已移除的行为——"写 ''：空值视为未设置……子进程会从自己的模块重新stamp"——而代码现在在这些情况下stamp的是 resolve(ownEntry)，且本 PR 自己的新测试也否定了该理由（"不是 ''：不能依赖子进程重新stamp"）。
失败场景：排查版本偏差的维护者读到的与代码实际行为相反；任何"恢复文档行为"的改动都会重新引入 #9113 缺陷。
修复：重写该段使其与代码一致。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| 'empty `test[]` and no `testScope`, and only its `note` says so — read ' + | ||
| 'the note before calling the dimension finished, and report it as ' + | ||
| 'unfinished when no suite ran. A resumed call skips install and build and ' + |
There was a problem hiding this comment.
[Critical] R3-18: this continuation paragraph contradicts the role-7 brief bullet (agent-briefs.ts:539) on the third shape, inside the SAME prompt: the paragraph gates --resume on testScope.notRun/clamped only and says "report it as unfinished when no suite ran" for the empty-test[]/no-testScope shape, while the bullet says "Any of them means the dimension is unfinished: re-run … with --resume". — Failure scenario: single-package repo whose budget ran out before its suite: if the paragraph wins, the owed suite never runs across rounds (the truncation this PR exists to eliminate); if the bullet wins, the resume lands in R1-13's misdiagnosing neverTested path (zero calls, "Re-run without --resume") and burns one of 3 continuations. The field-identical no-test-script shape (ok:true) gets the same misleading imperative.
Witness (probe): buildRoleBrief(plan,'7',{planPath}) contains the bullet at index 2788 and this paragraph at index 8343 (same prompt); resume on a shape-3 report: calls=[], note "Nothing to resume: … ended before its test phase …" for both the budget-exhausted and no-test-script shapes.
Fix: align the two texts — narrow the bullet's "Any of them" to notRun/clamped, and split this imperative by the note's discriminator (budget-spent → work is left; defines-no-test-script → nothing was owed, report finished with the note).
中文说明
R3-18：这段续跑说明与 role-7 简报条目（agent-briefs.ts:539）在"第三种形状"上自相矛盾，且两者出现在同一个提示词里：本段只在 testScope.notRun/clamped 时才认可 --resume，对空 test[]/无 testScope 的形状说"若没有套件运行过则报告为未完成"；而条目说"三者中任何一个都意味着维度未完成：用 --resume 重跑"。
失败场景：预算在唯一套件之前耗尽的单包仓库——若本段占上风，欠下的套件跨轮也不会运行（正是本 PR 要消除的截断）；若条目占上风，续跑落入 R1-13 的误诊路径（零次调用、"请不用 --resume 重跑"），白白烧掉 3 次续跑之一。字段完全相同的"没有 test 脚本"形状（ok:true）也会得到同样误导性的指令。
修复：统一两处文本——把条目的"任何一项"收窄为 notRun/clamped，并把本段指令按 note 的判别拆开（预算耗尽→还有工作；没定义 test 脚本→没有欠账，带着 note 报告完成）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(resumeBlock[0]).toContain('review build-test'); | ||
| expect(resumeBlock[0]).toContain(`--plan ${resolve('/abs/tmp/plan.json')}`); |
There was a problem hiding this comment.
[Suggestion] R2-30: the CONTINUE test pins --plan and --out on the resume fence but not --worktree, although the fence carries it and the test's own comment ("an agent that has to re-derive them gets them wrong") applies to it equally. (Round-2 ledger entry, still standing.) — Failure scenario: a mutant replacing --worktree ${resolve(buildTree)} with a wrong path stays green (the whole-prompt --worktree assertions are satisfied by the launch block's identical line); Agent 7 resumes in the wrong tree and previousReport merges the wrong tree's results into the PR's report.
Witness (mutant): wrong-path worktree in the resume fence → Tests 235 passed (235).
| expect(resumeBlock[0]).toContain('review build-test'); | |
| expect(resumeBlock[0]).toContain(`--plan ${resolve('/abs/tmp/plan.json')}`); | |
| expect(resumeBlock[0]).toContain('review build-test'); | |
| expect(resumeBlock[0]).toContain(`--plan ${resolve('/abs/tmp/plan.json')}`); | |
| expect(resumeBlock[0]).toContain(`--worktree ${resolve(PR_PLAN.worktreePath)}`); |
中文说明
R2-30（第二轮账目项，仍然成立）：CONTINUE 测试对续跑围栏钉住了 --plan 和 --out，却没有钉住 --worktree——围栏里明明带着它，且测试自己的注释（"需要自己推导路径的 agent 会推错"）同样适用于它。
失败场景：把续跑围栏里的 --worktree 换成错误路径的突变体全绿（整篇提示词的 --worktree 断言被启动围栏的同名行满足）；Agent 7 会在错误的树上续跑，把错误树的结果合并进 PR 报告。
修复：见 suggestion（补上 --worktree 断言）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const scopeOk = | ||
| shape.testScope === undefined || | ||
| (typeof shape.testScope === 'object' && |
There was a problem hiding this comment.
[Critical] R3-5: previousReport computes scopeOk from shape.testScope BEFORE the base gate's parsed === null check — JSON.parse('null') returns null, so shape.testScope throws a raw TypeError: Cannot read properties of null instead of the structured refusal this function's comment says it exists to produce. Non-null primitives survive the read and are refused correctly; null is the one corner that slips. — Failure scenario: --resume --out report.json where the file contains null — a realistic shape in this pipeline where agent-written/repaired state files are the documented corruption source — crashes with a raw stack trace at the moment the caller most needs the named fix instruction.
Witness (probe through real runBuildTest({resume:true})): TypeError: Cannot read properties of null (reading 'testScope'); control with 42 gets the structured …file is not one. refusal; flip (hoisting the non-object guard above scopeOk) turns the probe into the structured refusal (reverted).
Fix: hoist the base-type gate (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) above any property access.
中文说明
R3-5：previousReport 在基础门禁的 parsed === null 检查之前就从 shape.testScope 读取——JSON.parse('null') 返回 null，于是 shape.testScope 抛出裸的 TypeError，而不是这个函数注释声称要给出的结构化拒绝。非 null 的原始值能通过属性读取并被正确拒绝；null 是唯一漏掉的角落。
失败场景：报告文件内容为 null（本管线中文档化的"agent 手改状态文件"损坏通道完全可能产生）时，调用方在最需要"怎么修"指引的时刻只得到裸栈。
修复：把基础类型门禁移到任何属性读取之前。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| !Array.isArray(shape.test) || | ||
| !Array.isArray(shape.build) || | ||
| !Array.isArray(shape.timedOut) || |
There was a problem hiding this comment.
[Critical] R3-19: the gate validates only that test/build/timedOut ARE arrays, never the shape of their ELEMENTS — a corrupt report with test: [null] or build: [null] clears the gate and dies on a raw TypeError inside resumeNpmToolchain (.filter(t => t.clamped) / .every(succeeded)); testScope.notRun: [0] passes scopeOk and crashes in shellArg's dir.replace. The structured refusal the gate's own doc calls "the whole value" is replaced by a stack trace. — Failure scenario: an agent-repaired report {"toolchain":"npm","test":[null],"build":[],"timedOut":[]} (the documented corruption channel — the PR's own compose-review.test.ts fixture records a live run rewriting its state file with perl -pi) clears the gate → TypeError: Cannot read properties of null (reading 'clamped').
Witness (probes): [test-null-element] THREW TypeError … structured=false; [build-null-element] THREW (reading 'exitCode'); [notRun-non-string] THREW dir.replace is not a function — all clear the gate.
Fix: extend the gate to element shape — every test/build entry a non-null object, every timedOut entry a string, every testScope.workspaces/notRun entry a string — same structured refusal message.
中文说明
R3-19：门禁只校验 test/build/timedOut 是数组，从不校验数组元素的形状——test: [null] 或 build: [null] 的损坏报告能通过门禁，然后在 resumeNpmToolchain 内部死于裸 TypeError；testScope.notRun: [0] 能通过 scopeOk，在 shellArg 的 dir.replace 上崩溃。门禁文档称之为"全部价值"的结构化拒绝被裸栈取代。
失败场景：agent 手改出的 {"toolchain":"npm","test":[null],…} 报告（文档化的损坏通道）通过门禁后崩溃。
修复：把门禁扩展到元素形状（同上），给出同一条结构化拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| for (const partial of [ | ||
| { toolchain: 'npm', test: [] }, | ||
| { toolchain: 'npm', test: [], build: [] }, |
There was a problem hiding this comment.
[Suggestion] R3-48: 'refuses a report missing the arrays the merge walks' includes test: [] in every fixture, so the gate's !Array.isArray(shape.test) clause is pinned by nothing — deleting that clause keeps all three toThrow assertions green (each fixture still fails on missing build/timedOut). — Failure scenario: a report lacking the test key entirely ({"toolchain":"npm","build":[],"timedOut":[]} — a realistic truncation/repair shape for agent-written state files) clears the mutant gate and dies at the first line of resumeNpmToolchain with TypeError: Cannot read properties of undefined (reading 'filter') — the raw crash the gate's own comment says it exists to replace.
Witness (probe): deleting !Array.isArray(shape.test) || leaves the pinned test green (mutant survives); the no-test report then dies with Cannot read properties of undefined (reading 'filter') instead of /is not one/. Restored gate → green.
| for (const partial of [ | |
| { toolchain: 'npm', test: [] }, | |
| { toolchain: 'npm', test: [], build: [] }, | |
| for (const partial of [ | |
| { toolchain: 'npm', test: [] }, | |
| { toolchain: 'npm', test: [], build: [] }, | |
| { toolchain: 'npm', build: [], timedOut: [] }, |
中文说明
R3-48："拒绝缺少合并所需数组的报告"的每个夹具都带 test: []，因此门禁的 !Array.isArray(shape.test) 子句什么都没钉住——删除该子句三个 toThrow 断言全绿（每个夹具仍会因缺 build/timedOut 而失败）。
失败场景：完全不带 test 键的报告（agent 手改状态文件的现实截断/修复形状）能通过突变后的门禁，在 resumeNpmToolchain 第一行死于裸 TypeError——正是门禁注释说要取代的崩溃。
修复：见 suggestion（补一个不带 test 键的夹具）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const found = latestLedger( | ||
| [ | ||
| { id: 2, user: { login: 'bot' }, submitted_at: at, body: marker(1) }, |
There was a problem hiding this comment.
[Suggestion] R3-34: same-round timestamp ordering (at > best.at, pr-context.ts:710) is decided by no fixture: every multi-review fixture either differs in round or ties completely — and this PR's diff removed the one old fixture where timestamp decided. — Failure scenario: mutants at < best.at / at >= best.at / clause deletion keep all 74 latestLedger tests green; the real trigger is the flow this PR normalizes (same round posted twice with different timestamps — a re-run after a transient failure, or bot+maintainer both executing round N): a regression to older-timestamp-wins hands the next round the older work list, silently retiring findings the re-run added — "the one failure the whole recovery exists to prevent".
Witness (probe): all three mutants → 74/74 green; distinguishing fixture (round 2 at 01-01 carrying [R2-1] vs round 2 at 01-03 carrying [R2-1, R2-2]): passes on PR code, fails under the mutants with expected ['R2-1'] to deeply equal ['R2-1', 'R2-2'].
Fix: add a fixture with two same-round markers at different submitted_at (distinct findings lists) asserting the newer timestamp's work list wins, in both array orders.
中文说明
R3-34：同轮次的时间戳排序（at > best.at）没有任何夹具决定：所有多评审夹具要么轮次不同、要么完全同秒——且本 PR 的 diff 删除了唯一由时间戳决定结果的旧夹具。
失败场景：三个突变体（<、>=、删除子句）全部 74/74 全绿；真实触发正是本 PR 常态化的流程（同一轮次带不同时间戳发布两次——瞬态失败后的重跑，或 bot 与维护者都执行了第 N 轮）：回退为"旧时间戳获胜"会把旧工作清单交给下一轮，悄悄抹掉重跑新增的发现——"整个恢复机制存在就是要防止的那一种失败"。
修复：补一个同轮次、不同 submitted_at（发现清单不同）的夹具，断言新时间戳的工作清单获胜，且覆盖两种数组顺序。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * Latest by submitted_at wins regardless of author — a newer round's work list | ||
| * is the relevant one — and `foreign` says whether it came from someone else, |
There was a problem hiding this comment.
[Suggestion] R3-35: the latestLedger JSDoc this PR edited still describes the PRE-PR selection algorithm — "Latest by submitted_at wins … Ties break on the review id" — while the code runs ROUND first, timestamp second, id third; the round-first rule appears only in the inline comment at the comparison. — Failure scenario: a maintainer diagnosing a recovery mis-selection reads the JSDoc, concludes timestamp dominates, and "fixes" the comparison to match — reintroducing the round-backward regression and id re-issuance round-first exists to prevent. The diff's own passing test contradicts the doc (round 7 posted 01-01 beats round 1 posted 01-09).
Fix: rewrite the paragraph — highest round wins; within a round, latest submitted_at; within a second, highest review id; on a full tie, own beats foreign.
中文说明
R3-35：本 PR 编辑过的 latestLedger JSDoc 仍在描述 PR 之前的选择算法——"latest by submitted_at 获胜……同秒按评审 id 决胜"——而代码现在是轮次优先、时间戳次之、id 第三；轮次优先规则只出现在比较处的行内注释里。
失败场景：诊断恢复选错的维护者读 JSDoc 后认定时间戳主导，把比较"修"成与文档一致——重新引入轮次倒退与编号重发，而轮次优先正是为了防止它们。diff 自己通过的测试就与文档矛盾（01-01 发布的第 7 轮胜过 01-09 发布的第 1 轮）。
修复：重写该段为实际顺序。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const newer = | ||
| !best || | ||
| ledger.round > best.ledger.round || |
There was a problem hiding this comment.
[Suggestion] R2-23: the documented rule "a tie between an own and a foreign review keeps the OWN one" is unreachable — the !foreign clause fires only when id === best.id, impossible between distinct GitHub reviews (ids are globally unique); on any realistic (round, submitted_at) tie the higher id wins regardless of ownership, and the clause is dead code. (Round-2 ledger entry, still standing.) — Failure scenario: own round-7 marker (id 100) and foreign round-7 marker (id 105) with the same submitted_at second — or both timestamps missing (submitted_at ?? '') — → id 105 wins, stripAnchor drops the sha, and the next round re-reviews the full diff; the documented optimization is silently lost. Outcome is safe (full-range never mis-scopes); the cost is the lost anchor plus a dead clause in the contract.
Witness (probe through real latestLedger): same round, same second, own id 100 vs foreign id 105 → author=stranger foreign=true sha=(stripped); same with both timestamps missing.
Fix: apply ownership before id within the (round, at) tie — (at === best.at && ((best.foreign && !foreign) || (foreign === best.foreign && id > best.id))) — plus a same-round/same-second/distinct-id own-lower-id test.
中文说明
R2-23（第二轮账目项，仍然成立）：文档化的规则"本账号与外部评审完全打平时保留本账号的"不可达——!foreign 子句只在 id === best.id 时触发，而不同的 GitHub 评审 id 全局唯一；任何现实的（轮次, 时间戳）平局都按更高 id 决定、与归属无关，该子句是死代码。
失败场景：本账号第 7 轮标记（id 100）与外部第 7 轮标记（id 105）同秒——或两者都缺时间戳——→ id 105 获胜，stripAnchor 丢掉 sha，下一轮全量复审；文档承诺的优化被悄悄丢掉。结果是安全方向（全量不会错切范围），代价是丢锚点加合同里的死子句。
修复：在（轮次, 时间戳）平局内先按归属再按 id 决胜，并补对应测试。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // filtered to keep the section signal-rich. | ||
| if (prevLedger) { | ||
| parts.push(renderLedgerSection(prevLedger)); | ||
| parts.push(renderLedgerSection(prevLedger, prevLedgerAuthor)); |
There was a problem hiding this comment.
[Suggestion] R1-40: buildMarkdown's ledger wiring is exercised by no test — all 18 buildMarkdown call sites in pr-context.test.ts pass at most the first six positional args (zero prevLedger occurrences), so dropping the new second argument or the entire if (prevLedger) block ships green. (Round-1 ledger entry, still standing.) — Failure scenario: this call is the only path by which foreign-account rendering reaches a production context file; dropped, every foreign ledger renders as "recovered from the marker this account's last posted review carried" — a foreign work list read as this account's own certified round — and the suite stays green even after R3-10's fix, because the wiring never delivers the author.
Witness (probe): mutants renderLedgerSection(prevLedger) (arg dropped) → 74/74 green; if (prevLedger) block deleted → 74/74 green; probe buildMarkdown(…, ledger, 'qwen-code-ci-bot') passes on PR code and fails under both mutants.
Fix: add a buildMarkdown test passing a ledger plus a non-null prevLedgerAuthor asserting the rendered markdown contains **@<author>** and "THEIR claims" (and an own-ledger case asserting neither).
中文说明
R1-40（第一轮账目项，仍然成立）：buildMarkdown 的账本接线没有任何测试覆盖——pr-context.test.ts 中全部 18 处 buildMarkdown 调用最多只传前六个位置参数（prevLedger 出现次数为零），因此删掉新增的第二个实参或整个 if (prevLedger) 块都能全绿合入。
失败场景：这个调用是外部账号渲染进入生产上下文文件的唯一通路；删掉后每个外部账本都会渲染为"从本账号最后发布的评审标记恢复"——外部工作清单被读成本账号自己认证的轮次——即使修了 R3-10 套件也全绿，因为接线从未把作者送达。
修复：补一个 buildMarkdown 测试（传入账本 + 非空 prevLedgerAuthor，断言渲染包含 **@<作者>** 与 "THEIR claims"；再补一个本账号账本的反向断言）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Every blocker this round came from last round's fixes. That is worth saying plainly: the repairs were made under review pressure and shipped their own defects, and the pipeline caught each one. Two were outright bugs in the report guard. It read a field off the parsed value before checking that value was an object, so a file containing the literal `null` produced the raw type error the guard exists to replace — and it checked that the command lists were lists without checking what was in them, so a list holding a null cleared the gate and died one layer deeper. Both refuse now, with the recovery instruction the sibling branches already carried. The Chinese clause had reintroduced the overlapping-quantifier shape this module's header bans and its linearity test exists for: four optional groups chained across whitespace matchers, in a language that does not put whitespace between those tokens. The whitespace bought nothing but the backtracking. It is gone, and the pathological input that walks that shape is now in the linearity test beside the others. The anchor decision was reading the dimension list after a pre-existing splice had already removed every entry mentioning the review time budget — a splice that exists so the body does not say one gap twice, and that matches on a phrase. An entry whose free-form reason merely mentions the budget was therefore invisible to the decision, and that entry is exactly the line-coverage claim the decision must respect. It now reads the list as disclosed and renders from the spliced one. And the two copies of the continuation rule disagreed with each other inside one prompt about the shape that cannot be continued at all. They now say the same thing: report the dimension unfinished, and do not spend a continuation on a report that has no scope for one to read.
commented
Aug 15, 2026
|
Round 3's five blockers are fixed in f39f3bf. All five came from round 2's fixes, which is the honest summary of this round: repairs made under review pressure shipped their own defects, and each was caught here. R3-5 and R3-19 were plain bugs in the guard I added last round. It read a field off the parsed value before checking that value was an object, so a file containing the literal R3-20 is the one I should have caught myself: the closed vocabulary chained four optional groups across whitespace matchers, which is the overlapping-quantifier shape this module's own header bans and its linearity test exists for. Chinese does not put whitespace between those tokens, so the quantifiers bought nothing except the backtracking. The pathological input that walks that shape is now in the linearity test beside the English ones. R3-43 is the subtlest finding filed against this branch and I would not have found it. The anchor decision read the dimension list after a pre-existing splice had already removed every entry mentioning the review time budget — a splice that exists so the body does not say one gap twice, and that matches on a phrase rather than on structure. An entry whose free-form reason merely mentions the budget was invisible to the decision, and that entry is precisely the line-coverage claim the decision must respect. It now decides from the list as disclosed and renders from the spliced one, with a test using your exact witness sentence. R3-18 — the two copies contradicted each other inside one prompt, and the resolution is the one your failure scenario implies: the third shape cannot be continued at all, so both now say to report the dimension unfinished and not to spend a continuation on a report that has no scope for one to read. The deferral from round 2 still stands for this round's suggestions, on the same grounds and now with more force: this is five commits and three review rounds past the original intent. One observation worth recording on the thread rather than in a commit message, since it is about this pipeline and not about this diff: each round's fixes have produced the next round's blockers (round 2's fixes produced all five of round 3's). That is the loop the repository's own five-round guidance anticipates, and it is the reason I am holding the line on suggestions rather than continuing to widen the diff. 中文说明第三轮的五条阻断项已在 f39f3bf 修复。五条全部来自第二轮的修复——这就是本轮如实的总结：在审查压力下做出的修补带来了它们自己的缺陷，而每一条都在这里被抓住了。 R3-5 与 R3-19 是我上一轮新增的那个守卫里的明确 bug。它在确认解析结果是对象之前就读取了其字段，因此一个内容为字面量 R3-20 是我本该自己发现的一条：闭合词表把四个可选分组用空白匹配串了起来，而这正是本模块自己的文件头明令禁止、其线性度测试专门为之存在的重叠量词形状。中文在这些词之间根本不写空白，所以这些量词除了回溯什么也没换来。能走通该形状的病态输入现在与英文那几个并列进了线性度测试。 R3-43 是本分支收到的最微妙的一条，我自己不会发现。锚点判定读取维度列表的时机，晚于一处既有的拼接——那处拼接是为了让正文不把同一个缺口说两遍，且它按短语而非结构匹配，因而会把任何在自由文本里提到 review time budget 的条目一并移除。一个只是顺带提到预算的条目，对判定就此不可见，而它恰恰是判定必须尊重的"行覆盖"主张。现在判定读取披露时的原始列表、渲染用拼接后的列表，并用你给出的原句作为测试。 R3-18 —— 两份副本在同一段提示词内自相矛盾，而解法正是你的失败场景所暗示的：第三种形态根本无法续跑，因此两处现在都说明报告该维度未完成，且不要为一份没有 scope 可读的报告消耗一次续跑。 第二轮的延后决定对本轮建议同样有效，理由不变而且更充分：本 diff 已比最初意图多出五个提交、经历三轮审查。 还有一条值得记在讨论串而不是提交信息里的观察，因为它关于这条管线本身而不关于这个 diff：每一轮的修复都产出了下一轮的阻断项（第二轮的修复产出了第三轮全部五条）。这正是仓库自身"五轮"指引所预期的循环，也是我坚持只落地阻断项、不再扩大 diff 的原因。 |
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) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
[Critical] R2-12: The Step-8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false, machine coverage clean) correctly loses the marker anchor but still writes .qwen/review-cache/pr-.json with lastCommitSha — the next same-SHA run short-circuits ('No new changes', Step 6 re-check skipped), and a later-SHA run scopes past the lines the lens never read, while the parenthetical claims 'the marker's anchor is withheld under exactly this net'. Witness: the PR's own test 'withholds the anchor when a dimension gap is about LINES' pins the code side (sha undefined) while grep shows dimensionGapsAreDepthOnly appears zero times in SKILL.md. Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false. (Unanchorable: the target paragraph is a single ~1500-char line the resolver cannot map.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
[Critical] R2-12: The Step-8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false, machine coverage clean) correctly loses the marker anchor but still writes .qwen/review-cache/pr-.json with lastCommitSha — the next same-SHA run short-circuits ('No new changes', Step 6 re-check skipped), and a later-SHA run scopes past the lines the lens never read, while the parenthetical claims 'the marker's anchor is withheld under exactly this net'. Witness: the PR's own test 'withholds the anchor when a dimension gap is about LINES' pins the code side (sha undefined) while grep shows dimensionGapsAreDepthOnly appears zero times in SKILL.md. Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false. (Unanchorable: the target paragraph is a single ~1500-char line the resolver cannot map.)
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const dimensionGapsAreDepthOnly = unreviewedAsDisclosed.every( | ||
| isNonDiffDimensionGap, | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-14: The unreviewed-dimension anchor exemption classifies only the pre-gate snapshot unreviewedAsDisclosed (:726), but scriptLintGate (:825) and layerAuditGate (:833) push their machine-owed debts into unreviewed AFTER the snapshot — so a round capped solely by a gate-detected gap classifies dimensionGapsAreDepthOnly vacuously true and the anchor RIDES. Pre-PR, cappedBy.length > 0 withheld on all of these. (Round-1 blocker; independently re-derived by two reverse auditors this round.)
— Failure scenario: A diff containing scripts/deploy.sh whose script-lint report is missing/stale while all chunks receipt cleanly: cappedBy=['unreviewed-dimension'] only, scopeUnproven false, snapshot saw only build-and-test → failClosed false → the marker carries fetchedSha; the next round scopes past the sha and the script is never linted in any round. Same shape via layerAuditGate on a modeled-executable-system diff whose owed layer was never receipted.
witness: probe (recheck, vitest against the real module): isNonDiffDimensionGap('the executable-script lint — …') = false and ('reverse-audit layer coverage — …') = false, yet [].every(isNonDiffDimensionGap) = true and the gate entries never reach the classifier — for the stipulated round failClosed=false and sha = plan.fetchedSha.
Suggested fix: Decide from the same list the cap counts: compute dimensionGapsAreDepthOnly from unreviewed after the gate pushes (the splice has already run by then); gate entry heads fail isNonDiffDimensionGap, restoring pre-PR withholding.
中文说明
新的锚点豁免只基于 budget-phrase splice 之前的快照 unreviewedAsDisclosed（compose-review.ts:726）做分类，但 scriptLintGate（:825）与 layerAuditGate（:833）在快照之后才把机器侧欠账推入 unreviewed —— 仅被门禁缺口封顶的轮次，dimensionGapsAreDepthOnly 空判为 true，锚点照常写出。修复前 cappedBy.length > 0 会扣留这些情形。失败场景：含 scripts/deploy.sh 的 diff 在 script-lint 报告缺失/过期、分块回执干净时，cap 仅 unreviewed-dimension，failClosed=false，marker 携带 fetchedSha，后续轮次越过该 sha，该脚本永远不再被 lint。修复：在门禁推入之后（此时 splice 已运行）改用 unreviewed 计算 dimensionGapsAreDepthOnly；门禁条目头部无法通过 isNonDiffDimensionGap，即恢复修复前的扣留行为。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // ROUND FIRST, timestamp second. Recovery now crosses accounts, and the | ||
| // round counter is an id space: `compose-review` stamps this round's |
There was a problem hiding this comment.
[Critical] R4-1: Round-first selection ingests an UNBOUNDED round from ANY user's marker into the pipeline's global finding-id space — parseLedger validates only Number.isInteger(round) && round >= 1 (no upper cap), and this PR is what made foreign rounds authoritative.
— Failure scenario: Any GitHub user posts one review carrying {"v":1,"round":9007199254740991,"findings":[]}; round-first selection guarantees it wins every recovery; ledgerMarkerFor stamps prevRound+1 → 2^53 posts, then 2^53+1 collapses to 2^53 in float64 — every subsequent round permanently re-stamps ids R9007199254740992-n against different findings, corrupting Step-6 carried-id tracking.
witness: probe end-to-end (parseLedger → latestLedger → buildLedger → serializeLedger): round 9007199254740991 accepted, wins as foreign, next round stamps R9007199254740992-1, the round after collapses (2^53+1 === 2^53 true) and re-stamps the same ids; pipeline ids (R-) stay under LEDGER_MAX_ID, so truncation does not mask the collision.
Suggested fix: Add LEDGER_MAX_ROUND (e.g. 10_000) enforced in parseLedger (fail-quiet like the other malformations) and mirrored in serializeLedger.
中文说明
round-first 选择把任意用户 marker 中无上限的 round 吸入管线全局 finding-id 空间 —— parseLedger 只校验整数且 >= 1，没有上限；本 PR 使外部账号的 round 成为权威。失败场景：任何 GitHub 用户发一条 round=9007199254740991 的 marker 即永远胜出；ledgerMarkerFor 盖 prevRound+1 → 2^53 发布，再下一轮 2^53+1 在 float64 塌回 2^53 —— 从此每轮用同一组 R9007199254740992-n id 盖到不同发现上，永久破坏 Step 6 的按 id 跟踪。修复：增加 LEDGER_MAX_ROUND（如 10_000），在 parseLedger 安静失败并镜像到 serializeLedger。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| } catch { | ||
| prevLedger = null; | ||
| prevLedgerAuthor = null; | ||
| } |
There was a problem hiding this comment.
[Critical] R4-2: A transient recovery failure (currentUser() throw → catch sets prevLedger=null) leaves the machine-local prev-ledger side file stale, and ledgerMarkerFor stamps prevRound+1 from it unconditionally — with rounds K+1..K+m posted from another account meanwhile, the degraded machine re-issues a round number the PR already carries. Pre-PR unreachable: the side file only ever held own-account rounds (A/B probed).
— Failure scenario: Machine A's last successful pr-context wrote round K; rounds K+1..K+m post from the CI bot; machine A's next run hits a transient gh api user blip → catch → side file stays at K → compose stamps K+1, colliding with the bot's posted K+1; Step-6 carried-id tracking re-keys different findings under the same R<K+1>-n ids.
witness: probe at HEAD + pre-PR A/B: latestLedger([bot r1-3, maintainer r4,7,9], 'bot') → round 9 foreign; reconstructed pre-PR latestOwnLedger on the same input → round 3 (stale own copy stamped what a successful run would have stamped — no collision); latestLedger(…, null) → round 9 anchor-stripped, the cheap fallback the catch omits.
Suggested fix: Isolate currentUser() in its own try/catch and pass null login into latestLedger on failure — recovery then succeeds as foreign (work list travels, anchor stripped) instead of nulling the whole recovery.
中文说明
一次瞬时恢复失败（currentUser() 抛错 → catch 置 prevLedger=null）会让本机 prev-ledger side file 保持过期，而 ledgerMarkerFor 无条件按其 round+1 盖章 —— 若其间另一账号已发布 K+1..K+m 轮，降级的机器将重发已存在的轮号。修复前不可达：side file 只存本账号轮次（已做修复前 A/B 探针）。失败场景：机器 A 上次成功恢复写了 K 轮；CI bot 陆续发布 K+1..K+m；机器 A 下次运行 gh api user 瞬时抖动 → side file 停在 K → compose 盖 K+1，与 bot 已发布的 K+1 冲突，Step 6 按 id 跟踪把不同发现重挂到相同 R<K+1>-n。修复：把 currentUser() 单独 try/catch，失败时以 null login 调 latestLedger —— 恢复按外部成功（工作清单到达、锚点剥离），而不是整个恢复置空。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(p).toContain('testScope.notRun'); | ||
| expect(p).toContain('"clamped": true'); |
There was a problem hiding this comment.
[Suggestion] R4-12: The CONTINUE test's two whole-prompt toContain assertions ('testScope.notRun', '"clamped": true') are satisfied by the role-7 sibling brief bullet, not by the continuation prose this PR adds — deleting the entire new prose paragraph keeps the test green. The test's own comment documents this exact false-green shape as the reason for its redesign.
— Failure scenario: Mutant verified: delete the continuation prose paragraph (fenced --resume block kept) → the test still passes; the masked regression is an orphaned --resume block losing its intro conditions while the test reports the behavior pinned.
witness: mutant at HEAD: prose deleted → CONTINUE test 1 passed; both literals exist in the agent-briefs.ts:538-539 bullet which buildRoleBrief appends unconditionally for role 7.
Suggested fix: Scope the assertions to the new prose (assert a phrase only it contains, e.g. 'merging into the SAME report file'), or drop them and rely on the fence/path assertions plus one intro-sentence pin.
中文说明
CONTINUE 测试的两个整提示 toContain 断言（testScope.notRun、"clamped": true）由 role-7 简报兄弟条目满足，而非本 PR 新增的续跑散文 —— 删除整段散文测试仍绿。测试自己的注释就写明这种假绿形状正是它重构的原因，却留下了这两个未限定范围的断言。失败场景（突变体已验证）：删除续跑散文段（保留 --resume 围栏）测试仍通过；被掩盖的回归是 --resume 块失去引入条件而测试报告行为已钉住。修复：把断言限定到新散文独有的短语（如 “merging into the SAME report file”），或删除它们、依靠围栏/路径断言加一个引入句钉住。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // `resolve` — not spelled as POSIX literals: on Windows the prompt carries | ||
| // `C:\\abs\\tmp\\plan.json`, and a hardcoded expectation fails there for a | ||
| // reason that has nothing to do with the continuation block. | ||
| expect(resumeBlock[0]).toContain('review build-test'); |
There was a problem hiding this comment.
[Suggestion] R2-30: The CONTINUE test pins 'review build-test', --plan and --out on the resume fence but not the --worktree line (nor the backslash continuation structure), although the fence carries it and the test's own comment ('an agent that has to re-derive them gets them wrong') applies to it equally. (Round-2 finding, still standing.)
— Failure scenario: Drop the --worktree line from the continuation block: the test passes, but review build-test declares worktree with demandOption:true, so every resume call dies in yargs validation ('Missing required argument: worktree') and no continuation ever runs — the truncated-dimension failure this PR exists to prevent. Dropping the trailing backslash after --out executes build-test WITHOUT --resume.
witness: build-test.ts's yargs builder declares worktree demandOption:true; the fence carries the line (agent-prompt.ts:1320); nothing in the test asserts it.
Suggested fix: Add expect(resumeBlock[0]).toContain(--worktree ${resolve(PR_PLAN.worktreePath)}) (the sibling first-block test already pins it), or pin the block's exact deterministic text.
中文说明
CONTINUE 测试钉住了 resume 围栏的 review build-test、--plan 与 --out，却没钉 --worktree 行（围栏明明带着它，测试注释“需要重新推导的 agent 会推导错”同样适用）。失败场景：从续跑块删掉 --worktree 行，测试仍绿，但 build-test 对 worktree 声明了 demandOption: true，所有续跑都会死于 yargs 校验（“Missing required argument: worktree”），任何续跑都无法运行 —— 正是本 PR 要防止的维度截断；删掉 --out 行尾的反斜杠则会执行不带 --resume 的 build-test。修复：补上 --worktree 断言（兄弟首块测试已有该模式），或钉住整块确定性文本。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| (ledger.round === best.ledger.round && | ||
| (at > best.at || |
There was a problem hiding this comment.
[Suggestion] R3-34: Same-round timestamp ordering (at > best.at, pr-context.ts:710) is decided by no fixture: every multi-review fixture either differs in round or ties completely — and this PR's diff removed the one old fixture where rounds matched and timestamps differed. (Round-3 finding, still standing.)
— Failure scenario: Flip at > best.at to at < best.at inside the same-round branch: all 74 tests stay green; recovery then prefers the OLDER work list within a round.
witness: recheck at HEAD: multi-review tests vary round (1/3/9, 7/1, 2/3) or tie completely; no fixture exercises same-round distinct-timestamp.
Suggested fix: Add a fixture: two markers at the same round, distinct submitted_at, and assert the later timestamp wins.
中文说明
同轮时间戳排序（at > best.at，pr-context.ts:710）没有任何 fixture 覆盖：多 review fixture 要么轮次不同，要么完全平局 —— 本 PR 的 diff 还删掉了唯一轮次相同而时间戳不同的旧 fixture。失败场景：把同轮分支里的 at > best.at 翻转为 at < best.at，全部 74 个测试仍绿；恢复将在同轮内取更旧的工作清单。修复：补两条同轮、不同 submitted_at 的 marker，断言较新时间戳胜出。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ledger.round > best.ledger.round || | ||
| (ledger.round === best.ledger.round && | ||
| (at > best.at || | ||
| (at === best.at && (id > best.id || (id === best.id && !foreign))))); |
There was a problem hiding this comment.
[Suggestion] R2-23: The documented rule 'a tie between an own and a foreign review keeps the OWN one' is unreachable — the !foreign clause fires only when id === best.id, impossible between distinct GitHub reviews; its pinning test uses two reviews with the same id:7, an input the API cannot produce. (Round-2 finding, still standing.)
— Failure scenario: On any realistic same-second own/foreign tie the higher id wins regardless of ownership — the documented own-preference never applies; the test gives false confidence in a guarantee the code does not implement.
witness: recheck at HEAD: !foreign is evaluated only inside id === best.id; the tie test's reviews share id: 7.
Suggested fix: Either implement own-preference as a real tiebreak term (before or after id) and pin it with distinct ids, or delete the claim from the doc and the equal-id test.
中文说明
文档规则“本账号与外部 review 平局时保留本账号的”不可达 —— !foreign 只在 id === best.id 时触发，而不同 GitHub review 的 id 必不相同；钉住它的测试用了两条相同 id: 7 的 review，这是 API 不可能产生的输入。失败场景：任何真实的同秒 own/foreign 平局都由更高 id 胜出，与归属无关 —— 文档承诺的 own 优先从不生效，测试为一个代码未实现的保证提供假信心。修复：把 own 优先实现为真实的平局判定项并用不同 id 钉住；或从文档与测试中删除该声明。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| export function renderLedgerSection( | ||
| ledger: Ledger, | ||
| author: string | null = null, | ||
| ): string { |
There was a problem hiding this comment.
[Suggestion] R4-7: A foreign ledger whose winning review carries no user object renders as THIS account's own certified round — latestLedger returns {foreign: true, author: null}, the caller computes prevLedgerAuthor = null, and renderLedgerSection(ledger, null) takes the own-account branch. The sha is still stripped (security half holds); the damage is attribution prose on a rare input.
— Failure scenario: A deleted/ghost account or an API shape without user wins the recovery: the section reads 'recovered from the marker this account's last posted review carried' for another account's work list — the misreading the author parameter exists to prevent.
witness: RawReview.user is optional (pr-context.ts:65); caller :1043
prevLedgerAuthor = found?.foreign ? found.author : null→ null; renderLedgerSection's null branch is the own-account prose.
Suggested fix: Express the third state at the seam: pass foreign alongside author, or render a placeholder ('another account (login unknown)') when foreign && author === null.
中文说明
外部账本的胜出 review 若无 user 对象，会被渲染成本账号自己的已认证轮次 —— latestLedger 返回 {foreign: true, author: null}，调用方算出 prevLedgerAuthor = null，renderLedgerSection(ledger, null) 走本账号分支。sha 仍被剥离（安全半区完好），受损的是罕见输入下的归属措辞。修复：在接缝处表达第三态 —— 同时传 foreign，或在 foreign && author === null 时渲染占位（如 “another account (login unknown)”）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // is nothing for its answer to match against, so it is not made. It is | ||
| // still needed with the split trust surface — not to FIND the ledger, but | ||
| // to decide whether this one is ours and may carry an anchor. | ||
| const found = reviews.length ? latestLedger(reviews, currentUser()) : null; |
There was a problem hiding this comment.
[Suggestion] R4-13: A currentUser() failure aborts the whole ledger recovery — the catch sets prevLedger=null although latestLedger(reviews, null) defines the safe degraded mode (everything foreign, anchor stripped, tested). The diff's own comment concedes login is 'not to FIND the ledger, but to decide whether this one is ours'.
— Failure scenario: A transient gh api user blip, secondary rate limit, or a token without /user access during recovery degrades the round to full-range (the 119-min/34M-token waste this feature exists to eliminate) and the side file is not refreshed — on a fresh environment the round additionally resets to 1, re-issuing R1-* ids.
witness: not run — settling it would require mocking the gh network layer; the control flow is fully quoted: currentUser() (gh.ts:308, execFileSync, throws on non-zero) is evaluated eagerly inside the recovery try; the catch (:1044-1047) nulls the recovery; the null-login mode is tested ('treats an unknown login as foreign').
Suggested fix: Resolve login in its own try/catch and always run the recovery: on failure pass null into latestLedger (everything classifies foreign, anchor stripped — safe).
中文说明
currentUser() 失败会中止整个账本恢复 —— catch 置 prevLedger=null，尽管 latestLedger(reviews, null) 定义了安全降级模式（全部外部、锚点剥离、有测试）。diff 自己的注释也承认 login “不是用来找账本，而是用来判断是不是自己的”。失败场景：gh api user 瞬时抖动、次级限流或 token 无 /user 权限，即把本轮降级为全量审查（本特性要消除的 119 分钟/3400 万 token 浪费），且 side file 不刷新；全新环境下轮次还会重置为 1，重发 R1-* id。修复：把 currentUser() 放进单独的 try/catch，失败时向 latestLedger 传 null，恢复照常进行。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // dropped is missing from `shared` when base fails it too, and missing from | ||
| // `netNew` when only the PR side does — the direction that loses a failure | ||
| // the PR caused. Hence the capture-time field; this is the legacy seam. | ||
| const prTruncated = |
There was a problem hiding this comment.
[Suggestion] R4-14: The capture-time precedence pair disagrees about null: t.failingFiles ?? failingFilesOf(…) treats null as absent (lossy re-parse of the TRIMMED stored output) while t.failingFiles === undefined treats null as present — suppressing the prTruncated disclosure exactly when the fallback was lossy; a non-array value crashes runTestDelta at .filter. The report is ingested via unvalidated JSON.parse, unlike build-test's previousReport gate this PR wrote.
— Failure scenario: A hand-edited/repaired report (the documented corruption channel) with "failingFiles": null on a failing, trimmed command takes the lossy fallback yet leaves prTruncated false — an undercounted set reads as complete measurement; "failingFiles": "src/a.test.ts" crashes the whole delta with a raw TypeError.
witness: not run — the null/undefined semantics of ?? vs === undefined and .filter on a string are not in question; both read sites and the unguarded JSON.parse (test-delta.ts:222-228) quoted from HEAD; the producer never emits null/non-array, so only hand-edited reports trigger it.
Suggested fix: Gate the shape once and use it for both decisions: const captureSet = Array.isArray(t.failingFiles) ? t.failingFiles : null; then prefer captureSet and set prTruncated on captureSet === null.
中文说明
捕获时优先级对 null 的判断不一致：t.failingFiles ?? failingFilesOf(…) 把 null 当缺失（回退到重解析已截断的存储输出），而 t.failingFiles === undefined 把 null 当存在 —— 恰在回退有损时压制 prTruncated 披露；非数组值会在 runTestDelta 的 .filter 处崩溃。报告经未校验的 JSON.parse 载入，不同于本 PR 特意为 previousReport 写的门禁。失败场景：手写/修复的报告在失败且已截断的命令上带 "failingFiles": null，走有损回退却显示“测量完整”；带字符串值则整个 delta 裸 TypeError。修复：一次性按形状门控（Array.isArray），两处决策共用该结果。
— qwen3.8-max via Qwen Code /review (v0.21.12)
The dimension list the anchor decision reads is written at three different points — the caller's own entries, the budget-phrase splice that removes some of them, and the deterministic gates that push their machine-owed debts in later — and the last two rounds each fixed one end by breaking the other. The first version read after the splice and missed the entries it removed; the second read before the gates and missed an unlinted script or an unwalked defect layer, either of which is a line-coverage claim, not a dimension nobody could run. It now reads the live list at the decision point plus the entries the splice removed, which is the only view that sees every writer. Two more from making foreign markers authoritative. The round a marker claims is the id space itself, so preferring the highest round hands an unbounded one from any poster a permanent win — and past 2^53 the increment stops advancing, so every later round re-stamps the same ids against different findings. Rounds are now capped on read and mirrored on write, fail-quiet like every other malformation. And a transient identity lookup no longer costs the recovery: it used to degrade to "no ledger", which leaves this machine's side file at the round it last wrote while other accounts post past it, so the next verdict re-issues ids the pull request already carries. The lookup is isolated now, and a failure recovers the work list as foreign — no anchor rides on an identity the run could not confirm.
commented
Aug 15, 2026
|
Round 4's three blockers are fixed in 04a52a5. R1-14 is the same defect as R3-43 from the other side, and the pair is worth naming as one thing: the dimension list has three writers at three points — the caller's entries, the budget-phrase splice that removes some of them, and the deterministic gates that push machine-owed debts in later. Round 3 read after the splice and missed what it removed; round 3's fix read before the gates and missed an unlinted script or an unwalked defect layer. Neither of those is a dimension nobody could run, so neither may exempt the anchor. The decision now reads the live list at the decision point plus the entries the splice removed, which is the only view that sees every writer, and the classification of both gate shapes is pinned directly. R4-1 — correct, and it follows from making foreign markers authoritative: the round a marker claims IS the id space, so preferring the highest round hands an unbounded one a permanent win, and past 2^53 the increment stops advancing so every later round re-stamps the same ids. Capped on read and mirrored on write, fail-quiet like the other malformations. R4-2 — also correct, and the fallback you suggested is the right one. A transient identity lookup used to degrade the whole recovery to "no ledger", which leaves this machine's side file at the round it last wrote while other accounts post past it. The lookup is isolated now, and a failure recovers the work list as foreign — so no anchor rides on an identity the run could not confirm. On this round's suggestions: still deferred, same grounds. A note on where this stands, addressed to whoever reads the thread rather than to the reviewer. Four rounds in, the blocker counts are 9, 10, 5, 3 — converging, but three of the last eight blockers were introduced by the previous round's fix, and two of those were the same list read from two different wrong points. That is a signal about the diff, not about the review: it is one change touching an anchor rule, a budget model, a trust boundary, a classifier and a report format, and each round's repair lands in a file the other repairs also moved. I would rather land this as several changes that can each be judged on its own than keep converging a diff whose parts do not need each other. If the maintainer prefers that, the split is by finding and I will do it; if not, the current head is coherent and green apart from a pre-existing local test failure that CI does not reproduce. 中文说明第四轮的三条阻断项已在 04a52a5 修复。 R1-14 与第三轮的 R3-43 是同一个缺陷的另一面，这一对值得作为一件事来讲：维度列表有三个写入点——调用方自己的条目、按短语移除其中一部分的拼接、以及稍后推入机器判定欠账的确定性门。第三轮读得太晚，漏掉了拼接移除的条目；第三轮的修复读得太早，漏掉了未 lint 的脚本或未走查的缺陷层。这两者都不是"没人跑得动的维度"，因此都不能豁免锚点。判定现在读取决策点的实时列表加上被拼接移除的条目——这是唯一能看到全部写入点的视角——并直接钉住了两种门形态的分类结果。 R4-1 —— 正确，而且它正是"让外部标记具有权威性"的推论：标记声明的轮次就是 id 空间，因此优先取最高轮次等于把一个无上限的值永久交给任何发布者；越过 2^53 之后自增不再前进，于是后续每一轮都用同样的 id 重新盖章。现在读取时设上限、写入时镜像，与其他畸形输入一样静默失败。 R4-2 —— 同样正确，你建议的兜底就是对的做法。身份查询的瞬时失败此前会把整个恢复降级为"没有账本"，而这会让本机的 side file 停留在它上次写入的轮次，与此同时其他账号继续往前发布。现在该查询被单独隔离，失败时以外部身份恢复工作清单——因此不会有锚点建立在本次运行无法确认的身份之上。 本轮建议依然延后，理由不变。 关于当前进展的一点说明，写给读这条讨论串的人而非评审者。四轮下来阻断项计数是 9、10、5、3——在收敛，但最近八条里有三条是上一轮修复引入的，其中两条还是同一个列表从两个不同的错误位置读取。这是关于这个 diff 的信号，而不是关于这次评审的：它是一次同时触及锚点规则、预算模型、信任边界、分类器与报告格式的改动，而每一轮的修补都落在其他修补也动过的文件里。我更倾向把它作为若干各自可独立判断的改动落地，而不是继续收敛一个各部分彼此并不需要的 diff。如果维护者认可，拆分以 finding 为边界，我来做；如果不认可，当前 head 是自洽且全绿的，唯一的失败是 CI 无法复现的本机既有测试问题。 |
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) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, and 5 all reported findings).
Not explored to full depth (tool budget reached): chunk 1: running agent-prompt.test.ts and build-test.test.ts under vitest — the review worktree has no node_modules , so execution was skipped in favor of the line-…; "agent reverse-audit (round 5)": none — nothing was cut short..
[Critical] R2-12: (round-2 blocker, cache channel still standing; the anchor channel is fixed at this head) The Step-8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false) correctly loses the marker anchor, but the skill's net lets .qwen/review-cache/pr-<n>.json be written with lastCommitSha — the next same-SHA run short-circuits ('No new changes', Step-6 re-check skipped), and a later-SHA run scopes past lines the lens never read. The parenthetical 'the marker's anchor is withheld under exactly this net' is false at head. Witness: grep -c dimensionGapsAreDepthOnly SKILL.md → 0; the PR's own green test 'withholds the anchor when a dimension gap is about LINES' pins scopeUnproven=false, cappedBy=['unreviewed-dimension'], sha=undefined — the exact state in which the skill's net fires nothing. Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false. (Unanchorable: the target paragraphs are single ~1500-char lines the resolver cannot map.)
[Critical] R1-11: (round-1 blocker, re-confirmed at this head; relocated from inline — its anchor line run.ts:405 collides with the existing R1-24 thread) childEnv stamps process.argv[1] through isUnusableScriptEntry, whose extension gate tests only .js/.mjs/.cjs — in the npm run dev flow argv[1] is packages/cli/index.ts (a .ts file, scripts/dev.js:62), the gate returns false ('usable'), and the .ts path is stamped; every skill subcommand "${QWEN_CODE_CLI:-qwen}" review … then directly execs a TypeScript file the shell cannot run (exit 126) — dying for the whole review where pre-PR this branch wrote '' and the :- fallback worked. Witness: gate probe 'ts entry (dev flow)': false; sh -c '"<ts>" --version' → Permission denied, exit 126; empty-var fallback → exit 0. Fix: treat non-JS script extensions as unusable at the producer (or blank the stamp for anything not an executable JS/binary entry), restoring the '' fallback for the dev flow.
[Critical] R3-53: (round-3 blocker, re-confirmed at this head; relocated from inline again — the anchor line run.ts:405 collides with the existing round-1 thread) childEnv stamps a DIRECTORY argv[1] as the CLI entry — isUnusableScriptEntry has no file-ness check and the consumer applies the same gate, so nothing blanks the stamp; trigger: scripts/start.js pushes join(root, 'packages', 'cli') as argv[1] (npm start). Witness: QWEN_CODE_CLI=<dir> sh -c '"${QWEN_CODE_CLI:-qwen}" review match-remote' → 'Permission denied', EXIT=126; pre-PR '' stamp → EXIT=0. Fix: require statSync(resolve(ownEntry)).isFile() in addition to the gate; write '' otherwise.
[Critical] R5-6: (new this round; relocated from inline — its anchor line pr-context.ts:708 collides with the existing R2-23 thread) Round-first selection + the parse cap accepting round == LEDGER_MAX_ROUND (the gate rejects only >) + the write-side clamp Math.min(round, LEDGER_MAX_ROUND) + compose's uncapped prevRound + 1 stamping form a permanent attacker-reachable fixed point at round 10 000: any GitHub user posts one review carrying {"v":1,"round":10000,"findings":[]}; it wins every recovery regardless of author; compose stamps new findings R10001-<n>; serializeLedger clamps the posted marker back to 10 000; every subsequent round recovers 10 000 and re-stamps R10001-<n> against DIFFERENT findings — colliding ids permanently, superseded-by references ambiguous, the marker's round field not matching its own ids' prefix. Pre-PR unreachable (latestOwnLedger parsed only own markers). Witness: [probe] parseLedger accepts round 10000; latestLedger recovered round=10000 foreign=true — a forged OLDER marker beats own round-3; stamped ids R10001-1 in rounds 1 and 2 alike — permanent fixed point. Fix: reject raw.round >= LEDGER_MAX_ROUND in parseLedger (no legitimate marker sits at the cap), or detect prev.round >= cap in ledgerMarkerFor and refuse to stamp; add a test pairing a foreign round:10_000 marker with the next round's stamped ids.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and the platform-specific behavior of the changed suites was not exercised locally。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, and 5 all reported findings)。
未探索到全部深度（达到工具调用预算）：chunk 1：running agent-prompt.test.ts and build-test.test.ts under vitest — the review worktree has no node_modules , so execution was skipped in favor of the line-…；"agent reverse-audit (round 5)"：none — nothing was cut short.。
[Critical] R2-12: (round-2 blocker, cache channel still standing; the anchor channel is fixed at this head) The Step-8 cache-skip net this PR writes into SKILL.md (~1244, and the marker description at ~1270) omits the dimensionGapsAreDepthOnly === false clause the marker net applies (compose-review.ts:621-625). A whiffed whole-diff lens round (sole cap 'unreviewed-dimension', scopeUnproven false) correctly loses the marker anchor, but the skill's net lets .qwen/review-cache/pr-<n>.json be written with lastCommitSha — the next same-SHA run short-circuits ('No new changes', Step-6 re-check skipped), and a later-SHA run scopes past lines the lens never read. The parenthetical 'the marker's anchor is withheld under exactly this net' is false at head. Witness: grep -c dimensionGapsAreDepthOnly SKILL.md → 0; the PR's own green test 'withholds the anchor when a dimension gap is about LINES' pins scopeUnproven=false, cappedBy=['unreviewed-dimension'], sha=undefined — the exact state in which the skill's net fires nothing. Fix: add the marker's fourth clause to both SKILL.md paragraphs — also skip the cache write whenever dimensionGapsAreDepthOnly is false. (Unanchorable: the target paragraphs are single ~1500-char lines the resolver cannot map.)
[Critical] R1-11: (round-1 blocker, re-confirmed at this head; relocated from inline — its anchor line run.ts:405 collides with the existing R1-24 thread) childEnv stamps process.argv[1] through isUnusableScriptEntry, whose extension gate tests only .js/.mjs/.cjs — in the npm run dev flow argv[1] is packages/cli/index.ts (a .ts file, scripts/dev.js:62), the gate returns false ('usable'), and the .ts path is stamped; every skill subcommand "${QWEN_CODE_CLI:-qwen}" review … then directly execs a TypeScript file the shell cannot run (exit 126) — dying for the whole review where pre-PR this branch wrote '' and the :- fallback worked. Witness: gate probe 'ts entry (dev flow)': false; sh -c '"<ts>" --version' → Permission denied, exit 126; empty-var fallback → exit 0. Fix: treat non-JS script extensions as unusable at the producer (or blank the stamp for anything not an executable JS/binary entry), restoring the '' fallback for the dev flow.
[Critical] R3-53: (round-3 blocker, re-confirmed at this head; relocated from inline again — the anchor line run.ts:405 collides with the existing round-1 thread) childEnv stamps a DIRECTORY argv[1] as the CLI entry — isUnusableScriptEntry has no file-ness check and the consumer applies the same gate, so nothing blanks the stamp; trigger: scripts/start.js pushes join(root, 'packages', 'cli') as argv[1] (npm start). Witness: QWEN_CODE_CLI=<dir> sh -c '"${QWEN_CODE_CLI:-qwen}" review match-remote' → 'Permission denied', EXIT=126; pre-PR '' stamp → EXIT=0. Fix: require statSync(resolve(ownEntry)).isFile() in addition to the gate; write '' otherwise.
[Critical] R5-6: (new this round; relocated from inline — its anchor line pr-context.ts:708 collides with the existing R2-23 thread) Round-first selection + the parse cap accepting round == LEDGER_MAX_ROUND (the gate rejects only >) + the write-side clamp Math.min(round, LEDGER_MAX_ROUND) + compose's uncapped prevRound + 1 stamping form a permanent attacker-reachable fixed point at round 10 000: any GitHub user posts one review carrying {"v":1,"round":10000,"findings":[]}; it wins every recovery regardless of author; compose stamps new findings R10001-<n>; serializeLedger clamps the posted marker back to 10 000; every subsequent round recovers 10 000 and re-stamps R10001-<n> against DIFFERENT findings — colliding ids permanently, superseded-by references ambiguous, the marker's round field not matching its own ids' prefix. Pre-PR unreachable (latestOwnLedger parsed only own markers). Witness: [probe] parseLedger accepts round 10000; latestLedger recovered round=10000 foreign=true — a forged OLDER marker beats own round-3; stamped ids R10001-1 in rounds 1 and 2 alike — permanent fixed point. Fix: reject raw.round >= LEDGER_MAX_ROUND in parseLedger (no legitimate marker sits at the cap), or detect prev.round >= cap in ledgerMarkerFor and refuse to stamp; add a test pairing a foreign round:10_000 marker with the next round's stamped ids.
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * install and build inside a budget the caller sized for suites, and produce a | ||
| * report that looks like a complete run of a tree it never finished compiling. | ||
| */ | ||
| function previousReport(out: string | undefined): BuildTestReport { |
There was a problem hiding this comment.
[Critical] R1-15: --resume validates the report's SHAPE but not its PROVENANCE — BuildTestReport records no run identity (no sha/root/timestamp), the --out path is stable per PR across rounds, fetch-pr's cleanStale removes only the worktree and branch ref, and cleanup sweeps side files only post-review — a stale report from an interrupted earlier round is silently resumed. — Failure scenario: round 1 ends after build-test writes but before cleanup (a session kill/timeout/crash — precisely the interrupted states --resume exists for); round 2's brief-invited resume continues round 1's report on round 2's freshly re-created worktree: old-commit passing test[]/build[] entries are kept ("install and build reused"), install is skipped where node_modules may not exist, only notRun/clamped re-run — certifying old-commit passes for the new commit, or framing missing-install failures as PR Criticals. resumeNpmToolchain ignores args.changedFiles, so even a plan mismatch cannot trip it.
Witness:
[probe] resumed a round-1 report on a fresh tree B:
calls on fresh tree B=["npm test --workspace=\"packages/a\""]
old-commit entries kept=["npm test --workspace=\"packages/core\"", "npm test --workspace=\"packages/a\""]
ok=true; note="Continued from a previous build-test call (install and build reused: 1 build command(s) already ran). Every suite in scope has now run, and everything passed."
Suggested fix: stamp run identity into the report (the plan's fetchedSha or diff hash plus resolved root, which runBuildTest already reads) and refuse a report whose identity does not match the current --plan/--worktree; or discard the stale --out file at the start of a non-resume call.
中文说明
--resume 校验报告的「形状」但不校验其「来源」：BuildTestReport 不记录任何运行身份（无 sha/root/时间戳），--out 路径在每个 PR 的多轮审查间是稳定的，fetch-pr 的 cleanStale 只删除 worktree 和分支引用，cleanup 只在审查后清扫 side file——被中断的上一轮留下的过期报告会被静默续跑。
失败场景：第 1 轮在 build-test 写入之后、cleanup 之前结束（会话被杀/超时/崩溃——恰是 --resume 为之存在的中断状态）；第 2 轮按简报指示续跑，会把第 1 轮的报告续在第 2 轮刚重建的 worktree 上：旧提交通过的 test[]/build[] 条目被保留（"install and build reused"），node_modules 可能根本不存在却跳过了安装，只重跑 notRun/clamped——为新提交背书旧提交的通过，或把缺安装的失败包装成 PR 的 Critical。resumeNpmToolchain 完全忽略 args.changedFiles，连 plan 不匹配都绊不倒它。
探针证据见英文部分。修复建议：把运行身份写进报告（plan 的 fetchedSha 或 diff 哈希加解析后的 root，runBuildTest 本来就读取它们），让 previousReport 拒绝身份与当前 --plan/--worktree 不匹配的报告；或在非续跑调用开始时丢弃过期的 --out 文件。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // The anchor never crosses accounts. Dropped here, at the recovery seam, so | ||
| // no consumer downstream has to remember the rule. | ||
| const ledger = best.foreign ? stripAnchor(best.ledger) : best.ledger; |
There was a problem hiding this comment.
[Critical] R5-18: the trust split strips only sha, but a foreign marker's finding ids are pipeline-owned namespace too — parseLedger validates ids by LENGTH only (LEDGER_MAX_ID = 24), never by namespace — so any GitHub user can squat the next round's entire R<round+1>-* id prefix through the seam this diff opened. — Failure scenario: the pipeline's latest marker is public in the review body and shows round N; any user posts a review carrying {"v":1,"round":N,"findings":[{"id":"R<N+1>-1",…}, …]} (up to 50 ids) — it wins selection on the later submitted_at (or round-first at a higher round), stripAnchor removes only the sha, so round and ids ride into the prev-ledger side file; compose stamps this round's new findings starting at R<N+1>-1, directly into the squatted prefix; re-reported squatted entries enter buildLedger's taken set via CARRIED_ID_RE, so the same claim lives under its original id AND the attacker's id (breaking "R1-2 names the same claim in every round"), genuinely new findings are renumbered past the squatted block, and superseded by <class-id> references go ambiguous; the attacker re-posts after each run to persist it. Pre-PR unreachable: latestOwnLedger parsed only the reviewing account's markers.
Witness:
[probe] SELECTION: foreign=true round=3 sha=undefined
FOREIGN IDS RIDE THROUGH: R4-1, R4-2, R4-3 … 50 total
stamped ids: R4-1, R4-2 | collisions: R4-1, R4-2 ← probe fails HEAD
with a one-line seam fix (drop foreign ids matching ^R${round+1}-\d+$): 0 collisions
Suggested fix: at this seam, drop any foreign finding id matching ^R${ledger.round + 1}-\d+$ (or clear all foreign carried ids so re-reported entries are renumbered and cannot enter taken ahead of the stamps); add a test with a foreign marker whose finding ids equal R<round+1>-* asserting the next round's stamped ids do not collide.
中文说明
信任拆分只剥离 sha，但外来标记里的发现 id 同样属于管线自有的命名空间——parseLedger 只按长度校验 id（LEDGER_MAX_ID = 24），从不校验命名空间——因此任何 GitHub 用户都可以通过这个 diff 打开的接缝，抢注下一轮的全部 R<round+1>-* id 前缀。
失败场景：管线最新的标记公开在评审正文里、显示轮次 N；任何用户发一条携带 {"v":1,"round":N,"findings":[{"id":"R<N+1>-1",…}, …]}（最多 50 个 id）的评审——它会凭更晚的 submitted_at（或更高轮次）赢得选择，stripAnchor 只移除 sha，轮次与 id 原样进入 prev-ledger side file；compose 给本轮新发现盖章时从 R<N+1>-1 开始，直接撞进被抢注的前缀；被重新报告的抢注条目经 CARRIED_ID_RE 进入 buildLedger 的 taken 集合，同一主张同时存在于原 id 与攻击者 id 之下（破坏 "R1-2 在每一轮都指代同一主张"），真正的新发现被挤到抢注块之后重新编号，superseded by <class-id> 引用变得有歧义；攻击者每轮之后重发即可持久化。PR 之前不可达：latestOwnLedger 只解析审查账号自己的标记。
修复建议：在这个接缝处丢弃匹配 ^R${ledger.round + 1}-\d+$ 的外来发现 id（或清空所有外来 carried id，使重报条目重新编号、无法抢先进入 taken）；补一个测试：外来标记的发现 id 等于 R<round+1>-* 时，断言下一轮盖章的 id 不冲突。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if ( | ||
| !commandsOk(shape.test) || | ||
| !commandsOk(shape.build) || | ||
| !Array.isArray(shape.timedOut) || | ||
| !scopeOk | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R4-10: pattern — the --resume report gate omits fields and element shapes the merge walks. Confirmed instances this round: notBuilt (resumeNpmToolchain reads previous.notBuilt?.length and calls .join(', ') at npm-toolchain.ts:154/161) and install (installFailureFraming emits `undefined` exited undefined … prose from an empty record). — Concrete cost: a hand-edited/corrupted report at the stable per-PR --out path with "notBuilt": "packages/cli" clears every gate check; --resume then throws TypeError: previous.notBuilt.join is not a function — the raw stack trace the gate's own comment exists to replace (contained by the handler catch: exit 1, report untouched).
Witness:
[probe] notBuilt: "packages/cli" → TypeError: previous.notBuilt.join is not a function
with the fix applied → "build-test: --resume expected a build-test report … is not one. Run build-test without --resume first."
| if ( | |
| !commandsOk(shape.test) || | |
| !commandsOk(shape.build) || | |
| !Array.isArray(shape.timedOut) || | |
| !scopeOk | |
| ) { | |
| if ( | |
| !commandsOk(shape.test) || | |
| !commandsOk(shape.build) || | |
| !Array.isArray(shape.timedOut) || | |
| (shape as { notBuilt?: unknown }).notBuilt !== undefined && | |
| !Array.isArray((shape as { notBuilt?: unknown }).notBuilt) || | |
| !scopeOk | |
| ) { |
中文说明
模式问题：--resume 报告门禁漏掉了合并流程会遍历的字段与元素形状。本轮确认的实例：notBuilt（resumeNpmToolchain 在 npm-toolchain.ts:154/161 读取 previous.notBuilt?.length 并调用 .join(', ')）和 install（installFailureFraming 会从空记录生成 `undefined` exited undefined … 文案）。
具体代价：在稳定的 per-PR --out 路径上放一份手改/损坏、带 "notBuilt": "packages/cli" 的报告，可通过全部门禁检查；--resume 随后抛出裸 TypeError: previous.notBuilt.join is not a function——正是门禁注释声称要取代的裸栈（由 handler catch 兜底：exit 1，报告文件不受影响）。suggestion 块给出门禁补充（notBuilt 若存在必须是数组）。按你偏好的写法调整亦可（如单独的 installOk/notBuiltOk 子句）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| unreviewedDimensions: [ | ||
| 'security — the review time budget ended the round before the security relaunch returned evidence', | ||
| ], |
There was a problem hiding this comment.
[Suggestion] R4-5: the test 'sees a lens gap the budget-phrase splice removes from the rendered list' never arms the splice it names — its plan is coveredPlan(…) with no writeBudgetStop call, so readBudgetStop(input.planPath) returns null and the splice loop (compose-review.ts:746-751) is skipped entirely; the security entry is judged through the live unreviewed list. (Round-4 finding, still standing; mutant re-verified this round.) — Concrete cost: deleting the splicedForBudgetPhrase.push(unreviewed[i]) line — or mis-routing spliced entries — turns no test red; no test anywhere combines a stop marker with a parseLedger(...)?.sha assertion. The related Critical R5-1 ships through exactly this unasserted path.
Suggested fix: in that test, write the marker first (writeBudgetStop(plan, {…}, 2) beside the covered plan) so the splice actually fires; keep the same expectations.
中文说明
测试「sees a lens gap the budget-phrase splice removes from the rendered list」从未启动它名义上要测的拼接逻辑——其 plan 是 coveredPlan(…) 且没有调用 writeBudgetStop，于是 readBudgetStop(input.planPath) 返回 null，拼接循环（compose-review.ts:746-751）被整段跳过；security 条目实际是经由 live unreviewed 列表被判定的。（第 4 轮发现，仍然成立；本轮重新做了突变体验证。）
具体代价：删除 splicedForBudgetPhrase.push(unreviewed[i]) 一行——或把拼接后的条目接错方向——不会让任何测试变红；整个测试套件里没有任何测试把停止标记与 parseLedger(...)?.sha 断言组合起来。相关的 Critical R5-1 恰好经由这条未被断言的路径出闸。
修复：在该测试里先写入标记（在 covered plan 旁调用 writeBudgetStop(plan, {…}, 2)），让拼接真正发生；保持原期望不变。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| raw?.v !== 1 || | ||
| !Number.isInteger(raw.round) || | ||
| raw.round < 1 || | ||
| raw.round > LEDGER_MAX_ROUND |
There was a problem hiding this comment.
[Suggestion] R4-9: the new caps ship with zero test coverage at either site — the parse-side round rejection (only the 2^53 shape is pinned, not the 10_000/10_001 boundary), the write-side Math.min(ledger.round, LEDGER_MAX_ROUND) clamp (~166), and id.slice(0, LEDGER_MAX_ID) on both paths. (Round-4 finding, still standing; mutants re-run this round.) — Failure scenario: deleting the serializer clamp or both id slices keeps all 324 ledger/pr-context/compose-review tests green — the serializer can then emit what its own parser refuses (the round-trip-to-nothing the added comment names): a marker with round > 10000 posts, parseLedger returns null on recovery, and every subsequent round silently degrades to a full-diff re-review with no work list.
Witness:
[probe] mutant (delete Math.min clamp) → 324 passed
mutant (delete both id.slice sites) → 324 passed
Suggested fix: round-trip tests: round: 10_001 → parse null; round: 10_000 parses; an id longer than 24 chars round-trips truncated on both write and read.
中文说明
新的上限在两侧都没有任何测试覆盖——解析侧的轮次拒绝（只钉住了 2^53 形状，没钉 10_000/10_001 边界）、写入侧的 Math.min(ledger.round, LEDGER_MAX_ROUND) 钳制（~166）、以及两条路径上的 id.slice(0, LEDGER_MAX_ID)。（第 4 轮发现，仍然成立；本轮重跑了突变体。）
失败场景：删除序列化侧钳制或两处 id 切片，ledger/pr-context/compose-review 全部 324 条测试仍绿——序列化器随后可以发出它自己的解析器拒绝接收的东西（正是新增注释所说的「round-trip to nothing」）：round > 10000 的标记发出去了，恢复时 parseLedger 返回 null，之后每一轮都静默退化为无工作清单的全量 diff 重审。
修复：补 round-trip 测试：round: 10_001 → 解析为 null；round: 10_000 可解析；超过 24 字符的 id 在写入与读取两侧都被截断。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| return /^(?:the\s+)?build[-\s]?(?:and|&)[-\s]?test(?:\s+check|\s+verification)?$/.test( | ||
| head, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R5-23: both tolerance branches of the isNonDiffDimensionGap exemption regex — the optional the prefix and the (?:\s+check|\s+verification)? suffix — are pinned by no test: the only positive inputs anywhere in the suite are the bare build-and-test — … shape (4451, 4478, 4529) plus two non-matching entries. — Failure scenario: mutant deletes (?:\s+check|\s+verification)? (or (?:the\s+)?) → the entire suite stays green. An orchestrator entry naming the dimension the way the machine itself labels it — the build-and-test check is Agent 7's publicLabel (agent-briefs.ts:529), rendered in every posted body — then classifies as a lens line-coverage claim, failClosed flips true, the anchor is withheld, and the next round re-reviews the full diff: the exact waste this exemption exists to end, measured in this module's own comment at 119 minutes and 34M tokens. Direction is safe (over-withhold), but the cost class is the regression's target.
Witness:
[probe] mutant (both tolerance groups deleted) → 233/233 green
both suggested pins fail under the mutant, pass on pristine code
Suggested fix: add unit expectations beside the existing three: expect(isNonDiffDimensionGap('the build-and-test check — the integration job was skipped')).toBe(true) and expect(isNonDiffDimensionGap('build & test verification — the suites did not fit the budget')).toBe(true).
中文说明
isNonDiffDimensionGap 豁免正则的两个容错分支——可选的 the 前缀和 (?:\s+check|\s+verification)? 后缀——没有任何测试钉住：整个测试套件里唯一的正向输入是裸的 build-and-test — … 形状（4451、4478、4529）加两个不匹配条目。
失败场景：突变体删除 (?:\s+check|\s+verification)?（或 (?:the\s+)?）→ 整个套件仍绿。当编排器用机器自己的标签命名维度时——the build-and-test check 正是 Agent 7 的 publicLabel（agent-briefs.ts:529），出现在每份发布的正文里——该条目会被判为 lens 行覆盖主张，failClosed 翻为 true，锚点被扣留，下一轮重审整个 diff：正是这个豁免要终结的浪费，模块自己的注释实测为 119 分钟和 3400 万 token。方向安全（过度扣留），但代价类别正是该回归的靶子。
修复：在现有三条旁补两条单元期望（见英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| }, | ||
| }); | ||
|
|
||
| expect(rep.note).not.toContain('Every suite in scope has now run'); |
There was a problem hiding this comment.
[Suggestion] R3-42: the completion claim resumedNote owes a finished resume ("Every suite in scope has now run, and everything passed.") is asserted only negatively — its only two test-file occurrences (lines 3211, 3276) are not.toContain — so deleting the positive emission keeps the suite green. (Ledger R3-42, still standing; re-verified at HEAD.) — Failure scenario: a mutant removing the completion sentence from resumedNote's nothing-left branch passes every test: a fully-completed resumed run no longer receives the claim that distinguishes it from the contradictory-note shapes, and the prose the brief renders first loses its one positive completion signal.
Suggested fix: add a positive assertion in a fully-completed resume test: expect(rep.note).toContain('Every suite in scope has now run').
中文说明
resumedNote 欠一个「完成的续跑」的完成声明（"Every suite in scope has now run, and everything passed."）只有负向断言——它在测试文件里仅有的两处出现（3211、3276 行）都是 not.toContain——删除正向输出整套测试仍绿。（账本 R3-42，仍然成立；已在 HEAD 复核。）
失败场景：从 resumedNote 的「无剩余工作」分支删掉完成句的突变体可以通过所有测试：一个完整跑完的续跑将不再获得把它与自相矛盾 note 形状区分开来的声明，而简报最先渲染的文案失去唯一的正向完成信号。修复：在完整跑完的续跑测试里补一条正向断言。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| return withNote( | ||
| `Cannot resume the suites: the run being continued left ` + | ||
| `${previous.notBuilt.join(', ')} unbuilt, and a suite run against ` + |
There was a problem hiding this comment.
[Suggestion] R3-27: the notBuilt --resume refusal returns the report with the resume-trigger fields intact (testScope.notRun, clamped entries) while the refusal lives only in note prose — the brief bullet defines unfinished work by exactly those fields. (Ledger R3-27, still standing.) — Failure scenario: a literal agent sees non-empty testScope.notRun in the refused report, reads the bullet's "work is left AND continuable", and resumes again — refused again — before the note's "Re-run build-test without --resume" is followed; each loop iteration spends a tool window re-deriving the same refusal (the loop hazard is R5-8's territory; this is the structured-shape half).
Suggested fix: clear the resume-trigger fields in the refused report (notRun emptied, clamped entries marked final) so the structured shape agrees with the prose refusal, or make the bullet defer to the note on this shape.
中文说明
notBuilt 的 --resume 拒绝返回的报告里，续跑触发字段原封不动（testScope.notRun、clamped 条目），而拒绝只存在于 note 文案里——简报条目恰恰用这些字段定义「未完成的工作」。（账本 R3-27，仍然成立。）
失败场景：字面执行的 agent 看到被拒报告里非空的 testScope.notRun，读到条目的「有剩余工作且可续跑」，于是再次续跑——再次被拒——直到 note 里的 "Re-run build-test without --resume" 被遵循；每次循环都烧掉一个工具窗口去重新推导同一个拒绝（循环本身是 R5-8 的领域；这里是结构化形状那一半）。
修复：在被拒报告里清空续跑触发字段（清空 notRun、把 clamped 条目标记为最终态），使结构化形状与文案拒绝一致；或让条目在这种形状上以 note 为准。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| previous, | ||
| mergedTest, | ||
| timedOut, | ||
| [...stillPending, ...unattemptedRetries], |
There was a problem hiding this comment.
[Suggestion] R3-46: an unattempted retry is listed TWICE in the merged note — once in the "still to run — not run" sentence (~263, via [...stillPending, ...unattemptedRetries]) and again in the "still provisional" sentence (~347) — because its stale clamped entry survives in mergedTest, so it lands in stillClamped too. (Ledger R3-46, still standing; re-verified at HEAD.) — Failure scenario: the merged note names the same command in both the "still to run" and "still provisional" lists; each sentence is individually true, but an agent counting remaining work from the prose double-counts the retry, and the two sentences steer differently (one to resume for the pending, one to resume for the provisional).
Suggested fix: exclude unattemptedRetries from stillClamped (they were never re-run, so they are not provisional results), or merge the two disclosures into one sentence listing each command once.
中文说明
未尝试的重试在合并 note 中被列了两次——一次在 "still to run — not run" 句（~263，经由 [...stillPending, ...unattemptedRetries]），又一次在 "still provisional" 句（~347）——因为它陈旧的 clamped 条目存活在 mergedTest 里，于是也落进了 stillClamped。（账本 R3-46，仍然成立；已在 HEAD 复核。）
失败场景：合并 note 在 "still to run" 与 "still provisional" 两个列表里点名同一命令；每句话单独看都真，但从文案清点剩余工作的 agent 会把该重试重复计数，且两句话的指引方向不同（一个为 pending 续跑、一个为 provisional 续跑）。
修复：把 unattemptedRetries 从 stillClamped 中排除（它们从未被重跑，不是临时结果），或把两处披露合并为一句、每个命令只列一次。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // `\\` and `:` in the path class: a Windows runner prints | ||
| // `FAIL C:\\repo\\src\\x.test.ts`, which the POSIX-only class missed — | ||
| // and a missed parse is an unattributed failure, not a loud error. | ||
| /(?:^|\s)(?:FAIL\s+|❯\s+)(?:\|([^|]+)\|\s+)?([\w@.:\\/-]+\.(?:test|spec)\.[cm]?[jt]sx?)\b([^\n]*)/gm; |
There was a problem hiding this comment.
[Suggestion] R4-4: class-level — failing-file attribution is hand-rolled regex over runner rendered output whose entrance space includes arbitrary test stdout. (Ledger R4-4, still standing — two new demonstrated entrances this round: the content-blind FAIL branch, R5-4, probe-confirmed Critical/low in Needs Human Review; and the extension-boundary trim fabrication, R5-11.) — Failure scenario: any future runner formatting change — or a failing suite whose own output contains FAIL-shaped lines (this repo's review suites are saturated with them) — can fabricate or suppress attribution entries, and the failure surfaces as a wrong netNew/shared verdict rather than a parse error. Each entrance gets patched one at a time; the surface itself is unbounded.
Suggested fix: structural options: have build-test record per-file results from the runner's machine-readable output (vitest --reporter=json / jest --json) when available, keeping the regex as fallback only; or bound the parse to runner-owned lines (line-start anchoring plus a cross-check against the runner's own summary counts).
中文说明
类级问题：失败文件归因是对 runner 渲染输出的手写正则，其入口空间包括任意测试 stdout。（账本 R4-4，仍然成立——本轮新增两个被证实的入口：内容盲的 FAIL 分支 R5-4（探针确认，Critical/低置信，见 Needs Human Review）与扩展名边界截断捏造 R5-11。）
失败场景：任何未来 runner 格式变化——或某个失败套件自身输出里含有 FAIL 形状的行（本仓库的 review 测试套件里到处都是）——都可能捏造或吞掉归因条目，而失败以错误的 netNew/shared 裁决而非解析错误的形式暴露。入口只能逐个修补；这个面本身是无界的。
修复（结构性选项）：让 build-test 在可用时从 runner 的机器可读输出（vitest --reporter=json / jest --json）记录逐文件结果，正则仅作回退；或把解析约束到 runner 自有的行（行首锚定，并与 runner 自己的汇总计数交叉校验）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…the pipeline's namespace The fifth round found the two holes still open in the two boundaries this branch loosened, and both fixes are about identity rather than shape. A continuation trusted any well-shaped report at the out path, and that path is stable across review rounds while nothing sweeps it on an interrupted one — so the report a crashed round leaves behind is exactly what the next round's resume finds. Continuing it keeps the old commit's passing entries on the new round's tree, certifying old-commit passes for the new commit, and skips the install the fresh worktree never had. Every report now records the run it belongs to — the tree it ran in, and the commit its plan fetched — and a continuation refuses anything else, a report with no identity included: one that cannot prove it belongs here reads the same as one that provably does not. And the trust split stripped the anchor from a foreign marker but let its finding ids through, which hands any account the pipeline's own namespace: a marker at round N carrying ids from round N+1 pre-claims exactly the prefix the next compose stamps, splitting one claim across two ids and renumbering every genuinely new finding past the squatted block. A legitimate marker cannot claim an id from a future round — a round stamps its own ids and carries older ones forward — so the parser now drops any finding that does, read-side only, because the writer cannot produce the violation.
commented
Aug 15, 2026
|
Round 5's two blockers are fixed in 2118866. Both are identity holes in the two boundaries this branch loosened, and both probes were right. R1-15 — the strongest finding across all five rounds, because the failure it names is the feature working as built: an interrupted round leaves a well-shaped report at a path that is stable across rounds, and the next round's brief invites the resume. Every report now records the run it belongs to — the tree it ran in and the commit its plan fetched — and a continuation refuses anything else, including a report with no identity at all: one that cannot prove it belongs here reads the same as one that provably does not. The delegation test's "the returned report IS the adapter's report" pin is updated to "the adapter's report plus exactly the identity stamp", which is now the truer statement. R5-18 — correct, and it completes the trust split this branch opened: the anchor was stripped, but ids are pipeline-owned namespace too, and round-first selection (my round-3 fix) is what gave a squatting marker its guaranteed win. The invariant is structural — a legitimate marker cannot carry an id from a future round, because a round stamps its own ids and carries older ones forward — so the parser drops violators, read-side only, since the writer cannot produce the violation. Suggestions remain deferred, same grounds. Two of them (the splice test that never arms the splice, the gate clause pinned by nothing) are test-strength debts from my own review-round fixes and are first in line for the follow-up. Blocker counts across rounds: 9, 10, 5, 3, 2 — and this round's two are the last of the round-1 findings still standing (R1-15) plus the closure of the namespace half of the trust split. The surface this branch opened is now guarded at both boundaries it loosened: what a continuation may trust, and what a foreign marker may claim. 中文说明第五轮的两条阻断项已在 2118866 修复。两条都是本分支放宽的两个边界上的身份漏洞，两个探针都是对的。 R1-15 —— 五轮里最有力的一条，因为它命名的失败恰恰是"功能按设计工作"的结果：被中断的轮次会在一个跨轮次稳定的路径上留下一份形状完好的报告，而下一轮的 brief 恰好邀请续跑它。现在每份报告都记录它所属的运行——运行所在的树、以及 plan 取回的提交——续跑拒绝其它一切，包括完全没有身份的报告：无法证明属于这里的报告，与可证明不属于这里的报告读法相同。委托测试里"返回的报告就是适配器的报告"这条钉子改为"适配器的报告加上且仅加上身份戳"，这是现在更真实的陈述。 R5-18 —— 正确，而且它补全了本分支打开的信任拆分：锚点被剥掉了，但 id 同样是管线自有的命名空间，而轮次优先选择（我第三轮的修复）恰恰给了抢占型 marker 必胜的地位。这条不变量是结构性的——合法 marker 不可能携带来自未来轮次的 id，因为每一轮盖自己的 id、并把更早的向前携带——所以解析器丢弃违例者，只在读取侧，因为写入侧根本产生不了违例。 建议依然延后，理由不变。其中两条（从未触发拼接的拼接测试、没有任何东西钉住的门条款）是我自己审查轮修复欠下的测试强度债，排在后续改动的最前面。 五轮阻断项计数：9、10、5、3、2——本轮两条分别是第一轮仍然站立的最后一条（R1-15），以及信任拆分中命名空间那一半的收口。本分支放宽的两个边界——续跑可以信任什么、外部 marker 可以主张什么——现在都设了守卫。 |
…efects # Conflicts: # packages/cli/src/commands/review/pr-context.test.ts # packages/cli/src/commands/review/pr-context.ts
Two crash shapes from the sixth round, both the same lesson one level deeper than the round-three fix that checked the lists: the elements are the payload. A continuation's not-run entries become shell commands, so a list holding a null cleared an arrays-only guard and crashed in the escaper instead of refusing with the named fix. And the failing-file set rides a report file anything may have edited, with exactly one consumer — a set that is not a string array reached the set arithmetic as-is, where the honest reading is the one an absent field has always taken: this seam supplied no measurement, fall back to re-parsing the stored output.
commented
Aug 15, 2026
|
A timing note first, then this round's disposition. The two Criticals this round filed — R1-15 and R5-18 — were reviewed on commit 04a52a5, which predates the commit that fixed them. The review's own Of the twenty-one suggestions, I took two now rather than deferring them, because they are crash-shaped correctness defects in code this branch adds — the same class as the round-three blocker, one level deeper — and the standing rule I am holding to defers polish, not correctness. R5-7: the continuation gate validated that the scope's lists are arrays but not what is in them, and a not-run entry becomes a shell command, so a list holding a null cleared the gate and crashed in the escaper instead of refusing with the named fix. R5-12: the failing-file set rides a report file anything may have edited, with exactly one consumer, and a set that is not a string array reached the set arithmetic as-is — the honest reading is the one an absent field has always taken: no measurement from this seam, fall back to re-parsing the stored output. Both fixed in a3c70ae, with the corrupt shapes added to the refusal table and a malformed-set test on the delta side. The remaining nineteen stay deferred on the standing grounds. The largest cluster is test-strength debt on my own review-round fixes (assertions satisfiable outside the block they guard, gate clauses no fixture pins, a completion claim asserted only negatively) plus one class-level finding (R4-4, the failing-file parser as an unbounded hand-rolled surface) that deserves its own change with its own review rather than a ninth commit here. 中文说明先说一个时序问题，再说本轮的处置。 本轮提出的两条 Critical——R1-15 与 R5-18——审查的是提交 04a52a5，而修复它们的提交晚于该基准。 审查自己的 二十一条建议里，我这次取了两条而不是延后，因为它们是本分支新增代码中崩溃形态的正确性缺陷——与第三轮那条阻断项同类、只是更深一层——而我坚持的规则延后的是打磨，不是正确性。R5-7：续跑门校验了 scope 的列表是数组，却没校验里面装的是什么，而 not-run 条目会变成 shell 命令，于是一个装着 null 的列表穿过了门，在转义器里崩溃，而不是带着指名修法拒绝。R5-12：失败文件集挂在一份任何人都可能编辑过的报告文件上，且只有一个消费者，一个不是字符串数组的集合会原样进入集合运算——诚实的读法是缺失字段一直以来的读法：此缝隙未提供测量，回落到重新解析存储的输出。两者已在 a3c70ae 修复，损坏形态加入了拒绝表，delta 侧补了畸形集合测试。 其余十九条按既定理由继续延后。最大的一组是我自己审查轮修复欠下的测试强度债（可被守护块之外文本满足的断言、没有任何 fixture 钉住的门条款、只从否定方向断言的完成声明），外加一条类级发现（R4-4，失败文件解析器作为无界手写面），它值得一次有自己审查的独立改动，而不是这里的第九个提交。 |
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally.
Not explored to full depth (tool budget reached): chunk 12: executed unit-test run of pr-context.test.ts / pr-context-persist.test.ts (blocked by broken node_modules in the review worktree, unrelated to the PR).
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：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。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally。
未探索到全部深度（达到工具调用预算）：chunk 12：executed unit-test run of pr-context.test.ts / pr-context-persist.test.ts (blocked by broken node_modules in the review worktree, unrelated to the PR)。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const dropped = | ||
| (ledger.dropped ?? 0) + | ||
| (bestOwn.ledger.dropped ?? 0) + | ||
| (merged.length - capped.length); |
There was a problem hiding this comment.
[Suggestion] When a foreign marker wins recovery and merges OVER this account's own findings, the rendered section misattributes provenance in two ways: the PARTIAL-list clause sums dropped across both markers but attributes the total to the single rendered round (the foreign winner's), and renderLedgerSection attributes the entire merged list — including this account's own certified entries — to the foreign author. — Concrete cost: own round-7 marker truncated at post (dropped: 3), foreign round-8 marker wins inside headroom → the context renders PARTIAL: 3 further finding(s) from round 8 did not fit the marker's size cap — asserting round 8 lost findings when it was round 7 — under THEIR claims for a list that is mostly this account's own work. A Step-6 reader reconciling entries against the named round's review body finds round 8 complete and may dismiss the partiality warning as stale.
Witness (probe through the real recoverLedger + renderLedgerSection; flip check: disabling the merge removed both misattributions):
Round 8, recovered from the marker **@stranger**'s last posted review carried — another account, so these are THEIR claims…
**This list is PARTIAL**: 3 further finding(s) from round 8 did not fit the marker's size cap
(with the account's own certified R7-1 Critical among the rows)
Suggested fix: when the merge branch fires, mark the ledger as merged (a flag or a distinct render path) and render further finding(s) from earlier rounds instead of from round <winner>; phrase the section as a merged work list — "this account's latest findings, merged with @author's round-N marker; no incremental anchor travelled with the foreign marker".
中文说明
当一个外部账号的标记赢得恢复并合并到本账号自己的发现之上时，渲染出的章节在两处误标来源：PARTIAL 清单子句把两个标记的 dropped 相加，却把总数归到唯一渲染轮次（外部获胜者的轮次）名下；renderLedgerSection 把整份合并清单——包括本账号自己已确认的条目——都归到外部作者名下（“这些是他们的主张”）。
具体代价：本账号第 7 轮标记在发布时被截断（dropped: 3），外部第 8 轮标记在 headroom 内获胜 → 上下文渲染出 “PARTIAL: 3 further finding(s) from round 8 did not fit the marker's size cap”——断言第 8 轮丢了发现，而实际是第 7 轮——并且对一份大部分是本账号自己工作的清单标注 “THEIR claims”。第 6 步的读者把条目与所指轮次的审查正文核对时，会发现第 8 轮是完整的，从而可能把这个“清单不完整”的警告当作过期信息忽略掉。
证据：对真实的 recoverLedger + renderLedgerSection 的探针（上方引文）；翻转检验：禁用合并后两处误标同时消失。
修复建议：合并分支触发时给账本打上“已合并”标记（一个标志位或独立的渲染路径），渲染 “further finding(s) from earlier rounds” 而不是 “from round <获胜轮次>”；把该章节表述为合并后的工作清单——“本账号的最新发现，与 @author 的第 N 轮标记合并；外部标记未携带任何增量锚点”。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const neverTested = previous.test.length === 0 && !previous.testScope; | ||
| return withNote( | ||
| neverTested |
There was a problem hiding this comment.
[Suggestion] neverTested conflates a COMPLETED zero-suite run with a run that ended before its test phase. — Failure scenario: a single-root package with no test script finishes a fresh run completely (test: [], no testScope, ok: true, note …the package defines no test script, so no tests ran); --resume on that report appends Nothing to resume: the run being continued ended before its test phase, so it left no scope to continue — no suite ran. Re-run build-test without --resume. — the claim is false (the test phase ran and legitimately found nothing to run), self-contradictory beside the carried note, and the re-run advice re-derives the same zero-suite answer at the cost of a full fresh install+build call. Note: 7f73fe8 (cited as addressing this shape) INTRODUCED this branch, and its test covers only the install-failed/aborted shape — the completed-no-test-script shape remains uncovered. Distinct from R1-13 (the budget-stop shape, for which the message is accurate).
Witness (probe at HEAD; flip check: adding && !previous.ok to neverTested changed the appendage to reached every suite in scope):
Built 1 of 1 workspaces … but the package defines no test script, so no tests ran. Nothing to resume: the run being continued ended before its test phase, so it left no scope to continue — no suite ran. Re-run build-test without --resume.
Suggested fix: distinguish the completed-zero-suite case before choosing the message — require evidence the run stopped early before asserting ended before its test phase (the probe used && previous.ok === false as a demonstration; whichever condition is chosen must keep the budget-stop shape — where the re-run advice IS useful — on this branch), and for a completed zero-suite run report Nothing to resume: the run being continued completed with no suite to run. without the re-run instruction.
中文说明
neverTested 把“完整跑完、但本来就没有套件可跑”的运行与“在测试阶段之前就结束”的运行混为一谈。
失败场景：一个没有 test 脚本的单根包完整地跑完了一次全新运行（test: []、没有 testScope、ok: true、note 写着 “……该包未定义 test 脚本，因此没有运行任何测试”）；对这份报告执行 --resume 会追加 “Nothing to resume: the run being continued ended before its test phase, so it left no scope to continue — no suite ran. Re-run build-test without --resume.”——这个断言是假的（测试阶段跑过了，只是确实没有东西可跑），与携带的 note 自相矛盾，而且“请不用 --resume 重跑”的建议会花一次完整的 install+build 调用重新推导出同样的零套件答案。注意：7f73fe8056（被引用为处理此形状的提交）恰恰引入了这个分支，其测试只覆盖 install 失败/中止的形状——“完整跑完但没有 test 脚本”的形状仍未被覆盖。与 R1-13（预算停止形状，对该形状此消息是准确的）不同。
证据：HEAD 上的探针（上方引文）；翻转检验：给 neverTested 加上 && !previous.ok 后，追加文本变为 “reached every suite in scope”。
修复建议：在选择消息之前先区分“完整跑完的零套件”情形——在断言 “ended before its test phase” 之前要求存在运行提前停止的证据（探针用 && previous.ok === false 作演示；无论选择哪个条件，都必须让预算停止形状——其重跑建议是有用的——留在本分支），对完整跑完的零套件运行则报告 “Nothing to resume: the run being continued completed with no suite to run.”，且不带重跑指令。
— qwen3.8-max via Qwen Code /review (v0.21.11)
commented
Aug 16, 2026
|
This round reports no blocker — nothing to fix under the Criticals-only rule now governing this PR. Both Suggestions are acknowledged as real (each carries a probe and a flip check) and deferred to the follow-up change with the rest of the backlog: the merged-ledger provenance rendering (dropped-count attribution and THEIR-claims phrasing over a merged list), and the 中文说明本轮无阻断项——按本 PR 现行的「只修 Critical」规则，无需改动。 两条 Suggestion 均确认为真实发现（各有探针与翻转检验），与既有积压一起延后到后续改动：合并账本的来源渲染（dropped 计数归属与对合并清单的 THEIR-claims 表述）、以及 |
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) and Test (windows-latest, Node 22.x) were skipped in CI; the changed workspaces' suites ran locally on Linux only.
Not explored to full depth (tool budget reached): chunk 7: none — but note one substitution: I could not run the actual vitest suites (no node_modules in the review worktree); I substituted direct execution of the ext….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
[Critical] R1-11 (round-1 blocker, re-checked at head e3e939d — still stands, probe-confirmed this round): the isUnusableScriptEntry gate shared by producer and consumer answers "usable" by extension enumeration (.js/.mjs/.cjs only) instead of positive exec evidence — any other path returns false ("usable") without an isFile()/X_OK/shebang check. The new childEnv() stamp (run.ts:405) can therefore hand skill subcommands an entry a shell cannot exec. Two demonstrated entrances, both probe-verified this round: (1) npm run dev -- review run <pr>: under tsx, process.argv[1] is packages/cli/index.ts (mode 100644); the gate answers "usable", the .ts path is stamped, and every "${QWEN_CODE_CLI:-qwen}" review … subcommand dies with Permission denied / exit 126 (pre-diff this case wrote '' and the qwen fallback survived); (2) node packages/cli sets argv[1] to the DIRECTORY, which the same early return also answers "usable" for — exec'ing it dies exit 126, and wiring .ts extensions into the regex does not close it. Fix: require positive evidence of shell-executability at the gate (treat non-regular files as unusable; script paths require X_OK plus a #! header); otherwise write ''.
中文说明
[Critical] R1-11（第 1 轮阻塞项，在 head e3e939d 上复查——仍然成立，本轮经探针确认）：生产端/消费端共享的 isUnusableScriptEntry 门禁按扩展名枚举（仅 .js/.mjs/.cjs）回答「可用」，而不是要求可执行的正面证据——任何其他路径都不做 isFile()/X_OK/shebang 检查便返回 false（「可用」）。新的 childEnv() 印章（run.ts:405）因此可能把一个 shell 无法执行的入口交给技能子命令。两条已演示的入口，均经本轮探针验证：(1) npm run dev -- review run <pr>：tsx 下 process.argv[1] 是 packages/cli/index.ts（mode 100644）；门禁回答「可用」，.ts 路径被盖章，每个 "${QWEN_CODE_CLI:-qwen}" review … 子命令都死于 Permission denied / exit 126（改动前这种情形写 ''，qwen 回退得以保留）；(2) node packages/cli 把 argv[1] 设为目录，同一个提前返回也回答「可用」——执行它死于 exit 126，且把 .ts 扩展名接入正则并不能堵住它。修复：在门禁处要求可执行的正面证据（非常规文件视为不可用；脚本路径要求 X_OK 加 #! 头），否则写 ''。
[Critical] R3-38 (round-3 blocker, re-checked at head e3e939d — still stands): --resume executes report-stored test[].command strings VERBATIM for clamped retries (npm-toolchain.ts:178-189) — no grammar gate — while the same codebase grammar-checks report-derived commands before re-executing them in test-delta (RERUNNABLE_COMMAND_RE, test-delta.ts:71). The resume shape gate validates element SHAPE (non-empty command string) but not grammar; the run-identity check admits any report matching this run's root/sha/tree/plan — including one hand-edited in place, which the gate's own corrupt-report fixtures declare the accepted threat model. — Failure scenario: a hand-edited or agent-corrupted report with test: [{ command: 'npm test; curl …', clamped: true }] matching the run identity passes the gate and the injected command executes under shell: true. Fix: apply the emitter's grammar (/^npm test(?: --workspace="[\w@./-]+")?$/) to retry commands before executing; refuse with a named fix otherwise.
中文说明
[Critical] R3-38（第 3 轮阻塞项，在 head e3e939d 上复查——仍然成立）：--resume 对 clamped 重试逐字执行报告中存储的 test[].command 字符串（npm-toolchain.ts:178-189）——没有语法门禁——而同一代码库在 test-delta 里对报告派生命令重执行前会做语法检查（RERUNNABLE_COMMAND_RE，test-delta.ts:71）。续跑形状门禁验证元素形状（非空命令字符串），但不验证语法；运行身份检查会接纳任何与本运行 root/sha/tree/plan 匹配的报告——包括被就地手工编辑的报告，而门禁自己的损坏报告夹具已声明这是其接受的威胁模型。— 失败场景：一份匹配运行身份、被手工编辑或被 agent 损坏的报告，其 test: [{ command: 'npm test; curl …', clamped: true }] 能通过门禁，注入的命令将在 shell: true 下执行。修复：对重试命令在执行前套用发出者语法（/^npm test(?: --workspace="[\w@./-]+")?$/），否则以指明修复方式拒绝。
[Critical] identityKnown empty-login (blocker re-checked at head e3e939d — still stands; mechanism probe-proven this round, production trigger frequency uncertain): identityKnown is set true for an EMPTY-STRING login (pr-context.ts:1381) while recoverLedger treats '' as an unknown identity (me = login ? … : null). Probe this round: a stub gh answering api user with empty output and exit 0 yields {login:'', identityKnown:true, sawOwnReview:false, deletionFlag:true} — the prev-ledger side file is deleted over an identity that was never proven, resetting the round counter; control with a real login: flag=false. Codebase precedent treats '' as unknown (presubmit.ts:500); the added test covers only the throw case. Trigger requires a stubbed/proxy/GHES gh answering empty-with-exit-0 — plausible in varied environments, not observed in production. Fix: identityKnown = login !== ''.
中文说明
[Critical] identityKnown 空登录（阻塞项在 head e3e939d 上复查——仍然成立；机制已于本轮经探针证明，生产触发频率不确定）：identityKnown 对空字符串登录被置为 true（pr-context.ts:1381），而 recoverLedger 把 '' 视为未知身份（me = login ? … : null）。本轮探针：一个对 api user 以空输出、exit 0 应答的桩 gh 得到 {login:'', identityKnown:true, sawOwnReview:false, deletionFlag:true}——prev-ledger side 文件在一个从未被证明的身份之上被删除，轮次计数器被重置；以真实登录作对照：flag=false。代码库先例把 '' 视为未知（presubmit.ts:500）；新增测试只覆盖了抛错情形。触发需要一个以空输出、exit 0 应答的桩/代理/GHES gh——在多样环境中可信，但未在生产中观察到。修复：identityKnown = login !== ''。
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the changed workspaces' suites ran locally on Linux only。
未探索到全部深度（达到工具调用预算）：chunk 7：none — but note one substitution: I could not run the actual vitest suites (no node_modules in the review worktree); I substituted direct execution of the ext…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
[Critical] R1-11 (round-1 blocker, re-checked at head e3e939d — still stands, probe-confirmed this round): the isUnusableScriptEntry gate shared by producer and consumer answers "usable" by extension enumeration (.js/.mjs/.cjs only) instead of positive exec evidence — any other path returns false ("usable") without an isFile()/X_OK/shebang check. The new childEnv() stamp (run.ts:405) can therefore hand skill subcommands an entry a shell cannot exec. Two demonstrated entrances, both probe-verified this round: (1) npm run dev -- review run <pr>: under tsx, process.argv[1] is packages/cli/index.ts (mode 100644); the gate answers "usable", the .ts path is stamped, and every "${QWEN_CODE_CLI:-qwen}" review … subcommand dies with Permission denied / exit 126 (pre-diff this case wrote '' and the qwen fallback survived); (2) node packages/cli sets argv[1] to the DIRECTORY, which the same early return also answers "usable" for — exec'ing it dies exit 126, and wiring .ts extensions into the regex does not close it. Fix: require positive evidence of shell-executability at the gate (treat non-regular files as unusable; script paths require X_OK plus a #! header); otherwise write ''.
中文说明
[Critical] R1-11（第 1 轮阻塞项，在 head e3e939d 上复查——仍然成立，本轮经探针确认）：生产端/消费端共享的 isUnusableScriptEntry 门禁按扩展名枚举（仅 .js/.mjs/.cjs）回答「可用」，而不是要求可执行的正面证据——任何其他路径都不做 isFile()/X_OK/shebang 检查便返回 false（「可用」）。新的 childEnv() 印章（run.ts:405）因此可能把一个 shell 无法执行的入口交给技能子命令。两条已演示的入口，均经本轮探针验证：(1) npm run dev -- review run <pr>：tsx 下 process.argv[1] 是 packages/cli/index.ts（mode 100644）；门禁回答「可用」，.ts 路径被盖章，每个 "${QWEN_CODE_CLI:-qwen}" review … 子命令都死于 Permission denied / exit 126（改动前这种情形写 ''，qwen 回退得以保留）；(2) node packages/cli 把 argv[1] 设为目录，同一个提前返回也回答「可用」——执行它死于 exit 126，且把 .ts 扩展名接入正则并不能堵住它。修复：在门禁处要求可执行的正面证据（非常规文件视为不可用；脚本路径要求 X_OK 加 #! 头），否则写 ''。
[Critical] R3-38 (round-3 blocker, re-checked at head e3e939d — still stands): --resume executes report-stored test[].command strings VERBATIM for clamped retries (npm-toolchain.ts:178-189) — no grammar gate — while the same codebase grammar-checks report-derived commands before re-executing them in test-delta (RERUNNABLE_COMMAND_RE, test-delta.ts:71). The resume shape gate validates element SHAPE (non-empty command string) but not grammar; the run-identity check admits any report matching this run's root/sha/tree/plan — including one hand-edited in place, which the gate's own corrupt-report fixtures declare the accepted threat model. — Failure scenario: a hand-edited or agent-corrupted report with test: [{ command: 'npm test; curl …', clamped: true }] matching the run identity passes the gate and the injected command executes under shell: true. Fix: apply the emitter's grammar (/^npm test(?: --workspace="[\w@./-]+")?$/) to retry commands before executing; refuse with a named fix otherwise.
中文说明
[Critical] R3-38（第 3 轮阻塞项，在 head e3e939d 上复查——仍然成立）：--resume 对 clamped 重试逐字执行报告中存储的 test[].command 字符串（npm-toolchain.ts:178-189）——没有语法门禁——而同一代码库在 test-delta 里对报告派生命令重执行前会做语法检查（RERUNNABLE_COMMAND_RE，test-delta.ts:71）。续跑形状门禁验证元素形状（非空命令字符串），但不验证语法；运行身份检查会接纳任何与本运行 root/sha/tree/plan 匹配的报告——包括被就地手工编辑的报告，而门禁自己的损坏报告夹具已声明这是其接受的威胁模型。— 失败场景：一份匹配运行身份、被手工编辑或被 agent 损坏的报告，其 test: [{ command: 'npm test; curl …', clamped: true }] 能通过门禁，注入的命令将在 shell: true 下执行。修复：对重试命令在执行前套用发出者语法（/^npm test(?: --workspace="[\w@./-]+")?$/），否则以指明修复方式拒绝。
[Critical] identityKnown empty-login (blocker re-checked at head e3e939d — still stands; mechanism probe-proven this round, production trigger frequency uncertain): identityKnown is set true for an EMPTY-STRING login (pr-context.ts:1381) while recoverLedger treats '' as an unknown identity (me = login ? … : null). Probe this round: a stub gh answering api user with empty output and exit 0 yields {login:'', identityKnown:true, sawOwnReview:false, deletionFlag:true} — the prev-ledger side file is deleted over an identity that was never proven, resetting the round counter; control with a real login: flag=false. Codebase precedent treats '' as unknown (presubmit.ts:500); the added test covers only the throw case. Trigger requires a stubbed/proxy/GHES gh answering empty-with-exit-0 — plausible in varied environments, not observed in production. Fix: identityKnown = login !== ''.
中文说明
[Critical] identityKnown 空登录（阻塞项在 head e3e939d 上复查——仍然成立；机制已于本轮经探针证明，生产触发频率不确定）：identityKnown 对空字符串登录被置为 true（pr-context.ts:1381），而 recoverLedger 把 '' 视为未知身份（me = login ? … : null）。本轮探针：一个对 api user 以空输出、exit 0 应答的桩 gh 得到 {login:'', identityKnown:true, sawOwnReview:false, deletionFlag:true}——prev-ledger side 文件在一个从未被证明的身份之上被删除，轮次计数器被重置；以真实登录作对照：flag=false。代码库先例把 '' 视为未知（presubmit.ts:500）；新增测试只覆盖了抛错情形。触发需要一个以空输出、exit 0 应答的桩/代理/GHES gh——在多样环境中可信，但未在生产中观察到。修复：identityKnown = login !== ''。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const ZH_PLACEHOLDER = | ||
| `${ZH_TOKEN}(?:${ZH_TAIL}` + |
There was a problem hiding this comment.
[Suggestion] R2-17 (re-report, probe-confirmed again at this head): Chinese placeholders wrapped in full-width parens （无 — 所有检查均完成） still survive as phantom gaps — the paren strip (budget.ts:885-888) is halfwidth-only and ZH_PLACEHOLDER is ^-anchored, so it fails at （ (U+FF08); ZH_TAIL contains no paren codepoints. — Failure scenario (probe at this head): budgetGapDisclosures('Budget gap: （无）') → ["（无）"], budgetGapDisclosures('预算缺口：（没有跳过的检查）') → kept, while the identical halfwidth-wrapped content → []. Under outputLanguage: 中文 full-width parens are the default paren shape; the retained phantom gap is handed to the orchestrator to rule on and rendered in the composed body quoting a sentence that asserts the opposite — the #9094 incident this classifier exists to fix. Flip-verified: stripping a symmetric full-width pair drops all fullwidth-wrapped placeholders while （无法验证 Windows 矩阵的集成测试） is still kept as a real gap. — Fix: extend the un-parenthesizing step to strip a symmetric full-width pair (startsWith('（') && endsWith('）') → slice), or add a fullwidth-paren arm mirroring the English one.
中文说明
[Suggestion] R2-17（重报，本轮在本提交上再次经探针确认）：以全角括号包裹的中文占位回答 （无 — 所有检查均完成） 仍会作为幻影缺口幸存——去括号步骤（budget.ts:885-888）只认半角括号，且 ZH_PLACEHOLDER 以 ^ 锚定，遇到 （（U+FF08）即失败；ZH_TAIL 不含任何括号码点。— 失败场景（本轮在本提交上探针）：budgetGapDisclosures('Budget gap: （无）') → ["（无）"]，budgetGapDisclosures('预算缺口：（没有跳过的检查）') → 保留，而完全相同的半角包裹内容 → []。在 outputLanguage: 中文 下全角括号是默认括号形态；被保留的幻影缺口会被交给编排者裁决，并被渲染进组合正文，引用的却是断言相反内容的句子——正是本分类器存在所要修复的 #9094 事故。已做翻转验证：剥离对称的全角括号对可丢弃所有全角包裹的占位回答，同时 （无法验证 Windows 矩阵的集成测试） 仍被保留为真实缺口。— 修复：扩展去括号步骤以剥离对称的全角括号对（startsWith('（') && endsWith('）') → slice），或仿照英文分支增加全角括号分支。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| [...stillPending, ...unattemptedRetries], | ||
| stillClamped, |
There was a problem hiding this comment.
[Suggestion] R10-17 (re-report, probe re-confirmed at this head): an unattempted retry is named by two additive-looking note clauses for a single command — "N command(s) still to run — not run: …" (via [...stillPending, ...unattemptedRetries]) AND "N command(s) are still provisional — killed on a deadline the budget shortened: …" (via stillClamped, since its old clamped entry survives in mergedTest) — and the provisional clause asserts every listed command was "killed on a shortened deadline" this call, which is false for a retry never attempted this call. — Failure scenario (probe witness at this head): with previous = clamped retryA+retryB, notRun [dirC], budget admitting only retryA (re-clamped), the note claims "2 command(s) still to run — not run: packages/b, npm test --workspace="packages/a"" AND "2 command(s) are still provisional …: npm test --workspace="packages/core", npm test --workspace="packages/a"" — 4 claimed slots for 3 distinct commands, with retryB in both. An agent planning against MAX_RESUME_CALLS=3 reads two or three unfinished commands where there is one. Structural fields stay correct (prose-only). Fix: exclude commands already named by the still-to-run clause from the provisional clause (stillClamped.filter((c) => !stillPending.includes(c)) for the prose), or merge the clauses when the sets intersect.
中文说明
[Suggestion] R10-17（重报，本轮探针再次确认）：一次未被尝试的重试会被两个看似相加的 note 子句同时点名——「N command(s) still to run — not run: …」（来自 [...stillPending, ...unattemptedRetries]）与「N command(s) are still provisional — killed on a deadline the budget shortened: …」（来自 stillClamped，因为其旧 clamped 条目在 mergedTest 中留存）——且 provisional 子句断言列出的每条命令都是本次调用「被缩短的期限杀死」的，对一条本次调用从未尝试的重试这是假的。— 失败场景（本轮在本提交上的探针见证）：previous = clamped retryA+retryB、notRun [dirC]、预算只准入 retryA（再次被 clamp）时，note 声称「2 command(s) still to run — not run: packages/b, npm test --workspace="packages/a"」并且「2 command(s) are still provisional …: npm test --workspace="packages/core", npm test --workspace="packages/a"」——3 条不同命令被声称为 4 个未完成槽位，retryB 出现在两处。按 MAX_RESUME_CALLS=3 规划的 agent 会把 1 条未完成读成两三条。结构化字段仍然正确（仅措辞问题）。修复：provisional 子句排除已被 still-to-run 子句点名的命令（对措辞用 stillClamped.filter((c) => !stillPending.includes(c))），或在两集合相交时合并子句。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (phrases.some((ph) => unreviewed[i].includes(ph))) { | ||
| splicedForBudgetPhrase.push(unreviewed[i]); |
There was a problem hiding this comment.
[Suggestion] R12-6: the render splice dedups by budget-phrase SUBSTRING while the anchor decision correctly moved to exact canonical text — so a genuine line-coverage disclosure that merely mentions the phrase is silently dropped from the posted body. — Failure scenario (probe-verified at this head): a budget-stop round whose orchestrator discloses security — the review time budget ended the round before the security relaunch returned evidence (the diff's own doc-comment example): the entry is spliced out of unreviewed, the posted "Not reviewed" section never names the security scope (bodyNamesSecurity: false), only the generic structural stop line renders — while the verdict cap and anchor withholding survive (dimensionGapsAreDepthOnly: false, sha: null). Harm is bounded to disclosure completeness, but the module's contract is that the gap reaches the author. Fix: splice on the same exact-text predicate the decision uses — splice only entries in canonicalStopEntries (marker-anchored), keep collecting the rest for the decision list.
中文说明
[Suggestion] R12-6：渲染拼接按预算短语子串去重，而锚点裁决已正确地改用规范文本精确匹配——于是仅仅提及该短语的真实行覆盖披露会被悄悄从发布正文中丢弃。— 失败场景（已在本提交上探针验证）：某预算停止轮次中编排者披露 security — the review time budget ended the round before the security relaunch returned evidence（正是本 diff 文档注释里的例子）：该条目被从 unreviewed 中剪掉，发布的「Not reviewed」小节从未点名 security 范围（bodyNamesSecurity: false），只渲染通用的结构性停止行——而裁决封顶与锚点扣留都幸存（dimensionGapsAreDepthOnly: false、sha: null）。危害限于披露完整性，但模块的契约是缺口必须送达作者。修复：改用与裁决相同的精确文本谓词做拼接——只剪掉属于 canonicalStopEntries（有标记锚定）的条目，其余继续收集进裁决列表。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const strings = (v: unknown): boolean => | ||
| Array.isArray(v) && v.every((e) => typeof e === 'string' && e.length > 0); | ||
| const affectedOk = strings((parsed as { affected?: unknown }).affected); |
There was a problem hiding this comment.
[Suggestion] R12-9: the --resume shape gate validates every field the continuation walks EXCEPT install and note, and the continuation reads both — installFailureFraming(previous.install) interpolates .command/.exitCode into the merged note (npm-toolchain.ts:111-118), withNote coerces previous.note via template literal. — Failure scenario (probe-verified at this head): a corrupted/hand-edited report carrying "install": true clears every gate check, passes the identity check with an intact run stamp, and the merged note opens with \undefined` exited undefined but left a usable `node_modules`, so the run went ahead…— degraded agent-facing framing where the gate's contract is a refusal with a named fix;note: 42surfaces as42 Nothing to resume: …. No crash (template coercion), and the gate's own corrupt-report fixtures establish hand-edited reports as the accepted threat model; the sibling fields affected/notBuiltwere validated this PR after R11-1 —install/noteare the remaining family members. Fix: extend the final gate with aninstallshape check (null or object with stringcommand+ number/nullexitCode) and typeof note === 'string'`.
中文说明
[Suggestion] R12-9：--resume 形状门禁验证了续跑会遍历的所有字段，唯独不验证 install 和 note，而续跑两者都会读——installFailureFraming(previous.install) 会把 .command/.exitCode 插值进合并 note（npm-toolchain.ts:111-118），withNote 会用模板字面量强制 previous.note。— 失败场景（已在本提交上探针验证）：携带 "install": true 的损坏/手工编辑报告能通过所有门禁检查、带着完好的 run 印章通过身份检查，合并 note 以 \undefined` exited undefined but left a usable `node_modules`, so the run went ahead… 开头——门禁契约本应是指明修复方式的拒绝，现在却是降级的 agent 可见措辞；note: 42会显示为42 Nothing to resume: …。无崩溃（模板强制），且门禁自己的损坏报告夹具已确立手工编辑报告是本代码接受的威胁模型；兄弟字段 affected/notBuilt 在 R11-1 之后已于本 PR 验证——install/note是剩下的同族成员。修复：在最终门禁中加上install形状检查（null 或带字符串command+ 数字/nullexitCode的对象）与typeof note === 'string'`。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| '## Previous /review round (machine ledger)', | ||
| '', | ||
| `Round ${ledger.round}${ledger.sha ? `, reviewed at \`${code(ledger.sha)}\`` : ''}, recovered from the marker this account's last posted review carried. **Every entry below is owed a this-round ruling** (fixed / still stands / cannot tell / superseded by <class-id>) under Step 6's previous-round rules — the ledger is a work list, not a verdict; re-assert each claim against the code before repeating or retiring it.${ledger.sha ? ` The reviewed-at sha is the incremental anchor Step 1's recovered-anchor check reads from the side file — when Step 1's recovered-anchor check rules a re-run admissible, pass it as \`--since <sha>\` on a \`fetch-pr\` re-run, which validates it against the fetched history and scopes the diff and plan; never run git against an anchor yourself.` : ''}`, | ||
| `Round ${ledger.round}${ledger.sha ? `, reviewed at \`${code(ledger.sha)}\`` : ''}, recovered from the marker ${author ? `**@${cell(author)}**'s last posted review carried — another account, so these are THEIR claims and no incremental anchor travelled with them (the sha never crosses accounts; this round is full-range unless a local cache supplies one)` : `this account's last posted review carried`}. **Every entry below is owed a this-round ruling** (fixed / still stands / cannot tell / superseded by <class-id>) under Step 6's previous-round rules — the ledger is a work list, not a verdict; re-assert each claim against the code before repeating or retiring it.${ledger.sha ? ` The reviewed-at sha is the incremental anchor Step 1's recovered-anchor check reads from the side file — when Step 1's recovered-anchor check rules a re-run admissible, pass it as \`--since <sha>\` on a \`fetch-pr\` re-run, which validates it against the fetched history and scopes the diff and plan; never run git against an anchor yourself.` : ''}`, |
There was a problem hiding this comment.
[Suggestion] R12-12: when a FOREIGN winner is merged OVER the own findings (the union this PR adds), the author sentence attributes the ENTIRE merged list — including this account's own certified entries — to the foreign poster as "THEIR claims". — Failure scenario (probe-verified at this head): own round-7 marker (R7-1, 'maintainer') + foreign round-8 marker ('ci-bot') → recoverLedger returns the union (merged ids: ["R7-1","R8-1"], foreign: true, author: ci-bot) and the section renders "Round 8, recovered from the marker @ci-bot's last posted review carried — another account, so these are THEIR claims…" above a table that includes the maintainer's OWN certified R7-1 — false provenance for the own subset, inverting the exact trust distinction the author parameter exists to enforce. The anchor is stripped unconditionally (safety intact); the cost is the provenance premise Step 6 rules from. No test covers the merged rendering (renderLedgerSection tests use a pure-foreign ledger). Fix: when own findings were merged in, qualify the sentence (entries this account certified are its own; the rest are another account's claims), or pass a flag distinguishing merged own entries.
中文说明
[Suggestion] R12-12：当一个外部获胜者被合并到本账号条目之上（本 PR 新增的并集）时，作者句会把整个合并清单——包括本账号自己认证过的条目——都归为外部发布者的「THEIR claims」。— 失败场景（已在本提交上探针验证）：本账号第 7 轮标记（R7-1，'maintainer'）+ 外部第 8 轮标记（'ci-bot'）→ recoverLedger 返回并集（merged ids: ["R7-1","R8-1"]、foreign: true, author: ci-bot），小节渲染为「Round 8, recovered from the marker @ci-bot's last posted review carried — another account, so these are THEIR claims…」，而表中包含维护者自己认证的 R7-1——对本账号子集给出虚假来源，恰好反转了 author 参数存在所要强制的信任区分。锚点被无条件剥离（安全性完好）；代价是 Step 6 裁定所依据的来源前提。没有测试覆盖合并后的渲染（renderLedgerSection 测试使用纯外部账本）。修复：当并集中含本账号条目时，限定句子措辞（本账号认证的条目属于本账号；其余是另一账号的主张），或传入标志以区分合并进来的本账号条目。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(first.testScope?.caveat).toContain('still to run: packages/b'); | ||
| expect(first.testScope?.caveat).not.toContain('; '); |
There was a problem hiding this comment.
[Suggestion] R12-20: every re-stop caveat assertion runs with liveCaveat: ''; no test combines a non-empty live caveat with a resume that stops again, so the live + '; ' + machine rejoin (npm-toolchain.ts:288-311) is unpinned. — Failure scenario (mutant-verified at this head): a mutant dropping the live segment when appending a fresh machine clause keeps every resume test green, and the intermediate (re-stop) report loses the live disclosure — a PR whose changed files sit in negated workspaces (live caveat) and whose chain budget-stops twice emits a first-stop report carrying only the machine clause, while the brief's contract is that a caveat present on the report is what the agent quotes as the scope limitation. liveCaveat still carries the segment structurally, so the FINAL report self-heals; the first-stop report does not. — Fix: add a budget-limited resume over a non-empty-live-caveat fixture asserting the live segment survives the rejoin (toContain(liveSegment) plus toContain('; ')).
中文说明
[Suggestion] R12-20：所有「再次停止」的 caveat 断言都在 liveCaveat: '' 下运行；没有测试把非空 live caveat 与再次停止的续跑组合起来，因此 live + '; ' + machine 重新拼接（npm-toolchain.ts:288-311）未被钉住。— 失败场景（本轮在本提交上经突变体验证）：在追加新机器子句时丢弃 live 段的突变体能让所有续跑测试保持绿色，且中间（再次停止的）报告丢失 live 披露——一个改动文件位于被排除 workspace（live caveat）且链条两次预算停止的 PR，其首次停止报告只携带机器子句，而简报的契约是：报告上存在的 caveat 正是 agent 作为范围限制加以引用的内容。liveCaveat 在结构上仍携带该段，所以最终报告会自愈；首次停止报告不会。— 修复：在非空 live caveat 夹具上增加一次预算受限的续跑，断言 live 段在重新拼接后幸存（toContain(liveSegment) 加 toContain('; ')）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(MAX_RESUME_CALLS).toBeGreaterThanOrEqual(1); | ||
| expect(MAX_RESUME_CALLS).toBeLessThanOrEqual(5); |
There was a problem hiding this comment.
[Suggestion] R12-21: the MAX_RESUME_CALLS bound test admits values its own cited policy rejects — the comment above defines the boundary as "Four calls of ten minutes" (= 1 initial + 3 continuations), but toBeLessThanOrEqual(5) admits up to 5 continuations (6 calls). — Failure scenario (mutant-verified at this head): MAX_RESUME_CALLS = 5 passes this test AND the entire review suite (82 files, 3203 tests), silently extending the agent-facing promises this constant feeds (at most ${MAX_RESUME_CALLS} continuations in agent-prompt.ts:1442 and agent-briefs.ts:538) — each extra continuation is a full 600-second shell call per dimension, past the point the test's own comment declares illegitimate ("past it, the honest answer is notRun"). — Fix: pin the documented policy: expect(MAX_RESUME_CALLS).toBeLessThanOrEqual(3).
中文说明
[Suggestion] R12-21：MAX_RESUME_CALLS 边界测试允许其引用的策略所拒绝的取值——上方注释把边界定义为「四次十分钟调用」（= 1 次初始 + 3 次续跑），但 toBeLessThanOrEqual(5) 允许最多 5 次续跑（6 次调用）。— 失败场景（本轮在本提交上经突变体验证）：MAX_RESUME_CALLS = 5 既通过本测试又通过整个 review 套件（82 个文件、3203 条测试），悄悄扩大该常量供给的面向 agent 的承诺（agent-prompt.ts:1442 与 agent-briefs.ts:538 中的 at most ${MAX_RESUME_CALLS} continuations）——每多一次续跑就是每个维度一次完整的 600 秒 shell 调用，越过了测试自己的注释宣布为不合法的分界（「超过它，诚实的回答是 notRun」）。— 修复：钉住文档化的策略：expect(MAX_RESUME_CALLS).toBeLessThanOrEqual(3)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // floor is one command deadline: a tiny --timeout must not turn the headroom | ||
| // into a negative budget that starves every suite. |
There was a problem hiding this comment.
[Suggestion] R12-22: the removed tiny---timeout guard has no surviving replacement: with the budget decoupled from --timeout, a sub-second --timeout fabricates genuine (non-clamped) timeouts for every suite, which --resume can never repair. — Failure scenario (probe-verified at this head, flips with a one-line bound): build-test --timeout 0.5 → callBudgetMs = max(0.5, 570) = 570s, so the 15s floor above — which compares remaining BUDGET, never the per-command deadline — never fires; every phase is attempted with deadline = min(500, remaining) = 500ms, every command is killed timedOut: true, and the clamp predicate 500 < 500 is false — every suite records as a REAL timeout: ok: false, neither resume detector (clamped, notRun) fires, and --resume certifies "reached every suite in scope". This comment still names the case ("a tiny --timeout must not … starve every suite"); pre-PR the derived budget collapsed with the timeout and disclosed an honest budget-stop. Pathological input only (defaults are safe), but it defeats a guard this diff deleted. — Fix: reject --timeout < 1 (or < BUDGET_MIN_ATTEMPT_MS / 1000) at the existing finiteness check (build-test.ts:567) — flip-tested.
中文说明
[Suggestion] R12-22：被删除的极小 --timeout 守卫没有幸存的替代物：预算与 --timeout 解耦后，亚秒级的 --timeout 会为每个套件伪造真实的（非 clamped 的）超时，而 --resume 永远无法修复它们。— 失败场景（本轮在本提交上经探针验证，一行下限即可翻转）：build-test --timeout 0.5 → callBudgetMs = max(0.5, 570) = 570 秒，于是上方这个 15 秒地板——它比较的是剩余预算，从不比较单命令期限——永远不会触发；每个阶段都以 deadline = min(500, remaining) = 500ms 尝试，每条命令都被杀死 timedOut: true，而 clamp 谓词 500 < 500 为假——每个套件都记录为真实超时：ok: false，两个续跑检测器（clamped、notRun）都不触发，--resume 还会认证「reached every suite in scope」。本注释仍点名此情形（「极小 --timeout 不得……饿死每个套件」）；改动前推导预算随 timeout 坍缩并披露诚实的预算停止。仅为病态输入（默认值安全），但它击败的是本 diff 自己删除的守卫。— 修复：在现有有限性检查（build-test.ts:567）处拒绝 --timeout < 1（或 < BUDGET_MIN_ATTEMPT_MS / 1000）——已做翻转测试。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| .mockReturnValueOnce([]) // issue comments | ||
| .mockReturnValueOnce([strangerReview]); // reviews |
There was a problem hiding this comment.
[Suggestion] R1-40 (round-1 finding, mutant re-confirmed at this head): no handler-level test carries a ledger marker, so runPrContext's foreign→author wiring (prevLedgerAuthor = prevRecovered?.foreign ? (prevRecovered.author ?? null) : null) and the foreign-ledger render/persist seam are unpinned — both handler describes use marker-less fixtures, so recoverLedger returns null in every handler test and the conditional is never executed. — Failure scenario (both mutants shipped green this round, 95/95): mutant A (prevLedgerAuthor = null): when a CI bot's marker is recovered cross-account — the primary case this PR names — the context file renders the foreign work list as "this account's last posted review carried" with no THEIR-claims/no-anchor disclosure. Mutant B (drop .foreign ?): an OWN anchored ledger renders as another account's claims, self-contradictorily ("no incremental anchor travelled" beside "reviewed at <sha>"). Fix: add a handler-level test whose reviews fixture carries a serializeLedger marker posted by an account different from currentUserMock's login, asserting the written context contains 'THEIR claims' / **@<poster>** and the side file is written without sha.
中文说明
[Suggestion] R1-40（第 1 轮发现，本轮突变体再次确认）：没有任何 handler 级测试携带账本标记，因此 runPrContext 的 foreign→author 接线（prevLedgerAuthor = prevRecovered?.foreign ? (prevRecovered.author ?? null) : null）与外部账本的渲染/持久化缝隙都没被钉住——两个 handler describe 都用无标记夹具，于是每个 handler 测试里 recoverLedger 都返回 null，那个条件分支从未执行。— 失败场景（本轮两个突变体均带绿，95/95）：突变体 A（prevLedgerAuthor = null）：当 CI bot 的标记被跨账号恢复——本 PR 点名的首要场景——上下文文件会把外部工作清单渲染成「this account's last posted review carried」，没有 THEIR claims / 无锚点披露。突变体 B（去掉 .foreign ?）：本账号带锚点的账本被渲染成另一账号的条目，自相矛盾（「no incremental anchor travelled」旁边写着「reviewed at <sha>」）。修复：新增一个 handler 级测试，reviews 夹具携带由与 currentUserMock 登录名不同的账号发布的 serializeLedger 标记，断言写出的上下文包含 'THEIR claims' / **@<poster>**，且 side 文件写出时不含 sha。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| owner_repo: 'o/r', | ||
| out: '/tmp/ctx.md', | ||
| }); | ||
| expect(rmSyncMock).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] R11-17 (re-report; half 1 mutant re-verified at this head): the handler-level identity tests pin neither half of their contract. (1) The identity-failure test pins only the deletion arm (rmSync NOT called), not the isolation it exists to protect: gating recoverLedger on identityKnown — recovery skipped entirely while the test stays green (probe this round: shipped test ✓ green under the skip-recovery mutant; a marker-carrying discriminator probe ✗ with zero prev-ledger writes) — degrades a rate-limit blip to "no ledger", and a fresh machine then restarts at round 1 re-issuing R1-* ids the PR already carries. (2) The deletion flag's !sawOwnReview and !recoveryThrew conjuncts are unpinned in either direction: no handler test walks a non-empty review list containing a marker-less OWN review; deleting either conjunct ships green — an own follow-up whose marker fails to parse then reads as proven absence, the side file is deleted mid-PR, the posture clock resets. Fix: (1) give the identity-failure fixture a marker and assert recovery survived the throw; (2) add handler tests for confirmed-identity + no-own-review → rmSync called, and confirmed-identity + marker-less own review present → rmSync NOT called.
中文说明
[Suggestion] R11-17（重报；半区 1 的突变体本轮再次验证）：handler 级身份测试对其契约的两个半区都未钉住。(1) 身份失败测试只钉了删除臂（rmSync 未被调用），没有钉它本要保护的隔离：把 recoverLedger 门控在 identityKnown 上——恢复被整体跳过而测试保持绿色（本轮探针：skip-recovery 突变下原测试 ✓ 绿；携带标记的判别探针 ✗，prev-ledger 写入为零）——会把一次限流抖动降级为「无账本」，新机器随后从第 1 轮重启，重新发布 PR 已有的 R1-* id。(2) 删除标志的 !sawOwnReview 与 !recoveryThrew 两个合取项在任一方向都未被钉住：没有 handler 测试遍历包含无标记本账号 review 的非空列表；删除任一合取项都带绿——一个标记解析失败的本账号后续 review 会被读成「确证不存在」，side 文件在 PR 中途被删除，轮次时钟被重置。修复：(1) 给身份失败夹具一个标记并断言恢复在抛错后幸存；(2) 增加 handler 测试：已确认身份 + 无本账号 review → rmSync 被调用；已确认身份 + 存在无标记本账号 review → rmSync 未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…rammar, and an empty login
Three blockers from the automatic review, all probe-confirmed on the
merge head.
R1-11 (isUnusableScriptEntry): the gate answered 'usable' by extension
enumeration — anything outside .js/.mjs/.cjs returned usable with no
isFile/X_OK/shebang check — so the childEnv stamp could hand skill
subcommands an entry no shell can exec: a tsx dev launch stamps the 0644
index.ts, and 'node <pkg-dir>' stamps the DIRECTORY (which passes an
X_OK probe as search permission). Every '"${QWEN_CODE_CLI:-qwen}"'
then died on exit 126 where empty would have fallen back. The gate now
demands positive evidence: a regular file with the execute bit, a '#!'
header for any known script extension (the TS family included), no
shebang needed for native binaries. Both entrances are pinned at the
childEnv level and the primitive's own suite covers the three shapes.
R3-38 (--resume injection): the continuation re-executed report-stored
test[].command strings verbatim under shell: true, while the identity
check pins a report to this run's tree — not to this program's
authorship — and the gate's own corrupt-report fixtures declare the
edited-in-place report the accepted threat model. Stored commands are
now held to the emitter's grammar (npm test [--workspace="<dir>"]),
the same policy test-delta already applies before re-running report
commands; an alien command refuses the resume with a named fix, before
anything runs.
identityKnown empty-login: currentUser() answering empty-with-exit-0 (a
stubbed or proxied gh) set identityKnown=true while recoverLedger reads
'' as unknown, so the deletion arm could remove the prev-ledger side
file — resetting the round counter — over an identity that was never
proven. Empty is now unknown, matching presubmit's own '' handling.
Each fix is pinned by a test its reverted mutant fails.
commented
Aug 16, 2026
|
Round 13's three blockers are fixed in e792d44. R1-11 — taken with both demonstrated entrances. The gate now demands positive evidence instead of enumerating known-bad shapes: a regular file (the directory shape dies here — X_OK on a directory is search permission, which is why only R3-38 — taken as filed. Stored identityKnown — Each fix was mutation-verified: reverting it fails exactly its new test. The round's Suggestions are deferred without exception, joining the standing backlog for the follow-up change. 中文说明第十三轮的三个阻断项已在 e792d44 修复。 R1-11 —— 连同两条已演示的入口一并采纳。门禁改为要求正面证据而非枚举已知坏形态：常规文件（目录形态死在这一关——对目录的 X_OK 是搜索权限，所以只有 R3-38 —— 按原样采纳。报告存储的 identityKnown —— 三个修复均做了变异验证：还原任一修复，恰好其新测试失败。本轮 Suggestion 一律延后，并入既有积压等待后续改动。 |
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — all checks above ran to completion (~24 tool calls)..
中文说明
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — all checks above ran to completion (~24 tool calls).。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| let login: string | null = null; | ||
| try { | ||
| login = currentUser(); |
There was a problem hiding this comment.
[Critical] R13-1: During an identity-lookup outage (currentUser() throws → login null), recoverLedger computes no bestOwn, so the union merge's precondition if (best.foreign && bestOwn) is never satisfied and the winning FOREIGN marker REPLACES this account's latest certified findings in the persisted side file. — Failure scenario: any authenticated GitHub user posts a ledger marker at this account's current round (visible on the PR, inside FOREIGN_ROUND_HEADROOM of the zero base); a transient gh api user failure then makes every marker foreign, round-first selection adopts the drive-by (later submitted_at or higher review id), persistRecoveredLedger's never-lower guard passes (equal round, larger review id), and the account's certified R<round>-* entries exit the marker chain permanently — the next identity-restored run's bestOwn is the round+1 marker, which never contained them. The suppression class the union merge exists to kill, reopened whenever the identity endpoint is down; the attacker's marker persists, so every future outage reopens it. Sibling state of R4-2 in the opposite direction (R4-2 is under-recovery during an outage; this is over-trust of the anonymous recovery), distinct mechanism introduced by this redesign.
Witness (handler-level probe, mocked gh with currentUser() throwing):
BASE (outage): side file = {"round":7,"ids":["R7-2"],"reviewId":101} ← stranger's list replaced own R7-1
healthy arm: ids ["R7-1","R7-2"] (union merge works whenever identity is known)
fix arm: {"round":7,"ids":["R7-1"]} — probe flips
Suggested fix: do not let an identity-unproven recovery overwrite the side file — when login is null, degrade to the conservative strip path (keep the existing file, strip commitId/reviewId) exactly like the recoveryThrew case, e.g. condition the overwrite in persistRecoveredLedger on identityKnown. This is a design call: that guard re-exposes R4-2 (stale file during an outage → id re-issue), so the adopted shape may need to distinguish strictly-higher anonymous rounds; the probe harness above re-checks any candidate.
中文说明
身份查询故障期间（currentUser() 抛错 → login 为 null），recoverLedger 算不出 bestOwn，联合合并的前置条件 if (best.foreign && bestOwn) 永不成立，胜出的外部标记会替换 side file 中本账号最新已认证的发现清单。失败场景：任何已认证的 GitHub 用户都能在本账号当前轮次（PR 页面可见、落在零基线的 FOREIGN_ROUND_HEADROOM 之内）投递账本标记；一次 gh api user 瞬时故障即使所有标记都成为外部标记，轮次优先选择采纳该顺手投递（更晚的 submitted_at 或更大的 review id），never-lower 守卫放行（同轮、更大 review id），本账号已认证的 R<round>-* 条目永久退出标记链——下一次身份恢复后的运行的 bestOwn 是 round+1 的标记，其中从未包含这些条目。这正是联合合并要消灭的压制类缺陷，只要身份端点故障就会重新敞开；攻击者的标记持续留存，此后每次故障都会再次敞开。与 R4-2 是同一触发条件下的孪生状态、方向相反（R4-2 是故障期间的恢复不足，本条是对匿名恢复的过度信任），机制由本次重新设计引入。探针证据（handler 级探针，mock gh 使 currentUser() 抛错）：故障时 side file = {"round":7,"ids":["R7-2"],"reviewId":101}（陌生人的清单替换了本账号的 R7-1）；身份正常分支 ids 为 ["R7-1","R7-2"]；修复分支（以 identityKnown 为覆写条件）为 {"round":7,"ids":["R7-1"]}，探针翻转。修复建议：身份未证实的恢复不得覆写 side file——login 为 null 时降级到保守的 strip 路径（保留现有文件、剥离 commitId/reviewId），与 recoveryThrew 情形一致（例如在 persistRecoveredLedger 中以 identityKnown 为覆写条件）。注意这是设计决策：该守卫会重新暴露 R4-2（故障期间文件过期 → id 重发），采纳的形态可能需要区分「严格更高的匿名轮次」。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| commitId: best.commitId, | ||
| reviewId: best.id, | ||
| foreign: best.foreign, |
There was a problem hiding this comment.
[Suggestion] R13-2: A foreign winner's commitId/reviewId ride into the prev-ledger side file with no foreign filter — two gaps in one seam. (1) Behavior: the bundled SKILL.md rounds-2–5 code-age rule consumes the side file's commitId as its deferral reference and the reviewId body as the only oracle for "the previous round read this code" — a foreign review was never produced by this pipeline, so its silence reads as full coverage: a drive-by marker at ownMax+1 with a disclosure-free body and commit_id = current HEAD ages every new high-confidence Suggestion the legitimate account would post as "unchanged" and defers it (rounds 2–5 only; Criticals still post; the rule fails open in its named doubt states — bounded, disclosed suppression, a weaker class than anchor forgery). (2) Coverage: no foreign-winner fixture sets commit_id — the commitId: best.foreign ? null : best.commitId mutant leaves 96/96 tests green. — Concrete cost: one marker post by any authenticated user defers the account's Suggestions for rounds 2–5, and the ride-through decision itself is unpinned in both directions.
Witness (probe): recoverLedger([own round-3 marker with R3-1, stranger round-4 marker (empty findings, commit_id bbb…)], 'maintainer') → {"commitId":"bbb…","reviewId":77,"foreign":true} with the own work list merged intact; the implied fix flips it to commitId:null. Mutant arm: pr-context suite 96/96 green.
| commitId: best.commitId, | |
| reviewId: best.id, | |
| foreign: best.foreign, | |
| commitId: best.foreign ? null : best.commitId, | |
| reviewId: best.foreign ? null : best.id, | |
| foreign: best.foreign, |
—or gate the age rule on the winner being the own account; either way add a foreign-winner fixture with a valid 40-hex commit_id pinning the intended direction.
中文说明
外部胜出者的 commitId/reviewId 未经外部过滤就进入 prev-ledger side file——同一个缝隙上的两个缺口。(1) 行为面：bundled SKILL.md 的第 2–5 轮 code-age 规则以 side file 的 commitId 作为延迟判定的参照、以 reviewId 指向的正文作为「上一轮读过这些代码」的唯一神谕——外部审查并非本管线产出，其沉默会被当作完整覆盖：一个落在 ownMax+1、正文无任何未审查披露、commit_id 即当前 HEAD 的顺手标记，会把合法账号所有新的高置信 Suggestion 判为「代码未变化」而延迟（仅第 2–5 轮；Critical 照常发布；规则在其列出的存疑状态下放行——有界、已披露的压制，弱于锚点伪造一类）。(2) 覆盖面：没有任何外部胜出者夹具设置 commit_id——commitId: best.foreign ? null : best.commitId 突变体下 96/96 测试全绿。具体代价：任何已认证用户投递一个标记即可在第 2–5 轮延迟该账号的 Suggestion，且 ride-through 决策本身双向都未被钉住。修复：在缝隙处对外部胜出者置空 commitId/reviewId（或把 age 规则限定在本账号胜出），并补一个带合法 40 位十六进制 commit_id 的外部胜出者夹具钉住预期方向。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * a dir exotic enough to fall outside it costs that report its resume (a | ||
| * named refusal, pointing at a fresh run), never a verbatim re-execution. | ||
| */ | ||
| export const TEST_COMMAND_RE = /^npm test(?: --workspace="[\w@./-]+")?$/; |
There was a problem hiding this comment.
[Suggestion] R13-3: TEST_COMMAND_RE is a byte-identical second copy of test-delta.ts's RERUNNABLE_COMMAND_RE (~line 71); both gate shell re-execution of report-stored commands against the one emitter (testCommand/shellArg), and nothing ties the two copies together. — Failure scenario: a future grammar change (widened character class or a new flag) applied to one copy only silently diverges the two re-execution gates: build-test --resume refuses its own fresh reports (safe direction), or test-delta skips re-running a valid stored command and attributes nothing (the under-measurement direction that file exists to avoid). The new copy's own comment concedes it applies "the same policy test-delta already applies". Concrete cost: the gates are the shell-re-execution defense; drift between them is silent in the dangerous direction.
Suggested fix: have test-delta.ts import TEST_COMMAND_RE from ./lib/npm-toolchain.js (no import cycle — npm-toolchain imports nothing from test-delta) and delete the local copy.
中文说明
TEST_COMMAND_RE 与 test-delta.ts 的 RERUNNABLE_COMMAND_RE（约第 71 行）逐字节相同；两者都为「对报告中存储的命令做 shell 重执行」把关，依据的是同一个发出者（testCommand/shellArg），但两份副本之间没有任何联系。失败场景：未来某次语法变更（放宽字符类或新增旗标）只应用到其中一份副本，两个重执行门禁就会悄然分叉：build-test --resume 拒绝自己新写出的报告（安全方向），或 test-delta 跳过对合法存储命令的重跑、不做归因（该文件竭力避免的测量不足方向）。新副本的注释自己也承认它应用的是「test-delta 已经应用的同一策略」。具体代价：这两个门禁是 shell 重执行的防线，二者漂移时在危险方向上是无声的。修复：让 test-delta.ts 从 ./lib/npm-toolchain.js 导入 TEST_COMMAND_RE（无循环导入——npm-toolchain 不依赖 test-delta），删除本地副本。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * matched, and only against the ONE dimension whose brief sets | ||
| * `readsDiff: false`. | ||
| */ | ||
| export function isNonDiffDimensionGap(entry: string): boolean { |
There was a problem hiding this comment.
[Suggestion] R13-5: isNonDiffDimensionGap hardcodes three prose heads of the build-and-test dimension, decoupled from the machine source of truth it documents — BRIEFS[role].readsDiff / publicLabel in lib/agent-briefs.ts (Agent 7 is the single readsDiff: false role, publicLabel: 'the build-and-test check'). No test pins the two together (grep finds no BRIEFS/agent-briefs reference in compose-review.test.ts). — Failure scenario: a rename of Agent 7's label/publicLabel in agent-briefs (or a second readsDiff: false role added) silently stops or fails to extend the exemption: every round on a repo whose suites don't fit the whole-call budget then withholds the incremental anchor again, re-creating the measured 119-minute / 34M-token full-diff re-review loop this PR exists to kill. Direction is over-withholding (safe); the cost is the re-review.
Suggested fix: derive the exempt heads from the briefs (match the head against the publicLabels of roles with readsDiff: false), or at minimum add a collocated test asserting isNonDiffDimensionGap accepts every readsDiff: false brief's publicLabel and rejects every readsDiff: true one, so a label change fails loudly.
中文说明
isNonDiffDimensionGap 把 build-and-test 维度的三种文案头硬编码了，与其注释声称的机器事实来源——lib/agent-briefs.ts 中的 BRIEFS[role].readsDiff / publicLabel（Agent 7 是唯一 readsDiff: false 的角色，publicLabel: 'the build-and-test check'）——脱钩。没有任何测试把两者钉在一起（grep 确认 compose-review.test.ts 中没有 BRIEFS/agent-briefs 引用）。失败场景：重命名 agent-briefs 中 Agent 7 的 label/publicLabel（或新增第二个 readsDiff: false 角色）会悄无声息地让豁免失效（或未能扩展）：在套件装不进整次调用预算的仓库上，每一轮都会重新扣留增量锚点，重新制造本 PR 要消灭的、实测 119 分钟 / 3400 万 token 的全量重审循环。方向是过度扣留（安全方向），代价是重审。修复：从 briefs 派生豁免头（用 readsDiff: false 角色的 publicLabel 匹配头），至少也要加一个同文件测试，断言 isNonDiffDimensionGap 接受每个 readsDiff: false 角色的 publicLabel、拒绝每个 readsDiff: true 的，使 label 变更时测试响亮失败。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // the cap, so prevRound can reach it only AT the cap, where staying | ||
| // there loses id uniqueness across those rounds and nothing else — | ||
| // against a counter no real PR approaches. | ||
| Math.min(prevRound + 1, LEDGER_MAX_ROUND), |
There was a problem hiding this comment.
[Suggestion] R13-6: The new round clamp was applied to the ledger-marker stamp only; the deferred-suggestions clause's deferredRound = prevRound + 1 (~line 2270, pre-existing) is unclamped, breaking the invariant two comments assert — prevRoundFor's JSDoc ("the two cannot disagree about which round this is") and the clause's own comment ("the clause and the marker cannot disagree about which round deferred"). — Failure scenario: prevRound can reach exactly LEDGER_MAX_ROUND (parseLedger accepts round == cap, refusing only >): the marker then stamps round 10000 with R10000-* ids while the posted body says "Deferred under the convergence posture (round 10001)" (zh 第 10001 轮) — a past-cap round number beside a round-10000 marker. Pre-diff the two consumers agreed.
Witness (probe): at the cap {"markerRound":10000,"clauseRound":"10001"}; one round below {"markerRound":10000,"clauseRound":"10000"}; applying the one-line clamp to deferredRound flips the observation to 10000/10000 (reverted).
Suggested fix: clamp the clause side identically — const deferredRound = deferredSuggestions.length ? Math.min(prevRound + 1, LEDGER_MAX_ROUND) : 0; — and update the prevRoundFor docstring's "(which stamps prevRound + 1)" to the clamped expression.
中文说明
新的轮次钳制只应用到了账本标记的 stamp 上；延迟建议子句的 deferredRound = prevRound + 1（约第 2270 行，既有代码）没有钳制，破坏了两个注释断言的不变量——prevRoundFor 的 JSDoc（「两者不能对这是哪一轮产生分歧」）和该子句自己的注释（「子句与标记不能对哪一轮被延迟产生分歧」）。失败场景：prevRound 可以恰好到达 LEDGER_MAX_ROUND（parseLedger 接受 round == 上限、只拒绝 >）：标记 stamp 出第 10000 轮、ids 为 R10000-*，而发布的正文却写「Deferred under the convergence posture (round 10001)」（中文：第 10001 轮）——超过上限的轮次数字出现在第 10000 轮的标记旁边。改动前两个消费方是一致的。修复：对子句一侧做同样钳制，并把 prevRoundFor 文档字符串里的「（stamp prevRound + 1）」改为钳制后的表达式。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // and carries OLDER ones forward. | ||
| const squatting = | ||
| 'LGTM <!-- qwen-review-ledger {"v":1,"round":3,"findings":[' + | ||
| '{"id":"R4-1","sev":"C","file":"a.ts","title":"squat"},' + |
There was a problem hiding this comment.
[Suggestion] R13-8: The squat-filter fixture's only future-round id is the immediate-next round (R4-1 against a round-3 marker), so parseLedger's general idRound > markerRound predicate is unpinned. — Failure scenario: the mutant Number(m[1]) === raw.round + 1 (instead of > raw.round) survived the entire pr-context + ledger + compose-review suites — 384 passed — so a deeper-future squat (R5-1/R6-1) parses, rides the marker chain, and collides when compose stamps genuinely new findings two rounds later: one claim under two ids, and every genuinely new finding renumbered past the squatted block — the exact harm the filter's comment says it prevents.
Witness (probe): the === round + 1 mutant leaves 384/384 tests green across the three suites (predicate restored afterwards; green re-verified).
| '{"id":"R4-1","sev":"C","file":"a.ts","title":"squat"},' + | |
| '{"id":"R4-1","sev":"C","file":"a.ts","title":"squat"},' + | |
| '{"id":"R6-1","sev":"S","file":"e.ts","title":"deep squat"},' + |
(keep the deep id out of the expected surviving ids — the > predicate is then pinned against the === mutant).
中文说明
squat 过滤夹具中唯一的未来轮次 id 是紧邻的下一轮（3 轮标记上的 R4-1），因此 parseLedger 的一般谓词 idRound > markerRound 没有被钉住。失败场景：突变体 Number(m[1]) === raw.round + 1（替代 > raw.round）在 pr-context + ledger + compose-review 全部 384 条测试中存活，于是更远的未来 squat（R5-1/R6-1）能通过解析、随标记链传递，并在两轮后 compose 为真正的新发现 stamp 时发生冲突：同一主张出现两个 id，每个真正的新发现都被重新编号到 squat 块之后——正是过滤器注释声称要防止的损害。修复：在同一夹具中补一个更远的未来 id（如 R6-1），并让它不出存活的 id 期望列表——> 谓词随即对 === 突变体形成钉扎。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ], | ||
| 'maintainer', | ||
| ).recovered; | ||
| expect(restored?.ledger.findings.map((f) => f.id).sort()).toEqual([ |
There was a problem hiding this comment.
[Suggestion] R13-9: The union merge's own-first ordering — load-bearing under the findings cap — is pinned by no test: all three merge-case fixtures hold 1–2 entries, and the only multi-entry assertion .sort()s the ids. — Failure scenario: both an own marker and a foreign winner can legitimately carry up to LEDGER_MAX_FINDINGS (50) findings each, so a union of up to 100 disjoint ids is reachable on a long-lived PR with a CI-bot interleave. Own-first is what makes the cap trim FOREIGN entries, honoring the merge's own rationale ("a displaced or doctored marker cannot retire a certified entry from view"); a mutant reversing the concatenation survived the whole suite, and the cap would then trim THIS account's certified entries first — reintroducing the suppression class the union was added to kill.
Witness (probe): the reversed-concatenation mutant leaves pr-context.test.ts 91/91 green; a 50-own + 50-foreign disjoint probe fails under the mutant (expected [] to have a length of 50 but got +0 — 0 of 50 own entries survive the cap) and passes on restored code.
Suggested fix: add a merge case where own carries LEDGER_MAX_FINDINGS findings and the foreign winner adds ≥1 disjoint id, asserting every own id survives the cap and dropped counts the trimmed foreign entries.
中文说明
联合合并的「本账号优先」顺序在发现条数上限下是承重逻辑，却没有任何测试钉住：三个合并用例夹具都只有 1–2 条，唯一的多条断言还对 id 做了 .sort()。失败场景：本账号标记与外部胜出者各自都可以合法携带至多 LEDGER_MAX_FINDINGS（50）条发现，因此在有 CI 机器人交错的长寿命 PR 上，至多 100 个不相交 id 的并集是可达的。「本账号优先」正是让上限裁剪外部条目的原因，与合并自身的设计理由一致（「被替换或伪造的标记不能让已认证条目从视野中消失」）；反转拼接顺序的突变体在整个测试套件中存活，上限会转而优先裁剪本账号已认证的条目——重新引入联合合并要消灭的压制类缺陷。修复：补一个合并用例——本账号携带 LEDGER_MAX_FINDINGS 条、外部胜出者再增加至少 1 个不相交 id，断言所有本账号 id 在上限下存活、dropped 计入被裁剪的外部条目。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| it('yields nothing with no login, no marker, or a malformed one', () => { | ||
| it('treats an unknown login as foreign — an anchor needs a proven owner', () => { | ||
| const found = latestLedger( | ||
| [review('bot', '2026-01-01T00:00:00Z', serializeLedger(anchored))], |
There was a problem hiding this comment.
[Suggestion] R13-10: The headroom bound under a NULL login (failed identity lookup — every marker foreign, ownMax 0) is unpinned: the only null-login test uses the round-2 anchored fixture, which clears any plausible headroom value, though the FOREIGN_ROUND_HEADROOM doc explicitly promises "Under a FAILED identity lookup (null login) … recovery is bounded to rounds ≤ the headroom." — Failure scenario: a mutant conditioning the bound on identity being known (guarding it with me) survived the whole suite; during a rate-limit blip (login null) a squatter's round-9999 marker beside the legitimate bot's round-3 marker is adopted round-first, compose's capped stamp pins the counter at LEDGER_MAX_ROUND, and every later round re-issues the same ids against different findings — the permanent win the headroom exists to prevent, reopened exactly during the identity-outage fallback. Distinct from R13-7 (constant value, non-null path) and R13-14 (the deletion arm).
Witness (probe): if (me && foreign && ...) mutant leaves 91/91 green; with login null + a round-9999 marker the mutant arm recovers it (expected { ledger: {...} } to be null — Received: { author: 'stranger', foreign: true }); the restored arm declines.
Suggested fix: extend the null-login test (or add a sibling) with a second fixture pair — marker(3) by the bot plus marker(500) by a stranger, login null — asserting round 3 wins.
中文说明
空登录（身份查询失败——所有标记都算外部、ownMax 为 0）下的 headroom 上界没有被钉住：唯一的空登录测试用的是 2 轮的 anchored 夹具，对任何合理的 headroom 取值都直接通过，而 FOREIGN_ROUND_HEADROOM 的文档明确承诺「身份查询失败（空登录）时……恢复被限制在 ≤ headroom 的轮次」。失败场景：把上界条件改为「身份已知才生效」（以 me 作守卫）的突变体在整个套件中存活；限流抖动（login 为 null）期间，合法机器人的 3 轮标记旁边一个抢占者的 9999 轮标记会被轮次优先采纳，compose 的钳制 stamp 把计数器钉死在 LEDGER_MAX_ROUND，此后每一轮都对不同的发现重发相同 id——正是 headroom 要防止的永久性胜出，恰恰在身份故障回退路径上重新敞开。修复：给空登录测试补第二组夹具——机器人 marker(3) + 陌生人 marker(500)、login 为 null——断言 3 轮胜出。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| } | ||
| }); | ||
|
|
||
| it('a DIRECTORY entry is filtered — search permission is not executability', () => { |
There was a problem hiding this comment.
[Suggestion] R13-12: The new directory test does not pin the statSync(path).isFile() branch it claims to: with isFile() deleted, openSync(dir, 'r') succeeds and readSync throws EISDIR into the same catch, yielding the identical unusable = true verdict — the mutant survives the whole suite. — Failure scenario: the branch's real value is the shape no test can safely run: a mode-0755 FIFO passes X_OK, and openSync without O_NONBLOCK blocks until a writer appears — hanging the event loop at the first spawn for that entry (the verdict is cached per path, so one hang is enough). isFile() is the only structural refusal, and an invited "redundant check" cleanup would reintroduce the hazard. The file's own test philosophy pins exactly this shape of gap ("A regression in the header read … would have passed every test").
Witness (probe): on a real directory, open succeeded, read threw EISDIR; the live gate verdict with isFile() removed is identical (unusable=true) — mutant survives the suite.
Suggested fix: spy fs.statSync and assert it is called and its isFile() result decides the directory case (or assert the catch path is NOT reached); also correct the comment's "only the regular-file check can refuse this shape", which on Linux is additionally refused by the EISDIR catch.
中文说明
新的目录测试并没有钉住它声称要覆盖的 statSync(path).isFile() 分支：删掉 isFile() 后，openSync(dir, 'r') 成功、readSync 抛 EISDIR 落进同一个 catch，得到完全相同的 unusable = true 判定——该突变体在整个套件中存活。失败场景：该分支真正的价值在于任何测试都无法安全运行的形态：mode-0755 的 FIFO 能通过 X_OK，不带 O_NONBLOCK 的 openSync 会阻塞直到出现写入者——在该入口的首次 spawn 时挂起事件循环（判定按路径缓存，挂起一次就够了）。isFile() 是唯一的结构性拒绝，一次「冗余检查」式的清理就会重新引入该隐患。修复：spy fs.statSync 并断言它被调用、且其 isFile() 结果决定了目录用例（或断言没有走进 catch）；同时修正注释里「只有常规文件检查能拒绝这种形态」的说法——在 Linux 上 EISDIR catch 也会拒绝它。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const attempt = (): unknown => | ||
| runBuildTest({ |
There was a problem hiding this comment.
[Suggestion] R13-13: The identity gate's refusal-before-execution property is unpinned: every identity-mismatch case in "refuses to continue another run's report" runs through this attempt closure with the non-recording exec: okResult, so the suite proves only the throw, not that nothing executed first. — Failure scenario: a refactor relocating the run-identity verification from the top of runBuildTest into the resume/merge step left all 102 tests green under the probe mutant — while the recording probe flipped: the stale report's notRun suite executed against this round's tree before the refusal (spending the per-call budget and running side-effecting suites on a tree the gate says has no installed/built state). The sibling grammar-gate test pins exactly this property two tests earlier (expect(calls).toEqual([]), "Refused BEFORE anything ran") — the asymmetry is accidental, not deliberate.
Witness (probe): relocation mutant — build-test.test.ts 102 passed; recording probe expected [ 'npm test --workspace="packages/a"' ] to deeply equal [] (original code: probe green with calls = []).
Suggested fix: give the identity cases a recording exec (exec: (command) => { calls.push(command); return okResult(command); }) and assert expect(calls).toEqual([]) after each expect(attempt).toThrow(...), mirroring the grammar-gate test.
中文说明
身份门禁「先拒绝、后执行」的性质没有被钉住：「refuses to continue another run's report」中的每个身份不匹配用例都通过这个 attempt 闭包、使用不记录的 exec: okResult 运行，因此套件只证明了抛错，没有证明「没有任何命令先被执行」。失败场景：把 run-identity 校验从 runBuildTest 顶部挪到 resume/merge 步骤的重构，在探针突变体下全部 102 条测试依旧绿——而带记录的探针翻转了：陈旧报告的 notRun 套件在拒绝之前就对着本轮的树执行了（花掉整次调用预算，并在门禁声称「没有安装/构建状态」的树上运行有副作用的套件）。上面两条的语法门禁测试恰好钉住了同一性质（expect(calls).toEqual([])、「Refused BEFORE anything ran」）——这里的不对称是意外而非刻意。修复：给身份用例换上带记录的 exec，并在每个 expect(attempt).toThrow(...) 后断言 expect(calls).toEqual([])，与语法门禁测试对齐。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…ot swap the list R13-1: during an identity-lookup outage (currentUser throws, or answers empty) every marker walks as FOREIGN — there is no me — so the union that protects the certified work list never has an own side to merge over, and the recovered winner was written to the side file WHOLESALE: a drive-by marker posted at this account's current round (visible to any authenticated user, inside the zero-base headroom) replaced this machine's last known-good list on equal round + larger review id, and the swap persisted — the marker stays on the PR, so every later outage reopened it. The suppression class the union merge exists to kill, reopened through the anonymous path. persistRecoveredLedger now takes identityKnown and gives the anonymous recovery its own outcome: with a readable existing file, a same-round winner changes NOTHING, and a strictly-higher round advances only the round counter (refusing that too would re-expose R4-2's id-space collision — a lagging counter re-issues ids the PR already carries) plus the reviewId tiebreak; the findings stay this machine's own, and sha/commitId are dropped — an anonymous round cannot be re-vouched, and an anchor superseded by rounds this account never certified must not scope the next review (the healthy foreign-winner path strips it at the recovery seam for the same reason). With no readable file there is nothing to protect and the write stays wholesale. The new suite pins the drive-by (file byte-identical), the advance (counter moves, list survives, anchor and age reference gone — with noOwnReview deliberately true so a positional swap of the two booleans deletes the file and fails the test), and the no-file arm; the guard mutant fails exactly the first two.
commented
Aug 17, 2026
|
Round 14's blocker is fixed in 7f05080. R13-1 — taken, in the shape your design note asked for: the anonymous path distinguishes strictly-higher rounds instead of choosing between the two regressions. One residual named honestly: this guards the persisted state, not the rendered context — during the outage the round still REVIEWS with the anonymous winner's list (attributed as THEIR claims), so an entry absent from it is not re-asserted that round. The side file now survives to the next identity-restored run, which is the recovery point. The new suite pins all three arms, with 中文说明第十四轮的阻断项已在 7f05080 修复。 R13-1 —— 按你设计备注要求的形态采纳：匿名路径区分「严格更高的轮次」，而不是在两个回归之间二选一。 如实说明一处残余：此守卫保护的是持久化状态，不是渲染上下文——故障期间本轮仍以匿名胜者的清单（标注为 THEIR claims）进行评审，缺席的条目该轮不会被重新主张。side 文件现在能存活到下一次身份恢复的运行，那才是恢复点。 新套件钉住全部三个分支，推进夹具刻意传 |
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Unresolved, please confirm:
- [Critical] packages/cli/src/commands/review/lib/budget.ts L464-489 (round-2-era ZH budget-gap classifier, 6 Critical threads) — bodies truncated in the context file; the classifier region was rewritten since and its variants were not individually trac…
- [Critical] packages/cli/src/commands/review/compose-review.ts:962,1748 (R1-14, R6-17 anchor-exemption family) — bodies truncated; mechanism not traced at HEAD
- [Critical] 3 entries — bodies truncated; not traced at HEAD:
- packages/cli/src/commands/review/compose-review.ts (R3-43, R7-1, R8-8)
- packages/cli/src/commands/review/agent-prompt.ts + agent-prompt.test.ts (R2-9, R3-18 resume-rule shapes; R2-37 POSIX-literal/Windows)
- packages/core/src/skills/bundled/review/SKILL.md (3 threads) and packages/core/src/utils/shellContextEnv.ts:55
- [Critical] packages/cli/src/commands/review/lib/npm-toolchain.ts (R1-2, R1-3, R2-42 x2, R7-2 x2 resume-merge caveat family) — bodies truncated; merge semantics not traced at HEAD
- [Critical] packages/cli/src/commands/review/pr-context.ts rounds-1-11 threads (L691, L695, L717, L837, L1024, L1047, L1356) — file heavily rewritten since; bodies truncated, not traced at HEAD
- [Critical] Note: none of these re-asserted themselves in round 13's ledger (round 13 carried zero pre-R13 entries and certified scope at e792d44), but each mechanism above was not independently traced at HEAD this round — recorded cannot tell rather …
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the changed suites ran locally on Linux only.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未决，请确认：共 8 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the changed suites ran locally on Linux only。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the CLI integration surface was not exercised locally。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // Separately from deletion: an ANONYMOUS recovery (identity unknown) | ||
| // must not replace the persisted work list — the helper's fourth | ||
| // outcome. Every marker walks as foreign without a `me`, so the union |
There was a problem hiding this comment.
[Suggestion] This commit adds a fourth persistence outcome (anonymous recovery over an existing file → counter/tiebreak advance only), but two summaries in the same file still say "three outcomes" and are now contradicted by the diff: the docblock header at pr-context.ts:919 ("Three outcomes, each honest about what this run learned:") now introduces four bullets, and the call-site comment at pr-context.ts:1456 still reads "recovered → written whole (any account ...)" — which is now false for anonymous recoveries — while this added comment calls the new branch "the helper's fourth outcome". — Failure scenario: a maintainer later changing the deletion/persistence logic reads the authoritative-looking call-site summary, believes any recovered ledger is written whole regardless of identity, and "simplifies" the anonymous branch away — re-opening the permanent certified-list swap (R13-1) this commit closes.
Fix: change the docblock header at :919 to "Four outcomes" (or nest the anonymous bullet under "Recovered"), and extend the call-site summary at :1456, e.g. recovered anonymously over an existing file → round counter and tiebreak id advanced only, findings kept, anchor/age reference stripped.
中文说明
本提交为 persistRecoveredLedger 新增了第四种持久化结果（对已有文件的匿名恢复 → 只推进轮次计数器与平局判定 id、保留本机 findings），但同一文件中仍有两处摘要写着「三种结果」，与本 diff 自相矛盾：pr-context.ts:919 的 docblock 标题（"Three outcomes, each honest about what this run learned:"）下面现在列着四条；pr-context.ts:1456 的调用点注释仍写着 "recovered → written whole (any account ...)"——对匿名恢复而言这已不再成立；而本 diff 新增的注释却称新分支是「helper 的第四种结果」。失败场景：日后修改删除/持久化逻辑的维护者读了这份看似权威的调用点摘要，以为任何恢复出的 ledger 都会不分身份整体写入，从而把匿名分支「简化」掉——重新打开本提交刚关上的认证列表被整体替换的漏洞（R13-1）。修复：把 :919 的标题改为「Four outcomes」，并在 :1456 的枚举中补上匿名情形。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Confirmed at head 7f05080: the docblock at L919 still says "Three outcomes" while the helper now documents four (anonymous recovery is the fourth), and the call-site comment at L1456 ("recovered → written whole (any account …)") no longer holds for anonymous recoveries. Valid comment-accuracy fix. Deferred under the round-14+ scope rule (Suggestions deferred; no behavioral defect at HEAD) — worth folding into the next commit that touches this region. Not pushing to this branch while the author's /triage verification is running.
| // it is ignored on the recovered path, so a positional swap of the two | ||
| // booleans would delete the file and fail both assertions. |
There was a problem hiding this comment.
[Suggestion] The comment's stated pin mechanism is wrong: with recovered non-null the helper always returns inside the first if (recovered) branch, so the noOwnReview/rmSync deletion branch is unreachable here — a positional swap of the two booleans does not "delete the file"; it makes identityKnown true, skips the anonymous guard, whole-writes the stranger's ledger, and the toEqual assertion fails. Probe at this head: after the swap the file EXISTS and contains the stranger's ledger (their findings, their sha); the content assertions are what catch the swap. — Failure scenario: a maintainer restructuring the helper's branch order or this test relies on the comment's claim that the swap is caught via deletion and forms the wrong mental model of which line of defense protects this deliberately adversarial argument layout.
| // it is ignored on the recovered path, so a positional swap of the two | |
| // booleans would delete the file and fail both assertions. | |
| // it is ignored on the recovered path, so a positional swap of the two | |
| // booleans would skip the anonymous guard and whole-write the stranger's | |
| // ledger, failing the content assertions below. |
中文说明
该注释描述的钉住机制有误：当 recovered 非空时，helper 总是在第一个 if (recovered) 分支内返回，noOwnReview/rmSync 删除分支在此不可达——两个布尔参数位置互换并不会「删除文件」，而是使 identityKnown 变为 true、跳过匿名守卫、把陌生人的 ledger 整体写入，最终由 toEqual 断言捕获。本轮探针实测：互换后文件仍然存在、内容为陌生人的 ledger（其 findings、其 sha），是内容断言而非删除捕获了互换。失败场景：日后重构 helper 分支顺序或该测试的维护者，按注释所述以为互换是经删除分支被捕获，对这个刻意设计的对抗性参数布局形成错误的防御心智模型。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Confirmed at head 7f05080: with recovered non-null the helper returns inside the first if (recovered) branch, so the deletion branch is unreachable in this test — a positional swap of the two booleans is caught by the content assertions (stranger's ledger whole-written), not by file deletion. The comment's stated pin mechanism is wrong as written. Comment-only fix; deferred under the round-14+ scope rule.
| noOwnReview: boolean, | ||
| identityKnown: boolean, |
There was a problem hiding this comment.
[Suggestion] Two adjacent positional booleans encode a safety invariant (noOwnReview ⇒ identity proven) that the helper neither enforces nor can distinguish from a swapped call, and the sole production wiring (pr-context.ts:1475-1480) is pinned by no test — the unit tests call the helper directly, and the handler tests in pr-context.test.ts mock node:fs and never exercise any persist outcome. — Failure scenario: a swap of the two arguments at the call site compiles cleanly (both are boolean); on an identity-known run where recoverLedger throws (recovered === null), the swapped call passes raw identityKnown (true) as noOwnReview → the deletion branch fires → the side file is removed, the round counter resets, and round ids the PR already carries are re-issued — with the entire suite green. Probe at this head: with the call-site arguments swapped, all four touch-point suites (pr-context-persist 8, pr-context 91, submit 67, compose-review 270) passed — 436/436.
Fix: take an options object (persistRecoveredLedger(side, recovered, { noOwnReview, identityKnown })), or make the deletion branch self-enforcing by conjoining identityKnown there as well — plus one handler-level test pinning the wiring (identity-known + recovery-threw → no deletion).
中文说明
两个相邻的位置布尔参数编码了一条安全不变量（noOwnReview ⇒ 身份已证实），但 helper 既不强制校验、也无法将其与参数互换区分开来；且唯一的生产调用点（pr-context.ts:1475-1480）没有任何测试钉住——单元测试直接调用 helper，而 pr-context.test.ts 的 handler 测试 mock 了 node:fs、从不触达任何持久化结果。失败场景：调用点两个实参互换可以通过编译（两者都是 boolean）；在一次身份已知、但 recoverLedger 抛错（recovered === null）的运行中，互换后的调用把裸的 identityKnown（true）传成 noOwnReview → 删除分支触发 → side 文件被删除、轮次计数器被重置、PR 上已存在的轮次 id 被重新签发——而整套测试全绿。本轮探针：互换调用点实参后，四个相关套件（pr-context-persist 8、pr-context 91、submit 67、compose-review 270）全部通过——436/436。修复：改用选项对象，或在删除分支内自行合取 identityKnown 使不变量自强制，并补一个钉住接线的 handler 级测试（身份已知 + 恢复抛错 → 不得删除）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
The hazard is real at head 7f05080: the call site (L1475–1481) passes two adjacent bare-boolean expressions positionally, and a swap compiles cleanly with the observed bad outcome. The fix is either a signature refactor (named options) or a mutation-pinning test — both outside the round-14+ scope rule (no drive-by refactor, no mutation-only hardening without a reproduced regression). Deferring as a follow-up; no behavioral defect on the current wiring, which the four touch-point suites pin as-is.
commented
Aug 17, 2026
|
Confirming the eight cannot-tell families. Every thread listed was answered in its own round with a fix commit and a per-round reply on this PR; none was left open. Mapping each family to where it was resolved:
Consistent with your own observation: round 13's ledger carried zero pre-R13 entries with scope certified at e792d44 — the prior full round's reverse audit ruled these threads and none re-asserted. This round's three Suggestions are deferred without exception. 中文说明确认八个 cannot-tell 家族：所列线程均已在各自轮次以修复提交 + 逐轮回复处理，无一遗留。对应关系：budget.ts ZH 分类器（第 2 轮时代 6 条）→ e0524b0 与 f39f3bf 重写为封闭词表；compose-review 锚点豁免家族（R1-14、R6-17）→ d024939 精确规范文本 + 694f30a 机械化缓存规则；R3-43/R7-1/R8-8 → f39f3bf/661d1bab96/d0249394de；agent-prompt R2-9/R3-18/R2-37 → e0524b0、f39f3bf54b；SKILL.md 三条 → d024939/694f30a4fb/8c27f230cd；shellContextEnv.ts:55 即 R1-11 → e792d44（第 13 轮）；npm-toolchain 续跑 caveat 家族 → 661d1ba 首修 + 8c527fa 结构化 liveCaveat；pr-context 轮 1-11 线程 → 恢复重设计系列（04a52a5e5f → 2118866 → 8c527fa → 8c27f23 → f0efe67 → 7f05080）。 与你自己的观察一致：第 13 轮账本零条 pre-R13 条目、scope 在 e792d44 认证——上一整轮反审已裁决这些线程且无一复现。本轮三条 Suggestion 一律延后。 |
commented
Aug 17, 2026
|
@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: 63 passed · 0 failed · 63 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：63 通过 · 0 失败 · 63 总计 Verification reportPR #9175 deep verification —
|
| Claim | Status |
|---|---|
| Central — anchor rides when the only cap is an unreviewable dimension; still withheld on coverage doubt / whiffed lens | A/B proven (§Anchor A/B) |
| Secondary 1 — ledger cross-account recovery: work list travels, anchor never does | A/B proven (§Ledger A/B) |
| Secondary 2 — failing-file attribution measured off untrimmed output (11 vs 1) | A/B proven (§Delta A/B) |
Build-and-test --resume continuation |
Unit-gated only (see Not covered) |
Bundle-launch entry propagation (run.ts + core isUnusableScriptEntry) |
Unit-gated only |
| Chinese "nothing to disclose" classifier | Unit-gated + mutation + scaling ladder |
Count/list validation in the --resume report guard |
Unit-gated only |
A/B method (common to all three tables)
Base control: scratch worktree at HEAD^1 (7091b8c), core + cli rebuilt with tsc --build (nested node_modules deps symlinked from the root install — the lockfile is untouched by this PR; @lydell/node-pty is wired because core's tsconfig paths entry resolves relative to the worktree). Purity asserted before trusting the control: readlink -f tmp/base-tree/node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/tmp/base-tree/packages/core, and base dist contains 0 occurrences of dimensionGapsAreDepthOnly vs 8 in head. Harnesses (h-anchor.mjs, h-ledger.mjs, h-delta.mjs) import the compiled dist/ of the arm under test and drive real functions — no mocks; the anchor fixture plumbing mirrors compose-review.test.ts's transcript/plan records. driver.mjs encodes every expectation, including the expected base-arm reds, so an intended control failure counts as a passing assertion. Raw per-cell outputs: raw-arm-outputs.json; witness: 01-ab-expectation-table-63-cells.png.
Central claim — the incremental anchor
Cells C1–C8 (harness h-anchor.mjs): a fully-covered composed round (receipt-proven chunk reads, Step 4/5 records) with one manipulated input per cell. The ledger marker rides in both arms; the sha is the withheld half ("findings always ride").
| Cell | Scenario | Base (7091b8c) | Head (7f05080) | Flip? |
|---|---|---|---|---|
| C1 | only cap unreviewed-dimension (build-and-test — …) |
sha withheld | sha = fetchedSha, scopeUnproven=false, dimensionGapsAreDepthOnly=true |
✅ |
| C2 | bare build-and-test head |
sha withheld | sha rides | ✅ |
| C3 | whiffed lens (security — the agent whiffed twice) |
sha withheld | sha withheld (dimensionGapsAreDepthOnly=false) |
invariant holds |
| C4 | coverage evidence short (agents never opened the diff) | sha withheld | sha withheld (scopeUnproven=true) |
invariant holds |
| C5a/b | budget stop marker, entry dropped vs relayed | sha withheld | sha rides in both; bodies byte-identical (hash e80756b6…) |
✅ |
| C6 | stop-shaped prose, no marker file | sha withheld | sha withheld (marker-anchored exemption) | invariant holds |
| C7a/b | round-cap stop, dropped vs relayed | sha withheld | sha rides in both; bodies byte-identical (hash 107d3079…) |
✅ |
| C8 | cannotTellCriticals present |
sha withheld | sha withheld | invariant holds |
5 flips, 6 invariants, 26 scripted assertions in this table. Witness: 01-ab-expectation-table-63-cells.png.
Secondary 1 — ledger cross-account recovery (split trust)
Cells L1–L6 (harness h-ledger.mjs, driving recoverLedger/recoverOwnLedger, parseLedger, persistRecoveredLedger, renderLedgerSection from each arm's dist):
| Cell | Scenario | Base | Head |
|---|---|---|---|
| L1 | bot-only marker (round 3, sha, R3-1) | not recovered (null) |
work list recovered, sha stripped, foreign=true, author=ci-bot, commitId rides |
| L2 | own round 2 + bot round 3 | sees only own round 2 (own sha rides) | round-first → round 3 wins; findings merged ["R2-1","R3-1"] (own authoritative); sha stripped; case-insensitive login |
| L3a/b/c | foreign-round headroom | foreign markers ignored entirely | ownMax+65 (anon) refused; ownMax+64 adopted (boundary); ownMax(2)+66 refused, own kept |
| L4 | future-round id squat (R4-1 inside round-3 marker) |
accepted (["R4-1","R3-1"]) |
filtered (["R3-1"]) |
| L5 | anonymous recovery over an existing side file (drive-by round 7) | no recovery at all; strip path drops reviewId |
own findings kept, counter advances to 7, sha/commitId dropped — the fourteenth-round fix |
| L6 | renderLedgerSection(ledger, 'ci-bot') |
author arg ignored ("this account's…") | names @ci-bot, says "another account", offers no anchor |
21 assertions. The L4 base cell shows the pre-PR read path accepted a squat that would pre-claim the next round's id prefix; head filters it read-side only, matching the code comment's claim that the pipeline's own writer can never produce one (min(prevRound+1, LEDGER_MAX_ROUND)).
Secondary 2 — failing-file attribution off untrimmed output
Cells T1–T3 (harness h-delta.mjs): one PR-side report fixture shared by both arms — trimmed stored output with one visible FAIL line plus the capture-time failingFiles (11 entries), exactly the shape head's build-test writes and the shape of the live incident the PR describes. Base rerun stubbed through the exported exec seam.
| Cell | Scenario | Base | Head |
|---|---|---|---|
| T1 | base rerun passes → all PR failures net-new | re-parses trim: netNew = 1 of 11, prTruncated=true + disclosure |
capture-time set: netNew = 11, prTruncated=false, no disclosure |
| T2 | base fails the same 11 | shared understates (1 of 11) |
shared = 11, netNew empty |
| T3 | legacy report (no failingFiles field) |
re-parses trim → 1 file | identical fallback → 1 file, prTruncated=true (old reports unchanged) |
10 assertions. T1 is the 1→11 flip the PR's evidence table claims; T3 proves the backward-compat path is behavior-preserving.
Vacuity / mutation matrix
Four single-point mutants of the head source; each run against the exact vitest cell added for it, then restored (git status clean after every round). A green unmutated control ran first for the central cell.
| Mutant | Guard removed | Suite cell | Result |
|---|---|---|---|
| M1 | failClosed → base expression (cappedBy.length > 0) |
still ANCHORS a round whose only cap is an unreviewable dimension |
KILLED — expected undefined to be 'deadbeef00112233' at the sha assertion (witness: 02-mutation-m1-failclosed-revert-kills-anchor-test.png) |
| M2 | drop measured ?? precedence (force trim re-parse) |
prefers the PR side's capture-time set, so a trimmed netNew survives |
KILLED — expected [] to deeply equal [ 'src/z.test.ts' ] |
| M3 | skip stripAnchor at the recovery seam |
drops the anchor from ANOTHER account, keeping the work list |
KILLED — expected 'abc1234def567890' to be undefined |
| M4 | remove ZH_PLACEHOLDER branch from PLACEHOLDER_GAP_RE |
drops the SAME non-answers written in Chinese |
KILLED (1 failed / 2 passed of the -t "Chinese" selection) |
Every mutant died on the behavioural mismatch its test exists to catch, not on an import/compile break. Positive control: the M1 cell passes unmutated (12.99s run), and the whole suite is green unmutated (below). Directional note on M4: the two sibling KEEP-tests still pass under this mutant by construction — they pin the opposite failure mode (an over-greedy classifier swallowing real gaps), so the closed-vocabulary claim is pinned jointly by the drop- and keep-halves, not by M4 alone.
Targeted gates (live)
packages/clireview suite at the merge commit: 82 files, 3244 passed, 0 failed, 4 skipped (witness:03-gate-review-suite-3244-pass.png; log:gate-review-suite.log). The author's reported macOS-only failure (bundled-skill quoting) does not reproduce on Linux, consistent with the PR body.packages/coreshellContextEnv.test.ts: 31/31 passed (log:gate-core-shellcontext.log).- Gate liveness is proven by the four mutation kills above (the same vitest installation went red on command).
- Premise check for the budget redesign: the shell tool's 600s per-call ceiling is real code (
packages/core/src/tools/shell.ts:5124,'Timeout cannot exceed 600000ms (10 minutes).'), andDEFAULT_WHOLE_CALL_BUDGET_S = 600 − 30/DEFAULT_COMMAND_TIMEOUT_S = 540are arithmetically inside it.
Scaling ladder — new ZH placeholder regex
New regex over agent-written disclosure lines (budget.ts); the module header claims linearity. Ladder run per rung under timeout 30 (harness h-ladder.mjs, witness 04-zh-classifier-ladder-linear.png, transcript ladder-transcript.txt):
| Shape | 2k chars | 3k | 5k | 20k (60k for negation-walk) |
|---|---|---|---|---|
所有×N after token+separator |
1.85 ms | 1.64 ms | 1.69 ms | 2.06 ms |
| punctuation-tail ×N | 1.98 ms | 1.52 ms | 1.51 ms | 1.59 ms |
| non-matching junk tail | 1.60 ms | 1.63 ms | 1.64 ms | 1.76 ms |
| negation walk + affirmed tail | 2.09 ms | 1.78 ms | 1.90 ms | 4.47 ms |
Flat across four rungs — no superlinear curve; semantics spot-checked (placeholder dropped, real gaps kept, 还没有完成… kept).
Findings
F1 (Suggestion) — the deadline this PR fixes in build-test survives at the old value in test-delta, on a path this PR newly opens
The PR's own words on the per-command deadline: "measured at 401 seconds against a 300-second deadline — which is not a thin margin but a guaranteed kill". It raises build-test's default to DEFAULT_COMMAND_TIMEOUT_S = 540 (base had default: 300). But test-delta — the downstream half of the same measurement chain, which reruns the failing suite in the base tree — is untouched at exactly the value the PR calls a guaranteed kill:
packages/cli/src/commands/review/test-delta.ts:214 const DEFAULT_TIMEOUT_S = 300; // identical at base
packages/cli/src/commands/review/test-delta.ts:461 default: 300 // identical at base
while head's build-test.ts:806 now defaults to DEFAULT_COMMAND_TIMEOUT_S (540), and test-delta's own TOTAL_BUDGET_MS = 540_000 shows the 540 scale was the intent. The agent brief that invokes test-delta (agent-briefs.ts) passes no --timeout.
Why it matters now and not at base: at base, build-test itself died at 300s on this repo's packages/cli suite, so a failing cli suite never reached test-delta as a rerunnable entry. This PR makes build-test finish it (540s + --resume), so the flow failing PR-side suite → test-delta base rerun is newly reachable — and ends at the old ceiling: the base rerun of the same 401s suite is killed at 300s, the entry lands in "timed out — attributes nothing", and the review falls back to the path rule, losing exactly the measurement this PR exists to sharpen. Blast radius: every review where a PR-side suite fails and its natural duration exceeds 300s — on this very repo, the packages/cli suite named in the PR's own measurement.
Bounds (what it does NOT do): fails safe — the timeout is disclosed in the report, nothing is fabricated, and no verdict is waved through on false evidence; it is a loss of attribution, not a wrong attribution.
Suggested fix (measured, scratch): raise both DEFAULT_TIMEOUT_S and the yargs default to 540 (or derive from build-budget's DEFAULT_COMMAND_TIMEOUT_S, the way the briefs now quote one number). Applied in a scratch copy: test-delta.test.ts stays green (29/29), restored after. The suite is green both with and without the bump, i.e. nothing pins this axis — the fix should ship with a fixture asserting the default deadline ≥ the slowest measured suite (a rerun of a 401s-shaped command admitted with a full window, not clamped).
F2 (low, observation) — the dimension exemption matches English heads only; over-withholding in the safe direction
isNonDiffDimensionGap accepts build-and-test / Build & Test — reason / the build and test check / BUILD-AND-TEST (probed), but not 构建与测试 — … (the Chinese dimension name under outputLanguage: 中文) nor build-test verification. Both miss → dimensionGapsAreDepthOnly=false → anchor withheld. That is the safe direction (over-withholding never scopes past unread lines), so this is not a defect — but a zh-orchestrated round would keep paying the full-diff re-review cost this PR removes. Worth a one-line widening to the canonical zh name if zh reviews are common; no urgency.
Not covered
--resumecontinuation end to end (real npm install/build/suites across calls, provisional replacement, refusal over unbuilt packages). Covered at unit level by the suite (the--resume: the ceiling is per call, not per runblock and theended before its test phasecell are green within the 3244), but no live multi-call run was performed: its natural cost is multiples of this round's whole budget on this repo. This reproduces the shape via unit oracles, not the live trigger.- Bundle-launch entry propagation (
run.tschildEnv+ coreisUnusableScriptEntry): unit tests green (run.test.tscells inside the gate; core 31/31). No bundle was built and driven; theargv[1]-is-a-directory and tsx-entry shapes are pinned only by those tests. - Per-commit attribution: the checkout is depth 2 and shallow;
git rev-list HEAD^1..HEAD^2returns 1 at the boundary while the metadata snapshot lists 14 commits — per-commit verification was out of reach. The aggregateHEAD^1..HEADdiff is what every cell above exercised. - Live model runs (four headless reviews against qwen3.8-max) — no model credentials in this sandbox, by design.
- Repo-wide gates — only
src/commands/review/(cli) andshellContextEnv(core) ran; the core change is one exported utility, and no non-review core source changed beyondindex.ts's export line. - The
--resumereport guard's count/list refusal (the seventh fix) is pinned bybuild-test.test.tscells in the gate; no dedicated A/B cell was built for it.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/9175/merge (depth 2); npm ci + npm run build pre-done at HEAD. Base control: git worktree add tmp/base-tree HEAD^1, core and cli compiled with the root tree's tsc (5.x) against symlinked nested node_modules (lockfile unchanged by the PR); control purity asserted by realpath and by content grep before use. Harnesses drove the compiled dist/ of each arm directly (compose fixtures reproduce compose-review.test.ts's transcript/prompt-record plumbing on disk; the delta harness uses the exported exec seam as its only stub — the base rerun is stubbed, the PR-side parsing path under test is not). Mutations were single-hunk source edits run through the workspace vitest, restored with git checkout -- and verified clean after each. Captures produced with scripts/verify-capture.mjs. Raw logs: gate-review-suite.log, gate-core-shellcontext.log, raw-arm-outputs.json, ladder-transcript.txt; harnesses h-anchor.mjs, h-ledger.mjs, h-delta.mjs, h-ladder.mjs, driver.mjs are rerunnable from this directory.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
commented
Aug 19, 2026
|
Released in v0.21.14. |
4 participants
What this PR does
This repairs seven defects in the review pipeline, all of them found by running four complete reviews against live pull requests and watching each step rather than reading the code. Two are structural. The incremental anchor is no longer withheld because a dimension nobody could run was disclosed, so a re-review of an unchanged pull request stops paying for the whole diff again. And the build-and-test dimension can now finish on a repository whose suites do not fit inside a single shell call, by continuing the same run across calls instead of truncating it.
The rest are narrower but each cost a live run something. A ledger posted by a different account — the continuous-integration bot, in practice — is now recoverable for its work list while its reviewed-at commit still is not, which is the half that decides what the next round is allowed to skip. Test-failure attribution now measures the failing files where the raw output still exists instead of re-reading a bounded copy of it. A review launched from the bundle now tells its own helper commands which build they belong to, instead of letting them resolve whatever is installed on the path. A "nothing to disclose" answer written in Chinese is recognised as one rather than published as a gap. And the one count field sitting in a run of list fields now says what to send when it is handed a list.
Why it's needed
Every item here was measured, not inferred, and several of them had been silently shaping every review this repository produces.
The anchor chain is the expensive one. The build-and-test check cannot fit this repository's suites into the ten minutes a single shell call is allowed, so it always disclosed an unfinished dimension; that disclosure capped the verdict; the cap withheld the reviewed-at commit; and without that commit the next round reviewed the entire diff again. One run spent 119 minutes and 34M input tokens re-reviewing a pull request whose code had not changed a line since the round before. Worse, the ruling that started the chain is a judgment: two of the four runs capped and two did not, on identical mechanical facts, so whether the next round could be incremental came down to how a sentence was phrased. The anchor now answers only to the coverage evidence recomputed from the harness's own transcripts — a chunk nobody read, an agent that never opened the diff, transcripts that could not be read — because that evidence is about which lines were reached, which is the anchor's only claim.
The budget could not be fixed by spending it better. Even with every consumer build removed, install plus the builds the suites need plus the core suite plus the CLI suite already exceeds the whole-call budget, and the ten-minute ceiling is enforced by the shell tool itself. So the shape had to change rather than the numbers: the ceiling is per call, and a run may now use more than one. The per-command deadline was also below the slowest suite it had to run — measured at 401 seconds against a 300-second deadline — which is not a thin margin but a guaranteed kill, and it happened in all three runs where that suite was in scope.
The ledger's account filter turned off cross-round recovery exactly where the mechanism was designed to help. Its own documentation says the local cache is absent on continuous integration, another clone, or a colleague's checkout, and that the posted marker exists to survive that; but continuous integration posts as a bot while a maintainer runs as themselves, so the accounts differ in the ordinary case and both pull requests reviewed here had a bot marker that a local run could not see. Splitting the trust surface keeps the safety property that mattered: a work list is re-ruled entry by entry against the code before anything is repeated or retired, whereas an anchor decides which lines are never looked at again.
The failure-attribution defect ran in the dangerous direction. The pull-request side's failing files were re-parsed from a report that had already been trimmed, and on a live run that recovered one file out of eleven. A file missing from that set cannot be measured as pre-existing — but neither can it be reported as caused by the change, and the second one is the finding a reviewer needed.
The remaining three each wasted a turn or published something untrue: a review whose second helper command died because it had reached a different installation, a disclosure that announced a budget was exhausted while quoting the agent saying it was not, and a validation error that left the caller to guess which end of the mismatch was wrong.
Reviewer Test Plan
How to verify
The unit suites cover each repair, and the two structural ones can also be confirmed end to end.
For the continued build-and-test run, point the check at any workspace of this repository with a whole-call budget too small to reach every suite. The first call should stop with suites named as not run, and — if it started a suite it could not finish — record that suite as provisional rather than as an ordinary timeout. Running the same command again with the continuation flag should skip installation and building entirely, run only what is left, replace the provisional result with a real one, and leave a single entry per command in the merged report. Continuing a run that had left packages unbuilt should be refused with an explanation rather than run suites against artifacts that were never compiled.
For the anchor, compose a verdict whose only cap is an unreviewable dimension and confirm the reviewed-at commit now rides the posted marker; then compose one where the coverage evidence itself is short — an agent that made no tool call, for instance — and confirm the anchor is withheld exactly as before.
For the ledger, recover a marker posted by another account and confirm the findings arrive while the reviewed-at commit does not, and that the rendered context says whose claims they are.
npx vitest run packages/cli/src/commands/review/passes 2549 of 2550 on macOS. The single failure is pre-existing and unrelated: a bundled-skill quoting test that fails on this machine before these changes as well, and passes on Linux in continuous integration.Evidence (Before & After)
Continuing a build-and-test run, measured on this repository with real npm commands:
Failing-file attribution, on the same core suite that reports eleven failing files:
Helper commands reaching this build, same prompt, two launch paths:
Tested on
Environment (optional)
macOS 15 (darwin 25.6.0), Node 24.18.1, npm 11.16.0. The four reviews ran headless through the non-interactive review entry point against qwen3.8-max, each with an isolated home directory; the build-and-test evidence above was produced by running the check directly against this repository with real npm commands.
Risk & Scope
Linked Issues
None. Every defect here was found by running the pipeline rather than from a filed report.
中文说明
这个 PR 做了什么
本 PR 修复审查管线里的七处缺陷，全部来自对四个真实 PR 完整跑审查、并逐步骤观察——不是读代码读出来的。其中两处是结构性的：增量锚点不再因为「某个没人跑得动的维度被披露」而被扣留，因此对一个未改动的 PR 重新审查时，不必再为整个 diff 付一次钱；而在套件装不进单次 shell 调用的仓库上，构建与测试维度现在可以真正跑完——办法是让同一次运行跨调用继续，而不是被截断。
其余几处范围更窄，但每一处都让某次真实运行付出了代价。由另一个账号发布的账本——实际场景里就是持续集成的机器人——现在它的工作清单可以被恢复，而它记录的「已审查提交」仍然不可以，因为后者才是决定下一轮允许跳过哪些行的那一半。测试失败归因现在在原始输出还在的地方测量失败文件集合，而不是去重读一份被截断的副本。从打包产物启动的审查，现在会告诉它自己派生的辅助命令属于哪个构建，而不是任由它们解析到机器上恰好装着的那一份。用中文写的「没有需要披露的内容」现在会被识别为如此，而不是被当作真实缺口发布出去。以及，那个夹在一串列表字段中间的唯一计数字段，在被传入列表时会说明应该传什么。
为什么需要它
这里的每一项都是量出来的，不是推断出来的，而且其中好几项一直在悄悄影响这个仓库产出的每一次审查。
锚点这条链是最昂贵的。构建与测试检查无法把本仓库的套件塞进单次 shell 调用被允许的十分钟里，所以它总是会披露一个未完成的维度；这个披露给裁决封顶；封顶扣留了「已审查提交」；而没有这个提交，下一轮就会重新审查整个 diff。有一次运行花了 119 分钟、3400 万输入 token，去重新审查一个自上一轮起代码一行未改的 PR。更糟的是，启动这条链的那个裁定是一种判断：四次运行里两次封顶、两次没有，而机械事实完全相同——也就是说，下一轮能否增量，取决于一句话怎么措辞。现在锚点只回应从工具自身运行记录重新计算出来的覆盖证据——没人读过的分块、从未打开 diff 的 agent、读不到的运行记录——因为这些证据说的是哪些行被读到，而这正是锚点唯一的主张。
预算问题无法靠「把钱花得更好」解决。即便去掉全部消费者构建，安装加上套件所需的构建、加上核心套件、再加上 CLI 套件，就已经超过整次调用的预算，而十分钟的上限是 shell 工具本身强制的。所以要改的是形状而不是数字：上限是每次调用的，而一次运行现在可以用不止一次调用。单条命令的期限此前也低于它必须运行的最慢套件——实测 401 秒，期限却是 300 秒——这不是余量偏薄，而是必然被杀，并且在那个套件进入范围的三次运行里全部发生了。
账本的账号过滤，恰恰在这套机制被设计来帮忙的场景里把它关掉了。它自己的文档写着：本地缓存在持续集成、另一个克隆或同事的检出上都是不存在的，而发布出去的标记正是为了熬过这一点；但持续集成以机器人身份发布，维护者以自己的身份运行，所以在通常情况下账号就是不同的——这里审查的两个 PR 上都有一个本地运行看不见的机器人标记。拆分信任面保留了真正重要的安全性质：工作清单在被重复或撤销之前，本来就要逐条对照代码重新裁定；而锚点决定的是哪些行从此不再被看。
失败归因的缺陷则落在危险的方向上。PR 侧的失败文件是从一份已经被截断的报告里重新解析的，在一次真实运行中十一个文件只认回一个。缺失的文件无法被测量为既有失败——但同样也无法被报告为本次改动造成的，而后者才是审查者需要的那条发现。
剩下三处，每一处要么浪费一个回合，要么发布了不真实的内容：一次审查的第二条辅助命令因为打到了另一个安装而失败；一条披露宣称预算耗尽，引用的却是 agent 说「未触及预算上限」的原话；以及一个校验错误，把「不匹配的是哪一端」留给调用方去猜。
审查者验证计划
如何验证
单元测试覆盖了每一处修复，两处结构性修复还可以端到端确认。
对于可续跑的构建与测试：把该检查指向本仓库的任意工作区，并给一个小到无法覆盖全部套件的整次调用预算。第一次调用应当停下并列出未运行的套件——如果它启动了一个跑不完的套件，该套件应被记为临时结果而不是普通超时。用续跑标志再跑同一条命令，应当完全跳过安装与构建，只运行剩下的部分，用真实结果替换临时结果，并且合并后的报告里每条命令只保留一条记录。对一个留下了未编译包的运行执行续跑，应当被拒绝并给出解释，而不是让套件跑在从未编译过的产物上。
对于锚点：合成一个唯一封顶原因是「无法审查的维度」的裁决，确认「已审查提交」现在会随发布的标记一起写出；再合成一个覆盖证据本身不足的场景——例如一个没有发起任何工具调用的 agent——确认锚点仍然像以前一样被扣留。
对于账本：恢复一个由其他账号发布的标记，确认发现清单到达而「已审查提交」没有，并且渲染出的上下文说明了这些主张属于谁。
npx vitest run packages/cli/src/commands/review/在 macOS 上 2550 条通过 2549 条。唯一的失败是既有且无关的：一条打包技能引用检查在本机上在这些改动之前同样失败，而在持续集成的 Linux 上通过。证据（前后对比）
续跑构建与测试，在本仓库上用真实 npm 命令实测：
失败文件归因，同一个报告十一个失败文件的核心套件：
辅助命令能否打到本构建，同一段提示词、两种启动方式：
测试环境
环境（可选）
macOS 15（darwin 25.6.0），Node 24.18.1，npm 11.16.0。四次审查通过非交互审查入口以 headless 方式对 qwen3.8-max 运行，每次使用独立的主目录；上面的构建与测试证据则是直接在本仓库上用真实 npm 命令跑出来的。
风险与范围
关联 Issue
无。这里的每一处缺陷都来自实跑管线，而不是来自已提交的报告。