Skip to content

Fix sign-in links, the OAuth callback and per-account state (T2: AUTH-01, 18, 17, 02, 19, 06, 07, 13) - #202

Merged
Programcode merged 15 commits into
mainfrom
fix/t2-auth-callback
Oct 23, 2026
Merged

Programcode merged 15 commits into
mainfrom
fix/t2-auth-callback

Conversation

@Programcode

Copy link
Copy Markdown
Owner

Fixes the sign-in bugs from the 22 Oct QA report on prod: AUTH-01, 18, 17, 02, 19, 06, 07 and 13. Most of the change is in AuthProvider, plus a new /auth/callback route.

Before you merge: Supabase Auth redirect allowlist

These are not changed by this PR. Add them in the Supabase dashboard (Authentication → URL Configuration → Redirect URLs):

  • Production (uziwdtkzofptihahfooo): https://debate.pi-stack.org/auth/callback**
  • Staging (brifeyypjauyusmkptkx): https://debatelab-staging.vercel.app/auth/callback**

The ** matters. Redirects now carry ?sb_flow_id=… and sometimes ?next=…, and an exact entry stops matching once a query is added. GoTrue also accepts any redirect on the Site URL's own host, so if prod's Site URL is https://debate.pi-stack.org the callback works before the entry is added. Until then, a host that isn't the Site URL host (previews, the tailnet) falls back to the Site URL. The existing preview wildcards (…vercel.app/**, tailnet :91*/**) already cover /auth/callback.

What changed, per issue

AUTH-01 (High), the emailed link lands on the homepage signed out. I reproduced this first in the local stack on main's code by sending a code, forcing a 429 on Resend, then opening the emailed link. It failed with AuthPKCECodeVerifierMissingError, swallowed as [Auth] init failed, and ended on / signed out. Now signInWithOtp sets emailRedirectTo to /auth/callback. The redirect carries this sign-in's own flow id, and the exchange uses that flow's verifier (see AUTH-18). A failure shows a localized message on /login instead of a console warning.

AUTH-18 (Medium), every flow shares one PKCE verifier. The client sets experimental.appendPkceFlowIdToRedirects (auth-js 2.114). The exchange passes { flowId } from sb_flow_id, so a failed resend or a Google sign-in in another tab only drops its own verifier.

AUTH-17 (Medium), /login flashes after Google, and a first sign-in can stick. Google and email links return to /auth/callback, which RequireAuth does not guard. AuthProvider holds loading until the code exchange answers. The page shows "Signing you in…" and then opens next (default /app).

How the callback works:

  • While the exchange has no answer, the provider ignores auth-js's session events, including another account's stored session and the exchange's own event. Only the exchange's answer decides which account opens. That is the new account on success, or the stored account (after the usual server check) on a definitive failure.
  • On a 15 s timeout the code stays in the URL. The page offers Try again (a reload that reuses the code) and Back to sign-in.
    • From the timeout on, auth events are handled again, so a sign-in on /login works.
    • The late answer is still final. A success opens the app by itself, and a failure gives its real reason.
  • The stored-session user lookup and a dead session's sign-out are each bounded at 4 s, so nothing can hold the spinner forever.

AUTH-02 (Medium), a used or expired link shows nothing. error/error_code in the callback URL (query or fragment) and failed exchanges map to localized messages (EN/ES/HU):

  • otp_expired and flow-state errors: "This sign-in link has expired or was already used. Send a new code."
  • A missing verifier: "This sign-in link only works in the browser where you asked for it. Send a new code here."
  • Anything else: "Sign-in didn't finish. Try again."

They show on /login. If the browser still has another session, that account opens and the message shows as a toast.

Links already in inboxes. /?code=, /app?code= and error-only variants are moved onto /auth/callback before the first render (mountApp). They still sign in, which I checked in the real stack. They carry no flow id, so an old link plus a failed resend still fails, now with the "only works in the browser where you asked for it" message instead of silently.

AUTH-19 (Low), sign-out is global. signOut() uses { scope: 'local' }.

AUTH-06 (Medium), the next account lands on the previous account's last page. There was no persisted route store. The leak was the from that RequireAuth puts in the /login history state when the session ends. RequireAuth now stamps that state with the account that was signed in, and LoginPage honors from only for that same account. A different account lands on /app; the same account returning after an expired session goes back to its page.

