From 9562d9776fe418dd8eebd787cfea18c45d6092a4 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 14 May 2026 18:22:08 -0400 Subject: [PATCH] Fix mobile notification tap routing (#1871) Co-authored-by: Orca --- mobile/app/_layout.tsx | 71 ++++++++++++++++++- mobile/app/index.tsx | 2 +- mobile/src/components/ProtocolBlockScreen.tsx | 18 +++-- .../src/notifications/mobile-notifications.ts | 11 +-- .../notification-routing.test.ts | 46 ++++++++++++ .../src/notifications/notification-routing.ts | 60 ++++++++++++++++ 6 files changed, 196 insertions(+), 12 deletions(-) create mode 100644 mobile/src/notifications/notification-routing.test.ts create mode 100644 mobile/src/notifications/notification-routing.ts diff --git a/mobile/app/_layout.tsx b/mobile/app/_layout.tsx index 05d634ca9..188d52fe6 100644 --- a/mobile/app/_layout.tsx +++ b/mobile/app/_layout.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect } from 'react' +import { useCallback, useEffect, useRef } from 'react' import { View, StyleSheet } from 'react-native' import { Stack, useRouter } from 'expo-router' import { StatusBar } from 'expo-status-bar' @@ -8,6 +8,8 @@ import * as Linking from 'expo-linking' import { colors } from '../src/theme/mobile-theme' import { OrcaLogo } from '../src/components/OrcaLogo' import { RpcClientProvider } from '../src/transport/client-context' +import { getNotificationNavigationPath } from '../src/notifications/notification-routing' +import { loadHosts } from '../src/transport/host-store' // Why: keeps the native splash screen visible until the React tree is mounted // and ready to render. Without this the user sees a blank white/black frame @@ -49,6 +51,7 @@ function extractPairCode(url: string): string | null { export default function RootLayout() { const router = useRouter() + const handledNotificationIdsRef = useRef>(new Set()) // Why: route `orca://pair#` deep links to the confirm screen so // the same pairing flow runs whether the link arrived via QR scan, @@ -71,6 +74,72 @@ export default function RootLayout() { return () => sub.remove() }, [router]) + // Why: iOS delivers local notification taps through expo-notifications, + // not Linking. Route both cold-start and warm-start responses to the host + // and worktree that scheduled the notification. + useEffect(() => { + let disposed = false + + function clearLastNotificationResponse() { + try { + Notifications.clearLastNotificationResponse() + } catch { + // Older native shells may not expose the clear API; duplicate guards + // still protect the current JS runtime. + } + } + + function getInitialNotificationResponse(): Notifications.NotificationResponse | null { + try { + return Notifications.getLastNotificationResponse() + } catch { + return null + } + } + + async function getNavigationPath(data: unknown): Promise { + const hosts = await loadHosts().catch(() => null) + return getNotificationNavigationPath(data, { + knownHostIds: hosts ? new Set(hosts.map((host) => host.id)) : undefined + }) + } + + async function handleNotificationResponse(response: Notifications.NotificationResponse) { + if (response.actionIdentifier !== Notifications.DEFAULT_ACTION_IDENTIFIER) { + clearLastNotificationResponse() + return + } + + const notificationId = response.notification.request.identifier + if (handledNotificationIdsRef.current.has(notificationId)) { + return + } + handledNotificationIdsRef.current.add(notificationId) + + const path = await getNavigationPath(response.notification.request.content.data) + clearLastNotificationResponse() + if (disposed) { + return + } + if (path) { + router.push(path) + } + } + + const initialResponse = getInitialNotificationResponse() + if (initialResponse) { + void handleNotificationResponse(initialResponse) + } + + const sub = Notifications.addNotificationResponseReceivedListener((response) => { + void handleNotificationResponse(response) + }) + return () => { + disposed = true + sub.remove() + } + }, [router]) + // Why: hide the native splash only once the navigation Stack has been laid // out — this is the earliest moment the user will see actual app content. // Previously the splash hid when a placeholder View rendered, leaving a diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 1981d24db..9e857ba08 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -400,7 +400,7 @@ export default function HomeScreen() { const wireUp = (state: ConnectionState) => { if (state === 'connected') { if (!unsubNotif) { - unsubNotif = subscribeToDesktopNotifications(entry.client) + unsubNotif = subscribeToDesktopNotifications(entry.client, entry.hostId) } if (!unsubAccounts) { unsubAccounts = entry.client.subscribe('accounts.subscribe', null, (payload) => { diff --git a/mobile/src/components/ProtocolBlockScreen.tsx b/mobile/src/components/ProtocolBlockScreen.tsx index 7fa19a91b..6f64a9075 100644 --- a/mobile/src/components/ProtocolBlockScreen.tsx +++ b/mobile/src/components/ProtocolBlockScreen.tsx @@ -2,7 +2,6 @@ import { Linking, Platform, Pressable, StyleSheet, Text, View } from 'react-nati import { router } from 'expo-router' import { colors, radii, spacing, typography } from '../theme/mobile-theme' import type { CompatVerdict } from '../transport/protocol-compat' -import { MOBILE_PROTOCOL_VERSION } from '../transport/protocol-version' const RELEASES_URL = 'https://github.com/stablyai/orca/releases' const IOS_APP_STORE_URL = 'itms-apps://apps.apple.com/app/orca-ide/id6766130217' @@ -23,10 +22,12 @@ export function ProtocolBlockScreen({ verdict }: Props) { : null : { label: 'Open GitHub Releases', url: RELEASES_URL } - const title = isMobileTooOld ? 'Update Orca Mobile' : 'Update Orca desktop' + const title = isMobileTooOld ? 'Update Orca Mobile' : 'Update Orca on your computer' const body = isMobileTooOld - ? `The Orca desktop on this host requires Orca Mobile v${verdict.requiredMobileVersion ?? '?'}+. You have v${MOBILE_PROTOCOL_VERSION}.\n\nUpdate Orca Mobile from ${mobileUpdateTarget.storeName} to continue.` - : `Orca Mobile requires Orca desktop v${verdict.requiredDesktopVersion ?? '?'}+ to use this host. The desktop is reporting v${verdict.desktopVersion}.` + ? `This desktop needs a newer Orca Mobile app. Update Orca Mobile from ${mobileUpdateTarget.storeName}, then try this host again.` + : 'This paired desktop app is too old for your current Orca Mobile app. Update Orca on your computer, then try this host again.' + const recoveryNote = + 'Already updated? Go back to Hosts and refresh the connection. If this message stays, remove this host and pair it again.' return ( @@ -53,8 +54,9 @@ export function ProtocolBlockScreen({ verdict }: Props) { router.replace('/') }} > - Pair a different host + Back to hosts + {recoveryNote} ) @@ -109,6 +111,12 @@ const styles = StyleSheet.create({ fontWeight: '600', color: colors.textPrimary }, + recoveryNote: { + fontSize: typography.metaSize, + color: colors.textMuted, + lineHeight: 17, + marginTop: spacing.md + }, pressed: { opacity: 0.7 } diff --git a/mobile/src/notifications/mobile-notifications.ts b/mobile/src/notifications/mobile-notifications.ts index 987eed64e..f22b459a6 100644 --- a/mobile/src/notifications/mobile-notifications.ts +++ b/mobile/src/notifications/mobile-notifications.ts @@ -2,10 +2,11 @@ import * as Notifications from 'expo-notifications' import { Platform } from 'react-native' import type { RpcClient } from '../transport/rpc-client' import { loadPushNotificationsEnabled } from '../storage/preferences' +import { buildLocalNotificationData, type DesktopNotificationSource } from './notification-routing' type NotificationEvent = { type: 'notification' - source: 'agent-task-complete' | 'terminal-bell' | 'test' + source: DesktopNotificationSource title: string body: string worktreeId?: string @@ -55,7 +56,7 @@ function configureNotificationChannel(): void { } } -async function showLocalNotification(event: NotificationEvent): Promise { +async function showLocalNotification(event: NotificationEvent, hostId: string): Promise { const enabled = await loadPushNotificationsEnabled() if (!enabled) return @@ -66,7 +67,7 @@ async function showLocalNotification(event: NotificationEvent): Promise { content: { title: event.title, body: event.body, - data: { source: event.source, worktreeId: event.worktreeId }, + data: buildLocalNotificationData(event, hostId), ...(Platform.OS === 'android' ? { channelId: 'orca-desktop' } : {}) }, trigger: null @@ -76,7 +77,7 @@ async function showLocalNotification(event: NotificationEvent): Promise { // Why: each host connection gets its own notification subscription. When the // connection drops, the unsubscribe function cleans up the streaming RPC. // Returns an unsubscribe function. -export function subscribeToDesktopNotifications(client: RpcClient): () => void { +export function subscribeToDesktopNotifications(client: RpcClient, hostId: string): () => void { configureNotificationChannel() let subscriptionId: string | null = null @@ -103,7 +104,7 @@ export function subscribeToDesktopNotifications(client: RpcClient): () => void { } if (disposed) return if (event.type === 'notification') { - void showLocalNotification(event as NotificationEvent) + void showLocalNotification(event as NotificationEvent, hostId) } }) diff --git a/mobile/src/notifications/notification-routing.test.ts b/mobile/src/notifications/notification-routing.test.ts new file mode 100644 index 000000000..68e701810 --- /dev/null +++ b/mobile/src/notifications/notification-routing.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { buildLocalNotificationData, getNotificationNavigationPath } from './notification-routing' + +describe('notification routing', () => { + it('includes the host id in locally scheduled notification data', () => { + expect( + buildLocalNotificationData( + { + source: 'agent-task-complete', + worktreeId: 'repo::/Users/me/orca/workspaces/feature' + }, + 'host-1' + ) + ).toEqual({ + source: 'agent-task-complete', + hostId: 'host-1', + worktreeId: 'repo::/Users/me/orca/workspaces/feature' + }) + }) + + it('routes notification taps to the worktree terminal screen', () => { + expect( + getNotificationNavigationPath({ + hostId: 'host-1', + worktreeId: 'repo::/Users/me/orca/workspaces/feature' + }) + ).toBe('/h/host-1/session/repo%3A%3A%2FUsers%2Fme%2Forca%2Fworkspaces%2Ffeature') + }) + + it('falls back to the host screen when the payload has no worktree id', () => { + expect(getNotificationNavigationPath({ hostId: 'host-1' })).toBe('/h/host-1') + }) + + it('ignores payloads that cannot identify the paired host', () => { + expect(getNotificationNavigationPath({ worktreeId: 'repo::/tmp/worktree' })).toBeNull() + }) + + it('ignores payloads for hosts that are no longer paired', () => { + expect( + getNotificationNavigationPath( + { hostId: 'removed-host', worktreeId: 'repo::/tmp/worktree' }, + { knownHostIds: new Set(['host-1']) } + ) + ).toBeNull() + }) +}) diff --git a/mobile/src/notifications/notification-routing.ts b/mobile/src/notifications/notification-routing.ts new file mode 100644 index 000000000..b8db59d41 --- /dev/null +++ b/mobile/src/notifications/notification-routing.ts @@ -0,0 +1,60 @@ +export type DesktopNotificationSource = 'agent-task-complete' | 'terminal-bell' | 'test' + +export type DesktopNotificationEvent = { + source: DesktopNotificationSource + worktreeId?: string +} + +export type LocalNotificationData = { + source: DesktopNotificationSource + hostId: string + worktreeId?: string +} + +export type NotificationNavigationOptions = { + knownHostIds?: ReadonlySet +} + +function readNonEmptyString(value: unknown): string | null { + return typeof value === 'string' && value.trim().length > 0 ? value : null +} + +export function buildLocalNotificationData( + event: DesktopNotificationEvent, + hostId: string +): LocalNotificationData { + const data: LocalNotificationData = { + source: event.source, + hostId + } + if (event.worktreeId) { + data.worktreeId = event.worktreeId + } + return data +} + +export function getNotificationNavigationPath( + data: unknown, + options: NotificationNavigationOptions = {} +): string | null { + if (!data || typeof data !== 'object') { + return null + } + + const record = data as Record + const hostId = readNonEmptyString(record.hostId) + if (!hostId) { + return null + } + if (options.knownHostIds && !options.knownHostIds.has(hostId)) { + return null + } + + const hostPath = `/h/${encodeURIComponent(hostId)}` + const worktreeId = readNonEmptyString(record.worktreeId) + if (!worktreeId) { + return hostPath + } + + return `${hostPath}/session/${encodeURIComponent(worktreeId)}` +}