From 84fedd5924cd3cd6930f78d48104944e9162b8dc Mon Sep 17 00:00:00 2001 From: Jiang Bohan Date: Wed, 22 Apr 2026 15:04:18 +0800 Subject: [PATCH] fix(desktop): pin notification routing to source workspace + mark read on URL select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs GPT-Boy caught on PR #1445: 1. A notification from workspace A used `getCurrentSlug()` at click time, so if the user switched to workspace B before clicking the banner (macOS Notification Center persists banners), routing landed on `/B/inbox?issue=` and 404'd. Fix: round-trip the emit-time `slug` through the IPC payload and use it in the click handler. 2. Notification-click navigation set the URL param but never fired the mark-read mutation (only InboxPage's click-handler did). The row stayed unread and the dock badge didn't decrement. Fix: move the mark-read logic from handleSelect into a useEffect keyed on the selected item — it now covers both click-to-select and URL-param-select. IPC payload gains `slug` and `itemId`; preload types + main handler + the desktop bridge are updated to match. --- apps/desktop/src/main/index.ts | 19 +++++++++++-- apps/desktop/src/preload/index.d.ts | 10 ++++++- apps/desktop/src/preload/index.ts | 27 ++++++++++++++----- .../src/components/desktop-layout.tsx | 14 ++++++---- packages/core/realtime/use-realtime-sync.ts | 18 ++++++++++--- .../views/inbox/components/inbox-page.tsx | 21 ++++++++++----- 6 files changed, 84 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 862bf0d11..2761f9e06 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -211,10 +211,18 @@ if (!gotTheLock) { ( _event, { + slug, + itemId, issueKey, title, body, - }: { issueKey: string; title: string; body: string }, + }: { + slug: string; + itemId: string; + issueKey: string; + title: string; + body: string; + }, ) => { if (!Notification.isSupported()) return; const notification = new Notification({ title, body }); @@ -223,7 +231,14 @@ if (!gotTheLock) { if (mainWindow.isMinimized()) mainWindow.restore(); mainWindow.show(); mainWindow.focus(); - mainWindow.webContents.send("inbox:open", issueKey); + // Ship the full context back — the renderer pins the route to the + // source workspace (slug), marks the row read (itemId), and uses + // issueKey as the ?issue=<…> selector. + mainWindow.webContents.send("inbox:open", { + slug, + itemId, + issueKey, + }); }); notification.show(); }, diff --git a/apps/desktop/src/preload/index.d.ts b/apps/desktop/src/preload/index.d.ts index f8592e336..245966acd 100644 --- a/apps/desktop/src/preload/index.d.ts +++ b/apps/desktop/src/preload/index.d.ts @@ -11,6 +11,8 @@ interface DesktopAPI { setImmersiveMode: (immersive: boolean) => Promise; /** Show a native OS notification for a new inbox item. */ showNotification: (payload: { + slug: string; + itemId: string; issueKey: string; title: string; body: string; @@ -18,7 +20,13 @@ interface DesktopAPI { /** Update the OS dock / taskbar unread badge. Pass 0 to clear. */ setUnreadBadge: (count: number) => void; /** Listen for "open inbox row" requests from notification clicks. Returns an unsubscribe function. */ - onInboxOpen: (callback: (issueKey: string) => void) => () => void; + onInboxOpen: ( + callback: (payload: { + slug: string; + itemId: string; + issueKey: string; + }) => void, + ) => () => void; } interface DaemonStatus { diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 33be4f8c9..6cb92a7e6 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -28,10 +28,15 @@ const desktopAPI = { /** * Show a native OS notification for a new inbox item. Fired from the * renderer only when the app is unfocused — in-focus feedback is the - * inbox sidebar's unread styling. `issueKey` is round-tripped on click so - * the main process can route the user back to the exact inbox row. + * inbox sidebar's unread styling. `slug`, `itemId`, and `issueKey` are + * all round-tripped on click: slug pins routing to the source workspace + * (the user may switch workspaces before clicking the banner), itemId + * lets the renderer mark the row read, issueKey maps to the inbox URL + * param. */ showNotification: (payload: { + slug: string; + itemId: string; issueKey: string; title: string; body: string; @@ -45,12 +50,20 @@ const desktopAPI = { /** * Subscribe to "open this inbox row" requests sent by the main process * when the user clicks an OS notification banner. Returns an unsubscribe - * function. The payload is the same `issueKey` that was passed to - * `showNotification`. + * function. The payload echoes the `slug`, `itemId`, and `issueKey` that + * were passed to `showNotification`. */ - onInboxOpen: (callback: (issueKey: string) => void) => { - const handler = (_event: Electron.IpcRendererEvent, issueKey: string) => - callback(issueKey); + onInboxOpen: ( + callback: (payload: { + slug: string; + itemId: string; + issueKey: string; + }) => void, + ) => { + const handler = ( + _event: Electron.IpcRendererEvent, + payload: { slug: string; itemId: string; issueKey: string }, + ) => callback(payload); ipcRenderer.on("inbox:open", handler); return () => { ipcRenderer.removeListener("inbox:open", handler); diff --git a/apps/desktop/src/renderer/src/components/desktop-layout.tsx b/apps/desktop/src/renderer/src/components/desktop-layout.tsx index 9765ffe67..acf31c091 100644 --- a/apps/desktop/src/renderer/src/components/desktop-layout.tsx +++ b/apps/desktop/src/renderer/src/components/desktop-layout.tsx @@ -100,20 +100,24 @@ function useInternalLinkHandler() { /** * Bridge between the renderer and the Electron main process for inbox-level * OS integration. Mounted inside WorkspaceSlugProvider so it can resolve the - * current workspace's id for the badge hook and its slug for click-routing. + * current workspace's id for the badge hook. * * Two responsibilities: * 1. Mirror the unread inbox count onto the dock/taskbar badge. - * 2. When the user clicks an OS notification, open a new tab on the - * workspace's inbox focused on the notified item. + * 2. When the user clicks an OS notification, open the notified + * workspace's inbox focused on that item. The route uses the `slug` + * that the notification was *emitted* with — not the currently active + * workspace — so a notification from workspace A always opens A's + * inbox even if the user has since switched to workspace B. Marking + * the row read is handled by InboxPage's selected-item effect, which + * covers both click-to-select and URL-param-select paths. */ function DesktopInboxBridge() { const workspace = useCurrentWorkspace(); useDesktopUnreadBadge(workspace?.id ?? null); useEffect(() => { - return window.desktopAPI.onInboxOpen((issueKey) => { - const slug = getCurrentSlug(); + return window.desktopAPI.onInboxOpen(({ slug, issueKey }) => { if (!slug) return; const inboxPath = `${paths.workspace(slug).inbox()}?issue=${encodeURIComponent(issueKey)}`; window.dispatchEvent( diff --git a/packages/core/realtime/use-realtime-sync.ts b/packages/core/realtime/use-realtime-sync.ts index aabeb072e..c595f8ce8 100644 --- a/packages/core/realtime/use-realtime-sync.ts +++ b/packages/core/realtime/use-realtime-sync.ts @@ -199,10 +199,19 @@ export function useRealtimeSync( // styling is enough — no need to interrupt with a banner. `desktopAPI` // is injected by the preload script; its absence (web app) skips silently. if (typeof document !== "undefined" && document.hasFocus()) return; + // Capture the source workspace slug at emit time. The user may switch + // workspaces before clicking the banner (macOS Notification Center + // holds banners), so routing must not read "current slug" at click + // time — otherwise notifications from workspace A click through to + // workspace B's inbox and 404. + const slug = getCurrentSlug(); + if (!slug) return; const desktopAPI = ( window as unknown as { desktopAPI?: { showNotification?: (payload: { + slug: string; + itemId: string; issueKey: string; title: string; body: string; @@ -210,11 +219,12 @@ export function useRealtimeSync( }; } ).desktopAPI; - // `issueKey` matches the inbox page's selector: it's the issue id when - // the item is attached to an issue, otherwise the inbox item id. The - // click handler in the main process round-trips this back to the - // renderer so `/inbox?issue=` selects the correct row. + // `issueKey` matches the inbox page's URL selector (issue id when the + // item is attached to an issue, otherwise the inbox item id). `itemId` + // is the inbox row's own id, needed to fire markInboxRead on click. desktopAPI?.showNotification?.({ + slug, + itemId: item.id, issueKey: item.issue_id ?? item.id, title: item.title, body: item.body ?? "", diff --git a/packages/views/inbox/components/inbox-page.tsx b/packages/views/inbox/components/inbox-page.tsx index df7b0c3cf..f2589fc67 100644 --- a/packages/views/inbox/components/inbox-page.tsx +++ b/packages/views/inbox/components/inbox-page.tsx @@ -99,14 +99,23 @@ export function InboxPage() { const archiveAllReadMutation = useArchiveAllReadInbox(); const archiveCompletedMutation = useArchiveCompletedInbox(); - // Click-to-read: select + auto-mark-read + // Auto-mark-read whenever a selected item is unread — covers both click- + // to-select and URL-param-select (e.g. OS notification click on desktop). + // The mutation flips `read: true` optimistically, so this effect settles + // in one pass and can't loop. Kept in a `useEffect` rather than inlined + // in handleSelect so URL-driven selection triggers it too. + const markReadMutate = markReadMutation.mutate; + const selectedId = selected?.id; + const selectedRead = selected?.read; + useEffect(() => { + if (!selectedId || selectedRead) return; + markReadMutate(selectedId, { + onError: () => toast.error("Failed to mark as read"), + }); + }, [selectedId, selectedRead, markReadMutate]); + const handleSelect = (item: InboxItem) => { setSelectedKey(item.issue_id ?? item.id); - if (!item.read) { - markReadMutation.mutate(item.id, { - onError: () => toast.error("Failed to mark as read"), - }); - } }; const handleArchive = (id: string) => {