diff --git a/desktop/src/app/AppShellContext.tsx b/desktop/src/app/AppShellContext.tsx index 4a64de0cb2..533ba16fb3 100644 --- a/desktop/src/app/AppShellContext.tsx +++ b/desktop/src/app/AppShellContext.tsx @@ -14,7 +14,7 @@ type AppShellContextValue = { readAt: string | null | undefined, options?: { topLevelOnly?: boolean }, ) => void; - markChannelUnread: (channelId: string) => void; + markChannelUnread: (channelId: string) => boolean; openBrowseChannels: () => void; openCreateChannel: () => void; openChannelManagement: (channelId?: string) => void; @@ -56,7 +56,7 @@ type AppShellContextValue = { const AppShellContext = React.createContext({ markAllChannelsRead: () => {}, markChannelRead: () => {}, - markChannelUnread: () => {}, + markChannelUnread: () => false, openBrowseChannels: () => {}, openCreateChannel: () => {}, openChannelManagement: () => {}, diff --git a/desktop/src/features/channels/nip-rs-unread-ui.test.mjs b/desktop/src/features/channels/nip-rs-unread-ui.test.mjs index e53a34dee5..1f427e076e 100644 --- a/desktop/src/features/channels/nip-rs-unread-ui.test.mjs +++ b/desktop/src/features/channels/nip-rs-unread-ui.test.mjs @@ -29,6 +29,7 @@ import { } from "./readStateOverride.ts"; import { forcedUnreadStore } from "./forcedUnreadStore.ts"; import { resolveChannelReadMarker } from "./useUnreadChannels.ts"; +import { applyMarkUnreadDividerOverlay } from "./ui/useChannelUnreadState.ts"; // ── localStorage mock ──────────────────────────────────────────────────────── @@ -138,83 +139,138 @@ test("applyOverrideUnread_load_incomplete_returns_false", () => { }); // ── Tests: applyOverrideRead ───────────────────────────────────────────────── +// +// New semantics (NIP-RS:537-539): +// 1. !isReadStateReady → fail closed (overrideStillActive) +// 2. isReadStateReady + null liveness → known absence (overrideCleared, no C-bump) +// 3. isReadStateReady + register exists → ALWAYS attempt C-bump, then re-check -test("applyOverrideRead_null_liveness_pre_init_returns_overrideStillActive", () => { - // null ≠ inactive: pre-init must fail closed - const result = applyOverrideRead("ch-1", { - markChannelUnread: () => ({ success: true }), - markChannelRead: () => { - throw new Error("should not be called"); - }, - getOverrideLiveness: () => null, - }); - assert.equal(result, "overrideStillActive"); -}); - -test("applyOverrideRead_inactive_liveness_returns_overrideCleared_without_c_bump", () => { - // Frontier advance made override inactive; no C-bump needed - let bumpCalled = false; - const result = applyOverrideRead("ch-1", { - markChannelUnread: () => ({ success: true }), - markChannelRead: () => { - bumpCalled = true; - return { success: true }; - }, - getOverrideLiveness: () => ({ active: false, frontier: 200 }), - }); - assert.equal(result, "overrideCleared"); - assert.equal(bumpCalled, false, "C-bump not called when already inactive"); -}); - -test("applyOverrideRead_active_liveness_success_then_inactive_returns_overrideCleared", () => { - // Active → C-bump succeeds → re-read shows inactive - let callCount = 0; - const result = applyOverrideRead("ch-1", { +// Helper: builds a minimal OverrideAPIs object with sensible defaults. +function makeApis(overrides = {}) { + return { + isReadStateReady: true, markChannelUnread: () => ({ success: true }), markChannelRead: () => ({ success: true }), - getOverrideLiveness: () => { - callCount++; - // First call (pre-bump check): active. Second call (post-bump): inactive. - return callCount === 1 - ? { active: true, frontier: 50 } - : { active: false, frontier: 50 }; - }, - }); + getOverrideLiveness: () => null, + ...overrides, + }; +} + +test("applyOverrideRead_manager_not_ready_fails_closed_overrideStillActive", () => { + // Step 1: !isReadStateReady → fail closed without calling manager. + let bumped = false; + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: false, + markChannelRead: () => { + bumped = true; + return { success: true }; + }, + getOverrideLiveness: () => ({ active: true, frontier: 50 }), + }), + ); + assert.equal(result, "overrideStillActive"); + assert.equal(bumped, false, "C-bump must not fire when manager not ready"); +}); + +test("applyOverrideRead_ready_null_liveness_no_register_overrideCleared", () => { + // Step 2: ready + null liveness = known no-register → cleared, no C-bump. + let bumped = false; + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => { + bumped = true; + return { success: true }; + }, + getOverrideLiveness: () => null, + }), + ); + assert.equal(result, "overrideCleared"); + assert.equal(bumped, false, "no C-bump when no register exists"); +}); + +test("applyOverrideRead_inactive_register_cbump_attempted_then_clears", () => { + // Step 3 with inactive register: C-bump must be attempted (NIP-RS:537-539). + // After the bump, post-call liveness is still inactive → overrideCleared. + let bumped = false; + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => { + bumped = true; + return { success: true }; + }, + getOverrideLiveness: () => ({ active: false, frontier: 200 }), + }), + ); + assert.equal(result, "overrideCleared"); + assert.equal(bumped, true, "C-bump attempted even when already inactive"); +}); + +test("applyOverrideRead_active_liveness_success_then_inactive_overrideCleared", () => { + // Active register → C-bump succeeds → re-read shows inactive → cleared. + let callCount = 0; + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => ({ success: true }), + getOverrideLiveness: () => { + callCount++; + return callCount === 1 + ? { active: true, frontier: 50 } + : { active: false, frontier: 50 }; + }, + }), + ); assert.equal(result, "overrideCleared"); }); -test("applyOverrideRead_active_liveness_uint32_overflow_returns_overrideStillActive", () => { - // Active → C-bump refuses → re-read still active → overrideStillActive - let _callCount = 0; - const result = applyOverrideRead("ch-1", { - markChannelUnread: () => ({ success: true }), - markChannelRead: () => ({ success: false, reason: "uint32_overflow" }), - getOverrideLiveness: () => { - _callCount++; - return { active: true, frontier: 50 }; - }, - }); +test("applyOverrideRead_active_liveness_load_incomplete_overrideStillActive", () => { + // Active → C-bump refuses (load_incomplete) → liveness still active → retained. + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => ({ success: false, reason: "load_incomplete" }), + getOverrideLiveness: () => ({ active: true, frontier: 50 }), + }), + ); assert.equal(result, "overrideStillActive"); }); -test("applyOverrideRead_active_liveness_load_incomplete_returns_overrideStillActive", () => { - // Active → C-bump refuses (load_incomplete) → overrideStillActive - const result = applyOverrideRead("ch-1", { - markChannelUnread: () => ({ success: true }), - markChannelRead: () => ({ success: false, reason: "load_incomplete" }), - getOverrideLiveness: () => ({ active: true, frontier: 50 }), - }); - assert.equal(result, "overrideStillActive"); +test("applyOverrideRead_already_inactive_race_overrideCleared", () => { + // already_inactive race: cleared by another device → silent success. + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => ({ success: false, reason: "already_inactive" }), + getOverrideLiveness: () => ({ active: true, frontier: 50 }), + }), + ); + assert.equal(result, "overrideCleared"); }); -test("applyOverrideRead_already_inactive_race_returns_overrideCleared", () => { - // already_inactive: cleared by another device between liveness check and - // clear call — treat as silent success. - const result = applyOverrideRead("ch-1", { - markChannelUnread: () => ({ success: true }), - markChannelRead: () => ({ success: false, reason: "already_inactive" }), - getOverrideLiveness: () => ({ active: true, frontier: 50 }), - }); +test("applyOverrideRead_post_call_null_liveness_treated_as_cleared", () => { + // Post-bump liveness is null (register deleted remotely) → overrideCleared. + let callCount = 0; + const result = applyOverrideRead( + "ch-1", + makeApis({ + isReadStateReady: true, + markChannelRead: () => ({ success: true }), + getOverrideLiveness: () => { + callCount++; + // Pre-call: active register. Post-call: register gone (null). + return callCount === 1 ? { active: true, frontier: 50 } : null; + }, + }), + ); assert.equal(result, "overrideCleared"); }); @@ -372,6 +428,37 @@ test("applyOverrideUnread_refusal_caller_must_not_mutate_overlay", () => { ); }); +// ── Tests: applyMarkUnreadDividerOverlay (active-channel handleMarkUnread) ──── +// +// This is the extracted production transition from handleMarkUnread in +// useChannelUnreadState. Tests verify: success adds to overlay, refusal does not. + +test("applyMarkUnreadDividerOverlay_success_adds_to_overlay_returns_true", () => { + const overlay = new Set(); + const result = applyMarkUnreadDividerOverlay( + "ch-active", + () => true, // markFn returns true = accepted + overlay, + ); + assert.equal(result, true, "returns true on acceptance"); + assert.equal(overlay.has("ch-active"), true, "adds to overlay on success"); +}); + +test("applyMarkUnreadDividerOverlay_refusal_does_not_add_to_overlay_returns_false", () => { + const overlay = new Set(); + const result = applyMarkUnreadDividerOverlay( + "ch-refused", + () => false, // markFn returns false = refused + overlay, + ); + assert.equal(result, false, "returns false on refusal"); + assert.equal( + overlay.has("ch-refused"), + false, + "overlay not mutated on refusal", + ); +}); + // ── Tests: markAllChannelsRead partial refusal pattern ──────────────────────── // // Witnesses that per-channel gating in markAllChannelsRead correctly clears @@ -387,6 +474,7 @@ test("applyOverrideRead_partial_refusal_pattern_clears_only_inactive", () => { }; let clearCallCount = 0; const apis = { + isReadStateReady: true, markChannelUnread: () => ({ success: true }), markChannelRead: (id) => { clearCallCount++; diff --git a/desktop/src/features/channels/readStateOverride.ts b/desktop/src/features/channels/readStateOverride.ts index 9052334c41..99f51078be 100644 --- a/desktop/src/features/channels/readStateOverride.ts +++ b/desktop/src/features/channels/readStateOverride.ts @@ -42,6 +42,7 @@ export function overrideErrorMessage( /** Slice-2 override APIs from ReadStateManager, proxied through useReadState. */ export type OverrideAPIs = { + isReadStateReady: boolean; markChannelUnread: (channelId: string) => MarkResult; markChannelRead: (channelId: string) => MarkResult; getOverrideLiveness: (channelId: string) => OverrideLiveness | null; @@ -68,13 +69,13 @@ export function applyOverrideUnread( /** * Outcome of the explicit mark-read transition for one channel. * - * `overrideCleared` — the NIP-RS override is now known inactive (either it was - * already inactive, or the C-bump succeeded and the resulting liveness is - * inactive). Callers should clear local presentation. + * `overrideCleared` — the NIP-RS override is now known inactive (no register + * exists, the frontier deactivated it, or the C-bump succeeded). Callers + * should clear local presentation. * - * `overrideStillActive` — the C-bump was refused (or returned a result where - * liveness remains active / unknown). Callers must NOT clear local unread - * presentation; the spec MUST at NIP-RS:537-539 requires success only when + * `overrideStillActive` — the manager is unavailable (fail-closed) or the + * C-bump was refused and liveness remains active. Callers MUST NOT clear + * local unread presentation; NIP-RS:537-539 requires success only when * resulting override_active == false. */ export type OverrideReadOutcome = "overrideCleared" | "overrideStillActive"; @@ -83,47 +84,48 @@ export type OverrideReadOutcome = "overrideCleared" | "overrideStillActive"; * Attempt to clear the NIP-RS override for `channelId` as part of an explicit * mark-read transition. Models the transition as a single outcome: * - * 1. If liveness is null (pre-init): fail-closed — return overrideStillActive - * without calling the manager (null ≠ inactive). - * 2. If liveness is inactive: return overrideCleared immediately (no C-bump - * needed; the frontier advance already made override_active == false). - * 3. If liveness is active: call markChannelRead, then re-read liveness. - * - If resulting liveness is inactive (or already_inactive race): cleared. - * - If the manager refused for any reason that keeps liveness active: - * show a toast and return overrideStillActive. - * - * Special edge (permitted by spec): if the caller's frontier advance made - * F > B, liveness is already inactive before the C-bump — this is detected at - * step 2, so no error toast fires in that case. + * 1. Manager unavailable (!isReadStateReady): fail-closed — + * return overrideStillActive (manager unavailable ≠ no register). + * 2. liveness === null (ready but no register): return overrideCleared + * (known absence; no C-bump needed). + * 3. liveness exists (active or inactive): always call markChannelRead — + * spec NIP-RS:537-539 requires the C-bump even when the frontier already + * deactivated the override. Then re-read liveness. + * a. Resulting liveness inactive (or already_inactive race): cleared. + * b. uint32 refusal AND F > B (frontier already deactivated): cleared, + * no error toast (representable C-bump was not possible, frontier + * already provides the inactive verdict). + * c. Other refusal keeping liveness active: show toast, overrideStillActive. */ export function applyOverrideRead( channelId: string, apis: OverrideAPIs, ): OverrideReadOutcome { + // Step 1: manager unavailable → fail closed. + if (!apis.isReadStateReady) return "overrideStillActive"; + const liveness = apis.getOverrideLiveness(channelId); - // Pre-init: null ≠ inactive. Fail closed. - if (liveness === null) return "overrideStillActive"; + // Step 2: ready but no register at all → known absence, cleared. + if (liveness === null) return "overrideCleared"; - // Already inactive (frontier advance made F > B, or never active). - if (!liveness.active) return "overrideCleared"; - - // Active override: attempt the C-bump. + // Step 3: register exists — always attempt the C-bump (NIP-RS:537-539). const result = apis.markChannelRead(channelId); if (!result.success && result.reason === "already_inactive") { - // Race condition: cleared by another device between our liveness check and - // the clear call. Silent success. + // Race: cleared by another device between our check and the call. return "overrideCleared"; } // Re-read liveness after the attempt. const afterLiveness = apis.getOverrideLiveness(channelId); - if (afterLiveness !== null && !afterLiveness.active) { + // Covers: successful C-bump (inactive after), F>B deactivation (inactive after), + // and uint32 refusal where F>B already deactivated the register. + if (afterLiveness === null || !afterLiveness.active) { return "overrideCleared"; } - // Refused and still active (or unknown post-call liveness). + // Refused and still active. if (!result.success) { const msg = overrideErrorMessage("read", result.reason); if (msg) toast.error(msg); diff --git a/desktop/src/features/channels/ui/useChannelUnreadState.ts b/desktop/src/features/channels/ui/useChannelUnreadState.ts index 298a52f54d..843f342d36 100644 --- a/desktop/src/features/channels/ui/useChannelUnreadState.ts +++ b/desktop/src/features/channels/ui/useChannelUnreadState.ts @@ -26,6 +26,26 @@ import { isConversationalUnreadKind } from "@/shared/constants/kinds"; import { useWelcomeInitialUnreadSuppression } from "./useWelcomeInitialUnreadSuppression"; +/** + * Pure helper: apply the mark-unread divider overlay. + * + * Calls `markFn(channelId)` and, if it returns true (accepted), adds the + * channel to the `overlay` set. Returns the same boolean so callers can gate + * any additional side-effects on success. + * + * Extracted for testability: `handleMarkUnread` calls this instead of + * inlining the same two-step logic. + */ +export function applyMarkUnreadDividerOverlay( + channelId: string, + markFn: (id: string) => boolean, + overlay: Set, +): boolean { + const accepted = markFn(channelId); + if (accepted) overlay.add(channelId); + return accepted; +} + type UseChannelUnreadStateOptions = { activeChannelId: string | null; timelineMessages: TimelineMessage[]; @@ -36,7 +56,7 @@ type UseChannelUnreadStateOptions = { openThreadMessages?: MainTimelineEntry[]; getChannelReadAt: (channelId: string) => number | null; getMessageReadAt: (messageId: string) => number | null; - markChannelUnread: (channelId: string) => void; + markChannelUnread: (channelId: string) => boolean; markMessageRead: (messageId: string, timestamp: number) => void; isThreadMuted: (rootId: string) => boolean; readStateVersion: number; @@ -404,19 +424,11 @@ export function useChannelUnreadState({ const handleMarkUnread = React.useCallback(() => { if (!activeChannelId) return; - // Call the manager first — the sidebar dot is driven by liveness, not the - // session overlay. Only update the session overlay after the manager call - // so a refused mark-unread does not spuriously suppress the timeline marker. - markChannelUnread(activeChannelId); - // The `forcedUnreadRef` overlay here gates the timeline "New" divider - // suppression (see computeChannelUnreadMarker above). We must not set it - // before the manager call or on refusal. Since `markChannelUnread` has a - // void return type at this boundary, we rely on readStateVersion bumping - // on success (manager subscribe fires → outer hook re-renders → liveness - // is now active). The overlay is intentionally NOT set here — the divider - // suppression is driven by liveness via isActiveChannelForcedUnread only - // when the sidebar dot confirms active override state. Setting the overlay - // unconditionally would suppress the divider even on a refused call. + applyMarkUnreadDividerOverlay( + activeChannelId, + markChannelUnread, + forcedUnreadRef.current, + ); forceUnreadRender(); }, [activeChannelId, markChannelUnread]); diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index a47898252e..add6e2a2fb 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -149,15 +149,12 @@ export function useUnreadChannels( } = options; const activeChannelId = activeChannel?.id ?? null; const normalizedPubkey = pubkey?.toLowerCase() ?? null; - // Scoped relay key for activity storage; empty string when relay not yet known - // so rows from an unknown relay never load into the wrong community. + // Scoped relay key; empty string when relay not yet known. const normalizedRelayUrl = relayUrlOption ? normalizeRelayUrl(relayUrlOption) : ""; - // Single identity for the in-memory thread-activity buffer — computed once - // per render and used at reset, both writers, and the return fence. The - // helper returns "" when either value is absent, which never matches a valid - // loaded scope, so the fence returns [] until the buffer is seeded. + // Activity scope key: "" when pubkey/relay absent (never matches a valid + // loaded scope, so the fence returns [] until the buffer is seeded). const currentActivityScope = activityScopeKey( normalizedPubkey, normalizedRelayUrl, @@ -176,14 +173,19 @@ export function useUnreadChannels( markChannelRead: markChannelOverrideRead, getOverrideLiveness, } = useReadState(pubkey, relayClient); - // Stable object wrapping the three override APIs. const overrideApis = React.useMemo( () => ({ + isReadStateReady, markChannelUnread: markChannelOverrideUnread, markChannelRead: markChannelOverrideRead, getOverrideLiveness, }), - [getOverrideLiveness, markChannelOverrideRead, markChannelOverrideUnread], + [ + getOverrideLiveness, + isReadStateReady, + markChannelOverrideRead, + markChannelOverrideUnread, + ], ); // Observed "latest external trigger event" per channel — derived relay evidence @@ -335,13 +337,11 @@ export function useUnreadChannels( [markContextRead, overrideApis, pubkey], ); - // Manually mark a channel unread (right-click → "mark unread"). Publishes a - // NIP-RS override event to the relay (S bump + B set), then updates the - // local cache. Shows a toast on failure (budget exhaustion, counter overflow, - // or incomplete load). + // Manually mark a channel unread (right-click). Returns true when the manager + // accepted (use to gate session-local UI overlays on success only). const markChannelUnread = React.useCallback( - (channelId: string) => { - if (!applyOverrideUnread(channelId, overrideApis)) return; + (channelId: string): boolean => { + if (!applyOverrideUnread(channelId, overrideApis)) return false; if ( persistForcedUnread( channelId, @@ -351,6 +351,7 @@ export function useUnreadChannels( ) ) bumpLatestVersion(); + return true; }, [getOwnTimestamp, overrideApis, pubkey], ); diff --git a/desktop/src/features/communities/communityUnreadObserver.test.mjs b/desktop/src/features/communities/communityUnreadObserver.test.mjs index 6386d06102..1b3a71adbf 100644 --- a/desktop/src/features/communities/communityUnreadObserver.test.mjs +++ b/desktop/src/features/communities/communityUnreadObserver.test.mjs @@ -641,7 +641,7 @@ test("fetchCommunityUnread threaded reply whose root is in mutedRootIds → hasU assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); -// ── Forced-unread persistence gate tests ───────────────────────────────── +// ── Override register authoritative source tests ────────────────────────── // A relay that serves one channel (CHANNEL_ID) as a member channel, // with no read state and no incoming events. @@ -727,8 +727,12 @@ function quietRelayWithReadState(readAtSeconds) { ]); } -test("fetchCommunityUnread forced-unread channel lights rail dot (hasUnread:true)", async () => { +test("fetchCommunityUnread stored active register lights rail (hasUnread:true)", async () => { + // Authoritative source (a): stored register is active (S=1,C=0,B=0 with F=0). + // The fetch returns no override events — stored register is the authority. const relay = quietRelay(); + // Active register: S=1, C=0, B=0 (baseline), frontier=0 → override_active = true + const storedRegisters = new Map([[CHANNEL_ID, { s: 1, c: 0, b: 0 }]]); const result = await fetchCommunityUnread({ client: relay, @@ -737,20 +741,18 @@ test("fetchCommunityUnread forced-unread channel lights rail dot (hasUnread:true decryptReadState: async (v) => v, decryptMutes: async (v) => v, readThreadRelationships: readRelationships(), - // Forced-unread map: CHANNEL_ID forced when marker was null (no prior read) - readForcedUnread: () => ({ [CHANNEL_ID]: null }), + readStoredRegisters: () => storedRegisters, }); assert.deepEqual(result, { hasUnread: true, mentionCount: 0 }); }); -test("fetchCommunityUnread forced-unread channel not in member list → hasUnread:false", async () => { - // The channel is marked forced-unread but the user is not a member of ANY - // channel — forced-unread is not consulted when the channel set is empty. - const relay = relayFor([ - // 1. member events — empty - () => [], - ]); +test("fetchCommunityUnread stored tombstoned register does NOT light rail (hasUnread:false)", async () => { + // Authoritative source (a): stored register is tombstoned (S=1,C=1 → S=C, inactive). + // Local cache cannot contradict this — must not light the dot. + const relay = quietRelay(); + // Tombstoned: S=1, C=1, B=0 → override_active = false (S > C fails: 1 > 1 = false) + const storedRegisters = new Map([[CHANNEL_ID, { s: 1, c: 1, b: 0 }]]); const result = await fetchCommunityUnread({ client: relay, @@ -759,13 +761,38 @@ test("fetchCommunityUnread forced-unread channel not in member list → hasUnrea decryptReadState: async (v) => v, decryptMutes: async (v) => v, readThreadRelationships: readRelationships(), - readForcedUnread: () => ({ [CHANNEL_ID]: null }), + readStoredRegisters: () => storedRegisters, }); assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); -test("fetchCommunityUnread forced-unread channel that is also muted → hasUnread:false", async () => { +test("fetchCommunityUnread old active register beyond horizon still lights rail", async () => { + // An active register that is older than the 7-day horizon must still light + // the rail. The fetch is tag-free and has no `since`, so it returns no + // events here, but the stored projection (authoritative source a) is used. + const relay = quietRelay(); + const VERY_OLD_FRONTIER = 1; // far beyond any 7-day horizon + // S=1, C=0, B=VERY_OLD_FRONTIER, and frontier=VERY_OLD_FRONTIER → active + const storedRegisters = new Map([ + [CHANNEL_ID, { s: 1, c: 0, b: VERY_OLD_FRONTIER }], + ]); + + const result = await fetchCommunityUnread({ + client: relay, + pubkey: PUBKEY, + nowSeconds: 100 + 7 * 24 * 3600 + 1, // past the old 7-day cutoff + decryptReadState: async (v) => v, + decryptMutes: async (v) => v, + readThreadRelationships: readRelationships(), + readStoredRegisters: () => storedRegisters, + }); + + assert.deepEqual(result, { hasUnread: true, mentionCount: 0 }); +}); + +test("fetchCommunityUnread no stored register and empty fetch → falls through to relay gate", async () => { + // No stored registers, no override event fetched, but a real unread event exists. const relay = relayFor([ // 1. member events () => [ @@ -787,16 +814,20 @@ test("fetchCommunityUnread forced-unread channel that is also muted → hasUnrea ], // 3. visibility () => [], - // 4. read-state (parallel with mutes) + // 4. read-state — empty () => [], - // 5. mutes — CHANNEL_ID is muted + // 5. mutes + () => [], + // 6. unread events — one real unread () => [ event({ - pubkey: PUBKEY, - content: mutesContent([CHANNEL_ID]), + id: "real-unread".padEnd(64, "0"), + created_at: 20, + tags: [["h", CHANNEL_ID]], }), ], - // No per-channel fetches expected — muted channel is skipped + // 7. mention events + () => [], ]); const result = await fetchCommunityUnread({ @@ -806,15 +837,33 @@ test("fetchCommunityUnread forced-unread channel that is also muted → hasUnrea decryptReadState: async (v) => v, decryptMutes: async (v) => v, readThreadRelationships: readRelationships(), - // CHANNEL_ID is both forced-unread AND muted — mute wins - readForcedUnread: () => ({ [CHANNEL_ID]: null }), + readStoredRegisters: () => new Map(), // no stored registers + }); + + assert.deepEqual(result, { hasUnread: true, mentionCount: 0 }); +}); + +test("fetchCommunityUnread channel not in member list → hasUnread:false", async () => { + // No members — register lookup is never reached. + const relay = relayFor([ + () => [], // member events empty + ]); + + const result = await fetchCommunityUnread({ + client: relay, + pubkey: PUBKEY, + nowSeconds: 100, + decryptReadState: async (v) => v, + decryptMutes: async (v) => v, + readThreadRelationships: readRelationships(), + readStoredRegisters: () => new Map([[CHANNEL_ID, { s: 1, c: 0, b: 0 }]]), }); assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); -test("fetchCommunityUnread readForcedUnread returns empty map → falls through to relay gate", async () => { - // No forced-unread, but there IS a real unread event → hasUnread:true via relay +test("fetchCommunityUnread active register muted channel → hasUnread:false", async () => { + // Override is active, but the channel is muted — mute wins. const relay = relayFor([ // 1. member events () => [ @@ -838,18 +887,14 @@ test("fetchCommunityUnread readForcedUnread returns empty map → falls through () => [], // 4. read-state () => [], - // 5. mutes - () => [], - // 6. unread events + // 5. mutes — CHANNEL_ID is muted () => [ event({ - id: "real-unread".padEnd(64, "0"), - created_at: 20, - tags: [["h", CHANNEL_ID]], + pubkey: PUBKEY, + content: mutesContent([CHANNEL_ID]), }), ], - // 7. mention events - () => [], + // No per-channel fetches expected — muted channel is skipped ]); const result = await fetchCommunityUnread({ @@ -859,33 +904,19 @@ test("fetchCommunityUnread readForcedUnread returns empty map → falls through decryptReadState: async (v) => v, decryptMutes: async (v) => v, readThreadRelationships: readRelationships(), - readForcedUnread: () => ({}), // empty — no forced-unread - }); - - assert.deepEqual(result, { hasUnread: true, mentionCount: 0 }); -}); - -test("fetchCommunityUnread forced-unread + synced marker advanced PAST baseline → hasUnread:false", async () => { - // markerAtWhenForced = 50, observed readAt = 100 (100 > 50 → cross-device read wins) - const relay = quietRelayWithReadState(100); - - const result = await fetchCommunityUnread({ - client: relay, - pubkey: PUBKEY, - nowSeconds: 200, - decryptReadState: async (v) => v, - decryptMutes: async (v) => v, - readThreadRelationships: readRelationships(), - // Forced at marker=50; synced marker is now 100 — covers the force - readForcedUnread: () => ({ [CHANNEL_ID]: 50 }), + // Active register, but channel is muted — mute wins + readStoredRegisters: () => new Map([[CHANNEL_ID, { s: 1, c: 0, b: 0 }]]), }); assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); -test("fetchCommunityUnread forced-unread + synced marker NOT advanced past baseline → hasUnread:true", async () => { - // markerAtWhenForced = 100, observed readAt = 100 (equal → force still stands) +test("fetchCommunityUnread frontier advance deactivates stored register → hasUnread:false", async () => { + // Register: S=1, C=0, B=50 (active only when F<=50). + // Fetched frontier: 100 > 50 → override_active = false. const relay = quietRelayWithReadState(100); + // B=50 means active only when F<=50; fetched frontier is 100 → inactive + const storedRegisters = new Map([[CHANNEL_ID, { s: 1, c: 0, b: 50 }]]); const result = await fetchCommunityUnread({ client: relay, @@ -894,27 +925,7 @@ test("fetchCommunityUnread forced-unread + synced marker NOT advanced past basel decryptReadState: async (v) => v, decryptMutes: async (v) => v, readThreadRelationships: readRelationships(), - // Forced at marker=100; synced marker is still 100 — no newer read - readForcedUnread: () => ({ [CHANNEL_ID]: 100 }), - }); - - assert.deepEqual(result, { hasUnread: true, mentionCount: 0 }); -}); - -test("fetchCommunityUnread forced-unread with null baseline + synced marker present → hasUnread:false", async () => { - // markerAtWhenForced = null (no marker at force-time), but readAt = 50 now - // A cross-device read appeared after the force → do NOT light the dot - const relay = quietRelayWithReadState(50); - - const result = await fetchCommunityUnread({ - client: relay, - pubkey: PUBKEY, - nowSeconds: 200, - decryptReadState: async (v) => v, - decryptMutes: async (v) => v, - readThreadRelationships: readRelationships(), - // Forced when no marker existed; a cross-device read has since appeared - readForcedUnread: () => ({ [CHANNEL_ID]: null }), + readStoredRegisters: () => storedRegisters, }); assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); diff --git a/desktop/src/features/communities/communityUnreadObserver.ts b/desktop/src/features/communities/communityUnreadObserver.ts index 698a0355b4..95161da63d 100644 --- a/desktop/src/features/communities/communityUnreadObserver.ts +++ b/desktop/src/features/communities/communityUnreadObserver.ts @@ -1,15 +1,14 @@ import { makeRootIdStore } from "@/features/channels/unreadRootIdStore"; -import { - forcedUnreadStore, - type ForcedUnreadMap, -} from "@/features/channels/forcedUnreadStore"; import { DM_NOTIFIABLE_EVENT_KINDS } from "@/features/channels/isDmNotifiableKind"; import { mergeReadStateEventsStructured } from "@/features/channels/readState/readStateSnapshot"; import { isOverrideActive, maxReadAt, msgContextKey, + type OverrideRegister, } from "@/features/channels/readState/readStateFormat"; +import { readStoredReadState } from "@/features/channels/readState/readStateStorage"; +import { deduplicateByCoordinate } from "@/features/channels/readState/readStateFencedLoader"; import { getThreadReference, isBroadcastReply, @@ -87,7 +86,6 @@ const METADATA_LIMIT = 1000; const UNREAD_EXISTENCE_LIMIT = 50; const MENTION_COUNT_LIMIT = 100; const READ_STATE_FETCH_LIMIT = 500; -const READ_STATE_HORIZON_SECONDS = 7 * 24 * 60 * 60; export type CommunityUnreadObserverResult = { hasUnread: boolean; @@ -160,16 +158,26 @@ export async function fetchCommunityUnread(args: { decryptReadState?: (ciphertext: string) => Promise; decryptMutes?: (ciphertext: string) => Promise; readThreadRelationships?: (pubkey: string) => ThreadRelationships; - readForcedUnread?: (pubkey: string) => ForcedUnreadMap; + /** Authoritative source (a): persisted override register projection. Defaults + * to `readStoredReadState(pubkey).overrideRegisters`. Inject for tests. */ + readStoredRegisters?: ( + pubkey: string, + ) => ReadonlyMap; }): Promise { const { client, pubkey } = args; const normalizedPubkey = pubkey.toLowerCase(); - const nowSeconds = args.nowSeconds ?? Math.floor(Date.now() / 1_000); const decryptMutes = args.decryptMutes ?? nip44DecryptFromSelf; const readRelationships = args.readThreadRelationships ?? defaultReadThreadRelationships; - const readForcedUnread = - args.readForcedUnread ?? ((pk) => forcedUnreadStore.read(pk)); + const getStoredRegisters = + args.readStoredRegisters ?? + ((pk: string) => { + try { + return readStoredReadState(pk).overrideRegisters; + } catch { + return new Map(); + } + }); const channels = await fetchObservedChannels(client, pubkey); if (channels.length === 0) { @@ -177,11 +185,11 @@ export async function fetchCommunityUnread(args: { } const [readStateEvents, mutesEvents] = await Promise.all([ + // Override state is exempt from finite-horizon fetching: a register may be + // older than seven days and must not be missed. Tag-free, no `since` filter. client.fetchEvents({ kinds: [KIND_READ_STATE], authors: [pubkey], - "#t": ["read-state"], - since: nowSeconds - READ_STATE_HORIZON_SECONDS, limit: READ_STATE_FETCH_LIMIT, }), client.fetchEvents({ @@ -192,12 +200,34 @@ export async function fetchCommunityUnread(args: { }), ]); + // Authoritative source (b): coordinate-deduped registers decoded from fetched + // events. deduplicateByCoordinate retains greatest created_at, lowest id. const readState = await mergeReadStateEventsStructured( - readStateEvents, + deduplicateByCoordinate(readStateEvents), pubkey, args.decryptReadState, ); + // Authoritative source (a): persisted full-state projection from the manager's + // last complete load. Merge with source (b): max() per field, so a register + // present in only one source is retained. + const storedRegisters = getStoredRegisters(normalizedPubkey); + const mergedOverrides = new Map( + readState.overrides, + ); + for (const [rawCtx, stored] of storedRegisters) { + const fetched = mergedOverrides.get(rawCtx); + if (!fetched) { + mergedOverrides.set(rawCtx, stored); + } else { + mergedOverrides.set(rawCtx, { + s: Math.max(fetched.s, stored.s), + c: Math.max(fetched.c, stored.c), + b: Math.max(fetched.b, stored.b), + }); + } + } + let mutedIds = new Set(); if (mutesEvents.length > 0) { try { @@ -218,11 +248,6 @@ export async function fetchCommunityUnread(args: { mutedRootIds, } = readRelationships(normalizedPubkey); - // Channels manually marked unread on this device — local cache of the NIP-RS - // override layer. Used only as a fast path; the authoritative verdict is the - // structured override register evaluated against the merged frontier. - const forcedUnreadMap = readForcedUnread(normalizedPubkey); - let hasUnread = false; let mentionCount = 0; @@ -232,26 +257,13 @@ export async function fetchCommunityUnread(args: { // Compute readAt from the merged frontiers. const readAt = readState.frontiers.get(channel.id) ?? null; - // Forced-unread: evaluate the structured override register using the same - // override_active(S, C, B, F) predicate as the active-community path. The - // local cache entry is a hint only — the register is the authoritative source. + // Override liveness: evaluate the merged authoritative register. + // "No register in fetched events" is ambiguity, not absence — the local + // cache is NOT a fallback verdict source here. if (!hasUnread) { - const reg = readState.overrides.get(channel.id); + const reg = mergedOverrides.get(channel.id); if (reg !== undefined && isOverrideActive(reg, readAt ?? 0)) { hasUnread = true; - } else if ( - reg === undefined && - Object.hasOwn(forcedUnreadMap, channel.id) - ) { - // No register in fetched events: fall back to the local cache as a best- - // effort hint (override event may not have reached the relay yet). - const markerAtWhenForced = forcedUnreadMap[channel.id]; - if ( - readAt === null || - (markerAtWhenForced !== null && readAt <= markerAtWhenForced) - ) { - hasUnread = true; - } } }