feat(review): run-session ledger and cross-session agent evidence - #9091
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
This was referenced Aug 13, 2026
Collaborator
|
Re-run on the rebased head — the branch moved a long way since the Template looks good ✓ Problem: real and observable, unchanged from the prior pass — the review workflow's retry loop documents it itself ("a retry re-runs the whole review from scratch"), re-fetching and re-launching agents whose work is already on disk. This PR is the groundwork that makes resume possible. Direction: aligned, unchanged. 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), area clearly relevant. Size: 6278 changed lines across 22 files — 1972 production / 4306 test (~69% test; the growth since the last triage is rounds 7–9 of audit hardening and their pins, not scope creep). Core is touched by exactly one line: a re-export of the already-existing Approach: the shape is still right, and the two non-blocking nits from the prior gate have landed: Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明在被 rebase 后的新 head 上重跑——自 模板完整 ✓ 问题：真实且可观测，与上次结论一致——评审 workflow 的重试循环自己的注释就写着重试会从头重跑整个评审，重新抓取、重新发射那些工作已经在磁盘上的 agent。本 PR 正是让续跑成为可能的地基。 方向：对齐，结论不变。这是评审 harness 自己的检查点叙事——prompt 记录是义务面、transcript 是证据面、plan mtime 是 run epoch——本 PR 恰好补上阻碍续跑的缺口：读取方看不到前一次尝试的 transcripts，因为 transcript 目录以 session id 为键。CHANGELOG 无对应条目（内部工具），但该领域明显相关。 规模：22 个文件共 6278 行改动——1972 行生产代码 / 4306 行测试（约 69% 是测试；自上次门评以来的增长是第 7–9 轮审计加固及其钉测，不是范围蔓延）。核心路径只被触及一行：从 core agents index 重新导出已存在的 方案：形态依然正确，上次门评的两个非阻塞项已落地： 风险：无升级风险信号——未触及任何与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewMy independent proposal for "let a resumed review see its interrupted attempt's evidence" (written before reading the diff, same baseline as the August pass): a harness-written run→sessions ledger beside the prompt records — never model-supplied — epoch-fenced on the plan mtime so a fresh run starts clean, with entries that are only addresses (charset-gated ids, corrupt/unreadable reads as empty), and transcript readers that union the session dirs asymmetrically: a missing current dir is an infrastructure fact (throw), a missing prior dir is only missing evidence (skip). Credit must still require the existing content-shaped pairing, so a fabricated entry can point a reader at a directory but cannot mint coverage. The alternatives are worse: copying transcripts smudges provenance; threading session ids through the orchestrator makes the model the supplier of its own credit path. This pass read the entire production diff at the rebased head (all 12 production files; 22 files total). The implementation matches that shape and exceeds it on the adversarial axis — the growth since the last triage is nine review rounds of hardening, and I verified the claimed round-9 blocker fixes are actually in this head rather than taking the reply threads' word:
The property that makes a groundwork PR safe — no ledger ⇒ every reader is exactly the reader it was before — is pinned from the adversarial side: orphan invisibility ("sees nothing from a prior session the ledger never recorded"), zero Non-blocking, and tracked rather than lost: the descriptor-containment family (round 8's class finding) is deliberately carried by stacked #9163, whose branch base makes it a mandatory successor of this stack; the certification-bar atom divergence (one of the two threads still open on the PR) is structurally closed by #9257; the rest of the open threads are Suggestion-severity deferrals recorded per the review-round policy. Two small nits still standing at this head, both cosmetic: 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
participant P2 as run-sessions ledger
participant P3 as resume marker
participant P4 as certifying readers
participant P5 as prior session dirs
participant P6 as current session dir
P1->>P2: append own session id (charset-gated, epoch-fenced)
P4->>P3: is this session an authorized resume
P3-->>P4: marker names this session
P4->>P2: ask for prior session ids
P2-->>P4: validated ids, current excluded
P4->>P6: read current transcripts (missing dir throws)
P4->>P5: read prior transcripts (missing or symlinked dir skipped)
P5-->>P4: records marked fromPriorSession, windowed per attempt
P4->>P4: certify only via content-shaped pairing
Files changed (22 of 22 shown)
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 the stack's Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查读 diff 之前的独立方案（与八月那轮相同的基线）：harness 写入、以 plan mtime 为 epoch 栅栏的 run→sessions 台账（绝不接受模型提供），条目只是地址（封闭字符集把关、损坏/不可读按空处理），transcript 读取方不对称地并集各 session 目录——当前目录缺失是基础设施事实（抛错），先前目录缺失只是证据缺失（跳过）；采信仍须走现有的内容型配对，伪造条目指得了目录、换不来 coverage。替代方案都更差：复制 transcript 模糊出处；让编排器传 session id 等于让模型供给自己的采信路径。 本轮在被 rebase 的 head 上通读了全部生产 diff（12 个生产文件、共 22 个文件）。实现与上述形态一致，并在对抗面上更严格——自上次门评以来的增长是九轮审查的加固；我逐一核实了 round-9 阻断项的修复确实在当前 head 上，而不是只信回复线程的说辞：单读占用者分类决定整个写入（瞬态故障无法再击穿覆盖保护）、plant 分类与自愈、先验证再排序去重最后截断、plan mtime 精确栅栏（1ms 容差吸收 utimesSync 末位损耗）、resume 授权门（台账是地址簿、marker 是授权、计数不是证据）、并集读取的精确保留（只吸收 ENOENT 且仅当存在先前证据）、symlink 先前目录跳过、会话戳防伪（嫁接/复制两侧都拒）、returned 生命周期判定（sidecar 状态 + 文本后无工具流量）、各认证读取方丢弃未返回的先前记录且替代者必须已返回、验证底线收窄到当前 findings digest（含 findings 写入失败时回退到 prompt 记录定年）、layer 门 delivered() 的单射性与 brief 实读要求、retirement 的 findings-read 底线只门 dry 分支、成本台账按尝试窗口折叠先前 session、repo-context 以"先戳时间再改名 + 改名前复查身份"保全 run epoch。 地基型 PR 的安全性质——没有台账时每个读取方与改动前完全一致——从对抗面被钉住：孤儿不可见、未续跑运行 recoveredAgents 为零、写入与读取两侧拒绝穿越形 id、损坏按空读、前次运行的条目被栅栏挡掉。唯一超出"接受先前 session"的共享行为变化——所有权戳现在也过滤当前目录的记录——是安全方向的加固，同样有钉测。 非阻塞、且有去向：描述符收容族（round 8 的类级发现）由堆叠的 #9163 承接（其分支基座决定它必须随后落地）；认证标准原子分歧（PR 上仍开放的两个线程之一）由 #9257 结构性关闭；其余开放线程均为按评审轮次政策记录在案的 Suggestion 级延期项。当前 head 上两个小瑕疵，均为表面问题：certifies() 里有一个空分支的无效条件；recordedPromptPath 插在 briefPath 的文档注释与 briefPath 之间，使该注释悬空。 此处仅静态审查——按本门规则不执行任何 PR 代码；下方测试证据来自 PR 自己的 CI。 （时序图见英文部分：fetch-pr 写入台账 → 读取方先验 resume 授权 → 取先前 session id → 当前目录必读、先前目录缺则跳过 → 仅凭内容型配对采信。） 测试证据无人值守 CI 运行，审查时不执行 PR 代码。截至本条评论，被审 commit 上的 本 PR 无用户可见面——台账在 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — adversarially hardened groundwork whose audit-found holes each have a visible, tested patch at this head; what remains is tracked follow-ups, not doubt about this diff. Stepping back: my independent proposal for "let a resumed review see its interrupted attempt's evidence" is exactly what this implements — a harness-written, epoch-fenced, charset-gated address book with asymmetric union reads — and the alternatives stay worse for the same reasons as the August pass (copying transcripts smudges provenance; orchestrator-supplied ids make the model the supplier of its own credit path). What this pass adds over that one: the branch went through rounds 7–9 of the review loop in between, and I verified the claimed fixes against the current head instead of trusting the reply threads — single-read writers, lifecycle-aware returns, digest narrowing, the injectivity guard, the findings-read floor, and the compare-and-refuse plan commit are all present and all pinned by the new suites. 273 of 275 review threads are resolved; the two still open are both Suggestion-severity — a read-once memoization, and the certification-bar atom divergence that #9257 defines exactly once. The one reservation worth stating plainly: the descriptor-containment family is NOT in this diff, by the maintainer's explicit call — it is carried by stacked #9163, whose branch base makes it a mandatory successor of this stack, so the window between this merge and that one is a recorded, bounded tradeoff (the ledger's own threat model bounds a forged entry to the harness's tree, and directory symlinks are skipped), not an oversight. The surface is also still dormant — if the 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，审计发现的每个洞在当前 head 上都有可见且已测试的补丁；剩下的是有去向的后续项，而不是对本 diff 的疑虑。 退一步看：我对"让续跑评审看到中断尝试的证据"的独立方案正是本 PR 所实现的——harness 写入、epoch 栅栏、封闭字符集把关的地址簿，加上不对称的并集读取——替代方案仍因同样原因更差（复制 transcript 模糊出处；编排器供给 id 让模型成为自己采信路径的供给方）。本轮比八月那轮多做的事情：期间分支经历了第 7–9 轮审查循环，我逐一对照当前 head 核实了所声称的修复，而不是只信回复线程——单读写入方、生命周期感知的 returned、digest 收窄、单射性保护、findings-read 底线、compare-and-refuse 的 plan 提交，全部在场且都有新套件钉住。275 个评审线程中 273 个已解决；仍开放的两个都是 Suggestion 级——一个是读一次/记忆化建议，另一个是 #9257 会"每个原子只定义一次"地结构性关闭的认证标准分歧。 一个需要直说的保留：描述符收容族不在本 diff 里，这是维护者的明确决定——由堆叠的 #9163 承接，其分支基座决定它必须在本系列之后落地，因此本次合并与那次合并之间的窗口是一个有记录、有边界的权衡（台账自身的威胁模型把伪造条目限制在 harness 自己的树内，且目录 symlink 会被跳过），而不是疏忽。接口面也仍是休眠的——如果 被审 commit 上的 CI 仍在进行（审查时 ubuntu 单元测试运行中），因此批准推迟到 CI 在该 commit 上转绿——届时 finalize 步骤会发布钉住该 commit 的批准；若有检查转红或 head 移动，则不会批准。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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
previously approved these changes
Aug 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
reviewed
Aug 13, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
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)
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
wenshao
commented
Aug 13, 2026
| * missing ENVIRONMENT (no session id, no project dir) still throws: that is | ||
| * an infrastructure fact whichever session it is. | ||
| */ | ||
| export function readRunTranscripts( |
Collaborator
Author
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.
Collaborator
Author
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
commented
Aug 13, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
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
requested changes
Aug 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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
requested changes
Aug 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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
reviewed
Aug 13, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
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
requested changes
Aug 14, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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)
wenshao
force-pushed
the
review-resume/1-session-ledger
branch
from
August 14, 2026 04:48
1f9c9df to
6ccd125
Compare
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
wenshao
commented
Aug 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
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。
— qwen3.8-max via Qwen Code /review (v0.21.11)
This was referenced Aug 14, 2026
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.
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.
…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.
Code: the recovery count's uncoverable veto is chunk-scoped like the walk's (a recovered whole-diff auditor legitimately QUOTES the declarations it audited); the layer gate delivers only against reverse-audit records (a concatenated launch verbatim-matched a sibling role's record and satisfied the brief bar with the sibling's brief); retirement extracts the findings pointer ONCE from the raw prompt (the re-extraction from trim-normalized lines defeated the anchors that reject indented quotations) and the orphaned memo-contract JSDoc is back on findingsListFor; the endsAtMs doc and the diffSha256 comment now say what the code does at this commit. Fixtures: the flake family is closed — the three suites' prior-session helpers backdate their files ten seconds, so a CI stall between the ledger stamp and the write can no longer fence the fixtures out through the until clamp. And a dozen probes that could not discriminate the guard they name now can: the over-budget ledger carries a valid entry, the symlinked target carries a fenced entry, the marker's charset gates are pinned on both sides with valid fences, the alias fold's entries sit inside the window, the marker's plan fence survives a 3s nudge instead of dying on the epoch window, resumeAuthorized folds case, the stale gate record sits INSIDE the would-be slack, the identity anchor refuses a substring mention, a delivered auditor with zero diff reads stays refused, the digest narrowing accepts the compliant current digest, per-record granularity is pinned with a mixed prior session, the prior-side floors and the per-session ceiling have events inside their discriminating windows, the plan-swap abort fires on the restored-mtime rename shape, and the epoch test asserts the rewrite's content.
…-aware returns, floor narrowing run-ledger: the clobber guards decided from a SECOND read, so a transient fault clearing between the two reads let an append rewrite the whole ledger from the empty fallback; all three writers now classify the occupant once and decide everything from that one read, with the plan stat shared between the fence and the new entry. Plant shapes that can never be legitimate state — a directory (EISDIR on every rename, swallowed forever), an oversize regular file (frozen as "state to preserve" for the life of the plan) — are healed by the writers instead of freezing recording. The entry cap keeps the NEWEST end, so a backdated flood of distinct ids truncates itself rather than evicting the genuine entries and laundering the eviction on the next append. transcripts: `returned` now consults the harness's own lifecycle record — the `agent-<id>.meta.json` sidecar — so an agent killed after a text flush (which ends identically to a completed one in transcript content) no longer certifies; thought parts are excluded from the return text, so a thinking-mode agent killed between ROUND_TEXT and its tool calls does not hand its internal reasoning downstream as a verdict; and a transcript whose records carry two different session stamps is rejected whole — the grafted-tail shape that defeated the first-stamp ownership check. coverage: `currentDigestKeys` dates a findings-file-less key by its always-present prompt record instead of dropping it, closing the mirror case where the CURRENT digest's inlined-fallback keys were dropped and a previous round's verifier vouched for a list nobody opened; the same narrowing now applies to the Step 5 reverse-audit floor; and the Uncoverable supersession guard excludes records that themselves declare the same chunk, so two honest declarers no longer annihilate each other into a permanently unreviewable `missingChunks` loop. layer-audit-gate / retirement: both readers now require `returned` before consuming receipts — a died-mid-flight auditor's narration can carry receipt forms — and the gate's `delivered()` refuses a transcript that verbatim-matches more than one reverse-audit record (the concatenated-launch shape whose union territory corroborated layers no walk touched). Retirement's findings-read floor moves INTO the dry branch of `classifyReturn`, so a filed yield from an auditor that skipped the list read keeps its chunk hot instead of vanishing before classification. repo-context: the enriched plan is committed by a temp file that is stamped with the anchor's times BEFORE the rename and identity-checked against the anchor immediately before it — there is no longer an instant where the plan path carries an advanced epoch (the kill-window that permanently orphaned same-run evidence), and a concurrent capture landing during the read+compare+write window is refused instead of silently overwritten. Every fix carries a probe pinned by mutation: reverting each guard in isolation reddens at least one new test.
…e inode ext4 reuses a just-freed inode number immediately, so the delete-then-create fixture came back as the same (ino, mtime) identity — indistinguishable by design, and red only on Linux. The imposter is now created while the original exists and renamed over it, so its inode is distinct on every filesystem.
wenshao
dismissed stale reviews from yiliang114, ytahdn, and qwen-code-ci-bot
via
August 16, 2026 12:18
852b9e8
wenshao
force-pushed
the
review-resume/1-session-ledger
branch
from
August 16, 2026 12:18
9a3c330 to
852b9e8
Compare
yiliang114
approved these changes
Aug 16, 2026
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
ytahdn
approved these changes
Aug 16, 2026
wenshao
added a commit
to wenshao/qwen-code
that referenced
this pull request
Aug 16, 2026
…#9091 findings-read bar)
5 tasks
12 tasks
yiliang114
added a commit
that referenced
this pull request
Aug 16, 2026
Round-13 review, two Criticals plus three hardenings: - CRITICAL (#9091-class race): an edited run without a base change is a guaranteed no-op, yet joined the shared qwen-pr-review-pr-{N} concurrency group — a routine title/body edit could supersede a PENDING synchronize run (pending runs are replaced by the newest queued run, cancel-in-progress notwithstanding) and the pushed head would silently never review. Carve edited-without-changes.base out of the PR group into the per-run group, mirroring review_requested; base-retarget edits keep the shared group (superseding a stale pending run there reviews live state). Verbatim group pin updated. - CRITICAL (TOCTOU): Stage 1f read BASE_SHA fresh at outcome-post time while HEAD_SHA follows the capture-once/re-read-before-post discipline — a retarget landing mid-run minted a pin matching the NEW base pair, skipping a diff the triage never certified. Capture BASE_SHA once at review start alongside HEAD_SHA and re-read/compare before the outcome post, bailing on a moved base (exact mirror of the head guard). Triage-suite pins updated to the compare shape. - Reclaim the wasted preamble: the same edited clause now gates precheck-pr, authorize and review-pr at the job level (the in-step early exit stays as the tie-breaker). - Invalidate the stale marker comment when the stale-pin strip succeeds: markers are upserted but never deleted, so a later manually re-applied park label hit the stale-strip again instead of the manual-park carve-out. The strip now rewrites the marker via upsert-bot-comment.sh --update-only with a non-marker body. - Pin the '- edited' trigger subscription itself (the behavioural harness bypasses the on: block; deleting the line kept the suite green). Suites: triage 140/140, pr-review 174 green + 2 pre-existing root-user env failures unrelated to this change. eslint + prettier clean.
Collaborator
|
Released in v0.21.13. |
doudouOUC
pushed a commit
to doudouOUC/qwen-code
that referenced
this pull request
Aug 17, 2026
…enLM#9091) * feat(review): run-session ledger and cross-session agent evidence 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). * fix(review): address review feedback on the session ledger 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. * fix(review): keep the run epoch stable and the continuity note non-capping 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. * fix(review): contain, bound and supersede the prior-session reads 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. * docs(review): say what the recoveredAgents bar actually is 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. * fix(review): bill a resumed run once, clamp its agents, and say what the pairing proves Five blockers from the round-3 review of the ledger PR. The cost ledger pushed TWO overlapping spans for a prior session — its chat window and the agent window nested inside it — so a resumed run's wall time double-counted the nested minutes; it now folds one span per session from the union. Prior-session agent transcripts were read with only the floor, so unrelated subagents launched in the interrupted CLI session after the review died billed to the review; readAgentDir now takes the same ceiling the chat already had. The two new chmod-000 fault tests get the repo's platform/root guard: on Windows chmod only toggles the read-only attribute and readdir still succeeds, which would have turned the test_windows merge-queue job red on this PR's own tests. And the module header's claim that the content-shaped pairing is something "fabrication cannot satisfy" was refuted by probe: an actor who can write into the harness tree can read a recorded prompt back and plant a matching transcript. The prose now states the guarantee that holds — it defeats an orchestrator fabricating its REPORT of the run, not an actor with write access to the tree, and that bar is the same one the current session's records have always cleared. Also from the same round: the ledger read paths refuse anything that is not a regular file (a planted FIFO blocked readFileSync forever while the write side was already noFollow-hardened), the epoch fence gained its upper half (a future-dated entry outlived every later rewrite), endsAtMs is derived in time order rather than file order so the cost clamp cannot invert, transcriptDirsForRun is gone (no production caller), and the tests gain the missing verdict assertion, the two zero-launch cases that actually exercise currentDirOptional at the retirement and layer-audit call sites, a findIndex-miss-proof ordering check, and a compose fixture that is no longer re-contaminated by base()'s eagerly evaluated default. * fix(review): refuse mid-flight credit, key the fence exactly, fold the ids A batch of findings that had gone untriaged (their threads were opened under the author account, so a reviewer-side filter had hidden them). A prior attempt's agent that died mid-flight — verbatim prompt, a logged diff read, no return — passed every coverage guard and marked its chunk covered, so the resumed run skipped the relaunch and the chunk's findings never existed anywhere. A prior-session record with an empty final text now credits nothing, in the coverage walk and in the recovered count alike. The current session keeps today's semantics: an empty return there is an agent still running, which the idle checks own. Session ids become PATH segments, so `s1` and `S1` are one directory on APFS and Windows: a case-variant ledger entry passed the string exclusion and re-read the current session as a prior one — every record twice, `recoveredAgents` minted on a run that never resumed, and the current chat folded into the prior totals. The exclusion and the dedup are now case-insensitive. The fresh-run fence was inexact by its own slack: a previous run that appended within it survived this run's plan write. Each entry now records the plan mtime it saw, and a reader keeps only entries that saw THIS plan — exact by construction, with the window as the fallback for entries written before the field existed. Also: the run-epoch fence had a verbatim private copy in three modules and now lives once in prompt-record.ts, and the prior-directory read no longer re-implements the files-to-records pipeline — one `recordsIn`, so a future record-level filter cannot apply to live evidence while bypassing recovered evidence. * fix(review): gate prior evidence on an authorized resume, and bound what it reads The round's central finding: the ledger is an address book, not permission. Any session pointing at an old plan could union the ledgered attempts' transcripts and inherit their coverage — after head drift, stale evidence could certify code nobody reviewed. Reading prior evidence now requires this session's own entry in the resume marker, which `fetch-pr --resume` writes only after every probe passed. One gate, in the accessor every reader goes through. With it, several narrower holes close too. A transcript is checked against the session that owns its directory, so a record COPIED into another attempt's directory cannot earn that attempt's credit. Each prior attempt's evidence is bounded above by the moment the next attempt began, so a session that kept running after the resume took over is no longer credited to the review. The exact fresh-run fence loses its legacy fallback — the field ships in the same change as the ledger, so there are no older files to be lenient toward, and the fallback was the only way a previous run's entry could survive a plan rewrite. And both ledger files are bounded in bytes and entries before they are parsed: a planted huge or duplicated ledger could otherwise stall every command that touches bookkeeping, or spend the resume cap. The fixtures now build their state with the real writers instead of hand-written JSON, which is why several of them changed shape: a hand-written ledger has no plan stamp, and a re-homed transcript that keeps its old session id is exactly the misplaced shape production now refuses. * fix(review): drop unfinished prior records at the source, and not count superseded ones Two halves of the same principle. A prior attempt's agent that never returned is excluded from the record set EVERY gate reads, not just from the coverage walk — the same record would otherwise still satisfy roster matching and the Step 4/5 delivery floor, and a gate reading a different evidence set than its siblings is how a review certifies work nobody did. And `recoveredAgents` no longer counts a prior record whose obligation a current relaunch already satisfied: the count is what the continuity note reports, so claiming recovery for work this run re-did would misdescribe it. The compose fixture that hid this is fixed too — `base()`'s literal evaluates its `coveredPlan()` default even when the caller overrides `planPath`, which re-created the current-session record the test had just re-homed. * fix(review): bill each attempt from its own start, and require delivery for a receipt The cost floor was the plan's mtime for every stream, so a review that began inside an existing CLI session billed that session's earlier, unrelated turns — and on a resumed run each prior attempt was floored the same way. Each stream is now floored at the moment its session became a review attempt, from its own ledger entry, and a stream that exists but cannot be read is counted and disclosed rather than silently skipped: a lower total must not be presented as a complete one. The layer-audit gate now requires a receipt-contributing auditor to have been launched with the CLI's own recorded prompt and to have opened the brief it points at. Territory alone let a compliant sibling satisfy the floor while a hand-written auditor supplied the claims. repo-context skips its enrichment write entirely when the serialized plan is unchanged — the common resumed case, where the commit-then-restore window simply cannot open — and when it does write, a failure to restore the plan's timestamp is reported instead of silently advancing the run epoch out from under this run's own evidence. Tests: verificationGaps is pinned directly on the zero-launch continuation (evidence only in a prior session, no current transcript dir), the resumed-run ledger line is asserted in both numbers and by its absence, and the compose auditor fixture now delivers the recorded prompt it always claimed to. * fix(review): close the certification, epoch and supersession gaps the audit found Five behavioural defects, each mutation-proven and each now pinned by a probe that reddens when the fix is reverted. The Step 4/5 gate read run-scoped evidence WITHOUT the dead-record filter its coverage sibling applies. Delivery is checked as recorded prompt plus opened brief and never consults the return, so an interrupted attempt's verifier that opened its brief and died satisfied the floor — a verification nobody performed, certified. The filter now lives in one function both gates call: a gate reading a different evidence set than its siblings is how a review certifies work nobody did, and that is exactly what the split allowed. The plan-epoch restore passed `Date` objects to `utimesSync`. A `Date` carries whole milliseconds; APFS and ext4 keep nanoseconds. The restored mtime was therefore CLOSE to the original and not equal to it, the exact `planMtimeMs === planMtime` fence read it as a different plan, and every session entry was dropped — the resume ledger silently emptied on the filesystems this actually runs on. Restored from float seconds now, and the verification compares with a millisecond of tolerance instead of exact float equality, which was firing on every content-changing enrichment (28 leaked WARNINGs in the suite, and not one test noticed). The recovery count's supersession check iterated ALL records, so two prior records for one obligation — a whiff-relaunch inside the interrupted attempt — superseded each other and both vanished from the count while coverage still credited their chunk. Narrowed to current-session records: the count answers "what did this run reuse", so only a relaunch HERE supersedes. Two test-only defects in the same pass. The check-coverage test titled as accepting prior-session Step 4/5 evidence contained none: with no such records both steps fail as not-built and merge into one gap whose subject is the combined `'verification and reverse audit'`, which equals neither name the assertions excluded — it passed on a review where nothing was verified. It now builds real Step 4/5 fixtures and asserts no gaps at all. The current-session billing floor had no discriminating test (reverting it kept all 56 green), and the APPROVE-path continuity separator had none either (dropping the clause glued the note onto the verdict with a single space, all 230 still green). Also: three JSDoc blocks that mid-file insertions had stranded on the wrong symbols, and a paragraph promising a window fallback for entries without `planMtimeMs` that the code stopped honouring when the fallback was removed — two contradictory contracts in one file, with the doc's version being the one that loses a resume its evidence. * fix(review): compare the plan-mtime fence within a millisecond, not exactly 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. * feat(review): expose the ledger's session count without the evidence 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. * fix(review): verify against the CURRENT findings digest, and close the 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`. * fix(review): close the round-4 blockers a paginated sweep surfaced 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. * fix(review): validate ledger entries before the cap consumes them 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. * fix(review): work through the round-3-to-6 suggestion backlog 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. * fix(review): close round 7 on the ledger — cap order, prior prefix, digest 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. * fix(review): work through the round-7 suggestions on the ledger PR Code: the recovery count's uncoverable veto is chunk-scoped like the walk's (a recovered whole-diff auditor legitimately QUOTES the declarations it audited); the layer gate delivers only against reverse-audit records (a concatenated launch verbatim-matched a sibling role's record and satisfied the brief bar with the sibling's brief); retirement extracts the findings pointer ONCE from the raw prompt (the re-extraction from trim-normalized lines defeated the anchors that reject indented quotations) and the orphaned memo-contract JSDoc is back on findingsListFor; the endsAtMs doc and the diffSha256 comment now say what the code does at this commit. Fixtures: the flake family is closed — the three suites' prior-session helpers backdate their files ten seconds, so a CI stall between the ledger stamp and the write can no longer fence the fixtures out through the until clamp. And a dozen probes that could not discriminate the guard they name now can: the over-budget ledger carries a valid entry, the symlinked target carries a fenced entry, the marker's charset gates are pinned on both sides with valid fences, the alias fold's entries sit inside the window, the marker's plan fence survives a 3s nudge instead of dying on the epoch window, resumeAuthorized folds case, the stale gate record sits INSIDE the would-be slack, the identity anchor refuses a substring mention, a delivered auditor with zero diff reads stays refused, the digest narrowing accepts the compliant current digest, per-record granularity is pinned with a mixed prior session, the prior-side floors and the per-session ceiling have events inside their discriminating windows, the plan-swap abort fires on the restored-mtime rename shape, and the epoch test asserts the rewrite's content. * fix(review): round-9 blockers — single-read ledger writers, lifecycle-aware returns, floor narrowing run-ledger: the clobber guards decided from a SECOND read, so a transient fault clearing between the two reads let an append rewrite the whole ledger from the empty fallback; all three writers now classify the occupant once and decide everything from that one read, with the plan stat shared between the fence and the new entry. Plant shapes that can never be legitimate state — a directory (EISDIR on every rename, swallowed forever), an oversize regular file (frozen as "state to preserve" for the life of the plan) — are healed by the writers instead of freezing recording. The entry cap keeps the NEWEST end, so a backdated flood of distinct ids truncates itself rather than evicting the genuine entries and laundering the eviction on the next append. transcripts: `returned` now consults the harness's own lifecycle record — the `agent-<id>.meta.json` sidecar — so an agent killed after a text flush (which ends identically to a completed one in transcript content) no longer certifies; thought parts are excluded from the return text, so a thinking-mode agent killed between ROUND_TEXT and its tool calls does not hand its internal reasoning downstream as a verdict; and a transcript whose records carry two different session stamps is rejected whole — the grafted-tail shape that defeated the first-stamp ownership check. coverage: `currentDigestKeys` dates a findings-file-less key by its always-present prompt record instead of dropping it, closing the mirror case where the CURRENT digest's inlined-fallback keys were dropped and a previous round's verifier vouched for a list nobody opened; the same narrowing now applies to the Step 5 reverse-audit floor; and the Uncoverable supersession guard excludes records that themselves declare the same chunk, so two honest declarers no longer annihilate each other into a permanently unreviewable `missingChunks` loop. layer-audit-gate / retirement: both readers now require `returned` before consuming receipts — a died-mid-flight auditor's narration can carry receipt forms — and the gate's `delivered()` refuses a transcript that verbatim-matches more than one reverse-audit record (the concatenated-launch shape whose union territory corroborated layers no walk touched). Retirement's findings-read floor moves INTO the dry branch of `classifyReturn`, so a filed yield from an auditor that skipped the list read keeps its chunk hot instead of vanishing before classification. repo-context: the enriched plan is committed by a temp file that is stamped with the anchor's times BEFORE the rename and identity-checked against the anchor immediately before it — there is no longer an instant where the plan path carries an advanced epoch (the kill-window that permanently orphaned same-run evidence), and a concurrent capture landing during the read+compare+write window is refused instead of silently overwritten. Every fix carries a probe pinned by mutation: reverting each guard in isolation reddens at least one new test. * test(review): give the inode probe an imposter that cannot recycle the inode ext4 reuses a just-freed inode number immediately, so the delete-then-create fixture came back as the same (ino, mtime) identity — indistinguishable by design, and red only on Linux. The imposter is now created while the original exists and renamed over it, so its inode is distinct on every filesystem.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 堆叠在本分支上。