From 33bd67e057ed919498beb2ebdbbb9b3820b58021 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:34:14 +0000 Subject: [PATCH] fix(notifications): tapping a phone push opens the agent that sent it (#31773) --- .../use-notification-pane-navigation.test.tsx | 27 ++++++++++++++++--- .../use-notification-pane-navigation.ts | 5 ++-- .../notification-delivery-service.test.ts | 16 ++++++++--- .../notification-delivery-service.ts | 3 +++ src/main/runtime/push/push-dispatcher.ts | 1 + src/main/runtime/push/push-gateway-client.ts | 1 + .../runtime-mobile-notification-controller.ts | 2 ++ ...red-agent-session-mobile-attention.test.ts | 8 ++++++ ...ructured-agent-session-mobile-attention.ts | 8 +++++- .../runtime/terminal-agent-phone-push.test.ts | 1 + src/main/runtime/terminal-agent-phone-push.ts | 1 + src/shared/terminal-surface-id.ts | 13 +++++++++ 12 files changed, 75 insertions(+), 11 deletions(-) diff --git a/mobile/src/session/use-notification-pane-navigation.test.tsx b/mobile/src/session/use-notification-pane-navigation.test.tsx index eb5c428f4ae..7f10da56c58 100644 --- a/mobile/src/session/use-notification-pane-navigation.test.tsx +++ b/mobile/src/session/use-notification-pane-navigation.test.tsx @@ -61,9 +61,15 @@ const TABS: MobileSessionTab[] = [ const switched: MobileSessionTab[] = [] -function Probe({ terminalsLoaded }: { terminalsLoaded: boolean }): null { +function Probe({ + terminalsLoaded, + sessionTabs = TABS +}: { + terminalsLoaded: boolean + sessionTabs?: MobileSessionTab[] +}): null { useNotificationPaneNavigation({ - sessionTabs: TABS, + sessionTabs, terminalsLoaded, switchSessionTab: (tab) => switched.push(tab) }) @@ -184,15 +190,28 @@ describe('the page pane hook', () => { expect(bridge.cleared).toEqual([]) }) - it('consumes a request for a pane that has since closed, so no later render serves it', () => { + it('stays on the open tab for a pane that has since closed', () => { render(true) deliver(`gone:${LEAF}`) expect(switched).toEqual([]) - // A re-render with nothing new delivered must not go looking for it again. + // A later render still finds no such pane. deliver('') expect(switched).toEqual([]) }) + it('opens a pane missing from a list loaded before the push once a fresh list names it', () => { + const tree = render(true) + act(() => { + tree.update(createElement(Probe, { terminalsLoaded: true, sessionTabs: [TABS[0]!] })) + }) + deliver(`tab-b:${LEAF}`) + expect(switched).toEqual([]) + act(() => { + tree.update(createElement(Probe, { terminalsLoaded: true, sessionTabs: TABS })) + }) + expect(switched.map((tab) => tab.id)).toEqual(['second']) + }) + it('does nothing at all when the page was opened on no pane', () => { render(true) expect(switched).toEqual([]) diff --git a/mobile/src/session/use-notification-pane-navigation.ts b/mobile/src/session/use-notification-pane-navigation.ts index e00958bacdf..e96002c4511 100644 --- a/mobile/src/session/use-notification-pane-navigation.ts +++ b/mobile/src/session/use-notification-pane-navigation.ts @@ -72,12 +72,11 @@ export function useNotificationPaneNavigation({ if (paneKey === null || paneKey === '' || !terminalsLoaded) { return } - // Consumed even when the pane has since closed: a request left standing is one a later render - // would serve. - pending.current = null applied.current = paneKey const tab = notificationPaneTab(sessionTabs, paneKey) + // Held while missing: a list loaded before a reconnect can predate the agent the push names. if (tab) { + pending.current = null switchSessionTab(tab) } }, [asked, sessionTabs, switchSessionTab, terminalsLoaded]) diff --git a/src/main/notifications/notification-delivery-service.test.ts b/src/main/notifications/notification-delivery-service.test.ts index 6b3930ff63f..edb674e7a98 100644 --- a/src/main/notifications/notification-delivery-service.test.ts +++ b/src/main/notifications/notification-delivery-service.test.ts @@ -6,6 +6,7 @@ import { import { beforeEach, describe, expect, it, vi } from 'vitest' import type { BrowserWindow } from 'electron' import { createNotificationDeliveryService } from './notification-delivery-service' +import { toWebTerminalSurfaceTabId } from '../../shared/terminal-surface-id' import type { NotificationDeliveryDependencies } from './notification-delivery-service' import type { NotificationDispatchRequest, @@ -27,6 +28,8 @@ function makeSettings(overrides: Partial = {}): Notificati } } +const LEAF = '11111111-1111-4111-8111-111111111111' + function makeRequest( overrides: Partial = {} ): NotificationDispatchRequest { @@ -277,8 +280,14 @@ it('reports a muted host before a disabled source', () => { ).toEqual({ delivered: false, reason: 'host-muted' }) }) -it("tells the phone which server runs the workspace, through the gateway's push", async () => { - for (const executionHost of [undefined, 'runtime:qa'] as const) { +it("tells the phone which server and pane raised it, through the gateway's push", async () => { + const cases = [ + [undefined, `tab-1:${LEAF}`, `tab-1:${LEAF}`], + ['runtime:qa', `${toWebTerminalSurfaceTabId('host-tab')}:${LEAF}`, `host-tab:${LEAF}`], + // A per-surface mirror tab encodes the server's `tab::leaf` surface id. + ['runtime:qa', `${toWebTerminalSurfaceTabId(`host-tab::${LEAF}`)}:${LEAF}`, `host-tab:${LEAF}`] + ] as const + for (const [executionHost, paneKey, pushedPaneKey] of cases) { const push = createPushHarness({ devices: [{ deviceId: 'phone', pushRegistration: registration() }] }) @@ -286,10 +295,11 @@ it("tells the phone which server runs the workspace, through the gateway's push" harness.deps.dispatchMobileNotification = (event) => push.dispatcher.enqueue({ ...event, notificationSeq: 1, notificationEpoch: 'epoch' }) createNotificationDeliveryService(harness.deps).dispatch( - makeRequest({ agentState: 'done', ...(executionHost ? { executionHost } : {}) }) + makeRequest({ agentState: 'done', paneKey, ...(executionHost ? { executionHost } : {}) }) ) await flush() expect(push.sends[0]?.notification.worktreeId).toBe('wt-1') + expect(push.sends[0]?.notification.paneKey).toBe(pushedPaneKey) expect(push.sends[0]?.notification.executionHost).toBe(executionHost) } }) diff --git a/src/main/notifications/notification-delivery-service.ts b/src/main/notifications/notification-delivery-service.ts index 979f92f96b2..148857430b8 100644 --- a/src/main/notifications/notification-delivery-service.ts +++ b/src/main/notifications/notification-delivery-service.ts @@ -15,6 +15,7 @@ import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { buildNotificationOptions } from '../ipc/notification-options' import { translateMain } from '../i18n/main-i18n' import { reserveNotificationCooldown } from '../../shared/notification-burst-cooldown' +import { toHostSessionPaneKey } from '../../shared/terminal-surface-id' export type NotificationDeliveryDependencies = { readNotificationSettings: () => NotificationSettings @@ -120,6 +121,8 @@ export function createNotificationDeliveryService( title: notificationOptions.title, body: notificationOptions.body, worktreeId: request.worktreeId, + // The phone asks the execution host for tabs, which never name this window's mirror tabs. + ...(request.paneKey ? { paneKey: toHostSessionPaneKey(request.paneKey) } : {}), ...(request.executionHost ? { executionHost: request.executionHost } : {}), ...(request.notificationId ? { notificationId: request.notificationId } : {}), // Why: background push needs the agent's real state to pick "needs input" diff --git a/src/main/runtime/push/push-dispatcher.ts b/src/main/runtime/push/push-dispatcher.ts index 2a95c11809e..f4d494a2868 100644 --- a/src/main/runtime/push/push-dispatcher.ts +++ b/src/main/runtime/push/push-dispatcher.ts @@ -190,6 +190,7 @@ export class PushDispatcher { title: clip(event.title, PUSH_TITLE_MAX_LENGTH), body: clip(event.body, PUSH_BODY_MAX_LENGTH), ...(event.worktreeId ? { worktreeId: event.worktreeId } : {}), + ...(event.paneKey ? { paneKey: event.paneKey } : {}), ...(event.executionHost ? { executionHost: event.executionHost } : {}) } } diff --git a/src/main/runtime/push/push-gateway-client.ts b/src/main/runtime/push/push-gateway-client.ts index 34376ae1fbd..72ec18c5772 100644 --- a/src/main/runtime/push/push-gateway-client.ts +++ b/src/main/runtime/push/push-gateway-client.ts @@ -48,6 +48,7 @@ export type PushSendNotification = { title: string body: string worktreeId?: string + paneKey?: string executionHost?: `runtime:${string}` } diff --git a/src/main/runtime/runtime-mobile-notification-controller.ts b/src/main/runtime/runtime-mobile-notification-controller.ts index 3849466ca78..87e37c7f41b 100644 --- a/src/main/runtime/runtime-mobile-notification-controller.ts +++ b/src/main/runtime/runtime-mobile-notification-controller.ts @@ -29,6 +29,8 @@ export type MobileNotificationDispatchEvent = { title: string body: string worktreeId?: string + /** The pane the phone opens on tap; old phones ignore it and open the workspace's default tab. */ + paneKey?: string /** See `NotificationDispatchRequest.executionHost`; old phones ignore it and open the desktop's. */ executionHost?: `runtime:${string}` notificationId?: string diff --git a/src/main/runtime/structured-agent-session-mobile-attention.test.ts b/src/main/runtime/structured-agent-session-mobile-attention.test.ts index cf5631f5465..3a93bb7fdfb 100644 --- a/src/main/runtime/structured-agent-session-mobile-attention.test.ts +++ b/src/main/runtime/structured-agent-session-mobile-attention.test.ts @@ -6,6 +6,10 @@ import { } from '../../shared/agent-session-attention' import type { AgentSessionStatusSummary } from '../../shared/agent-session-wire' import type { NotificationSettings } from '../../shared/notification-settings-types' +import { + structuredAgentSessionPaneKey, + structuredAgentSessionTabId +} from '../../shared/structured-agent-session-projection' import { createStructuredAttentionMobileDelivery, readStructuredAttentionWorkspaceLabels, @@ -95,6 +99,10 @@ describe('structured attention mobile delivery', () => { title: 'orca / feature - Claude needs input', body: 'May I run the migration?', worktreeId: SCOPE.workspaceId, + paneKey: structuredAgentSessionPaneKey( + structuredAgentSessionTabId('session-1'), + 'session-1' + ), notificationId: key, attentionKey: key, agentState: 'blocked' diff --git a/src/main/runtime/structured-agent-session-mobile-attention.ts b/src/main/runtime/structured-agent-session-mobile-attention.ts index 97b84c894cd..41f3c3ba536 100644 --- a/src/main/runtime/structured-agent-session-mobile-attention.ts +++ b/src/main/runtime/structured-agent-session-mobile-attention.ts @@ -21,6 +21,10 @@ import type { NotificationDispatchRequest, NotificationSettings } from '../../shared/notification-settings-types' +import { + structuredAgentSessionPaneKey, + structuredAgentSessionTabId +} from '../../shared/structured-agent-session-projection' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' import { parseWorkspaceKey } from '../../shared/workspace-scope' import { buildNotificationOptions } from '../ipc/notification-options' @@ -88,7 +92,7 @@ export function createStructuredAttentionMobileDelivery( if (!news) { return } - const scope = edge.type === 'prompt' ? edge.prompt.scope : edge.completion.scope + const { scope, sessionId } = edge.type === 'prompt' ? edge.prompt : edge.completion if (deps.isWorkspaceMuted(scope)) { return } @@ -121,6 +125,8 @@ export function createStructuredAttentionMobileDelivery( title, body, worktreeId: scope.workspaceId, + // The host has no window tab id; the phone matches this session-derived key to the chat. + paneKey: structuredAgentSessionPaneKey(structuredAgentSessionTabId(sessionId), sessionId), notificationId: attentionKey, attentionKey, ...(structuredAttentionOrigin(edge) diff --git a/src/main/runtime/terminal-agent-phone-push.test.ts b/src/main/runtime/terminal-agent-phone-push.test.ts index e423802676c..0ed95930acf 100644 --- a/src/main/runtime/terminal-agent-phone-push.test.ts +++ b/src/main/runtime/terminal-agent-phone-push.test.ts @@ -130,6 +130,7 @@ describe('TerminalAgentPhonePush', () => { title: 'orca / feature - Claude finished', body: 'Fixed it.', worktreeId: WT, + paneKey: PANE, notificationId: ID, agentState: 'done', emittedAt: 11_500 diff --git a/src/main/runtime/terminal-agent-phone-push.ts b/src/main/runtime/terminal-agent-phone-push.ts index 16b12c51f13..e745dbbd82d 100644 --- a/src/main/runtime/terminal-agent-phone-push.ts +++ b/src/main/runtime/terminal-agent-phone-push.ts @@ -93,6 +93,7 @@ export class TerminalAgentPhonePush { title, body, worktreeId, + paneKey, ...(notificationId ? { notificationId } : {}), agentState: row.state } diff --git a/src/shared/terminal-surface-id.ts b/src/shared/terminal-surface-id.ts index 00425089e1a..032036014d6 100644 --- a/src/shared/terminal-surface-id.ts +++ b/src/shared/terminal-surface-id.ts @@ -1,3 +1,5 @@ +import { makePaneKey, parsePaneKey } from './stable-pane-id' + export const WEB_TERMINAL_SURFACE_TAB_PREFIX = 'web-terminal-' export const HOST_TERMINAL_SURFACE_SEPARATOR = '::' @@ -19,6 +21,17 @@ export function toHostSessionTabId(tabId: string): string { } } +/** The pane key as the execution host's own session tabs name that pane. */ +export function toHostSessionPaneKey(paneKey: string): string { + const pane = parsePaneKey(paneKey) + if (!pane) { + return paneKey + } + // A per-surface mirror decodes to `tab::leaf`; the host's session tabs name the parent tab. + const [hostTabId] = toHostSessionTabId(pane.tabId).split(HOST_TERMINAL_SURFACE_SEPARATOR) + return hostTabId && !hostTabId.includes(':') ? makePaneKey(hostTabId, pane.leafId) : paneKey +} + export function isWebTerminalSurfaceTabId(tabId: string): boolean { return tabId.startsWith(WEB_TERMINAL_SURFACE_TAB_PREFIX) }