From c044294d1a3d46e92e9c189b5d97fb7952e20670 Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Tue, 14 Jul 2026 19:18:52 -0700
Subject: [PATCH] fix(mobile): harden notification opt-in onboarding

---
 mobile/app/notification-opt-in.tsx            | 139 ++++++++++--------
 mobile/app/notifications.tsx                  |   3 +-
 .../mobile-notifications.test.ts              |  30 +++-
 .../src/notifications/mobile-notifications.ts |   7 +-
 .../notification-opt-in-gate.test.ts          |  22 ++-
 .../notifications/notification-opt-in-gate.ts |   3 +
 6 files changed, 137 insertions(+), 67 deletions(-)

diff --git a/mobile/app/notification-opt-in.tsx b/mobile/app/notification-opt-in.tsx
index 41f2c2168e2..a4476687e09 100644
--- a/mobile/app/notification-opt-in.tsx
+++ b/mobile/app/notification-opt-in.tsx
@@ -1,5 +1,13 @@
 import { useCallback, useState } from 'react'
-import { ActivityIndicator, BackHandler, Pressable, StyleSheet, Text, View } from 'react-native'
+import {
+  ActivityIndicator,
+  BackHandler,
+  Pressable,
+  ScrollView,
+  StyleSheet,
+  Text,
+  View
+} from 'react-native'
 import { useFocusEffect, useLocalSearchParams, useRouter } from 'expo-router'
 import { SafeAreaView } from 'react-native-safe-area-context'
 import { BellRing } from 'lucide-react-native'
@@ -49,63 +57,65 @@ export default function NotificationOptInScreen() {
 
   return (
     <SafeAreaView style={styles.container}>
-      <View style={styles.brandRow}>
-        <OrcaLogo size={22} />
-        <Text style={styles.brandName}>Orca</Text>
-      </View>
-
-      <View style={styles.content}>
-        <View style={styles.iconSurface}>
-          <BellRing size={30} color={colors.textPrimary} />
+      <ScrollView contentContainerStyle={styles.scrollContent} showsVerticalScrollIndicator={false}>
+        <View style={styles.brandRow}>
+          <OrcaLogo size={22} />
+          <Text style={styles.brandName}>Orca</Text>
         </View>
-        <Text style={styles.eyebrow}>Notifications</Text>
-        <Text style={styles.title}>Stay updated while away</Text>
-        <Text style={styles.body}>
-          Get notified on this device when an agent needs your input or finishes a task.
-        </Text>
-      </View>
 
-      <View style={styles.footer}>
-        {error ? (
-          <Text style={styles.error} accessibilityRole="alert">
-            {error}
+        <View style={styles.content}>
+          <View style={styles.iconSurface}>
+            <BellRing size={30} color={colors.textPrimary} />
+          </View>
+          <Text style={styles.eyebrow}>Notifications</Text>
+          <Text style={styles.title}>Stay updated while away</Text>
+          <Text style={styles.body}>
+            Get notified on this device when an agent needs your input or finishes a task.
           </Text>
-        ) : null}
-        <Pressable
-          accessibilityRole="button"
-          accessibilityLabel="Enable agent notifications"
-          disabled={busyChoice !== null}
-          style={({ pressed }) => [
-            styles.primaryButton,
-            pressed && styles.buttonPressed,
-            busyChoice !== null && styles.buttonDisabled
-          ]}
-          onPress={() => void choose('enable')}
-        >
-          {busyChoice === 'enable' ? (
-            <ActivityIndicator color={colors.bgBase} />
-          ) : (
-            <Text style={styles.primaryButtonText}>Enable notifications</Text>
-          )}
-        </Pressable>
-        <Pressable
-          accessibilityRole="button"
-          disabled={busyChoice !== null}
-          style={({ pressed }) => [
-            styles.secondaryButton,
-            pressed && styles.buttonPressed,
-            busyChoice !== null && styles.buttonDisabled
-          ]}
-          onPress={() => void choose('skip')}
-        >
-          {busyChoice === 'skip' ? (
-            <ActivityIndicator color={colors.textSecondary} />
-          ) : (
-            <Text style={styles.secondaryButtonText}>Not now</Text>
-          )}
-        </Pressable>
-        <Text style={styles.footerNote}>You can change this any time in Settings.</Text>
-      </View>
+        </View>
+
+        <View style={styles.footer}>
+          {error ? (
+            <Text style={styles.error} accessibilityRole="alert">
+              {error}
+            </Text>
+          ) : null}
+          <Pressable
+            accessibilityRole="button"
+            accessibilityLabel="Enable agent notifications"
+            disabled={busyChoice !== null}
+            style={({ pressed }) => [
+              styles.primaryButton,
+              pressed && styles.buttonPressed,
+              busyChoice !== null && styles.buttonDisabled
+            ]}
+            onPress={() => void choose('enable')}
+          >
+            {busyChoice === 'enable' ? (
+              <ActivityIndicator color={colors.bgBase} />
+            ) : (
+              <Text style={styles.primaryButtonText}>Enable notifications</Text>
+            )}
+          </Pressable>
+          <Pressable
+            accessibilityRole="button"
+            disabled={busyChoice !== null}
+            style={({ pressed }) => [
+              styles.secondaryButton,
+              pressed && styles.buttonPressed,
+              busyChoice !== null && styles.buttonDisabled
+            ]}
+            onPress={() => void choose('skip')}
+          >
+            {busyChoice === 'skip' ? (
+              <ActivityIndicator color={colors.textSecondary} />
+            ) : (
+              <Text style={styles.secondaryButtonText}>Not now</Text>
+            )}
+          </Pressable>
+          <Text style={styles.footerNote}>You can change this any time in Settings.</Text>
+        </View>
+      </ScrollView>
     </SafeAreaView>
   )
 }
