feat(backend, clerk-js): Support origin outage mode #6678
Conversation
🦋 Changeset detectedLatest commit: 8b2a3dc The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 GitHub.
|
📝 WalkthroughWalkthroughAdds a new MissingExpiredTokenError class and exports it; changes Token.create to accept a Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (10)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/src/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.ts?(x)📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)packages/shared/src/errors/missingExpiredTokenError.ts (1)
⏰ 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). (25)
🔇 Additional comments (1)
Comment |
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
3a21810 to
0b098e5
Compare
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
0b098e5 to
f347347
Compare
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
f347347 to
929883f
Compare
![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 (1)
packages/backend/src/tokens/handshake.ts (1)
152-154: Verify security implications of passing session tokens in URL query parameters.Passing session tokens as query parameters can expose them through:
- Browser history and autocomplete
- Server access logs
- Referer headers when navigating to external sites
- Network monitoring tools
- Proxy logs
If this is intentional for the handshake flow (e.g., for cookie-less environments or cross-origin scenarios), ensure that:
- The token has a very short TTL
- It's single-use/nonce-based
- The endpoint validates and immediately invalidates it
- Documentation warns developers about the security trade-offs
Additionally, using
constants.Cookies.Sessionas a URL query parameter name (not a cookie) may be confusing. Consider whether a separate constant likeconstants.QueryParameters.Sessionwould be clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.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/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/__tests__/Token.test.ts
🧬 Code graph analysis (5)
packages/backend/src/tokens/__tests__/handshake.test.ts (1)
packages/backend/src/tokens/handshake.ts (1)
HandshakeService(84-360)
packages/backend/src/tokens/handshake.ts (1)
packages/backend/src/constants.ts (1)
constants(79-85)
packages/clerk-js/src/core/resources/Session.ts (1)
packages/clerk-js/src/core/resources/Token.ts (1)
Token(6-54)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (4)
packages/clerk-js/src/core/tokenCache.ts (1)
SessionTokenCache(407-407)packages/clerk-js/src/core/events.ts (1)
eventBus(31-31)packages/clerk-js/src/test/core-fixtures.ts (1)
clerkMock(249-256)packages/clerk-js/src/core/resources/Session.ts (1)
Session(44-442)
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (2)
packages/clerk-js/src/test/core-fixtures.ts (1)
mockFetch(262-272)packages/clerk-js/src/core/fapiClient.ts (1)
createFapiClient(80-299)
🔇 Additional comments (4)
packages/backend/src/tokens/__tests__/handshake.test.ts (1)
435-467: LGTM! Comprehensive test coverage for session token handling.The tests properly verify both scenarios:
- Session token is included in the handshake URL when present
- Session token is excluded when absent (using
toBeNull())The test setup correctly creates isolated contexts and the assertions are accurate.
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (1)
108-147: LGTM! Well-structured tests for the new search parameter support.The test suite properly validates:
- Search parameters are correctly included in the API request URL
- Backward compatibility is maintained when search parameters are omitted
The assertions check both URL construction and request options, providing comprehensive coverage.
packages/clerk-js/src/core/resources/Session.ts (1)
2-2: LGTM! Well-implemented retry mechanism for origin outage mode.The error handling logic correctly:
- Catches the specific error condition (422 with
missing_expired_tokencode)- Validates all preconditions before retrying (including array bounds check)
- Retries with the expired token passed as a search parameter
- Re-throws other errors without modification
- Maintains proper caching behavior (tokenResolver set before catch)
The implementation avoids infinite recursion since the retry uses different parameters, and the logic aligns with the comprehensive test coverage in Session.test.ts.
Also applies to: 408-419
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)
1-1: LGTM! Comprehensive test coverage for origin outage mode fallback.The test suite thoroughly validates the retry mechanism:
Successful retry scenario: Verifies that when a 422 with
missing_expired_tokenis returned andlastActiveTokenexists, the code makes a second request with the expired token in the search parameters.Edge cases properly covered:
- No retry when
lastActiveTokenis unavailable- No retry for non-422 HTTP errors
- No retry when the error code differs
The test setup with spies on
eventBus.emitandBaseResource._fetchis appropriate, and the assertions correctly validate both the number of fetch calls and the request shapes.Also applies to: 1090-1235
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
929883f to
771560c
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (1)
124-128: Consider adding body assertion for consistency.The existing tests and the second test in this suite (lines 140-145) all assert
body: ''in their expectations. For consistency, consider adding the same assertion here.expect(options).toMatchObject({ method: 'POST', + body: '', credentials: 'include', headers: expect.any(Headers), });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/clerk-js/src/core/resources/Session.ts
- packages/clerk-js/src/core/resources/tests/Session.test.ts
- packages/backend/src/tokens/handshake.ts
- packages/clerk-js/src/core/resources/Token.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.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/resources/__tests__/Token.test.tspackages/backend/src/tokens/__tests__/handshake.test.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (3)
packages/clerk-js/src/test/core-fixtures.ts (1)
mockFetch(262-272)packages/clerk-js/src/core/fapiClient.ts (1)
createFapiClient(80-299)packages/clerk-js/src/core/resources/Token.ts (1)
Token(6-54)
packages/backend/src/tokens/__tests__/handshake.test.ts (2)
packages/backend/src/tokens/handshake.ts (1)
HandshakeService(84-360)packages/backend/src/constants.ts (1)
constants(79-85)
⏰ 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). (4)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/backend/src/tokens/__tests__/handshake.test.ts (2)
435-450: LGTM! Well-structured test for session token passthrough.The test correctly verifies that when a session token is present in the
AuthenticateContext, it's properly appended to the handshake URL as a query parameter. Good test isolation by creating a dedicated service instance.
452-467: LGTM! Comprehensive negative test case.This test properly verifies that when the session token is absent, the query parameter is not added to the handshake URL. Together with the previous test, this provides complete coverage for the session token passthrough feature.
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (2)
108-147: Good test coverage for the new search parameters feature.The test suite properly covers both the new search parameters functionality and backward compatibility. The mock setup, assertions on URL construction, and request options verification are well-structured.
4-4: No issues found.The import of
mockJwton line 4 is valid. The export exists in core-fixtures, and it is correctly used in the new test cases (lines 115 and 132) to mock API responses. Both new tests are well-structured: the first verifies search parameter inclusion in the URL, and the second ensures backward compatibility when no search parameters are provided.
|
Found 23 test failures on Blacksmith runners:
|
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
771560c to
11fb32b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/clerk-js/src/core/resources/Token.ts (2)
11-20: JSDoc documentation is still missing.Despite being marked as addressed in a previous review, the
createmethod lacks JSDoc documentation. The coding guidelines require "All public APIs must be documented with JSDoc."Add comprehensive JSDoc comments above the method documenting:
- The method's purpose and when to use it
- Each parameter (
path,body,search) with types and descriptions- The return value (
Promise<TokenResource>)- Any errors that may be thrown
Example JSDoc structure:
/** * Creates a new token resource by making a POST request to the specified path. * * @param path - The API endpoint path for token creation * @param body - The request body payload (default: empty object) * @param search - URL search/query parameters to include in the request (default: empty object) * @returns A promise that resolves to a TokenResource instance * @throws {Error} If the API request fails or returns an error */ static async create(path: string, body: unknown = {}, search: Record<string, string> = {}): Promise<TokenResource> {As per coding guidelines
11-11: Thebody: anytype violation persists.Despite being marked as addressed in a previous review, the
bodyparameter still uses theanytype, which violates the coding guideline: "Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards."Replace
anywithunknownor define a specific type/interface for the body parameter.Apply this diff to replace
anywithunknown:- static async create(path: string, body: any = {}, search: Record<string, string> = {}): Promise<TokenResource> { + static async create(path: string, body: unknown = {}, search: Record<string, string> = {}): Promise<TokenResource> {As per coding guidelines
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/clerk-js/src/core/resources/Session.ts
- packages/backend/src/tokens/tests/handshake.test.ts
- packages/clerk-js/src/core/resources/tests/Session.test.ts
- packages/clerk-js/src/core/resources/tests/Token.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.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/resources/Token.tspackages/backend/src/tokens/handshake.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/handshake.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/handshake.ts (1)
packages/backend/src/constants.ts (1)
constants(79-85)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
5f7f54b to
4a663f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/Session.ts (1)
408-419: Consider explicit type annotation for catch parameter.The error handling logic is correct and defensive. However, explicitly typing the catch parameter would improve code clarity and align with TypeScript best practices.
Apply this diff:
- const tokenResolver = Token.create(path, params).catch(e => { + const tokenResolver = Token.create(path, params).catch((e: unknown) => { if ( e instanceof ClerkAPIResponseError && e.status === 422 &&As per coding guidelines
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/backend/src/tokens/handshake.ts
- packages/clerk-js/src/core/resources/Token.ts
- packages/clerk-js/src/core/resources/tests/Token.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/Session.tspackages/clerk-js/src/core/resources/__tests__/Session.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/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
🧬 Code graph analysis (3)
packages/backend/src/tokens/__tests__/handshake.test.ts (2)
packages/backend/src/tokens/handshake.ts (1)
HandshakeService(84-360)packages/backend/src/constants.ts (1)
constants(79-85)
packages/clerk-js/src/core/resources/Session.ts (1)
packages/clerk-js/src/core/resources/Token.ts (1)
Token(6-54)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (4)
packages/clerk-js/src/core/tokenCache.ts (1)
SessionTokenCache(407-407)packages/clerk-js/src/core/events.ts (1)
eventBus(32-32)packages/clerk-js/src/test/core-fixtures.ts (1)
clerkMock(249-256)packages/clerk-js/src/core/resources/Session.ts (1)
Session(44-442)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/backend/src/tokens/__tests__/handshake.test.ts (1)
435-467: LGTM! Comprehensive test coverage for sessionToken handling.Both test cases properly validate the conditional inclusion of the session token in the handshake URL, with appropriate isolation and assertions.
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)
1090-1235: LGTM! Excellent test coverage for origin outage mode fallback.The test suite comprehensively validates all branches of the retry logic:
- Successful retry path with
expired_tokenquery parameter on 422 withmissing_expired_tokencode- No retry when
lastActiveTokenis unavailable- No retry for non-422 errors
- No retry when error code differs
The test structure is clean, with proper setup/teardown, clear assertions, and good mock isolation.
bratsos
changed the title
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
4a663f9 to
7dc1fa1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)
1207-1234: Remove unnecessary type cast.Line 1218 uses
as unknown as SessionJSONwhenlast_active_tokenis set to a valid token object. This cast is unnecessary and should be removed for type safety. The cast was needed in the test at line 1161 wherelast_active_tokenisnull, but not here.Apply this diff:
- } as unknown as SessionJSON); + } as SessionJSON);Otherwise, the test correctly validates that retry only occurs for the specific
missing_expired_tokenerror code, not other 422 errors.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/bundlewatch.config.json(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/clerk-js/bundlewatch.config.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/clerk-js/src/core/resources/tests/Token.test.ts
- packages/backend/src/tokens/tests/handshake.test.ts
- packages/backend/src/tokens/handshake.ts
- packages/clerk-js/src/core/resources/Session.ts
- packages/clerk-js/src/core/resources/Token.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/resources/__tests__/Session.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/resources/__tests__/Session.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/__tests__/Session.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/resources/__tests__/Session.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
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/resources/__tests__/Session.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/resources/__tests__/Session.test.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (4)
packages/clerk-js/src/core/tokenCache.ts (1)
SessionTokenCache(407-407)packages/clerk-js/src/core/events.ts (1)
eventBus(32-32)packages/clerk-js/src/test/core-fixtures.ts (1)
clerkMock(249-256)packages/clerk-js/src/core/resources/Session.ts (1)
Session(44-442)
⏰ 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). (33)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (5)
1-1: LGTM! Import supports the new error handling tests.The import of
ClerkAPIResponseErroris correctly added to support the new origin outage mode fallback tests.
1090-1105: LGTM! Proper test suite setup and teardown.The test suite setup correctly initializes spies on
eventBus.emitandBaseResource._fetch, and the teardown ensures proper cleanup by restoring spies and clearing the clerk instance.
1107-1148: LGTM! Comprehensive test of retry logic.This test correctly validates the origin outage mode fallback behavior: when the API returns a 422 with
missing_expired_tokenand alastActiveTokenexists, the request is retried with the expired token as a search parameter. The assertions verify both the initial request and the retry request parameters.
1150-1179: LGTM! Validates no retry without lastActiveToken.This test correctly verifies that when
lastActiveTokenis not available, the request fails without attempting a retry, even when the API returns a 422 withmissing_expired_token.
1181-1205: LGTM! Validates retry is specific to 422 errors.This test correctly verifies that the retry logic is only triggered for 422 status codes, ensuring that other error responses (like 400) are not retried even when a
lastActiveTokenis available.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/constants.ts (1)
27-40: The__sessionquery parameter does expose the actual session token—ensure the parameter is stripped from the URL after the handshake exchange completes.The code appends the session token directly to query parameters in
buildRedirectToHandshake(), which sends it to the Clerk backend. While other handshake parameters (Handshake,HandshakeHelp,DevBrowser,HandshakeNonce) are deleted from the redirect URL in development mode, theSessionparameter is missing from this cleanup logic. This leaves the token exposed in browser history and potential logs until it's replaced by secure cookies. AddnewUrl.searchParams.delete(constants.QueryParameters.Session);to the parameter cleanup, and verify integration tests confirm the parameter is removed after the cookie exchange.
🧹 Nitpick comments (1)
packages/backend/src/constants.ts (1)
27-40: PreferCookies.Sessionover duplicating'__session'inQueryParameters.
Keeps the “same value” guarantee true over time (and matches the comment on Line 31).const QueryParameters = { ClerkSynced: '__clerk_synced', SuffixedCookies: 'suffixed_cookies', ClerkRedirectUrl: '__clerk_redirect_url', // use the reference to Cookies to indicate that it's the same value DevBrowser: Cookies.DevBrowser, Handshake: Cookies.Handshake, HandshakeHelp: '__clerk_help', LegacyDevBrowser: '__dev_session', HandshakeReason: '__clerk_hs_reason', HandshakeNonce: Cookies.HandshakeNonce, HandshakeFormat: 'format', - Session: '__session', + Session: Cookies.Session, } as const;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/constants.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/tokens/handshake.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/constants.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/constants.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/constants.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/backend/src/constants.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
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
Implement proper logging with different levels
Files:
packages/backend/src/constants.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/constants.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{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
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/constants.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/backend/src/constants.ts
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/backend/src/constants.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). (26)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
a40d623 to
c477def
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/clerk-js/src/core/resources/Session.ts (1)
401-408: FixToken.create(..., search)typing; don’t passundefined(and decide skip-cache on retry).
This repeats a concern from earlier review:Token.createtakessearch: Record<string, string> = {}, soundefinedis an avoidable TS footgun, and the retry currently ignores thedebug=skip_cacheflag.- const tokenResolver = Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined).catch(e => { + const search: Record<string, string> = skipCache ? { debug: 'skip_cache' } : {}; + + const tokenResolver = Token.create(path, params, search).catch(e => { if (MissingExpiredTokenError.is(e) && lastActiveToken) { - return Token.create(path, { ...params }, { expired_token: lastActiveToken }); + return Token.create(path, { ...params }, { ...search, expired_token: lastActiveToken }); } throw e; });#!/bin/bash # Verify TS strictness expectations and ensure no other callers pass `undefined` for `search`. rg -n "strictNullChecks|\"strict\"\s*:" -S packages/clerk-js/tsconfig*.json tsconfig*.json || true rg -nP "Token\.create\([^)]*,\s*[^)]*,\s*undefined\s*\)" --type=ts -S packages/clerk-js/src || truepackages/clerk-js/src/core/resources/Token.ts (1)
12-18:Token.createusesanyand lacks JSDoc documentation.The
bodyparameter violates the type safety guidelines (avoidany), and as a public API method on an exported class, it requires comprehensive JSDoc comments including parameter, return, and error documentation per the coding guidelines.export class Token extends BaseResource implements TokenResource { pathRoot = 'tokens'; jwt?: JWT; + /** + * Create (mint) a token via FAPI. + * + * @param path - FAPI path to mint the token from. + * @param body - POST body payload. + * @param search - Query-string parameters (e.g. debug flags, expired token recovery). + * @returns The minted token resource. + * @throws ClerkAPIResponseError on non-2xx responses. + */ - static async create(path: string, body: any = {}, search: Record<string, string> = {}): Promise<TokenResource> { + static async create( + path: string, + body: Record<string, unknown> = {}, + search: Record<string, string> = {}, + ): Promise<TokenResource> { const json = (await BaseResource._fetch<TokenJSON>({ method: 'POST', path, body, search, })) as unknown as TokenJSON;
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)
1090-1285: Origin-outage fallback coverage is solid (retry/no-retry matrix).
Nice set of cases: retries only for 422missing_expired_token, cookie fallback whenlast_active_tokenis missing, and no retry for other 4xx / other codes.
Optional: add a small assertion that__internal_getSessionCookieis not consulted whenlastActiveTokenis present (to lock in the precedence order).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
.changeset/chatty-tigers-see.md(1 hunks)packages/backend/src/constants.ts(1 hunks)packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)packages/shared/src/error.ts(1 hunks)packages/shared/src/errors/missingExpiredTokenError.ts(1 hunks)packages/shared/src/types/clerk.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/shared/src/types/clerk.ts
- packages/clerk-js/src/core/clerk.ts
- packages/clerk-js/src/core/resources/tests/Token.test.ts
- packages/shared/src/errors/missingExpiredTokenError.ts
- .changeset/chatty-tigers-see.md
- packages/backend/src/tokens/handshake.ts
- packages/backend/src/constants.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
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
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{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
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.tspackages/shared/src/error.tspackages/clerk-js/src/core/resources/__tests__/Session.test.tspackages/clerk-js/src/core/resources/Session.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
packages/backend/src/tokens/__tests__/handshake.test.tspackages/clerk-js/src/core/resources/__tests__/Session.test.ts
🧬 Code graph analysis (2)
packages/backend/src/tokens/__tests__/handshake.test.ts (2)
packages/backend/src/tokens/handshake.ts (1)
HandshakeService(84-360)packages/backend/src/constants.ts (1)
constants(80-86)
packages/clerk-js/src/core/resources/Session.ts (2)
packages/clerk-js/src/core/resources/Token.ts (1)
Token(7-55)packages/shared/src/errors/missingExpiredTokenError.ts (1)
MissingExpiredTokenError(16-45)
⏰ 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). (25)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/shared/src/error.ts (1)
6-6: Public re-export looks fine; ensure it’s intentional + documented as a public API.
Re-exportingMissingExpiredTokenErrormakes it part of@clerk/shared/error’s surface—please ensure the changeset/docs reflect this addition (and that it’s meant to be supported long-term).packages/clerk-js/src/core/resources/__tests__/Session.test.ts (1)
1-1: Import change is aligned with new error handling tests.packages/clerk-js/src/core/resources/Session.ts (1)
2-2: Import addition is correct for the new retry gate.
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
c477def to
37a7dba
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/clerk-js/src/core/resources/Token.ts (1)
12-12: Address the persistinganytype violation.The
bodyparameter still usesanytype, which violates the coding guideline: "Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards." A past review comment flagged this issue and was marked as addressed in commit 771560c, but theanytype remains in the current code.Consider applying a type-safe solution:
- static async create(path: string, body: any = {}, search: Record<string, string> = {}): Promise<TokenResource> { + static async create(path: string, body: Record<string, unknown> = {}, search: Record<string, string> = {}): Promise<TokenResource> {As per coding guidelines
packages/backend/src/tokens/__tests__/handshake.test.ts (2)
435-450: Critical: Test uses wrong constant despite being marked as fixed.Line 449 uses
constants.Cookies.Sessionto verify the query parameter, but the production code atpackages/backend/src/tokens/handshake.ts:153sets it usingconstants.QueryParameters.Session. This mismatch was flagged in a previous review and marked as addressed, but the issue persists in the current code.Apply this fix:
- expect(url.searchParams.get(constants.Cookies.Session)).toBe('test_session_token_123'); + expect(url.searchParams.get(constants.QueryParameters.Session)).toBe('test_session_token_123');
452-467: Critical: Test uses wrong constant (same issue as previous test).Line 466 has the identical issue—it uses
constants.Cookies.Sessioninstead ofconstants.QueryParameters.Sessionto check the URL query parameter.Apply this fix:
- expect(url.searchParams.get(constants.Cookies.Session)).toBeNull(); + expect(url.searchParams.get(constants.QueryParameters.Session)).toBeNull();
🧹 Nitpick comments (1)
packages/backend/src/tokens/handshake.ts (1)
152-154: Consider adding a comment to document the security trade-off.The implementation correctly uses
constants.QueryParameters.Sessionand aligns with the origin outage mode requirements. However, as discussed in a previous review, passing session tokens in URLs is generally discouraged per RFC 6750 due to exposure risks. While this is acceptable here with short-lived tokens (~10s to expiration), adding an inline comment would help future maintainers understand this intentional trade-off.Consider adding a comment like:
+ // Passing session token in URL for origin outage mode handshake. + // Token is short-lived (~10s to expiration) to minimize exposure risk. if (this.authenticateContext.sessionToken) { url.searchParams.append(constants.QueryParameters.Session, this.authenticateContext.sessionToken); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
.changeset/chatty-tigers-see.md(1 hunks)packages/backend/src/constants.ts(1 hunks)packages/backend/src/tokens/__tests__/handshake.test.ts(1 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(1 hunks)packages/clerk-js/src/core/resources/Session.ts(2 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.test.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(2 hunks)packages/shared/src/error.ts(1 hunks)packages/shared/src/errors/missingExpiredTokenError.ts(1 hunks)packages/shared/src/types/clerk.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/clerk-js/src/core/clerk.ts
- packages/clerk-js/src/core/resources/tests/Session.test.ts
- packages/clerk-js/src/core/resources/Session.ts
- packages/shared/src/errors/missingExpiredTokenError.ts
- packages/shared/src/error.ts
- packages/backend/src/constants.ts
- packages/shared/src/types/clerk.ts
- packages/clerk-js/src/core/resources/tests/Token.test.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
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
Implement proper logging with different levels
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{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
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/backend/src/tokens/handshake.tspackages/clerk-js/src/core/resources/Token.tspackages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/handshake.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/handshake.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
packages/backend/src/tokens/__tests__/handshake.test.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/handshake.ts (1)
packages/backend/src/constants.ts (1)
constants(80-86)
⏰ 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). (25)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.changeset/chatty-tigers-see.md (1)
1-6: LGTM! Changeset properly documents the feature addition.The changeset correctly specifies minor version bumps for both affected packages and clearly describes the new feature.
packages/clerk-js/src/core/resources/Token.ts (1)
12-21: Signature change improves flexibility for token requests.Replacing the
skipCacheboolean with asearchparameters object provides more flexibility for passing query parameters, which aligns with the origin outage mode feature requirements. The updated fetch call correctly includes bothbodyandsearchparameters.
|
|
||
| // TODO: update template endpoint to accept organizationId | ||
| const params: Record<string, string | null> = template ? {} : { organizationId }; | ||
| const lastActiveToken = this.lastActiveToken?.getRawString() ?? Session.clerk.__internal_getSessionCookie?.(); |
There was a problem hiding this comment.
❓ Why do we need this fallback specifically? In a multi-session scenario, it's feasible (though unlikely) that the session cookie would not match the token in memory. Is this a safe-guard or a to address a specific scenario?
If there isn't a specific scenario in mind, it would be preferable to not need to add this internal property and reference the core Clerk instance here, to avoid tight coupling
There was a problem hiding this comment.
Thanks for the call-out!
Double check me on this, but I think in a real origin outage mode, v1/client API call will fail - which is what populates this.lastActiveToken if I'm not mistaken - but we may already have a token stored in the cookies.
There was a problem hiding this comment.
We have a fallback in place already that reads from the session cookie:
javascript/packages/clerk-js/src/core/clerk.ts
Lines 2730 to 2768 in 7c12ada
There was a problem hiding this comment.
Oh nice I missed that. Then we can simplify this a lot, I'll just drop this commit f076cc3
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
2 times, most recently
from
ad4fdba to
33c6d84
Compare
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
33c6d84 to
fa870c2
Compare
.changeset/chatty-tigers-see.md
Outdated
| '@clerk/backend': minor | ||
| --- | ||
|
|
||
| Adds support for origin outage mode |
There was a problem hiding this comment.
Let's update this to be user-focused. What does it mean for users at a high level?
There was a problem hiding this comment.
Sure! How about this: Improves resilience by keeping users logged in when Clerk's origin is temporarily unavailable using edge-based token generation
bratsos
force-pushed
the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
from
ead5018 to
83ba391
Compare
bratsos
deleted the
alexbratsos/plat-1654-add-logic-to-mint-new-session-tokens-based-on-existing-ones
branch
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests
Public API
✏️ Tip: You can customize this high-level summary in your review settings.