From cd0eedd13a33988b7f2f0aeaa0c94558fcab56fb Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 2 Sep 2026 15:01:01 -0400 Subject: [PATCH] refactor(sidebar-sync): single-call readOutboxWithMeta recovers preservedKey from foreign records; add P3 reconnect+restart hook regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thufir pass-2 findings — all four corrections applied: 1. P3 restart durability: replace two-function readOutbox+readOutboxPreservedKey with readOutboxWithMeta that enumerates records once and returns { store, preservedKey }. Key selected by max queuedAt across ALL records (own and foreign); after quit, the prior window's nonce is gone so its record is foreign — the old isOwn filter returned undefined, losing the reservation. Both hooks forward outboxMeta.preservedKey directly to publish* on bootstrap. 2. Two-read snapshot/provenance split closed: the capacity bound now applies inside readOutboxWithMeta using the selected preservedKey, so the clicked channel is never evicted before it reaches the manager. 3. discardPending() clears pendingPreservedKey: minor correctness fix so a confirmed publish resets the field alongside pendingStore and clearOutbox. 4. Test consolidation: remove non-causal reconnect/remount tests from channelStarsSync.test.mjs (+120) and channelMutesSync.test.mjs (+192). Replace with two parameterized regressions in mergeLaneHook.shared.test.mjs: - P3 reconnect: triggers the actual subscribeToReconnects callback registered by the hook, asserts clicked channel survives 501-entry merge. Failing mutation: revert hook reconnect to publish*(pending). - P3 restart: seeds a foreign-nonce outbox envelope with preservedKey, mounts hook (bootstrap reads foreign record → recovers key → publishes), fires 2s debounce, asserts clicked channel survives 501-entry merge. Failing mutation: revert to readOutboxPreservedKey (isOwn filter) or drop bootstrap preservedKey forward. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../sidebar/lib/channelMutesStorage.ts | 43 ++- .../sidebar/lib/channelMutesSync.test.mjs | 133 +-------- .../sidebar/lib/channelStarsStorage.ts | 43 ++- .../sidebar/lib/channelStarsSync.test.mjs | 131 +-------- .../sidebar/lib/mergeLaneHook.shared.test.mjs | 261 ++++++++++++++++++ .../sidebar/lib/mergeLaneStorage.shared.ts | 78 +++--- .../sidebar/lib/mergeLaneSyncManager.ts | 1 + .../sidebar/lib/multiWindowOutbox.test.mjs | 26 +- .../features/sidebar/lib/useChannelMutes.ts | 25 +- .../features/sidebar/lib/useChannelStars.ts | 25 +- 10 files changed, 398 insertions(+), 368 deletions(-) diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.ts b/desktop/src/features/sidebar/lib/channelMutesStorage.ts index 9fc0c9fc8b..41c9047866 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.ts +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.ts @@ -1,11 +1,11 @@ import { clearOutbox, makeStorageKey, - readOutbox, - readOutboxPreservedKey, + readOutboxWithMeta, readStore, reclaimSubsumedOutbox, writeOutbox, + type OutboxWithMeta, } from "./mergeLaneStorage.shared"; const STORAGE_KEY_PREFIX = "buzz-channel-mutes.v1"; @@ -296,38 +296,31 @@ export function writeChannelMutesOutbox( } /** - * Merge every window's persisted unpublished edit into one store for resume, or - * null when none exists. Per-entry `mergeStores` is order-independent, so two - * windows' concurrent clicks on different channels both survive. + * Merge every window's persisted unpublished edit into one store for resume, + * together with the most-recent explicit `preservedKey` across all records + * (own and foreign), or null when no records exist. + * + * The preserved key is selected deterministically: the record with the highest + * `queuedAt` that carries an explicit `preservedKey`; ties broken by storage + * key string (max). This recovers the durable reservation after a quit and + * restart, when the prior window's record is now foreign (its session nonce is + * gone) but its `preservedKey` is still in localStorage (Kalvin P3). + * + * Both the fold and the capacity bound apply the selected key so the clicked + * channel is never evicted during the read itself. */ -export function readChannelMutesOutbox( +export function readChannelMutesOutboxWithMeta( pubkey: string, relayUrl: string, -): ChannelMuteStore | null { - return readOutbox( +): OutboxWithMeta | null { + return readOutboxWithMeta( OUTBOX_KEY_PREFIX, pubkey, relayUrl, parseMutePayload, mergeStores, DEFAULT_STORE, - ); -} - -/** - * Read the `preservedKey` from this window's own outbox record, or `undefined` - * when none exists. Used on bootstrap to restore the capacity-bounding - * reservation across remount (Kalvin P3). - */ -export function readChannelMutesOutboxPreservedKey( - pubkey: string, - relayUrl: string, -): string | undefined { - return readOutboxPreservedKey( - OUTBOX_KEY_PREFIX, - pubkey, - relayUrl, - parseMutePayload, + boundMuteStore, ); } diff --git a/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs b/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs index 881ce20d13..40a8fc19b8 100644 --- a/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs +++ b/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs @@ -8,7 +8,7 @@ import test, { mock } from "node:test"; import { relayClient } from "@/shared/api/relayClient"; import { parseMutePayload, - readChannelMutesOutbox, + readChannelMutesOutboxWithMeta, } from "./channelMutesStorage.ts"; import { ChannelMuteSyncManager } from "./channelMutesSync.ts"; import { @@ -94,7 +94,7 @@ test("mutes wire: outbox/subsumption callbacks are wired to mutes storage (not s m.publishMutes(store); // Outbox must be written synchronously on publish — proves writeChannelMutesOutbox is wired. assert.ok( - readChannelMutesOutbox("pk-outbox-mutes", RELAY) !== null, + readChannelMutesOutboxWithMeta("pk-outbox-mutes", RELAY) !== null, "publishMutes must write to the mutes outbox (not stars)", ); // Drive through a full publish cycle with a subsuming head. After discardPending, @@ -107,7 +107,10 @@ test("mutes wire: outbox/subsumption callbacks are wired to mutes storage (not s ); fw._fireTimer(); // fires debounce → doPublish → fetchOwnBlob → publish → confirmRetained await new Promise((r) => setTimeout(r, 30)); - assert.equal(readChannelMutesOutbox("pk-outbox-mutes", RELAY), null); + assert.equal( + readChannelMutesOutboxWithMeta("pk-outbox-mutes", RELAY), + null, + ); m.destroy(); } finally { tauri.restore(); @@ -211,124 +214,6 @@ test("P3: clicked channel is preserved through pre-publish mergeWithRemote at ca } }); -// Mutation: calling public publish() on reconnect resets pendingPreservedKey, -// letting a 501-entry pre-publish merge evict the clicked channel again (P3). -test("P3 reconnect: retryReconnectMutesPublish preserves clicked channel key across reconnect", async () => { - const MAX = 500; - const clickedId = "ch-reconnect-mute"; - const clickedEntry = { muted: true, updatedAt: 1, rev: 1 }; // oldest - const localChannels = { [clickedId]: clickedEntry }; - for (let i = 0; i < MAX - 1; i++) { - localChannels[`ch-local-${i}`] = { muted: false, updatedAt: 100, rev: 0 }; - } - const localStore = { version: 1, channels: localChannels }; - - // Remote: a new channel not in local, making merged = 501 - const remoteChannels = {}; - for (let i = 0; i < MAX - 1; i++) { - remoteChannels[`ch-local-${i}`] = { muted: false, updatedAt: 100, rev: 0 }; - } - remoteChannels["ch-remote-rc"] = { muted: false, updatedAt: 100, rev: 0 }; - const remoteStore = { version: 1, channels: remoteChannels }; - - const fw = makeFakeWindow(); - const restore = installFakeWindow(fw); - const tauri = installEchoTauri("pk-p3-reconnect-mutes"); - const remoteHead = tauri.mintHead(remoteStore, 50, "evt-rc-mutes"); - remoteHead.tags = [["d", "channel-mutes"]]; - - let fetchCalls = 0; - mock.method(relayClient, "fetchEvents", () => { - fetchCalls++; - if (fetchCalls === 1) return Promise.resolve([remoteHead]); - return Promise.resolve([]); - }); - let publishedEvent = null; - mock.method(relayClient, "publishEvent", (evt) => { - publishedEvent = evt; - return Promise.resolve(); - }); - - try { - const m = new ChannelMuteSyncManager("pk-p3-reconnect-mutes", RELAY); - // Initial click: sets pendingPreservedKey = clickedId - m.publishMutes(localStore, clickedId); - // Cancel the debounce (simulates quit before the 2s fires). - fw._fireTimer(); // flush pending timer - // Let any microtasks settle — abort the current cycle if one started - await new Promise((r) => setTimeout(r, 0)); - // Reset: simulate the manager having a pending edit with preservedKey set - // but the cycle not yet complete. We reinitialise with a fresh timer. - m.cancelPendingMutePublish(); - // Re-publish with key to set pendingPreservedKey properly - fetchCalls = 0; - publishedEvent = null; - m.publishMutes(localStore, clickedId); - // Use retryReconnectMutesPublish — must NOT reset pendingPreservedKey - m.retryReconnectMutesPublish(); - fw._fireTimer(); - await new Promise((r) => setTimeout(r, 30)); - // If retryReconnect properly preserved the key, clicked channel survives. - assert.ok(publishedEvent !== null, "publish must have fired after retry"); - const plaintext = tauri.capturedPlaintext(); - const published = JSON.parse(plaintext); - assert.ok( - clickedId in published.channels, - `clicked channel must survive 501-entry merge after retryReconnectMutesPublish`, - ); - m.destroy(); - } finally { - tauri.restore(); - restore(); - mock.reset(); - } -}); - -// Mutation: omitting preservedKey from writeChannelMutesOutbox means it is not -// persisted in the envelope, so readChannelMutesOutboxPreservedKey returns -// undefined and bootstrap replay calls publishMutes(outbox) without protection -// — a 501-entry pre-publish merge can evict the clicked channel on remount (P3). -test("P3 remount: writeChannelMutesOutbox persists preservedKey; readChannelMutesOutboxPreservedKey recovers it", async () => { - const { - readChannelMutesOutbox, - readChannelMutesOutboxPreservedKey, - writeChannelMutesOutbox, - } = await import("./channelMutesStorage.ts"); - - const fw = makeFakeWindow(); - const restore = installFakeWindow(fw); - - const pubkey = "pk-p3-remount-mutes-storage"; - const relayUrl = "wss://r.remount-mutes"; - const clickedId = "ch-remount-mutes"; - const store = { - version: 1, - channels: { [clickedId]: { muted: true, updatedAt: 1, rev: 0 } }, - }; - - try { - writeChannelMutesOutbox(pubkey, store, relayUrl, clickedId); - - assert.ok( - readChannelMutesOutbox(pubkey, relayUrl) !== null, - "outbox must be written", - ); - - const restoredKey = readChannelMutesOutboxPreservedKey(pubkey, relayUrl); - assert.equal( - restoredKey, - clickedId, - "preservedKey must be readable back from the written mutes outbox record", - ); - - writeChannelMutesOutbox(pubkey, store, "wss://r.no-key", undefined); - const noKey = readChannelMutesOutboxPreservedKey(pubkey, "wss://r.no-key"); - assert.equal( - noKey, - undefined, - "absent preservedKey must read back as undefined", - ); - } finally { - restore(); - } -}); +// P3 reconnect and P3 remount/restart scenarios are in mergeLaneHook.shared.test.mjs, +// parameterized across both lanes, triggering the actual registered reconnect callback +// and seeding foreign-nonce outbox envelopes to model quit/restart (Kalvin P3). diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.ts b/desktop/src/features/sidebar/lib/channelStarsStorage.ts index e282bed7ba..ca65a7a736 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.ts @@ -1,11 +1,11 @@ import { clearOutbox, makeStorageKey, - readOutbox, - readOutboxPreservedKey, + readOutboxWithMeta, readStore, reclaimSubsumedOutbox, writeOutbox, + type OutboxWithMeta, } from "./mergeLaneStorage.shared"; const STORAGE_KEY_PREFIX = "buzz-channel-stars.v1"; @@ -298,38 +298,31 @@ export function writeChannelStarsOutbox( } /** - * Merge every window's persisted unpublished edit into one store for resume, or - * null when none exists. Per-entry `mergeStores` is order-independent, so two - * windows' concurrent clicks on different channels both survive. + * Merge every window's persisted unpublished edit into one store for resume, + * together with the most-recent explicit `preservedKey` across all records + * (own and foreign), or null when no records exist. + * + * The preserved key is selected deterministically: the record with the highest + * `queuedAt` that carries an explicit `preservedKey`; ties broken by storage + * key string (max). This recovers the durable reservation after a quit and + * restart, when the prior window's record is now foreign (its session nonce is + * gone) but its `preservedKey` is still in localStorage (Kalvin P3). + * + * Both the fold and the capacity bound apply the selected key so the clicked + * channel is never evicted during the read itself. */ -export function readChannelStarsOutbox( +export function readChannelStarsOutboxWithMeta( pubkey: string, relayUrl: string, -): ChannelStarStore | null { - return readOutbox( +): OutboxWithMeta | null { + return readOutboxWithMeta( OUTBOX_KEY_PREFIX, pubkey, relayUrl, parseStarPayload, mergeStores, DEFAULT_STORE, - ); -} - -/** - * Read the `preservedKey` from this window's own outbox record, or `undefined` - * when none exists. Used on bootstrap to restore the capacity-bounding - * reservation across remount (Kalvin P3). - */ -export function readChannelStarsOutboxPreservedKey( - pubkey: string, - relayUrl: string, -): string | undefined { - return readOutboxPreservedKey( - OUTBOX_KEY_PREFIX, - pubkey, - relayUrl, - parseStarPayload, + boundStarStore, ); } diff --git a/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs b/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs index e1fb38d748..e58037427b 100644 --- a/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs +++ b/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs @@ -8,7 +8,7 @@ import test, { mock } from "node:test"; import { relayClient } from "@/shared/api/relayClient"; import { parseStarPayload, - readChannelStarsOutbox, + readChannelStarsOutboxWithMeta, } from "./channelStarsStorage.ts"; import { ChannelStarSyncManager } from "./channelStarsSync.ts"; import { @@ -95,7 +95,7 @@ test("stars wire: outbox/subsumption callbacks are wired to stars storage (not m m.publishStars(store); // Outbox must be written synchronously on publish — proves writeChannelStarsOutbox is wired. assert.ok( - readChannelStarsOutbox("pk-outbox-stars", RELAY) !== null, + readChannelStarsOutboxWithMeta("pk-outbox-stars", RELAY) !== null, "publishStars must write to the stars outbox (not mutes)", ); // Drive through a full publish cycle: fire debounce, return a subsuming head on @@ -109,7 +109,10 @@ test("stars wire: outbox/subsumption callbacks are wired to stars storage (not m ); fw._fireTimer(); // fires debounce → doPublish → fetchOwnBlob → publish → confirmRetained await new Promise((r) => setTimeout(r, 30)); - assert.equal(readChannelStarsOutbox("pk-outbox-stars", RELAY), null); + assert.equal( + readChannelStarsOutboxWithMeta("pk-outbox-stars", RELAY), + null, + ); m.destroy(); } finally { tauri.restore(); @@ -230,122 +233,6 @@ test("P3: clicked channel is preserved through pre-publish mergeWithRemote at ca } }); -// Mutation: calling public publish() on reconnect resets pendingPreservedKey, -// letting a 501-entry pre-publish merge evict the clicked channel (P3 reconnect). -test("P3 reconnect: retryReconnectStarsPublish preserves clicked channel key across reconnect", async () => { - const MAX = 500; - const clickedId = "ch-reconnect-star"; - const clickedEntry = { starred: true, updatedAt: 1, rev: 1 }; // oldest - const localChannels = { [clickedId]: clickedEntry }; - for (let i = 0; i < MAX - 1; i++) { - localChannels[`ch-local-${i}`] = { starred: false, updatedAt: 100, rev: 0 }; - } - const localStore = { version: 1, channels: localChannels }; - - const remoteChannels = {}; - for (let i = 0; i < MAX - 1; i++) { - remoteChannels[`ch-local-${i}`] = { - starred: false, - updatedAt: 100, - rev: 0, - }; - } - remoteChannels["ch-remote-rc"] = { starred: false, updatedAt: 100, rev: 0 }; - const remoteStore = { version: 1, channels: remoteChannels }; - - const fw = makeFakeWindow(); - const restore = installFakeWindow(fw); - const tauri = installEchoTauri("pk-p3-reconnect-stars"); - const remoteHead = tauri.mintHead(remoteStore, 50, "evt-rc-stars"); - remoteHead.tags = [["d", "channel-stars"]]; - - let fetchCalls = 0; - mock.method(relayClient, "fetchEvents", () => { - fetchCalls++; - if (fetchCalls === 1) return Promise.resolve([remoteHead]); - return Promise.resolve([]); - }); - let publishedEvent = null; - mock.method(relayClient, "publishEvent", (evt) => { - publishedEvent = evt; - return Promise.resolve(); - }); - - try { - const m = new ChannelStarSyncManager("pk-p3-reconnect-stars", RELAY); - m.publishStars(localStore, clickedId); - m.cancelPendingStarPublish(); - fetchCalls = 0; - publishedEvent = null; - // Re-publish to set pendingPreservedKey, then retry via reconnect path. - m.publishStars(localStore, clickedId); - m.retryReconnectStarsPublish(); - fw._fireTimer(); - await new Promise((r) => setTimeout(r, 30)); - assert.ok(publishedEvent !== null, "publish must have fired after retry"); - const plaintext = tauri.capturedPlaintext(); - const published = JSON.parse(plaintext); - assert.ok( - clickedId in published.channels, - `clicked channel must survive 501-entry merge after retryReconnectStarsPublish`, - ); - m.destroy(); - } finally { - tauri.restore(); - restore(); - mock.reset(); - } -}); - -// Mutation: omitting preservedKey from writeChannelStarsOutbox means it is not -// persisted in the envelope, so readChannelStarsOutboxPreservedKey returns -// undefined and bootstrap replay calls publishStars(outbox) without protection -// — a 501-entry pre-publish merge can evict the clicked channel on remount (P3). -test("P3 remount: writeChannelStarsOutbox persists preservedKey; readChannelStarsOutboxPreservedKey recovers it", async () => { - const { - readChannelStarsOutbox, - readChannelStarsOutboxPreservedKey, - writeChannelStarsOutbox, - } = await import("./channelStarsStorage.ts"); - - const fw = makeFakeWindow(); - const restore = installFakeWindow(fw); - - const pubkey = "pk-p3-remount-storage"; - const relayUrl = "wss://r.remount-storage"; - const clickedId = "ch-remount-storage"; - const store = { - version: 1, - channels: { [clickedId]: { starred: true, updatedAt: 1, rev: 0 } }, - }; - - try { - // Write WITH preservedKey — models a click that calls publishStars(store, clickedId). - writeChannelStarsOutbox(pubkey, store, relayUrl, clickedId); - - // The merged outbox must be present. - assert.ok( - readChannelStarsOutbox(pubkey, relayUrl) !== null, - "outbox must be written", - ); - - // The preserved key must round-trip through the storage envelope. - const restoredKey = readChannelStarsOutboxPreservedKey(pubkey, relayUrl); - assert.equal( - restoredKey, - clickedId, - "preservedKey must be readable back from the written outbox record", - ); - - // Writing WITHOUT preservedKey (old behaviour / other-window record) returns undefined. - writeChannelStarsOutbox(pubkey, store, "wss://r.no-key", undefined); - const noKey = readChannelStarsOutboxPreservedKey(pubkey, "wss://r.no-key"); - assert.equal( - noKey, - undefined, - "absent preservedKey must read back as undefined", - ); - } finally { - restore(); - } -}); +// P3 reconnect and P3 remount/restart scenarios are in mergeLaneHook.shared.test.mjs, +// parameterized across both lanes, triggering the actual registered reconnect callback +// and seeding foreign-nonce outbox envelopes to model quit/restart (Kalvin P3). diff --git a/desktop/src/features/sidebar/lib/mergeLaneHook.shared.test.mjs b/desktop/src/features/sidebar/lib/mergeLaneHook.shared.test.mjs index 651a6665ff..46eeb5d895 100644 --- a/desktop/src/features/sidebar/lib/mergeLaneHook.shared.test.mjs +++ b/desktop/src/features/sidebar/lib/mergeLaneHook.shared.test.mjs @@ -449,4 +449,265 @@ export function runMergeLaneHookSuite({ restore(); } }); + + // P3 reconnect: hook's registered subscribeToReconnects callback must use + // retryReconnect*Publish(), NOT publish*(pending), so pendingPreservedKey is + // not reset. With 500 local entries and 1 fresh remote entry, merged = 501. + // Without the fix, publish*(pending) resets pendingPreservedKey → the clicked + // channel is evicted. Reverting the hook's reconnect effect to publish*(pending) + // fails this test while leaving the manager-level tests green. + // + // Failing mutation: revert hook reconnect from retryReconnect*Publish() to + // publish*(outbox or pending). + test(`${label}: P3 reconnect — registered reconnect callback preserves clicked channel through 501-entry merge`, async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + + const MAX = MAX_ENTRIES; + const clickedId = `ch-rc-click-${label}`; + const relayUrl = "wss://relay-p3-rc"; + + // Local: clicked channel (oldest) + 499 others at updatedAt=100 + const localChannels = { + [clickedId]: { [entryValueField]: true, updatedAt: 1, rev: 1 }, + }; + for (let i = 0; i < MAX - 1; i++) { + localChannels[`ch-rc-${i}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + } + // Remote: same 499 + one fresh channel → merged = 501 + const remoteChannels = {}; + for (let i = 0; i < MAX - 1; i++) { + remoteChannels[`ch-rc-${i}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + } + remoteChannels[`ch-rc-new-${label}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + + const pubkey = `pk-p3-rc-${label}`; + const reconnect = {}; + const restore = stubRelay(relayClient, { reconnect }); + const tauri = installEchoTauri(pubkey); + const remoteHead = tauri.mintHead( + { version: 1, channels: remoteChannels }, + 50, + `evt-rc-${label}`, + ); + remoteHead.tags = [["d", dTag]]; + remoteHead.pubkey = pubkey; + remoteHead.kind = 30078; + + const publishCalls = []; + let fetchCalls = 0; + relayClient.fetchEvents = async () => { + fetchCalls++; + return fetchCalls === 1 ? [remoteHead] : []; + }; + relayClient.publishEvent = async (evt) => { + publishCalls.push(evt); + }; + + // Use timer bed so the 2s click debounce does not fire unexpectedly. + // retryReconnectPublish cancels the debounce and calls startCycle directly, + // so no timer needs to be fired after the reconnect — just drain microtasks. + const { fireDelay, restore: restoreTimers } = makeHookTimerBed(); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + + // Seed local store so the hook mounts with 500 entries + window.localStorage.setItem( + storageKey(pubkey, relayUrl), + JSON.stringify({ version: 1, channels: localChannels }), + ); + + let hook = null; + try { + // Mount and let bootstrap settle + await act(async () => { + hook = renderHook(() => useHook(pubkey, relayUrl)); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + + // Click the channel — registers it in pendingPreservedKey, schedules 2s debounce + await act(async () => hook.result.current[trueAction](clickedId)); + + // Trigger the registered reconnect callback (exercises the real production seam). + // The callback: fetchRemote*() → retryReconnect*Publish() → startCycle() → doPublish(). + // retryReconnectPublish cancels the debounce and drives the cycle directly. + assert.ok( + typeof reconnect.cb === "function", + "subscribeToReconnects must register a callback", + ); + fetchCalls = 0; + publishCalls.length = 0; + await act(async () => { + // Drive the reconnect callback's async chain: fetchRemote → retryReconnect. + const p = reconnect.cb(); + // Drain microtasks to let fetchRemote resolve and retryReconnectPublish run. + for (let i = 0; i < 100; i++) await Promise.resolve(); + await p; + // Drain doPublish's internal async chain: fetchOwn → publishEvent → confirm. + for (let i = 0; i < 100; i++) await Promise.resolve(); + }); + + assert.ok( + publishCalls.length > 0, + "publish must have fired after reconnect", + ); + const plaintext = tauri.capturedPlaintext(); + assert.ok(plaintext !== null, "encrypt must have been called"); + const published = JSON.parse(plaintext); + assert.ok( + clickedId in published.channels, + `clicked channel "${clickedId}" must survive 501-entry merge after reconnect — ` + + `reverting hook to publish*(pending) resets pendingPreservedKey and fails this`, + ); + + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + tauri.restore(); + restoreTimers(); + restore(); + window.localStorage.clear(); + } + }); + + // P3 restart: after a quit the prior window's outbox record is foreign (the + // session nonce is gone). Bootstrap must recover the preservedKey from the + // foreign record and forward it to publishStars/publishMutes. Without the fix + // (readOutboxPreservedKey only finding own records), preservedKey is undefined + // and the 501-entry pre-publish merge evicts the clicked channel. + // + // Failing mutations: + // (a) Revert to readOutboxPreservedKey (isOwn filter): returns undefined for + // the foreign record → publish with no key → clicked channel evicted. + // (b) Drop bootstrap's preservedKey forward (publish(outbox, undefined)): + // same eviction even if key was recovered. + test(`${label}: P3 restart — foreign-nonce outbox with preservedKey is recovered on bootstrap; clicked channel survives 501-entry merge`, async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + + const MAX = MAX_ENTRIES; + const clickedId = `ch-restart-click-${label}`; + const relayUrl = "wss://relay-p3-restart"; + const pubkey = `pk-p3-restart-${label}`; + const encodedRelay = encodeURIComponent(relayUrl); + + // Local: clicked channel (oldest updatedAt=1) + 499 others at updatedAt=100 + const localChannels = { + [clickedId]: { [entryValueField]: true, updatedAt: 1, rev: 1 }, + }; + for (let i = 0; i < MAX - 1; i++) { + localChannels[`ch-rs-${i}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + } + // Remote: same 499 + one fresh channel → merged = 501 + const remoteChannels = {}; + for (let i = 0; i < MAX - 1; i++) { + remoteChannels[`ch-rs-${i}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + } + remoteChannels[`ch-rs-new-${label}`] = { + [entryValueField]: false, + updatedAt: 100, + rev: 0, + }; + + // Seed local store + window.localStorage.setItem( + storageKey(pubkey, relayUrl), + JSON.stringify({ version: 1, channels: localChannels }), + ); + + // Seed a FOREIGN-NONCE outbox envelope (models the prior window's record after quit). + // The nonce "foreign-nonce-xyz" won't match this window's outboxWindowNonce(), + // so enumerateOutbox finds it as a non-own record. The envelope carries preservedKey. + const foreignKey = `${outboxKeyPrefix}:${pubkey}:${encodedRelay}:foreign-nonce-xyz:0000`; + const foreignEnvelope = JSON.stringify({ + store: { version: 1, channels: localChannels }, + queuedAt: 1000, + preservedKey: clickedId, + }); + window.localStorage.setItem(foreignKey, foreignEnvelope); + + const restore = stubRelay(relayClient); + const tauri = installEchoTauri(pubkey); + const remoteHead = tauri.mintHead( + { version: 1, channels: remoteChannels }, + 50, + `evt-rs-${label}`, + ); + remoteHead.tags = [["d", dTag]]; + remoteHead.pubkey = pubkey; + remoteHead.kind = 30078; + + const publishCalls = []; + let fetchCalls = 0; + relayClient.fetchEvents = async () => { + fetchCalls++; + return fetchCalls === 1 ? [remoteHead] : []; + }; + relayClient.publishEvent = async (evt) => { + publishCalls.push(evt); + }; + + // Use the timer bed so the 2s debounce is captured; fire it to trigger + // the bootstrap publish cycle. + const { fireDelay, restore: restoreTimers } = makeHookTimerBed(); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + + let hook = null; + try { + // Mount and drive bootstrap (outbox replay calls publish, schedules debounce) + await act(async () => { + hook = renderHook(() => useHook(pubkey, relayUrl)); + for (let i = 0; i < 40; i++) await Promise.resolve(); + }); + + // Fire the 2s debounce to trigger the publish cycle + await fireDelay(2000); + // Drain doPublish's internal chain (fetchOwn → publishEvent → confirm) + for (let i = 0; i < 100; i++) await Promise.resolve(); + + assert.ok( + publishCalls.length > 0, + "bootstrap must trigger a publish for the resumed outbox", + ); + const plaintext = tauri.capturedPlaintext(); + assert.ok(plaintext !== null, "encrypt must have been called"); + const published = JSON.parse(plaintext); + assert.ok( + clickedId in published.channels, + `clicked channel "${clickedId}" must survive 501-entry merge after restart — ` + + `readOutboxPreservedKey with isOwn filter returns undefined for foreign record and fails this`, + ); + + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + tauri.restore(); + restoreTimers(); + restore(); + window.localStorage.clear(); + } + }); } diff --git a/desktop/src/features/sidebar/lib/mergeLaneStorage.shared.ts b/desktop/src/features/sidebar/lib/mergeLaneStorage.shared.ts index 58db6f07e7..75851baf62 100644 --- a/desktop/src/features/sidebar/lib/mergeLaneStorage.shared.ts +++ b/desktop/src/features/sidebar/lib/mergeLaneStorage.shared.ts @@ -16,6 +16,11 @@ import { writeOwnOutbox, } from "./sidebarSyncWatermark"; +export type OutboxWithMeta = { + store: S; + preservedKey?: string; +}; + // ─── Key helpers ────────────────────────────────────────────────────────────── /** @@ -172,17 +177,30 @@ export function writeOutbox( /** * Merge every window's persisted unpublished edit into one store for resume, - * or null when none exists. Per-entry merge is order-independent so two - * windows' concurrent clicks on different channels both survive. + * returning both the merged store and the most-recent explicit `preservedKey` + * across ALL records (own and foreign), or null when no records exist. + * + * The preserved key is selected deterministically: the record with the highest + * `queuedAt` that carries an explicit `preservedKey`; ties broken by key + * string (max). This works whether the record was written by the current window + * (isOwn) or by a prior window that has since closed — after a quit the nonce + * is gone and the record is foreign, but its `preservedKey` is still durable + * in localStorage and is recovered here (Kalvin P3 restart durability). + * + * Per-entry merge is order-independent so two windows' concurrent clicks on + * different channels both survive. Both the fold and the bound apply the + * selected preserved key so the clicked channel is never evicted before it + * reaches the manager. */ -export function readOutbox( +export function readOutboxWithMeta( outboxPrefix: string, pubkey: string, relayUrl: string, parse: (json: unknown) => S | null, merge: (a: S, b: S) => S, defaultStore: S, -): S | null { + bound: (s: S, preservedKey?: string) => S, +): OutboxWithMeta | null { const records = enumerateOutbox( outboxPrefix, legacyOutboxKey(outboxPrefix, pubkey, relayUrl), @@ -191,7 +209,29 @@ export function readOutbox( parse, ); if (records.length === 0) return null; - return records.reduce((acc, r) => merge(acc, r.store), defaultStore); + // Select the surviving preservedKey: max queuedAt among records that carry + // an explicit preservedKey; ties broken by key string (max). + let bestKey: string | undefined; + let bestQueuedAt = -1; + let bestStorageKey = ""; + for (const r of records) { + if (r.preservedKey === undefined) continue; + if ( + r.queuedAt > bestQueuedAt || + (r.queuedAt === bestQueuedAt && r.key > bestStorageKey) + ) { + bestKey = r.preservedKey; + bestQueuedAt = r.queuedAt; + bestStorageKey = r.key; + } + } + // Fold all records and apply the capacity bound with the selected key so the + // clicked channel is never evicted during the outbox read itself. + const merged = records.reduce( + (acc, r) => merge(acc, r.store), + defaultStore, + ); + return { store: bound(merged, bestKey), preservedKey: bestKey }; } /** Clear this window's own outbox key (its edit published or is a no-op). */ @@ -203,34 +243,6 @@ export function clearOutbox( clearOwnOutbox(outboxPrefix, pubkey, relayUrl); } -/** - * Read the `preservedKey` stored by the most recent click in THIS window's own - * outbox record, or `undefined` when no own record exists or it carries none. - * - * The `preservedKey` identifies the channel that must survive capacity bounding - * across a remount. It is intentionally read from this window's own record - * only — merging multiple windows' records into one store loses the per-click - * provenance, so we recover protection from the authoritative own-window entry. - */ -export function readOutboxPreservedKey( - outboxPrefix: string, - pubkey: string, - relayUrl: string, - parseStore: (json: unknown) => unknown, -): string | undefined { - const records = enumerateOutbox( - outboxPrefix, - legacyOutboxKey(outboxPrefix, pubkey, relayUrl), - pubkey, - relayUrl, - parseStore, - ); - // Return the preservedKey from this window's own record (if any). Own records - // are the only ones that carry a preservedKey written by this session. - const own = records.find((r) => r.isOwn); - return own?.preservedKey; -} - /** * Reclaim foreign outbox keys the fetched relay head already subsumes: a * record is redundant when merging it into `head` yields `head` unchanged. diff --git a/desktop/src/features/sidebar/lib/mergeLaneSyncManager.ts b/desktop/src/features/sidebar/lib/mergeLaneSyncManager.ts index 7008dd7d4b..309e1509cd 100644 --- a/desktop/src/features/sidebar/lib/mergeLaneSyncManager.ts +++ b/desktop/src/features/sidebar/lib/mergeLaneSyncManager.ts @@ -368,6 +368,7 @@ export class MergeLaneSyncManager { private discardPending(gen: number): void { if (gen !== this.pendingGeneration) return; this.pendingStore = null; + this.pendingPreservedKey = undefined; this.config.clearOutbox(this.pubkey, this.relayUrl); } diff --git a/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs b/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs index 579b6ef049..db7855a46b 100644 --- a/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs +++ b/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs @@ -87,7 +87,7 @@ test("(i+iii+ix) reclaim deletes proven-stale key and keeps coexisting fresh edi assert.ok(!ls.has(stale), "proven-stale key reclaimed"); assert.ok(ls.has(fresh), "fresh edit survives"); assert.deepEqual( - stars.readChannelStarsOutbox(PK, RELAY).channels.z, + stars.readChannelStarsOutboxWithMeta(PK, RELAY).store.channels.z, starEntry(true, 300, 9), ); }); @@ -144,9 +144,9 @@ test("(ii) merge-lane and whole-blob resume across window teardown/remount", () starStore({ b: starEntry(true, 200, 1) }), 200, ); - const resumed = stars.readChannelStarsOutbox(PK, RELAY); - assert.deepEqual(resumed.channels.a, starEntry(true, 100, 1)); - assert.deepEqual(resumed.channels.b, starEntry(true, 200, 1)); + const resumed = stars.readChannelStarsOutboxWithMeta(PK, RELAY); + assert.deepEqual(resumed.store.channels.a, starEntry(true, 100, 1)); + assert.deepEqual(resumed.store.channels.b, starEntry(true, 200, 1)); writeAt( ls, foreignKey("sort", "A", 0), @@ -172,7 +172,7 @@ test("(iv) stars: legacy shared key resumes and is never reclaimed", () => JSON.stringify(starStore({ a: starEntry(true, 100, 1) })), ); assert.deepEqual( - stars.readChannelStarsOutbox(PK, RELAY).channels.a, + stars.readChannelStarsOutboxWithMeta(PK, RELAY).store.channels.a, starEntry(true, 100, 1), "legacy entry resumes", ); @@ -202,9 +202,9 @@ test("(v) crash between write-new/delete-old: merge-lane coalesces, whole-blob r starStore({ b: starEntry(true, 200, 1) }), 200, ); - const resumed = stars.readChannelStarsOutbox(PK, RELAY); - assert.deepEqual(resumed.channels.a, starEntry(true, 100, 1)); - assert.deepEqual(resumed.channels.b, starEntry(true, 200, 1)); + const resumed = stars.readChannelStarsOutboxWithMeta(PK, RELAY); + assert.deepEqual(resumed.store.channels.a, starEntry(true, 100, 1)); + assert.deepEqual(resumed.store.channels.b, starEntry(true, 200, 1)); }); withStorage((ls) => { const base = `${PREFIX.sort}:${SCOPE}:${outboxWindowNonce()}`; @@ -243,7 +243,7 @@ test("(vi-viii) reload seq above surviving keys; whole-blob key tiebreak; merge- "new key strictly above surviving", ); assert.deepEqual( - stars.readChannelStarsOutbox(PK, RELAY).channels.b, + stars.readChannelStarsOutboxWithMeta(PK, RELAY).store.channels.b, starEntry(true, 100, 1), ); }); @@ -279,7 +279,7 @@ test("(vi-viii) reload seq above surviving keys; whole-blob key tiebreak; merge- 200, ); assert.deepEqual( - stars.readChannelStarsOutbox(PK, RELAY).channels.c, + stars.readChannelStarsOutboxWithMeta(PK, RELAY).store.channels.c, starEntry(true, 100, 5), "higher rev wins", ); @@ -360,16 +360,16 @@ test("(x) legacy outbox: replay-once, rewritten-blob replay, crash-recovery, and legacyKey("stars"), JSON.stringify(starStore({ a: starEntry(true, 100, 1) })), ); - const outbox = stars.readChannelStarsOutbox(PK, RELAY); + const outboxMeta = stars.readChannelStarsOutboxWithMeta(PK, RELAY); assert.ok( stars.isStarsStoreSubsumedBy( - outbox, + outboxMeta.store, starStore({ a: starEntry(true, 100, 1) }), ), "head-subsumed legacy fold is publish-free", ); assert.ok( - !stars.isStarsStoreSubsumedBy(outbox, starStore({})), + !stars.isStarsStoreSubsumedBy(outboxMeta.store, starStore({})), "unsubsumed legacy click still needs a publish", ); }); diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.ts b/desktop/src/features/sidebar/lib/useChannelMutes.ts index ab4c2239aa..d0c0cdc9bf 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.ts +++ b/desktop/src/features/sidebar/lib/useChannelMutes.ts @@ -8,8 +8,7 @@ import { isMutesStoreSubsumedBy, mergeStores, mutedChannelIdsFromStore, - readChannelMutesOutbox, - readChannelMutesOutboxPreservedKey, + readChannelMutesOutboxWithMeta, readChannelMutesStore, reclaimSubsumedMutesOutbox, storageKey, @@ -115,8 +114,8 @@ export function useChannelMutes( // quit/community-switch so a click made <2s before teardown still syncs. // Replay runs BEFORE reclamation so a same-second record the head appears // to supersede is consumed into pending here and can never be GC'd out. - const outbox = readChannelMutesOutbox(pubkey, relayUrl); - if (outbox) { + const outboxMeta = readChannelMutesOutboxWithMeta(pubkey, relayUrl); + if (outboxMeta) { // Skip the publish only when the fetched head already subsumes the // fold — a lingering never-deleted legacy key or head-subsumed record // would otherwise re-drive an identical publish on every boot. A `hold` @@ -125,17 +124,17 @@ export function useChannelMutes( // noise only. const subsumed = result.action === "apply-remote" && - isMutesStoreSubsumedBy(outbox, result.data.store); + isMutesStoreSubsumedBy(outboxMeta.store, result.data.store); if (!subsumed) { - // Restore the preservedKey written alongside this window's outbox - // record so the clicked channel's capacity reservation survives - // remount (Kalvin P3). Old records without the field produce - // undefined, which publishes without a reservation — same as before. - const preservedKey = readChannelMutesOutboxPreservedKey( - pubkey, - relayUrl, + // Forward the preserved key so the clicked channel's capacity + // reservation survives remount and restart. The key is selected + // from all records (own and foreign) by max queuedAt, so it is + // recovered even when the prior window's record is now foreign + // after a quit (Kalvin P3). + managerRef.current?.publishMutes( + outboxMeta.store, + outboxMeta.preservedKey, ); - managerRef.current?.publishMutes(outbox, preservedKey); } } else { clearChannelMutesOutbox(pubkey, relayUrl); diff --git a/desktop/src/features/sidebar/lib/useChannelStars.ts b/desktop/src/features/sidebar/lib/useChannelStars.ts index 91857be1f8..f60ec92e82 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.ts +++ b/desktop/src/features/sidebar/lib/useChannelStars.ts @@ -7,8 +7,7 @@ import { DEFAULT_STORE, isStarsStoreSubsumedBy, mergeStores, - readChannelStarsOutbox, - readChannelStarsOutboxPreservedKey, + readChannelStarsOutboxWithMeta, readChannelStarsStore, reclaimSubsumedStarsOutbox, starredChannelIdsFromStore, @@ -116,8 +115,8 @@ export function useChannelStars( // syncs. Merges all records (order-independent), then publishes. Replay // runs BEFORE reclamation so a same-second record the head appears to // supersede is consumed into pending here and can never be GC'd out. - const outbox = readChannelStarsOutbox(pubkey, relayUrl); - if (outbox) { + const outboxMeta = readChannelStarsOutboxWithMeta(pubkey, relayUrl); + if (outboxMeta) { // Skip the publish only when the fetched head already subsumes the // fold — a lingering never-deleted legacy key or head-subsumed record // would otherwise re-drive an identical publish on every boot. A `hold` @@ -126,17 +125,17 @@ export function useChannelStars( // noise only. const subsumed = result.action === "apply-remote" && - isStarsStoreSubsumedBy(outbox, result.data.store); + isStarsStoreSubsumedBy(outboxMeta.store, result.data.store); if (!subsumed) { - // Restore the preservedKey written alongside this window's outbox - // record so the clicked channel's capacity reservation survives - // remount (Kalvin P3). Old records without the field produce - // undefined, which publishes without a reservation — same as before. - const preservedKey = readChannelStarsOutboxPreservedKey( - pubkey, - relayUrl, + // Forward the preserved key so the clicked channel's capacity + // reservation survives remount and restart. The key is selected + // from all records (own and foreign) by max queuedAt, so it is + // recovered even when the prior window's record is now foreign + // after a quit (Kalvin P3). + managerRef.current?.publishStars( + outboxMeta.store, + outboxMeta.preservedKey, ); - managerRef.current?.publishStars(outbox, preservedKey); } } else { clearChannelStarsOutbox(pubkey, relayUrl);