fix(clerk-js): Correct signout behavior for multisession #6515
Conversation
🦋 Changeset detectedLatest commit: a5999e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughAdds a changeset for a patch release and updates Clerk’s multisession sign-out flow. In core logic, after removing the active session, the code now selects a remaining signed-in session as active, updates lastActiveSessionId, refreshes accessors, emits updated state, and tracks unload/redirect during sign-out. A new unit test validates that after signing out from one session in a multisession setup, the app remains signed in with the remaining session and navigates to “/”. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.changeset/strong-schools-shine.md (1)
5-5: Fix typos and tighten release note phrasing.Current: “Fixing redirect behavior when signing out from a multisession app with multple singed in accounts”
Suggested edit:
- Correct spelling (“multiple”, “signed”)
- Prefer concise imperative mood
-Fixing redirect behavior when signing out from a multisession app with multple singed in accounts +Fix redirect behavior when signing out from a multi-session app with multiple signed-in accounts.packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
813-847: Good coverage for the happy path; add a guard for non-current session removal.This test validates switching to the remaining session when the current one is removed. Please also add a test ensuring that removing a non-current session does not switch the active session.
Example outline:
- Start with session1 active and session2 also signed-in
- Call signOut({ sessionId: '2' })
- Assert sut.session remains session1 and no navigation occurs
Optionally, add a multi-tab test around the broadcast semantics if you gate
UserSignOutemission.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/strong-schools-shine.md(1 hunks)packages/clerk-js/src/core/__tests__/clerk.test.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/strong-schools-shine.md
**/*.{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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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
Useconst assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/__tests__/clerk.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__/clerk.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__/clerk.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__/clerk.test.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/core/clerk.ts (2)
packages/clerk-js/src/utils/beforeUnloadTracker.ts (1)
createBeforeUnloadTracker(43-62)packages/clerk-js/src/core/events.ts (2)
eventBus(28-28)events(6-13)
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
packages/clerk-js/src/core/clerk.ts (1)
Clerk(195-2897)
🔇 Additional comments (1)
packages/clerk-js/src/core/clerk.ts (1)
544-561: Cross-tab sign-out broadcast is intentional and correct
The library treats each browser tab as part of the same session (backed by a single cookie). Emittingevents.UserSignOuton sign-out ensures all tabs clear their active session when the cookie is removed. Gating the broadcast on a non-existentclient.signedInSessionsarray would not work and isn’t needed—after sign-out the server will no longer return a valid session, so every tab will drop intohandleUnauthenticatedand clear state appropriately.
| if (this.client && this.client.signedInSessions.length > 0) { | ||
| const nextSession = this.client.signedInSessions[0]; | ||
| if (nextSession) { | ||
| this.client.lastActiveSessionId = nextSession.id; | ||
| this.#setAccessors(nextSession); | ||
| this.#emit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not change the active session when a non-current session is removed.
This block always switches to the first remaining session, even when the removed session is not the current one. That will unexpectedly flip the active session for the user.
Restrict the “pick next session” logic to only when the removed session was the current session. Optionally prefer an “active” session over “pending”.
- if (this.client && this.client.signedInSessions.length > 0) {
- const nextSession = this.client.signedInSessions[0];
- if (nextSession) {
- this.client.lastActiveSessionId = nextSession.id;
- this.#setAccessors(nextSession);
- this.#emit();
- }
- }
+ if (shouldSignOutCurrent && this.client && this.client.signedInSessions.length > 0) {
+ // Prefer an active session if available; otherwise fall back to the first remaining session
+ const nextSession =
+ this.client.signedInSessions.find(s => s.status === 'active') ?? this.client.signedInSessions[0];
+ this.client.lastActiveSessionId = nextSession.id;
+ this.#setAccessors(nextSession);
+ this.#emit();
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 534 to 541, the code
currently always picks the first remaining signed-in session and makes it
active; change it so this "pick next session" logic only runs if the session
that was removed was the current/lastActiveSession. Concretely: compare the
removed session id to this.client.lastActiveSessionId and only if they match,
select the next session (prefer one with status 'active' then fallback to first)
and then set this.client.lastActiveSessionId, call #setAccessors(nextSession)
and #emit(); otherwise do nothing to avoid flipping active session. Also handle
the no-sessions case by clearing lastActiveSessionId and accessors if needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.changeset/strong-schools-shine.md (1)
5-5: Fix typos and tighten the messageUse “multiple signed-in accounts” and end with a period.
-Fixing redirect behavior when signing out from a multisession app with multple singed in accounts +Fix redirect behavior when signing out from a multisession app with multiple signed-in accounts.packages/clerk-js/src/core/clerk.ts (1)
544-561: Tracker-based flow LGTM; confirm cross-tab behavior and cookie sync in practiceThe before-unload tracker and onAfterSetActive sequencing is solid and aligns with avoiding UI flicker. Emitting UserSignOut while another session remains should be safe because background tabs handleUnauthenticated() early-return if a session exists, but please verify this across tabs.
- If you adopt the cookie sync suggestion above (await nextSession.getToken()), the new session’s cookie will be in place before navigation, reducing the chance of transient mismatches on the landing route.
- Double-check that in Safari/iOS the before-unload listener reliably detects unload to avoid calling onAfterSetActive after navigation.
To sanity-check cross-tab behavior, you can run the existing Jest suite plus a manual test with two tabs:
- Tab A: two sessions; sign out the current session.
- Tab B: should remain signed in and not flicker to signed out.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/strong-schools-shine.md(1 hunks)packages/clerk-js/src/core/__tests__/clerk.test.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/strong-schools-shine.md
**/*.{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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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
Useconst assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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.tspackages/clerk-js/src/core/__tests__/clerk.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.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/__tests__/clerk.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__/clerk.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__/clerk.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__/clerk.test.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/core/clerk.ts (2)
packages/clerk-js/src/utils/beforeUnloadTracker.ts (1)
createBeforeUnloadTracker(43-62)packages/clerk-js/src/core/events.ts (2)
eventBus(28-28)events(6-13)
packages/clerk-js/src/core/__tests__/clerk.test.ts (2)
packages/clerk-js/src/core/clerk.ts (1)
Clerk(195-2897)packages/types/src/clerk.ts (1)
Clerk(163-888)
|
|
||
| it('properly restores auth state from remaining sessions after multisession sign-out', async () => { | ||
| const mockClient = { | ||
| signedInSessions: [mockSession1, mockSession2], | ||
| sessions: [mockSession1, mockSession2], | ||
| destroy: mockClientDestroy, | ||
| lastActiveSessionId: '1', | ||
| }; | ||
|
|
||
| mockSession1.remove = jest.fn().mockImplementation(() => { | ||
| mockClient.signedInSessions = mockClient.signedInSessions.filter(s => s.id !== '1'); | ||
| mockClient.sessions = mockClient.sessions.filter(s => s.id !== '1'); | ||
| return Promise.resolve(mockSession1); | ||
| }); | ||
|
|
||
| mockClientFetch.mockReturnValue(Promise.resolve(mockClient)); | ||
|
|
||
| const sut = new Clerk(productionPublishableKey); | ||
| sut.navigate = jest.fn(); | ||
| await sut.load(); | ||
|
|
||
| expect(sut.session).toBe(mockSession1); | ||
| expect(sut.isSignedIn).toBe(true); | ||
|
|
||
| await sut.signOut({ sessionId: '1' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockSession1.remove).toHaveBeenCalled(); | ||
| expect(mockClientDestroy).not.toHaveBeenCalled(); | ||
| expect(sut.navigate).toHaveBeenCalledWith('/'); | ||
| }); | ||
|
|
||
| expect(mockClient.lastActiveSessionId).toBe('2'); | ||
| expect(sut.session).toBe(mockSession2); | ||
| expect(sut.isSignedIn).toBe(true); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Great addition; add guard-rail tests for non-current removal and active session selection
This verifies the main regression. Please add:
- A test ensuring that removing a non-current session does not switch the active session.
- A test ensuring that when the first remaining session is pending, we pick an active one as next.
- Optionally assert that the next session’s getToken is invoked to sync cookies.
Example additions you can append near this block:
it('does not switch active session when signing out a non-current session', async () => {
const mockClient = {
signedInSessions: [mockSession1, mockSession2],
sessions: [mockSession1, mockSession2],
destroy: mockClientDestroy,
lastActiveSessionId: '1',
};
mockClientFetch.mockReturnValue(Promise.resolve(mockClient));
const sut = new Clerk(productionPublishableKey);
await sut.load();
await sut.signOut({ sessionId: '2' });
await waitFor(() => {
expect(mockSession2.remove).toHaveBeenCalled();
expect(sut.session).toBe(mockSession1);
expect(mockClient.lastActiveSessionId).toBe('1');
});
});
it('prefers an active next session over pending when switching after current-session sign-out', async () => {
const active = { ...mockSession2, id: '2', status: 'active', getToken: jest.fn() };
const pending = { ...mockSession3, id: '3', status: 'pending', getToken: jest.fn() };
const mockClient = {
signedInSessions: [pending, active], // pending first to validate selection
sessions: [mockSession1, pending, active],
destroy: mockClientDestroy,
lastActiveSessionId: '1',
};
mockSession1.remove = jest.fn().mockImplementation(() => {
mockClient.signedInSessions = mockClient.signedInSessions.filter(s => s.id !== '1');
mockClient.sessions = mockClient.sessions.filter(s => s.id !== '1');
return Promise.resolve(mockSession1);
});
mockClientFetch.mockReturnValue(Promise.resolve(mockClient));
const sut = new Clerk(productionPublishableKey);
await sut.load();
await sut.signOut({ sessionId: '1' });
await waitFor(() => {
expect(sut.session).toBe(active);
expect(mockClient.lastActiveSessionId).toBe('2');
// If you adopt token sync, assert:
// expect(active.getToken).toHaveBeenCalled();
});
});🤖 Prompt for AI Agents
In packages/clerk-js/src/core/__tests__/clerk.test.ts around lines 812 to 847,
the reviewer asks for two additional guard-rail tests: one ensuring removing a
non-current session does not switch the active session, and one ensuring that
when choosing the next session after removing the current session we prefer an
active session over a pending one (and optionally call its getToken to sync
cookies). Add a test that creates mockClient with signedInSessions
[mockSession1, mockSession2], sessions [mockSession1, mockSession2],
lastActiveSessionId '1', stub mockClientFetch to return it, load sut, call
signOut({ sessionId: '2' }), and assert mockSession2.remove was called,
sut.session remains mockSession1, and mockClient.lastActiveSessionId stays '1'.
Add a second test that constructs pending and active mocks (pending first in
signedInSessions), sets mockSession1.remove to remove session1 from client
arrays, stub mockClientFetch, load sut, signOut({ sessionId: '1' }), and assert
sut.session becomes the active session, mockClient.lastActiveSessionId updates
to the active id, and optionally assert active.getToken was called to sync
cookies.
| if (this.client && this.client.signedInSessions.length > 0) { | ||
| const nextSession = this.client.signedInSessions[0]; | ||
| if (nextSession) { | ||
| this.client.lastActiveSessionId = nextSession.id; | ||
| this.#setAccessors(nextSession); | ||
| this.#emit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Only switch active session if you signed out the current session; prefer an active next session and sync cookies
Currently this always selects the first remaining session and updates state, even when removing a non-active session. That can inadvertently switch the active session. Also, the chosen next session might be pending, and cookies are not synced to the new session.
Consider:
- Switch to a next session only if shouldSignOutCurrent is true.
- Prefer an active session if available; fall back to the first remaining session.
- Sync cookies by fetching a token for the new session before emitting, mirroring setActive behavior.
- if (this.client && this.client.signedInSessions.length > 0) {
- const nextSession = this.client.signedInSessions[0];
- if (nextSession) {
- this.client.lastActiveSessionId = nextSession.id;
- this.#setAccessors(nextSession);
- this.#emit();
- }
- }
+ if (shouldSignOutCurrent && this.client && this.client.signedInSessions.length > 0) {
+ // Prefer an active session; fall back to the first remaining session
+ const nextSession =
+ (this.client.signedInSessions.find(s => s.status === 'active') as SignedInSessionResource | undefined) ??
+ this.client.signedInSessions[0];
+ if (nextSession) {
+ this.client.lastActiveSessionId = nextSession.id;
+ // Sync cookies (__session, __client_uat) to the new session before emitting
+ try {
+ // Touch when in active tab or for non-standard browsers, similar to setActive
+ if (inActiveBrowserTab() || !this.#options.standardBrowser) {
+ await this.#touchCurrentSession(nextSession);
+ }
+ await nextSession.getToken();
+ } catch {
+ // If token retrieval fails, proceed with best-effort state update
+ }
+ this.#setAccessors(nextSession);
+ this.#emit();
+ }
+ }📝 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.
| if (this.client && this.client.signedInSessions.length > 0) { | |
| const nextSession = this.client.signedInSessions[0]; | |
| if (nextSession) { | |
| this.client.lastActiveSessionId = nextSession.id; | |
| this.#setAccessors(nextSession); | |
| this.#emit(); | |
| } | |
| } | |
| if (shouldSignOutCurrent && this.client && this.client.signedInSessions.length > 0) { | |
| // Prefer an active session; fall back to the first remaining session | |
| const nextSession = | |
| (this.client.signedInSessions.find(s => s.status === 'active') as SignedInSessionResource | undefined) ?? | |
| this.client.signedInSessions[0]; | |
| if (nextSession) { | |
| this.client.lastActiveSessionId = nextSession.id; | |
| // Sync cookies (__session, __client_uat) to the new session before emitting | |
| try { | |
| // Touch when in active tab or for non-standard browsers, similar to setActive | |
| if (inActiveBrowserTab() || !this.#options.standardBrowser) { | |
| await this.#touchCurrentSession(nextSession); | |
| } | |
| await nextSession.getToken(); | |
| } catch { | |
| // If token retrieval fails, proceed with best-effort state update | |
| } | |
| this.#setAccessors(nextSession); | |
| this.#emit(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 534 to 541, the code always
picks the first remaining session and switches active state immediately when a
session is removed; change this so that we only switch the active session when
shouldSignOutCurrent is true, prefer an already-active session from
this.client.signedInSessions (fall back to the first remaining session if none
are active), and before calling #setAccessors and #emit fetch and apply the
session token (sync cookies) the same way setActive does — i.e., obtain the
token for the chosen session and update cookie/auth state, then set
lastActiveSessionId, call #setAccessors and finally #emit; keep all existing
guards for client and session existence.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.changeset/strong-schools-shine.md (1)
5-5: Fix typos and tighten phrasing in changeset summaryProposed wording:
- “Fix redirect behavior when signing out from a multisession app with multiple signed-in accounts.”
This improves clarity and fixes typos (“multple”, “singed”).
-Fixing redirect behavior when signing out from a multisession app with multple singed in accounts +Fix redirect behavior when signing out from a multisession app with multiple signed-in accountspackages/clerk-js/src/core/clerk.ts (1)
528-533: Guard against invalid sessionId inputsIf opts.sessionId doesn’t match any session, session?.remove() is a no-op, but the subsequent logic may still change the active session unintentionally. Early-exit when the target session is not found.
- const session = this.client.signedInSessions.find(s => s.id === opts.sessionId); + const session = this.client.signedInSessions.find(s => s.id === opts.sessionId); const shouldSignOutCurrent = session?.id && this.session?.id === session.id; - await session?.remove(); + if (!session) { + // No-op: requested session not found. + return; + } + await session.remove();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/strong-schools-shine.md(1 hunks)packages/clerk-js/src/core/__tests__/clerk.test.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/strong-schools-shine.md
**/*.{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/__tests__/clerk.test.tspackages/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/__tests__/clerk.test.tspackages/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/__tests__/clerk.test.tspackages/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/__tests__/clerk.test.tspackages/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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
Useconst assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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/__tests__/clerk.test.tspackages/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/clerk-js/src/core/__tests__/clerk.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__/clerk.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/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.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__/clerk.test.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/core/clerk.ts (2)
packages/clerk-js/src/utils/beforeUnloadTracker.ts (1)
createBeforeUnloadTracker(43-62)packages/clerk-js/src/core/events.ts (2)
eventBus(28-28)events(6-13)
🔇 Additional comments (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
813-847: LGTM: solid regression test for multi-session sign-out flowThe test covers the happy path: removes the current session, navigates to “/”, updates lastActiveSessionId, and preserves signed-in state with the remaining session.
| it('properly restores auth state from remaining sessions after multisession sign-out', async () => { | ||
| const mockClient = { | ||
| signedInSessions: [mockSession1, mockSession2], | ||
| sessions: [mockSession1, mockSession2], | ||
| destroy: mockClientDestroy, | ||
| lastActiveSessionId: '1', | ||
| }; | ||
|
|
||
| mockSession1.remove = jest.fn().mockImplementation(() => { | ||
| mockClient.signedInSessions = mockClient.signedInSessions.filter(s => s.id !== '1'); | ||
| mockClient.sessions = mockClient.sessions.filter(s => s.id !== '1'); | ||
| return Promise.resolve(mockSession1); | ||
| }); | ||
|
|
||
| mockClientFetch.mockReturnValue(Promise.resolve(mockClient)); | ||
|
|
||
| const sut = new Clerk(productionPublishableKey); | ||
| sut.navigate = jest.fn(); | ||
| await sut.load(); | ||
|
|
||
| expect(sut.session).toBe(mockSession1); | ||
| expect(sut.isSignedIn).toBe(true); | ||
|
|
||
| await sut.signOut({ sessionId: '1' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockSession1.remove).toHaveBeenCalled(); | ||
| expect(mockClientDestroy).not.toHaveBeenCalled(); | ||
| expect(sut.navigate).toHaveBeenCalledWith('/'); | ||
| }); | ||
|
|
||
| expect(mockClient.lastActiveSessionId).toBe('2'); | ||
| expect(sut.session).toBe(mockSession2); | ||
| expect(sut.isSignedIn).toBe(true); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add assertions for cookie/token sync and event emissions to prevent regressions
To ensure reload consistency, assert that the next session’s token is propagated or that a TokenUpdate was emitted. Also validate that UserSignOut is not emitted when removing a non-current session.
Examples:
- Spy on eventBus.emit for events.TokenUpdate and verify it’s called with the next session token.
- For non-current sign-out, assert eventBus.emit is not called with events.UserSignOut.
- Optionally assert nextSession.getToken() is called if you choose that path.
I can draft these test additions if helpful.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/__tests__/clerk.test.ts around lines 813 to 847,
the test lacks assertions that the next session’s token is synced and that
correct events are emitted/omitted after removing a non-current session; add a
spy on eventBus.emit, assert that events.TokenUpdate is emitted with the next
session token (or assert nextSession.getToken() is called and its value is
propagated), and assert that events.UserSignOut is NOT emitted for the removed
non-current session; keep existing assertions about navigation and client
destruction and place the new event/token assertions after awaiting signOut so
they run against the final state.
| if (this.client && this.client.signedInSessions.length > 0) { | ||
| const nextSession = this.client.signedInSessions[0]; | ||
| if (nextSession) { | ||
| this.client.lastActiveSessionId = nextSession.id; | ||
| this.#setAccessors(nextSession); | ||
| this.#emit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Only switch active session when signing out the current session; also sync auth token for the new session
Currently the next-session selection runs even when a non-current session is removed, which can inadvertently change the active session if the array ordering differs from the current. Also, after switching, cookies/tokens are not refreshed, risking an inconsistent state on reload.
- Guard the switch with shouldSignOutCurrent.
- After switching, emit a TokenUpdate (or call getToken) to sync cookies to the new session.
- if (this.client && this.client.signedInSessions.length > 0) {
- const nextSession = this.client.signedInSessions[0];
- if (nextSession) {
- this.client.lastActiveSessionId = nextSession.id;
- this.#setAccessors(nextSession);
- this.#emit();
- }
- }
+ if (shouldSignOutCurrent && this.client && this.client.signedInSessions.length > 0) {
+ // Prefer selecting based on lastActiveSessionId if present; otherwise first signed-in session.
+ const nextSession =
+ this.client.signedInSessions.find(s => s.id === this.client.lastActiveSessionId) ??
+ this.client.signedInSessions[0];
+
+ if (nextSession) {
+ this.client.lastActiveSessionId = nextSession.id;
+ this.#setAccessors(nextSession);
+ // Ensure downstream consumers receive up-to-date auth. Option A (no network):
+ eventBus.emit(events.TokenUpdate, { token: nextSession.lastActiveToken });
+ // Option B (safer but async): await nextSession.getToken().catch(() => eventBus.emit(events.TokenUpdate, { token: null }));
+ this.#emit();
+ }
+ }📝 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.
| if (this.client && this.client.signedInSessions.length > 0) { | |
| const nextSession = this.client.signedInSessions[0]; | |
| if (nextSession) { | |
| this.client.lastActiveSessionId = nextSession.id; | |
| this.#setAccessors(nextSession); | |
| this.#emit(); | |
| } | |
| } | |
| if (shouldSignOutCurrent && this.client && this.client.signedInSessions.length > 0) { | |
| // Prefer selecting based on lastActiveSessionId if present; otherwise first signed-in session. | |
| const nextSession = | |
| this.client.signedInSessions.find(s => s.id === this.client.lastActiveSessionId) ?? | |
| this.client.signedInSessions[0]; | |
| if (nextSession) { | |
| this.client.lastActiveSessionId = nextSession.id; | |
| this.#setAccessors(nextSession); | |
| // Ensure downstream consumers receive up-to-date auth. Option A (no network): | |
| eventBus.emit(events.TokenUpdate, { token: nextSession.lastActiveToken }); | |
| // Option B (safer but async): await nextSession.getToken().catch(() => eventBus.emit(events.TokenUpdate, { token: null })); | |
| this.#emit(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 534 to 541, the code
unconditionally picks the first signedInSessions entry which can change the
active session when a different (non-current) session is removed and does not
refresh auth cookies/tokens; wrap the next-session selection and assignment
behind a check for shouldSignOutCurrent so we only switch when we intended to
sign out the current session, and after setting lastActiveSessionId and calling
#setAccessors/#emit, trigger a TokenUpdate or call getToken() (or equivalent
token-refresh flow) to sync cookies/tokens for the newly active session so state
remains consistent on reload.
| const tracker = createBeforeUnloadTracker(this.#options.standardBrowser); | ||
|
|
||
| eventBus.emit(events.UserSignOut, null); | ||
|
|
||
| await tracker.track(async () => { | ||
| if (signOutCallback) { | ||
| await signOutCallback(); | ||
| } else { | ||
| await this.navigate(redirectUrl); | ||
| } | ||
| }); | ||
|
|
||
| if (tracker.isUnloading()) { | ||
| return; | ||
| } | ||
|
|
||
| await onAfterSetActive(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify side effects of UserSignOut during multi-session switch; ensure cookies aren’t cleared after selecting the next session
In the multi-session/current-session path, you emit UserSignOut after switching to the next session. If any listener clears __session cookies on this event, the tab may end up signed-out on reload despite the in-memory switch. Either:
- Ensure UserSignOut does not clear cookies when a next active session exists, or
- Re-sync cookies (TokenUpdate or getToken) after the tracker when not unloading.
To confirm event side effects and avoid regressions, search for the UserSignOut handlers:
If a cookie-clearing listener exists, consider moving TokenUpdate re-sync after the tracker when tracker.isUnloading() is false, or introduce a specialized event for multi-session single sign-out that does not clear cookies globally.
🏁 Script executed:
#!/bin/bash
# Find all listeners reacting to UserSignOut and inspect for cookie-clearing.
rg -n "UserSignOut" -A 8 -B 4Length of output: 8105
🏁 Script executed:
#!/bin/bash
# Inspect AuthCookieService.handleSignOut implementation to see if it clears cookies unconditionally
rg -n "handleSignOut" -A 10 -B 2 packages/clerk-js/src/core/auth/AuthCookieService.tsLength of output: 1027
🏁 Script executed:
#!/bin/bash
# Find TokenUpdate handler in AuthCookieService to see when cookies are set/updated
rg -n "eventBus.on" -A 10 -B 0 packages/clerk-js/src/core/auth/AuthCookieService.tsLength of output: 728
Prevent clearing cookies on multi‐session switch
We confirmed that AuthCookieService.handleSignOut() (packages/clerk-js/src/core/auth/AuthCookieService.ts:217–221) unconditionally removes both the session and active‐context cookies whenever events.UserSignOut is emitted. In the “switch to next session” path (packages/clerk-js/src/core/clerk.ts:544–561), you’re emitting UserSignOut before the in‐memory session swap, causing those cookies to be wiped and leading to an unintended sign-out on reload.
To fix this, pick one of two approaches:
• Introduce a new “session switch” event that only signals other tabs (via broadcast) and doesn’t trigger cookie clearance:
– Define events.SessionSwitch = 'user:sessionSwitch'.
– In the multi-session branch, replace
ts eventBus.emit(events.UserSignOut, null);
with
ts eventBus.emit(events.SessionSwitch, null);
– Leave the existing UserSignOut listeners untouched (they still clear cookies on real sign-out).
• Or, re-sync cookies immediately after the in-memory switch when not unloading:
- Keep emitting
UserSignOut. - After
insert:
await tracker.track(/*…*/); if (tracker.isUnloading()) return; await onAfterSetActive();
// restore cookies for the newly active session const token = this.session?.token ?? null; eventBus.emit(events.TokenUpdate, { token });
Locations to update:
- packages/clerk-js/src/core/clerk.ts:544–561
- packages/clerk-js/src/core/events.ts (add
SessionSwitchif using the first approach) - packages/clerk-js/src/core/auth/AuthCookieService.ts (no change if you use dedicated switch event; otherwise ensure token re-sync is wired up)
Description
Fixes: USER-2549
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit