feat(review): run-session ledger and cross-session agent evidence - #9091
feat(review): run-session ledger and cross-session agent evidence#9091wenshao wants to merge 18 commits into
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: real and observable — the review workflow's retry loop documents it itself ( Direction: aligned. This is the review harness's own checkpoint story — prompt records are the obligations, transcripts the evidence, plan mtime the run epoch — and the PR closes exactly the gap that blocks resume: readers can't see an earlier attempt's transcripts because the transcript dir is keyed on the session id. No CHANGELOG reference (internal tooling), but the area is clearly relevant. Size: no core paths touched — everything sits under Approach: the shape is right — the ledger is written by Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实且可观测——评审 workflow 的重试循环自己的注释就写着"A retry re-runs the whole review from scratch rather than resuming the failed run",会重新抓取、重新发射那些工作已经在磁盘上的 agent。不是理论性问题。 方向:对齐。这正是评审 harness 自己的检查点叙事——prompt 记录是义务面、transcript 是证据面、plan mtime 是 run epoch——本 PR 恰好补上阻碍续跑的那个缺口:读取方看不到前一次尝试的 transcripts,因为 transcript 目录以 session id 为键。CHANGELOG 无对应条目(内部工具),但该领域明显相关。 规模:未触及核心路径——全部改动在 方案:形态正确——台账由 风险:无升级风险信号——未触及任何与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: a resumed run needs a persistent run→sessions mapping that the orchestrator cannot tamper with, fenced so a fresh run of the same PR starts clean, with transcript readers unioning the session dirs asymmetrically — a missing current dir is an infrastructure fact (throw), a missing prior dir is just missing evidence (skip). The PR lands exactly this shape: The cost-ledger reordering is correct: the empty-current-chat throw now precedes the Non-blocking, both raised in the gate comment: Nothing blocking found. Static review only — per this gate's rules no PR code is executed here; test evidence below is the PR's own CI. sequenceDiagram
participant P1 as fetch-pr command
participant P2 as run-sessions.json ledger
participant P3 as prior session transcripts (S0)
participant P4 as current session transcripts (S1)
participant P5 as certifying readers
P1->>P1: capture diff bytes, hash diffSha256
P1->>P2: append session id (charset-gated, epoch-fenced)
P5->>P2: ask for prior session ids
P2-->>P5: S0, validated, current excluded
P5->>P4: read current transcripts (missing dir throws)
P5->>P3: read prior transcripts (missing dir skipped)
P3-->>P5: records marked fromPriorSession
P5->>P5: certify only via content-shaped pairing
Test evidence — the PR's own CI (unattended run, PR code never executed here)At the time of this pass the This PR has no user-visible surface — the ledger is dormant until PR 2 wires in Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 中文说明代码审查读 diff 之前我的独立思路:续跑需要一个"运行 → sessions"的持久映射,且编排器无法篡改;要有栅栏保证同一 PR 的全新运行从零开始;transcript 读取方对各 session 目录做不对称并集——当前目录缺失是基础设施事实(抛错),先前目录缺失只是证据缺失(跳过)。PR 落地的正是这个形态: 成本台账的重排是正确的:当前 chat 为空即抛的检查现在先于 非阻塞项(与门评一致): 未发现阻塞项。此处仅静态审查——按本门规则不执行任何 PR 代码;下方测试证据来自 PR 自己的 CI。 (时序图见上文英文部分,流程相同:fetch-pr 写入台账 → 读取方验证后取先前 session id → 当前目录必读、先前目录缺则跳过 → 仅凭内容型配对采信。) 测试证据本次为无人值守 CI 运行,审查时不执行 PR 代码。截至本条评论, 本 PR 无用户可见面——台账在 PR 2 接入 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, tightly-tested groundwork whose only reservations are non-blocking: a small future-facing surface waiting on PR 2, and a duplicated epoch helper. Stepping back: my independent proposal for "let a resumed review see its interrupted attempt's evidence" was exactly what this PR does — a harness-written, epoch-fenced, charset-gated ledger of session ids, with readers unioning the session dirs asymmetrically (missing current dir throws, missing prior dir skips). The alternatives are worse: copying transcripts duplicates evidence and smudges provenance, and passing prior session ids through the orchestrator makes the model the supplier of its own credit path. So the approach earns its shape rather than over-engineering it. The property that makes a groundwork PR safe is the degeneration property — with no ledger, every changed reader is exactly the reader it was before — and this one tests it from the adversarial side rather than just the happy path: fabricated entries, traversal-shaped ids on both write and read, corrupt files, stale epochs, orphaned transcripts with no ledger entry. That is the coverage I would have asked for, and it's why the two non-blocking nits (the resume-marker/diffSha256 surface with no caller until PR 2, the verbatim copy of the deadline fence helper) don't hold this back: both are small, both are tested, and the series is announced and stacked. The one honest caveat: the ledger stays dormant until the follow-ups land, so if PR 2 stalls this becomes the dead code the gate worries about. The stack should keep moving. CI on the reviewed commit is still in flight (the ubuntu unit suite was running at review time), so approval is deferred until CI lands green on 中文说明置信度:4/5 —— 干净、测试扎实的地基型 PR,唯一的保留是非阻塞项:一小块等待 PR 2 的前瞻接口面,以及一个重复的 epoch 辅助函数。 退一步看:我对"让续跑评审看到中断尝试的证据"的独立方案,正是本 PR 所做的——harness 写入、以 epoch 为栅栏、封闭字符集把关的 session id 台账,读取方对各 session 目录做不对称并集(当前目录缺失则抛错,先前目录缺失则跳过)。替代方案都更差:复制 transcript 会让证据重复并模糊出处;让编排器传递先前 session id 则让模型成了自己采信路径的供给方。所以这个方案对得起它的形态,而非过度设计。 让地基型 PR 安全的性质是退化性质——没有台账时,每个被改动的读取方与改动前完全一致——而本 PR 是从对抗面而非仅从 happy path 去测它:伪造条目、写入与读取两侧的穿越形 id、损坏文件、过期 epoch、无台账条目的孤儿 transcript。这正是我会要求的覆盖度,也因此两个非阻塞项(PR 2 之前没有调用方的 resume marker/diffSha256 面、逐字复制的 deadline 栅栏辅助函数)不构成阻碍:都很小、都有测试、系列已声明且堆叠。 一个诚实的提醒:台账在后续 PR 落地前是休眠的,如果 PR 2 停滞,这就会变成门所担心的死代码。系列应继续推进。 被审 commit 上的 CI 仍在进行(审查时 ubuntu 单元测试运行中),因此批准推迟到 CI 在该 commit 上转绿——届时 finalize 步骤会发布钉住该 commit 的批准。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 3 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): This PR lays the groundwork for resuming an interrupted ...: none — the full scope was examined within the soft ceiling.; This PR lays the groundwork for resuming an interrupted ...: none (all reads within ceiling).; This PR lays the groundwork for resuming an interrupted ...: None. I stayed within the ~49-call soft ceiling, using 28 tool calls across reads, greps, and source file inspections.`.
中文说明
已审查。 建议见行内评论。 3 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未探索到全部深度(达到工具调用预算):This PR lays the groundwork for resuming an interrupted ...:none — the full scope was examined within the soft ceiling.;This PR lays the groundwork for resuming an interrupted ...:none (all reads within ceiling).;This PR lays the groundwork for resuming an interrupted ...:None. I stayed within the ~49-call soft ceiling, using 28 tool calls across reads, greps, and source file inspections.`。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
| * missing ENVIRONMENT (no session id, no project dir) still throws: that is | ||
| * an infrastructure fact whichever session it is. | ||
| */ | ||
| export function readRunTranscripts( |
There was a problem hiding this comment.
One compose-review invocation reads the run's transcripts two to three times (layerAuditGate, coverageFromTranscripts, verificationGaps each call readRunTranscripts), and each call re-reads run-sessions.json, re-stats the plan, resolves transcriptPaths three times, and fully re-parses every prior-session transcript — read once per command and pass the records, or memoize on planPath + plan mtime.
composeReviewBody calls layerAuditGate (compose-review.ts:721), coverageFromTranscripts (:767) and verificationGaps (:928) in one process; each readRunTranscripts call re-resolves transcriptPaths(env) three times (transcripts.ts:452, via readTranscripts:375, via transcriptDirsForRun:413) and re-runs priorSessionIds -> readSessions -> runEpochMs (a second statSync of a plan whose mtime the caller already holds). On a resumed run with dozens of large prior transcripts the whole prior-attempt evidence tree is read and JSON-parsed repeatedly; bounded per review, but a single up-front read pays it once.
There was a problem hiding this comment.
(Late reply — this thread was opened under my own account by the automated review and my triage filter skipped self-authored comments; a GraphQL sweep surfaced the batch.) Declining the memoization, deliberately, and saying why: a cache keyed on planPath+mtime would be stale in exactly the read-after-write shape this pipeline uses (records appear DURING a run while the plan mtime is deliberately frozen), so it would trade a real correctness property for a constant-factor read. The honest fix is threading one read through compose-review's three call sites, which is a separate change with its own review surface. What this round did take from the observation: the per-directory pipeline is now shared (recordsIn), so the duplicated work is at least one implementation.
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— gpt-5.6-sol via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks I opened were completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks I opened were completed within budget., and 7 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks I opened were completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks I opened were completed within budget.,另有 7 条。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks I opened were completed within budget (~10 of ~46 calls).; "You are review agent reverse-audit — Reverse audit agent…": none — the walk finished within budget.; "Context: this PR lays groundwork for resuming interrupted…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget., and 4 more.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"You are review agent reverse-audit — Reverse audit agent…":none — all checks I opened were completed within budget (~10 of ~46 calls).;"You are review agent reverse-audit — Reverse audit agent…":none — the walk finished within budget.;"Context: this PR lays groundwork for resuming interrupted…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.,另有 4 条。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.11)
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): chunk 6: none — all checks completed within the budget.; This PR (review-resume/1-session-ledger) lays the groundw...: none — all deletions were examined from the available diff..
Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
已审查。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未探索到全部深度(达到工具调用预算):chunk 6:none — all checks completed within the budget.;This PR (review-resume/1-session-ledger) lays the groundw...:none — all deletions were examined from the available diff.。
未审查:反向审计——没有审计 agent 是用本 skill 构建的 prompt 启动的——负责搜寻评审其余部分遗漏问题的这道工序,即便运行过,也缺失了 brief 承载的方法。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI (merge-queue-only jobs); the Windows run is where the new chmod tests will fail (see the inline Critical).
Not explored to full depth (tool budget reached): chunk 6: running run-ledger.test.ts under vitest — the review worktree has no node_modules (parent checkout has none either); a full install + build was not attempte…; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed (~9 of ~45 calls).; "You are review agent reverse-audit — Reverse audit agent…": none — all checks I started were completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget., and 2 more.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI (merge-queue-only jobs); the Windows run is where the new chmod tests will fail (see the inline Critical)。
未探索到全部深度(达到工具调用预算):chunk 6:running run-ledger.test.ts under vitest — the review worktree has no node_modules (parent checkout has none either); a full install + build was not attempte…;"You are review agent reverse-audit — Reverse audit agent…":none — all planned checks completed (~9 of ~45 calls).;"You are review agent reverse-audit — Reverse audit agent…":none — all checks I started were completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all checks above completed within budget.;"You are review agent reverse-audit — Reverse audit agent…":none — all planned checks completed within budget.,另有 2 条。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.11)
Groundwork for resuming an interrupted review run. fetch-pr now records its session id in a run ledger beside the prompt records and stamps the plan with a sha256 of the captured diff bytes; a new readRunTranscripts reads the harness transcripts of every session the ledger names (the current session's contract is unchanged), so coverage, retirement, the layer-audit gate and the cost ledger can credit an earlier attempt's certified work. Coverage counts such agents as recoveredAgents and discloses the continuity; the cost ledger folds the earlier sessions' main loop and agents into the run's totals and reports priorSessions. A run that never resumes sees no behavior change: with no ledger entries every reader reduces to its previous single-session read. Fabricated ledger entries grant nothing — they only name directories under the harness's own subagents tree, and credit still requires the existing content-shaped pairing (verbatim-delivered prompt, opened brief, diff reads).
Ledger.main drops its stale | null (computeLedger throws before folding when the current chat holds no above-floor record, so the renderLedger guard was dead code); recordResume/recordRestart gain dedup guards so a caller-side retry cannot double-count toward the resume cap or fake a second restart; fetch-pr's ledger append is now wired-tested (called with the plan path, after the plan write); and the layer-audit gate's real reader gets its own prior-session tests, including the discriminating partial-walk shape that separates invisible from credited.
…pping Two Criticals from the automatic review. repo-context's enrichment rewrite advanced the plan's mtime — the run epoch every fence keys on — which orphaned the session-ledger entry fetch-pr had appended an orchestrator turn earlier: on a resume the prior attempt's transcripts were invisible and the feature silently re-ran everything in the primary medium/high flow. The rewrite now restores the plan's mtime (enrichment is not a re-capture), pinned by a test that backdates the plan and asserts the mtime survives the command. The continuity disclosure also moved off the capping disclose() channel: compose-review folds every disclosure into the unreviewed-dimension cap and the "Not reviewed:" rendering, so any resumed run that recovered work was permanently downgraded to COMMENT and its reused work called not-reviewed — unrepairably, since the prior records never leave the ledger. Coverage now only counts recoveredAgents, and compose-review renders its own continuity block beside the other disclosed-but-not-capping notes (deferred lint, test-plan rulings), on every verdict including Approve. A new compose test pins the clean resumed run at APPROVE with the note and without the partial-review opener.
Round-2 blockers on the ledger PR. The currentDirOptional escape is now narrow on both axes — only ENOENT (the error's cause travels with it) and only when the run ledger actually names prior sessions — and it is passed at every reader that runs before the resumed session launches anything: coverageFromTranscripts, verificationGaps, the retirement scheduler and the layer-audit gate, which without it failed OPEN on the layers a prior attempt never walked. Prior-session directories now come from one guarded accessor that skips a symlinked subagents/<id>, so a planted link cannot feed foreign transcripts to the transcript union or to the cost ledger, which reads file content with no certification step. The ledger also clamps each prior chat to the moment the next attempt began (an interrupted CLI session that kept serving unrelated turns was billed as review cost), discloses an unreadable prior agent dir instead of silently flooring it, and sums each session's own span for wall time rather than the envelope across the dead gap. Coverage's Uncoverable declaration takes the supersession guard its sibling flags already had: a stale declaration deleted live coverage post-loop and order-independently, so no relaunch could ever clear the cap; a record that declared a chunk unreachable is also no longer counted as recovered. The ledger read path deduplicates and applies the session-id charset gate to the resume marker too.
It is a strict subset of the live credit bars, not "the same bar": no drift rescues, because the count reports reuse and caps nothing, so it should under-claim rather than vouch for a delivery the pairing could not fully confirm.
…xactly Linux CI caught what macOS hid. Restoring the plan's mtime through `utimesSync` costs a unit in the last place on ext4 — 1786717283911.999 goes back as 1786717283911.998 — because `mtimeMs` is a double over a nanosecond clock and `utimesSync` takes seconds as a double. Restoring from float seconds rather than a `Date` narrowed the drift from whole milliseconds to a fraction of a microsecond, but the ledger compared EXACTLY, so the run's own plan still read as a different one and every session entry was still dropped. The resume ledger emptied itself on the filesystem the CI runs on. The fence now allows a millisecond. That is orders of magnitude above the representation noise and orders of magnitude below the thing it must still separate: a fresh capture of the same PR rewrites the plan seconds or minutes later, never inside the same millisecond. The two epoch tests compared exactly for the same reason and are now on the same tolerance — and the sub-millisecond one no longer infers the property from a timestamp at all: it writes the ledger entry `fetch-pr` would have written, runs the enrichment, and asserts the entry is still visible to the continuation. That is the consequence the timestamp was standing in for, and it cannot pass on a filesystem whose round trip loses the fraction.
| // below (`for (const id of uncoverable) covered.delete(id)` is | ||
| // post-loop and order-independent), so no compliant relaunch can ever | ||
| // clear it and the verdict caps on lines this run demonstrably read. | ||
| if (!superseded(rec, chunk)) uncoverable.add(chunk); |
There was a problem hiding this comment.
Mutual supersession erases a genuine Uncoverable: declaration (and this fires on non-resumed runs too, so it is not the "zero behavior change" the description claims).
superseded() here only asks whether another record for the same chunk was launched verbatim and read the diff — it does not ask whether that other record itself declared the chunk uncoverable. Concrete scenario: chunk 5 really is unreachable (a huge generated hunk), agent A is launched verbatim, reads the diff, and returns Uncoverable: chunk 5; the skill's relaunch guidance produces agent B for chunk 5, also verbatim, also diffToolCalls > 0, which returns the same Uncoverable: chunk 5.
superseded(A, 5)→ B qualifies → true → not added.superseded(B, 5)→ A qualifies → true → not added.
uncoverable ends up empty. Both records then continue, so neither contributes coverage, and chunk 5 falls into missingChunks — the honest disclosure is replaced by a "no agent covered this chunk, launch one" remediation. Worse, if any whole-diff agent's merged ranges span chunk 5, covered.add(5) runs and the body reports as reviewed a chunk two agents said they could not reach — exactly the self-contradiction the for (const id of uncoverable) covered.delete(id) comment below says this file exists to prevent.
The sibling gapsSuperseded already solves this exact class by requiring the superseding record to be gap-free itself; the same narrowing is needed here (a superseding record must not itself declare the chunk uncoverable).
| // DIFFERENT plan and drops every session entry, silently emptying the | ||
| // resume ledger on a filesystem that keeps finer time than a `Date` can | ||
| // hold. Passing `mtimeMs / 1000` preserves the fraction. | ||
| utimesSync(planPath, planStat.atimeMs / 1000, planStat.mtimeMs / 1000); |
There was a problem hiding this comment.
Unguarded utimesSync turns a best-effort restore into a hard command failure.
The next four lines make the intent explicit: a restore that does not land is a warning, not a failure. But a utimesSync that throws never reaches that warning — it propagates straight out of runRepoContext, and by then both outPath and the enriched planPath have already been written successfully. utimes can fail with EPERM when the process does not own the file (a plan written by another uid, e.g. a container/runner uid switch), on some network/CI mounts, and on Windows when the file is briefly held open by an indexer/AV — none of which are reasons to abort Step 1b after the enrichment already succeeded.
Wrap it and fall through to the existing warning:
| utimesSync(planPath, planStat.atimeMs / 1000, planStat.mtimeMs / 1000); | |
| try { | |
| utimesSync(planPath, planStat.atimeMs / 1000, planStat.mtimeMs / 1000); | |
| } catch { | |
| // A restore we could not perform is the same fact as one that did not | |
| // land: warned below, never fatal — the enrichment already succeeded. | |
| } |
| // and opened the brief it points at. Territory alone let a compliant | ||
| // sibling satisfy the floor while a hand-written auditor supplied the | ||
| // receipt — the launch is exactly what the built record proves. | ||
| const built = readRecordedPrompts(planPath); |
There was a problem hiding this comment.
The new delivered() requirement makes the gate hard-cap all six layers whenever the prompt-record dir is gone but the transcripts survive.
readRecordedPrompts returns an empty Map on any readdir failure (catch { return out; }), so built.size === 0 makes delivered() return false for every auditor, corroborated is [], and control falls to if (identityMatched > 0) return { unreviewed: owedLayerDimensions([]) } — every layer dimension owed, capping the verdict.
Those two trees are independent: the prompt records live in promptRecordDir(planPath) under the review tmp dir (which cleanup.ts sweeps, and which the workflow sweeps at start-of-run), while the transcripts live under <projectDir>/subagents/. So a re-compose after Step 9 cleanup, or a compose against a plan whose tmp record dir was swept, now produces a fully-capped modeled-system review where before the transcript + territory reads alone corroborated. That is the same class of "could not MEASURE" the catch below deliberately fails open on, but it slips past it because nothing throws.
Suggest treating an empty built as "cannot measure delivery" and keeping the old corroboration bar in that case, rather than owing all six layers.
Minor, same block: readRecordedPrompts(planPath) is called without since, while every other site that pairs records against transcripts (e.g. scheduleReverseAuditRound) passes the plan mtime; the no-arg form is documented as the obligations reader.
| // A review that starts inside an EXISTING session must not bill that | ||
| // session's earlier turns; its ledger entry says when it became an attempt. | ||
| const own = currentSessionEntry(planPath, env); | ||
| const floorMs = own === null ? planMs : Math.max(planMs, own.atMs); |
There was a problem hiding this comment.
A ledger entry's atMs can silently disable the whole cost ledger.
readSessions only bounds atMs above by runCeilingMs() = Date.now() + 2000, and currentSessionEntry deliberately applies no authorization gate (unlike priorSessionEntries, which goes through resumeAuthorized). So an entry for the current session stamped up to ~2s in the future — a hand-written one in promptRecordDir, which run-ledger.ts's own comment concedes is "a directory the orchestrator can reach" — pushes floorMs past every timestamp in the chat file. readUsage then drops all of them, mainEvents.length === 0 fires the throw below, and runCostLedger catches it and prints cost-ledger unavailable — …: the review's accounting vanishes with no indication that it was suppressed rather than broken.
The old code used only the plan's mtime, which the orchestrator cannot move. Consider clamping the raised floor, e.g. Math.min(own.atMs, Date.now()) or refusing an entry stamped after planMs + RUN_EPOCH_SLACK_MS for this purpose — the legitimate case (fetch-pr appends milliseconds after writing the plan; fetch-pr --resume appends at resume time) is unaffected either way.
| endsAtMs: number | null; | ||
| }> = []; | ||
| for (const { sessionId, endsAtMs } of priorSessionEntries(planPath, env)) { | ||
| const dir = join(projectDir, 'subagents', sessionId); |
There was a problem hiding this comment.
Path built from the raw ledger id, but the harness writes the directory under a sanitized name.
The harness's own accessor is getSubagentSessionDir(projectDir, sessionId) = join(projectDir, 'subagents', sanitizeFilenameComponent(sessionId)), and sanitizeFilenameComponent is value.replace(/[^a-zA-Z0-9_-]/g, '_') — it maps . to _. SESSION_ID_RE in run-ledger.ts explicitly admits . ("room for prefixed variants"), so a session id like resume.1 is written by the harness to subagents/resume_1 while this lookup goes to subagents/resume.1. lstatSync throws, the continue fires, and every prior directory is dropped — the entire resume-evidence path silently reads nothing and the cost ledger silently omits that session.
Fail-safe in direction, but it makes the feature a no-op for any id outside [A-Za-z0-9_-]. Building the path with the exported sanitizeFilenameComponent (or narrowing SESSION_ID_RE to the sanitizer's own charset) removes the divergence. Note transcriptPaths above has the same gap for the current session — worth fixing in one place.
| const entries = readSessions(planPath); | ||
| if (entries.some((e) => e.sessionId === id)) return; | ||
| const mtime = planMtimeMs(planPath); | ||
| entries.push({ |
There was a problem hiding this comment.
When planMtimeMs() returns null this writes an entry that can never be read back.
readSessions hard-requires the field — typeof (e as SessionEntry).planMtimeMs === 'number' && planMtime !== null && … — with no fallback (the doc block above planMtimeMs says so explicitly, and that "entries without the field never exist in the wild"). But this spread produces exactly such an entry whenever statSync(planPath) failed. The write is guaranteed dead: on the next read the entry is dropped, and worse, the next appendRunSession rewrites the file from the filtered list, so the id is silently lost rather than retried.
If the plan cannot be stat'ed the append has nothing useful to record, so an early if (mtime === null) return; before the push is both honest and equivalent in effect — and lets the ...(mtime === null ? {} : …) spread collapse to a plain planMtimeMs: mtime.
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen-code via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI (merge-queue-only jobs).
Not explored to full depth (tool budget reached): chunk 9: executed run of run-ledger.test.ts via vitest — node_modules is absent in the worktree and the parent checkout, and a monorepo npm install exceeds the chu…; "agent reverse-audit (round 1)": none — all checks I started were completed (~13 tool calls)..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI (merge-queue-only jobs)。
未探索到全部深度(达到工具调用预算):chunk 9:executed run of run-ledger.test.ts via vitest — node_modules is absent in the worktree and the parent checkout, and a monorepo npm install exceeds the chu…;"agent reverse-audit (round 1)":none — all checks I started were completed (~13 tool calls).。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.12)
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 4)": none — the chunk was read in full, no check was cut short.; chunk 8: executed test run of run-ledger.test.ts (blocked by broken worktree node_modules, not by findings).
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 4)":none — the chunk was read in full, no check was cut short.;chunk 8:executed test run of run-ledger.test.ts (blocked by broken worktree node_modules, not by findings)。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.11)
…gate The resume cap reads two counters so that deleting one cannot reset it. The second counter never worked: it came from `priorSessionIds`, which is gated on the calling session already appearing in the resume marker, and that entry is written only after a ruling passes — so at ruling time the ledger term was structurally zero, and deleting `resume.json` reset the cap the ledger was supposed to backstop. The gate protects EVIDENCE — it stops a session that was never granted a resume from reading another attempt's transcripts. A count is not evidence: it says how many times this review has been picked up and nothing about what any attempt did. So the count gets its own ungated accessor, running through the same `readSessions` fences as everything else, and the cap can be wired to it.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R2-2 descriptor-discipline family (comments 3782037614, 3782037617, 3782037623, 3782037612, 3782037608): the full no-follow descriptor handling asked for is deliberately staged to stacked PR #9163 per the author's replies; this diff hardens only the two ledger files (readLedgerFile), so the fuller mechanism cannot be verified from this commit
- [Critical] R1-5 verification-digest matching (comment 3777311839): author declined as pre-existing (verify keys are per-findings-digest and the record dir was never fenced); whether a stale digest's verifier can satisfy the floor is not fully traceable from this diff alone
Not reviewed: reverse audit — stopped after round 2 without two consecutive dry rounds: round 2 surfaced one new Suggestion (R6-11 — verified, then demoted to low-confidence and kept terminal-only); both rounds' remaining returns re-derived claims already carried by the PR's open threads.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — but note two adjacent checks outside my chunk I deliberately left to their owners: the symlink/FIFO/ noFollow write-path tests and the lstatSync → read….
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未决,请确认:共 2 条(原文未翻译,列表见上方英文部分)。
未审查:reverse audit — stopped after round 2 without two consecutive dry rounds: round 2 surfaced one new Suggestion (R6-11 — verified, then demoted to low-confidence and kept terminal-only); both rounds' remaining returns re-derived claims already carried by the PR's open threads。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)":none — but note two adjacent checks outside my chunk I deliberately left to their owners: the symlink/FIFO/ noFollow write-path tests and the lstatSync → read…。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…e owed pins R1-5, deferred two rounds ago and now closed: `verificationGaps` took the best delivery across every `verify--<digest>` key ever recorded, so a verifier that succeeded against an EARLIER findings list satisfied the floor for a list it never opened. The behaviour predates the resume work, but widening the record set to prior sessions is what made it reachable in practice, so it lands here rather than as the follow-up it was parked as. The keys are narrowed to the newest digest before ranking, dated by the digest's own findings file — shard keys of one digest land within a moment of each other, a previous list's are a round older — and keys with no findings file stay in, because they cannot be dated and also cannot reach `ok`, so they only ever make the verdict stricter. Two invariants acknowledged in earlier threads now have their probes. The empty-current-chat refusal deliberately precedes the prior-session fold — prior events must not vouch for a broken current recorder — and every refusal test predated the ledger, so the faithful mutation (prior sessions excuse the emptiness) shipped green; it now reddens a probe with a healthy prior chat and an empty current one. The rendered "totals include N earlier session(s)" line is asserted on the rendered text, where it can be deleted or crash at print time with every return-value test still green. And the fixture drift named in review is repaired at both call sites: the `runLedger` helper with a dead first parameter, and the layer-audit fixture hardcoding the derived `plan-prompts` name instead of asking `promptRecordDir`.
Nine Criticals, missed for two rounds because the triage sweep read the first GraphQL page of review threads and this PR crossed one hundred — the finding list below is what a paginated read surfaced. A RETURN is now a fact, not an inference from non-empty text. `finalText` keeps the last non-empty assistant message, which includes progress narrated between tool calls — an agent that said "reading the diff now…" and died carried plausible text that certified coverage and the Step 4/5 floor. `parseTranscript` marks text with tool traffic after it as progress, and every certification consumer (liveRecords, certifies, all four supersession predicates) requires the record to have returned. That last group also closes the probe-proven fail-open where an unreturned verbatim relaunch suppressed an honest `Uncoverable:` declaration and earned the chunk off the told-range presumption, let two honest declarations annihilate into `missingChunks`, and silenced a prior attempt's `Budget gap:` disclosure as a "genuine repair". The layer-audit gate no longer treats NAMING the brief as reading it — a grep whose args contain the path cleared `delivered()` while the auditor never opened its instructions — and its record read now takes the run-epoch fence `readRecordedPrompts` documents as mandatory for history readers: without it, a dead attempt's records beside the stable plan path let a hand-launched stale prompt corroborate a run whose builder never emitted an auditor, a fail-open on the gate's own withhold-only invariant. Retirement receipts classify only from auditors that READ the cumulative findings list their prompt points at: the comparison against known findings is the audit's method, and two skipping receipts retired a chunk on a comparison nobody made. Transcript ownership is now checked for the current session, not only prior ones — a foreign-stamped file planted under the current directory was trusted as current evidence, `since` blind to it (a copy gets a fresh mtime) and the prompt pairing deterministic. And the session-directory lookup applies the harness's own filename sanitizer: the harness writes `subagents/<sanitized>` while the ledger's charset admits dots, so a dotted id read a path that does not exist and every reader silently saw nothing. `appendRunSession` refuses to write an entry when the plan cannot be stat'ed — `readSessions` hard-requires the field, so the entry was a guaranteed-dead write that silently lost the id on the next append's rewrite. `repo-context` captures the plan's identity before the providers run and refuses to write if it moved — a concurrent capture otherwise had this run's stale contents restored under the other run's epoch, whose ledger then passed an exact fence against a plan it never described. Every fix is pinned, and each pin was mutation-verified against the exact regression the finding names.
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: the entire diff — no agent reported covering it; nobody read it.
Not reviewed: reverse audit — the original run completed both adversarial rounds only for chunks 7, 12, and 13; the full 15-chunk convergence pair was not run, and the HEAD-drift restart re-reviewed only the 12-file increment rather than repeating the full reverse-audit fan-out.
Not reviewed: build-and-test — the full packages/cli suite timed out after 300 seconds in the original run; on the final HEAD, six targeted review test files passed (263 tests), but the full CLI suite and skipped Integration Tests (CLI, No Sandbox), macOS Node 22, and Windows Node 22 CI dimensions were not rerun locally.
Not reviewed: the executable-script lint — the report is stale or its diff could not be verified; re-run qwen review script-lint.
Not reviewed: every dimension — none of the 21 required agents is on record as launched with a prompt this skill built, so this diff was reviewed, if at all, from prompts the run wrote for itself: no record shows the severity bar, the finding format or this project's own rules reaching an agent.
Not reviewed: verification and reverse audit — neither the verifier nor the reverse auditor was launched with a prompt this skill builds — the posted findings were ruled on, and the misses the rest of the review left were hunted, if at all, without the briefs this skill certifies against.
中文说明
未审查:整个 diff——没有 agent 报告覆盖过这部分,也没有人读过它。
未审查:reverse audit — the original run completed both adversarial rounds only for chunks 7, 12, and 13; the full 15-chunk convergence pair was not run, and the HEAD-drift restart re-reviewed only the 12-file increment rather than repeating the full reverse-audit fan-out。
未审查:build-and-test — the full packages/cli suite timed out after 300 seconds in the original run; on the final HEAD, six targeted review test files passed (263 tests), but the full CLI suite and skipped Integration Tests (CLI, No Sandbox), macOS Node 22, and Windows Node 22 CI dimensions were not rerun locally。
未审查:the executable-script lint — the report is stale or its diff could not be verified; re-run qwen review script-lint。
未审查:所有维度——21 个必需 agent 中没有任何一个有记录表明是用本 skill 构建的 prompt 启动的,这个 diff 即便被审查过,也是基于这次 run 自行编写的 prompt:没有记录表明严重级别标准、发现格式或本项目自己的规则到达过任何 agent。
未审查:验证与反向审计——验证 agent 与反向审计 agent 都没有用本 skill 构建的 prompt 启动——发布的发现即便被裁定过、评审其余部分遗漏的问题即便被搜寻过,也都缺失了本 skill 用以认证的 brief。
— gpt-5.6-sol via Qwen Code /review (v0.21.11)
R5-1: all three bounded reads sliced the untrusted array BEFORE validating it, so sixty-four malformed entries at the front consumed the whole cap and hid every real entry behind them — `sessionEntryCount` read zero and the resume cap reset, which is precisely the attack the count exists to survive; the marker's resumes and restarts had the same shape, resetting the once-per-review restart bound the same way. Filter first, cap the survivors. The validation cost the original order was avoiding is bounded by MAX_LEDGER_BYTES — the file cannot hold enough entries for the cheap field checks to matter — while the cap's actual job, bounding the per-entry directory reads consumers pay, is done by capping what is RETURNED, and that stands either way. Pinned from both sides: seventy junk entries ahead of one valid entry count as one, and seventy valid entries still cap at sixty-four.
The behavioural fixes. Session identity now folds on the PATH the id becomes — sanitized and lowercased — everywhere at once: dedup, the current-session exclusion, the marker's dedup, and both write-side duplicate checks. Folding on the raw id left every alias the filesystem or the harness sanitizer collapses (case variants, trailing dots, sanitized '.') open as a second session wearing the first one's evidence. Read-time dedup keeps the EARLIEST duplicate rather than the first in file order, which handed an out-of-order hand-written duplicate the session's identity and erased the window between the real start and itself from billing. The resume marker takes the same exact plan fence as the session ledger — the window alone is inexact by its own slack, and a previous run's resumes surviving a rewrite arrive with the cap already spent — and restarts dedupe for the same reason resumes always did. Marker writes stamp the plan mtime and refuse when the plan cannot be stat'ed, like the ledger's own dead-write refusal. Appends refuse to rewrite over a ledger that EXISTS as a regular file but could not be read: rewriting from the empty fallback on a transient fault erased every previously recorded entry, and the guard is keyed on the file's type so the pinned self-healing over planted symlinks stands. The layer-audit gate's record fence is the STRICT plan mtime, not the slacked epoch — record mtimes and the plan's come off the same clock, and the slack would re-admit a dead attempt's records written in the two seconds before a re-capture. The epoch JSDoc now says which artifacts key on which fence instead of claiming one definition covers all. The cost refusal message names the boundary that actually filtered (this attempt's start, on a resumed run). repo-context's three stale comments now match the shipped tolerance fence, and the run-ledger module header claims containment only for the certifying readers — cost is accounting, and the honest claim stops there. Twenty-odd guarantees that could be deleted with the suite green are now pinned, each mutation-verified: the byte and entry caps, the per-session authorization property, currentSessionEntry at all, the charset gate at read WITH a valid fence, both write-side guards observed through the raw file, the marker's schemaVersion/case-dedup/noFollow/swallow properties, the symmetric plan fence, the earliest-duplicate rule, the prior-side ownership and window clamps, the byte-vs-string diff hash (an invalid-UTF-8 buffer, which no string fixture can express), the whole-diff recovery branch (exact count), the key-shaped recovery count, the Step 4/5 refusal by name, the `contributed > 0` guard, two-prior-session folding, the handoff boundary operators, the gate's territory clause and identity filter on fixtures that pass every OTHER clause, and repo-context's write-skip through the real serializer.
This comment has been minimized.
This comment has been minimized.
|
@qwen-code /review |
A bot-directed review_requested run joined the shared PR group on the requested reviewer's identity, but whether it reviews anything is decided later by authorize on the requester's write permission. A requester without write produces a guaranteed all-skipped run that can still supersede a lifecycle run sitting PENDING behind a still-terminating review — the same lost-review race as #9091, through the bot-request door. Gate the shared group on the action alone so no review_requested run can supersede a pending lifecycle run; an authorized bot request still reviews immediately, at the cost of an occasional duplicate review of the same head.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R2-2 descriptor-discipline family (5 threads: 3782037614, 3782037617, 3782037623, 3782037612, 3782037608) — the full single-descriptor containment fix is staged to stacked PR #9163 (still OPEN at this review; author states merge order #9091 → #9092 → #9153 → #9163, 'nothing ships without it'); the fuller mechanism cannot be verified from this commit
Not reviewed: reverse audit — stopped at the 5-round cap without two consecutive dry rounds; every round reported new findings.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; unit suites ran locally on Linux only.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; unit suites ran locally on Linux only.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — the currentDigestKeys function that consumes DIGEST_WINDOW_MS (coverage.ts:1615–1636) sits in the next chunk; I read it only far enough to confirm th….
Test Plan (not a blocker): lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未决,请确认:共 1 条(原文未翻译,列表见上方英文部分)。
未审查:reverse audit — stopped at the 5-round cap without two consecutive dry rounds; every round reported new findings。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; unit suites ran locally on Linux only。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; unit suites ran locally on Linux only。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)":none — the currentDigestKeys function that consumes DIGEST_WINDOW_MS (coverage.ts:1615–1636) sits in the next chunk; I read it only far enough to confirm th…。
Test Plan(非阻断):lib/run-ledger.test.ts — no such file or directory; lib/transcripts.test.ts — no such file or directory; lib/retirement.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const old = new Date(Date.now() - 600_000); | ||
| utimesSync(findingsFilePath(p, 'verify--old11111111'), old, old); |
There was a problem hiding this comment.
[Suggestion] R7-20: The digest-dating guards are under-pinned in two directions: (1) the only two-digest fixture makes the current digest non-compliant — the ACCEPTANCE direction (a compliant current-digest verifier clears the floor while an older digest's records sit beside it) is pinned nowhere; (2) nothing pins that the window is anchored to the NEWEST list's mtime rather than to wall clock.
Failure scenario: Mutant A — keep only the newest-dated key, or refuse multi-generation key sets — ships green and, in the ordinary multi-round flow, demotes actually-verified findings (unverifiedFindings flips true). Mutant B — d.mtimeMs >= Date.now() - DIGEST_WINDOW_MS — ships green and breaks the zero-launch continuation this PR exists for: a resumed attempt reading its own list >5s after it was written filters out every datable verify key → bestDelivery([]) → spurious re-verification owed.
Witness:
Mutant A: companion probe FAILED while the shipped OLDER test stayed green. Mutant B (from-now window): 579/579 green across all 8 review suites; the companion fails under the mutant and passes on clean code.
Suggested fix: Add an acceptance companion: two compliant verify digests, backdate the older one's findings file (the same -600_000 utimesSync), assert ok === true — pins both the acceptance direction and the newest-mtime anchor against both mutants.
中文说明
[Suggestion] R7-20:digest 定年栅栏在两个方向上都欠钉扎:(1) 唯一的双 digest fixture 让当前 digest 不合规——接纳方向(合规的当前 digest 验证者通过门槛,而旧 digest 的记录并存)没有任何测试覆盖;(2) 没有任何测试钉住窗口是以最新列表的 mtime 为锚,而非墙上时钟。
故障场景: 变异 A——只保留最新的可定期键,或拒绝多代键集合——绿色通过,并在普通多轮流程中把实际已验证的发现降级(unverifiedFindings 翻转为 true)。变异 B——d.mtimeMs >= Date.now() - DIGEST_WINDOW_MS——绿色通过,且破坏本 PR 为之而生的零发射续跑:续跑尝试在列表写入 5 秒后读取自己的列表,会把每个可定期的 verify 键过滤掉 → bestDelivery([]) → 欠下一笔莫须有的重新验证。
证据: 变异 A:伴随探针失败,而已交付的 OLDER 测试保持绿色。变异 B(from-now 窗口):全部 8 个 review 套件 579/579 绿;伴随测试在该变异下失败、在干净代码上通过。
建议修复: 新增接纳方向的伴随测试:两个合规的 verify digest,把较旧者的 findings 文件回溯(同样的 -600_000 utimesSync),断言 ok === true——同时钉住接纳方向与“最新 mtime 为锚”,可杀死两种变异。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const ids = [ | ||
| step45(p, 'verify', { returned: false }), | ||
| step45(p, 'reverse-audit', { returned: false }), | ||
| ]; |
There was a problem hiding this comment.
[Suggestion] R7-28: Both prior-session verificationGaps fixtures are symmetric (all agents returned, or all died), so the PER-RECORD granularity of the liveRecords refusal is pinned only by construction; a session-granular refactor (drop a prior session's evidence wholesale when ANY of its agents died) ships green.
Failure scenario: An interrupted attempt whose reverse-audit returned cleanly but whose verifier died mid-flight: per-record behavior owes only verification (gaps subject ['verification']); a session-granular mutant drops the returned reverse-audit record too, both steps read not-launched, and verificationGaps emits the combined 'verification and reverse audit' gap — the resumed run re-spends a completed whole-diff reverse audit on the strength of a sibling's death, and the gap text names the wrong role.
Witness:
Session-granular mutant of liveRecords:
Tests 579 passed (579); mixed probe under the mutant gives subjects ['verification and reverse audit'] where per-record behavior returns ['verification']; probe passes on clean code.
Suggested fix: Add a mixed fixture: step45(p, 'verify', { returned: false }) + step45(p, 'reverse-audit') (returned), both moved to S0 with a ledger; assert gaps.map(g => g.subject) toEqual(['verification']) and unverifiedFindings true.
中文说明
[Suggestion] R7-28:两个先前会话 verificationGaps fixture 都是对称的(要么全部返回、要么全部死亡),因此 liveRecords 拒绝的逐记录粒度只在构造上成立;会话粒度的重构(任一会话有任何 agent 死亡就整体丢弃该会话的证据)绿色通过。
故障场景: 被中断的尝试中 reverse-audit 干净返回、验证者却中途死亡:逐记录行为只欠验证(gaps subject 为 ['verification']);会话粒度的变异体会把已返回的 reverse-audit 记录也丢弃,两步都读作 not-launched,verificationGaps 输出合并的 'verification and reverse audit' gap——续跑运行仅因兄弟的死亡就重新花费一次已完成的 whole-diff 反向审计,且 gap 文本点名了错误的角色。
证据: liveRecords 的会话粒度变异体:Tests 579 passed (579);混合探针在该变异体下 subjects 为 ['verification and reverse audit'],而逐记录行为返回 ['verification'];探针在干净代码上通过。
建议修复: 新增混合 fixture:step45(p, 'verify', { returned: false }) + step45(p, 'reverse-audit')(已返回),两者连同 ledger 移入 S0;断言 gaps.map(g => g.subject) toEqual(['verification']) 且 unverifiedFindings 为 true。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // Prior attempts first, the current one stamped slightly later: each | ||
| // attempt's window closes when the next one opened, and a fixture that | ||
| // stamped them together would fence out transcripts written "now". | ||
| const now = Date.now(); | ||
| sessionIds.forEach((id, i) => { | ||
| appendRunSession( |
There was a problem hiding this comment.
[Suggestion] R7-27-3: Fixture-stall flake family (this file): planWithLedger captures now and closes the prior session's window at now+1500, but the prior-session transcript files are written at wall clock AFTERWARD — a >1.5s stall fences the prior records out via recordsIn's until clamp. Same pattern confirmed in layer-audit-gate.test.ts and retirement.test.ts.
Failure scenario: Under CI load, 'unions prior-session transcripts, marked fromPriorSession' and 'absorbs a missing CURRENT dir when asked' fail intermittently with the prior record missing; during the same stall, 'refuses a foreign-stamped transcript in a PRIOR directory' is dropped by the WINDOW instead of the ownership guard, so it passes without discriminating the guard it exists to pin.
Suggested fix: Write the prior-session transcript files BEFORE capturing now (or utimesSync them back into the window), as the sibling fixtures that avoid this already do.
中文说明
[Suggestion] R7-27-3:Fixture 停顿 flake 家族(本文件):planWithLedger 捕获 now 并把先前会话窗口关闭在 now+1500,但先前会话的 transcript 文件是之后才以墙上时钟写入——超过 1.5 秒的停顿会使先前记录被 recordsIn 的 until 钳制挡掉。同样的模式已在 layer-audit-gate.test.ts 与 retirement.test.ts 确认。
故障场景: 在 CI 负载下,“unions prior-session transcripts, marked fromPriorSession”与“absorbs a missing CURRENT dir when asked”会因先前记录缺失而间歇性失败;同一次停顿中,“refuses a foreign-stamped transcript in a PRIOR directory”被窗口而非所有权关卡挡掉,于是它通过了,却没有区分它本要钉住的关卡。
建议修复: 在捕获 now 之前写入先前会话的 transcript 文件(或用 utimesSync 把它们回拨进窗口),仿照已经避开此问题的同类 fixture。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const dir = join( | ||
| projectDir, | ||
| 'subagents', | ||
| sanitizeFilenameComponent(sessionId), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R7-25: The prior-session sanitized-dir lookup (sanitizeFilenameComponent in priorSessionDirs) is pinned by no test: every fixture uses dot-free ids where the sanitizer is identity — deleting the call ships green. The ledger charset explicitly admits dots, and the current-session side of the identical lookup IS tested ('S.dot').
Failure scenario: A dotted prior session id under a raw-join regression hits lstatSync ENOENT one underscore away from the real records; the catch-continue skips that attempt silently — recovered evidence is lost (work re-owed) and the cost ledger omits the attempt's agent cost without even its non-ENOENT WARNING (which fires only on readdir faults of an EXISTING dir).
Witness:
Mutation run (sanitizer removed from priorSessionDirs): full review suite green,
Tests 2645 passed | 4 skipped. Dotted-prior-id probe (ledger id 'S0.1', harness dir 'S0_1') goes red under the mutation:expected [ 'a1' ] to deeply equal [ 'a0', 'a1' ].
Suggested fix: Add a prior-session analogue of the 'S.dot' test: ledger a dotted prior id, create subagents/ with its transcripts, and assert priorSessionDirs/readRunTranscripts find it.
中文说明
[Suggestion] R7-25:先前会话的带消毒目录查找(priorSessionDirs 中的 sanitizeFilenameComponent)没有任何测试钉住:所有 fixture 都使用无点 id(此时消毒是恒等映射)——删除该调用绿色通过。账台字符集明确允许点号,而同一查找的当前会话侧有测试('S.dot')。
故障场景: 在 raw-join 回归下,带点的先前会话 id 会在距离真实记录一个下划线处命中 lstatSync ENOENT;catch-continue 静默跳过该尝试——恢复的证据丢失(工作被欠下重做),成本台账遗漏该尝试的 agent 成本,甚至连非 ENOENT WARNING 都没有(它只对存在目录的 readdir 故障触发)。
证据: 变异运行(从 priorSessionDirs 移除消毒):整个 review 套件绿,Tests 2645 passed | 4 skipped。带点先前 id 探针(账台 id 'S0.1',harness 目录 'S0_1')在该变异下变红:expected [ 'a1' ] to deeply equal [ 'a0', 'a1' ]。
建议修复: 新增 'S.dot' 测试的先前会话版本:把一个带点的先前 id 记入账台,创建 subagents/<消毒后> 及其 transcripts,断言 priorSessionDirs/readRunTranscripts 能找到它。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // The rewrite itself still happened: the plan carries no context here, | ||
| // but the write path ran (content is re-serialized). | ||
| expect(() => JSON.parse(readFileSync(planPath, 'utf8'))).not.toThrow(); |
There was a problem hiding this comment.
[Suggestion] R7-14: The mtime-preservation test's comment claims 'The rewrite itself still happened … the write path ran', but the test never asserts the rewrite actually ran — a regression that skips the write when content changed passes both mtime tests.
Failure scenario: Measured mitigation: the feared regression (enrichment silently never persisted) does NOT ship green — five pre-existing persistence tests assert readJson(planPath)).toHaveProperty('repositoryContext', …) and fail under both never-write and inverted-condition mutations (5 failed | 32 passed). The residual is a comment-vs-assertion gap, not an unprotected suite.
Witness:
Surgical never-write mutation (
if (false)):5 failed | 32 passed— all five failures pre-existing persistence tests; both new mtime tests stay green, proving they do not pin the rewrite.
Suggested fix: Assert the re-serialized content in test 1, e.g. expect(readFileSync(planPath, 'utf8')).toBe(stringifyPlanReport({ files: [{ path: 'src/a.ts' }] })) — one line removes the gap.
中文说明
[Suggestion] R7-14:mtime 保持测试的注释声称“重写确实发生了……写入路径执行了”,但测试从未断言重写真的执行过——一个在内容变化时跳过写入的回归可以通过两个 mtime 测试。
故障场景: 实测的缓解:所担心的回归(enrichment 静默地从未持久化)并不会绿色通过——五个既有持久化测试断言 readJson(planPath)).toHaveProperty('repositoryContext', …),在“从不写入”与“条件取反”两种变异下均失败(5 failed | 32 passed)。残留的是注释与断言之间的落差,而非套件失去保护。
证据: 精准的“从不写入”变异(if (false)):5 failed | 32 passed——五个失败全部是既有持久化测试;两个新 mtime 测试保持绿色,证明它们并未钉住重写。
建议修复: 在 test 1 中断言重新序列化后的内容,例如 expect(readFileSync(planPath, 'utf8')).toBe(stringifyPlanReport({ files: [{ path: 'src/a.ts' }] }))——一行即可消除落差。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if ( | ||
| Math.abs(planStat.mtimeMs - planStatBefore.mtimeMs) > 1 || | ||
| planStat.ino !== planStatBefore.ino | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R7-13: The compare-and-refuse check's inode disjunct is exercised by no test: the abort test's racer does an in-place writeFileSync (same inode) plus a 60s mtime move, firing only the mtime disjunct.
Failure scenario: Mutation — delete planStat.ino !== planStatBefore.ino — ships the whole repo-context suite green (37/37). The shape the disjunct exists for — rename-replacement with restored mtime, which this diff's own enrichment write produces on every content change (atomic rename = new inode, float utimesSync restore within ~µs) — reopens with no red signal.
Witness:
Mutation run (inode disjunct deleted):
Test Files 1 passed (1) / Tests 37 passed (37); baseline on unmodified code also 37/37.
Suggested fix: Add a racer test that swaps the plan with mtime held: write a sibling file, utimesSync it to the captured original mtime, renameSync it over the plan (inode changes, mtime unchanged), and expect the 'changed while repository context was being computed' throw.
中文说明
[Suggestion] R7-13:compare-and-refuse 检查的 inode 分支没有被任何测试演练:abort 测试的竞争者做的是就地 writeFileSync(同 inode)+ 60 秒 mtime 移动,只触发 mtime 分支。
故障场景: 变异——删除 planStat.ino !== planStatBefore.ino——repo-context 全套件绿色通过(37/37)。该分支存在的目标形态——mtime 已恢复的 rename 替换(本 diff 自己的 enrichment 写入在每次内容变更时都会产生:原子 rename = 新 inode,float utimesSync 在 ~µs 内恢复)——将无任何红色信号地重新打开。
证据: 变异运行(删除 inode 分支):Test Files 1 passed (1) / Tests 37 passed (37);未改动代码的基线同样 37/37。
建议修复: 新增一个保持 mtime 的替换竞争测试:写一个兄弟文件,utimesSync 到捕获的原始 mtime,renameSync 覆盖 plan(inode 变、mtime 不变),断言抛出 'changed while repository context was being computed'。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (Math.abs(statSync(planPath).mtimeMs - planStat.mtimeMs) > 1) { | ||
| writeStderrLine( | ||
| `WARNING: could not restore the plan's timestamp at ${planPath}; ` + |
There was a problem hiding this comment.
[Suggestion] R7-26: The post-utimesSync restore-verification and its WARNING — the only disclosure for a moved run epoch — is pinned by no positive test; the suite's only reference is a not.toContain assertion, which passes harder when the block is deleted.
Failure scenario: Mutation — delete or invert the if-block — ships repo-context.test.ts green (37/37). If a future restore regression drifts the epoch past the 1ms fence, the run's pre-enrichment evidence (fetch-pr's session entry, prompt records, transcripts) is fenced out and the one diagnostic designed for exactly that outcome never prints — recreating the silent-orphaning failure this PR was written to fix, silently.
Witness:
Mutation run (verification block deleted):
Tests 37 passed (37); restored → green again. The warning string appears only at the emission site and the single negative assertion.
Suggested fix: Add a positive test that forces a restore drift (e.g. stub utimesSync to a timestamp >1ms off) and asserts the WARNING is emitted; keep the existing negative assertion as the other half of the boundary.
中文说明
[Suggestion] R7-26:utimesSync 之后的恢复校验及其 WARNING——运行 epoch 移动时的唯一披露——没有任何正向测试钉住;套件中唯一的引用是一个 not.toContain 断言,删除该块后它反而更容易通过。
故障场景: 变异——删除或取反该 if 块——repo-context.test.ts 绿色通过(37/37)。若未来恢复逻辑回归、使 epoch 漂移超过 1ms 栅栏,该运行 enrichment 之前的证据(fetch-pr 的会话条目、prompt 记录、transcripts)会被栅栏挡在外面,而为这一结果专门设计的唯一诊断永远不会输出——静默地重现本 PR 要修复的静默孤儿化故障。
证据: 变异运行(删除校验块):Tests 37 passed (37);恢复后再次全绿。警告字符串只出现在发射点与唯一的负向断言处。
建议修复: 新增正向测试,强制恢复漂移(如把 utimesSync 打桩为偏差 >1ms 的时间戳)并断言 WARNING 被输出;保留既有负向断言作为边界的另一半。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…igest keys Three probe-proven blockers, all in code earlier rounds added. The bounded read capped in FILE order before sorting and deduplicating, so 64 valid hand-written duplicates at the front evicted every genuine entry — and the next append rewrote the file from the filtered survivors, laundering the plant permanently; with 64 distinct entries the just-appended current entry (always file-last) was the one dropped, nulling the cost floor. The pipeline is now sort → dedup → cap, each step defeating the payload the next one cannot. `priorSessionEntries` classified every non-current entry as prior, so a twice-resumed run read as the MIDDLE attempt received its own successor as a "prior session" with an unbounded window — its later unrelated activity folded into this attempt's bill, its records entered the evidence pool with no ceiling. Prior now means the PREFIX strictly before this session's own entry, and the last prior's window closes at this session's start. `currentDigestKeys` kept undatable verify keys on the premise that they cannot reach ok — false for the write-failure fallback, whose inlined list leaves no findings file and no pointer, making the findings-read floor vacuously true. A stale pointerless verifier could vouch for a newer dated list no verifier opened. Undatable keys are now dropped once any dated key exists; with no dated key at all they are the only evidence and stay. `sessionEntryCount` gains an exclude-current option for the cap's ledger term (consumed by the stack's fetch-pr): counting the session's own entry in either term refuses a same-session retry of the last permitted resume, whose fresh fall-through then destroys the very state being resumed.
…group (QwenLM#9210) * fix(ci): keep no-op review requests out of the PR review concurrency group Co-authored-by: Qwen-Coder <[email protected]> * test(ci): pin precheck-pr bot login to the review constants * test(ci): share one bot-login extraction across review-workflow suites * fix(ci): route every review request to a per-run concurrency group A bot-directed review_requested run joined the shared PR group on the requested reviewer's identity, but whether it reviews anything is decided later by authorize on the requester's write permission. A requester without write produces a guaranteed all-skipped run that can still supersede a lifecycle run sitting PENDING behind a still-terminating review — the same lost-review race as QwenLM#9091, through the bot-request door. Gate the shared group on the action alone so no review_requested run can supersede a pending lifecycle run; an authorized bot request still reviews immediately, at the cost of an occasional duplicate review of the same head. --------- Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: qwen-code-dev-bot <[email protected]>
What this PR does
Lays the groundwork for resuming an interrupted
/reviewrun, with zero behavior change for runs that never resume.fetch-prnow records its CLI session id in a small run-sessions ledger beside the prompt records, and stamps the plan with a SHA-256 of the captured diff bytes (diffSha256— the content identity of what the run reviews). A newreadRunTranscriptsreads the harness subagent transcripts of every session the ledger names, so the readers that certify agent work —check-coverage,verificationGaps, the reverse-audit retirement scheduler, the layer-audit gate and the cost ledger — can credit work an interrupted earlier attempt demonstrably finished. Coverage counts such agents in a newrecoveredAgentsfield and pushes a bilingual continuity disclosure through the existing disclosures pipeline; the cost ledger folds the earlier sessions' main loop and agents into the run's totals and reportspriorSessions, so a resumed review never under-reports what it cost.Why it's needed
The CI review workflow's retry loop re-runs a dead review from scratch (its own comment says so), spending minutes of the shared time budget re-fetching, re-chunking and re-launching agents whose work is already on disk. The persistent checkpoint largely exists already — prompt records are the obligations, harness transcripts are the evidence, and the plan's mtime is the run epoch every fence keys on. What is missing is only that the readers cannot see an earlier attempt's transcripts, because a resumed run continues under a new
QWEN_CODE_SESSION_IDand the transcript directory is keyed on it. This PR closes exactly that gap, as the first of a three-PR series (the--resumeflow itself and its wiring follow).Security shape: the ledger entry is only ever an address, never a verdict. Session ids pass a closed character-set gate on write and on read (no traversal), the directory is assembled from the env's project dir, and a fabricated entry can at most point a reader at a directory inside the harness's own
subagents/tree — where credit still requires the existing content-shaped pairing (verbatim-delivered CLI prompt, opened brief, diff reads) that fabrication cannot satisfy. An unreadable or malformed ledger reads as empty: invisible evidence re-runs work, never the reverse.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review— the new suites arelib/run-ledger.test.ts(19 tests: epoch fencing, traversal-shaped id refusal on write and read, corrupt-file-reads-as-empty, the resume-marker bookkeeping) and the new describe blocks inlib/transcripts.test.ts(union reads, prior-dir-missing is silent, current-dir-missing still throws),check-coverage.test.ts(prior-session credit passes the 3D gate with a continuity disclosure; no ledger → prior transcripts are invisible — the orphan-invisibility guard; a compliant relaunch supersedes the prior attempt's failure),lib/retirement.test.ts(dry receipts earned by the interrupted attempt retire a chunk on the continuation),cost-ledger.test.ts(prior main loop + agents fold into totals;priorSessionscounting) andfetch-pr.test.ts(diffSha256is the hash of the captured bytes, null when no diff was captured).Key property to confirm in review: with no ledger entries every changed reader reduces exactly to its previous single-session read (the union is
[current]), so a normal review is untouched.Evidence (Before & After)
N/A — no user-visible change; all consumers of the new fields land in the follow-up PRs.
Tested on
Environment (optional)
Unit tests via vitest; also exercised end-to-end against the real bundle in the series' verification (see PR 2 of the stack).
Risk & Scope
diffSha256are additive; the one shared-behavior change is that transcript readers now accept records from ledger-named prior sessions — gated on a file onlyfetch-prwrites, with credit still requiring the existing two-author pairing.--resumeflow itself (PR 2), the/review/review run/CI wiring (PR 3), local (non-PR) reviews.diffSha256is a new nullable field, old plans simply lack it.Linked Issues
First of a three-PR series implementing review resume; the follow-ups are stacked on this branch.
中文说明
本 PR 做了什么
为续跑被中断的
/review打地基,对从不续跑的运行零行为变化。fetch-pr现在把自己的 CLI session id 记入 prompt 记录目录旁的 run-sessions 台账,并在 plan 里写入所捕获 diff 原始字节的 SHA-256(diffSha256——本次评审对象的内容身份)。新增的readRunTranscripts会读取台账所列每个 session 的 harness 子代理 transcript,因此负责认证 agent 工作的读取方——check-coverage、verificationGaps、反向审计退休调度器、layer-audit 门与成本台账——能够采信被中断的前一次尝试确实完成的工作。coverage 用新的recoveredAgents字段计数此类 agent,并通过现有 disclosures 管道推送双语的续跑披露;成本台账把先前 session 的主循环与 agent 开销并入本次运行的总计并报告priorSessions,续跑的评审绝不少报成本。为什么需要
CI 评审 workflow 的重试循环会把挂掉的评审整个从头重跑(它自己的注释就这么写),在共享时间预算里花几分钟重新抓取、重新切块、重新发射那些工作已经在磁盘上的 agent。持久化检查点其实早已存在——prompt 记录是义务面,harness transcript 是证据面,plan 的 mtime 是所有栅栏共用的 run epoch。缺的只是读取方看不到前一次尝试的 transcripts,因为续跑运行在新的
QWEN_CODE_SESSION_ID下,而 transcript 目录以它为键。本 PR 恰好补上这个缺口,是三个 PR 系列的第一个(--resume流程本身及其接线在后续 PR)。安全性形态:台账条目永远只是地址,绝不是判词。session id 在写入与读取两侧都要过封闭字符集门(无法路径穿越),目录由环境变量的 project dir 拼出,伪造的条目至多把读取方指向 harness 自己
subagents/树内的某个目录——在那里获得采信仍然需要现有的内容型配对(逐字送达的 CLI prompt、打开过 brief、读过 diff),伪造无法满足。不可读或畸形的台账按空处理:看不见的证据意味着重做工作,绝不反向。审阅者验证方案
如何验证
cd packages/cli && npx vitest run src/commands/review——新增套件为lib/run-ledger.test.ts(19 个用例:epoch 栅栏、写入与读取双侧拒绝穿越形 id、损坏文件按空读、resume marker 簿记)以及lib/transcripts.test.ts(并集读、prior 目录缺失静默、当前目录缺失仍抛)、check-coverage.test.ts(prior session 的采信通过 3D 门并带续跑披露;没有台账则 prior transcripts 不可见——孤儿不可见守卫;合规重发射会取代前一次尝试的失败记录)、lib/retirement.test.ts(中断尝试挣得的 dry 收据在续跑轮退休对应 chunk)、cost-ledger.test.ts(prior 主循环与 agent 并入总计;priorSessions计数)、fetch-pr.test.ts(diffSha256是捕获字节的哈希,未捕获 diff 时为 null)的新增 describe 块。审阅时要确认的关键性质:没有台账条目时,每个被改动的读取方都精确退化为原先的单 session 读取(并集即
[当前]),正常评审不受影响。证据(前后对比)
N/A——无用户可见变化;新字段的消费方在后续 PR 落地。
已测试平台
环境(可选)
vitest 单元测试;系列的端到端验证中还针对真实 bundle 做了完整演练(见堆叠中的 PR 2)。
风险与范围
diffSha256均为增量;唯一的共享行为变化是 transcript 读取方现在接受台账所列先前 session 的记录——由仅fetch-pr写入的文件把关,且采信仍需现有的双作者配对。--resume流程本身(PR 2)、/review/review run/CI 接线(PR 3)、本地(非 PR)评审。diffSha256是新的可空字段,旧 plan 只是没有它。关联 Issue
实现评审续跑的三 PR 系列之首;后续 PR 堆叠在本分支上。