Fix onboarding flow not appearing after anonymous sign-in from public room thread by MelvinBot · Pull Request #85660 · Expensify/App

MelvinBot · 2026-03-18T14:24:54Z

Explanation of Change

When an anonymous user signs in via the SignInModal (e.g., tapping "Reply in thread" in a public room), Navigation.dismissModal() and openApp() were firing in parallel. The dismiss triggered the OnboardingGuard while IS_LOADING_APP was still true, causing the guard to skip the onboarding redirect entirely. This left new users stuck without onboarding and with an infinite loading state on the Inbox.

This PR fixes the issue by ensuring openApp(true) fully completes (so IS_LOADING_APP transitions to false and NVP_ONBOARDING is loaded) before dismissing the modal. This way, when dismissModal() triggers the OnboardingGuard, it evaluates with accurate data and properly redirects new users to onboarding.

The fix uses Promise.all to run openApp (with its queue idle waits) and Navigation.isNavigationReady() concurrently, only calling dismissModal() after both complete.

Fixed Issues

$ #83092
PROPOSAL: #83092 (comment)

Tests

Open a public room link as an anonymous user (e.g., https://new.expensify.com/r/1540486774487195)
Navigate to a thread
Hold/tap a message to open the action menu and select "Reply in thread"
Complete sign-in with a new account
Verify the onboarding flow appears after sign-in
Complete onboarding and verify you can navigate back to Inbox without infinite loading

Verify that no errors appear in the JS console

Offline tests

Sign in from a public room thread reply as described above
Toggle airplane mode/disconnect network after sign-in completes
Verify the app doesn't hang in a loading state — it should either show onboarding or the Inbox depending on connectivity timing

QA Steps

As an anonymous user, visit a public room link on iOS mWeb Safari
Navigate to a thread within that room
Tap on a message and select "Reply in thread"
Complete sign-in with a brand new account
Verify the onboarding flow appears after sign-in
Complete onboarding
Navigate back to Inbox and verify it loads correctly (no infinite loading/skeleton state)

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…gn-in When an anonymous user signs in via the SignInModal (e.g. "Reply in thread" in a public room), Navigation.dismissModal() and openApp() were firing in parallel. The dismiss triggered the OnboardingGuard while IS_LOADING_APP was still true, causing it to skip the onboarding redirect entirely. Now we ensure openApp(true) fully completes (IS_LOADING_APP transitions to false, NVP_ONBOARDING is loaded) before dismissing the modal, so the OnboardingGuard evaluates with accurate data and properly redirects new users to onboarding. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-18T14:25:12Z

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-fixOnboardingAfterAnonymousSignIn
git checkout -b claude-fixOnboardingAfterAnonymousSignIn upstream/claude-fixOnboardingAfterAnonymousSignIn
git push -u origin claude-fixOnboardingAfterAnonymousSignIn

Then you can close this PR and open a new one from your fork.

eh2077 · 2026-03-20T08:35:28Z

@MelvinBot Can you add unit test to cover the change?

Tests cover: - Anonymous users don't trigger openApp or dismissModal - Non-anonymous users trigger openApp then dismissModal - dismissModal waits for both openApp chain and isNavigationReady (Promise.all) - Session transition from anonymous to authenticated triggers the correct flow Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T08:46:11Z

Added unit tests for the SignInModal useEffect behavior in tests/unit/SignInModalTest.tsx. The tests cover:

Anonymous user — verifies openApp and dismissModal are NOT called when authTokenType is ANONYMOUS
Non-anonymous user — verifies waitForIdle, openApp(true), and dismissModal are all called
Promise.all ordering — verifies dismissModal waits for BOTH the openApp chain and isNavigationReady() to complete before firing (the core behavior this PR introduces)
Session transition — verifies the correct flow when a session transitions from anonymous to authenticated

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T08:47:54Z

Fixed the failing Prettier check: ran prettier on tests/unit/SignInModalTest.tsx which reordered the SignInModal import to be grouped with the other imports at the top of the file.

- Add type-safe casts for require() calls inside jest.mock factories - Add eslint-disable comments for __esModule naming convention - Add file-level disable for no-non-null-assertion - Use React.createElement instead of JSX in mock factories Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T08:53:25Z

Fixed the failing ESLint check in tests/unit/SignInModalTest.tsx (19 errors):

Added type-safe casts for require() calls inside jest.mock factories (as typeof React, as {View: typeof RNView}) to resolve no-unsafe-assignment, no-unsafe-return, no-unsafe-call, and no-unsafe-member-access errors
Added eslint-disable-next-line @typescript-eslint/naming-convention comments for __esModule properties
Added file-level eslint-disable @typescript-eslint/no-non-null-assertion for non-null assertions used after expect guards
Switched mock components from JSX to React.createElement to avoid needing React in scope inside factory closures

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T08:58:10Z

Fixed the failing checks in tests/unit/SignInModalTest.tsx:

ESLint (no-unsafe-return): Replaced wrapper arrow functions with direct mock references for dismissModal and goBack to avoid returning any
TypeScript (spread argument): Replaced wrapper for openApp with direct mock reference to fix tuple type mismatch
TypeScript (ref on View): Removed unused ref parameter from the MockSignInPage forwardRef since the mock doesn't need to forward refs

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:02:54Z

Fixed the failing test (job 6) check: the SignInModalTest.tsx mock for @libs/Browser was missing the isSafari function, which src/styles/index.ts imports at module load time. Added isSafari: jest.fn(() => false) to the mock.

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:08:35Z

Fixed the failing test (job 6) check: the @libs/Browser mock in tests/unit/SignInModalTest.tsx was missing getBrowser (and other exports). The styles module calls getBrowser() during initialization, which caused a TypeError when the test suite loaded. Added all Browser module exports to the mock.

- Add missing `flush` export to SequentialQueue mock (Network module imports it during initialization) - Mock `useSession` from OnyxListItemProvider to avoid requiring the full context provider tree in unit tests - Update tests to set session data via the mock instead of Onyx.merge Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:21:17Z

Fixed the failing test (job 6) check — SignInModalTest.tsx had two issues:

Missing flush in SequentialQueue mock: src/libs/Network/index.ts imports {flush} from SequentialQueue at module init time, but the test only mocked waitForIdle. Added flush: jest.fn() to the mock.
Missing useSession context provider: SignInModal calls useSession() from OnyxListItemProvider, which requires a SessionProvider ancestor. Added a mock for @components/OnyxListItemProvider that provides useSession directly, and updated tests to set session data via the mock.

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:31:28Z

Fixed the failing test (job 6) check. The SignInModalTest.tsx mock for openApp was directly assigning mockOpenApp in the jest.mock factory, but due to Jest's hoisting behavior, mockOpenApp was undefined when the factory executed. Changed to a wrapper function (...args) => mockOpenApp(...args) to defer the lookup — the same pattern already used for waitForIdle in the same test file.

The inline mock factory for @libs/actions/App was not properly overriding the manual __mocks__/App.ts mock. Switch to defining jest.fn() directly in the factory and importing from the mocked module, matching the pattern used by other tests (e.g. useConfirmReadyToOpenApp.test.ts). Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:40:53Z

Fixed the failing test (job 6) check: the SignInModalTest.tsx mock for @libs/actions/App was using a closure over an external variable (mockOpenApp) in the mock factory, which was being interfered with by the manual __mocks__/App.ts mock. Switched to defining jest.fn() directly in the factory and importing openApp from the mocked module, matching the pattern used by other tests like useConfirmReadyToOpenApp.test.ts.

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:49:07Z

Fixed the failing test in SignInModalTest.tsx. The issue was a Jest mock hoisting problem: mockDismissModal and mockGoBack were captured by value in the jest.mock factory, but at factory execution time (triggered by imports), those const variables hadn't been assigned yet — so they were undefined.

Changed the mock to use wrapper functions ((...args) => mockDismissModal(...args)) that read the variable at call time instead of at factory creation time, matching the pattern already used for isNavigationReady.

Remove __esModule/default wrapper from Navigation mock (matching all other test files) and use block bodies to avoid returning any values that trigger no-unsafe-return ESLint errors. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-20T09:57:09Z

Fixed the failing checks:

ESLint no-unsafe-return: The Navigation mock wrapper functions were implicitly returning any from jest.fn() calls. Changed to block bodies to avoid the unsafe return.
Jest test failures (Navigation.dismissModal is not a function): The Navigation mock used __esModule: true with a default wrapper, which doesn't match how other tests in this codebase mock Navigation. Simplified the mock to put functions directly on the mock object (matching the pattern in PromotedActionsBarTest, useSubPageTest, etc.), which allows Babel's _interopRequireDefault to correctly resolve the default import.

MelvinBot · 2026-03-20T10:11:15Z

The failing checks are:

checklist — This is the PR Author Checklist check, which is expected/informational.
perf-tests — This failure is unrelated to this PR. The error is:

Render count difference exceeded the allowed deviation of 0. Current difference: 0.8

It's failing on useAdvancedSearchFilters hook initial render with policies, which has nothing to do with the files changed in this PR (SignInModal.tsx and SignInModalTest.tsx). This appears to be a flaky perf test.

codecov · 2026-03-20T10:21:38Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/pages/signin/SignInModal.tsx	`88.88% <ø> (+88.88%)`	⬆️
... and 9 files with indirect coverage changes

JS00001 · 2026-03-24T12:54:07Z

Rerunning the tests

MelvinBot · 2026-03-24T13:07:54Z

The failing checks are unrelated to this PR's changes.

perf-tests failure: The test [SelectionList] should scroll and select a few items reports a render count difference of 2 (allowed deviation: 0). This PR only modifies src/pages/signin/SignInModal.tsx and tests/unit/SignInModalTest.tsx — neither of which is related to SelectionList rendering. This appears to be a pre-existing or flaky perf regression.

checklist failure: This is the PR Author/Reviewer Checklist check, which is unrelated to code.

JS00001 · 2026-03-24T13:08:07Z

@eh2077 can you pull main

eh2077 · 2026-03-24T14:17:03Z

@MelvinBot Can you merge with main branch? If not, then I can help

…fterAnonymousSignIn Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

MelvinBot · 2026-03-24T14:28:13Z

Merged main into this branch — no conflicts.

MelvinBot · 2026-03-24T14:31:40Z

The only non-checklist failing check is Reassure Performance Tests, which is failing due to a [SelectionList] should scroll and select a few items render count regression (5 → 7 renders, +2).

This is unrelated to this PR's changes. This PR only modifies SignInModal.tsx and SignInModalTest.tsx, which have no connection to SelectionList rendering. The same perf test is also failing on other unrelated branches (e.g. feat/last-12-months-preset, @GCyganek/unlock-landscape-mode), confirming this is a pre-existing/flaky issue on main.

The latest checks (after the merge with main) are currently still in progress — all completed checks so far are passing.

MelvinBot · 2026-03-24T14:40:20Z

The failing test (job 6) check is unrelated to this PR — it's a CI infrastructure issue.

Analysis: The job had zero steps and timed out at exactly 10 minutes, meaning the runner never allocated or initialized. This is a transient CI runner problem, not a code failure.

Evidence: This PR only modifies src/pages/signin/SignInModal.tsx and tests/unit/SignInModalTest.tsx. The job never got far enough to run any tests.

The other failing check (checklist) is the PR Author Checklist, which is unrelated to code.

Recommendation: Re-run the failed test (job 6) job to resolve this.

github-actions · 2026-03-24T14:57:48Z

🚧 @JS00001 has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-03-24T15:02:32Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #85660.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-ae06156-55a46af-7db938231a0f59ee3d50848d99e6874f888a1e76/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-ae06156-55a46af-fb1389cfddb6fb506430e1d76f0ac286e8f55fd9/index.html

Web 🕸️
https://85660.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-03-24T15:04:24Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2026-03-25T16:40:06Z

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.44-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

OSBotify · 2026-03-25T18:52:21Z

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.47-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

izarutskaya · 2026-03-26T03:05:00Z

@eh2077 For some reason public room didn't display after sign in
mWeb/Safari
Build 9.3.48-1

Screenrecording.03-26-2026.4-59-29.Am.1.mp4

eh2077 · 2026-03-26T03:13:26Z

Hmm, it worked for me when testing #85660 (comment)

eh2077 · 2026-03-26T03:15:25Z

@izarutskaya Are you able to reproduce it consistently?

izarutskaya · 2026-03-26T05:11:01Z

Another tester also cann't see room after sign in

1774496144497.retest.mp4

bernhardoj · 2026-03-26T09:03:55Z

This also caused the sign-in modal not to close

web.mp4

eh2077 · 2026-03-26T09:14:02Z

Thanks for your comments, going to revert it to unblock issues.

…AfterAnonymousSignIn" This reverts commit ae06156, reversing changes made to 064cafd.

OSBotify · 2026-03-26T14:17:42Z

🚀 Deployed to production by https://github.com/grgia in version: 9.3.48-2 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

OSBotify · 2026-03-26T14:42:44Z

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.49-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

OSBotify · 2026-03-27T12:24:52Z

🚀 Deployed to production by https://github.com/grgia in version: 9.3.49-2 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot assigned eh2077 Mar 18, 2026

MelvinBot requested a review from a team March 18, 2026 14:24

MelvinBot mentioned this pull request Mar 18, 2026

[$250] mWeb/Safari - Public room-Onboarding flow not appear when sign in from reply thread of public room #83092

Open

7 tasks

Fix: run prettier on SignInModalTest.tsx

8bd17bd

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

Fix: resolve ESLint and TypeScript errors in SignInModalTest

1902a19

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

Fix: add missing isSafari mock in SignInModalTest

47b7a58

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

Fix: add missing getBrowser and other Browser module exports to mock

bbc0e39

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

Fix: use wrapper function for openApp mock to avoid hoisting issue

69202e7

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

Fix: use wrapper functions in Navigation mock to avoid hoisting issue

b566ec7

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

melvin-bot bot requested a review from JS00001 March 24, 2026 12:46

Merge remote-tracking branch 'origin/main' into claude-fixOnboardingA…

dbff2c5

…fterAnonymousSignIn Co-authored-by: Eric Han <eh2077@users.noreply.github.com>

JS00001 approved these changes Mar 24, 2026

View reviewed changes

JS00001 merged commit ae06156 into main Mar 24, 2026
45 of 48 checks passed

JS00001 deleted the claude-fixOnboardingAfterAnonymousSignIn branch March 24, 2026 14:56

OSBotify mentioned this pull request Mar 25, 2026

Deploy Checklist: New Expensify 2026-03-25 #86329

Closed

71 tasks

bernhardoj mentioned this pull request Mar 26, 2026

Preserve deep-linked report route during onboarding guard redirect #86390

Open

52 tasks

MelvinBot mentioned this pull request Mar 26, 2026

Revert: Fix onboarding flow not appearing after anonymous sign-in #86407

Merged

53 tasks

JS00001 pushed a commit that referenced this pull request Mar 26, 2026

Revert "Merge pull request #85660 from Expensify/claude-fixOnboarding…

8495e82

…AfterAnonymousSignIn" This reverts commit ae06156, reversing changes made to 064cafd.

Conversation

MelvinBot commented Mar 18, 2026 • edited by JS00001 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

MelvinBot commented Mar 18, 2026

Uh oh!

eh2077 commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

MelvinBot commented Mar 20, 2026

Uh oh!

codecov bot commented Mar 20, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JS00001 commented Mar 24, 2026

Uh oh!

MelvinBot commented Mar 24, 2026

Uh oh!

JS00001 commented Mar 24, 2026

Uh oh!

eh2077 commented Mar 24, 2026

Uh oh!

MelvinBot commented Mar 24, 2026

Uh oh!

MelvinBot commented Mar 24, 2026

Uh oh!

MelvinBot commented Mar 24, 2026

Uh oh!

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

OSBotify commented Mar 24, 2026

Uh oh!

OSBotify commented Mar 25, 2026

Uh oh!

OSBotify commented Mar 25, 2026

Uh oh!

izarutskaya commented Mar 26, 2026

Uh oh!

eh2077 commented Mar 26, 2026

Uh oh!

eh2077 commented Mar 26, 2026

Uh oh!

izarutskaya commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bernhardoj commented Mar 26, 2026

Uh oh!

eh2077 commented Mar 26, 2026

MelvinBot commented Mar 18, 2026 •

edited by JS00001

Loading

codecov bot commented Mar 20, 2026 •

edited by github-actions bot

Loading

izarutskaya commented Mar 26, 2026 •

edited

Loading