Skip to content

fix(auth): eliminate redirect loops in ensureAuthBootstrapped#878

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/improve-ensure-auth-bootstrapped
Open

fix(auth): eliminate redirect loops in ensureAuthBootstrapped#878
Copilot wants to merge 3 commits into
mainfrom
copilot/improve-ensure-auth-bootstrapped

Conversation

Copilot AI commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

The redirectUserGuard was never consumed after firing, so every subsequent null event from onAuthStateChanged would re-push the redirect snapshot — causing bounce loops back to the sign-in screen. Additionally, the previous onAuthStateChanged listener was never unsubscribed before attaching a new one, allowing accumulation across HMR reloads and test teardown/retry cycles.

Changes

  • One-shot guard consumption — guard state (redirectUserGuard, redirectUserSnapshot, redirectUserGuardSetAt) is cleared atomically on first use, so future null events propagate naturally instead of re-triggering the snapshot path
  • Time-bound TTLREDIRECT_GUARD_TTL_MS = 10_000 as a belt-and-suspenders cutoff; the guard cannot fire after 10 s regardless of one-shot logic
  • Listener cleanup before re-attachunsubscribeOngoing?.() is called before every onAuthStateChanged call, preventing listener accumulation on HMR / teardown+retry runs
  • Unconditional guard clear in normal path — previously only cleared when signedInUser was truthy; now cleared unconditionally so stale/expired guard state can never linger
  • redirectUserGuardSetAt propagated to resetsclearRedirectAuthGuard and teardownAuthBootstrapForTests both reset the new timestamp field
if (!signedInUser && redirectUserGuard && redirectUserSnapshot && guardAge < REDIRECT_GUARD_TTL_MS) {
  // Consume the guard immediately — do not re-trigger on future null events.
  const snapshot = redirectUserSnapshot;
  redirectUserGuard = false;
  redirectUserSnapshot = null;
  redirectUserGuardSetAt = null;
  notifyUserListeners(snapshot);
  // ...resolve promise
  return;
}

// Normal path: clear any stale guard state unconditionally.
redirectUserGuard = false;
redirectUserSnapshot = null;
redirectUserGuardSetAt = null;

The persistence → getRedirectResultonAuthStateChanged ordering is preserved.

Original prompt

Improve ensureAuthBootstrapped in vessel/src/lib/authBootstrap.ts to eliminate remaining redirect loops.

Current problems:

  • onAuthStateChanged listener is never unsubscribed in production (only in tests)
  • redirectUserGuard can stay active and cause repeated null → snapshot pushes
  • Multiple bootstrap calls or HMR can accumulate listeners
  • Race conditions between resolveFirebaseRedirectResult and the listener still cause bouncing to sign-in screen

Desired behavior:

  • Always clean up previous onAuthStateChanged listener before setting a new one
  • Make the redirect guard time-bound or more strictly one-shot
  • Ensure the bootstrap promise only resolves once
  • Keep the existing persistence → getRedirectResult → listener ordering
  • Clear sessionStorage redirect pending flag reliably
  • Add clear comments explaining the guard logic

Implement a cleaner, more defensive version of ensureAuthBootstrapped and related functions while preserving the overall architecture.

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
shipyard Ready Ready Preview, Comment Jun 28, 2026 4:28pm

- Add REDIRECT_GUARD_TTL_MS (10 s) constant for time-bound guard expiry
- Track redirectUserGuardSetAt timestamp when guard is armed
- Unsubscribe previous onAuthStateChanged listener before attaching a new
  one, preventing listener accumulation on HMR / teardown+retry runs
- Make redirect guard strictly one-shot: consume (clear) on first use so
  repeated null events never re-trigger the snapshot path
- Also enforce TTL as a belt-and-suspenders cutoff
- Clear all guard state unconditionally in the normal auth-event path
- Update clearRedirectAuthGuard and teardownAuthBootstrapForTests to
  reset redirectUserGuardSetAt
- Add explanatory comments throughout guard logic
Copilot AI changed the title [WIP] Refactor ensureAuthBootstrapped to eliminate redirect loops fix(auth): eliminate redirect loops in ensureAuthBootstrapped Jun 28, 2026
Copilot AI requested a review from DHCross June 28, 2026 16:00
@DHCross DHCross marked this pull request as ready for review June 28, 2026 16:16
Rebase PR #878 onto current main while preserving REDIRECT_PENDING_MAX_AGE_MS,
isFreshGoogleRedirectPending, and REDIRECT_INCOMPLETE_MESSAGE behavior from main.

Add one-shot redirectUserGuard consumption with TTL, unsubscribe previous
onAuthStateChanged before attaching a new listener, and comprehensive tests.

Co-authored-by: DHCross <DHCross@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

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.

3 participants