Skip to content

fix(mobile): keep the logged-in screens mounted through an account switch - #29622

Draft
chrisnojima wants to merge 3 commits into
nojima/HOTPOT-crash-fixfrom
nojima/HOTPOT-nav-crash
Draft

fix(mobile): keep the logged-in screens mounted through an account switch#29622
chrisnojima wants to merge 3 commits into
nojima/HOTPOT-crash-fixfrom
nojima/HOTPOT-nav-crash

Conversation

@chrisnojima

Copy link
Copy Markdown
Contributor

Problem

Switching accounts on iOS (long-pressing the header avatar, or picking an account in the switcher) could leave the app frozen. The screen still drew, but every tap went nowhere. The service and the JS thread were both idle and healthy. The native log showed each touch being dropped:

UIManagerBinding.cpp:143] instanceHandle is null, event of type topTouchStart will be dropped

The visible views belonged to React components that had already unmounted. Switches also sometimes logged a navigation error, where native-stack reported the root loggedIn screen as dismissed while it was still the only route in JS state:

[NAV] Unhandled action: POP {"payload": {"count": 1}, "source": "loggedIn-…", ...}
The screen 'loggedIn' was removed natively but didn't get removed from JS state.

Cause

During a switch, the service sends loggedOut and then loggedIn, so config.loggedIn flaps false and back to true. On mobile, the root stack's loggedIn/loggedOut groups followed that flag directly. So every switch rebuilt the native root stack three times in about 130 ms:

  1. loggedIn → false: the logged-in screens are swapped for the login stack.
  2. loggedIn → true, before the new username arrives: the logged-in screens mount again under the old navigator.
  3. The username changes, useUserSwitchNavKey changes the key, and the whole navigator remounts.

react-native-screens logged Unbalanced calls to begin/end appearance transitions for <RNSScreen> on nearly every switch. That's the native side starting a transition before the previous one finished. The navigator it tore down could be left on screen, over the live one, which is the freeze. Its screens could also report a dismissal after React had already let go of them, which is the POP.

Desktop already avoids this: its groups use s.loggedIn || s.userSwitching to keep the app mounted through the gap.

Fix

1. Keep the logged-in screens mounted through a switch that started logged in. Mobile's useIsLoggedInNative / useIsLoggedOutNative now use showLoggedInScreens (router-v2/account-switch.tsx): logged in, or a switch in progress that started while logged in. That switch's logged-in screens stay mounted until the navigator remounts for the new account, so each switch does one native rebuild instead of three.

  • setUserSwitching records whether the switch started logged in (userSwitchingFromLoggedIn), and that survives the mid-switch store reset.
  • A switch that starts logged out, such as a notification tap on the login screen, keeps the logged-out screens and their switching spinner until it lands, as before. Holding the logged-in screens there too would mount the tabs with no user. If that login then failed, the root would swap back, which is the same churn this PR removes.
  • Desktop keeps its existing loggedIn || userSwitching gate.

2. Clear userSwitching when a switch's login ends without logging in. Until now it was cleared only by onNativeReady (after the remount) and by setLoginError. login() swallows two outcomes without calling either:

  • a cancellation of one of its own prompts, which includes handing an account that needs a new device off to the provisioning flow;
  • a failure that isn't an RPCError.

In both cases the service has already logged the old account out, so loggedIn is false. userSwitching stayed true forever. With (1), a switch that started logged in would then stay on the old account's screens with the stores reset, unable to reach the logged-out or provisioning screens. Desktop's routers already hold the logged-in screens while the flag is set, so desktop had the same hole. Both branches now clear userSwitching, and a failed switch falls through to the logged-out flow on both platforms.

