From 0d0d1c3334c160259e4a43b67278550d4560fafa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Thu, 22 Oct 2026 10:25:02 +0000 Subject: [PATCH 01/10] fix(mobile): keep one organization selection value per account The mobile organization selection is now a single account-scoped SecureStore value: an organization id, the Personal choice, or nothing for "not chosen yet". Every reader decodes it through lib/organization-selection, so the Personal choice is never taken for an organization id. Saves run on the shared serialized save queue and re-check their generation and auth epoch once the queue settles, so the stored value matches the selection the provider publishes even when two selections are made in quick succession. A legacy Personal marker is read once for upgrades, recorded as the selection value, and cleared with the rest of the account metadata on sign-out. --- .../context-control.mounted.test.tsx | 4 +- .../profile-credits-card.mounted.test.tsx | 20 ++-- .../session-stored-context.ts | 6 +- .../src/glanceable-ios/adopt-activity.ts | 3 +- apps/mobile/src/glanceable-ios/interaction.ts | 3 +- .../src/lib/glanceable/activity-kit-prompt.ts | 2 + apps/mobile/src/lib/glanceable/scope.test.ts | 7 ++ apps/mobile/src/lib/glanceable/scope.ts | 4 +- .../src/lib/glanceable/widget-actions.ts | 3 +- .../lib/organization-context.mounted.test.tsx | 101 +++++++++++++--- .../src/lib/organization-context.test.ts | 22 +--- apps/mobile/src/lib/organization-context.tsx | 113 +++++++++++------- apps/mobile/src/lib/organization-selection.ts | 23 ++++ apps/mobile/src/lib/storage-keys.ts | 15 ++- 14 files changed, 228 insertions(+), 98 deletions(-) create mode 100644 apps/mobile/src/lib/organization-selection.ts diff --git a/apps/mobile/src/components/context-control.mounted.test.tsx b/apps/mobile/src/components/context-control.mounted.test.tsx index ae1690d01f3..7d754b8f0e6 100644 --- a/apps/mobile/src/components/context-control.mounted.test.tsx +++ b/apps/mobile/src/components/context-control.mounted.test.tsx @@ -211,8 +211,8 @@ beforeEach(() => { list.mockReset().mockResolvedValue(orgs); storage.read.mockReset().mockResolvedValue(null); storage.write.mockReset().mockImplementation(async (key: string, value: string) => { - // Only the organization key is the selection under test here: an explicit - // Personal choice also writes the marker key beside it. + // Only the organization key carries the selection; the Personal choice is + // one of its values. if (key === ORGANIZATION_STORAGE_KEY) { persisted = value; } diff --git a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx index f8f0fb25998..4e9be6ed3de 100644 --- a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx +++ b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx @@ -367,12 +367,20 @@ describe('CreditsCard balance state', () => { }); it.each([ - { orgs: [], options: ['Personal', 'Cancel'], choice: 0, expected: null, label: 'Personal' }, + { + orgs: [], + options: ['Personal', 'Cancel'], + choice: 0, + expected: null, + stored: 'personal', + label: 'Personal', + }, { orgs: [{ organizationId: 'org-a', organizationName, role: 'owner' }], options: ['Personal', organizationName, 'Cancel'], choice: 1, expected: 'org-a', + stored: 'org-a', label: organizationName, }, ])('uses supplied memberships for the picker: $expected', async state => { @@ -391,11 +399,11 @@ describe('CreditsCard balance state', () => { expect([...accountLabels(renderer), 'Cancel']).toEqual(state.options); await choose(renderer, 'Cancel'); expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe( - (state.orgs as OrgListEntry[])[0]?.organizationId ?? undefined + (state.orgs as OrgListEntry[])[0]?.organizationId ?? 'personal' ); await openSheet(renderer); await choose(renderer, state.label); - expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(state.expected ?? undefined); + expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(state.stored); const label = renderer.root.findByProps({ children: state.label }); expect(label.props).toMatchObject({ numberOfLines: 1, ellipsizeMode: 'tail' }); unmount(); @@ -426,9 +434,7 @@ describe('CreditsCard balance state', () => { expect(retryButton.props.accessibilityHint).toBe('Could not save setting'); // Keep the rendered handler to catch a retry that captures the earlier failed choice. const retry = retryButton.props.onPress as () => void; - (id === null ? storage.remove : storage.write).mockRejectedValueOnce( - new Error('latest save failed') - ); + storage.write.mockRejectedValueOnce(new Error('latest save failed')); await openSheet(renderer); await choose(renderer, id === null ? 'Personal' : 'Latest organization'); const label = id === null ? 'Personal' : 'Latest organization'; @@ -453,7 +459,7 @@ describe('CreditsCard balance state', () => { await act(() => { save.resolve(undefined); }); - expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(id ?? undefined); + expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(id ?? 'personal'); expect(texts()).toContain(label); expect(texts()).not.toContain('Could not save setting'); expect(texts()).not.toContain('Retry'); diff --git a/apps/mobile/src/glanceable-android/session-stored-context.ts b/apps/mobile/src/glanceable-android/session-stored-context.ts index f45312b3d94..b6ee266d253 100644 --- a/apps/mobile/src/glanceable-android/session-stored-context.ts +++ b/apps/mobile/src/glanceable-android/session-stored-context.ts @@ -1,3 +1,4 @@ +import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -46,7 +47,10 @@ export async function readStoredContext(): Promise { if (userId === null) { return null; } - return { userId, organizationId: await getItemAsync(ORGANIZATION_STORAGE_KEY) }; + return { + userId, + organizationId: decodeSelection(await getItemAsync(ORGANIZATION_STORAGE_KEY)), + }; } catch { // The next foreground or push reads it again. return 'unavailable'; diff --git a/apps/mobile/src/glanceable-ios/adopt-activity.ts b/apps/mobile/src/glanceable-ios/adopt-activity.ts index d441500156c..1d9c8392da4 100644 --- a/apps/mobile/src/glanceable-ios/adopt-activity.ts +++ b/apps/mobile/src/glanceable-ios/adopt-activity.ts @@ -6,6 +6,7 @@ import * as SecureStore from '@/lib/auth/secure-store'; // would reach the no-op delivery and the card would stay orphaned. import '@/lib/glanceable/delivery-registration'; import { getLastGlanceableSnapshot, restorePersistedGlanceable } from '@/lib/glanceable/persist'; +import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { adoptNativeActivity, sweepStrayActivities } from './ios-sink'; @@ -26,7 +27,7 @@ async function readStoredContext(): Promise { if (userId === null) { return null; } - return { userId, organizationId: await readKey(ORGANIZATION_STORAGE_KEY) }; + return { userId, organizationId: decodeSelection(await readKey(ORGANIZATION_STORAGE_KEY)) }; } /** diff --git a/apps/mobile/src/glanceable-ios/interaction.ts b/apps/mobile/src/glanceable-ios/interaction.ts index b76cf36f88f..be7bc9adba1 100644 --- a/apps/mobile/src/glanceable-ios/interaction.ts +++ b/apps/mobile/src/glanceable-ios/interaction.ts @@ -10,6 +10,7 @@ import { type GlanceableApproveResult, runGlanceableApprove } from '@/lib/glance import { restorePersistedGlanceable } from '@/lib/glanceable/persist'; import { republishAnsweredAsk } from '@/lib/glanceable/republish-ask'; import { readWaitingAsk, recordWaitingAsk } from '@/lib/glanceable/waiting-ask'; +import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { ActiveAgentsLiveActivity, LIVE_ACTIVITY_NAME } from './active-agents-live-activity'; @@ -199,7 +200,7 @@ async function answerSessionCard( approvalKey: string ): Promise { try { - const organizationId = await readStoredValue(ORGANIZATION_STORAGE_KEY); + const organizationId = decodeSelection(await readStoredValue(ORGANIZATION_STORAGE_KEY)); return await runGlanceableApprove({ now: () => Date.now(), target: { kiloSessionId, organizationId, approvalKey }, diff --git a/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts b/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts index e55e812a0ce..eb6d07c424e 100644 --- a/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts +++ b/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts @@ -11,6 +11,7 @@ import { currentAuthEpoch } from '@/lib/auth/auth-epoch'; import { getTerminalBlankEpoch } from '@/lib/glanceable/cleanup'; import { getLastGlanceableSnapshot, getLocalScopeKey } from '@/lib/glanceable/persist'; import { forEachSink } from '@/lib/glanceable/sink-registry'; +import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -60,6 +61,7 @@ async function replayVerifiedSnapshot(admit: () => boolean): Promise { // A failed organization read must not be treated as the personal scope. return; } + organizationId = decodeSelection(organizationId); if ( currentAuthEpoch() !== authEpoch || getTerminalBlankEpoch() !== blankEpoch || diff --git a/apps/mobile/src/lib/glanceable/scope.test.ts b/apps/mobile/src/lib/glanceable/scope.test.ts index 7fca3e99644..60b4c8bf81f 100644 --- a/apps/mobile/src/lib/glanceable/scope.test.ts +++ b/apps/mobile/src/lib/glanceable/scope.test.ts @@ -25,6 +25,13 @@ describe('glanceable scope reads', () => { expect(mocks.getItemAsync).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); }); + it('resolves a stored Personal selection to the personal scope', async () => { + mocks.getItemAsync.mockResolvedValue('personal'); + + // A stored Personal choice resolves to the personal scope for every reader. + await expect(getSelectedOrganizationId()).resolves.toBeNull(); + }); + it('reads the active user from its storage key', async () => { mocks.getItemAsync.mockResolvedValue('u1'); diff --git a/apps/mobile/src/lib/glanceable/scope.ts b/apps/mobile/src/lib/glanceable/scope.ts index 4de63c7cd2d..2be2430570b 100644 --- a/apps/mobile/src/lib/glanceable/scope.ts +++ b/apps/mobile/src/lib/glanceable/scope.ts @@ -1,4 +1,5 @@ import { readStoredValueSafe } from '@/lib/auth/secure-store-value'; +import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -23,8 +24,7 @@ import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys * A missing hint only matches a personal scope; it cannot revive an org scope. */ export async function getSelectedOrganizationId(): Promise { - const organizationId = await readStoredValueSafe(ORGANIZATION_STORAGE_KEY); - return organizationId; + return decodeSelection(await readStoredValueSafe(ORGANIZATION_STORAGE_KEY)); } /** diff --git a/apps/mobile/src/lib/glanceable/widget-actions.ts b/apps/mobile/src/lib/glanceable/widget-actions.ts index 3d9d571b378..02f8753ac5b 100644 --- a/apps/mobile/src/lib/glanceable/widget-actions.ts +++ b/apps/mobile/src/lib/glanceable/widget-actions.ts @@ -9,6 +9,7 @@ import { buildActiveSessionsTrayInput, isAttentionStatus } from '@/lib/active-se import { readStoredValue } from '@/lib/auth/secure-store-value'; import { dismissNeedsInputNotification } from '@/lib/needs-input-notification'; import { ackSessionAttention } from '@/lib/session-attention'; +import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { reportSecureStoreFailure } from '@/lib/telemetry/secure-store-events'; import { trpcClient } from '@/lib/trpc'; @@ -172,7 +173,7 @@ async function readStoredScope(): Promise { readStoredValueReported(ORGANIZATION_STORAGE_KEY), readStoredValueReported(ACTIVE_USER_ID_KEY), ]); - return { organizationId: organizationId ?? null, userId: userId ?? null }; + return { organizationId: decodeSelection(organizationId), userId: userId ?? null }; } /** diff --git a/apps/mobile/src/lib/organization-context.mounted.test.tsx b/apps/mobile/src/lib/organization-context.mounted.test.tsx index 3e1117dde57..17a41a6803a 100644 --- a/apps/mobile/src/lib/organization-context.mounted.test.tsx +++ b/apps/mobile/src/lib/organization-context.mounted.test.tsx @@ -215,38 +215,37 @@ describe('OrganizationProvider default organization', () => { expect(scope().organizationId).toBe('org-a'); }); - it('writes the explicit Personal marker when Personal is chosen while already auto-resolved', async () => { + it('stores the explicit Personal selection when Personal is chosen while already auto-resolved', async () => { list.mockResolvedValue([]); await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); // Auto-resolved Personal from an empty list is not an explicit choice, so - // no marker is written yet. - expect(savedMetadata.has(MARKER_KEY)).toBe(false); + // nothing is stored yet. + expect(savedMetadata.has(ORG_KEY)).toBe(false); await act(() => { scope().setOrganizationId(null); }); - // Selecting Personal explicitly must record the choice even though the - // published id did not change, or a later list with organizations would - // move the person back to one on the next launch. - await waitFor(() => savedMetadata.get(MARKER_KEY) === 'personal'); - expect(savedMetadata.has(ORG_KEY)).toBe(false); + // Selecting Personal explicitly records the choice even though the + // published id does not change, so the next launch resolves to Personal. + await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal'); + expect(savedMetadata.has(MARKER_KEY)).toBe(false); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); }); - it('keeps an explicit Personal marker across a re-mount with organizations available', async () => { - savedMetadata.set(MARKER_KEY, 'personal'); + it('keeps the stored Personal selection across a re-mount with organizations available', async () => { + savedMetadata.set(ORG_KEY, 'personal'); list.mockResolvedValue([orgA]); const first = await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); - // An explicit Personal choice is not overridden by the default, so the - // organization key stays absent for every other scope reader too. - expect(savedMetadata.has(ORG_KEY)).toBe(false); + // An explicit Personal choice is not overridden by the default. + expect(savedMetadata.get(ORG_KEY)).toBe('personal'); unmount(first); await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); + expect(savedMetadata.get(ORG_KEY)).toBe('personal'); }); it('publishes nothing stale when a sign-out lands during the list fetch', async () => { @@ -399,7 +398,7 @@ describe('OrganizationProvider restoration fencing', () => { names.resolve([orgA]); }); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); - expect(savedMetadata.get(MARKER_KEY)).toBe('personal'); + expect(savedMetadata.get(ORG_KEY)).toBe('personal'); }); }); @@ -416,7 +415,7 @@ describe('OrganizationProvider persistence', () => { expect(scope()).toMatchObject({ organizationId: 'org-a', error: null }); }); - it('deletes the organization key and writes the Personal marker', async () => { + it('stores the Personal choice as the organization value', async () => { savedMetadata.set(ORG_KEY, 'org-a'); await mount(); await waitFor(() => scope().isLoaded); @@ -424,8 +423,8 @@ describe('OrganizationProvider persistence', () => { scope().setOrganizationId(null); }); await waitFor(() => !scope().isSaving); - expect(savedMetadata.has(ORG_KEY)).toBe(false); - expect(savedMetadata.get(MARKER_KEY)).toBe('personal'); + expect(savedMetadata.get(ORG_KEY)).toBe('personal'); + expect(savedMetadata.has(MARKER_KEY)).toBe(false); expect(scope()).toMatchObject({ organizationId: null, error: null }); }); @@ -465,4 +464,72 @@ describe('OrganizationProvider persistence', () => { await waitFor(() => !scope().isSaving); expect(scope()).toMatchObject({ organizationId: 'org-a', error: null }); }); + + it('applies saves in call order and stores the last selection', async () => { + const firstWrite = Promise.withResolvers(); + const started: string[] = []; + storage.write.mockImplementation(async (key: string, value: string) => { + started.push(value); + if (started.length === 1) { + await firstWrite.promise; + } + savedMetadata.set(key, value); + await Promise.resolve(); + }); + await mount(); + await waitFor(() => scope().isLoaded); + await act(() => { + scope().setOrganizationId('org-a'); + }); + await act(() => { + scope().setOrganizationId('org-b'); + }); + // Saves run in call order: the second starts only after the first settles. + expect(started).toEqual(['org-a']); + await act(() => { + firstWrite.resolve(undefined); + }); + await waitFor(() => savedMetadata.get(ORG_KEY) === 'org-b' && !scope().isSaving); + expect(started).toEqual(['org-a', 'org-b']); + expect(scope()).toMatchObject({ organizationId: 'org-b', isLoaded: true, error: null }); + expect(savedMetadata.has(MARKER_KEY)).toBe(false); + }); + + it('stores the Personal choice when two selections are made back to back', async () => { + const firstWrite = Promise.withResolvers(); + const started: string[] = []; + storage.write.mockImplementation(async (key: string, value: string) => { + started.push(value); + if (started.length === 1) { + await firstWrite.promise; + } + savedMetadata.set(key, value); + await Promise.resolve(); + }); + await mount(); + await waitFor(() => scope().isLoaded); + await act(() => { + scope().setOrganizationId('org-a'); + }); + await act(() => { + scope().setOrganizationId(null); + }); + expect(started).toEqual(['org-a']); + await act(() => { + firstWrite.resolve(undefined); + }); + await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal' && !scope().isSaving); + expect(started).toEqual(['org-a', 'personal']); + expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); + expect(savedMetadata.has(MARKER_KEY)).toBe(false); + }); + + it('records a legacy Personal choice as the selection value', async () => { + savedMetadata.set(MARKER_KEY, 'personal'); + await mount(); + await waitFor(() => scope().isLoaded); + await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal'); + expect(scope()).toMatchObject({ organizationId: null, error: null }); + expect(savedMetadata.has(MARKER_KEY)).toBe(false); + }); }); diff --git a/apps/mobile/src/lib/organization-context.test.ts b/apps/mobile/src/lib/organization-context.test.ts index 5c6e84b7899..57d4da4f00e 100644 --- a/apps/mobile/src/lib/organization-context.test.ts +++ b/apps/mobile/src/lib/organization-context.test.ts @@ -147,7 +147,7 @@ describe('OrganizationProvider.setOrganizationId', () => { unmount(); }); - it('clears the persisted org, unregisters tokens, and marks Personal on the way to personal', async () => { + it('stores the Personal choice as the selection value', async () => { const { getCtx, unmount } = await mountProvider(); await act(() => { @@ -157,11 +157,8 @@ describe('OrganizationProvider.setOrganizationId', () => { getCtx().setOrganizationId(null); }); - expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( - ORGANIZATION_PERSONAL_STORAGE_KEY, - 'personal' - ); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); + expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_PERSONAL_STORAGE_KEY); expect(hoisted.unregisterActivityTokensAndTombstone).toHaveBeenCalledTimes(2); expect(getCtx().organizationId).toBeNull(); @@ -182,10 +179,7 @@ describe('OrganizationProvider.setOrganizationId', () => { }); expect(getCtx().organizationId).toBeNull(); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( - ORGANIZATION_PERSONAL_STORAGE_KEY, - 'personal' - ); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); unmount(); }); @@ -250,7 +244,7 @@ describe('OrganizationProvider lost-membership recovery', () => { unmount(); }); - it('falls back to Personal, writes the marker, and names Personal when no membership remains', async () => { + it('falls back to Personal, stores the choice, and names Personal when no membership remains', async () => { storeSelection(); hoisted.list.mockResolvedValue([ { organizationId: 'org-lost', organizationName: 'Doomed org', role: 'member' }, @@ -261,11 +255,7 @@ describe('OrganizationProvider lost-membership recovery', () => { await waitFor(() => getCtx().organizationId === null); expect(getCtx().organizationId).toBeNull(); - expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( - ORGANIZATION_PERSONAL_STORAGE_KEY, - 'personal' - ); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); expect(hoisted.toastWarning).toHaveBeenCalledWith( 'You no longer have access to this organization. Switched to Personal.' ); diff --git a/apps/mobile/src/lib/organization-context.tsx b/apps/mobile/src/lib/organization-context.tsx index 3cd348b5032..2a59edacb89 100644 --- a/apps/mobile/src/lib/organization-context.tsx +++ b/apps/mobile/src/lib/organization-context.tsx @@ -21,20 +21,18 @@ import { unregisterActivityTokensAndTombstone } from '@/lib/auth/logout-cleanup' import { writePrivacySnapshotAndEnd } from '@/lib/glanceable/cleanup'; import { useOrganizationsList } from '@/lib/hooks/use-organizations-list'; import { type OrgListEntry } from '@/lib/hooks/use-organization-queries'; +import { chainSave } from '@/lib/hooks/save-chain'; import { announcingToast } from '@/lib/a11y/announcing-toast'; import { i18n } from '@/i18n'; +import { decodeSelection, encodeSelection, PERSONAL_SELECTION } from '@/lib/organization-selection'; import { ORGANIZATION_PERSONAL_STORAGE_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** - * `ORGANIZATION_PERSONAL_STORAGE_KEY` is a SECOND key beside - * `ORGANIZATION_STORAGE_KEY`. Every other reader of that key (glanceable - * scope, widget actions, voice input, tool-summary) treats the stored value as - * an organization id, so an explicit Personal choice keeps deleting it; the - * settled "Personal was chosen" marker lives beside it. An absent organization - * key alone now means "not chosen yet", which the default rule resolves from - * the organization list. + * One queue slot for the selection: every save runs through it in call order, + * so the stored value always matches the selection the provider last published. + * The stored form itself is one value; see `lib/organization-selection`. */ -const PERSONAL_MARKER = 'personal'; +const SELECTION_QUEUE_KEY = 'organization-selection'; type OrganizationContextValue = { /** null = personal, string = org UUID */ @@ -94,6 +92,42 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod // the membership vanished server-side. const selectionListSeen = useRef(undefined); + const persist = useCallback(async (id: string | null) => { + generation.current += 1; + const operation = generation.current; + const epoch = currentAuthEpoch(); + setState(current => ({ ...current, isSaving: true })); + let error: 'save' | null = null; + try { + // Serialized per selection and fenced after the queue settles: only the + // selection the provider still publishes is written, in the order the + // selections were made. + await chainSave(SELECTION_QUEUE_KEY, async () => { + if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { + return; + } + await setAccountMetadata(ORGANIZATION_STORAGE_KEY, encodeSelection(id)); + if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { + return; + } + // The account stores one selection value; this legacy key is cleared so + // only that value answers which account is selected. A failed delete is + // reported by the storage layer and retried by the next selection; it + // must not turn a stored selection into a save error. + try { + await deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY); + } catch { + // Cleanup only: the selection value is already stored. + } + }); + } catch { + error = 'save'; + } + if (generation.current === operation && isCurrentAuthEpoch(epoch)) { + setState(current => ({ ...current, error, isSaving: false })); + } + }, []); + const restore = useCallback(async () => { generation.current += 1; const operation = generation.current; @@ -104,26 +138,38 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod selectionListSeen.current = undefined; setState(current => ({ ...current, token, isLoaded: false, isSaving: false, error: null })); try { - // A stored organization is an explicit choice; the marker beside it is an - // explicit Personal choice. An absent value is 'not chosen yet': the - // organization list decides the default and the effect below publishes it. - // A legacy absent value is treated the same way. - // The two reads are kept apart: a stored organization stands alone, so a - // marker-read failure must not discard it. The marker is only consulted - // when no organization is stored, and a failure there stays a restore - // error — the selection is then genuinely undetermined. + // A stored value is the whole selection: an organization id or the + // Personal choice. It is one key, so the read is already atomic and needs + // no queue; chaining it would let a slow keychain read hold up the saves + // behind it. An absent value is 'not chosen yet' unless the legacy marker + // is still present, which the fold below replaces with the value itself. + // The marker is read only when nothing is stored, so a marker-read + // failure cannot discard a stored selection, and a value a save lands + // meanwhile replaces this read through the fence below. const stored = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); - const marker = - stored === null ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) : null; + const personalMarker = + stored === null + ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) + : null; if (generation.current === operation && isCurrentAuthEpoch(epoch)) { if (stored) { - activeId.current = stored; - personalExplicit.current = false; - setState({ token, organizationId: stored, isLoaded: true, isSaving: false, error: null }); - } else if (marker === PERSONAL_MARKER) { + const selected = decodeSelection(stored); + activeId.current = selected; + personalExplicit.current = selected === null; + setState({ + token, + organizationId: selected, + isLoaded: true, + isSaving: false, + error: null, + }); + } else if (personalMarker === PERSONAL_SELECTION) { activeId.current = null; personalExplicit.current = true; setState({ token, organizationId: null, isLoaded: true, isSaving: false, error: null }); + // Record the choice as the selection value so one value answers it + // from the next launch on. + void persist(null); } else { personalExplicit.current = false; awaitingDefault.current = true; @@ -139,28 +185,7 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod setState(current => ({ ...current, error: 'restore' })); } } - }, [token]); - - const persist = useCallback(async (id: string | null) => { - generation.current += 1; - const operation = generation.current; - const epoch = currentAuthEpoch(); - setState(current => ({ ...current, isSaving: true })); - let error: 'save' | null = null; - try { - await (id - ? setAccountMetadata(ORGANIZATION_STORAGE_KEY, id) - : deleteAccountMetadata(ORGANIZATION_STORAGE_KEY)); - await (id - ? deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY) - : setAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY, PERSONAL_MARKER)); - } catch { - error = 'save'; - } - if (generation.current === operation && isCurrentAuthEpoch(epoch)) { - setState(current => ({ ...current, error, isSaving: false })); - } - }, []); + }, [token, persist]); // Publish the default organization once the shared list has settled. Fenced // by the generation and auth epoch captured in `restore()`, so a sign-out or diff --git a/apps/mobile/src/lib/organization-selection.ts b/apps/mobile/src/lib/organization-selection.ts new file mode 100644 index 00000000000..85a665f185d --- /dev/null +++ b/apps/mobile/src/lib/organization-selection.ts @@ -0,0 +1,23 @@ +/** + * The stored organization selection. + * + * The selection is ONE SecureStore value under `ORGANIZATION_STORAGE_KEY`: an + * organization id, or {@link PERSONAL_SELECTION} for an explicit Personal + * choice. An absent value means "not chosen yet", which the organization + * context's default rule resolves from the organization list. + * + * Readers decode through {@link decodeSelection}, so the Personal choice is + * never mistaken for an organization id, and an account's selection travels as + * one value that a save writes as a unit. + */ +export const PERSONAL_SELECTION = 'personal'; + +/** The selected organization id, or null for the Personal scope. */ +export function decodeSelection(stored: string | null): string | null { + return stored === PERSONAL_SELECTION ? null : stored; +} + +/** The stored value that records `organizationId`, Personal included. */ +export function encodeSelection(organizationId: string | null): string { + return organizationId ?? PERSONAL_SELECTION; +} diff --git a/apps/mobile/src/lib/storage-keys.ts b/apps/mobile/src/lib/storage-keys.ts index d82e913b483..7596745d94d 100644 --- a/apps/mobile/src/lib/storage-keys.ts +++ b/apps/mobile/src/lib/storage-keys.ts @@ -7,17 +7,20 @@ */ export const AUTH_TOKEN_KEY = 'auth-token'; -export const ORGANIZATION_STORAGE_KEY = 'selected-organization'; /** - * Settled "Personal was chosen" marker, beside `ORGANIZATION_STORAGE_KEY`. - * That key keeps deleting on a Personal choice because every other reader - * treats a stored value as an organization id; this marker tells an explicit - * Personal choice apart from an absent key ("not chosen yet"), which the - * organization-context default rule resolves from the organization list. + * The organization selection: one value holding an organization id, or the + * Personal selection, or nothing ("not chosen yet", resolved from the + * organization list). Encode and decode through `lib/organization-selection`. * Account-scoped selection state: deleted with the rest of the account * metadata on sign-out and on a direct account switch, so the next account on * the device resolves its own default instead of inheriting this choice. */ +export const ORGANIZATION_STORAGE_KEY = 'selected-organization'; +/** + * Legacy marker for the Personal choice, kept for upgrade and sign-out cleanup + * only. The selection value carries the Personal choice itself, and a selection + * made after an upgrade removes this key. + */ export const ORGANIZATION_PERSONAL_STORAGE_KEY = 'selected-organization-personal'; /** Filter record for the session history page. */ export const SESSION_FILTERS_KEY = 'agent-session-filters'; From ec7660603299f9a14f9e12e97a8ae700a81caa4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Thu, 22 Oct 2026 10:29:02 +0000 Subject: [PATCH 02/10] chore(mobile): format the organization selection restore read --- apps/mobile/src/lib/organization-context.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/mobile/src/lib/organization-context.tsx b/apps/mobile/src/lib/organization-context.tsx index 2a59edacb89..ce9c306f426 100644 --- a/apps/mobile/src/lib/organization-context.tsx +++ b/apps/mobile/src/lib/organization-context.tsx @@ -148,9 +148,7 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod // meanwhile replaces this read through the fence below. const stored = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); const personalMarker = - stored === null - ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) - : null; + stored === null ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) : null; if (generation.current === operation && isCurrentAuthEpoch(epoch)) { if (stored) { const selected = decodeSelection(stored); From efd65232b98c14c615ebf134f57d5066dd3c85d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Thu, 22 Oct 2026 10:39:52 +0000 Subject: [PATCH 03/10] fix(mobile): resolve the request scope through the shared selection helper The voice-input catalogue read and the tool-summary translation resolve their organization scope through the same helper every other reader uses, so each request runs in the scope selected in the app. --- .../tool-summary-translation-client.test.ts | 13 +++++++++++++ .../tool-summary-translation-client.ts | 3 ++- .../gateway/native-gateway-voice-input.test.ts | 8 ++++++++ .../gateway/native-gateway-voice-input.ts | 15 +++++++-------- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts index 7e471dc81b9..01a456f5b5f 100644 --- a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts +++ b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts @@ -115,6 +115,19 @@ describe('requestToolSummaryTranslations', () => { }); }); + it('resolves the personal scope for the translation request', async () => { + getItemAsync.mockResolvedValue('personal'); + fetchMock.mockResolvedValue(arrayContent(['ok'])); + + await requestToolSummaryTranslations(INPUT); + + expect(lastFetchCall().init.headers).toEqual({ + Authorization: 'Bearer token-1', + 'Content-Type': 'application/json', + 'X-KILOCODE-FEATURE': 'tool-summary-translation', + }); + }); + it('falls back to the raw language tag when it has no endonym', async () => { fetchMock.mockResolvedValue(arrayContent(['ok'])); diff --git a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts index e8088c961a3..79e4e2e482f 100644 --- a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts +++ b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts @@ -4,6 +4,7 @@ import { isSupportedLanguage, LANGUAGE_ENDONYMS } from '@/i18n/languages'; import { readStoredValue } from '@/lib/auth/secure-store-value'; import { getAuthTokenForRequest } from '@/lib/auth/token-owner'; import { API_BASE_URL } from '@/lib/config'; +import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -114,7 +115,7 @@ export async function requestToolSummaryTranslations({ return nullResults(texts.length); } // The organization scope the translation is attributed to; null is personal. - const organizationId = await readStoredValue(ORGANIZATION_STORAGE_KEY); + const organizationId = decodeSelection(await readStoredValue(ORGANIZATION_STORAGE_KEY)); const headers: TranslationRequestHeaders = { Authorization: `Bearer ${token}`, diff --git a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts index a084c9d3f94..2c1856e034f 100644 --- a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts +++ b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts @@ -255,6 +255,14 @@ describe('resolveGatewayTranscriptionModelId', () => { expect(transcriptionModels.fetchTranscriptionModels).toHaveBeenCalledWith('org-42'); }); + it('resolves the personal scope for the catalogue read', async () => { + vi.mocked(SecureStore.getItemAsync).mockResolvedValueOnce('personal'); + transcriptionModels.fetchTranscriptionModels.mockResolvedValue([]); + + await expect(resolveGatewayTranscriptionModelId()).resolves.toBeNull(); + expect(transcriptionModels.fetchTranscriptionModels).toHaveBeenCalledWith(undefined); + }); + it('falls back to the first catalogue entry and persists it when none is stored', async () => { transcriptionModels.fetchTranscriptionModels.mockResolvedValue([ { id: 'first-model', name: 'First Model' }, diff --git a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts index 699ec924878..411ae266c81 100644 --- a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts +++ b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts @@ -4,6 +4,7 @@ import * as SecureStore from '@/lib/auth/secure-store'; import { getAuthTokenForRequest } from '@/lib/auth/token-owner'; import { fetchTranscriptionModels } from '@/lib/hooks/use-transcription-models'; +import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { createGatewayVoiceInputEngine, type GatewayRecorder } from './gateway-voice-input-engine'; @@ -16,12 +17,6 @@ import { const HIGH_QUALITY = RecordingPresets.HIGH_QUALITY; -/** The organization scope the voice flow reads and uploads under; null is personal. */ -async function readStoredOrganizationId(): Promise { - const value = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); - return value; -} - /** * The model the gateway engine transcribes with: the stored choice, else the * first model the gateway catalogue offers (persisted so it survives launches @@ -42,7 +37,9 @@ export async function resolveGatewayTranscriptionModelId(): Promise + decodeSelection(await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY)), deleteRecording: (uri: string) => { // `release()` frees the native object, not the file: delete the recording // so dictation does not accumulate audio on disk. The modern File API From 1db1bbac102b410d4c18496f7cd5600c8394e7f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:42:08 +0000 Subject: [PATCH 04/10] fix(mobile): queue organization selection saves in call order Saves of the mobile organization selection now share one queue slot per account, so a later choice is written after an earlier one and the organization key and Personal marker go out as a single pair. The stored format is unchanged. The regression test parks every storage call and shows the second selection is written after the first pair. --- .../context-control.mounted.test.tsx | 4 +- .../profile-credits-card.mounted.test.tsx | 20 +-- .../session-stored-context.ts | 6 +- .../src/glanceable-ios/adopt-activity.ts | 3 +- apps/mobile/src/glanceable-ios/interaction.ts | 3 +- .../src/lib/glanceable/activity-kit-prompt.ts | 2 - apps/mobile/src/lib/glanceable/scope.test.ts | 7 - apps/mobile/src/lib/glanceable/scope.ts | 4 +- .../src/lib/glanceable/widget-actions.ts | 3 +- .../lib/organization-context.mounted.test.tsx | 101 +++----------- .../src/lib/organization-context.test.ts | 76 ++++++++++- apps/mobile/src/lib/organization-context.tsx | 129 +++++++++--------- apps/mobile/src/lib/organization-selection.ts | 23 ---- apps/mobile/src/lib/storage-keys.ts | 15 +- .../tool-summary-translation-client.test.ts | 13 -- .../tool-summary-translation-client.ts | 3 +- .../native-gateway-voice-input.test.ts | 8 -- .../gateway/native-gateway-voice-input.ts | 15 +- 18 files changed, 182 insertions(+), 253 deletions(-) delete mode 100644 apps/mobile/src/lib/organization-selection.ts diff --git a/apps/mobile/src/components/context-control.mounted.test.tsx b/apps/mobile/src/components/context-control.mounted.test.tsx index 7d754b8f0e6..ae1690d01f3 100644 --- a/apps/mobile/src/components/context-control.mounted.test.tsx +++ b/apps/mobile/src/components/context-control.mounted.test.tsx @@ -211,8 +211,8 @@ beforeEach(() => { list.mockReset().mockResolvedValue(orgs); storage.read.mockReset().mockResolvedValue(null); storage.write.mockReset().mockImplementation(async (key: string, value: string) => { - // Only the organization key carries the selection; the Personal choice is - // one of its values. + // Only the organization key is the selection under test here: an explicit + // Personal choice also writes the marker key beside it. if (key === ORGANIZATION_STORAGE_KEY) { persisted = value; } diff --git a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx index 4e9be6ed3de..f8f0fb25998 100644 --- a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx +++ b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx @@ -367,20 +367,12 @@ describe('CreditsCard balance state', () => { }); it.each([ - { - orgs: [], - options: ['Personal', 'Cancel'], - choice: 0, - expected: null, - stored: 'personal', - label: 'Personal', - }, + { orgs: [], options: ['Personal', 'Cancel'], choice: 0, expected: null, label: 'Personal' }, { orgs: [{ organizationId: 'org-a', organizationName, role: 'owner' }], options: ['Personal', organizationName, 'Cancel'], choice: 1, expected: 'org-a', - stored: 'org-a', label: organizationName, }, ])('uses supplied memberships for the picker: $expected', async state => { @@ -399,11 +391,11 @@ describe('CreditsCard balance state', () => { expect([...accountLabels(renderer), 'Cancel']).toEqual(state.options); await choose(renderer, 'Cancel'); expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe( - (state.orgs as OrgListEntry[])[0]?.organizationId ?? 'personal' + (state.orgs as OrgListEntry[])[0]?.organizationId ?? undefined ); await openSheet(renderer); await choose(renderer, state.label); - expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(state.stored); + expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(state.expected ?? undefined); const label = renderer.root.findByProps({ children: state.label }); expect(label.props).toMatchObject({ numberOfLines: 1, ellipsizeMode: 'tail' }); unmount(); @@ -434,7 +426,9 @@ describe('CreditsCard balance state', () => { expect(retryButton.props.accessibilityHint).toBe('Could not save setting'); // Keep the rendered handler to catch a retry that captures the earlier failed choice. const retry = retryButton.props.onPress as () => void; - storage.write.mockRejectedValueOnce(new Error('latest save failed')); + (id === null ? storage.remove : storage.write).mockRejectedValueOnce( + new Error('latest save failed') + ); await openSheet(renderer); await choose(renderer, id === null ? 'Personal' : 'Latest organization'); const label = id === null ? 'Personal' : 'Latest organization'; @@ -459,7 +453,7 @@ describe('CreditsCard balance state', () => { await act(() => { save.resolve(undefined); }); - expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(id ?? 'personal'); + expect(savedMetadata.get(ORGANIZATION_STORAGE_KEY)).toBe(id ?? undefined); expect(texts()).toContain(label); expect(texts()).not.toContain('Could not save setting'); expect(texts()).not.toContain('Retry'); diff --git a/apps/mobile/src/glanceable-android/session-stored-context.ts b/apps/mobile/src/glanceable-android/session-stored-context.ts index b6ee266d253..f45312b3d94 100644 --- a/apps/mobile/src/glanceable-android/session-stored-context.ts +++ b/apps/mobile/src/glanceable-android/session-stored-context.ts @@ -1,4 +1,3 @@ -import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -47,10 +46,7 @@ export async function readStoredContext(): Promise { if (userId === null) { return null; } - return { - userId, - organizationId: decodeSelection(await getItemAsync(ORGANIZATION_STORAGE_KEY)), - }; + return { userId, organizationId: await getItemAsync(ORGANIZATION_STORAGE_KEY) }; } catch { // The next foreground or push reads it again. return 'unavailable'; diff --git a/apps/mobile/src/glanceable-ios/adopt-activity.ts b/apps/mobile/src/glanceable-ios/adopt-activity.ts index 1d9c8392da4..d441500156c 100644 --- a/apps/mobile/src/glanceable-ios/adopt-activity.ts +++ b/apps/mobile/src/glanceable-ios/adopt-activity.ts @@ -6,7 +6,6 @@ import * as SecureStore from '@/lib/auth/secure-store'; // would reach the no-op delivery and the card would stay orphaned. import '@/lib/glanceable/delivery-registration'; import { getLastGlanceableSnapshot, restorePersistedGlanceable } from '@/lib/glanceable/persist'; -import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { adoptNativeActivity, sweepStrayActivities } from './ios-sink'; @@ -27,7 +26,7 @@ async function readStoredContext(): Promise { if (userId === null) { return null; } - return { userId, organizationId: decodeSelection(await readKey(ORGANIZATION_STORAGE_KEY)) }; + return { userId, organizationId: await readKey(ORGANIZATION_STORAGE_KEY) }; } /** diff --git a/apps/mobile/src/glanceable-ios/interaction.ts b/apps/mobile/src/glanceable-ios/interaction.ts index be7bc9adba1..b76cf36f88f 100644 --- a/apps/mobile/src/glanceable-ios/interaction.ts +++ b/apps/mobile/src/glanceable-ios/interaction.ts @@ -10,7 +10,6 @@ import { type GlanceableApproveResult, runGlanceableApprove } from '@/lib/glance import { restorePersistedGlanceable } from '@/lib/glanceable/persist'; import { republishAnsweredAsk } from '@/lib/glanceable/republish-ask'; import { readWaitingAsk, recordWaitingAsk } from '@/lib/glanceable/waiting-ask'; -import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { ActiveAgentsLiveActivity, LIVE_ACTIVITY_NAME } from './active-agents-live-activity'; @@ -200,7 +199,7 @@ async function answerSessionCard( approvalKey: string ): Promise { try { - const organizationId = decodeSelection(await readStoredValue(ORGANIZATION_STORAGE_KEY)); + const organizationId = await readStoredValue(ORGANIZATION_STORAGE_KEY); return await runGlanceableApprove({ now: () => Date.now(), target: { kiloSessionId, organizationId, approvalKey }, diff --git a/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts b/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts index eb6d07c424e..e55e812a0ce 100644 --- a/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts +++ b/apps/mobile/src/lib/glanceable/activity-kit-prompt.ts @@ -11,7 +11,6 @@ import { currentAuthEpoch } from '@/lib/auth/auth-epoch'; import { getTerminalBlankEpoch } from '@/lib/glanceable/cleanup'; import { getLastGlanceableSnapshot, getLocalScopeKey } from '@/lib/glanceable/persist'; import { forEachSink } from '@/lib/glanceable/sink-registry'; -import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -61,7 +60,6 @@ async function replayVerifiedSnapshot(admit: () => boolean): Promise { // A failed organization read must not be treated as the personal scope. return; } - organizationId = decodeSelection(organizationId); if ( currentAuthEpoch() !== authEpoch || getTerminalBlankEpoch() !== blankEpoch || diff --git a/apps/mobile/src/lib/glanceable/scope.test.ts b/apps/mobile/src/lib/glanceable/scope.test.ts index 60b4c8bf81f..7fca3e99644 100644 --- a/apps/mobile/src/lib/glanceable/scope.test.ts +++ b/apps/mobile/src/lib/glanceable/scope.test.ts @@ -25,13 +25,6 @@ describe('glanceable scope reads', () => { expect(mocks.getItemAsync).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); }); - it('resolves a stored Personal selection to the personal scope', async () => { - mocks.getItemAsync.mockResolvedValue('personal'); - - // A stored Personal choice resolves to the personal scope for every reader. - await expect(getSelectedOrganizationId()).resolves.toBeNull(); - }); - it('reads the active user from its storage key', async () => { mocks.getItemAsync.mockResolvedValue('u1'); diff --git a/apps/mobile/src/lib/glanceable/scope.ts b/apps/mobile/src/lib/glanceable/scope.ts index 2be2430570b..4de63c7cd2d 100644 --- a/apps/mobile/src/lib/glanceable/scope.ts +++ b/apps/mobile/src/lib/glanceable/scope.ts @@ -1,5 +1,4 @@ import { readStoredValueSafe } from '@/lib/auth/secure-store-value'; -import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -24,7 +23,8 @@ import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys * A missing hint only matches a personal scope; it cannot revive an org scope. */ export async function getSelectedOrganizationId(): Promise { - return decodeSelection(await readStoredValueSafe(ORGANIZATION_STORAGE_KEY)); + const organizationId = await readStoredValueSafe(ORGANIZATION_STORAGE_KEY); + return organizationId; } /** diff --git a/apps/mobile/src/lib/glanceable/widget-actions.ts b/apps/mobile/src/lib/glanceable/widget-actions.ts index 02f8753ac5b..3d9d571b378 100644 --- a/apps/mobile/src/lib/glanceable/widget-actions.ts +++ b/apps/mobile/src/lib/glanceable/widget-actions.ts @@ -9,7 +9,6 @@ import { buildActiveSessionsTrayInput, isAttentionStatus } from '@/lib/active-se import { readStoredValue } from '@/lib/auth/secure-store-value'; import { dismissNeedsInputNotification } from '@/lib/needs-input-notification'; import { ackSessionAttention } from '@/lib/session-attention'; -import { decodeSelection } from '@/lib/organization-selection'; import { ACTIVE_USER_ID_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { reportSecureStoreFailure } from '@/lib/telemetry/secure-store-events'; import { trpcClient } from '@/lib/trpc'; @@ -173,7 +172,7 @@ async function readStoredScope(): Promise { readStoredValueReported(ORGANIZATION_STORAGE_KEY), readStoredValueReported(ACTIVE_USER_ID_KEY), ]); - return { organizationId: decodeSelection(organizationId), userId: userId ?? null }; + return { organizationId: organizationId ?? null, userId: userId ?? null }; } /** diff --git a/apps/mobile/src/lib/organization-context.mounted.test.tsx b/apps/mobile/src/lib/organization-context.mounted.test.tsx index 17a41a6803a..3e1117dde57 100644 --- a/apps/mobile/src/lib/organization-context.mounted.test.tsx +++ b/apps/mobile/src/lib/organization-context.mounted.test.tsx @@ -215,37 +215,38 @@ describe('OrganizationProvider default organization', () => { expect(scope().organizationId).toBe('org-a'); }); - it('stores the explicit Personal selection when Personal is chosen while already auto-resolved', async () => { + it('writes the explicit Personal marker when Personal is chosen while already auto-resolved', async () => { list.mockResolvedValue([]); await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); // Auto-resolved Personal from an empty list is not an explicit choice, so - // nothing is stored yet. - expect(savedMetadata.has(ORG_KEY)).toBe(false); + // no marker is written yet. + expect(savedMetadata.has(MARKER_KEY)).toBe(false); await act(() => { scope().setOrganizationId(null); }); - // Selecting Personal explicitly records the choice even though the - // published id does not change, so the next launch resolves to Personal. - await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal'); - expect(savedMetadata.has(MARKER_KEY)).toBe(false); + // Selecting Personal explicitly must record the choice even though the + // published id did not change, or a later list with organizations would + // move the person back to one on the next launch. + await waitFor(() => savedMetadata.get(MARKER_KEY) === 'personal'); + expect(savedMetadata.has(ORG_KEY)).toBe(false); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); }); - it('keeps the stored Personal selection across a re-mount with organizations available', async () => { - savedMetadata.set(ORG_KEY, 'personal'); + it('keeps an explicit Personal marker across a re-mount with organizations available', async () => { + savedMetadata.set(MARKER_KEY, 'personal'); list.mockResolvedValue([orgA]); const first = await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); - // An explicit Personal choice is not overridden by the default. - expect(savedMetadata.get(ORG_KEY)).toBe('personal'); + // An explicit Personal choice is not overridden by the default, so the + // organization key stays absent for every other scope reader too. + expect(savedMetadata.has(ORG_KEY)).toBe(false); unmount(first); await mount(); await waitFor(() => scope().isLoaded); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); - expect(savedMetadata.get(ORG_KEY)).toBe('personal'); }); it('publishes nothing stale when a sign-out lands during the list fetch', async () => { @@ -398,7 +399,7 @@ describe('OrganizationProvider restoration fencing', () => { names.resolve([orgA]); }); expect(scope()).toMatchObject({ organizationId: null, isLoaded: true }); - expect(savedMetadata.get(ORG_KEY)).toBe('personal'); + expect(savedMetadata.get(MARKER_KEY)).toBe('personal'); }); }); @@ -415,7 +416,7 @@ describe('OrganizationProvider persistence', () => { expect(scope()).toMatchObject({ organizationId: 'org-a', error: null }); }); - it('stores the Personal choice as the organization value', async () => { + it('deletes the organization key and writes the Personal marker', async () => { savedMetadata.set(ORG_KEY, 'org-a'); await mount(); await waitFor(() => scope().isLoaded); @@ -423,8 +424,8 @@ describe('OrganizationProvider persistence', () => { scope().setOrganizationId(null); }); await waitFor(() => !scope().isSaving); - expect(savedMetadata.get(ORG_KEY)).toBe('personal'); - expect(savedMetadata.has(MARKER_KEY)).toBe(false); + expect(savedMetadata.has(ORG_KEY)).toBe(false); + expect(savedMetadata.get(MARKER_KEY)).toBe('personal'); expect(scope()).toMatchObject({ organizationId: null, error: null }); }); @@ -464,72 +465,4 @@ describe('OrganizationProvider persistence', () => { await waitFor(() => !scope().isSaving); expect(scope()).toMatchObject({ organizationId: 'org-a', error: null }); }); - - it('applies saves in call order and stores the last selection', async () => { - const firstWrite = Promise.withResolvers(); - const started: string[] = []; - storage.write.mockImplementation(async (key: string, value: string) => { - started.push(value); - if (started.length === 1) { - await firstWrite.promise; - } - savedMetadata.set(key, value); - await Promise.resolve(); - }); - await mount(); - await waitFor(() => scope().isLoaded); - await act(() => { - scope().setOrganizationId('org-a'); - }); - await act(() => { - scope().setOrganizationId('org-b'); - }); - // Saves run in call order: the second starts only after the first settles. - expect(started).toEqual(['org-a']); - await act(() => { - firstWrite.resolve(undefined); - }); - await waitFor(() => savedMetadata.get(ORG_KEY) === 'org-b' && !scope().isSaving); - expect(started).toEqual(['org-a', 'org-b']); - expect(scope()).toMatchObject({ organizationId: 'org-b', isLoaded: true, error: null }); - expect(savedMetadata.has(MARKER_KEY)).toBe(false); - }); - - it('stores the Personal choice when two selections are made back to back', async () => { - const firstWrite = Promise.withResolvers(); - const started: string[] = []; - storage.write.mockImplementation(async (key: string, value: string) => { - started.push(value); - if (started.length === 1) { - await firstWrite.promise; - } - savedMetadata.set(key, value); - await Promise.resolve(); - }); - await mount(); - await waitFor(() => scope().isLoaded); - await act(() => { - scope().setOrganizationId('org-a'); - }); - await act(() => { - scope().setOrganizationId(null); - }); - expect(started).toEqual(['org-a']); - await act(() => { - firstWrite.resolve(undefined); - }); - await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal' && !scope().isSaving); - expect(started).toEqual(['org-a', 'personal']); - expect(scope()).toMatchObject({ organizationId: null, isLoaded: true, error: null }); - expect(savedMetadata.has(MARKER_KEY)).toBe(false); - }); - - it('records a legacy Personal choice as the selection value', async () => { - savedMetadata.set(MARKER_KEY, 'personal'); - await mount(); - await waitFor(() => scope().isLoaded); - await waitFor(() => savedMetadata.get(ORG_KEY) === 'personal'); - expect(scope()).toMatchObject({ organizationId: null, error: null }); - expect(savedMetadata.has(MARKER_KEY)).toBe(false); - }); }); diff --git a/apps/mobile/src/lib/organization-context.test.ts b/apps/mobile/src/lib/organization-context.test.ts index 57d4da4f00e..013c2d34d1d 100644 --- a/apps/mobile/src/lib/organization-context.test.ts +++ b/apps/mobile/src/lib/organization-context.test.ts @@ -147,7 +147,61 @@ describe('OrganizationProvider.setOrganizationId', () => { unmount(); }); - it('stores the Personal choice as the selection value', async () => { + it('lands each selection after the previous one settled', async () => { + const stored = new Map(); + const gates: (() => void)[] = []; + /** Parks one storage call until the test releases it. */ + function nextGate(): Promise { + const { promise, resolve } = Promise.withResolvers(); + gates.push(resolve); + return promise; + } + // Every storage call parks until released, so the test decides when each + // write completes: the first selection completes last. + hoisted.setAccountMetadata.mockImplementation(async (key: string, value: string) => { + await nextGate(); + stored.set(key, value); + }); + hoisted.deleteAccountMetadata.mockImplementation(async (key: string) => { + await nextGate(); + stored.delete(key); + }); + + const { getCtx, unmount } = await mountProvider(); + + await act(() => { + getCtx().setOrganizationId('org-a'); + }); + await act(() => { + getCtx().setOrganizationId('org-b'); + }); + + // One selection at a time: the second one is written after the first pair. + expect(hoisted.setAccountMetadata).toHaveBeenCalledTimes(1); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'org-a'); + + // Release the writes as they arrive until both selections have landed. + for (let round = 0; round < 6 && gates.length > 0; round += 1) { + await act(async () => { + const batch = gates.splice(0); + batch.forEach(release => { + release(); + }); + await Promise.resolve(); + }); + } + + expect(stored.get(ORGANIZATION_STORAGE_KEY)).toBe('org-b'); + expect(stored.has(ORGANIZATION_PERSONAL_STORAGE_KEY)).toBe(false); + expect(hoisted.setAccountMetadata.mock.calls.at(-1)).toEqual([ + ORGANIZATION_STORAGE_KEY, + 'org-b', + ]); + + unmount(); + }); + + it('clears the persisted org, unregisters tokens, and marks Personal on the way to personal', async () => { const { getCtx, unmount } = await mountProvider(); await act(() => { @@ -157,8 +211,11 @@ describe('OrganizationProvider.setOrganizationId', () => { getCtx().setOrganizationId(null); }); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); - expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_PERSONAL_STORAGE_KEY); + expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( + ORGANIZATION_PERSONAL_STORAGE_KEY, + 'personal' + ); expect(hoisted.unregisterActivityTokensAndTombstone).toHaveBeenCalledTimes(2); expect(getCtx().organizationId).toBeNull(); @@ -179,7 +236,10 @@ describe('OrganizationProvider.setOrganizationId', () => { }); expect(getCtx().organizationId).toBeNull(); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( + ORGANIZATION_PERSONAL_STORAGE_KEY, + 'personal' + ); unmount(); }); @@ -244,7 +304,7 @@ describe('OrganizationProvider lost-membership recovery', () => { unmount(); }); - it('falls back to Personal, stores the choice, and names Personal when no membership remains', async () => { + it('falls back to Personal, writes the marker, and names Personal when no membership remains', async () => { storeSelection(); hoisted.list.mockResolvedValue([ { organizationId: 'org-lost', organizationName: 'Doomed org', role: 'member' }, @@ -255,7 +315,11 @@ describe('OrganizationProvider lost-membership recovery', () => { await waitFor(() => getCtx().organizationId === null); expect(getCtx().organizationId).toBeNull(); - expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'personal'); + expect(hoisted.deleteAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY); + expect(hoisted.setAccountMetadata).toHaveBeenCalledWith( + ORGANIZATION_PERSONAL_STORAGE_KEY, + 'personal' + ); expect(hoisted.toastWarning).toHaveBeenCalledWith( 'You no longer have access to this organization. Switched to Personal.' ); diff --git a/apps/mobile/src/lib/organization-context.tsx b/apps/mobile/src/lib/organization-context.tsx index ce9c306f426..6696a6ad7b2 100644 --- a/apps/mobile/src/lib/organization-context.tsx +++ b/apps/mobile/src/lib/organization-context.tsx @@ -24,13 +24,24 @@ import { type OrgListEntry } from '@/lib/hooks/use-organization-queries'; import { chainSave } from '@/lib/hooks/save-chain'; import { announcingToast } from '@/lib/a11y/announcing-toast'; import { i18n } from '@/i18n'; -import { decodeSelection, encodeSelection, PERSONAL_SELECTION } from '@/lib/organization-selection'; import { ORGANIZATION_PERSONAL_STORAGE_KEY, ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** - * One queue slot for the selection: every save runs through it in call order, - * so the stored value always matches the selection the provider last published. - * The stored form itself is one value; see `lib/organization-selection`. + * `ORGANIZATION_PERSONAL_STORAGE_KEY` is a SECOND key beside + * `ORGANIZATION_STORAGE_KEY`. Every other reader of that key (glanceable + * scope, widget actions, voice input, tool-summary) treats the stored value as + * an organization id, so an explicit Personal choice keeps deleting it; the + * settled "Personal was chosen" marker lives beside it. An absent organization + * key alone now means "not chosen yet", which the default rule resolves from + * the organization list. + */ +const PERSONAL_MARKER = 'personal'; + +/** + * One queue slot for the whole selection: a save runs after the previous save + * settled, so the two keys above are written as one pair and in the order the + * selections were made, and the stored pair always names the selection the + * provider publishes. */ const SELECTION_QUEUE_KEY = 'organization-selection'; @@ -92,42 +103,6 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod // the membership vanished server-side. const selectionListSeen = useRef(undefined); - const persist = useCallback(async (id: string | null) => { - generation.current += 1; - const operation = generation.current; - const epoch = currentAuthEpoch(); - setState(current => ({ ...current, isSaving: true })); - let error: 'save' | null = null; - try { - // Serialized per selection and fenced after the queue settles: only the - // selection the provider still publishes is written, in the order the - // selections were made. - await chainSave(SELECTION_QUEUE_KEY, async () => { - if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { - return; - } - await setAccountMetadata(ORGANIZATION_STORAGE_KEY, encodeSelection(id)); - if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { - return; - } - // The account stores one selection value; this legacy key is cleared so - // only that value answers which account is selected. A failed delete is - // reported by the storage layer and retried by the next selection; it - // must not turn a stored selection into a save error. - try { - await deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY); - } catch { - // Cleanup only: the selection value is already stored. - } - }); - } catch { - error = 'save'; - } - if (generation.current === operation && isCurrentAuthEpoch(epoch)) { - setState(current => ({ ...current, error, isSaving: false })); - } - }, []); - const restore = useCallback(async () => { generation.current += 1; const operation = generation.current; @@ -138,36 +113,27 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod selectionListSeen.current = undefined; setState(current => ({ ...current, token, isLoaded: false, isSaving: false, error: null })); try { - // A stored value is the whole selection: an organization id or the - // Personal choice. It is one key, so the read is already atomic and needs - // no queue; chaining it would let a slow keychain read hold up the saves - // behind it. An absent value is 'not chosen yet' unless the legacy marker - // is still present, which the fold below replaces with the value itself. - // The marker is read only when nothing is stored, so a marker-read - // failure cannot discard a stored selection, and a value a save lands - // meanwhile replaces this read through the fence below. + // A stored organization is an explicit choice; the marker beside it is an + // explicit Personal choice. An absent value is 'not chosen yet': the + // organization list decides the default and the effect below publishes it. + // A legacy absent value is treated the same way. The reads are direct: + // restore decides whether the app is loaded, so it never waits on a save. + // The two reads are kept apart: a stored organization stands alone, so a + // marker-read failure must not discard it. The marker is only consulted + // when no organization is stored, and a failure there stays a restore + // error — the selection is then genuinely undetermined. const stored = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); - const personalMarker = + const marker = stored === null ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) : null; if (generation.current === operation && isCurrentAuthEpoch(epoch)) { if (stored) { - const selected = decodeSelection(stored); - activeId.current = selected; - personalExplicit.current = selected === null; - setState({ - token, - organizationId: selected, - isLoaded: true, - isSaving: false, - error: null, - }); - } else if (personalMarker === PERSONAL_SELECTION) { + activeId.current = stored; + personalExplicit.current = false; + setState({ token, organizationId: stored, isLoaded: true, isSaving: false, error: null }); + } else if (marker === PERSONAL_MARKER) { activeId.current = null; personalExplicit.current = true; setState({ token, organizationId: null, isLoaded: true, isSaving: false, error: null }); - // Record the choice as the selection value so one value answers it - // from the next launch on. - void persist(null); } else { personalExplicit.current = false; awaitingDefault.current = true; @@ -183,7 +149,42 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod setState(current => ({ ...current, error: 'restore' })); } } - }, [token, persist]); + }, [token]); + + const persist = useCallback(async (id: string | null) => { + generation.current += 1; + const operation = generation.current; + const epoch = currentAuthEpoch(); + setState(current => ({ ...current, isSaving: true })); + let error: 'save' | null = null; + try { + // One slot per account: a save queued behind this one only starts once + // both keys of this pair have settled, and the newest selection is always + // the last one written. + await chainSave(SELECTION_QUEUE_KEY, async () => { + // Only the selection the provider still publishes is written: a newer + // selection, a sign-in, or a sign-out owns the stored state, and its + // own save (or the sign-out cleanup, whose deletes chain behind these + // keys) writes the pair this save would have written. + if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { + return; + } + // The two writes are one pair, so they are never split: once the first + // lands, the second lands too, and a newer save rewrites both. + await (id + ? setAccountMetadata(ORGANIZATION_STORAGE_KEY, id) + : deleteAccountMetadata(ORGANIZATION_STORAGE_KEY)); + await (id + ? deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY) + : setAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY, PERSONAL_MARKER)); + }); + } catch { + error = 'save'; + } + if (generation.current === operation && isCurrentAuthEpoch(epoch)) { + setState(current => ({ ...current, error, isSaving: false })); + } + }, []); // Publish the default organization once the shared list has settled. Fenced // by the generation and auth epoch captured in `restore()`, so a sign-out or diff --git a/apps/mobile/src/lib/organization-selection.ts b/apps/mobile/src/lib/organization-selection.ts deleted file mode 100644 index 85a665f185d..00000000000 --- a/apps/mobile/src/lib/organization-selection.ts +++ /dev/null @@ -1,23 +0,0 @@ -/** - * The stored organization selection. - * - * The selection is ONE SecureStore value under `ORGANIZATION_STORAGE_KEY`: an - * organization id, or {@link PERSONAL_SELECTION} for an explicit Personal - * choice. An absent value means "not chosen yet", which the organization - * context's default rule resolves from the organization list. - * - * Readers decode through {@link decodeSelection}, so the Personal choice is - * never mistaken for an organization id, and an account's selection travels as - * one value that a save writes as a unit. - */ -export const PERSONAL_SELECTION = 'personal'; - -/** The selected organization id, or null for the Personal scope. */ -export function decodeSelection(stored: string | null): string | null { - return stored === PERSONAL_SELECTION ? null : stored; -} - -/** The stored value that records `organizationId`, Personal included. */ -export function encodeSelection(organizationId: string | null): string { - return organizationId ?? PERSONAL_SELECTION; -} diff --git a/apps/mobile/src/lib/storage-keys.ts b/apps/mobile/src/lib/storage-keys.ts index 7596745d94d..d82e913b483 100644 --- a/apps/mobile/src/lib/storage-keys.ts +++ b/apps/mobile/src/lib/storage-keys.ts @@ -7,20 +7,17 @@ */ export const AUTH_TOKEN_KEY = 'auth-token'; +export const ORGANIZATION_STORAGE_KEY = 'selected-organization'; /** - * The organization selection: one value holding an organization id, or the - * Personal selection, or nothing ("not chosen yet", resolved from the - * organization list). Encode and decode through `lib/organization-selection`. + * Settled "Personal was chosen" marker, beside `ORGANIZATION_STORAGE_KEY`. + * That key keeps deleting on a Personal choice because every other reader + * treats a stored value as an organization id; this marker tells an explicit + * Personal choice apart from an absent key ("not chosen yet"), which the + * organization-context default rule resolves from the organization list. * Account-scoped selection state: deleted with the rest of the account * metadata on sign-out and on a direct account switch, so the next account on * the device resolves its own default instead of inheriting this choice. */ -export const ORGANIZATION_STORAGE_KEY = 'selected-organization'; -/** - * Legacy marker for the Personal choice, kept for upgrade and sign-out cleanup - * only. The selection value carries the Personal choice itself, and a selection - * made after an upgrade removes this key. - */ export const ORGANIZATION_PERSONAL_STORAGE_KEY = 'selected-organization-personal'; /** Filter record for the session history page. */ export const SESSION_FILTERS_KEY = 'agent-session-filters'; diff --git a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts index 01a456f5b5f..7e471dc81b9 100644 --- a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts +++ b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.test.ts @@ -115,19 +115,6 @@ describe('requestToolSummaryTranslations', () => { }); }); - it('resolves the personal scope for the translation request', async () => { - getItemAsync.mockResolvedValue('personal'); - fetchMock.mockResolvedValue(arrayContent(['ok'])); - - await requestToolSummaryTranslations(INPUT); - - expect(lastFetchCall().init.headers).toEqual({ - Authorization: 'Bearer token-1', - 'Content-Type': 'application/json', - 'X-KILOCODE-FEATURE': 'tool-summary-translation', - }); - }); - it('falls back to the raw language tag when it has no endonym', async () => { fetchMock.mockResolvedValue(arrayContent(['ok'])); diff --git a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts index 79e4e2e482f..e8088c961a3 100644 --- a/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts +++ b/apps/mobile/src/lib/tool-summary-translation/tool-summary-translation-client.ts @@ -4,7 +4,6 @@ import { isSupportedLanguage, LANGUAGE_ENDONYMS } from '@/i18n/languages'; import { readStoredValue } from '@/lib/auth/secure-store-value'; import { getAuthTokenForRequest } from '@/lib/auth/token-owner'; import { API_BASE_URL } from '@/lib/config'; -import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; /** @@ -115,7 +114,7 @@ export async function requestToolSummaryTranslations({ return nullResults(texts.length); } // The organization scope the translation is attributed to; null is personal. - const organizationId = decodeSelection(await readStoredValue(ORGANIZATION_STORAGE_KEY)); + const organizationId = await readStoredValue(ORGANIZATION_STORAGE_KEY); const headers: TranslationRequestHeaders = { Authorization: `Bearer ${token}`, diff --git a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts index 2c1856e034f..a084c9d3f94 100644 --- a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts +++ b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.test.ts @@ -255,14 +255,6 @@ describe('resolveGatewayTranscriptionModelId', () => { expect(transcriptionModels.fetchTranscriptionModels).toHaveBeenCalledWith('org-42'); }); - it('resolves the personal scope for the catalogue read', async () => { - vi.mocked(SecureStore.getItemAsync).mockResolvedValueOnce('personal'); - transcriptionModels.fetchTranscriptionModels.mockResolvedValue([]); - - await expect(resolveGatewayTranscriptionModelId()).resolves.toBeNull(); - expect(transcriptionModels.fetchTranscriptionModels).toHaveBeenCalledWith(undefined); - }); - it('falls back to the first catalogue entry and persists it when none is stored', async () => { transcriptionModels.fetchTranscriptionModels.mockResolvedValue([ { id: 'first-model', name: 'First Model' }, diff --git a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts index 411ae266c81..699ec924878 100644 --- a/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts +++ b/apps/mobile/src/lib/voice-input/gateway/native-gateway-voice-input.ts @@ -4,7 +4,6 @@ import * as SecureStore from '@/lib/auth/secure-store'; import { getAuthTokenForRequest } from '@/lib/auth/token-owner'; import { fetchTranscriptionModels } from '@/lib/hooks/use-transcription-models'; -import { decodeSelection } from '@/lib/organization-selection'; import { ORGANIZATION_STORAGE_KEY } from '@/lib/storage-keys'; import { createGatewayVoiceInputEngine, type GatewayRecorder } from './gateway-voice-input-engine'; @@ -17,6 +16,12 @@ import { const HIGH_QUALITY = RecordingPresets.HIGH_QUALITY; +/** The organization scope the voice flow reads and uploads under; null is personal. */ +async function readStoredOrganizationId(): Promise { + const value = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); + return value; +} + /** * The model the gateway engine transcribes with: the stored choice, else the * first model the gateway catalogue offers (persisted so it survives launches @@ -37,9 +42,7 @@ export async function resolveGatewayTranscriptionModelId(): Promise - decodeSelection(await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY)), + readOrganizationId: readStoredOrganizationId, deleteRecording: (uri: string) => { // `release()` frees the native object, not the file: delete the recording // so dictation does not accumulate audio on disk. The modern File API From 9a17f636a17d384f0f884a2883594e980d7d52ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:52:35 +0000 Subject: [PATCH 05/10] fix(mobile): fence organization metadata writes Keep each organization selection save ordered and verify the active selection before each account metadata write. The saved keys retain their existing format. --- apps/mobile/src/lib/organization-context.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/mobile/src/lib/organization-context.tsx b/apps/mobile/src/lib/organization-context.tsx index 6696a6ad7b2..4287ff3dc37 100644 --- a/apps/mobile/src/lib/organization-context.tsx +++ b/apps/mobile/src/lib/organization-context.tsx @@ -169,11 +169,15 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { return; } - // The two writes are one pair, so they are never split: once the first - // lands, the second lands too, and a newer save rewrites both. + // Keep the pair ordered with saves, but stop between keys if a new + // selection or auth epoch supersedes this one. Its queued save, or + // sign-out cleanup, owns the remaining storage update. await (id ? setAccountMetadata(ORGANIZATION_STORAGE_KEY, id) : deleteAccountMetadata(ORGANIZATION_STORAGE_KEY)); + if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { + return; + } await (id ? deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY) : setAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY, PERSONAL_MARKER)); From 6e184c901521b9c6bff4547da3281b1f2dafbd08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:56:04 +0000 Subject: [PATCH 06/10] fix(mobile): check active selection before metadata writes The ordered selection save checks the current generation and auth epoch before each storage write. Organization selection remains in its established key-and-marker format. --- .../src/lib/organization-context.test.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/apps/mobile/src/lib/organization-context.test.ts b/apps/mobile/src/lib/organization-context.test.ts index 013c2d34d1d..c7e58d67bbf 100644 --- a/apps/mobile/src/lib/organization-context.test.ts +++ b/apps/mobile/src/lib/organization-context.test.ts @@ -1,4 +1,5 @@ /* oxlint-disable @typescript-eslint/no-unsafe-call @typescript-eslint/no-unsafe-member-access */ +/* eslint-disable max-lines -- selection persistence and membership recovery share one provider harness */ import { type QueryClient } from '@tanstack/react-query'; import { createElement } from 'react'; import { act } from '@/test/renderer'; @@ -151,10 +152,12 @@ describe('OrganizationProvider.setOrganizationId', () => { const stored = new Map(); const gates: (() => void)[] = []; /** Parks one storage call until the test releases it. */ - function nextGate(): Promise { - const { promise, resolve } = Promise.withResolvers(); - gates.push(resolve); - return promise; + async function nextGate(): Promise { + const { promise, resolve } = Promise.withResolvers(); + gates.push(() => { + resolve(undefined); + }); + await promise; } // Every storage call parks until released, so the test decides when each // write completes: the first selection completes last. @@ -180,16 +183,20 @@ describe('OrganizationProvider.setOrganizationId', () => { expect(hoisted.setAccountMetadata).toHaveBeenCalledTimes(1); expect(hoisted.setAccountMetadata).toHaveBeenCalledWith(ORGANIZATION_STORAGE_KEY, 'org-a'); - // Release the writes as they arrive until both selections have landed. - for (let round = 0; round < 6 && gates.length > 0; round += 1) { - await act(async () => { - const batch = gates.splice(0); - batch.forEach(release => { + async function releaseNext(): Promise { + const release = gates.shift(); + if (release) { + await act(async () => { release(); + await Promise.resolve(); }); - await Promise.resolve(); - }); + } } + await releaseNext(); + await releaseNext(); + await releaseNext(); + await releaseNext(); + await releaseNext(); expect(stored.get(ORGANIZATION_STORAGE_KEY)).toBe('org-b'); expect(stored.has(ORGANIZATION_PERSONAL_STORAGE_KEY)).toBe(false); From 67c27d67d3938e585842d49bb07a526a39c90071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:09:11 +0000 Subject: [PATCH 07/10] ci(mobile): run native build jobs independently Allow each platform build request to run independently instead of sharing a native-hash concurrency group. The existing artifact check still reuses a published build. Update the workflow assertion for the expanded job condition. --- .github/workflows/mobile-native-build.yml | 134 ++++++++++++++++++++-- dev/local/mobile-native-gate.test.ts | 4 +- 2 files changed, 128 insertions(+), 10 deletions(-) diff --git a/.github/workflows/mobile-native-build.yml b/.github/workflows/mobile-native-build.yml index 942e9b4db6b..86ff7f2829c 100644 --- a/.github/workflows/mobile-native-build.yml +++ b/.github/workflows/mobile-native-build.yml @@ -9,6 +9,15 @@ # workflow_dispatch. The gate job skips platforms whose artifact already # exists, so a PR whose nativeHash is already published costs one ubuntu gate # job and no macOS runner. +# +# A completed artifact is keyed on the native inputs. Platform jobs check for +# the matching artifact before building, allowing each workflow invocation to +# proceed independently while reusing artifacts already published. +# +# A run that must publish the durable artifact (push, workflow_dispatch) but +# finds only the one-day pull_request artifact promotes it: the promote job +# copies the artifact under the same name with the durable retention. The +# promote jobs run independently; duplicate copies contain the same bytes. name: mobile-native-build on: @@ -65,6 +74,12 @@ jobs: android_hash: ${{ steps.decide.outputs.android_hash }} need_ios: ${{ steps.decide.outputs.need_ios }} need_android: ${{ steps.decide.outputs.need_android }} + promote_ios: ${{ steps.decide.outputs.promote_ios }} + promote_android: ${{ steps.decide.outputs.promote_android }} + promote_ios_artifact_id: ${{ steps.decide.outputs.promote_ios_artifact_id }} + promote_android_artifact_id: ${{ steps.decide.outputs.promote_android_artifact_id }} + promote_ios_name: ${{ steps.decide.outputs.promote_ios_name }} + promote_android_name: ${{ steps.decide.outputs.promote_android_name }} prefix: ${{ steps.decide.outputs.prefix }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -95,20 +110,88 @@ jobs: android_hash=$(pnpm exec tsx dev/local/mobile-native-hash.ts android) echo "ios_hash=$ios_hash" >> "$GITHUB_OUTPUT" echo "android_hash=$android_hash" >> "$GITHUB_OUTPUT" - # dev/local/mobile-native-gate.ts decides the prefix and which - # platforms still need building. A pull_request run that edits this + # dev/local/mobile-native-gate.ts decides the prefix, which platforms + # still need building, and which are copied from the one-day + # pull_request artifact instead. A pull_request run that edits this # workflow is validating that edit, so it publishes under # pr-- and hosts never install it; a pull_request # run whose native input changed publishes the plain - # mobile-native-- name a host installs, and - # skips a platform whose artifact already exists. + # mobile-native-- name a host installs. pnpm exec tsx dev/local/mobile-native-gate.ts "$ios_hash" "$android_hash" - ios: + # A run that must publish the durable artifact, but finds only the one-day + # pull_request one, copies it instead of rebuilding. The copy carries the + # same name, so hosts and the gate look it up exactly as they look up a + # build. + # + # No job-level concurrency here on purpose. A cancelled promote reports + # `result == 'cancelled'`, which the build jobs read as "not success" and + # answer with a full build — the cost this job exists to remove. A duplicate + # promote is a one-minute re-upload of the same bytes. + promote_ios: needs: gate - if: needs.gate.outputs.need_ios == 'true' + if: needs.gate.outputs.promote_ios == 'true' + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Download the short-lived artifact + env: + GH_TOKEN: ${{ github.token }} + ARTIFACT_ID: ${{ needs.gate.outputs.promote_ios_artifact_id }} + run: | + set -euo pipefail + gh api "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID/zip" \ + > "$RUNNER_TEMP/artifact.zip" + mkdir -p "$RUNNER_TEMP/unpacked" + unzip -q "$RUNNER_TEMP/artifact.zip" -d "$RUNNER_TEMP/unpacked" + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ${{ needs.gate.outputs.promote_ios_name }} + path: ${{ runner.temp }}/unpacked + compression-level: 0 + retention-days: 90 + if-no-files-found: error + + promote_android: + needs: gate + if: needs.gate.outputs.promote_android == 'true' + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Download the short-lived artifact + env: + GH_TOKEN: ${{ github.token }} + ARTIFACT_ID: ${{ needs.gate.outputs.promote_android_artifact_id }} + run: | + set -euo pipefail + gh api "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID/zip" \ + > "$RUNNER_TEMP/artifact.zip" + mkdir -p "$RUNNER_TEMP/unpacked" + unzip -q "$RUNNER_TEMP/artifact.zip" -d "$RUNNER_TEMP/unpacked" + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ${{ needs.gate.outputs.promote_android_name }} + path: ${{ runner.temp }}/unpacked + compression-level: 0 + retention-days: 90 + if-no-files-found: error + + ios: + needs: [gate, promote_ios] + # Build when nothing is published, or when the promote of the short-lived + # artifact failed: a failed promote must not leave the hash without a + # durable build. `always()` keeps this job alive when the promote job was + # skipped, which is the normal case. + if: | + always() && !cancelled() && + needs.gate.result == 'success' && + (needs.gate.outputs.need_ios == 'true' || + (needs.gate.outputs.promote_ios == 'true' && needs.promote_ios.result != 'success')) runs-on: macos-26 timeout-minutes: 90 + # The artifact check below lets this invocation reuse a published build. steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -126,10 +209,21 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # The concurrency group above serializes this hash, so the build that + # held the slot may have published the artifact while this job queued. + - name: Check whether another run published this artifact + id: published + env: + GH_TOKEN: ${{ github.token }} + ARTIFACT: ${{ needs.gate.outputs.prefix }}mobile-native-ios-${{ needs.gate.outputs.ios_hash }} + run: pnpm exec tsx dev/local/mobile-native-gate.ts --published "$ARTIFACT" + - name: Build trpc types + if: steps.published.outputs.published != 'true' run: pnpm --filter @kilocode/trpc run build - name: Build harness SDK + if: steps.published.outputs.published != 'true' run: pnpm --filter @kilocode/harness-sdk run build # A consumer computes the hash on macOS; the gate computed it on @@ -137,6 +231,7 @@ jobs: # so fail loudly instead. Android is checked here too: it builds on # Linux but is consumed on macOS, and nothing else compares the two. - name: Verify the hashes are platform-independent + if: steps.published.outputs.published != 'true' run: | set -euo pipefail check() { @@ -150,9 +245,11 @@ jobs: check android "${{ needs.gate.outputs.android_hash }}" - name: Expo prebuild + if: steps.published.outputs.published != 'true' run: CI=1 pnpm --filter kilo-app exec expo prebuild --platform ios - name: Build + if: steps.published.outputs.published != 'true' run: | set -euo pipefail xcodebuild \ @@ -169,6 +266,7 @@ jobs: # simulator SDK so a consumer refuses a binary its target simulator # runtime cannot launch (see dev/local/mobile-remote-native.ts). - name: Package + if: steps.published.outputs.published != 'true' run: | set -euo pipefail products="$RUNNER_TEMP/DerivedData/Build/Products/Debug-iphonesimulator" @@ -180,6 +278,7 @@ jobs: -czf "ios-${{ needs.gate.outputs.ios_hash }}.tar.gz" Kilo.app toolchain.json - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: steps.published.outputs.published != 'true' with: name: ${{ needs.gate.outputs.prefix }}mobile-native-ios-${{ needs.gate.outputs.ios_hash }} path: ios-${{ needs.gate.outputs.ios_hash }}.tar.gz @@ -188,10 +287,15 @@ jobs: if-no-files-found: error android: - needs: gate - if: needs.gate.outputs.need_android == 'true' + needs: [gate, promote_android] + if: | + always() && !cancelled() && + needs.gate.result == 'success' && + (needs.gate.outputs.need_android == 'true' || + (needs.gate.outputs.promote_android == 'true' && needs.promote_android.result != 'success')) runs-on: ubuntu-latest timeout-minutes: 60 + # The artifact check below lets this invocation reuse a published build. steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -215,16 +319,29 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # The concurrency group above serializes this hash, so the build that + # held the slot may have published the artifact while this job queued. + - name: Check whether another run published this artifact + id: published + env: + GH_TOKEN: ${{ github.token }} + ARTIFACT: ${{ needs.gate.outputs.prefix }}mobile-native-android-${{ needs.gate.outputs.android_hash }} + run: pnpm exec tsx dev/local/mobile-native-gate.ts --published "$ARTIFACT" + - name: Build trpc types + if: steps.published.outputs.published != 'true' run: pnpm --filter @kilocode/trpc run build - name: Build harness SDK + if: steps.published.outputs.published != 'true' run: pnpm --filter @kilocode/harness-sdk run build - name: Expo prebuild + if: steps.published.outputs.published != 'true' run: CI=1 pnpm --filter kilo-app exec expo prebuild --platform android - name: Build + if: steps.published.outputs.published != 'true' run: | set -euo pipefail cd apps/mobile/android @@ -232,6 +349,7 @@ jobs: cp app/build/outputs/apk/debug/app-debug.apk "$RUNNER_TEMP/Kilo.apk" - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: steps.published.outputs.published != 'true' with: name: ${{ needs.gate.outputs.prefix }}mobile-native-android-${{ needs.gate.outputs.android_hash }} path: ${{ runner.temp }}/Kilo.apk diff --git a/dev/local/mobile-native-gate.test.ts b/dev/local/mobile-native-gate.test.ts index bbd4a794577..2162e0c3780 100644 --- a/dev/local/mobile-native-gate.test.ts +++ b/dev/local/mobile-native-gate.test.ts @@ -113,8 +113,8 @@ test('the gate reads the pull request changed files', () => { test('a platform is skipped by the gate output it publishes', () => { const workflow = readWorkflow(); - assert.equal(workflow.jobs.ios.if, "needs.gate.outputs.need_ios == 'true'"); - assert.equal(workflow.jobs.android.if, "needs.gate.outputs.need_android == 'true'"); + assert.match(workflow.jobs.ios.if ?? '', /needs\.gate\.outputs\.need_ios == 'true'/); + assert.match(workflow.jobs.android.if ?? '', /needs\.gate\.outputs\.need_android == 'true'/); const iosUpload = workflow.jobs.ios.steps.find(item => (item.uses ?? '').startsWith('actions/upload-artifact') ); From 5ccdea9685dd41da2c765c8b0d96a47dbb06c0f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:43:58 +0000 Subject: [PATCH 08/10] docs(ci): describe the artifact re-check without a concurrency slot The heavy jobs check for an already published artifact right before they build, and concurrent runs of one nativeHash may build at once. The gate script header, the check's comment, and its test now say that instead of describing a job-level concurrency group that no longer exists. --- dev/local/mobile-native-gate.test.ts | 8 ++++---- dev/local/mobile-native-gate.ts | 21 ++++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/dev/local/mobile-native-gate.test.ts b/dev/local/mobile-native-gate.test.ts index 2b244659324..c410e06e0be 100644 --- a/dev/local/mobile-native-gate.test.ts +++ b/dev/local/mobile-native-gate.test.ts @@ -625,10 +625,10 @@ test('the gate CLI keeps an edited workflow out of the installed namespace', () assert.doesNotMatch(ghLog, /artifacts\?name=mobile-native-ios-ioshash/); }); -// The heavy jobs re-check their own artifact once the hash's concurrency slot -// frees. The check must agree with the gate: a pull_request job may take the -// one-day artifact it would publish itself, a push job must not. -test('the published check reports the durable artifact to a waiting job', () => { +// The heavy jobs re-check their own artifact before building. The check must +// agree with the gate: a pull_request job may take the one-day artifact it +// would publish itself, a push job must not. +test('the published check reports the durable artifact to a job that is about to build', () => { const { outputs } = runGateCli({ eventName: 'push', published: 'mobile-native-ios-ioshash', diff --git a/dev/local/mobile-native-gate.ts b/dev/local/mobile-native-gate.ts index 37a3adb5508..8c49de23087 100644 --- a/dev/local/mobile-native-gate.ts +++ b/dev/local/mobile-native-gate.ts @@ -16,10 +16,12 @@ // durable retention) instead of spending a runner on a second build. // // A finished artifact is the only thing the gate can see, and one native build -// takes longer than the gap between two pushes of the same native inputs, so -// the workflow also serializes the jobs of one nativeHash behind a job-level -// concurrency group and each heavy job re-checks its artifact (`--published`) -// before it builds. +// takes longer than the gap between two pushes of the same native inputs, so a +// later run can start before an earlier one publishes. Each heavy job therefore +// re-checks its own artifact (`--published`) immediately before it builds. That +// re-check is best-effort deduplication, not serialization: two runs of one +// nativeHash may build at the same time, and the cost of a missed skip is a +// duplicate build publishing the same bytes, never a hash left without one. import { execFileSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import fs from 'node:fs'; @@ -208,11 +210,12 @@ function lookupOrMissing(repository: string, name: string): ArtifactLookup { } } -// A heavy job re-checks its own artifact once its concurrency slot is free: -// the run that held the slot may have published it, and a second build of the -// same nativeHash is the waste this workflow exists to avoid. A pull_request -// run accepts the short-lived artifact it would publish itself; anything else -// must find the durable one. +// A heavy job re-checks its own artifact immediately before it builds: another +// run of the same nativeHash may have published it while this one was starting, +// and a second build of it is the waste this check avoids. Concurrent builds +// are accepted, so this is a skip when the artifact is already there, not a +// wait for other runs. A pull_request run accepts the short-lived artifact it +// would publish itself; anything else must find the durable one. function isPublished(name: string): boolean { const repository = process.env.GITHUB_REPOSITORY ?? ''; const found = lookupOrMissing(repository, name); From 256b7c02f84a5c187f89b6d4d639a7991c7c1508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:08:32 +0000 Subject: [PATCH 09/10] revert(ci): restore native build workflow and gate to main The organization selection fix does not need a workflow change. --- .github/workflows/mobile-native-build.yml | 45 ++++++++++++++++++----- dev/local/mobile-native-gate.test.ts | 25 +++++++++---- dev/local/mobile-native-gate.ts | 21 +++++------ 3 files changed, 61 insertions(+), 30 deletions(-) diff --git a/.github/workflows/mobile-native-build.yml b/.github/workflows/mobile-native-build.yml index 531ce3f3913..5e6703cfbee 100644 --- a/.github/workflows/mobile-native-build.yml +++ b/.github/workflows/mobile-native-build.yml @@ -10,14 +10,22 @@ # exists, so a PR whose nativeHash is already published costs one ubuntu gate # job and no macOS runner. # -# A completed artifact is keyed on the native inputs. Platform jobs check for -# the matching artifact before building, allowing each workflow invocation to -# proceed independently while reusing artifacts already published. -# -# A run that must publish the durable artifact (push, workflow_dispatch) but -# finds only the one-day pull_request artifact promotes it: the promote job -# copies the artifact under the same name with the durable retention. The -# promote jobs run independently; duplicate copies contain the same bytes. +# A finished artifact is the only thing the gate can see, and one native build +# takes longer than the gap between two pushes of the same native inputs. +# Two mechanisms keep that from multiplying the work: +# - Each heavy job takes a job-level concurrency slot keyed on the nativeHash +# and the event, and re-checks its artifact (`mobile-native-gate.ts +# --published`) before it builds. A run that arrives while the first build of +# that hash is running waits for the slot, then exits in seconds instead of +# spending a second runner. The event name is part of the key so a push build +# is never the pending job that a later pull_request job cancels, which would +# leave main without the durable artifact. +# - A run that must publish the durable artifact (push, workflow_dispatch) but +# finds only the one-day pull_request artifact promotes it: the promote job +# copies the artifact under the same name with the durable retention, which +# costs a minute on ubuntu instead of a second full build. The promote jobs +# take no concurrency slot on purpose: a cancelled promote reads as "not +# success" and would answer with the full build it exists to avoid. name: mobile-native-build on: @@ -191,7 +199,15 @@ jobs: (needs.gate.outputs.promote_ios == 'true' && needs.promote_ios.result != 'success')) runs-on: macos-26 timeout-minutes: 90 - # The artifact check below lets this invocation reuse a published build. + # Serialized on the nativeHash so a sibling build of the same inputs makes + # this job wait and then exit at the re-check. The event name is part of the + # key so a push build is never the pending job that a later pull_request + # job cancels: main would then keep no durable artifact for that hash. The + # measured duplicates were all pull_request runs, and those still share one + # group. + concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-ios-${{ needs.gate.outputs.ios_hash }} + cancel-in-progress: false steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -209,6 +225,8 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # The concurrency group above serializes this hash, so the build that + # held the slot may have published the artifact while this job queued. - name: Check whether another run published this artifact id: published env: @@ -293,7 +311,12 @@ jobs: (needs.gate.outputs.promote_android == 'true' && needs.promote_android.result != 'success')) runs-on: ubuntu-latest timeout-minutes: 60 - # The artifact check below lets this invocation reuse a published build. + # Same serialization as the ios job: pull_request runs of one nativeHash + # queue behind each other, and a push build keeps its own group so a later + # pull_request job cannot cancel it while it waits. + concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-android-${{ needs.gate.outputs.android_hash }} + cancel-in-progress: false steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -317,6 +340,8 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # The concurrency group above serializes this hash, so the build that + # held the slot may have published the artifact while this job queued. - name: Check whether another run published this artifact id: published env: diff --git a/dev/local/mobile-native-gate.test.ts b/dev/local/mobile-native-gate.test.ts index c410e06e0be..89229c4b0b8 100644 --- a/dev/local/mobile-native-gate.test.ts +++ b/dev/local/mobile-native-gate.test.ts @@ -151,12 +151,21 @@ test('the gate job exposes the promote outputs the workflow reads', () => { } }); -// Each platform job checks its matching artifact before running build steps. -test('platform jobs run independently and re-check their artifacts', () => { +// One native build takes longer than the gap between two pushes of the same +// native inputs, so the gate's artifact lookup cannot see the build in +// flight. The job-level slot plus the re-check is what stops the duplicate. +// The slot is keyed on the event as well: a push build must not be the pending +// job that a later pull_request job cancels, or main keeps no durable artifact. +test('each heavy job is serialized on its nativeHash and re-checks its artifact', () => { const workflow = readWorkflow(); for (const platform of ['ios', 'android'] as const) { const job = workflow.jobs[platform]; - assert.equal(job.concurrency, undefined); + assert.deepEqual(job.concurrency, { + group: + `\${{ github.workflow }}-\${{ github.event_name }}-${platform}` + + `-\${{ needs.gate.outputs.${platform}_hash }}`, + 'cancel-in-progress': false, + }); const published = step(job, 'published'); assert.match( published.run ?? '', @@ -168,7 +177,7 @@ test('platform jobs run independently and re-check their artifacts', () => { assert.equal( later.if, "steps.published.outputs.published != 'true'", - `${platform}: build steps must skip when an artifact is available` + `${platform}: every step after the re-check must skip when another run published` ); } } @@ -625,10 +634,10 @@ test('the gate CLI keeps an edited workflow out of the installed namespace', () assert.doesNotMatch(ghLog, /artifacts\?name=mobile-native-ios-ioshash/); }); -// The heavy jobs re-check their own artifact before building. The check must -// agree with the gate: a pull_request job may take the one-day artifact it -// would publish itself, a push job must not. -test('the published check reports the durable artifact to a job that is about to build', () => { +// The heavy jobs re-check their own artifact once the hash's concurrency slot +// frees. The check must agree with the gate: a pull_request job may take the +// one-day artifact it would publish itself, a push job must not. +test('the published check reports the durable artifact to a waiting job', () => { const { outputs } = runGateCli({ eventName: 'push', published: 'mobile-native-ios-ioshash', diff --git a/dev/local/mobile-native-gate.ts b/dev/local/mobile-native-gate.ts index 8c49de23087..37a3adb5508 100644 --- a/dev/local/mobile-native-gate.ts +++ b/dev/local/mobile-native-gate.ts @@ -16,12 +16,10 @@ // durable retention) instead of spending a runner on a second build. // // A finished artifact is the only thing the gate can see, and one native build -// takes longer than the gap between two pushes of the same native inputs, so a -// later run can start before an earlier one publishes. Each heavy job therefore -// re-checks its own artifact (`--published`) immediately before it builds. That -// re-check is best-effort deduplication, not serialization: two runs of one -// nativeHash may build at the same time, and the cost of a missed skip is a -// duplicate build publishing the same bytes, never a hash left without one. +// takes longer than the gap between two pushes of the same native inputs, so +// the workflow also serializes the jobs of one nativeHash behind a job-level +// concurrency group and each heavy job re-checks its artifact (`--published`) +// before it builds. import { execFileSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import fs from 'node:fs'; @@ -210,12 +208,11 @@ function lookupOrMissing(repository: string, name: string): ArtifactLookup { } } -// A heavy job re-checks its own artifact immediately before it builds: another -// run of the same nativeHash may have published it while this one was starting, -// and a second build of it is the waste this check avoids. Concurrent builds -// are accepted, so this is a skip when the artifact is already there, not a -// wait for other runs. A pull_request run accepts the short-lived artifact it -// would publish itself; anything else must find the durable one. +// A heavy job re-checks its own artifact once its concurrency slot is free: +// the run that held the slot may have published it, and a second build of the +// same nativeHash is the waste this workflow exists to avoid. A pull_request +// run accepts the short-lived artifact it would publish itself; anything else +// must find the durable one. function isPublished(name: string): boolean { const repository = process.env.GITHUB_REPOSITORY ?? ''; const found = lookupOrMissing(repository, name); From 015dcd60830903a613e1198a1fe786f9a8f1532a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sat, 24 Oct 2026 22:21:05 +0000 Subject: [PATCH 10/10] fix(mobile): order selection saves independently of restores A selection save now carries its own order, so a restore that runs for the same account while a save is pending leaves that save to finish and reads the pair it wrote. Saves stay ordered among themselves and stay fenced by the auth epoch. --- .../src/lib/organization-context.test.ts | 140 +++++++++++++++++- apps/mobile/src/lib/organization-context.tsx | 84 +++++++---- 2 files changed, 193 insertions(+), 31 deletions(-) diff --git a/apps/mobile/src/lib/organization-context.test.ts b/apps/mobile/src/lib/organization-context.test.ts index c7e58d67bbf..263fb9463a5 100644 --- a/apps/mobile/src/lib/organization-context.test.ts +++ b/apps/mobile/src/lib/organization-context.test.ts @@ -1,7 +1,7 @@ /* oxlint-disable @typescript-eslint/no-unsafe-call @typescript-eslint/no-unsafe-member-access */ /* eslint-disable max-lines -- selection persistence and membership recovery share one provider harness */ import { type QueryClient } from '@tanstack/react-query'; -import { createElement } from 'react'; +import { type ComponentType, createElement, type ReactNode, useState } from 'react'; import { act } from '@/test/renderer'; import { renderWithProviders, waitFor } from '@/test/render-with-providers'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -66,7 +66,22 @@ type OrganizationContextValue = { setOrganizationId: (id: string | null) => void; }; -async function mountProvider(): Promise<{ +/** + * Hands the provider a new token while the tree stays mounted, the way an + * access-token rotation does: the same account and no auth-epoch bump. + */ +const tokenRotation: { set: (token: string) => void } = { set: () => undefined }; + +function RotatingTokenAuth({ children }: { children: ReactNode }): ReactNode { + const [token, setToken] = useState('t'); + tokenRotation.set = setToken; + hoisted.useAuth.mockReturnValue({ token }); + return createElement(OrganizationProvider, null, children); +} + +async function mountProvider( + wrapper: ComponentType<{ children: ReactNode }> = OrganizationProvider +): Promise<{ getCtx: () => OrganizationContextValue; queryClient: QueryClient; unmount: () => void; @@ -77,7 +92,7 @@ async function mountProvider(): Promise<{ return null; } - const ui = await renderWithProviders(createElement(Consumer), { wrapper: OrganizationProvider }); + const ui = await renderWithProviders(createElement(Consumer), { wrapper }); // Let the provider's restore read settle. The list may still be pending, so // this only guarantees the read decided whether a default resolution is owed. await act(async () => { @@ -250,6 +265,125 @@ describe('OrganizationProvider.setOrganizationId', () => { unmount(); }); + + /** + * Storage double backed by one map whose calls park on demand, so a test can + * rotate the token while a save is in flight and then release it. + */ + function installStorageDouble(): { + stored: Map; + park: () => void; + releaseAll: () => Promise; + } { + const stored = new Map(); + const gates: (() => void)[] = []; + let parked = false; + async function waitWhileParked(): Promise { + if (!parked) { + return; + } + const { promise, resolve } = Promise.withResolvers(); + gates.push(() => { + resolve(undefined); + }); + await promise; + } + hoisted.getItemAsync.mockImplementation(async (key: string) => { + await Promise.resolve(); + return stored.get(key) ?? null; + }); + hoisted.setAccountMetadata.mockImplementation(async (key: string, value: string) => { + await waitWhileParked(); + stored.set(key, value); + }); + hoisted.deleteAccountMetadata.mockImplementation(async (key: string) => { + await waitWhileParked(); + stored.delete(key); + }); + return { + stored, + park: () => { + parked = true; + }, + releaseAll: async () => { + parked = false; + // Every parked call is released at once; callers await the observable + // state with `waitFor` rather than a fixed delay. + await act(async () => { + for (const release of gates.splice(0)) { + release(); + } + await Promise.resolve(); + }); + }, + }; + } + + /** Rotates the access token without a sign-out, as the refresh path does. */ + async function rotateToken(): Promise { + await act(async () => { + tokenRotation.set('t2'); + await Promise.resolve(); + }); + } + + it('keeps a fresh Personal choice when the token rotates while its save is in flight', async () => { + const storage = installStorageDouble(); + hoisted.list.mockResolvedValue([ + { organizationId: 'org-a', organizationName: 'A org', role: 'member' }, + ]); + const { getCtx, unmount } = await mountProvider(RotatingTokenAuth); + // The default organization is the stored selection before the switch. + await waitFor(() => storage.stored.get(ORGANIZATION_STORAGE_KEY) === 'org-a'); + expect(getCtx().organizationId).toBe('org-a'); + + // The explicit Personal choice parks on its first write, and a rotation + // lands while that save is still in flight. + storage.park(); + await act(() => { + getCtx().setOrganizationId(null); + }); + await rotateToken(); + await storage.releaseAll(); + + // The pair the user chose is the one stored, and the restore publishes it + // instead of falling back to the default organization. + await waitFor(() => storage.stored.get(ORGANIZATION_PERSONAL_STORAGE_KEY) === 'personal'); + expect(storage.stored.has(ORGANIZATION_STORAGE_KEY)).toBe(false); + await waitFor(() => getCtx().organizationId === null); + expect(getCtx().organizationId).toBeNull(); + + unmount(); + }); + + it('lands the last of two queued selections when the token rotates', async () => { + const storage = installStorageDouble(); + hoisted.list.mockResolvedValue([ + { organizationId: 'org-a', organizationName: 'A org', role: 'member' }, + { organizationId: 'org-b', organizationName: 'B org', role: 'member' }, + { organizationId: 'org-c', organizationName: 'C org', role: 'member' }, + ]); + const { getCtx, unmount } = await mountProvider(RotatingTokenAuth); + await waitFor(() => storage.stored.get(ORGANIZATION_STORAGE_KEY) === 'org-a'); + + storage.park(); + await act(() => { + getCtx().setOrganizationId('org-b'); + }); + await act(() => { + getCtx().setOrganizationId('org-c'); + }); + expect(getCtx().organizationId).toBe('org-c'); + await rotateToken(); + await storage.releaseAll(); + + await waitFor(() => storage.stored.get(ORGANIZATION_STORAGE_KEY) === 'org-c'); + expect(storage.stored.has(ORGANIZATION_PERSONAL_STORAGE_KEY)).toBe(false); + await waitFor(() => getCtx().organizationId === 'org-c'); + expect(getCtx().organizationId).toBe('org-c'); + + unmount(); + }); }); describe('OrganizationProvider lost-membership recovery', () => { diff --git a/apps/mobile/src/lib/organization-context.tsx b/apps/mobile/src/lib/organization-context.tsx index 4287ff3dc37..b385f933218 100644 --- a/apps/mobile/src/lib/organization-context.tsx +++ b/apps/mobile/src/lib/organization-context.tsx @@ -82,6 +82,14 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod error: null, }); const generation = useRef(0); + // Orders the stored pair's writers: a newer selection, a sign-out, or an + // account switch supersedes an in-flight save. `restore` deliberately does + // not bump it: a token rotation re-runs `restore` for the same account, and + // that must not cancel the save of a selection the user just made. + const saveGeneration = useRef(0); + // Settles when the last scheduled save has settled. A restore reads the pair + // a save that began before it left behind, instead of a half-written one. + const savesSettled = useRef>(Promise.resolve()); const activeId = useRef(null); // Set by `restore` when nothing explicit is stored: the organization list // owns the publish then. A ref (not state) so `setOrganizationId` clears it @@ -116,12 +124,15 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod // A stored organization is an explicit choice; the marker beside it is an // explicit Personal choice. An absent value is 'not chosen yet': the // organization list decides the default and the effect below publishes it. - // A legacy absent value is treated the same way. The reads are direct: - // restore decides whether the app is loaded, so it never waits on a save. + // A legacy absent value is treated the same way. A save that began before + // this restore owns the pair until it settles, so it is awaited first; a + // save that begins later supersedes this restore (it bumps `generation`), + // so the reads below are discarded rather than published half-written. // The two reads are kept apart: a stored organization stands alone, so a // marker-read failure must not discard it. The marker is only consulted // when no organization is stored, and a failure there stays a restore // error — the selection is then genuinely undetermined. + await savesSettled.current; const stored = await SecureStore.getItemAsync(ORGANIZATION_STORAGE_KEY); const marker = stored === null ? await SecureStore.getItemAsync(ORGANIZATION_PERSONAL_STORAGE_KEY) : null; @@ -152,40 +163,52 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod }, [token]); const persist = useCallback(async (id: string | null) => { + // A selection supersedes any restore still deciding what to publish. generation.current += 1; - const operation = generation.current; + saveGeneration.current += 1; + const save = saveGeneration.current; const epoch = currentAuthEpoch(); setState(current => ({ ...current, isSaving: true })); let error: 'save' | null = null; + // Registered before the first await, so a restore that starts now waits for + // this whole pair: it reads what the save leaves behind, never a key of it. + const writes = chainSave(SELECTION_QUEUE_KEY, async () => { + // Only the selection the provider still publishes is written: a newer + // selection, a sign-in, or a sign-out owns the stored state, and its + // own save (or the sign-out cleanup, whose deletes chain behind these + // keys) writes the pair this save would have written. A restore does + // not own the pair: it reads it after this save settles. + if (saveGeneration.current !== save || !isCurrentAuthEpoch(epoch)) { + return; + } + // Keep the pair ordered with saves, but stop between keys if a new + // selection or auth epoch supersedes this one. Its queued save, or + // sign-out cleanup, owns the remaining storage update. + await (id + ? setAccountMetadata(ORGANIZATION_STORAGE_KEY, id) + : deleteAccountMetadata(ORGANIZATION_STORAGE_KEY)); + if (saveGeneration.current !== save || !isCurrentAuthEpoch(epoch)) { + return; + } + await (id + ? deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY) + : setAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY, PERSONAL_MARKER)); + }); + // A restore only needs the pair to have settled, so its own rejection is + // swallowed here; this save reports the failure in its state below. + savesSettled.current = (async () => { + try { + await writes; + } catch { + // Reported by this save's own `error` state. + } + })(); try { - // One slot per account: a save queued behind this one only starts once - // both keys of this pair have settled, and the newest selection is always - // the last one written. - await chainSave(SELECTION_QUEUE_KEY, async () => { - // Only the selection the provider still publishes is written: a newer - // selection, a sign-in, or a sign-out owns the stored state, and its - // own save (or the sign-out cleanup, whose deletes chain behind these - // keys) writes the pair this save would have written. - if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { - return; - } - // Keep the pair ordered with saves, but stop between keys if a new - // selection or auth epoch supersedes this one. Its queued save, or - // sign-out cleanup, owns the remaining storage update. - await (id - ? setAccountMetadata(ORGANIZATION_STORAGE_KEY, id) - : deleteAccountMetadata(ORGANIZATION_STORAGE_KEY)); - if (generation.current !== operation || !isCurrentAuthEpoch(epoch)) { - return; - } - await (id - ? deleteAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY) - : setAccountMetadata(ORGANIZATION_PERSONAL_STORAGE_KEY, PERSONAL_MARKER)); - }); + await writes; } catch { error = 'save'; } - if (generation.current === operation && isCurrentAuthEpoch(epoch)) { + if (saveGeneration.current === save && isCurrentAuthEpoch(epoch)) { setState(current => ({ ...current, error, isSaving: false })); } }, []); @@ -296,6 +319,9 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod void restore(); } else { generation.current += 1; + // A signed-out account owns no stored selection: a save still in flight + // must not write the pair after the sign-out cleanup deleted it. + saveGeneration.current += 1; activeId.current = null; awaitingDefault.current = false; defaultFence.current = null; @@ -305,6 +331,8 @@ export function OrganizationProvider({ children }: { readonly children: ReactNod setState({ token, organizationId: null, isLoaded: true, isSaving: false, error: null }); } return () => { + // A token change cancels the previous restore's publication, not a save: + // a rotation is the same account, and its save must still land. generation.current += 1; }; }, [token, restore]);