From 72d96d9d15d55de8ed77cf9d54ee77bb48e86f4d Mon Sep 17 00:00:00 2001 From: Tyler Longwell Date: Thu, 14 May 2026 11:41:19 -0400 Subject: [PATCH] fix(desktop): drive unread badges from live subscription, not refetched lastMessageAt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Tauri `get_channels` command always returns `last_message_at: null` (the kind:40901 summary-sidecar path exists in `nostr_convert.rs` but isn't wired up). Live messages were bumping the React Query cache via `updateChannelLastMessageAt`, then the 60s `refetchInterval` on `useChannelsQuery` was replacing that cache with the server's null value, wiping the bump. The sidebar memo then dropped the channel from the unread set because `lastMessageAt` was null again. Visible symptom: a phantom unread indicator appears on a channel after a live message, then disappears within ~60s without user interaction. Stop using `channel.lastMessageAt` for the unread computation. Maintain an in-session `Map` inside `useUnreadChannels`, fed by a new `onChannelMessage` callback on `useLiveChannelUpdates`. Unread is now: latest live timestamp strictly greater than the NIP-RS read marker. The callback is gated to human-visible content kinds (chat + forum posts/comments) and skipped for the current user's own events — reactions, edits, deletions, and system messages must not create unread state, and your own outgoing messages must never make a channel unread. `channelCache.ts` and all `updateChannelLastMessageAt` call sites are deleted. The first-load read-state seed now reads `channel.lastMessageAt` directly (today a no-op; future-proof for when the backend wires it) rather than the live map, so a live event racing ahead of read-state readiness can't be silently swallowed as already-read. Does not fix: - backend `get_channels` returning a real `last_message_at` (separate work: wire kind:40901 or compute on read); - the separate first-load render-ordering issue where the unread memo treats `readAt === null` as unread before the seed effect commits; - KIND_STREAM_MESSAGE_V2 (40002) missing from the desktop `CHANNEL_EVENT_KINDS` filter (pre-existing). Signed-off-by: Tyler Longwell Test fix: the e2e smoke tests "sidebar shows unread indicator for newly active channels" and "... for new forum posts" injected mock messages via `send_channel_message`, which authors events with the current identity's pubkey. They worked under the old code because nothing filtered self-authored events. Under this change, self-authored events correctly don't make a channel unread, so the tests need to inject as another user. Switched both to `__SPROUT_E2E_EMIT_MOCK_MESSAGE__` with `pubkey: TEST_IDENTITIES.alice.pubkey`, extending the helper to accept a `kind` argument (previously hardcoded to 9). The DM unread test already used this pattern. --- .../src/features/channels/lib/channelCache.ts | 66 --------- .../channels/useLiveChannelUpdates.ts | 44 ++++-- .../features/channels/useUnreadChannels.ts | 129 ++++++++++++++---- desktop/src/features/messages/hooks.ts | 14 -- desktop/src/testing/e2eBridge.ts | 21 ++- desktop/tests/e2e/channels.spec.ts | 93 ++++--------- 6 files changed, 185 insertions(+), 182 deletions(-) delete mode 100644 desktop/src/features/channels/lib/channelCache.ts diff --git a/desktop/src/features/channels/lib/channelCache.ts b/desktop/src/features/channels/lib/channelCache.ts deleted file mode 100644 index 340854953..000000000 --- a/desktop/src/features/channels/lib/channelCache.ts +++ /dev/null @@ -1,66 +0,0 @@ -import type { QueryClient } from "@tanstack/react-query"; - -import type { Channel } from "@/shared/api/types"; -import { channelsQueryKey } from "@/features/channels/hooks"; - -function parseTimestamp(value: string | null | undefined) { - if (!value) { - return null; - } - - const timestamp = Date.parse(value); - return Number.isNaN(timestamp) ? null : timestamp; -} - -function isNewerTimestamp( - candidate: string | null | undefined, - current: string | null | undefined, -) { - const candidateTimestamp = parseTimestamp(candidate); - if (candidateTimestamp === null) { - return false; - } - - const currentTimestamp = parseTimestamp(current); - return currentTimestamp === null || candidateTimestamp > currentTimestamp; -} - -export function updateChannelLastMessageAt( - queryClient: QueryClient, - channelId: string, - lastMessageAt: string | null | undefined, -) { - const lastMessageTimestamp = parseTimestamp(lastMessageAt); - const normalizedLastMessageAt = - lastMessageTimestamp === null - ? null - : new Date(lastMessageTimestamp).toISOString(); - - if (!normalizedLastMessageAt) { - return; - } - - queryClient.setQueryData(channelsQueryKey, (current) => { - if (!current) { - return current; - } - - let didUpdate = false; - const nextChannels = current.map((channel) => { - if ( - channel.id !== channelId || - !isNewerTimestamp(normalizedLastMessageAt, channel.lastMessageAt) - ) { - return channel; - } - - didUpdate = true; - return { - ...channel, - lastMessageAt: normalizedLastMessageAt, - }; - }); - - return didUpdate ? nextChannels : current; - }); -} diff --git a/desktop/src/features/channels/useLiveChannelUpdates.ts b/desktop/src/features/channels/useLiveChannelUpdates.ts index f712178c2..c20a57b1d 100644 --- a/desktop/src/features/channels/useLiveChannelUpdates.ts +++ b/desktop/src/features/channels/useLiveChannelUpdates.ts @@ -1,27 +1,45 @@ import * as React from "react"; import { useQueryClient } from "@tanstack/react-query"; -import { updateChannelLastMessageAt } from "@/features/channels/lib/channelCache"; import { channelsQueryKey } from "@/features/channels/hooks"; import { mergeTimelineCacheMessages } from "@/features/messages/hooks"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; import { getChannelIdFromTags } from "@/features/messages/lib/threading"; import { relayClient } from "@/shared/api/relayClient"; -import { CHANNEL_EVENT_KINDS } from "@/shared/constants/kinds"; +import { + CHANNEL_EVENT_KINDS, + KIND_FORUM_COMMENT, + KIND_FORUM_POST, + KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_V2, +} from "@/shared/constants/kinds"; import type { Channel, RelayEvent } from "@/shared/api/types"; export type UseLiveChannelUpdatesOptions = { currentPubkey?: string; onDmMessage?: (event: RelayEvent, channel: Channel) => void; onLiveMention?: () => void; + /** + * Fired for live "new content" events in a member channel (chat messages, + * forum posts/comments) authored by someone other than the current user. + * Used to drive the in-session "latest message at" map that powers sidebar + * unread badges. See `UNREAD_TRIGGER_KINDS` for the exact kind set. + */ + onChannelMessage?: (channelId: string, event: RelayEvent) => void; }; const LIVE_SUBSCRIPTION_RETRY_BASE_MS = 1_000; const LIVE_SUBSCRIPTION_RETRY_MAX_MS = 30_000; -function getMessageTimestamp(event: RelayEvent) { - return new Date(event.created_at * 1_000).toISOString(); -} +// Only "new content" kinds should bump unread state. Reactions, edits, +// diffs, deletions, and system messages can all arrive after the last +// human-visible message and would otherwise create phantom unreads. +const UNREAD_TRIGGER_KINDS = new Set([ + KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_V2, + KIND_FORUM_POST, + KIND_FORUM_COMMENT, +]); function isExternalMentionEvent(event: RelayEvent, currentPubkey: string) { return ( @@ -135,12 +153,22 @@ export function useLiveChannelUpdates( return; } - // Always update the cache — even for the active channel. + // Notify the unread tracker. Restricted to human-visible message kinds + // and to events authored by someone other than the current user — your + // own outgoing messages should never make a channel unread, and + // reactions / edits / system messages aren't "new content". + if ( + UNREAD_TRIGGER_KINDS.has(event.kind) && + (normalizedCurrentPubkey.length === 0 || + event.pubkey.toLowerCase() !== normalizedCurrentPubkey) + ) { + options.onChannelMessage?.(channelId, event); + } + + // Merge into the timeline cache for the active channel. // useChannelSubscription also writes to this cache, but there's a // race window where it hasn't connected yet. Writes are idempotent // (mergeTimelineCacheMessages deduplicates by event ID). - const messageTimestamp = getMessageTimestamp(event); - updateChannelLastMessageAt(queryClient, channelId, messageTimestamp); queryClient.setQueryData( channelMessagesKey(channelId), (current) => { diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index e20cff8c4..059b7c566 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -5,7 +5,7 @@ import { } from "@/features/channels/useLiveChannelUpdates"; import { useReadState } from "@/features/channels/readState/useReadState"; import type { RelayClient } from "@/shared/api/relayClientSession"; -import type { Channel } from "@/shared/api/types"; +import type { Channel, RelayEvent } from "@/shared/api/types"; type UseUnreadChannelsOptions = UseLiveChannelUpdatesOptions & { pubkey?: string; @@ -50,51 +50,115 @@ export function useUnreadChannels( seedContextRead, } = useReadState(pubkey, relayClient); + // In-session "latest message at" per channel (unix seconds), driven by the + // live subscription. The backend doesn't populate Channel.lastMessageAt, so + // unread state cannot rely on it; this map is the authoritative source for + // sidebar badges. Monotonic: only advances. Reset when the identity or + // relay changes. Stale entries for channels the user has left are silently + // ignored by the memo (it iterates the current channels list, not the map). + const latestByChannelRef = React.useRef(new Map()); + + // Channels manually marked unread this session (e.g., right-click → "mark + // unread"). Tracked separately from latestByChannelRef so we don't have to + // synthesise a "latest message" timestamp and risk the corresponding read + // marker becoming sticky. Cleared when the user opens the channel. + const forcedUnreadRef = React.useRef(new Set()); + + const [latestVersion, bumpLatestVersion] = React.useReducer( + (x: number) => x + 1, + 0, + ); + // Track whether channels have been initialized (for first-load seeding) const hasInitializedChannelsRef = React.useRef(false); + // Reset the in-session state when the identity or relay changes. + // biome-ignore lint/correctness/useExhaustiveDependencies: pubkey/relayClient are intentional reset signals + React.useEffect(() => { + latestByChannelRef.current = new Map(); + forcedUnreadRef.current = new Set(); + hasInitializedChannelsRef.current = false; + bumpLatestVersion(); + }, [pubkey, relayClient]); + const markChannelRead = React.useCallback( (channelId: string, readAt: string | null | undefined) => { const unixSeconds = toUnixSeconds(readAt); if (unixSeconds === null) return; + // Reading clears any prior manual mark-unread. + if (forcedUnreadRef.current.delete(channelId)) { + bumpLatestVersion(); + } markContextRead(channelId, unixSeconds); }, [markContextRead], ); + // Manually mark a channel unread (e.g., right-click → "mark unread"). Sets + // the in-session forced-unread flag so the sidebar badge appears immediately + // without us inventing a synthetic latest-message timestamp, and rolls the + // NIP-RS read marker back so the unread state syncs across devices. The + // forced flag is cleared when the user opens the channel (markChannelRead). const markChannelUnread = React.useCallback( (channelId: string, lastMessageAt: string | null | undefined) => { + if (!forcedUnreadRef.current.has(channelId)) { + forcedUnreadRef.current.add(channelId); + bumpLatestVersion(); + } const unixSeconds = toUnixSeconds(lastMessageAt); - if (unixSeconds === null) return; - markContextUnread(channelId, unixSeconds); + if (unixSeconds !== null) { + markContextUnread(channelId, unixSeconds); + } }, [markContextUnread], ); - // Seed new channels so they don't flash as unread on first load. - // For channels the user hasn't read yet, initialize read-at to the - // channel's current lastMessageAt so they appear as "read." + // Opportunistic backfill: if Channel.lastMessageAt is ever populated by the + // backend (today it isn't), seed the in-session map. Strict max — never + // overwrites a more recent live value. + React.useEffect(() => { + if (channels.length === 0) return; + let didAdvance = false; + for (const channel of channels) { + const seedUnix = toUnixSeconds(channel.lastMessageAt); + if (seedUnix === null) continue; + const current = latestByChannelRef.current.get(channel.id) ?? 0; + if (seedUnix > current) { + latestByChannelRef.current.set(channel.id, seedUnix); + didAdvance = true; + } + } + if (didAdvance) bumpLatestVersion(); + }, [channels]); + + // Seed read state on first load so existing channels don't flash as unread + // when the backend reports a non-null Channel.lastMessageAt. We deliberately + // seed from the backend-provided value (not from the live map), so a live + // event that races ahead of NIP-RS readiness can't be silently swallowed as + // already-read. Today the backend always returns null and this is a no-op; + // it becomes meaningful once last_message_at is wired up server-side. React.useEffect(() => { if (!isReadStateReady) return; if (channels.length === 0) return; + if (hasInitializedChannelsRef.current) return; for (const channel of channels) { const existing = getEffectiveTimestamp(channel.id); if (existing !== null) continue; - // Only seed on first initialization, not when new channels appear later - if (hasInitializedChannelsRef.current) continue; - - const lastMsgUnix = toUnixSeconds(channel.lastMessageAt); - if (lastMsgUnix !== null) { - seedContextRead(channel.id, lastMsgUnix); + const seedUnix = toUnixSeconds(channel.lastMessageAt); + if (seedUnix !== null) { + seedContextRead(channel.id, seedUnix); } } hasInitializedChannelsRef.current = true; }, [channels, getEffectiveTimestamp, isReadStateReady, seedContextRead]); - // Mark the active channel as read when it changes or new messages arrive + // Mark the active channel as read when it changes or new messages arrive. + // Honours the caller's contract that a null activeReadAt suppresses + // read-marking until the timeline reports a real position. Manual + // mark-unread state is cleared inside markChannelRead, not here. React.useEffect(() => { if (!isReadStateReady) return; if (!activeChannelId) return; @@ -106,14 +170,31 @@ export function useUnreadChannels( markChannelRead, ]); - // Keep live channel updates (drives channel.lastMessageAt cache updates) - useLiveChannelUpdates(channels, activeChannelId, liveUpdateOptions); + // Feed the in-session "latest message at" map from live channel events. + // Composes with any caller-supplied onChannelMessage handler. + const callerOnChannelMessage = liveUpdateOptions.onChannelMessage; + const handleChannelMessage = React.useCallback( + (channelId: string, event: RelayEvent) => { + const current = latestByChannelRef.current.get(channelId) ?? 0; + if (event.created_at > current) { + latestByChannelRef.current.set(channelId, event.created_at); + bumpLatestVersion(); + } + callerOnChannelMessage?.(channelId, event); + }, + [callerOnChannelMessage], + ); + + useLiveChannelUpdates(channels, activeChannelId, { + ...liveUpdateOptions, + onChannelMessage: handleChannelMessage, + }); - // Compute unread channel IDs by comparing channel.lastMessageAt against - // the NIP-RS effective timestamp. - // readStateVersion is intentionally included to force recomputation when - // cross-device state arrives (getEffectiveTimestamp is referentially stable). - // biome-ignore lint/correctness/useExhaustiveDependencies: readStateVersion is an intentional invalidation signal + // Unread = channels (excluding active) that have either been manually + // marked unread this session, or whose in-session latest message timestamp + // is strictly newer than their NIP-RS read marker. + // readStateVersion and latestVersion are intentional invalidation signals. + // biome-ignore lint/correctness/useExhaustiveDependencies: readStateVersion and latestVersion are intentional invalidation signals const unreadChannelIds = React.useMemo(() => { if (!isReadStateReady) { return new Set(); @@ -123,11 +204,12 @@ export function useUnreadChannels( channels .filter((channel) => channel.id !== activeChannelId) .filter((channel) => { - const lastMsgUnix = toUnixSeconds(channel.lastMessageAt); - if (lastMsgUnix === null) return false; + if (forcedUnreadRef.current.has(channel.id)) return true; + const latest = latestByChannelRef.current.get(channel.id); + if (latest === undefined) return false; const readAt = getEffectiveTimestamp(channel.id); - return readAt === null || lastMsgUnix > readAt; + return readAt === null || latest > readAt; }) .map((channel) => channel.id), ); @@ -136,6 +218,7 @@ export function useUnreadChannels( channels, getEffectiveTimestamp, isReadStateReady, + latestVersion, readStateVersion, ]); diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 14199a483..65ea1c1b1 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -1,7 +1,6 @@ import { useEffect, useEffectEvent } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { updateChannelLastMessageAt } from "@/features/channels/lib/channelCache"; import { channelMessagesKey, dedupeMessagesById, @@ -179,11 +178,6 @@ export function useChannelSubscription(channel: Channel | null) { return; } - updateChannelLastMessageAt( - queryClient, - channelId, - new Date(event.created_at * 1_000).toISOString(), - ); queryClient.setQueryData( channelMessagesKey(channelId), (current = []) => mergeTimelineCacheMessages(current, event), @@ -400,14 +394,6 @@ export function useSendMessageMutation( queryClient.setQueryData(context.queryKey, context.previousMessages); }, onSuccess: (message, _variables, context) => { - if (channel) { - updateChannelLastMessageAt( - queryClient, - channel.id, - new Date(message.created_at * 1_000).toISOString(), - ); - } - if (!context) { return; } diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 10dc352d1..c6f6713db 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -432,6 +432,7 @@ declare global { content: string; parentEventId?: string | null; pubkey?: string; + kind?: number; }) => RelayEvent; __SPROUT_E2E_EMIT_MOCK_TYPING__?: (input: { channelName: string; @@ -1716,9 +1717,16 @@ function emitMockChannelMessage( content: string, parentEventId?: string | null, pubkey?: string, + kind?: number, ) { + const eventKind = kind ?? 9; if (!parentEventId) { - const event = createMockEvent(9, content, [["h", channelId]], pubkey); + const event = createMockEvent( + eventKind, + content, + [["h", channelId]], + pubkey, + ); recordMockMessage(channelId, event); emitMockLiveEvent(channelId, event); return event; @@ -1736,7 +1744,7 @@ function emitMockChannelMessage( const rootEventId = parentThread.rootEventId ?? parentEventId; const authorPubkey = pubkey ?? DEFAULT_MOCK_IDENTITY.pubkey; const event = createMockEvent( - 9, + eventKind, content, buildReplyMessageTags( channelId, @@ -4501,6 +4509,7 @@ export function maybeInstallE2eTauriMocks() { content, parentEventId, pubkey, + kind, }) => { const channel = mockChannels.find( (candidate) => candidate.name === channelName, @@ -4509,7 +4518,13 @@ export function maybeInstallE2eTauriMocks() { throw new Error(`Mock channel ${channelName} not found.`); } - return emitMockChannelMessage(channel.id, content, parentEventId, pubkey); + return emitMockChannelMessage( + channel.id, + content, + parentEventId, + pubkey, + kind, + ); }; window.__SPROUT_E2E_EMIT_MOCK_TYPING__ = ({ channelName, pubkey }) => { const channel = mockChannels.find( diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index ebf56bb9a..71ea5fada 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -52,63 +52,6 @@ async function waitForMockLiveSubscription( .toBe(true); } -async function sendMockChannelMessage( - page: import("@playwright/test").Page, - { - channelName, - content, - kind, - mentionPubkeys, - }: { - channelName: string; - content: string; - kind?: number | null; - mentionPubkeys?: string[] | null; - }, -) { - await page.evaluate( - async ({ - channelName: targetChannelName, - content, - kind, - mentionPubkeys, - }) => { - const tauriWindow = window as Window & { - __TAURI_INTERNALS__?: { - invoke: ( - command: string, - payload?: Record, - ) => Promise; - }; - }; - - const invoke = tauriWindow.__TAURI_INTERNALS__?.invoke; - if (!invoke) { - throw new Error("Tauri invoke bridge is unavailable."); - } - - const channels = (await invoke("get_channels")) as Array<{ - id: string; - name: string; - }>; - const channel = channels.find(({ name }) => name === targetChannelName); - if (!channel) { - throw new Error(`Channel not found: ${targetChannelName}`); - } - - await invoke("send_channel_message", { - channelId: channel.id, - content, - kind: kind ?? null, - mediaTags: null, - mentionPubkeys: mentionPubkeys ?? null, - parentEventId: null, - }); - }, - { channelName, content, kind, mentionPubkeys }, - ); -} - async function openMemberMenu( page: import("@playwright/test").Page, pubkey: string, @@ -631,12 +574,19 @@ test("sidebar shows unread indicator for newly active channels", async ({ await expect(page.getByTestId("channel-unread-random")).toHaveCount(0); await waitForMockLiveSubscription(page, "random"); - await sendMockChannelMessage(page, { - channelName: "random", - content: "Unread update for #random", - kind: 40002, - mentionPubkeys: [MOCK_IDENTITY_PUBKEY], - }); + // The unread tracker ignores the current user's own messages, so emit as + // alice — simulating a real "another user posted while I was elsewhere". + await page.evaluate( + ({ pubkey }) => { + window.__SPROUT_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "random", + content: "Unread update for #random", + kind: 40002, + pubkey, + }); + }, + { pubkey: TEST_IDENTITIES.alice.pubkey }, + ); await expect(page.getByTestId("channel-unread-random")).toBeVisible(); @@ -654,11 +604,18 @@ test("sidebar shows unread indicator for new forum posts", async ({ page }) => { await expect(page.getByTestId("channel-unread-watercooler")).toHaveCount(0); await waitForMockLiveSubscription(page, "watercooler"); - await sendMockChannelMessage(page, { - channelName: "watercooler", - content: "Unread update for the forum", - kind: 45001, - }); + // Emit as alice — the unread tracker ignores self-authored messages. + await page.evaluate( + ({ pubkey }) => { + window.__SPROUT_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "watercooler", + content: "Unread update for the forum", + kind: 45001, + pubkey, + }); + }, + { pubkey: TEST_IDENTITIES.alice.pubkey }, + ); await expect(page.getByTestId("channel-unread-watercooler")).toBeVisible();