3. Land the provisioning hand-off after the logged-out stack mounts. When a switch reaches an account that needs a new device, login() cancels the prompt and pushes username. That route lives in the logged-out stack. With (1), that stack isn't mounted yet when the push is dispatched, so the push was dropped, and clearing the flag afterwards only mounted the stack on login. Desktop already dropped it the same way.

  • The PromptNewDeviceName handler now clears userSwitching first. It then pushes username through a new navigateAppendOnceRootHas('loggedOut', …) in constants/router.tsx.
  • navigateAppendOnceRootHas pushes right away if the root stack already has that route, which is what happens for a normal login from the logged-out screen. Otherwise it waits for the navigator's next state event where the route exists.
  • It gives up after 5 s, so a stack that never mounts can't trigger the push at some unrelated later time.

4. Drop a notification tap's pending notification when its switch ends without landing. Tapping a notification for another account starts a switch and stores the notification as pending. Once (2) and (3) clear userSwitching on a failed switch, that notification could stay pending. The account-list replay in push-listener would then re-run the switch to that account the next time the accounts refresh, for example after the user logs into a different account.

The push store now remembers which account a notification tap is switching to. When a switch ends, its config subscription drops the pending notification only if it's that tap's, the same way it already drops it on a login error. A successful switch has already consumed the notification by then: push-listener replays it as soon as the current user's uid changes, and the router clears the flag afterwards in onNativeReady. A notification parked for an account that isn't configured yet belongs to no switch, so an unrelated switch ending leaves it for the account-list replay.

5. End a switch that lands on the navigator that's already mounted. On success, only the new navigator's onReady cleared userSwitching. useUserSwitchNavKey remounts the navigator only when the username changes from the last account's. So a switch that ends on the same account left the flag set for good. For example: log out, which keeps the stored secret, then tap a notification for that same account. So did the first switch after launching logged out.

A stuck flag drops every deep link (linking.tsx), skips later notification taps as "switch already in progress", and keeps background FS RPCs off. On desktop, whose routers hold the logged-in screens while the flag is set, a later logout also kept the logged-in screens.

  • setUserSwitching(true, username) now records the switch's target (userSwitchingTo), and it survives the mid-switch store reset. All four places that start a switch pass it: the account switcher, the header avatar, the desktop tab-bar quick switch and a notification tap.
  • When a username arrives and useUserSwitchNavKey isn't going to remount, it ends the switch if the username is the switch's target.
  • Matching the target, rather than just "no remount", keeps a stale username that arrives mid-switch from ending a switch still in flight.
  • A switch that does remount still leaves the flag for the new navigator's onReady, so a replayed notification can't be handled by the old navigator.
  • Navigation readiness has the same gap. A logout's store reset sets navigationReady to false, and only onReady set it back. So after logging out and back in on the same navigator, every deep link and notification intent stayed queued (linking.tsx). The switch above depends on that working, and plain logout and re-login to the same account already had the gap. When a username arrives after the user was blank and there's no remount, useUserSwitchNavKey now marks the mounted navigator ready for that account. It does that before ending the switch, so the intent the switch replays can run. The first render still leaves readiness to onReady.

Testing

