fix(mobile): persist browser destination in user selection order (#31689)

Delayed hydration and out-of-order storage writes can reverse a browser destination selection. Order durable writes and preserve the confirmed value when saving fails.
This commit is contained in:
Neil
2026-10-21 21:20:06 -07:00
committed by GitHub
parent fc9f826cd9
commit f719ec0845
4 changed files with 153 additions and 8 deletions
@@ -0,0 +1,91 @@
import { createElement } from 'react'
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
import { afterEach, describe, expect, it, vi } from 'vitest'
import BrowserSettingsScreen from './browser-settings-screen'
import { PickerModal } from '../components/PickerModal'
import {
loadTerminalLinkOpenMode,
saveTerminalLinkOpenMode,
type MobileTerminalLinkOpenMode
} from '../storage/preferences'
vi.mock('react-native', () => ({
View: 'View',
Text: 'Text',
Pressable: 'Pressable',
ScrollView: 'ScrollView',
StyleSheet: { create: (value: unknown) => value }
}))
vi.mock('react-native-safe-area-context', () => ({
useSafeAreaInsets: () => ({ top: 0, bottom: 0 })
}))
vi.mock('expo-router', () => ({ useRouter: () => ({ back() {} }) }))
vi.mock('lucide-react-native', () => ({ ChevronLeft: 'Icon', ChevronRight: 'Icon', Globe: 'Icon' }))
vi.mock('../components/PickerModal', () => ({ PickerModal: () => null }))
vi.mock('../storage/preferences', () => ({
loadTerminalLinkOpenMode: vi.fn(),
saveTerminalLinkOpenMode: vi.fn()
}))
let renderer: ReactTestRenderer
afterEach(() => {
act(() => renderer?.unmount())
vi.resetAllMocks()
})
describe('browser settings persistence', () => {
it('does not replace a new selection with an older pending hydration read', async () => {
let resolveLoad = (_value: MobileTerminalLinkOpenMode) => {}
vi.mocked(loadTerminalLinkOpenMode).mockImplementation(
() =>
new Promise((resolve) => {
resolveLoad = resolve
})
)
vi.mocked(saveTerminalLinkOpenMode).mockResolvedValue()
await act(async () => {
renderer = create(createElement(BrowserSettingsScreen))
})
await act(async () => {
renderer.root.findByType(PickerModal).props.onSelect('phone-browser')
})
await act(async () => {
resolveLoad('orca-browser')
})
expect(renderer.root.findByType(PickerModal).props.selected).toBe('phone-browser')
expect(JSON.stringify(renderer.toJSON())).toContain('Phone browser')
})
it('restores the saved destination when changing the destination fails', async () => {
vi.mocked(loadTerminalLinkOpenMode).mockResolvedValue('orca-browser')
vi.mocked(saveTerminalLinkOpenMode).mockRejectedValue(new Error('storage unavailable'))
await act(async () => {
renderer = create(createElement(BrowserSettingsScreen))
})
await act(async () => {
renderer.root.findByType(PickerModal).props.onSelect('phone-browser')
})
expect(renderer.root.findByType(PickerModal).props.selected).toBe('orca-browser')
expect(JSON.stringify(renderer.toJSON())).toContain('Could not save browser preferences')
})
it('restores the hydrated destination when the save fails before the first read finishes', async () => {
let resolveLoad = (_value: MobileTerminalLinkOpenMode) => {}
vi.mocked(loadTerminalLinkOpenMode).mockImplementation(
() =>
new Promise((resolve) => {
resolveLoad = resolve
})
)
vi.mocked(saveTerminalLinkOpenMode).mockRejectedValue(new Error('storage unavailable'))
await act(async () => {
renderer = create(createElement(BrowserSettingsScreen))
})
await act(async () => {
renderer.root.findByType(PickerModal).props.onSelect('orca-browser')
})
await act(async () => resolveLoad('phone-browser'))
expect(renderer.root.findByType(PickerModal).props.selected).toBe('phone-browser')
expect(JSON.stringify(renderer.toJSON())).toContain('Could not save browser preferences')
})
})
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useEffect, useRef, useState } from 'react'
import { Pressable, ScrollView, StyleSheet, Text, View } from 'react-native'
import { useSafeAreaInsets } from 'react-native-safe-area-context'
import { useRouter } from 'expo-router'
@@ -40,13 +40,24 @@ export default function BrowserSettingsScreen({
const [linkMode, setLinkMode] = useState<MobileTerminalLinkOpenMode>('orca-browser')
const [pickerOpen, setPickerOpen] = useState(false)
const [error, setError] = useState<string | null>(null)
const selectionRevision = useRef(0)
const failedSelectionRevision = useRef<number | null>(null)
const savedMode = useRef<MobileTerminalLinkOpenMode>('orca-browser')
const hasSaved = useRef(false)
useEffect(() => {
let active = true
void loadTerminalLinkOpenMode().then(
(mode) => {
if (active) {
setLinkMode(mode)
if (!hasSaved.current) {
savedMode.current = mode
}
if (
active &&
(selectionRevision.current === 0 ||
failedSelectionRevision.current === selectionRevision.current)
) {
setLinkMode(savedMode.current)
}
},
() => {
@@ -62,10 +73,20 @@ export default function BrowserSettingsScreen({
const selectLinkMode = useCallback((mode: MobileTerminalLinkOpenMode) => {
setError(null)
// Optimistic, as base was: the row shows the tapped mode before the write lands.
const revision = ++selectionRevision.current
setLinkMode(mode)
void saveTerminalLinkOpenMode(mode).catch(() =>
setError('Could not save browser preferences. Try again.')
void saveTerminalLinkOpenMode(mode).then(
() => {
savedMode.current = mode
hasSaved.current = true
},
() => {
if (revision === selectionRevision.current) {
failedSelectionRevision.current = revision
setLinkMode(savedMode.current)
setError('Could not save browser preferences. Try again.')
}
}
)
}, [])
+21
View File
@@ -500,6 +500,27 @@ describe('terminal link open mode preference', () => {
await expect(loadTerminalLinkOpenMode()).resolves.toBe('orca-browser')
})
it('orders writes across settings mounts and makes readers wait for the latest choice', async () => {
const firstWrite = deferred<void>()
let stored: string | null = 'orca-browser'
vi.mocked(AsyncStorage.getItem).mockImplementation(async () => stored)
vi.mocked(AsyncStorage.setItem).mockImplementation(async (_key, value) => {
if (value === 'phone-browser') {
await firstWrite.promise
}
stored = value
})
const firstMount = saveTerminalLinkOpenMode('phone-browser')
const secondMount = saveTerminalLinkOpenMode('orca-browser')
const reader = loadTerminalLinkOpenMode()
await Promise.resolve()
expect(stored).toBe('orca-browser')
firstWrite.resolve()
await Promise.all([firstMount, secondMount])
await expect(reader).resolves.toBe('orca-browser')
expect(stored).toBe('orca-browser')
})
it('persists the selected mode', async () => {
await saveTerminalLinkOpenMode('phone-browser')
+14 -2
View File
@@ -236,8 +236,10 @@ export type MobileTerminalLinkOpenMode = 'orca-browser' | 'phone-browser'
const TERMINAL_LINK_OPEN_MODE_KEY = 'orca:terminalLinkOpenMode'
export const DEFAULT_TERMINAL_LINK_OPEN_MODE: MobileTerminalLinkOpenMode = 'orca-browser'
let terminalLinkWriteBarrier: Promise<void> | null = null
export async function loadTerminalLinkOpenMode(): Promise<MobileTerminalLinkOpenMode> {
await terminalLinkWriteBarrier
try {
const raw = await AsyncStorage.getItem(TERMINAL_LINK_OPEN_MODE_KEY)
return raw === 'phone-browser' || raw === 'orca-browser' ? raw : DEFAULT_TERMINAL_LINK_OPEN_MODE
@@ -246,8 +248,18 @@ export async function loadTerminalLinkOpenMode(): Promise<MobileTerminalLinkOpen
}
}
export async function saveTerminalLinkOpenMode(mode: MobileTerminalLinkOpenMode): Promise<void> {
await persistMirrored(TERMINAL_LINK_OPEN_MODE_KEY, mode)
export function saveTerminalLinkOpenMode(mode: MobileTerminalLinkOpenMode): Promise<void> {
const write = (terminalLinkWriteBarrier ?? Promise.resolve()).then(() =>
persistMirrored(TERMINAL_LINK_OPEN_MODE_KEY, mode)
)
const barrier = write.catch(() => undefined)
terminalLinkWriteBarrier = barrier
void barrier.then(() => {
if (terminalLinkWriteBarrier === barrier) {
terminalLinkWriteBarrier = null
}
})
return write
}
function stringArray(value: unknown): string[] {