Skip to content

Conversation

jacekradko
Copy link
Member

@jacekradko jacekradko commented Sep 30, 2025

Description

When fetching a new Session token, broadcast the token value to other tabs so they can warm their in-memory Session Token cache with the most recent token.

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features

    • Share freshly fetched session tokens across open tabs for faster, cache-first authentication.
    • New token ID utilities exported for consistent token identification.
  • Bug Fixes

    • Prevent duplicate network requests when requesting tokens concurrently.
    • More robust cross-tab sign-out and token handling.
  • Chores

    • Added a new build variant and related dev scripts/artifacts.
    • Deprecation notice for legacy localStorage-based broadcast fallback.
  • Tests

    • Extensive unit and integration tests for token caching, cross-tab sync, expiration, and multi-session isolation.

Copy link

changeset-bot bot commented Sep 30, 2025

🦋 Changeset detected

Latest commit: be2b34b

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

This PR includes changesets to release 19 packages
Name Type
@clerk/clerk-js Minor
@clerk/shared Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/tanstack-react-start Patch
@clerk/testing Patch
@clerk/vue 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

Copy link

vercel bot commented Sep 30, 2025

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

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Ready Preview Comment Oct 3, 2025 8:26pm

Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

Walkthrough

Switches cross-tab messaging from LocalStorageBroadcastChannel to the native BroadcastChannel; implements a memory-backed session token cache with expiration, leeway, and cross-tab sync; updates Session.getToken to dedupe concurrent fetches and broadcast resolved tokens; adds TokenId utilities, a new channel build variant, and tests.

Changes

