From e97b109aacdcb9855084db13bd188de035c25f29 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 19 May 2026 15:09:39 -0700 Subject: [PATCH] Focus terminal pane when notification is clicked (#2356) * fix: address review findings * increase highlight brightness --- src/main/ipc/notifications.test.ts | 56 +++++++++++++++++++ src/main/ipc/notifications.ts | 17 +++++- src/preload/api-types.ts | 8 ++- src/preload/index.ts | 16 +++++- src/renderer/src/assets/terminal.css | 12 ++-- .../terminal-pane/pty-connection.test.ts | 23 ++++++-- .../terminal-pane/pty-connection.ts | 2 +- .../use-notification-dispatch.ts | 1 + .../src/hooks/useAutoAckViewedAgent.test.ts | 22 ++++++++ src/renderer/src/hooks/useIpcEvents.ts | 34 +++++++---- src/shared/types.ts | 2 + 11 files changed, 165 insertions(+), 28 deletions(-) diff --git a/src/main/ipc/notifications.test.ts b/src/main/ipc/notifications.test.ts index 85087fd38..e01bb6576 100644 --- a/src/main/ipc/notifications.test.ts +++ b/src/main/ipc/notifications.test.ts @@ -113,6 +113,14 @@ describe('registerNotificationHandlers', () => { return call[1] as (event: unknown) => unknown } + function getNotificationEventHandler(eventName: string): () => void { + const call = notificationOnMock.mock.calls.find((c: unknown[]) => c[0] === eventName) + if (!call) { + throw new Error(`Notification ${eventName} handler not registered`) + } + return call[1] as () => void + } + it('registers the IPC handler', () => { registerNotificationHandlers({ getSettings: () => ({ @@ -199,6 +207,54 @@ describe('registerNotificationHandlers', () => { expect(notificationShowMock).toHaveBeenCalledTimes(1) }) + it('focuses the originating terminal pane when a notification with paneKey is clicked', () => { + const webContentsSend = vi.fn() + const restore = vi.fn() + const focus = vi.fn() + getAllWindowsMock.mockReturnValue([ + { + isDestroyed: () => false, + isFocused: () => false, + isMinimized: () => true, + restore, + focus, + webContents: { send: webContentsSend } + } as never + ]) + registerNotificationHandlers({ + getSettings: () => ({ + notifications: { + enabled: true, + agentTaskComplete: true, + terminalBell: true, + suppressWhenFocused: true + } + }) + } as never) + + const paneKey = 'tab-1:11111111-1111-4111-8111-111111111111' + const handler = getDispatchHandler() + expect( + handler({}, { source: 'agent-task-complete', worktreeId: 'repo::wt1', paneKey }) + ).toEqual({ delivered: true }) + + getNotificationEventHandler('click')() + + expect(restore).toHaveBeenCalledTimes(1) + expect(focus).toHaveBeenCalledTimes(1) + expect(webContentsSend).toHaveBeenCalledWith('ui:activateWorktree', { + repoId: 'repo', + worktreeId: 'repo::wt1' + }) + expect(webContentsSend).toHaveBeenCalledWith('ui:focusTerminal', { + tabId: 'tab-1', + worktreeId: 'repo::wt1', + leafId: '11111111-1111-4111-8111-111111111111', + ackPaneKeyOnSuccess: paneKey, + flashFocusedPane: true + }) + }) + it('formats agent-task-complete with the agent response when a status snapshot is present', () => { registerNotificationHandlers({ getSettings: () => ({ diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index 544859c3b..2eae2e4bb 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -11,6 +11,7 @@ import type { import { getRepoIdFromWorktreeId } from '../../shared/worktree-id' import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { buildNotificationOptions } from './notification-options' +import { parsePaneKey } from '../../shared/stable-pane-id' const NOTIFICATION_COOLDOWN_MS = 5000 const MAX_NOTIFICATION_SOUND_BYTES = 10 * 1024 * 1024 @@ -148,9 +149,9 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime setTimeout(release, 5 * 60 * 1000) // Why: clicking a notification should bring Orca to the foreground and - // switch to the worktree that triggered it. We reuse the existing - // ui:activateWorktree IPC channel that the renderer already handles - // (setActiveRepo, setActiveView, setActiveWorktree, revealInSidebar). + // switch to the worktree/pane that triggered it. Worktree activation owns + // repo/sidebar state; the optional focusTerminal follow-up uses the stable + // pane leaf id so split-pane notifications land on the exact pane. // Why: worktreeId is formatted as "repoId::worktreePath". If the // separator is missing we cannot reliably extract a repoId, so skip // the click-to-navigate binding — the notification still fires but @@ -174,6 +175,16 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime repoId, worktreeId: args.worktreeId }) + const paneTarget = args.paneKey ? parsePaneKey(args.paneKey) : null + if (paneTarget) { + win.webContents.send('ui:focusTerminal', { + tabId: paneTarget.tabId, + worktreeId: args.worktreeId, + leafId: paneTarget.leafId, + ackPaneKeyOnSuccess: args.paneKey, + flashFocusedPane: true + }) + } }) } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index c1fbb55cb..cab919c09 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1685,7 +1685,13 @@ export type PreloadApi = { callback: (data: { tabId: string; title: string | null }) => void ) => () => void onFocusTerminal: ( - callback: (data: { tabId: string; worktreeId: string; leafId?: string | null }) => void + callback: (data: { + tabId: string + worktreeId: string + leafId?: string | null + ackPaneKeyOnSuccess?: string + flashFocusedPane?: boolean + }) => void ) => () => void onFocusEditorTab: ( callback: (data: { tabId: string; worktreeId: string }) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index f25dd86f1..a8ca87f6c 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2427,11 +2427,23 @@ const api = { return () => ipcRenderer.removeListener('ui:renameTerminal', listener) }, onFocusTerminal: ( - callback: (data: { tabId: string; worktreeId: string; leafId?: string | null }) => void + callback: (data: { + tabId: string + worktreeId: string + leafId?: string | null + ackPaneKeyOnSuccess?: string + flashFocusedPane?: boolean + }) => void ): (() => void) => { const listener = ( _event: Electron.IpcRendererEvent, - data: { tabId: string; worktreeId: string; leafId?: string | null } + data: { + tabId: string + worktreeId: string + leafId?: string | null + ackPaneKeyOnSuccess?: string + flashFocusedPane?: boolean + } ) => callback(data) ipcRenderer.on('ui:focusTerminal', listener) return () => ipcRenderer.removeListener('ui:focusTerminal', listener) diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index e44529c7e..cb618ae83 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -134,13 +134,11 @@ inset: 1px; z-index: 30; pointer-events: none; - border: 1.5px solid color-mix(in srgb, var(--terminal-pane-locate) 52%, transparent); - border-right-color: color-mix(in srgb, var(--terminal-pane-locate) 68%, transparent); - border-left-color: color-mix(in srgb, var(--terminal-pane-locate) 68%, transparent); + border: 1.5px solid color-mix(in srgb, var(--terminal-pane-locate) 68%, transparent); border-radius: 2px; box-shadow: 0 0 0 2px color-mix(in srgb, var(--terminal-pane-locate) 34%, transparent), - 0 0 20px color-mix(in srgb, var(--terminal-pane-locate) 40%, transparent); + 0 0 18px color-mix(in srgb, var(--terminal-pane-locate) 36%, transparent); animation: pane-focus-rim-flash 1.5s ease-out forwards; } @@ -148,6 +146,12 @@ 0% { opacity: 1; } + 24% { + opacity: 0.42; + } + 46% { + opacity: 0; + } 100% { opacity: 0; } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 584e6c0da..a6cedd118 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -1292,7 +1292,12 @@ describe('connectPanePty', () => { expect(deps.markTerminalTabUnread).toHaveBeenCalledWith('tab-1') expect(deps.dispatchNotification).not.toHaveBeenCalled() vi.advanceTimersByTime(250) - expect(deps.dispatchNotification).toHaveBeenCalledWith({ source: 'terminal-bell' }) + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ + source: 'terminal-bell', + paneKey: makePaneKey('tab-1', LEAF_1) + }) + ) }) it('lets concurrent agent-complete notifications win over terminal bell notifications', async () => { @@ -1382,7 +1387,9 @@ describe('connectPanePty', () => { idleHandler('* Codex done') vi.advanceTimersByTime(250) - expect(deps.dispatchNotification).toHaveBeenCalledWith({ source: 'terminal-bell' }) + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ source: 'terminal-bell' }) + ) vi.advanceTimersByTime(1000) expect(deps.dispatchNotification).not.toHaveBeenCalledWith( expect.objectContaining({ source: 'agent-task-complete' }) @@ -1629,7 +1636,9 @@ describe('connectPanePty', () => { bellHandler() idleHandler('* Codex done') vi.advanceTimersByTime(250) - expect(deps.dispatchNotification).not.toHaveBeenCalledWith({ source: 'terminal-bell' }) + expect(deps.dispatchNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ source: 'terminal-bell' }) + ) mockStoreState.settings = { ...mockStoreState.settings, @@ -1641,7 +1650,9 @@ describe('connectPanePty', () => { notifyStoreSubscribers() vi.advanceTimersByTime(250) - expect(deps.dispatchNotification).toHaveBeenCalledWith({ source: 'terminal-bell' }) + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ source: 'terminal-bell' }) + ) }) it('requires fresh working evidence after notifications are disabled', async () => { @@ -1891,7 +1902,9 @@ describe('connectPanePty', () => { workingHandler() vi.advanceTimersByTime(250) - expect(deps.dispatchNotification).toHaveBeenCalledWith({ source: 'terminal-bell' }) + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ source: 'terminal-bell' }) + ) expect(deps.dispatchNotification).not.toHaveBeenCalledWith( expect.objectContaining({ source: 'agent-task-complete' }) ) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index f504d1737..b5879e6f7 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -359,7 +359,7 @@ export function connectPanePty( return } pendingTerminalBellNotification = false - deps.dispatchNotification({ source: 'terminal-bell' }) + deps.dispatchNotification({ source: 'terminal-bell', paneKey: cacheKey }) }, AGENT_TASK_COMPLETE_NOTIFICATION_GRACE_MS) } diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index 5bc53a903..60796813d 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -168,6 +168,7 @@ export function dispatchTerminalNotification( .dispatch({ source: event.source, worktreeId, + paneKey: event.paneKey, repoLabel: repo?.displayName, worktreeLabel: worktree?.displayName || worktree?.branch || worktreeId, hasMultipleActiveRepos: countReposNeedingNotificationDisambiguation(state) > 1, diff --git a/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts b/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts index 52ebd4e8d..7fda03331 100644 --- a/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts +++ b/src/renderer/src/hooks/useAutoAckViewedAgent.test.ts @@ -141,6 +141,28 @@ describe('computeAutoAckTargets — codex retain race regression', () => { expect(computeAutoAckTargets(store.getState(), 'tab-codex', CODEX_LEAF_ID)).toEqual([]) }) + it('skips sibling panes in the same terminal tab', () => { + const store = createTestStore() + const activeTabId = 'tab-split' + const activePaneKey = makePaneKey(activeTabId, CODEX_LEAF_ID) + const siblingPaneKey = makePaneKey(activeTabId, OTHER_LEAF_ID) + + store.getState().setAgentStatus(activePaneKey, { + state: 'done', + prompt: 'visible pane', + agentType: 'codex' + }) + store.getState().setAgentStatus(siblingPaneKey, { + state: 'done', + prompt: 'hidden sibling pane', + agentType: 'claude' + }) + + expect(computeAutoAckTargets(store.getState(), activeTabId, CODEX_LEAF_ID)).toEqual([ + activePaneKey + ]) + }) + it('acks a paneKey present in BOTH live and retained without throwing', () => { vi.useFakeTimers() vi.setSystemTime(new Date('2026-05-05T12:00:00.000Z')) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 2e5604b0a..5487a4d4e 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -45,6 +45,7 @@ import { import { isGitRepoKind } from '../../../shared/repo-kind' import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' +import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' import { focusRuntimeTerminalSurface } from '@/runtime/sync-runtime-graph' import { setFitOverride, hydrateOverrides } from '@/lib/pane-manager/mobile-fit-overrides' import { setDriverForPty, hydrateDrivers } from '@/lib/pane-manager/mobile-driver-state' @@ -977,19 +978,28 @@ export function useIpcEvents(): void { ) unsubs.push( - window.api.ui.onFocusTerminal(({ tabId, worktreeId, leafId }) => { - const store = useAppStore.getState() - store.setActiveWorktree(worktreeId) - // Why: CLI-driven focus is a user-initiated switch; stamp focus - // recency for Cmd+J. See docs/cmd-j-empty-query-ordering.md. - store.markWorktreeVisited(worktreeId) - store.setActiveView('terminal') - store.setActiveTab(tabId) - store.revealWorktreeInSidebar(worktreeId) - if (!focusRuntimeTerminalSurface(tabId, leafId)) { - focusTerminalTabSurface(tabId, leafId) + window.api.ui.onFocusTerminal( + ({ tabId, worktreeId, leafId, ackPaneKeyOnSuccess, flashFocusedPane }) => { + const store = useAppStore.getState() + store.setActiveWorktree(worktreeId) + // Why: CLI-driven focus is a user-initiated switch; stamp focus + // recency for Cmd+J. See docs/cmd-j-empty-query-ordering.md. + store.markWorktreeVisited(worktreeId) + store.setActiveView('terminal') + store.setActiveTab(tabId) + store.revealWorktreeInSidebar(worktreeId) + if (ackPaneKeyOnSuccess || flashFocusedPane) { + activateTabAndFocusPane(tabId, leafId ?? null, { + ...(ackPaneKeyOnSuccess ? { ackPaneKeyOnSuccess } : {}), + ...(flashFocusedPane ? { flashFocusedPane: true } : {}) + }) + return + } + if (!focusRuntimeTerminalSurface(tabId, leafId)) { + focusTerminalTabSurface(tabId, leafId) + } } - }) + ) ) unsubs.push( diff --git a/src/shared/types.ts b/src/shared/types.ts index d82f47942..bdc8c2090 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1748,6 +1748,8 @@ export type NotificationEventSource = 'agent-task-complete' | 'terminal-bell' | export type NotificationDispatchRequest = { source: NotificationEventSource worktreeId?: string + /** Stable `${tabId}:${leafId}` terminal pane key for click-to-focus routing. */ + paneKey?: string repoLabel?: string worktreeLabel?: string hasMultipleActiveRepos?: boolean