fix(native-chat): fail closed on unreadable view overrides

This commit is contained in:
Brennan Benson
2026-07-16 19:09:01 -07:00
parent c7bf535f97
commit cc174c6e13
4 changed files with 76 additions and 19 deletions
@@ -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<string, MobileSessionView>
}): Promise<void> {
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<Map<string, MobileSessionView>>()
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<Map<string, MobileSessionView>>()
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<Map<string, MobileSessionView>>()
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)
})
})
@@ -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<Map<string, MobileSessionView>>
loadPromise: Promise<SessionViewOverridesPreference>
currentOverrides: Map<string, MobileSessionView>
mutationRevisions: Map<string, number>
}
@@ -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)
})
+6
View File
@@ -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'
)
+14 -4
View File
@@ -45,12 +45,14 @@ export function saveDefaultSessionView(view: MobileSessionView): Promise<void> {
return write
}
type SessionViewOverridesRead = {
export type SessionViewOverridesPreference = {
overrides: Map<string, MobileSessionView>
loaded: boolean
}
async function readSessionViewOverrides(key: string): Promise<SessionViewOverridesRead> {
async function readSessionViewOverridesStorage(
key: string
): Promise<SessionViewOverridesPreference> {
let raw: string | null
try {
raw = await AsyncStorage.getItem(key)
@@ -95,9 +97,17 @@ export async function loadSessionViewOverrides(
hostId: string,
worktreeId: string
): Promise<Map<string, MobileSessionView>> {
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<SessionViewOverridesPreference> {
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) {