Skip to content

fix: multi-session clerk bug#1374

Open
ifroshty wants to merge 9 commits into
mainfrom
fix/clerk-multisession-cross-tab-cookie-race
Open

fix: multi-session clerk bug#1374
ifroshty wants to merge 9 commits into
mainfrom
fix/clerk-multisession-cross-tab-cookie-race

Conversation

@ifroshty

@ifroshty ifroshty commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Summary

Fixes a Clerk multi-session bug where a user signed into two accounts in the same browser would intermittently see their profile/character switch to the other account ("I'm on my main, and it sometimes changes my profile to my alt").

Why / Root Cause

Under Clerk multi-session, the active session is per-tab in memory, but the __session cookie the server reads is shared across all browser tabs and reflects whichever tab is currently focused.

Our tRPC client authenticated with that ambient cookie only (no token), so a background request from a non-focused tab (the 5-minute profile.getUser refetch, or a Pusher-triggered getUser.invalidate()) authenticated server-side as whichever account last wrote the cookie (the alt). Because profile.getUser was queried with no account in its React Query key and a global staleTime: Infinity, that wrong-account response was written into the tab's cache and pinned there. The same ambient-cookie path also affected mutations, not just the displayed profile.

Verified against the installed @clerk/backend source: authenticateRequest is header-first. When an Authorization Bearer token is present it is used and the cookie/handshake path is skipped. getToken() returns the calling tab's own in-memory session token, independent of the shared cookie.

What Was Implemented

