From 5230819bf636c4cb92ce8e047434830c575d73ab Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Fri, 12 Dec 2025 20:23:43 -0700 Subject: [PATCH 1/3] fix(desktop): fix notification click not opening workspace/tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract NotificationIds interface for shared type across notification system - Add resolveNotificationTarget utility to resolve missing IDs from state - Refactor useAgentHookListener to use shared resolution logic - Handle cases where paneId or tabId is missing by looking up from state - Add unit tests for notification target resolution 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../src/lib/trpc/routers/notifications.ts | 19 +- .../src/main/lib/notifications/server.ts | 5 +- apps/desktop/src/main/windows/main.ts | 1 - .../stores/tabs/useAgentHookListener.ts | 46 +++-- .../utils/resolve-notification-target.test.ts | 169 ++++++++++++++++++ .../tabs/utils/resolve-notification-target.ts | 44 +++++ 6 files changed, 244 insertions(+), 40 deletions(-) create mode 100644 apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.test.ts create mode 100644 apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts diff --git a/apps/desktop/src/lib/trpc/routers/notifications.ts b/apps/desktop/src/lib/trpc/routers/notifications.ts index 928b1de61a9..8b0f87e1086 100644 --- a/apps/desktop/src/lib/trpc/routers/notifications.ts +++ b/apps/desktop/src/lib/trpc/routers/notifications.ts @@ -1,33 +1,24 @@ import { observable } from "@trpc/server/observable"; import { type AgentCompleteEvent, + type NotificationIds, notificationsEmitter, } from "main/lib/notifications/server"; import { publicProcedure, router } from ".."; type NotificationEvent = | { type: "agent-complete"; data: AgentCompleteEvent } - | { - type: "focus-tab"; - data: { paneId: string; tabId: string; workspaceId: string }; - }; + | { type: "focus-tab"; data: NotificationIds }; export const createNotificationsRouter = () => { return router({ - /** - * Subscribe to notification events (completions and focus requests). - */ subscribe: publicProcedure.subscription(() => { return observable((emit) => { - const onComplete = (event: AgentCompleteEvent) => { - emit.next({ type: "agent-complete", data: event }); + const onComplete = (data: AgentCompleteEvent) => { + emit.next({ type: "agent-complete", data }); }; - const onFocusTab = (data: { - paneId: string; - tabId: string; - workspaceId: string; - }) => { + const onFocusTab = (data: NotificationIds) => { emit.next({ type: "focus-tab", data }); }; diff --git a/apps/desktop/src/main/lib/notifications/server.ts b/apps/desktop/src/main/lib/notifications/server.ts index a0ffd099835..c40eb9e7048 100644 --- a/apps/desktop/src/main/lib/notifications/server.ts +++ b/apps/desktop/src/main/lib/notifications/server.ts @@ -1,10 +1,13 @@ import { EventEmitter } from "node:events"; import express from "express"; -export interface AgentCompleteEvent { +export interface NotificationIds { paneId?: string; tabId?: string; workspaceId?: string; +} + +export interface AgentCompleteEvent extends NotificationIds { eventType: "Stop" | "PermissionRequest"; } diff --git a/apps/desktop/src/main/windows/main.ts b/apps/desktop/src/main/windows/main.ts index e374799c5ff..6c563fe6857 100644 --- a/apps/desktop/src/main/windows/main.ts +++ b/apps/desktop/src/main/windows/main.ts @@ -134,7 +134,6 @@ export async function MainWindow() { notification.on("click", () => { window.show(); window.focus(); - // Request focus on the specific pane notificationsEmitter.emit("focus-tab", { paneId: event.paneId, tabId: event.tabId, diff --git a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts index 5cddfc90414..38a80a889f5 100644 --- a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts +++ b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts @@ -2,6 +2,7 @@ import { trpc } from "renderer/lib/trpc"; import { useSetActiveWorkspace } from "renderer/react-query/workspaces/useSetActiveWorkspace"; import { useAppStore } from "../app-state"; import { useTabsStore } from "./store"; +import { resolveNotificationTarget } from "./utils/resolve-notification-target"; /** * Hook that listens for notification events via tRPC subscription. @@ -13,15 +14,14 @@ export function useAgentHookListener() { trpc.notifications.subscribe.useSubscription(undefined, { onData: (event) => { - if (event.type === "agent-complete") { - const { paneId, workspaceId } = event.data; - if (!paneId || !workspaceId) return; + const state = useTabsStore.getState(); + const target = resolveNotificationTarget(event.data, state); + if (!target) return; - const state = useTabsStore.getState(); + const { paneId, tabId, workspaceId } = target; - // Find the tab containing this pane - const pane = state.panes[paneId]; - if (!pane) return; + if (event.type === "agent-complete") { + if (!paneId) return; // Only show red dot if not already viewing this pane const activeTabId = state.activeTabIds[workspaceId]; @@ -33,36 +33,34 @@ export function useAgentHookListener() { state.setNeedsAttention(paneId, true); } } else if (event.type === "focus-tab") { - const { paneId, workspaceId } = event.data; - if (!paneId || !workspaceId) return; - // Switch to workspace view if not already there const appState = useAppStore.getState(); if (appState.currentView !== "workspace") { appState.setView("workspace"); } - // Switch to the workspace first, then look up pane/tab from fresh state + // Switch to the workspace first, then focus tab/pane setActiveWorkspace.mutate( { id: workspaceId }, { onSuccess: () => { - // Get fresh state after workspace switch - const currentState = useTabsStore.getState(); - - // Look up pane from current state - const pane = currentState.panes[paneId]; - if (!pane) return; + // Re-resolve from fresh state after workspace switch + const freshState = useTabsStore.getState(); + const freshTarget = resolveNotificationTarget(event.data, freshState); + if (!freshTarget?.tabId) return; - const tabId = pane.tabId; + const freshTab = freshState.tabs.find( + (t) => t.id === freshTarget.tabId, + ); + if (!freshTab || freshTab.workspaceId !== workspaceId) return; - // Validate tab belongs to the target workspace - const tab = currentState.tabs.find((t) => t.id === tabId); - if (!tab || tab.workspaceId !== workspaceId) return; + // Set active tab + freshState.setActiveTab(workspaceId, freshTarget.tabId); - // Set active tab and focused pane - currentState.setActiveTab(workspaceId, tabId); - currentState.setFocusedPane(tabId, paneId); + // Focus the pane if it exists + if (freshTarget.paneId && freshState.panes[freshTarget.paneId]) { + freshState.setFocusedPane(freshTarget.tabId, freshTarget.paneId); + } }, }, ); diff --git a/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.test.ts b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.test.ts new file mode 100644 index 00000000000..309d5c78d20 --- /dev/null +++ b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.test.ts @@ -0,0 +1,169 @@ +import { describe, expect, it } from "bun:test"; +import { resolveNotificationTarget } from "./resolve-notification-target"; + +describe("resolveNotificationTarget", () => { + const createPane = (id: string, tabId: string) => ({ + id, + tabId, + type: "terminal" as const, + name: "Terminal", + }); + + const createTab = (id: string, workspaceId: string) => ({ + id, + name: "Tab", + workspaceId, + createdAt: Date.now(), + layout: id, + }); + + describe("with all IDs provided", () => { + it("returns the provided IDs", () => { + const state = { + panes: { "pane-1": createPane("pane-1", "tab-1") }, + tabs: [createTab("tab-1", "ws-1")], + }; + + const result = resolveNotificationTarget( + { paneId: "pane-1", tabId: "tab-1", workspaceId: "ws-1" }, + state, + ); + + expect(result).toEqual({ + paneId: "pane-1", + tabId: "tab-1", + workspaceId: "ws-1", + }); + }); + }); + + describe("with missing workspaceId", () => { + it("resolves workspaceId from tab", () => { + const state = { + panes: { "pane-1": createPane("pane-1", "tab-1") }, + tabs: [createTab("tab-1", "ws-1")], + }; + + const result = resolveNotificationTarget( + { paneId: "pane-1", tabId: "tab-1" }, + state, + ); + + expect(result).toEqual({ + paneId: "pane-1", + tabId: "tab-1", + workspaceId: "ws-1", + }); + }); + }); + + describe("with missing tabId", () => { + it("resolves tabId from pane", () => { + const state = { + panes: { "pane-1": createPane("pane-1", "tab-1") }, + tabs: [createTab("tab-1", "ws-1")], + }; + + const result = resolveNotificationTarget( + { paneId: "pane-1", workspaceId: "ws-1" }, + state, + ); + + expect(result).toEqual({ + paneId: "pane-1", + tabId: "tab-1", + workspaceId: "ws-1", + }); + }); + }); + + describe("with missing tabId and workspaceId", () => { + it("resolves both from pane and tab chain", () => { + const state = { + panes: { "pane-1": createPane("pane-1", "tab-1") }, + tabs: [createTab("tab-1", "ws-1")], + }; + + const result = resolveNotificationTarget({ paneId: "pane-1" }, state); + + expect(result).toEqual({ + paneId: "pane-1", + tabId: "tab-1", + workspaceId: "ws-1", + }); + }); + }); + + describe("with only tabId", () => { + it("resolves workspaceId from tab", () => { + const state = { + panes: {}, + tabs: [createTab("tab-1", "ws-1")], + }; + + const result = resolveNotificationTarget({ tabId: "tab-1" }, state); + + expect(result).toEqual({ + paneId: undefined, + tabId: "tab-1", + workspaceId: "ws-1", + }); + }); + }); + + describe("with only workspaceId", () => { + it("returns workspaceId with undefined pane and tab", () => { + const state = { + panes: {}, + tabs: [], + }; + + const result = resolveNotificationTarget({ workspaceId: "ws-1" }, state); + + expect(result).toEqual({ + paneId: undefined, + tabId: undefined, + workspaceId: "ws-1", + }); + }); + }); + + describe("with no resolvable workspaceId", () => { + it("returns null when no IDs provided", () => { + const state = { panes: {}, tabs: [] }; + + const result = resolveNotificationTarget({}, state); + + expect(result).toBeNull(); + }); + + it("returns null when pane not found", () => { + const state = { panes: {}, tabs: [] }; + + const result = resolveNotificationTarget({ paneId: "missing" }, state); + + expect(result).toBeNull(); + }); + + it("returns null when tab not found", () => { + const state = { panes: {}, tabs: [] }; + + const result = resolveNotificationTarget({ tabId: "missing" }, state); + + expect(result).toBeNull(); + }); + }); + + describe("with pane pointing to missing tab", () => { + it("returns null when tab not in state", () => { + const state = { + panes: { "pane-1": createPane("pane-1", "missing-tab") }, + tabs: [], + }; + + const result = resolveNotificationTarget({ paneId: "pane-1" }, state); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts new file mode 100644 index 00000000000..96f8bb47a4a --- /dev/null +++ b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts @@ -0,0 +1,44 @@ +import type { NotificationIds } from "main/lib/notifications/server"; +import type { Pane, Tab } from "../types"; + +interface TabsState { + panes: Record; + tabs: Tab[]; +} + +interface ResolvedTarget extends NotificationIds { + workspaceId: string; // Required in resolved target +} + +/** + * Resolves notification target IDs by looking up missing values from state. + * Priority: event data > pane's tab > tab's workspace + */ +export function resolveNotificationTarget( + ids: NotificationIds, + state: TabsState, +): ResolvedTarget | null { + const { paneId, tabId, workspaceId } = ids; + + // Try to find pane from state + const pane = paneId ? state.panes[paneId] : undefined; + + // Resolve tabId: prefer pane's tabId, fallback to event tabId + const resolvedTabId = pane?.tabId ?? tabId; + + // Try to find tab from state + const tab = resolvedTabId + ? state.tabs.find((t) => t.id === resolvedTabId) + : undefined; + + // Resolve workspaceId: prefer event, fallback to tab's workspace + const resolvedWorkspaceId = workspaceId || tab?.workspaceId; + + if (!resolvedWorkspaceId) return null; + + return { + paneId, + tabId: resolvedTabId, + workspaceId: resolvedWorkspaceId, + }; +} From 71a2e3de6c5efadac0f69be88895782d56eaad8c Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 13 Dec 2025 12:11:42 -0800 Subject: [PATCH 2/3] fix --- .../src/renderer/stores/tabs/useAgentHookListener.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts index 38a80a889f5..2bac477999c 100644 --- a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts +++ b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts @@ -18,7 +18,7 @@ export function useAgentHookListener() { const target = resolveNotificationTarget(event.data, state); if (!target) return; - const { paneId, tabId, workspaceId } = target; + const { paneId, workspaceId } = target; if (event.type === "agent-complete") { if (!paneId) return; @@ -46,7 +46,10 @@ export function useAgentHookListener() { onSuccess: () => { // Re-resolve from fresh state after workspace switch const freshState = useTabsStore.getState(); - const freshTarget = resolveNotificationTarget(event.data, freshState); + const freshTarget = resolveNotificationTarget( + event.data, + freshState, + ); if (!freshTarget?.tabId) return; const freshTab = freshState.tabs.find( @@ -59,7 +62,10 @@ export function useAgentHookListener() { // Focus the pane if it exists if (freshTarget.paneId && freshState.panes[freshTarget.paneId]) { - freshState.setFocusedPane(freshTarget.tabId, freshTarget.paneId); + freshState.setFocusedPane( + freshTarget.tabId, + freshTarget.paneId, + ); } }, }, From 352cb2041a3c3d133cb1fc315a23042f8c781268 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 13 Dec 2025 12:14:06 -0800 Subject: [PATCH 3/3] deslop --- apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts | 4 ---- .../renderer/stores/tabs/utils/resolve-notification-target.ts | 2 -- 2 files changed, 6 deletions(-) diff --git a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts index 2bac477999c..2bb887208a9 100644 --- a/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts +++ b/apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts @@ -39,12 +39,10 @@ export function useAgentHookListener() { appState.setView("workspace"); } - // Switch to the workspace first, then focus tab/pane setActiveWorkspace.mutate( { id: workspaceId }, { onSuccess: () => { - // Re-resolve from fresh state after workspace switch const freshState = useTabsStore.getState(); const freshTarget = resolveNotificationTarget( event.data, @@ -57,10 +55,8 @@ export function useAgentHookListener() { ); if (!freshTab || freshTab.workspaceId !== workspaceId) return; - // Set active tab freshState.setActiveTab(workspaceId, freshTarget.tabId); - // Focus the pane if it exists if (freshTarget.paneId && freshState.panes[freshTarget.paneId]) { freshState.setFocusedPane( freshTarget.tabId, diff --git a/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts index 96f8bb47a4a..b3224cb0b88 100644 --- a/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts +++ b/apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts @@ -20,13 +20,11 @@ export function resolveNotificationTarget( ): ResolvedTarget | null { const { paneId, tabId, workspaceId } = ids; - // Try to find pane from state const pane = paneId ? state.panes[paneId] : undefined; // Resolve tabId: prefer pane's tabId, fallback to event tabId const resolvedTabId = pane?.tabId ?? tabId; - // Try to find tab from state const tab = resolvedTabId ? state.tabs.find((t) => t.id === resolvedTabId) : undefined;