Skip to content

fix(clerk-js): apply session tokens monotonically on a single tab#9030

Open
nikosdouvlis wants to merge 14 commits into
mainfrom
fix/clerk-js-monotonic-session-token
Open

fix(clerk-js): apply session tokens monotonically on a single tab#9030
nikosdouvlis wants to merge 14 commits into
mainfrom
fix/clerk-js-monotonic-session-token

Conversation

@nikosdouvlis

@nikosdouvlis nikosdouvlis commented Jun 29, 2026

Copy link
Copy Markdown
Member

Why
A stale edge-minted session token (older oiat header) could overwrite a fresher one on the same tab. The freshness guard only ran in the cross-tab broadcast receiver, so the __session cookie, Session.lastActiveToken, and the token cache each just took whatever token landed last. A slow or out-of-order edge read could revert a newer token.

What changed
Reuse the existing pickFreshestJwt comparison across all three stores so a token only moves forward in freshness. The token cache keeps its live entry monotonic and carries the prior token and its expiry forward. Session resolves getToken and lastActiveToken to the freshest of the fetched and cached token, and fromJSON won't adopt a staler or wrong-org token during hydration. The cookie service drops tokens for a different session/org or strictly staler than the same-context cookie.

Freshness is decided by oiat then iat, and a drop only happens when both tokens carry oiat and the incoming one is strictly older. A missing oiat or a different context always lets the token through, so this fails open.

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened monotonic session token handling across cookie updates, broadcast events, and background refreshes to prevent stale tokens from overwriting newer data.
    • Added context-aware dropping of stale/wrong-context tokens and improved safeguards for session lastActiveToken and token cache consistency, with fail-open behavior when freshness info is missing.
  • New Features

    • Exposed token freshness helpers (e.g., isStrictlyStalerJwt, pickFreshestOrIncoming, token oiat/sid/org extraction).
  • Tests

    • Added extensive coverage for monotonic guard, freshness utilities, cache race resolution, and session hydration edge cases.

… tab

A stale edge-minted session token (older oiat header) could overwrite a fresher
one on the same tab. The only freshness guard ran solely in the cross-tab
broadcast receiver, so the __session cookie, Session.lastActiveToken, and the
token cache each accepted whatever token arrived last, letting a slow or
out-of-order edge read revert a newer token.

Reuse pickFreshestJwt across all three stores:

- tokenCache.setInternal keeps the live entry's resolvedToken monotonic
  regardless of which resolver owns the slot, carries the prior token and its
  expiry forward, and derives expiry/refresh timers from the winner.
- Session.#fetchToken and #refreshTokenInBackground resolve to the freshest of
  the fetched token and the live cache entry, so coalesced waiters and the
  dispatch receive the winner; lastActiveToken never regresses to a
  strictly-staler token.
- Session.fromJSON reconciles an incoming last_active_token against the active
  cache slot, never adopting a strictly-staler or wrong-org token nor polluting
  the slot getToken reads.
- AuthCookieService.updateSessionCookie drops tokens for a different active
  session/org or strictly staler than the same-context cookie, failing open for
  tokens without oiat.

Freshness is decided by oiat then iat; drops happen only when both tokens carry
oiat and the incoming is strictly older, so a missing oiat or a different
context always lets the token through.
@changeset-bot

changeset-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8aaf690

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/electron Patch
@clerk/expo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Jul 1, 2026 2:46pm
swingset Ready Ready Preview, Comment Jul 1, 2026 2:46pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 29, 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
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds monotonic staleness checks across shared token helpers, session token caching, session hydration and refresh flows, and session cookie updates.

Changes

Monotonic Session Token Guard

