From d5d83e09092b4e26bcf083ced68872273551ef8b Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 21 Aug 2026 20:24:10 -0400 Subject: [PATCH] fix(sidebar): correct equal-timestamp tie-break in stars, mutes, sort useChannelStars, useChannelMutes, and useChannelSortPreference carried the same inverted equal-timestamp comparator as channel sections: applyRemote kept the largest event id, opposite the relay/database canonical order (created_at DESC, id ASC -> lowest id wins). Two devices writing the same second could leave the UI showing an event the relay did not store. Apply a strictly-lower id and ignore ids >= the last applied, matching the sections fix and the relay winner across all four 30078 sidebar surfaces. Each hook gains a regression test: larger-then-lower id delivery at equal timestamp, lower-id store wins (mutation-checked - reverting >= to <= fails each). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../sidebar/lib/useChannelMutes.test.mjs | 86 ++++++++++++++ .../features/sidebar/lib/useChannelMutes.ts | 6 +- .../lib/useChannelSortPreference.test.mjs | 110 ++++++++++++++++++ .../sidebar/lib/useChannelSortPreference.ts | 6 +- .../sidebar/lib/useChannelStars.test.mjs | 86 ++++++++++++++ .../features/sidebar/lib/useChannelStars.ts | 6 +- 6 files changed, 297 insertions(+), 3 deletions(-) create mode 100644 desktop/src/features/sidebar/lib/useChannelSortPreference.test.mjs diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs b/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs index df41f40311..a3610d7678 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs +++ b/desktop/src/features/sidebar/lib/useChannelMutes.test.mjs @@ -77,3 +77,89 @@ test("same-second mute and unmute mutations survive at capacity", async () => { relayClient.subscribeToReconnects = originalSubscribeToReconnects; } }); + +// 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 () => { + 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 = () => () => {}; + // 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; + } +}); diff --git a/desktop/src/features/sidebar/lib/useChannelMutes.ts b/desktop/src/features/sidebar/lib/useChannelMutes.ts index 20b5745325..85ec9aeb8e 100644 --- a/desktop/src/features/sidebar/lib/useChannelMutes.ts +++ b/desktop/src/features/sidebar/lib/useChannelMutes.ts @@ -73,9 +73,13 @@ export function useChannelMutes( 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 + remote.eventId >= lastAppliedEventId.current ) return prev; lastAppliedRemoteTs.current = remote.createdAt; diff --git a/desktop/src/features/sidebar/lib/useChannelSortPreference.test.mjs b/desktop/src/features/sidebar/lib/useChannelSortPreference.test.mjs new file mode 100644 index 0000000000..a9a65754b0 --- /dev/null +++ b/desktop/src/features/sidebar/lib/useChannelSortPreference.test.mjs @@ -0,0 +1,110 @@ +import assert from "node:assert/strict"; +import { after, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); +}); + +after(() => dom.window.close()); + +// 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 its whole-blob store must replace the applied state. Reverting +// applyRemote's `>=` back to `<=` wrongly ignores the lower id (the relay winner). +test("equal-timestamp tie-break applies the lower event id (relay canonical winner)", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { relayClient } = await import("@/shared/api/relayClient"); + const { useChannelSortPreference } = await import( + "./useChannelSortPreference.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 setting a distinct group's mode to "recent". + window.__TAURI_INTERNALS__ = { + invoke: (cmd, args) => { + if (cmd === "nip44_decrypt_from_self") { + const id = args?.ciphertext ?? ""; + return Promise.resolve( + JSON.stringify({ version: 1, groups: { [id]: "recent" } }), + ); + } + return Promise.reject(new Error(`unmocked ${cmd}`)); + }, + }; + + const pubkey = "pk-sort-tie"; + const relayUrl = "wss://r.tie"; + let hook = null; + try { + await act(async () => { + hook = renderHook(() => useChannelSortPreference(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 group key + kind: 30078, + tags: [["d", "channel-sort"]], + 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, whose whole-blob store must replace the state. + await deliver("bbbb"); + await deliver("aaaa"); + + assert.equal( + hook.result.current.sortModeFor("aaaa"), + "recent", + "lower event id (relay canonical winner) must be applied, not rejected", + ); + assert.equal( + hook.result.current.sortModeFor("bbbb"), + "alpha", + "larger id's store must be superseded by the lower-id whole-blob winner", + ); + hook.unmount(); + } finally { + cleanup(); + relayClient.fetchEvents = origFetch; + relayClient.subscribeLive = origLive; + relayClient.subscribeToReconnects = origReconnect; + window.__TAURI_INTERNALS__ = origTauri; + } +}); diff --git a/desktop/src/features/sidebar/lib/useChannelSortPreference.ts b/desktop/src/features/sidebar/lib/useChannelSortPreference.ts index 85c07b1c39..a692bbb31a 100644 --- a/desktop/src/features/sidebar/lib/useChannelSortPreference.ts +++ b/desktop/src/features/sidebar/lib/useChannelSortPreference.ts @@ -86,9 +86,13 @@ export function useChannelSortPreference( 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 + remote.eventId >= lastAppliedEventId.current ) return prev; lastAppliedRemoteTs.current = remote.createdAt; diff --git a/desktop/src/features/sidebar/lib/useChannelStars.test.mjs b/desktop/src/features/sidebar/lib/useChannelStars.test.mjs index a8e1b4b4ae..7ed562e3a0 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.test.mjs +++ b/desktop/src/features/sidebar/lib/useChannelStars.test.mjs @@ -77,3 +77,89 @@ test("same-second star and unstar mutations survive at capacity", async () => { relayClient.subscribeToReconnects = originalSubscribeToReconnects; } }); + +// 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 () => { + 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 = () => () => {}; + // 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; + } +}); diff --git a/desktop/src/features/sidebar/lib/useChannelStars.ts b/desktop/src/features/sidebar/lib/useChannelStars.ts index 855c8de858..d3d4b8f8fb 100644 --- a/desktop/src/features/sidebar/lib/useChannelStars.ts +++ b/desktop/src/features/sidebar/lib/useChannelStars.ts @@ -73,9 +73,13 @@ export function useChannelStars( 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 + remote.eventId >= lastAppliedEventId.current ) return prev; lastAppliedRemoteTs.current = remote.createdAt;