diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index 4eb0a42bbe..77a311629e 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -342,6 +342,7 @@ export function AppShell() { mutedRootIds, muteThread, unmuteThread, + getProjection, } = useUnreadChannels(sidebarChannels, activeChannel, { pubkey: identityQuery.data?.pubkey, relayClient, @@ -404,8 +405,6 @@ export function AppShell() { mutedRootIds, channels, ); - - // Badge count consumes the shared NIP-RS read-state from useUnreadChannels. const { homeBadgeCount, homeBadgeCountExcludingHighPriority } = useHomeFeedNotificationState( homeFeedQuery.data, @@ -733,6 +732,7 @@ export function AppShell() { threadActivityFeedItems, feedItemState, onOpenSettings: handleOpenSettings, + getProjection, }} > diff --git a/desktop/src/app/AppShellContext.tsx b/desktop/src/app/AppShellContext.tsx index 533ba16fb3..dde4d8c947 100644 --- a/desktop/src/app/AppShellContext.tsx +++ b/desktop/src/app/AppShellContext.tsx @@ -1,5 +1,6 @@ import * as React from "react"; import type { ContextParentResolver } from "@/features/channels/readState/readStateManager"; +import type { ReadStateProjection } from "@/features/channels/readState/readStateManager"; import type { ThreadActivityItem } from "@/features/channels/useUnreadChannels"; import type { FeedItemState } from "@/features/home/useFeedItemState"; import type { FeedItem } from "@/shared/api/types"; @@ -51,6 +52,9 @@ type AppShellContextValue = { // that render under AppShell (channel, home, projects, pulse, agents). // Used by config-nudge cards to deep-link to Settings → Agents. onOpenSettings: ((section: SettingsSection) => void) | null; + // Manager projection accessor for community rail polls — re-read per poll, + // never cached across notification boundaries. + getProjection: () => ReadStateProjection | null; }; const AppShellContext = React.createContext({ @@ -83,6 +87,7 @@ const AppShellContext = React.createContext({ unreadSet: EMPTY_SET, }, onOpenSettings: null, + getProjection: () => null, }); export function AppShellProvider({ 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 3262e29c32..7162b6d895 100644 --- a/desktop/src/features/channels/nip-rs-unread-ui.test.mjs +++ b/desktop/src/features/channels/nip-rs-unread-ui.test.mjs @@ -22,6 +22,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + applyMarkAllReadTransition, applyOverrideRead, applyOverrideUnread, overrideErrorMessage, @@ -544,15 +545,15 @@ test("adversarial-3: incomplete_load_null_liveness_not_known_absent_overrideStil ); }); -test("adversarial-4: markAllChannelsRead_confirmed_active_refusal_keeps_evidence", () => { - // IMPORTANT: markAllChannelsRead must NOT clear latestByChannelRef or - // observedUnreadEventsByChannelRef when the override read is refused and - // liveness remains active. Evidence must be preserved so the UI can still - // represent the failed channel's unread state. +test("adversarial-4: applyMarkAllReadTransition_both_branches", () => { + // IMPORTANT: applyMarkAllReadTransition is the extracted production transition + // used inside markAllChannelsRead. Testing it directly (not a simulation) + // ensures production regressions to unconditional deletion are caught. // - // This test uses applyOverrideRead directly to simulate the per-channel loop - // in markAllChannelsRead and verifies that refused channels keep their evidence - // while cleared channels discard theirs. + // ch-cleared: applyOverrideRead succeeds (liveness goes inactive) → + // forcedUnread, latestByChannel, observedUnreadEvents all removed. + // ch-refused: applyOverrideRead refused (liveness stays active) → + // all three maps preserved intact. const livenessMap = { "ch-cleared": { active: true, frontier: 100 }, "ch-refused": { active: true, frontier: 100 }, // uint32_overflow refusal @@ -566,13 +567,11 @@ test("adversarial-4: markAllChannelsRead_confirmed_active_refusal_keeps_evidence livenessMap["ch-cleared"] = { active: false, frontier: 101 }; return { success: true }; } - // ch-refused: refuse; liveness stays active return { success: false, reason: "uint32_overflow" }; }, getOverrideLiveness: (id) => livenessMap[id] ?? null, }; - // latestByChannel and observedEvents are the evidence maps. const latestByChannel = new Map([ ["ch-cleared", 200], ["ch-refused", 200], @@ -582,49 +581,54 @@ test("adversarial-4: markAllChannelsRead_confirmed_active_refusal_keeps_evidence ["ch-refused", new Map([["ev2", { kind: 9, createdAt: 200 }]])], ]); const forcedMap = { "ch-cleared": 99, "ch-refused": 99 }; + const maps = { + forcedUnread: forcedMap, + latestByChannel, + observedUnreadEvents: observedEvents, + }; - // Simulate the production per-channel loop in markAllChannelsRead. - for (const channelId of ["ch-cleared", "ch-refused"]) { - const outcome = applyOverrideRead(channelId, apis); - if (outcome === "overrideCleared") { - delete forcedMap[channelId]; - latestByChannel.delete(channelId); - observedEvents.delete(channelId); - } - // On overrideStillActive: do NOT touch latestByChannel or observedEvents. - } + // Drive the production transition directly (not a reimplementation). + const clearedOutcome = applyMarkAllReadTransition("ch-cleared", apis, maps); + const refusedOutcome = applyMarkAllReadTransition("ch-refused", apis, maps); - // ch-cleared: override confirmed inactive → evidence discarded. + assert.equal(clearedOutcome, "overrideCleared", "cleared channel: outcome"); + assert.equal( + refusedOutcome, + "overrideStillActive", + "refused channel: outcome", + ); + + // ch-cleared: override confirmed inactive → all three maps purged. assert.equal( latestByChannel.has("ch-cleared"), false, - "cleared channel: latestByChannel removed", + "cleared: latestByChannel removed", ); assert.equal( observedEvents.has("ch-cleared"), false, - "cleared channel: observedEvents removed", + "cleared: observedEvents removed", ); assert.equal( Object.hasOwn(forcedMap, "ch-cleared"), false, - "cleared channel: forcedMap entry removed", + "cleared: forcedMap removed", ); - // ch-refused: override still active → evidence preserved. + // ch-refused: override still active → all three maps preserved. assert.equal( latestByChannel.has("ch-refused"), true, - "refused channel: latestByChannel preserved", + "refused: latestByChannel preserved", ); assert.equal( observedEvents.has("ch-refused"), true, - "refused channel: observedEvents preserved", + "refused: observedEvents preserved", ); assert.equal( Object.hasOwn(forcedMap, "ch-refused"), true, - "refused channel: forcedMap entry preserved", + "refused: forcedMap preserved", ); }); diff --git a/desktop/src/features/channels/readStateOverride.ts b/desktop/src/features/channels/readStateOverride.ts index 1bec1708c4..f93ea4744a 100644 --- a/desktop/src/features/channels/readStateOverride.ts +++ b/desktop/src/features/channels/readStateOverride.ts @@ -13,6 +13,7 @@ import { forcedUnreadStore, type ForcedUnreadMap, } from "@/features/channels/forcedUnreadStore"; +import type { ObservedUnreadEvent } from "@/features/channels/unreadChannelCounts"; export type { MarkResult, OverrideLiveness }; @@ -158,3 +159,36 @@ export function persistForcedUnread( if (pubkey) forcedUnreadStore.write(pubkey, forcedMap); return true; } + +/** + * Per-channel evidence maps that `markAllChannelsRead` mutates. + * Extracted so the per-channel transition can be tested without the full hook. + */ +export type MarkAllEvidenceMaps = { + forcedUnread: ForcedUnreadMap; + latestByChannel: Map; + observedUnreadEvents: Map>; +}; + +/** + * Apply the per-channel mark-all-read transition for a single channel ID. + * + * Calls `applyOverrideRead` to attempt the NIP-RS C-bump. On `overrideCleared`, + * removes the channel's entry from all three evidence maps. On + * `overrideStillActive` (refused or load incomplete), all maps are left + * intact — the caller must NOT wipe evidence for channels whose clear was + * refused. Returns the outcome for caller use. + */ +export function applyMarkAllReadTransition( + channelId: string, + apis: OverrideAPIs, + maps: MarkAllEvidenceMaps, +): OverrideReadOutcome { + const outcome = applyOverrideRead(channelId, apis); + if (outcome === "overrideCleared") { + delete maps.forcedUnread[channelId]; + maps.latestByChannel.delete(channelId); + maps.observedUnreadEvents.delete(channelId); + } + return outcome; +} diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index ee1c91aee7..c2bf1b426d 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -22,6 +22,7 @@ import { type ForcedUnreadMap, } from "@/features/channels/forcedUnreadStore"; import { + applyMarkAllReadTransition, applyOverrideRead, applyOverrideUnread, persistForcedUnread, @@ -172,6 +173,7 @@ export function useUnreadChannels( markChannelUnread: markChannelOverrideUnread, markChannelRead: markChannelOverrideRead, getOverrideLiveness, + getProjection, } = useReadState(pubkey, relayClient); const overrideApis = React.useMemo( () => ({ @@ -952,7 +954,6 @@ export function useUnreadChannels( const markAllChannelsRead = React.useCallback(() => { for (const channelId of unreadChannelIdsRef.current) { - // Frontier advance first, then C-bump; gate local clear on confirmed liveness. const unixSeconds = latestByChannelRef.current.get(channelId) ?? getEffectiveTimestamp(channelId) ?? @@ -960,11 +961,11 @@ export function useUnreadChannels( if (unixSeconds !== null) { markContextRead(channelId, unixSeconds); } - if (applyOverrideRead(channelId, overrideApis) === "overrideCleared") { - delete forcedUnreadRef.current[channelId]; - latestByChannelRef.current.delete(channelId); - observedUnreadEventsByChannelRef.current.delete(channelId); - } + applyMarkAllReadTransition(channelId, overrideApis, { + forcedUnread: forcedUnreadRef.current, + latestByChannel: latestByChannelRef.current, + observedUnreadEvents: observedUnreadEventsByChannelRef.current, + }); } if (pubkey) { forcedUnreadStore.write(pubkey, forcedUnreadRef.current); @@ -998,10 +999,8 @@ export function useUnreadChannels( markAllChannelsRead, markChannelRead, markChannelUnread, - // Exposed so other surfaces (e.g. Home) can project per-item read state - // off the same NIP-RS read marker without instantiating a second - // ReadStateManager. readStateVersion is the invalidation signal callers - // should include in memo deps. + // Exposed for surfaces projecting per-item read state (Home) off the shared + // NIP-RS marker without a second ReadStateManager; readStateVersion = invalidation signal. getEffectiveTimestamp, getOwnTimestamp, readStateVersion, @@ -1017,5 +1016,6 @@ export function useUnreadChannels( mutedRootIds: mutedRootIdsRef.current as ReadonlySet, muteThread, unmuteThread, + getProjection, }; } diff --git a/desktop/src/features/communities/communityUnreadObserver.test.mjs b/desktop/src/features/communities/communityUnreadObserver.test.mjs index 69534d67c7..29481aa7f6 100644 --- a/desktop/src/features/communities/communityUnreadObserver.test.mjs +++ b/desktop/src/features/communities/communityUnreadObserver.test.mjs @@ -1060,3 +1060,86 @@ test("fetchCommunityUnread adversarial-2: projection with frontier deactivates r assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); + +test("fetchCommunityUnread adversarial-5: stale_complete_projection_yieldsTo_fresher_fetched_tombstone", async () => { + // CRITICAL 2 ordering fix: `loadComplete` establishes enumeration completeness, + // NOT temporal ordering. A complete projection with a live register + // (S=5,C=0,B=100,F=50) must yield to a fresher fetched tombstone (S=0,C=4,B=0) + // for the same channel. Without the fetchedTombstoneChannels guard, the projection + // overrides authoritative map (S=5,C=0,B=100) would produce hasUnread:true; + // with the guard, the fetched tombstone wins and the rail stays dark. + // + // Wire: fetched event has ov_c:CHANNEL_ID=4 but no ov_s: → decoded as {s:0,c:4,b:0}. + // fetchedTombstoneChannels includes CHANNEL_ID (s===0). Override liveness check is + // skipped for that channel even though authoritative (projection) has s=5. + const SLOT = "a".repeat(32); // valid 32-hex slot + const relay = relayFor([ + // 1. member events + () => [ + event({ + tags: [ + ["d", CHANNEL_ID], + ["p", PUBKEY], + ], + }), + ], + // 2. metadata events + () => [ + event({ + tags: [ + ["d", CHANNEL_ID], + ["t", "stream"], + ], + }), + ], + // 3. visibility events + () => [], + // 4. read-state: tombstone wire format (ov_c present, ov_s absent → s=0,c=4,b=0) + // Frontier F=50 is also present so the projection frontier is not strictly + // newer — only the tombstone vs live-register ordering is under test. + () => [ + event({ + pubkey: PUBKEY, + created_at: 400, + tags: [ + ["d", `read-state:${SLOT}`], + ["t", "read-state"], + ], + content: JSON.stringify({ + v: 1, + client_id: "client", + contexts: { + [CHANNEL_ID]: 50, + [`ov_c:${CHANNEL_ID}`]: 4, + // ov_s: intentionally absent → tombstone (s=0) + }, + }), + }), + ], + // 5. mutes + () => [], + // 6. unread events — none + () => [], + // 7. mention events — none + () => [], + ]); + + // Complete projection: live register S=5,C=0,B=100 and frontier F=50. + // F=50 ≤ B=100, S=5 > C=0 → isOverrideActive would return true WITHOUT the guard. + const overrides = new Map([[CHANNEL_ID, { s: 5, c: 0, b: 100 }]]); + const frontiers = new Map([[CHANNEL_ID, 50]]); + + const result = await fetchCommunityUnread({ + client: relay, + pubkey: PUBKEY, + nowSeconds: 500, + decryptReadState: async (v) => v, + decryptMutes: async (v) => v, + readThreadRelationships: readRelationships(), + getProjection: () => ({ loadComplete: true, frontiers, overrides }), + }); + + // Fetched tombstone (s=0) is authoritative; projection's stale live register must not + // resurrect unread when the relay has confirmed a clear. + assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); +}); diff --git a/desktop/src/features/communities/communityUnreadObserver.ts b/desktop/src/features/communities/communityUnreadObserver.ts index 7fcb31cf05..0947d41c70 100644 --- a/desktop/src/features/communities/communityUnreadObserver.ts +++ b/desktop/src/features/communities/communityUnreadObserver.ts @@ -145,9 +145,10 @@ export async function fetchObservedChannels( export async function pollCommunityUnread( community: Community, pubkey: string, + getProjection?: () => ReadStateProjection | null, ): Promise { return withReadOnlyRelayClient(community.relayUrl, (client) => - fetchCommunityUnread({ client, pubkey }), + fetchCommunityUnread({ client, pubkey, getProjection }), ); } @@ -220,6 +221,21 @@ export async function fetchCommunityUnread(args: { const projection = args.getProjection?.() ?? null; const completeProjection = projection?.loadComplete ? projection : null; + // Override liveness resolution: projection wins when complete, BUT a fetched + // wire tombstone (s===0, c>0) for the same coordinate is treated as + // authoritative — it proves the relay consensus has a clear with no + // countering mark-unread, and the projection's live S counter may be from + // an uncommitted/unpublished local action that the relay hasn't confirmed. + // + // fetchedTombstoneChannels is the set of channel IDs whose fetched register + // has s===0 (tombstone wire format). We skip projection liveness for those. + const fetchedTombstoneChannels = new Set(); + if (completeProjection !== null) { + for (const [ctx, reg] of readState.overrides) { + if (reg.s === 0) fetchedTombstoneChannels.add(ctx); + } + } + const authoritative: ReadonlyMap = completeProjection !== null ? completeProjection.overrides @@ -279,9 +295,15 @@ export async function fetchCommunityUnread(args: { // Override liveness: evaluate the authoritative register against the // effective frontier. Uses projection.overrides when complete (no // per-field max with fetched); fetched-deduped otherwise. + // Skip projection liveness when fetched relay has a wire tombstone (s=0) + // for this channel — a fresher relay tombstone is authoritative. if (!hasUnread) { const reg = authoritative.get(channel.id); - if (reg !== undefined && isOverrideActive(reg, readAt ?? 0)) { + if ( + reg !== undefined && + !fetchedTombstoneChannels.has(channel.id) && + isOverrideActive(reg, readAt ?? 0) + ) { hasUnread = true; } } diff --git a/desktop/src/features/communities/useCommunityUnread.ts b/desktop/src/features/communities/useCommunityUnread.ts index 851c77c955..9985e1e887 100644 --- a/desktop/src/features/communities/useCommunityUnread.ts +++ b/desktop/src/features/communities/useCommunityUnread.ts @@ -3,6 +3,7 @@ import * as React from "react"; import { getIdentity } from "@/shared/api/tauriIdentity"; import { markCommunityRead } from "@/features/communities/communityMarkRead"; import { pollCommunityUnread } from "@/features/communities/communityUnreadObserver"; +import { useAppShell } from "@/app/AppShellContext"; import type { Community } from "./types"; @@ -54,6 +55,7 @@ export function useCommunityUnread( unreadByCommunity: Record; markCommunityRead: (communityId: string) => Promise; } { + const { getProjection } = useAppShell(); const [unreadByCommunity, setUnreadByCommunity] = React.useState< Record >(() => seedCommunityStates(communities, {})); @@ -131,7 +133,11 @@ export function useCommunityUnread( if (cancelled) return; markLoading(community.id); try { - const result = await pollCommunityUnread(community, pubkey); + const result = await pollCommunityUnread( + community, + pubkey, + getProjection, + ); if (cancelled) return; markReady(community.id, result); } catch (error) { @@ -155,7 +161,7 @@ export function useCommunityUnread( window.clearTimeout(pollTimer); } }; - }, [activeCommunityId, communities]); + }, [activeCommunityId, communities, getProjection]); const communitiesRef = React.useRef(communities); communitiesRef.current = communities;