Skip to content

test#90980

Closed
mkhutornyi wants to merge 1 commit into
Expensify:mainfrom
mkhutornyi:revert-89750-fix/navigate-to-workspaces-tab
Closed

test#90980
mkhutornyi wants to merge 1 commit into
Expensify:mainfrom
mkhutornyi:revert-89750-fix/navigate-to-workspaces-tab

Conversation

@mkhutornyi
Copy link
Copy Markdown
Contributor

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines Coverage Δ
src/pages/workspace/AccessOrNotFoundWrapper.tsx 74.60% <100.00%> (ø)
src/pages/workspace/WorkspacePageWithSections.tsx 72.85% <ø> (+0.63%) ⬆️
...xpensifyCard/WorkspaceExpensifyCardDetailsPage.tsx 0.00% <ø> (ø)
src/hooks/useRestoreWorkspacesTabOnNavigate.ts 96.42% <94.11%> (-3.58%) ⬇️
...ibs/Navigation/helpers/navigateToWorkspacesPage.ts 95.91% <96.42%> (+0.56%) ⬆️
src/pages/workspace/WorkspaceInitialPage.tsx 0.00% <0.00%> (ø)
...igator/createWorkspaceNavigator/WorkspaceRouter.ts 0.00% <0.00%> (ø)
src/libs/Navigation/NavigationRoot.tsx 77.55% <50.00%> (-0.45%) ⬇️
... and 7 files with indirect coverage changes

@mkhutornyi
Copy link
Copy Markdown
Contributor Author

mkhutornyi commented May 18, 2026

@MelvinBot find out offending PR (merged newer than 5 days ago, older than 3 days ago) which caused this regression:

  1. Logout the app
  2. Refresh browser
  3. Login with existing account
  4. After entering magic code and login successfully, not found page shows

@MelvinBot
Copy link
Copy Markdown
Contributor

Regression Analysis: "Not Found" page after login

Most likely culprit: PR #90044 — "Fix reveal navigation under RHP + re-land reveal workspace route under RHP before dismissing on workspace creation" (merged May 15)

This PR made fundamental changes to the root navigation router's history management (addRootHistoryRouterExtension.ts) that affect every navigation action. Specifically, the default code path in getStateForAction now runs applyRevealPaddingOffset on all actions (line 95-96), which carries forward CUSTOM_HISTORY_ENTRY_REVEAL_PADDING sentinels from state.history. If these sentinels corrupt the browser history alignment during the PublicScreens → AuthScreens transition after login, useLinking would compute an incorrect historyDelta, causing React Navigation to resolve an unmatched URL that falls through to the [SCREENS.NOT_FOUND]: '*' wildcard.

Secondary suspect: PR #90612 — "Renamed .website to .web for simplicity" (merged May 14). This renamed ValidateLoginPage/index.website.tsxindex.web.tsx and updated the webpack extension resolution order. If the bundler resolution has any edge case, the validate login page's post-auth redirect could malfunction. However, verification shows all files were properly renamed.

Lower probability candidates:

  • PR #90489 — "Fix bottom tab bar incorrectly visible on cold-start to a deep screen" (merged May 14). Only touches getFocusedLeafScreenName.ts (2 files), unlikely to cause NOT_FOUND.
  • PR #90523 — "Send log when redirecting to sign in after revoking current device" (merged May 13). Single logging addition, no navigation logic change.
Investigation details

Auth transition flow (why this matters)

After logout → refresh → magic code login, the transition works like this:

  1. signInWithValidateCode() sets SESSION.authToken in Onyx
  2. useIsAuthenticated() returns true, triggering AppNavigator to swap PublicScreensAuthScreens
  3. React Navigation calls getRehydratedState to reconcile old state with new screen config
  4. useLinking syncs browser URL with the new navigation state

If getRehydratedState or getStateForAction produces a state where the browser URL doesn't match any route in the linking config, the wildcard '*' catches it → NOT_FOUND screen.

Why PR #90044 is suspicious

The key change is at addRootHistoryRouterExtension.ts:92-96:

// Default path (previously just returned rehydrated state):
const rehydrated = rehydrate(newState, configOptions);
return applyRevealPaddingOffset(state, rehydrated);

applyRevealPaddingOffset counts leading reveal padding sentinels in state.history and re-injects them into the new state. If sentinels leak across the auth boundary, useLinking would compute wrong historyDelta values → browser history corruption → unmatched URL → NOT_FOUND.

Additionally, the pendingReveal closure variable (line 27) persists across navigation actions and could carry stale state through the navigator swap.

What would confirm this

Note: PR #89750 ("jump to existing workspaces tab") merged today (May 18), outside the 3-5 day window. It also touches NavigationRoot.tsx and workspace navigation — worth investigating separately if the regression started today rather than 3-5 days ago.


Next Steps: Reply with @MelvinBot <your feedback> to refine this analysis.

@MelvinBot
Copy link
Copy Markdown
Contributor

Analysis

Root cause: openReportFromDeepLink captures isAuthenticated = false on cold start (before login). After the user signs in via magic code, waitForUserSignIn() resolves, hasCompletedGuidedSetupFlowSelector(val) returns true for an existing account, and handleDeeplinkNavigation() fires. The stale isAuthenticated = false bypasses the early-return guard at line 326, and since route is '' (empty string from the base URL — no actual deep link), Navigation.navigate('' as Route) is called at line 347, which doesn't match any defined route and falls through to [SCREENS.NOT_FOUND]: '*'.

Why removing hasCompletedGuidedSetupFlowSelector(val) || fixes it: Without it, only isAnonymousUser() controls whether handleDeeplinkNavigation() runs. For a signed-in user this is false, so the buggy navigation path is never reached.

Recommended fix (no revert needed): Guard against empty routes inside handleDeeplinkNavigation. Add this check after the Onyx disconnect at line 307:

const handleDeeplinkNavigation = () => {
    Onyx.disconnect(connection);

    // No meaningful deep link to navigate to (e.g., cold start from base URL).
    // Normal post-login navigation is handled by react-navigation's AuthScreens.
    if (!route) {
        return;
    }

    // ... rest of existing logic unchanged

This prevents navigation to empty route on cold-start login while preserving actual deep link handling (e.g., r/123, settings/profile, etc., which are all truthy strings).

Data flow
  1. User logs out → refreshes browser → URL is base URL (e.g., https://new.expensify.com/)
  2. DeepLinkHandler calls Linking.getInitialURL() → gets base URL → isCurrentlyAuthenticated = hasAuthToken() = false
  3. openReportFromDeepLink(url, ..., false, ...) is called → route = getRouteFromLink(url) = '', reportID = null
  4. InteractionManager.runAfterInteractionswaitForUserSignIn() blocks until login
  5. User enters magic code → signs in → waitForUserSignIn() resolves
  6. NVP_ONBOARDING fires → hasCompletedGuidedSetupFlowSelector(val) = true (existing account)
  7. handleDeeplinkNavigation() runs → disconnects Onyx → passes all guards (stale isAuthenticated = false, shouldSkipDeepLinkNavigation('') = false, '' === 'home' = false)
  8. navigateHandler()reportID is falsy → Navigation.navigate('' as Route) → NOT_FOUND

Next Steps: Reply with @MelvinBot implement this to create a draft PR, or @MelvinBot <your feedback> to refine this analysis.

@mkhutornyi mkhutornyi closed this May 18, 2026
@mkhutornyi mkhutornyi deleted the revert-89750-fix/navigate-to-workspaces-tab branch May 18, 2026 16:39
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.

2 participants