Layer / File(s) Summary
tokenFreshness predicates and selectors
packages/clerk-js/src/core/tokenFreshness.ts, packages/clerk-js/src/core/__tests__/tokenFreshness.test.ts
Adds tokenOiat, tokenSid, tokenOrgId, normalizeOrgId, pickFreshestOrIncoming, and isStrictlyStalerJwt; tests cover the new helpers and richer JWT claim shapes.
SessionTokenCache monotonic resolve and broadcast
packages/clerk-js/src/core/tokenCache.ts, packages/clerk-js/src/core/__tests__/tokenCache.test.ts
setInternal reconciles resolved tokens with pickFreshestOrIncoming, preserves expiration metadata, rewrites async resolution against the live cache entry, and broadcasts the winner token; tests cover overwrite order, tie handling, carry-forward, and timer behavior.
Session freshness-gated token updates
packages/clerk-js/src/core/resources/Session.ts, packages/clerk-js/src/core/resources/__tests__/Session.test.ts
Session applies incoming last_active_token values through org-aware monotonic checks, skips stale token event resolution, and selects a winner token during fetch and background refresh; tests cover concurrency, org/template separation, hydration, and repeated deserialization.
AuthCookieService stale-token backstop
packages/clerk-js/src/core/auth/AuthCookieService.ts, packages/clerk-js/src/core/__tests__/clerk.test.ts
updateSessionCookie returns early for stale incoming session tokens after decoding JWTs and comparing session and org context; tests cover same-context freshness, cross-context filtering, fail-open cases, and null-token removal.
Changeset
.changeset/monotonic-session-token-guard.md
Marks @clerk/clerk-js for a patch release describing monotonic session token application.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 Fresh tokens hop, old ones pause,
The cache now guards its gentler laws.
Session cookies, swift and bright,
Pick the newest token right.
Hop hop—stale crumbs lose the race!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making clerk-js session token updates monotonic within a single tab.
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.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/clerk-js/src/core/resources/Session.ts (1)

212-242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the explicit void return type.

This new helper only mutates session/cache state; annotate it to satisfy the TypeScript return-type rule. As per coding guidelines, "**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs".