@@ -116,6 +126,11 @@ const styles = StyleSheet.create({
     backgroundColor: colors.bgBase,
     paddingHorizontal: spacing.xl
   },
+  // Why: this decision screen cannot be dismissed with Back, so every action
+  // must remain reachable in landscape and with accessibility text scaling.
+  scrollContent: {
+    flexGrow: 1
+  },
   brandRow: {
     minHeight: 52,
     flexDirection: 'row',
@@ -128,10 +143,10 @@ const styles = StyleSheet.create({
     fontWeight: '700'
   },
   content: {
-    flex: 1,
+    flexGrow: 1,
     alignItems: 'center',
     justifyContent: 'center',
-    paddingBottom: spacing.xl
+    paddingVertical: spacing.xl
   },
   iconSurface: {
     width: 64,
@@ -173,11 +188,12 @@ const styles = StyleSheet.create({
     paddingBottom: spacing.lg
   },
   primaryButton: {
-    height: 44,
+    minHeight: 44,
     alignItems: 'center',
     justifyContent: 'center',
     borderRadius: radii.button,
-    backgroundColor: colors.surfaceBright
+    backgroundColor: colors.surfaceBright,
+    paddingVertical: spacing.sm
   },
   primaryButtonText: {
     color: colors.bgBase,
@@ -185,11 +201,12 @@ const styles = StyleSheet.create({
     fontWeight: '600'
   },
   secondaryButton: {
-    height: 44,
+    minHeight: 44,
     alignItems: 'center',
     justifyContent: 'center',
     borderRadius: radii.button,
-    marginTop: spacing.xs
+    marginTop: spacing.xs,
+    paddingVertical: spacing.sm
   },
   secondaryButtonText: {
     color: colors.textSecondary,
diff --git a/mobile/app/notifications.tsx b/mobile/app/notifications.tsx
index 4adacfc016f..d9696251a94 100644
--- a/mobile/app/notifications.tsx
+++ b/mobile/app/notifications.tsx
@@ -17,7 +17,8 @@ import {
 const DEFAULT_PERMISSION_STATE: NotificationPermissionState = {
   granted: false,
   status: 'undetermined',
-  canAskAgain: true
+  canAskAgain: true,
+  authorizationReflectsUserChoice: false
 }
 
 export default function NotificationsScreen() {
diff --git a/mobile/src/notifications/mobile-notifications.test.ts b/mobile/src/notifications/mobile-notifications.test.ts
index 2d6cf868461..b1f7c89578e 100644
--- a/mobile/src/notifications/mobile-notifications.test.ts
+++ b/mobile/src/notifications/mobile-notifications.test.ts
@@ -1,6 +1,8 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import * as Notifications from 'expo-notifications'
+import { Platform } from 'react-native'
 import {
+  getNotificationPermissionState,
   setScheduledNotificationsMaxForTests,
   subscribeToDesktopNotifications
 } from './mobile-notifications'
@@ -18,7 +20,7 @@ vi.mock('expo-notifications', () => ({
 }))
 
 vi.mock('react-native', () => ({
-  Platform: { OS: 'ios' }
+  Platform: { OS: 'ios', Version: 18 }
 }))
 
 // Why: mobile-notifications now persists the catch-up watermark to
@@ -35,6 +37,32 @@ vi.mock('../storage/preferences', () => ({
   loadPushNotificationsEnabled: vi.fn()
 }))
 
+beforeEach(() => {
+  Object.assign(Platform, { OS: 'ios', Version: 18 })
+})
+
+describe('getNotificationPermissionState', () => {
+  it.each([
+    { os: 'android', version: 32, expected: false },
+    { os: 'android', version: 33, expected: true },
+    { os: 'ios', version: 18, expected: true }
+  ])(
+    'reports whether a granted $os $version authorization reflects user choice',
+    async ({ os, version, expected }) => {
+      Object.assign(Platform, { OS: os, Version: version })
+      vi.mocked(Notifications.getPermissionsAsync).mockResolvedValue({
+        status: 'granted',
+        canAskAgain: true
+      } as never)
+
+      await expect(getNotificationPermissionState()).resolves.toMatchObject({
+        granted: true,
+        authorizationReflectsUserChoice: expected
+      })
+    }
+  )
+})
+
 describe('subscribeToDesktopNotifications', () => {
   beforeEach(() => {
     vi.clearAllMocks()
diff --git a/mobile/src/notifications/mobile-notifications.ts b/mobile/src/notifications/mobile-notifications.ts
index f91e5981366..d3866393116 100644
--- a/mobile/src/notifications/mobile-notifications.ts
+++ b/mobile/src/notifications/mobile-notifications.ts
@@ -82,6 +82,7 @@ export type NotificationPermissionState = {
   granted: boolean
   status: string
   canAskAgain: boolean
+  authorizationReflectsUserChoice: boolean
 }
 
 export async function getNotificationPermissionState(): Promise<NotificationPermissionState> {
@@ -89,7 +90,11 @@ export async function getNotificationPermissionState(): Promise<NotificationPerm
   return {
     granted: status === 'granted',
     status,
-    canAskAgain
+    canAskAgain,
+    // Why: Android before API 33 has no runtime notification permission, so
+    // Expo's default "granted" state is capability evidence, not user consent.
+    authorizationReflectsUserChoice:
+      status === 'granted' && (Platform.OS !== 'android' || Number(Platform.Version) >= 33)
   }
 }
 
diff --git a/mobile/src/notifications/notification-opt-in-gate.test.ts b/mobile/src/notifications/notification-opt-in-gate.test.ts
index fb5a95ac642..ded3d7eca39 100644
--- a/mobile/src/notifications/notification-opt-in-gate.test.ts
+++ b/mobile/src/notifications/notification-opt-in-gate.test.ts
@@ -27,7 +27,8 @@ describe('notification opt-in gate', () => {
     vi.mocked(getNotificationPermissionState).mockResolvedValue({
       granted: false,
       status: 'undetermined',
-      canAskAgain: true
+      canAskAgain: true,
+      authorizationReflectsUserChoice: false
     })
 
     await expect(shouldPresentNotificationOptIn()).resolves.toBe(true)
@@ -46,19 +47,34 @@ describe('notification opt-in gate', () => {
     vi.mocked(getNotificationPermissionState).mockResolvedValue({
       granted: true,
       status: 'granted',
-      canAskAgain: true
+      canAskAgain: true,
+      authorizationReflectsUserChoice: true
     })
 
     await expect(shouldPresentNotificationOptIn()).resolves.toBe(false)
     expect(savePushNotificationsEnabled).toHaveBeenCalledWith(true)
   })
 
+  it('still presents when a pre-Android 13 default grant is not an opt-in decision', async () => {
+    vi.mocked(readPushNotificationsPreference).mockResolvedValue({ value: null, loaded: true })
+    vi.mocked(getNotificationPermissionState).mockResolvedValue({
+      granted: true,
+      status: 'granted',
+      canAskAgain: true,
+      authorizationReflectsUserChoice: false
+    })
+
+    await expect(shouldPresentNotificationOptIn()).resolves.toBe(true)
+    expect(savePushNotificationsEnabled).not.toHaveBeenCalled()
+  })
+
   it('skips the gate when iOS has already denied permission', async () => {
     vi.mocked(readPushNotificationsPreference).mockResolvedValue({ value: null, loaded: true })
     vi.mocked(getNotificationPermissionState).mockResolvedValue({
       granted: false,
       status: 'denied',
-      canAskAgain: false
+      canAskAgain: false,
+      authorizationReflectsUserChoice: false
     })
 
     await expect(shouldPresentNotificationOptIn()).resolves.toBe(false)
diff --git a/mobile/src/notifications/notification-opt-in-gate.ts b/mobile/src/notifications/notification-opt-in-gate.ts
index 9d7b4eed6cc..a909fad16d7 100644
--- a/mobile/src/notifications/notification-opt-in-gate.ts
+++ b/mobile/src/notifications/notification-opt-in-gate.ts
@@ -13,6 +13,9 @@ export async function shouldPresentNotificationOptIn(): Promise<boolean> {
   try {
     const permission = await getNotificationPermissionState()
     if (permission.granted) {
+      if (!permission.authorizationReflectsUserChoice) {
+        return true
+      }
       // Why: an already-authorized device should inherit the useful default
       // without seeing an onboarding decision it has effectively made.
       await savePushNotificationsEnabled(true)