Cohort / File(s) Summary
Broadcast channel migration
packages/clerk-js/src/core/clerk.ts, packages/shared/src/localStorageBroadcastChannel.ts, .changeset/slick-ducks-bet.md
Replaces LocalStorageBroadcastChannel with native BroadcastChannel; removes exported ClerkCoreBroadcastChannelEvent type; updates listener signatures and safety of event.data access; adds runtime/JSDoc deprecation for LocalStorageBroadcastChannel and a changeset documenting deprecation.
Session token cache implementation
packages/clerk-js/src/core/tokenCache.ts
Adds MemoryTokenCache with expiration, leeway, timeout management, broadcasting via BroadcastChannel, TokenCacheKey utilities, SessionTokenEvent payload, and public SessionTokenCache; extends TokenCache interface (get(..., leeway?), set, clear, close, size) and augments entry/value shapes.
Session token retrieval flow
packages/clerk-js/src/core/resources/Session.ts
Updates Session.getToken to use TokenId, cache promise resolvers immediately to dedupe concurrent calls, log debug/info, emit TokenUpdate/SessionTokenResolved, update lastActiveToken, and return `token.getRawString()
TokenId utilities
packages/clerk-js/src/utils/tokenId.ts, packages/clerk-js/src/utils/index.ts, packages/clerk-js/src/utils/__tests__/tokenId.test.ts
Adds TokenId (build/parse/extractTemplate) and ParsedTokenId interface; re-exports via utils/index.ts; adds comprehensive tests for building/parsing/extraction and edge cases.
Tests: token cache and session
packages/clerk-js/src/core/__tests__/tokenCache.test.ts, packages/clerk-js/src/core/resources/__tests__/Session.test.ts
Adds extensive unit tests covering broadcast handling, JWT validation, monotonicity, expiration/leeway, pending resolvers, dedupe concurrency, cache lifecycle (clear/close), and BroadcastChannel mocking/timer control.
Integration: session token cache
integration/tests/session-token-cache/single-session.test.ts, integration/tests/session-token-cache/multi-session.test.ts
Adds Playwright integration tests validating cross-tab BroadcastChannel token sharing, cache pre-warm (no duplicate network requests), multi-session isolation, and per-tab session independence.
Build/config: channel variant
packages/clerk-js/rspack.config.js, packages/clerk-js/src/global.d.ts, packages/clerk-js/vitest.config.mts, packages/clerk-js/package.json, packages/clerk-js/bundlewatch.config.json, integration/presets/envs.ts, .changeset/every-dryers-refuse.md
Introduces clerk.channel.browser build variant, exposes __BUILD_VARIANT_CHANNEL__ flag in types and test config, adds dev:channel script and bundlewatch entry, adds env preset for BroadcastChannel, and documents token pre-warm behavior in a changeset.
Chips browser entry cleanup
packages/clerk-js/src/index.chips.browser.ts
Removes automatic global initialization, renderer binding, script-attribute/global-variable config parsing, and HMR acceptance from the chips browser entry.
Local deprecation helper
packages/shared/src/localStorageBroadcastChannel.ts
Adds runtime deprecation notice and JSDoc to LocalStorageBroadcastChannel advising migration to native BroadcastChannel.
Minor test tidy
packages/clerk-js/src/ui/elements/PhoneInput/__tests__/useFormattedPhoneNumber.test.ts
Consolidates @testing-library/react imports (style change only).

Sequence Diagram(s)

sequenceDiagram
  participant Tab1 as Tab1 (Page A)
  participant Cache1 as MemoryTokenCache A
  participant API as Backend
  participant BC as BroadcastChannel "clerk"
  participant Tab2 as Tab2 (Page B)
  participant Cache2 as MemoryTokenCache B

  Note over Tab1,API: Request token flow
  Tab1->>Cache1: get(tokenId, leeway)
  alt cache miss or expired
    Cache1-->>Tab1: undefined
    Tab1->>API: fetch token
    API-->>Tab1: token (jwt with iat/exp, sid)
    Tab1->>Cache1: set({key, token, createdAt})
    Cache1-->>BC: postMessage(SessionTokenEvent)
  else cache hit (fresh/pending)
    Cache1-->>Tab1: entry (value or promise)
  end

  Note over BC,Cache2: Cross-tab sync
  BC-->>Cache2: message(SessionTokenEvent)
  Cache2->>Cache2: validate tokenId & jwt (iat/exp/sid)
  Cache2->>Cache2: apply if newer (no rebroadcast)
  Tab2->>Cache2: get(tokenId, leeway)
  Cache2-->>Tab2: token (served from cache, no network)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I’m a busy rabbit, ears alert and quick,
I pass tokens softly, a hop and a tick.
I cache with care, give five seconds of play,
Broadcast the carrot so no tab needs to stray.
Thump—two burrows synced, no needless trick. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “feat(clerk-js): Dedupe getToken requests” correctly highlights the deduplication of concurrent token fetches, which is part of the changeset, even though the PR also introduces broader cross-tab token broadcasting and cache warming. It refers to a real aspect of the update without being misleading or off-topic. Therefore, it satisfies the partial relevance criterion.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/multitab-poller

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5dcac2a and be2b34b.

📒 Files selected for processing (1)
  • packages/clerk-js/package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/package.json
packages/*/package.json

📄 CodeRabbit inference engine (.cursor/rules/global.mdc)

All publishable packages should be placed under the packages/ directory

packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.

Files:

  • packages/clerk-js/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
  • GitHub Check: Integration Tests (react-router, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (machine, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (custom, chrome)
  • GitHub Check: Integration Tests (billing, chrome)
  • GitHub Check: Integration Tests (tanstack-react-start, chrome)
  • GitHub Check: Integration Tests (nuxt, chrome)
  • GitHub Check: Integration Tests (tanstack-react-router, chrome)
  • GitHub Check: Integration Tests (vue, chrome)
  • GitHub Check: Integration Tests (astro, chrome)
  • GitHub Check: Integration Tests (handshake, chrome)
  • GitHub Check: Integration Tests (expo-web, chrome)
  • GitHub Check: Integration Tests (localhost, chrome)
  • GitHub Check: Integration Tests (handshake:staging, chrome)
  • GitHub Check: Integration Tests (elements, chrome)
  • GitHub Check: Integration Tests (ap-flows, chrome)
  • GitHub Check: Integration Tests (generic, chrome)
  • GitHub Check: Integration Tests (sessions:staging, chrome)
  • GitHub Check: Integration Tests (express, chrome)
  • GitHub Check: Integration Tests (sessions, chrome)
  • GitHub Check: Integration Tests (quickstart, chrome)
  • GitHub Check: Static analysis
  • GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
  • GitHub Check: Unit Tests (22, **)
  • GitHub Check: Publish with pkg-pr-new
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/package.json (1)

45-45: Addition looks good.

The new dev:channel script mirrors the existing variant scripts and provides an easy way to run the channel build locally. Nicely aligned with the new build variant workflow.


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

Copy link

pkg-pr-new bot commented Sep 30, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@6891

@clerk/astro

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

@clerk/backend

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

@clerk/chrome-extension

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

@clerk/clerk-js

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

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@6891

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@6891

@clerk/clerk-expo

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

@clerk/expo-passkeys

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

@clerk/express

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

@clerk/fastify

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

@clerk/localizations

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

@clerk/nextjs

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

@clerk/nuxt

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

@clerk/clerk-react

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

@clerk/react-router

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

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@6891

@clerk/shared

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

@clerk/tanstack-react-start

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

@clerk/testing

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

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@6891

@clerk/types

npm i https://pkg.pr.new/@clerk/types@6891

@clerk/upgrade

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

@clerk/vue

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

commit: be2b34b

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dce68e1 and c86fe2d.

📒 Files selected for processing (5)
  • packages/clerk-js/src/core/clerk.ts (4 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (5 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
  • packages/types/src/clerk.ts (1 hunks)
  • packages/types/src/session.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/clerk.ts
  • packages/types/src/session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/tokenCache.ts
🧬 Code graph analysis (3)
packages/clerk-js/src/core/clerk.ts (2)
packages/shared/src/index.ts (1)
  • LocalStorageBroadcastChannel (27-27)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/resources/Session.ts (5)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (178-178)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/clerk-js/src/core/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (167-178)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
packages/clerk-js/src/core/tokenCache.ts (1)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c86fe2d and 334d024.

📒 Files selected for processing (1)
  • packages/clerk-js/src/core/clerk.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/clerk.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/clerk.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (2)
packages/shared/src/index.ts (1)
  • LocalStorageBroadcastChannel (27-27)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
🪛 Biome (2.1.2)
packages/clerk-js/src/core/clerk.ts

[error] 2744-2793: expected a semicolon to end the class property, but found none

(parse)


[error] 2793-2793: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found ')'.

Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.

(parse)

🪛 GitHub Actions: CI
packages/clerk-js/src/core/clerk.ts

[error] 2793-2793: Module build failed: Expected a semicolon / Unexpected token ')' at line 2793 during bundling. Build step pnpm build:bundle failed.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

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

545-547: Prefer native BroadcastChannel; fall back to LocalStorage for non‑sensitive use

To avoid persisting token payloads in storage, use native BroadcastChannel when available. Keep LocalStorageBroadcastChannel only as a fallback and avoid sending raw tokens when on the fallback.

-    this.#broadcastChannel = new LocalStorageBroadcastChannel('clerk');
-    this.__internal_broadcastChannel = this.#broadcastChannel;
+    const channel =
+      typeof BroadcastChannel !== 'undefined'
+        ? (new BroadcastChannel('clerk') as unknown as ClerkBroadcastChannel)
+        : new LocalStorageBroadcastChannel<ClerkCoreBroadcastChannelEvent>('clerk');
+    this.#broadcastChannel = channel;
+    this.__internal_broadcastChannel = channel;

Remember to import ClerkBroadcastChannel type from @clerk/types and update the #broadcastChannel field type.

🧹 Nitpick comments (3)
packages/types/src/clerk.ts (2)

65-77: Event union looks good; add brief docs for stability

Please add short JSDoc on the event and payload fields (especially semantics of template/organizationId/tokenId) to keep the public surface self‑documented.


78-81: Expose removeEventListener to avoid listener leaks

The channel interface only offers addEventListener/postMessage. Add a matching removeEventListener signature so consumers can unsubscribe cleanly and align with native BroadcastChannel.

 export interface ClerkBroadcastChannel {
   addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
   postMessage(data: ClerkCoreBroadcastChannelEvent): void;
+  removeEventListener?(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
+  // Optional close for parity with native BroadcastChannel
+  close?(): void;
 }
packages/clerk-js/src/core/clerk.ts (1)

263-264: Type the public prop to the interface to decouple implementation

Use the exported ClerkBroadcastChannel type (or ClerkInterface['__internal_broadcastChannel']) instead of the concrete LocalStorageBroadcastChannel to keep flexibility and ensure interface compliance.

-  public __internal_broadcastChannel?: LocalStorageBroadcastChannel<ClerkCoreBroadcastChannelEvent>;
+  public __internal_broadcastChannel?: ClerkBroadcastChannel;

Also change #broadcastChannel’s type accordingly.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 334d024 and f5ff843.

📒 Files selected for processing (3)
  • packages/clerk-js/src/core/clerk.ts (4 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (6 hunks)
  • packages/types/src/clerk.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/clerk.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/clerk.ts (2)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/resources/Session.ts (6)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (178-178)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/types/src/clerk.ts (1)

901-906: Typed channel replaces any — LGTM

Replacing any with a concrete ClerkBroadcastChannel tightens the surface as requested earlier.

packages/clerk-js/src/core/resources/Session.ts (2)

358-375: Nice: guarded document access for non‑DOM runtimes

The computed isActiveTab removes SSR/RN hazards from earlier.


441-463: No milliseconds conversion needed for createdAt
TokenCacheEntry.createdAt is measured in seconds and the cache uses Math.floor(Date.now()/1000) to compute time—token.jwt.claims.iat already provides seconds. Drop the suggested * 1000 conversion.

Likely an incorrect or invalid review comment.

packages/clerk-js/src/core/clerk.ts (1)

2757-2780: Good: broadcast handler now respects sessionId targeting

Scoping updates to payload.sessionId prevents poisoning another session’s cache in multi‑session scenarios.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/clerk-js/src/core/resources/Session.ts (1)

358-375: SSR-safe tab detection — good fix.

The document guards avoid crashes in non‑DOM runtimes and address the prior critical.

🧹 Nitpick comments (11)
packages/shared/src/index.ts (1)

28-28: Public surface expansion: mark intent and stability.

Exporting ClerkBroadcastChannel makes it part of the shared public API. Add JSDoc on the class (in clerkBroadcastChannel.ts) clarifying that it’s internal/experimental to avoid consumers taking a hard dependency on it. As per coding guidelines.

packages/types/src/clerk.ts (2)

65-77: Do not expose raw JWT in public types; mark these as internal.

ClerkCoreBroadcastChannelEvent includes tokenRaw. Since this file is public types, either:

  • Mark the type as internal, or
  • Drop tokenRaw from the public surface and keep it in an internal namespace.

At minimum, add JSDoc to make intent clear.

Apply JSDoc annotations:

+/**
+ * Cross-tab internal events for Clerk core.
+ * @internal
+ */
 export type ClerkCoreBroadcastChannelEvent =
   | { type: 'signout' }
   | {
       type: 'session_token';
       payload: {
         organizationId?: string | null;
         sessionId: string;
         template?: string;
         tokenId: string;
-        tokenRaw: string;
+        /**
+         * Raw JWT string. Sensitive — only used on secure in-memory channels.
+         * @internal
+         */
+        tokenRaw: string;
       };
     };

78-82: Consider delivery acknowledgment from postMessage.

Today postMessage returns void, and the implementation drops session_token when native BroadcastChannel is unavailable. Returning boolean (delivered or dropped) would let callers log accurately and potentially fallback.

Proposed signature change:

-export interface ClerkBroadcastChannel {
-  addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
-  close(): void;
-  postMessage(data: ClerkCoreBroadcastChannelEvent): void;
-}
+export interface ClerkBroadcastChannel {
+  addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
+  close(): void;
+  /**
+   * Returns true if the message was delivered to a native channel; false if dropped.
+   */
+  postMessage(data: ClerkCoreBroadcastChannelEvent): boolean;
+}
packages/clerk-js/src/core/resources/Session.ts (2)

148-149: Cache key delimiter: minor collision risk.

Joining with - can theoretically collide if template or org IDs contain -. Consider a sentinel delimiter or tuple-like join.

Example:

-    return [this.id, template, resolvedOrganizationId].filter(Boolean).join('-');
+    return [this.id, template ?? '', resolvedOrganizationId ?? ''].join('|');

465-496: Accurate logging and delivery semantics for broadcast.

Session logs “Broadcasting token update” unconditionally, but ClerkBroadcastChannel drops session_token when native BroadcastChannel is unavailable. Either:

  • Gate the log on delivery (have postMessage return boolean and only log on true), or
  • Move the responsibility to the channel to log when a message is dropped.

Also consider broadcasting only minimal, non‑secret metadata when dedupe (not pre‑warm) is sufficient.

Example (if postMessage returns boolean):

-    debugLogger.info(
-      'Broadcasting token update to other tabs',
-      { tokenId, template, organizationId, sessionId: this.id },
-      'session',
-    );
-
-    const message: ClerkCoreBroadcastChannelEvent = {
+    const message: ClerkCoreBroadcastChannelEvent = {
       type: 'session_token',
       payload: {
         organizationId,
         sessionId: this.id,
         template,
         tokenId,
         tokenRaw,
       },
     };
-
-    broadcastChannel.postMessage(message);
+    if (broadcastChannel.postMessage(message)) {
+      debugLogger.info(
+        'Broadcasted token update to other tabs',
+        { tokenId, template, organizationId, sessionId: this.id },
+        'session',
+      );
+    } else {
+      debugLogger.debug('Token broadcast dropped (no native channel)', { tokenId }, 'session');
+    }
packages/shared/src/clerkBroadcastChannel.ts (4)

3-16: Avoid duplicating event types; import from @clerk/types and implement the contract

Keep the runtime and public types in lockstep to prevent drift. Import ClerkCoreBroadcastChannelEvent (and the interface) from @clerk/types and have this class implement it.

+import type {
+  ClerkCoreBroadcastChannelEvent,
+  ClerkBroadcastChannel as ClerkBroadcastChannelContract,
+} from '@clerk/types';
-
-type ClerkCoreBroadcastChannelEvent =
-  | { type: 'signout' }
-  | {
-      type: 'session_token';
-      payload: {
-        organizationId?: string | null;
-        sessionId: string;
-        template?: string;
-        tokenId: string;
-        tokenRaw: string;
-      };
-    };
@@
-export class ClerkBroadcastChannel {
+export class ClerkBroadcastChannel implements ClerkBroadcastChannelContract {

As per coding guidelines (packages should export types; prefer type-only imports).


54-61: Emit a debug when dropping session_token due to lack of native BroadcastChannel

Right now, the event is silently ignored when native BroadcastChannel isn’t available. A low‑level log helps diagnose why cross‑tab token warming didn’t occur.

       case 'session_token':
-        if (this.nativeChannel) {
-          this.nativeChannel.postMessage(event);
-        }
+        if (this.nativeChannel) {
+          this.nativeChannel.postMessage(event);
+        } else {
+          // Intentionally not falling back to localStorage for sensitive data
+          console.debug('[ClerkBroadcastChannel] session_token dropped: BroadcastChannel unavailable');
+        }
         break;

47-52: Support listener removal (or return a disposer) to prevent accumulation

You maintain a Set of listeners but offer no way to remove one. Add removeEventListener (keeps interface compatibility) so internal callers can unsubscribe.

   public addEventListener = (eventName: 'message', listener: Listener): void => {
     if (eventName !== 'message') {
       return;
     }
     this.listeners.add(listener);
   };
+
+  // Not part of the public interface but useful internally
+  public removeEventListener = (eventName: 'message', listener: Listener): void => {
+    if (eventName !== 'message') {
+      return;
+    }
+    this.listeners.delete(listener);
+  };

23-37: Add JSDoc on public methods

Public APIs should include brief JSDoc for addEventListener, postMessage, and close (include event shapes, security note about token events using native channel only).

Also applies to: 47-75

packages/clerk-js/src/core/clerk.ts (2)

262-263: Document __internal_broadcastChannel as unstable/internal

This is publicly visible; add JSDoc (e.g., “@internal” or “@experimental”) to set expectations and avoid external reliance on unstable behavior.


2544-2546: Close broadcast channel on page unload
Add a createBeforeUnloadTracker hook in packages/clerk-js/src/core/clerk.ts after instantiating the channel to call this.#broadcastChannel.close() and free resources:

 this.#pageLifecycle = createPageLifecycle();
 this.#broadcastChannel = new ClerkBroadcastChannel('clerk');
 this.__internal_broadcastChannel = this.#broadcastChannel;
 this.#setupBrowserListeners();
+ this.#beforeUnloadTracker = createBeforeUnloadTracker(() => {
+   this.#broadcastChannel.close();
+ });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5fe502c and de83b39.

📒 Files selected for processing (5)
  • packages/clerk-js/src/core/clerk.ts (5 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (6 hunks)
  • packages/shared/src/clerkBroadcastChannel.ts (1 hunks)
  • packages/shared/src/index.ts (1 hunks)
  • packages/types/src/clerk.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/clerk.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/shared/src/clerkBroadcastChannel.ts
packages/**/index.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use tree-shaking friendly exports

Files:

  • packages/shared/src/index.ts
**/index.ts

📄 CodeRabbit inference engine (.cursor/rules/react.mdc)

Use index.ts files for clean imports but avoid deep barrel exports

Avoid barrel files (index.ts re-exports) as they can cause circular dependencies

Files:

  • packages/shared/src/index.ts
🧬 Code graph analysis (4)
packages/types/src/clerk.ts (2)
packages/shared/src/clerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-76)
packages/shared/src/index.ts (1)
  • ClerkBroadcastChannel (28-28)
packages/clerk-js/src/core/clerk.ts (3)
packages/shared/src/clerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-76)
packages/types/src/clerk.ts (1)
  • ClerkBroadcastChannel (78-82)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/resources/Session.ts (6)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (178-178)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/shared/src/clerkBroadcastChannel.ts (2)
packages/types/src/clerk.ts (2)
  • ClerkCoreBroadcastChannelEvent (65-76)
  • ClerkBroadcastChannel (78-82)
packages/shared/src/index.ts (2)
  • ClerkBroadcastChannel (28-28)
  • LocalStorageBroadcastChannel (27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/types/src/clerk.ts (1)

902-909: Approve typed internal channel update
Replaced any with ClerkBroadcastChannel; verified all assignments and references in clerk.ts and Session.ts align with this interface.

packages/clerk-js/src/core/clerk.ts (2)

228-229: Channel field type update looks good

Switch to ClerkBroadcastChannel aligns with the new transport.


2753-2780: Ignore outdated cleanup suggestion

Listener correctly handles signout and session_token, resolving sessions by sessionId and skipping non-matching ones to prevent cross-session poisoning. The remaining LocalStorageBroadcastChannel code in packages/shared and the tokenUpdate comment in Session.ts are intentional and outside this PR’s scope.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

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 (5)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (1)

238-245: Strengthen close semantics: also detach listeners and close localStorage channel

Currently tests only assert native close and listener clearing. To prevent leaks, the implementation should also remove the native listener and close the localStorage channel. Add assertions here once implementation is updated.

Proposed implementation change (in packages/shared/src/ClerkBroadcastChannel.ts):

   public close = (): void => {
-    this.nativeChannel?.close();
-    this.listeners.clear();
+    // Detach native listener before closing
+    if (this.nativeChannel) {
+      this.nativeChannel.removeEventListener('message', this.handleNativeMessage);
+      this.nativeChannel.close();
+    }
+    // Close localStorage channel and detach its listener
+    // (LocalStorageBroadcastChannel should expose close())
+    // @ts-expect-error: ensure LocalStorageBroadcastChannel has close()
+    this.localStorageChannel.close?.();
+    this.listeners.clear();
   };

And extend this test block with:

   it('closes native BroadcastChannel when available', () => {
     const channel = new ClerkBroadcastChannel(channelName);

     channel.close();

     expect(nativeBroadcastChannelMock.close).toHaveBeenCalled();
+    expect(nativeBroadcastChannelMock.removeEventListener).toHaveBeenCalledWith(
+      'message',
+      expect.any(Function),
+    );
   });

Also applies to: 254-279

packages/clerk-js/src/core/clerk.ts (3)

228-229: Channel lifecycle: consider closing on teardown/pagehide

We create a long‑lived channel but never close it. Consider closing on pagehide/unload or when Clerk is disposed to detach listeners.

 this.#pageLifecycle = createPageLifecycle();
+// Optionally:
+window.addEventListener('pagehide', () => this.#broadcastChannel?.close(), { once: true });

262-263: Public API surface: document internal broadcaster

This adds a public member. Please add JSDoc marking it internal/unstable to avoid ecosystem reliance.

/** @internal For SDK internal wiring and tests. Subject to change without notice. */
public __internal_broadcastChannel?: ClerkBroadcastChannel;

2754-2779: Session‑scoped token updates: correct and safe

The handler scopes updates by payload.sessionId and skips when no match is present; good fix for multi‑session tabs. Consider adding organizationId to the log context for completeness.

-          { tokenId: data.payload.tokenId, sessionId: data.payload.sessionId },
+          { tokenId: data.payload.tokenId, sessionId: data.payload.sessionId, organizationId: data.payload.organizationId ?? null },
packages/shared/src/ClerkBroadcastChannel.ts (1)

72-75: Tear down the LocalStorage listener on close().

We add a message listener to LocalStorageBroadcastChannel in the constructor but never remove it here. That keeps this instance reachable (and still reacting to signout messages) after close() and prevents GC. Please mirror the native cleanup with a matching removeEventListener/close on the localStorage channel before clearing listeners.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between de83b39 and d66e697.

📒 Files selected for processing (4)
  • packages/clerk-js/src/core/clerk.ts (5 hunks)
  • packages/shared/src/ClerkBroadcastChannel.ts (1 hunks)
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (1 hunks)
  • packages/shared/src/index.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/shared/src/ClerkBroadcastChannel.ts
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
  • packages/shared/src/index.ts
  • packages/clerk-js/src/core/clerk.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
packages/**/index.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use tree-shaking friendly exports

Files:

  • packages/shared/src/index.ts
**/index.ts

📄 CodeRabbit inference engine (.cursor/rules/react.mdc)

Use index.ts files for clean imports but avoid deep barrel exports

Avoid barrel files (index.ts re-exports) as they can cause circular dependencies

Files:

  • packages/shared/src/index.ts
🧬 Code graph analysis (3)
packages/shared/src/ClerkBroadcastChannel.ts (2)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/shared/src/index.ts (2)
  • ClerkBroadcastChannel (28-28)
  • LocalStorageBroadcastChannel (27-27)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (1)
packages/shared/src/ClerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-76)
packages/clerk-js/src/core/clerk.ts (3)
packages/shared/src/ClerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-76)
packages/types/src/clerk.ts (1)
  • ClerkBroadcastChannel (78-82)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
🪛 GitHub Check: Static analysis
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts

[failure] 44-44:
'channel' is assigned a value but never used. Allowed unused vars must match /^_/u


[failure] 2-2:
'LocalStorageBroadcastChannel' is defined but never used


[failure] 2-2:
'LocalStorageBroadcastChannel' is defined but never used. Allowed unused vars must match /^_/u

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (generic, chrome)
🔇 Additional comments (2)
packages/shared/src/index.ts (1)

28-28: Public export looks good; ensure docs/types are aligned

The named export is tree‑shake friendly and path casing matches the file. Please add/update JSDoc in the class file so this public API is discoverable from @clerk/shared and ensure types reflect it across packages.

packages/clerk-js/src/core/clerk.ts (1)

2544-2546: Broadcast channel initialization is correct and well‑scoped

Creating a single channel and wiring __internal_broadcastChannel keeps diagnostics accessible while avoiding duplicate listeners.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d66e697 and 469a7d2.

📒 Files selected for processing (2)
  • packages/shared/src/ClerkBroadcastChannel.ts (1 hunks)
  • packages/shared/src/localStorageBroadcastChannel.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/shared/src/ClerkBroadcastChannel.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep/ci
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (2)

1-1: Previous lint issue resolved.

The earlier unused import of LocalStorageBroadcastChannel is no longer present. Nothing to do.


43-49: Constructor variable now used; prior “unused var” feedback addressed.

channel is referenced in the assertion; the earlier warning is resolved.

🧹 Nitpick comments (2)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (2)

6-9: Strengthen test typings (avoid any); define minimal mock types.

Tighten types for mocks to satisfy ESLint “no-explicit-any” and improve IDE help.

Apply this diff:

@@
 describe('ClerkBroadcastChannel', () => {
-  let localStorageMock: any;
-  let nativeBroadcastChannelMock: any;
-  let originalBroadcastChannel: any;
+  type MockBroadcastChannel = Pick<BroadcastChannel, 'addEventListener' | 'removeEventListener' | 'postMessage' | 'close'> & {
+    addEventListener: jest.Mock;
+    removeEventListener: jest.Mock;
+    postMessage: jest.Mock;
+    close: jest.Mock;
+  };
+  type LocalStorageMock = Pick<Storage, 'getItem' | 'setItem' | 'removeItem'> & {
+    getItem: jest.Mock;
+    setItem: jest.Mock;
+    removeItem: jest.Mock;
+  };
+
+  let localStorageMock: LocalStorageMock;
+  let nativeBroadcastChannelMock: MockBroadcastChannel;
+  let originalBroadcastChannel: typeof BroadcastChannel | undefined;
@@
-    nativeBroadcastChannelMock = {
+    nativeBroadcastChannelMock = {
       addEventListener: jest.fn(),
       close: jest.fn(),
       postMessage: jest.fn(),
       removeEventListener: jest.fn(),
-    };
+    } as unknown as MockBroadcastChannel;
@@
-    originalBroadcastChannel = global.BroadcastChannel;
-    global.BroadcastChannel = jest.fn(() => nativeBroadcastChannelMock) as any;
+    originalBroadcastChannel = global.BroadcastChannel;
+    global.BroadcastChannel = jest.fn(() => nativeBroadcastChannelMock) as unknown as typeof BroadcastChannel;

Also applies to: 26-35


225-233: Also assert removeEventListener is called on close for proper cleanup.

Verifies we detach the native listener, not just close the channel.

Apply this diff:

   it('closes native BroadcastChannel when available', () => {
     const channel = new ClerkBroadcastChannel(channelName);

     channel.close();

     expect(nativeBroadcastChannelMock.close).toHaveBeenCalled();
+    expect(nativeBroadcastChannelMock.removeEventListener).toHaveBeenCalledWith('message', expect.any(Function));
   });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 469a7d2 and 7410198.

📒 Files selected for processing (1)
  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts
🧬 Code graph analysis (1)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts (1)
packages/shared/src/ClerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-81)
🪛 Gitleaks (8.28.0)
packages/shared/src/__tests__/ClerkBroadcastChannel.test.ts

[high] 94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 165-165: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 212-212: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 255-255: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 277-277: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 321-321: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 338-338: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
packages/shared/src/localStorageBroadcastChannel.ts (1)

46-64: Guard all window access and add SSR fallback

  • Initialize eventTarget with typeof window !== 'undefined' ? window : new EventTarget() instead of unconditionally using window.
  • In setupLocalStorageListener(), return early if typeof window === 'undefined'.
  • In close(), also guard window.removeEventListener behind typeof window !== 'undefined'.
🧹 Nitpick comments (3)
packages/shared/src/localStorageBroadcastChannel.ts (3)

16-19: Return an unsubscribe to prevent listener leaks.

There’s no way to remove the bound wrapper you add to window, which can leak handlers across re-initializations. Return a cleanup function from addEventListener that removes the exact bound listener.

-  public addEventListener = (eventName: 'message', listener: Listener<E>): void => {
-    this.eventTarget.addEventListener(this.prefixEventName(eventName), e => {
-      listener(e as MessageEvent<E>);
-    });
-  };
+  public addEventListener = (
+    eventName: 'message',
+    listener: Listener<E>
+  ): (() => void) => {
+    const type = this.prefixEventName(eventName);
+    const handler = (e: Event) => listener(e as MessageEvent<E>);
+    this.eventTarget.addEventListener(type, handler as EventListener);
+    return () => this.eventTarget.removeEventListener(type, handler as EventListener);
+  };

Also add a short JSDoc explaining the returned disposer. As per coding guidelines.


21-26: Expand close() (optional): drain message listeners too.

If you adopt an unsubscribe-returning API, consider tracking them and draining in close() for RAII-style cleanup; otherwise document that close() only detaches the storage listener.

// class field (outside this hunk)
private readonly unsubscribers = new Set<() => void>();

// when adding a listener
const unsub = this.addEventListener('message', listener);
this.unsubscribers.add(unsub);

// in close()
for (const unsub of this.unsubscribers) unsub();
this.unsubscribers.clear();

42-44: Use a delimiter in the prefixed event name.

Avoid accidental collisions like "__lsbc__abmessage" vs "__lsbc__a" + "bmessage" by adding a separator.

-  private prefixEventName(eventName: string): string {
-    return this.channelKey + eventName;
-  }
+  private prefixEventName(eventName: string): string {
+    return `${this.channelKey}:${eventName}`;
+  }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7410198 and 5d04576.

📒 Files selected for processing (1)
  • packages/shared/src/localStorageBroadcastChannel.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/shared/src/localStorageBroadcastChannel.ts
🔇 Additional comments (2)
packages/shared/src/localStorageBroadcastChannel.ts (2)

8-8: Good: keep a handle for cleanup.

Storing storageListener enables deterministic removal in close(). LGTM.


15-15: Typed payload preserved — resolves prior concern.

Casting to MessageEvent<E> restores the generic payload type for listeners. Nice.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

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

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

294-307: Fix expiry scheduling: tokens can outlive exp due to timer based on full lifetime (exp−iat) from insertion time

Schedule eviction using remainingSeconds (exp−now), and align age math by setting createdAt = iat once the token resolves. This prevents serving expired tokens and makes get()’s remainingSeconds computation correct.

       const expiresAt = newToken.jwt.claims.exp;
       const issuedAt = newToken.jwt.claims.iat;
-      const expiresIn: Seconds = expiresAt - issuedAt;
-
-      // Mutate cached value and set expirations
-      value.expiresIn = expiresIn;
-      timer = setTimeout(deleteKey, expiresIn * 1000);
-
-      // Teach ClerkJS not to block the exit of the event loop when used in Node environments.
-      // More info at https://nodejs.org/api/timers.html#timeoutunref
-      if (typeof timer.unref === 'function') {
-          timer.unref();
-      }
+      const expiresIn: Seconds = expiresAt - issuedAt; // full lifetime
+      const nowSeconds = Math.floor(Date.now() / 1000);
+      const remainingSeconds: Seconds = Math.max(0, expiresAt - nowSeconds); // time left
+
+      // Mutate cached value and set expirations
+      value.expiresIn = expiresIn;
+      value.createdAt = issuedAt; // align age with JWT iat
+      value.timeout = setTimeout(deleteKey, remainingSeconds * 1000);
+
+      // Teach ClerkJS not to block the exit of the event loop when used in Node environments.
+      // More info at https://nodejs.org/api/timers.html#timeoutunref
+      if (typeof value.timeout?.unref === 'function') {
+        value.timeout.unref();
+      }
🧹 Nitpick comments (11)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (4)

1-1: Type the mock channel against the public interface for better safety

Import and use the ClerkBroadcastChannel interface to avoid any and catch signature drift.

-import type { ClerkCoreBroadcastChannelEvent } from '@clerk/types';
+import type { ClerkBroadcastChannel, ClerkCoreBroadcastChannelEvent } from '@clerk/types';
@@
-  let mockBroadcastChannel: {
-    addEventListener: ReturnType<typeof vi.fn>;
-    close: ReturnType<typeof vi.fn>;
-    postMessage: ReturnType<typeof vi.fn>;
-  };
+  let mockBroadcastChannel: Pick<
+    ClerkBroadcastChannel,
+    'addEventListener' | 'close' | 'postMessage'
+  >;

Also applies to: 9-13


16-29: Ensure mocks are fully reset between tests

You recreate the mock object per test (good). Optionally also call vi.clearAllMocks() in afterEach for isolation when more spies are added later.

Also applies to: 31-34


155-167: Avoid brittle dependency on a specific iat value

This test assumes a fixed iat in mockJwt. Compute the older JWT relative to the cached entry to future‑proof the test.

-// mockJwt has iat: 1666648250, so create an older one with iat: 1666648190 (60 seconds earlier)
-const olderJwt = ...
+const cachedEntryAfterNewer = SessionTokenCache.get({ tokenId: 'session_123' })!;
+const newerIat = cachedEntryAfterNewer.createdAt!;
+const olderJwt = makeFakeJwt({ iat: newerIat - 60, exp: newerIat + 300 });

Also applies to: 171-176


36-55: Add a test for mismatched sessionId to constrain cache pollution

Currently we test tokenId format mismatches; add a case where payload.sessionId !== expected session to ensure we drop it (or at least don’t warm unrelated keys).

Would you like me to draft the additional test using a different sessionId and asserting size/keys unchanged?

Also applies to: 199-209

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

398-407: Background‑tab dedupe: consider a short wait-for-broadcast before fetching

To reduce duplicate network calls across tabs, in non‑active tabs you can delay fetch briefly (e.g., 250–500ms) and resolve early if a broadcast warms the cache.

Sketch:

  • If !isActiveTab && !skipCache && !cachedEntry, await a Promise.race between:
    • a once('session_token') broadcast that populates SessionTokenCache, and
    • a timeout (e.g., 300ms).
  • Proceed to fetch only if the cache is still cold.

Happy to draft a concrete patch if you want to try this.

Also applies to: 419-429

packages/clerk-js/src/core/clerk.ts (2)

2545-2549: Attach/detach the broadcast channel on lifecycle transitions

You create and attach the channel during load; add cleanup on page hide/unload to avoid dangling listeners and to detach from SessionTokenCache.

 this.#pageLifecycle = createPageLifecycle();
@@
 this.#broadcastChannel = new ClerkBroadcastChannel('clerk');
 this.__internal_broadcastChannel = this.#broadcastChannel;
 SessionTokenCache.attachBroadcastChannel(this.#broadcastChannel);
+
+// Cleanup on lifecycle end
+this.#pageLifecycle?.onPageHide(() => {
+  SessionTokenCache.attachBroadcastChannel(null);
+  this.#broadcastChannel?.close();
+  this.#broadcastChannel = null;
+  this.__internal_broadcastChannel = undefined;
+});

229-230: Document new public surface __internal_broadcastChannel

It’s public (even if internal). Add a brief JSDoc to meet “All public APIs must be documented” and clarify behavior.

Example:

/**
 * Internal: cross-tab event channel used by Clerk for signout and token sync.
 * Not part of the public API surface; subject to change without notice.
 */
public __internal_broadcastChannel?: ClerkBroadcastChannel;

Also applies to: 263-264

packages/clerk-js/src/core/tokenCache.ts (4)

102-121: Guard against near‑expiry using absolute remaining time

After aligning createdAt = iat, the current math is consistent. As a robustness improvement, compute remainingSeconds directly to avoid any drift if createdAt is ever inconsistent.

-    const elapsedSeconds = nowSeconds - value.createdAt;
-    const expiresSoon = value.expiresIn !== undefined && value.expiresIn - elapsedSeconds < (leeway || 1) + SYNC_LEEWAY;
+    const remainingSeconds =
+      value.expiresIn !== undefined ? value.createdAt + value.expiresIn - nowSeconds : undefined;
+    const expiresSoon =
+      remainingSeconds !== undefined && remainingSeconds < (leeway || 1) + SYNC_LEEWAY;

7-11: Channel type should support removal for proper cleanup

Consider adding an optional removeEventListener to allow detaching without closing the channel.

 type ClerkBroadcastChannel = {
   addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
+  removeEventListener?(
+    eventName: 'message',
+    listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void,
+  ): void;
   close(): void;
   postMessage(data: ClerkCoreBroadcastChannelEvent): void;
 };

If implemented, prefer removeEventListener over close() in re‑attach logic.


23-28: Add JSDoc for exported TokenBroadcastMetadata

Public API should be documented per guidelines (purpose, field semantics, security notes about broadcasting raw JWT within same‑origin tabs).

As per coding guidelines.

-export interface TokenBroadcastMetadata {
+/**
+ * Metadata required to broadcast a newly fetched session token to sibling tabs on the same origin.
+ * Note: tokenRaw is the raw JWT and will be posted over a same‑origin BroadcastChannel.
+ */
+export interface TokenBroadcastMetadata {
   organizationId?: string | null;
   sessionId: string;
   template?: string;
   tokenRaw: string;
 }

72-75: Add a unit test asserting computeTokenId(sessionId, template, organizationId) matches Session#getCacheId(template, organizationId)
No existing tests cover this parity—adding one will prevent future drift.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5d04576 and d2ba027.

📒 Files selected for processing (4)
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1 hunks)
  • packages/clerk-js/src/core/clerk.ts (6 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (4 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Visual regression testing should be performed for UI components.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
🧬 Code graph analysis (4)
packages/clerk-js/src/core/resources/Session.ts (4)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (322-322)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
packages/clerk-js/src/core/clerk.ts (4)
packages/shared/src/ClerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-81)
packages/types/src/clerk.ts (1)
  • ClerkBroadcastChannel (78-82)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (322-322)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (3)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (322-322)
packages/clerk-js/src/test/core-fixtures.ts (1)
  • mockJwt (18-19)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (144-148)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
🪛 Gitleaks (8.28.0)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts

[high] 117-117: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


[high] 157-157: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1)

232-241: Broadcast behavior looks correct

  • Broadcasts exactly once when metadata is provided.
  • Skips when metadata is absent.
  • Validates tokenId before broadcasting.

Also applies to: 255-256, 277-278

packages/clerk-js/src/core/resources/Session.ts (3)

144-148: Cache key change is sensible and unambiguous

Using [sessionId, template, organizationId].filter(Boolean).join('-') is stable and avoids collisions so long as consumers don’t parse it. If any downstream parses tokenId, consider a tuple object instead of a delimited string.

Run a quick grep to ensure no code parses tokenId by splitting on -.


357-374: Non‑DOM guard for isActiveTab is correct

The typeof document/hasFocus/visibilityState checks prevent SSR/React Native crashes and centralize the flag for consistent logging.


419-429: Secure broadcast integration looks good

Passing metadata (including tokenRaw) into SessionTokenCache.set is fine given ClerkBroadcastChannel routes session_token events only over native BroadcastChannel (no localStorage persistence). Good separation of concerns.

Also applies to: 431-443

packages/clerk-js/src/core/clerk.ts (2)

2758-2766: Background-tab handling is scoped appropriately

Listener handles only signout and defers token sync to SessionTokenCache, reducing Clerk’s surface area here. Good separation.


2769-2773: Emit signout on all tabs — good routing

Posting { type: 'signout' } via ClerkBroadcastChannel fans out via localStorage fallback, ensuring wide coverage without leaking tokens.

packages/clerk-js/src/core/tokenCache.ts (1)

315-317: size() is fine

API returns the number of cached entries. No issues.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
packages/types/src/clerk.ts (3)

65-82: Document new public types and consider exporting event-name constants.

These are public surface areas; add concise JSDoc and (optional) runtime constants to prevent string‑literal drift across packages.

-export type ClerkCoreBroadcastChannelEvent =
+/** Cross-tab events used by Clerk internals for session coordination. */
+export type ClerkCoreBroadcastChannelEvent =
   | { type: 'signout' }
   | {
       type: 'session_token';
       payload: {
         organizationId?: string | null;
         sessionId: string;
         template?: string;
         tokenId: string;
         tokenRaw: string;
       };
     };
 
-export interface ClerkBroadcastChannel {
+/** Minimal BroadcastChannel-like interface used by Clerk for cross-tab messaging. */
+export interface ClerkBroadcastChannel {
   addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
   close(): void;
   postMessage(data: ClerkCoreBroadcastChannelEvent): void;
 }

Optional constants (new code block):

export const ClerkCoreBroadcastEvent = {
  Signout: 'signout',
  SessionToken: 'session_token',
} as const;

68-76: Optional: include issuedAt in session_token payload to optimize dedupe logic

Current cache dedupe compares iat by decoding tokenRaw (e.g. in clerk-js/src/core/tokenCache.ts); adding an optional issuedAt?: number to the payload lets you do monotonicity checks directly without repeated JWT parsing.

       payload: {
         organizationId?: string | null;
         sessionId: string;
         template?: string;
         tokenId: string;
         tokenRaw: string;
+        /** Unix seconds since epoch when the token was issued. */
+        issuedAt?: number;
       };

65-77: Approve typed broadcast event union

  • Consistent use of 'signout' and 'session_token' across producers and consumers confirmed.
  • Consider adding removeEventListener/onmessage to the ClerkBroadcastChannel interface for symmetry with addEventListener.
  • Add JSDoc to ClerkCoreBroadcastChannelEvent and ClerkBroadcastChannel, and optionally export event-name constants for reuse.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d2ba027 and 308800d.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/clerk.ts (5 hunks)
  • packages/types/src/clerk.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/core/clerk.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/types/src/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/types/src/clerk.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/clerk.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/clerk.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/types/src/clerk.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/clerk.ts
🧬 Code graph analysis (1)
packages/types/src/clerk.ts (1)
packages/shared/src/index.ts (1)
  • ClerkBroadcastChannel (28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
packages/clerk-js/src/core/tokenCache.ts (3)

81-86: Close previous broadcast channel on re-attach to avoid listener leaks.

Re-attaching without removing the old listener or closing the previous channel leaks handlers and can duplicate message processing.

Apply this diff to properly clean up the old channel:

   const attachBroadcastChannel = (channel: ClerkBroadcastChannel | null) => {
+    if (broadcastChannel && broadcastChannel !== channel) {
+      try {
+        broadcastChannel.close();
+      } catch {
+        // noop
+      }
+    }
     broadcastChannel = channel;
     if (channel) {
       channel.addEventListener('message', handleBroadcastMessage);
     }
   };

194-202: Monotonicity check compares against entry.createdAt instead of cached token's iat.

The check compares existingEntry.createdAt (which may be the current time if the token was locally fetched) against the incoming token.jwt.claims.iat. This can incorrectly reject newer broadcasts from other tabs.

Apply this diff to compare against the cached token's actual issued-at:

-    const existingEntry = get({ tokenId: payload.tokenId });
-    if (existingEntry?.createdAt && existingEntry.createdAt >= token.jwt.claims.iat) {
+    const existing = cache.get(new TokenCacheKey(prefix, { tokenId: payload.tokenId }).toKey());
+    if (existing?.createdAt && existing.createdAt >= token.jwt.claims.iat) {
       debugLogger.debug(
         'Ignoring older token broadcast (monotonicity check)',
-        { existingCreatedAt: existingEntry.createdAt, incomingIat: token.jwt.claims.iat, tokenId: payload.tokenId },
+        { existingCreatedAt: existing.createdAt, incomingIat: token.jwt.claims.iat, tokenId: payload.tokenId },
         'tokenCache',
       );
       return;
     }

Additionally, in setInternal (line 271), align createdAt to the token's iat when the token resolves:

     entry.tokenResolver
       .then(newToken => {
         if (!newToken.jwt) {
           return deleteKey();
         }

         const expiresAt = newToken.jwt.claims.exp;
         const issuedAt = newToken.jwt.claims.iat;
         const expiresIn: Seconds = expiresAt - issuedAt;

+        // Align createdAt to the token's actual issued-at for monotonicity checks
+        value.createdAt = issuedAt;
         // Mutate cached value and set expirations
         value.expiresIn = expiresIn;

222-260: Validate tokenRaw presence before broadcasting.

The broadcast logic doesn't check if broadcast.tokenRaw is present. Broadcasting an empty or falsy token string could cause issues in receiving tabs.

Apply this diff to add validation:

     if (broadcast && broadcastChannel) {
       const expectedTokenId = computeTokenId(broadcast.sessionId, broadcast.template, broadcast.organizationId);
       if (entry.tokenId !== expectedTokenId) {
         debugLogger.warn(
           'Skipping broadcast - tokenId mismatch',
           { expectedTokenId, providedTokenId: entry.tokenId },
           'tokenCache',
         );
         return;
       }

+      if (!broadcast.tokenRaw) {
+        debugLogger.warn('Skipping broadcast - empty tokenRaw', { tokenId: entry.tokenId }, 'tokenCache');
+        return;
+      }

       debugLogger.info(
         'Broadcasting token update to other tabs',
🧹 Nitpick comments (1)
packages/clerk-js/src/core/tokenCache.ts (1)

7-11: Refactor: import ClerkBroadcastChannel from @clerk/types instead of redefining.

The ClerkBroadcastChannel type is already defined in packages/types/src/clerk.ts (lines 77-81). Redefining it here creates duplication and potential drift.

Apply this diff to use the existing type:

-import type { ClerkCoreBroadcastChannelEvent, TokenResource } from '@clerk/types';
+import type { ClerkBroadcastChannel, ClerkCoreBroadcastChannelEvent, TokenResource } from '@clerk/types';

 import { debugLogger } from '@/utils/debug';

 import { Token } from './resources/internal';

-type ClerkBroadcastChannel = {
-  addEventListener(eventName: 'message', listener: (e: MessageEvent<ClerkCoreBroadcastChannelEvent>) => void): void;
-  close(): void;
-  postMessage(data: ClerkCoreBroadcastChannelEvent): void;
-};
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 308800d and f8c5df3.

📒 Files selected for processing (3)
  • packages/clerk-js/src/core/clerk.ts (5 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (4 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/core/resources/Session.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/core/tokenCache.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/clerk.ts (3)
packages/shared/src/ClerkBroadcastChannel.ts (1)
  • ClerkBroadcastChannel (23-81)
packages/types/src/clerk.ts (1)
  • ClerkBroadcastChannel (78-82)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (334-334)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/clerk.ts (1)
  • ClerkCoreBroadcastChannelEvent (65-76)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (144-148)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/clerk-js/src/core/clerk.ts (4)

2-2: LGTM! Imports are correct.

The imports for ClerkBroadcastChannel and SessionTokenCache are properly structured and align with the cross-tab broadcasting architecture.

Also applies to: 163-163


229-229: LGTM! Field declaration updated correctly.

The broadcast channel field type has been properly updated to use the new ClerkBroadcastChannel interface.


2756-2763: LGTM! Broadcast listener correctly handles signout.

The listener properly delegates token synchronization to SessionTokenCache and handles signout events without causing broadcast loops.


2768-2770: LGTM! Signout broadcast emission is correct.

The event bus correctly broadcasts signout events to other tabs using the new ClerkBroadcastChannel.

packages/clerk-js/src/core/tokenCache.ts (6)

18-28: LGTM! Interface extensions support cross-tab broadcasting.

The additions to TokenCacheEntry, TokenBroadcastMetadata, TokenCacheValue, and TokenCache properly support the cross-tab token synchronization feature.

Also applies to: 32-37, 39-45


73-75: LGTM! Token ID computation matches Session resource pattern.

The computeTokenId helper correctly mirrors the cache ID generation in Session.ts (line 143-147).


88-95: LGTM! Per-entry timer cleanup is now correct.

The clear() method now properly iterates over all cached values and clears their individual timeout handles before clearing the cache.


97-142: LGTM! Enhanced get() correctly handles expiration and leeway.

The token retrieval logic properly validates expiration with configurable leeway, includes the sync poller interval, and provides helpful debug logging.


262-325: LGTM! Internal set logic correctly tracks timers and handles broadcasts.

The setInternal method properly manages per-entry timeouts, respects explicit createdAt from broadcasts, and includes the unref() call for Node compatibility.


327-334: LGTM! Export and size method are correct.

The size() method and SessionTokenCache singleton export are properly implemented.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

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

396-413: Two-phase cache update correctly prevents duplicate fetches and enables cross-tab warming.

The immediate promise cache (line 397) deduplicates concurrent calls, and the resolved-token update (lines 403-413) with broadcast metadata allows other tabs to warm their caches.

Consider adding cache hardening from previous review.

A past review suggested validating tokenId, wrapping new Token(...) in try/catch, and enforcing monotonicity in __internal_updateTokenCache. While not critical for this PR's core functionality, these improvements would increase robustness against malformed broadcasts and race conditions.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8bf6b64 and cf813c8.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/resources/Session.ts (4 hunks)
  • packages/clerk-js/src/core/resources/__tests__/Session.test.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/core/resources/tests/Session.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/resources/Session.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/resources/Session.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/resources/Session.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/resources/Session.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/resources/Session.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/resources/Session.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/resources/Session.ts (5)
packages/clerk-js/src/utils/tokenId.ts (1)
  • buildTokenId (13-15)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (420-420)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
packages/clerk-js/src/core/resources/Session.ts (5)

28-35: LGTM!

The import additions (debugLogger, buildTokenId) and the passkeys path refactor align with the changes in the token fetch flow and cache ID generation.


148-148: LGTM!

Replacing the previous cache ID construction with buildTokenId standardizes the token identifier format and removes timestamp components that would prevent cross-tab cache hits—essential for the multi-tab token deduplication objective.


364-370: LGTM!

The debug log provides useful visibility into cache hits without performance overhead.


379-387: LGTM!

The info log appropriately captures new token fetches and provides sufficient context for debugging.


426-426: LGTM!

Reusing tokenRaw avoids a redundant getRawString() call and is semantically equivalent.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
integration/tests/session-token-cache/multi-session.test.ts (1)

91-92: Replace hardcoded timeout with deterministic wait.

The hardcoded 1-second timeout is unreliable and can cause flaky tests. Consider waiting for a specific condition instead.

Replace with a deterministic wait:

-      // eslint-disable-next-line playwright/no-wait-for-timeout
-      await page2.waitForTimeout(1000);
+      await page2.waitForFunction(() => {
+        const clerk = (window as any).Clerk;
+        return clerk?.session?.id && clerk?.user?.id;
+      });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cf813c8 and 80de3da.

📒 Files selected for processing (2)
  • integration/tests/session-token-cache/multi-session.test.ts (1 hunks)
  • integration/tests/session-token-cache/single-session.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • integration/tests/session-token-cache/single-session.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
integration/**

📄 CodeRabbit inference engine (.cursor/rules/global.mdc)

Framework integration templates and E2E tests should be placed under the integration/ directory

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
integration/**/*

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

End-to-end tests and integration templates must be located in the 'integration/' directory.

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
integration/**/*.{test,spec}.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Integration tests should use Playwright.

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • integration/tests/session-token-cache/multi-session.test.ts
🧬 Code graph analysis (1)
integration/tests/session-token-cache/multi-session.test.ts (2)
integration/testUtils/index.ts (1)
  • createTestUtils (24-86)
integration/presets/index.ts (1)
  • appConfigs (15-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
  • GitHub Check: Integration Tests (nuxt, chrome)
  • GitHub Check: Integration Tests (custom, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (tanstack-react-router, chrome)
  • GitHub Check: Integration Tests (vue, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (astro, chrome)
  • GitHub Check: Integration Tests (tanstack-react-start, chrome)
  • GitHub Check: Integration Tests (expo-web, chrome)
  • GitHub Check: Integration Tests (react-router, chrome)
  • GitHub Check: Integration Tests (billing, chrome)
  • GitHub Check: Integration Tests (machine, chrome)
  • GitHub Check: Integration Tests (ap-flows, chrome)
  • GitHub Check: Integration Tests (handshake:staging, chrome)
  • GitHub Check: Integration Tests (quickstart, chrome)
  • GitHub Check: Integration Tests (handshake, chrome)
  • GitHub Check: Integration Tests (sessions:staging, chrome)
  • GitHub Check: Integration Tests (elements, chrome)
  • GitHub Check: Integration Tests (express, chrome)
  • GitHub Check: Integration Tests (localhost, chrome)
  • GitHub Check: Integration Tests (sessions, chrome)
  • GitHub Check: Integration Tests (generic, chrome)
  • GitHub Check: Publish with pkg-pr-new
  • GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
  • GitHub Check: Static analysis
  • GitHub Check: Unit Tests (22, **)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (11)
integration/tests/session-token-cache/multi-session.test.ts (11)

1-6: LGTM!

Imports are well-organized and appropriate for the test suite.


7-17: LGTM!

The test suite is well-documented with clear objectives and proper environment configuration.


18-29: LGTM!

Test setup properly configures serial execution and creates necessary test users.


31-35: LGTM!

Proper cleanup ensures test isolation and resource management.


37-53: LGTM!

Comprehensive test flow documentation clearly explains the multi-session scenario and expected behaviors.


54-86: LGTM!

Tab1 setup correctly signs in user1, captures session information, and fetches an initial token with proper cache bypass.


108-141: LGTM!

The programmatic sign-in approach correctly preserves the existing session while adding a second one, with good error handling and informative comments.


142-166: LGTM!

Proper validation of the second session with correct assertions verifying session isolation between users.


167-180: LGTM!

Comprehensive verification of multi-session state with proper assertions covering session count, session IDs, and active session.


181-214: LGTM!

Excellent validation of token isolation and cache behavior using network interception to confirm tokens are served from cache without additional API calls.


216-228: LGTM!

Final validation correctly verifies tab independence, confirming that tab1's session remained unchanged despite tab2's multi-session operations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/clerk-js/src/utils/tokenId.ts (2)

23-25: Validate that sessionId is non-empty.

The filter(Boolean) will remove an empty string sessionId, producing an incorrect token identifier. While upstream callers likely never pass empty strings, adding a runtime check improves robustness.

Apply this diff to add validation:

 build: (sessionId: string, template?: string, organizationId?: string | null): string => {
+  if (!sessionId) {
+    throw new Error('sessionId cannot be empty');
+  }
   return [sessionId, template, organizationId].filter(Boolean).join('-');
 },

42-49: Consider validating that tokenId starts with sessionId.

The method assumes tokenId is well-formed and starts with sessionId, but doesn't validate this. If a caller passes mismatched parameters, extractTemplate will produce incorrect results without any indication of the error.

Apply this diff to add validation:

 parse: (tokenId: string, sessionId: string, organizationId?: string | null): ParsedTokenId => {
+  if (!tokenId.startsWith(sessionId)) {
+    throw new Error(`tokenId "${tokenId}" does not start with sessionId "${sessionId}"`);
+  }
   const template = TokenId.extractTemplate(tokenId, sessionId, organizationId);
   return {
     organizationId,
     sessionId,
     template,
   };
 },
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7d442e9 and 60a7e8f.

⛔ Files ignored due to path filters (1)
  • .typedoc/__tests__/__snapshots__/file-structure.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (14)
  • .changeset/every-dryers-refuse.md (1 hunks)
  • .changeset/slick-ducks-bet.md (1 hunks)
  • integration/tests/session-token-cache/multi-session.test.ts (1 hunks)
  • integration/tests/session-token-cache/single-session.test.ts (1 hunks)
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1 hunks)
  • packages/clerk-js/src/core/clerk.ts (3 hunks)
  • packages/clerk-js/src/core/resources/Session.ts (4 hunks)
  • packages/clerk-js/src/core/resources/__tests__/Session.test.ts (2 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
  • packages/clerk-js/src/ui/elements/PhoneInput/__tests__/useFormattedPhoneNumber.test.ts (1 hunks)
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts (1 hunks)
  • packages/clerk-js/src/utils/index.ts (1 hunks)
  • packages/clerk-js/src/utils/tokenId.ts (1 hunks)
  • packages/shared/src/localStorageBroadcastChannel.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/Session.test.ts
  • integration/tests/session-token-cache/multi-session.test.ts
  • .changeset/slick-ducks-bet.md
  • packages/clerk-js/src/ui/elements/PhoneInput/tests/useFormattedPhoneNumber.test.ts
  • packages/shared/src/localStorageBroadcastChannel.ts
  • .changeset/every-dryers-refuse.md
  • packages/clerk-js/src/utils/index.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • integration/tests/session-token-cache/single-session.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • integration/tests/session-token-cache/single-session.test.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • integration/tests/session-token-cache/single-session.test.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/tokenId.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/resources/Session.ts
  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • integration/tests/session-token-cache/single-session.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Visual regression testing should be performed for UI components.

Files:

  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/clerk-js/src/utils/__tests__/tokenId.test.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
integration/**

📄 CodeRabbit inference engine (.cursor/rules/global.mdc)

Framework integration templates and E2E tests should be placed under the integration/ directory

Files:

  • integration/tests/session-token-cache/single-session.test.ts
integration/**/*

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

End-to-end tests and integration templates must be located in the 'integration/' directory.

Files:

  • integration/tests/session-token-cache/single-session.test.ts
integration/**/*.{test,spec}.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Integration tests should use Playwright.

Files:

  • integration/tests/session-token-cache/single-session.test.ts
🧬 Code graph analysis (6)
packages/clerk-js/src/utils/tokenId.ts (1)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
packages/clerk-js/src/core/resources/Session.ts (6)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (400-400)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/events.ts (2)
  • eventBus (31-31)
  • events (6-14)
packages/clerk-js/src/core/resources/Base.ts (1)
  • path (164-177)
packages/clerk-js/src/core/resources/Token.ts (1)
  • Token (6-53)
packages/clerk-js/src/utils/__tests__/tokenId.test.ts (2)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (3)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (400-400)
packages/clerk-js/src/test/core-fixtures.ts (1)
  • mockJwt (18-19)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
integration/tests/session-token-cache/single-session.test.ts (2)
integration/testUtils/index.ts (1)
  • createTestUtils (24-86)
integration/presets/index.ts (1)
  • appConfigs (15-32)
🪛 Gitleaks (8.28.0)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts

[high] 159-159: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


[high] 179-179: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


[high] 215-215: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
  • GitHub Check: Integration Tests (expo-web, chrome)
  • GitHub Check: Integration Tests (machine, chrome)
  • GitHub Check: Integration Tests (vue, chrome)
  • GitHub Check: Integration Tests (react-router, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (astro, chrome)
  • GitHub Check: Integration Tests (custom, chrome)
  • GitHub Check: Integration Tests (tanstack-react-start, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (billing, chrome)
  • GitHub Check: Integration Tests (ap-flows, chrome)
  • GitHub Check: Integration Tests (quickstart, chrome)
  • GitHub Check: Integration Tests (nuxt, chrome)
  • GitHub Check: Integration Tests (tanstack-react-router, chrome)
  • GitHub Check: Integration Tests (handshake:staging, chrome)
  • GitHub Check: Integration Tests (localhost, chrome)
  • GitHub Check: Integration Tests (sessions, chrome)
  • GitHub Check: Integration Tests (sessions:staging, chrome)
  • GitHub Check: Integration Tests (handshake, chrome)
  • GitHub Check: Integration Tests (elements, chrome)
  • GitHub Check: Integration Tests (express, chrome)
  • GitHub Check: Integration Tests (generic, chrome)
  • GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
  • GitHub Check: Publish with pkg-pr-new
  • GitHub Check: Unit Tests (22, **)
  • GitHub Check: Static analysis
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
packages/clerk-js/src/utils/tokenId.ts (2)

1-5: LGTM!

The interface definition is clean and properly typed, with appropriate use of optional and nullable types.


62-78: Logic correctly handles the documented token format.

The extraction logic properly handles the sessionId[-template][-organizationId] format with appropriate early returns and string manipulation. The implementation correctly strips the trailing organizationId when present.

Note: If a template name itself contains the organizationId as a substring (e.g., template "org_456-feature" with organizationId "org_456"), the trailing strip at Line 73-75 could produce unexpected results. However, this is consistent with the documented format where organizationId appears only at the end, and templates containing organizationId substrings would be considered invalid usage.

packages/clerk-js/src/core/clerk.ts (4)

227-227: LGTM! Native BroadcastChannel is the right choice.

The switch from the custom LocalStorageBroadcastChannel to the native BroadcastChannel API is appropriate and simplifies the implementation while maintaining cross-tab communication.


2553-2555: LGTM! Proper feature detection and initialization.

The feature detection correctly checks for BroadcastChannel availability before instantiation, which gracefully handles environments where the API is not supported (e.g., older browsers, some SSR contexts).


2767-2771: LGTM! Proper message validation in place.

The listener correctly validates the message type before acting, preventing unintended signouts from other message types. The use of optional chaining safely handles edge cases where data might be undefined.


2776-2778: LGTM! Consistent message structure.

The structured message format { type: 'signout' } is consistent with the listener validation and allows for future extension with additional message types if needed.

@jacekradko
Copy link
Member Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @jacekradko - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.1.37-snapshot.v20251002194847
@clerk/astro 2.13.5-snapshot.v20251002194847
@clerk/backend 2.17.1-snapshot.v20251002194847
@clerk/chrome-extension 2.7.2-snapshot.v20251002194847
@clerk/clerk-js 5.98.0-snapshot.v20251002194847
@clerk/elements 0.23.68-snapshot.v20251002194847
@clerk/clerk-expo 2.15.5-snapshot.v20251002194847
@clerk/expo-passkeys 0.4.5-snapshot.v20251002194847
@clerk/express 1.7.36-snapshot.v20251002194847
@clerk/fastify 2.4.36-snapshot.v20251002194847
@clerk/localizations 3.25.6-snapshot.v20251002194847
@clerk/nextjs 6.33.2-snapshot.v20251002194847
@clerk/nuxt 1.9.3-snapshot.v20251002194847
@clerk/clerk-react 5.50.0-snapshot.v20251002194847
@clerk/react-router 2.0.2-snapshot.v20251002194847
@clerk/remix 4.13.2-snapshot.v20251002194847
@clerk/shared 3.27.2-snapshot.v20251002194847
@clerk/tanstack-react-start 0.25.2-snapshot.v20251002194847
@clerk/testing 1.13.2-snapshot.v20251002194847
@clerk/themes 2.4.24-snapshot.v20251002194847
@clerk/types 4.91.0-snapshot.v20251002194847
@clerk/vue 1.14.2-snapshot.v20251002194847

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/[email protected] --save-exact

@clerk/astro

npm i @clerk/[email protected] --save-exact

@clerk/backend

npm i @clerk/[email protected] --save-exact

@clerk/chrome-extension

npm i @clerk/[email protected] --save-exact

@clerk/clerk-js

npm i @clerk/[email protected] --save-exact

@clerk/elements

npm i @clerk/[email protected] --save-exact

@clerk/clerk-expo

npm i @clerk/[email protected] --save-exact

@clerk/expo-passkeys

npm i @clerk/[email protected] --save-exact

@clerk/express

npm i @clerk/[email protected] --save-exact

@clerk/fastify

npm i @clerk/[email protected] --save-exact

@clerk/localizations

npm i @clerk/[email protected] --save-exact

@clerk/nextjs

npm i @clerk/[email protected] --save-exact

@clerk/nuxt

npm i @clerk/[email protected] --save-exact

@clerk/clerk-react

npm i @clerk/[email protected] --save-exact

@clerk/react-router

npm i @clerk/[email protected] --save-exact

@clerk/remix

npm i @clerk/[email protected] --save-exact

@clerk/shared

npm i @clerk/[email protected] --save-exact

@clerk/tanstack-react-start

npm i @clerk/[email protected] --save-exact

@clerk/testing

npm i @clerk/[email protected] --save-exact

@clerk/themes

npm i @clerk/[email protected] --save-exact

@clerk/types

npm i @clerk/[email protected] --save-exact

@clerk/vue

npm i @clerk/[email protected] --save-exact

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1)

157-276: Generate offending JWTs at runtime to avoid gitleaks hits

Hardcoded JWT-shaped literals (invalidJwtWithoutIat, invalidJwtWithoutExp, olderJwt, plus the inline futureJwt/pastJwt/soonJwt strings) were previously flagged and will still trip secret scanning. Please build these tokens with the existing helper (or a new generic helper) instead of embedding the base64 strings.

+function createJwt(claims: Record<string, unknown>): string {
+  const payloadB64 = btoa(JSON.stringify(claims)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
+  const headerB64 = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9';
+  return `${headerB64}.${payloadB64}.test-signature`;
+}
+
 function createJwtWithTtl(iatSeconds: number, ttlSeconds: number): string {
-  const payload = {
-    data: 'test-data',
-    exp: iatSeconds + ttlSeconds,
-    iat: iatSeconds,
-  };
-  const payloadString = JSON.stringify(payload);
-  const payloadB64 = btoa(payloadString).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
-  const headerB64 = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9';
-  const signature = 'test-signature';
-  return `${headerB64}.${payloadB64}.${signature}`;
+  return createJwt({
+    data: 'test-data',
+    exp: iatSeconds + ttlSeconds,
+    iat: iatSeconds,
+  });
 }
@@
-      const invalidJwtWithoutIat =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U';
+      const invalidJwtWithoutIat = createJwt({ exp: 1666648850 });
@@
-      const invalidJwtWithoutExp =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE2NjY2NDgyNTB9.NjdkMzg4YzYwZTQxZWQ2MTJkNmQ1ZDQ5YzY4ZTQxNjI';
+      const invalidJwtWithoutExp = createJwt({ iat: 1666648250 });
@@
-      const olderJwt =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE2NjY2NDg4NTAsImlhdCI6MTY2NjY0ODE5MH0.Z1BC47lImYvaAtluJlY-kBo0qOoAk42Xb-gNrB2SxJg';
+      const olderJwt = createJwt({ exp: 1666648850, iat: 1666648190 });
@@
-      const futureJwt = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.${btoa(
-        JSON.stringify({ iat: Math.floor(Date.now() / 1000), exp: futureExp }),
-      )}.signature`;
+      const futureJwt = createJwt({ iat: Math.floor(Date.now() / 1000), exp: futureExp });
@@
-      const pastJwt = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.${btoa(JSON.stringify({ iat, exp }))}.signature`;
+      const pastJwt = createJwt({ iat, exp });

Apply the same helper for soonJwt and similar fixtures in this file.
This eliminates the literals and keeps CI green.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 60a7e8f and 3139d59.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Visual regression testing should be performed for UI components.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (3)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (395-395)
packages/clerk-js/src/test/core-fixtures.ts (1)
  • mockJwt (18-19)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Comment on lines +321 to +335
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;

// Mutate cached value and set expirations
value.expiresIn = expiresIn;
timer = setTimeout(deleteKey, expiresIn * 1000);

const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
value.timeoutId = timeoutId;

// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof timer.unref === 'function') {
timer.unref();
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}

Copy link
Contributor

@coderabbitai coderabbitai bot Oct 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix expiration timer to use remaining lifetime

expiresIn is computed as exp - iat, but the timeout is scheduled for that full duration starting now. If the token was issued earlier (common case), the cache keeps it alive well past real expiry and get() will keep serving an already-expired token. Compute the remaining seconds from exp - currentTime, guard against non-positive values, update the stored createdAt, and schedule the timeout from that remaining interval.

-        const expiresAt = claims.exp;
-        const issuedAt = claims.iat;
-        const expiresIn: Seconds = expiresAt - issuedAt;
-
-        value.expiresIn = expiresIn;
-
-        const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
+        const expiresAt = claims.exp;
+        const nowAfterResolve = Math.floor(Date.now() / 1000);
+        const remainingSeconds = expiresAt - nowAfterResolve;
+        if (remainingSeconds <= 0) {
+          deleteKey();
+          return;
+        }
+
+        value.createdAt = nowAfterResolve;
+        value.expiresIn = remainingSeconds;
+
+        const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;
// Mutate cached value and set expirations
value.expiresIn = expiresIn;
timer = setTimeout(deleteKey, expiresIn * 1000);
const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
value.timeoutId = timeoutId;
// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof timer.unref === 'function') {
timer.unref();
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}
const expiresAt = claims.exp;
const nowAfterResolve = Math.floor(Date.now() / 1000);
const remainingSeconds = expiresAt - nowAfterResolve;
if (remainingSeconds <= 0) {
deleteKey();
return;
}
value.createdAt = nowAfterResolve;
value.expiresIn = remainingSeconds;
const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
value.timeoutId = timeoutId;
// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/tokenCache.ts around lines 321 to 335, the code
computes expiresIn as exp - iat and schedules the deletion from now for that
full lifetime; instead compute the remaining lifetime as remainingSeconds =
expiresAt - Math.floor(Date.now() / 1000), if remainingSeconds <= 0 call
deleteKey immediately and set value.expiresIn = 0 (and value.createdAt to now),
otherwise set value.expiresIn = remainingSeconds, update value.createdAt to the
current time, schedule the timeout using remainingSeconds * 1000, store the
timeout id on value.timeoutId, and still call unref() when available; ensure you
guard types when calling unref.

Copy link
Member Author

Choose a reason for hiding this comment

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

Your suggestion does not take clock skew into account

Copy link
Contributor

Choose a reason for hiding this comment

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

@jacekradko You're right to flag clock skew—good catch! Let me clarify the trade-offs:

The issue with my original suggestion:
Using exp - currentWallClock for the timeout is vulnerable if the client clock drifts significantly from the server clock that issued the token.

A clock-skew-resistant approach:

  1. Store createdAt as iat (server timestamp) to track when the token was actually issued
  2. Compute expiresIn as exp - iat (the token's lifetime in seconds, from the server's perspective)
  3. In get(), compute elapsed as currentWallClock - iat and check expiresIn - elapsed against leeway
  4. For the timeout, schedule it based on the remaining wall-clock time: max(0, exp - currentWallClock)

This hybrid approach:

  • Tracks expiration using server timestamps (immune to clock skew during caching)
  • The get() method's leeway (10s + 5s sync) absorbs minor clock drift
  • Schedules the cleanup timeout based on wall-clock remaining time (necessary for setTimeout)

Revised suggestion:

const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;

// Use server-issued timestamp for createdAt to be immune to client clock drift
value.createdAt = issuedAt;
value.expiresIn = expiresIn;

// Schedule cleanup based on remaining wall-clock time
const nowWallClock = Math.floor(Date.now() / 1000);
const remainingSeconds = Math.max(0, expiresAt - nowWallClock);

if (remainingSeconds === 0) {
  deleteKey();
  return;
}

const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
value.timeoutId = timeoutId;

if (typeof (timeoutId as any).unref === 'function') {
  (timeoutId as any).unref();
}

This way, expiration tracking uses server time (iat/exp), while the cleanup timer uses the client's wall clock—and get() with leeway handles the boundary cases. Does this address your concern?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

284-287: Use createJwtWithTtl helper for consistency.

For maintainability and consistency with the rest of the test suite, replace the inline JWT construction with the existing createJwtWithTtl helper.

Apply this diff:

-      const futureExp = Math.floor(Date.now() / 1000) + 3600; // 1 hour from now
-      const futureJwt = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.${btoa(
-        JSON.stringify({ iat: Math.floor(Date.now() / 1000), exp: futureExp }),
-      )}.signature`;
+      const nowSeconds = Math.floor(Date.now() / 1000);
+      const futureJwt = createJwtWithTtl(nowSeconds, 3600);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3139d59 and acf1e78.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts (1 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Unit tests should use Jest or Vitest as the test runner.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Visual regression testing should be performed for UI components.

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
**/__tests__/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces

Files:

  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts (3)
packages/clerk-js/src/core/tokenCache.ts (1)
  • SessionTokenCache (403-403)
packages/clerk-js/src/test/core-fixtures.ts (1)
  • mockJwt (18-19)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
🪛 Gitleaks (8.28.0)
packages/clerk-js/src/core/__tests__/tokenCache.test.ts

[high] 159-159: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


[high] 179-179: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


[high] 215-215: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/tokenCache.ts (1)

329-342: Verify timeout scheduling accounts for token age.

The timeout is scheduled for expiresIn * 1000 milliseconds from now (line 335), where expiresIn = exp - iat. If the token was issued earlier (common for broadcast messages), the cache will keep the entry past its actual expiration time. While get() correctly rejects expired tokens via leeway checks, the cleanup timer fires late, causing entries to linger in memory.

Example: Token issued at T=0 with 60s lifetime, received at T=30. Timeout scheduled for T=90 (30s after actual expiry at T=60).

The past review thread discussed this concern. The current approach may be intentional to avoid clock skew issues (using server-issued iat/exp timestamps rather than client wall-clock). However, this trades off timely memory cleanup for clock-skew resilience. Consider if this is the desired trade-off, or if a hybrid approach is needed:

// Option 1: Schedule based on remaining wall-clock time (past suggestion)
const nowWallClock = Math.floor(Date.now() / 1000);
const remainingSeconds = Math.max(0, expiresAt - nowWallClock);
if (remainingSeconds === 0) {
  deleteKey();
  return;
}
const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);

