[PR] fix(storage): pre-hydration receipt no longer blocks the notice-store legacy→kv migration (#5761)

src/notice-store.ts

@@ -37,6 +37,10 @@ const DISMISS_FILE = "upgrade-notices.json";
 
 let cache: Map<string, number> | null = null;
 let hydrating = false;
+// #5761: independent of the lazy cache — cache !== null only means "some
+// dismissal touched this process", NOT "hydration completed". Bailing on it
+// (the old gate) let one pre-hydration receipt block the migration forever.
+let hydrated = false;
 let warned = false;
 
 function warnOnce(tag: string, err: unknown): void {
@@ -85,39 +89,49 @@ export function dismissedCache(): ReadonlySet<string> | null {
 
 /** Once per process (wired at cli start): load kv rows + import the legacy
  *  file one time. Legacy-only instances keep the pure-file path forever
- *  (cache intentionally left null). Never throws. */
+ *  (cache intentionally left null). Never throws.
+ *  #5761 (twin of the #5757 host-plugin fix): UNION into whatever cache a
+ *  pre-hydration dismissal already lazily created instead of bailing — that
+ *  early receipt used to block both the one-time file→kv migration and every
+ *  sibling lane's rows for the whole process lifetime. */
 export async function hydrateNoticeReceipts(): Promise<void> {
-    if (cache !== null || hydrating || !unifiedReceiptsActive()) return;
+    if (hydrated || hydrating || !unifiedReceiptsActive()) return;
     hydrating = true;
-    const merged = new Map<string, number>();
     const now = Date.now();
-    let importedFromFile = 0;
     try {
         const dbRows = await getStore().listKvAsync(NS);
-        for (const r of dbRows) merged.set(r.key, r.updatedAt);
-        for (const k of fileKeysOf(process.env)) {
-            if (!merged.has(k)) {
-                merged.set(k, now);
-                importedFromFile += 1;
+        const rowKeys = new Set(dbRows.map((r) => r.key));
+        // One-time migration: every file key not yet in kv goes into kv
+        // (best-effort, no format policing — the API path validates, the file
+        // may not). The view below unions file keys directly, so each is honored
+        // in-process whether or not its kv write landed.
+        const fileKeys = fileKeysOf(process.env);
+        const toImport = [...fileKeys].filter((k) => !rowKeys.has(k));
+        for (const k of toImport) {
+            try {
+                await getStore().setKvAsync(NS, k, { at: now });
+            } catch (err) {
+                warnOnce("migrate", err);
+                break;
             }
         }
-        if (importedFromFile > 0) {
-            for (const [k] of merged) {
-                if (![...dbRows].some((r) => r.key === k)) {
-                    try {
-                        await getStore().setKvAsync(NS, k, { at: now });
-                    } catch (err) {
-                        warnOnce("migrate", err);
-                        break;
-                    }
-                }
-            }
-            loggerLog("info", `[notices] imported ${importedFromFile} receipt(s) from the legacy JSON file into unified storage (kv_store namespace "notices")`);
+        if (toImport.length > 0) {
+            loggerLog("info", `[notices] imported ${toImport.length} receipt(s) from the legacy JSON file into unified storage (kv_store namespace "notices")`);
+        }
+        // Snapshot the LIVE view only now: nothing awaits between here and the
+        // publish below, so a dismissal landing any time up to this point
+        // (pre-hydration lazy adoption included) survives the final
+        // assignment instead of being clobbered until restart.
+        const merged = new Map<string, number>(cache ?? []);
+        for (const r of dbRows) merged.set(r.key, r.updatedAt);
+        for (const k of fileKeys) {
+            if (!merged.has(k)) merged.set(k, now);
         }
+        cache = merged;
+        hydrated = true;
     } catch (err) {
         warnOnce("hydrate", err);
     }
-    cache = merged;
     hydrating = false;
 }
 
@@ -162,5 +176,6 @@ export function persistDismissedKey(key: string): void {
 export function _resetNoticeReceiptsForTest(): void {
     cache = null;
     hydrating = false;
+    hydrated = false;
     warned = false;
 }

tests/notice-unified-store.test.ts

@@ -90,3 +90,64 @@ test("cap trims oldest non-builtin keys only; built-ins survive", async () => {
     const small = dismissedCache()!;
     for (const k of two) assert.ok(small.has(k), "under-cap keys all retained");
 });
+
+// #5761: a dismissal landing BEFORE the boot hydration lazily created the
+// cache, which made the hydrate bail — blocking the one-time file→kv
+// migration (and hiding sibling lanes' rows) for the process lifetime.
+// Twin of the #5757 host-plugin regression case.
+test("a pre-hydration dismissal no longer blocks the migration", async () => {
+    try { fs.rmSync(path.join(stateBili, "upgrade-notices.json")); } catch { /* fresh volume */ }
+    _resetNoticeReceiptsForTest();
+
+    // the admin action lands before cli-start hydration has run — the lazy
+    // adoption sees an EMPTY file, so nothing below can come from it:
+    assert.equal(dismissUpgradeNotice(WM_FASTPATH_INTRO_ID), true);
+    const lateKey = `late-file@${"b".repeat(16)}`;
+    fs.mkdirSync(stateBili, { recursive: true });
+    fs.writeFileSync(path.join(stateBili, "upgrade-notices.json"), JSON.stringify({ dismissed: [WM_FASTPATH_INTRO_ID, lateKey] }, null, 2));
+    await new Promise((r) => setTimeout(r, 50)); // let the queued receipt land in kv
+    await hydrateNoticeReceipts();
+
+    const seen = dismissedUpgradeNotices();
+    assert.ok(seen.has(WM_FASTPATH_INTRO_ID), "the pre-dismissed id stays visible");
+    assert.ok(seen.has(lateKey), "hydration ran despite the pre-existing lazy cache");
+    const rows = await getStore().listKvAsync("notices");
+    assert.ok(rows.some((r) => r.key === WM_FASTPATH_INTRO_ID), "receipt landed in kv_store");
+    assert.ok(rows.some((r) => r.key === lateKey), "one-time migration ran despite the pre-existing lazy cache");
+});
+
+// #5761 variant: a dismissal landing inside the hydrate's await gap used to
+// be clobbered by the final cache assignment until restart. The publish must
+// fold in everything added up to it. Both KV directions are gated so the
+// dismissal's own durable write cannot land before the list resolves —
+// otherwise the old code would re-derive the key from kv and mask the loss.
+test("a dismissal landing during hydration survives the final publish", async () => {
+    try { fs.rmSync(path.join(stateBili, "upgrade-notices.json")); } catch { /* fresh volume */ }
+    _resetNoticeReceiptsForTest();
+    const store = getStore();
+    const origList = store.listKvAsync.bind(store);
+    const origSet = store.setKvAsync.bind(store);
+    let releaseList: () => void = () => undefined;
+    let releaseSet: () => void = () => undefined;
+    const listGate = new Promise<void>((resolve) => { releaseList = resolve; });
+    const setGate = new Promise<void>((resolve) => { releaseSet = resolve; });
+    store.listKvAsync = async (ns?: string, prefix?: string) => {
+        await listGate;
+        return origList(ns, prefix);
+    };
+    store.setKvAsync = async (ns: string, key: string, value: unknown) => {
+        await setGate;
+        return origSet(ns, key, value);
+    };
+    const hydr = hydrateNoticeReceipts();
+    await new Promise((r) => setTimeout(r, 20)); // park the hydrate inside the gated list
+    const midKey = `mid-hydrate@${"c".repeat(16)}`;
+    assert.equal(dismissUpgradeNotice(midKey), true); // memory now, durable write parked on setGate
+    releaseList(); // list resolves WITHOUT midKey — only the live-cache union can save it
+    await hydr;
+    store.listKvAsync = origList;
+    store.setKvAsync = origSet;
+    releaseSet();
+    const seen = dismissedUpgradeNotices();
+    assert.ok(seen.has(midKey), "mid-hydration dismissal survived the final publish");
+});