Skip to content

Conversation

jacekradko
Copy link
Member

@jacekradko jacekradko commented Aug 11, 2025

Description

Fixes: USER-2549

When signing out of a multisession app, remove the matching session and redirect to /sign-in/choose if the session being removed is the current one

Screenshot 2025-08-14 at 8 54 45 PM

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

  • Bug Fixes

    • Multi-session sign-out: signing out the current session clears the active session and redirects to the configured post-sign-out host URL, preserving remaining sessions (no automatic migration).
  • New Features

    • Configurable afterSignOutOneUrl to control host-based post-sign-out redirects.
  • Tests

    • Added unit tests for current-session sign-out, non-current-session sign-out (no navigation), and host-based redirect behavior.
  • Chores

    • Added a changeset entry for the patch release.
  • Style

    • Added debug logging for session removal.

Copy link

changeset-bot bot commented Aug 11, 2025

🦋 Changeset detected

Latest commit: a9029ac

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

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

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

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

Copy link

vercel bot commented Aug 11, 2025

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

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Preview Comment Aug 15, 2025 2:36pm

Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

📝 Walkthrough

Walkthrough

Adds a changeset entry and tests for multisession sign-out behavior. DisplayConfig gains a new afterSignOutOneUrl property. Clerk sign-out flow was modified to always remove the targeted session, clear accessors when the removed session is the active one via a new #setAccessors(session?) call, emit state changes, and navigate using a new URL builder for post-multisession sign-out. Several helper methods and URL-building utilities were added to Clerk, a public assertComponentsReady(...) method was introduced, and Session.remove() now emits a debug log.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
Prevent redirect to sign-in with redirect_url when signing out active account in multisession [USER-2549]
Preserve signed-in state by switching to remaining session after sign-out [USER-2549] Code clears active accessors and tests assert that state is not migrated to the remaining session.
Ensure navigation does not get stuck on sign-in URL and updates appropriately [USER-2549]

Out-of-scope changes

Code Change Explanation
Add changeset entry (.changeset/strong-schools-shine.md) Release metadata; not required to implement USER-2549 behavioral changes.
Add debugLogger.debug call in Session.remove() (packages/clerk-js/src/core/resources/Session.ts) Logging side-effect unrelated to the issue objectives.
Add public method assertComponentsReady in Clerk (packages/clerk-js/src/core/clerk.ts) New public API not referenced by USER-2549 and not required for the multisession sign-out fix.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • 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 13971e8 and a9029ac.

📒 Files selected for processing (1)
  • packages/clerk-js/src/core/clerk.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/core/clerk.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). (20)
  • GitHub Check: Integration Tests (tanstack-react-router, chrome)
  • GitHub Check: Integration Tests (vue, chrome)
  • GitHub Check: Integration Tests (machine, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (astro, chrome)
  • GitHub Check: Integration Tests (expo-web, chrome)
  • GitHub Check: Integration Tests (sessions, chrome)
  • GitHub Check: Integration Tests (billing, chrome)
  • GitHub Check: Integration Tests (nuxt, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (react-router, chrome)
  • GitHub Check: Integration Tests (tanstack-react-start, chrome)
  • GitHub Check: Integration Tests (elements, chrome)
  • GitHub Check: Integration Tests (localhost, chrome)
  • GitHub Check: Integration Tests (quickstart, chrome)
  • GitHub Check: Integration Tests (ap-flows, chrome)
  • GitHub Check: Integration Tests (generic, chrome)
  • GitHub Check: Static analysis
  • GitHub Check: Unit Tests (22, **)
  • GitHub Check: semgrep-cloud-platform/scan

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

pkg-pr-new bot commented Aug 11, 2025

Open in StackBlitz

@clerk/agent-toolkit

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

@clerk/astro

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

@clerk/backend

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

@clerk/chrome-extension

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

@clerk/clerk-js

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

@clerk/dev-cli

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

@clerk/elements

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

@clerk/clerk-expo

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

@clerk/expo-passkeys

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

@clerk/express

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

@clerk/fastify

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

@clerk/localizations

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

@clerk/nextjs

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

@clerk/nuxt

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

@clerk/clerk-react

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

@clerk/react-router

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

@clerk/remix

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

@clerk/shared

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

@clerk/tanstack-react-start

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

@clerk/testing

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

@clerk/themes

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

@clerk/types

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

@clerk/upgrade

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

@clerk/vue

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

commit: a9029ac

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)
.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 UserSignOut emission.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1269b61 and a5999e3.

📒 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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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
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/__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.ts
  • 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/clerk.ts
  • packages/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). Emitting events.UserSignOut on sign-out ensures all tabs clear their active session when the cookie is removed. Gating the broadcast on a non-existent client.signedInSessions array would not work and isn’t needed—after sign-out the server will no longer return a valid session, so every tab will drop into handleUnauthenticated and clear state appropriately.

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)
.changeset/strong-schools-shine.md (1)

