diff --git a/apps/mobile-rn/App.tsx b/apps/mobile-rn/App.tsx index dd52318..4f43e50 100644 --- a/apps/mobile-rn/App.tsx +++ b/apps/mobile-rn/App.tsx @@ -58,6 +58,10 @@ import { MobileAdsProvider } from './src/lib/mobile-ads-context' import { DeviceProvider } from './src/lib/device-context' import { useNotificationRuntime } from './src/features/notifications/use-notification-runtime' import type { NotificationNavigationTarget } from './src/features/notifications/notification-contract' +import { + notificationNavigationQueue, + type NotificationNavigationReadiness, +} from './src/features/notifications/notification-navigation-queue' import { loadPendingInviteToken } from './src/features/teams/pending-invite' import { listTeams } from './src/features/teams/team-service' import { listHistoryPage } from './src/features/history/history-service' @@ -200,7 +204,6 @@ function RootNavigator(): React.ReactElement { } = useMobilePreferences() const [authEntryRoute, setAuthEntryRoute] = useState('Login') const navigationRef = useNavigationContainerRef() - const pendingNotificationTarget = useRef(null) const lastRoutedIncomingMediaId = useRef(null) const previousNeedsOnboarding = useRef(null) const navigationTheme = useMemo( @@ -213,17 +216,37 @@ function RootNavigator(): React.ReactElement { !authLoading && privacyCleanupState === 'ready', ) - const navigateFromNotification = useCallback((target: NotificationNavigationTarget): void => { - if (user === null && target.name !== 'InviteAccept') return - if (!navigationRef.isReady()) { - pendingNotificationTarget.current = target - return + // Readiness is read through a ref so the notification callbacks keep a + // stable identity; the flush effect below re-runs when it changes. + const authenticatedAppReady = !authLoading + && !preferencesLoading + && user !== null + && !needsOnboarding + && !recoveryMode + const authenticatedAppReadyRef = useRef(authenticatedAppReady) + authenticatedAppReadyRef.current = authenticatedAppReady + + const flushNotificationNavigation = useCallback((): void => { + const readiness: NotificationNavigationReadiness = { + navigationReady: navigationRef.isReady(), + authenticatedAppReady: authenticatedAppReadyRef.current, } + const target = notificationNavigationQueue.take(readiness) + if (target === null) return const navigator = navigationRef as unknown as { navigate: (name: string, params: Record) => void } navigator.navigate(target.name, target.params ?? {}) - }, [navigationRef, user]) + }, [navigationRef]) + + // Opened notifications can arrive before auth bootstrap finished (cold + // start: the native payload is one-shot) or while signed out. Queue the + // target in the app-lifetime queue and replay it once the authenticated + // stack is ready instead of dropping it. + const navigateFromNotification = useCallback((target: NotificationNavigationTarget): void => { + notificationNavigationQueue.offer(target) + flushNotificationNavigation() + }, [flushNotificationNavigation]) const routeIncomingMedia = useCallback(async (): Promise => { if (user === null || needsOnboarding || recoveryMode || !navigationRef.isReady()) return @@ -285,6 +308,12 @@ function RootNavigator(): React.ReactElement { navigationRef.resetRoot({ index: 0, routes: [{ name: 'Main' }] }) }, [authLoading, navigationRef, needsOnboarding, preferencesLoading, user]) + // Declared after the onboarding reset so a replayed notification target is + // pushed on top of the fresh Main root rather than wiped by resetRoot. + useEffect(() => { + if (authenticatedAppReady) flushNotificationNavigation() + }, [authenticatedAppReady, flushNotificationNavigation]) + useEffect(() => { if (!user?.id) return let active = true @@ -319,9 +348,7 @@ function RootNavigator(): React.ReactElement { theme={navigationTheme} linking={linking} onReady={() => { - const pending = pendingNotificationTarget.current - pendingNotificationTarget.current = null - if (pending !== null) navigateFromNotification(pending) + flushNotificationNavigation() void routeIncomingMedia() }} > diff --git a/apps/mobile-rn/__tests__/notification-cold-start-redteam-r2-30.test.tsx b/apps/mobile-rn/__tests__/notification-cold-start-redteam-r2-30.test.tsx new file mode 100644 index 0000000..d6938f4 --- /dev/null +++ b/apps/mobile-rn/__tests__/notification-cold-start-redteam-r2-30.test.tsx @@ -0,0 +1,198 @@ +import React from 'react' +import { act, create } from 'react-test-renderer' +import type { NotificationNavigationTarget } from '../src/features/notifications/notification-contract' +import { + canNavigateToNotificationTarget, + createNotificationNavigationQueue, + PENDING_NOTIFICATION_TARGET_TTL_MS, +} from '../src/features/notifications/notification-navigation-queue' +import { createNotificationOpenDedupe } from '../src/features/notifications/notification-open-dedupe' + +const mockGetInitialNativeNotification = jest.fn() +let mockOpenedListener: ((payload: Record) => void) | null = null + +jest.mock('../src/lib/auth-context', () => ({ + useAuth: () => ({ user: null }), +})) +jest.mock('../src/lib/device-context', () => ({ + useDevice: () => ({ currentDevice: null }), +})) +jest.mock('../src/features/teams/pending-invite', () => ({ + savePendingInviteToken: jest.fn(async () => undefined), +})) +jest.mock('../src/features/notifications/notification-service', () => ({ + synchronizePushRegistration: jest.fn(), +})) +jest.mock('../src/features/notifications/notification-native', () => ({ + clearNativeFullSyncRequired: jest.fn(async () => undefined), + getInitialNativeNotification: (...args: unknown[]) => mockGetInitialNativeNotification(...args), + getNativeNotificationStatus: jest.fn(async () => ({ fullSyncRequired: false })), + onNotificationOpened: (listener: (payload: Record) => void) => { + mockOpenedListener = listener + return { remove: () => { mockOpenedListener = null } } + }, + onPushRegistrationChanged: () => ({ remove: () => undefined }), +})) + +import { useNotificationRuntime } from '../src/features/notifications/use-notification-runtime' + +const HISTORY_ID = '11111111-1111-4111-8111-111111111111' +const TRANSCRIPTION_PAYLOAD: Record = { + schema_version: '1', + event_type: 'transcription.completed', + resource_id: HISTORY_ID, + route: 'HistoryDetail', + history_id: HISTORY_ID, +} +const HISTORY_TARGET: NotificationNavigationTarget = { + name: 'HistoryDetail', + params: { historyId: HISTORY_ID }, +} +const INVITE_TARGET: NotificationNavigationTarget = { + name: 'InviteAccept', + params: { token: 'invite-token-abcdefghijklmnop' }, +} + +function Harness({ + navigate, + onError, +}: { + navigate: (target: NotificationNavigationTarget) => void + onError?: (error: unknown) => void +}): null { + useNotificationRuntime({ navigate, onError }) + return null +} + +async function flushMicrotasks(): Promise { + for (let index = 0; index < 5; index += 1) await Promise.resolve() +} + +describe('notification navigation queue (cold start while auth bootstraps)', () => { + it('holds an account target while signed out / bootstrapping and replays it once the app stack is ready', () => { + const queue = createNotificationNavigationQueue() + queue.offer(HISTORY_TARGET) + + // Cold start: container not mounted, then mounted but user still null. + expect(queue.take({ navigationReady: false, authenticatedAppReady: false })).toBeNull() + expect(queue.take({ navigationReady: true, authenticatedAppReady: false })).toBeNull() + expect(queue.hasPending()).toBe(true) + + // Session restored: the target is delivered exactly once. + expect(queue.take({ navigationReady: true, authenticatedAppReady: true })).toEqual(HISTORY_TARGET) + expect(queue.take({ navigationReady: true, authenticatedAppReady: true })).toBeNull() + }) + + it('lets invitation links through while signed out but never before navigation is ready', () => { + expect(canNavigateToNotificationTarget(INVITE_TARGET, { + navigationReady: true, + authenticatedAppReady: false, + })).toBe(true) + expect(canNavigateToNotificationTarget(INVITE_TARGET, { + navigationReady: false, + authenticatedAppReady: true, + })).toBe(false) + expect(canNavigateToNotificationTarget(HISTORY_TARGET, { + navigationReady: true, + authenticatedAppReady: false, + })).toBe(false) + }) + + it('keeps only the latest target and expires stale taps', () => { + let now = 1_000 + const queue = createNotificationNavigationQueue({ now: () => now }) + queue.offer(HISTORY_TARGET) + queue.offer(INVITE_TARGET) + expect(queue.take({ navigationReady: true, authenticatedAppReady: true })).toEqual(INVITE_TARGET) + + queue.offer(HISTORY_TARGET) + now += PENDING_NOTIFICATION_TARGET_TTL_MS + 1 + expect(queue.hasPending()).toBe(false) + expect(queue.take({ navigationReady: true, authenticatedAppReady: true })).toBeNull() + }) +}) + +describe('notification open dedupe', () => { + it('records a key only after delivery completes and releases it on abort', () => { + const dedupe = createNotificationOpenDedupe() + expect(dedupe.begin('a')).toBe(true) + expect(dedupe.begin('a')).toBe(false) // in flight + dedupe.abort('a') + expect(dedupe.begin('a')).toBe(true) // failed attempt does not block retry + dedupe.complete('a') + expect(dedupe.begin('a')).toBe(false) + }) + + it('bounds the handled set', () => { + const dedupe = createNotificationOpenDedupe(2) + for (const key of ['a', 'b', 'c']) { + dedupe.begin(key) + dedupe.complete(key) + } + expect(dedupe.begin('a')).toBe(true) + expect(dedupe.begin('c')).toBe(false) + }) +}) + +describe('useNotificationRuntime one-shot initial notification', () => { + beforeEach(() => { + mockGetInitialNativeNotification.mockReset() + mockOpenedListener = null + }) + + it('delivers the one-shot initial payload even if navigate changed identity while it resolved', async () => { + let resolveInitial: (payload: Record | null) => void = () => undefined + mockGetInitialNativeNotification.mockImplementation(() => new Promise((resolve) => { + resolveInitial = resolve + })) + const firstNavigate = jest.fn() + const secondNavigate = jest.fn() + + let renderer: ReturnType + await act(async () => { + renderer = create() + await flushMicrotasks() + }) + // Auth bootstrap commits the user: the caller's navigate callback changes. + await act(async () => { + renderer!.update() + await flushMicrotasks() + }) + await act(async () => { + resolveInitial(TRANSCRIPTION_PAYLOAD) + await flushMicrotasks() + }) + + expect(firstNavigate).not.toHaveBeenCalled() + expect(secondNavigate).toHaveBeenCalledWith(HISTORY_TARGET) + // The native module is one-shot; the hook must not re-read it per render. + expect(mockGetInitialNativeNotification).toHaveBeenCalledTimes(1) + }) + + it('does not mark a tap as handled when delivering it failed', async () => { + mockGetInitialNativeNotification.mockResolvedValue(TRANSCRIPTION_PAYLOAD) + const navigate = jest.fn() + .mockImplementationOnce(() => { throw new Error('navigation_unavailable') }) + const onError = jest.fn() + + await act(async () => { + create() + await flushMicrotasks() + }) + expect(onError).toHaveBeenCalledTimes(1) + + await act(async () => { + mockOpenedListener?.(TRANSCRIPTION_PAYLOAD) + await flushMicrotasks() + }) + expect(navigate).toHaveBeenCalledTimes(2) + expect(navigate).toHaveBeenLastCalledWith(HISTORY_TARGET) + + // Once delivered, a duplicate report of the same tap is suppressed. + await act(async () => { + mockOpenedListener?.(TRANSCRIPTION_PAYLOAD) + await flushMicrotasks() + }) + expect(navigate).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/mobile-rn/src/features/notifications/notification-navigation-queue.ts b/apps/mobile-rn/src/features/notifications/notification-navigation-queue.ts new file mode 100644 index 0000000..d5fb286 --- /dev/null +++ b/apps/mobile-rn/src/features/notifications/notification-navigation-queue.ts @@ -0,0 +1,92 @@ +import type { NotificationNavigationTarget } from './notification-contract' + +/** + * What the navigation shell can currently accept. Kept as plain booleans so + * the routing policy stays independent of React, auth and navigation code. + */ +export interface NotificationNavigationReadiness { + /** The NavigationContainer is mounted and can dispatch actions. */ + navigationReady: boolean + /** + * Auth and preferences bootstrap finished, a user is signed in and the + * authenticated app stack (not onboarding / password recovery) is mounted. + */ + authenticatedAppReady: boolean +} + +/** + * Invitation links are the only targets reachable while signed out; every + * other notification points at account data and must wait for the + * authenticated stack. + */ +export function canNavigateToNotificationTarget( + target: NotificationNavigationTarget, + readiness: NotificationNavigationReadiness, +): boolean { + if (!readiness.navigationReady) return false + return target.name === 'InviteAccept' || readiness.authenticatedAppReady +} + +/** + * A notification opened while signed out (or during a cold start) may wait + * for a login, but not forever: a stale tap must not hijack navigation long + * after the user moved on. + */ +export const PENDING_NOTIFICATION_TARGET_TTL_MS = 10 * 60 * 1000 + +export interface NotificationNavigationQueue { + /** Remember the latest opened notification target (latest wins). */ + offer(target: NotificationNavigationTarget): void + /** + * Remove and return the pending target when the shell can navigate to it + * now; otherwise keep it for a later attempt and return null. + */ + take(readiness: NotificationNavigationReadiness): NotificationNavigationTarget | null + clear(): void + hasPending(): boolean +} + +interface QueueOptions { + now?: () => number + ttlMs?: number +} + +export function createNotificationNavigationQueue({ + now = Date.now, + ttlMs = PENDING_NOTIFICATION_TARGET_TTL_MS, +}: QueueOptions = {}): NotificationNavigationQueue { + let pending: { target: NotificationNavigationTarget; offeredAt: number } | null = null + + const dropExpired = (): void => { + if (pending !== null && now() - pending.offeredAt > ttlMs) pending = null + } + + return { + offer(target) { + pending = { target, offeredAt: now() } + }, + take(readiness) { + dropExpired() + if (pending === null) return null + if (!canNavigateToNotificationTarget(pending.target, readiness)) return null + const { target } = pending + pending = null + return target + }, + clear() { + pending = null + }, + hasPending() { + dropExpired() + return pending !== null + }, + } +} + +/** + * App-lifetime queue. It deliberately lives outside React: the native + * initial-notification payload is one-shot, and the root navigator can + * unmount and remount while auth/preferences bootstrap (for example when the + * preferences owner switches from the installation to the signed-in user). + */ +export const notificationNavigationQueue = createNotificationNavigationQueue() diff --git a/apps/mobile-rn/src/features/notifications/notification-open-dedupe.ts b/apps/mobile-rn/src/features/notifications/notification-open-dedupe.ts new file mode 100644 index 0000000..26eb31a --- /dev/null +++ b/apps/mobile-rn/src/features/notifications/notification-open-dedupe.ts @@ -0,0 +1,40 @@ +/** + * Suppresses duplicate deliveries of the same opened notification (the + * initial-notification read and the opened event can both report one tap). + * + * A key is only recorded as handled once the caller confirms delivery, so a + * failed or abandoned attempt never blocks a later retry of the same tap. + */ +export interface NotificationOpenDedupe { + /** Returns false when the key was already delivered or is being delivered. */ + begin(key: string): boolean + /** Mark the key as delivered. */ + complete(key: string): void + /** Release an in-flight key without recording it (delivery failed). */ + abort(key: string): void +} + +export const NOTIFICATION_OPEN_DEDUPE_LIMIT = 100 + +export function createNotificationOpenDedupe( + limit = NOTIFICATION_OPEN_DEDUPE_LIMIT, +): NotificationOpenDedupe { + let handled = new Set() + const inFlight = new Set() + + return { + begin(key) { + if (handled.has(key) || inFlight.has(key)) return false + inFlight.add(key) + return true + }, + complete(key) { + inFlight.delete(key) + handled.add(key) + if (handled.size > limit) handled = new Set([key]) + }, + abort(key) { + inFlight.delete(key) + }, + } +} diff --git a/apps/mobile-rn/src/features/notifications/use-notification-runtime.ts b/apps/mobile-rn/src/features/notifications/use-notification-runtime.ts index 62f6102..edd1b77 100644 --- a/apps/mobile-rn/src/features/notifications/use-notification-runtime.ts +++ b/apps/mobile-rn/src/features/notifications/use-notification-runtime.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react' +import { useEffect, useRef, useState } from 'react' import { useAuth } from '../../lib/auth-context' import { useDevice } from '../../lib/device-context' import { savePendingInviteToken } from '../teams/pending-invite' @@ -14,9 +14,16 @@ import { onNotificationOpened, onPushRegistrationChanged, } from './notification-native' +import { createNotificationOpenDedupe } from './notification-open-dedupe' import { synchronizePushRegistration } from './notification-service' interface NotificationRuntimeOptions { + /** + * Receives every opened notification target. It may be invoked after the + * caller re-rendered or unmounted (the native initial payload is one-shot), + * so it must accept the target even when it cannot navigate yet — e.g. by + * queueing it until the navigation shell is ready. + */ navigate: (target: NotificationNavigationTarget) => void onFullSyncRequired?: () => Promise onError?: (error: unknown) => void @@ -30,7 +37,6 @@ export function useNotificationRuntime({ const { user } = useAuth() const { currentDevice } = useDevice() const authRef = useRef({ user, currentDevice }) - const handledRef = useRef(new Set()) authRef.current = { user, currentDevice } useEffect(() => { @@ -63,23 +69,34 @@ export function useNotificationRuntime({ } }, [currentDevice, onError, onFullSyncRequired, user]) + // The native initial notification is one-shot (the module clears it on + // read), so the open listener must not be torn down and re-registered each + // time the caller's navigate callback changes identity: a payload resolved + // by a cancelled effect would otherwise be dropped. Always hand targets to + // the latest callback instead. + const navigateRef = useRef(navigate) + const onErrorRef = useRef(onError) + const [dedupe] = useState(createNotificationOpenDedupe) + navigateRef.current = navigate + onErrorRef.current = onError + useEffect(() => { let active = true const handle = async (payload: Record): Promise => { + let dedupeKey: string | null = null try { const notification = parseNotificationPayload(payload) - const dedupeKey = `${notification.eventType}:${notification.resourceId}` - if (handledRef.current.has(dedupeKey)) return - handledRef.current.add(dedupeKey) - if (handledRef.current.size > 100) { - handledRef.current = new Set([dedupeKey]) - } + const key = `${notification.eventType}:${notification.resourceId}` + if (!dedupe.begin(key)) return + dedupeKey = key if (notification.eventType === 'team.invite.created') { await savePendingInviteToken(notification.inviteToken) } - if (active) navigate(notificationNavigationTarget(notification)) + navigateRef.current(notificationNavigationTarget(notification)) + dedupe.complete(key) } catch (error) { - if (active) onError?.(error) + if (dedupeKey !== null) dedupe.abort(dedupeKey) + if (active) onErrorRef.current?.(error) } } @@ -88,12 +105,12 @@ export function useNotificationRuntime({ .then((payload) => { if (payload !== null) return handle(payload) }) - .catch((error) => { if (active) onError?.(error) }) + .catch((error) => { if (active) onErrorRef.current?.(error) }) return () => { active = false opened.remove() } - }, [navigate, onError]) + }, [dedupe]) useEffect(() => { let active = true