From 7e8001b1010610ae69571694e57010bb0076b018 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Sat, 18 Jul 2026 20:29:54 -0400 Subject: [PATCH 1/2] fix(desktop): stop DMs from firing duplicate desktop notifications An incoming DM notified twice: once via the live WebSocket path (useUnreadChannels -> handleDmNotification) and once via the home-feed polling path (useFeedDesktopNotifications). The intended DM-exclusion guard in eligibleFeedNotificationItems filtered on item.channelType !== "dm", but the backend feed emits channel_type: None and channel_name: "" (feed_item_from_event), so every DM passed the guard as undefined !== "dm". Channel enrichment ran only after eligibility, too late to rescue the filter. Fix at the eligibility seam: - enrichFeedItemChannel now fills channelName and channelType independently instead of early-returning whenever a name is present. - eligibleFeedNotificationItems takes the loaded channel list and enriches mention and needs-action items BEFORE the DM filter. - useFeedDesktopNotifications passes channels through and drops the now-redundant post-hoc enrichment in the notify loop. Failover invariant preserved: a known DM notifies via WS only; a DM in a channel not yet in the channel list stays feed-eligible (the WS path also bails on unknown channels), so no notification is lost. Adds unit coverage for the three guard cases and a relay-mode Playwright spec proving exactly one notification per incoming DM (red=2 before the fix, green=1 after, 3/3 repeats). Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/playwright.config.ts | 1 + .../features/notifications/lib/feed.test.mjs | 66 +++++++++- .../src/features/notifications/lib/feed.ts | 26 +++- .../use-feed-desktop-notifications.ts | 16 +-- .../tests/e2e/dm-double-notification.spec.ts | 117 ++++++++++++++++++ 5 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 desktop/tests/e2e/dm-double-notification.spec.ts 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/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"); +}); From aa8664895dd9d25fe0b862a1a22b5aef696f70f3 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Sat, 18 Jul 2026 22:10:05 -0400 Subject: [PATCH 2/2] fix(desktop): canonicalize null feed channel_type at the API boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live testing the DM dedupe fix against the native Tauri binary still produced two toasts for a plain DM. Root cause: native get_feed serializes FeedItemInfo.channel_type (Option) as null — the key is always present — while the enrichment guard in enrichFeedItemChannel tests channelType === undefined. null !== undefined, so DM feed items were never enriched and null !== "dm" sailed through the exclusion filter. The relay-mode e2e bridge omitted the key entirely (undefined), which is why the Playwright spec went green while the real binary stayed red. Fix at one seam: RawFeedItem.channel_type is now honestly string | null, and fromRawFeedItem canonicalizes null to undefined so FeedItem's declared optional contract holds at runtime everywhere downstream. The e2e bridge feed now emits channel_type: null exactly like native serde, so the existing dm-double-notification spec exercises the native wire shape: with canonicalization removed (via a cast — the null-honest type makes the naive regression a compile error), the spec fails with a duplicate toast; with it, green. Adds unit coverage for the null and present channel_type shapes. The leaked toast titled 'Needs Action in #alice-tyler' was a mention item, not a needs_action item: notificationTitle compares category === "mention" but native emits "mentions", so mention items fall through to the fallback title. That plural/singular category contract bug is broader than this fix and is filed separately. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/shared/api/tauri.ts | 11 +++++-- desktop/src/shared/api/tauriFeed.test.mjs | 35 +++++++++++++++++++++++ desktop/src/testing/e2eBridge.ts | 9 +++++- 3 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 desktop/src/shared/api/tauriFeed.test.mjs 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, }; });