1. Per-tab token on every tRPC request (the fix)

  • New app/src/app/_trpc/authHeaders.ts with buildClerkRequestHeaders(token, isMultiSession): returns { Authorization: Bearer <token> } when the tab has its own token (and never Bearer null); otherwise a fail-closed marker (see PhpMyAdmin container in docker-compose - $10 reward #3) or {}.
  • app/src/app/_trpc/Provider.tsx: TrpcClientProvider reads useAuth().getToken and useClerk() (kept in refs so the once-created client always uses the latest values) and attaches headers via httpBatchLink's async headers(). Because Clerk authenticates header-first, the server now resolves this tab's session instead of the shared cookie — for queries and mutations.

2. Per-account scoping + server guard (defense-in-depth)

  • New app/src/server/api/viewerGuard.ts with assertViewerMatchesSession(sessionUserId, viewerId?), which throws FORBIDDEN when the client-asserted account does not match the server-authenticated session. Identity for the actual work is always taken from ctx.userId; viewerId is only checked, never trusted.
  • profile.getUser gains an optional { viewerId } input and calls the guard. The React Query cache is now scoped per account.
  • Client call sites pass the tab's Clerk id via a small getUserQueryInput(userId) helper: UserContext.tsx (useQuery plus two optimistic setData) and PusherHandler.tsx (setData). The ~120 argless invalidate() / cancel() calls match by key-prefix and are unchanged.
  • Provider.tsx handleTrpcError silently ignores the transient "Viewer/session mismatch" guard error (matched by message and FORBIDDEN code), since it self-resolves on the next refetch and is never user-facing.

3. Fail-closed when the per-tab token cannot be fetched (defense-in-depth)

getToken() yields no usable token in two cases: it returns null when the tab has no active session, and it throws (e.g. ClerkOfflineError) on an offline / token-refresh blip. Either way, falling back to the shared cookie could authenticate as another tab's account. Now:

  • Provider.tsx headers() routes both the null and the thrown path through the same no-token branch of buildClerkRequestHeaders. It sends the x-tnr-auth-required marker only when the browser has more than one signed-in session (client.signedInSessions, which excludes ended/expired/replaced sessions); a single-session browser falls back to the cookie (its own account), so ordinary network blips do not cause needless auth failures for the common single-account case.
  • New app/src/server/api/authContext.ts with resolveAuthedUserId(sessionUserId, { hasAuthorizationHeader, tabAuthFailed }): returns null (treat as unauthenticated) when the marker is present and no Authorization header arrived, instead of authenticating via the shared cookie. Wired into the tRPC context in trpc.ts (the marker is detected by header presence).

4. Tests

  • app/tests/app/_trpc/authHeaders.test.ts: buildClerkRequestHeaders — Bearer format, never Bearer null, fail-closed marker when no token + multi-session, and no marker for a single-session browser.
  • app/tests/server/api/viewerGuard.test.ts: match passes, mismatch throws FORBIDDEN, no assertion when no viewerId.
  • app/tests/server/api/authContext.test.ts: normal request passes through, Authorization-header-present passes through, marker-without-header fails closed to null, no-session is null.

Files Changed

File Change
app/src/app/_trpc/authHeaders.ts new
app/src/app/_trpc/Provider.tsx modified
app/src/server/api/viewerGuard.ts new
app/src/server/api/authContext.ts new
app/src/server/api/trpc.ts modified
app/src/server/api/routers/profile.ts modified
app/src/utils/UserContext.tsx modified
app/src/layout/PusherHandler.tsx modified
app/tests/app/_trpc/authHeaders.test.ts new
app/tests/server/api/viewerGuard.test.ts new
app/tests/server/api/authContext.test.ts new
app/package.json / app/bun.lock modified (test-only devDependency pin, see Testing Steps)

Breaking Changes

None.

  • profile.getUser's new input is optional, so all existing callers (including server-side createCaller / MCP usages that pass no input) keep working unchanged. The guard is a no-op when no viewerId is supplied.
  • No database / schema migration. No public API or runtime-dependency change.
  • The only dependency change is a test-only devDependency pin (@vitejs/plugin-react ^6^5) described under Testing Steps.

Conformance

The approach was validated against current official docs (Clerk v7, tRPC v11, TanStack Query v5, React 19): the getToken() + Authorization: Bearer pattern with a fail-closed marker is exactly what Clerk prescribes for multi-tab background fetches; async headers() on the terminating httpBatchLink, optional .input for cache-keying with ctx-derived identity, and per-account query keys are all idiomatic for their respective libraries.

Testing Steps

Automated (from repo root)

make typecheck   # passes, 0 errors
make lint        # clean
cd app && bunx vitest run tests/app/_trpc/authHeaders.test.ts tests/server/api/viewerGuard.test.ts tests/server/api/authContext.test.ts

Note: this branch also pins @vitejs/plugin-react from ^6 to ^5. @vitejs/plugin-react@6 requires vite 8, but vitest@4 uses vite 7, which broke the test runner at config load (vite/internal not exported). ^5 peers vite 7, so vitest / make test run normally again. This is a test-only devDependency change.

Manual: confirm the fix is wired (single account, any instance)

  1. Sign in, open DevTools and go to the Network tab.
  2. Inspect any /api/trpc request and check Request Headers; it now carries Authorization: Bearer .... (Previously there was no Authorization header.)

Manual: behavioral repro (requires a Clerk instance with multi-session enabled)

  1. Tab A: sign in as account A, sit on /profile.
  2. Tab B: sign in as account B (same browser, Clerk multi-session).
  3. Focus Tab B, leave Tab A idle and do not interact with it.
  4. Expected (fixed): Tab A stays account A. Before this change it would flip to B on the next background getUser.
  5. To trigger quickly instead of waiting on the 5-minute timer, temporarily set refetchInterval from 300000 to 3000 in app/src/utils/UserContext.tsx, reproduce, then revert.

Note: multi-session is a per-instance Clerk Dashboard setting and differs between development and production instances. In single-session mode the second sign-in is redirected away, so the bug (and this repro) only occur where multi-session is enabled.

Screenshots

N/A. No visual/UI changes; the fix is in request authentication and query caching.

Evaluated and intentionally not included

  • SSR: auth() / currentUser() in server components reads the shared cookie. Verified this is not an exposure here: initialIsSignedIn is an account-agnostic boolean used only for the pre-hydration layout shell, and the only server components calling currentUser() are forum pages that render on foreground navigation (focused tab = correct cookie), use force-dynamic (so prefetch does not run their data fetch), and surface only public forum content. There is also no clean way to pass a per-tab bearer token to a server component during document render, so no change was made.
  • Uploadthing: api/uploadthing/core.ts is still cookie-authenticated and does not traverse the tRPC link. Uploads are foreground/click-driven (the focused tab's cookie is the correct account), so the cross-account precondition is essentially never met. Pre-existing and out of scope for this PR.

License

By making this pull request, I confirm that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the Studie-Tech ApS organization has the copyright to use and modify my contribution for perpetuity.

Summary by CodeRabbit

  • New Features
    • Added per-tab Clerk session pinning to keep authentication and user data scoped to the pinned viewer/account.
    • Updated profile user lookup to accept an optional viewer id and enforce viewer/session match.
  • Bug Fixes
    • Improved fail-closed handling for multi-session per-tab auth failures to avoid incorrect session fallback.
    • Reduced user-facing noise by suppressing transient viewer/session mismatch errors.
    • Scoped battle-related profile cache updates to the current pinned viewer/account.
  • Tests
    • Added coverage for header composition, multi-session detection, fail-closed auth resolution, viewer/session enforcement, and session pinning helpers.
  • Chores
    • Updated the Vite React plugin version.

@vercel

vercel Bot commented Jun 20, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
the-ninja-ai Ready Ready Preview, Comment Jun 29, 2026 10:18pm
tnr Ready Ready Preview, Comment Jun 29, 2026 10:18pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)

Walkthrough

Adds Clerk per-tab session pinning for tRPC auth, server-side fail-closed user resolution, viewer/session matching checks, scoped viewer cache updates, and a Vite React plugin version change.

Changes

Clerk Multi-Session tRPC Auth

Layer / File(s) Summary
Session pinning utilities
app/src/app/_trpc/sessionPin.ts, app/tests/app/_trpc/sessionPin.test.ts
sessionPin.ts stores and resolves the pinned Clerk session id, and its tests cover storage behavior and pin-decision transitions.
Session pin provider and layout
app/src/app/_trpc/SessionPinProvider.tsx, app/src/app/layout.tsx
SessionPinProvider initializes pinned-session state, exposes pinned-session helpers, keys the subtree on the pinned session, and layout.tsx wraps the app with that provider.
Auth headers and client request flow
app/src/app/_trpc/authHeaders.ts, app/src/app/_trpc/Provider.tsx, app/tests/app/_trpc/authHeaders.test.ts
authHeaders.ts defines the Clerk request-header rules and multi-session detection, Provider.tsx builds headers from pinned-session refs and suppresses viewer mismatch errors, and the header tests cover token, fail-closed, and session-count cases.
Server auth resolution and viewer guard
app/src/server/api/authContext.ts, app/src/server/api/trpc.ts, app/src/server/api/viewerGuard.ts, app/src/server/api/routers/profile.ts, app/tests/server/api/authContext.test.ts, app/tests/server/api/viewerGuard.test.ts
authContext.ts resolves the authenticated user id with fail-closed behavior, trpc.ts derives context user ids from request headers, viewerGuard.ts enforces viewer/session matching, and profile.ts applies the guard to getUser. Tests cover the auth resolver and viewer guard branches.
Viewer-scoped cache updates
app/src/utils/UserContext.tsx, app/src/layout/PusherHandler.tsx
UserContext.tsx reads the pinned Clerk user id, scopes profile.getUser queries and optimistic cache writes by { viewerId }, adjusts Sentry user context, and PusherHandler.tsx scopes battle updates to the current viewer.

Vite Plugin Dependency Update

Layer / File(s) Summary
Vite React plugin version downgrade
app/package.json
app/package.json changes @vitejs/plugin-react from ^6.0.1 to ^5.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

size:M

Suggested reviewers

  • MathiasGruber

Poem

🐇 I pinned my tab with a hop and a cheer,
No shared-cookie slipperiness sneaks in here.
If viewer and session don’t quite align,
The guard says “nope” in a very firm sign.
Hop-hop! The cache now knows who it is.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the main change: a Clerk multi-session bug fix.
Description check ✅ Passed The description follows the template and includes the implementation, rationale, testing, screenshots, breaking changes, and license sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/clerk-multisession-cross-tab-cookie-race

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the size:M 30-99 effective changed lines (test files excluded in mixed PRs). label Jun 20, 2026
@github-actions github-actions Bot added size:L 100-499 effective changed lines (test files excluded in mixed PRs). and removed size:M 30-99 effective changed lines (test files excluded in mixed PRs). labels Jun 20, 2026
@greptile-apps

greptile-apps Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge. The fix is additive: new optional input on getUser, new context provider, new headers on the tRPC link. All existing call sites continue to work unchanged.

All three defense layers have dedicated unit tests. The decidePinnedSession logic handles first load, stale storage, momentarily-empty sessions list, and auth-route deferral — all verified by named test scenarios. Server-side changes are minimal and non-breaking.

No files require special attention. The most complex new file is SessionPinProvider.tsx, but the sessionPin.test.ts suite covers all branch cases.

Important Files Changed

Filename Overview
app/src/app/_trpc/SessionPinProvider.tsx New provider that pins each browser tab to a specific Clerk session via sessionStorage. Correctly handles edge cases: stale pins, momentarily-empty sessions list, auth-route deferral.
app/src/app/_trpc/sessionPin.ts Pure helpers for the session pin: sessionStorage I/O, resolvePinnedSession, isAuthRoute, and the central decidePinnedSession decision function. Fully unit-tested.
app/src/app/_trpc/Provider.tsx Adds async headers() to httpBatchLink to attach the pinned session Bearer token; fall-through error adds Sentry breadcrumb. Mismatch-error handler samples 5% to Sentry.
app/src/app/_trpc/authHeaders.ts Isomorphic module keeping client-filter and server-throw message in lockstep.
app/src/server/api/viewerGuard.ts Defense-in-depth guard: throws FORBIDDEN when viewerId doesn't match ctx.userId. Identity always from session, never from client-asserted id.
app/src/server/api/authContext.ts Fail-closed resolver: returns null when the tab sent x-tnr-auth-required but no Authorization header arrived.
app/src/server/api/trpc.ts Wires resolveAuthedUserId into the tRPC context. No behavioral change for single-session requests.
app/src/server/api/routers/profile.ts Adds optional { viewerId } input to getUser; .min(1) prevents empty-string guard bypass.
app/src/utils/UserContext.tsx Switches identity to pinnedUserId, scopes React Query cache key per-account. Sentry email guard prevents attaching another account's email.
app/src/layout/PusherHandler.tsx setData for Pusher battle-status update scoped to the pinned account's cache key.

Reviews (7): Last reviewed commit: "fix: route-gating and DRY" | Re-trigger Greptile

Comment thread app/src/app/_trpc/Provider.tsx Outdated
Comment thread app/src/app/_trpc/Provider.tsx Outdated
coderabbitai[bot]

This comment was marked as outdated.

@theeneon theeneon added the 40 SS label Jun 20, 2026
Comment thread app/src/layout/PusherHandler.tsx Outdated
@MathiasGruber

Copy link
Copy Markdown
Collaborator

/tnr-review-now

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

@github-actions github-actions Bot added size:XL 500-999 effective changed lines (test files excluded in mixed PRs). and removed size:L 100-499 effective changed lines (test files excluded in mixed PRs). labels Jun 24, 2026
coderabbitai[bot]

This comment was marked as outdated.

@MathiasGruber

Copy link
Copy Markdown
Collaborator

/tnr-review-now

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Scope covered

  • Reviewed the PR’s multi-session/auth changes in source to target testing: session pinning, per-tab auth headers, fail-closed auth resolution, and profile.getUser viewer/session matching.
  • Validated the preview deployment as a player using headless Chromium with two provisioned accounts.
  • Covered:
    • First-account login flow
    • Second-account login in a separate tab
    • Cross-tab multi-session behavior
    • Reload persistence after both sessions existed

Test steps executed

  1. Opened the preview deployment and confirmed it loaded.
  2. Queried the live preview’s village list because the village names in the prompt did not exist on this deployment.
  3. Provisioned two test users through the AI test-user broker:
    • reviewa / username tnrprevi9536 / village Tsukimori
    • reviewb / username tnrprevicb86 / village Akasumi
  4. Logged into tab 1 with reviewa’s Clerk ticket and confirmed redirect to /profile.
  5. Opened tab 2 and logged in with reviewb’s Clerk ticket.
  6. Verified tab 2 still rendered reviewa’s profile instead of reviewb’s.
  7. Inspected in-browser Clerk state on tab 2:
    • 2 active sessions existed
    • Clerk activeUserId was reviewb
    • sessionStorage['tnr-pinned-clerk-session-id'] pointed to reviewa
  8. Reloaded tab 1 and confirmed it stayed on reviewa.
  9. Reloaded tab 2 and confirmed it still incorrectly stayed on reviewa.

Findings (pass/fail, bugs, risks)

  • Fail: the core multi-session fix is not working in the second tab.
    Repro:

    1. Sign into user A in tab 1.
    2. Open a new tab and sign into user B.
    3. Expected: tab 2 shows user B.
    4. Actual: tab 2 shows user A (tnrprevi9536, Tsukimori).
  • Pass: the original tab remains stable after a second session is added.
    Tab 1 correctly stayed on user A after both sessions existed, including after reload.

  • Risk: the UI/account identity can diverge from Clerk’s active account.
    In the broken tab, Clerk reported user B as active while the app still rendered user A. From a player perspective, that is high risk because actions can be taken while the visible/account context is misleading.

Screenshot index

  • tab1-reviewa-profile.png — Tab 1 after signing into reviewa; profile shows tnrprevi9536 / Tsukimori
  • tab2-reviewb-expected-userb-got-usera.png — Tab 2 after signing in with reviewb’s ticket; page still shows reviewa

Recommendation

Needs follow-up.


Screenshots

tab1 reviewa profile
tab1 reviewa profile

tab2 reviewb expected userb got usera
tab2 reviewb expected userb got usera


Run: View workflow run

@MathiasGruber MathiasGruber left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified the prior round of feedback against the current branch. One item remains unaddressed.

Comment thread app/src/layout/PusherHandler.tsx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

"Completed" label removed

This PR has 1 unresolved review thread(s). The "Completed" label has been automatically removed.

Please resolve all review threads before re-applying the label.

Unresolved threads:

  • @MathiasGruber: Issue: The cache-key shape { viewerId: userId } is still duplicated here. `Use...

@MathiasGruber MathiasGruber left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Build / test runtime

Comment thread app/tests/app/_trpc/sessionPin.test.ts Outdated

@MathiasGruber MathiasGruber left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Audit of prior feedback against the rebased branch: all 10 previously-raised review threads are addressed. However, the /tnr-review-now finding from 2026-06-28 still reproduces, and there is one DRY contract worth tightening.

Comment thread app/src/app/_trpc/SessionPinProvider.tsx Outdated
Comment thread app/src/server/api/viewerGuard.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

"Completed" label removed

This PR has 2 unresolved review thread(s). The "Completed" label has been automatically removed.

Please resolve all review threads before re-applying the label.

Unresolved threads:

  • @MathiasGruber: Critical: the multi-session fix does not hold when a user signs in a second ac...
  • @MathiasGruber: The literal "Viewer/session mismatch" is a contract between this guard and the...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

40 SS Completed size:XL 500-999 effective changed lines (test files excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants