Repository navigation
[PR] fix: make full-mode storage tests deterministic (#4004) - #4011
ranxianglei wants to merge 6 commits into
Conversation
Under the #3818 default (storage.unified=full) reads go unified-first and the sqlite index is authoritative, which bypasses the envelope integrity guards that four long-standing tests assert — so they failed non-deterministically locally. Two more broke on the on-disk layout once index.db lands in the sessions dir. Align the tests with shipped reality instead of changing production code: - Pin the envelope-guard assertions to shadow mode (envelope = read path), where those guards actually fire. - Make the "exactly one top-level entry" layout assertions ignore the unified sidecar's index.db(+wal/shm). - Add one full-mode test pinning the authority semantic: an intact index row serves over a tampered legacy envelope. Test-only; no production behavior changes.
ranxianglei
changed the title
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.6129-pr.4011.ce77a0bThis exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr4011.tgz
npm install -g packageInstall instructions are refreshed on each push. |
(ai: vllm-qwen/qwen3.8-27b) 查重：不重复——#4011 是 #4004 的执行 PR，与在途其他 PR 无重叠。但审核发现一个关键事实：这个 PR 已被 master 上的 标签：pr、bug、scope/ci、p3（测试基建问题，纯测试代码，零用户影响）。 先回答你上次「没看懂啥意思」的问题（option a vs b，白话版）背景一句话：新默认（full 模式）下，会话状态以 sqlite 索引为准，JSON 文件退化成冗余备份。老测试断言的是「篡改 JSON 文件 → 必须被拒绝」，但 full 模式下读取根本不看 JSON，这个断言就失效了。两个选项：
结论：维持选项 a，不需要反转。 理由一句话说清：索引行是 bili 把「记录 id + 内容」一起原子写进去的，直接从索引取不可能张冠李戴；而选项 b 是在权威读路径上依赖那份冗余备份的健康度——备份坏了会把本来能用的会话变成不可用，安全性却一分没涨。真正的「索引↔JSON 漂移」有专门通道（ 为什么 PR 可以关：wave 3 做得更全
本地实测（node v22）：干净 master 上三个相关文件全绿（proxy-persist 23/23、issue2129 4/4、storage-full-flip 10/10 含 ⑦）；rebase 到当前 master 后的 PR head 反而是 23/24——PR 的新测试因文件头 legacy 钉法被压成 legacy 而确定性失败。即：合并前不 rebase 会带红，rebase 后则整份 diff 冗余且自伤。 原 CI 红的根因（不是竞态，是环境差异）CI 所有测试文件经统一 preload（ 同一次 CI 里的其余红（#3769 rotation-hidden flake ×2 车道、plugin-bypass #920、一车道 exit-1 但无失败断言行）与本 PR 的两文件 diff 无关，属既有 flake 候选，不在此处置。 结构化八问
交付物
收工判定：PR #4011 可关闭、无需合并；全部修复已在 master 上。 |
…lp-merge sweep)
…lp-merge sweep)
master window commits 293ae65/de5e03659 bumped kernel/package.json to 0.0.165 without regenerating the lockfiles; lock-sync re-resolution then rewrites the stale recorded versions (root 0.0.163, kernel 0.0.164) and fails. Pure recorded-version sync via npm install --package-lock-only; no dependency or semantic change.
… PR body kernel-guard reads github.event.pull_request.body at event time; a body edit alone does not re-run workflows. Empty commit carries no file change.
(ai: vllm-qwen/qwen3.8-27b) 查重:不重复。 按失败签名(lock-sync / kernel-guard + 具体错误行)检索 open+closed issue/PR:历史同类 lockfile 漂移修复(#2717/#3782/#4419/#4420)均已关闭;本轮漂移无在途修复条目(哨兵的 ci-fixes 本次唤醒处理了该分支连续两道门禁红(attempt 1/2 → 2/2),均已解决并实测: ① lock-sync 红(head dbab850)— 继承性失败,非本分支代码问题。 master 最近两次 kernel bump( ② kernel-guard 红(head c9d9058)— ①的直接后果。 merge ci-fixes 后 PR diff 触及 八问:
行为变更披露:无(纯 lock 元数据同步,旧值→新值仅为记录版本号,零语义变化)。 判定:CI 侧收工——新 head |
Closes #4004
What / why
Under the #3818 default (
storage.unified=full) reads go unified-first and the sqlite index is the authoritative carrier. Six long-standing persistence tests broke under that default — the four named in the issue plus two more on-disk-layout assertions in the same file:loadSyncid-mismatch rejection,loadAllfilename-mismatch skip, protocol-meta location, and the Restart loses measurement state: buildSession() omits stats fields buildRecord() persists (k̂, usage anchor, baseline origin, localInputEstimate) #2129 value-rejection) live on the envelope read path. Full mode serves the intact index row before reaching them, so tampering only the JSON envelope no longer triggers a reject. These failed non-deterministically (whether the index import beat the load).index.db(+wal/shm) into the sessions dir, so tworeaddirSync(dir).length === 1assertions read 4 instead of 1. Deterministic.Fix — align the tests with shipped reality (test-only, no production change)
The "is the index authoritative" question was already answered by #3818 shipping full mode as the default for the index to become the load path. The index row is minted record-id + payload together by bili, so serving it cannot misattribute; genuine index↔envelope drift is handled separately by
driftedSessions(#3135④). Adding an id-consistency guard to the unified-first path (the issue's option b) would couple the authoritative read to the health of the redundant fallback copy — an availability regression with no safety gain. So:withShadowTempStorehelper (proxy-persist) and a scoped env pin (issue2129).index.db*.Behavior-change disclosure (Issue-Work Deliverables rule 4)
No production behavior changes. The runtime already behaved exactly as the corrected tests assert; this only makes the stale shadow-era test expectations deterministic again under the new default. No config surface touched.
Verification
master(e98ccda); clean (neither file was touched by the intervening commits).tsc --noEmitclean;tests/proxy-persist.test.ts24/24 andtests/issue2129-restart-measurement-state.test.ts4/4 on the rebased head.If option (b) was actually intended — the unified read also rejects on envelope-id mismatch even when the index is intact — say so and I'll reverse it; I don't believe that's the right semantics.
Kernel Boundary declaration
kernel-change: incidental lockfile metadata only — NO kernel source/config/semantic change. This PR's diff touches
kernel/package-lock.jsonsolely via the ci-fixes merge (dfadc21b3), which syncs recorded-version metadata (0.0.164→0.0.165) after master's kernel bumps293ae6542/de5e03659skipped lockfile regeneration (#2704 drift class; the repair that unblocks lock-sync on master tipcdac1c099). No change under kernel/src/**, no kernel/package.json version edit (already 0.0.165), no behavior impact. The matching root package-lock.json sync rides in the same merge.