diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index ae8717364b..5b9b96468c 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -124,6 +124,7 @@ export default defineConfig({ "**/onboarding.spec.ts", "**/stream.spec.ts", "**/integration.spec.ts", + "**/dm-double-notification.spec.ts", "**/profile.spec.ts", "**/sidebar.spec.ts", "**/sidebar-relay-card.spec.ts", diff --git a/desktop/src/features/notifications/lib/feed.test.mjs b/desktop/src/features/notifications/lib/feed.test.mjs index 4f1c40f55e..d4903241d4 100644 --- a/desktop/src/features/notifications/lib/feed.test.mjs +++ b/desktop/src/features/notifications/lib/feed.test.mjs @@ -1,7 +1,11 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { enrichFeedItemChannel, notificationTitle } from "./feed.ts"; +import { + eligibleFeedNotificationItems, + enrichFeedItemChannel, + notificationTitle, +} from "./feed.ts"; const feedItem = (overrides = {}) => ({ id: "event-id", @@ -56,3 +60,63 @@ test("does not replace direct-message notification titles", () => { assert.equal(notificationTitle(item, "Taylor"), "Taylor"); }); + +const feedResponse = (mentions, needsAction = []) => ({ + feed: { mentions, needsAction }, +}); + +const allSlots = { mentions: true, needsAction: true }; + +test("excludes a DM mention whose feed item is missing channel metadata but whose channel is loaded", () => { + // The backend feed emits channel_type: None and channel_name: "" for DMs; + // the DM-exclusion guard must resolve the type from the channel list + // BEFORE filtering, or every DM double-notifies alongside the WS path. + const dmItem = feedItem({ + category: "mention", + channelId: "dm-channel", + channelName: "", + channelType: undefined, + }); + const items = eligibleFeedNotificationItems( + feedResponse([dmItem]), + allSlots, + [{ id: "dm-channel", name: "alice-tyler", channelType: "dm" }], + ); + + assert.equal(items.length, 0); +}); + +test("keeps a mention eligible when its channel is not in the loaded channel list", () => { + // Unknown DM channels get no WS notification (handleDmEvent bails on + // unknown channels), so the feed path must remain the failover. + const unknownItem = feedItem({ + category: "mention", + channelId: "brand-new-dm", + channelName: "", + channelType: undefined, + }); + const items = eligibleFeedNotificationItems( + feedResponse([unknownItem]), + allSlots, + [{ id: "some-other-channel", name: "general", channelType: "stream" }], + ); + + assert.equal(items.length, 1); + assert.equal(items[0].id, unknownItem.id); +}); + +test("resolves and excludes a DM whose feed item has a name but no type", () => { + const namedItem = feedItem({ + category: "mention", + channelId: "dm-channel", + channelName: "alice-tyler", + channelType: undefined, + }); + const items = eligibleFeedNotificationItems( + feedResponse([namedItem]), + allSlots, + [{ id: "dm-channel", name: "alice-tyler", channelType: "dm" }], + ); + + assert.equal(items.length, 0); +}); diff --git a/desktop/src/features/notifications/lib/feed.ts b/desktop/src/features/notifications/lib/feed.ts index 85a0680ad1..f8413c4313 100644 --- a/desktop/src/features/notifications/lib/feed.ts +++ b/desktop/src/features/notifications/lib/feed.ts @@ -10,7 +10,9 @@ export function enrichFeedItemChannel( item: FeedItem, channels: readonly NotificationChannel[], ): FeedItem { - if (!item.channelId || item.channelName.trim()) { + const needsName = !item.channelName.trim(); + const needsType = item.channelType === undefined; + if (!item.channelId || (!needsName && !needsType)) { return item; } @@ -19,10 +21,13 @@ export function enrichFeedItemChannel( return item; } + // Fill each missing field independently: the backend feed may supply a + // channel name but no type (or vice versa), and the DM-exclusion filter in + // eligibleFeedNotificationItems depends on channelType being resolved. return { ...item, - channelName: channel.name, - channelType: item.channelType ?? channel.channelType, + channelName: needsName ? channel.name : item.channelName, + channelType: needsType ? channel.channelType : item.channelType, }; } @@ -73,19 +78,28 @@ export function collectHomeAlertItems(feed: HomeFeedResponse) { export function eligibleFeedNotificationItems( feed: HomeFeedResponse, options: { mentions: boolean; needsAction: boolean }, + channels: readonly NotificationChannel[] = [], ) { const items: FeedItem[] = []; // DM notifications are handled by the real-time WebSocket hook, so we - // exclude DM items here to avoid duplicate toasts. + // exclude DM items here to avoid duplicate toasts. The backend feed emits + // no channelType, so resolve it from the loaded channel list BEFORE + // filtering — otherwise every DM sails through as `undefined !== "dm"`. if (options.mentions) { items.push( - ...feed.feed.mentions.filter((item) => item.channelType !== "dm"), + ...feed.feed.mentions + .map((item) => enrichFeedItemChannel(item, channels)) + .filter((item) => item.channelType !== "dm"), ); } if (options.needsAction) { - items.push(...feed.feed.needsAction); + items.push( + ...feed.feed.needsAction.map((item) => + enrichFeedItemChannel(item, channels), + ), + ); } return items.sort((left, right) => left.createdAt - right.createdAt); diff --git a/desktop/src/features/notifications/use-feed-desktop-notifications.ts b/desktop/src/features/notifications/use-feed-desktop-notifications.ts index 501a8fb65b..e7b4d726a3 100644 --- a/desktop/src/features/notifications/use-feed-desktop-notifications.ts +++ b/desktop/src/features/notifications/use-feed-desktop-notifications.ts @@ -10,7 +10,6 @@ import type { FeedItem, HomeFeedResponse } from "@/shared/api/types"; import { collectHomeAlertItems, eligibleFeedNotificationItems, - enrichFeedItemChannel, type NotificationChannel, notificationBody, notificationTitle, @@ -160,10 +159,14 @@ export function useFeedDesktopNotifications( const nextSeenItemIds = new Set(seenItemIdsRef.current); const newItems = settings.desktopEnabled - ? eligibleFeedNotificationItems(feed, { - mentions: settings.slotAlertsEnabled.mention, - needsAction: settings.slotAlertsEnabled.needs_action, - }) + ? eligibleFeedNotificationItems( + feed, + { + mentions: settings.slotAlertsEnabled.mention, + needsAction: settings.slotAlertsEnabled.needs_action, + }, + channels, + ) .filter((item) => !nextSeenItemIds.has(item.id)) .filter( (item) => @@ -195,8 +198,7 @@ export function useFeedDesktopNotifications( void autoRequestPermissionIfNeeded(); } - for (const rawItem of newItems) { - const item = enrichFeedItemChannel(rawItem, channels); + for (const item of newItems) { const resolvedLabel = profiles ? resolveUserLabel({ pubkey: item.pubkey, diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 75835d509a..0898016132 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -54,7 +54,9 @@ type RawFeedItem = { created_at: number; channel_id: string | null; channel_name: string; - channel_type: string; + // Native FeedItemInfo.channel_type is Option: serde emits `null`, + // never omits the key. + channel_type: string | null; tags: string[][]; category: "mention" | "needs_action" | "activity" | "agent_activity"; }; @@ -275,7 +277,7 @@ export async function invokeTauri( } } -function fromRawFeedItem(item: RawFeedItem) { +export function fromRawFeedItem(item: RawFeedItem) { return { id: item.id, kind: item.kind, @@ -284,7 +286,10 @@ function fromRawFeedItem(item: RawFeedItem) { createdAt: item.created_at, channelId: item.channel_id, channelName: item.channel_name, - channelType: item.channel_type, + // Canonicalize the wire `null` to undefined so FeedItem's optional + // channelType contract holds at runtime (enrichment and the DM + // notification filter both key off `=== undefined`). + channelType: item.channel_type ?? undefined, tags: item.tags, category: item.category, }; diff --git a/desktop/src/shared/api/tauriFeed.test.mjs b/desktop/src/shared/api/tauriFeed.test.mjs new file mode 100644 index 0000000000..5e73988d87 --- /dev/null +++ b/desktop/src/shared/api/tauriFeed.test.mjs @@ -0,0 +1,35 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { fromRawFeedItem } from "./tauri.ts"; + +const rawFeedItem = (overrides = {}) => ({ + id: "event-id", + kind: 9, + pubkey: "author", + content: "hello", + created_at: 1, + channel_id: "channel-id", + channel_name: "", + channel_type: null, + tags: [["h", "channel-id"]], + category: "mention", + ...overrides, +}); + +test("canonicalizes the native null channel_type to undefined", () => { + // Native get_feed serializes FeedItemInfo.channel_type (Option) + // as `null`, never omitting the key. FeedItem declares channelType as + // optional, and the DM notification filter distinguishes "unresolved" + // via `=== undefined` — so null must not survive the boundary. + const item = fromRawFeedItem(rawFeedItem()); + + assert.equal(item.channelType, undefined); + assert.ok("channelType" in item); +}); + +test("passes a present channel_type through unchanged", () => { + const item = fromRawFeedItem(rawFeedItem({ channel_type: "dm" })); + + assert.equal(item.channelType, "dm"); +}); diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 03513e67e7..de9190f284 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -484,7 +484,9 @@ type RawFeedItem = { created_at: number; channel_id: string | null; channel_name: string; - channel_type?: string; + // Mirrors native FeedItemInfo.channel_type (Option): the Tauri + // backend always emits the key, as `null` when unknown. + channel_type?: string | null; tags: string[][]; category: "mention" | "needs_action" | "activity" | "agent_activity"; }; @@ -6565,6 +6567,11 @@ async function handleGetFeed( tags: (ev.tags ?? []) as string[][], channel_id: chId, channel_name: chId ? (channelNameMap.get(chId) ?? "") : "", + // Native-shaped: get_feed emits channel_type: null (Option), + // never omits the key. Keeping the bridge faithful here is what lets + // the DM dedupe e2e catch null-vs-undefined regressions at the API + // conversion seam. + channel_type: null, category: "mention" as const, }; }); diff --git a/desktop/tests/e2e/dm-double-notification.spec.ts b/desktop/tests/e2e/dm-double-notification.spec.ts new file mode 100644 index 0000000000..0678fff913 --- /dev/null +++ b/desktop/tests/e2e/dm-double-notification.spec.ts @@ -0,0 +1,117 @@ +import { expect, test } from "@playwright/test"; +import { finalizeEvent } from "nostr-tools/pure"; +import { hexToBytes } from "@noble/hashes/utils.js"; + +import { installRelayBridge, TEST_IDENTITIES } from "../helpers/bridge"; +import { assertRelaySeeded } from "../helpers/seed"; + +const isCi = Boolean(process.env.CI); +const relaySeedHookTimeoutMs = isCi ? 90_000 : 30_000; + +const RELAY_HTTP_URL = + process.env.BUZZ_E2E_RELAY_URL ?? "http://localhost:3000"; + +// setup-desktop-test-data.sh: uuid5(NAMESPACE_DNS, "buzz.channel.dm.alice-tyler") +const ALICE_TYLER_DM_CHANNEL_ID = "5a9c064e-0411-5242-ae6b-0363ba99b8e6"; + +async function getLoggedNotifications(page: import("@playwright/test").Page) { + return page.evaluate(() => { + const win = window as Window & { + __BUZZ_E2E_NOTIFICATIONS__?: Array<{ + body: string | null; + title: string; + }>; + }; + + return win.__BUZZ_E2E_NOTIFICATIONS__ ?? []; + }); +} + +/** + * Publishes a REAL signed DM message from alice through the relay ingest + * path. Like every DM send in the product, it carries a recipient `p` tag + * (see messageMentionPubkeys) — which is exactly what makes the event match + * both the live DM subscription and the home-feed mention query. + */ +async function publishAliceDm(content: string) { + const event = finalizeEvent( + { + kind: 9, + content, + tags: [ + ["h", ALICE_TYLER_DM_CHANNEL_ID], + ["p", TEST_IDENTITIES.tyler.pubkey], + ], + created_at: Math.floor(Date.now() / 1000), + }, + hexToBytes(TEST_IDENTITIES.alice.privateKey), + ); + + const response = await fetch(`${RELAY_HTTP_URL}/events`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-Pubkey": event.pubkey, + }, + body: JSON.stringify(event), + }); + if (!response.ok) { + throw new Error( + `POST /events failed (${response.status}): ${await response.text()}`, + ); + } +} + +test.beforeAll(async () => { + test.setTimeout(relaySeedHookTimeoutMs); + await assertRelaySeeded(); +}); + +test("an incoming DM produces exactly one desktop notification", async ({ + page, +}) => { + await installRelayBridge(page, "tyler"); + + // Deterministically wait for the home feed's initial mention query + // (kinds 9/... + #p filter) to complete before publishing: the feed + // dedupe-seen set must be initialized, otherwise the duplicate is + // accidentally swallowed as "initial backlog" and the repro goes flaky. + const feedInitialized = page.waitForResponse( + (response) => + response.url().includes("/query") && + (response.request().postData() ?? "").includes('"#p"'), + { timeout: 15_000 }, + ); + await page.goto("/"); + + // Wait until the DM channel is loaded — live subscriptions (channel + home + // feed mention) are established once the channel list resolves. + await expect(page.getByTestId("dm-list")).toContainText("alice-tyler"); + await feedInitialized; + // Small buffer for the feed effect (seen-set initialization) to run. + await page.waitForTimeout(1_000); + + const message = `dm dedupe probe ${Date.now()}`; + await publishAliceDm(message); + + // Wait for the DM toast to arrive. + await expect + .poll(async () => (await getLoggedNotifications(page)).length, { + timeout: 15_000, + }) + .toBeGreaterThan(0); + + // Give the duplicate (home-feed mention path) time to fire — it arrives via + // the onLiveMention → feed refetch round trip, which lags the WS toast. + await page.waitForTimeout(5_000); + + const notifications = await getLoggedNotifications(page); + expect( + notifications, + `expected exactly one notification for a single DM, got: ${JSON.stringify(notifications)}`, + ).toHaveLength(1); + + // The survivor must be the live WebSocket DM toast (titled with the DM + // channel name), not the home-feed mention duplicate ("… mentioned you in …"). + expect(notifications[0].title).toBe("alice-tyler"); +});