From bbd2fd6ff5733e7a5e9472af57aa2114547dcdce Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Wed, 26 Aug 2026 15:41:09 +0100 Subject: [PATCH 1/7] Resurface hidden DMs from client activity Signed-off-by: kenny lopez --- desktop/src/app/AppShell.tsx | 6 + .../features/channels/dmResurface.test.mjs | 82 ++++++ desktop/src/features/channels/dmResurface.ts | 89 +++++++ .../channels/hiddenDmResurfaceAction.test.mjs | 101 ++++++++ .../channels/hiddenDmResurfaceAction.ts | 50 ++++ desktop/src/features/channels/hooks.ts | 21 +- .../channels/useDmResurfaceFromMessages.ts | 96 +++++++ .../src/features/channels/useHiddenDmIds.ts | 50 ++++ .../home/hiddenDmInboxAction.test.mjs | 141 +++++++++++ .../src/features/home/hiddenDmInboxAction.ts | 76 ++++++ desktop/src/features/home/ui/HomeScreen.tsx | 33 +-- desktop/src/features/home/ui/HomeView.tsx | 44 ++-- .../home/useHiddenDmInboxNavigation.ts | 130 ++++++++++ .../lib/features/activity/activity_page.dart | 51 +++- .../features/activity/activity_provider.dart | 102 +++++++- .../lib/features/activity/dm_resurface.dart | 27 ++ .../features/channels/channels_provider.dart | 20 +- .../channels/channels_provider_lifecycle.dart | 15 ++ .../activity/activity_provider_test.dart | 239 +++++++++++++++++- .../features/activity/dm_resurface_test.dart | 40 +++ 20 files changed, 1351 insertions(+), 62 deletions(-) create mode 100644 desktop/src/features/channels/dmResurface.test.mjs create mode 100644 desktop/src/features/channels/dmResurface.ts create mode 100644 desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs create mode 100644 desktop/src/features/channels/hiddenDmResurfaceAction.ts create mode 100644 desktop/src/features/channels/useDmResurfaceFromMessages.ts create mode 100644 desktop/src/features/channels/useHiddenDmIds.ts create mode 100644 desktop/src/features/home/hiddenDmInboxAction.test.mjs create mode 100644 desktop/src/features/home/hiddenDmInboxAction.ts create mode 100644 desktop/src/features/home/useHiddenDmInboxNavigation.ts create mode 100644 mobile/lib/features/activity/dm_resurface.dart create mode 100644 mobile/lib/features/channels/channels_provider_lifecycle.dart create mode 100644 mobile/test/features/activity/dm_resurface_test.dart diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index 468435e15ec..eb5ab5a95d8 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -34,6 +34,7 @@ import { useHideDmMutation, useOpenDmMutation, } from "@/features/channels/hooks"; +import { useDmResurfaceFromMessages } from "@/features/channels/useDmResurfaceFromMessages"; import { useUnreadChannels } from "@/features/channels/useUnreadChannels"; import { useMembershipNotifications } from "@/features/channels/useMembershipNotifications"; import { useFeedItemState } from "@/features/home/useFeedItemState"; @@ -505,6 +506,11 @@ export function AppShell() { const { applyCanvas, applyAgents } = useApplyTemplate(); const openDmMutation = useOpenDmMutation(); const hideDmMutation = useHideDmMutation(); + useDmResurfaceFromMessages({ + pubkey: identityQuery.data?.pubkey, + relayUrl: communitiesHook.activeCommunity?.relayUrl, + reopen: openDmMutation.mutateAsync, + }); const { browseDialogType, openBrowseChannels: handleOpenBrowseChannels, diff --git a/desktop/src/features/channels/dmResurface.test.mjs b/desktop/src/features/channels/dmResurface.test.mjs new file mode 100644 index 00000000000..324e5a005ec --- /dev/null +++ b/desktop/src/features/channels/dmResurface.test.mjs @@ -0,0 +1,82 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + dmPeerPubkeysFromMembers, + isIncomingDmMessageFeedItem, + isIncomingDmMessageRelayEvent, + markHiddenDmFeedItems, +} from "./dmResurface.ts"; + +const SELF = "1".repeat(64); +const ALICE = "2".repeat(64); +const BOB = "3".repeat(64); + +function item(overrides = {}) { + return { + id: "event-1", + kind: 9, + pubkey: ALICE, + content: "hello", + createdAt: 10, + channelId: "dm-1", + channelName: "", + tags: [ + ["h", "dm-1"], + ["p", SELF], + ["p", BOB], + ], + category: "mention", + ...overrides, + }; +} + +test("DM resurface derives peers from authoritative membership", () => { + const members = [{ pubkey: SELF }, { pubkey: ALICE }, { pubkey: BOB }]; + assert.deepEqual(dmPeerPubkeysFromMembers(members, SELF), [ALICE, BOB]); + assert.deepEqual(dmPeerPubkeysFromMembers([{ pubkey: ALICE }], SELF), []); +}); + +test("only external addressed human messages qualify", () => { + assert.equal(isIncomingDmMessageFeedItem(item(), SELF), true); + assert.equal(isIncomingDmMessageFeedItem(item({ kind: 7 }), SELF), false); + assert.equal( + isIncomingDmMessageFeedItem(item({ pubkey: SELF }), SELF), + false, + ); + assert.equal( + isIncomingDmMessageFeedItem(item({ tags: [["h", "dm-1"]] }), SELF), + false, + ); +}); + +test("hidden feed items are projected as DMs for Inbox presentation", () => { + const feed = { + feed: { + mentions: [item()], + needsAction: [], + activity: [], + agentActivity: [], + }, + meta: { since: 0, total: 1, generatedAt: 10 }, + }; + const marked = markHiddenDmFeedItems(feed, new Set(["dm-1"])); + assert.equal(marked.feed.mentions[0].channelType, "dm"); +}); + +test("relay events use the same eligibility contract", () => { + const relayEvent = { + id: "event-1", + kind: 40002, + pubkey: ALICE, + content: "hello", + created_at: 10, + tags: [ + ["h", "dm-1"], + ["p", SELF], + ["p", BOB], + ], + sig: "", + }; + assert.equal(isIncomingDmMessageRelayEvent(relayEvent, SELF), true); +}); diff --git a/desktop/src/features/channels/dmResurface.ts b/desktop/src/features/channels/dmResurface.ts new file mode 100644 index 00000000000..5e4bf6a2f54 --- /dev/null +++ b/desktop/src/features/channels/dmResurface.ts @@ -0,0 +1,89 @@ +import type { + ChannelMember, + FeedItem, + HomeFeedResponse, + RelayEvent, +} from "@/shared/api/types"; +import { CHANNEL_MESSAGE_EVENT_KINDS } from "@/shared/constants/kinds"; +import { normalizePubkey } from "@/shared/lib/pubkey"; + +const CHANNEL_MESSAGE_KINDS = new Set(CHANNEL_MESSAGE_EVENT_KINDS); +const HEX_PUBKEY = /^[0-9a-f]{64}$/; + +export function dmPeerPubkeysFromMembers( + members: readonly Pick[], + currentPubkey: string | undefined, +): string[] { + const self = normalizePubkey(currentPubkey ?? ""); + const normalized = [ + ...new Set(members.map((member) => normalizePubkey(member.pubkey))), + ].filter((pubkey) => HEX_PUBKEY.test(pubkey)); + if (!HEX_PUBKEY.test(self) || !normalized.includes(self)) return []; + return normalized.filter((pubkey) => pubkey !== self); +} + +export function isIncomingDmMessageFeedItem( + item: FeedItem, + currentPubkey: string | undefined, +): boolean { + const self = normalizePubkey(currentPubkey ?? ""); + if ( + !item.channelId || + self.length === 0 || + !CHANNEL_MESSAGE_KINDS.has(item.kind) || + normalizePubkey(item.pubkey) === self + ) { + return false; + } + + return item.tags.some( + (tag) => tag[0] === "p" && normalizePubkey(tag[1] ?? "") === self, + ); +} + +export function isIncomingDmMessageRelayEvent( + event: RelayEvent, + currentPubkey: string | undefined, +): boolean { + return isIncomingDmMessageFeedItem( + { + id: event.id, + kind: event.kind, + pubkey: event.pubkey, + content: event.content, + createdAt: event.created_at, + channelId: + event.tags.find((tag) => tag[0] === "h" && tag[1])?.[1] ?? null, + channelName: "", + tags: event.tags, + category: "mention", + }, + currentPubkey, + ); +} + +export function relayEventChannelId(event: RelayEvent): string | null { + return event.tags.find((tag) => tag[0] === "h" && tag[1])?.[1] ?? null; +} + +export function markHiddenDmFeedItems( + feed: HomeFeedResponse, + hiddenDmIds: ReadonlySet, +): HomeFeedResponse { + if (hiddenDmIds.size === 0) return feed; + + const mark = (item: FeedItem): FeedItem => + item.channelId && hiddenDmIds.has(item.channelId) + ? { ...item, channelType: "dm" } + : item; + + return { + ...feed, + feed: { + mentions: feed.feed.mentions.map(mark), + needsAction: feed.feed.needsAction.map(mark), + activity: feed.feed.activity.map(mark), + agentActivity: feed.feed.agentActivity.map(mark), + }, + }; +} diff --git a/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs b/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs new file mode 100644 index 00000000000..640ef8a1f9c --- /dev/null +++ b/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs @@ -0,0 +1,101 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resurfaceHiddenDmMessage } from "./hiddenDmResurfaceAction.ts"; + +const SELF = "1".repeat(64); +const ALICE = "2".repeat(64); +const BOB = "3".repeat(64); + +function event() { + return { + id: "event-1", + kind: 40002, + pubkey: ALICE, + content: "hello", + created_at: 10, + // Message p tags are intentionally incomplete for this group DM. + tags: [ + ["h", "hidden-dm"], + ["p", SELF], + ], + sig: "", + }; +} + +function member(pubkey) { + return { + pubkey, + role: "member", + isAgent: false, + joinedAt: "", + displayName: null, + }; +} + +test("reopens the source hidden group DM from authoritative membership", async () => { + const inputs = []; + assert.equal( + await resurfaceHiddenDmMessage({ + event: event(), + expectedRelayUrl: "wss://relay.example", + expectedSignerPubkey: SELF, + fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + fetchMembers: async () => [member(SELF), member(ALICE), member(BOB)], + isCurrent: () => true, + reopen: async (input) => { + inputs.push(input); + return { id: "hidden-dm" }; + }, + }), + true, + ); + assert.deepEqual(inputs, [ + { + pubkeys: [ALICE, BOB], + expectedRelayUrl: "wss://relay.example", + expectedSignerPubkey: SELF, + }, + ]); +}); + +test("a suspended old-community read cannot reopen a DM", async () => { + let current = true; + let resume; + const members = new Promise((resolve) => { + resume = resolve; + }); + let reopenCount = 0; + const result = resurfaceHiddenDmMessage({ + event: event(), + expectedRelayUrl: "wss://old.example", + expectedSignerPubkey: SELF, + fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + fetchMembers: async () => members, + isCurrent: () => current, + reopen: async () => { + reopenCount += 1; + return { id: "hidden-dm" }; + }, + }); + await Promise.resolve(); + current = false; + resume([member(SELF), member(ALICE)]); + assert.equal(await result, false); + assert.equal(reopenCount, 0); +}); + +test("rejects a reopen result for any channel other than the source", async () => { + await assert.rejects( + resurfaceHiddenDmMessage({ + event: event(), + expectedRelayUrl: "wss://relay.example", + expectedSignerPubkey: SELF, + fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + fetchMembers: async () => [member(SELF), member(ALICE)], + isCurrent: () => true, + reopen: async () => ({ id: "alternate-dm" }), + }), + /different DM conversation/, + ); +}); diff --git a/desktop/src/features/channels/hiddenDmResurfaceAction.ts b/desktop/src/features/channels/hiddenDmResurfaceAction.ts new file mode 100644 index 00000000000..c740a3f2d29 --- /dev/null +++ b/desktop/src/features/channels/hiddenDmResurfaceAction.ts @@ -0,0 +1,50 @@ +import type { ChannelMember, RelayEvent } from "@/shared/api/types"; +import type { OpenDmInput } from "@/shared/api/tauriChannels"; +import { + dmPeerPubkeysFromMembers, + isIncomingDmMessageRelayEvent, + relayEventChannelId, +} from "./dmResurface"; + +type HiddenDmResurfaceActionOptions = { + event: RelayEvent; + expectedRelayUrl: string; + expectedSignerPubkey: string; + fetchHiddenDmIds: () => Promise>; + fetchMembers: (channelId: string) => Promise; + isCurrent: () => boolean; + reopen: (input: OpenDmInput) => Promise<{ id: string }>; +}; + +export async function resurfaceHiddenDmMessage({ + event, + expectedRelayUrl, + expectedSignerPubkey, + fetchHiddenDmIds, + fetchMembers, + isCurrent, + reopen, +}: HiddenDmResurfaceActionOptions): Promise { + if (!isIncomingDmMessageRelayEvent(event, expectedSignerPubkey)) return false; + const channelId = relayEventChannelId(event); + if (!channelId) return false; + + const hiddenDmIds = await fetchHiddenDmIds(); + if (!isCurrent() || !hiddenDmIds.has(channelId)) return false; + + const members = await fetchMembers(channelId); + if (!isCurrent()) return false; + const pubkeys = dmPeerPubkeysFromMembers(members, expectedSignerPubkey); + if (pubkeys.length === 0) return false; + + const opened = await reopen({ + pubkeys, + expectedRelayUrl, + expectedSignerPubkey, + }); + if (!isCurrent()) return false; + if (opened.id !== channelId) { + throw new Error("Relay reopened a different DM conversation."); + } + return true; +} diff --git a/desktop/src/features/channels/hooks.ts b/desktop/src/features/channels/hooks.ts index 9069b052da4..87035fc15ea 100644 --- a/desktop/src/features/channels/hooks.ts +++ b/desktop/src/features/channels/hooks.ts @@ -53,6 +53,7 @@ import { CHANNEL_MEMBERS_STALE_TIME_MS, channelMembersQueryKey, } from "@/features/channels/rosterFreshness"; +import { dmVisibilityQueryKey } from "@/features/channels/useHiddenDmIds"; export const channelsQueryKey = ["channels"] as const; /** Keeps focused polling at the established one-minute cadence. */ @@ -537,6 +538,14 @@ export function useOpenDmMutation() { queryClient.setQueryData(channelsQueryKey, (current) => upsertCachedChannel(current, openedChannel), ); + queryClient.setQueriesData>( + { queryKey: dmVisibilityQueryKey }, + (current) => { + const next = new Set(current); + next.delete(openedChannel.id); + return next; + }, + ); }, onSettled: () => { // The relay-returned DM is already in the cache. Mark the list stale so @@ -546,6 +555,7 @@ export function useOpenDmMutation() { queryKey: channelsQueryKey, refetchType: "none", }); + void queryClient.invalidateQueries({ queryKey: dmVisibilityQueryKey }); }, }); } @@ -591,8 +601,17 @@ export function useHideDmMutation() { queryClient.setQueryData(channelsQueryKey, context.previous); } }, + onSuccess: (_data, channelId) => { + queryClient.setQueriesData>( + { queryKey: dmVisibilityQueryKey }, + (current) => new Set(current).add(channelId), + ); + }, onSettled: async () => { - await queryClient.invalidateQueries({ queryKey: channelsQueryKey }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey: channelsQueryKey }), + queryClient.invalidateQueries({ queryKey: dmVisibilityQueryKey }), + ]); }, }); } diff --git a/desktop/src/features/channels/useDmResurfaceFromMessages.ts b/desktop/src/features/channels/useDmResurfaceFromMessages.ts new file mode 100644 index 00000000000..b0a7c0a6b79 --- /dev/null +++ b/desktop/src/features/channels/useDmResurfaceFromMessages.ts @@ -0,0 +1,96 @@ +import * as React from "react"; + +import { relayClient } from "@/shared/api/relayClient"; +import type { RelayEvent } from "@/shared/api/types"; +import { + getChannelMembers, + type OpenDmInput, +} from "@/shared/api/tauriChannels"; +import { CHANNEL_MESSAGE_EVENT_KINDS } from "@/shared/constants/kinds"; +import { relayEventChannelId } from "./dmResurface"; +import { resurfaceHiddenDmMessage } from "./hiddenDmResurfaceAction"; +import { fetchHiddenDmIds } from "./useHiddenDmIds"; + +type UseDmResurfaceFromMessagesOptions = { + pubkey: string | undefined; + relayUrl: string | undefined; + reopen: (input: OpenDmInput) => Promise<{ id: string }>; +}; + +export function useDmResurfaceFromMessages({ + pubkey, + relayUrl, + reopen, +}: UseDmResurfaceFromMessagesOptions) { + const handledEventIdsRef = React.useRef(new Set()); + const pendingChannelIdsRef = React.useRef(new Set()); + const generationRef = React.useRef(0); + const reopenLatest = React.useEffectEvent(reopen); + + React.useEffect(() => { + const expectedSignerPubkey = pubkey?.trim().toLowerCase() ?? ""; + const expectedRelayUrl = relayUrl?.trim() ?? ""; + const generation = ++generationRef.current; + handledEventIdsRef.current.clear(); + pendingChannelIdsRef.current.clear(); + if (!expectedSignerPubkey || !expectedRelayUrl) return; + + let disposed = false; + let unsubscribe: (() => Promise) | undefined; + const isCurrent = () => !disposed && generationRef.current === generation; + const handleEvent = async (event: RelayEvent) => { + if (!isCurrent() || !handledEventIdsRef.current.add(event.id)) return; + const channelId = relayEventChannelId(event); + if (!channelId || pendingChannelIdsRef.current.has(channelId)) return; + pendingChannelIdsRef.current.add(channelId); + + try { + await resurfaceHiddenDmMessage({ + event, + expectedRelayUrl, + expectedSignerPubkey, + fetchHiddenDmIds: () => fetchHiddenDmIds(expectedSignerPubkey), + fetchMembers: getChannelMembers, + isCurrent, + reopen: reopenLatest, + }); + } catch (error) { + handledEventIdsRef.current.delete(event.id); + if (isCurrent()) { + console.error("Failed to resurface hidden DM", channelId, error); + } + } finally { + pendingChannelIdsRef.current.delete(channelId); + } + }; + + void relayClient + .subscribeLive( + { + kinds: [...CHANNEL_MESSAGE_EVENT_KINDS], + "#p": [expectedSignerPubkey], + since: Math.floor(Date.now() / 1_000), + limit: 100, + }, + (event) => void handleEvent(event), + ) + .then((dispose) => { + if (!isCurrent()) { + void dispose().catch(() => {}); + return; + } + unsubscribe = dispose; + }) + .catch((error) => { + if (isCurrent()) { + console.error("Failed to subscribe to hidden DM activity", error); + } + }); + + return () => { + disposed = true; + generationRef.current += 1; + void unsubscribe?.().catch(() => {}); + }; + }, [pubkey, relayUrl]); +} diff --git a/desktop/src/features/channels/useHiddenDmIds.ts b/desktop/src/features/channels/useHiddenDmIds.ts new file mode 100644 index 00000000000..521d49666fa --- /dev/null +++ b/desktop/src/features/channels/useHiddenDmIds.ts @@ -0,0 +1,50 @@ +import * as React from "react"; +import { useQuery } from "@tanstack/react-query"; + +import { useCommunities } from "@/features/communities/useCommunities"; +import { relayClient } from "@/shared/api/relayClient"; +import type { RelayEvent } from "@/shared/api/types"; +import { KIND_DM_VISIBILITY } from "@/shared/constants/kinds"; +import { normalizePubkey } from "@/shared/lib/pubkey"; + +export const dmVisibilityQueryKey = ["dm-visibility"] as const; + +export function extractHiddenDmIds(events: readonly RelayEvent[]): Set { + const latest = events.reduce( + (current, event) => + current === null || event.created_at > current.created_at + ? event + : current, + null, + ); + return new Set( + (latest?.tags ?? []) + .filter((tag) => tag[0] === "h" && tag[1]) + .map((tag) => tag[1]), + ); +} + +export async function fetchHiddenDmIds(pubkey: string): Promise> { + const normalizedPubkey = normalizePubkey(pubkey); + if (normalizedPubkey.length === 0) return new Set(); + const events = await relayClient.fetchEvents({ + kinds: [KIND_DM_VISIBILITY], + "#p": [normalizedPubkey], + limit: 1, + }); + return extractHiddenDmIds(events); +} + +export function useHiddenDmIds(pubkey: string | undefined) { + const { activeCommunity } = useCommunities(); + const normalizedPubkey = normalizePubkey(pubkey ?? ""); + const relayUrl = activeCommunity?.relayUrl ?? ""; + const query = useQuery({ + queryKey: [...dmVisibilityQueryKey, relayUrl, normalizedPubkey], + queryFn: () => fetchHiddenDmIds(normalizedPubkey), + enabled: relayUrl.length > 0 && normalizedPubkey.length > 0, + staleTime: 30_000, + }); + + return React.useMemo(() => query.data ?? new Set(), [query.data]); +} diff --git a/desktop/src/features/home/hiddenDmInboxAction.test.mjs b/desktop/src/features/home/hiddenDmInboxAction.test.mjs new file mode 100644 index 00000000000..0e98a9a4cb9 --- /dev/null +++ b/desktop/src/features/home/hiddenDmInboxAction.test.mjs @@ -0,0 +1,141 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { openHiddenDmInboxContext } from "./hiddenDmInboxAction.ts"; + +const SELF = "1".repeat(64); +const ALICE = "2".repeat(64); +const BOB = "3".repeat(64); +const inboxItem = { + id: "event-1", + item: { + channelType: "dm", + // Incomplete message tags must not choose the recreated membership. + tags: [ + ["h", "hidden-dm"], + ["p", SELF], + ], + }, +}; + +function member(pubkey) { + return { + pubkey, + role: "member", + isAgent: false, + joinedAt: "", + displayName: null, + }; +} + +function options(overrides = {}) { + return { + item: inboxItem, + channelId: "hidden-dm", + messageId: "event-1", + availableChannelIds: new Set(), + expectedRelayUrl: "wss://relay.example", + expectedSignerPubkey: SELF, + pendingChannelIds: new Set(), + fetchMembers: async () => [member(SELF), member(ALICE), member(BOB)], + openDm: async () => ({ id: "hidden-dm" }), + isCurrent: () => true, + onOpenContext: () => {}, + onError: () => {}, + onPendingChange: () => {}, + ...overrides, + }; +} + +test("Inbox reopens the original hidden group DM from channel membership", async () => { + const inputs = []; + const navigations = []; + const result = await openHiddenDmInboxContext( + options({ + openDm: async (input) => { + inputs.push(input); + return { id: "hidden-dm" }; + }, + onOpenContext: (...args) => navigations.push(args), + }), + ); + assert.equal(result, true); + assert.deepEqual(inputs[0].pubkeys, [ALICE, BOB]); + assert.deepEqual(navigations, [["hidden-dm", "event-1", undefined]]); +}); + +test("double activation is deduplicated while a reopen is pending", async () => { + let resume; + const members = new Promise((resolve) => { + resume = resolve; + }); + let openCount = 0; + const shared = options({ + fetchMembers: async () => members, + openDm: async () => { + openCount += 1; + return { id: "hidden-dm" }; + }, + }); + const first = openHiddenDmInboxContext(shared); + const second = openHiddenDmInboxContext(shared); + resume([member(SELF), member(ALICE)]); + assert.equal(await second, false); + assert.equal(await first, true); + assert.equal(openCount, 1); +}); + +test("a failed reopen stays put, reports an error, and can be retried", async () => { + let attempts = 0; + let errors = 0; + let navigations = 0; + const shared = options({ + openDm: async () => { + attempts += 1; + if (attempts === 1) throw new Error("offline"); + return { id: "hidden-dm" }; + }, + onError: () => { + errors += 1; + }, + onOpenContext: () => { + navigations += 1; + }, + }); + assert.equal(await openHiddenDmInboxContext(shared), false); + assert.equal(navigations, 0); + assert.equal(errors, 1); + assert.equal(shared.pendingChannelIds.size, 0); + + assert.equal(await openHiddenDmInboxContext(shared), true); + assert.equal(attempts, 2); + assert.equal(navigations, 1); +}); + +test("an unmounted Inbox action cannot navigate after reopen settles", async () => { + let current = true; + let resume; + const reopened = new Promise((resolve) => { + resume = resolve; + }); + let navigations = 0; + let pendingChanges = 0; + const result = openHiddenDmInboxContext( + options({ + openDm: async () => reopened, + isCurrent: () => current, + onOpenContext: () => { + navigations += 1; + }, + onPendingChange: () => { + pendingChanges += 1; + }, + }), + ); + await Promise.resolve(); + current = false; + resume({ id: "hidden-dm" }); + assert.equal(await result, false); + assert.equal(navigations, 0); + assert.equal(pendingChanges, 1); +}); diff --git a/desktop/src/features/home/hiddenDmInboxAction.ts b/desktop/src/features/home/hiddenDmInboxAction.ts new file mode 100644 index 00000000000..60b2ef15d2f --- /dev/null +++ b/desktop/src/features/home/hiddenDmInboxAction.ts @@ -0,0 +1,76 @@ +import { dmPeerPubkeysFromMembers } from "@/features/channels/dmResurface"; +import type { InboxItem } from "@/features/home/lib/inbox"; +import type { ChannelMember } from "@/shared/api/types"; +import type { OpenDmInput } from "@/shared/api/tauriChannels"; + +type HiddenDmInboxActionOptions = { + item: InboxItem; + channelId: string; + messageId: string; + threadRootId?: string | null; + availableChannelIds: ReadonlySet; + expectedRelayUrl: string; + expectedSignerPubkey: string; + pendingChannelIds: Set; + fetchMembers: (channelId: string) => Promise; + openDm: (input: OpenDmInput) => Promise<{ id: string }>; + isCurrent: () => boolean; + onOpenContext: ( + channelId: string, + messageId: string, + threadRootId?: string | null, + ) => void; + onError: () => void; + onPendingChange: () => void; +}; + +export async function openHiddenDmInboxContext({ + item, + channelId, + messageId, + threadRootId, + availableChannelIds, + expectedRelayUrl, + expectedSignerPubkey, + pendingChannelIds, + fetchMembers, + openDm, + isCurrent, + onOpenContext, + onError, + onPendingChange, +}: HiddenDmInboxActionOptions): Promise { + if (availableChannelIds.has(channelId) || item.item.channelType !== "dm") { + if (isCurrent()) onOpenContext(channelId, messageId, threadRootId); + return true; + } + if (pendingChannelIds.has(channelId)) return false; + + pendingChannelIds.add(channelId); + onPendingChange(); + try { + const members = await fetchMembers(channelId); + if (!isCurrent()) return false; + const pubkeys = dmPeerPubkeysFromMembers(members, expectedSignerPubkey); + if (pubkeys.length === 0) { + throw new Error("Could not determine the DM membership."); + } + const opened = await openDm({ + pubkeys, + expectedRelayUrl, + expectedSignerPubkey, + }); + if (!isCurrent()) return false; + if (opened.id !== channelId) { + throw new Error("Relay reopened a different DM conversation."); + } + onOpenContext(channelId, messageId, threadRootId); + return true; + } catch { + if (isCurrent()) onError(); + return false; + } finally { + pendingChannelIds.delete(channelId); + if (isCurrent()) onPendingChange(); + } +} diff --git a/desktop/src/features/home/ui/HomeScreen.tsx b/desktop/src/features/home/ui/HomeScreen.tsx index b6512816e38..3dcfbbc3835 100644 --- a/desktop/src/features/home/ui/HomeScreen.tsx +++ b/desktop/src/features/home/ui/HomeScreen.tsx @@ -1,6 +1,8 @@ import * as React from "react"; import { useAppShell } from "@/app/AppShellContext"; +import { markHiddenDmFeedItems } from "@/features/channels/dmResurface"; +import { useHiddenDmIds } from "@/features/channels/useHiddenDmIds"; import { useHomeFeedQuery } from "@/features/home/hooks"; import { HomeView } from "@/features/home/ui/HomeView"; import type { HomeFeedResponse } from "@/shared/api/types"; @@ -26,24 +28,25 @@ export function HomeScreen({ }: HomeScreenProps) { const homeFeedQuery = useHomeFeedQuery(); const { threadActivityFeedItems } = useAppShell(); + const hiddenDmIds = useHiddenDmIds(currentPubkey); const augmentedFeed = React.useMemo((): HomeFeedResponse | undefined => { if (!homeFeedQuery.data) return undefined; - if (threadActivityFeedItems.length === 0) { - return homeFeedQuery.data; - } - - return { - ...homeFeedQuery.data, - feed: { - ...homeFeedQuery.data.feed, - activity: [ - ...homeFeedQuery.data.feed.activity, - ...threadActivityFeedItems, - ], - }, - }; - }, [homeFeedQuery.data, threadActivityFeedItems]); + const withThreadActivity = + threadActivityFeedItems.length === 0 + ? homeFeedQuery.data + : { + ...homeFeedQuery.data, + feed: { + ...homeFeedQuery.data.feed, + activity: [ + ...homeFeedQuery.data.feed.activity, + ...threadActivityFeedItems, + ], + }, + }; + return markHiddenDmFeedItems(withThreadActivity, hiddenDmIds); + }, [hiddenDmIds, homeFeedQuery.data, threadActivityFeedItems]); return (
diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 893b3c309c6..889cc61be83 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -2,9 +2,8 @@ import * as React from "react"; import { RefreshCcw } from "lucide-react"; import { useAppShell } from "@/app/AppShellContext"; -import { useAppNavigation } from "@/app/navigation/useAppNavigation"; import { useKnownAgentPubkeys } from "@/features/agents/useKnownAgentPubkeys"; -import { useChannelsQuery, useOpenDmMutation } from "@/features/channels/hooks"; +import { useChannelsQuery } from "@/features/channels/hooks"; import { RightAuxiliaryPane } from "@/features/channels/ui/RightAuxiliaryPane"; import { ChannelManagementSheet } from "@/features/channels/ui/ChannelManagementSheet"; import { @@ -28,6 +27,7 @@ import { useHomeInboxAutoSelection } from "@/features/home/useHomeInboxAutoSelec import { useHomeInboxContextMessages } from "@/features/home/useHomeInboxContextMessages"; import { useHomePersonalInbox } from "@/features/home/useHomePersonalInbox"; import { useInboxThreadContext } from "@/features/home/useInboxThreadContext"; +import { useHiddenDmInboxNavigation } from "@/features/home/useHiddenDmInboxNavigation"; import { type ProfilePanelTab, type ProfilePanelView, @@ -171,9 +171,6 @@ export function HomeView({ const [membersChannel, setMembersChannel] = React.useState( null, ); - const { goChannel } = useAppNavigation(); - const openDmMutation = useOpenDmMutation(); - const openDm = openDmMutation.mutateAsync; const handleUserSelectItem = React.useCallback( (itemId: string | null) => { setAutoSelectedEventId(null); @@ -219,13 +216,6 @@ export function HomeView({ const [emptyDeleteId, setEmptyDeleteId] = React.useState(null); const [editTargetId, setEditTargetId] = React.useState(null); const [isSendingReply, setIsSendingReply] = React.useState(false); - const handleOpenDm = React.useCallback( - async (pubkeys: string[]) => { - const dm = await openDm({ pubkeys }); - await goChannel(dm.id); - }, - [goChannel, openDm], - ); const { activeReminderEventIds, openReminder } = useRemindLater(); const [localRepliesByItemId, setLocalRepliesByItemId] = React.useState< Record @@ -460,6 +450,17 @@ export function HomeView({ } return null; }, [filteredItems, selectedConversationId, selectedEventId]); + const { + canOpenSelected, + handleOpenDirect, + handleOpenDm, + handleOpenSelectedContext, + } = useHiddenDmInboxNavigation({ + availableChannelIds, + currentPubkey, + onOpenContext, + selectedItem, + }); const deleteInboxMessage = React.useCallback( async (eventId: string) => { const channelId = selectedItem?.item.channelId; @@ -700,17 +701,7 @@ export function HomeView({ onFilterChange={handleFilterChange} onMarkRead={markItemRead} onMarkUnread={markItemUnread} - onOpenDirect={(item) => { - const channelId = item.item.channelId; - if (!channelId) { - return; - } - onOpenContext( - channelId, - item.id, - getThreadReference(item.item.tags).rootId, - ); - }} + onOpenDirect={handleOpenDirect} onRemindLater={(item) => { const channelId = item.item.channelId; if (!channelId) { @@ -788,10 +779,7 @@ export function HomeView({ ; + currentPubkey: string | undefined; + onOpenContext: ( + channelId: string, + messageId: string, + threadRootId?: string | null, + ) => void; + selectedItem: InboxItem | null; +}; + +export function useHiddenDmInboxNavigation({ + availableChannelIds, + currentPubkey, + onOpenContext, + selectedItem, +}: UseHiddenDmInboxNavigationOptions) { + const { goChannel } = useAppNavigation(); + const { activeCommunity } = useCommunities(); + const openDm = useOpenDmMutation().mutateAsync; + const expectedRelayUrl = activeCommunity?.relayUrl ?? ""; + const expectedSignerPubkey = currentPubkey?.trim().toLowerCase() ?? ""; + const scopeKey = + expectedRelayUrl && expectedSignerPubkey + ? `${expectedRelayUrl}\u0000${expectedSignerPubkey}` + : ""; + const pendingChannelIdsRef = React.useRef(new Set()); + const generationRef = React.useRef(0); + const [, setPendingVersion] = React.useState(0); + React.useEffect(() => { + if (!scopeKey) return; + generationRef.current += 1; + pendingChannelIdsRef.current.clear(); + setPendingVersion((version) => version + 1); + return () => { + generationRef.current += 1; + pendingChannelIdsRef.current.clear(); + }; + }, [scopeKey]); + + const openContext = React.useCallback( + async ( + item: InboxItem, + channelId: string, + messageId: string, + threadRootId?: string | null, + ) => { + const generation = generationRef.current; + await openHiddenDmInboxContext({ + item, + channelId, + messageId, + threadRootId, + availableChannelIds, + expectedRelayUrl, + expectedSignerPubkey, + pendingChannelIds: pendingChannelIdsRef.current, + fetchMembers: getChannelMembers, + openDm, + isCurrent: () => generationRef.current === generation, + onOpenContext, + onError: () => toast.error("Could not reopen conversation. Try again."), + onPendingChange: () => setPendingVersion((version) => version + 1), + }); + }, + [ + availableChannelIds, + expectedRelayUrl, + expectedSignerPubkey, + onOpenContext, + openDm, + ], + ); + + const selectedChannelId = selectedItem?.item.channelId ?? null; + + return { + canOpenSelected: Boolean( + selectedChannelId && + !pendingChannelIdsRef.current.has(selectedChannelId) && + (availableChannelIds.has(selectedChannelId) || + (selectedItem?.item.channelType === "dm" && + expectedRelayUrl.length > 0 && + expectedSignerPubkey.length > 0)), + ), + handleOpenDirect: React.useCallback( + (item: InboxItem) => { + const channelId = item.item.channelId; + if (!channelId) return; + void openContext( + item, + channelId, + item.id, + getThreadReference(item.item.tags).rootId, + ); + }, + [openContext], + ), + handleOpenDm: React.useCallback( + async (pubkeys: string[]) => { + const dm = await openDm({ + pubkeys, + expectedRelayUrl, + expectedSignerPubkey, + }); + await goChannel(dm.id); + }, + [expectedRelayUrl, expectedSignerPubkey, goChannel, openDm], + ), + handleOpenSelectedContext: React.useCallback( + (channelId: string, messageId: string, threadRootId?: string | null) => { + if (selectedItem) { + void openContext(selectedItem, channelId, messageId, threadRootId); + } + }, + [openContext, selectedItem], + ), + }; +} diff --git a/mobile/lib/features/activity/activity_page.dart b/mobile/lib/features/activity/activity_page.dart index 04cb2ff8cdd..b4dc54fd1ea 100644 --- a/mobile/lib/features/activity/activity_page.dart +++ b/mobile/lib/features/activity/activity_page.dart @@ -22,6 +22,7 @@ import '../../shared/widgets/message_author_meta.dart'; import '../../shared/widgets/modal_presentation.dart'; import '../channels/channel.dart'; import '../channels/channel_detail_page.dart'; +import '../channels/channel_management_provider.dart'; import '../channels/channels_provider.dart'; import '../channels/dm_channel_labels.dart'; import '../channels/message_content.dart'; @@ -31,6 +32,7 @@ import '../../shared/profile/user_cache_provider.dart'; import '../../shared/profile/user_profile.dart'; import 'activity_provider.dart'; import 'compose_drafts_provider.dart'; +import 'dm_resurface.dart'; import 'inbox_item.dart'; import 'inbox_local_state_provider.dart'; import 'inbox_read_state.dart'; @@ -189,7 +191,7 @@ class ActivityPage extends HookConsumerWidget { .markUnread(groupedInboxItemIds(item)); } - void openItem(InboxItem item) { + Future openItem(InboxItem item) async { final channelId = item.item.channelId; if (channelId == null) { ScaffoldMessenger.maybeOf(context)?.showSnackBar( @@ -197,13 +199,53 @@ class ActivityPage extends HookConsumerWidget { ); return; } - final channel = channelById[channelId]; + var channel = channelById[channelId]; + if (channel == null && + myPk != null && + ref.read(channelsProvider.notifier).hiddenDmIds.contains(channelId)) { + final expectedPubkey = myPk.toLowerCase(); + final expectedRelayUrl = ref.read(relayConfigProvider).baseUrl; + bool isCurrentScope() => + context.mounted && + ref.read(myPubkeyProvider)?.toLowerCase() == expectedPubkey && + ref.read(relayConfigProvider).baseUrl == expectedRelayUrl; + try { + final members = await ref.read( + channelMembersProvider(channelId).future, + ); + if (!isCurrentScope()) return; + final peers = dmPeerPubkeysFromMembers( + members.map((member) => member.pubkey), + expectedPubkey, + ); + if (peers.isEmpty) { + throw StateError('Could not determine the DM membership.'); + } + final reopened = await ref + .read(channelActionsProvider) + .openDm(pubkeys: peers.toList()); + if (!isCurrentScope()) return; + if (reopened.id != channelId) { + throw StateError('Relay reopened a different DM conversation.'); + } + channel = reopened; + } catch (error) { + if (!isCurrentScope()) return; + if (!context.mounted) return; + ScaffoldMessenger.maybeOf(context)?.showSnackBar( + SnackBar(content: Text('Could not reopen conversation: $error')), + ); + return; + } + } if (channel == null) { + if (!context.mounted) return; ScaffoldMessenger.maybeOf(context)?.showSnackBar( const SnackBar(content: Text('Channel not found in this workspace.')), ); return; } + final resolvedChannel = channel; // Deep-link to the represented message: oldest unread in the group, // falling back to the latest event. @@ -214,10 +256,11 @@ class ActivityPage extends HookConsumerWidget { ? null : thread.parentId; + if (!context.mounted) return; Navigator.of(context).push( MaterialPageRoute( builder: (_) => ChannelDetailPage( - channel: channel, + channel: resolvedChannel, initialMessageId: target.id, initialThreadRootId: threadRootId, initialThreadRouteBehavior: @@ -356,7 +399,7 @@ class ActivityPage extends HookConsumerWidget { channel: channel, currentPubkey: myPk, isDone: isDone(item), - onTap: () => openItem(item), + onTap: () => unawaited(openItem(item)), onMarkRead: () => markItemRead(item), onMarkUnread: () => markItemUnread(item), ), diff --git a/mobile/lib/features/activity/activity_provider.dart b/mobile/lib/features/activity/activity_provider.dart index a1badf20453..e0a0f9bc843 100644 --- a/mobile/lib/features/activity/activity_provider.dart +++ b/mobile/lib/features/activity/activity_provider.dart @@ -5,10 +5,23 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import '../../shared/relay/relay.dart'; import '../channels/channel.dart'; +import '../channels/channel_management_provider.dart'; import '../channels/channels_provider.dart'; +import 'dm_resurface.dart'; import 'feed_item.dart'; import 'inbox_item.dart'; +typedef DmResurfaceAction = Future Function(List pubkeys); + +final dmResurfaceActionProvider = Provider( + (ref) => (pubkeys) async { + final channel = await ref + .read(channelActionsProvider) + .openDm(pubkeys: pubkeys); + return channel.id; + }, +); + /// Builds the Activity inbox feed over the relay websocket. /// /// Sources mirror desktop's Home inbox (`useHomeFeedQuery` + `get_feed`): @@ -43,6 +56,9 @@ class ActivityNotifier extends AsyncNotifier { int? _refreshGeneration; bool _refreshQueued = false; int _subscriptionGeneration = 0; + final Set _handledDmResurfaceEventIds = {}; + final Set _pendingDmResurfaceChannelIds = {}; + String? _dmResurfaceScope; @override Future build() async { @@ -54,6 +70,14 @@ class ActivityNotifier extends AsyncNotifier { ref.watch(channelsProvider.select(_dmChannelKey)); final generation = ++_subscriptionGeneration; + final currentPubkey = ref.read(myPubkeyProvider)?.toLowerCase(); + final currentScope = + '${ref.read(relayConfigProvider).baseUrl}\u0000$currentPubkey'; + if (_dmResurfaceScope != currentScope) { + _dmResurfaceScope = currentScope; + _handledDmResurfaceEventIds.clear(); + _pendingDmResurfaceChannelIds.clear(); + } _clearLiveSubscriptions(); ref.onDispose(() { _subscriptionGeneration += 1; @@ -84,7 +108,7 @@ class ActivityNotifier extends AsyncNotifier { since: since, limit: 100, ), - (_) => _scheduleLiveRefresh(generation), + (event) => _handleAddressedLiveEvent(event, generation), ); if (generation != _subscriptionGeneration) { unsubscribeAddressed(); @@ -120,6 +144,82 @@ class ActivityNotifier extends AsyncNotifier { } } + void _handleAddressedLiveEvent(NostrEvent event, int generation) { + _scheduleLiveRefresh(generation); + final myPk = ref.read(myPubkeyProvider); + if (myPk == null || !isIncomingDmMessageEvent(event, myPk)) { + return; + } + if (!ref.read(channelsProvider.notifier).hasLoaded) { + unawaited( + _resurfaceAfterChannelDiscovery(event, myPk, _dmResurfaceScope), + ); + return; + } + unawaited(_resurfaceHiddenDm(event, myPk, generation)); + } + + Future _resurfaceAfterChannelDiscovery( + NostrEvent event, + String myPk, + String? expectedScope, + ) async { + try { + await ref.read(channelsProvider.future); + } catch (error) { + if (expectedScope == _dmResurfaceScope) { + debugPrint( + '[ActivityNotifier] channel discovery failed before DM resurface: $error', + ); + } + return; + } + if (expectedScope != _dmResurfaceScope) return; + await _resurfaceHiddenDm(event, myPk, _subscriptionGeneration); + } + + Future _resurfaceHiddenDm( + NostrEvent event, + String myPk, + int generation, + ) async { + final channelId = event.channelId; + if (channelId == null || generation != _subscriptionGeneration) return; + final channelsNotifier = ref.read(channelsProvider.notifier); + if (!channelsNotifier.hasLoaded || + !channelsNotifier.hiddenDmIds.contains(channelId) || + !_handledDmResurfaceEventIds.add(event.id)) { + return; + } + if (!_pendingDmResurfaceChannelIds.add(channelId)) return; + + try { + final members = await ref.read(channelMembersProvider(channelId).future); + if (generation != _subscriptionGeneration) return; + final peers = dmPeerPubkeysFromMembers( + members.map((member) => member.pubkey), + myPk, + ); + if (peers.isEmpty) return; + final openedChannelId = await ref.read(dmResurfaceActionProvider)( + peers.toList(), + ); + if (generation != _subscriptionGeneration) return; + if (openedChannelId != channelId) { + throw StateError('Relay reopened a different DM conversation.'); + } + } catch (error) { + _handledDmResurfaceEventIds.remove(event.id); + if (generation == _subscriptionGeneration) { + debugPrint( + '[ActivityNotifier] failed to resurface hidden DM $channelId: $error', + ); + } + } finally { + _pendingDmResurfaceChannelIds.remove(channelId); + } + } + void _scheduleLiveRefresh(int generation) { if (generation != _subscriptionGeneration) return; _liveRefreshTimer?.cancel(); diff --git a/mobile/lib/features/activity/dm_resurface.dart b/mobile/lib/features/activity/dm_resurface.dart new file mode 100644 index 00000000000..ba96b4c3643 --- /dev/null +++ b/mobile/lib/features/activity/dm_resurface.dart @@ -0,0 +1,27 @@ +import '../../shared/relay/relay.dart'; + +final _hexPubkey = RegExp(r'^[0-9a-f]{64}$'); + +Set dmPeerPubkeysFromMembers( + Iterable memberPubkeys, + String currentPubkey, +) { + final self = currentPubkey.trim().toLowerCase(); + final members = memberPubkeys + .map((pubkey) => pubkey.trim().toLowerCase()) + .where(_hexPubkey.hasMatch) + .toSet(); + if (!_hexPubkey.hasMatch(self) || !members.contains(self)) return {}; + return members..remove(self); +} + +bool isIncomingDmMessageEvent(NostrEvent event, String currentPubkey) { + final self = currentPubkey.trim().toLowerCase(); + return event.channelId != null && + EventKind.channelMessageEventKinds.contains(event.kind) && + event.pubkey.toLowerCase() != self && + event.tags.any( + (tag) => + tag.length >= 2 && tag[0] == 'p' && tag[1].toLowerCase() == self, + ); +} diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 03f29c58a1b..d3bb29a0708 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -20,6 +20,7 @@ import 'unread_badge/observed_unread_event.dart'; import 'unread_badge/should_notify_for_event.dart'; part 'channel_directory.dart'; +part 'channels_provider_lifecycle.dart'; const _channelTypeOrder = {'stream': 0, 'forum': 1, 'dm': 2}; const _unreadCatchUpLimit = 1000; @@ -56,6 +57,7 @@ class ChannelsNotifier extends AsyncNotifier> { String? _memberSnapshotPubkey; Map> _memberSnapshotsByChannelId = const {}; List _directoryMetas = const []; + Set _hiddenDmIds = const {}; /// Fences directory responses to the relay and identity that requested them. late final _ChannelRefreshCoordinator _refreshCoordinator = @@ -71,6 +73,10 @@ class ChannelsNotifier extends AsyncNotifier> { Map get latestObservedByChannel => Map.unmodifiable(_latestObservedByChannel); + Set get hiddenDmIds => Set.unmodifiable(_hiddenDmIds); + + bool get hasLoaded => _hasLoaded; + Map> get observedUnreadEventsByChannel => Map>.unmodifiable({ @@ -88,6 +94,7 @@ class ChannelsNotifier extends AsyncNotifier> { _memberSnapshotPubkey = pubkey; _memberSnapshotsByChannelId = const {}; _directoryMetas = const []; + _hiddenDmIds = const {}; // Retire any in-flight directory request: its response describes the // previous relay or identity and must not reach this scope's state. _refreshCoordinator.retireInFlight(); @@ -220,6 +227,7 @@ class ChannelsNotifier extends AsyncNotifier> { ); final hiddenDmIds = await _fenced(fence, _fetchHiddenDmIds(session, myPk)); + _hiddenDmIds = Set.unmodifiable(hiddenDmIds); // Fetch the authoritative membership snapshots before filtering Huddle // backing channels. The relay-signed kind:39000 metadata identifies the // relay, not the channel creator; the owner role in kind:39002 is the @@ -980,18 +988,6 @@ class ChannelsNotifier extends AsyncNotifier> { : AsyncData(previousChannels); } } - - void _clearLiveSubscriptions() { - _subscriptionVersion++; - _desiredLiveChannelIds = const {}; - for (final unsubscribe in _unsubscribersByChannel.values) { - unsubscribe(); - } - _unsubscribersByChannel.clear(); - _subscriptionRelayBaseUrl = null; - _backstopTimer?.cancel(); - _backstopTimer = null; - } } final channelsProvider = AsyncNotifierProvider>( diff --git a/mobile/lib/features/channels/channels_provider_lifecycle.dart b/mobile/lib/features/channels/channels_provider_lifecycle.dart new file mode 100644 index 00000000000..0440470e95e --- /dev/null +++ b/mobile/lib/features/channels/channels_provider_lifecycle.dart @@ -0,0 +1,15 @@ +part of 'channels_provider.dart'; + +extension _ChannelsNotifierSubscriptionCleanup on ChannelsNotifier { + void _clearLiveSubscriptions() { + _subscriptionVersion++; + _desiredLiveChannelIds = const {}; + for (final unsubscribe in _unsubscribersByChannel.values) { + unsubscribe(); + } + _unsubscribersByChannel.clear(); + _subscriptionRelayBaseUrl = null; + _backstopTimer?.cancel(); + _backstopTimer = null; + } +} diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index 820a6d1560e..95be4ce99fb 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:buzz/features/activity/activity_provider.dart'; import 'package:buzz/features/channels/channel.dart'; +import 'package:buzz/features/channels/channel_management_provider.dart'; import 'package:buzz/features/channels/channels_provider.dart'; import 'package:buzz/shared/relay/relay.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -298,6 +299,211 @@ void main() { }, ); + test( + 'live activity resurfaces a hidden DM through the existing open action', + () async { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + const bob = + '3333333333333333333333333333333333333333333333333333333333333333'; + final session = _RecordingSessionNotifier(); + final reopened = >[]; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => _FixedChannelsNotifier( + const [], + hiddenDmIds: const {'hidden-dm'}, + ), + ), + channelMembersProvider('hidden-dm').overrideWith( + (ref) async => [ + ChannelMember( + pubkey: self, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: alice, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: bob, + role: 'member', + joinedAt: DateTime(2026), + ), + ], + ), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopened.add(pubkeys); + return 'hidden-dm'; + }), + ], + ); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + + session.emit( + const NostrEvent( + id: 'hidden-dm-message', + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: [ + ['p', self], + ['h', 'hidden-dm'], + ], + content: 'Hello again', + sig: '', + ), + ); + + await _waitFor(() => reopened.isNotEmpty); + expect(reopened, [ + [alice, bob], + ]); + }, + ); + + test( + 'queues a hidden DM message until channel discovery completes', + () async { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + final session = _RecordingSessionNotifier(); + late _DeferredChannelsNotifier channels; + final reopened = >[]; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => channels = _DeferredChannelsNotifier( + hiddenDmIds: const {'hidden-dm'}, + ), + ), + channelMembersProvider('hidden-dm').overrideWith( + (ref) async => [ + ChannelMember( + pubkey: self, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: alice, + role: 'member', + joinedAt: DateTime(2026), + ), + ], + ), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopened.add(pubkeys); + return 'hidden-dm'; + }), + ], + ); + addTearDown(container.dispose); + + container.read(channelsProvider); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + session.emit( + const NostrEvent( + id: 'hidden-dm-during-discovery', + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: [ + ['p', self], + ['h', 'hidden-dm'], + ], + content: 'Hello during startup', + sig: '', + ), + ); + expect(reopened, isEmpty); + + channels.complete(const []); + await container.read(channelsProvider.future); + await _waitFor(() => reopened.isNotEmpty); + expect(reopened, [ + [alice], + ]); + }, + ); + + test('a suspended membership read cannot mutate a rebuilt scope', () async { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + final session = _RecordingSessionNotifier(); + final membersRequested = Completer(); + final membersGate = Completer>(); + var reopenCount = 0; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => _FixedChannelsNotifier( + const [], + hiddenDmIds: const {'hidden-dm'}, + ), + ), + channelMembersProvider('hidden-dm').overrideWith((ref) { + if (!membersRequested.isCompleted) membersRequested.complete(); + return membersGate.future; + }), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopenCount += 1; + return 'hidden-dm'; + }), + ], + ); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + session.emit( + const NostrEvent( + id: 'hidden-dm-stale-message', + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: [ + ['p', self], + ['h', 'hidden-dm'], + ], + content: 'Hello again', + sig: '', + ), + ); + await membersRequested.future; + container.invalidate(activityProvider); + await container.read(activityProvider.future); + membersGate.complete([ + ChannelMember(pubkey: self, role: 'member', joinedAt: DateTime(2026)), + ChannelMember(pubkey: alice, role: 'member', joinedAt: DateTime(2026)), + ]); + await Future.delayed(const Duration(milliseconds: 20)); + expect(reopenCount, 0); + }); + test( 'serializes live refreshes and catches up events queued mid-fetch', () async { @@ -409,9 +615,40 @@ void main() { class _FixedChannelsNotifier extends ChannelsNotifier { final List channels; + final Set _hiddenDmIds; + + _FixedChannelsNotifier(this.channels, {Set hiddenDmIds = const {}}) + : _hiddenDmIds = hiddenDmIds; - _FixedChannelsNotifier(this.channels); + @override + bool get hasLoaded => true; + + @override + Set get hiddenDmIds => _hiddenDmIds; @override Future> build() async => channels; } + +class _DeferredChannelsNotifier extends ChannelsNotifier { + _DeferredChannelsNotifier({required Set hiddenDmIds}) + : _hiddenDmIds = hiddenDmIds; + + final Set _hiddenDmIds; + final _gate = Completer>(); + bool _hasLoaded = false; + + @override + bool get hasLoaded => _hasLoaded; + + @override + Set get hiddenDmIds => _hiddenDmIds; + + @override + Future> build() => _gate.future; + + void complete(List channels) { + _hasLoaded = true; + _gate.complete(channels); + } +} diff --git a/mobile/test/features/activity/dm_resurface_test.dart b/mobile/test/features/activity/dm_resurface_test.dart new file mode 100644 index 00000000000..9086084c9c9 --- /dev/null +++ b/mobile/test/features/activity/dm_resurface_test.dart @@ -0,0 +1,40 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:buzz/features/activity/dm_resurface.dart'; +import 'package:buzz/shared/relay/relay.dart'; + +void main() { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + const bob = + '3333333333333333333333333333333333333333333333333333333333333333'; + + test('derives group DM peers from authoritative membership', () { + expect(dmPeerPubkeysFromMembers([self, alice, bob], self), {alice, bob}); + expect(dmPeerPubkeysFromMembers([alice, bob], self), isEmpty); + }); + + test('accepts only external addressed human-message events', () { + NostrEvent event({int kind = EventKind.streamMessage, String? author}) => + NostrEvent( + id: 'event-1', + pubkey: author ?? alice, + createdAt: 1, + kind: kind, + tags: const [ + ['h', 'dm-1'], + ['p', self], + ], + content: 'hello', + sig: 'sig', + ); + + expect(isIncomingDmMessageEvent(event(), self), isTrue); + expect( + isIncomingDmMessageEvent(event(kind: EventKind.reaction), self), + isFalse, + ); + expect(isIncomingDmMessageEvent(event(author: self), self), isFalse); + }); +} From 2607cb86b8a7ebedfda0a452eaa47d33b1508ce8 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 26 Aug 2026 13:47:38 -0400 Subject: [PATCH 2/7] fix(client): make hidden-DM resurface actually fire on new activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resurface trigger subscribed globally (`#p`), but the relay's fan_out_scoped only routes channel-scoped events (all kind:9 DMs carry a channel_id) to channel-scoped subscriptions, so the sub received zero events and hidden DM rows never reappeared. Subscribe channel-scoped over the hidden-DM id set (`#h`) instead, re-registering whenever the set changes. Hiding a DM never drops membership, so relay `#h` authorization holds. Because every delivered event is already for a hidden DM the reader belongs to, eligibility no longer needs the self `p` tag — untagged CLI/agent DMs resurface too — and no per-event visibility fetch is required. Also coalesce resurface attempts per channel so a concurrent follower that arrives while a reopen is in flight triggers a retry on failure instead of being marked handled and dropped; add the cheap synchronous DM/self gate before any fetch, the -5s skew buffer on the desktop live sub, a busy state on the inbox row/context-menu "Open in channel" action while a reopen is pending, exact dm-visibility query keys, and a comment explaining the mobile lifecycle part-file exists only for the file-size ratchet. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../features/channels/dmResurface.test.mjs | 59 +++++--- desktop/src/features/channels/dmResurface.ts | 45 ++---- .../channels/hiddenDmResurfaceAction.test.mjs | 26 +++- .../channels/hiddenDmResurfaceAction.ts | 15 +- desktop/src/features/channels/hooks.ts | 36 +++-- .../channels/useDmResurfaceFromMessages.ts | 109 ++++++++++---- .../src/features/channels/useHiddenDmIds.ts | 14 +- desktop/src/features/home/ui/HomeView.tsx | 2 + .../src/features/home/ui/InboxListPane.tsx | 19 ++- .../home/useHiddenDmInboxNavigation.ts | 5 + .../features/activity/activity_provider.dart | 141 ++++++++++++------ .../lib/features/activity/dm_resurface.dart | 12 +- .../channels/channels_provider_lifecycle.dart | 3 + .../activity/activity_provider_test.dart | 80 ++++++++++ .../features/activity/dm_resurface_test.dart | 49 ++++-- 15 files changed, 428 insertions(+), 187 deletions(-) diff --git a/desktop/src/features/channels/dmResurface.test.mjs b/desktop/src/features/channels/dmResurface.test.mjs index 324e5a005ec..81e09fa89ea 100644 --- a/desktop/src/features/channels/dmResurface.test.mjs +++ b/desktop/src/features/channels/dmResurface.test.mjs @@ -3,8 +3,7 @@ import test from "node:test"; import { dmPeerPubkeysFromMembers, - isIncomingDmMessageFeedItem, - isIncomingDmMessageRelayEvent, + isIncomingChannelMessageFromOther, markHiddenDmFeedItems, } from "./dmResurface.ts"; @@ -31,23 +30,52 @@ function item(overrides = {}) { }; } +function relayEvent(overrides = {}) { + return { + id: "event-1", + kind: 40002, + pubkey: ALICE, + content: "hello", + created_at: 10, + tags: [ + ["h", "dm-1"], + ["p", SELF], + ["p", BOB], + ], + sig: "", + ...overrides, + }; +} + test("DM resurface derives peers from authoritative membership", () => { const members = [{ pubkey: SELF }, { pubkey: ALICE }, { pubkey: BOB }]; assert.deepEqual(dmPeerPubkeysFromMembers(members, SELF), [ALICE, BOB]); assert.deepEqual(dmPeerPubkeysFromMembers([{ pubkey: ALICE }], SELF), []); }); -test("only external addressed human messages qualify", () => { - assert.equal(isIncomingDmMessageFeedItem(item(), SELF), true); - assert.equal(isIncomingDmMessageFeedItem(item({ kind: 7 }), SELF), false); +test("only external channel messages qualify, regardless of p tags", () => { + // #h-scoped delivery already guarantees relevance, so eligibility no longer + // requires a self `p` tag — an untagged DM from another sender still counts. + assert.equal(isIncomingChannelMessageFromOther(relayEvent(), SELF), true); assert.equal( - isIncomingDmMessageFeedItem(item({ pubkey: SELF }), SELF), + isIncomingChannelMessageFromOther(relayEvent({ kind: 7 }), SELF), false, ); assert.equal( - isIncomingDmMessageFeedItem(item({ tags: [["h", "dm-1"]] }), SELF), + isIncomingChannelMessageFromOther(relayEvent({ pubkey: SELF }), SELF), false, ); + assert.equal( + isIncomingChannelMessageFromOther(relayEvent({ tags: [] }), SELF), + false, + ); + assert.equal( + isIncomingChannelMessageFromOther( + relayEvent({ tags: [["h", "dm-1"]] }), + SELF, + ), + true, + ); }); test("hidden feed items are projected as DMs for Inbox presentation", () => { @@ -63,20 +91,3 @@ test("hidden feed items are projected as DMs for Inbox presentation", () => { const marked = markHiddenDmFeedItems(feed, new Set(["dm-1"])); assert.equal(marked.feed.mentions[0].channelType, "dm"); }); - -test("relay events use the same eligibility contract", () => { - const relayEvent = { - id: "event-1", - kind: 40002, - pubkey: ALICE, - content: "hello", - created_at: 10, - tags: [ - ["h", "dm-1"], - ["p", SELF], - ["p", BOB], - ], - sig: "", - }; - assert.equal(isIncomingDmMessageRelayEvent(relayEvent, SELF), true); -}); diff --git a/desktop/src/features/channels/dmResurface.ts b/desktop/src/features/channels/dmResurface.ts index 5e4bf6a2f54..48e0c2e24c4 100644 --- a/desktop/src/features/channels/dmResurface.ts +++ b/desktop/src/features/channels/dmResurface.ts @@ -22,43 +22,20 @@ export function dmPeerPubkeysFromMembers( return normalized.filter((pubkey) => pubkey !== self); } -export function isIncomingDmMessageFeedItem( - item: FeedItem, - currentPubkey: string | undefined, -): boolean { - const self = normalizePubkey(currentPubkey ?? ""); - if ( - !item.channelId || - self.length === 0 || - !CHANNEL_MESSAGE_KINDS.has(item.kind) || - normalizePubkey(item.pubkey) === self - ) { - return false; - } - - return item.tags.some( - (tag) => tag[0] === "p" && normalizePubkey(tag[1] ?? "") === self, - ); -} - -export function isIncomingDmMessageRelayEvent( +// The resurface subscription is `#h`-scoped to the hidden-DM set, so the relay +// only delivers events already addressed to a hidden channel the reader belongs +// to. Eligibility therefore drops the `#p` requirement — an untagged DM (a CLI +// or agent send that omits participant `p` tags) still resurfaces the row. +export function isIncomingChannelMessageFromOther( event: RelayEvent, currentPubkey: string | undefined, ): boolean { - return isIncomingDmMessageFeedItem( - { - id: event.id, - kind: event.kind, - pubkey: event.pubkey, - content: event.content, - createdAt: event.created_at, - channelId: - event.tags.find((tag) => tag[0] === "h" && tag[1])?.[1] ?? null, - channelName: "", - tags: event.tags, - category: "mention", - }, - currentPubkey, + const self = normalizePubkey(currentPubkey ?? ""); + return ( + self.length > 0 && + CHANNEL_MESSAGE_KINDS.has(event.kind) && + relayEventChannelId(event) !== null && + normalizePubkey(event.pubkey) !== self ); } diff --git a/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs b/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs index 640ef8a1f9c..78d8f0da992 100644 --- a/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs +++ b/desktop/src/features/channels/hiddenDmResurfaceAction.test.mjs @@ -40,7 +40,7 @@ test("reopens the source hidden group DM from authoritative membership", async ( event: event(), expectedRelayUrl: "wss://relay.example", expectedSignerPubkey: SELF, - fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + hiddenDmIds: new Set(["hidden-dm"]), fetchMembers: async () => [member(SELF), member(ALICE), member(BOB)], isCurrent: () => true, reopen: async (input) => { @@ -59,6 +59,26 @@ test("reopens the source hidden group DM from authoritative membership", async ( ]); }); +test("ignores an event for a channel outside the hidden set", async () => { + let reopenCount = 0; + assert.equal( + await resurfaceHiddenDmMessage({ + event: event(), + expectedRelayUrl: "wss://relay.example", + expectedSignerPubkey: SELF, + hiddenDmIds: new Set(["other-dm"]), + fetchMembers: async () => [member(SELF), member(ALICE)], + isCurrent: () => true, + reopen: async () => { + reopenCount += 1; + return { id: "hidden-dm" }; + }, + }), + false, + ); + assert.equal(reopenCount, 0); +}); + test("a suspended old-community read cannot reopen a DM", async () => { let current = true; let resume; @@ -70,7 +90,7 @@ test("a suspended old-community read cannot reopen a DM", async () => { event: event(), expectedRelayUrl: "wss://old.example", expectedSignerPubkey: SELF, - fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + hiddenDmIds: new Set(["hidden-dm"]), fetchMembers: async () => members, isCurrent: () => current, reopen: async () => { @@ -91,7 +111,7 @@ test("rejects a reopen result for any channel other than the source", async () = event: event(), expectedRelayUrl: "wss://relay.example", expectedSignerPubkey: SELF, - fetchHiddenDmIds: async () => new Set(["hidden-dm"]), + hiddenDmIds: new Set(["hidden-dm"]), fetchMembers: async () => [member(SELF), member(ALICE)], isCurrent: () => true, reopen: async () => ({ id: "alternate-dm" }), diff --git a/desktop/src/features/channels/hiddenDmResurfaceAction.ts b/desktop/src/features/channels/hiddenDmResurfaceAction.ts index c740a3f2d29..78ee087475f 100644 --- a/desktop/src/features/channels/hiddenDmResurfaceAction.ts +++ b/desktop/src/features/channels/hiddenDmResurfaceAction.ts @@ -2,7 +2,7 @@ import type { ChannelMember, RelayEvent } from "@/shared/api/types"; import type { OpenDmInput } from "@/shared/api/tauriChannels"; import { dmPeerPubkeysFromMembers, - isIncomingDmMessageRelayEvent, + isIncomingChannelMessageFromOther, relayEventChannelId, } from "./dmResurface"; @@ -10,7 +10,7 @@ type HiddenDmResurfaceActionOptions = { event: RelayEvent; expectedRelayUrl: string; expectedSignerPubkey: string; - fetchHiddenDmIds: () => Promise>; + hiddenDmIds: ReadonlySet; fetchMembers: (channelId: string) => Promise; isCurrent: () => boolean; reopen: (input: OpenDmInput) => Promise<{ id: string }>; @@ -20,17 +20,16 @@ export async function resurfaceHiddenDmMessage({ event, expectedRelayUrl, expectedSignerPubkey, - fetchHiddenDmIds, + hiddenDmIds, fetchMembers, isCurrent, reopen, }: HiddenDmResurfaceActionOptions): Promise { - if (!isIncomingDmMessageRelayEvent(event, expectedSignerPubkey)) return false; + if (!isIncomingChannelMessageFromOther(event, expectedSignerPubkey)) { + return false; + } const channelId = relayEventChannelId(event); - if (!channelId) return false; - - const hiddenDmIds = await fetchHiddenDmIds(); - if (!isCurrent() || !hiddenDmIds.has(channelId)) return false; + if (!channelId || !hiddenDmIds.has(channelId)) return false; const members = await fetchMembers(channelId); if (!isCurrent()) return false; diff --git a/desktop/src/features/channels/hooks.ts b/desktop/src/features/channels/hooks.ts index 87035fc15ea..18eb2699d2e 100644 --- a/desktop/src/features/channels/hooks.ts +++ b/desktop/src/features/channels/hooks.ts @@ -53,7 +53,7 @@ import { CHANNEL_MEMBERS_STALE_TIME_MS, channelMembersQueryKey, } from "@/features/channels/rosterFreshness"; -import { dmVisibilityQueryKey } from "@/features/channels/useHiddenDmIds"; +import { dmVisibilityQueryKeyFor } from "@/features/channels/useHiddenDmIds"; export const channelsQueryKey = ["channels"] as const; /** Keeps focused polling at the established one-minute cadence. */ @@ -531,6 +531,12 @@ export function useCreateChannelMutation() { export function useOpenDmMutation() { const queryClient = useQueryClient(); + const { activeCommunity } = useCommunities(); + const identityQuery = useIdentityQuery(); + const dmVisibilityKey = dmVisibilityQueryKeyFor( + activeCommunity?.relayUrl, + identityQuery.data?.pubkey, + ); return useMutation({ mutationFn: (input: OpenDmInput) => openDm(input), @@ -538,14 +544,11 @@ export function useOpenDmMutation() { queryClient.setQueryData(channelsQueryKey, (current) => upsertCachedChannel(current, openedChannel), ); - queryClient.setQueriesData>( - { queryKey: dmVisibilityQueryKey }, - (current) => { - const next = new Set(current); - next.delete(openedChannel.id); - return next; - }, - ); + queryClient.setQueryData>(dmVisibilityKey, (current) => { + const next = new Set(current); + next.delete(openedChannel.id); + return next; + }); }, onSettled: () => { // The relay-returned DM is already in the cache. Mark the list stale so @@ -555,7 +558,7 @@ export function useOpenDmMutation() { queryKey: channelsQueryKey, refetchType: "none", }); - void queryClient.invalidateQueries({ queryKey: dmVisibilityQueryKey }); + void queryClient.invalidateQueries({ queryKey: dmVisibilityKey }); }, }); } @@ -585,6 +588,12 @@ export function useUpsertCachedChannel() { export function useHideDmMutation() { const queryClient = useQueryClient(); + const { activeCommunity } = useCommunities(); + const identityQuery = useIdentityQuery(); + const dmVisibilityKey = dmVisibilityQueryKeyFor( + activeCommunity?.relayUrl, + identityQuery.data?.pubkey, + ); return useMutation({ mutationFn: (channelId: string) => hideDm(channelId), @@ -602,15 +611,14 @@ export function useHideDmMutation() { } }, onSuccess: (_data, channelId) => { - queryClient.setQueriesData>( - { queryKey: dmVisibilityQueryKey }, - (current) => new Set(current).add(channelId), + queryClient.setQueryData>(dmVisibilityKey, (current) => + new Set(current).add(channelId), ); }, onSettled: async () => { await Promise.all([ queryClient.invalidateQueries({ queryKey: channelsQueryKey }), - queryClient.invalidateQueries({ queryKey: dmVisibilityQueryKey }), + queryClient.invalidateQueries({ queryKey: dmVisibilityKey }), ]); }, }); diff --git a/desktop/src/features/channels/useDmResurfaceFromMessages.ts b/desktop/src/features/channels/useDmResurfaceFromMessages.ts index b0a7c0a6b79..d925073b84e 100644 --- a/desktop/src/features/channels/useDmResurfaceFromMessages.ts +++ b/desktop/src/features/channels/useDmResurfaceFromMessages.ts @@ -9,7 +9,7 @@ import { import { CHANNEL_MESSAGE_EVENT_KINDS } from "@/shared/constants/kinds"; import { relayEventChannelId } from "./dmResurface"; import { resurfaceHiddenDmMessage } from "./hiddenDmResurfaceAction"; -import { fetchHiddenDmIds } from "./useHiddenDmIds"; +import { useHiddenDmIds } from "./useHiddenDmIds"; type UseDmResurfaceFromMessagesOptions = { pubkey: string | undefined; @@ -17,62 +17,111 @@ type UseDmResurfaceFromMessagesOptions = { reopen: (input: OpenDmInput) => Promise<{ id: string }>; }; +/** + * Resurfaces a hidden DM row the moment new activity lands in it. + * + * The subscription is `#h`-scoped to the current hidden-DM id set: channel + * messages carry a `channel_id`, and the relay only fans channel-scoped events + * to channel-scoped subscriptions (`fan_out_scoped`), so a community-global + * `#p` filter would never receive them. Scoping to `#h` also means every + * delivered event is already for a hidden DM the reader belongs to (hiding + * never drops membership), so no per-event visibility fetch is needed and + * untagged CLI/agent DMs resurface too. The subscription re-registers whenever + * the hidden set changes. + */ export function useDmResurfaceFromMessages({ pubkey, relayUrl, reopen, }: UseDmResurfaceFromMessagesOptions) { - const handledEventIdsRef = React.useRef(new Set()); - const pendingChannelIdsRef = React.useRef(new Set()); + const hiddenDmIds = useHiddenDmIds(pubkey); + // Coalesce per channel: the reopen action is idempotent, so concurrent + // messages for the same DM share one in-flight attempt. `retry` records that + // a follower event arrived while the attempt was in flight, so a failed + // reopen re-runs instead of silently dropping that follower. + const pendingChannelsRef = React.useRef( + new Map(), + ); const generationRef = React.useRef(0); const reopenLatest = React.useEffectEvent(reopen); + // Stable dependency for the hidden-set membership, order-independent. + const hiddenDmKey = React.useMemo( + () => [...hiddenDmIds].sort().join(","), + [hiddenDmIds], + ); + React.useEffect(() => { const expectedSignerPubkey = pubkey?.trim().toLowerCase() ?? ""; const expectedRelayUrl = relayUrl?.trim() ?? ""; + const channelIds = hiddenDmKey.length > 0 ? hiddenDmKey.split(",") : []; const generation = ++generationRef.current; - handledEventIdsRef.current.clear(); - pendingChannelIdsRef.current.clear(); - if (!expectedSignerPubkey || !expectedRelayUrl) return; + pendingChannelsRef.current.clear(); + if (!expectedSignerPubkey || !expectedRelayUrl || channelIds.length === 0) { + return; + } + const hiddenDmIdSet = new Set(channelIds); let disposed = false; let unsubscribe: (() => Promise) | undefined; const isCurrent = () => !disposed && generationRef.current === generation; - const handleEvent = async (event: RelayEvent) => { - if (!isCurrent() || !handledEventIdsRef.current.add(event.id)) return; - const channelId = relayEventChannelId(event); - if (!channelId || pendingChannelIdsRef.current.has(channelId)) return; - pendingChannelIdsRef.current.add(channelId); + // Latest event seen per channel drives the in-flight/retry attempt so a + // coalesced follower reopens from a real event, not a captured stale one. + const latestEventByChannel = new Map(); + + const attempt = async (channelId: string) => { + const state = { retry: false }; + pendingChannelsRef.current.set(channelId, state); try { - await resurfaceHiddenDmMessage({ - event, - expectedRelayUrl, - expectedSignerPubkey, - fetchHiddenDmIds: () => fetchHiddenDmIds(expectedSignerPubkey), - fetchMembers: getChannelMembers, - isCurrent, - reopen: reopenLatest, - }); - } catch (error) { - handledEventIdsRef.current.delete(event.id); - if (isCurrent()) { - console.error("Failed to resurface hidden DM", channelId, error); - } + do { + state.retry = false; + const event = latestEventByChannel.get(channelId); + if (!event) return; + try { + await resurfaceHiddenDmMessage({ + event, + expectedRelayUrl, + expectedSignerPubkey, + hiddenDmIds: hiddenDmIdSet, + fetchMembers: getChannelMembers, + isCurrent, + reopen: reopenLatest, + }); + return; + } catch (error) { + if (isCurrent()) { + console.error("Failed to resurface hidden DM", channelId, error); + } + } + } while (state.retry && isCurrent()); } finally { - pendingChannelIdsRef.current.delete(channelId); + pendingChannelsRef.current.delete(channelId); + } + }; + + const handleEvent = (event: RelayEvent) => { + if (!isCurrent()) return; + const channelId = relayEventChannelId(event); + if (!channelId || !hiddenDmIdSet.has(channelId)) return; + latestEventByChannel.set(channelId, event); + const pending = pendingChannelsRef.current.get(channelId); + if (pending) { + pending.retry = true; + return; } + void attempt(channelId); }; void relayClient .subscribeLive( { kinds: [...CHANNEL_MESSAGE_EVENT_KINDS], - "#p": [expectedSignerPubkey], - since: Math.floor(Date.now() / 1_000), + "#h": channelIds, + since: Math.floor(Date.now() / 1_000) - 5, limit: 100, }, - (event) => void handleEvent(event), + handleEvent, ) .then((dispose) => { if (!isCurrent()) { @@ -92,5 +141,5 @@ export function useDmResurfaceFromMessages({ generationRef.current += 1; void unsubscribe?.().catch(() => {}); }; - }, [pubkey, relayUrl]); + }, [pubkey, relayUrl, hiddenDmKey]); } diff --git a/desktop/src/features/channels/useHiddenDmIds.ts b/desktop/src/features/channels/useHiddenDmIds.ts index 521d49666fa..865d434e45d 100644 --- a/desktop/src/features/channels/useHiddenDmIds.ts +++ b/desktop/src/features/channels/useHiddenDmIds.ts @@ -9,6 +9,18 @@ import { normalizePubkey } from "@/shared/lib/pubkey"; export const dmVisibilityQueryKey = ["dm-visibility"] as const; +/** Exact query key for one relay+identity scope's DM-visibility snapshot. */ +export function dmVisibilityQueryKeyFor( + relayUrl: string | undefined, + pubkey: string | undefined, +) { + return [ + ...dmVisibilityQueryKey, + relayUrl ?? "", + normalizePubkey(pubkey ?? ""), + ] as const; +} + export function extractHiddenDmIds(events: readonly RelayEvent[]): Set { const latest = events.reduce( (current, event) => @@ -40,7 +52,7 @@ export function useHiddenDmIds(pubkey: string | undefined) { const normalizedPubkey = normalizePubkey(pubkey ?? ""); const relayUrl = activeCommunity?.relayUrl ?? ""; const query = useQuery({ - queryKey: [...dmVisibilityQueryKey, relayUrl, normalizedPubkey], + queryKey: dmVisibilityQueryKeyFor(relayUrl, normalizedPubkey), queryFn: () => fetchHiddenDmIds(normalizedPubkey), enabled: relayUrl.length > 0 && normalizedPubkey.length > 0, staleTime: 30_000, diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 889cc61be83..41b4cebff97 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -455,6 +455,7 @@ export function HomeView({ handleOpenDirect, handleOpenDm, handleOpenSelectedContext, + isReopenPending, } = useHiddenDmInboxNavigation({ availableChannelIds, currentPubkey, @@ -702,6 +703,7 @@ export function HomeView({ onMarkRead={markItemRead} onMarkUnread={markItemUnread} onOpenDirect={handleOpenDirect} + isReopenPending={isReopenPending} onRemindLater={(item) => { const channelId = item.item.channelId; if (!channelId) { diff --git a/desktop/src/features/home/ui/InboxListPane.tsx b/desktop/src/features/home/ui/InboxListPane.tsx index f2d4421081d..6b83c0289b7 100644 --- a/desktop/src/features/home/ui/InboxListPane.tsx +++ b/desktop/src/features/home/ui/InboxListPane.tsx @@ -215,6 +215,7 @@ type InboxListPaneProps = { onMarkRead: (itemId: string) => void; onMarkUnread: (itemId: string) => void; onOpenDirect: (item: InboxItem) => void; + isReopenPending?: (channelId: string | null | undefined) => boolean; onRemindLater: (item: InboxItem) => void; onSelect: (itemId: string) => void; onSelectDraft: (draftKey: string) => void; @@ -243,6 +244,7 @@ export function InboxListPane({ onMarkRead, onMarkUnread, onOpenDirect, + isReopenPending, onRemindLater, onSelect, onSelectDraft, @@ -303,6 +305,13 @@ export function InboxListPane({ (eventId) => activeReminderEventIds?.has(eventId) ?? false, ); const hasChannelTarget = Boolean(item.item.channelId); + const isReopening = isReopenPending?.(item.item.channelId) ?? false; + const canOpen = hasChannelTarget && !isReopening; + const openLabel = !hasChannelTarget + ? "No channel link" + : isReopening + ? "Reopening…" + : "Open in channel"; const typeLabel = getInboxTypeLabel(item); const videoReviewCommentRootId = getInboxVideoReviewCommentRootId(item); const isSenderAgent = @@ -468,8 +477,8 @@ export function InboxListPane({ )} onOpenDirect(item)} > @@ -509,15 +518,15 @@ export function InboxListPane({ )} { - if (hasChannelTarget) { + if (canOpen) { onOpenDirect(item); } }} > - {hasChannelTarget ? "Open in channel" : "No channel link"} + {openLabel} 0 && expectedSignerPubkey.length > 0)), ), + isReopenPending: React.useCallback( + (channelId: string | null | undefined) => + Boolean(channelId && pendingChannelIdsRef.current.has(channelId)), + [], + ), handleOpenDirect: React.useCallback( (item: InboxItem) => { const channelId = item.item.channelId; diff --git a/mobile/lib/features/activity/activity_provider.dart b/mobile/lib/features/activity/activity_provider.dart index e0a0f9bc843..81c1d279dce 100644 --- a/mobile/lib/features/activity/activity_provider.dart +++ b/mobile/lib/features/activity/activity_provider.dart @@ -51,13 +51,16 @@ class ActivityNotifier extends AsyncNotifier { void Function()? _unsubscribeAddressed; void Function()? _unsubscribeDms; + void Function()? _unsubscribeHiddenDms; Timer? _liveRefreshTimer; Future? _refreshInFlight; int? _refreshGeneration; bool _refreshQueued = false; int _subscriptionGeneration = 0; - final Set _handledDmResurfaceEventIds = {}; - final Set _pendingDmResurfaceChannelIds = {}; + // Per-hidden-channel resurface coalescing. Presence of a key means an attempt + // is in flight; its value records whether a follower event arrived while it + // was running, so a failed reopen re-runs instead of dropping the follower. + final Map _pendingDmResurfaceRetry = {}; String? _dmResurfaceScope; @override @@ -75,8 +78,7 @@ class ActivityNotifier extends AsyncNotifier { '${ref.read(relayConfigProvider).baseUrl}\u0000$currentPubkey'; if (_dmResurfaceScope != currentScope) { _dmResurfaceScope = currentScope; - _handledDmResurfaceEventIds.clear(); - _pendingDmResurfaceChannelIds.clear(); + _pendingDmResurfaceRetry.clear(); } _clearLiveSubscriptions(); ref.onDispose(() { @@ -116,27 +118,51 @@ class ActivityNotifier extends AsyncNotifier { } _unsubscribeAddressed = unsubscribeAddressed; + final channels = + ref.read(channelsProvider).asData?.value ?? const []; final dmChannelIds = [ - for (final channel - in ref.read(channelsProvider).asData?.value ?? const []) + for (final channel in channels) if (channel.isDm && channel.isMember) channel.id, ]; - if (dmChannelIds.isEmpty) return; + if (dmChannelIds.isNotEmpty) { + final unsubscribeDms = await session.subscribe( + NostrFilter( + kinds: const [9], + tags: {'#h': dmChannelIds}, + since: since, + limit: 100, + ), + (_) => _scheduleLiveRefresh(generation), + ); + if (generation != _subscriptionGeneration) { + unsubscribeDms(); + return; + } + _unsubscribeDms = unsubscribeDms; + } - final unsubscribeDms = await session.subscribe( - NostrFilter( - kinds: const [9], - tags: {'#h': dmChannelIds}, - since: since, - limit: 100, - ), - (_) => _scheduleLiveRefresh(generation), - ); - if (generation != _subscriptionGeneration) { - unsubscribeDms(); - return; + // Resurface trigger: hidden DMs are dropped from the visible-DM sub above, + // so subscribe to them separately. Channel messages carry a channel_id and + // the relay only fans channel-scoped events to channel-scoped subs, so an + // `#h` filter (never `#p`, which the relay treats as global) is required. + // Hiding never drops membership, so `#h` authorization holds. + final hiddenDmIds = ref.read(channelsProvider.notifier).hiddenDmIds; + if (hiddenDmIds.isNotEmpty) { + final unsubscribeHiddenDms = await session.subscribe( + NostrFilter( + kinds: EventKind.channelMessageEventKinds, + tags: {'#h': hiddenDmIds.toList()}, + since: since, + limit: 100, + ), + (event) => _handleHiddenDmLiveEvent(event, generation), + ); + if (generation != _subscriptionGeneration) { + unsubscribeHiddenDms(); + return; + } + _unsubscribeHiddenDms = unsubscribeHiddenDms; } - _unsubscribeDms = unsubscribeDms; } catch (error) { if (generation == _subscriptionGeneration) { debugPrint('[ActivityNotifier] live subscription failed: $error'); @@ -146,10 +172,13 @@ class ActivityNotifier extends AsyncNotifier { void _handleAddressedLiveEvent(NostrEvent event, int generation) { _scheduleLiveRefresh(generation); + } + + void _handleHiddenDmLiveEvent(NostrEvent event, int generation) { + if (generation != _subscriptionGeneration) return; final myPk = ref.read(myPubkeyProvider); - if (myPk == null || !isIncomingDmMessageEvent(event, myPk)) { - return; - } + if (myPk == null || !isIncomingChannelMessageFromOther(event, myPk)) return; + _scheduleLiveRefresh(generation); if (!ref.read(channelsProvider.notifier).hasLoaded) { unawaited( _resurfaceAfterChannelDiscovery(event, myPk, _dmResurfaceScope), @@ -187,36 +216,50 @@ class ActivityNotifier extends AsyncNotifier { if (channelId == null || generation != _subscriptionGeneration) return; final channelsNotifier = ref.read(channelsProvider.notifier); if (!channelsNotifier.hasLoaded || - !channelsNotifier.hiddenDmIds.contains(channelId) || - !_handledDmResurfaceEventIds.add(event.id)) { + !channelsNotifier.hiddenDmIds.contains(channelId)) { + return; + } + // Coalesce per channel: a concurrent follower for the same DM marks the + // in-flight attempt for retry rather than being dropped, so a failed reopen + // re-runs instead of leaving the row hidden. + if (_pendingDmResurfaceRetry.containsKey(channelId)) { + _pendingDmResurfaceRetry[channelId] = true; return; } - if (!_pendingDmResurfaceChannelIds.add(channelId)) return; + _pendingDmResurfaceRetry[channelId] = false; try { - final members = await ref.read(channelMembersProvider(channelId).future); - if (generation != _subscriptionGeneration) return; - final peers = dmPeerPubkeysFromMembers( - members.map((member) => member.pubkey), - myPk, - ); - if (peers.isEmpty) return; - final openedChannelId = await ref.read(dmResurfaceActionProvider)( - peers.toList(), - ); - if (generation != _subscriptionGeneration) return; - if (openedChannelId != channelId) { - throw StateError('Relay reopened a different DM conversation.'); - } - } catch (error) { - _handledDmResurfaceEventIds.remove(event.id); - if (generation == _subscriptionGeneration) { - debugPrint( - '[ActivityNotifier] failed to resurface hidden DM $channelId: $error', - ); - } + do { + _pendingDmResurfaceRetry[channelId] = false; + try { + final members = await ref.read( + channelMembersProvider(channelId).future, + ); + if (generation != _subscriptionGeneration) return; + final peers = dmPeerPubkeysFromMembers( + members.map((member) => member.pubkey), + myPk, + ); + if (peers.isEmpty) return; + final openedChannelId = await ref.read(dmResurfaceActionProvider)( + peers.toList(), + ); + if (generation != _subscriptionGeneration) return; + if (openedChannelId != channelId) { + throw StateError('Relay reopened a different DM conversation.'); + } + return; + } catch (error) { + if (generation == _subscriptionGeneration) { + debugPrint( + '[ActivityNotifier] failed to resurface hidden DM $channelId: $error', + ); + } + } + } while ((_pendingDmResurfaceRetry[channelId] ?? false) && + generation == _subscriptionGeneration); } finally { - _pendingDmResurfaceChannelIds.remove(channelId); + _pendingDmResurfaceRetry.remove(channelId); } } @@ -277,6 +320,8 @@ class ActivityNotifier extends AsyncNotifier { _unsubscribeAddressed = null; _unsubscribeDms?.call(); _unsubscribeDms = null; + _unsubscribeHiddenDms?.call(); + _unsubscribeHiddenDms = null; } /// Stable identity for the joined DM channel set: null while channels are diff --git a/mobile/lib/features/activity/dm_resurface.dart b/mobile/lib/features/activity/dm_resurface.dart index ba96b4c3643..3c09026948b 100644 --- a/mobile/lib/features/activity/dm_resurface.dart +++ b/mobile/lib/features/activity/dm_resurface.dart @@ -15,13 +15,13 @@ Set dmPeerPubkeysFromMembers( return members..remove(self); } -bool isIncomingDmMessageEvent(NostrEvent event, String currentPubkey) { +// The resurface subscription is `#h`-scoped to the hidden-DM set, so the relay +// only delivers events already addressed to a hidden channel the reader belongs +// to. Eligibility therefore drops the `#p` requirement — an untagged DM (a CLI +// or agent send that omits participant `p` tags) still resurfaces the row. +bool isIncomingChannelMessageFromOther(NostrEvent event, String currentPubkey) { final self = currentPubkey.trim().toLowerCase(); return event.channelId != null && EventKind.channelMessageEventKinds.contains(event.kind) && - event.pubkey.toLowerCase() != self && - event.tags.any( - (tag) => - tag.length >= 2 && tag[0] == 'p' && tag[1].toLowerCase() == self, - ); + event.pubkey.toLowerCase() != self; } diff --git a/mobile/lib/features/channels/channels_provider_lifecycle.dart b/mobile/lib/features/channels/channels_provider_lifecycle.dart index 0440470e95e..95cb0889ced 100644 --- a/mobile/lib/features/channels/channels_provider_lifecycle.dart +++ b/mobile/lib/features/channels/channels_provider_lifecycle.dart @@ -1,5 +1,8 @@ part of 'channels_provider.dart'; +// This extension exists only to keep `channels_provider.dart` under the +// desktop/mobile file-size ratchet; `_clearLiveSubscriptions` is not a +// standalone semantic boundary and belongs to `ChannelsNotifier`. extension _ChannelsNotifierSubscriptionCleanup on ChannelsNotifier { void _clearLiveSubscriptions() { _subscriptionVersion++; diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index 95be4ce99fb..6db51a6115d 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -444,6 +444,86 @@ void main() { }, ); + test('a concurrent follower survives a failed in-flight reopen', () async { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + final session = _RecordingSessionNotifier(); + final reopenAttempts = >[]; + final firstReopenGate = Completer(); + var reopenCalls = 0; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => _FixedChannelsNotifier( + const [], + hiddenDmIds: const {'hidden-dm'}, + ), + ), + channelMembersProvider('hidden-dm').overrideWith( + (ref) async => [ + ChannelMember( + pubkey: self, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: alice, + role: 'member', + joinedAt: DateTime(2026), + ), + ], + ), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopenCalls += 1; + reopenAttempts.add(pubkeys); + if (reopenCalls == 1) { + // Hold the first attempt open so the follower coalesces into it, + // then fail it — the follower must not be silently dropped. + await firstReopenGate.future; + throw StateError('transient reopen failure'); + } + return 'hidden-dm'; + }), + ], + ); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + + NostrEvent hiddenDmEvent(String id) => NostrEvent( + id: id, + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: const [ + ['p', self], + ['h', 'hidden-dm'], + ], + content: 'Hello again', + sig: '', + ); + + session.emit(hiddenDmEvent('message-a')); + await _waitFor(() => reopenCalls == 1); + // Follower arrives while attempt A is in flight → coalesced for retry. + session.emit(hiddenDmEvent('message-b')); + await Future.delayed(const Duration(milliseconds: 10)); + firstReopenGate.complete(); + + await _waitFor(() => reopenCalls >= 2); + expect(reopenAttempts, [ + [alice], + [alice], + ]); + }); + test('a suspended membership read cannot mutate a rebuilt scope', () async { const self = '1111111111111111111111111111111111111111111111111111111111111111'; diff --git a/mobile/test/features/activity/dm_resurface_test.dart b/mobile/test/features/activity/dm_resurface_test.dart index 9086084c9c9..b06084e186d 100644 --- a/mobile/test/features/activity/dm_resurface_test.dart +++ b/mobile/test/features/activity/dm_resurface_test.dart @@ -15,26 +15,47 @@ void main() { expect(dmPeerPubkeysFromMembers([alice, bob], self), isEmpty); }); - test('accepts only external addressed human-message events', () { - NostrEvent event({int kind = EventKind.streamMessage, String? author}) => - NostrEvent( - id: 'event-1', - pubkey: author ?? alice, - createdAt: 1, - kind: kind, - tags: const [ + test('accepts external channel messages regardless of p tags', () { + NostrEvent event({ + int kind = EventKind.streamMessage, + String? author, + List>? tags, + }) => NostrEvent( + id: 'event-1', + pubkey: author ?? alice, + createdAt: 1, + kind: kind, + tags: + tags ?? + const [ ['h', 'dm-1'], ['p', self], ], - content: 'hello', - sig: 'sig', - ); + content: 'hello', + sig: 'sig', + ); - expect(isIncomingDmMessageEvent(event(), self), isTrue); + expect(isIncomingChannelMessageFromOther(event(), self), isTrue); + expect( + isIncomingChannelMessageFromOther(event(kind: EventKind.reaction), self), + isFalse, + ); expect( - isIncomingDmMessageEvent(event(kind: EventKind.reaction), self), + isIncomingChannelMessageFromOther(event(author: self), self), isFalse, ); - expect(isIncomingDmMessageEvent(event(author: self), self), isFalse); + // #h-scoped delivery already guarantees relevance: an untagged DM from + // another sender still qualifies. + expect( + isIncomingChannelMessageFromOther( + event( + tags: const [ + ['h', 'dm-1'], + ], + ), + self, + ), + isTrue, + ); }); } From 9f78c019d40ad75c505d812b866a5bf37e344fcd Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 26 Aug 2026 14:24:08 -0400 Subject: [PATCH 3/7] fix(dm-resurface): give resurface coalescing per-generation ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A subscription rebuild bumps the generation but the pending resurface map survived, so a follower on the new generation coalesced into a suspended old-generation attempt whose finally deleted the shared key — the follower never reopened, permanently dropping the hidden DM on mobile. Each pending entry now owns its generation: a follower only coalesces into a current-generation attempt, and cleanup removes an entry only when the map still holds that exact one. Desktop extracts a per-generation coordinator with a private map so a torn-down generation cannot stomp the replacement's entries, which also makes the coordinator seam unit-testable. Adds a desktop coordinator test and a mobile rebuild regression (A suspended -> rebuild -> B on the replacement subscription -> A retires -> B reopens). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../hiddenDmResurfaceCoordinator.test.mjs | 92 +++++++++++++++++++ .../channels/hiddenDmResurfaceCoordinator.ts | 60 ++++++++++++ .../channels/useDmResurfaceFromMessages.ts | 71 +++++--------- .../features/activity/activity_provider.dart | 48 +++++++--- .../activity/activity_provider_test.dart | 89 ++++++++++++++++++ 5 files changed, 299 insertions(+), 61 deletions(-) create mode 100644 desktop/src/features/channels/hiddenDmResurfaceCoordinator.test.mjs create mode 100644 desktop/src/features/channels/hiddenDmResurfaceCoordinator.ts diff --git a/desktop/src/features/channels/hiddenDmResurfaceCoordinator.test.mjs b/desktop/src/features/channels/hiddenDmResurfaceCoordinator.test.mjs new file mode 100644 index 00000000000..b3ea41b4ce3 --- /dev/null +++ b/desktop/src/features/channels/hiddenDmResurfaceCoordinator.test.mjs @@ -0,0 +1,92 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { createHiddenDmResurfaceCoordinator } from "./hiddenDmResurfaceCoordinator.ts"; + +function deferred() { + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +test("a follower arriving mid-attempt retries from the latest event after a failure", async () => { + const seen = []; + const gates = [deferred(), deferred()]; + const errors = []; + const coordinator = createHiddenDmResurfaceCoordinator({ + resurface: async (event) => { + const index = seen.length; + seen.push(event.id); + await gates[index].promise; + }, + isCurrent: () => true, + onError: (channelId, error) => errors.push([channelId, error]), + }); + + coordinator.handle("dm-1", { id: "event-a" }); + await Promise.resolve(); + // Follower B lands while attempt A is still in flight. + coordinator.handle("dm-1", { id: "event-b" }); + assert.deepEqual(seen, ["event-a"]); + + // A fails; the retry re-runs from the latest event (B), which succeeds. + gates[0].reject(new Error("boom")); + await Promise.resolve(); + await Promise.resolve(); + assert.deepEqual(seen, ["event-a", "event-b"]); + + gates[1].resolve(); + await gates[1].promise; + await Promise.resolve(); + + assert.equal(errors.length, 1); + assert.equal(errors[0][0], "dm-1"); +}); + +test("a stale generation's attempt does not delete the live generation's entry", async () => { + // Model two generations: each real subscription generation creates its own + // coordinator, so a torn-down generation's cleanup touches only its own map. + let staleCurrent = true; + const staleGate = deferred(); + const staleSeen = []; + const stale = createHiddenDmResurfaceCoordinator({ + resurface: async (event) => { + staleSeen.push(event.id); + await staleGate.promise; + }, + isCurrent: () => staleCurrent, + }); + + const liveSeen = []; + const live = createHiddenDmResurfaceCoordinator({ + resurface: async (event) => { + liveSeen.push(event.id); + }, + isCurrent: () => true, + }); + + // Attempt A begins on the stale generation and suspends. + stale.handle("dm-1", { id: "event-a" }); + await Promise.resolve(); + assert.deepEqual(staleSeen, ["event-a"]); + + // Generation flips; event B is delivered to the live coordinator and reopens. + staleCurrent = false; + live.handle("dm-1", { id: "event-b" }); + await Promise.resolve(); + assert.deepEqual(liveSeen, ["event-b"]); + + // Stale A now retires. Its cleanup cannot touch the live coordinator's map, + // so a subsequent live follower still starts a fresh attempt. + staleGate.resolve(); + await staleGate.promise; + await Promise.resolve(); + + live.handle("dm-1", { id: "event-c" }); + await Promise.resolve(); + assert.deepEqual(liveSeen, ["event-b", "event-c"]); +}); diff --git a/desktop/src/features/channels/hiddenDmResurfaceCoordinator.ts b/desktop/src/features/channels/hiddenDmResurfaceCoordinator.ts new file mode 100644 index 00000000000..ec3e08150c6 --- /dev/null +++ b/desktop/src/features/channels/hiddenDmResurfaceCoordinator.ts @@ -0,0 +1,60 @@ +import type { RelayEvent } from "@/shared/api/types"; + +type CoordinatorOptions = { + resurface: (event: RelayEvent) => Promise; + isCurrent: () => boolean; + onError?: (channelId: string, error: unknown) => void; +}; + +/** + * Per-channel coalescing for hidden-DM resurface attempts. + * + * The reopen action is idempotent, so concurrent messages for the same DM + * share one in-flight attempt. A follower that lands while an attempt is + * running flags it for retry (from the latest event) instead of being + * dropped, so a failed reopen re-runs rather than leaving the row hidden. + * + * A coordinator owns its own pending map, so callers create one per + * subscription generation: an attempt from a torn-down generation can never + * delete or coalesce into an entry owned by the live one. + */ +export function createHiddenDmResurfaceCoordinator({ + resurface, + isCurrent, + onError, +}: CoordinatorOptions) { + const pending = new Map(); + const latestEventByChannel = new Map(); + + const attempt = async (channelId: string) => { + const state = { retry: false }; + pending.set(channelId, state); + try { + do { + state.retry = false; + const event = latestEventByChannel.get(channelId); + if (!event) return; + try { + await resurface(event); + return; + } catch (error) { + onError?.(channelId, error); + } + } while (state.retry && isCurrent()); + } finally { + pending.delete(channelId); + } + }; + + return { + handle(channelId: string, event: RelayEvent) { + latestEventByChannel.set(channelId, event); + const existing = pending.get(channelId); + if (existing) { + existing.retry = true; + return; + } + void attempt(channelId); + }, + }; +} diff --git a/desktop/src/features/channels/useDmResurfaceFromMessages.ts b/desktop/src/features/channels/useDmResurfaceFromMessages.ts index d925073b84e..78a17210ddf 100644 --- a/desktop/src/features/channels/useDmResurfaceFromMessages.ts +++ b/desktop/src/features/channels/useDmResurfaceFromMessages.ts @@ -8,6 +8,7 @@ import { } from "@/shared/api/tauriChannels"; import { CHANNEL_MESSAGE_EVENT_KINDS } from "@/shared/constants/kinds"; import { relayEventChannelId } from "./dmResurface"; +import { createHiddenDmResurfaceCoordinator } from "./hiddenDmResurfaceCoordinator"; import { resurfaceHiddenDmMessage } from "./hiddenDmResurfaceAction"; import { useHiddenDmIds } from "./useHiddenDmIds"; @@ -35,13 +36,6 @@ export function useDmResurfaceFromMessages({ reopen, }: UseDmResurfaceFromMessagesOptions) { const hiddenDmIds = useHiddenDmIds(pubkey); - // Coalesce per channel: the reopen action is idempotent, so concurrent - // messages for the same DM share one in-flight attempt. `retry` records that - // a follower event arrived while the attempt was in flight, so a failed - // reopen re-runs instead of silently dropping that follower. - const pendingChannelsRef = React.useRef( - new Map(), - ); const generationRef = React.useRef(0); const reopenLatest = React.useEffectEvent(reopen); @@ -56,7 +50,6 @@ export function useDmResurfaceFromMessages({ const expectedRelayUrl = relayUrl?.trim() ?? ""; const channelIds = hiddenDmKey.length > 0 ? hiddenDmKey.split(",") : []; const generation = ++generationRef.current; - pendingChannelsRef.current.clear(); if (!expectedSignerPubkey || !expectedRelayUrl || channelIds.length === 0) { return; } @@ -66,51 +59,33 @@ export function useDmResurfaceFromMessages({ let unsubscribe: (() => Promise) | undefined; const isCurrent = () => !disposed && generationRef.current === generation; - // Latest event seen per channel drives the in-flight/retry attempt so a - // coalesced follower reopens from a real event, not a captured stale one. - const latestEventByChannel = new Map(); - - const attempt = async (channelId: string) => { - const state = { retry: false }; - pendingChannelsRef.current.set(channelId, state); - try { - do { - state.retry = false; - const event = latestEventByChannel.get(channelId); - if (!event) return; - try { - await resurfaceHiddenDmMessage({ - event, - expectedRelayUrl, - expectedSignerPubkey, - hiddenDmIds: hiddenDmIdSet, - fetchMembers: getChannelMembers, - isCurrent, - reopen: reopenLatest, - }); - return; - } catch (error) { - if (isCurrent()) { - console.error("Failed to resurface hidden DM", channelId, error); - } - } - } while (state.retry && isCurrent()); - } finally { - pendingChannelsRef.current.delete(channelId); - } - }; + // A coordinator owned by this generation: coalescing and cleanup touch only + // its private map, so a torn-down generation's in-flight attempt can never + // drop a follower coalesced onto the replacement subscription. + const coordinator = createHiddenDmResurfaceCoordinator({ + resurface: (event) => + resurfaceHiddenDmMessage({ + event, + expectedRelayUrl, + expectedSignerPubkey, + hiddenDmIds: hiddenDmIdSet, + fetchMembers: getChannelMembers, + isCurrent, + reopen: reopenLatest, + }), + isCurrent, + onError: (channelId, error) => { + if (isCurrent()) { + console.error("Failed to resurface hidden DM", channelId, error); + } + }, + }); const handleEvent = (event: RelayEvent) => { if (!isCurrent()) return; const channelId = relayEventChannelId(event); if (!channelId || !hiddenDmIdSet.has(channelId)) return; - latestEventByChannel.set(channelId, event); - const pending = pendingChannelsRef.current.get(channelId); - if (pending) { - pending.retry = true; - return; - } - void attempt(channelId); + coordinator.handle(channelId, event); }; void relayClient diff --git a/mobile/lib/features/activity/activity_provider.dart b/mobile/lib/features/activity/activity_provider.dart index 81c1d279dce..e1a69c2faf3 100644 --- a/mobile/lib/features/activity/activity_provider.dart +++ b/mobile/lib/features/activity/activity_provider.dart @@ -58,9 +58,12 @@ class ActivityNotifier extends AsyncNotifier { bool _refreshQueued = false; int _subscriptionGeneration = 0; // Per-hidden-channel resurface coalescing. Presence of a key means an attempt - // is in flight; its value records whether a follower event arrived while it - // was running, so a failed reopen re-runs instead of dropping the follower. - final Map _pendingDmResurfaceRetry = {}; + // is in flight; the entry records the owning subscription generation and + // whether a follower event arrived while it was running, so a failed reopen + // re-runs instead of dropping the follower. The entry is generation-owned so + // a follower on a rebuilt subscription starts its own attempt instead of + // coalescing into a suspended old-generation attempt that will never resume. + final Map _pendingDmResurfaceRetry = {}; String? _dmResurfaceScope; @override @@ -219,18 +222,22 @@ class ActivityNotifier extends AsyncNotifier { !channelsNotifier.hiddenDmIds.contains(channelId)) { return; } - // Coalesce per channel: a concurrent follower for the same DM marks the - // in-flight attempt for retry rather than being dropped, so a failed reopen - // re-runs instead of leaving the row hidden. - if (_pendingDmResurfaceRetry.containsKey(channelId)) { - _pendingDmResurfaceRetry[channelId] = true; + // Coalesce per channel within a generation: a concurrent follower for the + // same DM marks the in-flight attempt for retry rather than being dropped, + // so a failed reopen re-runs instead of leaving the row hidden. A follower + // whose attempt was started by a superseded generation does not coalesce — + // that attempt can never resume, so this generation starts a fresh one. + final existing = _pendingDmResurfaceRetry[channelId]; + if (existing != null && existing.generation == generation) { + existing.retry = true; return; } - _pendingDmResurfaceRetry[channelId] = false; + final pending = _PendingResurface(generation); + _pendingDmResurfaceRetry[channelId] = pending; try { do { - _pendingDmResurfaceRetry[channelId] = false; + pending.retry = false; try { final members = await ref.read( channelMembersProvider(channelId).future, @@ -256,10 +263,14 @@ class ActivityNotifier extends AsyncNotifier { ); } } - } while ((_pendingDmResurfaceRetry[channelId] ?? false) && - generation == _subscriptionGeneration); + } while (pending.retry && generation == _subscriptionGeneration); } finally { - _pendingDmResurfaceRetry.remove(channelId); + // Only clear the entry if it is still the one this attempt installed; a + // newer generation may have replaced it, and stomping that entry would + // let its follower be dropped. + if (identical(_pendingDmResurfaceRetry[channelId], pending)) { + _pendingDmResurfaceRetry.remove(channelId); + } } } @@ -533,6 +544,17 @@ class ActivityNotifier extends AsyncNotifier { } } +/// A generation-owned, in-flight hidden-DM resurface attempt. `generation` ties +/// the entry to the subscription that started it so a follower on a rebuilt +/// subscription never coalesces into a suspended attempt that will never +/// resume; `retry` records that a follower arrived mid-attempt. +class _PendingResurface { + _PendingResurface(this.generation); + + final int generation; + bool retry = false; +} + final activityProvider = AsyncNotifierProvider( ActivityNotifier.new, diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index 6db51a6115d..58c23395229 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -524,6 +524,95 @@ void main() { ]); }); + test( + 'a follower on a rebuilt subscription reopens after the old attempt retires', + () async { + const self = + '1111111111111111111111111111111111111111111111111111111111111111'; + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + final session = _RecordingSessionNotifier(); + final reopenAttempts = []; + final firstReopenGate = Completer(); + var reopenCalls = 0; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => _FixedChannelsNotifier( + const [], + hiddenDmIds: const {'hidden-dm'}, + ), + ), + channelMembersProvider('hidden-dm').overrideWith( + (ref) async => [ + ChannelMember( + pubkey: self, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: alice, + role: 'member', + joinedAt: DateTime(2026), + ), + ], + ), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopenCalls += 1; + reopenAttempts.add(pubkeys.single); + if (reopenCalls == 1) { + // Hold attempt A open past the rebuild so the follower lands on a + // new generation while A is still suspended. + await firstReopenGate.future; + } + return 'hidden-dm'; + }), + ], + ); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + + NostrEvent hiddenDmEvent(String id) => NostrEvent( + id: id, + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: const [ + ['p', self], + ['h', 'hidden-dm'], + ], + content: 'Hello again', + sig: '', + ); + + // Attempt A starts on generation N and suspends inside the reopen. + session.emit(hiddenDmEvent('message-a')); + await _waitFor(() => reopenCalls == 1); + + // Activity rebuilds (generation N+1) with the pending map preserved, then + // a follower lands on the replacement subscription. + container.invalidate(activityProvider); + await container.read(activityProvider.future); + await Future.delayed(const Duration(milliseconds: 10)); + session.emit(hiddenDmEvent('message-b')); + + // B owns generation N+1, so it starts its own attempt and reopens even + // though A has not yet retired. + await _waitFor(() => reopenCalls >= 2); + + // A retires; its cleanup must not delete B's live entry. + firstReopenGate.complete(); + await Future.delayed(const Duration(milliseconds: 10)); + expect(reopenAttempts, [alice, alice]); + }, + ); + test('a suspended membership read cannot mutate a rebuilt scope', () async { const self = '1111111111111111111111111111111111111111111111111111111111111111'; From 417a90d41b6c9c3ea94efeb78fc3e9f8cca5bf28 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 26 Aug 2026 14:44:54 -0400 Subject: [PATCH 4/7] test(dm-resurface): exercise stale-owner cleanup in mobile rebuild regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior rebuild regression let follower B fully reopen before old-generation attempt A retired, so the pending map was already empty when A's finally ran — reverting the instance-checked cleanup to an unconditional remove left the test green, so it did not actually protect the guard. Now suspend both A and B concurrently: A retires while B's entry still occupies the map, then follower C arrives and must coalesce into B (reopen count stays 2). Under unconditional cleanup A evicts B's live entry, so C wrongly starts a third overlapping attempt and the assertion fails. Verified the mutation turns the test red. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../activity/activity_provider_test.dart | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index 58c23395229..f00e9b34548 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -533,7 +533,8 @@ void main() { '2222222222222222222222222222222222222222222222222222222222222222'; final session = _RecordingSessionNotifier(); final reopenAttempts = []; - final firstReopenGate = Completer(); + final gateA = Completer(); + final gateB = Completer(); var reopenCalls = 0; final container = ProviderContainer( overrides: [ @@ -561,12 +562,17 @@ void main() { ], ), dmResurfaceActionProvider.overrideWithValue((pubkeys) async { - reopenCalls += 1; + final call = ++reopenCalls; reopenAttempts.add(pubkeys.single); - if (reopenCalls == 1) { - // Hold attempt A open past the rebuild so the follower lands on a - // new generation while A is still suspended. - await firstReopenGate.future; + // Suspend attempt A (call 1, old generation) and attempt B (call 2, + // new generation) so both are in flight simultaneously: A must + // retire while B's entry still occupies the pending map. B's first + // attempt then fails so its retry loop drains the coalesced + // follower C as call 3. + if (call == 1) await gateA.future; + if (call == 2) { + await gateB.future; + throw StateError('transient reopen failure'); } return 'hidden-dm'; }), @@ -595,21 +601,36 @@ void main() { session.emit(hiddenDmEvent('message-a')); await _waitFor(() => reopenCalls == 1); - // Activity rebuilds (generation N+1) with the pending map preserved, then - // a follower lands on the replacement subscription. + // Activity rebuilds (generation N+1). Follower B lands on the replacement + // subscription: A's entry belongs to the old generation, so B does not + // coalesce — it installs its own entry and starts a second attempt, which + // suspends. A and B are now both in flight. container.invalidate(activityProvider); await container.read(activityProvider.future); await Future.delayed(const Duration(milliseconds: 10)); session.emit(hiddenDmEvent('message-b')); + await _waitFor(() => reopenCalls == 2); - // B owns generation N+1, so it starts its own attempt and reopens even - // though A has not yet retired. - await _waitFor(() => reopenCalls >= 2); + // A retires while B is still pending. Its instance-checked cleanup must + // leave B's entry in the map; unconditional removal would evict B's live + // entry here. + gateA.complete(); + await Future.delayed(const Duration(milliseconds: 10)); + + // Follower C arrives on the current generation. Because B's entry is still + // present, C coalesces into it (no new attempt). With unconditional + // cleanup, A would have evicted B's entry and C would wrongly start a + // third overlapping attempt. + session.emit(hiddenDmEvent('message-c')); + await Future.delayed(const Duration(milliseconds: 10)); + expect(reopenCalls, 2); - // A retires; its cleanup must not delete B's live entry. - firstReopenGate.complete(); + // Releasing B fails its first attempt; the coalesced follower C drives one + // retry, which succeeds as the third reopen. + gateB.complete(); + await _waitFor(() => reopenCalls == 3); await Future.delayed(const Duration(milliseconds: 10)); - expect(reopenAttempts, [alice, alice]); + expect(reopenAttempts, [alice, alice, alice]); }, ); From b975a062e37394d39c7a2beb6d63946ec374094c Mon Sep 17 00:00:00 2001 From: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz> Date: Wed, 26 Aug 2026 18:40:55 -0400 Subject: [PATCH 5/7] fix(dm-resurface): batch hidden-DM live subscriptions under the relay cap A single `#h` subscription over the whole hidden-DM set exceeded the relay's MAX_EXPLICIT_CHANNEL_VALUES (128) REQ limit once a user hid more than 128 DMs, so the relay rejected the REQ and live resurfacing was silently disabled for exactly the users with the most hidden DMs. Split the hidden set into batches of at most that many ids, each its own subscription, on both clients. Every batch is owned by the current effect generation (desktop) / subscription generation (mobile) and torn down together on hidden-set/relay/signer change, so an over-limit set can no longer disable resurfacing and no batch leaks past the generation that owns it. Mobile keeps subscribing later batches when one batch's REQ is rejected. Both clients read a shared kMaxExplicitChannelValues / MAX_EXPLICIT_CHANNEL_VALUES constant mirroring the relay's, rather than a bare 128. Also make the Inbox reopen affordance perceivable: a persistent role="status" region with aria-busy renders while a reopen is pending and surfaces a keyboard-reachable Retry on failure, replacing the prior pointer-events-none tooltip that gave no visible progress. Adds rendered coverage proving one command per activation across pointer/context-menu/ keyboard, pending-state duplicate suppression, withheld navigation until resolve, and a genuine keyboard-driven Retry that finally navigates; and a desktop batching test covering the 128/129 boundary, final-batch delivery, partial failure, and teardown mid-setup. Co-authored-by: Duncan Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../channels/useDmResurfaceBatching.test.mjs | 278 ++++++++ .../channels/useDmResurfaceFromMessages.ts | 75 +- desktop/src/features/home/ui/HomeView.tsx | 4 + .../src/features/home/ui/InboxDetailPane.tsx | 48 ++ .../src/features/home/ui/InboxListPane.tsx | 40 +- .../home/ui/inboxReopenNavigation.test.mjs | 675 ++++++++++++++++++ .../home/useHiddenDmInboxNavigation.ts | 18 +- desktop/src/shared/api/relayClientShared.ts | 9 + .../features/activity/activity_provider.dart | 73 +- mobile/lib/shared/relay/nostr_filters.dart | 7 + .../activity/activity_provider_test.dart | 191 ++++- 11 files changed, 1370 insertions(+), 48 deletions(-) create mode 100644 desktop/src/features/channels/useDmResurfaceBatching.test.mjs create mode 100644 desktop/src/features/home/ui/inboxReopenNavigation.test.mjs diff --git a/desktop/src/features/channels/useDmResurfaceBatching.test.mjs b/desktop/src/features/channels/useDmResurfaceBatching.test.mjs new file mode 100644 index 00000000000..da426d8faff --- /dev/null +++ b/desktop/src/features/channels/useDmResurfaceBatching.test.mjs @@ -0,0 +1,278 @@ +/** + * Batching lifecycle for useDmResurfaceFromMessages. + * + * The relay rejects a REQ whose aggregate explicit `#h` values exceed + * MAX_EXPLICIT_CHANNEL_VALUES, so a hidden set larger than the cap must be + * split into multiple subscriptions or every hidden DM loses its resurface + * trigger. These tests exercise exactly that split and its teardown, which a + * pure helper test cannot reach because the batching only happens inside the + * mounted effect. + */ + +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, + localStorage: dom.window.localStorage, + }); + globalThis.__TAURI_INTERNALS__ = { + invoke: (cmd) => { + if (cmd === "get_channel_members") { + return Promise.resolve({ + members: [ + { pubkey: VIEWER, role: "member", is_agent: false }, + { pubkey: "b".repeat(64), role: "member", is_agent: false }, + ], + }); + } + return Promise.reject(new Error(`unmocked Tauri command: ${cmd}`)); + }, + transformCallback: () => Math.random(), + }; + dom.window.__TAURI_INTERNALS__ = globalThis.__TAURI_INTERNALS__; +}); + +after(() => dom.window.close()); + +const RELAY_URL = "wss://relay.example"; +const VIEWER = "a".repeat(64); + +function seedCommunity() { + window.localStorage.setItem( + "buzz-communities", + JSON.stringify([ + { + id: "community-a", + name: "Community A", + relayUrl: RELAY_URL, + addedAt: "2026-01-01T00:00:00Z", + }, + ]), + ); + window.localStorage.setItem("buzz-active-community-id", "community-a"); +} + +function hiddenDmSnapshot(count) { + return { + id: "snapshot-1", + kind: 30622, + pubkey: VIEWER, + content: "", + created_at: 1, + tags: Array.from({ length: count }, (_, index) => [ + "h", + `dm-${String(index).padStart(4, "0")}`, + ]), + sig: "", + }; +} + +async function mount(hiddenCount, subscribeImpl, reopen) { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const React = (await import("react")).default; + const { QueryClient, QueryClientProvider } = await import( + "@tanstack/react-query" + ); + const { CommunitiesProvider } = await import( + "@/features/communities/useCommunities.tsx" + ); + const { relayClient } = await import("@/shared/api/relayClient"); + const { useDmResurfaceFromMessages } = await import( + "./useDmResurfaceFromMessages.ts" + ); + + const originalFetchEvents = relayClient.fetchEvents; + const originalSubscribeLive = relayClient.subscribeLive; + + relayClient.fetchEvents = async () => + hiddenCount > 0 ? [hiddenDmSnapshot(hiddenCount)] : []; + relayClient.subscribeLive = subscribeImpl; + + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: 0 } }, + }); + + const wrapper = ({ children }) => + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(CommunitiesProvider, null, children), + ); + + const hook = renderHook( + () => + useDmResurfaceFromMessages({ + pubkey: VIEWER, + relayUrl: RELAY_URL, + reopen: reopen ?? (async () => ({ id: "x" })), + }), + { wrapper }, + ); + + return { + act, + async settle() { + for (let i = 0; i < 6; i++) { + await act(async () => { + await new Promise((r) => setTimeout(r, 5)); + }); + } + }, + unmount: hook.unmount, + restore() { + hook.unmount(); + queryClient.clear(); + queryClient.unmount(); + cleanup(); + relayClient.fetchEvents = originalFetchEvents; + relayClient.subscribeLive = originalSubscribeLive; + }, + }; +} + +test("128 hidden DMs use a single subscription within the cap", async () => { + seedCommunity(); + const batches = []; + const harness = await mount(128, async (filter) => { + batches.push(filter["#h"]); + return async () => {}; + }); + try { + await harness.settle(); + assert.equal(batches.length, 1); + assert.equal(batches[0].length, 128); + } finally { + harness.restore(); + } +}); + +test("129 hidden DMs split into two subscriptions, both within the cap", async () => { + seedCommunity(); + const batches = []; + const harness = await mount(129, async (filter) => { + batches.push(filter["#h"]); + return async () => {}; + }); + try { + await harness.settle(); + assert.equal(batches.length, 2); + assert.deepEqual( + batches.map((batch) => batch.length), + [128, 1], + ); + // Every hidden id appears exactly once across the batches. + const all = batches.flat(); + assert.equal(new Set(all).size, 129); + for (const batch of batches) { + assert.ok(batch.length <= 128); + } + } finally { + harness.restore(); + } +}); + +test("activity delivered on the final batch resurfaces its DM", async () => { + seedCommunity(); + const handlers = []; + const reopened = []; + const targetIdRef = { id: null }; + const harness = await mount( + 129, + async (filter, onEvent) => { + handlers.push({ ids: filter["#h"], onEvent }); + return async () => {}; + }, + async ({ pubkeys }) => { + reopened.push(pubkeys); + // The reopen contract returns the resurfaced channel id; the action + // rejects a mismatch, so echo the target the event carried. + return { id: targetIdRef.id }; + }, + ); + try { + await harness.settle(); + const finalBatch = handlers[1]; + const targetId = finalBatch.ids[0]; + targetIdRef.id = targetId; + // Deliver a peer message on the FINAL batch's subscription. If batching + // dropped that batch's handler, this event would never reach the + // coordinator and reopen would never fire. + await harness.act(async () => { + finalBatch.onEvent({ + id: "evt-1", + kind: 9, + pubkey: "b".repeat(64), + content: "hi", + created_at: 2, + tags: [["h", targetId]], + sig: "", + }); + await new Promise((r) => setTimeout(r, 5)); + }); + assert.equal(reopened.length, 1); + assert.deepEqual(reopened[0], ["b".repeat(64)]); + } finally { + harness.restore(); + } +}); + +test("a batch subscription failure does not abort the other batches", async () => { + seedCommunity(); + const batches = []; + let call = 0; + const harness = await mount(129, async (filter) => { + call += 1; + if (call === 1) throw new Error("relay rejected batch 1"); + batches.push(filter["#h"]); + return async () => {}; + }); + try { + await harness.settle(); + // The second batch still subscribed even though the first threw. + assert.equal(batches.length, 1); + assert.equal(batches[0].length, 1); + } finally { + harness.restore(); + } +}); + +test("teardown while batch setup is pending disposes every settled batch", async () => { + seedCommunity(); + let disposeCount = 0; + const releases = []; + const harness = await mount(129, async () => { + // Each subscribe parks until the test releases it, so the effect can be + // torn down while batch setup is still in flight. + await new Promise((resolve) => releases.push(resolve)); + return async () => { + disposeCount += 1; + }; + }); + try { + await harness.settle(); + // Unmount before any subscribe resolves. + await harness.act(async () => { + harness.unmount(); + }); + // Now let both subscribes resolve; each must be disposed since its owning + // generation is gone. + await harness.act(async () => { + for (const release of releases) release(); + await new Promise((r) => setTimeout(r, 5)); + }); + assert.equal(disposeCount, 2); + } finally { + harness.restore(); + } +}); diff --git a/desktop/src/features/channels/useDmResurfaceFromMessages.ts b/desktop/src/features/channels/useDmResurfaceFromMessages.ts index 78a17210ddf..fddd056f25c 100644 --- a/desktop/src/features/channels/useDmResurfaceFromMessages.ts +++ b/desktop/src/features/channels/useDmResurfaceFromMessages.ts @@ -2,6 +2,7 @@ import * as React from "react"; import { relayClient } from "@/shared/api/relayClient"; import type { RelayEvent } from "@/shared/api/types"; +import { MAX_EXPLICIT_CHANNEL_VALUES } from "@/shared/api/relayClientShared"; import { getChannelMembers, type OpenDmInput, @@ -27,8 +28,13 @@ type UseDmResurfaceFromMessagesOptions = { * `#p` filter would never receive them. Scoping to `#h` also means every * delivered event is already for a hidden DM the reader belongs to (hiding * never drops membership), so no per-event visibility fetch is needed and - * untagged CLI/agent DMs resurface too. The subscription re-registers whenever - * the hidden set changes. + * untagged CLI/agent DMs resurface too. + * + * The relay rejects a REQ whose aggregate explicit `#h` values exceed + * {@link MAX_EXPLICIT_CHANNEL_VALUES}, so the hidden set is split into batches + * of at most that size, each its own subscription. Every batch is owned by the + * current effect generation and disposed together on hidden-set/relay/signer + * change, so an over-limit hidden set no longer silently disables resurfacing. */ export function useDmResurfaceFromMessages({ pubkey, @@ -56,12 +62,14 @@ export function useDmResurfaceFromMessages({ const hiddenDmIdSet = new Set(channelIds); let disposed = false; - let unsubscribe: (() => Promise) | undefined; + const unsubscribers: Array<() => Promise> = []; const isCurrent = () => !disposed && generationRef.current === generation; // A coordinator owned by this generation: coalescing and cleanup touch only // its private map, so a torn-down generation's in-flight attempt can never - // drop a follower coalesced onto the replacement subscription. + // drop a follower coalesced onto the replacement subscription. One + // coordinator spans every batch — the map is keyed by channel id, so which + // batch delivered an event is irrelevant. const coordinator = createHiddenDmResurfaceCoordinator({ resurface: (event) => resurfaceHiddenDmMessage({ @@ -88,33 +96,46 @@ export function useDmResurfaceFromMessages({ coordinator.handle(channelId, event); }; - void relayClient - .subscribeLive( - { - kinds: [...CHANNEL_MESSAGE_EVENT_KINDS], - "#h": channelIds, - since: Math.floor(Date.now() / 1_000) - 5, - limit: 100, - }, - handleEvent, - ) - .then((dispose) => { - if (!isCurrent()) { - void dispose().catch(() => {}); - return; - } - unsubscribe = dispose; - }) - .catch((error) => { - if (isCurrent()) { - console.error("Failed to subscribe to hidden DM activity", error); - } - }); + const since = Math.floor(Date.now() / 1_000) - 5; + for ( + let start = 0; + start < channelIds.length; + start += MAX_EXPLICIT_CHANNEL_VALUES + ) { + const batch = channelIds.slice( + start, + start + MAX_EXPLICIT_CHANNEL_VALUES, + ); + void relayClient + .subscribeLive( + { + kinds: [...CHANNEL_MESSAGE_EVENT_KINDS], + "#h": batch, + since, + limit: 100, + }, + handleEvent, + ) + .then((dispose) => { + if (!isCurrent()) { + void dispose().catch(() => {}); + return; + } + unsubscribers.push(dispose); + }) + .catch((error) => { + if (isCurrent()) { + console.error("Failed to subscribe to hidden DM activity", error); + } + }); + } return () => { disposed = true; generationRef.current += 1; - void unsubscribe?.().catch(() => {}); + for (const unsubscribe of unsubscribers) { + void unsubscribe().catch(() => {}); + } }; }, [pubkey, relayUrl, hiddenDmKey]); } diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 41b4cebff97..0a16f27c4d0 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -456,6 +456,7 @@ export function HomeView({ handleOpenDm, handleOpenSelectedContext, isReopenPending, + isReopenErrored, } = useHiddenDmInboxNavigation({ availableChannelIds, currentPubkey, @@ -704,6 +705,7 @@ export function HomeView({ onMarkUnread={markItemUnread} onOpenDirect={handleOpenDirect} isReopenPending={isReopenPending} + isReopenErrored={isReopenErrored} onRemindLater={(item) => { const channelId = item.item.channelId; if (!channelId) { @@ -820,6 +822,8 @@ export function HomeView({ onEditSave={editMessage} onRequestEmptyEditDelete={setEmptyDeleteId} onOpenContext={handleOpenSelectedContext} + reopenPending={isReopenPending(selectedItem?.item.channelId)} + reopenErrored={isReopenErrored(selectedItem?.item.channelId)} onSendReply={async ({ content, mediaTags, diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 373ef80f452..4a29a1b358c 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -125,6 +125,10 @@ type InboxDetailPaneProps = { messageId: string, threadRootId?: string | null, ) => void; + /** True while the selected hidden DM is being reopened on the relay. */ + reopenPending?: boolean; + /** True when the last reopen of the selected hidden DM failed. */ + reopenErrored?: boolean; onSendReply: (input: { content: string; mediaTags?: string[][]; @@ -189,6 +193,8 @@ function InboxMessageDetailPane({ onRequestEmptyEditDelete, onManageChannel, onOpenContext, + reopenPending = false, + reopenErrored = false, onSendReply, onToggleReaction, }: InboxDetailPaneProps) { @@ -589,6 +595,48 @@ function InboxMessageDetailPane({
+ {reopenPending || reopenErrored ? ( +
+ {reopenPending ? ( + <> + + Reopening… + + ) : ( + <> + + Couldn’t reopen + {contextChannelId ? ( + + ) : null} + + )} +
+ ) : null} {canOpenChannel && contextChannelId ? ( diff --git a/desktop/src/features/home/ui/InboxListPane.tsx b/desktop/src/features/home/ui/InboxListPane.tsx index 6b83c0289b7..953b783a682 100644 --- a/desktop/src/features/home/ui/InboxListPane.tsx +++ b/desktop/src/features/home/ui/InboxListPane.tsx @@ -1,4 +1,12 @@ -import { Bell, Clock, Ellipsis, ExternalLink, MailOpen } from "lucide-react"; +import { + AlertCircle, + Bell, + Clock, + Ellipsis, + ExternalLink, + LoaderCircle, + MailOpen, +} from "lucide-react"; import * as React from "react"; import { @@ -216,6 +224,7 @@ type InboxListPaneProps = { onMarkUnread: (itemId: string) => void; onOpenDirect: (item: InboxItem) => void; isReopenPending?: (channelId: string | null | undefined) => boolean; + isReopenErrored?: (channelId: string | null | undefined) => boolean; onRemindLater: (item: InboxItem) => void; onSelect: (itemId: string) => void; onSelectDraft: (draftKey: string) => void; @@ -245,6 +254,7 @@ export function InboxListPane({ onMarkUnread, onOpenDirect, isReopenPending, + isReopenErrored, onRemindLater, onSelect, onSelectDraft, @@ -306,6 +316,7 @@ export function InboxListPane({ ); const hasChannelTarget = Boolean(item.item.channelId); const isReopening = isReopenPending?.(item.item.channelId) ?? false; + const hasReopenError = isReopenErrored?.(item.item.channelId) ?? false; const canOpen = hasChannelTarget && !isReopening; const openLabel = !hasChannelTarget ? "No channel link" @@ -440,6 +451,33 @@ export function InboxListPane({
) : null} + {isReopening || hasReopenError ? ( +
+ {isReopening ? ( + <> + + Reopening… + + ) : ( + <> + + Couldn’t reopen + + )} +
+ ) : null} +
null;\n", + }; + } + if (url === "buzz-inbox-stub:UpdateIndicator") { + // The real UpdateIndicator pulls in UpdaterProvider's background-check + // setInterval, which keeps the event loop alive past the test. It has + // nothing to do with the reopen contract, so stub it to a null render. + return { + format: "module", + shortCircuit: true, + source: "export const UpdateIndicator = () => null;\n", + }; + } + return nextLoad(url, context); + }, +}); + +const SELF = "1".repeat(64); +const PEER = "2".repeat(64); +const HIDDEN_DM_ID = "hidden-dm-channel"; +const SOURCE_EVENT_ID = "e".repeat(64); +const RELAY_URL = "wss://relay.example"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +class NoopObserver { + disconnect() {} + observe() {} + unobserve() {} +} + +// Neither pane nor the reopen path needs a live relay socket; a real +// (undici) WebSocket would open a connection to the seeded relay URL and +// leak an open handle that keeps the test process alive. Stub it to a +// non-connecting shell. +class NoopWebSocket { + close() {} + send() {} + addEventListener() {} + removeEventListener() {} +} +globalThis.WebSocket = NoopWebSocket; +dom.window.WebSocket = NoopWebSocket; + +Object.assign(globalThis, { + IS_REACT_ACT_ENVIRONMENT: true, + IntersectionObserver: NoopObserver, + MutationObserver: dom.window.MutationObserver, + ResizeObserver: NoopObserver, + document: dom.window.document, + localStorage: dom.window.localStorage, + self: dom.window, + window: dom.window, +}); +// Bulk-copy DOM constructors Radix / React reference without a window prefix. +for (const key of Object.getOwnPropertyNames(dom.window)) { + if ( + !(key in globalThis) && + (key.startsWith("HTML") || + key.startsWith("SVG") || + [ + "Element", + "DOMRect", + "DOMRectReadOnly", + "Node", + "NodeFilter", + "NodeList", + "NamedNodeMap", + "Event", + "CustomEvent", + "MouseEvent", + "KeyboardEvent", + "FocusEvent", + "InputEvent", + "PointerEvent", + "Text", + "Comment", + "DocumentFragment", + "Range", + "Selection", + ].includes(key)) + ) { + const value = dom.window[key]; + if (value !== undefined) globalThis[key] = value; + } +} +Object.defineProperty(globalThis, "navigator", { + configurable: true, + value: dom.window.navigator, + writable: true, +}); +globalThis.getComputedStyle = dom.window.getComputedStyle.bind(dom.window); +dom.window.matchMedia = () => ({ + matches: false, + addEventListener() {}, + removeEventListener() {}, +}); +globalThis.matchMedia = dom.window.matchMedia; +dom.window.requestAnimationFrame = (callback) => setTimeout(callback, 0); +dom.window.cancelAnimationFrame = (id) => clearTimeout(id); +globalThis.requestAnimationFrame = dom.window.requestAnimationFrame; +globalThis.cancelAnimationFrame = dom.window.cancelAnimationFrame; + +// Radix DismissableLayer/FocusScope dispatch plain objects; JSDOM's strict +// Event validation throws on them. Drop non-Event objects silently. +const _origDispatch = dom.window.EventTarget.prototype.dispatchEvent; +dom.window.EventTarget.prototype.dispatchEvent = function dispatchEvent(event) { + if (!(event instanceof dom.window.Event)) return false; + return _origDispatch.call(this, event); +}; +globalThis.EventTarget = dom.window.EventTarget; + +// JSDOM does not perform a native + ) : null} )}
diff --git a/desktop/src/features/home/ui/inboxReopenNavigation.test.mjs b/desktop/src/features/home/ui/inboxReopenNavigation.test.mjs index de7dd3388c6..385195944ca 100644 --- a/desktop/src/features/home/ui/inboxReopenNavigation.test.mjs +++ b/desktop/src/features/home/ui/inboxReopenNavigation.test.mjs @@ -279,6 +279,26 @@ const HIDDEN_DM_ITEM = { unreadCount: 1, }; +// A second, already-open conversation used as the selected detail while the +// hidden DM row fails to reopen — so the detail pane's Retry belongs to a +// different channel and cannot cover the failed row. +const OTHER_CHANNEL_ID = "other-open-channel"; +const OTHER_EVENT_ID = "f".repeat(64); +const OTHER_ITEM = { + ...HIDDEN_DM_ITEM, + conversationId: OTHER_CHANNEL_ID, + id: OTHER_EVENT_ID, + item: { + ...HIDDEN_DM_ITEM.item, + id: OTHER_EVENT_ID, + channelId: OTHER_CHANNEL_ID, + channelName: "other", + channelType: "channel", + }, + channelLabel: "other", + senderLabel: "Other", +}; + let React; let act; let createRoot; @@ -332,7 +352,8 @@ after(() => dom.window.close()); * router is not needed: the reopen path terminates at onOpenContext, which we * capture, and goChannel is only reached via handleOpenDm (not exercised here). */ -async function mountInbox() { +async function mountInbox(options = {}) { + const { items = [HIDDEN_DM_ITEM], selectedItem = HIDDEN_DM_ITEM } = options; seedCommunity(); openDmCalls = 0; const navigations = []; @@ -350,7 +371,7 @@ async function mountInbox() { currentPubkey: SELF, onOpenContext: (channelId, messageId, threadRootId) => navigations.push([channelId, messageId, threadRootId ?? null]), - selectedItem: HIDDEN_DM_ITEM, + selectedItem, }); return React.createElement( React.Fragment, @@ -360,7 +381,7 @@ async function mountInbox() { draftItems: [], doneSet: new Set(), filter: "all", - items: [HIDDEN_DM_ITEM], + items, onFilterChange() {}, onDeleteDraft() {}, onMarkRead() {}, @@ -373,7 +394,7 @@ async function mountInbox() { onSelectDraft() {}, onSelectReminder() {}, onUnreadOnlyChange() {}, - selectedConversationId: HIDDEN_DM_ID, + selectedConversationId: selectedItem.conversationId, selectedDraftKey: null, dueReminderCount: 0, reminders: [], @@ -387,8 +408,8 @@ async function mountInbox() { channel: null, currentPubkey: SELF, editTargetId: null, - item: HIDDEN_DM_ITEM, - selectedEventId: SOURCE_EVENT_ID, + item: selectedItem, + selectedEventId: selectedItem.id, onDelete() {}, onDeleteMessage() {}, onEditTargetChange() {}, @@ -396,8 +417,8 @@ async function mountInbox() { onRequestEmptyEditDelete() {}, onManageChannel() {}, onOpenContext: nav.handleOpenSelectedContext, - reopenPending: nav.isReopenPending(HIDDEN_DM_ID), - reopenErrored: nav.isReopenErrored(HIDDEN_DM_ID), + reopenPending: nav.isReopenPending(selectedItem.item.channelId), + reopenErrored: nav.isReopenErrored(selectedItem.item.channelId), onSendReply: async () => {}, }), ); @@ -673,3 +694,96 @@ test("failed reopen surfaces a keyboard-operable Retry that then navigates", asy await inbox.unmount(); } }); + +test("a failed reopen from an unselected row exposes its own keyboard Retry that navigates", async () => { + // The selected detail is a DIFFERENT, already-open conversation, so the + // detail pane's Retry belongs to OTHER_CHANNEL_ID and cannot reopen the + // hidden DM. The failed hidden-DM row must therefore carry its own Retry. + const inbox = await mountInbox({ + items: [OTHER_ITEM, HIDDEN_DM_ITEM], + selectedItem: OTHER_ITEM, + }); + try { + let attempts = 0; + openDmHandler = async () => { + attempts += 1; + if (attempts === 1) throw new Error("relay offline"); + return { id: HIDDEN_DM_ID, channel_type: "dm" }; + }; + const openButton = inbox.container.querySelector( + '[data-testid="home-inbox-item-' + + SOURCE_EVENT_ID + + '"] [aria-label="Open in channel"]', + ); + assert.ok( + openButton, + "the unselected hidden-DM row must render its action", + ); + await act(async () => { + click(openButton); + }); + await inbox.settle(); + + assert.equal(attempts, 1, "one reopen attempt was made"); + assert.equal( + inbox.navigations.length, + 0, + "a failed reopen does not navigate", + ); + // The selected detail pane belongs to another channel, so its status + // region must not be showing this failure. + const detailStatus = inbox.container.querySelector( + '[data-testid="home-inbox-reopen-status"]', + ); + assert.equal( + detailStatus, + null, + "the other-channel detail pane must not show the hidden DM's error", + ); + + // The failed row itself surfaces a keyboard-operable Retry. + const rowStatus = inbox.container.querySelector( + '[data-testid="home-inbox-reopen-status-' + SOURCE_EVENT_ID + '"]', + ); + assert.ok(rowStatus, "the failed row must show its own error status"); + const retry = inbox.container.querySelector( + '[data-testid="home-inbox-reopen-retry-' + SOURCE_EVENT_ID + '"]', + ); + assert.ok(retry, "the failed row must expose its own Retry"); + assert.equal(retry.tagName, "BUTTON"); + assert.equal(retry.disabled, false); + assert.notEqual( + dom.window.getComputedStyle(retry).pointerEvents, + "none", + "the row Retry must not be pointer-events-blocked", + ); + await act(async () => { + retry.focus(); + }); + assert.equal( + dom.window.document.activeElement, + retry, + "the row Retry must accept keyboard focus", + ); + await act(async () => { + dom.window.document.activeElement.dispatchEvent( + new dom.window.KeyboardEvent("keydown", { + key: "Enter", + bubbles: true, + cancelable: true, + }), + ); + await new Promise((resolve) => setTimeout(resolve, 5)); + }); + await inbox.settle(); + + assert.equal(attempts, 2, "the row Retry issues exactly one more reopen"); + assert.deepEqual( + inbox.navigations, + [[HIDDEN_DM_ID, SOURCE_EVENT_ID, null]], + "the successful row retry navigates to the hidden DM", + ); + } finally { + await inbox.unmount(); + } +}); diff --git a/mobile/lib/features/activity/activity_provider.dart b/mobile/lib/features/activity/activity_provider.dart index 2d1490d6c1c..7e272d9d6a9 100644 --- a/mobile/lib/features/activity/activity_provider.dart +++ b/mobile/lib/features/activity/activity_provider.dart @@ -165,6 +165,15 @@ class ActivityNotifier extends AsyncNotifier { start < hiddenDmIds.length; start += kMaxExplicitChannelValues ) { + // Never open a batch REQ for a superseded generation: check before + // each subscribe so teardown mid-setup stops issuing new REQs, and + // tear down everything this generation already opened. + if (generation != _subscriptionGeneration) { + for (final unsubscribe in hiddenUnsubscribers) { + unsubscribe(); + } + return; + } final end = start + kMaxExplicitChannelValues < hiddenDmIds.length ? start + kMaxExplicitChannelValues : hiddenDmIds.length; @@ -179,21 +188,38 @@ class ActivityNotifier extends AsyncNotifier { ), (event) => _handleHiddenDmLiveEvent(event, generation), ); + // A newer generation may have superseded us while this batch's REQ + // was in flight. Dispose the just-resolved subscription plus every + // batch this generation already opened, and return without + // initiating another REQ. + if (generation != _subscriptionGeneration) { + unsubscribeHiddenDms(); + for (final unsubscribe in hiddenUnsubscribers) { + unsubscribe(); + } + return; + } hiddenUnsubscribers.add(unsubscribeHiddenDms); } catch (error) { + // Superseded while this batch's REQ was rejected: tear down what we + // opened and stop rather than initiating another REQ. + if (generation != _subscriptionGeneration) { + for (final unsubscribe in hiddenUnsubscribers) { + unsubscribe(); + } + return; + } // One batch's REQ was rejected; keep subscribing the rest so a // partially-rejected hidden set still resurfaces every other batch. - if (generation == _subscriptionGeneration) { - debugPrint( - '[ActivityNotifier] hidden-DM batch subscription failed: $error', - ); - } + debugPrint( + '[ActivityNotifier] hidden-DM batch subscription failed: $error', + ); } } - // A newer generation may have superseded us while a batch's REQ was in - // flight; tear down every batch this generation opened so none leak past - // the generation that owns them, and never publish into the field the - // successor already cleared. + // A newer generation may have superseded us after the final batch + // settled; tear down every batch this generation opened so none leak + // past the generation that owns them, and never publish into the field + // the successor already cleared. if (generation != _subscriptionGeneration) { for (final unsubscribe in hiddenUnsubscribers) { unsubscribe(); diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index b8de3132076..df5f83519b1 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -24,6 +24,10 @@ class _RecordingSessionNotifier extends RelaySessionNotifier { Completer? hiddenSubscribeGate; bool failNextHiddenSubscribe = false; int hiddenUnsubscribeCount = 0; + // Per-batch gates keyed by hidden-subscribe call order (0-based), so a test + // can park an individual batch's REQ while letting earlier ones settle. + final Map> hiddenSubscribeGatesByCall = {}; + int hiddenSubscribeCallCount = 0; @override SessionState build() => const SessionState(status: SessionStatus.connected); @@ -109,6 +113,9 @@ class _RecordingSessionNotifier extends RelaySessionNotifier { filter.tags.containsKey('#h') && filter.kinds.length == EventKind.channelMessageEventKinds.length; if (isHidden) { + final callIndex = hiddenSubscribeCallCount++; + final perCallGate = hiddenSubscribeGatesByCall[callIndex]; + if (perCallGate != null) await perCallGate.future; final gate = hiddenSubscribeGate; if (gate != null) await gate.future; if (failNextHiddenSubscribe) { @@ -970,21 +977,47 @@ void main() { expect(batches.single, hasLength(1)); }); + test('teardown during batch-1 setup self-disposes batch 1 and never starts ' + 'batch 2', () async { + final session = _RecordingSessionNotifier() + ..hiddenSubscribeGatesByCall[0] = Completer(); + final container = containerFor(session, hiddenIds(129)); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + // Batch 1's REQ is parked in flight; batch 2 has not been requested. + await _waitFor(() => session.hiddenSubscribeCallCount == 1); + + // Supersede this generation mid-setup, then let batch 1's REQ resolve. + container.dispose(); + session.hiddenSubscribeGatesByCall[0]!.complete(); + + // Batch 1 resolves stale and self-disposes; batch 2 is never requested. + await _waitFor(() => session.hiddenUnsubscribeCount == 1); + expect(session.hiddenUnsubscribeCount, 1); + expect(session.hiddenSubscribeCallCount, 1); + expect(session.hiddenDmSubscriptionBatches, isEmpty); + }); + test( - 'teardown while batch setup is pending disposes every settled batch', + 'teardown after batch 1 settles while batch 2 is pending tears down both', () async { final session = _RecordingSessionNotifier() - ..hiddenSubscribeGate = Completer(); + ..hiddenSubscribeGatesByCall[1] = Completer(); final container = containerFor(session, hiddenIds(129)); await container.read(channelsProvider.future); await container.read(activityProvider.future); - // Dispose while both batches are parked on the gate: the generation is - // superseded, so every batch that later resolves must be torn down. + // Batch 1 has settled and registered; batch 2's REQ is parked in flight. + await _waitFor(() => session.hiddenSubscribeCallCount == 2); + expect(session.hiddenDmSubscriptionBatches, hasLength(1)); + + // Supersede this generation with batch 2 pending, then let it resolve. container.dispose(); - session.hiddenSubscribeGate!.complete(); - await _waitFor(() => session.hiddenUnsubscribeCount == 2); + session.hiddenSubscribeGatesByCall[1]!.complete(); + // Batch 2 resolves stale and disposes itself plus the settled batch 1. + await _waitFor(() => session.hiddenUnsubscribeCount == 2); expect(session.hiddenUnsubscribeCount, 2); expect(session.hiddenDmSubscriptionBatches, isEmpty); }, From 7368b50f9d8f538ac9e81843e47c75da54a6d3b1 Mon Sep 17 00:00:00 2001 From: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz> Date: Thu, 27 Aug 2026 09:57:24 -0400 Subject: [PATCH 7/7] fix(dm-resurface): batch visible-DM live sub and make pending status announceable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review fixes for Jude's two new P2s. P1: mobile setup placed every visible member DM into one `#h` filter before any hidden-DM batch registered. The relay rejects a REQ whose explicit `#h` values exceed the cap, so 129 visible DMs failed the outer subscribe, hit the catch, and no hidden batch ever started — over-cap visible sets silently disabled resurfacing. Extract the generation-owned batching loop into a shared helper and run the visible-DM subscription through it too, so every REQ stays within the cap and a per-batch rejection stays isolated. P2: `aria-busy` on the `role=status` pending regions let AT defer the live update, and the region unmounts on success before ever committing `aria-busy=false`, so "Reopening…" could go unannounced. Remove `aria-busy` from both the list-row and detail status regions so the pending state is immediately announceable. Native Retry behavior is unchanged. Add a mobile regression proving 129 visible DMs split 128+1 while the hidden subscription still registers and hidden activity resurfaces. Update the rendered reopen test to require the pending status carry no `aria-busy` suppression. Co-authored-by: Duncan Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src/features/home/ui/InboxDetailPane.tsx | 1 - .../src/features/home/ui/InboxListPane.tsx | 1 - .../home/ui/inboxReopenNavigation.test.mjs | 11 +- .../features/activity/activity_provider.dart | 211 ++++++++++-------- .../activity/activity_provider_test.dart | 91 ++++++++ 5 files changed, 219 insertions(+), 96 deletions(-) diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 4a29a1b358c..1cf6f979731 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -597,7 +597,6 @@ function InboxMessageDetailPane({ {reopenPending || reopenErrored ? (
{ ]; void Function()? _unsubscribeAddressed; - void Function()? _unsubscribeDms; + final List _unsubscribeDms = []; final List _unsubscribeHiddenDms = []; Timer? _liveRefreshTimer; Future? _refreshInFlight; @@ -127,111 +127,138 @@ class ActivityNotifier extends AsyncNotifier { for (final channel in channels) if (channel.isDm && channel.isMember) channel.id, ]; - if (dmChannelIds.isNotEmpty) { - final unsubscribeDms = await session.subscribe( - NostrFilter( - kinds: const [9], - tags: {'#h': dmChannelIds}, - since: since, - limit: 100, - ), - (_) => _scheduleLiveRefresh(generation), - ); - if (generation != _subscriptionGeneration) { - unsubscribeDms(); - return; - } - _unsubscribeDms = unsubscribeDms; - } + // The relay rejects a REQ whose explicit `#h` values exceed + // [kMaxExplicitChannelValues]. A single over-cap visible-DM REQ would be + // rejected and, hitting the outer catch, abort the whole live setup — + // including the hidden batches below — so more than that many visible DMs + // used to silently disable resurfacing. Batch it under the same cap so no + // single REQ can be rejected for size and a per-batch rejection stays + // isolated to that batch. + final visibleUnsubscribers = await _subscribeChannelBatches( + session, + generation, + channelIds: dmChannelIds, + kinds: const [9], + since: since, + onEvent: (_) => _scheduleLiveRefresh(generation), + ); + if (visibleUnsubscribers == null) return; + _unsubscribeDms.addAll(visibleUnsubscribers); // Resurface trigger: hidden DMs are dropped from the visible-DM sub above, // so subscribe to them separately. Channel messages carry a channel_id and // the relay only fans channel-scoped events to channel-scoped subs, so an // `#h` filter (never `#p`, which the relay treats as global) is required. - // Hiding never drops membership, so `#h` authorization holds. - // - // The relay rejects a REQ whose aggregate explicit `#h` values exceed - // [kMaxExplicitChannelValues], so the hidden set is split into batches of - // at most that size, each its own subscription. All batches are owned by - // this generation and torn down together, so an over-limit hidden set no - // longer silently disables resurfacing. + // Hiding never drops membership, so `#h` authorization holds. The hidden + // set is batched under the same cap and owned by this generation, so an + // over-limit hidden set no longer silently disables resurfacing. final hiddenDmIds = ref .read(channelsProvider.notifier) .hiddenDmIds .toList(); - final hiddenUnsubscribers = []; - for ( - var start = 0; - start < hiddenDmIds.length; - start += kMaxExplicitChannelValues - ) { - // Never open a batch REQ for a superseded generation: check before - // each subscribe so teardown mid-setup stops issuing new REQs, and - // tear down everything this generation already opened. + final hiddenUnsubscribers = await _subscribeChannelBatches( + session, + generation, + channelIds: hiddenDmIds, + kinds: EventKind.channelMessageEventKinds, + since: since, + onEvent: (event) => _handleHiddenDmLiveEvent(event, generation), + ); + if (hiddenUnsubscribers == null) return; + _unsubscribeHiddenDms.addAll(hiddenUnsubscribers); + } catch (error) { + if (generation == _subscriptionGeneration) { + debugPrint('[ActivityNotifier] live subscription failed: $error'); + } + } + } + + /// Subscribes to a channel-scoped live feed split into batches that never + /// exceed [kMaxExplicitChannelValues] explicit `#h` values, since the relay + /// rejects a REQ that does. Each batch is its own subscription owned by + /// [generation]. The generation is checked before every `subscribe` and + /// immediately after each await, so teardown mid-setup disposes everything + /// this call already opened and stops issuing REQs. A single batch's + /// rejection is isolated so later batches still register. + /// + /// Returns the accumulated unsubscribers for the caller to retain, or `null` + /// if a newer generation superseded this call — in which case it has already + /// torn down everything it opened and the caller must return without + /// publishing into a field the successor already cleared. + Future?> _subscribeChannelBatches( + RelaySessionNotifier session, + int generation, { + required List channelIds, + required List kinds, + required int since, + required void Function(NostrEvent) onEvent, + }) async { + final unsubscribers = []; + for ( + var start = 0; + start < channelIds.length; + start += kMaxExplicitChannelValues + ) { + // Never open a batch REQ for a superseded generation: check before each + // subscribe so teardown mid-setup stops issuing new REQs, and tear down + // everything this call already opened. + if (generation != _subscriptionGeneration) { + for (final unsubscribe in unsubscribers) { + unsubscribe(); + } + return null; + } + final end = start + kMaxExplicitChannelValues < channelIds.length + ? start + kMaxExplicitChannelValues + : channelIds.length; + final batch = channelIds.sublist(start, end); + try { + final unsubscribe = await session.subscribe( + NostrFilter( + kinds: kinds, + tags: {'#h': batch}, + since: since, + limit: 100, + ), + onEvent, + ); + // A newer generation may have superseded us while this batch's REQ was + // in flight. Dispose the just-resolved subscription plus every batch + // this call already opened, and stop without initiating another REQ. if (generation != _subscriptionGeneration) { - for (final unsubscribe in hiddenUnsubscribers) { - unsubscribe(); + unsubscribe(); + for (final accumulated in unsubscribers) { + accumulated(); } - return; + return null; } - final end = start + kMaxExplicitChannelValues < hiddenDmIds.length - ? start + kMaxExplicitChannelValues - : hiddenDmIds.length; - final batch = hiddenDmIds.sublist(start, end); - try { - final unsubscribeHiddenDms = await session.subscribe( - NostrFilter( - kinds: EventKind.channelMessageEventKinds, - tags: {'#h': batch}, - since: since, - limit: 100, - ), - (event) => _handleHiddenDmLiveEvent(event, generation), - ); - // A newer generation may have superseded us while this batch's REQ - // was in flight. Dispose the just-resolved subscription plus every - // batch this generation already opened, and return without - // initiating another REQ. - if (generation != _subscriptionGeneration) { - unsubscribeHiddenDms(); - for (final unsubscribe in hiddenUnsubscribers) { - unsubscribe(); - } - return; - } - hiddenUnsubscribers.add(unsubscribeHiddenDms); - } catch (error) { - // Superseded while this batch's REQ was rejected: tear down what we - // opened and stop rather than initiating another REQ. - if (generation != _subscriptionGeneration) { - for (final unsubscribe in hiddenUnsubscribers) { - unsubscribe(); - } - return; + unsubscribers.add(unsubscribe); + } catch (error) { + // Superseded while this batch's REQ was rejected: tear down what we + // opened and stop rather than initiating another REQ. + if (generation != _subscriptionGeneration) { + for (final accumulated in unsubscribers) { + accumulated(); } - // One batch's REQ was rejected; keep subscribing the rest so a - // partially-rejected hidden set still resurfaces every other batch. - debugPrint( - '[ActivityNotifier] hidden-DM batch subscription failed: $error', - ); - } - } - // A newer generation may have superseded us after the final batch - // settled; tear down every batch this generation opened so none leak - // past the generation that owns them, and never publish into the field - // the successor already cleared. - if (generation != _subscriptionGeneration) { - for (final unsubscribe in hiddenUnsubscribers) { - unsubscribe(); + return null; } - return; + // One batch's REQ was rejected; keep subscribing the rest so a + // partially-rejected set still covers every other batch. + debugPrint( + '[ActivityNotifier] channel batch subscription failed: $error', + ); } - _unsubscribeHiddenDms.addAll(hiddenUnsubscribers); - } catch (error) { - if (generation == _subscriptionGeneration) { - debugPrint('[ActivityNotifier] live subscription failed: $error'); + } + // A newer generation may have superseded us after the final batch settled; + // tear down every batch this call opened so none leak past the generation + // that owns them. + if (generation != _subscriptionGeneration) { + for (final unsubscribe in unsubscribers) { + unsubscribe(); } + return null; } + return unsubscribers; } void _handleAddressedLiveEvent(NostrEvent event, int generation) { @@ -390,8 +417,10 @@ class ActivityNotifier extends AsyncNotifier { _refreshQueued = false; _unsubscribeAddressed?.call(); _unsubscribeAddressed = null; - _unsubscribeDms?.call(); - _unsubscribeDms = null; + for (final unsubscribe in _unsubscribeDms) { + unsubscribe(); + } + _unsubscribeDms.clear(); for (final unsubscribe in _unsubscribeHiddenDms) { unsubscribe(); } diff --git a/mobile/test/features/activity/activity_provider_test.dart b/mobile/test/features/activity/activity_provider_test.dart index df5f83519b1..eda77f46ebf 100644 --- a/mobile/test/features/activity/activity_provider_test.dart +++ b/mobile/test/features/activity/activity_provider_test.dart @@ -140,6 +140,16 @@ class _RecordingSessionNotifier extends RelaySessionNotifier { subscription.filter.tags['#h']!, ]; + /// The `#h` value lists of every registered visible-DM subscription (kind 9 + /// only), in order. + List> get visibleDmSubscriptionBatches => [ + for (final subscription in _subscriptions) + if (subscription.filter.tags.containsKey('#h') && + subscription.filter.kinds.length == 1 && + subscription.filter.kinds.single == 9) + subscription.filter.tags['#h']!, + ]; + void emit(NostrEvent event) { _history.add(event); for (final subscription in List.of(_subscriptions)) { @@ -1022,6 +1032,87 @@ void main() { expect(session.hiddenDmSubscriptionBatches, isEmpty); }, ); + + test( + '129 visible DMs batch under the cap and hidden activity still resurfaces', + () async { + // An over-cap visible-DM set previously rejected as one REQ, aborting + // the whole live setup — so the hidden batches never registered and + // resurfacing silently died. Batching the visible set keeps every REQ + // within the cap and leaves the hidden subscription intact. + const alice = + '2222222222222222222222222222222222222222222222222222222222222222'; + final visibleChannels = [ + for (var i = 0; i < 129; i++) + _dmChannel('visible-${i.toString().padLeft(4, '0')}'), + ]; + const hiddenTarget = 'hidden-dm'; + final session = _RecordingSessionNotifier(); + final reopened = >[]; + final container = ProviderContainer( + overrides: [ + relayConfigProvider.overrideWith(_FixedRelayConfigNotifier.new), + myPubkeyProvider.overrideWithValue(self), + relaySessionProvider.overrideWith(() => session), + channelsProvider.overrideWith( + () => _FixedChannelsNotifier( + visibleChannels, + hiddenDmIds: const {hiddenTarget}, + ), + ), + channelMembersProvider(hiddenTarget).overrideWith( + (ref) async => [ + ChannelMember( + pubkey: self, + role: 'member', + joinedAt: DateTime(2026), + ), + ChannelMember( + pubkey: alice, + role: 'member', + joinedAt: DateTime(2026), + ), + ], + ), + dmResurfaceActionProvider.overrideWithValue((pubkeys) async { + reopened.add(pubkeys); + return hiddenTarget; + }), + ], + ); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(activityProvider.future); + await _waitFor(() => session.hiddenDmSubscriptionBatches.isNotEmpty); + + // Every visible-DM REQ stays within the cap, split 128 + 1. + final visibleBatches = session.visibleDmSubscriptionBatches; + expect(visibleBatches.map((batch) => batch.length), [128, 1]); + expect(visibleBatches.expand((batch) => batch).toSet(), hasLength(129)); + // The hidden subscription registered despite the over-cap visible set. + expect(session.hiddenDmSubscriptionBatches, hasLength(1)); + + // Hidden activity still resurfaces its DM. + session.emit( + NostrEvent( + id: 'hidden-message', + pubkey: alice, + createdAt: 1_700_000_000, + kind: EventKind.streamMessageV2, + tags: [ + ['p', self], + ['h', hiddenTarget], + ], + content: 'Hello again', + sig: '', + ), + ); + + await _waitFor(() => reopened.isNotEmpty); + expect(reopened.single, [alice]); + }, + ); }); }