diff --git a/desktop/src/features/sidebar/lib/channelSectionsStorage.ts b/desktop/src/features/sidebar/lib/channelSectionsStorage.ts index 80fb2ba871..633c56d775 100644 --- a/desktop/src/features/sidebar/lib/channelSectionsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelSectionsStorage.ts @@ -257,13 +257,15 @@ export function clearChannelSectionsOutbox( } /** - * Reclaim foreign outbox keys the relay head itself supersedes: a whole-blob - * record queued at or before the durable head's `created_at` (`queuedAt` ≤ - * `headCreatedAt`) lost LWW to a blob the relay already holds, so dropping it - * matches the relay's own resolution. A record queued AFTER the head is live - * intent the head does not yet reflect and is kept — never reclaimed merely for - * losing a local max-`queuedAt` comparison. Never touches this window's own key - * and rechecks each key before removing. Call only after a successful fetch. + * Reclaim foreign outbox keys the relay head itself STRICTLY supersedes: a + * whole-blob record queued strictly before the durable head's `created_at` + * (`queuedAt` < `headCreatedAt`) lost LWW to a blob the relay already holds, so + * dropping it matches the relay's own resolution. A same-second record + * (`queuedAt` == `headCreatedAt`) is kept — one-second clock granularity cannot + * prove it lost, so it drains only when a strictly-newer head lands. A record + * queued after the head is live intent and is likewise kept. Records are + * write-once so the delete needs no recheck; never touches this window's own + * keys or the legacy shared key. Call only after a successful fetch. */ export function reclaimSupersededSectionsOutbox( pubkey: string, @@ -276,6 +278,6 @@ export function reclaimSupersededSectionsOutbox( pubkey, relayUrl, parseChannelSectionPayload, - (record) => record.queuedAt <= headCreatedAt, + (record) => record.queuedAt < headCreatedAt, ); } diff --git a/desktop/src/features/sidebar/lib/channelSortPreference.ts b/desktop/src/features/sidebar/lib/channelSortPreference.ts index c1fadb620e..db0a49be68 100644 --- a/desktop/src/features/sidebar/lib/channelSortPreference.ts +++ b/desktop/src/features/sidebar/lib/channelSortPreference.ts @@ -192,13 +192,15 @@ export function clearChannelSortOutbox(pubkey: string, relayUrl: string): void { } /** - * Reclaim foreign outbox keys the relay head itself supersedes: a whole-blob - * record queued at or before the durable head's `created_at` (`queuedAt` ≤ - * `headCreatedAt`) lost LWW to a blob the relay already holds, so dropping it - * matches the relay's own resolution. A record queued AFTER the head is live - * intent the head does not yet reflect and is kept — never reclaimed merely for - * losing a local max-`queuedAt` comparison. Never touches this window's own key - * and rechecks each key before removing. Call only after a successful fetch. + * Reclaim foreign outbox keys the relay head itself STRICTLY supersedes: a + * whole-blob record queued strictly before the durable head's `created_at` + * (`queuedAt` < `headCreatedAt`) lost LWW to a blob the relay already holds, so + * dropping it matches the relay's own resolution. A same-second record + * (`queuedAt` == `headCreatedAt`) is kept — one-second clock granularity cannot + * prove it lost, so it drains only when a strictly-newer head lands. A record + * queued after the head is live intent and is likewise kept. Records are + * write-once so the delete needs no recheck; never touches this window's own + * keys or the legacy shared key. Call only after a successful fetch. */ export function reclaimSupersededSortOutbox( pubkey: string, @@ -211,7 +213,7 @@ export function reclaimSupersededSortOutbox( pubkey, relayUrl, parseChannelSortPayload, - (record) => record.queuedAt <= headCreatedAt, + (record) => record.queuedAt < headCreatedAt, ); } diff --git a/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs b/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs index 878a15682e..25c5dddc20 100644 --- a/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs +++ b/desktop/src/features/sidebar/lib/multiWindowOutbox.test.mjs @@ -5,21 +5,25 @@ import test from "node:test"; // // All four sidebar-sync lanes persist an unpublished edit so it survives a // quit/community-switch inside the 2s publish debounce. localStorage offers no -// atomic compare-and-delete or transactional read-modify-write, so a single key -// shared across windows can never be mutated safely: one window's read→write or -// read→remove races a peer's write and drops its still-unpublished edit. +// atomic compare-and-delete or transactional read-modify-write, so neither a +// single shared key nor a per-window key a window OVERWRITES can be reclaimed +// by a peer safely: the value can change between the reclaim decision-read and +// the delete (the recheck race a byte-compare narrows but cannot close). // -// The fix keys the outbox PER WINDOW: `:::`, where -// the nonce is stable per window (sessionStorage). Each window is the sole -// writer of its own key, so a hot-path write is one unconditional `setItem` — no -// read, no shared-key contention (prong b is designed out, not guarded). Resume -// enumerates ALL windows' keys: merge lanes (stars/mutes) fold every record -// (order-independent); whole-blob lanes (sort/sections) replay the max-`queuedAt` -// record, ties broken by key. Redundant FOREIGN keys are reclaimed at boot, -// gated on durable relay evidence and re-read immediately before removal so a -// live peer's fresh write in the recheck gap is never destroyed. A window never -// removes another window's key on the hot path and never touches its own key -// during reclamation. +// The fix keys the outbox per window AND write-once: +// `::::`, where the nonce is stable per +// window (sessionStorage) and `seq` is a per-window monotonic counter. A window +// NEVER rewrites a key: a new edit writes a NEW key, then deletes its own older +// keys (write-before-delete, so a crash leaves ≥1 record, never zero). Because +// records are immutable, a booting peer that proves a foreign key reclaimable +// against durable relay evidence can delete it with no recheck — nothing can +// have changed at that key since the proof. Resume enumerates ALL windows' +// keys: merge lanes (stars/mutes) fold every record (order-independent); +// whole-blob lanes (sort/sections) replay the max-`queuedAt` record, ties broken +// by key. Reclamation runs AFTER replay so a same-second record the head appears +// to supersede is consumed into pending first. Whole-blob supersession is STRICT +// (`queuedAt` < head `created_at`) so a same-second record is never dropped, and +// the legacy v1 shared key is never deleted by v2 (it is mutable; only replayed). // // This matrix drives the shared helpers directly through a mock Storage seam — // no relay, no timers — so each interleaving is deterministic. The seam mocks @@ -102,8 +106,12 @@ const PREFIX = { }; // A foreign window's key: same (pubkey, relay) scope, a different nonce than -// this process's own. `legacyKey` is the pre-per-window shared key (no nonce). -const foreignKey = (lane, nonce) => `${PREFIX[lane]}:${SCOPE}:${nonce}`; +// this process's own, plus a zero-padded `seq`. `legacyKey` is the +// pre-per-window shared key (no nonce/seq). +const SEQ_WIDTH = 12; +const pad = (seq) => String(seq).padStart(SEQ_WIDTH, "0"); +const foreignKey = (lane, nonce, seq) => + `${PREFIX[lane]}:${SCOPE}:${nonce}:${pad(seq)}`; const legacyKey = (lane) => `${PREFIX[lane]}:${SCOPE}`; const writeAt = (ls, key, store, queuedAt) => ls.setItem(key, JSON.stringify({ store, queuedAt })); @@ -119,56 +127,64 @@ const sectionStore = (secs, assignments = {}) => ({ assignments, }); -// ── (i) Recheck skips a foreign write injected between decision-read and delete ─ +// ── (i) Reclaim never deletes a foreign edit written in the decision→delete gap ─ // -// reclaimOutbox re-reads each foreign key immediately before removing it and -// skips when the value changed since the reclaim decision. A peer that rewrote -// its key in that gap owns a fresh edit that must survive. Merge lane (stars): -// the head subsumes the enumerated value, so reclaim would fire — but the peer's -// interposed write must be preserved. +// Records are write-once: an owner's fresh edit lands on a NEW key, never a +// rewrite of the enumerated one. reclaimOutbox proves the enumerated (stale) +// key reclaimable and deletes it; a peer edit injected during enumeration lives +// under its own key and is evaluated on its own merits (kept when the head does +// not subsume/supersede it). We interpose that fresh write via the read-mutate +// seam to prove GC removes only the proven-stale key and never the fresh one. -test("(i) stars: reclaim recheck skips a foreign key rewritten in the decision→delete gap", () => { +test("(i) stars: reclaim deletes the proven-stale key and keeps a fresh edit written in the gap", () => { withStorage((ls) => { - const key = foreignKey("stars", "peerB"); + const stale = foreignKey("stars", "peerB", 0); // Foreign edit the fetched head has already absorbed → decision = reclaim. - writeAt(ls, key, starStore({ a: starEntry(true, 100, 1) }), 100); - // Between the enumerate read (#1) and the recheck read (#2), the peer writes - // a FRESH edit the head does not reflect. - ls.onReadMutate(key, 1, (map) => + writeAt(ls, stale, starStore({ a: starEntry(true, 100, 1) }), 100); + // During enumeration (read of the stale key), the peer commits a FRESH edit + // to a NEW key the head does not reflect — the write-once analogue of the + // owner rewriting in the gap. + const fresh = foreignKey("stars", "peerB", 1); + ls.onReadMutate(stale, 1, (map) => map.set( - key, + fresh, JSON.stringify({ - store: starStore({ a: starEntry(false, 300, 9) }), + store: starStore({ z: starEntry(true, 300, 9) }), queuedAt: 300, }), ), ); - // Head carries `a` at (100,1) — subsumes the enumerated value. + // Head carries `a` at (100,1) — subsumes the stale value, not the fresh one. stars.reclaimSubsumedStarsOutbox( PK, RELAY, starStore({ a: starEntry(true, 100, 1) }), ); - assert.ok(ls.has(key), "peer's fresh edit must survive the recheck"); - const survived = stars.readChannelStarsOutbox(PK, RELAY); - assert.deepEqual(survived.channels.a, starEntry(false, 300, 9)); + assert.ok(!ls.has(stale), "proven-stale key is reclaimed"); + assert.ok(ls.has(fresh), "peer's fresh edit under a new key survives"); + assert.deepEqual( + stars.readChannelStarsOutbox(PK, RELAY).channels.z, + starEntry(true, 300, 9), + ); }); }); -test("(i) sort: reclaim recheck skips a foreign key rewritten in the decision→delete gap", () => { +test("(i) sort: reclaim deletes the proven-stale key and keeps a fresh edit written in the gap", () => { withStorage((ls) => { - const key = foreignKey("sort", "peerB"); - writeAt(ls, key, sortStore({ dms: "alpha" }), 100); - ls.onReadMutate(key, 1, (map) => + const stale = foreignKey("sort", "peerB", 0); + writeAt(ls, stale, sortStore({ dms: "alpha" }), 100); + const fresh = foreignKey("sort", "peerB", 1); + ls.onReadMutate(stale, 1, (map) => map.set( - key, + fresh, JSON.stringify({ store: sortStore({ dms: "recent" }), queuedAt: 500 }), ), ); - // Head created_at 200 supersedes the queuedAt=100 decision, but not the - // interposed queuedAt=500 write. + // Head created_at 200 strictly supersedes the queuedAt=100 stale record, + // not the interposed queuedAt=500 fresh key. sort.reclaimSupersededSortOutbox(PK, RELAY, 200); - assert.ok(ls.has(key), "peer's fresh edit must survive the recheck"); + assert.ok(!ls.has(stale), "proven-stale key is reclaimed"); + assert.ok(ls.has(fresh), "peer's fresh edit under a new key survives"); assert.equal(sort.readChannelSortOutbox(PK, RELAY).groups.dms, "recent"); }); }); @@ -183,13 +199,13 @@ test("(ii) stars (merge): both windows' distinct-channel edits resume", () => { withStorage((ls) => { writeAt( ls, - foreignKey("stars", "A"), + foreignKey("stars", "A", 0), starStore({ a: starEntry(true, 100, 1) }), 100, ); writeAt( ls, - foreignKey("stars", "B"), + foreignKey("stars", "B", 0), starStore({ b: starEntry(true, 200, 1) }), 200, ); @@ -203,13 +219,13 @@ test("(ii) mutes (merge): both windows' distinct-channel edits resume", () => { withStorage((ls) => { writeAt( ls, - foreignKey("mutes", "A"), + foreignKey("mutes", "A", 0), muteStore({ a: muteEntry(true, 100, 1) }), 100, ); writeAt( ls, - foreignKey("mutes", "B"), + foreignKey("mutes", "B", 0), muteStore({ b: muteEntry(true, 200, 1) }), 200, ); @@ -221,10 +237,15 @@ test("(ii) mutes (merge): both windows' distinct-channel edits resume", () => { test("(ii) sort (whole-blob): the newest queued window resumes; older is LWW-superseded", () => { withStorage((ls) => { - writeAt(ls, foreignKey("sort", "A"), sortStore({ channels: "alpha" }), 100); writeAt( ls, - foreignKey("sort", "B"), + foreignKey("sort", "A", 0), + sortStore({ channels: "alpha" }), + 100, + ); + writeAt( + ls, + foreignKey("sort", "B", 0), sortStore({ channels: "recent" }), 200, ); @@ -239,13 +260,13 @@ test("(ii) sections (whole-blob): the newest queued window resumes; older is LWW withStorage((ls) => { writeAt( ls, - foreignKey("sections", "A"), + foreignKey("sections", "A", 0), sectionStore([{ id: "s1", name: "One", order: 0 }]), 100, ); writeAt( ls, - foreignKey("sections", "B"), + foreignKey("sections", "B", 0), sectionStore([{ id: "s2", name: "Two", order: 0 }]), 200, ); @@ -255,13 +276,54 @@ test("(ii) sections (whole-blob): the newest queued window resumes; older is LWW }); }); -// ── (iii) Legacy v1 shared key: replays, then reclaimed only by relay gating ─── +// ── (iii) Same-second whole-blob: head at t keeps a record queued at t ───────── // -// A pre-per-window build wrote one shared key. It enumerates as one more record -// (queuedAt 0 for a bare store), resumes, and is reclaimed by the same relay- -// gated rule — never dropped on replay alone (mixed dev/DMG fleet residual). +// One-second clock granularity means a record queued in the same second as the +// head cannot be proven to have lost LWW. Strict supersession (`queuedAt` < +// head `created_at`) keeps it; a strictly-earlier record is reclaimed. -test("(iii) stars: legacy shared key resumes, then is reclaimed once the head subsumes it", () => { +test("(iii) sort: same-second record kept, strictly-earlier reclaimed", () => { + withStorage((ls) => { + const sameSecond = foreignKey("sort", "same", 0); + const earlier = foreignKey("sort", "old", 0); + writeAt(ls, sameSecond, sortStore({ dms: "recent" }), 100); + writeAt(ls, earlier, sortStore({ dms: "alpha" }), 99); + sort.reclaimSupersededSortOutbox(PK, RELAY, 100); + assert.ok(ls.has(sameSecond), "same-second record (queuedAt == head) kept"); + assert.ok(!ls.has(earlier), "strictly-earlier record reclaimed"); + }); +}); + +test("(iii) sections: same-second record kept, strictly-earlier reclaimed", () => { + withStorage((ls) => { + const sameSecond = foreignKey("sections", "same", 0); + const earlier = foreignKey("sections", "old", 0); + writeAt( + ls, + sameSecond, + sectionStore([{ id: "s2", name: "Two", order: 0 }]), + 100, + ); + writeAt( + ls, + earlier, + sectionStore([{ id: "s1", name: "One", order: 0 }]), + 99, + ); + sections.reclaimSupersededSectionsOutbox(PK, RELAY, 100); + assert.ok(ls.has(sameSecond), "same-second record (queuedAt == head) kept"); + assert.ok(!ls.has(earlier), "strictly-earlier record reclaimed"); + }); +}); + +// ── (iv) Legacy v1 shared key: replays, and is NEVER deleted by v2 ───────────── +// +// A pre-per-window build wrote one shared, MUTABLE key. v2 enumerates it as one +// more record (queuedAt 0 for a bare store) so it resumes, but never deletes it +// under any relay gating — a live old-build window may be rewriting it, and +// queuedAt 0 makes supersession meaningless (mixed dev/DMG fleet residual). + +test("(iv) stars: legacy shared key resumes and is never reclaimed", () => { withStorage((ls) => { // Legacy bare store (no envelope) from a pre-per-window build. ls.setItem( @@ -274,27 +336,20 @@ test("(iii) stars: legacy shared key resumes, then is reclaimed once the head su starEntry(true, 100, 1), "legacy entry resumes", ); - // A head that does NOT subsume it (older rev) keeps it. - stars.reclaimSubsumedStarsOutbox( - PK, - RELAY, - starStore({ a: starEntry(true, 50, 0) }), - ); - assert.ok(ls.has(legacyKey("stars")), "unsubsumed legacy entry is kept"); - // A head that subsumes it reclaims it. + // Even a head that fully subsumes it must not delete the mutable v1 key. stars.reclaimSubsumedStarsOutbox( PK, RELAY, starStore({ a: starEntry(true, 100, 1) }), ); assert.ok( - !ls.has(legacyKey("stars")), - "subsumed legacy entry is reclaimed", + ls.has(legacyKey("stars")), + "legacy v1 key is never deleted by v2", ); }); }); -test("(iii) sections: legacy shared key resumes, then is reclaimed once the head supersedes it", () => { +test("(iv) sections: legacy shared key resumes and is never reclaimed", () => { withStorage((ls) => { // Legacy entry as a {store, queuedAt} envelope from an interim build. writeAt( @@ -306,52 +361,138 @@ test("(iii) sections: legacy shared key resumes, then is reclaimed once the head assert.deepEqual(sections.readChannelSectionsOutbox(PK, RELAY).sections, [ { id: "s1", name: "One", order: 0 }, ]); - // Head created_at before the queued stamp keeps it (not yet superseded). - sections.reclaimSupersededSectionsOutbox(PK, RELAY, 50); + // A head strictly past the queued stamp still must not delete the v1 key. + sections.reclaimSupersededSectionsOutbox(PK, RELAY, 999); assert.ok( ls.has(legacyKey("sections")), - "un-superseded legacy entry is kept", - ); - // Head created_at at/after the queued stamp supersedes and reclaims it. - sections.reclaimSupersededSectionsOutbox(PK, RELAY, 100); - assert.ok( - !ls.has(legacyKey("sections")), - "superseded legacy entry is reclaimed", + "legacy v1 key is never deleted by v2", ); }); }); -// ── (iv) Whole-blob replay tie → deterministic nonce (key) tiebreak ──────────── +// ── (v) Owner crash between write-new and delete-old → both replay-coalesce ──── +// +// writeOwnOutbox writes the new key BEFORE deleting older own keys. A crash in +// that gap leaves two own records for the same window; replay coalesces them +// (merge fold / whole-blob max) with no loss and no duplicate publish. -test("(iv) sort: equal-queuedAt records resolve by key so replay is deterministic", () => { +test("(v) stars: two own records from a write-new/delete-old crash merge-coalesce", () => { withStorage((ls) => { - writeAt(ls, foreignKey("sort", "aaa"), sortStore({ forums: "alpha" }), 100); + const base = `${PREFIX.stars}:${SCOPE}:${outboxWindowNonce()}`; + // Simulate the crash residue: the pre-crash key (seq 0) plus the freshly + // written key (seq 1) both present. Merge must keep both channels. writeAt( ls, - foreignKey("sort", "zzz"), + `${base}:${pad(0)}`, + starStore({ a: starEntry(true, 100, 1) }), + 100, + ); + writeAt( + ls, + `${base}:${pad(1)}`, + 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)); + }); +}); + +test("(v) sort: two own records from a crash resume the newer seq (padded key order)", () => { + withStorage((ls) => { + const base = `${PREFIX.sort}:${SCOPE}:${outboxWindowNonce()}`; + // Same-second seqs crossing a digit boundary: unpadded, "9" > "10" + // lexically and would wrongly resume the OLDER edit. Zero-padding makes the + // higher seq win the whole-blob tiebreak. + writeAt(ls, `${base}:${pad(9)}`, sortStore({ dms: "alpha" }), 100); + writeAt(ls, `${base}:${pad(10)}`, sortStore({ dms: "recent" }), 100); + assert.equal( + sort.readChannelSortOutbox(PK, RELAY).groups.dms, + "recent", + "newer seq resumes despite the digit-boundary crossing", + ); + }); +}); + +// ── (vi) Reload seeds seq above surviving own keys → no key reuse/overwrite ──── +// +// After a reload the sessionStorage nonce survives but the in-memory seq counter +// restarts. A fresh write must allocate a seq ABOVE the max surviving own key so +// it never overwrites (and thus mutates) an existing immutable record. + +test("(vi) reload: a write after surviving own keys allocates a strictly-higher key", () => { + withStorage((ls) => { + const base = `${PREFIX.stars}:${SCOPE}:${outboxWindowNonce()}`; + // A surviving own key from before the (simulated) reload. + writeAt( + ls, + `${base}:${pad(5)}`, + starStore({ a: starEntry(true, 50, 1) }), + 50, + ); + // First write of the "new session" — seq counter cold, must seed above 5. + stars.writeChannelStarsOutbox( + PK, + starStore({ b: starEntry(true, 100, 1) }), + RELAY, + ); + // The pre-reload key must NOT have been overwritten; the new write lands on + // a strictly-higher key, and delete-old drops the seq-5 key. + const ownKeys = []; + for (let i = 0; i < ls.length; i++) { + const k = ls.key(i); + if (k?.startsWith(`${base}:`)) ownKeys.push(k); + } + assert.equal(ownKeys.length, 1, "delete-old leaves exactly one own key"); + assert.ok( + ownKeys[0] > `${base}:${pad(5)}`, + "new key seq is strictly above the surviving key", + ); + // No data loss: the newest edit resumes. + assert.deepEqual( + stars.readChannelStarsOutbox(PK, RELAY).channels.b, + starEntry(true, 100, 1), + ); + }); +}); + +// ── (vii) Whole-blob replay tie → deterministic nonce (key) tiebreak ─────────── + +test("(vii) sort: equal-queuedAt records resolve by key so replay is deterministic", () => { + withStorage((ls) => { + writeAt( + ls, + foreignKey("sort", "aaa", 0), + sortStore({ forums: "alpha" }), + 100, + ); + writeAt( + ls, + foreignKey("sort", "zzz", 0), sortStore({ forums: "recent" }), 100, ); - // Same queuedAt → the lexicographically-greater key wins (…:zzz). + // Same queuedAt → the lexicographically-greater key wins (…:zzz:…). assert.equal(sort.readChannelSortOutbox(PK, RELAY).groups.forums, "recent"); }); }); -// ── (v) Merge-lane replay is order-independent ───────────────────────────────── +// ── (viii) Merge-lane replay is order-independent ────────────────────────────── -test("(v) stars: same-channel records fold to the max entry regardless of key order", () => { +test("(viii) stars: same-channel records fold to the max entry regardless of key order", () => { withStorage((ls) => { // Lower-rev record under a lexicographically-greater key (enumerated later) // must still lose to the higher-rev record — merge is order-independent. writeAt( ls, - foreignKey("stars", "aaa"), + foreignKey("stars", "aaa", 0), starStore({ c: starEntry(true, 100, 5) }), 100, ); writeAt( ls, - foreignKey("stars", "zzz"), + foreignKey("stars", "zzz", 0), starStore({ c: starEntry(false, 100, 2) }), 200, ); @@ -364,7 +505,7 @@ test("(v) stars: same-channel records fold to the max entry regardless of key or }); }); -// ── (vi) GC no-op when the head subsumes/supersedes nothing ──────────────────── +// ── (ix) GC no-op when the head subsumes/supersedes nothing ──────────────────── // // The hook calls reclaim only inside the `apply-remote` branch, so a `failed` // head fetch (or `absent`) never invokes it — that guard is structural in the @@ -372,9 +513,9 @@ test("(v) stars: same-channel records fold to the max entry regardless of key or // subsumes/supersedes nothing removes nothing: a foreign edit newer than the // head is live intent and is kept, so a stale/empty head can never over-collect. -test("(vi) stars: a head that subsumes nothing reclaims nothing", () => { +test("(ix) stars: a head that subsumes nothing reclaims nothing", () => { withStorage((ls) => { - const key = foreignKey("stars", "B"); + const key = foreignKey("stars", "B", 0); writeAt(ls, key, starStore({ a: starEntry(true, 300, 2) }), 300); // Empty head subsumes no channel → keep everything. stars.reclaimSubsumedStarsOutbox(PK, RELAY, starStore({})); @@ -382,9 +523,9 @@ test("(vi) stars: a head that subsumes nothing reclaims nothing", () => { }); }); -test("(vi) sort: a head older than the queued edit supersedes nothing", () => { +test("(ix) sort: a head older than the queued edit supersedes nothing", () => { withStorage((ls) => { - const key = foreignKey("sort", "B"); + const key = foreignKey("sort", "B", 0); writeAt(ls, key, sortStore({ dms: "recent" }), 300); // headCreatedAt=0 (absent-equivalent) < queuedAt → keep. sort.reclaimSupersededSortOutbox(PK, RELAY, 0); @@ -394,26 +535,33 @@ test("(vi) sort: a head older than the queued edit supersedes nothing", () => { // ── Own-key round trip: write, read, clear (single-window baseline) ──────────── -test("own key: write resumes, clear removes only this window's own key", () => { +test("own key: write resumes, clear removes only this window's own keys", () => { withStorage((ls) => { stars.writeChannelStarsOutbox( PK, starStore({ a: starEntry(true, 100, 1) }), RELAY, ); - const ownKey = `${PREFIX.stars}:${SCOPE}:${outboxWindowNonce()}`; - assert.ok(ls.has(ownKey), "own edit is written under this window's nonce"); + const ownBase = `${PREFIX.stars}:${SCOPE}:${outboxWindowNonce()}`; + const hasOwn = () => { + for (let i = 0; i < ls.length; i++) { + const k = ls.key(i); + if (k?.startsWith(`${ownBase}:`)) return true; + } + return false; + }; + assert.ok(hasOwn(), "own edit is written under this window's nonce"); // A foreign peer key is untouched by an own-key clear. writeAt( ls, - foreignKey("stars", "peer"), + foreignKey("stars", "peer", 0), starStore({ z: starEntry(true, 9, 1) }), 9, ); stars.clearChannelStarsOutbox(PK, RELAY); - assert.ok(!ls.has(ownKey), "own key cleared"); + assert.ok(!hasOwn(), "own keys cleared"); assert.ok( - ls.has(foreignKey("stars", "peer")), + ls.has(foreignKey("stars", "peer", 0)), "foreign key untouched by own clear", ); }); diff --git a/desktop/src/features/sidebar/lib/sidebarSyncWatermark.ts b/desktop/src/features/sidebar/lib/sidebarSyncWatermark.ts index ebdd1aab90..8a7d602e5e 100644 --- a/desktop/src/features/sidebar/lib/sidebarSyncWatermark.ts +++ b/desktop/src/features/sidebar/lib/sidebarSyncWatermark.ts @@ -52,38 +52,42 @@ export function clampPublishCreatedAt( ); } -// ── Per-window durable outbox ──────────────────────────────────────────────── +// ── Per-window durable outbox (write-once, append-only) ────────────────────── // // Each sidebar-sync lane persists an unpublished edit so it survives a // quit/community-switch inside the 2s publish debounce. The durability boundary // is localStorage, which offers no atomic compare-and-delete or transactional -// read-modify-write, so a single key shared across every window can never be -// mutated safely: one window's read→write or read→remove can race a peer's -// write in the gap and drop its still-unpublished edit (the defect a per-write -// ownership token could narrow but not close). +// read-modify-write: a single key shared across windows can never be mutated +// safely, and even a per-window key a window OVERWRITES can change between a +// peer's reclaim decision-read and its delete (the recheck race a byte-compare +// narrows but cannot close). // -// The outbox is therefore keyed PER WINDOW: -// `:::`, where `nonce` is stable for one window's -// lifetime (parked in sessionStorage — survives reload, gone on window close). -// Each window is the ONLY writer of its own key, so a hot-path write is a single -// unconditional `setItem` with no read and no shared-key contention: the write -// race is designed out rather than guarded. +// The outbox is therefore keyed per window AND write-once. A key is +// `::::`, where `nonce` is stable for one +// window's lifetime (sessionStorage — survives reload, gone on window close) and +// `seq` is a per-window monotonic counter. A window NEVER rewrites a key: a new +// edit writes a NEW key (next `seq`) as a single unconditional `setItem`, then +// deletes its own now-superseded key(s). Both are own-key operations and the +// write precedes the delete, so a crash between them leaves ≥1 record for +// replay to coalesce, never zero. // -// A window clears only its own key once its publish completes. Redundant foreign -// keys (a peer that published then crashed, or an edit the relay has since -// absorbed) are reclaimed at boot, gated on durable relay evidence so a live -// peer's genuinely-unpublished edit is never destroyed: +// Because records are immutable, foreign reclamation is safe by construction: a +// booting window reads an immutable foreign record, proves it reclaimable +// against durable relay evidence, and deletes it. Nothing can have changed at +// that key since the proof — the only competing interleave is the owner +// deleting it first, and `removeItem` on an absent key is a no-op. No byte +// recheck and no destructive-path residual remain. // - merge lanes: delete a record the fetched relay head already subsumes. -// - whole-blob lanes: delete a record the head's own timestamp supersedes -// (`queuedAt` ≤ head `created_at`) — never one that -// merely loses a local comparison. -// Reclamation runs only when the head fetch succeeded, never on a failed fetch, -// and re-reads each foreign key immediately before removing it so a value that -// changed since the decision is left for its owner. This is not atomic — -// localStorage cannot be — but it narrows the residual to a recheck→remove gap -// that additionally requires the owner to quit before its own debounce publish -// (its in-memory pending re-drives otherwise). After any successful publish the -// relay head is newest, so foreign keys still drain to zero across boots. +// - whole-blob lanes: delete a record the head STRICTLY supersedes +// (`queuedAt` < head `created_at`); a same-second record +// is kept until a strictly-newer head lands. +// Reclamation runs only after a successful head fetch, never touches this +// window's own keys, and never touches the legacy v1 shared key — that key is +// mutable (a live old-build window may be rewriting it) and its `queuedAt=0` +// makes supersession meaningless, so no gating makes deleting it safe; v2 only +// ever replays it. The bounded cost is at most one lingering legacy key per lane +// per (pubkey, relay), and only when the last old-build session quit with an +// unpublished edit. const OUTBOX_WINDOW_NONCE_KEY = "buzz-sidebar-outbox-window.v1"; @@ -100,7 +104,7 @@ function mintNonce(): string { /** * The stable per-window nonce that scopes this window's outbox keys. Minted - * once and parked in sessionStorage so a reload re-owns the same key while a + * once and parked in sessionStorage so a reload re-owns the same keys while a * new window gets its own; an unavailable sessionStorage (private mode, test * harness without one) falls back to a process-lifetime in-memory nonce. */ @@ -126,12 +130,11 @@ function scopeSuffix(pubkey: string, relayUrl: string): string { return `${pubkey}:${encodeURIComponent(normalizeRelayUrl(relayUrl))}`; } -/** This window's own outbox key — the only key it ever writes or removes. */ -export function ownOutboxKey( - prefix: string, - pubkey: string, - relayUrl: string, -): string { +/** + * The base every own key for this (lane, pubkey, relay, window) shares: + * `:::`. Own keys append `:`. + */ +function ownKeyBase(prefix: string, pubkey: string, relayUrl: string): string { return `${prefix}:${scopeSuffix(pubkey, relayUrl)}:${outboxWindowNonce()}`; } @@ -144,6 +147,42 @@ function outboxScopePrefix( return `${prefix}:${scopeSuffix(pubkey, relayUrl)}:`; } +// Per-window monotonic `seq`, keyed by own-key base. Lazily seeded above the max +// `seq` among this window's surviving own keys so a reload (nonce survives in +// sessionStorage, this in-memory counter restarts) can never reuse — and thus +// overwrite — a key. Overwriting an own key would silently break immutability +// and reopen the reclaim race. +const ownSeqCounter = new Map(); + +// `seq` is zero-padded to a fixed width so a key's lexicographic order matches +// its numeric order. That makes the whole-blob replay tiebreak (max `queuedAt`, +// then key string) pick the higher `seq` — the newer edit — when a crash +// between write-new and delete-old leaves two same-second own keys with +// adjacent seqs that cross a digit boundary (…:9 vs …:10). 12 digits bound a +// window's edit count far past any reachable value. +const SEQ_WIDTH = 12; + +/** This window's own keys for a base (`:`). */ +function ownKeys(base: string): string[] { + const p = `${base}:`; + return localStorageKeys().filter((k) => k.startsWith(p)); +} + +/** Allocate this window's next write-once own key for a base. */ +function nextOwnKey(base: string): string { + let last = ownSeqCounter.get(base); + if (last === undefined) { + last = -1; + for (const k of ownKeys(base)) { + const seq = Number(k.slice(base.length + 1)); + if (Number.isInteger(seq) && seq > last) last = seq; + } + } + const seq = last + 1; + ownSeqCounter.set(base, seq); + return `${base}:${String(seq).padStart(SEQ_WIDTH, "0")}`; +} + /** A durable outbox record enumerated across all windows for a lane. */ export type OutboxRecord = { key: string; @@ -151,8 +190,6 @@ export type OutboxRecord = { // Seconds since epoch when the edit was queued (0 for a legacy entry written // before per-window keys, which therefore never wins a whole-blob tie). queuedAt: number; - // The exact stored string at enumeration time, for the pre-delete recheck. - raw: string; isOwn: boolean; }; @@ -193,10 +230,16 @@ function parseEnvelope( } /** - * Persist this window's unpublished edit under its own key. A single - * unconditional `setItem` — no read, no merge, no shared-key contention. - * Best-effort: the in-memory pending edit still drives this session's publish - * even if the persisted copy could not be written. + * Persist this window's unpublished edit under a fresh write-once key. + * + * Allocates the next `seq` for this window and `setItem`s the record there (a + * single unconditional write — no read, no merge, no shared-key contention), + * THEN deletes this window's older own keys. Write-before-delete: a crash + * between the two leaves ≥1 record for replay to coalesce, never zero. Because + * a key is written exactly once and never rewritten, a peer's boot-time reclaim + * of a proven-stale foreign key can never race a rewrite. Best-effort: the + * in-memory pending edit still drives this session's publish even if the + * persisted copy could not be written. */ export function writeOwnOutbox( prefix: string, @@ -205,24 +248,32 @@ export function writeOwnOutbox( store: unknown, nowSecs: number = Math.floor(Date.now() / 1_000), ): void { + const base = ownKeyBase(prefix, pubkey, relayUrl); try { + const key = nextOwnKey(base); window.localStorage.setItem( - ownOutboxKey(prefix, pubkey, relayUrl), + key, JSON.stringify({ store, queuedAt: nowSecs }), ); + // Drop this window's now-superseded own keys (all but the one just written). + for (const k of ownKeys(base)) { + if (k !== key) window.localStorage.removeItem(k); + } } catch { // Best-effort durability. } } -/** Remove this window's own outbox key (its edit published or is a no-op). */ +/** Remove all of this window's own outbox keys (its edit published or a no-op). */ export function clearOwnOutbox( prefix: string, pubkey: string, relayUrl: string, ): void { try { - window.localStorage.removeItem(ownOutboxKey(prefix, pubkey, relayUrl)); + for (const k of ownKeys(ownKeyBase(prefix, pubkey, relayUrl))) { + window.localStorage.removeItem(k); + } } catch { // Ignore — a stale own key is re-evaluated on the next publish/boot. } @@ -241,7 +292,8 @@ function localStorageKeys(): string[] { /** * Enumerate every window's durable outbox record for a lane, plus the single * legacy shared key from a pre-per-window build (treated as one more record so - * an edit persisted by a prior build still resumes and is later reclaimed). + * an edit persisted by a prior build still resumes; never reclaimed — see + * `reclaimOutbox`). */ export function enumerateOutbox( prefix: string, @@ -251,7 +303,7 @@ export function enumerateOutbox( parseStore: (json: unknown) => T | null, ): OutboxRecord[] { const scopePrefix = outboxScopePrefix(prefix, pubkey, relayUrl); - const own = ownOutboxKey(prefix, pubkey, relayUrl); + const ownPrefix = `${ownKeyBase(prefix, pubkey, relayUrl)}:`; const records: OutboxRecord[] = []; try { for (const key of localStorageKeys()) { @@ -263,8 +315,7 @@ export function enumerateOutbox( key, store: parsed.store, queuedAt: parsed.queuedAt, - raw, - isOwn: key === own, + isOwn: key.startsWith(ownPrefix), }); } } @@ -275,7 +326,6 @@ export function enumerateOutbox( key: legacyKey, store: legacy.store, queuedAt: legacy.queuedAt, - raw: legacyRaw, isOwn: false, }); } @@ -287,9 +337,12 @@ export function enumerateOutbox( /** * Reclaim redundant FOREIGN outbox keys whose edits durable relay evidence - * shows are safe to drop (`shouldReclaim`). Never touches this window's own - * key. Re-reads each key immediately before removing it and skips if the stored - * value changed since enumeration, so a live owner's fresh write is preserved. + * shows are safe to drop (`shouldReclaim`). Records are write-once, so a proven + * key cannot have changed since enumeration — the delete needs no recheck and + * cannot destroy a peer's fresh edit (a new edit lives under a new key). Never + * touches this window's own keys, and never touches the legacy v1 shared key: + * that key is mutable and `queuedAt=0`, so no gating makes deleting it safe; + * v2 only ever replays it. Call only after a successful head fetch. */ export function reclaimOutbox( prefix: string, @@ -306,15 +359,10 @@ export function reclaimOutbox( relayUrl, parseStore, )) { - if (record.isOwn) continue; + if (record.isOwn || record.key === legacyKey) continue; if (!shouldReclaim(record)) continue; try { - // Recheck: only remove if the value is byte-identical to what the - // reclaim decision was made against. A peer that rewrote the key in the - // gap owns a fresh edit we must not destroy. - if (window.localStorage.getItem(record.key) === record.raw) { - window.localStorage.removeItem(record.key); - } + window.localStorage.removeItem(record.key); } catch { // Leave for the next boot's reclamation. } diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.ts b/desktop/src/features/sidebar/lib/useChannelMutes.ts index 962a6a8f83..10506531a3 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.ts +++ b/desktop/src/features/sidebar/lib/useChannelMutes.ts @@ -107,21 +107,25 @@ export function useChannelMutes( if (cancelled) return; if (result.action === "apply-remote") { setStore(applyRemote(result.data)); - // Head fetch succeeded: reclaim any foreign window's outbox key the - // head already subsumes (a peer that published then quit). Gated on the - // fetched head and rechecked per key, so a live peer's unpublished edit - // is never destroyed. A `hold` (absent/failed head) reclaims nothing. - reclaimSubsumedMutesOutbox(pubkey, relayUrl, result.data.store); } - // "hold": seed already performed by bootstrap (if first-sync), or blocked. // Resume any edit persisted to the durable outbox before a prior // 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) { managerRef.current?.publishMutes(outbox); } else { clearChannelMutesOutbox(pubkey, relayUrl); } + if (result.action === "apply-remote") { + // Head fetch succeeded: reclaim any foreign window's write-once outbox + // key the head already subsumes (a peer that published then quit). + // Gated on the fetched head; records are immutable so no recheck is + // needed and a live peer's unpublished edit (under a different key) is + // never destroyed. A `hold` (absent/failed head) reclaims nothing. + reclaimSubsumedMutesOutbox(pubkey, relayUrl, result.data.store); + } }); return () => { cancelled = true; diff --git a/desktop/src/features/sidebar/lib/useChannelSections.ts b/desktop/src/features/sidebar/lib/useChannelSections.ts index 5ee2e6e222..edefdff4dc 100644 --- a/desktop/src/features/sidebar/lib/useChannelSections.ts +++ b/desktop/src/features/sidebar/lib/useChannelSections.ts @@ -144,26 +144,31 @@ export function useChannelSections( if (cancelled) return; if (result.action === "apply-remote") { setStore(applyRemote(result.data)); - // Head fetch succeeded: reclaim any foreign window's outbox key the - // relay head itself supersedes (`queuedAt` ≤ head `created_at`). Gated - // on the fetched head and rechecked per key, so a live peer's edit - // queued after the head is kept. A `hold` (absent/failed) reclaims none. - reclaimSupersededSectionsOutbox( - pubkey, - relayUrl, - result.data.createdAt, - ); } // "hold": seed already performed by bootstrap (if first-sync), or // blocked (failed fetch / prior watermark). The reconciliation effect // below retries a failed fetch; here we only resume any edit that was // persisted to the durable outbox before a prior quit/community-switch. + // 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 = readChannelSectionsOutbox(pubkey, relayUrl); if (outbox) { managerRef.current?.publishSections(outbox); } else { clearChannelSectionsOutbox(pubkey, relayUrl); } + if (result.action === "apply-remote") { + // Head fetch succeeded: reclaim any foreign window's write-once outbox + // key the head STRICTLY supersedes (`queuedAt` < head `created_at`). + // Gated on the fetched head; records are immutable so no recheck is + // needed and a live peer's edit queued at/after the head is kept. A + // `hold` (absent/failed) reclaims none. + reclaimSupersededSectionsOutbox( + pubkey, + relayUrl, + result.data.createdAt, + ); + } }); return () => { cancelled = true; diff --git a/desktop/src/features/sidebar/lib/useChannelSortPreference.ts b/desktop/src/features/sidebar/lib/useChannelSortPreference.ts index 855ee3f9e2..67b52bc57d 100644 --- a/desktop/src/features/sidebar/lib/useChannelSortPreference.ts +++ b/desktop/src/features/sidebar/lib/useChannelSortPreference.ts @@ -142,21 +142,26 @@ export function useChannelSortPreference( if (cancelled) return; if (result.action === "apply-remote") { setStore(applyRemote(result.data)); - // Head fetch succeeded: reclaim any foreign window's outbox key the - // relay head itself supersedes (`queuedAt` ≤ head `created_at`). Gated - // on the fetched head and rechecked per key, so a live peer's edit - // queued after the head is kept. A `hold` (absent/failed) reclaims none. - reclaimSupersededSortOutbox(pubkey, relayUrl, result.data.createdAt); } // "hold": seed already performed by bootstrap (if first-sync), or blocked // (failed fetch / prior watermark). Resume any edit persisted to the - // durable outbox before a prior quit/community-switch. + // durable outbox before a prior quit/community-switch. 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 = readChannelSortOutbox(pubkey, relayUrl); if (outbox) { managerRef.current?.publishSortPrefs(outbox); } else { clearChannelSortOutbox(pubkey, relayUrl); } + if (result.action === "apply-remote") { + // Head fetch succeeded: reclaim any foreign window's write-once outbox + // key the head STRICTLY supersedes (`queuedAt` < head `created_at`). + // Gated on the fetched head; records are immutable so no recheck is + // needed and a live peer's edit queued at/after the head is kept. A + // `hold` (absent/failed) reclaims none. + reclaimSupersededSortOutbox(pubkey, relayUrl, result.data.createdAt); + } }); return () => { cancelled = true; diff --git a/desktop/src/features/sidebar/lib/useChannelStars.ts b/desktop/src/features/sidebar/lib/useChannelStars.ts index 89679df8d4..f480c266b4 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.ts +++ b/desktop/src/features/sidebar/lib/useChannelStars.ts @@ -107,21 +107,26 @@ export function useChannelStars( if (cancelled) return; if (result.action === "apply-remote") { setStore(applyRemote(result.data)); - // Head fetch succeeded: reclaim any foreign window's outbox key the - // head already subsumes (a peer that published then quit). Gated on the - // fetched head and rechecked per key, so a live peer's unpublished edit - // is never destroyed. A `hold` (absent/failed head) reclaims nothing. - reclaimSubsumedStarsOutbox(pubkey, relayUrl, result.data.store); } // Resume every window's edit persisted to the durable outbox before a // prior quit/community-switch so a click made <2s before teardown still - // syncs. Merges all records (order-independent), then publishes. + // 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) { managerRef.current?.publishStars(outbox); } else { clearChannelStarsOutbox(pubkey, relayUrl); } + if (result.action === "apply-remote") { + // Head fetch succeeded: reclaim any foreign window's write-once outbox + // key the head already subsumes (a peer that published then quit). + // Gated on the fetched head; records are immutable so no recheck is + // needed and a live peer's unpublished edit (under a different key) is + // never destroyed. A `hold` (absent/failed head) reclaims nothing. + reclaimSubsumedStarsOutbox(pubkey, relayUrl, result.data.store); + } }); return () => { cancelled = true;