// Option 2: Keep current approach but document the trade-off
// Current: Uses token lifetime (expiresIn) for cleanup scheduling
// Pro: Immune to client clock skew
// Con: Entries linger past expiry; get() leeway handles correctness

If the current approach is intentional, consider adding a comment explaining the clock-skew vs. memory-cleanup trade-off.

Comment on lines +158 to +159
const invalidJwtWithoutIat =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U';
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Replace remaining hardcoded JWT literals to prevent gitleaks violations.

Despite multiple past review rounds, three hardcoded JWT strings remain and will trigger gitleaks failures in CI. The existing createJwtWithTtl helper should be used instead.

Apply this diff to generate these tokens at runtime:

-    it('skips token with missing iat claim', () => {
-      const invalidJwtWithoutIat =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U';
+    it('skips token with missing iat claim', () => {
+      // Generate JWT without iat claim
+      const payload = { sub: '1234567890', exp: 1666648310 };
+      const payloadB64 = btoa(JSON.stringify(payload)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
+      const headerB64 = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9';
+      const invalidJwtWithoutIat = `${headerB64}.${payloadB64}.test-signature`;
-    it('skips token with missing exp claim', () => {
-      // JWT with iat but no exp: {iat: 1666648250}
-      const invalidJwtWithoutExp =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE2NjY2NDgyNTB9.NjdkMzg4YzYwZTQxZWQ2MTJkNmQ1ZDQ5YzY4ZTQxNjI';
+    it('skips token with missing exp claim', () => {
+      // JWT with iat but no exp: {iat: 1666648250}
+      const payload = { iat: 1666648250 };
+      const payloadB64 = btoa(JSON.stringify(payload)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
+      const headerB64 = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9';
+      const invalidJwtWithoutExp = `${headerB64}.${payloadB64}.test-signature`;
-      // mockJwt has iat: 1666648250, so create an older one with iat: 1666648190 (60 seconds earlier)
-      const olderJwt =
-        'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE2NjY2NDg4NTAsImlhdCI6MTY2NjY0ODE5MH0.Z1BC47lImYvaAtluJlY-kBo0qOoAk42Xb-gNrB2SxJg';
+      // mockJwt has iat: 1666648250, so create an older one with iat: 1666648190 (60 seconds earlier)
+      const olderJwt = createJwtWithTtl(1666648190, 660);

Based on static analysis hints

Also applies to: 178-179, 214-215

🧰 Tools
🪛 Gitleaks (8.28.0)

[high] 159-159: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

🤖 Prompt for AI Agents
In packages/clerk-js/src/core/__tests__/tokenCache.test.ts around lines 158-159
(and also at lines 178-179 and 214-215), replace the hardcoded JWT string
literals with runtime-generated tokens using the existing createJwtWithTtl test
helper to avoid gitleaks violations; for each literal, call createJwtWithTtl
with the appropriate payload/ttl to produce the same test scenarios (e.g., a JWT
missing iat, expired token, and valid token variants used in those tests),
update any expectations to use the generated values, and ensure the helper is
imported if not already.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/clerk-js/src/core/tokenCache.ts (1)

174-202: Use nullish coalescing for leeway parameter.

Line 191 uses (leeway || 1) which treats an explicit 0 leeway as 1. Per a past review comment marked as addressed, this should use nullish coalescing:

-    const expiresSoon = value.expiresIn! - elapsed < (leeway || 1) + SYNC_LEEWAY;
+    const expiresSoon = value.expiresIn! - elapsed < (leeway ?? 1) + SYNC_LEEWAY;

This ensures that if a caller explicitly passes leeway = 0, it is respected rather than coerced to 1.

Based on past review comment claiming this was addressed in commits 2754a77 to 30af700, but the code still shows the old pattern.

🧹 Nitpick comments (2)
packages/clerk-js/src/core/tokenCache.ts (2)

144-161: Verify listener cleanup on re-initialization scenarios.

The ensureBroadcastChannel function returns early if broadcastChannel exists (line 149-151), which prevents duplicate listeners. However, if the channel is closed externally or becomes stale, re-initialization might occur without removing the old listener first.

While the current pattern (check existence, return early) prevents duplicates in normal flow, consider adding defensive cleanup if broadcastChannel is non-null but closed:

 const ensureBroadcastChannel = (): BroadcastChannel | null => {
   if (!__BUILD_VARIANT_CHANNEL__) {
     return null;
   }

   if (broadcastChannel) {
+    // Defensive: verify channel is still open
     return broadcastChannel;
   }

This is a low-probability edge case, but worth considering for robustness.


208-290: Add tokenRaw validation before parsing.

The function parses data.tokenRaw without first checking if it's non-empty (line 227). While the Token constructor may handle empty strings gracefully, explicit validation improves clarity and enables better error messages:

+  if (!data.tokenRaw) {
+    debugLogger.warn(
+      'Ignoring broadcast with empty tokenRaw',
+      { tokenId: data.tokenId, traceId: data.traceId },
+      'tokenCache',
+    );
+    return;
+  }
+
   let token: Token;
   try {

This makes the validation explicit and consistent with the pattern used in setInternal (lines 351-352).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between acf1e78 and 38323d5.

📒 Files selected for processing (6)
  • packages/clerk-js/bundlewatch.config.json (1 hunks)
  • packages/clerk-js/package.json (2 hunks)
  • packages/clerk-js/rspack.config.js (6 hunks)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
  • packages/clerk-js/src/global.d.ts (1 hunks)
  • packages/clerk-js/src/index.channel.browser.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/package.json
  • packages/clerk-js/rspack.config.js
  • packages/clerk-js/bundlewatch.config.json
  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
packages/*/package.json

📄 CodeRabbit inference engine (.cursor/rules/global.mdc)

All publishable packages should be placed under the packages/ directory

packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.

Files:

  • packages/clerk-js/package.json
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/rspack.config.js
  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/rspack.config.js
  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/index.channel.browser.ts
  • packages/clerk-js/src/core/tokenCache.ts
  • packages/clerk-js/src/global.d.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/index.channel.browser.ts (1)
packages/clerk-js/src/ui/Components.tsx (1)
  • mountComponentRenderer (182-222)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (21)
packages/clerk-js/bundlewatch.config.json (1)

5-5: LGTM!

The new bundlewatch entry for the channel variant follows the established pattern and uses an appropriate size limit consistent with the standard browser variant.

packages/clerk-js/src/global.d.ts (1)

14-14: LGTM!

The build-time feature flag declaration follows the established naming convention and is properly typed as a boolean, consistent with other variant flags.

packages/clerk-js/package.json (2)

29-29: LGTM!

Adding "channel" to the files array ensures the channel build artifacts are included in the published package, consistent with other variants.


46-46: LGTM!

The dev:channel script correctly references the clerk.channel.browser variant and follows the established pattern used by other variant dev scripts.

packages/clerk-js/src/index.channel.browser.ts (3)

1-10: LGTM!

The entry point structure correctly prioritizes webpack chunk loading setup and properly initializes the Clerk component renderer, following established patterns from other browser variants.


12-24: LGTM!

The configuration reading logic correctly falls back from script data attributes to window globals, consistent with other browser entry points.


34-36: LGTM!

Hot module replacement is appropriately configured for the development workflow.

packages/clerk-js/rspack.config.js (6)

20-20: LGTM!

The new channel variant constant follows the established naming convention and is properly positioned alongside other variants.


31-31: LGTM!

The source file mapping correctly points to the new channel browser entry point, maintaining consistency with the variant-to-source mapping pattern.


63-63: LGTM!

The build-time feature flag is properly defined and will be set to true only when building the channel variant, enabling conditional compilation of channel-specific code.


430-435: LGTM!

The production configuration for the channel browser variant correctly merges all necessary build configurations (entry, common settings, production optimizations, and chunking), following the established pattern used by other browser variants.


551-551: LGTM!

Including the channel browser variant in the production build exports ensures it will be built alongside other variants during production builds.


659-663: LGTM!

The development configuration for the channel browser variant correctly merges the necessary development settings, enabling proper dev server support with hot module replacement.

packages/clerk-js/src/core/tokenCache.ts (8)

1-14: LGTM! Imports and type definitions are correct.

The imports follow ES6 module conventions and the TokenCacheKeyJSON interface appropriately identifies cache entries by tokenId and optional audience.


16-43: LGTM! Interface definitions are well-documented.

The JSDoc comments for TokenCacheEntry and TokenCacheValue are comprehensive and follow coding guidelines. The addition of timeoutId field enables proper per-entry timer cleanup as requested in past reviews.


45-81: LGTM! Public API documentation is complete.

The TokenCache interface has comprehensive JSDoc covering all methods, parameters, return values, and side effects. This meets the coding guideline requirement for documenting all public APIs.


83-120: LGTM! TokenCacheKey implementation is well-structured.

The TokenCacheKey class provides clear conversion between object and string representations, with appropriate JSDoc documentation. The format prefix::tokenId::audience is explicit and the implementation handles optional audience correctly.


122-132: LGTM! SessionTokenEvent interface is well-defined.

The interface captures all necessary fields for cross-tab token synchronization, including a traceId for debugging. The JSDoc clearly describes its purpose.


165-172: LGTM! Timer cleanup is correctly implemented.

The clear() method iterates all cached values and cancels each entry's timeout before clearing the cache, preventing timer leaks as requested in past reviews.


292-296: LGTM! The set() method is clean.

The method correctly ensures the broadcast channel is initialized before delegating to setInternal with broadcasting enabled.


394-406: LGTM! Cleanup and utility methods are correct.

The close(), size(), and return statement correctly implement the public TokenCache interface. The close() method properly releases the broadcast channel and nullifies the reference.

Comment on lines 84 to +348
entry.tokenResolver
.then(newToken => {
if (!newToken.jwt) {
const claims = newToken.jwt?.claims;
if (!claims || typeof claims.exp !== 'number' || typeof claims.iat !== 'number') {
return deleteKey();
}

const expiresAt = newToken.jwt.claims.exp;
const issuedAt = newToken.jwt.claims.iat;
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;

// Mutate cached value and set expirations
value.expiresIn = expiresIn;
timer = setTimeout(deleteKey, expiresIn * 1000);

const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
value.timeoutId = timeoutId;

// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof timer.unref === 'function') {
timer.unref();
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Fix timeout scheduling to use remaining lifetime, not full token lifetime.

The current implementation schedules the cleanup timeout for the full token lifetime (expiresIn = exp - iat) starting from now (line 340). This is incorrect: if the token was issued earlier (common case, especially when receiving broadcasts from other tabs), the cache will retain the token well past its actual expiration time.

Example: A token issued 30 minutes ago with 1-hour expiration will be cached for another 1 hour from now, meaning it stays in cache for 1.5 hours total.

Impact: The get() method's leeway check (lines 174-202) will prevent returning expired tokens to callers, but the cache entries will linger unnecessarily, consuming memory and potentially causing confusion during debugging.

Solution: Compute the remaining lifetime from exp - currentTime and schedule the timeout for that duration:

         const expiresAt = claims.exp;
         const issuedAt = claims.iat;
-        const expiresIn: Seconds = expiresAt - issuedAt;
+        const nowSeconds = Math.floor(Date.now() / 1000);
+        const remainingSeconds = Math.max(0, expiresAt - nowSeconds);
 
-        value.expiresIn = expiresIn;
+        value.expiresIn = expiresAt - issuedAt;  // Store original lifetime for get() leeway checks
 
-        const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
+        if (remainingSeconds === 0) {
+          deleteKey();
+          return;
+        }
+
+        const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
         value.timeoutId = timeoutId;

Note on clock skew (per author's earlier comment):

  • Store value.createdAt = issuedAt (server timestamp) to make expiration checks immune to client clock drift
  • Schedule the timeout using remainingSeconds (wall-clock time) because setTimeout requires client time
  • The get() method's leeway (10s + 5s) absorbs minor clock drift at read time

This approach balances clock-skew resilience with correct cleanup scheduling.


🏁 Script executed:

#!/bin/bash
# Locate tokenCache.ts and inspect get() implementation
tokenCache=$(fd tokenCache.ts)
echo "File: $tokenCache"
echo "Lines 150-250:"
sed -n '150,250p' "$tokenCache"

Length of output: 3103


🏁 Script executed:

#!/bin/bash
file="packages/clerk-js/src/core/tokenCache.ts"

echo "Occurrences of 'createdAt' in $file:"
grep -n "createdAt" "$file"

echo -e "\nOccurrences of 'cache.set' in $file:"
grep -n "cache.set" "$file"

echo -e "\nContext around each cache.set:"
for ln in $(grep -n "cache.set" "$file" | cut -d: -f1); do
  start=$((ln-5>1?ln-5:1))
  end=$((ln+5))
  echo -e "\n--- Lines $start to $end ---"
  sed -n "${start},${end}p" "$file"
done

Length of output: 1182


🏁 Script executed:

#!/bin/bash
file="packages/clerk-js/src/core/tokenCache.ts"
grep -n "const setInternal" "$file"
sed -n '250,360p' "$file"

Length of output: 3772


Use remaining lifetime when scheduling token cleanup
The current setTimeout(deleteKey, (exp – iat)*1000) retains tokens past their real expiry if they were issued earlier. Compute:

const now = Math.floor(Date.now()/1000);
const remaining = Math.max(0, exp  now);
if (remaining === 0) return deleteKey();
setTimeout(deleteKey, remaining*1000);

and keep value.expiresIn = exp – iat for your leeway checks.

Comment on lines 26 to 32
if (!window.Clerk) {
window.Clerk = new Clerk(publishableKey, {
proxyUrl,
// @ts-expect-error
domain,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify the type error suppression on the domain property.

The @ts-expect-error directive on line 29 suggests the domain property may not be properly typed in the Clerk constructor options. This could indicate a missing or incorrect type definition.

Run the following script to check the Clerk constructor type signature and verify if domain should be included:


🏁 Script executed:

#!/bin/bash
# Description: Check Clerk constructor signature and options interface

# Search for the Clerk class constructor and its options type
ast-grep --pattern $'class Clerk {
  $$$
  constructor($_, $OPTIONS) {
    $$$
  }
  $$$
}'

# Search for ClerkOptions interface or type definition
rg -nP --type=ts -A 20 'interface\s+ClerkOptions|type\s+ClerkOptions\s*='

Length of output: 3560


🏁 Script executed:

rg -nP --type=ts 'domain' -n packages/types/src/clerk.ts

Length of output: 242


🏁 Script executed:

rg -nP --context=5 'domain: string;' -n packages/types/src/clerk.ts

Length of output: 305


Remove unnecessary @ts-expect-error on domain.
The domain option is declared in ClerkOptions (packages/types/src/clerk.ts:205) and accepted by the Clerk constructor—delete the // @ts-expect-error before domain.

🤖 Prompt for AI Agents
In packages/clerk-js/src/index.channel.browser.ts around lines 26 to 32, remove
the unnecessary TypeScript suppression: delete the "// @ts-expect-error" comment
before the domain property in the Clerk constructor options. The domain option
is already declared in ClerkOptions and accepted by the constructor, so simply
eliminate that comment and keep the domain property as-is to allow proper type
checking.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/clerk-js/src/core/tokenCache.ts (2)

174-201: Use nullish coalescing for leeway parameter

Line 190 uses (leeway || 1) which treats an explicit 0 leeway as falsy and replaces it with 1. This prevents callers from intentionally setting leeway to 0.

Apply this diff:

-    const expiresSoon = value.expiresIn! - elapsed < (leeway || 1) + SYNC_LEEWAY;
+    const expiresSoon = value.expiresIn! - elapsed < (leeway ?? 1) + SYNC_LEEWAY;

Note: A previous review comment claimed this was addressed in commits 2754a77 to 30af700, but the change wasn't applied.


348-384: LGTM: Comprehensive broadcast logic with minor suggestion

The broadcast implementation is well-structured and includes:

  • Proper guards for channel existence and broadcast flag
  • Token extraction and validation
  • Template parsing using TokenId.extractTemplate
  • Token ID validation against expected format
  • Trace ID generation for debugging
  • Comprehensive logging

The implicit tokenRaw validation via tokenRaw && claims.sid (line 351) is functional but could be more explicit for clarity.

Consider making the tokenRaw validation more explicit:

         const channel = broadcastChannel;
         if (channel && options.broadcast) {
           const tokenRaw = newToken.getRawString();
-          if (tokenRaw && claims.sid) {
+          if (!tokenRaw) {
+            debugLogger.warn('Skipping broadcast - empty tokenRaw', { tokenId: entry.tokenId }, 'tokenCache');
+            return;
+          }
+          if (claims.sid) {
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 38323d5 and 4f0c799.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/tokenCache.ts (4 hunks)
  • packages/clerk-js/vitest.config.mts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/tokenCache.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/tokenCache.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/tokenCache.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/tokenCache.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/tokenCache.ts (4)
packages/types/src/token.ts (1)
  • TokenResource (5-9)
packages/clerk-js/src/utils/tokenId.ts (1)
  • TokenId (11-79)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (150-179)
packages/clerk-js/src/core/resources/Session.ts (1)
  • template (145-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (9)
packages/clerk-js/src/core/tokenCache.ts (9)

1-31: LGTM: Imports and interface documentation

The imports are properly structured, and the JSDoc documentation for TokenCacheKeyJSON and TokenCacheEntry is comprehensive and follows coding guidelines. The createdAt field addition is well-documented with clear semantics (seconds since UNIX epoch).


33-43: LGTM: Timer tracking properly implemented

The addition of timeoutId to TokenCacheValue properly addresses the timer leak issue flagged in previous reviews. The JSDoc clearly documents the purpose of this internal type.


45-81: LGTM: Comprehensive interface documentation

The TokenCache interface now has excellent JSDoc documentation for all methods, including parameter descriptions, return types, and side effects. This meets the coding guideline requirements for public API documentation.


83-120: LGTM: Clean key abstraction

The TokenCacheKey class provides a clean abstraction for converting between object and string representations. The addition of BROADCAST and NO_BROADCAST constants improves code clarity. Documentation is comprehensive.


122-163: LGTM: BroadcastChannel initialization

The SessionTokenEvent interface is well-documented, and the ensureBroadcastChannel function correctly:

  • Guards initialization with the build variant flag
  • Uses native BroadcastChannel (not a custom wrapper)
  • Initializes only once (cached in broadcastChannel)
  • Properly attaches the message listener

Note: Previous review comments about implementing removeEventListener on a custom ClerkBroadcastChannel type don't apply here since this implementation uses the native BroadcastChannel API directly.


165-172: LGTM: Proper cleanup implementation

The clear() function correctly iterates through all cache entries and clears their individual timeouts before clearing the cache. This properly addresses the past review concern about only clearing a single timer.


207-289: LGTM: Robust broadcast message handling

The handleBroadcastMessage function includes comprehensive validation:

  • Token ID format validation against expected components
  • JWT parsing with error handling
  • Presence of required iat/exp claims
  • Monotonicity check by comparing the incoming iat against the existing token's iat
  • Proper logging at each decision point

The monotonicity check correctly compares the resolved token's iat (not createdAt), which properly addresses previous review concerns.


291-295: LGTM: Clean public set interface

The set() method correctly ensures the broadcast channel is initialized and delegates to the internal implementation with broadcast enabled.


393-407: LGTM: Clean resource management and export

The close() and size() methods are straightforward and correct. The export of SessionTokenCache as a singleton instance is appropriate for this use case.

Comment on lines 84 to +347
entry.tokenResolver
.then(newToken => {
if (!newToken.jwt) {
const claims = newToken.jwt?.claims;
if (!claims || typeof claims.exp !== 'number' || typeof claims.iat !== 'number') {
return deleteKey();
}

const expiresAt = newToken.jwt.claims.exp;
const issuedAt = newToken.jwt.claims.iat;
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;

// Mutate cached value and set expirations
value.expiresIn = expiresIn;
timer = setTimeout(deleteKey, expiresIn * 1000);

const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
value.timeoutId = timeoutId;

// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof timer.unref === 'function') {
timer.unref();
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Fix token expiration timeout to use remaining lifetime

The current implementation schedules token deletion using the full token lifetime (exp - iat) from now, not the remaining lifetime. This causes tokens to be retained well past their actual expiration time.

Example: A token issued 30 minutes ago with a 1-hour expiration will be cached for another full hour from now, meaning it stays in cache for 1.5 hours total.

Impact: While the get() method's leeway check prevents serving expired tokens to callers, cache entries linger unnecessarily, consuming memory and potentially causing confusion during debugging.

Apply this diff to fix the timeout scheduling:

         const expiresAt = claims.exp;
         const issuedAt = claims.iat;
-        const expiresIn: Seconds = expiresAt - issuedAt;
+        const expiresIn: Seconds = expiresAt - issuedAt;  // Store original lifetime for get() checks
 
         value.expiresIn = expiresIn;
+
+        // Compute remaining lifetime for timeout scheduling
+        const nowAfterResolve = Math.floor(Date.now() / 1000);
+        const remainingSeconds = expiresAt - nowAfterResolve;
+        
+        if (remainingSeconds <= 0) {
+          deleteKey();
+          return;
+        }
 
-        const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
+        const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
         value.timeoutId = timeoutId;

Note on clock skew: The existing code correctly uses iat for createdAt (line 313-314), which makes expiration checks in get() immune to client clock drift. The timeout uses client wall-clock time (necessary for setTimeout), and the get() method's leeway (10s + 5s) absorbs minor clock drift at read time.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
entry.tokenResolver
.then(newToken => {
if (!newToken.jwt) {
const claims = newToken.jwt?.claims;
if (!claims || typeof claims.exp !== 'number' || typeof claims.iat !== 'number') {
return deleteKey();
}
const expiresAt = newToken.jwt.claims.exp;
const issuedAt = newToken.jwt.claims.iat;
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;
// Mutate cached value and set expirations
value.expiresIn = expiresIn;
timer = setTimeout(deleteKey, expiresIn * 1000);
const timeoutId = setTimeout(deleteKey, expiresIn * 1000);
value.timeoutId = timeoutId;
// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof timer.unref === 'function') {
timer.unref();
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}
entry.tokenResolver
.then(newToken => {
const claims = newToken.jwt?.claims;
if (!claims || typeof claims.exp !== 'number' || typeof claims.iat !== 'number') {
return deleteKey();
}
const expiresAt = claims.exp;
const issuedAt = claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt; // Store original lifetime for get() checks
value.expiresIn = expiresIn;
// Compute remaining lifetime for timeout scheduling
const nowAfterResolve = Math.floor(Date.now() / 1000);
const remainingSeconds = expiresAt - nowAfterResolve;
if (remainingSeconds <= 0) {
deleteKey();
return;
}
const timeoutId = setTimeout(deleteKey, remainingSeconds * 1000);
value.timeoutId = timeoutId;
// Teach ClerkJS not to block the exit of the event loop when used in Node environments.
// More info at https://nodejs.org/api/timers.html#timeoutunref
if (typeof (timeoutId as any).unref === 'function') {
(timeoutId as any).unref();
}
});
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/tokenCache.ts around lines 326 to 347, the timeout
is currently scheduled using the token's full lifetime (exp - iat) instead of
the remaining lifetime; compute the remaining seconds as exp -
currentUnixSeconds, if remaining <= 0 call deleteKey() immediately, otherwise
set value.expiresIn = remaining and schedule setTimeout(deleteKey, remaining *
1000) (keep the unref logic), and ensure any types/guards handle the computed
remaining as a Seconds value.

plugins: [react({ jsxRuntime: 'automatic', jsxImportSource: '@emotion/react' }), viteSvgMockPlugin()],
define: {
__BUILD_DISABLE_RHC__: JSON.stringify(false),
__BUILD_VARIANT_CHANNEL__: JSON.stringify(true),
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

LGTM! Verify test coverage for the channel variant.

The addition of __BUILD_VARIANT_CHANNEL__ follows the existing pattern for build-time feature flags. Setting it to true in the test configuration ensures that tests run with the channel variant enabled, which is appropriate for the BroadcastChannel functionality introduced in this PR.

Please verify:

  1. That test coverage exists for the channel variant functionality (BroadcastChannel token synchronization).
  2. That this flag's value aligns with the rspack configuration for consistency.

Run the following script to check for channel variant tests and rspack configuration:


🏁 Script executed:

#!/bin/bash
# Description: Verify channel variant tests and rspack config consistency.

echo "=== Searching for channel variant tests ==="
rg -n --type=ts -C3 '__BUILD_VARIANT_CHANNEL__|BroadcastChannel' 'packages/clerk-js/src/**/*.test.ts'

echo -e "\n=== Checking rspack configuration ==="
rg -n -A5 '__BUILD_VARIANT_CHANNEL__' 'packages/clerk-js/rspack.config.js'

echo -e "\n=== Checking channel entry point ==="
fd -t f 'index.channel.browser.ts' 'packages/clerk-js/src'

Length of output: 499


Add tests for channel variant functionality

No tests found covering __BUILD_VARIANT_CHANNEL__ or BroadcastChannel logic; please add appropriate test cases.

🤖 Prompt for AI Agents
In packages/clerk-js/vitest.config.mts around line 29, there are no tests
covering the __BUILD_VARIANT_CHANNEL__ flag or BroadcastChannel behavior; add
unit tests that (1) run with __BUILD_VARIANT_CHANNEL__ enabled and disabled, (2)
mock or stub global BroadcastChannel to assert that the code path which
registers/uses BroadcastChannel is executed when the flag is true and skipped
when false, and (3) verify message send/receive handling and cleanup (close)
semantics; implement these as Vitest test files that set process.env or the
build-time define appropriately, import the module under test after setting the
flag, and use spies/mocks to assert BroadcastChannel construction, postMessage,
onmessage handling, and close calls for both variants.

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

Successfully merging this pull request may close these issues.

3 participants