From 641e060918151f901ef637e00563cf3516d79290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:36:29 +0000 Subject: [PATCH 1/7] fix(mobile): store a preference choice before showing it The shared SecureStore preference store wrote fire-and-forget: set() moved the value the app shows and started a write in the background, so a burst of toggles queued one write per toggle and the newest choice sat behind the older ones. A process teardown right after a choice left the app showing a value the disk did not hold. The store now keeps one write in flight, writes only the newest requested target, and applies a value only when its write lands. A failed write toasts and leaves the value on the last stored one. clear() fences an in-flight write so a sign-out cannot repaint the reset value, and mergeOnLoad still reconciles a choice made before the disk read settles. --- .../lib/hooks/secure-store-preference.test.ts | 60 ++++++- .../src/lib/hooks/secure-store-preference.ts | 158 +++++++++++++++--- 2 files changed, 196 insertions(+), 22 deletions(-) diff --git a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts index 903a61091af..eb4c087b411 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts @@ -99,7 +99,7 @@ describe('createSecureStorePreference', () => { } }); - it('shows a toast on a write failure while keeping the in-memory value', async () => { + it('shows a toast and keeps the last stored value when the write fails', async () => { getItemAsync.mockResolvedValue(null); setItemAsync.mockRejectedValue(new Error('disk full')); const store = createSecureStorePreference({ @@ -110,12 +110,66 @@ describe('createSecureStorePreference', () => { }); store.set(true); - expect(store.get()).toBe(true); + // The choice is not stored, so the app does not claim it. + expect(store.get()).toBe(false); - await flushMicrotasks(); + await store.whenPersisted(); expect(toastError).toHaveBeenCalledWith('Could not save setting'); + expect(store.get()).toBe(false); + }); + + it('applies a choice only once its write lands', async () => { + getItemAsync.mockResolvedValue(null); + const writeGate = Promise.withResolvers(); + setItemAsync.mockImplementation(async () => { + await writeGate.promise; + }); + const store = createSecureStorePreference({ + key: 'k', + defaultValue: false, + parse: raw => raw === 'true', + serialize: value => (value ? 'true' : 'false'), + }); + + store.set(true); + // In flight: the newest choice is what the next press builds on, but the + // rendered value is still the stored one. + expect(store.getRequested()).toBe(true); + expect(store.get()).toBe(false); + + writeGate.resolve(undefined); + await store.whenPersisted(); + expect(store.get()).toBe(true); + expect(setItemAsync).toHaveBeenCalledTimes(1); + }); + + it('writes the newest choice, not every choice, when several land during one write', async () => { + getItemAsync.mockResolvedValue(null); + const firstWriteGate = Promise.withResolvers(); + setItemAsync.mockImplementationOnce(async () => { + await firstWriteGate.promise; + }); + const store = createSecureStorePreference({ + key: 'k', + defaultValue: false, + parse: raw => raw === 'true', + serialize: value => (value ? 'true' : 'false'), + }); + + store.set(true); + store.set(false); + store.set(true); + store.set(false); + + firstWriteGate.resolve(undefined); + await store.whenPersisted(); + + // The first write was already on its way; every later choice collapsed into + // the newest one, which therefore lands after it. + expect(setItemAsync.mock.calls.map(call => call[1])).toEqual(['true', 'false']); + expect(store.get()).toBe(false); }); it('lets a set() before the initial load resolves win over the disk value', async () => { diff --git a/apps/mobile/src/lib/hooks/secure-store-preference.ts b/apps/mobile/src/lib/hooks/secure-store-preference.ts index f2ebb6d1e1d..5a50053c095 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.ts @@ -13,6 +13,12 @@ function noop(): void { * Module-level store for a SecureStore-backed preference so every hook * instance (settings sheet, message list, new-session screen) shares one * value and one disk read. Consume via useSyncExternalStore. + * + * The rendered value is the durable one: `set()` records a target, one write at + * a time carries the latest target to disk, and the value the app shows moves + * only when its write lands. A burst of choices therefore writes at most the + * in-flight value plus the newest one — never a queue of stale values — and the + * app never shows a choice the disk does not hold. */ export function createSecureStorePreference(options: { key: string; @@ -29,7 +35,21 @@ export function createSecureStorePreference(options: { }) { const { key, defaultValue, parse, serialize, mergeOnLoad } = options; let value = defaultValue; + /** + * The newest target whose write has not been persisted yet. A second choice + * made while one write is in flight replaces it, so superseded choices are + * never written and an older write can never land after a newer one. + */ + let requested: T | undefined = undefined; + /** + * The target of the write currently on its way to disk. A read that derives + * the next value (add to a list, flip a switch) must see it, or two changes in + * one frame would both start from the value before the first one. + */ + let inFlight: T | undefined = undefined; + let writing = false; let hasLoaded = false; + let loadSettled = false; // A set() or clear() before the initial load resolves must win over the // disk value. let dirty = false; @@ -37,6 +57,12 @@ export function createSecureStorePreference(options: { // value even when mergeOnLoad is set. let cleared = false; let loadStarted = false; + /** + * Bumped by clear(). A write that started under an earlier epoch still lands + * (the delete queued behind it wins), but it must not repaint the value the + * clear just reset. + */ + let writeEpoch = 0; let markLoaded = noop; // Resolves once the disk read settles, so a caller with no React tree (the // Android widget task) can await the stored value instead of reading the @@ -44,6 +70,9 @@ export function createSecureStorePreference(options: { const loaded = new Promise(resolve => { markLoaded = resolve; }); + // Callers that must observe durability (tests, the language apply) await the + // queue draining instead of polling. + let drainWaiters: (() => void)[] = []; const listeners = new Set<() => void>(); const emit = () => { @@ -52,17 +81,102 @@ export function createSecureStorePreference(options: { } }; + /** + * The newest value the app has asked for: the queued target, the target being + * written, or the durable value. Relative edits (a list add, a switch flip) + * start from this so two changes in one frame both survive. + */ + const currentRequested = (): T => requested ?? inFlight ?? value; + + const releaseDrainWaiters = () => { + if (writing || requested !== undefined) { + return; + } + const released = drainWaiters; + drainWaiters = []; + for (const release of released) { + release(); + } + }; + + /** Applies a value the disk now holds. Skipped once clear() superseded it. */ + const applyPersisted = (next: T, epoch: number) => { + if (epoch !== writeEpoch) { + return; + } + value = next; + hasLoaded = true; + emit(); + }; + + /** + * Writes the current target, then the newest target if one arrived while that + * write was in flight. One write at a time keeps the disk on the last choice; + * replacing {@link requested} keeps superseded choices off the disk. + */ + const drainWrites = async () => { + if (writing) { + return; + } + writing = true; + try { + while (requested !== undefined) { + const target = requested; + requested = undefined; + inFlight = target; + const epoch = writeEpoch; + try { + // eslint-disable-next-line no-await-in-loop -- one write at a time is the ordering guarantee + await setAccountMetadata(key, serialize(target)); + } catch { + inFlight = undefined; + // The stored value stays the last one that landed, so the app keeps + // showing it: the toast is the only claim the new choice failed. + toast.error(i18n.t('common.couldNotSaveSetting')); + if (!loadSettled && !cleared) { + // Nothing of the user's is pending any more; let the in-flight + // initial read describe what is actually stored. + dirty = false; + } + break; + } + applyPersisted(target, epoch); + inFlight = undefined; + } + } finally { + writing = false; + releaseDrainWaiters(); + } + }; + + const whenPersisted = async (): Promise => { + if (!writing && requested === undefined) { + return; + } + await new Promise(resolve => { + drainWaiters.push(resolve); + }); + }; + const load = async () => { try { const raw = await readStoredValueWithRetry(key); if (!dirty) { value = parse(raw); } else if (mergeOnLoad && !cleared) { - value = mergeOnLoad(parse(raw), value); - // The pending write raced the disk read and already persisted only - // the partial value; persist the merged value so the persisted list - // is not overwritten. - void persist(value); + // The disk value and the choice made before the read settled are both + // real entries, so the merge is applied at once: it already contains + // everything the disk holds. The write that raced the read is + // superseded here (its own value is inside the merge), so it must not + // paint over the merged value; the write queued below reconciles the + // disk with the merged list. + const pendingBeforeLoad = currentRequested(); + writeEpoch += 1; + inFlight = undefined; + value = mergeOnLoad(parse(raw), pendingBeforeLoad); + emit(); + requested = value; + void drainWrites(); } } catch (error) { // Keep the default on read failure — this runs on mount, before the @@ -75,23 +189,13 @@ export function createSecureStorePreference(options: { }, }); } finally { + loadSettled = true; hasLoaded = true; markLoaded(); emit(); } }; - const persist = async (next: T) => { - try { - await setAccountMetadata(key, serialize(next)); - } catch { - // Keep the in-memory preference so the session still works, but the - // change won't survive relaunch — tell the user so it's not a silent - // surprise later. - toast.error(i18n.t('common.couldNotSaveSetting')); - } - }; - const remove = async () => { try { await deleteAccountMetadata(key); @@ -127,14 +231,23 @@ export function createSecureStorePreference(options: { }, get: () => value, getHasLoaded: () => hasLoaded, + /** + * The value the next relative change starts from: the newest requested + * target (or the one being written) while a write is pending, otherwise the + * durable value. Toggles and list edits read this so rapid changes each + * build on the state the person last asked for, not a render that a pending + * write has not reached yet. + */ + getRequested: currentRequested, set: (next: T) => { - value = next; + requested = next; dirty = true; cleared = false; hasLoaded = true; - emit(); - void persist(next); + void drainWrites(); }, + /** Resolves once every requested value has been written or dropped. */ + whenPersisted, /** * Persist-then-apply write for callers that must not change memory on a * failed disk write (e.g. language apply). Writes disk first; on success @@ -143,6 +256,7 @@ export function createSecureStorePreference(options: { * toast language when the caller already switched the active language. */ setAsync: async (next: T, toastLng?: string): Promise => { + await whenPersisted(); try { await setAccountMetadata(key, serialize(next)); } catch { @@ -157,6 +271,11 @@ export function createSecureStorePreference(options: { }, /** Reset memory and disk (e.g. on sign-out). */ clear: () => { + // A write already on its way to disk must not repaint the value this + // clear resets; the delete it queues behind still removes the entry. + writeEpoch += 1; + requested = undefined; + inFlight = undefined; value = defaultValue; // Keep dirty set so an in-flight initial read does not restore the old // value after sign-out. @@ -166,6 +285,7 @@ export function createSecureStorePreference(options: { cleared = true; emit(); void remove(); + releaseDrainWaiters(); }, }; } From 8f1b17ee27bf13f645220bba5337ae3f66fabf20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:37:12 +0000 Subject: [PATCH 2/7] fix(mobile): build a preference change on the newest choice A preference now becomes visible when its write lands, so a read that derives the next value has to start from the newest requested one instead of the rendered one. Getters and list mutators (chat backends, remote MCP servers and their OAuth state, trusted hosts and images, the agent model map, the settings registry) read that value, and the two settings switches resolve it at press time through PreferenceRow's new resolver, so rapid taps each flip the state the person last asked for. --- ...tool-summary-translation-settings-screen.tsx | 4 +++- .../mobile/src/components/ui/preference-row.tsx | 11 ++++++++++- .../components/voice-input-settings-screen.tsx | 2 ++ apps/mobile/src/lib/chat/backend-store.ts | 8 ++++---- .../src/lib/chat/remote-mcp-oauth-store.ts | 6 +++--- apps/mobile/src/lib/chat/remote-mcp-store.ts | 10 +++++----- .../src/lib/chat/settings-tools-switch.test.ts | 4 +++- .../src/lib/chat/settings-tools-switch.ts | 2 +- apps/mobile/src/lib/chat/sign-out.test.ts | 10 ++++++++++ ...se-collapsed-connect-ctas-preference.test.ts | 17 ++++++++++++----- .../use-collapsed-connect-ctas-preference.ts | 2 +- .../use-condense-tool-calls-preference.test.ts | 9 ++++----- .../hooks/use-condense-tool-calls-preference.ts | 2 +- .../lib/hooks/use-hide-balance-preference.ts | 2 +- .../lib/hooks/use-hide-thinking-preference.ts | 2 +- .../lib/hooks/use-keep-screen-on-preference.ts | 2 +- .../src/lib/hooks/use-language-preference.ts | 4 ++-- .../hooks/use-live-activity-preference.test.ts | 4 +++- .../lib/hooks/use-live-activity-preference.ts | 2 +- .../src/lib/hooks/use-persisted-agent-model.ts | 4 ++-- .../hooks/use-pr-review-footer-preference.ts | 2 +- .../src/lib/hooks/use-reasoning-preference.ts | 2 +- .../use-return-sends-message-preference.ts | 2 +- .../src/lib/hooks/use-theme-preference.ts | 2 +- apps/mobile/src/lib/hooks/use-trusted-hosts.ts | 6 +++--- .../src/lib/hooks/use-trusted-image-hosts.ts | 6 +++--- .../tool-summary-translation-preference.ts | 17 +++++++++++++---- .../gateway/gateway-transcription-preference.ts | 15 ++++++++++++--- .../voice-input-language-preference.ts | 2 +- 29 files changed, 106 insertions(+), 55 deletions(-) diff --git a/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx b/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx index 7be9b329e97..51bcb316a88 100644 --- a/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx +++ b/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx @@ -32,7 +32,8 @@ const NO_MODEL_VALUE = '—'; */ export function ToolSummaryTranslationSettingsScreen() { const { organizationId } = useOrganization(); - const { enabled, model, hasLoaded, setEnabled, setModel } = useToolSummaryTranslationPreference(); + const { enabled, model, hasLoaded, setEnabled, setModel, getRequestedEnabled } = + useToolSummaryTranslationPreference(); const { models, isLoading, isError, isFetching, isFetched, refetch } = useAvailableModels( organizationId ?? undefined ); @@ -106,6 +107,7 @@ export function ToolSummaryTranslationSettingsScreen() { value={enabled} disabled={!hasLoaded} onValueChange={setEnabled} + resolveValue={getRequestedEnabled} /> void; + /** + * The value a press should flip. Defaults to the rendered `value`, which is + * the last render, so two presses inside one frame both flip the same state + * and the second one repeats the first. A store that lags its own writes + * passes its newest requested value here, and every press then flips the + * state the person last asked for. + */ + resolveValue?: () => boolean; /** * Accessibility label for the switch. Defaults to the row title; pass an * action label when the title names the setting and the action differs @@ -31,6 +39,7 @@ export function PreferenceRow({ disabled, busy = false, onValueChange, + resolveValue, switchAccessibilityLabel, }: PreferenceRowProps) { const colors = useThemeColors(); @@ -45,7 +54,7 @@ export function PreferenceRow({ className="min-h-11 min-w-0 flex-1 flex-row items-center gap-3 active:opacity-70" disabled={disabled} onPress={() => { - onValueChange(!value); + onValueChange(!(resolveValue?.() ?? value)); }} accessibilityRole="switch" accessibilityLabel={switchAccessibilityLabel ?? title} diff --git a/apps/mobile/src/components/voice-input-settings-screen.tsx b/apps/mobile/src/components/voice-input-settings-screen.tsx index 48002e2816b..d7489ddc113 100644 --- a/apps/mobile/src/components/voice-input-settings-screen.tsx +++ b/apps/mobile/src/components/voice-input-settings-screen.tsx @@ -38,6 +38,7 @@ export function VoiceInputSettingsScreen() { gatewayTranscriptionEnabled, hasLoaded: gatewayTranscriptionLoaded, setGatewayTranscriptionEnabled, + getRequestedGatewayTranscriptionEnabled, } = useGatewayTranscriptionPreference(); const { status, model, isFetching, refetch } = useGatewayTranscriptionModelSelection( organizationId ?? undefined @@ -101,6 +102,7 @@ export function VoiceInputSettingsScreen() { value={gatewayTranscriptionEnabled} disabled={!gatewayTranscriptionLoaded} onValueChange={setGatewayTranscriptionEnabled} + resolveValue={getRequestedGatewayTranscriptionEnabled} /> ({ }); store.preload(); -export const listChatBackends = store.get; +export const listChatBackends = store.getRequested; export const subscribeChatBackends = store.subscribe; export const getChatBackendsHasLoaded = store.getHasLoaded; export const waitForChatBackends = store.whenLoaded; @@ -236,7 +236,7 @@ export function addChatBackend(draft: ChatBackendDraft): StoredChatBackend { id: randomUUID(), revision: 1, }); - const profiles = [...store.get(), backend]; + const profiles = [...store.getRequested(), backend]; Object.freeze(profiles); store.set(profiles); return backend; @@ -246,7 +246,7 @@ export function updateChatBackend(id: string, draft: ChatBackendDraft): StoredCh if (!store.getHasLoaded()) { throw new BackendInputError('notLoaded'); } - const profiles = store.get(); + const profiles = store.getRequested(); const previous = profiles.find(profile => profile.id === id); if (!previous || previous.revision >= Number.MAX_SAFE_INTEGER) { throw new BackendInputError('staleBackend'); @@ -266,7 +266,7 @@ export function deleteChatBackend(id: string): void { if (!store.getHasLoaded()) { throw new BackendInputError('notLoaded'); } - const profiles = store.get().filter(profile => profile.id !== id); + const profiles = store.getRequested().filter(profile => profile.id !== id); Object.freeze(profiles); store.set(profiles); } diff --git a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.ts b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.ts index 43151476130..d9183d67824 100644 --- a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.ts +++ b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.ts @@ -78,7 +78,7 @@ export async function readRemoteMcpOAuth( url: string ): Promise { await store.whenLoaded(); - const state = store.get()[id]; + const state = store.getRequested()[id]; return state?.url === url ? state : undefined; } @@ -97,7 +97,7 @@ export async function writeRemoteMcpOAuth( return; } const { id, url } = owner; - const current = store.get(); + const current = store.getRequested(); const existing = current[id]?.url === url ? current[id] : undefined; store.set({ ...current, [id]: { ...existing, ...patch, url } }); } @@ -105,7 +105,7 @@ export async function writeRemoteMcpOAuth( /** Drops one server's state, which is what deleting the server or changing its URL does. */ export async function forgetRemoteMcpOAuth(id: string): Promise { await store.whenLoaded(); - const { [id]: forgotten, ...rest } = store.get(); + const { [id]: forgotten, ...rest } = store.getRequested(); if (forgotten !== undefined) { store.set(rest); } diff --git a/apps/mobile/src/lib/chat/remote-mcp-store.ts b/apps/mobile/src/lib/chat/remote-mcp-store.ts index b97265d2dfd..116f7e685bb 100644 --- a/apps/mobile/src/lib/chat/remote-mcp-store.ts +++ b/apps/mobile/src/lib/chat/remote-mcp-store.ts @@ -184,7 +184,7 @@ store.preload(); /** The stored servers, in the order they were added. */ export function listRemoteMcpServers(): StoredRemoteMcpServer[] { - return store.get(); + return store.getRequested(); } /** @@ -193,7 +193,7 @@ export function listRemoteMcpServers(): StoredRemoteMcpServer[] { * two rows pointing at one endpoint is a user mistake, not two servers. */ export function addRemoteMcpServer(draft: RemoteMcpServerDraft): StoredRemoteMcpServer { - const current = store.get(); + const current = store.getRequested(); const url = normalizeRemoteMcpUrl(draft.url); if (current.some(server => server.url === url)) { throw new Error('Remote MCP URL is already saved.'); @@ -219,7 +219,7 @@ export function updateRemoteMcpServer( id: string, patch: RemoteMcpServerPatch ): StoredRemoteMcpServer { - const current = store.get(); + const current = store.getRequested(); const existing = current.find(server => server.id === id); if (existing === undefined) { throw new Error('Remote MCP server not found.'); @@ -244,7 +244,7 @@ export function updateRemoteMcpServer( /** Removes one server, and its OAuth tokens with it, and returns it. */ export function deleteRemoteMcpServer(id: string): StoredRemoteMcpServer { - const current = store.get(); + const current = store.getRequested(); const removed = current.find(server => server.id === id); if (removed === undefined) { throw new Error('Remote MCP server not found.'); @@ -256,7 +256,7 @@ export function deleteRemoteMcpServer(id: string): StoredRemoteMcpServer { /** Flips one server's enabled flag and leaves every other server untouched. */ export function setRemoteMcpServerEnabled(id: string, enabled: boolean): StoredRemoteMcpServer { - const current = store.get(); + const current = store.getRequested(); const existing = current.find(server => server.id === id); if (existing === undefined) { throw new Error('Remote MCP server not found.'); diff --git a/apps/mobile/src/lib/chat/settings-tools-switch.test.ts b/apps/mobile/src/lib/chat/settings-tools-switch.test.ts index 16a4cd84a99..938a8dab9ef 100644 --- a/apps/mobile/src/lib/chat/settings-tools-switch.test.ts +++ b/apps/mobile/src/lib/chat/settings-tools-switch.test.ts @@ -50,15 +50,17 @@ describe('settings tools group switch', () => { expect(setItemAsync).toHaveBeenLastCalledWith('settings-tools-enabled', 'false'); }); - it('notifies subscribers when the switch flips', () => { + it('notifies subscribers once the change is stored', async () => { const listener = vi.fn<() => void>(); const unsubscribe = subscribeSettingsToolsEnabled(listener); setSettingsToolsEnabled(false); + await flushMicrotasks(); expect(listener).toHaveBeenCalledTimes(1); unsubscribe(); setSettingsToolsEnabled(true); + await flushMicrotasks(); expect(listener).toHaveBeenCalledTimes(1); }); diff --git a/apps/mobile/src/lib/chat/settings-tools-switch.ts b/apps/mobile/src/lib/chat/settings-tools-switch.ts index 1fe42324459..af2cde6a08a 100644 --- a/apps/mobile/src/lib/chat/settings-tools-switch.ts +++ b/apps/mobile/src/lib/chat/settings-tools-switch.ts @@ -26,7 +26,7 @@ const store = createSecureStorePreference({ store.preload(); export function isSettingsToolsEnabled(): boolean { - return store.get(); + return store.getRequested(); } export function setSettingsToolsEnabled(enabled: boolean): void { diff --git a/apps/mobile/src/lib/chat/sign-out.test.ts b/apps/mobile/src/lib/chat/sign-out.test.ts index 1c915c336ec..893114f5395 100644 --- a/apps/mobile/src/lib/chat/sign-out.test.ts +++ b/apps/mobile/src/lib/chat/sign-out.test.ts @@ -91,6 +91,14 @@ function addServer(): void { }); } +// A preference becomes visible (and refreshes the snapshots built from it) once +// its write lands, so the tests let the queued write settle before asserting. +async function settlePreferences(): Promise { + await new Promise(resolve => { + setImmediate(resolve); + }); +} + beforeEach(() => { forgetChatPlaces(); storedQuestions.clear(); @@ -189,6 +197,7 @@ describe('the remote MCP connection and its stored servers', () => { it('are dropped when the account signs out', async () => { addServer(); setSettingsToolsEnabled(false); + await settlePreferences(); expect(listRemoteMcpServers()).toHaveLength(1); expect(remoteMcpState()).toHaveLength(1); expect(isSettingsToolsEnabled()).toBe(false); @@ -205,6 +214,7 @@ describe('the remote MCP connection and its stored servers', () => { it('are dropped when another account signs in', async () => { addServer(); setSettingsToolsEnabled(false); + await settlePreferences(); expect(listRemoteMcpServers()).toHaveLength(1); expect(remoteMcpState()).toHaveLength(1); diff --git a/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.test.ts b/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.test.ts index f57f576a756..1a32222739f 100644 --- a/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.test.ts +++ b/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.test.ts @@ -142,7 +142,7 @@ describe('collapsed-connect-ctas store', () => { unsubscribe(); }); - it('keeps the in-memory value and toasts Could not save setting when the write fails', async () => { + it('keeps the last stored list and toasts Could not save setting when the write fails', async () => { getItemAsync.mockResolvedValue(null); setItemAsync.mockRejectedValue(new Error('disk full')); const { setConnectCtaCollapsed } = await import('./use-collapsed-connect-ctas-preference'); @@ -150,17 +150,24 @@ describe('collapsed-connect-ctas store', () => { setConnectCtaCollapsed('github', true); await flushMicrotasks(); await flushMicrotasks(); + // A failed write can take a few turns of the queue to reject. + await new Promise(resolve => { + setImmediate(resolve); + }); + await new Promise(resolve => { + setImmediate(resolve); + }); expect(toastError).toHaveBeenCalledWith('Could not save setting'); expect(setItemAsync).toHaveBeenLastCalledWith('collapsed-connect-ctas', '["github"]'); - // The failed write still changed the session value: un-collapsing it now - // builds the empty list instead of returning early on unchanged membership. + // The failed choice was dropped, so the platform is not collapsed: the next + // request is the same membership the stored list already has and is skipped + // instead of queueing a second write. setItemAsync.mockClear(); setConnectCtaCollapsed('github', false); await flushMicrotasks(); - await flushMicrotasks(); - expect(setItemAsync).toHaveBeenLastCalledWith('collapsed-connect-ctas', '[]'); + expect(setItemAsync).not.toHaveBeenCalled(); }); it('clearCollapsedConnectCtasPreference deletes the key', async () => { diff --git a/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.ts b/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.ts index 9861e134580..86a04aeba81 100644 --- a/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.ts +++ b/apps/mobile/src/lib/hooks/use-collapsed-connect-ctas-preference.ts @@ -58,7 +58,7 @@ export function setConnectCtaCollapsed(platform: RepoPlatform, collapsed: boolea // Start the disk read if it has not run, so the write merges with the // persisted list instead of replacing it with the empty default. store.preload(); - const current = store.get(); + const current = store.getRequested(); const isCollapsed = current.includes(platform); // No membership change: skip the write, so useSyncExternalStore never loops // on a store notification that carries the same value. diff --git a/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.test.ts b/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.test.ts index 7e3374283ae..f332d44938e 100644 --- a/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.test.ts +++ b/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.test.ts @@ -132,18 +132,17 @@ describe('condense-tool-calls store', () => { unsubscribe(); }); - it('keeps the in-memory value and toasts common.couldNotSaveSetting when the write fails', async () => { + it('keeps the last stored value and toasts common.couldNotSaveSetting when the write fails', async () => { getItemAsync.mockResolvedValue(null); setItemAsync.mockRejectedValue(new Error('disk full')); const store = await makeStore(); store.set(true); - expect(store.get()).toBe(true); - - await flushMicrotasks(); + await store.whenPersisted(); expect(toastError).toHaveBeenCalledWith('Could not save setting'); - expect(store.get()).toBe(true); + // Nothing was stored, so the row keeps showing the stored value. + expect(store.get()).toBe(false); }); it('toasts common.couldNotSaveSetting when the real preference write fails', async () => { diff --git a/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.ts b/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.ts index 4d49c82c0b2..fabe32a832a 100644 --- a/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.ts +++ b/apps/mobile/src/lib/hooks/use-condense-tool-calls-preference.ts @@ -25,7 +25,7 @@ export function setCondenseToolCalls(value: boolean) { /** The stored condense-tool-calls value for callers with no React tree. */ export function getCondenseToolCalls(): boolean { - return store.get(); + return store.getRequested(); } export function useCondenseToolCallsPreference() { diff --git a/apps/mobile/src/lib/hooks/use-hide-balance-preference.ts b/apps/mobile/src/lib/hooks/use-hide-balance-preference.ts index 09d50b346e1..67fc2bc9fdc 100644 --- a/apps/mobile/src/lib/hooks/use-hide-balance-preference.ts +++ b/apps/mobile/src/lib/hooks/use-hide-balance-preference.ts @@ -25,7 +25,7 @@ export function setHideBalance(value: boolean) { /** The stored hide-balance value for callers with no React tree (the settings registry). */ export function getHideBalance(): boolean { - return store.get(); + return store.getRequested(); } export function useHideBalancePreference() { diff --git a/apps/mobile/src/lib/hooks/use-hide-thinking-preference.ts b/apps/mobile/src/lib/hooks/use-hide-thinking-preference.ts index 9a712e5dd61..00c305e6213 100644 --- a/apps/mobile/src/lib/hooks/use-hide-thinking-preference.ts +++ b/apps/mobile/src/lib/hooks/use-hide-thinking-preference.ts @@ -28,7 +28,7 @@ export function setHideThinking(value: boolean) { /** The stored hide-thinking value for callers with no React tree (the settings registry). */ export function getHideThinking(): boolean { - return store.get(); + return store.getRequested(); } export function useHideThinkingPreference() { diff --git a/apps/mobile/src/lib/hooks/use-keep-screen-on-preference.ts b/apps/mobile/src/lib/hooks/use-keep-screen-on-preference.ts index 5ce3c552e73..4d57d0e3ecd 100644 --- a/apps/mobile/src/lib/hooks/use-keep-screen-on-preference.ts +++ b/apps/mobile/src/lib/hooks/use-keep-screen-on-preference.ts @@ -28,7 +28,7 @@ export function setKeepScreenOn(value: boolean) { /** The stored keep-screen-on value for callers with no React tree (the settings registry). */ export function getKeepScreenOn(): boolean { - return store.get(); + return store.getRequested(); } export function useKeepScreenOnPreference() { diff --git a/apps/mobile/src/lib/hooks/use-language-preference.ts b/apps/mobile/src/lib/hooks/use-language-preference.ts index 76d3afb14bb..efb3a4085a7 100644 --- a/apps/mobile/src/lib/hooks/use-language-preference.ts +++ b/apps/mobile/src/lib/hooks/use-language-preference.ts @@ -27,13 +27,13 @@ const store = createSecureStorePreference({ /** The active language: the stored override, or the resolved device language. */ export function getResolvedLanguage(): SupportedLanguage { - const preference = store.get(); + const preference = store.getRequested(); return preference === 'device' ? resolveDeviceLanguage() : preference; } /** The raw stored preference (device or a supported tag). */ export function getLanguagePreference(): LanguagePreference { - return store.get(); + return store.getRequested(); } /** Apply and persist a preference; returns false when the disk write failed. */ diff --git a/apps/mobile/src/lib/hooks/use-live-activity-preference.test.ts b/apps/mobile/src/lib/hooks/use-live-activity-preference.test.ts index 2492672ce8c..3e0027aab17 100644 --- a/apps/mobile/src/lib/hooks/use-live-activity-preference.test.ts +++ b/apps/mobile/src/lib/hooks/use-live-activity-preference.test.ts @@ -66,15 +66,17 @@ describe('live-activity preference store', () => { expect(getLiveActivityEnabled()).toBe(true); }); - it('mirrors every write into the sink switch', async () => { + it('mirrors every stored write into the sink switch', async () => { getItemAsync.mockResolvedValue(null); const { setLiveActivityEnabled } = await import('./use-live-activity-preference'); const sink = await import('@/lib/glanceable/live-activity-switch'); setLiveActivityEnabled(false); + await flushMicrotasks(); expect(sink.getLiveActivityEnabled()).toBe(false); setLiveActivityEnabled(true); + await flushMicrotasks(); expect(sink.getLiveActivityEnabled()).toBe(true); }); diff --git a/apps/mobile/src/lib/hooks/use-live-activity-preference.ts b/apps/mobile/src/lib/hooks/use-live-activity-preference.ts index a82b47df11e..e2221b50c88 100644 --- a/apps/mobile/src/lib/hooks/use-live-activity-preference.ts +++ b/apps/mobile/src/lib/hooks/use-live-activity-preference.ts @@ -43,7 +43,7 @@ export function setLiveActivityEnabled(value: boolean) { /** The stored Live Activity value for callers with no React tree (the settings registry). */ export function getLiveActivityEnabled(): boolean { - return store.get(); + return store.getRequested(); } export function useLiveActivityPreference() { diff --git a/apps/mobile/src/lib/hooks/use-persisted-agent-model.ts b/apps/mobile/src/lib/hooks/use-persisted-agent-model.ts index a6fd32c9ab3..0ef2a76f18d 100644 --- a/apps/mobile/src/lib/hooks/use-persisted-agent-model.ts +++ b/apps/mobile/src/lib/hooks/use-persisted-agent-model.ts @@ -34,7 +34,7 @@ export function clearAgentModelPreference() { export function getStoredModelPreference( organizationId?: string ): ModelPreferenceEntry | undefined { - return store.get()[contextKey(organizationId)]; + return store.getRequested()[contextKey(organizationId)]; } /** Persist the model/variant default for one organization context. */ @@ -42,7 +42,7 @@ export function setDefaultModelForContext( organizationId: string | undefined, entry: ModelPreferenceEntry ): void { - store.set({ ...store.get(), [contextKey(organizationId)]: entry }); + store.set({ ...store.getRequested(), [contextKey(organizationId)]: entry }); } export function usePersistedAgentModel() { diff --git a/apps/mobile/src/lib/hooks/use-pr-review-footer-preference.ts b/apps/mobile/src/lib/hooks/use-pr-review-footer-preference.ts index cda53c170fc..2932d852448 100644 --- a/apps/mobile/src/lib/hooks/use-pr-review-footer-preference.ts +++ b/apps/mobile/src/lib/hooks/use-pr-review-footer-preference.ts @@ -29,7 +29,7 @@ export function setPrReviewFooter(value: boolean) { /** The stored PR-review footer value for callers with no React tree. */ export function getPrReviewFooter(): boolean { - return store.get(); + return store.getRequested(); } export function usePrReviewFooterPreference() { diff --git a/apps/mobile/src/lib/hooks/use-reasoning-preference.ts b/apps/mobile/src/lib/hooks/use-reasoning-preference.ts index b4088c599c1..2481154fd4b 100644 --- a/apps/mobile/src/lib/hooks/use-reasoning-preference.ts +++ b/apps/mobile/src/lib/hooks/use-reasoning-preference.ts @@ -16,7 +16,7 @@ export function clearReasoningPreference() { /** The stored default-expanded value for callers with no React tree (the settings registry). */ export function getAutoExpandThinking(): boolean { - return store.get(); + return store.getRequested(); } export function setAutoExpandThinking(value: boolean) { diff --git a/apps/mobile/src/lib/hooks/use-return-sends-message-preference.ts b/apps/mobile/src/lib/hooks/use-return-sends-message-preference.ts index 14b95b6b2b0..d6f13476154 100644 --- a/apps/mobile/src/lib/hooks/use-return-sends-message-preference.ts +++ b/apps/mobile/src/lib/hooks/use-return-sends-message-preference.ts @@ -21,7 +21,7 @@ export function setReturnSendsMessage(value: boolean) { /** The stored return-sends-message value for callers with no React tree. */ export function getReturnSendsMessage(): boolean { - return store.get(); + return store.getRequested(); } export function useReturnSendsMessagePreference() { diff --git a/apps/mobile/src/lib/hooks/use-theme-preference.ts b/apps/mobile/src/lib/hooks/use-theme-preference.ts index 72f9fb13296..1b857a0f5f0 100644 --- a/apps/mobile/src/lib/hooks/use-theme-preference.ts +++ b/apps/mobile/src/lib/hooks/use-theme-preference.ts @@ -34,7 +34,7 @@ export function applyThemePreference(pref: ThemePreference): void { /** The stored theme preference for callers with no React tree (the settings registry). */ export function getThemePreference(): ThemePreference { - return store.get(); + return store.getRequested(); } export function setThemePreference(pref: ThemePreference): void { diff --git a/apps/mobile/src/lib/hooks/use-trusted-hosts.ts b/apps/mobile/src/lib/hooks/use-trusted-hosts.ts index 0a0b4d7928c..6597f629877 100644 --- a/apps/mobile/src/lib/hooks/use-trusted-hosts.ts +++ b/apps/mobile/src/lib/hooks/use-trusted-hosts.ts @@ -46,14 +46,14 @@ export function useTrustedHosts() { } export function isTrustedHost(host: string): boolean { - return store.get().includes(host); + return store.getRequested().includes(host); } export function trustHost(host: string): void { // Start the disk read if it has not run, so the Trust write merges with the // persisted list instead of replacing it with the empty default. store.preload(); - const current = store.get(); + const current = store.getRequested(); if (current.includes(host)) { return; } @@ -61,7 +61,7 @@ export function trustHost(host: string): void { } export function getTrustedHosts(): string[] { - return store.get(); + return store.getRequested(); } /** Replace the trusted-host list. Destructive: a caller confirms before writing. */ diff --git a/apps/mobile/src/lib/hooks/use-trusted-image-hosts.ts b/apps/mobile/src/lib/hooks/use-trusted-image-hosts.ts index 4cf23f50056..b1076140a47 100644 --- a/apps/mobile/src/lib/hooks/use-trusted-image-hosts.ts +++ b/apps/mobile/src/lib/hooks/use-trusted-image-hosts.ts @@ -154,14 +154,14 @@ export function subscribeTrustedImageHosts(listener: () => void): () => void { } export function isTrustedImageHost(host: string): boolean { - return store.get().includes(host); + return store.getRequested().includes(host); } export function trustImageHost(host: string): void { // Start the disk read if it has not run, so the trust write merges with the // persisted list instead of replacing it with the empty default. store.preload(); - const current = store.get(); + const current = store.getRequested(); if (current.includes(host)) { return; } @@ -169,7 +169,7 @@ export function trustImageHost(host: string): void { } export function revokeImageHost(host: string): void { - store.set(store.get().filter(item => item !== host)); + store.set(store.getRequested().filter(item => item !== host)); } export function clearTrustedImageHosts(): void { diff --git a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-preference.ts b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-preference.ts index cd83856d424..e8437e4e61d 100644 --- a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-preference.ts +++ b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-preference.ts @@ -52,11 +52,11 @@ const modelStore = createSecureStorePreference({ }); export function isToolSummaryTranslationEnabled(): boolean { - return enabledStore.get(); + return enabledStore.getRequested(); } export function readToolSummaryTranslationModel(): ToolSummaryTranslationModel { - return modelStore.get(); + return modelStore.getRequested(); } export function setToolSummaryTranslationEnabled(next: boolean): void { @@ -88,9 +88,11 @@ function syncRuntime(): void { if (!hasLoadedPreferences()) { return; } + // The runtime follows what the disk holds, so a choice whose write failed + // never sends transcripts to a translation the app does not claim to have. setConfig({ - enabled: isToolSummaryTranslationEnabled(), - model: readToolSummaryTranslationModel(), + enabled: enabledStore.get(), + model: modelStore.get(), }); } @@ -108,6 +110,12 @@ export type ToolSummaryTranslationPreference = { hasLoaded: boolean; setEnabled: (next: boolean) => void; setModel: (next: ToolSummaryTranslationModel) => void; + /** + * The switch state a press should flip: the newest chosen value while its + * write is still on its way to disk, otherwise the stored one. A press reads + * this at tap time so a burst of taps each flips what the person last chose. + */ + getRequestedEnabled: () => boolean; }; export function useToolSummaryTranslationPreference(): ToolSummaryTranslationPreference { @@ -121,6 +129,7 @@ export function useToolSummaryTranslationPreference(): ToolSummaryTranslationPre hasLoaded: enabledLoaded && modelLoaded, setEnabled: setToolSummaryTranslationEnabled, setModel: writeToolSummaryTranslationModel, + getRequestedEnabled: enabledStore.getRequested, }; } diff --git a/apps/mobile/src/lib/voice-input/gateway/gateway-transcription-preference.ts b/apps/mobile/src/lib/voice-input/gateway/gateway-transcription-preference.ts index d852926d383..c9bd4d7f885 100644 --- a/apps/mobile/src/lib/voice-input/gateway/gateway-transcription-preference.ts +++ b/apps/mobile/src/lib/voice-input/gateway/gateway-transcription-preference.ts @@ -53,7 +53,7 @@ enabledStore.preload(); modelStore.preload(); export function isGatewayTranscriptionEnabled(): boolean { - return enabledStore.get(); + return enabledStore.getRequested(); } /** Non-React subscription for module-scope consumers (e.g. the native binding). */ @@ -66,7 +66,7 @@ export function setGatewayTranscriptionEnabled(value: boolean): void { } export function readGatewayTranscriptionModel(): GatewayTranscriptionModel | null { - return modelStore.get(); + return modelStore.getRequested(); } /** Await the persisted model read. For callers with no React tree. */ @@ -85,7 +85,16 @@ export function useGatewayTranscriptionPreference() { enabledStore.get ); const hasLoaded = useSyncExternalStore(enabledStore.subscribe, enabledStore.getHasLoaded); - return { gatewayTranscriptionEnabled, hasLoaded, setGatewayTranscriptionEnabled }; + return { + gatewayTranscriptionEnabled, + hasLoaded, + setGatewayTranscriptionEnabled, + /** + * The state a press should flip: the newest chosen value while its write is + * still on its way to disk, otherwise the stored one. + */ + getRequestedGatewayTranscriptionEnabled: enabledStore.getRequested, + }; } /** Settings UI binding for the chosen transcription model. */ diff --git a/apps/mobile/src/lib/voice-input/voice-input-language-preference.ts b/apps/mobile/src/lib/voice-input/voice-input-language-preference.ts index 53ab28cd5d6..893eedca568 100644 --- a/apps/mobile/src/lib/voice-input/voice-input-language-preference.ts +++ b/apps/mobile/src/lib/voice-input/voice-input-language-preference.ts @@ -21,7 +21,7 @@ const languageStore = createSecureStorePreference({ languageStore.preload(); export function readVoiceInputLanguage(): string | null { - return languageStore.get(); + return languageStore.getRequested(); } export function writeVoiceInputLanguage(tag: string | null): void { From 2ef918f17be5d3fbcef7dd42aa1a5e3b0ed46081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:54:53 +0000 Subject: [PATCH 3/7] fix(mobile): keep the preference queue correct at its edges Review follow-ups on the preference store: - a failed write now drops the queued target as well as the in-flight one, so the queue releases its waiters instead of stranding a newer choice forever; - the newest-value lookup compares undefined explicitly, so a nullable store (the voice-input language "auto", no run-on destination) reports a requested null instead of the previous stored value; - revokeHost builds from the newest value like trustHost already does; - the row and the native switch share one flip, so both controls move the value the person last asked for. Each edge has a case in secure-store-preference.test.ts or preference-row.mounted.test.tsx. --- .../ui/preference-row.mounted.test.tsx | 30 +++++++++++ .../src/components/ui/preference-row.tsx | 12 +++-- .../lib/hooks/secure-store-preference.test.ts | 51 +++++++++++++++++++ .../src/lib/hooks/secure-store-preference.ts | 18 ++++++- .../mobile/src/lib/hooks/use-trusted-hosts.ts | 2 +- 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/apps/mobile/src/components/ui/preference-row.mounted.test.tsx b/apps/mobile/src/components/ui/preference-row.mounted.test.tsx index dd9dbb369cd..13666f8ac21 100644 --- a/apps/mobile/src/components/ui/preference-row.mounted.test.tsx +++ b/apps/mobile/src/components/ui/preference-row.mounted.test.tsx @@ -35,6 +35,7 @@ function renderRow(props: { value: boolean; disabled?: boolean; onValueChange: (next: boolean) => void; + resolveValue?: () => boolean; }) { act(() => { const element = createElement(PreferenceRow, { @@ -102,6 +103,35 @@ describe('PreferenceRow', () => { expect(toggle.props.accessible).toBe(false); }); + it('flips the newest requested value, not the rendered one, on either control', () => { + // The rendered value lags a store write. Three presses in a row must each + // flip what the last one asked for, whichever control receives them. + let requested = false; + const seen: boolean[] = []; + const root = renderRow({ + value: false, + resolveValue: () => requested, + onValueChange: next => { + seen.push(next); + requested = next; + }, + }); + const label = root.find( + node => + Object.is(node.type, 'Pressable') && + node.props.accessibilityLabel === 'Let Kilo change app settings' + ); + const toggle = root.find(node => Object.is(node.type, 'Switch')); + + act(() => { + (label.props.onPress as () => void)(); + (toggle.props.onValueChange as (next: boolean) => void)(false); + (label.props.onPress as () => void)(); + }); + + expect(seen).toEqual([true, false, true]); + }); + it('names only the pressable, so a tap on the title is one control not two', () => { const root = renderRow({ value: true, diff --git a/apps/mobile/src/components/ui/preference-row.tsx b/apps/mobile/src/components/ui/preference-row.tsx index 22078c904ba..e149cdbbb7c 100644 --- a/apps/mobile/src/components/ui/preference-row.tsx +++ b/apps/mobile/src/components/ui/preference-row.tsx @@ -43,6 +43,12 @@ export function PreferenceRow({ switchAccessibilityLabel, }: PreferenceRowProps) { const colors = useThemeColors(); + // Both ways of moving the switch flip the same value: a tap on the row and a + // tap on the thumb can arrive in either order, and a burst must not build one + // flip from the rendered value and the next from the native thumb state. + const flip = () => { + onValueChange(!(resolveValue?.() ?? value)); + }; return ( {/* One named control: the title is what a tap names, so it has to move @@ -53,9 +59,7 @@ export function PreferenceRow({ { - onValueChange(!(resolveValue?.() ?? value)); - }} + onPress={flip} accessibilityRole="switch" accessibilityLabel={switchAccessibilityLabel ?? title} accessibilityHint={subtitle} @@ -92,7 +96,7 @@ export function PreferenceRow({ accessible={false} importantForAccessibility="no" accessibilityElementsHidden - onValueChange={onValueChange} + onValueChange={flip} /> ); diff --git a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts index eb4c087b411..aa411f79e7e 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts @@ -172,6 +172,57 @@ describe('createSecureStorePreference', () => { expect(store.get()).toBe(false); }); + it('reports a requested null for a nullable store and releases waiters after a failure', async () => { + getItemAsync.mockResolvedValue(null); + const writeGate = Promise.withResolvers(); + setItemAsync.mockImplementationOnce(async () => { + await writeGate.promise; + }); + const store = createSecureStorePreference({ + key: 'k', + defaultValue: null, + parse: raw => raw, + serialize: value => value ?? '', + }); + + store.set('chosen'); + // A null target is a real choice ("auto"), not "nothing pending". + store.set(null); + expect(store.getRequested()).toBeNull(); + + writeGate.resolve(undefined); + await store.whenPersisted(); + expect(store.get()).toBeNull(); + }); + + it('releases whenPersisted() and drops a newer choice when the write fails', async () => { + getItemAsync.mockResolvedValue(null); + const writeGate = Promise.withResolvers(); + setItemAsync.mockImplementation(async (_key: string, value: string) => { + await writeGate.promise; + if (value === 'true') { + throw new Error('disk full'); + } + }); + const store = createSecureStorePreference({ + key: 'k', + defaultValue: false, + parse: raw => raw === 'true', + serialize: value => (value ? 'true' : 'false'), + }); + + store.set(true); + // A choice made while the failing write is in flight is dropped with it. + store.set(false); + + writeGate.resolve(undefined); + await store.whenPersisted(); + + expect(toastError).toHaveBeenCalledTimes(1); + expect(store.get()).toBe(false); + expect(store.getRequested()).toBe(false); + }); + it('lets a set() before the initial load resolves win over the disk value', async () => { getItemAsync.mockResolvedValue('true'); const store = createSecureStorePreference({ diff --git a/apps/mobile/src/lib/hooks/secure-store-preference.ts b/apps/mobile/src/lib/hooks/secure-store-preference.ts index 5a50053c095..5e70feb785c 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.ts @@ -85,8 +85,20 @@ export function createSecureStorePreference(options: { * The newest value the app has asked for: the queued target, the target being * written, or the durable value. Relative edits (a list add, a switch flip) * start from this so two changes in one frame both survive. + * + * `undefined` is the "nothing pending" marker, compared explicitly: a store + * whose value is nullable (`string | null`, the voice-input language "auto") + * must report a requested `null` instead of falling through to the old value. */ - const currentRequested = (): T => requested ?? inFlight ?? value; + const currentRequested = (): T => { + if (requested !== undefined) { + return requested; + } + if (inFlight !== undefined) { + return inFlight; + } + return value; + }; const releaseDrainWaiters = () => { if (writing || requested !== undefined) { @@ -130,8 +142,12 @@ export function createSecureStorePreference(options: { await setAccountMetadata(key, serialize(target)); } catch { inFlight = undefined; + // Drop every pending choice, including one that arrived while this + // write was in flight: the disk just refused, so retrying the newest + // one here would only repeat the toast and could reorder choices. // The stored value stays the last one that landed, so the app keeps // showing it: the toast is the only claim the new choice failed. + requested = undefined; toast.error(i18n.t('common.couldNotSaveSetting')); if (!loadSettled && !cleared) { // Nothing of the user's is pending any more; let the in-flight diff --git a/apps/mobile/src/lib/hooks/use-trusted-hosts.ts b/apps/mobile/src/lib/hooks/use-trusted-hosts.ts index 6597f629877..a5aacfd99f1 100644 --- a/apps/mobile/src/lib/hooks/use-trusted-hosts.ts +++ b/apps/mobile/src/lib/hooks/use-trusted-hosts.ts @@ -71,7 +71,7 @@ export async function setTrustedHosts(next: string[]): Promise { } export function revokeHost(host: string): void { - store.set(store.get().filter(item => item !== host)); + store.set(store.getRequested().filter(item => item !== host)); } export function clearTrustedHosts(): void { From be825a9f3c4ba30cb92855141635145d71da7f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:04:47 +0000 Subject: [PATCH 4/7] test(mobile): name the nullable-store case for what it covers The nullable test asserts the requested null; the failure release is covered by the following case, so the name no longer claims both. --- apps/mobile/src/lib/hooks/secure-store-preference.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts index aa411f79e7e..2b5c6d0b01b 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts @@ -172,7 +172,7 @@ describe('createSecureStorePreference', () => { expect(store.get()).toBe(false); }); - it('reports a requested null for a nullable store and releases waiters after a failure', async () => { + it('reports a requested null for a nullable store', async () => { getItemAsync.mockResolvedValue(null); const writeGate = Promise.withResolvers(); setItemAsync.mockImplementationOnce(async () => { From 97de7b93588c0eff9f3ec673622f4ca8f4fbacdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:12:59 +0000 Subject: [PATCH 5/7] test(mobile): provide the back handler the settings sheet subscribes to --- .../components/todos/todo-list-model-picker.mounted.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/mobile/src/components/todos/todo-list-model-picker.mounted.test.tsx b/apps/mobile/src/components/todos/todo-list-model-picker.mounted.test.tsx index 646724bf44a..08317eb3411 100644 --- a/apps/mobile/src/components/todos/todo-list-model-picker.mounted.test.tsx +++ b/apps/mobile/src/components/todos/todo-list-model-picker.mounted.test.tsx @@ -35,6 +35,8 @@ vi.mock('expo-router', async () => { vi.mock('react-native', async () => { const { NativeTextInput } = await import('./todo-list-model-picker.mounted.test-helpers'); return { + // The settings sheet keeps its form open on the Android hardware back key. + BackHandler: { addEventListener: () => ({ remove: vi.fn() }) }, I18nManager: { isRTL: false }, Keyboard: { dismiss: vi.fn() }, Pressable: 'Pressable', From fa32b74fe3ca74b24af0c3374d12296da3815820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:01:39 +0000 Subject: [PATCH 6/7] test(mobile): read a remote MCP OAuth write before the disk takes it --- .../lib/chat/remote-mcp-oauth-store.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts index f633e72c80a..4221dce6c06 100644 --- a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts +++ b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts @@ -68,6 +68,27 @@ describe('writing a remote MCP server OAuth state', () => { }); }); + it('reads a write the disk has not taken yet', async () => { + const { store, epoch } = await fresh(); + heldDiskRead(null)(); + const landing = Promise.withResolvers(); + setItemAsync.mockReturnValue(landing.promise); + + await store.writeRemoteMcpOAuth( + { ...SERVER, epoch: epoch.currentAuthEpoch() }, + { tokens: TOKENS } + ); + + // The store holds the choice before the disk does, and the endpoint match + // applies to that choice too. + expect(await store.readRemoteMcpOAuth(SERVER.id, 'https://demo.example/mcp/')).toEqual({ + url: SERVER.url, + tokens: TOKENS, + }); + expect(await store.readRemoteMcpOAuth(SERVER.id, 'https://other.example/mcp')).toBeUndefined(); + landing.resolve(); + }); + it('drops a write that a sign-out overtook while it waited on the disk read', async () => { const { store, epoch } = await fresh(); const answer = heldDiskRead(null); From a986b02bc73818145f5febae5c232e5af052885c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:13:08 +0000 Subject: [PATCH 7/7] test(mobile): drop the void type argument the mobile lint refuses --- apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts index 4221dce6c06..0abffb63b4e 100644 --- a/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts +++ b/apps/mobile/src/lib/chat/remote-mcp-oauth-store.test.ts @@ -71,7 +71,7 @@ describe('writing a remote MCP server OAuth state', () => { it('reads a write the disk has not taken yet', async () => { const { store, epoch } = await fresh(); heldDiskRead(null)(); - const landing = Promise.withResolvers(); + const landing = Promise.withResolvers(); setItemAsync.mockReturnValue(landing.promise); await store.writeRemoteMcpOAuth( @@ -86,7 +86,7 @@ describe('writing a remote MCP server OAuth state', () => { tokens: TOKENS, }); expect(await store.readRemoteMcpOAuth(SERVER.id, 'https://other.example/mcp')).toBeUndefined(); - landing.resolve(); + landing.resolve(undefined); }); it('drops a write that a sign-out overtook while it waited on the disk read', async () => {