From aa8d573b349b31bbb7ff139d281620080ad9c6a8 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Thu, 10 Sep 2026 17:03:00 -0400 Subject: [PATCH 1/3] fix(mobile): keep the logged-in screens mounted through an account switch A switch flaps config.loggedIn false and back to true. The mobile root stack followed it, so every switch swapped to the logged-out stack, back, and then remounted the navigator, three native rebuilds in about 130 ms. RNS logged unbalanced appearance transitions, and could leave the torn-down navigator's screens on top. Every touch was then dropped and the app looked frozen. It also sometimes logged an unhandled POP for the root 'loggedIn' screen. Hold the mobile logged-in screens through a switch that started logged in (showLoggedInScreens). A switch that starts logged out, e.g. a notification tap on the login screen, keeps the logged-out screens until it lands. Desktop keeps its loggedIn || userSwitching gate. Holding the logged-in screens means userSwitching must clear whenever a switch ends without the remount: - login() now clears it when it cancels one of its own prompts, and when it fails without an RPCError. Otherwise the app stayed on the old account's screens with reset stores. - The provisioning hand-off clears it, then pushes 'username' through the new navigateAppendOnceRootHas once the logged-out stack has mounted. A push dispatched before then was dropped. - When a switch started by a notification tap ends, the push store drops that tap's pending notification. A successful switch has already consumed it. Left behind, it would re-run the failed switch on the next account-list refresh. A notification parked for an account that isn't configured yet is left alone. - A switch that lands on the navigator that's already mounted (same account, or the first switch after launching logged out) gets no remount and so no onReady. useUserSwitchNavKey now ends it when the arriving username is the switch's recorded target. Matching the target, not just "no remount", keeps a stale username mid-switch from ending a switch still in flight. Before ending it, the hook marks the mounted navigator ready for the account: a logout's store reset clears navigation readiness and only onReady restored it, so after a re-login without a remount every deep link and notification intent stayed queued. --- .../navigate-append-once-root-has.test.ts | 85 +++++++++++++++ shared/constants/router.tsx | 27 +++++ .../account-switch-header-avatar.native.tsx | 2 +- shared/router-v2/account-switch.test.tsx | 22 ++++ shared/router-v2/account-switch.tsx | 12 ++ shared/router-v2/account-switcher/index.tsx | 2 +- shared/router-v2/router.tsx | 6 +- shared/router-v2/tab-bar.desktop.tsx | 2 +- .../use-user-switch-nav-key.test.tsx | 103 ++++++++++++++++++ shared/router-v2/use-user-switch-nav-key.tsx | 25 +++++ shared/stores/config.tsx | 33 +++++- shared/stores/push.tsx | 31 +++++- shared/stores/tests/as-mobile.ts | 6 + shared/stores/tests/config.test.ts | 101 +++++++++++++++++ shared/stores/tests/push.test.ts | 56 ++++++++++ 15 files changed, 496 insertions(+), 17 deletions(-) create mode 100644 shared/constants/navigate-append-once-root-has.test.ts create mode 100644 shared/stores/tests/as-mobile.ts create mode 100644 shared/stores/tests/push.test.ts diff --git a/shared/constants/navigate-append-once-root-has.test.ts b/shared/constants/navigate-append-once-root-has.test.ts new file mode 100644 index 000000000000..5a6a5cd1df06 --- /dev/null +++ b/shared/constants/navigate-append-once-root-has.test.ts @@ -0,0 +1,85 @@ +/// +import {navigateAppendOnceRootHas, navigationRef} from '@/constants/router' + +const dispatch = jest.fn() +const listeners = new Set<() => void>() +let rootState: unknown + +const loggedIn = {key: 'loggedIn-1', name: 'loggedIn'} +const loggedOut = { + key: 'loggedOut-1', + name: 'loggedOut', + state: {index: 0, key: 'loggedOutStack-1', routes: [{key: 'login-1', name: 'login'}], type: 'stack'}, +} + +const setRootRoutes = (routes: Array) => { + rootState = {index: routes.length - 1, key: 'root-1', routeNames: [], routes, stale: false, type: 'stack'} +} +const emitState = () => { + for (const l of [...listeners]) { + l() + } +} + +beforeEach(() => { + dispatch.mockReset() + listeners.clear() + // the jest mock's container ref is a plain object, so stub its methods directly + const nr = navigationRef as unknown as Record + nr['current'] = {} + nr['dispatch'] = dispatch + nr['getRootState'] = () => rootState + nr['isReady'] = () => true + nr['addListener'] = (_: string, cb: () => void) => { + listeners.add(cb) + return () => listeners.delete(cb) + } +}) + +afterEach(() => { + jest.useRealTimers() +}) + +// Each test pushes distinct params: navigateAppend's module-private `_pendingAppend` dupe cache +// would otherwise swallow a same-shaped push from an earlier test. +const pushOf = (username: string) => + expect.objectContaining({payload: {name: 'username', params: {username}}, type: 'PUSH'}) + +test('pushes right away when the root already has the route', () => { + setRootRoutes([loggedOut]) + + navigateAppendOnceRootHas('loggedOut', {name: 'username', params: {username: 'testuser-a'}} as never) + + expect(dispatch).toHaveBeenCalledTimes(1) + expect(dispatch).toHaveBeenCalledWith(pushOf('testuser-a')) +}) + +test('waits for the root route to mount, then pushes once', () => { + setRootRoutes([loggedIn]) + + navigateAppendOnceRootHas('loggedOut', {name: 'username', params: {username: 'testuser-b'}} as never) + expect(dispatch).not.toHaveBeenCalled() + + emitState() + expect(dispatch).not.toHaveBeenCalled() + + setRootRoutes([loggedOut]) + emitState() + expect(dispatch).toHaveBeenCalledTimes(1) + expect(dispatch).toHaveBeenCalledWith(pushOf('testuser-b')) + + emitState() + expect(dispatch).toHaveBeenCalledTimes(1) +}) + +test('gives up if the root route does not mount before the timeout', () => { + jest.useFakeTimers() + setRootRoutes([loggedIn]) + + navigateAppendOnceRootHas('loggedOut', {name: 'username', params: {username: 'testuser-c'}} as never, 5000) + jest.advanceTimersByTime(5000) + + setRootRoutes([loggedOut]) + emitState() + expect(dispatch).not.toHaveBeenCalled() +}) diff --git a/shared/constants/router.tsx b/shared/constants/router.tsx index 583a2edb7505..a06bcf764bbb 100644 --- a/shared/constants/router.tsx +++ b/shared/constants/router.tsx @@ -452,6 +452,33 @@ export function navigateAppend(path: NavigateAppendType, replace?: boolean): boo return true } +// Push once the root stack has a `rootRouteName` route. For a push whose target lives in a +// conditional root group that a store change is about to mount (e.g. the logged-out stack): a push +// dispatched before the group mounts reaches no navigator that can handle it and is dropped. Gives +// up after `timeoutMs` so a group that never mounts can't fire the push at some unrelated later time. +export const navigateAppendOnceRootHas = ( + rootRouteName: string, + path: NavigateAppendType, + timeoutMs = 5000 +) => { + const rootHas = () => getRootState()?.routes?.some(r => r.name === rootRouteName) ?? false + if (rootHas()) { + navigateAppend(path) + return + } + const n = _getNavigator() + if (!n) { + return + } + const timer = setTimeout(() => unsub(), timeoutMs) + const unsub = n.addListener('state', () => { + if (!rootHas()) return + clearTimeout(timer) + unsub() + navigateAppend(path) + }) +} + export const switchTab = (name: Tabs.AppTab) => { if (DEBUG_NAV) { console.log('[Nav] switchTab', {name}) diff --git a/shared/router-v2/account-switch-header-avatar.native.tsx b/shared/router-v2/account-switch-header-avatar.native.tsx index b12f24b89bc1..cae606b7058f 100644 --- a/shared/router-v2/account-switch-header-avatar.native.tsx +++ b/shared/router-v2/account-switch-header-avatar.native.tsx @@ -32,7 +32,7 @@ const AccountSwitchHeaderAvatar = () => { handledLongPressRef.current = true C.ignorePromise(Haptics.selectionAsync()) rememberAccountSwitchTab(username, recentAccount.username, C.Router2.getTab()) - setUserSwitching(true) + setUserSwitching(true, recentAccount.username) login(recentAccount.username, '') } diff --git a/shared/router-v2/account-switch.test.tsx b/shared/router-v2/account-switch.test.tsx index bd35eb979d10..0768364aa956 100644 --- a/shared/router-v2/account-switch.test.tsx +++ b/shared/router-v2/account-switch.test.tsx @@ -6,6 +6,7 @@ import { consumePendingAccountSwitchTab, getMostRecentlyUsedAccount, rememberAccountSwitchTab, + showLoggedInScreens, } from './account-switch' const account = (username: string, hasStoredSecret = true) => ({ @@ -73,3 +74,24 @@ describe('pending account-switch tab', () => { expect(consumePendingAccountSwitchTab('bob')).toBeUndefined() }) }) + +describe('showLoggedInScreens', () => { + const state = (loggedIn: boolean, userSwitching = false, userSwitchingFromLoggedIn = false) => ({ + loggedIn, + userSwitching, + userSwitchingFromLoggedIn, + }) + + test('follows loggedIn when no switch is running', () => { + expect(showLoggedInScreens(state(true))).toBe(true) + expect(showLoggedInScreens(state(false))).toBe(false) + }) + + test('holds the logged-in screens through the loggedIn flap of a switch that started logged in', () => { + expect(showLoggedInScreens(state(false, true, true))).toBe(true) + }) + + test('keeps the logged-out screens for a switch that started logged out', () => { + expect(showLoggedInScreens(state(false, true, false))).toBe(false) + }) +}) diff --git a/shared/router-v2/account-switch.tsx b/shared/router-v2/account-switch.tsx index 5568af4a57f9..6339d3b44ead 100644 --- a/shared/router-v2/account-switch.tsx +++ b/shared/router-v2/account-switch.tsx @@ -37,6 +37,18 @@ export const consumePendingAccountSwitchTab = (currentUsername: string) => { return pending.tab } +// Whether the root navigator shows the logged-in screens. A switch that starts while logged in flaps +// loggedIn false and back between the service's loggedOut and loggedIn notifications. Following +// that would swap the native root stack to loggedOut and back right before the navKey remount, and +// that churn leaves RNS screens from the unmounted navigator on screen, swallowing every touch. So +// hold the logged-in screens through such a switch. A switch that starts logged out (e.g. a +// notification tap on the login screen) keeps the logged-out screens until it lands. +export const showLoggedInScreens = (s: { + loggedIn: boolean + userSwitching: boolean + userSwitchingFromLoggedIn: boolean +}) => s.loggedIn || (s.userSwitching && s.userSwitchingFromLoggedIn) + export const clearPendingAccountSwitch = (currentUsername: string) => { if (pendingAccountSwitch?.targetUsername !== currentUsername) { pendingAccountSwitch = undefined diff --git a/shared/router-v2/account-switcher/index.tsx b/shared/router-v2/account-switcher/index.tsx index fd8a03d5fa25..d99a9974bd38 100644 --- a/shared/router-v2/account-switcher/index.tsx +++ b/shared/router-v2/account-switcher/index.tsx @@ -36,7 +36,7 @@ const AccountSwitcher = (p: {onSelected?: () => void}) => { if (isMobile) { rememberAccountSwitchTab(you, username, C.Router2.getTab()) } - setUserSwitching(true) + setUserSwitching(true, username) login(username, '') } diff --git a/shared/router-v2/router.tsx b/shared/router-v2/router.tsx index cb9916196462..bb00269359e2 100644 --- a/shared/router-v2/router.tsx +++ b/shared/router-v2/router.tsx @@ -32,7 +32,7 @@ import {createBottomTabNavigator} from '@react-navigation/bottom-tabs' import {isLiquidGlassSupported as _isLiquidGlassSupported} from '@callstack/liquid-glass' import {Platform, StatusBar, View} from 'react-native' import AccountSwitchHeaderAvatar from './account-switch-header-avatar' -import {clearPendingAccountSwitch, consumePendingAccountSwitchTab} from './account-switch' +import {clearPendingAccountSwitch, consumePendingAccountSwitchTab, showLoggedInScreens} from './account-switch' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' const isLiquidGlassSupported = isMobile ? (_isLiquidGlassSupported as boolean) : false @@ -604,8 +604,8 @@ if (isMobile) { } } - const useIsLoggedInNative = () => useConfigState(s => s.loggedIn) - const useIsLoggedOutNative = () => !useConfigState(s => s.loggedIn) + const useIsLoggedInNative = () => useConfigState(showLoggedInScreens) + const useIsLoggedOutNative = () => !useConfigState(showLoggedInScreens) const nativeModalScreensConfig = routeMapToStaticScreens(modalRoutes, makeLayout, true, false, false) const nativePhoneRootScreensConfig = routeMapToStaticScreens( diff --git a/shared/router-v2/tab-bar.desktop.tsx b/shared/router-v2/tab-bar.desktop.tsx index 4a3592c7adcf..8a5d71f9d2eb 100644 --- a/shared/router-v2/tab-bar.desktop.tsx +++ b/shared/router-v2/tab-bar.desktop.tsx @@ -265,7 +265,7 @@ function Tab(props: TabProps) { const accountRows = useConfigState.getState().configuredAccounts const row = accountRows.find(a => a.username !== current && a.hasStoredSecret) if (row) { - setUserSwitching(true) + setUserSwitching(true, row.username) login(row.username, '') } else { onSelectTab(tab) diff --git a/shared/router-v2/use-user-switch-nav-key.test.tsx b/shared/router-v2/use-user-switch-nav-key.test.tsx index 9acc547b40d7..4ba36b04e610 100644 --- a/shared/router-v2/use-user-switch-nav-key.test.tsx +++ b/shared/router-v2/use-user-switch-nav-key.test.tsx @@ -1,10 +1,23 @@ /** @jest-environment jsdom */ /// import {act, cleanup, renderHook} from '@testing-library/react' +import {navigationRef} from '@/constants/router' +import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' +import {useNavigationIntentsState} from '@/stores/navigation-intents' import {resetAllStores} from '@/util/zustand' import {useUserSwitchNavKey} from './use-user-switch-nav-key' +beforeEach(() => { + // the jest mock's container ref is a plain object, so stub the method the hook reads + ;(navigationRef as unknown as Record)['isReady'] = () => true +}) + +const readiness = () => { + const {navigationReady, navigationReadyForUid} = useNavigationIntentsState.getState() + return {navigationReady, navigationReadyForUid} +} + const setUsername = (username: string) => { act(() => { useCurrentUserState @@ -13,8 +26,24 @@ const setUsername = (username: string) => { }) } +const startSwitchTo = (username: string) => { + act(() => { + useConfigState.getState().dispatch.setUserSwitching(true, username) + }) +} + +// setLoggedIn(false) between the service's loggedOut and loggedIn notifications, and a logout, +// both run resetAllStores(), which blanks the current user +const blankCurrentUser = () => { + act(() => { + resetAllStores() + }) +} + afterEach(() => { cleanup() + // config's resetState carries the switch across resets, so end it explicitly + useConfigState.getState().dispatch.setUserSwitching(false) resetAllStores() }) @@ -50,3 +79,77 @@ test('an account switch that blanks username mid-flight still changes the nav ke setUsername('testuser-mac') expect(result.current).toBe('testuser-mac') }) + +test('a switch that lands back on the account the navigator shows ends the switch', () => { + setUsername('testuser') + const {result} = renderHook(() => useUserSwitchNavKey()) + blankCurrentUser() + startSwitchTo('testuser') + + setUsername('testuser') + + expect(result.current).toBe('') + expect(useConfigState.getState().userSwitching).toBe(false) +}) + +test('a first switch after starting logged out ends when its account arrives', () => { + const {result} = renderHook(() => useUserSwitchNavKey()) + startSwitchTo('testuser') + + setUsername('testuser') + + expect(result.current).toBe('') + expect(useConfigState.getState().userSwitching).toBe(false) +}) + +test('a stale username mid-switch does not end the switch, and the remount leaves it for onReady', () => { + setUsername('testuser') + const {result} = renderHook(() => useUserSwitchNavKey()) + startSwitchTo('testuser-mac') + blankCurrentUser() + + setUsername('testuser') + expect(result.current).toBe('') + expect(useConfigState.getState().userSwitching).toBe(true) + + setUsername('testuser-mac') + expect(result.current).toBe('testuser-mac') + expect(useConfigState.getState().userSwitching).toBe(true) +}) + +test('logging back in on the mounted navigator restores navigation readiness for that account', () => { + setUsername('testuser') + renderHook(() => useUserSwitchNavKey()) + // a logout's store reset clears readiness + blankCurrentUser() + expect(readiness().navigationReady).toBe(false) + + setUsername('testuser') + + expect(readiness()).toEqual({navigationReady: true, navigationReadyForUid: 'testuser'}) +}) + +test('a switch that lands on the mounted navigator ends only after readiness is back', () => { + setUsername('testuser') + renderHook(() => useUserSwitchNavKey()) + blankCurrentUser() + startSwitchTo('testuser') + let readyWhenSwitchEnded: boolean | undefined + const unsub = useConfigState.subscribe((s, p) => { + if (p.userSwitching && !s.userSwitching) { + readyWhenSwitchEnded = useNavigationIntentsState.getState().navigationReady + } + }) + + setUsername('testuser') + unsub() + + expect(readyWhenSwitchEnded).toBe(true) +}) + +test('the first render leaves navigation readiness to onReady', () => { + setUsername('testuser') + renderHook(() => useUserSwitchNavKey()) + + expect(readiness().navigationReady).toBe(false) +}) diff --git a/shared/router-v2/use-user-switch-nav-key.tsx b/shared/router-v2/use-user-switch-nav-key.tsx index 2b0cb8ed41ab..46877f6d817c 100644 --- a/shared/router-v2/use-user-switch-nav-key.tsx +++ b/shared/router-v2/use-user-switch-nav-key.tsx @@ -1,20 +1,45 @@ import * as React from 'react' +import {navigationRef} from '@/constants/router' +import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' +import {useNavigationIntentsState} from '@/stores/navigation-intents' // Remount the navigator when switching between two logged-in users. // A switch arrives as 'a' → '' → 'b' because the mid-switch setLoggedIn(false) // resets all stores, so only ever compare against the last non-empty username. // Ignore '' → username (initial login) so in-flight unbox requests aren't interrupted. +// +// A remount's onReady marks navigation ready for the new account and ends the switch. A user who +// arrives without a remount gets no onReady, so this hook does both: +// - After a logout or the mid-switch reset, which clear navigation readiness, the mounted navigator +// now serves the arriving account, so mark it ready. Otherwise every deep link and notification +// intent stays queued. +// - End a switch that landed on the mounted navigator (e.g. logged out, then a notification tap for +// that same account), after readiness so the intent it replays can run. Match the switch's target +// rather than just "no remount": a stale username mid-switch must not end a switch still in flight. export const useUserSwitchNavKey = () => { const username = useCurrentUserState(s => s.username) const [navKey, setNavKey] = React.useState('') const prevUsernameRef = React.useRef(username) + const lastSeenUsernameRef = React.useRef(username) React.useEffect(() => { + const cameFromBlank = !lastSeenUsernameRef.current + lastSeenUsernameRef.current = username if (!username) return const prev = prevUsernameRef.current prevUsernameRef.current = username if (prev && prev !== username) { setNavKey(username) + return + } + if (cameFromBlank && navigationRef.isReady()) { + useNavigationIntentsState + .getState() + .dispatch.setNavigationReady(true, useCurrentUserState.getState().uid) + } + const {dispatch, userSwitching, userSwitchingTo} = useConfigState.getState() + if (userSwitching && userSwitchingTo === username) { + dispatch.setUserSwitching(false) } }, [username]) return navKey diff --git a/shared/stores/config.tsx b/shared/stores/config.tsx index f0ab46999359..c8e49dd7ecbf 100644 --- a/shared/stores/config.tsx +++ b/shared/stores/config.tsx @@ -10,7 +10,7 @@ import type {Tab} from '@/constants/tabs' import {RPCError, convertToError, isErrorTransient, niceError} from '@/util/errors' import {type CommonResponseHandler} from '@/engine/types' import {invalidPasswordErrorString} from '@/constants/config' -import {navigateAppend} from '@/constants/router' +import {navigateAppendOnceRootHas} from '@/constants/router' import {onEngineConnected as onEngineConnectedInPlatform} from '@/util/storeless-actions' type Store = T.Immutable<{ @@ -50,6 +50,10 @@ type Store = T.Immutable<{ tab?: Tab } userSwitching: boolean + // The account an in-progress switch is logging into ('' when none or not known) + userSwitchingTo: string + // Whether the in-progress switch started while logged in + userSwitchingFromLoggedIn: boolean windowShownCount: Map }> @@ -88,6 +92,8 @@ const initialStore: Store = { loaded: false, }, userSwitching: false, + userSwitchingFromLoggedIn: false, + userSwitchingTo: '', windowShownCount: new Map(), } @@ -121,7 +127,7 @@ export type State = Store & { setStartupDetails: (st: Omit) => void setOutOfDate: (outOfDate: T.Config.OutOfDate) => void setUpdating: () => void - setUserSwitching: (sw: boolean) => void + setUserSwitching: (sw: boolean, to?: string) => void toggleRuntimeStats: () => void updateGregorCategory: (category: string, body: string, dtime?: {offset: number; time: number}) => void } @@ -238,8 +244,14 @@ export const useConfigState = Z.createZustand('config', (set, get) => { 'keybase.1.provisionUi.DisplayAndPromptSecret': cancelOnCallback, 'keybase.1.provisionUi.PromptNewDeviceName': (_, response) => { cancelOnCallback(undefined, response) - // this account needs provisioning; hand off to the provision flow - navigateAppend({name: 'username', params: {autoSubmit: true, username}}) + // This account needs provisioning; hand off to the provision flow. 'username' lives in + // the logged-out stack, which the routers keep unmounted while userSwitching is set, so + // end the switch and push once that stack is up. + get().dispatch.setUserSwitching(false) + navigateAppendOnceRootHas('loggedOut', { + name: 'username', + params: {autoSubmit: true, username}, + }) }, 'keybase.1.provisionUi.chooseDevice': cancelOnCallback, 'keybase.1.provisionUi.chooseGPGMethod': cancelOnCallback, @@ -282,15 +294,20 @@ export const useConfigState = Z.createZustand('config', (set, get) => { logger.info('login call succeeded') get().dispatch.setLoggedIn(true) } catch (error) { + // The routers keep the logged-in screens mounted while userSwitching is set, so a switch + // that ends here has to clear it or the logged-out screens can't mount. if (!(error instanceof RPCError)) { + get().dispatch.setUserSwitching(false) return } if (error.code === T.RPCGen.StatusCode.scalreadyloggedin) { get().dispatch.setLoggedIn(true) } else if (error.desc !== cancelDesc) { - // If we're canceling then ignore the error error.desc = niceError(error) get().dispatch.setLoginError(error) + } else { + // We cancelled one of our own prompts: not an error to show, but the switch is over. + get().dispatch.setUserSwitching(false) } } } @@ -463,6 +480,8 @@ export const useConfigState = Z.createZustand('config', (set, get) => { dispatch: s.dispatch, startup: {loaded: s.startup.loaded}, userSwitching: s.userSwitching, + userSwitchingFromLoggedIn: s.userSwitchingFromLoggedIn, + userSwitchingTo: s.userSwitchingTo, })) }, revoke: (name, wasCurrentDevice) => { @@ -579,9 +598,11 @@ export const useConfigState = Z.createZustand('config', (set, get) => { s.outOfDate.updating = true }) }, - setUserSwitching: sw => { + setUserSwitching: (sw, to) => { set(s => { s.userSwitching = sw + s.userSwitchingFromLoggedIn = sw && s.loggedIn + s.userSwitchingTo = sw ? (to ?? '') : '' }) }, toggleRuntimeStats: () => { diff --git a/shared/stores/push.tsx b/shared/stores/push.tsx index fc0b539289b6..1e1a04534b90 100644 --- a/shared/stores/push.tsx +++ b/shared/stores/push.tsx @@ -58,6 +58,10 @@ const mobileInitialStore: Store = { const initialStore: Store = isMobile ? mobileInitialStore : desktopInitialStore +// The account a notification tap is switching to, so that when the switch ends only that tap's +// pending notification is dropped (see the config subscription at the bottom of this file). +let pushSwitchForUid: string | undefined + export const usePushState = Z.createZustand('push', (set, get) => { if (!isMobile) { const dispatch: State['dispatch'] = { @@ -226,7 +230,8 @@ export const usePushState = Z.createZustand('push', (set, get) => { return } logger.info('[Push] switching to account for notification tap') - configDispatch.setUserSwitching(true) + pushSwitchForUid = forUid + configDispatch.setUserSwitching(true, account.username) set(s => { s.pendingPushNotification = notification }) @@ -432,9 +437,17 @@ export const usePushState = Z.createZustand('push', (set, get) => { } }) -// A login error used to clear the pending push notification via a direct call -// from config's setLoginError. Subscribing here instead keeps config from -// importing push (breaks the config <-> push require cycle). +// Drop the pending push notification when the login or account switch it was waiting on +// ends without reaching its account: +// - a login error; +// - a switch started by a notification tap that ends with that tap's notification still +// pending. A successful switch consumes it (push-listener replays it when the uid changes, +// before the router clears userSwitching), so if it's still there the switch never landed, +// e.g. its login cancelled its own prompt. Left pending, the account-list replay in +// push-listener would re-run that switch the next time the accounts refresh. A notification +// parked for an account that isn't configured yet belongs to no switch, so it's kept. +// Subscribing here instead of calling from config keeps config from importing push (breaks +// the config <-> push require cycle). // // Guard against HMR: the config store instance (and its subscribers) survive // hot reloads via Z.createZustand's registry, but this module re-evaluates, so @@ -444,7 +457,15 @@ const _g = globalThis as any if (!__DEV__ || !_g.__pushLoginErrorSubscribed) { if (__DEV__) _g.__pushLoginErrorSubscribed = true useConfigState.subscribe((s, p) => { - if (s.loginError && s.loginError !== p.loginError) { + const loginFailed = !!s.loginError && s.loginError !== p.loginError + let endedSwitchForUid: string | undefined + if (p.userSwitching && !s.userSwitching) { + endedSwitchForUid = pushSwitchForUid + pushSwitchForUid = undefined + } + const pending = usePushState.getState().pendingPushNotification + const pendingForUid = pending && 'forUid' in pending ? pending.forUid : undefined + if (loginFailed || (!!endedSwitchForUid && pendingForUid === endedSwitchForUid)) { usePushState.getState().dispatch.clearPendingPushNotification() } }) diff --git a/shared/stores/tests/as-mobile.ts b/shared/stores/tests/as-mobile.ts new file mode 100644 index 000000000000..d6a7de404d18 --- /dev/null +++ b/shared/stores/tests/as-mobile.ts @@ -0,0 +1,6 @@ +// Import first in a test file to load modules as mobile. jest.setup.js defaults isMobile to false, +// and some stores (e.g. push) pick their platform's dispatch once, when the module loads, so +// flipping the global inside a test is too late for them. +;(globalThis as unknown as {isMobile: boolean}).isMobile = true + +export {} diff --git a/shared/stores/tests/config.test.ts b/shared/stores/tests/config.test.ts index 1df0b762284f..f9a096c51e82 100644 --- a/shared/stores/tests/config.test.ts +++ b/shared/stores/tests/config.test.ts @@ -1,4 +1,12 @@ /// +jest.mock('@/constants/router', () => ({ + ...jest.requireActual('@/constants/router'), + navigateAppendOnceRootHas: jest.fn(), +})) + +import * as T from '@/constants/types' +import {navigateAppendOnceRootHas} from '@/constants/router' +import {RPCError} from '@/util/errors' import {noConversationIDKey} from '../../constants/types/chat/common' import {useConfigState} from '../config' @@ -21,6 +29,8 @@ const resetConfigState = () => { loaded: false, }, userSwitching: false, + userSwitchingFromLoggedIn: false, + userSwitchingTo: '', } as any) dispatch.resetState() } @@ -120,3 +130,94 @@ test('custom resetState preserves the fields config intentionally carries across expect(state.userSwitching).toBe(true) expect(state.globalError).toBeUndefined() }) + +const flush = async () => new Promise(resolve => setImmediate(resolve)) + +const switchWithLoginFailure = async (failure: unknown) => { + jest.spyOn(T.RPCGen, 'loginLoginRpcListener').mockRejectedValue(failure) + const {dispatch} = useConfigState.getState() + dispatch.setUserSwitching(true) + dispatch.login('testuser', '') + await flush() +} + +describe('login ending an account switch', () => { + const mockOnceRootHas = jest.mocked(navigateAppendOnceRootHas) + + afterEach(() => { + jest.restoreAllMocks() + mockOnceRootHas.mockReset() + }) + + test('an account that needs provisioning ends the switch before handing off to username', async () => { + let switchingAtHandOff: boolean | undefined + mockOnceRootHas.mockImplementation(() => { + switchingAtHandOff = useConfigState.getState().userSwitching + }) + const cancelled = jest.fn().mockRejectedValue(new RPCError('Canceling RPC', T.RPCGen.StatusCode.scgeneric)) + jest.spyOn(T.RPCGen, 'loginLoginRpcListener').mockImplementation(listener => { + const prompt = (listener as any).customResponseIncomingCallMap['keybase.1.provisionUi.PromptNewDeviceName'] + prompt({}, {error: jest.fn(), result: jest.fn()}) + return cancelled() + }) + const {dispatch} = useConfigState.getState() + dispatch.setUserSwitching(true) + dispatch.login('testuser', '') + await flush() + + expect(mockOnceRootHas).toHaveBeenCalledWith('loggedOut', { + name: 'username', + params: {autoSubmit: true, username: 'testuser'}, + }) + expect(switchingAtHandOff).toBe(false) + }) + + test('a prompt login cancelled itself clears userSwitching without a login error', async () => { + await switchWithLoginFailure(new RPCError('Canceling RPC', T.RPCGen.StatusCode.scgeneric)) + + const state = useConfigState.getState() + expect(state.userSwitching).toBe(false) + expect(state.loginError).toBeUndefined() + }) + + test('a failure that is not an RPCError clears userSwitching', async () => { + await switchWithLoginFailure(new Error('boom')) + + expect(useConfigState.getState().userSwitching).toBe(false) + }) + + test('an RPC error clears userSwitching and records the login error', async () => { + await switchWithLoginFailure(new RPCError('bad things', T.RPCGen.StatusCode.scgeneric)) + + const state = useConfigState.getState() + expect(state.userSwitching).toBe(false) + expect(state.loginError?.desc).toBeTruthy() + }) +}) + +test("setUserSwitching records the switch's target, clears it with the flag, and keeps it across resets", () => { + const {dispatch} = useConfigState.getState() + + dispatch.setUserSwitching(true, 'testuser') + dispatch.resetState() + expect(useConfigState.getState().userSwitchingTo).toBe('testuser') + + dispatch.setUserSwitching(false) + expect(useConfigState.getState().userSwitchingTo).toBe('') +}) + +test('setUserSwitching records whether the switch started logged in, through the mid-switch reset', () => { + const {dispatch} = useConfigState.getState() + + dispatch.setUserSwitching(true, 'testuser') + expect(useConfigState.getState().userSwitchingFromLoggedIn).toBe(false) + + dispatch.setLoggedIn(true) + dispatch.setUserSwitching(true, 'testuser') + // the service's loggedOut notification during a switch resets every store + dispatch.setLoggedIn(false) + expect(useConfigState.getState().userSwitchingFromLoggedIn).toBe(true) + + dispatch.setUserSwitching(false) + expect(useConfigState.getState().userSwitchingFromLoggedIn).toBe(false) +}) diff --git a/shared/stores/tests/push.test.ts b/shared/stores/tests/push.test.ts new file mode 100644 index 000000000000..ca514b51aa01 --- /dev/null +++ b/shared/stores/tests/push.test.ts @@ -0,0 +1,56 @@ +/// +import './as-mobile' +import * as T from '@/constants/types' +import {resetAllStores} from '../../util/zustand' +import {useConfigState} from '../config' +import {usePushState} from '../push' + +const flush = async () => new Promise(resolve => setImmediate(resolve)) + +const tapFor = (forUid: string) => + ({conversationIDKey: 'conv', forUid, type: 'chat.newmessage', userInteraction: true}) as any + +afterEach(() => { + jest.restoreAllMocks() + // config's resetState carries userSwitching across resets, so clear it explicitly + useConfigState.getState().dispatch.setUserSwitching(false) + resetAllStores() +}) + +test("a switch started by a notification tap that ends with the tap still pending drops it", async () => { + // the switch's login never answers; the test ends the switch itself + jest.spyOn(T.RPCGen, 'loginLoginRpcListener').mockReturnValue(new Promise(() => {})) + const config = useConfigState.getState().dispatch + config.setAccounts([{hasStoredSecret: true, uid: 'testuser-uid', username: 'testuser'}]) + + usePushState.getState().dispatch.handlePush(tapFor('testuser-uid')) + await flush() + expect(useConfigState.getState().userSwitching).toBe(true) + expect(usePushState.getState().pendingPushNotification).toEqual(tapFor('testuser-uid')) + + config.setUserSwitching(false) + + expect(usePushState.getState().pendingPushNotification).toBeUndefined() +}) + +test('a notification parked for an account that is not configured yet survives an unrelated switch', () => { + const config = useConfigState.getState().dispatch + config.setUserSwitching(true) + usePushState.getState().dispatch.setPendingPushNotification(tapFor('testuser-mac-uid')) + + config.setUserSwitching(false) + + expect(usePushState.getState().pendingPushNotification).toEqual(tapFor('testuser-mac-uid')) +}) + +test('a pending notification survives the store reset in the middle of a switch', () => { + const config = useConfigState.getState().dispatch + config.setLoggedIn(true) + config.setUserSwitching(true) + usePushState.getState().dispatch.setPendingPushNotification(tapFor('testuser-uid')) + + // the service's loggedOut notification during a switch resets every store + config.setLoggedIn(false) + + expect(usePushState.getState().pendingPushNotification).toEqual(tapFor('testuser-uid')) +}) From 0f78ad193a92ba3352744c83692d3451d021d787 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Thu, 10 Sep 2026 17:19:17 -0400 Subject: [PATCH 2/3] fix(ios): start the glass tab bar on the right tab instead of animating to it On cold start the native tab controller selects the first tab when it gets its children, then moves to the startup tab, sliding the iOS 26 glass pill across. Patch react-native-screens to make that first selection without animation. An account switch remounts the navigator on the first tab and jumped to the remembered tab after onReady. Start the remounted navigator on that tab via the linking initial URL instead; onReady still consumes it as a fallback. --- .../patches/react-native-screens+4.27.0.patch | 27 +++++++++++++++++++ shared/router-v2/account-switch.test.tsx | 9 +++++++ shared/router-v2/account-switch.tsx | 3 +++ shared/router-v2/linking.tsx | 8 ++++++ 4 files changed, 47 insertions(+) diff --git a/shared/patches/react-native-screens+4.27.0.patch b/shared/patches/react-native-screens+4.27.0.patch index 9e51059e0884..4554f9f2e3b0 100644 --- a/shared/patches/react-native-screens+4.27.0.patch +++ b/shared/patches/react-native-screens+4.27.0.patch @@ -114,6 +114,33 @@ index add33c4..8022575 100644 } } #endif // RNS_IPHONE_OS_VERSION_AVAILABLE(26_0) +diff --git a/node_modules/react-native-screens/ios/tabs/host/RNSTabBarController.mm b/node_modules/react-native-screens/ios/tabs/host/RNSTabBarController.mm +index 06c1957..222e098 100644 +--- a/node_modules/react-native-screens/ios/tabs/host/RNSTabBarController.mm ++++ b/node_modules/react-native-screens/ios/tabs/host/RNSTabBarController.mm +@@ -307,12 +307,22 @@ - (BOOL)updateSelectedViewControllerTo:(nullable UIViewController *)nextSelected + RCTAssert(![NSString rnscreens_isBlankOrNull:screenKey], + @"[RNScreens] The screenKey MUST NOT be null if the view controller is not null"); + ++ BOOL isInitialSelection = _navigationState == nil; + [self progressNavigationState:screenKey withOrigin:actionOrigin]; + + if (currSelectedViewController == nextSelectedViewController) { + return YES; + } + ++ // setViewControllers: already selected index 0; don't slide the iOS 26 glass pill to the startup tab. ++ if (isInitialSelection) { ++ [UIView performWithoutAnimation:^{ ++ [self setSelectedViewController:nextSelectedViewController]; ++ [self.tabBar layoutIfNeeded]; ++ }]; ++ return YES; ++ } ++ + [self setSelectedViewController:nextSelectedViewController]; + return YES; + } diff --git a/node_modules/react-native-screens/ios/utils/UINavigationBar+RNSUtility.h b/node_modules/react-native-screens/ios/utils/UINavigationBar+RNSUtility.h index 0e7010d..8e3af12 100644 --- a/node_modules/react-native-screens/ios/utils/UINavigationBar+RNSUtility.h diff --git a/shared/router-v2/account-switch.test.tsx b/shared/router-v2/account-switch.test.tsx index 0768364aa956..dcfeee0ec3cc 100644 --- a/shared/router-v2/account-switch.test.tsx +++ b/shared/router-v2/account-switch.test.tsx @@ -5,6 +5,7 @@ import { clearPendingAccountSwitch, consumePendingAccountSwitchTab, getMostRecentlyUsedAccount, + peekPendingAccountSwitchTab, rememberAccountSwitchTab, showLoggedInScreens, } from './account-switch' @@ -45,6 +46,14 @@ describe('pending account-switch tab', () => { expect(consumePendingAccountSwitchTab('bob')).toBeUndefined() }) + test('peeks the remembered tab for the target account without consuming it', () => { + rememberAccountSwitchTab('alice', 'bob', Tabs.fsTab) + + expect(peekPendingAccountSwitchTab('alice')).toBeUndefined() + expect(peekPendingAccountSwitchTab('bob')).toBe(Tabs.fsTab) + expect(consumePendingAccountSwitchTab('bob')).toBe(Tabs.fsTab) + }) + test('does not consume the tab before the account changes', () => { rememberAccountSwitchTab('alice', 'bob', Tabs.fsTab) diff --git a/shared/router-v2/account-switch.tsx b/shared/router-v2/account-switch.tsx index 6339d3b44ead..8148579f141e 100644 --- a/shared/router-v2/account-switch.tsx +++ b/shared/router-v2/account-switch.tsx @@ -30,6 +30,9 @@ export const rememberAccountSwitchTab = ( : undefined } +export const peekPendingAccountSwitchTab = (currentUsername: string) => + pendingAccountSwitch?.targetUsername === currentUsername ? pendingAccountSwitch.tab : undefined + export const consumePendingAccountSwitchTab = (currentUsername: string) => { const pending = pendingAccountSwitch if (pending?.targetUsername !== currentUsername) return diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index 6f3442f00728..5e3f2835e07b 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -8,6 +8,7 @@ import {usePushState} from '@/stores/push' import type {LinkingOptions} from '@react-navigation/native' import type {RootParamList} from './route-params' import {Linking} from 'react-native' +import {peekPendingAccountSwitchTab} from './account-switch' import {emitDeepLink, normalizeUrl, setInitialURLOnce} from './deep-link-emitter' // Re-exported so existing importers ('@/router-v2/linking') keep working; the // definitions live in the dependency-free './deep-link-emitter' leaf. @@ -255,6 +256,13 @@ export const createLinkingConfig = ( const {loggedIn, startup, androidShare} = useConfigState.getState() if (!loggedIn) return null + // An account switch remounts the navigator. Start it on the switcher's tab: switching there + // after mount slides the iOS 26 glass tab pill over from the first tab. + const accountSwitchTab = peekPendingAccountSwitchTab(useCurrentUserState.getState().username) + if (accountSwitchTab) { + return setInitialURLOnce(`keybase://${accountSwitchTab}`) + } + const {tab: startupTab, followUser: startupFollowUser} = startup let startupConversation = startup.conversation if (!isValidConversationIDKey(startupConversation)) { From 43c47111d123a65cda7b6ee695dacc6ad44c92a5 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Thu, 10 Sep 2026 21:36:57 -0400 Subject: [PATCH 3/3] test(router): cover the account-switch tab in getInitialURL --- shared/router-v2/linking-initial-url.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/shared/router-v2/linking-initial-url.test.ts b/shared/router-v2/linking-initial-url.test.ts index 4930318e5da8..b84bfdc6fc8a 100644 --- a/shared/router-v2/linking-initial-url.test.ts +++ b/shared/router-v2/linking-initial-url.test.ts @@ -6,6 +6,7 @@ import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' import {usePushState} from '@/stores/push' +import {peekPendingAccountSwitchTab, rememberAccountSwitchTab} from './account-switch' import {createLinkingConfig} from './linking' const setCurrentUser = (uid: string) => { @@ -53,9 +54,26 @@ beforeEach(() => { afterEach(() => { handleAppLink.mockReset() + rememberAccountSwitchTab('', '', undefined) resetAllStores() }) +test('an account switch starts on the switcher tab without consuming it before onReady', async () => { + rememberAccountSwitchTab('testuser', 'testuser-mac', Tabs.teamsTab) + setCurrentUser('testuser-mac') + setStartup({conversation: 'conv-1', conversationUid: 'testuser-mac', tab: Tabs.chatTab}) + + await expect(getInitialURL()).resolves.toBe(`keybase://${Tabs.teamsTab}`) + expect(peekPendingAccountSwitchTab('testuser-mac')).toBe(Tabs.teamsTab) +}) + +test('a switcher tab remembered for another account does not preempt the saved route', async () => { + rememberAccountSwitchTab('current-uid', 'testuser-mac', Tabs.teamsTab) + setStartup({tab: Tabs.chatTab}) + + await expect(getInitialURL()).resolves.toBe(`keybase://${Tabs.chatTab}`) +}) + test('a logged out app has no initial url', async () => { useConfigState.getState().dispatch.setLoggedIn(false) setStartup({tab: Tabs.chatTab})