diff --git a/apps/mobile/src/lib/auth/credentials.test.ts b/apps/mobile/src/lib/auth/credentials.test.ts index 364c9f6711..94ce7612c7 100644 --- a/apps/mobile/src/lib/auth/credentials.test.ts +++ b/apps/mobile/src/lib/auth/credentials.test.ts @@ -27,8 +27,6 @@ vi.mock('expo-secure-store', () => ({ }), })); -vi.mock('@/lib/config', () => ({ API_BASE_URL: 'https://api.example.com' })); - // The sign-out deletes live in auth-context.tsx; mounting it pulls in the full // teardown graph, so stub every side-effecting collaborator. vi.mock('@sentry/react-native', () => ({ @@ -143,7 +141,7 @@ vi.mock('@/lib/temp-file-registry', () => ({ })); import * as SecureStore from 'expo-secure-store'; -import { persistSignInCredentialsAtEpoch } from '@/lib/auth/credentials'; +import { performRefresh, persistSignInCredentialsAtEpoch } from '@/lib/auth/credentials'; import { bumpAuthEpoch } from '@/lib/auth/auth-epoch'; import { clearActiveToken, setSignOutTeardownActive } from '@/lib/auth/token-owner'; import { @@ -214,6 +212,56 @@ describe('bearer credential writes', () => { }); }); +describe('refresh rotation', () => { + beforeEach(() => { + vi.clearAllMocks(); + store.clear(); + clearActiveToken(); + setSignOutTeardownActive(false); + }); + + it('retries a rejected refresh-token read and still rotates the token', async () => { + store.set(REFRESH_TOKEN_KEY, 'r1'); + // The keychain rejects the first read — the transient class on a device + // that just foregrounded — and resolves the stored value on the retry. + let reads = 0; + vi.mocked(SecureStore.getItemAsync).mockImplementation(async (key: string) => { + await Promise.resolve(); + reads += 1; + if (reads === 1) { + throw new Error('keychain temporarily unavailable'); + } + return store.get(key) ?? null; + }); + const fetchMock = vi + .fn() + .mockResolvedValue( + Response.json({ token: 't2', refreshToken: 'r2', expiresIn: 3600 }, { status: 200 }) + ); + vi.stubGlobal('fetch', fetchMock); + + try { + const outcome = await performRefresh(); + + expect(outcome.ok).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(SecureStore.setItemAsync).toHaveBeenCalledWith(AUTH_TOKEN_KEY, 't2', expectedOptions); + expect(SecureStore.setItemAsync).toHaveBeenCalledWith( + REFRESH_TOKEN_KEY, + 'r2', + expectedOptions + ); + expect(SecureStore.setItemAsync).toHaveBeenCalledWith( + TOKEN_EXPIRES_AT_KEY, + expect.any(String), + expectedOptions + ); + } finally { + vi.unstubAllGlobals(); + } + }); +}); + describe('sign-out deletes', () => { // mountAndSignOut resets modules and imports the whole auth graph; under the // full related-suite running concurrently with the device stack that exceeds diff --git a/apps/mobile/src/lib/auth/credentials.ts b/apps/mobile/src/lib/auth/credentials.ts index ef6af237b8..57faa2e10a 100644 --- a/apps/mobile/src/lib/auth/credentials.ts +++ b/apps/mobile/src/lib/auth/credentials.ts @@ -3,6 +3,7 @@ import * as SecureStore from 'expo-secure-store'; import { API_BASE_URL } from '@/lib/config'; import { currentAuthEpoch, isCurrentAuthEpoch } from '@/lib/auth/auth-epoch'; import { parseTokenPair } from '@/lib/auth/native-auth-contract'; +import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read'; import { isSignOutTeardownActive, setActiveToken } from '@/lib/auth/token-owner'; import { chainSave } from '@/lib/hooks/save-chain'; import { AUTH_TOKEN_KEY, REFRESH_TOKEN_KEY, TOKEN_EXPIRES_AT_KEY } from '@/lib/storage-keys'; @@ -146,7 +147,7 @@ async function doRefresh(): Promise { return { ok: false, refused: false, superseded: true }; } try { - const storedRefreshToken = await SecureStore.getItemAsync(REFRESH_TOKEN_KEY); + const storedRefreshToken = await readStoredValueWithRetry(REFRESH_TOKEN_KEY); if (superseded()) { return { ok: false, refused: false, superseded: true }; } diff --git a/apps/mobile/src/lib/auth/token-owner.test.ts b/apps/mobile/src/lib/auth/token-owner.test.ts index 314e215f63..e9acb18477 100644 --- a/apps/mobile/src/lib/auth/token-owner.test.ts +++ b/apps/mobile/src/lib/auth/token-owner.test.ts @@ -87,6 +87,23 @@ describe('token-owner', () => { expect(SecureStore.getItemAsync).toHaveBeenCalledTimes(1); }); + it('retries a rejected cold read and serves the stored token', async () => { + store.set(AUTH_TOKEN_KEY, 'stored-token'); + // The keychain rejects the first read — the transient class on a device + // that just foregrounded — and resolves the stored value on the retry. + let reads = 0; + vi.mocked(SecureStore.getItemAsync).mockImplementation(async (key: string) => { + await Promise.resolve(); + reads += 1; + if (reads === 1) { + throw new Error('keychain temporarily unavailable'); + } + return store.get(key) ?? null; + }); + + await expect(getAuthTokenForRequest()).resolves.toBe('stored-token'); + }); + it('warms the owner on a cold read so the next read is in-memory', async () => { store.set(AUTH_TOKEN_KEY, 'stored-token'); await getAuthTokenForRequest(); diff --git a/apps/mobile/src/lib/auth/token-owner.ts b/apps/mobile/src/lib/auth/token-owner.ts index 882e039b62..846b6f8b2b 100644 --- a/apps/mobile/src/lib/auth/token-owner.ts +++ b/apps/mobile/src/lib/auth/token-owner.ts @@ -1,6 +1,5 @@ -import * as SecureStore from 'expo-secure-store'; - import { currentAuthEpoch, isCurrentAuthEpoch } from '@/lib/auth/auth-epoch'; +import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read'; import { AUTH_TOKEN_KEY } from '@/lib/storage-keys'; export type ActiveToken = { @@ -97,7 +96,7 @@ export async function getAuthTokenForRequest(): Promise { if (isSignOutTeardownActive()) { return null; } - const token = await SecureStore.getItemAsync(AUTH_TOKEN_KEY); + const token = await readStoredValueWithRetry(AUTH_TOKEN_KEY); // A sign-in or refresh may have published a newer owner while the cold read // was in flight: prefer it and never overwrite it with the stale read. const published = getActiveToken(); 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 8332f9c110..903a61091a 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.test.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.test.ts @@ -15,6 +15,14 @@ vi.mock('@sentry/react-native', () => ({ captureException })); const { toastError } = vi.hoisted(() => ({ toastError: vi.fn() })); vi.mock('sonner-native', () => ({ toast: { error: toastError } })); +// The retrying read imports @/lib/config, whose real module needs the app's +// baked build `extra` and cannot load under Vitest. Vitest also rejects a mock +// factory that omits an export the importer reads, so this stub answers the +// whole surface: every value reads as unset, which keeps the retry helper's +// fault window closed and leaves the failure to the SecureStore mock. +const configStub = vi.hoisted(() => new Proxy({}, { get: () => undefined, has: () => true })); +vi.mock('@/lib/config', () => configStub); + // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule function flushMicrotasks(): Promise { return new Promise(resolve => { @@ -25,6 +33,10 @@ function flushMicrotasks(): Promise { // eslint-disable-next-line no-empty-function -- listener body is irrelevant, only subscribe()'s side effect (starting the load) is under test function noopListener(): void {} +// The retrying read backs off 250/500/1000 ms, so a test that drives a +// rejection must let those timers fire before the read settles. +const RETRY_BUDGET_MS = 250 + 500 + 1000 + 250; + describe('createSecureStorePreference', () => { beforeEach(() => { getItemAsync.mockReset(); @@ -35,6 +47,8 @@ describe('createSecureStorePreference', () => { }); it('logs to Sentry (not a toast) on a read failure and keeps the default value', async () => { + // Every attempt rejects, so the retry budget is exhausted before the + // failure is surfaced. getItemAsync.mockRejectedValue(new Error('disk error')); const store = createSecureStorePreference({ key: 'k', @@ -43,16 +57,46 @@ describe('createSecureStorePreference', () => { serialize: value => (value ? 'true' : 'false'), }); - const unsubscribe = store.subscribe(noopListener); - await flushMicrotasks(); + vi.useFakeTimers(); + try { + const unsubscribe = store.subscribe(noopListener); + await vi.advanceTimersByTimeAsync(RETRY_BUDGET_MS); + + expect(store.get()).toBe(false); + expect(store.getHasLoaded()).toBe(true); + expect(getItemAsync).toHaveBeenCalledTimes(4); + expect(captureException).toHaveBeenCalledWith(expect.any(Error), { + tags: { 'error.subsystem': 'preferences', 'error.operation': 'load_secure_store' }, + }); + expect(toastError).not.toHaveBeenCalled(); + unsubscribe(); + } finally { + vi.useRealTimers(); + } + }); - expect(store.get()).toBe(false); - expect(store.getHasLoaded()).toBe(true); - expect(captureException).toHaveBeenCalledWith(expect.any(Error), { - tags: { 'error.subsystem': 'preferences', 'error.operation': 'load_secure_store' }, + it('retries a rejected read and applies the value from the retry', async () => { + getItemAsync + .mockRejectedValueOnce(new Error('keychain unavailable')) + .mockResolvedValueOnce('true'); + const store = createSecureStorePreference({ + key: 'k', + defaultValue: false, + parse: raw => raw === 'true', + serialize: value => (value ? 'true' : 'false'), }); - expect(toastError).not.toHaveBeenCalled(); - unsubscribe(); + + vi.useFakeTimers(); + try { + const unsubscribe = store.subscribe(noopListener); + await vi.advanceTimersByTimeAsync(RETRY_BUDGET_MS); + + expect(store.get()).toBe(true); + expect(store.getHasLoaded()).toBe(true); + unsubscribe(); + } finally { + vi.useRealTimers(); + } }); it('shows a toast on a write failure while keeping the in-memory 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 245442eaa5..f2ebb6d1e1 100644 --- a/apps/mobile/src/lib/hooks/secure-store-preference.ts +++ b/apps/mobile/src/lib/hooks/secure-store-preference.ts @@ -1,9 +1,9 @@ import * as Sentry from '@sentry/react-native'; -import * as SecureStore from 'expo-secure-store'; import { toast } from 'sonner-native'; import { i18n } from '@/i18n'; import { deleteAccountMetadata, setAccountMetadata } from '@/lib/auth/account-metadata-write'; +import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read'; function noop(): void { // Placeholder until the promise executor hands over its resolve. @@ -54,7 +54,7 @@ export function createSecureStorePreference(options: { const load = async () => { try { - const raw = await SecureStore.getItemAsync(key); + const raw = await readStoredValueWithRetry(key); if (!dirty) { value = parse(raw); } else if (mergeOnLoad && !cleared) { diff --git a/apps/mobile/src/lib/persist/encrypted-kv.test.ts b/apps/mobile/src/lib/persist/encrypted-kv.test.ts index 953c310210..c8bded4200 100644 --- a/apps/mobile/src/lib/persist/encrypted-kv.test.ts +++ b/apps/mobile/src/lib/persist/encrypted-kv.test.ts @@ -185,6 +185,7 @@ vi.mock('@sentry/react-native', () => ({ })); /* eslint-disable import/first */ +import * as SecureStore from 'expo-secure-store'; import * as SQLite from 'expo-sqlite'; import { PERSIST_DB_KEY } from '@/lib/storage-keys'; import { @@ -363,6 +364,23 @@ describe('single-flight open contract', () => { }); }); +describe('transient keychain read', () => { + it('retries a rejected key read instead of memoizing the open failure', async () => { + // The keychain read can reject transiently (device just rebooted, keystore + // still locked). The open must retry it: a memoized rejection here would + // reject every KV caller for the rest of the process. + const storedKey = 'a'.repeat(64); + store.set(PERSIST_DB_KEY, storedKey); + vi.mocked(SecureStore.getItemAsync).mockRejectedValueOnce(new Error('keychain unavailable')); + + await expect(getItem('s', 'k')).resolves.toBeNull(); + + // The retry read the stored key and the store opened on it. + expect(SQLite.openDatabaseSync).toHaveBeenCalledTimes(1); + expect(sqlLog).toContain(`PRAGMA key = "x'${storedKey}'"`); + }); +}); + describe('connection configuration', () => { it('configures busy_timeout and WAL after the key and before the first schema read', async () => { await setItem('s', 'a', 'x'); diff --git a/apps/mobile/src/lib/persist/encrypted-kv.ts b/apps/mobile/src/lib/persist/encrypted-kv.ts index 1f3adb1224..43f55fef51 100644 --- a/apps/mobile/src/lib/persist/encrypted-kv.ts +++ b/apps/mobile/src/lib/persist/encrypted-kv.ts @@ -9,6 +9,7 @@ import * as Crypto from 'expo-crypto'; import * as SecureStore from 'expo-secure-store'; import * as SQLite from 'expo-sqlite'; import migrations from '../../../drizzle/migrations'; +import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read'; import { PERSIST_DB_KEY } from '@/lib/storage-keys'; import { kv } from './schema'; @@ -92,7 +93,10 @@ async function generateHexKey(): Promise { } async function readOrCreateKey(): Promise { - const existing = await SecureStore.getItemAsync(PERSIST_DB_KEY); + // A transient keychain rejection must not be read as "no key": retrying it + // keeps one rejection from failing the open, which is memoized for the rest + // of the process. + const existing = await readStoredValueWithRetry(PERSIST_DB_KEY); if (existing) { return existing; } diff --git a/apps/mobile/src/lib/persist/read-cache.test.ts b/apps/mobile/src/lib/persist/read-cache.test.ts index b385e71383..5ce838bfa9 100644 --- a/apps/mobile/src/lib/persist/read-cache.test.ts +++ b/apps/mobile/src/lib/persist/read-cache.test.ts @@ -558,6 +558,24 @@ describe('cold-start restore and takeover', () => { expect(takeOverColdStartRestore()).toBeNull(); }); + it('retries a rejected identity-hint read and still hydrates the cache', async () => { + const { scopes } = createFakeKv(); + store.set(ACTIVE_USER_ID_KEY, 'u1'); + scopes.set( + 'cache:u1:1', + new Map([['read-cache', JSON.stringify(makePersistedClient({ id: 'u1' }))]]) + ); + // The keychain read can reject transiently; the restore must retry it + // rather than abandoning the whole cold-start restore. + vi.mocked(SecureStore.getItemAsync).mockRejectedValueOnce(new Error('keychain unavailable')); + const queryClient = new QueryClient(); + + await restorePersistedCacheOnColdStart(queryClient); + + expect(queryClient.getQueryData(GET_ME_QUERY_KEY)).toEqual({ id: 'u1' }); + expect(takeOverColdStartRestore()).toBe('cache:u1:1'); + }); + it('drops an expired blob instead of hydrating it', async () => { const { kv, scopes } = createFakeKv(); store.set(ACTIVE_USER_ID_KEY, 'u1'); diff --git a/apps/mobile/src/lib/persist/read-cache.ts b/apps/mobile/src/lib/persist/read-cache.ts index 03b6358c6a..ae11de9522 100644 --- a/apps/mobile/src/lib/persist/read-cache.ts +++ b/apps/mobile/src/lib/persist/read-cache.ts @@ -1,4 +1,3 @@ -import * as SecureStore from 'expo-secure-store'; import { type Query, type QueryClient } from '@tanstack/react-query'; import { createAsyncStoragePersister } from '@tanstack/query-async-storage-persister'; import { @@ -10,6 +9,7 @@ import { z } from 'zod'; import { buildAgentSessionListInput } from '@/lib/agent-session-input'; import { currentAuthEpoch, isCurrentAuthEpoch } from '@/lib/auth/auth-epoch'; +import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read'; import { isSignOutActive, setSignOutActive } from '@/lib/auth/sign-out-state'; import * as encryptedKv from '@/lib/persist/encrypted-kv'; import { ACTIVE_USER_ID_KEY } from '@/lib/storage-keys'; @@ -271,13 +271,17 @@ export async function restorePersistedCacheOnColdStart(queryClient: QueryClient) // this restore immediately after scheduling it. coldStartGeneration += 1; const generation = coldStartGeneration; - // Capture the epoch before the first SecureStore read: a sign-in or sign-out + // Capture the epoch before the first stored-value read: a sign-in or sign-out // that lands while the hint read or the KV read is in flight fences the // whole restore, so it can never hydrate (or claim a scope) after the auth // epoch moved — including after a logout that cleared the query client. const epoch = currentAuthEpoch(); try { - const hintUserId = await SecureStore.getItemAsync(ACTIVE_USER_ID_KEY); + // The identity hint read is retried on a transient keychain rejection: a + // rejected read must not abandon the restore, or the profile's cached + // identity is never hydrated and the first foreground refetch has nothing + // to render. + const hintUserId = await readStoredValueWithRetry(ACTIVE_USER_ID_KEY); if (generation !== coldStartGeneration || !hintUserId || !isCurrentAuthEpoch(epoch)) { return; } diff --git a/apps/mobile/src/lib/trpc.test.ts b/apps/mobile/src/lib/trpc.test.ts index 9f821a4cc7..88c11145b5 100644 --- a/apps/mobile/src/lib/trpc.test.ts +++ b/apps/mobile/src/lib/trpc.test.ts @@ -67,12 +67,6 @@ vi.mock('expo-secure-store', () => ({ WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'WHEN_UNLOCKED_THIS_DEVICE_ONLY', })); -vi.mock('@/lib/config', () => ({ - API_BASE_URL: 'https://api.example.com', - E2E_LATENCY_MESSAGES_MS: 0, - E2E_LATENCY_SESSION_MS: 0, -})); - vi.mock('@/lib/storage-keys', () => ({ AUTH_TOKEN_KEY: 'auth-token', TOKEN_EXPIRES_AT_KEY: 'token-expires-at', diff --git a/apps/mobile/vitest.mounted.config.ts b/apps/mobile/vitest.mounted.config.ts index 46b660c661..351d4629fa 100644 --- a/apps/mobile/vitest.mounted.config.ts +++ b/apps/mobile/vitest.mounted.config.ts @@ -18,6 +18,9 @@ export default defineProject({ test: { name: 'mobile-mounted', environment: 'node', + // The app build's config module cannot load in this project; the setup + // file stubs the exports its importers read. + setupFiles: ['./vitest.setup.ts'], include: ['src/**/*.mounted.test.tsx'], // Project configs do not inherit the root test options, and this suite // runs both projects in parallel: on a loaded host (dev stack, simulator, diff --git a/apps/mobile/vitest.pure.config.ts b/apps/mobile/vitest.pure.config.ts index e1456e3fb4..c5c7676302 100644 --- a/apps/mobile/vitest.pure.config.ts +++ b/apps/mobile/vitest.pure.config.ts @@ -17,6 +17,9 @@ export default defineProject({ test: { name: 'mobile-pure', environment: 'node', + // The app build's config module cannot load in this project; the setup + // file stubs the exports its importers read. + setupFiles: ['./vitest.setup.ts'], // Project configs do not inherit the root test options, and this suite // runs both projects in parallel: on a loaded host (dev stack, simulator, // Appium) workers starve and real-timer tests exceed the 5s default. One diff --git a/apps/mobile/vitest.setup.ts b/apps/mobile/vitest.setup.ts new file mode 100644 index 0000000000..2afe3c785a --- /dev/null +++ b/apps/mobile/vitest.setup.ts @@ -0,0 +1,11 @@ +import { vi } from 'vitest'; + +// The mobile vitest projects run with no app build, so `@/lib/config` cannot +// load: its real module needs the baked `extra`. Tests that exercise modules +// importing it (the auth retry helpers and everything built on them) would +// otherwise fail before their first assertion. +// +// A Proxy answers every export Vitest resolves, and an unset value keeps the +// build-gated E2E windows closed. A test that needs a config value still mocks +// this module itself; its later registration wins over this one. +vi.mock('@/lib/config', () => new Proxy({}, { get: () => undefined, has: () => true }));