5-5: Fix typos and tighten the message

Use “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 practice

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1269b61 and a5999e3.

📒 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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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
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/__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.ts
  • 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/clerk.ts
  • packages/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)

Comment on lines 812 to 847

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);
});
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

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.

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 (2)
.changeset/strong-schools-shine.md (1)

5-5: Fix typos and tighten phrasing in changeset summary

Proposed 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 accounts
packages/clerk-js/src/core/clerk.ts (1)

528-533: Guard against invalid sessionId inputs

If 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1269b61 and a5999e3.

📒 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.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/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.ts
  • packages/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.ts
  • packages/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 flow

The test covers the happy path: removes the current session, navigates to “/”, updates lastActiveSessionId, and preserves signed-in state with the remaining session.

Comment on lines 813 to 847
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);
});
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

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.

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

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

556-569: Always remove the targeted session; correct gating around clearing state, navigation, and logging

In packages/clerk-js/src/core/clerk.ts within the signOut() method (around lines 556–569), the code currently only removes a session when it’s the active one. As a result, calling signOut({ sessionId }) on a non-current session is a no-op but still emits a “complete” log with a redirect URL. We should:

  • Always invoke session?.remove() for the targeted session.
  • Only clear accessors, emit state changes, navigate, and log the redirect when the removed session was the current one.
  • Emit a distinct log entry when a non-current session is removed without navigation.
   // Multi-session handling
-  const session = this.client.signedInSessions.find(s => s.id === opts.sessionId);
-  const shouldSignOutCurrent = session?.id && this.session?.id === session.id;
-
-  if (shouldSignOutCurrent) {
-    await session?.remove();
-
-    this.#setAccessors(null);
-    this.#emit();
-
-    await this.navigate(this.buildAfterMultiSessionSingleSignOutUrl());
-  }
-  debugLogger.info('signOut() complete', { redirectUrl: this.buildAfterMultiSessionSingleSignOutUrl() }, 'clerk');
+  const session = this.client.signedInSessions.find(s => s.id === opts.sessionId);
+  const shouldSignOutCurrent = session?.id && this.session?.id === session.id;
+
+  // Always remove the targeted session
+  await session?.remove();
+
+  if (shouldSignOutCurrent) {
+    this.#setAccessors(null);
+    this.#emit();
+    await this.navigate(this.buildAfterMultiSessionSingleSignOutUrl());
+    debugLogger.info(
+      'signOut() complete',
+      { redirectUrl: stripOrigin(this.buildAfterMultiSessionSingleSignOutUrl()) },
+      'clerk',
+    );
+  } else {
+    // Non-current session removed without navigation
+    debugLogger.info('signOut() complete (non-current session removed)', {}, 'clerk');
+  }

Next steps:

  • Add or extend tests for signOut({ sessionId }) covering both current and non-current sessions.
  • Optionally return a status flag (e.g. boolean) to indicate whether navigation occurred.
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)

1572-1581: Verify redirect construction: using pathname: '/choose' may drop the sign-in base path or break SPA hash routes

Elsewhere in this file, Clerk’s prebuilt routes use hashPath when composing internal UI flows (e.g., '/continue', '/factor-two'). Switching to pathname: '/choose' against base: this.#options.signInUrl can:

  • Discard the base pathname (‘/sign-in’) and yield ‘/choose’ at origin.
  • Diverge from the SPA/hash routing convention used in other flows.

