Skip to content

Fix infinite onboarding navigation loop crash on iOS #88014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mountiny merged 3 commits into from
Apr 16, 2026
Merged

Fix infinite onboarding navigation loop crash on iOS #88014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
656cfdf
Fix infinite onboarding navigation loop crash (APP-7FR)
mountiny Apr 15, 2026
176bd19
Tighten idempotency check to focused route only
mountiny Apr 16, 2026
6a4298b
Rename negated variable to fix no-negated-variables lint rule
mountiny Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/libs/Navigation/guards/OnboardingGuard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,14 @@ const OnboardingGuard: NavigationGuard = {
return {type: 'ALLOW'};
}

// If the OnboardingModalNavigator is the currently focused route, the user is already
// on the onboarding flow. Redirecting again would produce a redundant state reset that
// triggers further actions, creating an infinite navigation loop (APP-7FR).

@francoisl francoisl Apr 16, 2026

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.

NAB - What is APP-7FR? Not sure it's relevant to add if it's not explained anywhere.

@mountiny mountiny Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is the issue id in Sentry
image

I agree it does not have to be there, but this is causing crashes for users in production so I would still go ahead and merge now

const isOnboardingFocused = state.routes[state.index]?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR;
if (isOnboardingFocused) {
return {type: 'ALLOW'};
}

// User needs onboarding - calculate the correct step and redirect
const onboardingRoute = getOnboardingRoute();

Expand Down
157 changes: 113 additions & 44 deletions tests/unit/Navigation/guards/OnboardingGuard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ describe('OnboardingGuard', () => {
});

describe('redirect to onboarding', () => {
it('should redirect when authenticated user needs onboarding', async () => {
it('should redirect when authenticated user needs onboarding and is not on onboarding', async () => {
// Given a new user from a public email domain who has not completed the guided setup flow
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
hasCompletedGuidedSetupFlow: false,
Expand All @@ -336,7 +336,7 @@ describe('OnboardingGuard', () => {
});
await waitForBatchedUpdates();

// When the guard evaluates a navigation action to a non-onboarding screen
// When the guard evaluates a navigation action while the user is on a non-onboarding screen
const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};

// Then the user should be redirected to onboarding because new users must complete the setup flow before accessing the app
Expand All @@ -355,25 +355,43 @@ describe('OnboardingGuard', () => {
});
await waitForBatchedUpdates();

// When the guard evaluates a navigation action
// When the guard evaluates a navigation action while the user is on a non-onboarding screen
const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};

// Then the user should be redirected to onboarding because their domain/policy context determines which onboarding step they should land on
expect(result.type).toBe('REDIRECT');
expect(result.route).toContain('onboarding');
});
});

it('should redirect when user tries to access wrong onboarding step', async () => {
// Given a new user from a public domain who is currently on the onboarding purpose screen but may need to be on a different step
const onboardingState: NavigationState = {
key: 'root',
index: 0,
routeNames: [SCREENS.ONBOARDING.PURPOSE],
routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}],
stale: false,
type: 'root',
};
describe('infinite loop prevention (APP-7FR)', () => {
// A realistic navigation state that matches what the guard's REDIRECT reset produces:
// HOME at the bottom, OnboardingModalNavigator on top (focused).
const stateWithOnboardingNavigator: NavigationState = {
key: 'root',
index: 1,
routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR],
routes: [
{key: 'home', name: SCREENS.HOME},
{
key: 'onboarding-modal',
name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR,
state: {
key: 'onboarding-stack',
index: 0,
routeNames: [SCREENS.ONBOARDING.WORK_EMAIL],
routes: [{key: 'work-email', name: SCREENS.ONBOARDING.WORK_EMAIL}],
stale: false,
type: 'stack',
},
},
],
stale: false,
type: 'stack',
};

it('should ALLOW when user is already on onboarding to prevent redirect loop', async () => {
// Given a HybridApp user who needs onboarding (all shouldSkipOnboarding conditions are false)
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
hasCompletedGuidedSetupFlow: false,
});
Expand All @@ -382,31 +400,39 @@ describe('OnboardingGuard', () => {
});
await waitForBatchedUpdates();

// When the guard evaluates a navigation action while the user is on a potentially incorrect onboarding step
const result = OnboardingGuard.evaluate(onboardingState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};
// When the guard evaluates any action while the user is already on the OnboardingModalNavigator
const result = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext);

// Then the user should be redirected to the correct onboarding step because the guard enforces the proper step sequence
expect(result.type).toBe('REDIRECT');
expect(result.route).toContain('onboarding');
// Then navigation should be ALLOWED because the user is already on onboarding;
// redirecting again would produce a redundant state reset that causes an infinite loop
expect(result.type).toBe('ALLOW');
});

it('should redirect when user in onboarding tries to access non-onboarding path', async () => {
// Given a new user from a public domain who is currently on the onboarding purpose screen
const onboardingState: NavigationState = {
key: 'root',
index: 0,
routeNames: [SCREENS.ONBOARDING.PURPOSE],
routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}],
stale: false,
type: 'root',
};
it('should prove the guard reaches a stable state (no infinite loop)', async () => {
// Given a user who needs onboarding
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
hasCompletedGuidedSetupFlow: false,
});
await Onyx.merge(ONYXKEYS.ACCOUNT, {
isFromPublicDomain: true,
});
await waitForBatchedUpdates();

// When the user attempts to navigate to the HOME screen before completing onboarding
const homeAction: NavigationAction = {
type: 'NAVIGATE',
payload: {name: SCREENS.HOME},
};
// When the guard first evaluates on a non-onboarding state, it redirects
const firstResult = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext);
expect(firstResult.type).toBe('REDIRECT');

// And then subsequent evaluations on the post-redirect state (OnboardingModalNavigator mounted)
// reach a stable ALLOW state, breaking any potential loop
const secondResult = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext);
expect(secondResult.type).toBe('ALLOW');

const thirdResult = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext);
expect(thirdResult.type).toBe('ALLOW');
});

it('should still redirect when user is NOT on onboarding and needs it', async () => {
// Given a user who needs onboarding and is on the HOME screen (not on onboarding)
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
hasCompletedGuidedSetupFlow: false,
});
Expand All @@ -415,22 +441,27 @@ describe('OnboardingGuard', () => {
});
await waitForBatchedUpdates();

const result = OnboardingGuard.evaluate(onboardingState, homeAction, authenticatedContext) as {type: 'REDIRECT'; route: string};
// When the guard evaluates on a state without OnboardingModalNavigator
const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};

// Then the user should be redirected back to onboarding because they must complete the setup flow before accessing other parts of the app
// Then the guard should redirect because the user needs onboarding and isn't on it yet
expect(result.type).toBe('REDIRECT');
expect(result.route).toContain('onboarding');
});

it('should always redirect to correct onboarding step when user needs onboarding', async () => {
// Given a new user from a public domain who is currently on the work-email onboarding step but the guard determines they belong on a different step
const onboardingState: NavigationState = {
it('should still redirect when onboarding is in routes but not focused', async () => {
// Given a user who needs onboarding, and a state where OnboardingModalNavigator
// exists in routes but HOME is focused (index: 0)
const stateWithOnboardingUnfocused: NavigationState = {
key: 'root',
index: 0,
routeNames: [SCREENS.ONBOARDING.WORK_EMAIL],
routes: [{key: 'work-email', name: SCREENS.ONBOARDING.WORK_EMAIL}],
routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR],
routes: [
{key: 'home', name: SCREENS.HOME},
{key: 'onboarding-modal', name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR},
],
stale: false,
type: 'root',
type: 'stack',
};

await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
Expand All @@ -441,12 +472,50 @@ describe('OnboardingGuard', () => {
});
await waitForBatchedUpdates();

// When the guard evaluates a navigation action while the user is on a specific onboarding step
const result = OnboardingGuard.evaluate(onboardingState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};
// When the guard evaluates while onboarding is NOT focused
const result = OnboardingGuard.evaluate(stateWithOnboardingUnfocused, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string};

// Then the guard should redirect to the correct onboarding step because the step sequence is dynamically determined by the user's account state
// Then the guard should still redirect because the user isn't actively on onboarding
expect(result.type).toBe('REDIRECT');
expect(result.route).toContain('onboarding');
});

it('should still BLOCK RESET to non-onboarding even when on onboarding', async () => {
// Given a user on onboarding who has not completed it
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {
hasCompletedGuidedSetupFlow: false,
});
await waitForBatchedUpdates();

// Note: shouldPreventReset uses findFocusedRoute which checks the deepest focused route name.
// In a state with onboarding screens at the root level (as used by shouldPreventReset tests),
// the focused route IS an onboarding screen name.
const onboardingRootState: NavigationState = {
key: 'root',
index: 0,
routeNames: [SCREENS.ONBOARDING.PURPOSE],
routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}],
stale: false,
type: 'root',
};

const resetToHome: NavigationAction = {
type: CONST.NAVIGATION_ACTIONS.RESET,
payload: {
key: 'root',
index: 0,
routeNames: [SCREENS.HOME],
routes: [{key: 'home', name: SCREENS.HOME}],
stale: false,
type: 'root',
},
};

const result = OnboardingGuard.evaluate(onboardingRootState, resetToHome, authenticatedContext) as {type: 'BLOCK'; reason?: string};

// Then the RESET should still be blocked by shouldPreventReset (runs before the new check)
expect(result.type).toBe('BLOCK');
expect(result.reason).toBe('Cannot reset to non-onboarding screen while on onboarding');
});
});
});
Loading