From 439df98240d990328aa08d954503435c94c00df9 Mon Sep 17 00:00:00 2001 From: Duncan Date: Tue, 25 Aug 2026 11:54:57 -0400 Subject: [PATCH] refactor(sidebar): fold channel stars/mutes onto Lamport-rev max-merge Replace the LWW register plus ownership/dirty-set/canonical-supersession machinery with a per-entry Lamport `rev` and a single max-merge on every path, mirroring the read-state data model. Each entry carries an additive optional `rev` (missing implies 0; payload stays `version: 1` so older builds keep parsing our blobs). One `mergeStores` orders by updatedAt then rev then the starred/muted-true leaf, and ends in the 500-entry bound. Clicks stamp `updatedAt = max(now, localEntry?.updatedAt ?? 0, maxUpdatedAtSeen(id))` and mint `rev = max(localEntry.rev, maxRevSeen(id)) + 1`, so a click strictly dominates every state its replica has observed and cannot lose to a same-second remote. The sync managers hold a per-channel two-field high-water map fed by a single `observe()` on every ingest path. Stars sync keeps the generation-CAS + single-flight lane and bounded-backoff retry plus a durable outbox so an in-flight publish can never clear a newer pending edit; mutes sync mirrors it. Remote ingestion never touches the pending lane. Deleted: mergeApplyingRemote, mergeStoresWithTie, mergeCanonicalSupersession, dirtyChannelIds, lastAppliedRemoteTs/lastAppliedEventId, and the event-clock branch. Sections and sort are untouched. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../sidebar/lib/channelMutesStorage.test.mjs | 490 +++++++------- .../sidebar/lib/channelMutesStorage.ts | 185 ++++-- .../sidebar/lib/channelMutesSync.test.mjs | 287 ++++++-- .../features/sidebar/lib/channelMutesSync.ts | 173 ++++- .../sidebar/lib/channelStarsStorage.test.mjs | 511 +++++++------- .../sidebar/lib/channelStarsStorage.ts | 185 ++++-- .../sidebar/lib/channelStarsSync.test.mjs | 279 ++++++-- .../features/sidebar/lib/channelStarsSync.ts | 173 ++++- .../sidebar/lib/useChannelMutes.test.mjs | 628 ++++++++---------- .../features/sidebar/lib/useChannelMutes.ts | 157 +++-- .../sidebar/lib/useChannelStars.test.mjs | 624 ++++++++--------- .../features/sidebar/lib/useChannelStars.ts | 157 +++-- 12 files changed, 2336 insertions(+), 1513 deletions(-) diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs index f506eb93f1..0071d4d0e9 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs @@ -4,34 +4,57 @@ import test from "node:test"; import { boundMuteStore, MAX_CHANNEL_MUTE_ENTRIES, - parseMutePayload, mergeStores, + parseMutePayload, mutedChannelIdsFromStore, } from "./channelMutesStorage.ts"; // ── parseMutePayload ────────────────────────────────────────────────────────── -test("parseMutePayload: valid payload with channels returns store", () => { +test("parseMutePayload: valid payload with channels returns store (rev preserved)", () => { const payload = { version: 1, channels: { - "chan-1": { muted: true, updatedAt: 1000 }, - "chan-2": { muted: false, updatedAt: 2000 }, + "chan-1": { muted: true, updatedAt: 1000, rev: 3 }, + "chan-2": { muted: false, updatedAt: 2000, rev: 0 }, }, }; - const result = parseMutePayload(payload); - assert.deepEqual(result, { + assert.deepEqual(parseMutePayload(payload), payload); +}); + +test("parseMutePayload: missing rev normalizes to 0 (old-build blob, entry kept)", () => { + const result = parseMutePayload({ + version: 1, + channels: { "chan-1": { muted: true, updatedAt: 1000 } }, + }); + assert.deepEqual(result.channels["chan-1"], { + muted: true, + updatedAt: 1000, + rev: 0, + }); +}); + +test("parseMutePayload: malformed rev (string / negative / non-integer / NaN) normalizes to 0", () => { + const result = parseMutePayload({ version: 1, channels: { - "chan-1": { muted: true, updatedAt: 1000 }, - "chan-2": { muted: false, updatedAt: 2000 }, + str: { muted: true, updatedAt: 1, rev: "5" }, + neg: { muted: true, updatedAt: 1, rev: -2 }, + frac: { muted: true, updatedAt: 1, rev: 1.5 }, + nan: { muted: true, updatedAt: 1, rev: NaN }, }, }); + for (const id of ["str", "neg", "frac", "nan"]) { + assert.equal(result.channels[id].rev, 0, `${id} rev normalized to 0`); + assert.equal(result.channels[id].muted, true, `${id} entry kept`); + } }); test("parseMutePayload: missing version returns null", () => { assert.equal( - parseMutePayload({ channels: { "chan-1": { muted: true, updatedAt: 1 } } }), + parseMutePayload({ + channels: { "chan-1": { muted: true, updatedAt: 1 } }, + }), null, ); }); @@ -46,18 +69,15 @@ test("parseMutePayload: wrong version returns null", () => { ); }); -test("parseMutePayload: null input returns null", () => { +test("parseMutePayload: null / non-object input returns null", () => { assert.equal(parseMutePayload(null), null); -}); - -test("parseMutePayload: non-object input returns null", () => { assert.equal(parseMutePayload("string"), null); assert.equal(parseMutePayload(42), null); assert.equal(parseMutePayload(true), null); }); test("parseMutePayload: malformed channel entries missing muted/updatedAt are filtered out", () => { - const payload = { + const result = parseMutePayload({ version: 1, channels: { "no-muted": { updatedAt: 1000 }, @@ -67,252 +87,251 @@ test("parseMutePayload: malformed channel entries missing muted/updatedAt are fi "updated-at-wrong-type": { muted: true, updatedAt: "now" }, null: null, }, - }; - const result = parseMutePayload(payload); + }); assert.deepEqual(result, { version: 1, - channels: { - valid: { muted: false, updatedAt: 500 }, - }, + channels: { valid: { muted: false, updatedAt: 500, rev: 0 } }, }); }); test("parseMutePayload: NaN/Infinity/negative updatedAt entries are filtered out", () => { - const payload = { + const result = parseMutePayload({ version: 1, channels: { nan: { muted: true, updatedAt: NaN }, inf: { muted: true, updatedAt: Infinity }, "neg-inf": { muted: true, updatedAt: -Infinity }, neg: { muted: true, updatedAt: -1 }, - valid: { muted: true, updatedAt: 100 }, + valid: { muted: true, updatedAt: 100, rev: 2 }, }, - }; - const result = parseMutePayload(payload); + }); assert.deepEqual(result, { version: 1, - channels: { valid: { muted: true, updatedAt: 100 } }, + channels: { valid: { muted: true, updatedAt: 100, rev: 2 } }, }); }); -test("parseMutePayload: empty channels returns store with empty channels", () => { - const result = parseMutePayload({ version: 1, channels: {} }); - assert.deepEqual(result, { version: 1, channels: {} }); -}); - -test("parseMutePayload: version 1 with no channels key returns store with empty channels", () => { - const result = parseMutePayload({ version: 1 }); - assert.deepEqual(result, { version: 1, channels: {} }); -}); - -// ── mergeStores ─────────────────────────────────────────────────────────────── - -test("mergeStores: non-overlapping channels returns union of both", () => { - const local = { +test("parseMutePayload: empty channels / no channels key returns empty store", () => { + assert.deepEqual(parseMutePayload({ version: 1, channels: {} }), { version: 1, - channels: { "chan-a": { muted: true, updatedAt: 100 } }, - }; - const remote = { + channels: {}, + }); + assert.deepEqual(parseMutePayload({ version: 1 }), { version: 1, - channels: { "chan-b": { muted: false, updatedAt: 200 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result, { - version: 1, - channels: { - "chan-a": { muted: true, updatedAt: 100 }, - "chan-b": { muted: false, updatedAt: 200 }, - }, + channels: {}, }); }); -test("mergeStores: overlapping channel with remote newer takes remote", () => { - const local = { +// ── mergeStores: tuple order (updatedAt → rev → value) ──────────────────────── + +const E = (muted, updatedAt, rev) => ({ muted, updatedAt, rev }); +const S = (entry) => ({ version: 1, channels: { c: entry } }); + +test("mergeStores: non-overlapping channels returns union", () => { + const result = mergeStores( + { version: 1, channels: { a: E(true, 100, 1) } }, + { version: 1, channels: { b: E(false, 200, 1) } }, + ); + assert.deepEqual(result, { version: 1, - channels: { "chan-a": { muted: false, updatedAt: 100 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { muted: true, updatedAt: 200 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { muted: true, updatedAt: 200 }); + channels: { a: E(true, 100, 1), b: E(false, 200, 1) }, + }); }); -test("mergeStores: overlapping channel with local newer takes local", () => { - const local = { - version: 1, - channels: { "chan-a": { muted: true, updatedAt: 300 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { muted: false, updatedAt: 100 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { muted: true, updatedAt: 300 }); +test("mergeStores: strictly-later updatedAt wins regardless of rev (primary key)", () => { + // Later updatedAt with LOWER rev still wins — updatedAt is primary. This is + // the old-build interop case: an old build's rev-0 fresh edit beats a stale + // rev-bearing new-build entry. + const result = mergeStores(S(E(false, 200, 0)), S(E(true, 100, 7))); + assert.deepEqual(result.channels.c, E(false, 200, 0)); }); -test("mergeStores: overlapping channel with same updatedAt local wins", () => { - const local = { - version: 1, - channels: { "chan-a": { muted: true, updatedAt: 500 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { muted: false, updatedAt: 500 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { muted: true, updatedAt: 500 }); +test("mergeStores: equal updatedAt → higher rev wins (same-second tiebreak)", () => { + const result = mergeStores(S(E(false, 100, 5)), S(E(true, 100, 2))); + assert.deepEqual(result.channels.c, E(false, 100, 5)); +}); + +test("mergeStores: equal updatedAt AND equal rev → muted=true wins (leaf)", () => { + const result = mergeStores(S(E(false, 100, 3)), S(E(true, 100, 3))); + assert.deepEqual(result.channels.c, E(true, 100, 3)); }); test("mergeStores: unmute with higher updatedAt overrides mute", () => { - const local = { - version: 1, - channels: { "chan-a": { muted: true, updatedAt: 100 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { muted: false, updatedAt: 999 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { muted: false, updatedAt: 999 }); + const result = mergeStores(S(E(true, 100, 9)), S(E(false, 999, 1))); + assert.deepEqual(result.channels.c, E(false, 999, 1)); }); -test("mergeStores: empty local returns remote entries", () => { - const local = { version: 1, channels: {} }; - const remote = { - version: 1, - channels: { "chan-b": { muted: true, updatedAt: 42 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels, { - "chan-b": { muted: true, updatedAt: 42 }, - }); -}); - -test("mergeStores: empty remote returns local entries", () => { - const local = { - version: 1, - channels: { "chan-a": { muted: false, updatedAt: 10 } }, - }; - const remote = { version: 1, channels: {} }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels, { - "chan-a": { muted: false, updatedAt: 10 }, - }); -}); - -test("mergeStores: both empty returns empty", () => { - const result = mergeStores( - { version: 1, channels: {} }, +test("mergeStores: empty local / empty remote / both empty", () => { + assert.deepEqual( + mergeStores({ version: 1, channels: {} }, S(E(true, 42, 1))).channels.c, + E(true, 42, 1), + ); + assert.deepEqual( + mergeStores(S(E(false, 10, 2)), { version: 1, channels: {} }).channels.c, + E(false, 10, 2), + ); + assert.deepEqual( + mergeStores({ version: 1, channels: {} }, { version: 1, channels: {} }), { version: 1, channels: {} }, ); - assert.deepEqual(result, { version: 1, channels: {} }); }); +// ── mergeStores: algebra (commutativity, associativity, idempotence) ────────── + +function randEntry(rng) { + return { + muted: rng() > 0.5, + updatedAt: Math.floor(rng() * 5), + rev: Math.floor(rng() * 5), + }; +} +function randStore(rng, ids) { + const channels = {}; + for (const id of ids) if (rng() > 0.3) channels[id] = randEntry(rng); + return { version: 1, channels }; +} +// Deterministic LCG so failures reproduce. +function lcg(seed) { + let s = seed >>> 0; + return () => { + s = (s * 1664525 + 1013904223) >>> 0; + return s / 4294967296; + }; +} + +test("mergeStores: commutative — merge(a,b) === merge(b,a)", () => { + const rng = lcg(12345); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + assert.deepEqual(mergeStores(a, b), mergeStores(b, a)); + } +}); + +test("mergeStores: associative — merge(merge(a,b),c) === merge(a,merge(b,c))", () => { + const rng = lcg(67890); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + const c = randStore(rng, ids); + assert.deepEqual( + mergeStores(mergeStores(a, b), c), + mergeStores(a, mergeStores(b, c)), + ); + } +}); + +test("mergeStores: idempotent — merge(a, merge(a,b)) === merge(a,b)", () => { + const rng = lcg(24680); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + const ab = mergeStores(a, b); + assert.deepEqual(mergeStores(a, ab), ab); + assert.deepEqual(mergeStores(ab, ab), ab); + } +}); + +// ── v1-blob bidirectional compatibility ─────────────────────────────────────── + +test("v1 compat: a rev-carrying blob round-trips through a rev-less parser view", () => { + // Simulate an old build reading our blob: JSON-serialize our rev-carrying + // payload, parse it back — version stays 1 so it is NOT rejected, and the + // core fields survive (old build simply ignores rev). + const ours = { + version: 1, + channels: { c: { muted: true, updatedAt: 100, rev: 7 } }, + }; + const roundTripped = parseMutePayload(JSON.parse(JSON.stringify(ours))); + assert.equal(roundTripped.version, 1, "version stays 1 — old build accepts"); + assert.equal(roundTripped.channels.c.muted, true); + assert.equal(roundTripped.channels.c.updatedAt, 100); +}); + +test("v1 compat: old-build unmute (no rev, updatedAt+1) beats our stale mute", () => { + // New build wrote {muted:true, rev:7, updatedAt:t}; old build (no rev) + // unmutes producing {muted:false, updatedAt:t+1}. The unmute wins on the + // primary updatedAt key — old builds can still edit upgraded channels. + const ours = S(E(true, 100, 7)); + const oldBuildUnmute = parseMutePayload({ + version: 1, + channels: { c: { muted: false, updatedAt: 101 } }, + }); + assert.deepEqual(mergeStores(ours, oldBuildUnmute).channels.c, { + muted: false, + updatedAt: 101, + rev: 0, + }); +}); + +// ── boundMuteStore ──────────────────────────────────────────────────────────── + test("boundMuteStore: retains newest entries regardless of muted value", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ - `active-${index}`, - { muted: true, updatedAt: index + 1 }, + Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 1, 0), ]), ); - channels["old-false"] = { muted: false, updatedAt: 0 }; - channels["new-false"] = { muted: false, updatedAt: 9999 }; - + channels["old-false"] = E(false, 0, 0); + channels["new-false"] = E(false, 9999, 0); const result = boundMuteStore({ version: 1, channels }); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_MUTE_ENTRIES); assert.equal(result.channels["old-false"], undefined); - assert.deepEqual(result.channels["new-false"], { - muted: false, - updatedAt: 9999, - }); + assert.deepEqual(result.channels["new-false"], E(false, 9999, 0)); assert.equal(result.channels["active-0"], undefined); - assert.deepEqual(result.channels["active-1"], { muted: true, updatedAt: 2 }); }); test("boundMuteStore: uses channel ID as an updatedAt tie-breaker", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES + 1 }, (_, index) => [ - `channel-${String(MAX_CHANNEL_MUTE_ENTRIES - index).padStart(3, "0")}`, - { muted: true, updatedAt: 1 }, + Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES + 1 }, (_, i) => [ + `channel-${String(MAX_CHANNEL_MUTE_ENTRIES - i).padStart(3, "0")}`, + E(true, 1, 0), ]), ); - const result = boundMuteStore({ version: 1, channels }); - assert.equal(result.channels["channel-000"], undefined); - assert.deepEqual(result.channels["channel-500"], { - muted: true, - updatedAt: 1, - }); + assert.deepEqual(result.channels["channel-500"], E(true, 1, 0)); }); -test("boundMuteStore: preserves a same-second mute mutation by key", () => { +test("boundMuteStore: preserves a same-second mutation by key", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ - `z-channel-${String(index).padStart(3, "0")}`, - { muted: true, updatedAt: 1 }, + Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, i) => [ + `z-channel-${String(i).padStart(3, "0")}`, + E(true, 1, 0), ]), ); - channels["a-target"] = { muted: true, updatedAt: 1 }; - + channels["a-target"] = E(false, 1, 1); const result = boundMuteStore({ version: 1, channels }, "a-target"); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_MUTE_ENTRIES); - assert.deepEqual(result.channels["a-target"], { - muted: true, - updatedAt: 1, - }); - assert.equal(result.channels["z-channel-000"], undefined); -}); - -test("boundMuteStore: preserves a same-second unmute mutation by key", () => { - const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ - `z-channel-${String(index).padStart(3, "0")}`, - { muted: true, updatedAt: 1 }, - ]), - ); - channels["a-target"] = { muted: false, updatedAt: 1 }; - - const result = boundMuteStore({ version: 1, channels }, "a-target"); - - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_MUTE_ENTRIES); - assert.deepEqual(result.channels["a-target"], { - muted: false, - updatedAt: 1, - }); + assert.deepEqual(result.channels["a-target"], E(false, 1, 1)); assert.equal(result.channels["z-channel-000"], undefined); }); test("mergeStores: a fresh at-capacity unmute defeats an older remote mute", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ - `active-${index}`, - { muted: true, updatedAt: index + 1 }, + Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 1, 0), ]), ); - channels["unmuted"] = { muted: false, updatedAt: 9999 }; + channels.unmuted = E(false, 9999, 1); const bounded = boundMuteStore({ version: 1, channels }); - const result = mergeStores(bounded, { version: 1, - channels: { unmuted: { muted: true, updatedAt: 9998 } }, - }); - - assert.deepEqual(result.channels.unmuted, { - muted: false, - updatedAt: 9999, + channels: { unmuted: E(true, 9998, 5) }, }); + assert.deepEqual(result.channels.unmuted, E(false, 9999, 1)); }); test("mergeStores: evicted remote ID re-enters and the oldest state is re-trimmed", () => { const localChannels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ - `active-${index}`, - { muted: true, updatedAt: index + 10 }, + Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 10, 0), ]), ); const result = mergeStores( @@ -320,56 +339,81 @@ test("mergeStores: evicted remote ID re-enters and the oldest state is re-trimme { version: 1, channels: { - "evicted-id": { muted: true, updatedAt: 9999 }, - "active-0": { muted: false, updatedAt: 9998 }, + "evicted-id": E(true, 9999, 0), + "active-0": E(false, 9998, 0), }, }, ); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_MUTE_ENTRIES); - assert.deepEqual(result.channels["evicted-id"], { - muted: true, - updatedAt: 9999, - }); - assert.deepEqual(result.channels["active-0"], { - muted: false, - updatedAt: 9998, - }); + assert.deepEqual(result.channels["evicted-id"], E(true, 9999, 0)); + assert.deepEqual(result.channels["active-0"], E(false, 9998, 0)); assert.equal(result.channels["active-1"], undefined); - assert.deepEqual(result.channels["active-2"], { muted: true, updatedAt: 12 }); }); -// ── mutedChannelIdsFromStore ────────────────────────────────────────────────── +// ── Eviction / remount (finding 3) ──────────────────────────────────────────── + +// Easy branch: X evicted at an OLDER second → remount (high-water lost) → click +// X at the current second → merge a remote carrying X at a high rev but an old +// updatedAt. The click's newer updatedAt wins on the primary key; the lost rev +// high-water is irrelevant. Closed by construction for all cross-second cases. +test("finding 3 easy branch: a fresh click beats an evicted high-rev entry at an older updatedAt", () => { + // Remount state: the user clicks X fresh at updatedAt=now, empty high-water + // (evicted), so rev mints to 1. + const click = S(E(true, 1000, 1)); + // The previously observed remote X sits at an OLD updatedAt with a high rev. + const remote = S(E(false, 500, 100)); + const merged = mergeStores(click, remote); + assert.deepEqual( + merged.channels.c, + E(true, 1000, 1), + "fresh click wins on the primary updatedAt key", + ); +}); + +// Hard branch (Thufir's exact equal-second counterexample): >500 entries all at +// the CURRENT second → X evicted by the id tiebreak (not because it is old) → +// remount in the same second → click X at rev 1 (empty high-water) → merge the +// previously observed remote X at rev 100, EQUAL updatedAt. updatedAt ties, rev +// decides, 100 > 1 — the click LOSES. Documented deterministic residual, proven +// here as the hard branch (not disguised as safety). +test("finding 3 hard branch: equal-second evicted click (rev 1) loses to observed remote (rev 100)", () => { + const NOW = 777; + // Remount click on the evicted channel: empty high-water → rev 1, updatedAt=NOW. + const click = S(E(true, NOW, 1)); + // The previously observed remote for the same channel at the same second, + // rev 100 (it may precede the remount — not genuinely concurrent). + const remote = S(E(false, NOW, 100)); + const merged = mergeStores(click, remote); + assert.deepEqual( + merged.channels.c, + E(false, NOW, 100), + "equal updatedAt → higher rev wins deterministically (documented residual)", + ); + // Deterministic either merge order — a lost click, never a divergence. + assert.deepEqual(mergeStores(remote, click).channels.c, E(false, NOW, 100)); +}); + +// ── mutedChannelIdsFromStore ──────────────────────────────────────────────── test("mutedChannelIdsFromStore: returns set of IDs where muted=true", () => { - const store = { + const result = mutedChannelIdsFromStore({ version: 1, channels: { - "chan-a": { muted: true, updatedAt: 100 }, - "chan-b": { muted: true, updatedAt: 200 }, - "chan-c": { muted: false, updatedAt: 300 }, + a: E(true, 100, 0), + b: E(true, 200, 0), + c: E(false, 300, 0), }, - }; - const result = mutedChannelIdsFromStore(store); - assert.equal(result.has("chan-a"), true); - assert.equal(result.has("chan-b"), true); - assert.equal(result.has("chan-c"), false); - assert.equal(result.size, 2); + }); + assert.deepEqual([...result].sort(), ["a", "b"]); }); -test("mutedChannelIdsFromStore: excludes IDs where muted=false", () => { - const store = { - version: 1, - channels: { - "chan-x": { muted: false, updatedAt: 1 }, - "chan-y": { muted: false, updatedAt: 2 }, - }, - }; - const result = mutedChannelIdsFromStore(store); - assert.equal(result.size, 0); -}); - -test("mutedChannelIdsFromStore: empty channels returns empty set", () => { - const result = mutedChannelIdsFromStore({ version: 1, channels: {} }); - assert.equal(result.size, 0); +test("mutedChannelIdsFromStore: all-false / empty returns empty set", () => { + assert.equal( + mutedChannelIdsFromStore({ + version: 1, + channels: { x: E(false, 1, 0) }, + }).size, + 0, + ); + assert.equal(mutedChannelIdsFromStore({ version: 1, channels: {} }).size, 0); }); diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.ts b/desktop/src/features/sidebar/lib/channelMutesStorage.ts index 90ede83eb1..1fc04424ac 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.ts +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.ts @@ -1,9 +1,16 @@ +import { normalizeRelayUrl } from "@/shared/lib/normalizeRelayUrl"; + const STORAGE_KEY_PREFIX = "buzz-channel-mutes.v1"; export const MAX_CHANNEL_MUTE_ENTRIES = 500; export type ChannelMuteEntry = { muted: boolean; updatedAt: number; + // Per-channel Lamport revision. Breaks a same-second `updatedAt` tie that the + // integer clock cannot resolve. Absent in blobs from an older build ⇒ read as + // 0 (a valid, mergeable value), so the payload stays `version: 1` and older + // builds still parse our blobs. + rev: number; }; export type ChannelMuteStore = { @@ -29,8 +36,8 @@ export function parseMutePayload(json: unknown): ChannelMuteStore | null { obj.channels !== null && !Array.isArray(obj.channels) ? Object.fromEntries( - Object.entries(obj.channels as Record).filter( - (entry): entry is [string, ChannelMuteEntry] => { + Object.entries(obj.channels as Record) + .filter((entry): entry is [string, Record] => { const v = entry[1]; return ( typeof v === "object" && @@ -42,8 +49,27 @@ export function parseMutePayload(json: unknown): ChannelMuteStore | null { ) && ((v as Record).updatedAt as number) >= 0 ); - }, - ), + }) + // Normalize `rev`: accept a non-negative integer, otherwise 0. An + // entry is never dropped solely because `rev` is absent (older + // build) or malformed — absence is a valid mergeable value. + .map(([id, v]) => { + const rawRev = v.rev; + const rev = + typeof rawRev === "number" && + Number.isInteger(rawRev) && + rawRev >= 0 + ? rawRev + : 0; + return [ + id, + { + muted: v.muted as boolean, + updatedAt: v.updatedAt as number, + rev, + }, + ]; + }), ) : {}; return boundMuteStore({ version: 1, channels }); @@ -108,79 +134,47 @@ export function writeChannelMutesStore( } } -export function mergeStores( - local: ChannelMuteStore, - remote: ChannelMuteStore, -): ChannelMuteStore { - return mergeStoresWithTie(local, remote, false); -} - /** - * Merge a remote store that has already won the event-level canonical tie-break - * (`created_at DESC, id ASC`) into the local store, resolving a per-entry - * `updatedAt` tie in favour of the *remote* value. Once the comparator has - * chosen this remote event as the stored winner, its per-entry values must - * survive, or a stale value from a superseded larger-id event delivered first - * would win the merge and silently undo the canonical winner. Strictly-newer - * local per-entry edits (`l.updatedAt > r.updatedAt`) still win. + * Merge two mute stores by a per-channel total order: + * `updatedAt` DESC → `rev` DESC → `muted === true` wins. This order is + * commutative, associative, and idempotent (before bounding), so every + * observation path (bootstrap, live, reconnect, reconcile, pre-publish, + * cross-window storage) applies it with no ordering or ownership overlay and + * all replicas converge. + * + * `updatedAt` is primary so a strictly-later edit — from any build, whether it + * carries `rev` or (older build) reads `rev: 0` — wins outright. `rev` breaks + * only a same-second `updatedAt` tie: the ambiguous integer-second window the + * clock cannot resolve, where a click that minted `rev = maxSeen + 1` dominates + * any same-second state it observed. On a full tie (equal `updatedAt` AND equal + * `rev`) `true` wins as the deterministic leaf. */ -export function mergeApplyingRemote( - local: ChannelMuteStore, - remote: ChannelMuteStore, -): ChannelMuteStore { - return mergeStoresWithTie(local, remote, true); -} - -function mergeStoresWithTie( - local: ChannelMuteStore, - remote: ChannelMuteStore, - preferRemoteOnTie: boolean, +export function mergeStores( + a: ChannelMuteStore, + b: ChannelMuteStore, ): ChannelMuteStore { const allIds = new Set([ - ...Object.keys(local.channels), - ...Object.keys(remote.channels), + ...Object.keys(a.channels), + ...Object.keys(b.channels), ]); const merged: Record = {}; for (const id of allIds) { - const l = local.channels[id]; - const r = remote.channels[id]; - if (l && r) { - const localWins = preferRemoteOnTie - ? l.updatedAt > r.updatedAt - : l.updatedAt >= r.updatedAt; - merged[id] = localWins ? l : r; - } else { - merged[id] = (l ?? r) as ChannelMuteEntry; - } + const l = a.channels[id]; + const r = b.channels[id]; + merged[id] = l && r ? pickMuteEntry(l, r) : ((l ?? r) as ChannelMuteEntry); } return boundMuteStore({ version: 1, channels: merged }); } -/** - * Apply a canonical lower-id correction (`mergeApplyingRemote`: remote wins a - * per-entry `updatedAt` tie) while preserving entries the user changed locally - * since the superseded head was applied. The correction canonicalises remote - * history, but a same-second local click carries integer-second `updatedAt` - * equal to the remote's, so the plain remote-wins tie would silently clobber - * it. For each `dirtyId` the local entry wins only the tie (`l.updatedAt >= - * r.updatedAt`) — a genuinely newer remote value still wins, so a stale dirty - * id can never override a later correction. - */ -export function mergeCanonicalSupersession( - local: ChannelMuteStore, - remote: ChannelMuteStore, - dirtyIds: ReadonlySet, -): ChannelMuteStore { - const applied = mergeApplyingRemote(local, remote); - if (dirtyIds.size === 0) return applied; - const channels = { ...applied.channels }; - for (const id of dirtyIds) { - const l = local.channels[id]; - if (!l) continue; - const r = remote.channels[id]; - if (!r || l.updatedAt >= r.updatedAt) channels[id] = l; - } - return boundMuteStore({ version: 1, channels }); +/** The winner of two entries under `updatedAt` → `rev` → `muted` order. */ +function pickMuteEntry( + l: ChannelMuteEntry, + r: ChannelMuteEntry, +): ChannelMuteEntry { + if (l.updatedAt !== r.updatedAt) return l.updatedAt > r.updatedAt ? l : r; + if (l.rev !== r.rev) return l.rev > r.rev ? l : r; + if (l.muted !== r.muted) return l.muted ? l : r; + return l; } export function mutedChannelIdsFromStore(store: ChannelMuteStore): Set { @@ -190,3 +184,62 @@ export function mutedChannelIdsFromStore(store: ChannelMuteStore): Set { .map(([id]) => id), ); } + +const OUTBOX_KEY_PREFIX = "buzz-channel-mutes-outbox.v1"; + +// The outbox is a per-relay sync-lane structure (like the watermark), so it is +// relay-scoped even though the main store stays pubkey-only: an edit made +// against relay A must never resume-publish onto relay B after a community +// switch. +function outboxKey(pubkey: string, relayUrl: string): string { + return `${OUTBOX_KEY_PREFIX}:${pubkey}:${encodeURIComponent(normalizeRelayUrl(relayUrl))}`; +} + +/** + * Persist an unpublished edit so it survives quit/community-switch within the + * 2s publish debounce. Written synchronously on every click; cleared once the + * edit is published or found identical to the last published store. Resumed on + * next mount so a durable intent is never silently dropped at teardown. + */ +export function writeChannelMutesOutbox( + pubkey: string, + store: ChannelMuteStore, + relayUrl: string, +): void { + try { + window.localStorage.setItem( + outboxKey(pubkey, relayUrl), + JSON.stringify(boundMuteStore(store)), + ); + } catch { + // Best-effort durability; the in-memory pendingStore still drives this + // session's publish even if the persisted copy could not be written. + } +} + +/** Read a persisted unpublished edit, or null when none/unparseable. */ +export function readChannelMutesOutbox( + pubkey: string, + relayUrl: string, +): ChannelMuteStore | null { + try { + const raw = window.localStorage.getItem(outboxKey(pubkey, relayUrl)); + if (!raw) return null; + return parseMutePayload(JSON.parse(raw)); + } catch { + return null; + } +} + +/** Clear the persisted outbox (edit published or a no-op). */ +export function clearChannelMutesOutbox( + pubkey: string, + relayUrl: string, +): void { + try { + window.localStorage.removeItem(outboxKey(pubkey, relayUrl)); + } catch { + // Ignore — a stale outbox entry is re-evaluated (and re-cleared if + // identical to the head) on the next publish attempt. + } +} diff --git a/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs b/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs index 845e5a5acc..1ef2cdd1b5 100644 --- a/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs +++ b/desktop/src/features/sidebar/lib/channelMutesSync.test.mjs @@ -4,8 +4,9 @@ import test, { mock } from "node:test"; import { relayClient } from "@/shared/api/relayClient"; import { ChannelMuteSyncManager } from "./channelMutesSync.ts"; import { - makeFakeWindow, installFakeWindow, + installTauriMock, + makeFakeWindow, } from "./sidebarSyncTestHelpers.mjs"; const RELAY = "wss://r.test"; @@ -14,12 +15,70 @@ const RELAY_KEY = encodeURIComponent(RELAY); function makeStore(channels = {}) { return { version: 1, channels }; } +const E = (muted, updatedAt, rev) => ({ muted, updatedAt, rev }); + +// Multi-slot timer fake keyed by delay, for overlapping-publish tests. Mirrors +// the sections suite convention (channelSectionsSync.test.mjs:407-432). +function makeMultiTimerWindow() { + const storage = new Map(); + const timers = new Map(); + let nextId = 1; + const win = { + localStorage: { + getItem: (k) => storage.get(k) ?? null, + setItem: (k, v) => storage.set(k, v), + removeItem: (k) => storage.delete(k), + }, + setTimeout: (fn, ms) => { + const id = nextId++; + timers.set(id, { fn, ms }); + return id; + }, + clearTimeout: (id) => timers.delete(id), + }; + return { + win, + storage, + timers, + fireDelay: async (ms) => { + const entry = [...timers.entries()].find(([, v]) => v.ms === ms); + assert.ok(entry, `expected a timer scheduled at ${ms}ms`); + timers.delete(entry[0]); + entry[1].fn(); + for (let i = 0; i < 50; i++) await Promise.resolve(); + }, + hasDelay: (ms) => [...timers.values()].some((t) => t.ms === ms), + }; +} + +// ─── observe() / high-water ingestion ───────────────────────────────────────── + +test("observe: high-water is per-channel max of rev and updatedAt, monotonic", () => { + const fw = makeFakeWindow(); + const restore = installFakeWindow(fw); + try { + const m = new ChannelMuteSyncManager("pk", RELAY); + m.observe(makeStore({ a: E(true, 100, 3), b: E(false, 50, 1) })); + assert.equal(m.maxRevSeen("a"), 3); + assert.equal(m.maxUpdatedAtSeen("a"), 100); + // A later observation raises each dimension independently; a lower one + // never regresses either. + m.observe(makeStore({ a: E(true, 90, 5) })); + assert.equal(m.maxRevSeen("a"), 5, "rev raised"); + assert.equal(m.maxUpdatedAtSeen("a"), 100, "updatedAt not regressed"); + m.observe(makeStore({ a: E(true, 200, 2) })); + assert.equal(m.maxUpdatedAtSeen("a"), 200, "updatedAt raised"); + assert.equal(m.maxRevSeen("a"), 5, "rev not regressed"); + // Unseen channel reports zero on both dimensions. + assert.equal(m.maxRevSeen("never"), 0); + assert.equal(m.maxUpdatedAtSeen("never"), 0); + } finally { + restore(); + } +}); // ─── destroy() must cancel pending publish, not flush ───────────────────────── -// Regression guard for the community-switch cross-relay publish vector: -// mute a channel in relay A → destroy() called (relayUrl dep change) → -// no publish should fire. test("destroy: cancels pending publish without flushing to the relay", () => { const publishCalls = []; mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); @@ -31,9 +90,9 @@ test("destroy: cancels pending publish without flushing to the relay", () => { const restore = installFakeWindow(fw); try { const manager = new ChannelMuteSyncManager("pk-test", RELAY); - manager.publishMutes(makeStore({ ch1: { muted: true, updatedAt: 100 } })); + manager.publishMutes(makeStore({ ch1: E(true, 100, 1) })); manager.destroy(); - assert.equal(publishCalls.length, 0); + assert.equal(publishCalls.length, 0, "no publish after destroy"); assert.equal(manager.getPendingMuteStore(), null); } finally { restore(); @@ -60,12 +119,16 @@ test("destroy: aborts in-flight doPublish after fetchOwnBlobBeforePublish resolv const restore = installFakeWindow(fw); try { const manager = new ChannelMuteSyncManager("pk-race", RELAY); - manager.publishMutes(makeStore({ ch1: { muted: true, updatedAt: 100 } })); + manager.publishMutes(makeStore({ ch1: E(true, 100, 1) })); fw._fireTimer(); manager.destroy(); releaseFetch(); await new Promise((r) => setTimeout(r, 0)); - assert.equal(publishCalls.length, 0); + assert.equal( + publishCalls.length, + 0, + "publishEvent must not be called after destroy", + ); } finally { restore(); mock.reset(); @@ -83,9 +146,175 @@ test("destroy: is safe to call with no pending publish", () => { } }); +// ─── Generation CAS: A-in-flight → B-click → A-completes (both variants) ────── + +// Finding 2 (A succeeds): an older in-flight publish that completes after a +// newer edit is queued must NOT clear the newer edit's pending store/outbox, +// and B must reach the relay via the completion re-drive. Mutation: dropping the +// generation CAS in discardPending lets A's success null out B's pending+outbox. +test("A-in-flight → B-click → A-succeeds: B stays pending and B publishes", async () => { + let releaseFirst = null; + const publishedContents = []; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + if (releaseFirst === null && publishedContents.length === 0) { + return new Promise((res) => { + releaseFirst = res; + }); + } + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + const outboxKey = `buzz-channel-mutes-outbox.v1:pk-ab:${RELAY_KEY}`; + try { + const manager = new ChannelMuteSyncManager("pk-ab", RELAY); + const storeA = makeStore({ a: E(true, 100, 1) }); + const storeB = makeStore({ b: E(true, 101, 1) }); + + manager.publishMutes(storeA); + await t.fireDelay(2000); // doPublish(A) awaits publishEvent + while (releaseFirst === null) await Promise.resolve(); + + // B arrives while A is in flight. + manager.publishMutes(storeB); + assert.deepEqual( + Object.keys(manager.getPendingMuteStore().channels), + ["b"], + "B is now pending", + ); + assert.ok(t.storage.get(outboxKey), "outbox holds B"); + + // A completes — must NOT clear B. + releaseFirst(); + for (let i = 0; i < 50; i++) await Promise.resolve(); + assert.deepEqual( + Object.keys(manager.getPendingMuteStore()?.channels ?? {}), + ["b"], + "older A completion leaves B pending", + ); + assert.ok(t.storage.get(outboxKey), "older A completion leaves B outbox"); + + // B's own debounce fires and B reaches the relay (published) with no kick. + const capturedBefore = tauri.capturedPlaintext(); + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + const captured = tauri.capturedPlaintext(); + assert.ok( + captured && captured !== capturedBefore && captured.includes('"b"'), + "B is published to the relay", + ); + assert.equal( + manager.getPendingMuteStore(), + null, + "B cleared after publish", + ); + assert.equal(t.storage.get(outboxKey), undefined, "B outbox cleared"); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// Finding 2 (A fails): A's publish rejects after B is queued. B must remain +// pending and be published by the serialized re-drive / retry — no manual kick. +test("A-in-flight → B-click → A-fails: B remains pending and B publishes", async () => { + let rejectFirst = null; + let publishCount = 0; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + publishCount++; + if (publishCount === 1) { + return new Promise((_res, rej) => { + rejectFirst = () => rej(new Error("socket error")); + }); + } + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + const outboxKey = `buzz-channel-mutes-outbox.v1:pk-abfail:${RELAY_KEY}`; + try { + const manager = new ChannelMuteSyncManager("pk-abfail", RELAY); + manager.publishMutes(makeStore({ a: E(true, 100, 1) })); + await t.fireDelay(2000); + while (rejectFirst === null) await Promise.resolve(); + + manager.publishMutes(makeStore({ b: E(true, 101, 1) })); + rejectFirst(); // A fails + for (let i = 0; i < 50; i++) await Promise.resolve(); + + assert.deepEqual( + Object.keys(manager.getPendingMuteStore()?.channels ?? {}), + ["b"], + "B still pending after A's failure", + ); + assert.ok(t.storage.get(outboxKey), "B outbox intact after A's failure"); + + // B's debounce fires and B publishes successfully. + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + const captured = tauri.capturedPlaintext(); + assert.ok(captured?.includes('"b"'), "B published"); + assert.equal(manager.getPendingMuteStore(), null, "B cleared"); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// ─── Bounded-backoff retry: failed publish on a healthy socket, no later edit ─ + +// Finding 2: a transient publish failure with the socket open and NO further +// click must self-heal via the bounded-backoff retry — the pending edit is kept +// and a retry timer is scheduled. Mutation: dropping scheduleRetry leaves the +// edit stranded (Will's "make another change to kick it" symptom). +test("failed publish schedules a bounded-backoff retry and keeps the pending edit", async () => { + let publishCount = 0; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + publishCount++; + if (publishCount === 1) return Promise.reject(new Error("timeout")); + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + try { + const manager = new ChannelMuteSyncManager("pk-retry", RELAY); + manager.publishMutes(makeStore({ a: E(true, 100, 1) })); + await t.fireDelay(2000); // debounce → doPublish → publishEvent rejects + assert.ok( + manager.getPendingMuteStore() !== null, + "pending edit retained after failure", + ); + assert.ok(t.hasDelay(2000), "a retry timer at RETRY_BASE_MS is scheduled"); + + // The retry fires and the second publish succeeds → pending cleared. + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + assert.equal(publishCount, 2, "retry re-published"); + assert.equal( + manager.getPendingMuteStore(), + null, + "pending cleared on retry success", + ); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + // ─── Boot seed-publish guard (the revert-fix regression suite) ───────────────── -// 1. fetch failed → hold, pendingStore null (mutation: remove failed guard → seed queued) test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstrap", async () => { mock.method(relayClient, "fetchEvents", () => Promise.reject(new Error("relay timeout")), @@ -95,9 +324,7 @@ test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstr const restore = installFakeWindow(fw); try { const manager = new ChannelMuteSyncManager("pk-fail", RELAY); - const result = await manager.bootstrap( - makeStore({ ch1: { muted: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.equal(manager.getPendingMuteStore(), null); } finally { @@ -106,7 +333,6 @@ test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstr } }); -// 2. absent + prior watermark → hold, pendingStore null (mutation: clear watermark → seed queued) test("revert-fix: absent fetch with prior watermark blocks seed-publish via bootstrap", async () => { mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); mock.method(relayClient, "publishEvent", () => Promise.resolve()); @@ -118,16 +344,7 @@ test("revert-fix: absent fetch with prior watermark blocks seed-publish via boot const restore = installFakeWindow(fw); try { const manager = new ChannelMuteSyncManager("pk-stale", RELAY); - assert.ok( - Number( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-mutes:pk-stale:${RELAY_KEY}`, - ) ?? "0", - ) > 0, - ); - const result = await manager.bootstrap( - makeStore({ ch1: { muted: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.equal(manager.getPendingMuteStore(), null); } finally { @@ -136,7 +353,6 @@ test("revert-fix: absent fetch with prior watermark blocks seed-publish via boot } }); -// 3. absent + zero watermark + non-empty → seed queued (mutation: remove seed call → pendingStore null) test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sync preserved)", async () => { mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); mock.method(relayClient, "publishEvent", () => Promise.resolve()); @@ -144,15 +360,7 @@ test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sy const restore = installFakeWindow(fw); try { const manager = new ChannelMuteSyncManager("pk-fresh", RELAY); - assert.equal( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-mutes:pk-fresh:${RELAY_KEY}`, - ), - null, - ); - const result = await manager.bootstrap( - makeStore({ ch1: { muted: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.ok(manager.getPendingMuteStore() !== null); } finally { @@ -161,8 +369,6 @@ test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sy } }); -// 4. relay-A / relay-B watermark isolation -// Mutation: using pubkey-only key (no relay) makes relay A's head suppress relay B's first-sync. test("revert-fix: relay-A watermark does not suppress first-sync seed on relay-B", async () => { const relayA = "wss://a.relay.test"; const relayB = "wss://b.relay.test"; @@ -176,16 +382,7 @@ test("revert-fix: relay-A watermark does not suppress first-sync seed on relay-B const restore = installFakeWindow(fw); try { const managerB = new ChannelMuteSyncManager("pk-iso", relayB); - assert.equal( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-mutes:pk-iso:${encodeURIComponent(relayB)}`, - ), - null, - "relay B watermark must be independent of relay A head", - ); - const result = await managerB.bootstrap( - makeStore({ ch1: { muted: true, updatedAt: 1 } }), - ); + const result = await managerB.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.ok( managerB.getPendingMuteStore() !== null, diff --git a/desktop/src/features/sidebar/lib/channelMutesSync.ts b/desktop/src/features/sidebar/lib/channelMutesSync.ts index 5e8a17e74d..ac88f3c4bc 100644 --- a/desktop/src/features/sidebar/lib/channelMutesSync.ts +++ b/desktop/src/features/sidebar/lib/channelMutesSync.ts @@ -7,8 +7,10 @@ import { import type { RelayEvent } from "@/shared/api/types"; import { KIND_CHANNEL_MUTES } from "@/shared/constants/kinds"; import { + clearChannelMutesOutbox, mergeStores, parseMutePayload, + writeChannelMutesOutbox, type ChannelMuteStore, } from "./channelMutesStorage"; import { @@ -22,6 +24,12 @@ const D_TAG = "channel-mutes"; const BLOB_TYPE = D_TAG; const DEBOUNCE_MS = 2_000; +// Bounded backoff for a retained pending edit whose publish failed transiently +// (timeout / socket error) on an otherwise-healthy socket, so it does not wait +// for a reconnect that may never fire. +const RETRY_BASE_MS = 2_000; +const RETRY_MAX_MS = 30_000; + export type RemoteMutes = { store: ChannelMuteStore; createdAt: number; @@ -43,10 +51,30 @@ export class ChannelMuteSyncManager { private pubkey: string; private relayUrl: string; private debounceTimer: number | null = null; + private retryTimer: number | null = null; + private retryDelayMs = RETRY_BASE_MS; private lastRemoteCreatedAt: number; private pendingStore: ChannelMuteStore | null = null; + // Monotonic id for the current pending edit. Every publishMutes() bumps it; + // every scheduled publish/retry captures the value it was queued for. A + // completion (success or no-op) may only clear pending state via + // compare-and-swap on this generation, so an older in-flight publish can + // never erase a newer edit that arrived while it was in flight. + private pendingGeneration = 0; + // Publish cycles are serialized: at most one runs at a time. A newer edit + // queued while a cycle is in flight defers; the in-flight cycle's completion + // re-drives it. Serialization guarantees there is never more than one + // fetch/publish sequence touching shared manager state. + private publishInFlight = false; private lastPublishedStore: ChannelMuteStore | null = null; private destroyed = false; + // Per-channel high-water of every `rev` and `updatedAt` this manager has + // observed (bootstrap, live, reconnect, reconcile, pre-publish, cross-window + // storage, and initial persisted state). A click reads both so its minted + // `updatedAt = max(now, maxUpdatedAtSeen)` never regresses below observed + // state (the read-state logical-monotonic idiom), and `rev = maxRevSeen + 1` + // wins the resulting same-second tie. + private highWater = new Map(); constructor(pubkey: string, relayUrl: string) { this.pubkey = pubkey; @@ -54,6 +82,29 @@ export class ChannelMuteSyncManager { this.lastRemoteCreatedAt = readWatermark(pubkey, BLOB_TYPE, relayUrl); } + /** + * Ingest a store into the per-channel high-water. Called synchronously before + * any merge is applied to React state, so a click that follows reads a current + * watermark on both dimensions. Monotonic (`Math.max`) and idempotent. + */ + observe(store: ChannelMuteStore): void { + for (const [id, entry] of Object.entries(store.channels)) { + const cur = this.highWater.get(id) ?? { rev: 0, updatedAt: 0 }; + this.highWater.set(id, { + rev: Math.max(cur.rev, entry.rev), + updatedAt: Math.max(cur.updatedAt, entry.updatedAt), + }); + } + } + + maxRevSeen(id: string): number { + return this.highWater.get(id)?.rev ?? 0; + } + + maxUpdatedAtSeen(id: string): number { + return this.highWater.get(id)?.updatedAt ?? 0; + } + async fetchRemoteMutes(): Promise> { try { const events = await relayClient.fetchEvents({ @@ -71,6 +122,7 @@ export class ChannelMuteSyncManager { if (!result) { return { status: "failed", createdAt: event.created_at }; } + this.observe(result.store); return { status: "found", data: result, @@ -94,6 +146,10 @@ export class ChannelMuteSyncManager { window.clearTimeout(this.debounceTimer); this.debounceTimer = null; } + if (this.retryTimer !== null) { + window.clearTimeout(this.retryTimer); + this.retryTimer = null; + } } getPendingMuteStore(): ChannelMuteStore | null { @@ -102,15 +158,51 @@ export class ChannelMuteSyncManager { publishMutes(store: ChannelMuteStore): void { this.pendingStore = store; + ++this.pendingGeneration; + // Persist synchronously so a click made <2s before quit/community-switch + // survives teardown and resumes on next mount (durable outbox). + writeChannelMutesOutbox(this.pubkey, store, this.relayUrl); if (this.debounceTimer !== null) { window.clearTimeout(this.debounceTimer); } + // A fresh edit supersedes any retry scheduled for the previous generation. + if (this.retryTimer !== null) { + window.clearTimeout(this.retryTimer); + this.retryTimer = null; + } + this.retryDelayMs = RETRY_BASE_MS; this.debounceTimer = window.setTimeout(() => { this.debounceTimer = null; - void this.doPublish(store); + this.startCycle(); }, DEBOUNCE_MS); } + /** + * Serialize publish cycles: at most one runs at a time. A debounce/retry timer + * that fires while a cycle is in flight defers — the in-flight cycle's + * completion re-drives if a pending edit still needs publishing. A newer edit + * queued during a cycle cannot start its own concurrent cycle, so a stale + * generation can never publish after a newer edit exists. + */ + private startCycle(): void { + if (this.destroyed || this.pendingStore === null) return; + if (this.publishInFlight) return; + const store = this.pendingStore; + const gen = this.pendingGeneration; + this.publishInFlight = true; + void this.doPublish(store, gen).finally(() => { + this.publishInFlight = false; + if ( + !this.destroyed && + this.pendingStore !== null && + this.debounceTimer === null && + this.retryTimer === null + ) { + this.startCycle(); + } + }); + } + private async fetchOwnBlobBeforePublish( store: ChannelMuteStore, ): Promise { @@ -127,6 +219,9 @@ export class ChannelMuteSyncManager { this.recordRemoteHead(event.created_at); const remote = await decryptAndParse(event); if (!remote) return store; + this.observe(remote.store); + // Max-merge: the local edit's per-entry winners survive by construction + // and any newer remote entries fold in, so no adopt step is needed. return mergeStores(store, remote.store); } catch { return store; @@ -144,22 +239,55 @@ export class ChannelMuteSyncManager { if ( !last || last.muted !== current.muted || - last.updatedAt !== current.updatedAt + last.updatedAt !== current.updatedAt || + last.rev !== current.rev ) return false; } return true; } - private async doPublish(store: ChannelMuteStore): Promise { + /** + * Clear the in-memory pending edit and its durable outbox — but only if the + * completing publish still owns the current generation. A publish for an + * older edit that finishes after a newer edit was queued must leave the newer + * edit (and its retry state) untouched. + */ + private discardPending(gen: number): void { + if (gen !== this.pendingGeneration) return; + this.pendingStore = null; + clearChannelMutesOutbox(this.pubkey, this.relayUrl); + } + + /** Schedule a bounded-backoff retry of the retained pending edit. */ + private scheduleRetry(gen: number): void { + if (this.destroyed || this.pendingStore === null) return; + // A newer edit has superseded this one; its own timer owns the retry. + if (gen !== this.pendingGeneration) return; + if (this.retryTimer !== null) return; + const delay = this.retryDelayMs; + this.retryDelayMs = Math.min(this.retryDelayMs * 2, RETRY_MAX_MS); + this.retryTimer = window.setTimeout(() => { + this.retryTimer = null; + this.startCycle(); + }, delay); + } + + private async doPublish(store: ChannelMuteStore, gen: number): Promise { + // A newer edit was queued after this publish was scheduled; it owns the + // pending state and will publish the latest store — abandon this stale run. + if (gen !== this.pendingGeneration) return; try { const merged = await this.fetchOwnBlobBeforePublish(store); // Guard: manager may have been destroyed while fetchOwnBlobBeforePublish - // was awaited (community switch during in-flight fetch). If so, abort - // before touching the relay. + // was awaited (community switch during in-flight fetch). if (this.destroyed) return; + // A newer edit was queued while we awaited the pre-publish fetch. It owns + // convergence now; the serialized cycle re-drives for it once this run + // unwinds. + if (gen !== this.pendingGeneration) return; if (this.isIdenticalToLastPublished(merged)) { - this.pendingStore = null; + this.discardPending(gen); return; } const payload = { @@ -180,17 +308,32 @@ export class ChannelMuteSyncManager { ["t", D_TAG], // relay discoverability; not used in our filters ], }); - if (this.destroyed) return; + // Final guard immediately before the network call: a newer edit may have + // been queued during the encrypt/sign await, or the manager destroyed. + if (this.destroyed || gen !== this.pendingGeneration) return; await relayClient.publishEvent( event, "Timed out publishing channel mutes.", "Failed to publish channel mutes.", ); this.recordRemoteHead(event.created_at); - this.lastPublishedStore = merged; - this.pendingStore = null; + this.observe(merged); + // Only claim this store as the published head if it is still the current + // edit; a newer edit queued mid-flight owns lastPublishedStore now. + if (gen === this.pendingGeneration) { + this.lastPublishedStore = merged; + this.retryDelayMs = RETRY_BASE_MS; + } + this.discardPending(gen); } catch (error) { + if (this.destroyed) return; + // Transient publish failure (timeout / socket error). Keep the pending + // edit and retry with backoff rather than waiting for a reconnect that a + // healthy socket never fires. Max-merge makes a duplicate publish + // idempotent, so a lost-ACK write that the relay actually accepted is + // harmless to re-send. console.warn("[channelMutesSync] publish failed:", error); + this.scheduleRetry(gen); } } @@ -211,6 +354,7 @@ export class ChannelMuteSyncManager { this.recordRemoteHead(event.created_at); void decryptAndParse(event).then((result) => { if (result) { + this.observe(result.store); onUpdate(result); } }); @@ -223,6 +367,9 @@ export class ChannelMuteSyncManager { * delegates the seed/hold/apply-remote decision to `runBootstrap`. */ async bootstrap(localStore: ChannelMuteStore) { + // Seed the high-water from the caller's persisted local store so a click + // before the remote fetch resolves already reflects retained entries. + this.observe(localStore); const fetchResult = await this.fetchRemoteMutes(); return runBootstrap({ fetchResult, @@ -236,10 +383,10 @@ export class ChannelMuteSyncManager { destroy(): void { // Cancel any pending publish and mark this manager as destroyed so any // in-flight doPublish() calls abort before reaching relayClient. - // Pending debounce-window changes are intentionally dropped: flushing - // could publish relay A's state to relay B via the shared relayClient - // singleton. Local entries survive because the apply/publish paths merge - // per-entry via mergeStores, so no local work is permanently lost. + // Debounce-window changes are NOT lost: publishMutes persisted them to the + // durable outbox synchronously, and the next mount resumes them. Flushing + // here is still avoided — it could publish relay A's state to relay B via + // the shared relayClient singleton. this.destroyed = true; this.cancelPendingMutePublish(); this.pendingStore = null; diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs index 1585a42d47..8239b9e8eb 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs @@ -4,29 +4,50 @@ import test from "node:test"; import { boundStarStore, MAX_CHANNEL_STAR_ENTRIES, - parseStarPayload, mergeStores, + parseStarPayload, starredChannelIdsFromStore, } from "./channelStarsStorage.ts"; // ── parseStarPayload ────────────────────────────────────────────────────────── -test("parseStarPayload: valid payload with channels returns store", () => { +test("parseStarPayload: valid payload with channels returns store (rev preserved)", () => { const payload = { version: 1, channels: { - "chan-1": { starred: true, updatedAt: 1000 }, - "chan-2": { starred: false, updatedAt: 2000 }, + "chan-1": { starred: true, updatedAt: 1000, rev: 3 }, + "chan-2": { starred: false, updatedAt: 2000, rev: 0 }, }, }; - const result = parseStarPayload(payload); - assert.deepEqual(result, { + assert.deepEqual(parseStarPayload(payload), payload); +}); + +test("parseStarPayload: missing rev normalizes to 0 (old-build blob, entry kept)", () => { + const result = parseStarPayload({ + version: 1, + channels: { "chan-1": { starred: true, updatedAt: 1000 } }, + }); + assert.deepEqual(result.channels["chan-1"], { + starred: true, + updatedAt: 1000, + rev: 0, + }); +}); + +test("parseStarPayload: malformed rev (string / negative / non-integer / NaN) normalizes to 0", () => { + const result = parseStarPayload({ version: 1, channels: { - "chan-1": { starred: true, updatedAt: 1000 }, - "chan-2": { starred: false, updatedAt: 2000 }, + str: { starred: true, updatedAt: 1, rev: "5" }, + neg: { starred: true, updatedAt: 1, rev: -2 }, + frac: { starred: true, updatedAt: 1, rev: 1.5 }, + nan: { starred: true, updatedAt: 1, rev: NaN }, }, }); + for (const id of ["str", "neg", "frac", "nan"]) { + assert.equal(result.channels[id].rev, 0, `${id} rev normalized to 0`); + assert.equal(result.channels[id].starred, true, `${id} entry kept`); + } }); test("parseStarPayload: missing version returns null", () => { @@ -48,18 +69,15 @@ test("parseStarPayload: wrong version returns null", () => { ); }); -test("parseStarPayload: null input returns null", () => { +test("parseStarPayload: null / non-object input returns null", () => { assert.equal(parseStarPayload(null), null); -}); - -test("parseStarPayload: non-object input returns null", () => { assert.equal(parseStarPayload("string"), null); assert.equal(parseStarPayload(42), null); assert.equal(parseStarPayload(true), null); }); test("parseStarPayload: malformed channel entries missing starred/updatedAt are filtered out", () => { - const payload = { + const result = parseStarPayload({ version: 1, channels: { "no-starred": { updatedAt: 1000 }, @@ -69,267 +87,251 @@ test("parseStarPayload: malformed channel entries missing starred/updatedAt are "updated-at-wrong-type": { starred: true, updatedAt: "now" }, null: null, }, - }; - const result = parseStarPayload(payload); + }); assert.deepEqual(result, { version: 1, - channels: { - valid: { starred: false, updatedAt: 500 }, - }, + channels: { valid: { starred: false, updatedAt: 500, rev: 0 } }, }); }); test("parseStarPayload: NaN/Infinity/negative updatedAt entries are filtered out", () => { - const payload = { + const result = parseStarPayload({ version: 1, channels: { nan: { starred: true, updatedAt: NaN }, inf: { starred: true, updatedAt: Infinity }, "neg-inf": { starred: true, updatedAt: -Infinity }, neg: { starred: true, updatedAt: -1 }, - valid: { starred: true, updatedAt: 100 }, - }, - }; - const result = parseStarPayload(payload); - assert.deepEqual(result, { - version: 1, - channels: { valid: { starred: true, updatedAt: 100 } }, - }); -}); - -test("parseStarPayload: empty channels returns store with empty channels", () => { - const result = parseStarPayload({ version: 1, channels: {} }); - assert.deepEqual(result, { version: 1, channels: {} }); -}); - -test("parseStarPayload: version 1 with no channels key returns store with empty channels", () => { - const result = parseStarPayload({ version: 1 }); - assert.deepEqual(result, { version: 1, channels: {} }); -}); - -// ── mergeStores ─────────────────────────────────────────────────────────────── - -test("mergeStores: non-overlapping channels returns union of both", () => { - const local = { - version: 1, - channels: { "chan-a": { starred: true, updatedAt: 100 } }, - }; - const remote = { - version: 1, - channels: { "chan-b": { starred: false, updatedAt: 200 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result, { - version: 1, - channels: { - "chan-a": { starred: true, updatedAt: 100 }, - "chan-b": { starred: false, updatedAt: 200 }, + valid: { starred: true, updatedAt: 100, rev: 2 }, }, }); -}); - -test("mergeStores: overlapping channel with remote newer takes remote", () => { - const local = { + assert.deepEqual(result, { version: 1, - channels: { "chan-a": { starred: false, updatedAt: 100 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { starred: true, updatedAt: 200 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { - starred: true, - updatedAt: 200, + channels: { valid: { starred: true, updatedAt: 100, rev: 2 } }, }); }); -test("mergeStores: overlapping channel with local newer takes local", () => { - const local = { +test("parseStarPayload: empty channels / no channels key returns empty store", () => { + assert.deepEqual(parseStarPayload({ version: 1, channels: {} }), { version: 1, - channels: { "chan-a": { starred: true, updatedAt: 300 } }, - }; - const remote = { + channels: {}, + }); + assert.deepEqual(parseStarPayload({ version: 1 }), { version: 1, - channels: { "chan-a": { starred: false, updatedAt: 100 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { - starred: true, - updatedAt: 300, + channels: {}, }); }); -test("mergeStores: overlapping channel with same updatedAt local wins", () => { - const local = { +// ── mergeStores: tuple order (updatedAt → rev → value) ──────────────────────── + +const E = (starred, updatedAt, rev) => ({ starred, updatedAt, rev }); +const S = (entry) => ({ version: 1, channels: { c: entry } }); + +test("mergeStores: non-overlapping channels returns union", () => { + const result = mergeStores( + { version: 1, channels: { a: E(true, 100, 1) } }, + { version: 1, channels: { b: E(false, 200, 1) } }, + ); + assert.deepEqual(result, { version: 1, - channels: { "chan-a": { starred: true, updatedAt: 500 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { starred: false, updatedAt: 500 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { - starred: true, - updatedAt: 500, + channels: { a: E(true, 100, 1), b: E(false, 200, 1) }, }); }); +test("mergeStores: strictly-later updatedAt wins regardless of rev (primary key)", () => { + // Later updatedAt with LOWER rev still wins — updatedAt is primary. This is + // the old-build interop case: an old build's rev-0 fresh edit beats a stale + // rev-bearing new-build entry. + const result = mergeStores(S(E(false, 200, 0)), S(E(true, 100, 7))); + assert.deepEqual(result.channels.c, E(false, 200, 0)); +}); + +test("mergeStores: equal updatedAt → higher rev wins (same-second tiebreak)", () => { + const result = mergeStores(S(E(false, 100, 5)), S(E(true, 100, 2))); + assert.deepEqual(result.channels.c, E(false, 100, 5)); +}); + +test("mergeStores: equal updatedAt AND equal rev → starred=true wins (leaf)", () => { + const result = mergeStores(S(E(false, 100, 3)), S(E(true, 100, 3))); + assert.deepEqual(result.channels.c, E(true, 100, 3)); +}); + test("mergeStores: unstar with higher updatedAt overrides star", () => { - const local = { - version: 1, - channels: { "chan-a": { starred: true, updatedAt: 100 } }, - }; - const remote = { - version: 1, - channels: { "chan-a": { starred: false, updatedAt: 999 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels["chan-a"], { - starred: false, - updatedAt: 999, - }); + const result = mergeStores(S(E(true, 100, 9)), S(E(false, 999, 1))); + assert.deepEqual(result.channels.c, E(false, 999, 1)); }); -test("mergeStores: empty local returns remote entries", () => { - const local = { version: 1, channels: {} }; - const remote = { - version: 1, - channels: { "chan-b": { starred: true, updatedAt: 42 } }, - }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels, { - "chan-b": { starred: true, updatedAt: 42 }, - }); -}); - -test("mergeStores: empty remote returns local entries", () => { - const local = { - version: 1, - channels: { "chan-a": { starred: false, updatedAt: 10 } }, - }; - const remote = { version: 1, channels: {} }; - const result = mergeStores(local, remote); - assert.deepEqual(result.channels, { - "chan-a": { starred: false, updatedAt: 10 }, - }); -}); - -test("mergeStores: both empty returns empty", () => { - const result = mergeStores( - { version: 1, channels: {} }, +test("mergeStores: empty local / empty remote / both empty", () => { + assert.deepEqual( + mergeStores({ version: 1, channels: {} }, S(E(true, 42, 1))).channels.c, + E(true, 42, 1), + ); + assert.deepEqual( + mergeStores(S(E(false, 10, 2)), { version: 1, channels: {} }).channels.c, + E(false, 10, 2), + ); + assert.deepEqual( + mergeStores({ version: 1, channels: {} }, { version: 1, channels: {} }), { version: 1, channels: {} }, ); - assert.deepEqual(result, { version: 1, channels: {} }); }); +// ── mergeStores: algebra (commutativity, associativity, idempotence) ────────── + +function randEntry(rng) { + return { + starred: rng() > 0.5, + updatedAt: Math.floor(rng() * 5), + rev: Math.floor(rng() * 5), + }; +} +function randStore(rng, ids) { + const channels = {}; + for (const id of ids) if (rng() > 0.3) channels[id] = randEntry(rng); + return { version: 1, channels }; +} +// Deterministic LCG so failures reproduce. +function lcg(seed) { + let s = seed >>> 0; + return () => { + s = (s * 1664525 + 1013904223) >>> 0; + return s / 4294967296; + }; +} + +test("mergeStores: commutative — merge(a,b) === merge(b,a)", () => { + const rng = lcg(12345); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + assert.deepEqual(mergeStores(a, b), mergeStores(b, a)); + } +}); + +test("mergeStores: associative — merge(merge(a,b),c) === merge(a,merge(b,c))", () => { + const rng = lcg(67890); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + const c = randStore(rng, ids); + assert.deepEqual( + mergeStores(mergeStores(a, b), c), + mergeStores(a, mergeStores(b, c)), + ); + } +}); + +test("mergeStores: idempotent — merge(a, merge(a,b)) === merge(a,b)", () => { + const rng = lcg(24680); + const ids = ["a", "b", "c", "d"]; + for (let i = 0; i < 200; i++) { + const a = randStore(rng, ids); + const b = randStore(rng, ids); + const ab = mergeStores(a, b); + assert.deepEqual(mergeStores(a, ab), ab); + assert.deepEqual(mergeStores(ab, ab), ab); + } +}); + +// ── v1-blob bidirectional compatibility ─────────────────────────────────────── + +test("v1 compat: a rev-carrying blob round-trips through a rev-less parser view", () => { + // Simulate an old build reading our blob: JSON-serialize our rev-carrying + // payload, parse it back — version stays 1 so it is NOT rejected, and the + // core fields survive (old build simply ignores rev). + const ours = { + version: 1, + channels: { c: { starred: true, updatedAt: 100, rev: 7 } }, + }; + const roundTripped = parseStarPayload(JSON.parse(JSON.stringify(ours))); + assert.equal(roundTripped.version, 1, "version stays 1 — old build accepts"); + assert.equal(roundTripped.channels.c.starred, true); + assert.equal(roundTripped.channels.c.updatedAt, 100); +}); + +test("v1 compat: old-build unstar (no rev, updatedAt+1) beats our stale star", () => { + // New build wrote {starred:true, rev:7, updatedAt:t}; old build (no rev) + // unstars producing {starred:false, updatedAt:t+1}. The unstar wins on the + // primary updatedAt key — old builds can still edit upgraded channels. + const ours = S(E(true, 100, 7)); + const oldBuildUnstar = parseStarPayload({ + version: 1, + channels: { c: { starred: false, updatedAt: 101 } }, + }); + assert.deepEqual(mergeStores(ours, oldBuildUnstar).channels.c, { + starred: false, + updatedAt: 101, + rev: 0, + }); +}); + +// ── boundStarStore ──────────────────────────────────────────────────────────── + test("boundStarStore: retains newest entries regardless of starred value", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ - `active-${index}`, - { starred: true, updatedAt: index + 1 }, + Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 1, 0), ]), ); - channels["old-false"] = { starred: false, updatedAt: 0 }; - channels["new-false"] = { starred: false, updatedAt: 9999 }; - + channels["old-false"] = E(false, 0, 0); + channels["new-false"] = E(false, 9999, 0); const result = boundStarStore({ version: 1, channels }); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_STAR_ENTRIES); assert.equal(result.channels["old-false"], undefined); - assert.deepEqual(result.channels["new-false"], { - starred: false, - updatedAt: 9999, - }); + assert.deepEqual(result.channels["new-false"], E(false, 9999, 0)); assert.equal(result.channels["active-0"], undefined); - assert.deepEqual(result.channels["active-1"], { - starred: true, - updatedAt: 2, - }); }); test("boundStarStore: uses channel ID as an updatedAt tie-breaker", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES + 1 }, (_, index) => [ - `channel-${String(MAX_CHANNEL_STAR_ENTRIES - index).padStart(3, "0")}`, - { starred: true, updatedAt: 1 }, + Array.from({ length: MAX_CHANNEL_STAR_ENTRIES + 1 }, (_, i) => [ + `channel-${String(MAX_CHANNEL_STAR_ENTRIES - i).padStart(3, "0")}`, + E(true, 1, 0), ]), ); - const result = boundStarStore({ version: 1, channels }); - assert.equal(result.channels["channel-000"], undefined); - assert.deepEqual(result.channels["channel-500"], { - starred: true, - updatedAt: 1, - }); + assert.deepEqual(result.channels["channel-500"], E(true, 1, 0)); }); -test("boundStarStore: preserves a same-second star mutation by key", () => { +test("boundStarStore: preserves a same-second mutation by key", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ - `z-channel-${String(index).padStart(3, "0")}`, - { starred: true, updatedAt: 1 }, + Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, i) => [ + `z-channel-${String(i).padStart(3, "0")}`, + E(true, 1, 0), ]), ); - channels["a-target"] = { starred: true, updatedAt: 1 }; - + channels["a-target"] = E(false, 1, 1); const result = boundStarStore({ version: 1, channels }, "a-target"); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_STAR_ENTRIES); - assert.deepEqual(result.channels["a-target"], { - starred: true, - updatedAt: 1, - }); - assert.equal(result.channels["z-channel-000"], undefined); -}); - -test("boundStarStore: preserves a same-second unstar mutation by key", () => { - const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ - `z-channel-${String(index).padStart(3, "0")}`, - { starred: true, updatedAt: 1 }, - ]), - ); - channels["a-target"] = { starred: false, updatedAt: 1 }; - - const result = boundStarStore({ version: 1, channels }, "a-target"); - - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_STAR_ENTRIES); - assert.deepEqual(result.channels["a-target"], { - starred: false, - updatedAt: 1, - }); + assert.deepEqual(result.channels["a-target"], E(false, 1, 1)); assert.equal(result.channels["z-channel-000"], undefined); }); test("mergeStores: a fresh at-capacity unstar defeats an older remote star", () => { const channels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ - `active-${index}`, - { starred: true, updatedAt: index + 1 }, + Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 1, 0), ]), ); - channels["unstarred"] = { starred: false, updatedAt: 9999 }; + channels.unstarred = E(false, 9999, 1); const bounded = boundStarStore({ version: 1, channels }); - const result = mergeStores(bounded, { version: 1, - channels: { unstarred: { starred: true, updatedAt: 9998 } }, - }); - - assert.deepEqual(result.channels.unstarred, { - starred: false, - updatedAt: 9999, + channels: { unstarred: E(true, 9998, 5) }, }); + assert.deepEqual(result.channels.unstarred, E(false, 9999, 1)); }); test("mergeStores: evicted remote ID re-enters and the oldest state is re-trimmed", () => { const localChannels = Object.fromEntries( - Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ - `active-${index}`, - { starred: true, updatedAt: index + 10 }, + Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, i) => [ + `active-${i}`, + E(true, i + 10, 0), ]), ); const result = mergeStores( @@ -337,59 +339,84 @@ test("mergeStores: evicted remote ID re-enters and the oldest state is re-trimme { version: 1, channels: { - "evicted-id": { starred: true, updatedAt: 9999 }, - "active-0": { starred: false, updatedAt: 9998 }, + "evicted-id": E(true, 9999, 0), + "active-0": E(false, 9998, 0), }, }, ); - assert.equal(Object.keys(result.channels).length, MAX_CHANNEL_STAR_ENTRIES); - assert.deepEqual(result.channels["evicted-id"], { - starred: true, - updatedAt: 9999, - }); - assert.deepEqual(result.channels["active-0"], { - starred: false, - updatedAt: 9998, - }); + assert.deepEqual(result.channels["evicted-id"], E(true, 9999, 0)); + assert.deepEqual(result.channels["active-0"], E(false, 9998, 0)); assert.equal(result.channels["active-1"], undefined); - assert.deepEqual(result.channels["active-2"], { - starred: true, - updatedAt: 12, - }); +}); + +// ── Eviction / remount (finding 3) ──────────────────────────────────────────── + +// Easy branch: X evicted at an OLDER second → remount (high-water lost) → click +// X at the current second → merge a remote carrying X at a high rev but an old +// updatedAt. The click's newer updatedAt wins on the primary key; the lost rev +// high-water is irrelevant. Closed by construction for all cross-second cases. +test("finding 3 easy branch: a fresh click beats an evicted high-rev entry at an older updatedAt", () => { + // Remount state: the user clicks X fresh at updatedAt=now, empty high-water + // (evicted), so rev mints to 1. + const click = S(E(true, 1000, 1)); + // The previously observed remote X sits at an OLD updatedAt with a high rev. + const remote = S(E(false, 500, 100)); + const merged = mergeStores(click, remote); + assert.deepEqual( + merged.channels.c, + E(true, 1000, 1), + "fresh click wins on the primary updatedAt key", + ); +}); + +// Hard branch (Thufir's exact equal-second counterexample): >500 entries all at +// the CURRENT second → X evicted by the id tiebreak (not because it is old) → +// remount in the same second → click X at rev 1 (empty high-water) → merge the +// previously observed remote X at rev 100, EQUAL updatedAt. updatedAt ties, rev +// decides, 100 > 1 — the click LOSES. Documented deterministic residual, proven +// here as the hard branch (not disguised as safety). +test("finding 3 hard branch: equal-second evicted click (rev 1) loses to observed remote (rev 100)", () => { + const NOW = 777; + // Remount click on the evicted channel: empty high-water → rev 1, updatedAt=NOW. + const click = S(E(true, NOW, 1)); + // The previously observed remote for the same channel at the same second, + // rev 100 (it may precede the remount — not genuinely concurrent). + const remote = S(E(false, NOW, 100)); + const merged = mergeStores(click, remote); + assert.deepEqual( + merged.channels.c, + E(false, NOW, 100), + "equal updatedAt → higher rev wins deterministically (documented residual)", + ); + // Deterministic either merge order — a lost click, never a divergence. + assert.deepEqual(mergeStores(remote, click).channels.c, E(false, NOW, 100)); }); // ── starredChannelIdsFromStore ──────────────────────────────────────────────── test("starredChannelIdsFromStore: returns set of IDs where starred=true", () => { - const store = { + const result = starredChannelIdsFromStore({ version: 1, channels: { - "chan-a": { starred: true, updatedAt: 100 }, - "chan-b": { starred: true, updatedAt: 200 }, - "chan-c": { starred: false, updatedAt: 300 }, + a: E(true, 100, 0), + b: E(true, 200, 0), + c: E(false, 300, 0), }, - }; - const result = starredChannelIdsFromStore(store); - assert.equal(result.has("chan-a"), true); - assert.equal(result.has("chan-b"), true); - assert.equal(result.has("chan-c"), false); - assert.equal(result.size, 2); + }); + assert.deepEqual([...result].sort(), ["a", "b"]); }); -test("starredChannelIdsFromStore: excludes IDs where starred=false", () => { - const store = { - version: 1, - channels: { - "chan-x": { starred: false, updatedAt: 1 }, - "chan-y": { starred: false, updatedAt: 2 }, - }, - }; - const result = starredChannelIdsFromStore(store); - assert.equal(result.size, 0); -}); - -test("starredChannelIdsFromStore: empty channels returns empty set", () => { - const result = starredChannelIdsFromStore({ version: 1, channels: {} }); - assert.equal(result.size, 0); +test("starredChannelIdsFromStore: all-false / empty returns empty set", () => { + assert.equal( + starredChannelIdsFromStore({ + version: 1, + channels: { x: E(false, 1, 0) }, + }).size, + 0, + ); + assert.equal( + starredChannelIdsFromStore({ version: 1, channels: {} }).size, + 0, + ); }); diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.ts b/desktop/src/features/sidebar/lib/channelStarsStorage.ts index 23e567f40b..9bb7979cb6 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.ts @@ -1,9 +1,16 @@ +import { normalizeRelayUrl } from "@/shared/lib/normalizeRelayUrl"; + const STORAGE_KEY_PREFIX = "buzz-channel-stars.v1"; export const MAX_CHANNEL_STAR_ENTRIES = 500; export type ChannelStarEntry = { starred: boolean; updatedAt: number; + // Per-channel Lamport revision. Breaks a same-second `updatedAt` tie that the + // integer clock cannot resolve. Absent in blobs from an older build ⇒ read as + // 0 (a valid, mergeable value), so the payload stays `version: 1` and older + // builds still parse our blobs. + rev: number; }; export type ChannelStarStore = { @@ -29,8 +36,8 @@ export function parseStarPayload(json: unknown): ChannelStarStore | null { obj.channels !== null && !Array.isArray(obj.channels) ? Object.fromEntries( - Object.entries(obj.channels as Record).filter( - (entry): entry is [string, ChannelStarEntry] => { + Object.entries(obj.channels as Record) + .filter((entry): entry is [string, Record] => { const v = entry[1]; return ( typeof v === "object" && @@ -42,8 +49,27 @@ export function parseStarPayload(json: unknown): ChannelStarStore | null { ) && ((v as Record).updatedAt as number) >= 0 ); - }, - ), + }) + // Normalize `rev`: accept a non-negative integer, otherwise 0. An + // entry is never dropped solely because `rev` is absent (older + // build) or malformed — absence is a valid mergeable value. + .map(([id, v]) => { + const rawRev = v.rev; + const rev = + typeof rawRev === "number" && + Number.isInteger(rawRev) && + rawRev >= 0 + ? rawRev + : 0; + return [ + id, + { + starred: v.starred as boolean, + updatedAt: v.updatedAt as number, + rev, + }, + ]; + }), ) : {}; return boundStarStore({ version: 1, channels }); @@ -108,79 +134,47 @@ export function writeChannelStarsStore( } } -export function mergeStores( - local: ChannelStarStore, - remote: ChannelStarStore, -): ChannelStarStore { - return mergeStoresWithTie(local, remote, false); -} - /** - * Merge a remote store that has already won the event-level canonical tie-break - * (`created_at DESC, id ASC`) into the local store, resolving a per-entry - * `updatedAt` tie in favour of the *remote* value. Once the comparator has - * chosen this remote event as the stored winner, its per-entry values must - * survive, or a stale value from a superseded larger-id event delivered first - * would win the merge and silently undo the canonical winner. Strictly-newer - * local per-entry edits (`l.updatedAt > r.updatedAt`) still win. + * Merge two star stores by a per-channel total order: + * `updatedAt` DESC → `rev` DESC → `starred === true` wins. This order is + * commutative, associative, and idempotent (before bounding), so every + * observation path (bootstrap, live, reconnect, reconcile, pre-publish, + * cross-window storage) applies it with no ordering or ownership overlay and + * all replicas converge. + * + * `updatedAt` is primary so a strictly-later edit — from any build, whether it + * carries `rev` or (older build) reads `rev: 0` — wins outright. `rev` breaks + * only a same-second `updatedAt` tie: the ambiguous integer-second window the + * clock cannot resolve, where a click that minted `rev = maxSeen + 1` dominates + * any same-second state it observed. On a full tie (equal `updatedAt` AND equal + * `rev`) `true` wins as the deterministic leaf. */ -export function mergeApplyingRemote( - local: ChannelStarStore, - remote: ChannelStarStore, -): ChannelStarStore { - return mergeStoresWithTie(local, remote, true); -} - -function mergeStoresWithTie( - local: ChannelStarStore, - remote: ChannelStarStore, - preferRemoteOnTie: boolean, +export function mergeStores( + a: ChannelStarStore, + b: ChannelStarStore, ): ChannelStarStore { const allIds = new Set([ - ...Object.keys(local.channels), - ...Object.keys(remote.channels), + ...Object.keys(a.channels), + ...Object.keys(b.channels), ]); const merged: Record = {}; for (const id of allIds) { - const l = local.channels[id]; - const r = remote.channels[id]; - if (l && r) { - const localWins = preferRemoteOnTie - ? l.updatedAt > r.updatedAt - : l.updatedAt >= r.updatedAt; - merged[id] = localWins ? l : r; - } else { - merged[id] = (l ?? r) as ChannelStarEntry; - } + const l = a.channels[id]; + const r = b.channels[id]; + merged[id] = l && r ? pickStarEntry(l, r) : ((l ?? r) as ChannelStarEntry); } return boundStarStore({ version: 1, channels: merged }); } -/** - * Apply a canonical lower-id correction (`mergeApplyingRemote`: remote wins a - * per-entry `updatedAt` tie) while preserving entries the user changed locally - * since the superseded head was applied. The correction canonicalises remote - * history, but a same-second local click carries integer-second `updatedAt` - * equal to the remote's, so the plain remote-wins tie would silently clobber - * it. For each `dirtyId` the local entry wins only the tie (`l.updatedAt >= - * r.updatedAt`) — a genuinely newer remote value still wins, so a stale dirty - * id can never override a later correction. - */ -export function mergeCanonicalSupersession( - local: ChannelStarStore, - remote: ChannelStarStore, - dirtyIds: ReadonlySet, -): ChannelStarStore { - const applied = mergeApplyingRemote(local, remote); - if (dirtyIds.size === 0) return applied; - const channels = { ...applied.channels }; - for (const id of dirtyIds) { - const l = local.channels[id]; - if (!l) continue; - const r = remote.channels[id]; - if (!r || l.updatedAt >= r.updatedAt) channels[id] = l; - } - return boundStarStore({ version: 1, channels }); +/** The winner of two entries under `updatedAt` → `rev` → `starred` order. */ +function pickStarEntry( + l: ChannelStarEntry, + r: ChannelStarEntry, +): ChannelStarEntry { + if (l.updatedAt !== r.updatedAt) return l.updatedAt > r.updatedAt ? l : r; + if (l.rev !== r.rev) return l.rev > r.rev ? l : r; + if (l.starred !== r.starred) return l.starred ? l : r; + return l; } export function starredChannelIdsFromStore( @@ -192,3 +186,62 @@ export function starredChannelIdsFromStore( .map(([id]) => id), ); } + +const OUTBOX_KEY_PREFIX = "buzz-channel-stars-outbox.v1"; + +// The outbox is a per-relay sync-lane structure (like the watermark), so it is +// relay-scoped even though the main store stays pubkey-only: an edit made +// against relay A must never resume-publish onto relay B after a community +// switch. +function outboxKey(pubkey: string, relayUrl: string): string { + return `${OUTBOX_KEY_PREFIX}:${pubkey}:${encodeURIComponent(normalizeRelayUrl(relayUrl))}`; +} + +/** + * Persist an unpublished edit so it survives quit/community-switch within the + * 2s publish debounce. Written synchronously on every click; cleared once the + * edit is published or found identical to the last published store. Resumed on + * next mount so a durable intent is never silently dropped at teardown. + */ +export function writeChannelStarsOutbox( + pubkey: string, + store: ChannelStarStore, + relayUrl: string, +): void { + try { + window.localStorage.setItem( + outboxKey(pubkey, relayUrl), + JSON.stringify(boundStarStore(store)), + ); + } catch { + // Best-effort durability; the in-memory pendingStore still drives this + // session's publish even if the persisted copy could not be written. + } +} + +/** Read a persisted unpublished edit, or null when none/unparseable. */ +export function readChannelStarsOutbox( + pubkey: string, + relayUrl: string, +): ChannelStarStore | null { + try { + const raw = window.localStorage.getItem(outboxKey(pubkey, relayUrl)); + if (!raw) return null; + return parseStarPayload(JSON.parse(raw)); + } catch { + return null; + } +} + +/** Clear the persisted outbox (edit published or a no-op). */ +export function clearChannelStarsOutbox( + pubkey: string, + relayUrl: string, +): void { + try { + window.localStorage.removeItem(outboxKey(pubkey, relayUrl)); + } catch { + // Ignore — a stale outbox entry is re-evaluated (and re-cleared if + // identical to the head) on the next publish attempt. + } +} diff --git a/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs b/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs index b023574467..d238f3c0fa 100644 --- a/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs +++ b/desktop/src/features/sidebar/lib/channelStarsSync.test.mjs @@ -4,8 +4,9 @@ import test, { mock } from "node:test"; import { relayClient } from "@/shared/api/relayClient"; import { ChannelStarSyncManager } from "./channelStarsSync.ts"; import { - makeFakeWindow, installFakeWindow, + installTauriMock, + makeFakeWindow, } from "./sidebarSyncTestHelpers.mjs"; const RELAY = "wss://r.test"; @@ -14,12 +15,70 @@ const RELAY_KEY = encodeURIComponent(RELAY); function makeStore(channels = {}) { return { version: 1, channels }; } +const E = (starred, updatedAt, rev) => ({ starred, updatedAt, rev }); + +// Multi-slot timer fake keyed by delay, for overlapping-publish tests. Mirrors +// the sections suite convention (channelSectionsSync.test.mjs:407-432). +function makeMultiTimerWindow() { + const storage = new Map(); + const timers = new Map(); + let nextId = 1; + const win = { + localStorage: { + getItem: (k) => storage.get(k) ?? null, + setItem: (k, v) => storage.set(k, v), + removeItem: (k) => storage.delete(k), + }, + setTimeout: (fn, ms) => { + const id = nextId++; + timers.set(id, { fn, ms }); + return id; + }, + clearTimeout: (id) => timers.delete(id), + }; + return { + win, + storage, + timers, + fireDelay: async (ms) => { + const entry = [...timers.entries()].find(([, v]) => v.ms === ms); + assert.ok(entry, `expected a timer scheduled at ${ms}ms`); + timers.delete(entry[0]); + entry[1].fn(); + for (let i = 0; i < 50; i++) await Promise.resolve(); + }, + hasDelay: (ms) => [...timers.values()].some((t) => t.ms === ms), + }; +} + +// ─── observe() / high-water ingestion ───────────────────────────────────────── + +test("observe: high-water is per-channel max of rev and updatedAt, monotonic", () => { + const fw = makeFakeWindow(); + const restore = installFakeWindow(fw); + try { + const m = new ChannelStarSyncManager("pk", RELAY); + m.observe(makeStore({ a: E(true, 100, 3), b: E(false, 50, 1) })); + assert.equal(m.maxRevSeen("a"), 3); + assert.equal(m.maxUpdatedAtSeen("a"), 100); + // A later observation raises each dimension independently; a lower one + // never regresses either. + m.observe(makeStore({ a: E(true, 90, 5) })); + assert.equal(m.maxRevSeen("a"), 5, "rev raised"); + assert.equal(m.maxUpdatedAtSeen("a"), 100, "updatedAt not regressed"); + m.observe(makeStore({ a: E(true, 200, 2) })); + assert.equal(m.maxUpdatedAtSeen("a"), 200, "updatedAt raised"); + assert.equal(m.maxRevSeen("a"), 5, "rev not regressed"); + // Unseen channel reports zero on both dimensions. + assert.equal(m.maxRevSeen("never"), 0); + assert.equal(m.maxUpdatedAtSeen("never"), 0); + } finally { + restore(); + } +}); // ─── destroy() must cancel pending publish, not flush ───────────────────────── -// Regression guard for the community-switch cross-relay publish vector: -// star a channel in relay A → destroy() called (relayUrl dep change) → -// no publish should fire. test("destroy: cancels pending publish without flushing to the relay", () => { const publishCalls = []; mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); @@ -31,7 +90,7 @@ test("destroy: cancels pending publish without flushing to the relay", () => { const restore = installFakeWindow(fw); try { const manager = new ChannelStarSyncManager("pk-test", RELAY); - manager.publishStars(makeStore({ ch1: { starred: true, updatedAt: 100 } })); + manager.publishStars(makeStore({ ch1: E(true, 100, 1) })); manager.destroy(); assert.equal(publishCalls.length, 0, "no publish after destroy"); assert.equal(manager.getPendingStarStore(), null); @@ -60,7 +119,7 @@ test("destroy: aborts in-flight doPublish after fetchOwnBlobBeforePublish resolv const restore = installFakeWindow(fw); try { const manager = new ChannelStarSyncManager("pk-race", RELAY); - manager.publishStars(makeStore({ ch1: { starred: true, updatedAt: 100 } })); + manager.publishStars(makeStore({ ch1: E(true, 100, 1) })); fw._fireTimer(); manager.destroy(); releaseFetch(); @@ -87,9 +146,175 @@ test("destroy: is safe to call with no pending publish", () => { } }); +// ─── Generation CAS: A-in-flight → B-click → A-completes (both variants) ────── + +// Finding 2 (A succeeds): an older in-flight publish that completes after a +// newer edit is queued must NOT clear the newer edit's pending store/outbox, +// and B must reach the relay via the completion re-drive. Mutation: dropping the +// generation CAS in discardPending lets A's success null out B's pending+outbox. +test("A-in-flight → B-click → A-succeeds: B stays pending and B publishes", async () => { + let releaseFirst = null; + const publishedContents = []; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + if (releaseFirst === null && publishedContents.length === 0) { + return new Promise((res) => { + releaseFirst = res; + }); + } + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + const outboxKey = `buzz-channel-stars-outbox.v1:pk-ab:${RELAY_KEY}`; + try { + const manager = new ChannelStarSyncManager("pk-ab", RELAY); + const storeA = makeStore({ a: E(true, 100, 1) }); + const storeB = makeStore({ b: E(true, 101, 1) }); + + manager.publishStars(storeA); + await t.fireDelay(2000); // doPublish(A) awaits publishEvent + while (releaseFirst === null) await Promise.resolve(); + + // B arrives while A is in flight. + manager.publishStars(storeB); + assert.deepEqual( + Object.keys(manager.getPendingStarStore().channels), + ["b"], + "B is now pending", + ); + assert.ok(t.storage.get(outboxKey), "outbox holds B"); + + // A completes — must NOT clear B. + releaseFirst(); + for (let i = 0; i < 50; i++) await Promise.resolve(); + assert.deepEqual( + Object.keys(manager.getPendingStarStore()?.channels ?? {}), + ["b"], + "older A completion leaves B pending", + ); + assert.ok(t.storage.get(outboxKey), "older A completion leaves B outbox"); + + // B's own debounce fires and B reaches the relay (published) with no kick. + const capturedBefore = tauri.capturedPlaintext(); + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + const captured = tauri.capturedPlaintext(); + assert.ok( + captured && captured !== capturedBefore && captured.includes('"b"'), + "B is published to the relay", + ); + assert.equal( + manager.getPendingStarStore(), + null, + "B cleared after publish", + ); + assert.equal(t.storage.get(outboxKey), undefined, "B outbox cleared"); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// Finding 2 (A fails): A's publish rejects after B is queued. B must remain +// pending and be published by the serialized re-drive / retry — no manual kick. +test("A-in-flight → B-click → A-fails: B remains pending and B publishes", async () => { + let rejectFirst = null; + let publishCount = 0; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + publishCount++; + if (publishCount === 1) { + return new Promise((_res, rej) => { + rejectFirst = () => rej(new Error("socket error")); + }); + } + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + const outboxKey = `buzz-channel-stars-outbox.v1:pk-abfail:${RELAY_KEY}`; + try { + const manager = new ChannelStarSyncManager("pk-abfail", RELAY); + manager.publishStars(makeStore({ a: E(true, 100, 1) })); + await t.fireDelay(2000); + while (rejectFirst === null) await Promise.resolve(); + + manager.publishStars(makeStore({ b: E(true, 101, 1) })); + rejectFirst(); // A fails + for (let i = 0; i < 50; i++) await Promise.resolve(); + + assert.deepEqual( + Object.keys(manager.getPendingStarStore()?.channels ?? {}), + ["b"], + "B still pending after A's failure", + ); + assert.ok(t.storage.get(outboxKey), "B outbox intact after A's failure"); + + // B's debounce fires and B publishes successfully. + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + const captured = tauri.capturedPlaintext(); + assert.ok(captured?.includes('"b"'), "B published"); + assert.equal(manager.getPendingStarStore(), null, "B cleared"); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// ─── Bounded-backoff retry: failed publish on a healthy socket, no later edit ─ + +// Finding 2: a transient publish failure with the socket open and NO further +// click must self-heal via the bounded-backoff retry — the pending edit is kept +// and a retry timer is scheduled. Mutation: dropping scheduleRetry leaves the +// edit stranded (Will's "make another change to kick it" symptom). +test("failed publish schedules a bounded-backoff retry and keeps the pending edit", async () => { + let publishCount = 0; + mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); + mock.method(relayClient, "publishEvent", () => { + publishCount++; + if (publishCount === 1) return Promise.reject(new Error("timeout")); + return Promise.resolve(); + }); + const t = makeMultiTimerWindow(); + const restore = installFakeWindow(t.win); + const tauri = installTauriMock("{}"); + try { + const manager = new ChannelStarSyncManager("pk-retry", RELAY); + manager.publishStars(makeStore({ a: E(true, 100, 1) })); + await t.fireDelay(2000); // debounce → doPublish → publishEvent rejects + assert.ok( + manager.getPendingStarStore() !== null, + "pending edit retained after failure", + ); + assert.ok(t.hasDelay(2000), "a retry timer at RETRY_BASE_MS is scheduled"); + + // The retry fires and the second publish succeeds → pending cleared. + await t.fireDelay(2000); + for (let i = 0; i < 50; i++) await Promise.resolve(); + assert.equal(publishCount, 2, "retry re-published"); + assert.equal( + manager.getPendingStarStore(), + null, + "pending cleared on retry success", + ); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + // ─── Boot seed-publish guard (the revert-fix regression suite) ───────────────── -// 1. fetch failed → hold, pendingStore null (mutation: remove failed guard → seed queued) test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstrap", async () => { mock.method(relayClient, "fetchEvents", () => Promise.reject(new Error("relay timeout")), @@ -99,9 +324,7 @@ test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstr const restore = installFakeWindow(fw); try { const manager = new ChannelStarSyncManager("pk-fail", RELAY); - const result = await manager.bootstrap( - makeStore({ ch1: { starred: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.equal(manager.getPendingStarStore(), null); } finally { @@ -110,7 +333,6 @@ test("revert-fix: fetch failed (error) does not trigger seed-publish via bootstr } }); -// 2. absent + prior watermark → hold, pendingStore null (mutation: clear watermark → seed queued) test("revert-fix: absent fetch with prior watermark blocks seed-publish via bootstrap", async () => { mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); mock.method(relayClient, "publishEvent", () => Promise.resolve()); @@ -122,16 +344,7 @@ test("revert-fix: absent fetch with prior watermark blocks seed-publish via boot const restore = installFakeWindow(fw); try { const manager = new ChannelStarSyncManager("pk-stale", RELAY); - assert.ok( - Number( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-stars:pk-stale:${RELAY_KEY}`, - ) ?? "0", - ) > 0, - ); - const result = await manager.bootstrap( - makeStore({ ch1: { starred: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.equal(manager.getPendingStarStore(), null); } finally { @@ -140,7 +353,6 @@ test("revert-fix: absent fetch with prior watermark blocks seed-publish via boot } }); -// 3. absent + zero watermark + non-empty → seed queued (mutation: remove seed call → pendingStore null) test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sync preserved)", async () => { mock.method(relayClient, "fetchEvents", () => Promise.resolve([])); mock.method(relayClient, "publishEvent", () => Promise.resolve()); @@ -148,15 +360,7 @@ test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sy const restore = installFakeWindow(fw); try { const manager = new ChannelStarSyncManager("pk-fresh", RELAY); - assert.equal( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-stars:pk-fresh:${RELAY_KEY}`, - ), - null, - ); - const result = await manager.bootstrap( - makeStore({ ch1: { starred: true, updatedAt: 1 } }), - ); + const result = await manager.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.ok(manager.getPendingStarStore() !== null); } finally { @@ -165,8 +369,6 @@ test("revert-fix: absent fetch with zero watermark seeds via bootstrap (first-sy } }); -// 4. relay-A / relay-B watermark isolation -// Mutation: using pubkey-only key (no relay) makes relay A's head suppress relay B's first-sync. test("revert-fix: relay-A watermark does not suppress first-sync seed on relay-B", async () => { const relayA = "wss://a.relay.test"; const relayB = "wss://b.relay.test"; @@ -180,16 +382,7 @@ test("revert-fix: relay-A watermark does not suppress first-sync seed on relay-B const restore = installFakeWindow(fw); try { const managerB = new ChannelStarSyncManager("pk-iso", relayB); - assert.equal( - fw.localStorage.getItem( - `buzz-sync-watermark.v1:channel-stars:pk-iso:${encodeURIComponent(relayB)}`, - ), - null, - "relay B watermark must be independent of relay A head", - ); - const result = await managerB.bootstrap( - makeStore({ ch1: { starred: true, updatedAt: 1 } }), - ); + const result = await managerB.bootstrap(makeStore({ ch1: E(true, 1, 0) })); assert.equal(result.action, "hold"); assert.ok( managerB.getPendingStarStore() !== null, diff --git a/desktop/src/features/sidebar/lib/channelStarsSync.ts b/desktop/src/features/sidebar/lib/channelStarsSync.ts index a5abec03fb..695cfc3ebc 100644 --- a/desktop/src/features/sidebar/lib/channelStarsSync.ts +++ b/desktop/src/features/sidebar/lib/channelStarsSync.ts @@ -7,8 +7,10 @@ import { import type { RelayEvent } from "@/shared/api/types"; import { KIND_CHANNEL_STARS } from "@/shared/constants/kinds"; import { + clearChannelStarsOutbox, mergeStores, parseStarPayload, + writeChannelStarsOutbox, type ChannelStarStore, } from "./channelStarsStorage"; import { @@ -22,6 +24,12 @@ const D_TAG = "channel-stars"; const BLOB_TYPE = D_TAG; const DEBOUNCE_MS = 2_000; +// Bounded backoff for a retained pending edit whose publish failed transiently +// (timeout / socket error) on an otherwise-healthy socket, so it does not wait +// for a reconnect that may never fire. +const RETRY_BASE_MS = 2_000; +const RETRY_MAX_MS = 30_000; + export type RemoteStars = { store: ChannelStarStore; createdAt: number; @@ -43,10 +51,30 @@ export class ChannelStarSyncManager { private pubkey: string; private relayUrl: string; private debounceTimer: number | null = null; + private retryTimer: number | null = null; + private retryDelayMs = RETRY_BASE_MS; private lastRemoteCreatedAt: number; private pendingStore: ChannelStarStore | null = null; + // Monotonic id for the current pending edit. Every publishStars() bumps it; + // every scheduled publish/retry captures the value it was queued for. A + // completion (success or no-op) may only clear pending state via + // compare-and-swap on this generation, so an older in-flight publish can + // never erase a newer edit that arrived while it was in flight. + private pendingGeneration = 0; + // Publish cycles are serialized: at most one runs at a time. A newer edit + // queued while a cycle is in flight defers; the in-flight cycle's completion + // re-drives it. Serialization guarantees there is never more than one + // fetch/publish sequence touching shared manager state. + private publishInFlight = false; private lastPublishedStore: ChannelStarStore | null = null; private destroyed = false; + // Per-channel high-water of every `rev` and `updatedAt` this manager has + // observed (bootstrap, live, reconnect, reconcile, pre-publish, cross-window + // storage, and initial persisted state). A click reads both so its minted + // `updatedAt = max(now, maxUpdatedAtSeen)` never regresses below observed + // state (the read-state logical-monotonic idiom), and `rev = maxRevSeen + 1` + // wins the resulting same-second tie. + private highWater = new Map(); constructor(pubkey: string, relayUrl: string) { this.pubkey = pubkey; @@ -54,6 +82,29 @@ export class ChannelStarSyncManager { this.lastRemoteCreatedAt = readWatermark(pubkey, BLOB_TYPE, relayUrl); } + /** + * Ingest a store into the per-channel high-water. Called synchronously before + * any merge is applied to React state, so a click that follows reads a current + * watermark on both dimensions. Monotonic (`Math.max`) and idempotent. + */ + observe(store: ChannelStarStore): void { + for (const [id, entry] of Object.entries(store.channels)) { + const cur = this.highWater.get(id) ?? { rev: 0, updatedAt: 0 }; + this.highWater.set(id, { + rev: Math.max(cur.rev, entry.rev), + updatedAt: Math.max(cur.updatedAt, entry.updatedAt), + }); + } + } + + maxRevSeen(id: string): number { + return this.highWater.get(id)?.rev ?? 0; + } + + maxUpdatedAtSeen(id: string): number { + return this.highWater.get(id)?.updatedAt ?? 0; + } + async fetchRemoteStars(): Promise> { try { const events = await relayClient.fetchEvents({ @@ -71,6 +122,7 @@ export class ChannelStarSyncManager { if (!result) { return { status: "failed", createdAt: event.created_at }; } + this.observe(result.store); return { status: "found", data: result, @@ -94,6 +146,10 @@ export class ChannelStarSyncManager { window.clearTimeout(this.debounceTimer); this.debounceTimer = null; } + if (this.retryTimer !== null) { + window.clearTimeout(this.retryTimer); + this.retryTimer = null; + } } getPendingStarStore(): ChannelStarStore | null { @@ -102,15 +158,51 @@ export class ChannelStarSyncManager { publishStars(store: ChannelStarStore): void { this.pendingStore = store; + ++this.pendingGeneration; + // Persist synchronously so a click made <2s before quit/community-switch + // survives teardown and resumes on next mount (durable outbox). + writeChannelStarsOutbox(this.pubkey, store, this.relayUrl); if (this.debounceTimer !== null) { window.clearTimeout(this.debounceTimer); } + // A fresh edit supersedes any retry scheduled for the previous generation. + if (this.retryTimer !== null) { + window.clearTimeout(this.retryTimer); + this.retryTimer = null; + } + this.retryDelayMs = RETRY_BASE_MS; this.debounceTimer = window.setTimeout(() => { this.debounceTimer = null; - void this.doPublish(store); + this.startCycle(); }, DEBOUNCE_MS); } + /** + * Serialize publish cycles: at most one runs at a time. A debounce/retry timer + * that fires while a cycle is in flight defers — the in-flight cycle's + * completion re-drives if a pending edit still needs publishing. A newer edit + * queued during a cycle cannot start its own concurrent cycle, so a stale + * generation can never publish after a newer edit exists. + */ + private startCycle(): void { + if (this.destroyed || this.pendingStore === null) return; + if (this.publishInFlight) return; + const store = this.pendingStore; + const gen = this.pendingGeneration; + this.publishInFlight = true; + void this.doPublish(store, gen).finally(() => { + this.publishInFlight = false; + if ( + !this.destroyed && + this.pendingStore !== null && + this.debounceTimer === null && + this.retryTimer === null + ) { + this.startCycle(); + } + }); + } + private async fetchOwnBlobBeforePublish( store: ChannelStarStore, ): Promise { @@ -127,6 +219,9 @@ export class ChannelStarSyncManager { this.recordRemoteHead(event.created_at); const remote = await decryptAndParse(event); if (!remote) return store; + this.observe(remote.store); + // Max-merge: the local edit's per-entry winners survive by construction + // and any newer remote entries fold in, so no adopt step is needed. return mergeStores(store, remote.store); } catch { return store; @@ -144,22 +239,55 @@ export class ChannelStarSyncManager { if ( !last || last.starred !== current.starred || - last.updatedAt !== current.updatedAt + last.updatedAt !== current.updatedAt || + last.rev !== current.rev ) return false; } return true; } - private async doPublish(store: ChannelStarStore): Promise { + /** + * Clear the in-memory pending edit and its durable outbox — but only if the + * completing publish still owns the current generation. A publish for an + * older edit that finishes after a newer edit was queued must leave the newer + * edit (and its retry state) untouched. + */ + private discardPending(gen: number): void { + if (gen !== this.pendingGeneration) return; + this.pendingStore = null; + clearChannelStarsOutbox(this.pubkey, this.relayUrl); + } + + /** Schedule a bounded-backoff retry of the retained pending edit. */ + private scheduleRetry(gen: number): void { + if (this.destroyed || this.pendingStore === null) return; + // A newer edit has superseded this one; its own timer owns the retry. + if (gen !== this.pendingGeneration) return; + if (this.retryTimer !== null) return; + const delay = this.retryDelayMs; + this.retryDelayMs = Math.min(this.retryDelayMs * 2, RETRY_MAX_MS); + this.retryTimer = window.setTimeout(() => { + this.retryTimer = null; + this.startCycle(); + }, delay); + } + + private async doPublish(store: ChannelStarStore, gen: number): Promise { + // A newer edit was queued after this publish was scheduled; it owns the + // pending state and will publish the latest store — abandon this stale run. + if (gen !== this.pendingGeneration) return; try { const merged = await this.fetchOwnBlobBeforePublish(store); // Guard: manager may have been destroyed while fetchOwnBlobBeforePublish - // was awaited (community switch during in-flight fetch). If so, abort - // before touching the relay. + // was awaited (community switch during in-flight fetch). if (this.destroyed) return; + // A newer edit was queued while we awaited the pre-publish fetch. It owns + // convergence now; the serialized cycle re-drives for it once this run + // unwinds. + if (gen !== this.pendingGeneration) return; if (this.isIdenticalToLastPublished(merged)) { - this.pendingStore = null; + this.discardPending(gen); return; } const payload = { @@ -180,17 +308,32 @@ export class ChannelStarSyncManager { ["t", D_TAG], // relay discoverability; not used in our filters ], }); - if (this.destroyed) return; + // Final guard immediately before the network call: a newer edit may have + // been queued during the encrypt/sign await, or the manager destroyed. + if (this.destroyed || gen !== this.pendingGeneration) return; await relayClient.publishEvent( event, "Timed out publishing channel stars.", "Failed to publish channel stars.", ); this.recordRemoteHead(event.created_at); - this.lastPublishedStore = merged; - this.pendingStore = null; + this.observe(merged); + // Only claim this store as the published head if it is still the current + // edit; a newer edit queued mid-flight owns lastPublishedStore now. + if (gen === this.pendingGeneration) { + this.lastPublishedStore = merged; + this.retryDelayMs = RETRY_BASE_MS; + } + this.discardPending(gen); } catch (error) { + if (this.destroyed) return; + // Transient publish failure (timeout / socket error). Keep the pending + // edit and retry with backoff rather than waiting for a reconnect that a + // healthy socket never fires. Max-merge makes a duplicate publish + // idempotent, so a lost-ACK write that the relay actually accepted is + // harmless to re-send. console.warn("[channelStarsSync] publish failed:", error); + this.scheduleRetry(gen); } } @@ -211,6 +354,7 @@ export class ChannelStarSyncManager { this.recordRemoteHead(event.created_at); void decryptAndParse(event).then((result) => { if (result) { + this.observe(result.store); onUpdate(result); } }); @@ -223,6 +367,9 @@ export class ChannelStarSyncManager { * delegates the seed/hold/apply-remote decision to `runBootstrap`. */ async bootstrap(localStore: ChannelStarStore) { + // Seed the high-water from the caller's persisted local store so a click + // before the remote fetch resolves already reflects retained entries. + this.observe(localStore); const fetchResult = await this.fetchRemoteStars(); return runBootstrap({ fetchResult, @@ -236,10 +383,10 @@ export class ChannelStarSyncManager { destroy(): void { // Cancel any pending publish and mark this manager as destroyed so any // in-flight doPublish() calls abort before reaching relayClient. - // Pending debounce-window changes are intentionally dropped: flushing - // could publish relay A's state to relay B via the shared relayClient - // singleton. Local entries survive because the apply/publish paths merge - // per-entry via mergeStores, so no local work is permanently lost. + // Debounce-window changes are NOT lost: publishStars persisted them to the + // durable outbox synchronously, and the next mount resumes them. Flushing + // here is still avoided — it could publish relay A's state to relay B via + // the shared relayClient singleton. this.destroyed = true; this.cancelPendingStarPublish(); this.pendingStore = null; diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs b/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs index 04c2c836d6..c06466a8f7 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs +++ b/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs @@ -18,35 +18,51 @@ before(() => { after(() => dom.window.close()); -test("same-second mute and unmute mutations survive at capacity", async () => { +// Shared harness: stub the relay so no network/live/reconnect fires unless a +// test installs its own live callback. Returns the captured live callback. +function stubRelay(relayClient, { live } = {}) { + const orig = { + fetchEvents: relayClient.fetchEvents, + subscribeLive: relayClient.subscribeLive, + subscribeToReconnects: relayClient.subscribeToReconnects, + }; + relayClient.fetchEvents = async () => []; + relayClient.subscribeLive = async (_f, cb) => { + if (live) live.cb = cb; + return async () => {}; + }; + relayClient.subscribeToReconnects = () => () => {}; + return () => Object.assign(relayClient, orig); +} + +function mutePayload(channels) { + return JSON.stringify({ version: 1, channels }); +} + +test("same-second star and unstar mutations survive at capacity", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); const { MAX_CHANNEL_MUTE_ENTRIES, readChannelMutesStore, storageKey } = await import("./channelMutesStorage.ts"); const { useChannelMutes } = await import("./useChannelMutes.ts"); - const originalFetchEvents = relayClient.fetchEvents; - const originalSubscribeLive = relayClient.subscribeLive; - const originalSubscribeToReconnects = relayClient.subscribeToReconnects; + const restore = stubRelay(relayClient); const originalDateNow = Date.now; const updatedAt = 1_234_567; Date.now = () => updatedAt * 1_000; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async () => async () => {}; - relayClient.subscribeToReconnects = () => () => {}; const relayUrl = "wss://relay.example"; const channels = Object.fromEntries( Array.from({ length: MAX_CHANNEL_MUTE_ENTRIES }, (_, index) => [ `z-channel-${String(index).padStart(3, "0")}`, - { muted: true, updatedAt }, + { muted: true, updatedAt, rev: 0 }, ]), ); try { - for (const [pubkey, action, expectedMuted] of [ - ["pk-mute", "muteChannel", true], - ["pk-unmute", "unmuteChannel", false], + for (const [pubkey, action, expectedStarred] of [ + ["pk-star", "muteChannel", true], + ["pk-unstar", "unmuteChannel", false], ]) { window.localStorage.setItem( storageKey(pubkey), @@ -55,412 +71,348 @@ test("same-second mute and unmute mutations survive at capacity", async () => { const { result, unmount } = renderHook(() => useChannelMutes(pubkey, relayUrl), ); - act(() => result.current[action]("a-target")); - const persisted = readChannelMutesStore(pubkey); assert.equal( Object.keys(persisted.channels).length, MAX_CHANNEL_MUTE_ENTRIES, ); - assert.deepEqual(persisted.channels["a-target"], { - muted: expectedMuted, - updatedAt, - }); + assert.equal(persisted.channels["a-target"].muted, expectedStarred); unmount(); } } finally { cleanup(); Date.now = originalDateNow; - relayClient.fetchEvents = originalFetchEvents; - relayClient.subscribeLive = originalSubscribeLive; - relayClient.subscribeToReconnects = originalSubscribeToReconnects; + restore(); } }); -// Equal-timestamp tie-break must match the relay's canonical winner -// (`created_at DESC, id ASC` → LOWEST id wins). Deliver the larger id first, -// then the lower id at the same timestamp; the lower id is the stored winner -// and must be applied, not rejected. Reverting applyRemote's `>=` back to `<=` -// wrongly ignores the lower id (the actual relay winner). -test("equal-timestamp tie-break applies the lower event id (relay canonical winner)", async () => { +// Symmetric mint (Thufir MINOR 2): a click mints +// updatedAt = max(now, localEntry.updatedAt, maxUpdatedAtSeen) and +// rev = max(localEntry.rev, maxRevSeen) + 1 on BOTH dimensions. A remount reads +// the persisted local entry, so the very first click advances past it. +test("persisted-local first click mints seen+1 on both dimensions", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelMutesStore, storageKey } = await import( + "./channelMutesStorage.ts" + ); const { useChannelMutes } = await import("./useChannelMutes.ts"); - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // Decrypt payload keyed off the event id embedded in the ciphertext so each - // delivered event yields a store muting a distinct channel we can assert on. - window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const id = args?.ciphertext ?? ""; - return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { [id]: { muted: true, updatedAt: 0 } }, - }), - ); - } - return Promise.reject(new Error(`unmocked ${cmd}`)); - }, - }; - - const pubkey = "pk-mute-tie"; - const relayUrl = "wss://r.tie"; - let hook = null; - try { - await act(async () => { - hook = renderHook(() => useChannelMutes(pubkey, relayUrl)); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 1000, - content: id, // decrypt echoes this into the muted channel id - kind: 30078, - tags: [["d", "channel-mutes"]], - sig: "s", - }); - await Promise.resolve(); - await Promise.resolve(); - }); - }; - - // Larger id first (applied), then the lower id at the same timestamp — the - // relay's canonical winner, which must NOT be rejected. - await deliver("bbbb"); - await deliver("aaaa"); - - assert.ok( - hook.result.current.mutedChannelIds.has("aaaa"), - "lower event id (relay canonical winner) must be applied, not rejected", - ); - hook.unmount(); - } finally { - cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - } -}); - -// Pass-2 finding 2: the comparator admitting the canonical lower id is -// necessary but not sufficient. Mutes are a per-entry store, so applyRemote -// merges the incoming blob into local state. On the SAME channel, a stale -// larger-id event delivered first (muted=true) must not survive the merge once -// the canonical lower-id winner (muted=false) arrives at the same entry -// `updatedAt`. Mutation: reverting the apply path to mergeStores (local/prev -// wins on tie) keeps the stale muted=true value. -test("canonical lower-id unmute replaces a stale larger-id mute at equal entry timestamp", async () => { - const { act, cleanup, renderHook } = await import("@testing-library/react"); - const { relayClient } = await import("@/shared/api/relayClient"); - const { useChannelMutes } = await import("./useChannelMutes.ts"); - - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // Both events target the SAME channel `shared` at the same entry updatedAt. - // The larger id `bbbb` says muted; the canonical lower id `aaaa` says not. - window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const canonicalLowerId = args?.ciphertext === "aaaa"; - return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { shared: { muted: !canonicalLowerId, updatedAt: 100 } }, - }), - ); - } - return Promise.reject(new Error(`unmocked ${cmd}`)); - }, - }; - - const pubkey = "pk-mute-shared-tie"; - const relayUrl = "wss://r.tie"; - let hook = null; - try { - await act(async () => { - hook = renderHook(() => useChannelMutes(pubkey, relayUrl)); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 1000, - content: id, - kind: 30078, - tags: [["d", "channel-mutes"]], - sig: "s", - }); - for (let i = 0; i < 20; i++) await Promise.resolve(); - }); - }; - - await deliver("bbbb"); // stale larger-id head says muted - await deliver("aaaa"); // canonical lower-id winner says unmuted - - assert.equal( - hook.result.current.mutedChannelIds.has("shared"), - false, - "canonical lower-id unmute must replace the stale larger-id mute", - ); - hook.unmount(); - } finally { - cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - } -}); - -// Fix round 3 (pass-3 finding 2): the remote-wins entry-tie merge and the -// pending-publish cancel apply ONLY to a canonical supersession (a lower id at -// the same event timestamp correcting an already-applied larger id). A plain -// live/bootstrap remote must NOT clobber a later same-second local click or -// cancel its pending publish. Entry `updatedAt` is whole seconds, so a click at -// 100.9s and an older remote entry at 100.1s both carry `updatedAt:100`; the -// later local intent must win and keep publishing. Mutation: applying -// mergeApplyingRemote + cancel unconditionally lets the delayed remote overwrite -// the click and drop its publish. -test("delayed same-second remote does not clobber a later local mute or cancel its publish", async () => { - const { act, cleanup, renderHook } = await import("@testing-library/react"); - const { relayClient } = await import("@/shared/api/relayClient"); - const { useChannelMutes } = await import("./useChannelMutes.ts"); - - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - const origSetTimeout = window.setTimeout; - const origClearTimeout = window.clearTimeout; + const restore = stubRelay(relayClient); const origDateNow = Date.now; + // Persisted entry is stamped in the FUTURE relative to wall clock, with a + // non-zero rev — the mint must not regress below either. + Date.now = () => 100 * 1_000; + const pubkey = "pk-persist"; + window.localStorage.setItem( + storageKey(pubkey), + mutePayload({ shared: { muted: true, updatedAt: 500, rev: 4 } }), + ); + try { + const { result, unmount } = renderHook(() => + useChannelMutes(pubkey, "wss://r"), + ); + act(() => result.current.unmuteChannel("shared")); + const persisted = readChannelMutesStore(pubkey); + assert.equal(persisted.channels.shared.muted, false, "unstar applied"); + assert.equal( + persisted.channels.shared.updatedAt, + 500, + "updatedAt held at persisted-local high-water (max(100,500,seen))", + ); + assert.equal( + persisted.channels.shared.rev, + 5, + "rev minted as persisted-local rev + 1", + ); + unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); + } +}); - const timers = new Map(); - let nextTimer = 1; - window.setTimeout = (fn, ms) => { - const id = nextTimer++; - timers.set(id, { fn, ms }); - return id; - }; - window.clearTimeout = (id) => timers.delete(id); - // The local click happens later within second 100. - Date.now = () => 100_900; +// Fast-clock veto fix (Thufir pass-2 finding 1): after observing a +// future-stamped remote (updatedAt = t+300, rev 7, unmuted), a slow device +// clicking star at wall-clock t must WIN — the logical-monotonic stamp lifts the +// click's updatedAt to t+300 and rev to 8, so it dominates the observed entry. +test("fast-clock veto fix: click after observing a future-stamped head wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelMutesStore } = await import("./channelMutesStorage.ts"); + const { useChannelMutes } = await import("./useChannelMutes.ts"); - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // The delayed remote entry sits earlier in the same second and says unmuted. + const live = {}; + const restore = stubRelay(relayClient, { live }); + const origTauri = window.__TAURI_INTERNALS__; + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; // slow device: wall clock t = 100 window.__TAURI_INTERNALS__ = { invoke: (cmd) => { if (cmd === "nip44_decrypt_from_self") return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { shared: { muted: false, updatedAt: 100 } }, - }), + mutePayload({ shared: { muted: false, updatedAt: 400, rev: 7 } }), ); return Promise.reject(new Error(`unmocked ${cmd}`)); }, }; - - const pubkey = "pk-mute-same-second"; - const relayUrl = "wss://r.same"; + const pubkey = "pk-fastclock"; let hook = null; try { await act(async () => { - hook = renderHook(() => useChannelMutes(pubkey, relayUrl)); + hook = renderHook(() => useChannelMutes(pubkey, "wss://r")); for (let i = 0; i < 20; i++) await Promise.resolve(); }); - assert.ok(live, "live subscription installed"); - - // Local optimistic click: muted=true at updatedAt=100 (Date.now=100.9s). + // Observe the future-stamped head (updatedAt 400 = t+300). await act(async () => { - hook.result.current.muteChannel("shared"); - }); - // An older remote entry from the same second decrypts and applies late. - await act(async () => { - live({ - id: "remote-before-click", + live.cb({ + id: "future-head", pubkey, - created_at: 100, - content: "remote", + created_at: 400, + content: "cipher", kind: 30078, tags: [["d", "channel-mutes"]], sig: "s", }); - for (let i = 0; i < 40; i++) await Promise.resolve(); + for (let i = 0; i < 20; i++) await Promise.resolve(); }); - + assert.equal( + hook.result.current.mutedChannelIds.has("shared"), + false, + "future head applied → unmuted", + ); + // Slow device clicks star at wall t=100. + await act(async () => hook.result.current.muteChannel("shared")); assert.equal( hook.result.current.mutedChannelIds.has("shared"), true, - "a later same-second local click must survive a delayed older remote", + "click must win despite the observed future timestamp", ); - assert.ok( - [...timers.values()].some((t) => t.ms === 2000), - "the local pending publish must remain scheduled", + const persisted = readChannelMutesStore(pubkey); + assert.equal( + persisted.channels.shared.updatedAt, + 400, + "mint lifted to t+300", ); + assert.equal(persisted.channels.shared.rev, 8, "rev = maxRevSeen+1"); hook.unmount(); } finally { cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - window.setTimeout = origSetTimeout; - window.clearTimeout = origClearTimeout; Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); } }); -// Fix round 4 (pass-4 finding 2): a canonical correction (lower id at the same -// event timestamp) knows the incoming event is the relay's winner, but NOT -// whether the user clicked between the superseded larger-id event and the -// correction. Sequence: stale `bbbb` applies → user clicks mute later in the -// same second → canonical `aaaa` decrypts late. `aaaa` and the click share -// integer `updatedAt`, so the plain remote-wins tie would clobber the click. -// The dirty-entry overlay keeps the click and the cancel is gone, so its -// publish stays scheduled. Mutation: dropping the dirty overlay (plain -// mergeApplyingRemote) lets `aaaa` erase the click; restoring the cancel drops -// its publish timer. -test("canonical correction preserves a same-second local click made after the larger-id event", async () => { +// Future-timestamp propagation (Thufir MINOR 1 / Paul MINOR 1): a poisoned +// far-future observation does not ratchet by itself — two opposite clicks keep +// the timestamp fixed at the observed future value while rev advances, and the +// latest click wins. No clamp; deterministic. +test("far-future observation: timestamp stays fixed, rev advances, latest click wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelMutesStore } = await import("./channelMutesStorage.ts"); + const { useChannelMutes } = await import("./useChannelMutes.ts"); + + const live = {}; + const restore = stubRelay(relayClient, { live }); + const origTauri = window.__TAURI_INTERNALS__; + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const FUTURE = 100 + 31_536_000; // +1yr + window.__TAURI_INTERNALS__ = { + invoke: (cmd) => { + if (cmd === "nip44_decrypt_from_self") + return Promise.resolve( + mutePayload({ shared: { muted: true, updatedAt: FUTURE, rev: 1 } }), + ); + return Promise.reject(new Error(`unmocked ${cmd}`)); + }, + }; + const pubkey = "pk-future"; + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelMutes(pubkey, "wss://r")); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + await act(async () => { + live.cb({ + id: "far-future", + pubkey, + created_at: FUTURE, + content: "cipher", + kind: 30078, + tags: [["d", "channel-mutes"]], + sig: "s", + }); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + await act(async () => hook.result.current.unmuteChannel("shared")); + let p = readChannelMutesStore(pubkey); + assert.equal(p.channels.shared.muted, false, "first click applied"); + assert.equal(p.channels.shared.updatedAt, FUTURE, "timestamp stays fixed"); + assert.equal(p.channels.shared.rev, 2, "rev advanced 1→2"); + await act(async () => hook.result.current.muteChannel("shared")); + p = readChannelMutesStore(pubkey); + assert.equal(p.channels.shared.muted, true, "latest click wins"); + assert.equal(p.channels.shared.updatedAt, FUTURE, "timestamp still fixed"); + assert.equal(p.channels.shared.rev, 3, "rev advanced 2→3"); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); + } +}); + +// Click-before-observation (design note gap test a): an empty-store click mints +// updatedAt=now, rev=1; a later bootstrap head carrying a HIGHER rev but an +// OLDER updatedAt for the opposite value must NOT reverse the click — updatedAt +// is primary. +test("empty-store click survives a later higher-rev head with an older updatedAt", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); const { useChannelMutes } = await import("./useChannelMutes.ts"); - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; + const live = {}; + const restore = stubRelay(relayClient, { live }); const origTauri = window.__TAURI_INTERNALS__; - const origSetTimeout = window.setTimeout; - const origClearTimeout = window.clearTimeout; const origDateNow = Date.now; - - const timers = new Map(); - let nextTimer = 1; - window.setTimeout = (fn, ms) => { - const id = nextTimer++; - timers.set(id, { fn, ms }); - return id; - }; - window.clearTimeout = (id) => timers.delete(id); - // The local click happens later within second 100. - Date.now = () => 100_900; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // bbbb (stale larger id) says muted; aaaa (canonical lower id) says not. - // Both carry the same entry updatedAt=100, tying the later local click. + Date.now = () => 1000 * 1_000; // click at updatedAt 1000 window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const canonicalLowerId = args?.ciphertext === "aaaa"; + invoke: (cmd) => { + if (cmd === "nip44_decrypt_from_self") + // older updatedAt (500) but higher rev (99), opposite value return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { shared: { muted: !canonicalLowerId, updatedAt: 100 } }, - }), + mutePayload({ shared: { muted: false, updatedAt: 500, rev: 99 } }), ); - } return Promise.reject(new Error(`unmocked ${cmd}`)); }, }; - - const pubkey = "pk-mute-canonical-dirty"; - const relayUrl = "wss://r.canon"; + const pubkey = "pk-empty"; let hook = null; try { await act(async () => { - hook = renderHook(() => useChannelMutes(pubkey, relayUrl)); + hook = renderHook(() => useChannelMutes(pubkey, "wss://r")); for (let i = 0; i < 20; i++) await Promise.resolve(); }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id, content) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 100, - content, - kind: 30078, - tags: [["d", "channel-mutes"]], - sig: "s", - }); - for (let i = 0; i < 40; i++) await Promise.resolve(); - }); - }; - - await deliver("bbbb", "bbbb"); // stale larger-id head applies (muted) + await act(async () => hook.result.current.muteChannel("shared")); // empty store → rev 1 @ 1000 await act(async () => { - hook.result.current.muteChannel("shared"); // user intent after bbbb + live.cb({ + id: "older-higher-rev", + pubkey, + created_at: 500, + content: "cipher", + kind: 30078, + tags: [["d", "channel-mutes"]], + sig: "s", + }); + for (let i = 0; i < 20; i++) await Promise.resolve(); }); - await deliver("aaaa", "aaaa"); // canonical correction decrypts late - assert.equal( hook.result.current.mutedChannelIds.has("shared"), true, - "a same-second local click must survive the canonical correction", - ); - assert.ok( - [...timers.values()].some((t) => t.ms === 2000), - "the local click's pending publish must remain scheduled", + "click at newer updatedAt survives an older higher-rev head", ); hook.unmount(); } finally { cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - window.setTimeout = origSetTimeout; - window.clearTimeout = origClearTimeout; Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); + } +}); + +// Cross-window storage: a peer window's write is observed into the high-water +// and max-merged, so a following click sees the peer's rev and no edit is lost. +test("cross-window storage event is observed and max-merged", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelMutesStore, storageKey } = await import( + "./channelMutesStorage.ts" + ); + const { useChannelMutes } = await import("./useChannelMutes.ts"); + + const restore = stubRelay(relayClient); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const pubkey = "pk-xwin"; + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelMutes(pubkey, "wss://r")); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + // A peer window wrote a higher-rev entry for `shared` at updatedAt 900. + window.localStorage.setItem( + storageKey(pubkey), + mutePayload({ shared: { muted: true, updatedAt: 900, rev: 12 } }), + ); + await act(async () => { + window.dispatchEvent( + new dom.window.StorageEvent("storage", { key: storageKey(pubkey) }), + ); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + assert.equal( + hook.result.current.mutedChannelIds.has("shared"), + true, + "peer write merged into this window", + ); + // A following click sees the peer's high-water: updatedAt held at 900, + // rev minted to 13. + await act(async () => hook.result.current.unmuteChannel("shared")); + const p = readChannelMutesStore(pubkey); + assert.equal(p.channels.shared.muted, false, "click applied"); + assert.equal(p.channels.shared.updatedAt, 900, "held at peer high-water"); + assert.equal(p.channels.shared.rev, 13, "rev = peer rev + 1"); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); + } +}); + +// Outbox resume: an edit persisted to the durable outbox before teardown is +// re-published on the next mount (bootstrap resume), so a click made <2s before +// quit/community-switch is never silently dropped. +test("bootstrap resumes a persisted outbox edit", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { useChannelMutes } = await import("./useChannelMutes.ts"); + + const restore = stubRelay(relayClient); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const pubkey = "pk-outbox"; + const relayUrl = "wss://r.outbox"; + const outboxKey = `buzz-channel-mutes-outbox.v1:${pubkey}:${encodeURIComponent(relayUrl)}`; + window.localStorage.setItem( + outboxKey, + mutePayload({ resumed: { muted: true, updatedAt: 90, rev: 2 } }), + ); + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelMutes(pubkey, relayUrl)); + for (let i = 0; i < 40; i++) await Promise.resolve(); + }); + // The resumed edit is queued for publish (pending), not silently dropped. + // We assert the pending publish debounce is scheduled by observing the + // outbox is still present (cleared only after publish completes). + assert.ok( + window.localStorage.getItem(outboxKey) !== null, + "outbox retained until the resumed publish completes", + ); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); } }); diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.ts b/desktop/src/features/sidebar/lib/useChannelMutes.ts index 65e7483c58..be28af6b7a 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.ts +++ b/desktop/src/features/sidebar/lib/useChannelMutes.ts @@ -3,10 +3,11 @@ import * as React from "react"; import { relayClient } from "@/shared/api/relayClient"; import { boundMuteStore, + clearChannelMutesOutbox, DEFAULT_STORE, - mergeCanonicalSupersession, mergeStores, mutedChannelIdsFromStore, + readChannelMutesOutbox, readChannelMutesStore, storageKey, writeChannelMutesStore, @@ -16,6 +17,13 @@ import { import { ChannelMuteSyncManager } from "./channelMutesSync"; import type { RemoteMutes } from "./channelMutesSync"; +// Reconciliation cadence. Steady interval re-fetches the head on a healthy +// socket so a silently-lost publish converges without waiting for a reconnect +// that may never fire; the retry window backs off while the fetch keeps failing. +const RECONCILE_STEADY_MS = 60_000; +const RECONCILE_RETRY_BASE_MS = 3_000; +const RECONCILE_RETRY_MAX_MS = 60_000; + export function useChannelMutes( pubkey: string | undefined, relayUrl?: string, @@ -32,28 +40,13 @@ export function useChannelMutes( }); const managerRef = React.useRef(null); - const lastAppliedRemoteTs = React.useRef(0); - const lastAppliedEventId = React.useRef(""); - // Channels the user changed locally within the current remote second. Their - // integer-second `updatedAt` ties the remote's, so a late canonical - // correction would clobber them on the remote-wins tie; the overlay in - // `mergeCanonicalSupersession` keeps them. Cleared whenever the remote clock - // strictly advances — a correction for an earlier second is then stale- - // rejected before it can apply, so the prior second's clicks need no cover. - const dirtyChannelIds = React.useRef>(new Set()); React.useEffect(() => { if (!pubkey || !relayUrl) { setStore(DEFAULT_STORE); - lastAppliedRemoteTs.current = 0; - lastAppliedEventId.current = ""; - dirtyChannelIds.current = new Set(); return; } setStore(readChannelMutesStore(pubkey)); - lastAppliedRemoteTs.current = 0; - lastAppliedEventId.current = ""; - dirtyChannelIds.current = new Set(); managerRef.current = new ChannelMuteSyncManager(pubkey, relayUrl); return () => { managerRef.current?.destroy(); @@ -61,6 +54,9 @@ export function useChannelMutes( }; }, [pubkey, relayUrl]); + // Cross-window sync: another window/tab wrote the shared store. Ingest it into + // the high-water and max-merge it into this window's state, so a click that + // follows sees the peer's revs/timestamps and no window's edit is clobbered. React.useEffect(() => { if (!pubkey) { return; @@ -70,7 +66,9 @@ export function useChannelMutes( if (e.key !== key) { return; } - setStore(readChannelMutesStore(pubkey)); + const incoming = readChannelMutesStore(pubkey); + managerRef.current?.observe(incoming); + setStore((prev) => mergeStores(prev, incoming)); }; window.addEventListener("storage", handler); return () => { @@ -78,47 +76,15 @@ export function useChannelMutes( }; }, [pubkey]); + // Every remote payload is observed by the manager before it reaches here + // (fetch/subscribe paths call observe() internally; the storage handler + // observes above), so this is a pure max-merge with no ordering or ownership + // overlay — "later" lives in the (updatedAt, rev) tuple. const applyRemote = React.useCallback( (remote: RemoteMutes): ((prev: ChannelMuteStore) => ChannelMuteStore) => { return (prev) => { if (!pubkey) return prev; - if (remote.createdAt < lastAppliedRemoteTs.current) return prev; - // Equal timestamps: the relay/database break ties by `id ASC` — the - // LOWEST event id is the canonical winner. Apply a strictly-lower id and - // ignore any id >= the last applied, so the UI converges on the same - // event the relay stored rather than the largest id seen. - if ( - remote.createdAt === lastAppliedRemoteTs.current && - remote.eventId >= lastAppliedEventId.current - ) - return prev; - // A canonical supersession corrects an already-applied same-timestamp - // LARGER-id head with the true winner: only here may the incoming blob's - // per-entry values win an equal-`updatedAt` tie. Any other application - // (bootstrap / live / newer timestamp) merges over optimistic local - // state with local-wins `mergeStores`. - const isCanonicalSupersession = - remote.createdAt === lastAppliedRemoteTs.current && - lastAppliedEventId.current !== "" && - remote.eventId < lastAppliedEventId.current; - // A strictly-newer remote second retires every locally-dirty entry: a - // later correction can only target this new second, so prior clicks - // need no cover and the set must not grow unbounded. - if (remote.createdAt > lastAppliedRemoteTs.current) - dirtyChannelIds.current = new Set(); - lastAppliedRemoteTs.current = remote.createdAt; - lastAppliedEventId.current = remote.eventId; - // A canonical correction must not erase a locally-owned entry the user - // changed within this same second (integer-second `updatedAt` ties the - // remote's). Overlay the dirty entries back on top of the correction, - // and never cancel the pending publish — the click still needs to sync. - const merged = isCanonicalSupersession - ? mergeCanonicalSupersession( - prev, - remote.store, - dirtyChannelIds.current, - ) - : mergeStores(prev, remote.store); + const merged = mergeStores(prev, remote.store); if (!writeChannelMutesStore(pubkey, merged)) return prev; return merged; }; @@ -136,12 +102,69 @@ export function useChannelMutes( setStore(applyRemote(result.data)); } // "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. + const outbox = readChannelMutesOutbox(pubkey, relayUrl); + if (outbox) { + managerRef.current?.publishMutes(outbox); + } else { + clearChannelMutesOutbox(pubkey, relayUrl); + } }); return () => { cancelled = true; }; }, [pubkey, relayUrl, applyRemote]); + // Reconciliation loop: a single scheduler that both retries a failed bootstrap + // fetch with bounded backoff and periodically re-fetches the head, so a + // silently-lost publish converges within the steady cadence without waiting + // for a reconnect a healthy socket never fires. Also refreshes on visibility. + React.useEffect(() => { + if (!pubkey || !relayUrl) return; + let cancelled = false; + let timer: number | null = null; + let delayMs = RECONCILE_RETRY_BASE_MS; + + const schedule = (ms: number) => { + if (cancelled) return; + if (timer !== null) window.clearTimeout(timer); + timer = window.setTimeout(tick, ms); + }; + + const tick = () => { + void managerRef.current?.fetchRemoteMutes().then((result) => { + if (cancelled) return; + if (result.status === "found") { + // max-merge folds the head into state without dropping a pending + // edit (that edit is in prev and owned by the manager's retry lane). + setStore(applyRemote(result.data)); + delayMs = RECONCILE_STEADY_MS; // relay answered → steady cadence + } else if (result.status === "absent") { + delayMs = RECONCILE_STEADY_MS; // answered (no blob) → steady cadence + } else { + delayMs = Math.min(delayMs * 2, RECONCILE_RETRY_MAX_MS); // failed → back off + } + schedule(delayMs); + }); + }; + + const onVisible = () => { + if (document.visibilityState === "visible") { + delayMs = RECONCILE_RETRY_BASE_MS; + tick(); + } + }; + document.addEventListener("visibilitychange", onVisible); + schedule(delayMs); + + return () => { + cancelled = true; + if (timer !== null) window.clearTimeout(timer); + document.removeEventListener("visibilitychange", onVisible); + }; + }, [pubkey, relayUrl, applyRemote]); + // biome-ignore lint/correctness/useExhaustiveDependencies: relayUrl is intentional — rebinds subscription when the active relay changes even though it is not used inside the effect body directly (the manager via managerRef.current carries it) React.useEffect(() => { if (!pubkey) return; @@ -196,11 +219,23 @@ export function useChannelMutes( const setMuteState = React.useCallback( (channelId: string, muted: boolean) => { if (!pubkey) return; - const entry: ChannelMuteEntry = { - muted, - updatedAt: Math.floor(Date.now() / 1000), - }; + const now = Math.floor(Date.now() / 1000); setStore((prev) => { + const manager = managerRef.current; + const localEntry = prev.channels[channelId]; + // Logical-monotonic mint: never regress below any (updatedAt, rev) this + // replica has observed for the channel (local entry OR manager + // high-water), so the click strictly dominates observed state in both + // merge keys — it can never lose to state it has already seen. + const updatedAt = Math.max( + now, + localEntry?.updatedAt ?? 0, + manager?.maxUpdatedAtSeen(channelId) ?? 0, + ); + const rev = + Math.max(localEntry?.rev ?? 0, manager?.maxRevSeen(channelId) ?? 0) + + 1; + const entry: ChannelMuteEntry = { muted, updatedAt, rev }; const next = boundMuteStore( { version: 1, @@ -209,11 +244,7 @@ export function useChannelMutes( channelId, ); if (!writeChannelMutesStore(pubkey, next)) return prev; - // Mark this channel locally-owned for the current remote second so a - // late canonical correction with the same integer `updatedAt` can't - // clobber the click before its publish syncs. - dirtyChannelIds.current.add(channelId); - managerRef.current?.publishMutes(next); + manager?.publishMutes(next); return next; }); }, diff --git a/desktop/src/features/sidebar/lib/useChannelStars.test.mjs b/desktop/src/features/sidebar/lib/useChannelStars.test.mjs index e8a1054c30..4317279ef5 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.test.mjs +++ b/desktop/src/features/sidebar/lib/useChannelStars.test.mjs @@ -18,6 +18,27 @@ before(() => { after(() => dom.window.close()); +// Shared harness: stub the relay so no network/live/reconnect fires unless a +// test installs its own live callback. Returns the captured live callback. +function stubRelay(relayClient, { live } = {}) { + const orig = { + fetchEvents: relayClient.fetchEvents, + subscribeLive: relayClient.subscribeLive, + subscribeToReconnects: relayClient.subscribeToReconnects, + }; + relayClient.fetchEvents = async () => []; + relayClient.subscribeLive = async (_f, cb) => { + if (live) live.cb = cb; + return async () => {}; + }; + relayClient.subscribeToReconnects = () => () => {}; + return () => Object.assign(relayClient, orig); +} + +function starPayload(channels) { + return JSON.stringify({ version: 1, channels }); +} + test("same-second star and unstar mutations survive at capacity", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); @@ -25,21 +46,16 @@ test("same-second star and unstar mutations survive at capacity", async () => { await import("./channelStarsStorage.ts"); const { useChannelStars } = await import("./useChannelStars.ts"); - const originalFetchEvents = relayClient.fetchEvents; - const originalSubscribeLive = relayClient.subscribeLive; - const originalSubscribeToReconnects = relayClient.subscribeToReconnects; + const restore = stubRelay(relayClient); const originalDateNow = Date.now; const updatedAt = 1_234_567; Date.now = () => updatedAt * 1_000; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async () => async () => {}; - relayClient.subscribeToReconnects = () => () => {}; const relayUrl = "wss://relay.example"; const channels = Object.fromEntries( Array.from({ length: MAX_CHANNEL_STAR_ENTRIES }, (_, index) => [ `z-channel-${String(index).padStart(3, "0")}`, - { starred: true, updatedAt }, + { starred: true, updatedAt, rev: 0 }, ]), ); @@ -55,416 +71,348 @@ test("same-second star and unstar mutations survive at capacity", async () => { const { result, unmount } = renderHook(() => useChannelStars(pubkey, relayUrl), ); - act(() => result.current[action]("a-target")); - const persisted = readChannelStarsStore(pubkey); assert.equal( Object.keys(persisted.channels).length, MAX_CHANNEL_STAR_ENTRIES, ); - assert.deepEqual(persisted.channels["a-target"], { - starred: expectedStarred, - updatedAt, - }); + assert.equal(persisted.channels["a-target"].starred, expectedStarred); unmount(); } } finally { cleanup(); Date.now = originalDateNow; - relayClient.fetchEvents = originalFetchEvents; - relayClient.subscribeLive = originalSubscribeLive; - relayClient.subscribeToReconnects = originalSubscribeToReconnects; + restore(); } }); -// Equal-timestamp tie-break must match the relay's canonical winner -// (`created_at DESC, id ASC` → LOWEST id wins). Deliver the larger id first, -// then the lower id at the same timestamp; the lower id is the stored winner -// and must be applied, not rejected. Reverting applyRemote's `>=` back to `<=` -// wrongly ignores the lower id (the actual relay winner). -test("equal-timestamp tie-break applies the lower event id (relay canonical winner)", async () => { +// Symmetric mint (Thufir MINOR 2): a click mints +// updatedAt = max(now, localEntry.updatedAt, maxUpdatedAtSeen) and +// rev = max(localEntry.rev, maxRevSeen) + 1 on BOTH dimensions. A remount reads +// the persisted local entry, so the very first click advances past it. +test("persisted-local first click mints seen+1 on both dimensions", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelStarsStore, storageKey } = await import( + "./channelStarsStorage.ts" + ); const { useChannelStars } = await import("./useChannelStars.ts"); - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // Decrypt payload keyed off the event id embedded in the ciphertext so each - // delivered event yields a store starring a distinct channel we can assert on. - window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const id = args?.ciphertext ?? ""; - return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { [id]: { starred: true, updatedAt: 0 } }, - }), - ); - } - return Promise.reject(new Error(`unmocked ${cmd}`)); - }, - }; - - const pubkey = "pk-star-tie"; - const relayUrl = "wss://r.tie"; - let hook = null; - try { - await act(async () => { - hook = renderHook(() => useChannelStars(pubkey, relayUrl)); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 1000, - content: id, // decrypt echoes this into the starred channel id - kind: 30078, - tags: [["d", "channel-stars"]], - sig: "s", - }); - await Promise.resolve(); - await Promise.resolve(); - }); - }; - - // Larger id first (applied), then the lower id at the same timestamp — the - // relay's canonical winner, which must NOT be rejected. - await deliver("bbbb"); - await deliver("aaaa"); - - assert.ok( - hook.result.current.starredChannelIds.has("aaaa"), - "lower event id (relay canonical winner) must be applied, not rejected", - ); - hook.unmount(); - } finally { - cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - } -}); - -// Pass-2 finding 2: the comparator admitting the canonical lower id is -// necessary but not sufficient. Stars are a per-entry store, so applyRemote -// merges the incoming blob into local state. On the SAME channel, a stale -// larger-id event delivered first (starred=true) must not survive the merge -// once the canonical lower-id winner (starred=false) arrives at the same entry -// `updatedAt`. Mutation: reverting the apply path to mergeStores (local/prev -// wins on tie) keeps the stale starred=true value. -test("canonical lower-id unstar replaces a stale larger-id star at equal entry timestamp", async () => { - const { act, cleanup, renderHook } = await import("@testing-library/react"); - const { relayClient } = await import("@/shared/api/relayClient"); - const { useChannelStars } = await import("./useChannelStars.ts"); - - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // Both events target the SAME channel `shared` at the same entry updatedAt. - // The larger id `bbbb` says starred; the canonical lower id `aaaa` says not. - window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const canonicalLowerId = args?.ciphertext === "aaaa"; - return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { - shared: { starred: !canonicalLowerId, updatedAt: 100 }, - }, - }), - ); - } - return Promise.reject(new Error(`unmocked ${cmd}`)); - }, - }; - - const pubkey = "pk-star-shared-tie"; - const relayUrl = "wss://r.tie"; - let hook = null; - try { - await act(async () => { - hook = renderHook(() => useChannelStars(pubkey, relayUrl)); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 1000, - content: id, - kind: 30078, - tags: [["d", "channel-stars"]], - sig: "s", - }); - for (let i = 0; i < 20; i++) await Promise.resolve(); - }); - }; - - await deliver("bbbb"); // stale larger-id head says starred - await deliver("aaaa"); // canonical lower-id winner says unstarred - - assert.equal( - hook.result.current.starredChannelIds.has("shared"), - false, - "canonical lower-id unstar must replace the stale larger-id star", - ); - hook.unmount(); - } finally { - cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - } -}); - -// Fix round 3 (pass-3 finding 2): the remote-wins entry-tie merge and the -// pending-publish cancel apply ONLY to a canonical supersession (a lower id at -// the same event timestamp correcting an already-applied larger id). A plain -// live/bootstrap remote must NOT clobber a later same-second local click or -// cancel its pending publish. Entry `updatedAt` is whole seconds, so a click at -// 100.9s and an older remote entry at 100.1s both carry `updatedAt:100`; the -// later local intent must win and keep publishing. Mutation: applying -// mergeApplyingRemote + cancel unconditionally lets the delayed remote overwrite -// the click and drop its publish. -test("delayed same-second remote does not clobber a later local star or cancel its publish", async () => { - const { act, cleanup, renderHook } = await import("@testing-library/react"); - const { relayClient } = await import("@/shared/api/relayClient"); - const { useChannelStars } = await import("./useChannelStars.ts"); - - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; - const origTauri = window.__TAURI_INTERNALS__; - const origSetTimeout = window.setTimeout; - const origClearTimeout = window.clearTimeout; + const restore = stubRelay(relayClient); const origDateNow = Date.now; + // Persisted entry is stamped in the FUTURE relative to wall clock, with a + // non-zero rev — the mint must not regress below either. + Date.now = () => 100 * 1_000; + const pubkey = "pk-persist"; + window.localStorage.setItem( + storageKey(pubkey), + starPayload({ shared: { starred: true, updatedAt: 500, rev: 4 } }), + ); + try { + const { result, unmount } = renderHook(() => + useChannelStars(pubkey, "wss://r"), + ); + act(() => result.current.unstarChannel("shared")); + const persisted = readChannelStarsStore(pubkey); + assert.equal(persisted.channels.shared.starred, false, "unstar applied"); + assert.equal( + persisted.channels.shared.updatedAt, + 500, + "updatedAt held at persisted-local high-water (max(100,500,seen))", + ); + assert.equal( + persisted.channels.shared.rev, + 5, + "rev minted as persisted-local rev + 1", + ); + unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); + } +}); - const timers = new Map(); - let nextTimer = 1; - window.setTimeout = (fn, ms) => { - const id = nextTimer++; - timers.set(id, { fn, ms }); - return id; - }; - window.clearTimeout = (id) => timers.delete(id); - // The local click happens later within second 100. - Date.now = () => 100_900; +// Fast-clock veto fix (Thufir pass-2 finding 1): after observing a +// future-stamped remote (updatedAt = t+300, rev 7, unstarred), a slow device +// clicking star at wall-clock t must WIN — the logical-monotonic stamp lifts the +// click's updatedAt to t+300 and rev to 8, so it dominates the observed entry. +test("fast-clock veto fix: click after observing a future-stamped head wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelStarsStore } = await import("./channelStarsStorage.ts"); + const { useChannelStars } = await import("./useChannelStars.ts"); - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // The delayed remote entry sits earlier in the same second and says unstarred. + const live = {}; + const restore = stubRelay(relayClient, { live }); + const origTauri = window.__TAURI_INTERNALS__; + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; // slow device: wall clock t = 100 window.__TAURI_INTERNALS__ = { invoke: (cmd) => { if (cmd === "nip44_decrypt_from_self") return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { shared: { starred: false, updatedAt: 100 } }, - }), + starPayload({ shared: { starred: false, updatedAt: 400, rev: 7 } }), ); return Promise.reject(new Error(`unmocked ${cmd}`)); }, }; - - const pubkey = "pk-star-same-second"; - const relayUrl = "wss://r.same"; + const pubkey = "pk-fastclock"; let hook = null; try { await act(async () => { - hook = renderHook(() => useChannelStars(pubkey, relayUrl)); + hook = renderHook(() => useChannelStars(pubkey, "wss://r")); for (let i = 0; i < 20; i++) await Promise.resolve(); }); - assert.ok(live, "live subscription installed"); - - // Local optimistic click: starred=true at updatedAt=100 (Date.now=100.9s). + // Observe the future-stamped head (updatedAt 400 = t+300). await act(async () => { - hook.result.current.starChannel("shared"); - }); - // An older remote entry from the same second decrypts and applies late. - await act(async () => { - live({ - id: "remote-before-click", + live.cb({ + id: "future-head", pubkey, - created_at: 100, - content: "remote", + created_at: 400, + content: "cipher", kind: 30078, tags: [["d", "channel-stars"]], sig: "s", }); - for (let i = 0; i < 40; i++) await Promise.resolve(); + for (let i = 0; i < 20; i++) await Promise.resolve(); }); - + assert.equal( + hook.result.current.starredChannelIds.has("shared"), + false, + "future head applied → unstarred", + ); + // Slow device clicks star at wall t=100. + await act(async () => hook.result.current.starChannel("shared")); assert.equal( hook.result.current.starredChannelIds.has("shared"), true, - "a later same-second local click must survive a delayed older remote", + "click must win despite the observed future timestamp", ); - assert.ok( - [...timers.values()].some((t) => t.ms === 2000), - "the local pending publish must remain scheduled", + const persisted = readChannelStarsStore(pubkey); + assert.equal( + persisted.channels.shared.updatedAt, + 400, + "mint lifted to t+300", ); + assert.equal(persisted.channels.shared.rev, 8, "rev = maxRevSeen+1"); hook.unmount(); } finally { cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - window.setTimeout = origSetTimeout; - window.clearTimeout = origClearTimeout; Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); } }); -// Fix round 4 (pass-4 finding 2): a canonical correction (lower id at the same -// event timestamp) knows the incoming event is the relay's winner, but NOT -// whether the user clicked between the superseded larger-id event and the -// correction. Sequence: stale `bbbb` applies → user clicks star later in the -// same second → canonical `aaaa` decrypts late. `aaaa` and the click share -// integer `updatedAt`, so the plain remote-wins tie would clobber the click. -// The dirty-entry overlay keeps the click and the cancel is gone, so its -// publish stays scheduled. Mutation: dropping the dirty overlay (plain -// mergeApplyingRemote) lets `aaaa` erase the click; restoring the cancel drops -// its publish timer. -test("canonical correction preserves a same-second local click made after the larger-id event", async () => { +// Future-timestamp propagation (Thufir MINOR 1 / Paul MINOR 1): a poisoned +// far-future observation does not ratchet by itself — two opposite clicks keep +// the timestamp fixed at the observed future value while rev advances, and the +// latest click wins. No clamp; deterministic. +test("far-future observation: timestamp stays fixed, rev advances, latest click wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelStarsStore } = await import("./channelStarsStorage.ts"); + const { useChannelStars } = await import("./useChannelStars.ts"); + + const live = {}; + const restore = stubRelay(relayClient, { live }); + const origTauri = window.__TAURI_INTERNALS__; + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const FUTURE = 100 + 31_536_000; // +1yr + window.__TAURI_INTERNALS__ = { + invoke: (cmd) => { + if (cmd === "nip44_decrypt_from_self") + return Promise.resolve( + starPayload({ shared: { starred: true, updatedAt: FUTURE, rev: 1 } }), + ); + return Promise.reject(new Error(`unmocked ${cmd}`)); + }, + }; + const pubkey = "pk-future"; + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelStars(pubkey, "wss://r")); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + await act(async () => { + live.cb({ + id: "far-future", + pubkey, + created_at: FUTURE, + content: "cipher", + kind: 30078, + tags: [["d", "channel-stars"]], + sig: "s", + }); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + await act(async () => hook.result.current.unstarChannel("shared")); + let p = readChannelStarsStore(pubkey); + assert.equal(p.channels.shared.starred, false, "first click applied"); + assert.equal(p.channels.shared.updatedAt, FUTURE, "timestamp stays fixed"); + assert.equal(p.channels.shared.rev, 2, "rev advanced 1→2"); + await act(async () => hook.result.current.starChannel("shared")); + p = readChannelStarsStore(pubkey); + assert.equal(p.channels.shared.starred, true, "latest click wins"); + assert.equal(p.channels.shared.updatedAt, FUTURE, "timestamp still fixed"); + assert.equal(p.channels.shared.rev, 3, "rev advanced 2→3"); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); + } +}); + +// Click-before-observation (design note gap test a): an empty-store click mints +// updatedAt=now, rev=1; a later bootstrap head carrying a HIGHER rev but an +// OLDER updatedAt for the opposite value must NOT reverse the click — updatedAt +// is primary. +test("empty-store click survives a later higher-rev head with an older updatedAt", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { relayClient } = await import("@/shared/api/relayClient"); const { useChannelStars } = await import("./useChannelStars.ts"); - const origFetch = relayClient.fetchEvents; - const origLive = relayClient.subscribeLive; - const origReconnect = relayClient.subscribeToReconnects; + const live = {}; + const restore = stubRelay(relayClient, { live }); const origTauri = window.__TAURI_INTERNALS__; - const origSetTimeout = window.setTimeout; - const origClearTimeout = window.clearTimeout; const origDateNow = Date.now; - - const timers = new Map(); - let nextTimer = 1; - window.setTimeout = (fn, ms) => { - const id = nextTimer++; - timers.set(id, { fn, ms }); - return id; - }; - window.clearTimeout = (id) => timers.delete(id); - // The local click happens later within second 100. - Date.now = () => 100_900; - - let live = null; - relayClient.fetchEvents = async () => []; - relayClient.subscribeLive = async (_f, cb) => { - live = cb; - return async () => {}; - }; - relayClient.subscribeToReconnects = () => () => {}; - // bbbb (stale larger id) says starred; aaaa (canonical lower id) says not. - // Both carry the same entry updatedAt=100, tying the later local click. + Date.now = () => 1000 * 1_000; // click at updatedAt 1000 window.__TAURI_INTERNALS__ = { - invoke: (cmd, args) => { - if (cmd === "nip44_decrypt_from_self") { - const canonicalLowerId = args?.ciphertext === "aaaa"; + invoke: (cmd) => { + if (cmd === "nip44_decrypt_from_self") + // older updatedAt (500) but higher rev (99), opposite value return Promise.resolve( - JSON.stringify({ - version: 1, - channels: { - shared: { starred: !canonicalLowerId, updatedAt: 100 }, - }, - }), + starPayload({ shared: { starred: false, updatedAt: 500, rev: 99 } }), ); - } return Promise.reject(new Error(`unmocked ${cmd}`)); }, }; - - const pubkey = "pk-star-canonical-dirty"; - const relayUrl = "wss://r.canon"; + const pubkey = "pk-empty"; let hook = null; try { await act(async () => { - hook = renderHook(() => useChannelStars(pubkey, relayUrl)); + hook = renderHook(() => useChannelStars(pubkey, "wss://r")); for (let i = 0; i < 20; i++) await Promise.resolve(); }); - assert.ok(live, "live subscription installed"); - - const deliver = async (id, content) => { - await act(async () => { - live({ - id, - pubkey, - created_at: 100, - content, - kind: 30078, - tags: [["d", "channel-stars"]], - sig: "s", - }); - for (let i = 0; i < 40; i++) await Promise.resolve(); - }); - }; - - await deliver("bbbb", "bbbb"); // stale larger-id head applies (starred) + await act(async () => hook.result.current.starChannel("shared")); // empty store → rev 1 @ 1000 await act(async () => { - hook.result.current.starChannel("shared"); // user intent after bbbb + live.cb({ + id: "older-higher-rev", + pubkey, + created_at: 500, + content: "cipher", + kind: 30078, + tags: [["d", "channel-stars"]], + sig: "s", + }); + for (let i = 0; i < 20; i++) await Promise.resolve(); }); - await deliver("aaaa", "aaaa"); // canonical correction decrypts late - assert.equal( hook.result.current.starredChannelIds.has("shared"), true, - "a same-second local click must survive the canonical correction", - ); - assert.ok( - [...timers.values()].some((t) => t.ms === 2000), - "the local click's pending publish must remain scheduled", + "click at newer updatedAt survives an older higher-rev head", ); hook.unmount(); } finally { cleanup(); - relayClient.fetchEvents = origFetch; - relayClient.subscribeLive = origLive; - relayClient.subscribeToReconnects = origReconnect; - window.__TAURI_INTERNALS__ = origTauri; - window.setTimeout = origSetTimeout; - window.clearTimeout = origClearTimeout; Date.now = origDateNow; + window.__TAURI_INTERNALS__ = origTauri; + restore(); + } +}); + +// Cross-window storage: a peer window's write is observed into the high-water +// and max-merged, so a following click sees the peer's rev and no edit is lost. +test("cross-window storage event is observed and max-merged", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { readChannelStarsStore, storageKey } = await import( + "./channelStarsStorage.ts" + ); + const { useChannelStars } = await import("./useChannelStars.ts"); + + const restore = stubRelay(relayClient); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const pubkey = "pk-xwin"; + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelStars(pubkey, "wss://r")); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + // A peer window wrote a higher-rev entry for `shared` at updatedAt 900. + window.localStorage.setItem( + storageKey(pubkey), + starPayload({ shared: { starred: true, updatedAt: 900, rev: 12 } }), + ); + await act(async () => { + window.dispatchEvent( + new dom.window.StorageEvent("storage", { key: storageKey(pubkey) }), + ); + for (let i = 0; i < 20; i++) await Promise.resolve(); + }); + assert.equal( + hook.result.current.starredChannelIds.has("shared"), + true, + "peer write merged into this window", + ); + // A following click sees the peer's high-water: updatedAt held at 900, + // rev minted to 13. + await act(async () => hook.result.current.unstarChannel("shared")); + const p = readChannelStarsStore(pubkey); + assert.equal(p.channels.shared.starred, false, "click applied"); + assert.equal(p.channels.shared.updatedAt, 900, "held at peer high-water"); + assert.equal(p.channels.shared.rev, 13, "rev = peer rev + 1"); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); + } +}); + +// Outbox resume: an edit persisted to the durable outbox before teardown is +// re-published on the next mount (bootstrap resume), so a click made <2s before +// quit/community-switch is never silently dropped. +test("bootstrap resumes a persisted outbox edit", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { useChannelStars } = await import("./useChannelStars.ts"); + + const restore = stubRelay(relayClient); + const origDateNow = Date.now; + Date.now = () => 100 * 1_000; + const pubkey = "pk-outbox"; + const relayUrl = "wss://r.outbox"; + const outboxKey = `buzz-channel-stars-outbox.v1:${pubkey}:${encodeURIComponent(relayUrl)}`; + window.localStorage.setItem( + outboxKey, + starPayload({ resumed: { starred: true, updatedAt: 90, rev: 2 } }), + ); + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelStars(pubkey, relayUrl)); + for (let i = 0; i < 40; i++) await Promise.resolve(); + }); + // The resumed edit is queued for publish (pending), not silently dropped. + // We assert the pending publish debounce is scheduled by observing the + // outbox is still present (cleared only after publish completes). + assert.ok( + window.localStorage.getItem(outboxKey) !== null, + "outbox retained until the resumed publish completes", + ); + hook.unmount(); + } finally { + cleanup(); + Date.now = origDateNow; + restore(); } }); diff --git a/desktop/src/features/sidebar/lib/useChannelStars.ts b/desktop/src/features/sidebar/lib/useChannelStars.ts index 7b6e87eccb..b4c869f0d3 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.ts +++ b/desktop/src/features/sidebar/lib/useChannelStars.ts @@ -3,9 +3,10 @@ import * as React from "react"; import { relayClient } from "@/shared/api/relayClient"; import { boundStarStore, + clearChannelStarsOutbox, DEFAULT_STORE, - mergeCanonicalSupersession, mergeStores, + readChannelStarsOutbox, readChannelStarsStore, starredChannelIdsFromStore, storageKey, @@ -16,6 +17,13 @@ import { import { ChannelStarSyncManager } from "./channelStarsSync"; import type { RemoteStars } from "./channelStarsSync"; +// Reconciliation cadence. Steady interval re-fetches the head on a healthy +// socket so a silently-lost publish converges without waiting for a reconnect +// that may never fire; the retry window backs off while the fetch keeps failing. +const RECONCILE_STEADY_MS = 60_000; +const RECONCILE_RETRY_BASE_MS = 3_000; +const RECONCILE_RETRY_MAX_MS = 60_000; + export function useChannelStars( pubkey: string | undefined, relayUrl?: string, @@ -32,28 +40,13 @@ export function useChannelStars( }); const managerRef = React.useRef(null); - const lastAppliedRemoteTs = React.useRef(0); - const lastAppliedEventId = React.useRef(""); - // Channels the user changed locally within the current remote second. Their - // integer-second `updatedAt` ties the remote's, so a late canonical - // correction would clobber them on the remote-wins tie; the overlay in - // `mergeCanonicalSupersession` keeps them. Cleared whenever the remote clock - // strictly advances — a correction for an earlier second is then stale- - // rejected before it can apply, so the prior second's clicks need no cover. - const dirtyChannelIds = React.useRef>(new Set()); React.useEffect(() => { if (!pubkey || !relayUrl) { setStore(DEFAULT_STORE); - lastAppliedRemoteTs.current = 0; - lastAppliedEventId.current = ""; - dirtyChannelIds.current = new Set(); return; } setStore(readChannelStarsStore(pubkey)); - lastAppliedRemoteTs.current = 0; - lastAppliedEventId.current = ""; - dirtyChannelIds.current = new Set(); managerRef.current = new ChannelStarSyncManager(pubkey, relayUrl); return () => { managerRef.current?.destroy(); @@ -61,6 +54,9 @@ export function useChannelStars( }; }, [pubkey, relayUrl]); + // Cross-window sync: another window/tab wrote the shared store. Ingest it into + // the high-water and max-merge it into this window's state, so a click that + // follows sees the peer's revs/timestamps and no window's edit is clobbered. React.useEffect(() => { if (!pubkey) { return; @@ -70,7 +66,9 @@ export function useChannelStars( if (e.key !== key) { return; } - setStore(readChannelStarsStore(pubkey)); + const incoming = readChannelStarsStore(pubkey); + managerRef.current?.observe(incoming); + setStore((prev) => mergeStores(prev, incoming)); }; window.addEventListener("storage", handler); return () => { @@ -78,47 +76,15 @@ export function useChannelStars( }; }, [pubkey]); + // Every remote payload is observed by the manager before it reaches here + // (fetch/subscribe paths call observe() internally; the storage handler + // observes above), so this is a pure max-merge with no ordering or ownership + // overlay — "later" lives in the (updatedAt, rev) tuple. const applyRemote = React.useCallback( (remote: RemoteStars): ((prev: ChannelStarStore) => ChannelStarStore) => { return (prev) => { if (!pubkey) return prev; - if (remote.createdAt < lastAppliedRemoteTs.current) return prev; - // Equal timestamps: the relay/database break ties by `id ASC` — the - // LOWEST event id is the canonical winner. Apply a strictly-lower id and - // ignore any id >= the last applied, so the UI converges on the same - // event the relay stored rather than the largest id seen. - if ( - remote.createdAt === lastAppliedRemoteTs.current && - remote.eventId >= lastAppliedEventId.current - ) - return prev; - // A canonical supersession corrects an already-applied same-timestamp - // LARGER-id head with the true winner: only here may the incoming blob's - // per-entry values win an equal-`updatedAt` tie. Any other application - // (bootstrap / live / newer timestamp) merges over optimistic local - // state with local-wins `mergeStores`. - const isCanonicalSupersession = - remote.createdAt === lastAppliedRemoteTs.current && - lastAppliedEventId.current !== "" && - remote.eventId < lastAppliedEventId.current; - // A strictly-newer remote second retires every locally-dirty entry: a - // later correction can only target this new second, so prior clicks - // need no cover and the set must not grow unbounded. - if (remote.createdAt > lastAppliedRemoteTs.current) - dirtyChannelIds.current = new Set(); - lastAppliedRemoteTs.current = remote.createdAt; - lastAppliedEventId.current = remote.eventId; - // A canonical correction must not erase a locally-owned entry the user - // changed within this same second (integer-second `updatedAt` ties the - // remote's). Overlay the dirty entries back on top of the correction, - // and never cancel the pending publish — the click still needs to sync. - const merged = isCanonicalSupersession - ? mergeCanonicalSupersession( - prev, - remote.store, - dirtyChannelIds.current, - ) - : mergeStores(prev, remote.store); + const merged = mergeStores(prev, remote.store); if (!writeChannelStarsStore(pubkey, merged)) return prev; return merged; }; @@ -136,12 +102,69 @@ export function useChannelStars( setStore(applyRemote(result.data)); } // "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. + const outbox = readChannelStarsOutbox(pubkey, relayUrl); + if (outbox) { + managerRef.current?.publishStars(outbox); + } else { + clearChannelStarsOutbox(pubkey, relayUrl); + } }); return () => { cancelled = true; }; }, [pubkey, relayUrl, applyRemote]); + // Reconciliation loop: a single scheduler that both retries a failed bootstrap + // fetch with bounded backoff and periodically re-fetches the head, so a + // silently-lost publish converges within the steady cadence without waiting + // for a reconnect a healthy socket never fires. Also refreshes on visibility. + React.useEffect(() => { + if (!pubkey || !relayUrl) return; + let cancelled = false; + let timer: number | null = null; + let delayMs = RECONCILE_RETRY_BASE_MS; + + const schedule = (ms: number) => { + if (cancelled) return; + if (timer !== null) window.clearTimeout(timer); + timer = window.setTimeout(tick, ms); + }; + + const tick = () => { + void managerRef.current?.fetchRemoteStars().then((result) => { + if (cancelled) return; + if (result.status === "found") { + // max-merge folds the head into state without dropping a pending + // edit (that edit is in prev and owned by the manager's retry lane). + setStore(applyRemote(result.data)); + delayMs = RECONCILE_STEADY_MS; // relay answered → steady cadence + } else if (result.status === "absent") { + delayMs = RECONCILE_STEADY_MS; // answered (no blob) → steady cadence + } else { + delayMs = Math.min(delayMs * 2, RECONCILE_RETRY_MAX_MS); // failed → back off + } + schedule(delayMs); + }); + }; + + const onVisible = () => { + if (document.visibilityState === "visible") { + delayMs = RECONCILE_RETRY_BASE_MS; + tick(); + } + }; + document.addEventListener("visibilitychange", onVisible); + schedule(delayMs); + + return () => { + cancelled = true; + if (timer !== null) window.clearTimeout(timer); + document.removeEventListener("visibilitychange", onVisible); + }; + }, [pubkey, relayUrl, applyRemote]); + // biome-ignore lint/correctness/useExhaustiveDependencies: relayUrl is intentional — rebinds subscription when the active relay changes even though it is not used inside the effect body directly (the manager via managerRef.current carries it) React.useEffect(() => { if (!pubkey) return; @@ -196,11 +219,23 @@ export function useChannelStars( const setStarState = React.useCallback( (channelId: string, starred: boolean) => { if (!pubkey) return; - const entry: ChannelStarEntry = { - starred, - updatedAt: Math.floor(Date.now() / 1000), - }; + const now = Math.floor(Date.now() / 1000); setStore((prev) => { + const manager = managerRef.current; + const localEntry = prev.channels[channelId]; + // Logical-monotonic mint: never regress below any (updatedAt, rev) this + // replica has observed for the channel (local entry OR manager + // high-water), so the click strictly dominates observed state in both + // merge keys — it can never lose to state it has already seen. + const updatedAt = Math.max( + now, + localEntry?.updatedAt ?? 0, + manager?.maxUpdatedAtSeen(channelId) ?? 0, + ); + const rev = + Math.max(localEntry?.rev ?? 0, manager?.maxRevSeen(channelId) ?? 0) + + 1; + const entry: ChannelStarEntry = { starred, updatedAt, rev }; const next = boundStarStore( { version: 1, @@ -209,11 +244,7 @@ export function useChannelStars( channelId, ); if (!writeChannelStarsStore(pubkey, next)) return prev; - // Mark this channel locally-owned for the current remote second so a - // late canonical correction with the same integer `updatedAt` can't - // clobber the click before its publish syncs. - dirtyChannelIds.current.add(channelId); - managerRef.current?.publishStars(next); + manager?.publishStars(next); return next; }); },