Safer options:

  • Prefer hash routing like other flows:
-        buildURL(
-          {
-            base: this.#options.signInUrl,
-            pathname: '/choose',
-          },
-          { stringify: true },
-        ),
+        buildURL(
+          {
+            base: this.#options.signInUrl,
+            hashPath: '/choose',
+          },
+          { stringify: true },
+        ),
  • Or, if true path routing is intended, compute '/sign-in/choose' by appending to the base path (not replacing it).

Please verify against your router configuration and the UI route for the “choose account” screen to avoid regressions. Add/adjust tests to assert the produced URL (with and without a configured signInUrl, and for both hash/path routers).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • 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 7362dd2 and ab7eae9.

📒 Files selected for processing (1)
  • packages/clerk-js/src/core/clerk.ts (2 hunks)
🧰 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:

  • 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
**/*

⚙️ 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.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/core/clerk.ts (2)
packages/react/src/isomorphicClerk.ts (1)
  • session (672-678)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (236-236)
⏰ 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: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • 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: 2

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

558-563: Consider clearing stale lastActiveSessionId or (optionally) auto-selecting a next active session

When the removed session is the current one, you correctly clear accessors. However, client.lastActiveSessionId may still point to the removed session. Two options:

  • Minimal: clear client.lastActiveSessionId when it matches the removed session id to avoid stale pointers on subsequent flows.
  • Optional UX: pick a next session only when removing the current one (prefer an already-active session, fall back to the first), and sync cookies via getToken() before emitting, similar to setActive. This keeps the app signed in without forcing navigation to the "choose" screen.

If you want, I can draft a concrete patch that prefers an active next session and performs token sync.


2783-2801: JSDoc added is helpful; add explicit return type to the private method for consistency

Nice addition. For consistency with project TS guidelines, add an explicit : void return type to #setAccessors.

Outside-this-hunk change (illustrative):

// Before
#setAccessors = (session?: SignedInSessionResource | null) => {
// After
#setAccessors = (session?: SignedInSessionResource | null): void => {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • 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 6fbce92 and 03a8ac0.

📒 Files selected for processing (1)
  • packages/clerk-js/src/core/clerk.ts (2 hunks)
🧰 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:

  • 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
**/*

⚙️ 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.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/core/clerk.ts (2)
packages/react/src/isomorphicClerk.ts (1)
  • session (672-678)
packages/clerk-js/src/utils/debug.ts (1)
  • debugLogger (236-236)
⏰ 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: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • 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

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

558-570: Only navigate/log redirect when removing the current session; compute URL once and call onAfterSetActive

Right now we always log a redirectUrl even when we don’t navigate (non-current session removal). Also, for parity with the single-session path, invoke onAfterSetActive after clearing accessors when removing the current session.

Apply this refactor to make behavior and logging precise:

-    const session = this.client.signedInSessions.find(s => s.id === opts.sessionId);
-    if (session) {
-      await session.remove();
-      if (this.session?.id === session.id) {
-        this.#setAccessors(null);
-        this.#emit();
-        await this.navigate(this.buildAfterMultiSessionSingleSignOutUrl());
-      }
-    }
-    debugLogger.info(
-      'signOut() complete',
-      { redirectUrl: stripOrigin(this.buildAfterMultiSessionSingleSignOutUrl()) },
-      'clerk',
-    );
+    const session = this.client.signedInSessions.find(s => s.id === opts.sessionId);
+    if (session) {
+      const shouldSignOutCurrent = this.session?.id === session.id;
+      await session.remove();
+      if (shouldSignOutCurrent) {
+        this.#setAccessors(null);
+        this.#emit();
+        await onAfterSetActive();
+        const afterSingleUrl = this.buildAfterMultiSessionSingleSignOutUrl();
+        await this.navigate(afterSingleUrl);
+        debugLogger.info('signOut() complete', { redirectUrl: stripOrigin(afterSingleUrl) }, 'clerk');
+      } else {
+        debugLogger.info('signOut() complete (non-active session removed)', { sessionId: session.id }, 'clerk');
+      }
+    }

