From c3c9b94c55efe95c5125605e9ce242c88187d7d4 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:05 -0400 Subject: [PATCH 1/7] perf(mobile): batch hosted page preference requests Every getItem crossed the shell bridge as its own one-key multiGet, so the five preference reads a settings screen issues on mount exhausted the shell's four-concurrent request grant. Adjacent same-action calls in a tick now fold into one request bounded by the contract's 64-key limit, and flushGetRequests dispatches the pending batch instead of doing nothing. That removes the reason product code hand-serialized its reads, so the sequential awaits in the terminal preference loaders and the ad-hoc in-flight accessory dedupe go with it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ....tsx => connection-diagnostics-screen.tsx} | 0 .../hosted-page-async-storage.test.ts | 45 ++++- .../mobile-web/hosted-page-async-storage.ts | 156 ++++++++++++++++-- .../terminal/terminal-settings-host.test.ts | 33 ++-- .../src/terminal/web-terminal-preferences.ts | 24 +-- .../web-terminal-settings-operations.ts | 12 +- 6 files changed, 216 insertions(+), 54 deletions(-) rename mobile/src/diagnostics/{hosted-connection-diagnostics-screen.tsx => connection-diagnostics-screen.tsx} (100%) diff --git a/mobile/src/diagnostics/hosted-connection-diagnostics-screen.tsx b/mobile/src/diagnostics/connection-diagnostics-screen.tsx similarity index 100% rename from mobile/src/diagnostics/hosted-connection-diagnostics-screen.tsx rename to mobile/src/diagnostics/connection-diagnostics-screen.tsx diff --git a/mobile/src/mobile-web/hosted-page-async-storage.test.ts b/mobile/src/mobile-web/hosted-page-async-storage.test.ts index 252b2012401..b3ddf5cc91d 100644 --- a/mobile/src/mobile-web/hosted-page-async-storage.test.ts +++ b/mobile/src/mobile-web/hosted-page-async-storage.test.ts @@ -22,6 +22,47 @@ describe('hosted page preference adapter', () => { ]) }) + it('folds concurrent reads of one tick into a single bridge request', async () => { + const pagePreferences = vi.fn( + async (payload: { action: string; keys?: string[] }): Promise => + payload.action === 'read' + ? { entries: payload.keys!.map((key) => [key, `${key}-value`]) } + : { updated: true } + ) + setMobileWebPagePreferencesClient({ + native: { pagePreferences } + } as unknown as MobileWebBridgeClient) + const keys = ['a', 'b', 'c', 'd', 'e'] + expect(await Promise.all(keys.map((key) => storage.getItem(key)))).toEqual( + keys.map((key) => `${key}-value`) + ) + expect(pagePreferences.mock.calls).toEqual([ + [{ namespace: 'expo.preferences', action: 'read', keys }] + ]) + }) + + it('keeps writes ordered after reads and splits past the request key limit', async () => { + const pagePreferences = vi.fn( + async (payload: { action: string; keys?: string[] }): Promise => + payload.action === 'read' + ? { entries: (payload.keys ?? []).map((key) => [key, null]) } + : { updated: true } + ) + setMobileWebPagePreferencesClient({ + native: { pagePreferences } + } as unknown as MobileWebBridgeClient) + const many = Array.from({ length: 70 }, (_, index) => `k${index}`) + await Promise.all([storage.getItem('a'), storage.setItem('b', '1'), storage.multiGet(many)]) + expect( + pagePreferences.mock.calls.map(([payload]) => [payload.action, payload.keys?.length ?? 1]) + ).toEqual([ + ['read', 1], + ['write', 1], + ['read', 64], + ['read', 6] + ]) + }) + it('fails clearly on old shells rather than pretending writes persisted', async () => { const error = new Error('unsupported_capability') setMobileWebPagePreferencesClient({ @@ -34,10 +75,12 @@ describe('hosted page preference adapter', () => { it('discards a response after document replacement', async () => { const pending = Promise.withResolvers() + const pagePreferences = vi.fn(() => pending.promise) setMobileWebPagePreferencesClient({ - native: { pagePreferences: () => pending.promise } + native: { pagePreferences } } as unknown as MobileWebBridgeClient) const read = storage.getItem('setting') + await vi.waitFor(() => expect(pagePreferences).toHaveBeenCalled()) setMobileWebPagePreferencesClient(null) pending.resolve({ entries: [['setting', 'from-old-host']] }) await expect(read).rejects.toMatchObject({ code: 'cancelled' }) diff --git a/mobile/src/mobile-web/hosted-page-async-storage.ts b/mobile/src/mobile-web/hosted-page-async-storage.ts index 491339c213e..6666d38975d 100644 --- a/mobile/src/mobile-web/hosted-page-async-storage.ts +++ b/mobile/src/mobile-web/hosted-page-async-storage.ts @@ -1,7 +1,129 @@ import { requestMobileWebPagePreferences } from '../../../src/mobile-web/src/mobile-web-page-preferences-channel' +import type { + MobileWebPagePreferencesPayload, + MobileWebPagePreferencesResult +} from '../../../src/shared/mobile-web/page-preferences-contract' type Callback = (error: Error | null, result?: T) => void +type Action = MobileWebPagePreferencesPayload['action'] +type Operation = { + action: Action + keys: readonly string[] + entries: readonly [string, string][] + settle: (result: MobileWebPagePreferencesResult) => void + reject: (error: unknown) => void +} + const namespace = 'expo.preferences' +// Keys/entries per request accepted by MobileWebPagePreferencesPayloadSchema. +const maxBatchItems = 64 +const queue: Operation[] = [] +let scheduled = false +let draining = false + +function chunk(items: readonly Item[]): Item[][] { + const parts: Item[][] = [] + for (let index = 0; index < items.length; index += maxBatchItems) { + parts.push(items.slice(index, index + maxBatchItems)) + } + return parts +} + +function enqueue( + request: { action: Action; keys?: readonly string[]; entries?: readonly [string, string][] }, + settle: (result: MobileWebPagePreferencesResult) => T +): Promise { + return new Promise((resolve, reject) => { + queue.push({ + action: request.action, + keys: request.keys ?? [], + entries: request.entries ?? [], + settle: (result) => resolve(settle(result)), + reject + }) + if (!scheduled && !draining) { + scheduled = true + queueMicrotask(() => { + scheduled = false + void drain() + }) + } + }) +} + +// Adjacent same-action calls fold into one bridge request so a screen's +// concurrent preference reads stay inside the shell's request grant. +function takeBatch(): Operation[] { + const first = queue.shift()! + const batch = [first] + if (first.action === 'keys' || first.action === 'clear') { + return batch + } + let items = first.keys.length + first.entries.length + while (queue[0]?.action === first.action) { + const next = queue[0]! + const combined = items + next.keys.length + next.entries.length + if (combined > maxBatchItems) { + break + } + items = combined + batch.push(queue.shift()!) + } + return batch +} + +function buildPayload(batch: readonly Operation[]): MobileWebPagePreferencesPayload { + const first = batch[0]! + if (first.action === 'keys' || first.action === 'clear') { + return { namespace, action: first.action } + } + if (first.action === 'write') { + const entries = new Map() + for (const operation of batch) { + for (const [key, value] of operation.entries) { + entries.set(key, value) + } + } + return { namespace, action: 'write', entries: [...entries] } + } + const keys = new Set() + for (const operation of batch) { + for (const key of operation.keys) { + keys.add(key) + } + } + return first.action === 'read' + ? { namespace, action: 'read', keys: [...keys] } + : { namespace, action: 'remove', keys: [...keys] } +} + +async function drain(): Promise { + if (draining) { + return + } + draining = true + try { + while (queue.length > 0) { + const batch = takeBatch() + try { + const result = await requestMobileWebPagePreferences(buildPayload(batch)) + for (const operation of batch) { + try { + operation.settle(result) + } catch (error: unknown) { + operation.reject(error) + } + } + } catch (error: unknown) { + for (const operation of batch) { + operation.reject(error) + } + } + } + } finally { + draining = false + } +} function callbackResult(work: Promise, callback?: Callback): Promise { return work.then( @@ -16,22 +138,30 @@ function callbackResult(work: Promise, callback?: Callback): Promise } ) } -async function multiGet(keys: readonly string[]): Promise<[string, string | null][]> { - const result = await requestMobileWebPagePreferences({ - namespace, - action: 'read', - keys: [...keys] - }) + +function readEntries(result: MobileWebPagePreferencesResult, keys: readonly string[]) { if (!('entries' in result)) { throw new Error('Invalid page preference response') } - return result.entries + const values = new Map(result.entries) + return keys.map((key): [string, string | null] => [key, values.get(key) ?? null]) +} + +async function multiGet(keys: readonly string[]): Promise<[string, string | null][]> { + const parts = await Promise.all( + chunk(keys).map((part) => + enqueue({ action: 'read', keys: part }, (result) => readEntries(result, part)) + ) + ) + return parts.flat() } async function multiSet(entries: readonly [string, string][]): Promise { - await requestMobileWebPagePreferences({ namespace, action: 'write', entries: [...entries] }) + await Promise.all( + chunk(entries).map((part) => enqueue({ action: 'write', entries: part }, () => {})) + ) } async function multiRemove(keys: readonly string[]): Promise { - await requestMobileWebPagePreferences({ namespace, action: 'remove', keys: [...keys] }) + await Promise.all(chunk(keys).map((part) => enqueue({ action: 'remove', keys: part }, () => {}))) } const storage = { getItem(key: string, callback?: Callback) { @@ -48,13 +178,13 @@ const storage = { }, clear(callback?: Callback) { return callbackResult( - requestMobileWebPagePreferences({ namespace, action: 'clear' }).then(() => {}), + enqueue({ action: 'clear' }, () => {}), callback ) }, getAllKeys(callback?: Callback) { return callbackResult( - requestMobileWebPagePreferences({ namespace, action: 'keys' }).then((result) => { + enqueue({ action: 'keys' }, (result) => { if (!('keys' in result)) { throw new Error('Invalid page preference response') } @@ -84,7 +214,9 @@ const storage = { callback ) }, - flushGetRequests() {} + flushGetRequests() { + void drain() + } } export function useAsyncStorage(key: string) { return { diff --git a/mobile/src/terminal/terminal-settings-host.test.ts b/mobile/src/terminal/terminal-settings-host.test.ts index 2d1f52d7e9a..511679c086a 100644 --- a/mobile/src/terminal/terminal-settings-host.test.ts +++ b/mobile/src/terminal/terminal-settings-host.test.ts @@ -1,44 +1,38 @@ import AsyncStorage from '@react-native-async-storage/async-storage' -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { nativeTerminalSettingsHost } from './native-terminal-settings-host' import { webTerminalSettingsHost, webTerminalSettingsOperations } from './web-terminal-settings-operations' +import hostedPageStorage from '../mobile-web/hosted-page-async-storage' +import { setMobileWebPagePreferencesClient } from '../../../src/mobile-web/src/mobile-web-page-preferences-channel' import type { RpcClient } from '../transport/rpc-client' import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' vi.mock('@react-native-async-storage/async-storage', () => ({ default: { getItem: vi.fn(async () => null) } })) +afterEach(() => setMobileWebPagePreferencesClient(null)) describe('terminal restore settings adapters', () => { - it('shares the single native accessory-read slot across settings sections', async () => { - const client = { - native: { - supports: () => true, - terminalAccessoryPreferences: vi.fn().mockResolvedValue({ - customKeys: [], - orderedBuiltInIds: ['escape'], - visibleBuiltInIds: [] - }) - } - } - const settings = webTerminalSettingsOperations(client as unknown as MobileWebBridgeClient) - await Promise.all([settings.loadKeys(), settings.loadLayout()]) - expect(client.native.terminalAccessoryPreferences).toHaveBeenCalledTimes(1) - }) - it('loads all settings sections within the page preference concurrency grant', async () => { + // The page bundle resolves async storage to the hosted bridge adapter. + vi.mocked(AsyncStorage.getItem).mockImplementation((key: string) => + hostedPageStorage.getItem(key) + ) let inFlight = 0 - vi.mocked(AsyncStorage.getItem).mockImplementation(async () => { + const pagePreferences = vi.fn(async (payload: { keys?: string[] }) => { if (++inFlight > 4) { inFlight-- throw new Error('page preference concurrency exhausted') } await new Promise((resolve) => setTimeout(resolve, 1)) inFlight-- - return null + return { entries: (payload.keys ?? []).map((key) => [key, null]) } }) + setMobileWebPagePreferencesClient({ + native: { pagePreferences } + } as unknown as MobileWebBridgeClient) const client = { native: { terminalPreferences: async () => ({ @@ -58,6 +52,7 @@ describe('terminal restore settings adapters', () => { await expect( Promise.all([settings.loadPreferences(), settings.loadKeys(), settings.loadLayout()]) ).resolves.toHaveLength(3) + expect(pagePreferences.mock.calls.length).toBeLessThan(4) } finally { vi.mocked(AsyncStorage.getItem).mockImplementation(async () => null) } diff --git a/mobile/src/terminal/web-terminal-preferences.ts b/mobile/src/terminal/web-terminal-preferences.ts index 81f5f6b586c..8abe87acf2c 100644 --- a/mobile/src/terminal/web-terminal-preferences.ts +++ b/mobile/src/terminal/web-terminal-preferences.ts @@ -10,22 +10,22 @@ import { loadTerminalAccessoryLayout } from './terminal-accessory-layout' export async function loadWebHostTerminalPreferences(client: MobileWebBridgeClient) { const native = await client.native.terminalPreferences() - // Settings sections mount together; keep their combined reads below the bridge grant. - const textScale = await loadTerminalTextScale({ - fallback: native.textScale, - rejectReadFailure: true - }) - const autocompleteEnabled = await loadTerminalAutocompleteEnabled({ - fallback: native.autocompleteEnabled, - rejectReadFailure: true - }) - const linkOpenMode = await loadTerminalLinkOpenMode(native.linkOpenMode) + const [textScale, autocompleteEnabled, linkOpenMode] = await Promise.all([ + loadTerminalTextScale({ fallback: native.textScale, rejectReadFailure: true }), + loadTerminalAutocompleteEnabled({ + fallback: native.autocompleteEnabled, + rejectReadFailure: true + }), + loadTerminalLinkOpenMode(native.linkOpenMode) + ]) return { textScale: textScale as MobileWebTerminalTextScale, autocompleteEnabled, linkOpenMode } } export async function loadWebHostTerminalAccessoryPreferences(client: MobileWebBridgeClient) { const native = await client.native.terminalAccessoryPreferences() - const customKeys = await loadCustomKeys({ fallback: native.customKeys, rejectReadFailure: true }) - const layout = await loadTerminalAccessoryLayout({ fallback: native, rejectReadFailure: true }) + const [customKeys, layout] = await Promise.all([ + loadCustomKeys({ fallback: native.customKeys, rejectReadFailure: true }), + loadTerminalAccessoryLayout({ fallback: native, rejectReadFailure: true }) + ]) return { customKeys, orderedBuiltInIds: layout.orderedBuiltInIds, diff --git a/mobile/src/terminal/web-terminal-settings-operations.ts b/mobile/src/terminal/web-terminal-settings-operations.ts index 025b7ae7b5b..4bcaa02c76d 100644 --- a/mobile/src/terminal/web-terminal-settings-operations.ts +++ b/mobile/src/terminal/web-terminal-settings-operations.ts @@ -12,19 +12,11 @@ import { export function webTerminalSettingsOperations( client: MobileWebBridgeClient ): TerminalSettingsOperations { - let accessoryRead: ReturnType | undefined - const readAccessories = () => { - // The two settings sections share the shell's single accessory-read slot. - accessoryRead ??= loadWebHostTerminalAccessoryPreferences(client).finally(() => { - accessoryRead = undefined - }) - return accessoryRead - } return { ...nativeTerminalSettingsOperations, loadPreferences: () => loadWebHostTerminalPreferences(client), - loadKeys: async () => (await readAccessories()).customKeys, - loadLayout: readAccessories + loadKeys: async () => (await loadWebHostTerminalAccessoryPreferences(client)).customKeys, + loadLayout: () => loadWebHostTerminalAccessoryPreferences(client) } } const fitMethods = ['terminal.getAutoRestoreFit', 'terminal.setAutoRestoreFit'] From 4bc5a1697d0aa64dfb08c715948c04f7d1867581 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:13 -0400 Subject: [PATCH 2/7] fix(mobile): recover hosted preferences from an unusable blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broker parsed the stored blob before dispatching on the action, so a corrupt or oversized blob failed every request forever — including the clear that would have repaired it. A reset now runs before the size check and the parse, and a read serves empty values with a single warning rather than a hard unavailable. Writes still refuse to overwrite data they cannot read. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-page-preferences-store.test.ts | 32 +++++++++++ .../mobile-web-page-preferences-store.ts | 55 ++++++++++++++----- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/mobile/src/mobile-web/mobile-web-page-preferences-store.test.ts b/mobile/src/mobile-web/mobile-web-page-preferences-store.test.ts index e6d4ed1bfe3..4c84ffbb512 100644 --- a/mobile/src/mobile-web/mobile-web-page-preferences-store.test.ts +++ b/mobile/src/mobile-web/mobile-web-page-preferences-store.test.ts @@ -83,6 +83,38 @@ describe('bounded host-scoped page preferences', () => { expect(f.values.get(mobileWebPagePreferencesStorageKey('limited'))).toBe(before) }) + it('keeps a reset and a read working over a corrupt or oversized blob', async () => { + const f = fixture() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const key = mobileWebPagePreferencesStorageKey('broken') + f.values.set(key, 'bad-json') + expect( + await f.run('broken', { namespace: 'settings', action: 'read', keys: ['a', 'b'] }) + ).toEqual({ + entries: [ + ['a', null], + ['b', null] + ] + }) + expect(f.values.get(key)).toBe('bad-json') + await expect(f.run('broken', { namespace: 'settings', action: 'clear' })).resolves.toEqual({ + updated: true + }) + await expect( + f.run('broken', { namespace: 'settings', action: 'write', entries: [['a', 'saved']] }) + ).resolves.toEqual({ updated: true }) + + f.values.set(key, JSON.stringify([['settings', [['a', 'x'.repeat(3 * 1024 * 1024)]]]])) + expect(await f.run('broken', { namespace: 'settings', action: 'read', keys: ['a'] })).toEqual({ + entries: [['a', null]] + }) + await expect(f.run('broken', { namespace: 'settings', action: 'clear' })).resolves.toEqual({ + updated: true + }) + expect(warn).toHaveBeenCalledOnce() + warn.mockRestore() + }) + it('does not overwrite unreadable storage and recovers after a failed write', async () => { const f = fixture() const key = mobileWebPagePreferencesStorageKey('broken') diff --git a/mobile/src/mobile-web/mobile-web-page-preferences-store.ts b/mobile/src/mobile-web/mobile-web-page-preferences-store.ts index da70408f05d..ba3e0377327 100644 --- a/mobile/src/mobile-web/mobile-web-page-preferences-store.ts +++ b/mobile/src/mobile-web/mobile-web-page-preferences-store.ts @@ -21,6 +21,11 @@ export function mobileWebPagePreferencesStorageKey(hostIdentity: string): string return `orca:hosted-page-preferences:v1:${hash}` } +// Unpairing owns no page session, so the blob can go without the request queue. +export async function deleteMobileWebPagePreferences(hostIdentity: string): Promise { + await AsyncStorage.removeItem(mobileWebPagePreferencesStorageKey(hostIdentity)) +} + export function runMobileWebPagePreferences( hostIdentity: string, input: MobileWebPagePreferencesPayload, @@ -53,21 +58,19 @@ async function applyPreferences( storage: Storage ): Promise { const raw = await storage.getItem(key) - if (raw && byteLength(raw) > MOBILE_WEB_PAGE_PREFERENCES_MAX_BYTES) { - throw new MobileWebBrokerError('too_large') - } - let stored - try { - stored = MobileWebPagePreferencesStoredSchema.parse(raw ? JSON.parse(raw) : []) - } catch { - throw new MobileWebBrokerError('unavailable') - } - const namespaces = new Map(stored.map(([namespace, entries]) => [namespace, new Map(entries)])) - if ( - namespaces.size !== stored.length || - stored.some(([, entries]) => new Map(entries).size !== entries.length) - ) { - throw new MobileWebBrokerError('invalid_message') + const oversized = raw !== null && byteLength(raw) > MOBILE_WEB_PAGE_PREFERENCES_MAX_BYTES + const namespaces = oversized ? null : readNamespaces(raw) + if (!namespaces) { + // A reset and a read must survive a blob this device can no longer use. + if (payload.action === 'clear') { + await storage.setItem(key, '[]') + return { updated: true } + } + if (payload.action === 'read') { + warnUnreadablePreferences() + return { entries: payload.keys.map((entryKey) => [entryKey, null]) } + } + throw new MobileWebBrokerError(oversized ? 'too_large' : 'unavailable') } const values = namespaces.get(payload.namespace) ?? new Map() if (payload.action === 'read') { @@ -107,6 +110,28 @@ async function applyPreferences( return { updated: true } } +function readNamespaces(raw: string | null): Map> | null { + let stored + try { + stored = MobileWebPagePreferencesStoredSchema.parse(raw ? JSON.parse(raw) : []) + } catch { + return null + } + const namespaces = new Map(stored.map(([namespace, entries]) => [namespace, new Map(entries)])) + const duplicated = + namespaces.size !== stored.length || + stored.some(([, entries]) => new Map(entries).size !== entries.length) + return duplicated ? null : namespaces +} + +let warnedUnreadablePreferences = false +function warnUnreadablePreferences(): void { + if (!warnedUnreadablePreferences) { + warnedUnreadablePreferences = true + console.warn('[mobile-web] page preferences unreadable — serving empty values') + } +} + function byteLength(value: string): number { return new TextEncoder().encode(value).byteLength } From dae554eb7ed3c51c10b7125912a8d8c1139b1f1e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:13 -0400 Subject: [PATCH 3/7] fix(mobile): release the terminal settings controls after a failed load The load path set the error but left busy pinned, and every control is disabled while busy, so a single failed preference read left the screen inert with no way to retry. Clear busy in a finally as the voice settings screen does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/src/terminal/use-terminal-settings-state.test.ts | 4 ++-- mobile/src/terminal/use-terminal-settings-state.ts | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/mobile/src/terminal/use-terminal-settings-state.test.ts b/mobile/src/terminal/use-terminal-settings-state.test.ts index a508422b4ef..c9ae7043658 100644 --- a/mobile/src/terminal/use-terminal-settings-state.test.ts +++ b/mobile/src/terminal/use-terminal-settings-state.test.ts @@ -59,11 +59,11 @@ describe('terminal settings state', () => { expect(state.busy).toBe(false) expect(state.autocompleteEnabled).toBe(true) }) - it('keeps controls disabled when storage cannot be read', async () => { + it('releases the controls when storage cannot be read', async () => { const operations = fixture() vi.mocked(operations.loadPreferences).mockRejectedValue(new Error('storage unavailable')) await mount(operations) - expect(state.busy).toBe(true) + expect(state.busy).toBe(false) expect(state.error).toContain('Could not load') }) it('preserves the confirmed setting when saving fails', async () => { diff --git a/mobile/src/terminal/use-terminal-settings-state.ts b/mobile/src/terminal/use-terminal-settings-state.ts index c0fb266eed1..67eff1fcf4f 100644 --- a/mobile/src/terminal/use-terminal-settings-state.ts +++ b/mobile/src/terminal/use-terminal-settings-state.ts @@ -35,13 +35,17 @@ export function useTerminalSettingsState( } setTextScale(preferences.textScale) setAutocompleteEnabled(preferences.autocompleteEnabled) - setBusy(false) }) .catch(() => { if (active) { setError('Could not load terminal preferences. Go back and try again.') } }) + .finally(() => { + if (active) { + setBusy(false) + } + }) return () => { active = false mounted.current = false From 5f8494f7ebf03c45cc7065d4f9ded312fed00cac Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:22 -0400 Subject: [PATCH 4/7] refactor(mobile): share one settings row list across both routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native and hosted settings routes each hardcoded the same seven rows and had already diverged: Troubleshooting carried a different icon in each and the availability flags only existed on one side. One builder now owns the rows and takes the hosted shell/page-preference availability, so the routes keep only what is genuinely theirs — credential cleanup on native, external links on hosted. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/settings.tsx | 34 +---------- mobile/host-web-app/settings.tsx | 59 ++----------------- ...obile-native-shell-route-ownership.test.ts | 9 ++- .../settings/mobile-settings-menu-items.ts | 46 +++++++++++++++ mobile/src/settings/mobile-settings-menu.tsx | 4 +- 5 files changed, 64 insertions(+), 88 deletions(-) create mode 100644 mobile/src/settings/mobile-settings-menu-items.ts diff --git a/mobile/app/settings.tsx b/mobile/app/settings.tsx index c6b22042ef6..82e56b767bd 100644 --- a/mobile/app/settings.tsx +++ b/mobile/app/settings.tsx @@ -1,19 +1,9 @@ import { useCallback, useRef, useState } from 'react' import { View, Text, StyleSheet, Pressable, Linking, ActivityIndicator } from 'react-native' import { useFocusEffect, useRouter } from 'expo-router' -import { - Info, - Bell, - Wrench, - Shield, - LifeBuoy, - Mic, - Globe, - MessageSquare, - Terminal as TerminalIcon, - KeyRound -} from 'lucide-react-native' +import { Shield, LifeBuoy, KeyRound } from 'lucide-react-native' import { MobileSettingsFrame, MobileSettingsSection } from '../src/settings/mobile-settings-menu' +import { mobileSettingsMenuItems } from '../src/settings/mobile-settings-menu-items' import { colors, radii, spacing, typography } from '../src/theme/mobile-theme' import { loadPendingHostCredentialCleanup, @@ -83,25 +73,7 @@ export default function SettingsScreen() { return ( - router.push('/terminal-settings') - }, - { - label: 'Chat UI', - icon: MessageSquare, - onPress: () => router.push('/native-chat-settings') - }, - { label: 'Browser', icon: Globe, onPress: () => router.push('/browser-settings') }, - { label: 'Voice', icon: Mic, onPress: () => router.push('/voice-settings') }, - { label: 'Notifications', icon: Bell, onPress: () => router.push('/notifications') }, - { label: 'Troubleshooting', icon: Wrench, onPress: () => router.push('/troubleshoot') }, - { label: 'About', icon: Info, onPress: () => router.push('/about') } - ]} - /> + router.push(route))} /> {showCredentialCleanup ? ( diff --git a/mobile/host-web-app/settings.tsx b/mobile/host-web-app/settings.tsx index 5a4f82446bb..2419026c83e 100644 --- a/mobile/host-web-app/settings.tsx +++ b/mobile/host-web-app/settings.tsx @@ -3,24 +3,14 @@ import { useState } from 'react' import { Text } from 'react-native' import { colors, typography, spacing } from '../src/theme/mobile-theme' import { useRouter } from 'expo-router' -import { - Globe, - MessageSquare, - Terminal, - Mic, - Bell, - Activity, - Info, - Shield, - LifeBuoy -} from 'lucide-react-native' +import { Shield, LifeBuoy } from 'lucide-react-native' import { MobileSettingsFrame, MobileSettingsSection } from '../src/settings/mobile-settings-menu' +import { mobileSettingsMenuItems } from '../src/settings/mobile-settings-menu-items' export default function HostedSettingsRoute() { const router = useRouter() const [linkError, setLinkError] = useState(null) const shell = useMobileWebNativeShell() - const disabled = !(shell.client?.native.supports('pagePreferences') ?? false) const linksDisabled = !(shell.client?.native.supports('openExternal') ?? false) const openExternal = (url: string) => { setLinkError(null) @@ -39,47 +29,10 @@ export default function HostedSettingsRoute() { }} > { - router.push('/terminal-settings') - } - }, - { - label: 'Chat UI', - disabled, - icon: MessageSquare, - onPress: () => router.push('/native-chat-settings') - }, - { - label: 'Browser', - disabled, - icon: Globe, - onPress: () => router.push('/browser-settings') - }, - { - label: 'Voice', - icon: Mic, - disabled: !shell.client, - onPress: () => router.push('/voice-settings') - }, - { - label: 'Notifications', - icon: Bell, - disabled: !shell.client, - onPress: () => router.push('/notifications') - }, - { - label: 'Troubleshooting', - icon: Activity, - disabled: !shell.client, - onPress: () => router.push('/troubleshoot') - }, - { label: 'About', icon: Info, onPress: () => router.push('/about') } - ]} + items={mobileSettingsMenuItems((route) => router.push(route), { + shell: Boolean(shell.client), + pagePreferences: shell.client?.native.supports('pagePreferences') ?? false + })} /> { for (const routeName of NATIVE_ROUTE_NAMES) { expect(nativeLayout).toContain(`name="${routeName}"`) } - expect(nativeSettings).toContain("router.push('/troubleshoot')") - expect(nativeSettings).toContain("router.push('/about')") + expect(settingsMenuItems).toContain("push('/troubleshoot')") + expect(settingsMenuItems).toContain("push('/about')") + expect(nativeSettings).toContain('mobileSettingsMenuItems((route) => router.push(route))') expect(nativeSettings).toContain("Linking.openURL('https://www.onorca.dev/privacy')") }) diff --git a/mobile/src/settings/mobile-settings-menu-items.ts b/mobile/src/settings/mobile-settings-menu-items.ts new file mode 100644 index 00000000000..50ac39e4f24 --- /dev/null +++ b/mobile/src/settings/mobile-settings-menu-items.ts @@ -0,0 +1,46 @@ +import { Bell, Globe, Info, MessageSquare, Mic, Terminal, Wrench } from 'lucide-react-native' +import type { MobileSettingsMenuItem } from './mobile-settings-menu' + +// The hosted route reaches these screens through the shell, so a missing shell +// or page-preference capability disables the rows that depend on it. +export type MobileSettingsMenuAvailability = { + shell?: boolean + pagePreferences?: boolean +} + +export function mobileSettingsMenuItems( + push: (route: string) => void, + availability: MobileSettingsMenuAvailability = {} +): MobileSettingsMenuItem[] { + const shell = availability.shell ?? true + const preferences = availability.pagePreferences ?? true + return [ + { + label: 'Terminal', + icon: Terminal, + disabled: !shell, + onPress: () => push('/terminal-settings') + }, + { + label: 'Chat UI', + icon: MessageSquare, + disabled: !preferences, + onPress: () => push('/native-chat-settings') + }, + { + label: 'Browser', + icon: Globe, + disabled: !preferences, + onPress: () => push('/browser-settings') + }, + { label: 'Voice', icon: Mic, disabled: !shell, onPress: () => push('/voice-settings') }, + { label: 'Notifications', icon: Bell, disabled: !shell, onPress: () => push('/notifications') }, + { + label: 'Troubleshooting', + icon: Wrench, + disabled: !shell, + onPress: () => push('/troubleshoot') + }, + { label: 'About', icon: Info, onPress: () => push('/about') } + ] +} diff --git a/mobile/src/settings/mobile-settings-menu.tsx b/mobile/src/settings/mobile-settings-menu.tsx index 00ee79472ee..23f989e073c 100644 --- a/mobile/src/settings/mobile-settings-menu.tsx +++ b/mobile/src/settings/mobile-settings-menu.tsx @@ -37,7 +37,7 @@ export function MobileSettingsFrame({ ) } -type SettingsMenuItem = { +export type MobileSettingsMenuItem = { label: string icon: LucideIcon onPress: () => void @@ -49,7 +49,7 @@ export function MobileSettingsSection({ items, spaced = false }: { - items: SettingsMenuItem[] + items: MobileSettingsMenuItem[] spaced?: boolean }) { return ( From ade5b90ba5e4b57ad01c0927eb109b05f1ab3758 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:22 -0400 Subject: [PATCH 5/7] refactor(mobile): drop the page-only voice settings confirmation The hosted adapter rejected a configure whose echoed receipt differed from the request while the native adapter accepted any receipt. The paired desktop is trusted and ships the page bundle, so the extra check only produced a failure the native path does not have. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../settings/web-voice-settings-operations.test.ts | 7 +++++-- .../src/settings/web-voice-settings-operations.ts | 13 ++----------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/mobile/src/settings/web-voice-settings-operations.test.ts b/mobile/src/settings/web-voice-settings-operations.test.ts index 86efc6e9818..22874593339 100644 --- a/mobile/src/settings/web-voice-settings-operations.test.ts +++ b/mobile/src/settings/web-voice-settings-operations.test.ts @@ -3,12 +3,15 @@ import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-w import { webVoiceSettingsOperations } from './web-voice-settings-operations' describe('hosted voice settings', () => { - it('requires configuration acknowledgement without replaying a mismatched receipt', async () => { + it('returns the desktop receipt for a configuration change without a second request', async () => { const request = vi.fn().mockResolvedValue({ enabled: false, models: [] }) const operations = webVoiceSettingsOperations({ host: { request } } as unknown as MobileWebBridgeClient) - await expect(operations.configure({ enabled: true })).rejects.toThrow('not confirmed') + await expect(operations.configure({ enabled: true })).resolves.toEqual({ + enabled: false, + models: [] + }) expect(request).toHaveBeenCalledOnce() }) it('does not replay an ambiguous model mutation through native speech', async () => { diff --git a/mobile/src/settings/web-voice-settings-operations.ts b/mobile/src/settings/web-voice-settings-operations.ts index fb9eb78d11e..f188704c8a3 100644 --- a/mobile/src/settings/web-voice-settings-operations.ts +++ b/mobile/src/settings/web-voice-settings-operations.ts @@ -7,17 +7,8 @@ export function webVoiceSettingsOperations(client: MobileWebBridgeClient): Voice client.host.request({ method, params }) return { load: async () => (await request('speech.models.list', {})) as MobileSpeechSetup, - configure: async (params) => { - const result = (await request('speech.dictation.setup', params)) as MobileSpeechSetup - if ( - (params.enabled !== undefined && result.enabled !== params.enabled) || - (params.modelId !== undefined && result.selectedModelId !== params.modelId) || - (params.dictationMode !== undefined && result.dictationMode !== params.dictationMode) - ) { - throw new Error('Voice settings update was not confirmed') - } - return result - }, + configure: async (params) => + (await request('speech.dictation.setup', params)) as MobileSpeechSetup, download: async (modelId) => { await request('speech.models.download', { modelId }) }, From 7d8a829f3d41e52685cdc6dafb3a5ffdce10411f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:31 -0400 Subject: [PATCH 6/7] refactor(mobile): run both connection log routes on one screen The native route inlined 245 lines of orchestration the hosted screen had already factored behind DiagnosticsDeviceOperations, so the two paths gathered, redacted, and submitted the same report differently. The screen now takes the device operations, a clipboard writer, and a host picker slot; the native route supplies the existing native operations and its host chips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/connection-log.tsx | 246 +++--------------- mobile/host-web-app/connection-log.tsx | 9 +- .../connection-diagnostics-screen.test.tsx | 106 ++++++++ .../connection-diagnostics-screen.tsx | 78 +++--- 4 files changed, 197 insertions(+), 242 deletions(-) create mode 100644 mobile/src/diagnostics/connection-diagnostics-screen.test.tsx diff --git a/mobile/app/connection-log.tsx b/mobile/app/connection-log.tsx index 6301d6615fd..abf01c6bee0 100644 --- a/mobile/app/connection-log.tsx +++ b/mobile/app/connection-log.tsx @@ -1,39 +1,18 @@ -import { ConnectionDiagnosticsView } from '../src/diagnostics/connection-diagnostics-view' -import { useCallback, useEffect, useMemo, useState, useSyncExternalStore } from 'react' -import { View, Text, Pressable, Platform } from 'react-native' +import { useCallback, useEffect, useMemo, useState } from 'react' +import { View, Text, Pressable } from 'react-native' import { useLocalSearchParams, useRouter } from 'expo-router' import * as Clipboard from 'expo-clipboard' -import Constants from 'expo-constants' import { loadHosts } from '../src/transport/host-store' -import { connectionLogStore } from '../src/transport/persisted-connection-log-store' import { useHostClient, useRpcClientContext } from '../src/transport/client-context' -import { - useConnectionPathStatus, - useReconnectAttempt -} from '../src/transport/client-context-connection-metrics' -import { buildConnectionDiagnosticsReport } from '../src/diagnostics/connection-diagnostics-report' -import { mobileWebDiagnosticsStore } from '../src/mobile-web/mobile-web-diagnostics-store' -import { - diagnoseConnection, - getReportableConnectionIncidentId -} from '../src/diagnostics/connection-diagnostics-analysis' -import { submitConnectionDiagnostics } from '../src/diagnostics/connection-diagnostics-submission' -import { - readHydratedConnectionLog, - readConnectionDiagnosticsSnapshot, - resolveDiagnosticsHostId, - getDiagnosticsSubmissionState, - updateDiagnosticsSubmissionState, - type DiagnosticsSubmissionStates -} from '../src/diagnostics/connection-diagnostics-screen-data' import { useHostStatusGates } from '../src/transport/host-status-gates' -import { loadHostAppVersion } from '../src/transport/host-app-version-store' +import { ConnectionDiagnosticsScreen } from '../src/diagnostics/connection-diagnostics-screen' +import { createNativeDiagnosticsOperations } from '../src/diagnostics/native-diagnostics-operations' +import { + resolveDiagnosticsHostId, + type DiagnosticsHostSelection +} from '../src/diagnostics/connection-diagnostics-screen-data' import { connectionDiagnosticsScreenStyles as styles } from '../src/diagnostics/connection-diagnostics-screen-styles' -import type { ConnectionLogEntry, HostProfile } from '../src/transport/types' - -// Why: getSnapshot must be referentially stable when there's no data — -// a fresh [] per call would make useSyncExternalStore re-render forever. -const EMPTY_ENTRIES: readonly ConnectionLogEntry[] = [] +import type { HostProfile } from '../src/transport/types' // Why: reading the log is most needed while a host is failing, so this // screen also *acquires* the host client — opening it kicks a dial and the @@ -44,21 +23,14 @@ export default function ConnectionLogScreen() { const params = useLocalSearchParams<{ hostId?: string }>() const routeKey = useMemo(() => ({}), [params.hostId]) const [hosts, setHosts] = useState([]) - const [manualSelection, setManualSelection] = useState<{ - hostId: string - requestedHostId: string | undefined - routeKey: object - } | null>(null) - const [copiedHostId, setCopiedHostId] = useState(null) - const [submissionStates, setSubmissionStates] = useState({}) + const [manualSelection, setManualSelection] = useState(null) useEffect(() => { let stale = false void loadHosts().then((loaded) => { - if (stale) { - return + if (!stale) { + setHosts(loaded) } - setHosts(loaded) }) return () => { stale = true @@ -66,179 +38,45 @@ export default function ConnectionLogScreen() { }, []) const selectedId = resolveDiagnosticsHostId(hosts, params.hostId, manualSelection, routeKey) - const selected = hosts.find((h) => h.id === selectedId) ?? null + const selected = hosts.find((host) => host.id === selectedId) ?? null const { client, state } = useHostClient(selected?.id) - const { desktopAppVersion: liveDesktopAppVersion } = useHostStatusGates({ - hostId: selected?.id, - client, - connState: state - }) - const reconnectAttempts = useReconnectAttempt(selected?.id) - const { activePath, pendingPath } = useConnectionPathStatus(selected?.id) - - useEffect(() => { - if (selectedId) { - void readHydratedConnectionLog(connectionLogStore, selectedId) - } - }, [selectedId]) - - const subscribe = useCallback( - (listener: () => void) => - selectedId ? connectionLogStore.subscribe(selectedId, listener) : () => {}, - [selectedId] + // Why: refreshes the persisted desktop version the diagnostics report reads back. + useHostStatusGates({ hostId: selected?.id, client, connState: state }) + const device = useMemo( + () => (selected ? createNativeDiagnosticsOperations(selected, clientContext) : null), + [selected, clientContext] ) - const getSnapshot = useCallback( - () => (selectedId ? connectionLogStore.get(selectedId) : EMPTY_ENTRIES), - [selectedId] + const select = useCallback( + (hostId: string) => setManualSelection({ hostId, requestedHostId: params.hostId, routeKey }), + [params.hostId, routeKey] ) - const entries = useSyncExternalStore(subscribe, getSnapshot) - const subscribeMobileWeb = useCallback( - (listener: () => void) => mobileWebDiagnosticsStore.subscribe(listener), - [] - ) - const getMobileWebSnapshot = useCallback( - () => mobileWebDiagnosticsStore.get(selectedId), - [selectedId] - ) - const mobileWebDiagnostics = useSyncExternalStore(subscribeMobileWeb, getMobileWebSnapshot) - const diagnosis = selected - ? diagnoseConnection({ endpoint: selected.endpoint, state, activePath, pendingPath, entries }) - : null - const incidentId = selected - ? getReportableConnectionIncidentId({ - endpoint: selected.endpoint, - state, - activePath, - pendingPath, - entries - }) - : null - const submissionKey = selected && incidentId ? `${selected.id}:${incidentId}` : null - const submissionState = getDiagnosticsSubmissionState(submissionStates, submissionKey) - const copied = copiedHostId === selectedId - - const copyDiagnostics = useCallback(async () => { - if (!selected) { - return - } - const desktopAppVersion = liveDesktopAppVersion ?? (await loadHostAppVersion(selected.id)) - const snapshot = await readConnectionDiagnosticsSnapshot( - clientContext, - connectionLogStore, - selected.id - ) - const report = buildConnectionDiagnosticsReport({ - endpoint: selected.endpoint, - state: snapshot.state, - reconnectAttempts: snapshot.reconnectAttempts, - lastConnectedAt: snapshot.lastConnectedAt, - platform: `${Platform.OS} ${Platform.Version ?? ''}`.trim(), - appVersion: Constants.expoConfig?.version ?? 'unknown', - desktopAppVersion, - entries: snapshot.entries, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - mobileWeb: mobileWebDiagnostics - }) - await Clipboard.setStringAsync(report) - setCopiedHostId(selected.id) - setTimeout(() => setCopiedHostId((hostId) => (hostId === selected.id ? null : hostId)), 2000) - }, [selected, liveDesktopAppVersion, clientContext, mobileWebDiagnostics]) - - const sendDiagnostics = useCallback(async () => { - if (!selected || !submissionKey || submissionState === 'sending') { - return - } - const startedKey = submissionKey - setSubmissionStates((states) => updateDiagnosticsSubmissionState(states, startedKey, 'sending')) - const appVersion = Constants.expoConfig?.version ?? 'unknown' - const platform = `${Platform.OS} ${Platform.Version ?? ''}`.trim() - const desktopAppVersion = liveDesktopAppVersion ?? (await loadHostAppVersion(selected.id)) - const snapshot = await readConnectionDiagnosticsSnapshot( - clientContext, - connectionLogStore, - selected.id - ) - const currentIncidentId = getReportableConnectionIncidentId({ - endpoint: selected.endpoint, - state: snapshot.state, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - entries: snapshot.entries - }) - if (`${selected.id}:${currentIncidentId ?? ''}` !== startedKey) { - setSubmissionStates((states) => updateDiagnosticsSubmissionState(states, startedKey, null)) - return - } - const report = buildConnectionDiagnosticsReport({ - endpoint: selected.endpoint, - state: snapshot.state, - reconnectAttempts: snapshot.reconnectAttempts, - lastConnectedAt: snapshot.lastConnectedAt, - platform, - appVersion, - desktopAppVersion, - entries: snapshot.entries, - activePath: snapshot.activePath, - pendingPath: snapshot.pendingPath, - mobileWeb: mobileWebDiagnostics - }) - const result = await submitConnectionDiagnostics({ report, appVersion, platform }) - setSubmissionStates((states) => - updateDiagnosticsSubmissionState(states, startedKey, result.ok ? 'sent' : 'failed') - ) - }, [ - selected, - submissionKey, - submissionState, - liveDesktopAppVersion, - clientContext, - mobileWebDiagnostics - ]) return ( - Clipboard.setStringAsync(report)} onBack={() => router.back()} hostPicker={ - <> - {' '} - {hosts.length > 1 && ( - - {hosts.map((host) => ( - - setManualSelection({ - hostId: host.id, - requestedHostId: params.hostId, - routeKey - }) - } + hosts.length > 1 ? ( + + {hosts.map((host) => ( + select(host.id)} + > + - - {host.name} - - - ))} - - )} - + {host.name} + + + ))} + + ) : null } /> ) diff --git a/mobile/host-web-app/connection-log.tsx b/mobile/host-web-app/connection-log.tsx index eac1997842d..9645ac90baf 100644 --- a/mobile/host-web-app/connection-log.tsx +++ b/mobile/host-web-app/connection-log.tsx @@ -1,6 +1,6 @@ import { useRouter } from 'expo-router' import { useMobileWebNativeShell } from '../../src/mobile-web/src/native-shell-channel' -import { HostedConnectionDiagnosticsScreen } from '../src/diagnostics/hosted-connection-diagnostics-screen' +import { ConnectionDiagnosticsScreen } from '../src/diagnostics/connection-diagnostics-screen' export default function HostedConnectionLogRoute() { const router = useRouter() @@ -8,10 +8,13 @@ export default function HostedConnectionLogRoute() { if (!shell.client) { return null } + const client = shell.client return ( - client.native.clipboardWrite(report)} onBack={() => (router.canGoBack() ? router.back() : router.replace('/settings'))} /> ) diff --git a/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx b/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx new file mode 100644 index 00000000000..15ae88dc912 --- /dev/null +++ b/mobile/src/diagnostics/connection-diagnostics-screen.test.tsx @@ -0,0 +1,106 @@ +import { createElement, useEffect } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ConnectionDiagnosticsScreen } from './connection-diagnostics-screen' +import type { DiagnosticsDeviceOperations } from './diagnostics-device-operations' +import type { DiagnosticsSnapshot } from '../../../src/shared/mobile-web/diagnostics-device-contract' + +vi.mock('react-native', () => ({ + View: 'View', + Text: 'Text', + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (value: unknown) => value, hairlineWidth: 1 } +})) +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0 }) +})) +vi.mock('expo-router', () => ({ + useFocusEffect: (callback: () => void | (() => void)) => useEffect(callback, [callback]) +})) +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'Icon', + Copy: 'Icon', + Check: 'Icon', + Send: 'Icon' +})) +vi.mock('../components/ConnectionLog', () => ({ ConnectionLog: () => null })) + +const SNAPSHOT: DiagnosticsSnapshot = { + state: 'disconnected', + reconnectAttempts: 3, + lastConnectedAt: null, + activePath: 'lan', + pendingPath: null, + endpointIsTailscale: false, + platform: 'ios 18', + appVersion: '1.0.0', + desktopAppVersion: '2.0.0', + entries: [], + mobileWeb: {} +} + +let renderer: ReactTestRenderer | undefined +afterEach(() => { + act(() => renderer?.unmount()) + renderer = undefined +}) + +function device(overrides: Partial = {}) { + return { + snapshot: vi.fn().mockResolvedValue(SNAPSHOT), + probe: vi.fn().mockResolvedValue({ reachable: true }), + submit: vi.fn().mockResolvedValue({ ok: true }), + ...overrides + } as unknown as DiagnosticsDeviceOperations +} + +async function mount(props: Parameters[0]) { + await act(async () => { + renderer = create(createElement(ConnectionDiagnosticsScreen, props)) + }) +} + +function texts(): string[] { + return renderer!.root + .findAllByType('Text') + .flatMap((node) => node.children.filter((child): child is string => typeof child === 'string')) +} + +describe('shared connection diagnostics screen', () => { + it('reports either host through one device operations seam', async () => { + const write = vi.fn().mockResolvedValue(undefined) + const operations = device() + await mount({ + device: operations, + hostName: 'Desk', + writeClipboard: write, + onBack: () => {} + }) + + expect(texts()).toContain('disconnected') + expect(texts()).toContain(' · attempt 3') + expect(operations.snapshot).toHaveBeenCalled() + await act(async () => { + await renderer!.root.findAllByType('Pressable')[1]!.props.onPress() + }) + expect(write).toHaveBeenCalledWith(expect.stringContaining('2.0.0')) + }) + + it('surfaces a device failure instead of a permanent loading state', async () => { + await mount({ + device: device({ snapshot: vi.fn().mockRejectedValue(new Error('unreachable')) }), + hostName: 'Desk', + writeClipboard: vi.fn(), + onBack: () => {} + }) + + expect(texts().some((text) => text.includes('Could not load network diagnostics'))).toBe(true) + }) + + it('renders no host rather than loading when no host is selected', async () => { + await mount({ device: null, hostName: null, writeClipboard: vi.fn(), onBack: () => {} }) + + expect(texts()).not.toContain('Loading network diagnostics…') + }) +}) diff --git a/mobile/src/diagnostics/connection-diagnostics-screen.tsx b/mobile/src/diagnostics/connection-diagnostics-screen.tsx index 9c76244d7d6..e756a44e2ce 100644 --- a/mobile/src/diagnostics/connection-diagnostics-screen.tsx +++ b/mobile/src/diagnostics/connection-diagnostics-screen.tsx @@ -1,7 +1,6 @@ -import { useCallback, useState } from 'react' +import { useCallback, useState, type ReactNode } from 'react' import { Text } from 'react-native' import { useFocusEffect } from 'expo-router' -import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' import type { DiagnosticsSnapshot } from '../../../src/shared/mobile-web/diagnostics-device-contract' import type { ConnectionLogEntry } from '../transport/types' import type { MobileWebDiagnosticsSnapshot } from '../mobile-web/mobile-web-diagnostics-store' @@ -17,21 +16,40 @@ import { type DiagnosticsSubmissionStates } from './connection-diagnostics-screen-data' import { connectionDiagnosticsScreenStyles as styles } from './connection-diagnostics-screen-styles' +import type { DiagnosticsDeviceOperations } from './diagnostics-device-operations' -export function HostedConnectionDiagnosticsScreen({ - client, - onBack +function reportable(snapshot: DiagnosticsSnapshot) { + return { + ...snapshot, + entries: snapshot.entries as ConnectionLogEntry[], + mobileWeb: snapshot.mobileWeb as MobileWebDiagnosticsSnapshot + } +} + +// Why: reading the log matters most while a host is failing, so the screen +// re-polls the device instead of rendering a snapshot taken on mount. +export function ConnectionDiagnosticsScreen({ + device, + hostName, + writeClipboard, + onBack, + hostPicker }: { - client: MobileWebBridgeClient + device: DiagnosticsDeviceOperations | null + hostName: string | null + writeClipboard: (report: string) => Promise onBack: () => void + hostPicker?: ReactNode }) { - const device = client.native.diagnosticsDevice const [snapshot, setSnapshot] = useState(null) const [error, setError] = useState(null) const [copied, setCopied] = useState(false) const [submissions, setSubmissions] = useState({}) useFocusEffect( useCallback(() => { + if (!device) { + return + } let active = true let pending = false const refresh = async () => { @@ -61,44 +79,31 @@ export function HostedConnectionDiagnosticsScreen({ } }, [device]) ) - const data = snapshot - ? { - ...snapshot, - entries: snapshot.entries as ConnectionLogEntry[], - mobileWeb: snapshot.mobileWeb as MobileWebDiagnosticsSnapshot - } - : null + const data = snapshot ? reportable(snapshot) : null const diagnosis = data ? diagnoseConnection(data) : null const incident = data ? getReportableConnectionIncidentId(data) : null const submissionState = getDiagnosticsSubmissionState(submissions, incident) const copyDiagnostics = async () => { + if (!device) { + return + } try { - const fresh = await device.snapshot() - await client.native.clipboardWrite( - buildConnectionDiagnosticsReport({ - ...fresh, - entries: fresh.entries as ConnectionLogEntry[], - mobileWeb: fresh.mobileWeb as MobileWebDiagnosticsSnapshot - }) - ) + await writeClipboard(buildConnectionDiagnosticsReport(reportable(await device.snapshot()))) setCopied(true) + setTimeout(() => setCopied(false), 2000) } catch { setError('Could not copy the report. Try again.') } } const sendDiagnostics = async () => { - if (!incident || submissionState === 'sending') { + if (!device || !incident || submissionState === 'sending') { return } const started = incident setSubmissions((states) => updateDiagnosticsSubmissionState(states, started, 'sending')) try { const fresh = await device.snapshot() - const reportData = { - ...fresh, - entries: fresh.entries as ConnectionLogEntry[], - mobileWeb: fresh.mobileWeb as MobileWebDiagnosticsSnapshot - } + const reportData = reportable(fresh) if (getReportableConnectionIncidentId(reportData) !== started) { setSubmissions((states) => updateDiagnosticsSubmissionState(states, started, null)) return @@ -117,8 +122,8 @@ export function HostedConnectionDiagnosticsScreen({ } return ( - {error} - - ) : null + <> + {hostPicker} + {error ? ( + + {error} + + ) : null} + } /> ) From 27a194c5b7dd12507c9046047a5f0394d893dc96 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 04:46:31 -0400 Subject: [PATCH 7/7] fix(mobile): drop hosted page preferences when a pairing is removed Unpairing cleared metadata, overlays, and credentials but left the host-scoped page preference blob behind forever, since nothing else is keyed by the pairing key. Remove it when no remaining host still holds that key, best-effort so it cannot fail an authoritative removal. The host list mutation queue moves to its own module to keep host-store under its line budget; it is the same chain, only named. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../src/transport/host-list-mutation-queue.ts | 32 +++++++ .../src/transport/host-store-unpair.test.ts | 69 ++++++++++++++ mobile/src/transport/host-store.ts | 95 +++++++++---------- 3 files changed, 147 insertions(+), 49 deletions(-) create mode 100644 mobile/src/transport/host-list-mutation-queue.ts create mode 100644 mobile/src/transport/host-store-unpair.test.ts diff --git a/mobile/src/transport/host-list-mutation-queue.ts b/mobile/src/transport/host-list-mutation-queue.ts new file mode 100644 index 00000000000..0e57de21d1d --- /dev/null +++ b/mobile/src/transport/host-list-mutation-queue.ts @@ -0,0 +1,32 @@ +import * as hostListLoads from './host-list-load-sharing' +import { readStoredHostProfilesForMutation, writeStoredHostProfiles } from './host-metadata-store' +import type { StoredHostProfile } from './types' + +// Why: serialize host metadata RMW so concurrent writers cannot drop updates. +let hostListMutation: Promise = Promise.resolve() + +// Why: writers hold the chain across their full RMW; readers wait so a load doesn't race a half-written list. +export function waitForHostListMutations(): Promise { + return hostListMutation +} + +export function enqueueHostListMutation(operation: () => Promise): Promise { + const mutation = hostListMutation.then(operation) + hostListMutation = mutation.catch(() => {}) + return mutation +} + +export function mutateStoredHosts( + update: (hosts: StoredHostProfile[]) => StoredHostProfile[] | Promise +): Promise { + return enqueueHostListMutation(async () => { + const current = await readStoredHostProfilesForMutation() + await writeStoredHostProfiles(await update(current)) + hostListLoads.dropSharedHostListLoad() + }) +} + +/** Test-only: drain the module mutation chain between cases. */ +export function resetHostListMutationQueueForTests(): void { + hostListMutation = Promise.resolve() +} diff --git a/mobile/src/transport/host-store-unpair.test.ts b/mobile/src/transport/host-store-unpair.test.ts new file mode 100644 index 00000000000..7815f26bc07 --- /dev/null +++ b/mobile/src/transport/host-store-unpair.test.ts @@ -0,0 +1,69 @@ +import AsyncStorage from '@react-native-async-storage/async-storage' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mobileWebPagePreferencesStorageKey } from '../mobile-web/mobile-web-page-preferences-store' +import { removeHost, resetHostStoreForTests } from './host-store' + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: { getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() } +})) + +vi.mock('expo-secure-store', () => ({ + getItemAsync: vi.fn(), + setItemAsync: vi.fn(), + deleteItemAsync: vi.fn(), + WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'WHEN_UNLOCKED_THIS_DEVICE_ONLY' +})) + +vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) + +vi.mock('./host-credential-cleanup', () => ({ + cancelPendingHostCredentialCleanup: vi.fn(), + recordHostCredentialCleanupIntent: vi.fn(), + scheduleHostCredentialCleanup: vi.fn(), + retryPendingHostCredentialCleanups: vi.fn() +})) + +const HOST = { + id: 'host-1', + name: 'Desk', + endpoint: 'ws://127.0.0.1:1', + publicKeyB64: 'pk', + lastConnected: 1 +} + +describe('unpairing device-local host state', () => { + beforeEach(() => { + vi.mocked(AsyncStorage.getItem).mockReset() + vi.mocked(AsyncStorage.setItem).mockReset().mockResolvedValue(undefined) + vi.mocked(AsyncStorage.removeItem).mockReset().mockResolvedValue(undefined) + resetHostStoreForTests() + }) + + it('drops the removed pairing hosted page preferences', async () => { + vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify([HOST])) + + await removeHost(HOST.id) + + expect(AsyncStorage.removeItem).toHaveBeenCalledWith( + mobileWebPagePreferencesStorageKey(HOST.publicKeyB64) + ) + }) + + it('keeps preferences a remaining host with the same pairing key still owns', async () => { + const sibling = { ...HOST, id: 'host-2', name: 'Laptop' } + vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify([HOST, sibling])) + + await removeHost(sibling.id) + + expect(AsyncStorage.removeItem).not.toHaveBeenCalledWith( + mobileWebPagePreferencesStorageKey(HOST.publicKeyB64) + ) + }) + + it('keeps the removal authoritative when the preference delete fails', async () => { + vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify([HOST])) + vi.mocked(AsyncStorage.removeItem).mockRejectedValue(new Error('storage unavailable')) + + await expect(removeHost(HOST.id)).resolves.toBeUndefined() + }) +}) diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index c32ac44ac77..74976163b19 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -1,3 +1,4 @@ +import { deleteMobileWebPagePreferences } from '../mobile-web/mobile-web-page-preferences-store' import { HostProfileSchema } from './types' import type { HostCatalogEntry, HostProfile, StoredHostProfile } from './types' import { getNextHostNameFromHosts } from './host-names' @@ -27,9 +28,9 @@ import { createUnpairedHostCredentialDeletion } from './unpaired-host-credential import { loadStoredHostProfiles, readStoredHostProfilesForMutation, - toStoredHostProfile, - writeStoredHostProfiles + toStoredHostProfile } from './host-metadata-store' +import * as hostListMutations from './host-list-mutation-queue' async function commitDeviceToken(hostId: string, token: string): Promise { markHostCredentialWrite(hostId) @@ -40,16 +41,12 @@ async function commitDeviceToken(hostId: string, token: string): Promise { // Why: Keychain reads are slow (50-200ms) and loadHosts() runs on every screen mount; cache per-hostId in memory, invalidate on save/remove. const tokenCache = new Map() -// Why: serialize host metadata RMW so concurrent writers cannot drop updates. -let hostListMutation: Promise = Promise.resolve() - export const loadHosts = async (): Promise => (await loadHostListSnapshot()).profiles export const loadHostCatalog = async (): Promise => (await loadHostListSnapshot()).catalog async function loadHostListSnapshot(): Promise { - // Why: writers hold the mutation chain across their full RMW; wait so a load doesn't race a half-written list. - await hostListMutation + await hostListMutations.waitForHostListMutations() // Why: deduplicate concurrent loadHosts() calls so simultaneously mounting screens share one Keychain read pass. return hostListLoads.shareHostListLoad(doLoadHostListSnapshot) } @@ -85,7 +82,7 @@ export async function resolvePairingHostIdentity( newHostId: string ): Promise<{ id: string; name: string }> { // Why: one durable read both preserves an existing identity and names a new host, avoiding duplicate cards. - await hostListMutation + await hostListMutations.waitForHostListMutations() const hosts = await readStoredHostProfilesForMutation() const match = hosts.find((host) => host.publicKeyB64 === publicKeyB64) return match @@ -94,7 +91,7 @@ export async function resolvePairingHostIdentity( } const deleteUnpairedHostCredentials = createUnpairedHostCredentialDeletion({ - waitForHostMutations: () => hostListMutation, + waitForHostMutations: hostListMutations.waitForHostListMutations, hasStoredHost: async (hostId) => (await readStoredHostProfilesForMutation()).some(({ id }) => id === hostId), onDeleted: (hostId) => { @@ -111,36 +108,33 @@ function scheduleUnpairedHostCredentialCleanup(hostId: string): Promise { } function cancelCleanupForStoredHost(hostId: string): void { - const cancellation = hostListMutation.then(async () => { - const hosts = await readStoredHostProfilesForMutation() - if (hosts.some(({ id }) => id === hostId)) { - // Register before later removals enqueue their intent, without blocking host loads on cleanup storage. - void cancelPendingHostCredentialCleanup(hostId).catch(() => undefined) - } - }) - hostListMutation = cancellation.catch(() => {}) + void hostListMutations + .enqueueHostListMutation(async () => { + const hosts = await readStoredHostProfilesForMutation() + if (hosts.some(({ id }) => id === hostId)) { + // Register before later removals enqueue their intent, without blocking host loads on cleanup storage. + void cancelPendingHostCredentialCleanup(hostId).catch(() => undefined) + } + }) + .catch(() => undefined) } async function cancelCleanupForDurablyStoredHosts(hostIds: Iterable): Promise { const targets = [...hostIds] - return enqueueHostListMutation(async () => { - const storedIds = new Set((await readStoredHostProfilesForMutation()).map(({ id }) => id)) - await Promise.all( - targets - .filter((hostId) => storedIds.has(hostId)) - .map((hostId) => cancelPendingHostCredentialCleanup(hostId).catch(() => undefined)) - ) - }).catch(() => undefined) -} - -function enqueueHostListMutation(operation: () => Promise): Promise { - const mutation = hostListMutation.then(operation) - hostListMutation = mutation.catch(() => {}) - return mutation + return hostListMutations + .enqueueHostListMutation(async () => { + const storedIds = new Set((await readStoredHostProfilesForMutation()).map(({ id }) => id)) + await Promise.all( + targets + .filter((hostId) => storedIds.has(hostId)) + .map((hostId) => cancelPendingHostCredentialCleanup(hostId).catch(() => undefined)) + ) + }) + .catch(() => undefined) } function removeOrphanOverlayIfUnpaired(hostId: string): Promise { - return enqueueHostListMutation(async () => { + return hostListMutations.enqueueHostListMutation(async () => { const hosts = await readStoredHostProfilesForMutation() if (!hosts.some(({ id }) => id === hostId)) { await removeMobileRelayHostOverlay(hostId) @@ -148,17 +142,6 @@ function removeOrphanOverlayIfUnpaired(hostId: string): Promise { }) } -async function mutateStoredHosts( - update: (hosts: StoredHostProfile[]) => StoredHostProfile[] | Promise -): Promise { - return enqueueHostListMutation(async () => { - const current = await readStoredHostProfilesForMutation() - const next = await update(current) - await writeStoredHostProfiles(next) - hostListLoads.dropSharedHostListLoad() - }) -} - export class MobileRelayUpgradeHostRemovedError extends Error {} export const saveHost = (host: HostProfile): Promise => persistHost(host, false) @@ -174,7 +157,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise let cleanupIntentRecordedBeforeMetadata = false let tokenCommittedBeforeMetadata = false try { - await mutateStoredHosts(async (hosts) => { + await hostListMutations.mutateStoredHosts(async (hosts) => { const index = hosts.findIndex((h) => h.id === stored.id) for (const candidate of hosts) { if (candidate.id !== stored.id && candidate.publicKeyB64 === stored.publicKeyB64) { @@ -256,15 +239,22 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise export async function removeHost(hostId: string): Promise { let cleanupIntentRecorded = false + let orphanedPublicKeyB64: string | null = null try { - await mutateStoredHosts(async (hosts) => { + await hostListMutations.mutateStoredHosts(async (hosts) => { try { await recordHostCredentialCleanupIntent(hostId) cleanupIntentRecorded = true } catch { // Removal remains authoritative when cleanup intent storage is unavailable. } - return hosts.filter((h) => h.id !== hostId) + const removedKey = hosts.find((h) => h.id === hostId)?.publicKeyB64 + const remaining = hosts.filter((h) => h.id !== hostId) + // Hosted page preferences are keyed by a pairing key a sibling row may still hold. + if (removedKey && !remaining.some((h) => h.publicKeyB64 === removedKey)) { + orphanedPublicKeyB64 = removedKey + } + return remaining }) } catch (error) { if (cleanupIntentRecorded) { @@ -279,6 +269,13 @@ export async function removeHost(hostId: string): Promise { } catch { // Base removal is authoritative; a retained overlay can't resurrect the host and is cleaned on a later retry. } + try { + if (orphanedPublicKeyB64) { + await deleteMobileWebPagePreferences(orphanedPublicKeyB64) + } + } catch { + // Base removal is authoritative; an orphaned preference blob is inert without the pairing. + } // Why: keychain delete can stall/reject; await only the durable cleanup intent so removeHost can't freeze the UI. try { await scheduleUnpairedHostCredentialCleanup(hostId) @@ -302,7 +299,7 @@ export async function updateHostNameAndEndpoint( hostId: string, updates: { name?: string; endpoint?: string } ): Promise { - await mutateStoredHosts((hosts) => { + await hostListMutations.mutateStoredHosts((hosts) => { const index = hosts.findIndex((host) => host.id === hostId) if (index === -1) { throw new Error('Host not found') @@ -319,7 +316,7 @@ export async function updateHostNameAndEndpoint( export async function updateLastConnected(hostId: string): Promise { try { - await mutateStoredHosts((hosts) => { + await hostListMutations.mutateStoredHosts((hosts) => { const index = hosts.findIndex((h) => h.id === hostId) if (index === -1) { return hosts @@ -335,7 +332,7 @@ export async function updateLastConnected(hostId: string): Promise { /** Test-only: drain module mutation chain between cases. */ export function resetHostStoreForTests(): void { - hostListMutation = Promise.resolve() + hostListMutations.resetHostListMutationQueueForTests() tokenCache.clear() resetHostCredentialWriteRevisionsForTests() hostListLoads.dropSharedHostListLoad()