diff --git a/mobile/app/_layout.tsx b/mobile/app/_layout.tsx index 775c452d54a..515fec3adb6 100644 --- a/mobile/app/_layout.tsx +++ b/mobile/app/_layout.tsx @@ -11,6 +11,7 @@ import { RpcClientProvider } from '../src/transport/client-context' import { getNotificationNavigationTarget } from '../src/notifications/notification-routing' import { useOpenNotificationRoute } from '../src/notifications/use-open-notification-route' import { + isRemotePushTrigger, pushNotificationRouteData, shouldSuppressForegroundPush } from '../src/notifications/push-receive' @@ -116,10 +117,17 @@ export default function RootLayout() { } } - async function getNavigationTarget(data: unknown) { + async function getNavigationTarget(notification: Notifications.Notification) { const hosts = await loadHostCatalog().catch(() => null) + const data: unknown = notification.request.content.data // A gateway push names its host by key fingerprint, not by this device's hostId. - const routeData = hosts ? pushNotificationRouteData(data, hosts) : data + // With no catalog to resolve against, such a push stays unrouted instead of + // falling back to whatever hostId its raw data carries. + const routeData = pushNotificationRouteData( + data, + hosts ?? [], + isRemotePushTrigger(notification.request.trigger) + ) return getNotificationNavigationTarget(routeData, { knownHostIds: hosts ? new Set(hosts.map((host) => host.id)) : undefined, credentialStatusByHostId: hosts @@ -148,7 +156,7 @@ export default function RootLayout() { } } - const target = await getNavigationTarget(response.notification.request.content.data) + const target = await getNavigationTarget(response.notification) clearLastNotificationResponse() if (disposed) { return diff --git a/mobile/src/notifications/push-receive.test.ts b/mobile/src/notifications/push-receive.test.ts index 67aa88f1895..c7b1a718610 100644 --- a/mobile/src/notifications/push-receive.test.ts +++ b/mobile/src/notifications/push-receive.test.ts @@ -8,7 +8,11 @@ import { getHostNotificationSession, resetHostNotificationSessionsForTests } from './notification-reconnect-catchup' -import { pushNotificationRouteData, shouldSuppressForegroundPush } from './push-receive' +import { + isRemotePushTrigger, + pushNotificationRouteData, + shouldSuppressForegroundPush +} from './push-receive' vi.mock('../transport/host-store', () => ({ loadHostCatalog: vi.fn() })) @@ -48,17 +52,29 @@ beforeEach(() => { describe('shouldSuppressForegroundPush', () => { it('suppresses a push whose id and seq the socket already delivered', async () => { - getHostNotificationSession('host-1').seen.add('id:agent:one#7') + const session = getHostNotificationSession('host-1') + session.lastDeliveredEpoch = 'epoch-1' + session.seen.add('id:agent:one#7') await expect( shouldSuppressForegroundPush( - apnsData({ hostFingerprint, notificationId: 'agent:one', notificationSeq: 7 }) + apnsData({ + hostFingerprint, + notificationId: 'agent:one', + notificationSeq: 7, + notificationEpoch: 'epoch-1' + }) ) ).resolves.toBe(true) }) it('shows an unseen push and marks it so the socket replay is dropped', async () => { - const data = apnsData({ hostFingerprint, notificationId: 'agent:one', notificationSeq: 7 }) + const data = apnsData({ + hostFingerprint, + notificationId: 'agent:one', + notificationSeq: 7, + notificationEpoch: 'epoch-1' + }) await expect(shouldSuppressForegroundPush(data)).resolves.toBe(false) @@ -67,25 +83,55 @@ describe('shouldSuppressForegroundPush', () => { }) it('reads the flat stringified fields an FCM data message carries', async () => { - getHostNotificationSession('host-1').seen.add('id:agent:one#7') + const session = getHostNotificationSession('host-1') + session.lastDeliveredEpoch = 'epoch-1' + session.seen.add('id:agent:one#7') await expect( shouldSuppressForegroundPush( - fcmData({ hostFingerprint, notificationId: 'agent:one', notificationSeq: 7 }) + fcmData({ + hostFingerprint, + notificationId: 'agent:one', + notificationSeq: 7, + notificationEpoch: 'epoch-1' + }) ) ).resolves.toBe(true) }) it('keys a terminal bell on its seq alone, since it carries no notification id', async () => { - getHostNotificationSession('host-1').seen.add('seq:4') + const session = getHostNotificationSession('host-1') + session.lastDeliveredEpoch = 'epoch-1' + session.seen.add('seq:4') await expect( shouldSuppressForegroundPush( - apnsData({ hostFingerprint, source: 'terminal-bell', notificationSeq: 4 }) + apnsData({ + hostFingerprint, + source: 'terminal-bell', + notificationSeq: 4, + notificationEpoch: 'epoch-1' + }) ) ).resolves.toBe(true) }) + it('shows a push that names no counter lifetime without letting it claim a key', async () => { + const session = getHostNotificationSession('host-1') + session.lastDeliveredEpoch = 'epoch-1' + session.seen.add('seq:4') + + // Without an epoch the seq cannot be tied to this counter, so a forged seq:4 + // must neither be swallowed against it nor stop the real bell at seq 4. + await expect( + shouldSuppressForegroundPush(apnsData({ hostFingerprint, notificationSeq: 4 })) + ).resolves.toBe(false) + await expect( + shouldSuppressForegroundPush(apnsData({ hostFingerprint, notificationSeq: 5 })) + ).resolves.toBe(false) + expect(session.seen.has('seq:5')).toBe(false) + }) + it('voids seen keys from a previous desktop lifetime before testing its own', async () => { const session = getHostNotificationSession('host-1') session.lastDeliveredEpoch = 'epoch-old' @@ -192,6 +238,28 @@ describe('pushNotificationRouteData', () => { expect(getNotificationNavigationTarget(data)).toBeNull() }) + it('leaves a remote push unrouted when no host catalog could be read', () => { + const data = { hostId: 'host-1', orca: { hostFingerprint, notificationId: 'agent:one' } } + + expect(pushNotificationRouteData(data, [], true)).toBeNull() + }) + + it('leaves a remote push with no fingerprint unrouted instead of treating it as local', () => { + const data = { hostId: 'host-1', worktreeId: 'wt-1', source: 'agent-task-complete' } + + expect(pushNotificationRouteData(data, hosts, true)).toBeNull() + // The same shape from this app's own scheduler still routes. + expect(pushNotificationRouteData(data, hosts, false)).toBe(data) + }) + + it('recognises only a provider-delivered trigger as remote', () => { + expect(isRemotePushTrigger({ type: 'push' })).toBe(true) + expect(isRemotePushTrigger({ type: 'timeInterval', seconds: 1 })).toBe(false) + expect(isRemotePushTrigger({ channelId: 'orca-desktop' })).toBe(false) + expect(isRemotePushTrigger(null)).toBe(false) + expect(isRemotePushTrigger(undefined)).toBe(false) + }) + it('drops a gateway payload that pairs an unresolvable fingerprint with a stray hostId', () => { const data = { hostId: 'host-1', diff --git a/mobile/src/notifications/push-receive.ts b/mobile/src/notifications/push-receive.ts index 06debf04d9f..36af16a15d8 100644 --- a/mobile/src/notifications/push-receive.ts +++ b/mobile/src/notifications/push-receive.ts @@ -41,6 +41,11 @@ export async function shouldSuppressForegroundPush(data: unknown): Promise { expect(session.lastDeliveredSeq).toBe(0) }) + it('drops a key that names no counter lifetime at all', () => { + const session = getHostNotificationSession('host-1') + session.lastDeliveredEpoch = 'epoch-1' + + markPresentedPushesSeen(session, [{ key: 'seq:4', epoch: undefined }]) + + // The desktop always sends an epoch; a key without one cannot be shown to belong + // to this counter, and claiming it would drop the real bell at seq 4. + expect(session.seen.has('seq:4')).toBe(false) + }) + it('drops a key from a desktop lifetime that has already been retired', () => { const session = getHostNotificationSession('host-1') session.lastDeliveredEpoch = 'epoch-2' diff --git a/mobile/src/notifications/push-tray-seen-seed.ts b/mobile/src/notifications/push-tray-seen-seed.ts index 9eab0e6fc4b..782e35e3d92 100644 --- a/mobile/src/notifications/push-tray-seen-seed.ts +++ b/mobile/src/notifications/push-tray-seen-seed.ts @@ -49,7 +49,10 @@ export async function readPresentedPushSeenKeys( } /** - * Claim the tray's keys on the session, skipping any from a dead counter lifetime. + * Claim the tray's keys on the session, skipping any that do not name the live + * counter lifetime. A push without an epoch cannot be tied to this counter, and + * the desktop always sends one, so it is left unclaimed rather than allowed to + * swallow a real event at the same seq. * * The watermark is deliberately untouched: a push seq proves one event was shown, * not that everything below it was, and advancing past a gap would make the desktop @@ -60,11 +63,7 @@ export function markPresentedPushesSeen( keys: readonly PresentedPushSeenKey[] ): void { for (const { key, epoch } of keys) { - if ( - epoch != null && - session.lastDeliveredEpoch != null && - epoch !== session.lastDeliveredEpoch - ) { + if (epoch == null || epoch !== session.lastDeliveredEpoch) { continue } session.seen.add(key)