Proposed fix
-  `#applyIncomingLastActiveToken`(raw: SessionJSON['last_active_token'] | null) {
+  `#applyIncomingLastActiveToken`(raw: SessionJSON['last_active_token'] | null): void {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/resources/Session.ts` around lines 212 - 242, The
new helper `#applyIncomingLastActiveToken` currently relies on type inference, but
it should have an explicit void return type to satisfy the TypeScript
return-type rule. Update the method signature on Session to declare the return
type explicitly, keeping the existing cache/token mutation logic unchanged.

Source: Coding guidelines

packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)

2074-2082: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid defaulting the helper generic to any.

Use unknown as the default so callers either narrow or provide a concrete type. Based on learnings, internal test helpers may omit explicit return types, but the guideline still disallows unjustified any.

Proposed fix
-    function deferred<T = any>() {
+    function deferred<T = unknown>() {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/resources/__tests__/Session.test.ts` around lines
2074 - 2082, The deferred helper in Session.test.ts defaults its generic to any,
which violates the typing guideline. Update deferred<T = any>() to use unknown
as the default generic instead, and keep the resolve/reject/promise shape
unchanged so callers of deferred() in the Session.test suite must either narrow
the value or supply a concrete type via the deferred symbol.

Sources: Coding guidelines, Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/clerk-js/src/core/__tests__/clerk.test.ts`:
- Around line 908-922: Add a positive test for an organization-scoped token
whose org_id matches the active organization. In clerk.test.ts рядом with the
existing loadClerkWithSession/emitToken cases, create a matching org JWT and
assert document.cookie contains it, so the org-context guard in the token
handling flow still accepts valid active-organization tokens while rejecting
wrong-org ones.

In `@packages/clerk-js/src/core/auth/AuthCookieService.ts`:
- Around line 221-227: The token filtering in AuthCookieService should not
reject a target-session `TokenUpdate` during `Clerk.setActive` just because
`this.clerk.session?.id` has not been committed yet. Update the session check in
the logic around `tokenSid(incoming)` so it can accept the token for the session
being activated, and make sure the `setActive` flow that calls
`newSession.getToken()` before `#updateAccessors(newSession)` still allows the
new `__session` cookie to be written. Use the existing `tokenSid`, `tokenOrgId`,
and `normalizeOrgId` comparisons in `AuthCookieService` to adjust the condition
without breaking org filtering.

In `@packages/clerk-js/src/core/tokenCache.ts`:
- Around line 360-372: The current TokenCache merge path in tokenCache.ts
carries forward an existing resolved token and TTL, but the rejection handling
still removes that live entry when a replacement resolver fails. Update the
rejection path associated with the refresh/skipCache resolver so it does not
delete or overwrite the carried-forward entry when
`existing.entry.resolvedToken` and `existing.expiresIn` were preserved; instead,
keep the prior token/TTL intact and only clear the new pending resolution state.
Use the `resolvedToken` merge logic and the cache entry write/remove flow in
`TokenCache` to locate and adjust the failure path.

---

Nitpick comments:
In `@packages/clerk-js/src/core/resources/__tests__/Session.test.ts`:
- Around line 2074-2082: The deferred helper in Session.test.ts defaults its
generic to any, which violates the typing guideline. Update deferred<T = any>()
to use unknown as the default generic instead, and keep the
resolve/reject/promise shape unchanged so callers of deferred() in the
Session.test suite must either narrow the value or supply a concrete type via
the deferred symbol.

In `@packages/clerk-js/src/core/resources/Session.ts`:
- Around line 212-242: The new helper `#applyIncomingLastActiveToken` currently
relies on type inference, but it should have an explicit void return type to
satisfy the TypeScript return-type rule. Update the method signature on Session
to declare the return type explicitly, keeping the existing cache/token mutation
logic unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 101fbb04-b962-438e-aded-5ec66a2d39c0

📥 Commits

Reviewing files that changed from the base of the PR and between de70b28 and 98d0fcc.

📒 Files selected for processing (9)
  • .changeset/monotonic-session-token-guard.md
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/__tests__/tokenFreshness.test.ts
  • packages/clerk-js/src/core/auth/AuthCookieService.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/resources/__tests__/Session.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/tokenFreshness.ts

Comment on lines +908 to +922
it('drops a token for a different organization', async () => {
await loadClerkWithSession();

const wrongOrg = createJwtWithOiat(1000, 200, { org: 'org_other' });
emitToken(wrongOrg);
expect(document.cookie).not.toContain(wrongOrg);
});

it('applies a personal-workspace token (no org) for the active personal workspace', async () => {
await loadClerkWithSession();

const personal = createJwtWithOiat(1000, 200);
emitToken(personal);
expect(document.cookie).toContain(personal);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a positive active-organization token case.

The suite covers wrong-org rejection and personal-workspace acceptance, but not a token whose org_id matches the active organization. Add that case so the new org-context guard cannot regress into rejecting valid organization-scoped tokens. As per coding guidelines, “Unit tests are required for all new functionality” and should “Verify proper error handling and edge cases.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/__tests__/clerk.test.ts` around lines 908 - 922,
Add a positive test for an organization-scoped token whose org_id matches the
active organization. In clerk.test.ts рядом with the existing
loadClerkWithSession/emitToken cases, create a matching org JWT and assert
document.cookie contains it, so the org-context guard in the token handling flow
still accepts valid active-organization tokens while rejecting wrong-org ones.

Source: Coding guidelines

Comment thread packages/clerk-js/src/core/auth/AuthCookieService.ts Outdated
Comment thread packages/clerk-js/src/core/tokenCache.ts
#applyIncomingLastActiveToken only used the active-org cache slot as the
freshness baseline. When that slot was empty (evicted, or an in-flight fetch
leaves resolvedToken null) a strictly-staler or wrong-org last_active_token
from fromJSON was adopted, regressing lastActiveToken.

Fall back to the token already held as the freshness baseline when the cache
slot is empty, but only when that held token is same-context, so a token left
over from the previous org cannot veto a valid token for the active org. When
no same-context baseline exists, adopt the incoming token instead of clearing
lastActiveToken: a wrong-org or staler token is still caught downstream by the
cookie guard, whereas an empty lastActiveToken is treated as sign-out and drops
the session cookie.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)

2441-2447: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid widening the session to any for hydration calls.

Use a narrow test-only interface/helper for fromJSON instead of repeated (session as any) casts.

♻️ Suggested refactor
+      const hydrateSession = (target: unknown, json: SessionJSON): void => {
+        (target as { fromJSON(json: SessionJSON): void }).fromJSON(json);
+      };
+
-        (session as any).fromJSON(sessionJsonWith(high));
+        hydrateSession(session, sessionJsonWith(high));

As per coding guidelines, “No any types without justification in code review.”

Also applies to: 2470-2476, 2486-2486, 2502-2508

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/resources/__tests__/Session.test.ts` around lines
2441 - 2447, The test is repeatedly widening Session to any just to call
fromJSON, which should be replaced with a narrow test-only helper or interface
for hydration. Update Session.test.ts to introduce a typed helper or local
interface that exposes fromJSON, then use that instead of repeated (session as
any) casts in the affected hydration cases. Keep the change limited to the test
code and reference the Session.fromJSON call sites so the casts are removed
consistently.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/clerk-js/src/core/resources/__tests__/Session.test.ts`:
- Around line 2441-2447: The test is repeatedly widening Session to any just to
call fromJSON, which should be replaced with a narrow test-only helper or
interface for hydration. Update Session.test.ts to introduce a typed helper or
local interface that exposes fromJSON, then use that instead of repeated
(session as any) casts in the affected hydration cases. Keep the change limited
to the test code and reference the Session.fromJSON call sites so the casts are
removed consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b3dc1eb-50c8-45d3-a022-b71890480952

📥 Commits

Reviewing files that changed from the base of the PR and between a3082a3 and 570b749.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/resources/__tests__/Session.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/core/resources/Session.ts

…c-session-token

# Conflicts:
#	packages/clerk-js/src/core/tokenCache.ts
@pkg-pr-new

pkg-pr-new Bot commented Jun 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@9030

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@9030

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@9030

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@9030

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@9030

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@9030

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@9030

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@9030

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@9030

@clerk/express

npm i https://pkg.pr.new/@clerk/express@9030

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@9030

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@9030

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@9030

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@9030

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@9030

@clerk/react

npm i https://pkg.pr.new/@clerk/react@9030

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@9030

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@9030

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@9030

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@9030

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@9030

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@9030

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@9030

commit: 8aaf690

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-07-01T14:47:18.958Z

Summary

Metric Count
Packages analyzed 19
Packages with changes 0
🔴 Breaking changes 0
🟡 Non-breaking changes 0
🟢 Additions 0

No API Changes Detected

All packages have stable APIs with no detected changes.


Report generated by Break Check

Last ran on 8aaf690.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/clerk-js/src/core/tokenCache.ts (2)

250-253: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Don’t block broadcast updates on a pending local resolver.

Line 253 can wait indefinitely on a slow/stuck local tokenResolver, preventing a fresher cross-tab token from being applied. If there’s no resolvedToken yet, proceed with setInternal; the resolver path already reconciles against the live cache before publishing.

Proposed fix
-        const existingToken = result.entry.resolvedToken ?? (await result.entry.tokenResolver);
-        if (pickFreshestJwt(existingToken, token) === existingToken) {
+        const existingToken = result.entry.resolvedToken;
+        if (existingToken && pickFreshestJwt(existingToken, token) === existingToken) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/tokenCache.ts` around lines 250 - 253, The local
token lookup in token cache handling is waiting on result.entry.tokenResolver
when resolvedToken is missing, which can block broadcast updates; update the
flow around get(...) and the existingToken resolution so it does not await a
pending local resolver before applying setInternal. Prefer using
result.entry.resolvedToken only, and let the tokenResolver path reconcile with
the live cache later before publishing.

382-414: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Schedule timers from remaining TTL, not full token lifetime.

expiresIn is exp - iat, but setTimeout and proactive refresh run relative to now. For an already-aged winner token, this delays cleanup/refresh by the token’s age and can keep the cache from refreshing before actual expiry.

Proposed fix
         const issuedAt = claims.iat;
         const expiresIn: Seconds = claims.exp - issuedAt;
+        const remainingTtl: Seconds = claims.exp - Math.floor(Date.now() / 1000);
+
+        if (remainingTtl <= 0) {
+          if (store.get(key) === live) {
+            clearTimeout(live.timeoutId);
+            clearTimeout(live.refreshTimeoutId);
+            store.delete(key);
+          }
+          return;
+        }
 
         live.createdAt = issuedAt;
         live.expiresIn = expiresIn;
@@
-        const timeoutId = setTimeout(liveDeleteKey, expiresIn * 1000);
+        const timeoutId = setTimeout(liveDeleteKey, remainingTtl * 1000);
@@
-        const refreshFireTime = expiresIn - leeway - refreshLeadTime;
+        const refreshFireTime = remainingTtl - leeway - refreshLeadTime;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/tokenCache.ts` around lines 382 - 414, The timer
scheduling in tokenCache.ts uses the token’s full lifetime (`expiresIn =
claims.exp - claims.iat`) instead of the remaining TTL from now, so cleanup and
proactive refresh can run too late for an already-aged token. In the logic
around `liveDeleteKey`, `timeoutId`, and `refreshFireTime`, compute the
remaining time until expiry from the current time and schedule both timers off
that remaining TTL rather than `exp - iat`. Keep the proactive refresh buffer
(`refreshLeadTime`, `BACKGROUND_REFRESH_THRESHOLD_IN_SECONDS`,
`POLLER_INTERVAL_IN_MS`) applied to the remaining time so refresh still fires
before the actual expiry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/clerk-js/src/core/tokenCache.ts`:
- Around line 250-253: The local token lookup in token cache handling is waiting
on result.entry.tokenResolver when resolvedToken is missing, which can block
broadcast updates; update the flow around get(...) and the existingToken
resolution so it does not await a pending local resolver before applying
setInternal. Prefer using result.entry.resolvedToken only, and let the
tokenResolver path reconcile with the live cache later before publishing.
- Around line 382-414: The timer scheduling in tokenCache.ts uses the token’s
full lifetime (`expiresIn = claims.exp - claims.iat`) instead of the remaining
TTL from now, so cleanup and proactive refresh can run too late for an
already-aged token. In the logic around `liveDeleteKey`, `timeoutId`, and
`refreshFireTime`, compute the remaining time until expiry from the current time
and schedule both timers off that remaining TTL rather than `exp - iat`. Keep
the proactive refresh buffer (`refreshLeadTime`,
`BACKGROUND_REFRESH_THRESHOLD_IN_SECONDS`, `POLLER_INTERVAL_IN_MS`) applied to
the remaining time so refresh still fires before the actual expiry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 4feb6c1e-fcaf-4f22-998f-4273468c2874

📥 Commits

Reviewing files that changed from the base of the PR and between 570b749 and dab53c6.

📒 Files selected for processing (4)
  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/auth/AuthCookieService.ts
  • packages/clerk-js/src/core/tokenCache.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/clerk-js/src/core/tests/clerk.test.ts
  • packages/clerk-js/src/core/auth/AuthCookieService.ts
  • packages/clerk-js/src/core/tests/tokenCache.test.ts

oiat is stamped at origin (origin-issued-at, set when claims are assembled
from the DB), not by the edge token minter, so a token that lacks oiat is a
pre-feature token and is genuinely staler than any token that has one. That
is exactly how pickFreshestJwt already ranks tokens.

isStrictlyStalerJwt reimplemented the same oiat-then-iat comparison only to
fail open on a missing oiat, guarding a fresh-token-without-oiat case that
cannot arise from edge rollout. Drop it and route the cookie, lastActiveToken
hydrate, and token-dispatch guards through pickFreshestJwt, removing the
duplicated comparison.
setInternal repeated the same teardown (clear both timers, delete the slot if
it still holds this value) in three places: the reject path, the malformed-claims
branch, and the expiry timer. Collapse them into one dropIfCurrent helper, which
also drops the redundant undefined-guards before clearTimeout (clearTimeout
ignores undefined).

#dispatchTokenEvents had a dead lastActiveToken null-check: pickFreshestOrIncoming
already returns the incoming token when the existing one is null, so the extra
branch could never change the result.
Comment thread packages/clerk-js/src/core/tokenFreshness.ts Outdated
pickFreshestOrIncoming only existed to return the incoming token when there was
no baseline. Teach pickFreshestJwt to tolerate a null/undefined existing token
directly and drop the wrapper, so the optional-baseline call sites and the ones
with a guaranteed baseline share one function.
…+org

The cookie guard dropped any token whose sid/org did not match the active
session/org. During a multi-session setActive({ session }) switch, clerk.session
lags behind the dispatched token (it is committed after getToken), so the new
session's token was dropped and the cookie held the old session's token until the
next poll.

Scope the guard to a same session+org comparison: it now only drops a token that
is strictly staler than a same-context cookie, and writes through (fails open) on
any cross-context token, matching pre-feature behavior across a session or org
switch. Cross-context cookie protection is deferred to a follow-up.
…kie guard

The cookie guard no longer drops a cross-context token downstream (it now fails
open on a session/org mismatch), so #applyIncomingLastActiveToken's note that a
bad token 'is still dropped downstream by the cookie guard' was false. Adopting
the incoming token with no same-context baseline is justified on its own: an
empty lastActiveToken reads as a sign-out, and the adopted token is not cached
under the active-org key.
Why:
The inline org-context freshness guard in the session token hydration path was dense and mixed the wrong-org veto with the monotonic freshness pick.

What changed:
Moved the same-context selection into pickSameContextFreshestJwt and isSameOrgContext in tokenFreshness.ts and restored the #hydrateCache name. No behavior change; the piggyback and monotonic guard suites still pass.
Why:
The guard added monotonic and org-context handling when applying a piggybacked last_active_token, but its necessity on that path was not established: the reproduced regression was the fetch path, and its unit tests only restated the guard's own behavior. Falling back to the simpler hydration until piggyback staleness is confirmed.

What changed:
Restored main's #hydrateCache (plain cache seed) and the direct fromJSON assignment, and removed the fromJSON piggyback guard test suite. Fetch-path monotonic guards are unchanged.
Why:
The Token import went unused after the piggyback last_active_token guard tests
were dropped; eslint's unused-imports rule flagged it as an error and failed
the Static analysis CI job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants