Repository navigation
[PR] fix(storage): pre-hydration receipt no longer blocks the notice-store legacy→kv migration (#5761) - #5768
ranxianglei wants to merge 2 commits into
Conversation
… legacy→kv migration (#5761) A dismissal landing before the cli-boot hydration lazily created the receipts cache, which made hydrateNoticeReceipts bail on `cache !== null` — skipping the one-time legacy JSON→kv migration for that process's lifetime (re-import churn on every boot) and hiding sibling lanes' kv rows until restart; a dismissal landing inside the hydrate's await gap was clobbered by the final cache assignment. Twin of the host-plugin-notice fix that landed in the #5757 line (commit 4d74613), ported onto master's API surface (unifiedKvActive/getStore; the kvStore() probe from that line is not merged yet): - hydrate unions into the existing lazy cache instead of bailing; a new hydrated latch (distinct from lazy-init) gates re-entry - the live-cache snapshot moves to just before the publish with no awaits in between, so mid-hydration dismissals survive the final assignment - publish only on success: a failed listKvAsync now leaves the cache null (legacy-file fallback stays live, next call retries) instead of clobbering it with an empty/partial map for the process lifetime Tests: two regression cases mirroring the #5757 shapes — both fail on the pre-fix code and pass with it; adjacent notice suites green.
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.9264-pr.5768.28ddb8fThis exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5768.tgz
npm install -g packageInstall instructions are refreshed on each push. |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) 接单确认 — 这是 [PR] 类任务(GitHub PR #5768 → issue #5761),按流程做独立 review,不重做。 查重: 不重复。 核对了全部 open [PR] 行,没有第二个 PR 针对 #5761(notice-store hydration race)。关联的 #5757(host-plugin-notice.ts 同模式修复,commit 4d74613)已合入 master,与本 PR 是同一模式在不同文件上的两个实例——#5761 本身作为独立 issue 立项合理,本 PR 与其无重叠。 标签(建议): 开始独立审核:读原 issue → 逐行审 diff → 本地定向测试 → CI 状态。结论稍后按八问体例补在本线程。 |
(ai: vllm-qwen/qwen3.8-27b) PR #5768 审核完成(单 commit cf85588,基于当前 master bfabcd1,0 commits 落后;独立拉分支核对,未照抄 PR 描述)。 ① 是什么问题 ② 什么原因导致的 ③ 影响范围与严重程度 ④ 怎么修复的 ⑤ 是否有回归问题
⑥ 是否破坏缓存 ⑦ 是否改了设计方向 ⑧ 是否引入了新的配置 两处有意的行为变更(旧→新 + 原因)
验证证据(独立复现,不只信 PR 描述)
一个观察(给 #5753 线,不阻本 PR):host-plugin-notice.ts 的 twin 实现(4d74613)把 live-cache 快照取在迁移循环的 awaits 之前,迁移窗口内落下的 dismissal 在那个文件里仍会被覆盖;本 PR 的 notice-store 版本把快照移到 publish 紧前方,严格更紧——#5753 合入时可顺手把同样顺序调整应用到 twin 侧。 判定:八问全过,diff 干净(2 文件,每行都与本修复相关),回归测试有牙且独立复现成立,CI 全绿——建议合并(合并权在你:https://github.com/ranxianglei/billion-context/pull/5768)。本次审核未推送任何 commit(未发现需要补修的缺陷)。 |
|
(ai: qwen/qwen3.8-27b) APPROVE(建议合并)。 注:本账号即 PR 作者,GitHub 禁止对本人 PR 提交正式 approve 事件(API 返回「Can not approve your own pull request」),故审核结论以本 PR 评论记录,合并权在 owner。storage-scope advisory fix;正确、最小、真回归测试——独立对照 master 已顺手修: 迁移注释与真实语义不符——原注释称「raw junk stays file-only」,但 八问:
测试有效性:两条新增用例是真回归钉——用例1(启动前 dismissal 不再挡迁移)与用例2(hydrate 间隙内 dismissal 存活)逐条推演旧代码必挂、新代码必过;用例2 对 kv list/set 双向设闸,防止旧代码从 kv 反推掩盖丢失,设计到位。可选增强(非阻塞):可再加一条「listKvAsync 抛错 → cache 仍 null → 文件兜底存活」钉住披露的失败语义变更;属增强钉而非 bug 捕获器,不拦。 判定:storage-scope 公告面修复,正确、最小、测试有牙且独立复现成立,两处行为变更均已披露且方向正确,无 kernel/config/wire/安全影响,diff 干净(2 文件,每行都相关)。建议合并(合并权在你:https://github.com/ranxianglei/billion-context/pull/5768)。 |
(ai: qwen/qwen3.8-27b) 审核收工。本 pass 在前一 pass(floor 6)结论之上补了两件事:
CI:新 head 判定:建议合并——storage-scope 公告面修复,正确、最小、真回归测试;两处行为变更均已披露且方向正确,无 kernel/config/wire/安全影响。合并权在 owner(agent 无权合并)。 |
|
该PR已按根因归类纳入 [kernel-compression] 根因汇总 #6265,由汇总单统一跟踪处理;本单不再单独跟进,故关闭。如认为归类有误或问题仍未解决,请直接回复,会重新打开。 |
|
状态更正(自动化脚本误操作): 本单此前被「纳入根因聚合」流程误关闭, 现重新打开。机器人工作已停, 本单由根因聚合线程统一跟踪, 根因修复落地后请手动关闭或在本单回复「close」。 |
|
更正说明:本单此前被归类聚合流程误关闭。按「只停机器人、不动 issue 本身」的口径重新打开;本单已被根因聚合线程纳入跟踪,机器人在本单上的工作已停止,不会再有自动处理。归类或纳入有误请直接回复指出。 |
|
重新打开:此前被归类聚合脚本误关。按 owner 口径,纳入根因聚合的 issue 只停机器人、不关闭,由根因聚合线程统一跟踪,根因修复落地后统一关闭。 |
|
(更正)此前按"清理"口径误关闭;按 owner 口径纠正:被根因聚合纳入的 issue 只停机器人、不关闭。本 issue 重新打开,机器人工作已停。 |
(ai: qwen/qwen3.8-27b) 收到。本单已重新打开、机器人在本单上的工作已停止——我不再重复审核、不再推送 commit、不关闭,根因修复落地后由聚合线程统一收口。 审核交付物此前已全部完成,无需重做:
另按 owner 邀请("如认为归类有误请直接回复")提一点,请 owner 定夺:本 PR 是 storage 范围( 机器人在本单上到此为止。 |
(ai: qwen/qwen3.8-27b) 收到,状态更正已知悉。归类/纳入的疑问我已在上一条回复中指出(本 PR 为 storage 范围、不涉及 |
(ai: qwen/qwen3.8-27b) 本单(PR #5768,storage 范围 advisory 通知回执 hydration 竞态修复)审核已完成,汇总如下: 已交付
当前状态
等 owner 手动关闭或在本单回复「close」。 |
Fixes #5761
What
hydrateNoticeReceipts()(src/notice-store.ts, #5548) usedcache !== nullas its "already hydrated" gate — butaddDismissedKeyInMemorylazily creates that same cache on first dismiss. So:void hydrateNoticeReceipts(), src/cli.ts:965) made hydration bail for the whole process: the one-time legacy JSON→kv migration was skipped (the file re-imports on every boot) and sibling lanes' kv rows stayed invisible in-process until restart;cache = merged.Twin of the pattern already fixed in host-plugin-notice.ts on the #5757 line (commit 4d74613) — notice-store was missing the same fix.
How
Ports the #5757 pattern onto master's API surface (
unifiedKvActive()+getStore()— thekvStore()probe introduced on that line is not merged yet, so this PR deliberately does not depend on it; #5757's branch touches neither of these files, so merge order is conflict-free either way):hydratedlatch (distinct from lazy-init) gates re-entry;Behavior changes (intentional)
listKvAsyncstill published the empty/partial map intocache, which killed the legacy-file fallback for the process lifetime and prevented any retry. Now the failure leavescachenull → file fallback stays live, next call retries. Advisory surface only; strictly more correct.No config-surface change, no wire-format/threshold/timing drift, no cache-invalidation impact, no new env/config keys.
Tests
Two regression cases in tests/notice-unified-store.test.ts mirroring the #5757 case shapes:
Local: targeted suite 7/7 green; adjacent notice suites (issue5079-pck-default-off, issue5548-notice-dismiss-endpoint, local-announcements, notice-registry, wm-fastpath-intro-notice) 32/32 green; typecheck + build clean. Full-suite verdict via CI.