This keeps redirects scoped to current-session removals, aligns logs with actual navigation, and preserves server revalidation semantics via onAfterSetActive.

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

871-879: Clarify test intent; consider adding a guard-rail for explicit afterMultiSessionSingleSignOutUrl/signInUrl

The comments/readjusted expectation are correct (redirectUrl is ignored for multisession). As a follow-up, add a test that:

  • sets options.afterMultiSessionSingleSignOutUrl and asserts it takes precedence, and
  • sets options.signInUrl and asserts we navigate to signInUrl#choose.

I can draft these tests if useful.


881-918: Add assertion that UserSignOut is NOT emitted in multisession single-sign-out

To prevent regressions of cookie clearing in multisession flows, assert we don’t emit events.UserSignOut in this path.

Apply within this block:

@@
-      const sut = new Clerk(productionPublishableKey);
+      const emitSpy = jest.spyOn(eventBus, 'emit');
+      const sut = new Clerk(productionPublishableKey);
@@
-      await waitFor(() => {
+      await waitFor(() => {
         expect(mockSession1.remove).toHaveBeenCalled();
         expect(mockClientDestroy).not.toHaveBeenCalled();
         // Since session '1' is the current session, navigation should happen
         expect(sut.navigate).toHaveBeenCalledWith(expect.any(String));
       });
@@
-      expect(mockClient.lastActiveSessionId).toBe('1');
+      expect(mockClient.lastActiveSessionId).toBe('1');
       expect(sut.session).toBe(null);
       expect(sut.isSignedIn).toBe(false);
+
+      // Ensure we did NOT broadcast a global sign-out (avoids clearing cookies for remaining sessions)
+      expect(emitSpy).not.toHaveBeenCalledWith(events.UserSignOut, expect.anything());
+      emitSpy.mockRestore();

920-957: Also assert no UserSignOut event when a non-current session is removed

This cements the “no global sign-out” behavior for non-current removals.

Apply within this block:

@@
-      const sut = new Clerk(productionPublishableKey);
+      const emitSpy = jest.spyOn(eventBus, 'emit');
+      const sut = new Clerk(productionPublishableKey);
@@
-      await waitFor(() => {
+      await waitFor(() => {
         expect(mockSession2.remove).toHaveBeenCalled();
         expect(mockClientDestroy).not.toHaveBeenCalled();
         // Since session '2' is NOT the current session, no navigation should happen
         expect(sut.navigate).not.toHaveBeenCalled();
       });
@@
       expect(sut.isSignedIn).toBe(true);
       expect(mockClient.lastActiveSessionId).toBe('1');
+
+      // Ensure a global sign-out was not broadcast
+      expect(emitSpy).not.toHaveBeenCalledWith(events.UserSignOut, expect.anything());
+      emitSpy.mockRestore();
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • 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 07a5851 and 3d24960.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/__tests__/clerk.test.ts (3 hunks)
  • packages/clerk-js/src/core/clerk.ts (2 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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/clerk-js/src/core/__tests__/clerk.test.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/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.ts
  • packages/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.ts
  • packages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/core/__tests__/clerk.test.ts (2)
packages/clerk-js/src/core/clerk.ts (1)
  • Clerk (201-2946)
packages/types/src/clerk.ts (1)
  • Clerk (164-886)
packages/clerk-js/src/core/clerk.ts (1)
packages/clerk-js/src/utils/url.ts (1)
  • stripOrigin (176-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: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/core/clerk.ts (1)

2786-2804: Nice addition: clear, focused JSDoc for #setAccessors

The doc explains undefined/null handling and intended side effects succinctly. Good improvement to maintainability.

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

67-68: LGTM: add DisplayConfig.afterSignOutOneUrl for host-based redirects

This matches the new multisession sign-out fallback and keeps tests deterministic.


855-855: LGTM: expectation updated to host-based afterSignOutOneUrl

This aligns with buildAfterMultiSessionSingleSignOutUrl() default behavior when no signInUrl option is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants