From f5a4ecf0c1d5be127d1a2be8ff0d4facb86111b2 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Tue, 10 Mar 2026 15:30:04 +0700 Subject: [PATCH 1/3] fix: App crashes with infinite OnboardingGuard redirect loop --- src/libs/Navigation/guards/OnboardingGuard.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index 34037a9a3ce8..4c3dd2f23e2c 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -132,11 +132,11 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { /** * Check if the navigation action is targeting an onboarding screen. - * This handles NAVIGATE/PUSH actions that target the OnboardingModalNavigator directly. + * This handles NAVIGATE/PUSH/REPLACE actions that target the OnboardingModalNavigator directly. */ function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { if ( - (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH) && + (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH || action.type === CONST.NAVIGATION.ACTION_TYPE.REPLACE) && (action.payload as {name?: string} | undefined)?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR ) { return true; @@ -172,7 +172,15 @@ const OnboardingGuard: NavigationGuard = { } const shouldSkipOnboarding = - CONFIG.SKIP_ONBOARDING || context.isLoading || isTransitioning || isOnboardingCompleted || isMigratedUser || isSingleEntry || needsExplanationModal || isInvitedOrGroupMember; + CONFIG.SKIP_ONBOARDING || + context.isLoading || + isTransitioning || + isOnboardingCompleted || + isMigratedUser || + isSingleEntry || + needsExplanationModal || + isInvitedOrGroupMember || + isNavigatingToOnboardingFlow(action); if (shouldSkipOnboarding) { return {type: 'ALLOW'}; From 7969dca7ab3e6ab2f31830291c96e3258e47fa0a Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Tue, 10 Mar 2026 16:02:22 +0700 Subject: [PATCH 2/3] update test case --- .../Navigation/guards/OnboardingGuard.test.ts | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 6416d3424c65..b17c87b60504 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -231,6 +231,26 @@ describe('OnboardingGuard', () => { expect(result.route).toBe('home'); }); + it('should redirect completed user for REPLACE actions targeting onboarding', async () => { + // Given a user who has completed the guided setup flow + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + // When a REPLACE action targets the OnboardingModalNavigator + const replaceAction: NavigationAction = { + type: CONST.NAVIGATION.ACTION_TYPE.REPLACE, + payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + }; + + const result = OnboardingGuard.evaluate(mockState, replaceAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + + // Then the user should be redirected to HOME because the OnboardingModalNavigator is not mounted for completed users + expect(result.type).toBe('REDIRECT'); + expect(result.route).toBe('home'); + }); + it('should ALLOW when completed user navigates to a non-onboarding route', async () => { // Given a user who has completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { @@ -304,25 +324,6 @@ describe('OnboardingGuard', () => { // Then navigation should be allowed because isNavigatingToOnboardingFlow only checks NAVIGATE/PUSH actions, not RESET — RESET with onboarding routes does not reach the completed-user redirect expect(result.type).toBe('ALLOW'); }); - - it('should NOT redirect completed user for REPLACE actions targeting onboarding', async () => { - // Given a user who has completed the guided setup flow - await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { - hasCompletedGuidedSetupFlow: true, - }); - await waitForBatchedUpdates(); - - // When a REPLACE action targets the OnboardingModalNavigator - const replaceAction: NavigationAction = { - type: CONST.NAVIGATION.ACTION_TYPE.REPLACE, - payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, - }; - - const result = OnboardingGuard.evaluate(mockState, replaceAction, authenticatedContext); - - // Then navigation should be allowed because isNavigatingToOnboardingFlow only handles NAVIGATE and PUSH action types, not REPLACE - expect(result.type).toBe('ALLOW'); - }); }); describe('redirect to onboarding', () => { From 45a31bc9c593657f464ed5c0342f41b4be7e1db9 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Wed, 11 Mar 2026 00:12:33 +0700 Subject: [PATCH 3/3] limit the bypass with REPLACE action --- src/libs/Navigation/guards/OnboardingGuard.ts | 14 +++++-- .../Navigation/guards/OnboardingGuard.test.ts | 39 +++++++++---------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index 4c3dd2f23e2c..a2bd157f805f 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -132,11 +132,11 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { /** * Check if the navigation action is targeting an onboarding screen. - * This handles NAVIGATE/PUSH/REPLACE actions that target the OnboardingModalNavigator directly. + * This handles NAVIGATE/PUSH actions that target the OnboardingModalNavigator directly. */ function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { if ( - (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH || action.type === CONST.NAVIGATION.ACTION_TYPE.REPLACE) && + (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH) && (action.payload as {name?: string} | undefined)?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR ) { return true; @@ -145,6 +145,14 @@ function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { return false; } +/** + * Check if the navigation action is targeting an onboarding screen. + * This handles REPLACE actions that target the OnboardingModalNavigator directly. + */ +function isNavigatingToOnboardingFlowWithReplaceAction(action: NavigationAction): boolean { + return action.type === CONST.NAVIGATION.ACTION_TYPE.REPLACE && (action.payload as {name?: string} | undefined)?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR; +} + /** * OnboardingGuard handles ONLY the core NewDot onboarding flow */ @@ -180,7 +188,7 @@ const OnboardingGuard: NavigationGuard = { isSingleEntry || needsExplanationModal || isInvitedOrGroupMember || - isNavigatingToOnboardingFlow(action); + isNavigatingToOnboardingFlowWithReplaceAction(action); if (shouldSkipOnboarding) { return {type: 'ALLOW'}; diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index b17c87b60504..6416d3424c65 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -231,26 +231,6 @@ describe('OnboardingGuard', () => { expect(result.route).toBe('home'); }); - it('should redirect completed user for REPLACE actions targeting onboarding', async () => { - // Given a user who has completed the guided setup flow - await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { - hasCompletedGuidedSetupFlow: true, - }); - await waitForBatchedUpdates(); - - // When a REPLACE action targets the OnboardingModalNavigator - const replaceAction: NavigationAction = { - type: CONST.NAVIGATION.ACTION_TYPE.REPLACE, - payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, - }; - - const result = OnboardingGuard.evaluate(mockState, replaceAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; - - // Then the user should be redirected to HOME because the OnboardingModalNavigator is not mounted for completed users - expect(result.type).toBe('REDIRECT'); - expect(result.route).toBe('home'); - }); - it('should ALLOW when completed user navigates to a non-onboarding route', async () => { // Given a user who has completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { @@ -324,6 +304,25 @@ describe('OnboardingGuard', () => { // Then navigation should be allowed because isNavigatingToOnboardingFlow only checks NAVIGATE/PUSH actions, not RESET — RESET with onboarding routes does not reach the completed-user redirect expect(result.type).toBe('ALLOW'); }); + + it('should NOT redirect completed user for REPLACE actions targeting onboarding', async () => { + // Given a user who has completed the guided setup flow + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + // When a REPLACE action targets the OnboardingModalNavigator + const replaceAction: NavigationAction = { + type: CONST.NAVIGATION.ACTION_TYPE.REPLACE, + payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + }; + + const result = OnboardingGuard.evaluate(mockState, replaceAction, authenticatedContext); + + // Then navigation should be allowed because isNavigatingToOnboardingFlow only handles NAVIGATE and PUSH action types, not REPLACE + expect(result.type).toBe('ALLOW'); + }); }); describe('redirect to onboarding', () => {