diff --git a/mobile/src/session/use-mobile-session-view-mode.test.ts b/mobile/src/session/use-mobile-session-view-mode.test.ts index 4008ea8bd13..6cf6687f22d 100644 --- a/mobile/src/session/use-mobile-session-view-mode.test.ts +++ b/mobile/src/session/use-mobile-session-view-mode.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { loadDefaultSessionView, loadSessionViewOverrides, + readSessionViewOverridesPreference, updateSessionViewOverride, type MobileSessionView } from '../storage/session-view-preferences' @@ -44,6 +45,7 @@ vi.mock('expo-router', async () => { vi.mock('../storage/session-view-preferences', () => ({ loadDefaultSessionView: vi.fn(), loadSessionViewOverrides: vi.fn(), + readSessionViewOverridesPreference: vi.fn(), updateSessionViewOverride: vi.fn() })) @@ -55,6 +57,9 @@ describe('useMobileSessionViewMode', () => { globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.mocked(loadDefaultSessionView).mockReset().mockResolvedValue('terminal') vi.mocked(loadSessionViewOverrides).mockReset().mockResolvedValue(new Map()) + vi.mocked(readSessionViewOverridesPreference) + .mockReset() + .mockResolvedValue({ overrides: new Map(), loaded: true }) vi.mocked(updateSessionViewOverride).mockReset().mockResolvedValue(undefined) focusEffectRuntime.callback = null }) @@ -70,7 +75,10 @@ describe('useMobileSessionViewMode', () => { overrides?: Map }): Promise { vi.mocked(loadDefaultSessionView).mockResolvedValue(args.defaultView) - vi.mocked(loadSessionViewOverrides).mockResolvedValue(args.overrides ?? new Map()) + vi.mocked(readSessionViewOverridesPreference).mockResolvedValue({ + overrides: args.overrides ?? new Map(), + loaded: true + }) function Harness(): null { controller = useMobileSessionViewMode({ hostId: 'h', worktreeId: 'w' }) return null @@ -124,7 +132,7 @@ describe('useMobileSessionViewMode', () => { expect(updateSessionViewOverride).toHaveBeenLastCalledWith('h', 'w', 't1', 'terminal') expect(controller?.isTabChatView('t1')).toBe(false) - expect(loadSessionViewOverrides).toHaveBeenCalledTimes(1) + expect(readSessionViewOverridesPreference).toHaveBeenCalledTimes(1) }) it('toggles a terminal-default tab into chat', async () => { @@ -141,10 +149,13 @@ describe('useMobileSessionViewMode', () => { it('does not expose overrides from the previous host while the next scope loads', async () => { const nextScopeLoad = deferred>() - vi.mocked(loadSessionViewOverrides).mockImplementation((hostId) => + vi.mocked(readSessionViewOverridesPreference).mockImplementation((hostId) => hostId === 'h1' - ? Promise.resolve(new Map([['same-tab-id', 'chat' as const]])) - : nextScopeLoad.promise + ? Promise.resolve({ + overrides: new Map([['same-tab-id', 'chat' as const]]), + loaded: true + }) + : nextScopeLoad.promise.then((overrides) => ({ overrides, loaded: true })) ) function Harness(props: { hostId: string; worktreeId: string }): null { controller = useMobileSessionViewMode(props) @@ -171,7 +182,9 @@ describe('useMobileSessionViewMode', () => { it('merges a toggle made during load with the other persisted overrides', async () => { const overridesLoad = deferred>() - vi.mocked(loadSessionViewOverrides).mockReturnValue(overridesLoad.promise) + vi.mocked(readSessionViewOverridesPreference).mockReturnValue( + overridesLoad.promise.then((overrides) => ({ overrides, loaded: true })) + ) function Harness(): null { controller = useMobileSessionViewMode({ hostId: 'h', worktreeId: 'w' }) return null @@ -218,7 +231,8 @@ describe('useMobileSessionViewMode', () => { }) expect(controller?.isTabChatView('t1')).toBe(false) - expect(loadSessionViewOverrides).toHaveBeenCalledTimes(2) + expect(readSessionViewOverridesPreference).toHaveBeenCalledTimes(1) + expect(loadSessionViewOverrides).toHaveBeenCalledTimes(1) }) it('does not let an older failed override roll back a newer choice', async () => { @@ -247,7 +261,9 @@ describe('useMobileSessionViewMode', () => { it('finishes an early toggle save after the route unmounts', async () => { const overridesLoad = deferred>() - vi.mocked(loadSessionViewOverrides).mockReturnValue(overridesLoad.promise) + vi.mocked(readSessionViewOverridesPreference).mockReturnValue( + overridesLoad.promise.then((overrides) => ({ overrides, loaded: true })) + ) function Harness(): null { controller = useMobileSessionViewMode({ hostId: 'h', worktreeId: 'w' }) return null @@ -283,6 +299,27 @@ describe('useMobileSessionViewMode', () => { await Promise.resolve() }) - expect(loadSessionViewOverrides).toHaveBeenCalledTimes(1) + expect(readSessionViewOverridesPreference).toHaveBeenCalledTimes(1) + expect(loadSessionViewOverrides).not.toHaveBeenCalled() + }) + + it('fails closed to terminal when overrides cannot be read under a chat default', async () => { + vi.mocked(loadDefaultSessionView).mockResolvedValue('chat') + vi.mocked(readSessionViewOverridesPreference).mockResolvedValue({ + overrides: new Map(), + loaded: false + }) + + function Harness(): null { + controller = useMobileSessionViewMode({ hostId: 'h', worktreeId: 'w' }) + return null + } + await act(async () => { + renderer = create(createElement(Harness)) + await Promise.resolve() + await Promise.resolve() + }) + + expect(controller?.isTabChatView('t1')).toBe(false) }) }) diff --git a/mobile/src/session/use-mobile-session-view-mode.ts b/mobile/src/session/use-mobile-session-view-mode.ts index 61668808acd..5b8faa59ef4 100644 --- a/mobile/src/session/use-mobile-session-view-mode.ts +++ b/mobile/src/session/use-mobile-session-view-mode.ts @@ -3,8 +3,10 @@ import { useFocusEffect } from 'expo-router' import { loadDefaultSessionView, loadSessionViewOverrides, + readSessionViewOverridesPreference, updateSessionViewOverride, - type MobileSessionView + type MobileSessionView, + type SessionViewOverridesPreference } from '../storage/session-view-preferences' type ViewOverridesState = { @@ -17,7 +19,7 @@ type ViewOverridesState = { type ViewOverridesRuntime = { hostId: string worktreeId: string - loadPromise: Promise> + loadPromise: Promise currentOverrides: Map mutationRevisions: Map } @@ -75,7 +77,7 @@ export function useMobileSessionViewMode(args: { const next: ViewOverridesRuntime = { hostId: scopeHostId, worktreeId: scopeWorktreeId, - loadPromise: loadSessionViewOverrides(scopeHostId, scopeWorktreeId), + loadPromise: readSessionViewOverridesPreference(scopeHostId, scopeWorktreeId), currentOverrides: new Map(), mutationRevisions: new Map() } @@ -91,15 +93,17 @@ export function useMobileSessionViewMode(args: { useEffect(() => { let active = true const runtime = ensureViewOverridesRuntime(hostId, worktreeId) - void runtime.loadPromise.then((persisted) => { + void runtime.loadPromise.then((preference) => { if (!active) { return } // Why: toggles made during the read are authoritative, but must not // discard unrelated persisted overrides from the same worktree. - const merged = mergeOverrides(persisted, runtime.currentOverrides) + const merged = mergeOverrides(preference.overrides, runtime.currentOverrides) runtime.currentOverrides = merged - const next = { hostId, worktreeId, overrides: merged, loaded: true } + // Why: an unreadable override store cannot safely be treated as empty when + // the default is chat; fail closed to terminal until a user toggles. + const next = { hostId, worktreeId, overrides: merged, loaded: preference.loaded } viewOverridesStateRef.current = next setViewOverridesState(next) }) diff --git a/mobile/src/storage/preferences.test.ts b/mobile/src/storage/preferences.test.ts index 5b84e889c6f..13e43d45b43 100644 --- a/mobile/src/storage/preferences.test.ts +++ b/mobile/src/storage/preferences.test.ts @@ -24,6 +24,7 @@ import { import { loadDefaultSessionView, loadSessionViewOverrides, + readSessionViewOverridesPreference, saveDefaultSessionView, updateSessionViewOverride } from './session-view-preferences' @@ -213,6 +214,11 @@ describe('session view preference', () => { it('does not replace saved overrides after a transient read failure', async () => { vi.mocked(AsyncStorage.getItem).mockRejectedValue(new Error('storage unavailable')) + await expect(readSessionViewOverridesPreference('host', 'worktree')).resolves.toEqual({ + overrides: new Map(), + loaded: false + }) + await expect(updateSessionViewOverride('host', 'worktree', 'tab', 'chat')).rejects.toThrow( 'Session view overrides could not be read' ) diff --git a/mobile/src/storage/session-view-preferences.ts b/mobile/src/storage/session-view-preferences.ts index a09b2198ac9..750167e3d53 100644 --- a/mobile/src/storage/session-view-preferences.ts +++ b/mobile/src/storage/session-view-preferences.ts @@ -45,12 +45,14 @@ export function saveDefaultSessionView(view: MobileSessionView): Promise { return write } -type SessionViewOverridesRead = { +export type SessionViewOverridesPreference = { overrides: Map loaded: boolean } -async function readSessionViewOverrides(key: string): Promise { +async function readSessionViewOverridesStorage( + key: string +): Promise { let raw: string | null try { raw = await AsyncStorage.getItem(key) @@ -95,9 +97,17 @@ export async function loadSessionViewOverrides( hostId: string, worktreeId: string ): Promise> { + return (await readSessionViewOverridesPreference(hostId, worktreeId)).overrides +} + +/** Reads overrides without conflating an empty preference with unavailable storage. */ +export async function readSessionViewOverridesPreference( + hostId: string, + worktreeId: string +): Promise { const key = sessionViewOverridesKey(hostId, worktreeId) await overrideUpdateBarriers.get(key) - return (await readSessionViewOverrides(key)).overrides + return readSessionViewOverridesStorage(key) } /** Persists one user mutation without replacing sibling overrides from another mount. */ @@ -110,7 +120,7 @@ export async function updateSessionViewOverride( const key = sessionViewOverridesKey(hostId, worktreeId) const previous = overrideUpdateBarriers.get(key) ?? Promise.resolve() const update = previous.then(async () => { - const current = await readSessionViewOverrides(key) + const current = await readSessionViewOverridesStorage(key) // Why: a transient read failure must not replace valid saved siblings with // a partial map containing only the latest tab. if (!current.loaded) {