AUTH-07 (Medium), the tour shows only for the first account and blocks taps on phones. The onboarding store is per account (completedUserIds, persist version 1, validated on every rehydrate). The old ownerless "done" flag (v0) goes to the next account that signs in, so existing users don't see the tour again, and every other account sees it once. Below lg (where the header menu is the navigation) the tour has no overlay, and its tooltip sits under the header and the open menu. I checked that the menu opens and navigates while the tour shows.

AUTH-13 (Nit), the code is in the email subject. supabase/config.toml adds [auth.email.template.confirmation] (a first email sign-in sends this one) with the subject "Your DebateLab sign-in code" and a code-only body, matching magic_link. There is no invite flow (inviteUserByEmail isn't used), so no invite template. Prod templates are being changed separately; nothing here touches Supabase settings.

Also, vercel.json rewrites /auth/callback to the app shell with noindex, and theme-init.js treats it as a private app-shell route. docs/architecture.md describes the callback. The verify-debatelab skill has two new recipes and updated feature docs.

Verification

Local only; GitHub Actions minutes are used up.

npm run verify on the final head f291919 passes (exit 0). It ran docs check, lint, typecheck, 500 unit tests, 30 collaboration tests, 406 edge tests, the build, the service worker check and 43/43 desktop @smoke e2e tests, with no flakes. Earlier heads had one unrelated flaky e2e each (free-tools timer, admin-announcements); both passed on retry.

Real app, verify-debatelab (local Supabase Auth + Mailpit), desktop-chromium and mobile-chromium, on the first version (b046055). All recipes/auth.spec.ts recipes pass (8/8):

  • auth-email-link: Send code, Resend forced to 429, then the emailed link (/verify?token=pkce_…, the link prod's email carries). It signs in on /app with /auth/callback in the path history and no /login. Reopening the used link shows the expired message on /login.
  • auth-switch-account: Sign out sends logout?scope=local. A second account signs in on the same /login page, lands on /app (not /app/settings) and gets its own tour. On mobile the menu opens and navigates to History while the tour shows.
  • recipes/current-debate.spec.ts (including the tour recipe), e2e/full-stack/first-run.spec.ts and e2e/full-stack/auth.spec.ts also pass.
  • An old-style /?code= link without a flow id still signs in. With a failed resend it ends on /login with the localized message. Before this PR it ended on / silently.
  • A new email user receives subject "Your DebateLab sign-in code" with the code only in the body.

Browser checks of the timeout paths on the final code (dev server with real auth-js, desktop and mobile, 4/4 pass). With /auth/v1/token stalled, the page stays on /auth/callback with code and sb_flow_id, shows the message, and Try again sends a new exchange. With /token answering after 18 s, the page shows the timeout and then opens /app/history by itself.

New unit tests cover:

  • the flow-id exchange, the loading hold (including another account's stored session), error mapping, URL cleanup timing, timeout and lookup bounds;
  • the next allowlist (open-redirect cases) and the legacy rewrite;
  • the account-switch landing page;
  • per-account onboarding and its migration and validation;
  • the callback page's toast and timeout states.

e2e/login.spec.ts now expects the callback redirect_to with a flow id, and its OTP mock matches the request's new ?redirect_to= query.

Not rerun on the final head. The review fixes after b046055 changed the callback's timeout, late-answer and event handling. I could not rerun the real-stack recipes on the final head, because another worktree held the shared local Supabase lock for over 90 minutes. Those paths are covered by unit tests and by the dev-server browser checks above, which use real auth-js. Run .claude/skills/verify-debatelab/bin/verify.sh drive recipes/auth.spec.ts on the final head before merging.

Review

Three Codex rounds on codex_cpa / gpt-6.1-sol, xhigh. The fourth hit Codex's 429 limit, so a fresh-context Claude (Opus) reviewer took over and rechecked each fix.

  • r1 (b046055), 4 findings, fixed in 3891981:
    • (M) a stored account's session event ended loading before another account's callback exchange settled;
    • (M) a failed link with a surviving session opened the app with no message, and any SIGNED_IN cleared the error;
    • (L) the onboarding store was not validated on rehydrate;
    • (L) a running tour could open for an account that had finished it.
    • Also moved RequireAuth's ref write into an effect, per docs/standards/async-and-state.md.
  • r2 (3891981), 1 finding, fixed in 6b26b63 + 42eb5a5: (M) the code was stripped from the URL before the exchange settled, so a reload lost a pending sign-in. It is now stripped only after success or a definitive failure, and kept on timeout.
  • r3 (1d718c7), 2 findings, fixed in d2b067a:
    • (M) after a timeout the page still navigated to /login and lost the code. It now shows a retry instead.
    • (M) the stored-session user lookup was unbounded during a callback. It now has a 4 s limit.
  • r4 (a84cd22): Codex returned 429 and the round was cancelled. The Claude reviewer found:
    • (M) a dead stored account could sign out or block the account a callback signed in;
    • (L/M) a surviving session hid the timeout retry, and a late success left a stale toast;
    • (L) a late failure reported the wrong reason;
    • (L) a dead session's sign-out had no time limit;
    • (nit) Back to sign-in dropped the deep link.
    • All fixed in 17e13f0.
  • Recheck of 17e13f0: (M) after a timeout, the stored account was checked while the late exchange could still sign someone in, so the wrong account could win. Fixed structurally in 7f1fc05, so only the exchange's answer signs an account in. Reading that commit, I found two of my own regressions and fixed them in the same commit. Google's Drive token and the previous account's Crossfire draft had only been handled by the listener.
  • Recheck of 7f1fc05: (M) after a timeout, auth events stayed ignored until the late answer, so a sign-in on /login was missed. (L) the stored-session restore could overwrite a newer sign-in. Both fixed in 074c9fe.
  • Recheck of 074c9fe: two Lows. The newer-sign-in guard now also covers the dead-session branch (f291919). The other is listed under known issues.

Known issues (Low)

  • A dead session's bounded sign-out keeps running. The bound stops loading waiting after 4 s, but auth-js's local sign-out continues. If its request hangs, then finishes after the user has signed in to another account, it removes that newer session. That needs a hung /logout, a new sign-in in between, and, for the second trigger, a tab switch during a callback timeout with a revoked but unexpired stored session. Before this PR, the same hang held the spinner forever. auth-js offers no clean hook to cancel it.
  • A late success overrides a sign-in made after the timeout. If the user signs in with a code after a callback timeout and the original exchange then succeeds, auth-js saves that account over theirs. App state and storage agree, so nothing mismatches.

Open / needs a manual check

  • Google sign-in end to end (consent screen, return to /auth/callback, a first-ever Google account) can't run locally or from automation. Check on staging or a preview once the allowlist entry exists.
  • The prod email link after the dashboard template change, opened on the same device and on a different device. The other-device case should show "only works in the browser where you asked for it".
  • Staging email templates. config.toml covers the local stack only. Staging needs the confirmation template applied the same way its magic_link one was.
  • Nothing to deploy. There are no migrations, no Edge Function changes, and no production settings changes.

🤖 Generated with Claude Code

Programcode and others added 11 commits October 22, 2026 20:50
…batch)

Google and the emailed sign-in link now return to /auth/callback, which
holds a loading screen until the code exchange settles and then opens the
deep link or /login. PKCE flow ids are appended to redirects and passed to
the exchange, so a failed resend or another tab's sign-in no longer deletes
the verifier a pending link needs. Auth errors in the callback URL and
failed exchanges show a localized message on /login instead of a console
warning. Links already in inboxes (/?code=, /app?code=) are moved onto the
callback route before the first render.

Sign-out uses local scope. After a sign-out, /login returns only the same
account to the page it left. The onboarding tour is tracked per account,
and on phones it no longer blocks the header menu. The local confirmation
email template is code-only with the code out of the subject.

Fixes AUTH-01, AUTH-18, AUTH-17, AUTH-02, AUTH-19, AUTH-06, AUTH-07, AUTH-13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ep link errors

- AuthProvider keeps loading while a callback exchange runs even when the
  browser's stored session (another account) is announced meanwhile, so the
  callback page never opens `next` as the wrong account.
- A callback error is no longer cleared by any SIGNED_IN (a session restore
  fires one). When the link fails but another session survives, the callback
  page opens that account and shows the failure as a toast.
- The onboarding store validates its stored copy on every rehydrate (merge),
  not only on migration.
- A tour left running never opens for an account that already finished it.
- RequireAuth records the last account in an effect, not during render
  (docs/standards/async-and-state.md).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex review r2: clearing `code` and `sb_flow_id` before the exchange meant
a reload during the spinner found no code and sent the user to /login with
an unused code and verifier. The URL is now cleaned once the exchange
succeeds, fails or times out; auth errors and empty callbacks are cleaned
at once, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A timeout is not a final answer: the code may still be good, so the URL
keeps it and a reload can retry, as main did before. Only a success or a
definitive failure cleans it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…okup

Codex review r3:
- After a timeout the provider kept the code, but the callback page still
  navigated to /login and lost it. A timeout now has its own error kind:
  the page stays on the callback URL and offers Try again (a reload that
  reuses the code) and Back to sign-in. A late success still opens the app.
  The provider stays the only owner of stripping callback parameters.
- confirmRestoredSession's user lookup had no deadline, and during a
  callback nothing else ends loading. It now times out after 4 s and, like
  any unanswered lookup, keeps the local session.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fresh review (Claude, replacing Codex r4, which hit a 429):
- While a callback signs an account in, the listener no longer checks or
  loads a session the browser already had. A dead stored account's lookup
  could sign out, or block, the account just signed in. init loads the
  outcome: the new account, or the stored one only if the callback fails.
  The blocked-session branch also respects callbackPending.
- After a timeout the exchange's late answer is final: a success clears the
  timeout notice and opens the app; a failure shows its real reason instead
  of a retry that would report a missing verifier. Either way the used code
  leaves the URL, and only while the callback page is still showing.
- The timeout page shows before an existing session would open the app, and
  its Back to sign-in keeps the deep link.
- A dead stored session's sign-out is bounded, so it cannot hold startup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up review: after a timeout, init restored and checked the stored
account while the late exchange could still sign someone else in. A late
success during that check got overwritten by the stored account (and the
new account's rows loaded under the old owner), and a dead stored account
could block the late success.

While the code exchange has no answer, even past the timeout, the
listener now ignores auth-js's events. init applies the answer itself:
the new account on success, or the stored account through the usual
server check once the exchange definitively failed. After a timeout
nothing is restored until the late answer arrives. A late failure sets
its error only while the callback page is still showing.

applySession now also does what the listener did for these sign-ins:
keep Google's provider token for Drive export and drop the previous
account's Crossfire draft.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…restore

Follow-up review of 7f1fc05:
- The listener ignored auth events until the slow exchange answered, so
  after the timeout a sign-in on /login was never seen (and never, if the
  request stalled for good). It now ignores them only until the exchange
  answers or times out. A late success arrives like any sign-in; it is
  applied directly only if a blocked restore dropped it. A late failure
  restores the stored session only if nobody signed in meanwhile.
- restoreStoredSession drops its result when a different account was
  applied while the stored one was being checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up review of 074c9fe (Low): the guard that keeps an account signed
in during the stored session's check ran only for a live stored account.
It now runs before the dead branch's drop as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Next included review available in 11 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 06b73de and da4638c.

📒 Files selected for processing: 35
  • .claude/skills/verify-debatelab/features/README.md
  • .claude/skills/verify-debatelab/features/auth-demo-login.md
  • .claude/skills/verify-debatelab/recipes/account.spec.ts
  • .claude/skills/verify-debatelab/recipes/auth.spec.ts
  • docs/architecture.md
  • e2e/login.spec.ts
  • public/theme-init.js
  • src/App.tsx
  • src/components/auth/RequireAuth.tsx
  • src/components/features/onboarding/OnboardingTour.tsx
  • src/hooks/use-onboarding.ts
  • src/lib/auth-callback.ts
  • src/lib/native.ts
  • src/lib/route-modules.ts
  • src/lib/seo/use-page-head.ts
  • src/lib/supabase/AuthProvider.tsx
  • src/lib/supabase/auth-context.ts
  • src/lib/supabase/client.ts
  • src/locales/en.json
  • src/locales/es.json
  • src/locales/hu.json
  • src/main.tsx
  • src/pages/AuthCallbackPage.tsx
  • src/pages/LoginPage.tsx
  • src/prerender/entry.tsx
  • src/test/auth/AuthProvider.behavior.test.tsx
  • src/test/auth/account-switch-route.test.tsx
  • src/test/auth/auth-callback-page.test.tsx
  • src/test/auth/auth-callback.test.ts
  • src/test/auth/onboarding-per-account.test.ts
  • src/test/pages/SharedPage.test.tsx
  • src/test/vercel-deployment-config.test.ts
  • supabase/config.toml
  • supabase/templates/confirmation.html
  • vercel.json
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

Comment @coderabbitai help to get the list of available commands.

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