Unit tests:

  • stores/tests/config.test.ts:
    • A switch whose login ends in a self-cancelled prompt, a non-RPCError failure, or an RPC error leaves userSwitching cleared.
    • The provisioning prompt hands off to username through navigateAppendOnceRootHas('loggedOut', …), with userSwitching already cleared at that moment.
  • router-v2/account-switch.test.tsx: showLoggedInScreens follows loggedIn with no switch running. It holds the logged-in screens through the flap of a switch that started logged in, and keeps the logged-out screens for a switch that started logged out.
  • constants/navigate-append-once-root-has.test.ts: the helper pushes right away when the route exists. Otherwise it waits for the route to mount and pushes once, and it gives up after the timeout.
  • stores/tests/push.test.ts:
    • A switch started by a notification tap, through handlePush, drops that notification when it ends without landing.
    • A notification parked for an account that isn't configured yet survives an unrelated switch ending.
    • A pending notification survives the store reset in the middle of a switch.
  • router-v2/use-user-switch-nav-key.test.tsx:
    • A switch that lands back on the account the navigator shows ends the switch without a remount.
    • The first switch after starting logged out ends when its account arrives.
    • Logging back in on the mounted navigator restores navigation readiness for that account, and a switch that lands there ends only after readiness is back. The first render leaves readiness to onReady.
    • A stale username mid-switch doesn't end the switch. The real target then remounts, and the flag is left for onReady.
  • stores/tests/config.test.ts: setUserSwitching records the target and whether the switch started logged in. It clears both with the flag and keeps them across the mid-switch reset.
  • Mutation checks:
    • With showLoggedInScreens ignoring where the switch started, only the "switch that started logged out" test fails.
    • With setUserSwitching never recording a logged-in start, only its config test fails.
    • With the hook never ending a switch, both landing tests fail.
    • Without the readiness restore, both readiness tests fail. Restoring readiness on every no-remount arrival, not just after a blank user, fails only the first-render test.
    • With the target match dropped, only the stale-username test fails.
    • With both new setUserSwitching(false) calls in the catch removed, both of their tests fail.
    • Without the hand-off clear, the provisioning test fails.
    • With the helper pushing immediately, its "waits" and timeout tests fail.
    • Without the "switch ended" clearing in push.tsx, only the notification-drop test fails.
    • Clearing on any switch end, without the account match, fails only the parked-notification test.
  • The push tests load the store as mobile (stores/tests/as-mobile.ts). Jest defaults to desktop, where the push store's dispatches are no-ops, so tests run that way would pass without exercising anything.

The provisioning path was not exercised in the simulator; only the unit tests cover it.

Simulator: rapid back-and-forth account switching in the iOS simulator by long-pressing the header avatar. The runs were instrumented with temporary logging of navigator mounts and native dismissals.

Before After
Switches ~40 84
Unbalanced calls to begin/end appearance transitions on nearly every switch 0
login stack mounted mid-switch once per switch 0
Root navigator mounts per switch 3 1
Frozen UI (every touch dropped) yes no
[NAV] Unhandled action: POP seen 0

@chrisnojima
chrisnojima added this pull request to stack #29623 September 10, 2026 21:03
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-nav-crash branch from 04c4ae4 to 97e1ad5 Compare September 10, 2026 21:26
@chrisnojima
chrisnojima requested a balanced review from Copilot September 10, 2026 21:26

This comment was marked as outdated.

@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-nav-crash branch from efdcab3 to 332a2a8 Compare September 11, 2026 01:48
@chrisnojima
chrisnojima requested a balanced review from Copilot September 11, 2026 01:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-nav-crash branch from 332a2a8 to 1c21068 Compare September 11, 2026 13:47
…itch

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.
…ng 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.
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-nav-crash branch from 1c21068 to 43c4711 Compare September 11, 2026 13:47
@zoom-ua

zoom-ua commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Findings

  1. setUserSwitching(true) can drop the logged-in hold if it runs after the logout flap (medium)

shared/stores/config.tsx lines 598-604

  setUserSwitching: (sw, to) => {
    set(s => {
      s.userSwitching = sw
      s.userSwitchingFromLoggedIn = sw && s.loggedIn
      s.userSwitchingTo = sw ? (to ?? '') : ''
    })
  },

Once the service’s loggedOut has set loggedIn to false, a second setUserSwitching(true, …) writes userSwitchingFromLoggedIn = false. That is exactly the group swap this PR is trying to prevent.

Header long-press and push taps bail if a switch is already running. The account switcher only disables rows while waitingKeyConfigLogin is set, so it is mostly safe today. Still, the flag should be latched: if a switch is already in progress, keep
userSwitchingFromLoggedIn and update userSwitchingTo instead of recomputing from current loggedIn.

  1. scalreadyloggedin still leaves userSwitching stuck (low–medium)

The new catch clears the flag for non-RPC failures and self-cancels. StatusCode.scalreadyloggedin only calls setLoggedIn(true). If that returns while the username is already the current one, useUserSwitchNavKey does not remount and does not run again, so the flag
never clears — the same class of bug this PR fixes for logout → notification → same account (dropped deep links, skipped later taps, FS background RPCs off).

If that status is reachable with doUserSwitch: true, clear the flag there too, or end the switch the same way as the no-remount username path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants