From fdaf6677e28920b3d8ce9d44224e22c2ca24051a Mon Sep 17 00:00:00 2001 From: toruiwasa <11441145+toruiwasa@users.noreply.github.com> Date: Thu, 3 Sep 2026 09:34:43 +1000 Subject: [PATCH 1/2] =?UTF-8?q?feat(mobile):=20core=20infrastructure=20?= =?UTF-8?q?=E2=80=94=20config,=20logger,=20supabase,=20query=20client,=20a?= =?UTF-8?q?uth=20store?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 9 (#12). Every screen from Task 10 onward depends on this layer; none of it renders anything itself. - src/lib/config.ts — reads the EXPO_PUBLIC_* values from the manifest and throws at import naming any that are missing. Turns issue #95's silent `undefined` endpoint into a startup error. - app.config.js — new: Task 7 shipped a static app.json, so nothing lifted those variables into `extra`. Spreads app.json rather than replacing it. - src/lib/logger.ts — MobileLogger, the only permitted console caller; no-console is now an error everywhere else. Data argument is Record so a Session or Error cannot be passed whole. - src/lib/supabase.ts — expo-secure-store adapter, detectSessionInUrl: false. - src/lib/queryClient.ts — MMKV-persisted query cache, AppState focus and NetInfo online listeners registered once at import. - src/store/authStore.ts, src/constants/{colors,thresholds}.ts Deviations from REQ-17, each recorded in the plan with its reason: react-native-mmkv v4 renamed the v3 API the spec used; the sync persister the spec named is deprecated in favour of the async one; APP_ENV is EXPO_PUBLIC_APP_ENV and an unrecognised value falls back to production, not development. Coverage: 100% on all seven new files. Closes #12 --- apps/mobile/.env.example | 9 +- apps/mobile/__tests__/authStore.test.ts | 35 +++++ apps/mobile/__tests__/config.test.ts | 73 ++++++++++ apps/mobile/__tests__/constants.test.ts | 34 +++++ apps/mobile/__tests__/logger.test.ts | 125 ++++++++++++++++ apps/mobile/__tests__/queryClient.test.ts | 168 ++++++++++++++++++++++ apps/mobile/__tests__/supabase.test.ts | 69 +++++++++ apps/mobile/app.config.js | 37 +++++ apps/mobile/app.json | 2 +- apps/mobile/eslint.config.js | 15 ++ apps/mobile/jest.config.js | 2 +- apps/mobile/package.json | 11 +- apps/mobile/src/constants/colors.ts | 48 +++++++ apps/mobile/src/constants/thresholds.ts | 9 ++ apps/mobile/src/lib/config.ts | 50 +++++++ apps/mobile/src/lib/logger.ts | 46 ++++++ apps/mobile/src/lib/queryClient.ts | 79 ++++++++++ apps/mobile/src/lib/supabase.ts | 28 ++++ apps/mobile/src/store/authStore.ts | 25 ++++ plans/REQ-17_Mobile_App_MVP.md | 59 +++++++- pnpm-lock.yaml | 142 +++++++++++++++++- 21 files changed, 1058 insertions(+), 8 deletions(-) create mode 100644 apps/mobile/__tests__/authStore.test.ts create mode 100644 apps/mobile/__tests__/config.test.ts create mode 100644 apps/mobile/__tests__/constants.test.ts create mode 100644 apps/mobile/__tests__/logger.test.ts create mode 100644 apps/mobile/__tests__/queryClient.test.ts create mode 100644 apps/mobile/__tests__/supabase.test.ts create mode 100644 apps/mobile/app.config.js create mode 100644 apps/mobile/src/constants/colors.ts create mode 100644 apps/mobile/src/constants/thresholds.ts create mode 100644 apps/mobile/src/lib/config.ts create mode 100644 apps/mobile/src/lib/logger.ts create mode 100644 apps/mobile/src/lib/queryClient.ts create mode 100644 apps/mobile/src/lib/supabase.ts create mode 100644 apps/mobile/src/store/authStore.ts diff --git a/apps/mobile/.env.example b/apps/mobile/.env.example index 973657c..bf6cf8f 100644 --- a/apps/mobile/.env.example +++ b/apps/mobile/.env.example @@ -8,6 +8,11 @@ # key, which must never leave the server). # # --------------------------------------------------------------------------- +# How these reach the app: app.config.js copies them into `expo.extra`, and +# src/lib/config.ts reads them from there and THROWS at import if any of the +# three below is missing — naming it. A build without them fails on launch with +# that message rather than reporting `undefined` endpoints as network errors. +# # This file covers LOCAL builds only. `.env` is gitignored and EAS does not # upload it, so a cloud build (`eas build --profile ...`) sees only the `env` # block in eas.json — today that is EXPO_PUBLIC_APP_ENV and nothing else. @@ -19,7 +24,9 @@ # which also decides whether the values live in eas.json or on expo.dev. # --------------------------------------------------------------------------- -# development | staging | production — drives log level, mirrors APP_ENV on web/api. +# development | staging | production — drives log level, mirrors APP_ENV on +# web/api. Anything else (including unset) is treated as production, i.e. warn +# and error only. EXPO_PUBLIC_APP_ENV=development EXPO_PUBLIC_SUPABASE_URL=https://your-project.supabase.co diff --git a/apps/mobile/__tests__/authStore.test.ts b/apps/mobile/__tests__/authStore.test.ts new file mode 100644 index 0000000..7aaa76a --- /dev/null +++ b/apps/mobile/__tests__/authStore.test.ts @@ -0,0 +1,35 @@ +import type { Session } from '@supabase/supabase-js'; + +import { useAuthStore } from '../src/store/authStore'; + +const session = { access_token: 'token', user: { id: 'user-1' } } as unknown as Session; + +beforeEach(() => { + useAuthStore.setState({ session: null }); +}); + +describe('useAuthStore', () => { + it('starts signed out', () => { + expect(useAuthStore.getState().session).toBeNull(); + }); + + it('stores the session passed to setSession', () => { + useAuthStore.getState().setSession(session); + + expect(useAuthStore.getState().session).toBe(session); + }); + + it('accepts null through setSession — onAuthStateChange emits it on SIGNED_OUT', () => { + useAuthStore.getState().setSession(session); + useAuthStore.getState().setSession(null); + + expect(useAuthStore.getState().session).toBeNull(); + }); + + it('resets to null on clearSession', () => { + useAuthStore.getState().setSession(session); + useAuthStore.getState().clearSession(); + + expect(useAuthStore.getState().session).toBeNull(); + }); +}); diff --git a/apps/mobile/__tests__/config.test.ts b/apps/mobile/__tests__/config.test.ts new file mode 100644 index 0000000..911af45 --- /dev/null +++ b/apps/mobile/__tests__/config.test.ts @@ -0,0 +1,73 @@ +/** + * Boundary: the build-time configuration gate. Everything downstream assumes + * Config's three URL/key fields are non-empty strings, so this is the only + * place that can catch a build shipped without them (issue #95). + */ +type Extra = Record | undefined; + +function loadConfig(extra: Extra) { + let loaded: typeof import('../src/lib/config') | undefined; + jest.isolateModules(() => { + jest.doMock('expo-constants', () => ({ + __esModule: true, + default: { expoConfig: extra === undefined ? null : { extra } }, + })); + // eslint-disable-next-line @typescript-eslint/no-require-imports + loaded = require('../src/lib/config'); + }); + return loaded!; +} + +const COMPLETE = { + supabaseUrl: 'https://project.supabase.co', + supabasePublishableKey: 'sb_publishable_test', + apiUrl: 'https://api.example.com', + appEnv: 'development', +}; + +afterEach(() => { + jest.resetModules(); + jest.dontMock('expo-constants'); +}); + +describe('Config', () => { + it('exposes every value when the manifest is complete', () => { + const { Config } = loadConfig(COMPLETE); + + expect(Config.supabaseUrl).toBe('https://project.supabase.co'); + expect(Config.supabasePublishableKey).toBe('sb_publishable_test'); + expect(Config.apiUrl).toBe('https://api.example.com'); + expect(Config.appEnv).toBe('development'); + }); + + it('throws naming every missing variable when the manifest has no extra', () => { + expect(() => loadConfig(undefined)).toThrow( + /Missing required env vars: supabaseUrl, supabasePublishableKey, apiUrl/ + ); + }); + + it('names only the variable that is missing', () => { + expect(() => loadConfig({ ...COMPLETE, apiUrl: undefined })).toThrow( + /Missing required env vars: apiUrl/ + ); + }); + + it('treats an empty string as missing — an unset EAS variable arrives that way', () => { + expect(() => loadConfig({ ...COMPLETE, supabaseUrl: '' })).toThrow( + /Missing required env vars: supabaseUrl/ + ); + }); + + it.each(['development', 'staging', 'production'])('passes through appEnv %s', (appEnv) => { + expect(loadConfig({ ...COMPLETE, appEnv }).Config.appEnv).toBe(appEnv); + }); + + it.each([ + ['absent', undefined], + ['unrecognised', 'prod'], + ])('falls back to production when appEnv is %s', (_label, appEnv) => { + // CLAUDE.md > Logging Strategy: an undefined environment logs at the + // quietest level rather than the most verbose. + expect(loadConfig({ ...COMPLETE, appEnv }).Config.appEnv).toBe('production'); + }); +}); diff --git a/apps/mobile/__tests__/constants.test.ts b/apps/mobile/__tests__/constants.test.ts new file mode 100644 index 0000000..160f285 --- /dev/null +++ b/apps/mobile/__tests__/constants.test.ts @@ -0,0 +1,34 @@ +/** + * The threshold numbers are the whole of REQ-17's watchlist states 5 and 6 — + * they are the spec, not an implementation detail — so they are asserted + * against the requirement rather than against themselves. + */ +import { colors } from '../src/constants/colors'; +import { STALE_DISCONNECTED_MS, STALE_WARNING_MS } from '../src/constants/thresholds'; + +describe('staleness thresholds', () => { + it('warns at 60 seconds', () => { + expect(STALE_WARNING_MS).toBe(60 * 1000); + }); + + it('reports disconnected at 5 minutes', () => { + expect(STALE_DISCONNECTED_MS).toBe(5 * 60 * 1000); + }); + + it('orders the warning before the disconnected state', () => { + expect(STALE_WARNING_MS).toBeLessThan(STALE_DISCONNECTED_MS); + }); +}); + +describe('colors', () => { + it('exposes every token as a 6-digit hex value', () => { + Object.entries(colors).forEach(([token, value]) => { + expect(`${token}: ${value}`).toMatch(/: #[0-9A-F]{6}$/); + }); + }); + + it('keeps the two banner families distinct', () => { + expect(colors.warningBackground).not.toBe(colors.dangerBackground); + expect(colors.warningText).not.toBe(colors.dangerText); + }); +}); diff --git a/apps/mobile/__tests__/logger.test.ts b/apps/mobile/__tests__/logger.test.ts new file mode 100644 index 0000000..b0070a3 --- /dev/null +++ b/apps/mobile/__tests__/logger.test.ts @@ -0,0 +1,125 @@ +/** + * Boundary: the only permitted console caller. Two guarantees are tested — + * the level gate per APP_ENV, and that sanitize() actually runs on the data + * argument (not merely that the call looked right). + */ +type AppEnv = 'development' | 'staging' | 'production'; + +function loadLogger(appEnv: AppEnv) { + let loaded: typeof import('../src/lib/logger') | undefined; + jest.isolateModules(() => { + jest.doMock('../src/lib/config', () => ({ + Config: { + supabaseUrl: 'https://project.supabase.co', + supabasePublishableKey: 'sb_publishable_test', + apiUrl: 'https://api.example.com', + appEnv, + }, + })); + // eslint-disable-next-line @typescript-eslint/no-require-imports + loaded = require('../src/lib/logger'); + }); + return loaded!.MobileLogger; +} + +const spies = { + debug: jest.spyOn(console, 'debug').mockImplementation(() => {}), + info: jest.spyOn(console, 'info').mockImplementation(() => {}), + warn: jest.spyOn(console, 'warn').mockImplementation(() => {}), + error: jest.spyOn(console, 'error').mockImplementation(() => {}), +}; + +beforeEach(() => { + Object.values(spies).forEach((spy) => spy.mockClear()); +}); + +afterEach(() => { + jest.resetModules(); + jest.dontMock('../src/lib/config'); +}); + +afterAll(() => { + Object.values(spies).forEach((spy) => spy.mockRestore()); +}); + +describe('MobileLogger level gate', () => { + it('emits every level in development', () => { + const logger = loadLogger('development'); + + logger.debug('AUTH', 'a'); + logger.info('AUTH', 'b'); + logger.warn('AUTH', 'c'); + logger.error('AUTH', 'd'); + + expect(spies.debug).toHaveBeenCalledTimes(1); + expect(spies.info).toHaveBeenCalledTimes(1); + expect(spies.warn).toHaveBeenCalledTimes(1); + expect(spies.error).toHaveBeenCalledTimes(1); + }); + + it('drops debug but keeps info in staging', () => { + const logger = loadLogger('staging'); + + logger.debug('AUTH', 'a'); + logger.info('AUTH', 'b'); + + expect(spies.debug).not.toHaveBeenCalled(); + expect(spies.info).toHaveBeenCalledTimes(1); + }); + + it('drops debug and info in production, keeps warn and error', () => { + const logger = loadLogger('production'); + + logger.debug('AUTH', 'a'); + logger.info('AUTH', 'b'); + logger.warn('AUTH', 'c'); + logger.error('AUTH', 'd'); + + expect(spies.debug).not.toHaveBeenCalled(); + expect(spies.info).not.toHaveBeenCalled(); + expect(spies.warn).toHaveBeenCalledTimes(1); + expect(spies.error).toHaveBeenCalledTimes(1); + }); +}); + +describe('MobileLogger data handling', () => { + it('redacts tokens and PII through sanitize()', () => { + const logger = loadLogger('development'); + + logger.debug('AUTH', 'signed in', { + access_token: 'never-log-me', + refresh_token: 'nor-this', + email: 'user@example.com', + hasSession: true, + }); + + expect(spies.debug).toHaveBeenCalledWith('[AUTH]', 'signed in', { + access_token: '[REDACTED]', + refresh_token: '[REDACTED]', + email: '[REDACTED]', + hasSession: true, + }); + }); + + it('redacts at warn and error too — the levels production still emits', () => { + const logger = loadLogger('production'); + + logger.warn('AUTH', 'refresh failed', { access_token: 'never-log-me' }); + logger.error('AUTH', 'fatal', { refresh_token: 'never-log-me' }); + + expect(spies.warn).toHaveBeenCalledWith('[AUTH]', 'refresh failed', { + access_token: '[REDACTED]', + }); + expect(spies.error).toHaveBeenCalledWith('[AUTH]', 'fatal', { + refresh_token: '[REDACTED]', + }); + }); + + it('passes no data argument at all when none was given', () => { + const logger = loadLogger('development'); + + logger.info('NAV', 'navigating to /dashboard'); + + expect(spies.info).toHaveBeenCalledWith('[NAV]', 'navigating to /dashboard'); + }); +}); diff --git a/apps/mobile/__tests__/queryClient.test.ts b/apps/mobile/__tests__/queryClient.test.ts new file mode 100644 index 0000000..355e4b8 --- /dev/null +++ b/apps/mobile/__tests__/queryClient.test.ts @@ -0,0 +1,168 @@ +/** + * Boundary: the query client's persisted-cache wiring. + * + * Three things here fail silently rather than loudly — a gcTime below the + * persister's maxAge (nothing is ever written to MMKV), a mis-mapped MMKV + * method (v4 renamed `delete` to `remove`), and a focus/online listener that + * was never registered. + */ +import type { AppStateStatus } from 'react-native'; + +type MmkvStore = Map; + +const mockStore: MmkvStore = new Map(); +const mockMmkvInstance = { + getString: jest.fn((key: string) => mockStore.get(key)), + set: jest.fn((key: string, value: string) => { + mockStore.set(key, String(value)); + }), + remove: jest.fn((key: string) => mockStore.delete(key)), +}; +const mockCreateMMKV = jest.fn(() => mockMmkvInstance); +type NetInfoListener = (state: { isConnected: boolean | null }) => void; + +const mockNetInfoAddEventListener = jest.fn((_listener: NetInfoListener) => () => {}); + +jest.mock('react-native-mmkv', () => ({ createMMKV: mockCreateMMKV })); +jest.mock('@react-native-community/netinfo', () => ({ + __esModule: true, + default: { addEventListener: mockNetInfoAddEventListener }, +})); +jest.mock('../src/lib/config', () => ({ + Config: { + supabaseUrl: 'https://project.supabase.co', + supabasePublishableKey: 'sb_publishable_test', + apiUrl: 'https://api.example.com', + appEnv: 'development', + }, +})); + +/* eslint-disable @typescript-eslint/no-require-imports */ +const { AppState } = require('react-native') as typeof import('react-native'); +const mockAppStateRemove = jest.fn(); +const appStateSpy = jest + .spyOn(AppState, 'addEventListener') + .mockReturnValue({ remove: mockAppStateRemove } as never); + +const { queryClient, persistOptions, mmkvPersister } = + require('../src/lib/queryClient') as typeof import('../src/lib/queryClient'); +const { focusManager, onlineManager } = + require('@tanstack/react-query') as typeof import('@tanstack/react-query'); +/* eslint-enable @typescript-eslint/no-require-imports */ + +// Captured at import: these listeners are registered exactly once, when the +// module is evaluated, so the call records must be read before any test clears +// a mock. +const createMmkvCall = mockCreateMMKV.mock.calls[0]; +const appStateCall = appStateSpy.mock.calls[0]; +const netInfoCall = mockNetInfoAddEventListener.mock.calls[0]; + +const EMPTY_CLIENT = { + timestamp: Date.now(), + buster: 'v1', + clientState: { mutations: [], queries: [] }, +}; + +describe('query defaults', () => { + const defaults = queryClient.getDefaultOptions().queries; + + it('sets staleTime 1s below the 15s price refetchInterval', () => { + expect(defaults?.staleTime).toBe(14_000); + }); + + it('keeps gcTime at least as long as the persister maxAge', () => { + expect(defaults?.gcTime).toBeGreaterThanOrEqual(persistOptions.maxAge); + expect(persistOptions.maxAge).toBe(1000 * 60 * 60 * 24); + }); + + it('retries twice and does not poll in the background', () => { + expect(defaults?.retry).toBe(2); + expect(defaults?.refetchIntervalInBackground).toBe(false); + }); + + it('sets no refetchInterval — each hook owns its own', () => { + expect(defaults?.refetchInterval).toBeUndefined(); + }); + + it('carries a cache buster so an old cached shape can be invalidated', () => { + expect(persistOptions.buster).toBe('v1'); + expect(persistOptions.persister).toBe(mmkvPersister); + }); +}); + +describe('MMKV persister', () => { + beforeEach(() => { + mockStore.clear(); + // Only the instance methods — clearing every mock would erase the + // import-time registrations the last describe block asserts on. + mockMmkvInstance.getString.mockClear(); + mockMmkvInstance.set.mockClear(); + mockMmkvInstance.remove.mockClear(); + }); + + it('opens its own MMKV instance rather than the default one', () => { + expect(createMmkvCall).toEqual([{ id: 'query-cache' }]); + }); + + it('round-trips a client through set() and getString()', async () => { + await mmkvPersister.persistClient(EMPTY_CLIENT); + + expect(mockMmkvInstance.set).toHaveBeenCalledTimes(1); + await expect(mmkvPersister.restoreClient()).resolves.toEqual(EMPTY_CLIENT); + }); + + it('restores undefined when MMKV holds nothing', async () => { + await expect(mmkvPersister.restoreClient()).resolves.toBeUndefined(); + expect(mockMmkvInstance.getString).toHaveBeenCalled(); + }); + + it('removes through remove() — v4 dropped the delete() the spec was written against', async () => { + await mmkvPersister.persistClient(EMPTY_CLIENT); + await mmkvPersister.removeClient(); + + expect(mockMmkvInstance.remove).toHaveBeenCalledTimes(1); + await expect(mmkvPersister.restoreClient()).resolves.toBeUndefined(); + }); +}); + +describe('focus and online listeners', () => { + it('registers an AppState listener at import', () => { + expect(appStateCall?.[0]).toBe('change'); + expect(typeof appStateCall?.[1]).toBe('function'); + }); + + it('treats only the active app state as focused', () => { + const handler = appStateCall?.[1] as (state: AppStateStatus) => void; + + handler('background'); + expect(focusManager.isFocused()).toBe(false); + + handler('active'); + expect(focusManager.isFocused()).toBe(true); + }); + + it('removes the AppState subscription when the focus listener is replaced', () => { + // React Query calls the cleanup a setEventListener callback returns when a + // new listener replaces it. A cleanup that does not unsubscribe leaks an + // AppState listener on every replacement. + focusManager.setEventListener(() => () => {}); + + expect(mockAppStateRemove).toHaveBeenCalledTimes(1); + }); + + it('registers a NetInfo listener and mirrors isConnected', () => { + expect(typeof netInfoCall?.[0]).toBe('function'); + const handler = netInfoCall![0]; + + handler({ isConnected: false }); + expect(onlineManager.isOnline()).toBe(false); + + handler({ isConnected: true }); + expect(onlineManager.isOnline()).toBe(true); + + // null is what NetInfo reports before the first probe resolves — it must + // not be read as "online". + handler({ isConnected: null }); + expect(onlineManager.isOnline()).toBe(false); + }); +}); diff --git a/apps/mobile/__tests__/supabase.test.ts b/apps/mobile/__tests__/supabase.test.ts new file mode 100644 index 0000000..c213a60 --- /dev/null +++ b/apps/mobile/__tests__/supabase.test.ts @@ -0,0 +1,69 @@ +/** + * Boundary: the Supabase client's construction options. detectSessionInUrl and + * the secure-store adapter are both silent-failure settings — a wrong value + * signs the user out on the next cold launch, or writes refresh tokens to + * unencrypted storage, with nothing failing at build time. + */ +type ClientOptions = { auth: Record }; + +const mockCreateClient = jest.fn( + (_url: string, _key: string, _options: ClientOptions) => ({ auth: {} }) +); +const mockGetItemAsync = jest.fn(async (_key: string) => 'stored-value'); +const mockSetItemAsync = jest.fn(async (_key: string, _value: string) => undefined); +const mockDeleteItemAsync = jest.fn(async (_key: string) => undefined); + +jest.mock('@supabase/supabase-js', () => ({ createClient: mockCreateClient })); +jest.mock('expo-secure-store', () => ({ + getItemAsync: mockGetItemAsync, + setItemAsync: mockSetItemAsync, + deleteItemAsync: mockDeleteItemAsync, +})); +jest.mock('../src/lib/config', () => ({ + Config: { + supabaseUrl: 'https://project.supabase.co', + supabasePublishableKey: 'sb_publishable_test', + apiUrl: 'https://api.example.com', + appEnv: 'development', + }, +})); + +// eslint-disable-next-line @typescript-eslint/no-require-imports +require('../src/lib/supabase'); + +type Storage = { + getItem: (key: string) => Promise; + setItem: (key: string, value: string) => Promise; + removeItem: (key: string) => Promise; +}; + +const [url, key, options] = mockCreateClient.mock.calls[0]!; + +describe('supabase client', () => { + it('is constructed from Config — the mock intercepted the real module', () => { + expect(mockCreateClient).toHaveBeenCalledTimes(1); + expect(url).toBe('https://project.supabase.co'); + expect(key).toBe('sb_publishable_test'); + }); + + it('disables detectSessionInUrl — true drops the stored session on mobile', () => { + expect(options.auth.detectSessionInUrl).toBe(false); + }); + + it('keeps the session refreshing and persisted', () => { + expect(options.auth.autoRefreshToken).toBe(true); + expect(options.auth.persistSession).toBe(true); + }); + + it('stores the session through expo-secure-store, not AsyncStorage or MMKV', async () => { + const storage = options.auth.storage as Storage; + + await expect(storage.getItem('sb-session')).resolves.toBe('stored-value'); + await storage.setItem('sb-session', 'token-payload'); + await storage.removeItem('sb-session'); + + expect(mockGetItemAsync).toHaveBeenCalledWith('sb-session'); + expect(mockSetItemAsync).toHaveBeenCalledWith('sb-session', 'token-payload'); + expect(mockDeleteItemAsync).toHaveBeenCalledWith('sb-session'); + }); +}); diff --git a/apps/mobile/app.config.js b/apps/mobile/app.config.js new file mode 100644 index 0000000..412463b --- /dev/null +++ b/apps/mobile/app.config.js @@ -0,0 +1,37 @@ +// Dynamic Expo config. +// +// Task 7 shipped the static app.json, which cannot read process.env. This file +// is evaluated by @expo/config at build time and receives app.json's contents +// as `config` (see @expo/config > Config.js: "If a function is exported from +// the app.config.js then a partial config will be passed as an argument"). +// Spreading it is what keeps app.json load-bearing rather than dead — @expo/config +// warns about an unused static config when a dynamic one ignores it. +// +// Its only job is to lift the EXPO_PUBLIC_* variables into `extra`, where +// src/lib/config.ts reads them through Constants.expoConfig. +// +// Why `extra` and not `process.env.EXPO_PUBLIC_*` directly in config.ts: +// babel-preset-expo's inline-env-vars plugin only runs when Metro passes +// `inlineEnvironmentVariables` (production bundles), so a direct read behaves +// differently in a production bundle than under jest-expo. Reading the manifest +// is the same code path everywhere and mocks cleanly in tests. +// +// REQ-17 specified `APP_ENV` here. eas.json and .env.example — both merged in +// Task 7 (#10) and referenced by issue #95 — settled on EXPO_PUBLIC_APP_ENV. +// That name wins; a build profile sets exactly one variable for this. +// +// REQ-17 also defaulted it to 'development' here. It is passed through +// undefined instead: CLAUDE.md > Logging Strategy fixes the undefined case at +// the *quietest* level, and defaulting to 'development' at this layer would +// mean a build that simply forgot the variable ships with debug logging on. +// config.ts owns that fallback. +module.exports = ({ config }) => ({ + ...config, + extra: { + ...config.extra, + supabaseUrl: process.env.EXPO_PUBLIC_SUPABASE_URL, + supabasePublishableKey: process.env.EXPO_PUBLIC_SUPABASE_PUBLISHABLE_KEY, + apiUrl: process.env.EXPO_PUBLIC_API_URL, + appEnv: process.env.EXPO_PUBLIC_APP_ENV, + }, +}); diff --git a/apps/mobile/app.json b/apps/mobile/app.json index 73fef86..870cbe6 100644 --- a/apps/mobile/app.json +++ b/apps/mobile/app.json @@ -4,7 +4,7 @@ "slug": "pulseticker", "version": "0.0.1", "orientation": "portrait", - "userInterfaceStyle": "automatic", + "userInterfaceStyle": "light", "scheme": "pulseticker", "ios": { "supportsTablet": false, diff --git a/apps/mobile/eslint.config.js b/apps/mobile/eslint.config.js index 95911d4..58e64b1 100644 --- a/apps/mobile/eslint.config.js +++ b/apps/mobile/eslint.config.js @@ -5,4 +5,19 @@ module.exports = [ { ignores: ['dist/*', '.expo/*', 'coverage/*'], }, + { + // A React Native log stream is readable by anyone holding the device, so + // every console call goes through MobileLogger, which sanitises its data + // argument and gates level by APP_ENV (CLAUDE.md > Logging Strategy). + rules: { + 'no-console': 'error', + }, + }, + { + // MobileLogger is the one permitted console caller. + files: ['src/lib/logger.ts'], + rules: { + 'no-console': 'off', + }, + }, ]; diff --git a/apps/mobile/jest.config.js b/apps/mobile/jest.config.js index bb31320..e2638b6 100644 --- a/apps/mobile/jest.config.js +++ b/apps/mobile/jest.config.js @@ -25,7 +25,7 @@ module.exports = { '^(\\.{1,2}/.*)\\.js$': '$1', }, - collectCoverageFrom: ['app/**/*.{ts,tsx}', '!**/*.d.ts'], + collectCoverageFrom: ['app/**/*.{ts,tsx}', 'src/**/*.{ts,tsx}', '!**/*.d.ts'], // No coverageThreshold yet — deliberately, and this is not an oversight to // fill in with a copied block. diff --git a/apps/mobile/package.json b/apps/mobile/package.json index c0868b7..66fe85e 100644 --- a/apps/mobile/package.json +++ b/apps/mobile/package.json @@ -16,15 +16,24 @@ "dependencies": { "@pulseticker/logging": "workspace:*", "@pulseticker/schemas": "workspace:*", + "@react-native-community/netinfo": "12.0.1", + "@supabase/supabase-js": "^2.112.3", + "@tanstack/query-async-storage-persister": "^5.102.6", + "@tanstack/react-query": "^5.102.6", + "@tanstack/react-query-persist-client": "^5.102.6", "expo": "57.0.15", "expo-constants": "~57.0.13", "expo-linking": "~57.0.7", "expo-router": "~57.0.15", + "expo-secure-store": "57.0.2", "expo-status-bar": "~57.0.1", "react": "19.2.3", "react-native": "0.86.2", + "react-native-mmkv": "^4.3.2", + "react-native-nitro-modules": "0.37.0", "react-native-safe-area-context": "~5.7.0", - "react-native-screens": "~4.26.0" + "react-native-screens": "~4.26.0", + "zustand": "^5.0.15" }, "devDependencies": { "@testing-library/react-native": "^14.0.1", diff --git a/apps/mobile/src/constants/colors.ts b/apps/mobile/src/constants/colors.ts new file mode 100644 index 0000000..30aef9b --- /dev/null +++ b/apps/mobile/src/constants/colors.ts @@ -0,0 +1,48 @@ +/** + * Mobile design tokens. + * + * Derived from the web palette in apps/web/src/styles.css (--pt-*) so the two + * clients read as one product. REQ-17 specifies the mobile UX in terms of + * behaviour ("amber banner", "red banner") and names no hex values, so the + * shared tokens are the source and only the two banner families are new. + * + * Light only, deliberately: REQ-17 Phase 1 specifies no dark palette, and a + * second unspecified palette would be invented, not designed. app.json's + * `userInterfaceStyle` is pinned to "light" to match — see the PR for #12. + * Dark mode is tracked separately. + */ +export const colors = { + // Brand / interactive — Google sign-in button, tab bar active tint + primary: '#2953B2', + primaryPressed: '#1E3F8A', + onPrimary: '#FFFFFF', + + // Surfaces + background: '#F9FAFB', + surface: '#FFFFFF', + elevated: '#F3F4F6', + border: '#E5E7EB', + + // Text + textPrimary: '#111827', + textSecondary: '#6B7280', + textMuted: '#9CA3AF', + + // Price movement — same pair the web watchlist uses + priceUp: '#34D399', + priceDown: '#F87171', + + // Stale warning banner (age 60s–5min) — REQ-17 watchlist state 5 + warningBackground: '#FEF3C7', + warningBorder: '#F59E0B', + warningText: '#92400E', + + // Disconnected / offline banner (age > 5min, or NetInfo offline) — + // REQ-17 watchlist states 6 and 7 + dangerBackground: '#FEE2E2', + dangerBorder: '#DC2626', + dangerText: '#991B1B', + + // Skeleton rows — REQ-17 watchlist state 1 + skeleton: '#E5E7EB', +} as const; diff --git a/apps/mobile/src/constants/thresholds.ts b/apps/mobile/src/constants/thresholds.ts new file mode 100644 index 0000000..23cfb7b --- /dev/null +++ b/apps/mobile/src/constants/thresholds.ts @@ -0,0 +1,9 @@ +/** + * Price staleness thresholds (REQ-17 watchlist states 5 and 6). + * + * Measured against the `fetchedAt` the query function records, not + * `dataUpdatedAt`: after an MMKV rehydration the cache carries the age it had + * when it was written, which is exactly what the banners must reflect. + */ +export const STALE_WARNING_MS = 60_000; // 60s → amber "Updated N min ago" +export const STALE_DISCONNECTED_MS = 300_000; // 5min → red "Prices may be outdated." + Retry diff --git a/apps/mobile/src/lib/config.ts b/apps/mobile/src/lib/config.ts new file mode 100644 index 0000000..81a9177 --- /dev/null +++ b/apps/mobile/src/lib/config.ts @@ -0,0 +1,50 @@ +import Constants from 'expo-constants'; + +/** + * Build-time configuration, validated at import. + * + * EXPO_PUBLIC_* values are inlined into the binary when it is built, so a + * missing one is not a transient outage that a retry recovers from — it is + * baked in. Failing here, naming the variable, turns issue #95's silent + * `undefined` endpoint into a startup error instead of a network error + * reported against Supabase or the API. + * + * Import this before anything that reads configuration (supabase.ts, + * queryClient.ts) so the failure precedes the first request. + */ +export type AppEnv = 'development' | 'staging' | 'production'; + +const extra = Constants.expoConfig?.extra ?? {}; + +const raw = { + supabaseUrl: extra.supabaseUrl as string | undefined, + supabasePublishableKey: extra.supabasePublishableKey as string | undefined, + apiUrl: extra.apiUrl as string | undefined, +}; + +const missing = Object.entries(raw) + .filter(([, value]) => !value) + .map(([key]) => key); + +if (missing.length > 0) { + throw new Error( + `[pulseticker] Missing required env vars: ${missing.join(', ')}. ` + + 'Set them in apps/mobile/.env for a local build, or in the EAS build profile for a cloud build.' + ); +} + +// appEnv is deliberately outside the required set. An absent or unrecognised +// value must not abort the app, and must not open logging up either: CLAUDE.md +// > Logging Strategy fixes the undefined case at the quietest level, so +// anything unrecognised is treated as production. +const appEnv: AppEnv = + extra.appEnv === 'production' || extra.appEnv === 'staging' || extra.appEnv === 'development' + ? extra.appEnv + : 'production'; + +export const Config = { + supabaseUrl: raw.supabaseUrl as string, + supabasePublishableKey: raw.supabasePublishableKey as string, + apiUrl: raw.apiUrl as string, + appEnv, +} as const; diff --git a/apps/mobile/src/lib/logger.ts b/apps/mobile/src/lib/logger.ts new file mode 100644 index 0000000..6b2da44 --- /dev/null +++ b/apps/mobile/src/lib/logger.ts @@ -0,0 +1,46 @@ +import { sanitize } from '@pulseticker/logging'; + +import { Config } from './config'; + +/** + * The only permitted console caller in apps/mobile. + * + * `data` is `Record` on purpose (CLAUDE.md > Logging + * Strategy): a `Session`, `User` or `Error` cannot be passed whole, so the + * compiler rejects the mistake at the call site instead of sanitize() having to + * catch it at runtime. sanitize() stays as the second layer, not the first. + * + * A browser console is readable by anyone with the device; the same applies to + * a React Native log stream on a development build. Safe to log: state flags + * ({ hasSession: true }), event names, `error.name`, navigation targets. Never: + * tokens, email/phone/name, raw Error objects, the OAuth `?code=` parameter. + */ +type LogData = Record; + +const isProduction = Config.appEnv === 'production'; + +function format(data?: LogData): [] | [Record] { + return data ? [sanitize(data)] : []; +} + +export const MobileLogger = { + /** Development-only flow tracing. Silent in staging and production. */ + debug(tag: string, message: string, data?: LogData): void { + if (Config.appEnv === 'development') console.debug(`[${tag}]`, message, ...format(data)); + }, + + /** Normal business events (sign-in completed, cache hydrated). */ + info(tag: string, message: string, data?: LogData): void { + if (!isProduction) console.info(`[${tag}]`, message, ...format(data)); + }, + + /** Abnormal but recoverable — auth failure, API error with a fallback. */ + warn(tag: string, message: string, data?: LogData): void { + console.warn(`[${tag}]`, message, ...format(data)); + }, + + /** Exceptions and fatal errors. Always recorded, at every environment. */ + error(tag: string, message: string, data?: LogData): void { + console.error(`[${tag}]`, message, ...format(data)); + }, +}; diff --git a/apps/mobile/src/lib/queryClient.ts b/apps/mobile/src/lib/queryClient.ts new file mode 100644 index 0000000..906998c --- /dev/null +++ b/apps/mobile/src/lib/queryClient.ts @@ -0,0 +1,79 @@ +import NetInfo from '@react-native-community/netinfo'; +import { createAsyncStoragePersister } from '@tanstack/query-async-storage-persister'; +import { focusManager, onlineManager, QueryClient } from '@tanstack/react-query'; +import { AppState } from 'react-native'; +import { createMMKV } from 'react-native-mmkv'; + +import './config'; // validates the build-time env vars before any request is made + +const CACHE_MAX_AGE_MS = 1000 * 60 * 60 * 24; // 24h + +const mmkv = createMMKV({ id: 'query-cache' }); + +// react-native-mmkv v4 renamed the v3 instance API REQ-17 was written against: +// `new MMKV()` → `createMMKV()`, and `delete(key)` → `remove(key)`. +const mmkvStorage = { + getItem: (key: string) => mmkv.getString(key) ?? null, + setItem: (key: string, value: string) => mmkv.set(key, value), + removeItem: (key: string) => { + mmkv.remove(key); + }, +}; + +// REQ-17 named createSyncStoragePersister. That export carries an +// `@deprecated` tag in @tanstack/query-sync-storage-persister@5.102 pointing +// here, so the supported package is used instead. MMKV's synchronous methods +// satisfy the AsyncStorage interface unchanged — its fields are MaybePromise — +// and PersistQueryClientProvider restores asynchronously either way, so the +// hydration behaviour REQ-17 describes is unaffected. +export const mmkvPersister = createAsyncStoragePersister({ + storage: mmkvStorage, + throttleTime: 1000, +}); + +export const persistOptions = { + persister: mmkvPersister, + maxAge: CACHE_MAX_AGE_MS, + // Bump when a cached response's schema changes. A restored cache is not + // re-validated against the Zod schemas, so an old shape would reach the + // screens as if it were fresh. + buster: 'v1', +}; + +export const queryClient = new QueryClient({ + defaultOptions: { + queries: { + // 1s below useWatchlistPrices' 15s refetchInterval, so returning to the + // foreground does not fire a second request alongside the scheduled one. + staleTime: 14_000, + // Must be >= the persister's maxAge: a query dropped from the cache is + // never written to MMKV, and the persisted cache is what makes the + // skeleton state a first-launch-only state. + gcTime: CACHE_MAX_AGE_MS, + retry: 2, + refetchIntervalInBackground: false, + }, + }, +}); + +// refetchInterval is deliberately absent from the defaults above — each hook +// sets its own. useWatchlistPrices must pass 15_000; omitting it stops price +// polling with no error and no stale banner until the age thresholds catch up. + +/** + * Registered at import, once, rather than inside a component: React Query keeps + * one focus and one online listener process-wide, and a component that mounts + * twice would install them twice. + */ +focusManager.setEventListener((handleFocus) => { + const subscription = AppState.addEventListener('change', (state) => { + handleFocus(state === 'active'); + }); + return () => subscription.remove(); +}); + +onlineManager.setEventListener((setOnline) => + NetInfo.addEventListener((state) => { + setOnline(!!state.isConnected); + }) +); diff --git a/apps/mobile/src/lib/supabase.ts b/apps/mobile/src/lib/supabase.ts new file mode 100644 index 0000000..aed16b0 --- /dev/null +++ b/apps/mobile/src/lib/supabase.ts @@ -0,0 +1,28 @@ +import { createClient } from '@supabase/supabase-js'; +import * as SecureStore from 'expo-secure-store'; + +import { Config } from './config'; + +/** + * Session storage backed by the platform secure enclave (iOS Keychain / + * Android Keystore). Never AsyncStorage and never MMKV: the query cache is + * unencrypted, and refresh tokens must not be readable from a device backup. + */ +const secureStoreAdapter = { + getItem: (key: string) => SecureStore.getItemAsync(key), + setItem: (key: string, value: string) => SecureStore.setItemAsync(key, value), + removeItem: (key: string) => SecureStore.deleteItemAsync(key), +}; + +export const supabase = createClient(Config.supabaseUrl, Config.supabasePublishableKey, { + auth: { + storage: secureStoreAdapter, + // CRITICAL on mobile. The web client parses the session out of + // window.location; there is no URL to parse here, and leaving this true + // makes supabase-js discard the stored session on start — the app then + // signs the user out on every cold launch, silently. + detectSessionInUrl: false, + autoRefreshToken: true, + persistSession: true, + }, +}); diff --git a/apps/mobile/src/store/authStore.ts b/apps/mobile/src/store/authStore.ts new file mode 100644 index 0000000..fd50113 --- /dev/null +++ b/apps/mobile/src/store/authStore.ts @@ -0,0 +1,25 @@ +import type { Session } from '@supabase/supabase-js'; +import { create } from 'zustand'; + +/** + * In-memory session state. Supabase owns persistence (expo-secure-store), so + * this store is populated from `supabase.auth.onAuthStateChange` in the root + * layout and never writes to storage itself. + * + * No `isLoading` or `error` field: both belong to the screen performing the + * operation, and a global one would be shared by unrelated callers. + * + * The session carries `access_token`. Pass it around, never log it — not even + * a field of it. + */ +interface AuthState { + session: Session | null; + setSession: (session: Session | null) => void; + clearSession: () => void; +} + +export const useAuthStore = create((set) => ({ + session: null, + setSession: (session) => set({ session }), + clearSession: () => set({ session: null }), +})); diff --git a/plans/REQ-17_Mobile_App_MVP.md b/plans/REQ-17_Mobile_App_MVP.md index 43da23f..7162913 100644 --- a/plans/REQ-17_Mobile_App_MVP.md +++ b/plans/REQ-17_Mobile_App_MVP.md @@ -241,6 +241,29 @@ Task 11 acceptance criterion ("force-close + reopen shows watchlist") must be ve ## TanStack Query config + MMKV persister (Task 9) +> **Correction — 2026-09-03 (Task 9 / #12, PR for `feat/mobile-core-infra`).** +> The snippet below is kept as written; three things in it no longer match the +> libraries it names, and the implementation follows this note where they differ. +> +> 1. **`react-native-mmkv` v4 replaced the v3 instance API.** `new MMKV({ id })` +> is now `createMMKV({ id })`, and `delete(key)` is now `remove(key)`. The v3 +> calls do not exist on the v4 instance, so the adapter below would throw on +> the first cache eviction rather than at construction. +> 2. **`createSyncStoragePersister` carries an `@deprecated` tag** in +> `@tanstack/query-sync-storage-persister@5.102`, pointing at +> `createAsyncStoragePersister` in `@tanstack/query-async-storage-persister`. +> The async persister is used instead. MMKV's synchronous methods satisfy its +> `AsyncStorage` interface unchanged (every field is `MaybePromise`), and +> `PersistQueryClientProvider` restores asynchronously with either persister, +> so the hydration behaviour described below is unaffected. +> 3. **Issue #12's body says `gcTime: 300_000`.** That predates this section and +> is wrong for a persisted cache: a query evicted after 5 minutes is never +> written to MMKV, which would make the skeleton state appear on every launch +> rather than only the first. `gcTime` is 24h, matching the persister's +> `maxAge`, as written below. Issue #12 also names `src/lib/query-client.ts` +> and `src/store/auth.ts`; the folder structure in this document is +> authoritative (`queryClient.ts`, `authStore.ts`). + ```typescript // src/lib/queryClient.ts import { MMKV } from 'react-native-mmkv'; @@ -363,7 +386,23 @@ apps/mobile/ --- -## app.config.js spec (Task 7) +## app.config.js spec (Task 7 — delivered in Task 9) + +> **Correction — 2026-09-03 (Task 9 / #12, PR for `feat/mobile-core-infra`).** +> Task 7 shipped a static `app.json` and no `app.config.js`, so nothing lifted +> the `EXPO_PUBLIC_*` variables into `extra` and the file below did not exist. +> Task 9 adds it, because `src/lib/config.ts` is the first code to read those +> values. It is written as `({ config }) => ({ ...config, extra: {...} })`: +> @expo/config passes the static `app.json` in as `config` and warns about an +> unused static config if a dynamic one ignores it, so app.json stays the home +> of everything that is not environment-derived. +> +> `userInterfaceStyle` is also changed from `"automatic"` to `"light"` here. +> `src/constants/colors.ts` ships a light palette only — REQ-17 Phase 1 +> specifies no dark palette, and a second one would be invented rather than +> designed — so "automatic" would pair a dark system chrome with light screens. +> Dark mode is tracked separately. + ```javascript export default { @@ -476,6 +515,24 @@ Export both from `packages/schemas/src/index.ts` alongside the existing `CreateA ## Env var validation (Task 9) — `src/lib/config.ts` +> **Correction — 2026-09-03 (Task 9 / #12, PR for `feat/mobile-core-infra`).** +> Two details of the snippet below changed on implementation. +> +> - **`appEnv` is not defaulted to `'development'`.** CLAUDE.md > Logging +> Strategy fixes the undefined environment at the *quietest* level, so a build +> that simply forgot the variable must not ship with debug logging on. An +> absent or unrecognised value is treated as `production`; `app.config.js` +> passes it through undefined rather than defaulting it. +> - **The variable is `EXPO_PUBLIC_APP_ENV`, not `APP_ENV`.** `eas.json` and +> `.env.example`, both merged in Task 7 (#10) and referenced by issue #95, +> already settled on that name for all three build profiles. +> +> `Constants.expoConfig.extra` is kept as the read path rather than reading +> `process.env.EXPO_PUBLIC_*` directly: babel-preset-expo's `inline-env-vars` +> plugin only runs when Metro passes `inlineEnvironmentVariables` (production +> bundles), so a direct read behaves differently in a production bundle than +> under jest-expo. The manifest is the same code path everywhere. + `EXPO_PUBLIC_*` values are inlined at build time, so a missing one is not a runtime outage that retries — it is baked into the binary. Fail loudly at import, before any consumer reads a `undefined` URL and reports it as a network error. `config.ts` must be diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 514986b..d81a6f0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -86,7 +86,7 @@ importers: devDependencies: '@eslint/eslintrc': specifier: ^3.3.6 - version: 3.3.6(supports-color@5.5.0) + version: 3.3.6 '@eslint/js': specifier: ^10.0.1 version: 10.0.1(eslint@10.8.1(supports-color@5.5.0)) @@ -174,6 +174,21 @@ importers: '@pulseticker/schemas': specifier: workspace:* version: link:../../packages/schemas + '@react-native-community/netinfo': + specifier: 12.0.1 + version: 12.0.1(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) + '@supabase/supabase-js': + specifier: ^2.112.3 + version: 2.112.3 + '@tanstack/query-async-storage-persister': + specifier: ^5.102.6 + version: 5.102.6 + '@tanstack/react-query': + specifier: ^5.102.6 + version: 5.102.6(react@19.2.3) + '@tanstack/react-query-persist-client': + specifier: ^5.102.6 + version: 5.102.6(@tanstack/react-query@5.102.6(react@19.2.3))(react@19.2.3) expo: specifier: 57.0.15 version: 57.0.15(@babel/core@8.0.1)(@expo/dom-webview@57.0.1)(@expo/metro-runtime@57.0.12)(expo-router@57.0.15)(react-native-worklets@0.12.1(@babel/core@8.0.1)(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3)(supports-color@5.5.0)(typescript@6.0.3) @@ -186,6 +201,9 @@ importers: expo-router: specifier: ~57.0.15 version: 57.0.15(48887d3eb22312fa95a709df1a6b5485) + expo-secure-store: + specifier: 57.0.2 + version: 57.0.2(expo@57.0.15) expo-status-bar: specifier: ~57.0.1 version: 57.0.1(expo@57.0.15)(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) @@ -195,12 +213,21 @@ importers: react-native: specifier: 0.86.2 version: 0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0) + react-native-mmkv: + specifier: ^4.3.2 + version: 4.3.2(react-native-nitro-modules@0.37.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) + react-native-nitro-modules: + specifier: 0.37.0 + version: 0.37.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) react-native-safe-area-context: specifier: ~5.7.0 version: 5.7.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) react-native-screens: specifier: ~4.26.0 version: 4.26.2(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) + zustand: + specifier: ^5.0.15 + version: 5.0.15(@types/react@19.2.18)(react@19.2.3) devDependencies: '@testing-library/react-native': specifier: ^14.0.1 @@ -2994,6 +3021,12 @@ packages: '@types/react': optional: true + '@react-native-community/netinfo@12.0.1': + resolution: {integrity: sha512-P/3caXIvfYSJG8AWJVefukg+ZGRPs+M4Lp3pNJtgcTYoJxCjWrKQGNnCkj/Cz//zWa/avGed0i/wzm0T8vV2IQ==} + peerDependencies: + react: '*' + react-native: '>=0.59' + '@react-native-masked-view/masked-view@0.3.2': resolution: {integrity: sha512-XwuQoW7/GEgWRMovOQtX3A4PrXhyaZm0lVUiY8qJDvdngjLms9Cpdck6SmGAUNqQwcj2EadHC1HwL0bEyoa/SQ==} peerDependencies: @@ -3580,6 +3613,26 @@ packages: peerDependencies: '@taiga-ui/design-tokens': ~0.315.0 + '@tanstack/query-async-storage-persister@5.102.6': + resolution: {integrity: sha512-8Lfd2N1AoWW2Iu4mfHwKTUSR/08YvDxEC07yJZmDohwo8f3mDmaIee2+hSX20DKVZtjn8eFBSOAbLm0d5snEPw==} + + '@tanstack/query-core@5.102.6': + resolution: {integrity: sha512-jp+GucyQ+fel2ILb8ZLQeqMaAqZL/Bzaj+dKOQiPk4vPdf2rQPEV7heUyEVIhatpY42j4AFHav4DzBa1oTIp/A==} + + '@tanstack/query-persist-client-core@5.102.6': + resolution: {integrity: sha512-F9YKO76vFSBZfUVZxZjr/4xIUjnnYKO+pHfbhkrsIp1je5A1/L/UhIUb4A3VmLGnLJYxJWJmADSx2TAVZVRYsA==} + + '@tanstack/react-query-persist-client@5.102.6': + resolution: {integrity: sha512-l1prWH9kMq8cprv9qvrEPKxKHBvKG4XAM8lknoaBA1tqOx3H2BTPn1jIQNSDr0sfFbkFaYqDl/G1GxPUYxQtAQ==} + peerDependencies: + '@tanstack/react-query': ^5.102.6 + react: ^18 || ^19 + + '@tanstack/react-query@5.102.6': + resolution: {integrity: sha512-ANz4KZ8z80D85fiz5IAHjpb8XBPLrjOb/0natlD9Ascyy/3p96V86Zw8UbOfA0HDLcvK/A4gNd95r723y5UT2w==} + peerDependencies: + react: ^18 || ^19 + '@testing-library/react-native@14.0.1': resolution: {integrity: sha512-2r2e2y8SkUNRWKRXlUGqDYunB+AkbdXUPn49Bs5rpv9oxRb0K3USBVugbPbAKJjfj4w5Ba91NJ8LcQCZyopUZA==} engines: {node: ^22.13.0 || >=24} @@ -5493,6 +5546,11 @@ packages: react-server-dom-webpack: optional: true + expo-secure-store@57.0.2: + resolution: {integrity: sha512-PhrPMKnI7YSObLEQZOoP9evB2ZOCv9kFuG2L7cZfNlOwWjGbir711PlppkXVoM3JE8vamDrTqzMLGv266FhJJg==} + peerDependencies: + expo: '*' + expo-server@57.0.3: resolution: {integrity: sha512-aK+LdKzauHSGmsOStZtyxdzv0zWssCkxTw3m4QuOhfDSJsZaMRTd9O41d8ixU/QfELTbaJ0oRNcF7JFV/7O9YQ==} engines: {node: '>=20.16.0'} @@ -7627,6 +7685,19 @@ packages: react: '*' react-native: '*' + react-native-mmkv@4.3.2: + resolution: {integrity: sha512-49OAyfkg0/TMWiWELZN6VuVQPZPhizwL4DTmp8b7B1md3dB/s3LH3mGfC3T+lp9W0y/rqxZMEnotLFTIbOAenQ==} + peerDependencies: + react: '*' + react-native: '*' + react-native-nitro-modules: '*' + + react-native-nitro-modules@0.37.0: + resolution: {integrity: sha512-ULS2CBZcdwGw5d4Y35EV/tbCOsNsQOnQ9LC9CehMhDNm5c90lwF/LxRamzKuFpjI547G7teTQgD0peqYSGnP9g==} + peerDependencies: + react: '*' + react-native: '*' + react-native-reanimated@4.6.0: resolution: {integrity: sha512-9vbQmok5BPX2J3RLPFt2UnwmK2QQoMDPDi0VB4a9JWIiEc55pdzvwEsdH91V/JXKNErqM6a9fElTBMc/1cbtBA==} peerDependencies: @@ -9072,6 +9143,24 @@ packages: zod@4.4.3: resolution: {integrity: sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==} + zustand@5.0.15: + resolution: {integrity: sha512-MpSEjRiBkA9crSYeOUH32rJC7SVqAbm0Fqcqge/bUi2PPoLcBWKOsG+C8mevmpr8TwXHBVkChbbJiyvkE+i/3A==} + engines: {node: '>=12.20.0'} + peerDependencies: + '@types/react': '>=18.0.0' + immer: '>=9.0.6' + react: '>=18.0.0' + use-sync-external-store: '>=1.2.0' + peerDependenciesMeta: + '@types/react': + optional: true + immer: + optional: true + react: + optional: true + use-sync-external-store: + optional: true + snapshots: '@ampproject/remapping@2.3.0': @@ -10271,7 +10360,7 @@ snapshots: dependencies: '@types/json-schema': 7.0.15 - '@eslint/eslintrc@3.3.6(supports-color@5.5.0)': + '@eslint/eslintrc@3.3.6': dependencies: ajv: 6.15.0 debug: 4.4.3(supports-color@5.5.0) @@ -12061,6 +12150,11 @@ snapshots: optionalDependencies: '@types/react': 19.2.18 + '@react-native-community/netinfo@12.0.1(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3)': + dependencies: + react: 19.2.3 + react-native: 0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0) + '@react-native-masked-view/masked-view@0.3.2(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3)': dependencies: react: 19.2.3 @@ -12577,6 +12671,28 @@ snapshots: dependencies: '@taiga-ui/design-tokens': 0.303.0 + '@tanstack/query-async-storage-persister@5.102.6': + dependencies: + '@tanstack/query-core': 5.102.6 + '@tanstack/query-persist-client-core': 5.102.6 + + '@tanstack/query-core@5.102.6': {} + + '@tanstack/query-persist-client-core@5.102.6': + dependencies: + '@tanstack/query-core': 5.102.6 + + '@tanstack/react-query-persist-client@5.102.6(@tanstack/react-query@5.102.6(react@19.2.3))(react@19.2.3)': + dependencies: + '@tanstack/query-persist-client-core': 5.102.6 + '@tanstack/react-query': 5.102.6(react@19.2.3) + react: 19.2.3 + + '@tanstack/react-query@5.102.6(react@19.2.3)': + dependencies: + '@tanstack/query-core': 5.102.6 + react: 19.2.3 + '@testing-library/react-native@14.0.1(jest@29.7.0(@types/node@26.2.0)(ts-node@10.9.2(@types/node@26.2.0)(typescript@6.0.3)))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3)(test-renderer@1.2.0(@types/react@19.2.18)(react@19.2.3))': dependencies: jest-matcher-utils: 30.4.1 @@ -14711,7 +14827,7 @@ snapshots: '@eslint/config-array': 0.21.2(supports-color@5.5.0) '@eslint/config-helpers': 0.4.2 '@eslint/core': 0.17.0 - '@eslint/eslintrc': 3.3.6(supports-color@5.5.0) + '@eslint/eslintrc': 3.3.6 '@eslint/js': 9.39.5 '@eslint/plugin-kit': 0.4.1 '@humanfs/node': 0.16.8 @@ -14949,6 +15065,10 @@ snapshots: - react-native-worklets - supports-color + expo-secure-store@57.0.2(expo@57.0.15): + dependencies: + expo: 57.0.15(@babel/core@8.0.1)(@expo/dom-webview@57.0.1)(@expo/metro-runtime@57.0.12)(expo-router@57.0.15)(react-native-worklets@0.12.1(@babel/core@8.0.1)(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3)(supports-color@5.5.0)(typescript@6.0.3) + expo-server@57.0.3: {} expo-status-bar@57.0.1(expo@57.0.15)(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3): @@ -17842,6 +17962,17 @@ snapshots: react: 19.2.3 react-native: 0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0) + react-native-mmkv@4.3.2(react-native-nitro-modules@0.37.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3): + dependencies: + react: 19.2.3 + react-native: 0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0) + react-native-nitro-modules: 0.37.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3) + + react-native-nitro-modules@0.37.0(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3): + dependencies: + react: 19.2.3 + react-native: 0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0) + react-native-reanimated@4.6.0(react-native-worklets@0.12.1(@babel/core@8.0.1)(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3))(react-native@0.86.2(@babel/core@8.0.1)(@react-native/jest-preset@0.86.2(@babel/core@8.0.1)(react@19.2.3))(@react-native/metro-config@0.87.0(@babel/core@8.0.1))(@types/react@19.2.18)(react@19.2.3)(supports-color@5.5.0))(react@19.2.3): dependencies: react: 19.2.3 @@ -19427,3 +19558,8 @@ snapshots: zod@3.25.76: {} zod@4.4.3: {} + + zustand@5.0.15(@types/react@19.2.18)(react@19.2.3): + optionalDependencies: + '@types/react': 19.2.18 + react: 19.2.3 From b1b8c256b85518cecdea87f04fe7c43362240982 Mon Sep 17 00:00:00 2001 From: toruiwasa <11441145+toruiwasa@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:07:41 +1000 Subject: [PATCH 2/2] =?UTF-8?q?fix(mobile):=20address=20PR=20#110=20review?= =?UTF-8?q?=20=E2=80=94=20deep=20redaction,=20*WithCause,=20RN=20refresh?= =?UTF-8?q?=20wiring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sanitize() redacted only top-level keys, so the guarantee was depth-dependent: { access_token } was redacted while { context: { access_token } } logged the token verbatim at warn, a level production still emits. Task 9 is what made this load-bearing — it routes every mobile log through one gateway whose doc comment sells sanitize() as the second defensive layer — so the fix lands in packages/logging and applies to apps/web and apps/api too. It now recurses through plain objects and arrays and guards cycles with a WeakSet. Class instances are still passed through untouched; that stays the LogData type's job, and the limitation is recorded rather than left implied. MobileLogger gains warnWithCause/errorWithCause, which CLAUDE.md > Logging Strategy §6 requires for Supabase-auth and JWT errors. Without them Task 10's auth paths had no compliant way to log a cause at all, since LogData deliberately rejects a raw Error. Its level gate now compares through the LEVELS table @pulseticker/logging already exports, as apps/web does, instead of a boolean per method. supabase.ts wires startAutoRefresh/stopAutoRefresh to AppState. auth-js drives the refresh from a 30s setInterval and React Native suspends JS timers while backgrounded, so autoRefreshToken alone left a resumed app issuing its next request with an expired JWT. The secure-store adapter now logs and rethrows instead of passing rejections through silently, where a refused write meant an unexplained sign-out on the next cold launch. authStore makes clearSession the single teardown path — setSession(null) delegates to it, so sign-out cleanup added later cannot be skipped by the onAuthStateChange caller. The Config fixture duplicated across four test suites moves to one helper, and the userInterfaceStyle note in colors.ts is corrected: the pin applies on iOS only, because expo-system-ui is not installed (#113). Filed rather than fixed here: #112 (single-source the web/mobile palette), #113 (expo-system-ui, needs a device build), #114 (packages/logging has no test project, so sanitize() has no measured coverage). pnpm build 6/6, pnpm test 6/6, mobile 59 tests, 100% on all seven src files. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NfHB8Q3zSQ5idotURDwkyv --- apps/mobile/__tests__/authStore.test.ts | 21 ++- apps/mobile/__tests__/config.test.ts | 11 +- apps/mobile/__tests__/helpers/mockConfig.ts | 19 +++ apps/mobile/__tests__/logger.test.ts | 119 ++++++++++++++-- apps/mobile/__tests__/queryClient.test.ts | 11 +- apps/mobile/__tests__/supabase.test.ts | 144 +++++++++++++++++--- apps/mobile/jest.config.js | 6 + apps/mobile/src/constants/colors.ts | 6 +- apps/mobile/src/lib/logger.ts | 64 +++++++-- apps/mobile/src/lib/supabase.ts | 41 +++++- apps/mobile/src/store/authStore.ts | 16 ++- packages/logging/src/index.ts | 42 +++++- plans/REQ-17_Mobile_App_MVP.md | 80 +++++++++++ 13 files changed, 518 insertions(+), 62 deletions(-) create mode 100644 apps/mobile/__tests__/helpers/mockConfig.ts diff --git a/apps/mobile/__tests__/authStore.test.ts b/apps/mobile/__tests__/authStore.test.ts index 7aaa76a..a36084e 100644 --- a/apps/mobile/__tests__/authStore.test.ts +++ b/apps/mobile/__tests__/authStore.test.ts @@ -4,8 +4,12 @@ import { useAuthStore } from '../src/store/authStore'; const session = { access_token: 'token', user: { id: 'user-1' } } as unknown as Session; +const originalClearSession = useAuthStore.getState().clearSession; + beforeEach(() => { - useAuthStore.setState({ session: null }); + // clearSession is restored too: one test replaces it to observe the + // delegation, and zustand's setState merges rather than resets. + useAuthStore.setState({ session: null, clearSession: originalClearSession }); }); describe('useAuthStore', () => { @@ -32,4 +36,19 @@ describe('useAuthStore', () => { expect(useAuthStore.getState().session).toBeNull(); }); + + it('routes setSession(null) through clearSession, so teardown cannot be skipped', () => { + // Both null paths are live — onAuthStateChange uses setSession(null) and + // screens call clearSession — so cleanup added to clearSession later has + // to run for either caller. + const clearSession = jest.fn(() => { + useAuthStore.setState({ session: null }); + }); + useAuthStore.setState({ session, clearSession }); + + useAuthStore.getState().setSession(null); + + expect(clearSession).toHaveBeenCalledTimes(1); + expect(useAuthStore.getState().session).toBeNull(); + }); }); diff --git a/apps/mobile/__tests__/config.test.ts b/apps/mobile/__tests__/config.test.ts index 911af45..664fe21 100644 --- a/apps/mobile/__tests__/config.test.ts +++ b/apps/mobile/__tests__/config.test.ts @@ -3,6 +3,8 @@ * Config's three URL/key fields are non-empty strings, so this is the only * place that can catch a build shipped without them (issue #95). */ +import { mockConfig } from './helpers/mockConfig'; + type Extra = Record | undefined; function loadConfig(extra: Extra) { @@ -18,12 +20,9 @@ function loadConfig(extra: Extra) { return loaded!; } -const COMPLETE = { - supabaseUrl: 'https://project.supabase.co', - supabasePublishableKey: 'sb_publishable_test', - apiUrl: 'https://api.example.com', - appEnv: 'development', -}; +// The same fixture the consumers mock Config with — here it stands in for the +// manifest `extra` those values are read out of, so the two cannot drift. +const COMPLETE: Record = mockConfig(); afterEach(() => { jest.resetModules(); diff --git a/apps/mobile/__tests__/helpers/mockConfig.ts b/apps/mobile/__tests__/helpers/mockConfig.ts new file mode 100644 index 0000000..2a2d1b4 --- /dev/null +++ b/apps/mobile/__tests__/helpers/mockConfig.ts @@ -0,0 +1,19 @@ +import type { AppEnv } from '../../src/lib/config'; + +/** + * The Config shape as every consumer's test mocks it. + * + * Shared because a jest.mock factory is never checked against the real + * module: when Config gains a field, a stale copy hands the module under test + * `undefined` silently instead of failing the build. One fixture means one + * place to add it. + * + * The AppEnv import is type-only, so it is erased at compile time and does not + * execute config.ts's import-time throw. + */ +export const mockConfig = (appEnv: AppEnv = 'development') => ({ + supabaseUrl: 'https://project.supabase.co', + supabasePublishableKey: 'sb_publishable_test', + apiUrl: 'https://api.example.com', + appEnv, +}); diff --git a/apps/mobile/__tests__/logger.test.ts b/apps/mobile/__tests__/logger.test.ts index b0070a3..4de9d2a 100644 --- a/apps/mobile/__tests__/logger.test.ts +++ b/apps/mobile/__tests__/logger.test.ts @@ -1,21 +1,17 @@ /** - * Boundary: the only permitted console caller. Two guarantees are tested — - * the level gate per APP_ENV, and that sanitize() actually runs on the data - * argument (not merely that the call looked right). + * Boundary: the only permitted console caller. Three guarantees are tested — + * the level gate per APP_ENV, that sanitize() actually runs on the data + * argument (not merely that the call looked right), and that the *WithCause + * helpers keep err.message out of staging and production. */ -type AppEnv = 'development' | 'staging' | 'production'; +import type { AppEnv } from '../src/lib/config'; + +import { mockConfig } from './helpers/mockConfig'; function loadLogger(appEnv: AppEnv) { let loaded: typeof import('../src/lib/logger') | undefined; jest.isolateModules(() => { - jest.doMock('../src/lib/config', () => ({ - Config: { - supabaseUrl: 'https://project.supabase.co', - supabasePublishableKey: 'sb_publishable_test', - apiUrl: 'https://api.example.com', - appEnv, - }, - })); + jest.doMock('../src/lib/config', () => ({ Config: mockConfig(appEnv) })); // eslint-disable-next-line @typescript-eslint/no-require-imports loaded = require('../src/lib/logger'); }); @@ -115,6 +111,53 @@ describe('MobileLogger data handling', () => { }); }); + it('redacts a listed key at any depth, not only the top level', () => { + const logger = loadLogger('production'); + + logger.warn('AUTH', 'refresh failed', { + context: { access_token: 'never-log-me', inner: { refresh_token: 'nor-this' } }, + hasSession: true, + }); + + expect(spies.warn).toHaveBeenCalledWith('[AUTH]', 'refresh failed', { + context: { access_token: '[REDACTED]', inner: { refresh_token: '[REDACTED]' } }, + hasSession: true, + }); + }); + + it('redacts through arrays', () => { + const logger = loadLogger('production'); + + logger.warn('AUTH', 'batch', { + sessions: [{ access_token: 'one' }, { access_token: 'two' }], + }); + + expect(spies.warn).toHaveBeenCalledWith('[AUTH]', 'batch', { + sessions: [{ access_token: '[REDACTED]' }, { access_token: '[REDACTED]' }], + }); + }); + + it('terminates on a circular structure instead of overflowing the stack', () => { + const logger = loadLogger('production'); + const cycle: Record = { access_token: 'never-log-me' }; + cycle['self'] = cycle; + + logger.warn('AUTH', 'cyclic', { cycle }); + + expect(spies.warn).toHaveBeenCalledWith('[AUTH]', 'cyclic', { + cycle: { access_token: '[REDACTED]', self: '[CIRCULAR]' }, + }); + }); + + it('leaves a non-plain object alone rather than serialising it to {}', () => { + const logger = loadLogger('production'); + const when = new Date('2026-01-01T00:00:00.000Z'); + + logger.warn('CACHE', 'evicted', { when }); + + expect(spies.warn).toHaveBeenCalledWith('[CACHE]', 'evicted', { when }); + }); + it('passes no data argument at all when none was given', () => { const logger = loadLogger('development'); @@ -123,3 +166,55 @@ describe('MobileLogger data handling', () => { expect(spies.info).toHaveBeenCalledWith('[NAV]', 'navigating to /dashboard'); }); }); + +describe('MobileLogger *WithCause', () => { + const tokenBearingError = () => new TypeError('rejected token eyJhbGciOiJIUzI1NiJ9.abc'); + + it('includes errorMessage and a stack in development', () => { + const logger = loadLogger('development'); + + logger.errorWithCause('AUTH', 'verify failed', tokenBearingError(), { hasSession: false }); + + expect(spies.error).toHaveBeenCalledWith( + '[AUTH]', + 'verify failed', + expect.objectContaining({ + errorName: 'TypeError', + errorMessage: 'rejected token eyJhbGciOiJIUzI1NiJ9.abc', + hasSession: false, + }) + ); + expect(spies.error.mock.calls[0]?.[2]).toHaveProperty('errorStack'); + }); + + it('records only errorName in production — err.message may carry a token', () => { + const logger = loadLogger('production'); + + logger.errorWithCause('AUTH', 'verify failed', tokenBearingError()); + + expect(spies.error).toHaveBeenCalledWith('[AUTH]', 'verify failed', { + errorName: 'TypeError', + }); + }); + + it('omits the stack from warnWithCause even in development', () => { + const logger = loadLogger('development'); + + logger.warnWithCause('AUTH', 'retrying', new Error('boom')); + + expect(spies.warn).toHaveBeenCalledWith('[AUTH]', 'retrying', { + errorName: 'Error', + errorMessage: 'boom', + }); + }); + + it('obeys the level gate — warnWithCause is silent at no level, errorWithCause never is', () => { + const logger = loadLogger('production'); + + logger.warnWithCause('AUTH', 'a', new Error('boom')); + logger.errorWithCause('AUTH', 'b', new Error('boom')); + + expect(spies.warn).toHaveBeenCalledTimes(1); + expect(spies.error).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/mobile/__tests__/queryClient.test.ts b/apps/mobile/__tests__/queryClient.test.ts index 355e4b8..4c21182 100644 --- a/apps/mobile/__tests__/queryClient.test.ts +++ b/apps/mobile/__tests__/queryClient.test.ts @@ -8,6 +8,8 @@ */ import type { AppStateStatus } from 'react-native'; +import { mockConfig } from './helpers/mockConfig'; + type MmkvStore = Map; const mockStore: MmkvStore = new Map(); @@ -28,14 +30,7 @@ jest.mock('@react-native-community/netinfo', () => ({ __esModule: true, default: { addEventListener: mockNetInfoAddEventListener }, })); -jest.mock('../src/lib/config', () => ({ - Config: { - supabaseUrl: 'https://project.supabase.co', - supabasePublishableKey: 'sb_publishable_test', - apiUrl: 'https://api.example.com', - appEnv: 'development', - }, -})); +jest.mock('../src/lib/config', () => ({ Config: mockConfig() })); /* eslint-disable @typescript-eslint/no-require-imports */ const { AppState } = require('react-native') as typeof import('react-native'); diff --git a/apps/mobile/__tests__/supabase.test.ts b/apps/mobile/__tests__/supabase.test.ts index c213a60..e18b6d1 100644 --- a/apps/mobile/__tests__/supabase.test.ts +++ b/apps/mobile/__tests__/supabase.test.ts @@ -1,14 +1,26 @@ /** - * Boundary: the Supabase client's construction options. detectSessionInUrl and - * the secure-store adapter are both silent-failure settings — a wrong value - * signs the user out on the next cold launch, or writes refresh tokens to - * unencrypted storage, with nothing failing at build time. + * Boundary: the Supabase client's construction options and the two mobile-only + * behaviours wrapped around it — the secure-store adapter and the + * AppState-driven refresh ticker. + * + * Every one of these fails silently rather than loudly. A wrong + * detectSessionInUrl signs the user out on the next cold launch; the wrong + * storage writes refresh tokens somewhere a device backup can read; a swallowed + * write rejection loses the session with nothing logged; and a ticker that is + * never restarted on resume leaves the app holding an expired JWT. None of them + * fails at build time. */ +import type { AppStateStatus } from 'react-native'; + +import { mockConfig } from './helpers/mockConfig'; + type ClientOptions = { auth: Record }; -const mockCreateClient = jest.fn( - (_url: string, _key: string, _options: ClientOptions) => ({ auth: {} }) -); +const mockStartAutoRefresh = jest.fn(async () => undefined); +const mockStopAutoRefresh = jest.fn(async () => undefined); +const mockCreateClient = jest.fn((_url: string, _key: string, _options: ClientOptions) => ({ + auth: { startAutoRefresh: mockStartAutoRefresh, stopAutoRefresh: mockStopAutoRefresh }, +})); const mockGetItemAsync = jest.fn(async (_key: string) => 'stored-value'); const mockSetItemAsync = jest.fn(async (_key: string, _value: string) => undefined); const mockDeleteItemAsync = jest.fn(async (_key: string) => undefined); @@ -19,17 +31,26 @@ jest.mock('expo-secure-store', () => ({ setItemAsync: mockSetItemAsync, deleteItemAsync: mockDeleteItemAsync, })); -jest.mock('../src/lib/config', () => ({ - Config: { - supabaseUrl: 'https://project.supabase.co', - supabasePublishableKey: 'sb_publishable_test', - apiUrl: 'https://api.example.com', - appEnv: 'development', - }, -})); +jest.mock('../src/lib/config', () => ({ Config: mockConfig() })); + +const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); +const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + +/* eslint-disable @typescript-eslint/no-require-imports */ +const { AppState } = require('react-native') as typeof import('react-native'); +const appStateSpy = jest + .spyOn(AppState, 'addEventListener') + .mockReturnValue({ remove: jest.fn() } as never); -// eslint-disable-next-line @typescript-eslint/no-require-imports require('../src/lib/supabase'); +/* eslint-enable @typescript-eslint/no-require-imports */ + +// Captured at import: the client is constructed and the listener registered +// exactly once, when the module is evaluated, so these records must be read +// before any test clears a mock. +const [url, key, options] = mockCreateClient.mock.calls[0]!; +const appStateCall = appStateSpy.mock.calls[0]; +const appStateHandler = appStateCall?.[1] as (state: AppStateStatus) => void; type Storage = { getItem: (key: string) => Promise; @@ -37,7 +58,12 @@ type Storage = { removeItem: (key: string) => Promise; }; -const [url, key, options] = mockCreateClient.mock.calls[0]!; +const storage = options.auth.storage as Storage; + +afterAll(() => { + errorSpy.mockRestore(); + warnSpy.mockRestore(); +}); describe('supabase client', () => { it('is constructed from Config — the mock intercepted the real module', () => { @@ -56,8 +82,6 @@ describe('supabase client', () => { }); it('stores the session through expo-secure-store, not AsyncStorage or MMKV', async () => { - const storage = options.auth.storage as Storage; - await expect(storage.getItem('sb-session')).resolves.toBe('stored-value'); await storage.setItem('sb-session', 'token-payload'); await storage.removeItem('sb-session'); @@ -67,3 +91,85 @@ describe('supabase client', () => { expect(mockDeleteItemAsync).toHaveBeenCalledWith('sb-session'); }); }); + +describe('secure store adapter failures', () => { + beforeEach(() => { + errorSpy.mockClear(); + }); + + it('logs and rethrows a refused write rather than losing the session quietly', async () => { + mockSetItemAsync.mockRejectedValueOnce(new RangeError('value too large')); + + await expect(storage.setItem('sb-session', 'x')).rejects.toThrow('value too large'); + expect(errorSpy).toHaveBeenCalledWith( + '[AUTH]', + 'Secure store setItem failed', + expect.objectContaining({ errorName: 'RangeError' }) + ); + }); + + it('logs and rethrows a failed read', async () => { + mockGetItemAsync.mockRejectedValueOnce(new Error('keychain locked')); + + await expect(storage.getItem('sb-session')).rejects.toThrow('keychain locked'); + expect(errorSpy).toHaveBeenCalledWith( + '[AUTH]', + 'Secure store getItem failed', + expect.objectContaining({ errorName: 'Error' }) + ); + }); + + it('logs and rethrows a failed delete', async () => { + mockDeleteItemAsync.mockRejectedValueOnce(new Error('keychain locked')); + + await expect(storage.removeItem('sb-session')).rejects.toThrow('keychain locked'); + expect(errorSpy).toHaveBeenCalledWith( + '[AUTH]', + 'Secure store removeItem failed', + expect.objectContaining({ errorName: 'Error' }) + ); + }); +}); + +describe('auto-refresh ticker', () => { + beforeEach(() => { + mockStartAutoRefresh.mockClear(); + mockStopAutoRefresh.mockClear(); + warnSpy.mockClear(); + }); + + it('registers an AppState listener at import', () => { + expect(appStateCall?.[0]).toBe('change'); + expect(typeof appStateHandler).toBe('function'); + }); + + it('starts the ticker when the app becomes active', async () => { + appStateHandler('active'); + await Promise.resolve(); + + expect(mockStartAutoRefresh).toHaveBeenCalledTimes(1); + expect(mockStopAutoRefresh).not.toHaveBeenCalled(); + }); + + it('stops the ticker when the app leaves the foreground', async () => { + appStateHandler('background'); + await Promise.resolve(); + + expect(mockStopAutoRefresh).toHaveBeenCalledTimes(1); + expect(mockStartAutoRefresh).not.toHaveBeenCalled(); + }); + + it('logs a failed toggle instead of leaving an unhandled rejection', async () => { + mockStartAutoRefresh.mockRejectedValueOnce(new Error('no session')); + + appStateHandler('active'); + await Promise.resolve(); + await Promise.resolve(); + + expect(warnSpy).toHaveBeenCalledWith( + '[AUTH]', + 'Auto-refresh toggle failed', + expect.objectContaining({ errorName: 'Error', appState: 'active' }) + ); + }); +}); diff --git a/apps/mobile/jest.config.js b/apps/mobile/jest.config.js index e2638b6..6c7b934 100644 --- a/apps/mobile/jest.config.js +++ b/apps/mobile/jest.config.js @@ -27,6 +27,12 @@ module.exports = { collectCoverageFrom: ['app/**/*.{ts,tsx}', 'src/**/*.{ts,tsx}', '!**/*.d.ts'], + // __tests__/helpers holds shared fixtures, not suites. Jest's default + // testMatch treats every .ts under __tests__ as a test file and would fail + // them for containing no tests. Setting this key replaces the default list + // rather than extending it, so node_modules has to be restated. + testPathIgnorePatterns: ['/node_modules/', '/__tests__/helpers/'], + // No coverageThreshold yet — deliberately, and this is not an oversight to // fill in with a copied block. // diff --git a/apps/mobile/src/constants/colors.ts b/apps/mobile/src/constants/colors.ts index 30aef9b..14df610 100644 --- a/apps/mobile/src/constants/colors.ts +++ b/apps/mobile/src/constants/colors.ts @@ -9,7 +9,11 @@ * Light only, deliberately: REQ-17 Phase 1 specifies no dark palette, and a * second unspecified palette would be invented, not designed. app.json's * `userInterfaceStyle` is pinned to "light" to match — see the PR for #12. - * Dark mode is tracked separately. + * That pin takes effect on iOS only: Expo's app config reference states it + * "Requires `expo-system-ui` be installed in your project to work on Android", + * and that package is not a dependency here. Nothing reads useColorScheme yet, + * so no screen is affected; adding the package needs a device build, which is + * blocked on #95. Dark mode is tracked separately. */ export const colors = { // Brand / interactive — Google sign-in button, tab bar active tint diff --git a/apps/mobile/src/lib/logger.ts b/apps/mobile/src/lib/logger.ts index 6b2da44..6b799e6 100644 --- a/apps/mobile/src/lib/logger.ts +++ b/apps/mobile/src/lib/logger.ts @@ -1,6 +1,6 @@ -import { sanitize } from '@pulseticker/logging'; +import { LEVELS, sanitize, type LogLevel } from '@pulseticker/logging'; -import { Config } from './config'; +import { Config, type AppEnv } from './config'; /** * The only permitted console caller in apps/mobile. @@ -8,39 +8,87 @@ import { Config } from './config'; * `data` is `Record` on purpose (CLAUDE.md > Logging * Strategy): a `Session`, `User` or `Error` cannot be passed whole, so the * compiler rejects the mistake at the call site instead of sanitize() having to - * catch it at runtime. sanitize() stays as the second layer, not the first. + * catch it at runtime. sanitize() stays as the second layer, not the first — + * it redacts a listed key at any depth, but it cannot see inside a class + * instance, which is what the type constraint above is for. * * A browser console is readable by anyone with the device; the same applies to * a React Native log stream on a development build. Safe to log: state flags * ({ hasSession: true }), event names, `error.name`, navigation targets. Never: * tokens, email/phone/name, raw Error objects, the OAuth `?code=` parameter. + * + * An `Error` whose message may carry a token fragment (Supabase auth, JWT + * verification) goes through warnWithCause/errorWithCause, never through the + * plain methods — CLAUDE.md > Logging Strategy §6. */ type LogData = Record; -const isProduction = Config.appEnv === 'production'; +// Mirrors LOG_LEVEL_MAP in apps/web/scripts/set-env.ts. The policy is one +// table in both clients rather than a boolean per method, so changing what +// staging emits is a single edit that cannot leave one level behind. +const MIN_LEVEL_BY_ENV: Record = { + development: 'debug', + staging: 'info', + production: 'warn', +}; + +const minLevel = LEVELS[MIN_LEVEL_BY_ENV[Config.appEnv]]; function format(data?: LogData): [] | [Record] { return data ? [sanitize(data)] : []; } +function emit(level: LogLevel, tag: string, message: string, data?: LogData): void { + if (LEVELS[level] < minLevel) return; + console[level](`[${tag}]`, message, ...format(data)); +} + +function withCause( + level: 'warn' | 'error', + tag: string, + message: string, + err: Error, + extraData?: LogData +): void { + const safe: LogData = { errorName: err.name, ...extraData }; + if (Config.appEnv === 'development') { + safe['errorMessage'] = err.message; + if (level === 'error') safe['errorStack'] = err.stack; + } + emit(level, tag, message, safe); +} + export const MobileLogger = { /** Development-only flow tracing. Silent in staging and production. */ debug(tag: string, message: string, data?: LogData): void { - if (Config.appEnv === 'development') console.debug(`[${tag}]`, message, ...format(data)); + emit('debug', tag, message, data); }, /** Normal business events (sign-in completed, cache hydrated). */ info(tag: string, message: string, data?: LogData): void { - if (!isProduction) console.info(`[${tag}]`, message, ...format(data)); + emit('info', tag, message, data); }, /** Abnormal but recoverable — auth failure, API error with a fallback. */ warn(tag: string, message: string, data?: LogData): void { - console.warn(`[${tag}]`, message, ...format(data)); + emit('warn', tag, message, data); }, /** Exceptions and fatal errors. Always recorded, at every environment. */ error(tag: string, message: string, data?: LogData): void { - console.error(`[${tag}]`, message, ...format(data)); + emit('error', tag, message, data); + }, + + /** + * For errors whose `message` may contain a token fragment — Supabase auth, + * jose/JWT verification. `errorName` is always recorded; the message (and, + * for errors, the stack) only in development. + */ + warnWithCause(tag: string, message: string, err: Error, extraData?: LogData): void { + withCause('warn', tag, message, err, extraData); + }, + + errorWithCause(tag: string, message: string, err: Error, extraData?: LogData): void { + withCause('error', tag, message, err, extraData); }, }; diff --git a/apps/mobile/src/lib/supabase.ts b/apps/mobile/src/lib/supabase.ts index aed16b0..083ee21 100644 --- a/apps/mobile/src/lib/supabase.ts +++ b/apps/mobile/src/lib/supabase.ts @@ -1,17 +1,36 @@ import { createClient } from '@supabase/supabase-js'; import * as SecureStore from 'expo-secure-store'; +import { AppState } from 'react-native'; import { Config } from './config'; +import { MobileLogger } from './logger'; /** * Session storage backed by the platform secure enclave (iOS Keychain / * Android Keystore). Never AsyncStorage and never MMKV: the query cache is * unencrypted, and refresh tokens must not be readable from a device backup. + * + * Every call is wrapped because a rejection here is otherwise invisible. A + * failed write means the session is never persisted and the user is signed out + * on the next cold launch with nothing logged to say why — and a Supabase + * session runs 2.5-4KB, which the platform is documented as free to refuse. + * Log and rethrow (CLAUDE.md > Logging Strategy §3): swallowing it would only + * move the silence. */ +async function guard(operation: string, run: () => Promise): Promise { + try { + return await run(); + } catch (err) { + MobileLogger.errorWithCause('AUTH', `Secure store ${operation} failed`, err as Error); + throw err; + } +} + const secureStoreAdapter = { - getItem: (key: string) => SecureStore.getItemAsync(key), - setItem: (key: string, value: string) => SecureStore.setItemAsync(key, value), - removeItem: (key: string) => SecureStore.deleteItemAsync(key), + getItem: (key: string) => guard('getItem', () => SecureStore.getItemAsync(key)), + setItem: (key: string, value: string) => + guard('setItem', () => SecureStore.setItemAsync(key, value)), + removeItem: (key: string) => guard('removeItem', () => SecureStore.deleteItemAsync(key)), }; export const supabase = createClient(Config.supabaseUrl, Config.supabasePublishableKey, { @@ -26,3 +45,19 @@ export const supabase = createClient(Config.supabaseUrl, Config.supabasePublisha persistSession: true, }, }); + +// autoRefreshToken alone is only reliable in the foreground. auth-js drives it +// from a 30s setInterval, and React Native suspends JS timers while the app is +// backgrounded, so an app resumed after the access token expired issues its +// next request with a stale JWT and takes a 401. supabase-js's React Native +// guidance is to drive the ticker from AppState — this is that wiring. +AppState.addEventListener('change', (state) => { + const toggled = + state === 'active' ? supabase.auth.startAutoRefresh() : supabase.auth.stopAutoRefresh(); + + toggled.catch((err: unknown) => { + MobileLogger.warnWithCause('AUTH', 'Auto-refresh toggle failed', err as Error, { + appState: state, + }); + }); +}); diff --git a/apps/mobile/src/store/authStore.ts b/apps/mobile/src/store/authStore.ts index fd50113..6de53da 100644 --- a/apps/mobile/src/store/authStore.ts +++ b/apps/mobile/src/store/authStore.ts @@ -11,6 +11,12 @@ import { create } from 'zustand'; * * The session carries `access_token`. Pass it around, never log it — not even * a field of it. + * + * `clearSession` is the single teardown path: `setSession(null)` delegates to + * it rather than duplicating the write. onAuthStateChange emits null on + * SIGNED_OUT and screens call clearSession directly, so both reach the same + * place — sign-out cleanup added here (resetting the query cache, wiping + * MMKV) cannot be skipped by one caller and not the other. */ interface AuthState { session: Session | null; @@ -18,8 +24,14 @@ interface AuthState { clearSession: () => void; } -export const useAuthStore = create((set) => ({ +export const useAuthStore = create((set, get) => ({ session: null, - setSession: (session) => set({ session }), + setSession: (session) => { + if (session === null) { + get().clearSession(); + return; + } + set({ session }); + }, clearSession: () => set({ session: null }), })); diff --git a/packages/logging/src/index.ts b/packages/logging/src/index.ts index 995b977..6e169d0 100644 --- a/packages/logging/src/index.ts +++ b/packages/logging/src/index.ts @@ -21,11 +21,49 @@ export const REDACTED_KEYS = new Set([ 'name', ]); -export function sanitize(data: Record): Record { +const isPlainObject = (value: unknown): value is Record => { + if (typeof value !== 'object' || value === null) return false; + const proto: unknown = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +}; + +// Recursion covers plain objects and arrays only. A class instance — Error, +// Date, a Supabase Session — is passed through untouched, because walking an +// arbitrary prototype serialises a Date to {} and can fire getters with side +// effects. Callers must not pass those in the first place: the LogData type +// each app's logger declares rejects them at the call site, and this function +// is the second layer, not the first. +function sanitizeValue(value: unknown, seen: WeakSet): unknown { + if (Array.isArray(value)) { + if (seen.has(value)) return '[CIRCULAR]'; + seen.add(value); + return value.map((item) => sanitizeValue(item, seen)); + } + if (isPlainObject(value)) { + if (seen.has(value)) return '[CIRCULAR]'; + seen.add(value); + return redactEntries(value, seen); + } + return value; +} + +function redactEntries( + data: Record, + seen: WeakSet +): Record { return Object.fromEntries( Object.entries(data).map(([k, v]) => [ k, - REDACTED_KEYS.has(k.toLowerCase()) ? '[REDACTED]' : v, + REDACTED_KEYS.has(k.toLowerCase()) ? '[REDACTED]' : sanitizeValue(v, seen), ]) ); } + +// A redacted key is redacted at every depth. Shallow redaction made the +// guarantee depth-dependent: { context: { access_token } } logged the token +// verbatim while { access_token } did not. +export function sanitize(data: Record): Record { + const seen = new WeakSet(); + seen.add(data); + return redactEntries(data, seen); +} diff --git a/plans/REQ-17_Mobile_App_MVP.md b/plans/REQ-17_Mobile_App_MVP.md index 7162913..187aeaa 100644 --- a/plans/REQ-17_Mobile_App_MVP.md +++ b/plans/REQ-17_Mobile_App_MVP.md @@ -231,6 +231,23 @@ With multiple users subscribing to different tickers, the union can exceed 50 an `SecureLogger` (NestJS-specific) lives in `apps/api/src/common/logger/` — NOT in the package. Safe to import `@pulseticker/logging` in the mobile app. +> **Correction — 2026-09-03 (Task 9 / #12, review of PR #110).** +> `sanitize()` redacted only top-level keys, so the guarantee was +> depth-dependent: `{ access_token }` was redacted and +> `{ context: { access_token } }` was logged verbatim. Task 9 is what made this +> load-bearing — it routes every mobile log through one gateway whose doc +> comment sells `sanitize()` as the second defensive layer — so the fix lands +> in the shared package and applies to `apps/web` and `apps/api` too. It now +> recurses through plain objects and arrays, guards cycles with a `WeakSet`, +> and returns `'[CIRCULAR]'` on a repeat visit. +> +> What it still does not do: walk a class instance. An `Error`, a `Date` or a +> Supabase `Session` is passed through untouched, because walking an arbitrary +> prototype serialises a `Date` to `{}` and can fire getters with side effects. +> That case stays the type constraint's job — `LogData` rejects those at the +> call site — and this is a limitation to know about, not one to rely on +> `sanitize()` for. + ### Development Build is the baseline EAS Development Build is used throughout — Expo Go is not used. @@ -571,6 +588,30 @@ these) from a silent `undefined` endpoint into a startup error naming the variab ## MobileLogger (Task 9) — `src/lib/logger.ts` +> **Correction — 2026-09-03 (Task 9 / #12, review of PR #110).** +> The snippet below is kept as written. Three things in it were wrong, and the +> implementation follows this note where they differ. +> +> 1. **The level gate is `LEVELS`, not a boolean per method.** As written, +> `isProd` gates `debug` and `info` identically, so staging emits `debug` — +> contradicting CLAUDE.md > Logging Strategy §5, which puts staging at +> `info`. The implementation maps `appEnv` to a minimum `LogLevel` and +> compares through the `LEVELS` table `@pulseticker/logging` already exports, +> which is what `apps/web`'s `LoggerService` does for the same gate. What +> this gives up: one indirection between reading a method and knowing +> whether it fires. What it buys: the env-to-level policy exists once, as +> data, in both clients. +> 2. **`warnWithCause` / `errorWithCause` were missing.** CLAUDE.md > Logging +> Strategy §6 requires them for Supabase-auth and JWT errors, whose +> `message` can carry a token fragment, and `LogData` deliberately rejects a +> raw `Error` — so without them Task 10's auth paths had no compliant way to +> log a cause at all. Added, mirroring `apps/web` and `apps/api`: +> `errorName` always, `errorMessage` only in development, and the stack only +> for `errorWithCause` in development. +> 3. **`data ? sanitize(data) : ''` logs a stray empty string.** The +> implementation spreads a tuple instead, so a call with no data passes no +> third argument. + ```typescript // src/lib/logger.ts import { sanitize } from '@pulseticker/logging'; @@ -610,6 +651,34 @@ Never log: `access_token`, `email`, `phone`, raw `Error` objects, the OAuth `?co ## expo-secure-store adapter + Supabase client (Task 9) +> **Correction — 2026-09-03 (Task 9 / #12, review of PR #110).** +> The snippet below is kept as written; two omissions in it are corrected in +> the implementation. +> +> 1. **The adapter swallows nothing, but reports nothing either.** Each of the +> three methods passes the `SecureStore` promise straight through, so a +> rejection surfaces as an unhandled rejection inside auth-js: the session +> is not persisted and the user is signed out on the next cold launch, with +> nothing logged to say why. A Supabase session runs 2.5-4KB and the +> platform is documented as free to refuse a value that large. The +> implementation wraps all three in a `guard()` that logs through +> `errorWithCause` and rethrows (CLAUDE.md > Logging Strategy §3). +> Rejected alternative: Supabase's `LargeSecureStore` (AES key in +> SecureStore, ciphertext in AsyncStorage). It removes the size ceiling but +> puts the encrypted session in unencrypted storage and adds a crypto +> dependency, and the old 2048-byte limit was removed in expo-secure-store +> SDK 55 — so the ceiling may no longer exist on the installed 57.0.2. Not +> worth adopting blind; making the failure loud is enough to find out. +> 2. **`autoRefreshToken: true` is not self-sufficient on React Native.** +> auth-js drives the refresh from a 30s `setInterval`, and React Native +> suspends JS timers while backgrounded, so an app resumed after the access +> token expired issues its next request with a stale JWT. auth-js's own +> docstring instructs RN apps to drive `startAutoRefresh` / `stopAutoRefresh` +> from an `AppState` listener; the implementation adds that listener beside +> the client. It is registered at module scope rather than in the root +> layout so it cannot be forgotten by a screen — the same reasoning the +> focus/online listeners in Task 9 already use. + ```typescript // src/lib/supabase.ts import * as SecureStore from 'expo-secure-store'; @@ -669,6 +738,17 @@ Both listeners must be registered once at app startup, not inside a component. ## Zustand auth store (Task 9) +> **Correction — 2026-09-03 (Task 9 / #12, review of PR #110).** +> `clearSession` and `setSession(null)` below are byte-for-byte the same write, +> and the store documents both as live: `onAuthStateChange` emits null on +> SIGNED_OUT while screens call `clearSession`. Sign-out cleanup added later — +> resetting the query cache, wiping MMKV — would run for one caller and not the +> other. The implementation makes `clearSession` the single teardown path and +> has `setSession(null)` delegate to it. Rejected alternative: deleting +> `clearSession` and letting every caller use `setSession(null)`. The +> three-method surface was deliberated in #12, and a named teardown method is +> the more obvious place to hang cleanup on. + ```typescript // src/store/authStore.ts import { create } from 'zustand';