diff --git a/src/notice-store.ts b/src/notice-store.ts index 9083337185..8da9f168bf 100644 --- a/src/notice-store.ts +++ b/src/notice-store.ts @@ -37,6 +37,10 @@ const DISMISS_FILE = "upgrade-notices.json"; let cache: Map | 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 | 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 { - if (cache !== null || hydrating || !unifiedReceiptsActive()) return; + if (hydrated || hydrating || !unifiedReceiptsActive()) return; hydrating = true; - const merged = new Map(); 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(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; } diff --git a/tests/notice-unified-store.test.ts b/tests/notice-unified-store.test.ts index 6a13ce55a0..62166e6249 100644 --- a/tests/notice-unified-store.test.ts +++ b/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((resolve) => { releaseList = resolve; }); + const setGate = new Promise((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"); +});