Fix doubled OpenApp on delegate (copilot) account switch#89254
Fix doubled OpenApp on delegate (copilot) account switch#89254mountiny merged 5 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@abzokhattab Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b95796dc0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@MelvinBot review |
|
@codex review |
Code ReviewWell-structured PR that correctly identifies and fixes two independent root causes. The two-layer defense (NetworkState suppression + Onyx transition helper) is the right approach. Approve with minor suggestions. Suggestions
Detailed analysisNetworkState fix — correct:
Tests — solid:
|
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
|
@MelvinBot please create new dedicated App issuefor this PR and assign mountiny and the pull reuqest author. Link the issue here |
|
@abzokhattab can you please prioritize the review? Thanks |
|
🤖 Created dedicated issue: #89265 — assigned to |
|
reviewing it |
There was a problem hiding this comment.
Pull request overview
Fixes a double-OpenApp request when switching into a copilot/delegate account by preventing two separate “recovery” paths from triggering an extra reconnect/open sequence.
Changes:
- Add a reconfigure-only suppression window in
NetworkStateto ignore NetInfo’s synthetic post-reconfigure reachability “recovery”. - Introduce
clearOnyxForDelegateTransition()to preserveIS_LOADING_APP=trueacross delegate-transitionOnyx.clear()soDelegateAccessHandlerdoesn’t trigger a duplicateopenApp(). - Add/extend unit tests to cover the reconfigure suppression behavior and the new Onyx-clear helper behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libs/NetworkState.ts | Suppresses synthetic reachability-restored events after NetInfo reconfigure to avoid unwanted reconnect/openApp. |
| src/libs/actions/Delegate.ts | Adds clearOnyxForDelegateTransition() and updates delegate flows to use it. |
| src/libs/actions/Session/index.ts | Uses clearOnyxForDelegateTransition() during OldDot→NewDot delegate switch flow. |
| tests/unit/NetworkStateReachabilityTest.ts | Adds regression tests for reconfigure suppression and real outage recovery behavior. |
| tests/actions/DelegateTest.ts | Adds a unit test for clearOnyxForDelegateTransition() preserving IS_LOADING_APP and clearing non-preserved keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // resets prev to undefined on reconfigure so the new subscription's first transitions | ||
| // are treated like boot, not recovery. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-04-30.at.17.52.28.movAndroid: mWeb ChromeScreen.Recording.2026-04-30.at.17.56.26.moviOS: HybridAppScreen.Recording.2026-04-30.at.17.50.21.mov
|
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
trjExpensify
left a comment
There was a problem hiding this comment.
Definitely seems roughly twice as fast to switch to me when comparing against staging.
Adhoc build:
2026-04-30_23-43-08.mp4
Staging:
2026-04-30_23-43-58.mp4
mountiny
left a comment
There was a problem hiding this comment.
Great! Thanks for such a quick fix
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.66-0 🚀
|
|
No help site changes are required for this PR. This is a purely internal performance fix that prevents |
Explanation of Change
When a user switches into a copilot/delegate account via the account switcher,
OpenAppwas firing twice — the first call resolves with skeletons visible, then a secondOpenAppruns while skeletons are still visible, causing a noticeable double-loading experience on large accounts.Two independent root causes, both fixed:
NetworkState reconfigure path: The
CONNECT_AS_DELEGATEresponse updatesSESSION.accountID. The SESSION subscriber inNetworkState.tscalledconfigureAndSubscribe()to rebuild the NetInfo subscription. Re-subscribing emits a syntheticnull → truetransition as NetInfo tears down its internal state during reconfigure, which the listener treated as a network recovery event →Reconnect.reconnect()→openApp(). Fixed by adding asuppressNextReachabilityRestoredflag that is set only on reconfigure calls (detected viaunsubscribeNetInfo !== nullat entry). The flag suppresses the synthetic post-reconfigure recovery transition and clears once reachability settles. Boot/normal behavior is unchanged.DelegateAccessHandler recovery effect path:
Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS)preservedHAS_LOADED_APP=truebut notIS_LOADING_APP. Right after the clear, the state was brieflyhasLoadedApp=true && isLoadingApp=undefined— exactly the trigger for the recovery effect atsrc/DelegateAccessHandler.tsx:62, which fired its ownopenApp()in the few microtasks beforeopenApp's optimistic data mergedIS_LOADING_APP=true. Fixed via a newclearOnyxForDelegateTransition()helper that mergesIS_LOADING_APP=truebefore clearing, so the post-clear render always seesIS_LOADING_APP=true. Used at all 4 call sites that previously calledOnyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS)directly.Test coverage added in
tests/unit/NetworkStateReachabilityTest.ts(reconfigure suppression + regression guard for genuine offline→online after reconfigure) andtests/actions/DelegateTest.ts(helper test).Fixed Issues
$ #89265
PROPOSAL:
Tests
OpenApp.OpenApprequest fires in the Network tab — before the fix, two were observed (the second queued behind the first, running once the first resolved).OpenApprequest fires.Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari