chore(repo): e2e resiliency #7476
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdded a dynamic integration-matrix and artifacts flow in CI, a script to compute affected integration suites, tarball-based test installs and environment-driven Playwright worker override; updated tarball resolution; and added synchronization/visibility waits in several Playwright integration tests and a page object. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/testing/src/playwright/unstable/page-objects/sessionTask.ts (1)
11-22: Consider adding explicit return types to public API methods.Per the TypeScript coding guidelines, public API functions should have explicit return types. Both
resolveForceOrganizationSelectionTaskandresolveResetPasswordTaskshould declare: Promise<void>as their return type.Apply this diff to add explicit return types:
- resolveForceOrganizationSelectionTask: async (fakeOrganization: { name: string; slug?: string }) => { + resolveForceOrganizationSelectionTask: async (fakeOrganization: { name: string; slug?: string }): Promise<void> => { const createOrganizationButton = page.getByRole('button', { name: /continue/i }); await expect(createOrganizationButton).toBeVisible(); await page.locator('input[name=name]').fill(fakeOrganization.name); if (fakeOrganization.slug) { await page.locator('input[name=slug]').fill(fakeOrganization.slug); } await createOrganizationButton.click(); }, - resolveResetPasswordTask: async ({ + resolveResetPasswordTask: async ({ newPassword, confirmPassword, }: { newPassword: string; confirmPassword: string; - }) => { + }): Promise<void> => { const newPasswordInput = page.locator('input[name=newPassword]'); const confirmPasswordInput = page.locator('input[name=confirmPassword]');Also applies to: 23-42
📜 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 (3)
integration/tests/session-tasks-multi-session.test.ts(1 hunks)integration/tests/session-tasks-sign-in-reset-password.test.ts(2 hunks)packages/testing/src/playwright/unstable/page-objects/sessionTask.ts(1 hunks)
🧰 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:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.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:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/session-tasks-sign-in-reset-password.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:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.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:
integration/tests/session-tasks-multi-session.test.tspackages/testing/src/playwright/unstable/page-objects/sessionTask.tsintegration/tests/session-tasks-sign-in-reset-password.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/testing/src/playwright/unstable/page-objects/sessionTask.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/testing/src/playwright/unstable/page-objects/sessionTask.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). (25)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
integration/tests/session-tasks-multi-session.test.ts (1)
84-86: Network idle wait improves synchronization.The addition of
waitForLoadState('networkidle')helps ensure the page is fully settled before interacting with the user button. This is a pragmatic approach to reducing flakiness in e2e tests.Note:
networkidlewaits for 500ms of network inactivity, which can be sensitive to background requests (analytics, polling, etc.). If this becomes flaky, consider using more specific waits likewaitForResponse()for critical requests or waiting for specific UI state changes.integration/tests/session-tasks-sign-in-reset-password.test.ts (2)
31-40: LGTM! Explicit visibility checks improve test reliability.The pattern of storing the button locator and waiting for visibility before interaction is excellent for preventing race conditions. The 10-second timeout provides sufficient buffer for varying network conditions in e2e testing.
88-97: Consistent pattern applied.The same visibility wait pattern is applied here as in the first test case, ensuring consistent behavior across test scenarios.
packages/testing/src/playwright/unstable/page-objects/sessionTask.ts (1)
30-37: LGTM! Locator pattern improves reliability and readability.Storing the locators in variables and asserting visibility before filling is a best practice for page objects. This makes the code more maintainable and prevents interactions with elements that aren't ready.
@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 (4)
integration/presets/utils.ts (1)
10-11: Consider using top-level import fornode:fs.The dynamic
require('node:fs')inside the function works but is unconventional in TypeScript/ESM codebases. Consider importingfsat the top of the file alongsidepathfor consistency.import path from 'node:path'; +import fs from 'node:fs';Then update the usage:
- // eslint-disable-next-line @typescript-eslint/no-require-imports - const fs = require('node:fs'); const prefix = `clerk-${pkg}-`; const entries = fs.readdirSync(tarballsDir);integration/playwright.config.ts (1)
17-19: Consider validatingPLAYWRIGHT_WORKERSto avoid NaN.If
PLAYWRIGHT_WORKERSis set to a non-numeric string,Number.parseIntreturnsNaN, which may cause unexpected Playwright behavior. A fallback could improve robustness.- workers: process.env.CI - ? (process.env.PLAYWRIGHT_WORKERS ? Number.parseInt(process.env.PLAYWRIGHT_WORKERS, 10) : '50%') - : '70%', + workers: process.env.CI + ? (process.env.PLAYWRIGHT_WORKERS + ? (Number.parseInt(process.env.PLAYWRIGHT_WORKERS, 10) || '50%') + : '50%') + : '70%',This falls back to
'50%'if parsing fails.scripts/ci/compute-integration-matrix.mjs (1)
42-50: Consider logging stderr for debugging failed turbo commands.Currently, stderr is ignored (
'ignore'), which may make it difficult to diagnose why a suite was incorrectly marked as not affected. Capturing stderr could help with CI debugging.const result = spawnSync( 'pnpm', ['turbo', 'run', `test:integration:${suite}`, '--affected', '--dry=json', ...turboArgs], - { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }, + { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }, ); if (result.status !== 0 || !result.stdout) { + if (result.stderr) { + console.error(`[${suite}] turbo stderr:`, result.stderr); + } return false; }.github/workflows/ci.yml (1)
428-433: Tarball path resolution may fail if multiple versions exist.Using
ls | head -1assumes exactly one tarball per package prefix. If the artifact contains multiple versions (unlikely but possible), this could silently pick the wrong one. Consider adding validation.- name: Resolve tarball paths id: tarballs run: | - echo "BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz | head -1)" >> "$GITHUB_OUTPUT" - echo "CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz | head -1)" >> "$GITHUB_OUTPUT" - echo "UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz | head -1)" >> "$GITHUB_OUTPUT" + BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz 2>/dev/null) + CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz 2>/dev/null) + UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz 2>/dev/null) + + # Validate single match + for name in BACKEND_TGZ CLERK_JS_TGZ UI_TGZ; do + count=$(echo "${!name}" | wc -w) + if [ "$count" -ne 1 ]; then + echo "::error::Expected exactly 1 match for $name, found $count" + exit 1 + fi + done + + echo "BACKEND_TGZ=$BACKEND_TGZ" >> "$GITHUB_OUTPUT" + echo "CLERK_JS_TGZ=$CLERK_JS_TGZ" >> "$GITHUB_OUTPUT" + echo "UI_TGZ=$UI_TGZ" >> "$GITHUB_OUTPUT"
📜 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 (4)
.github/workflows/ci.yml(4 hunks)integration/playwright.config.ts(1 hunks)integration/presets/utils.ts(1 hunks)scripts/ci/compute-integration-matrix.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
integration/playwright.config.tsintegration/presets/utils.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/playwright.config.tsintegration/presets/utils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
integration/playwright.config.tsintegration/presets/utils.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
integration/playwright.config.tsintegration/presets/utils.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:
integration/playwright.config.tsintegration/presets/utils.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
integration/playwright.config.tsintegration/presets/utils.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:
integration/playwright.config.tsintegration/presets/utils.ts
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
297-297: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
318-318: missing input "turbo-team" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
318-318: missing input "turbo-token" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
335-335: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
352-352: missing input "turbo-team" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
352-352: missing input "turbo-token" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Integration Artifacts
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
integration/presets/utils.ts (1)
6-19: LGTM! Tarball resolution logic is sound.The implementation correctly handles the three scenarios: tarball directory set (resolve file URL), CI without tarball directory (use
'*'), and non-CI (local package link). Error handling for missing tarballs is appropriate.scripts/ci/compute-integration-matrix.mjs (1)
7-62: LGTM! Well-structured matrix computation.The script correctly identifies affected suites by parsing Turborepo's dry-run output and filters the matrix accordingly. Error handling for non-zero exit codes and JSON parsing is appropriate.
.github/workflows/ci.yml (4)
293-329: Well-designed dynamic matrix computation job.The integration-matrix job cleanly computes affected suites and exposes
has-testsandmatrixoutputs. The conditionalhas-tests=falsefor empty matrices prevents unnecessary downstream job execution.
331-376: Good separation of artifact building from test execution.Building both default and RQ tarballs in a single job and uploading them as artifacts is efficient. The 1-day retention is appropriate for ephemeral CI artifacts.
420-426: LGTM! Clean tarball variant selection.The logic correctly selects the RQ or default tarball directory based on the matrix value.
316-318: [Your rewritten review comment text here]
[Exactly ONE classification tag]
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
498-498: Consider making PLAYWRIGHT_WORKERS configurable.The value is hardcoded to
"4", but according to the AI summary, the Playwright config supports environment-driven worker override. Different test suites in the matrix might benefit from different parallelism levels (e.g., faster suites could use more workers, slower or resource-intensive suites might need fewer).Consider making this configurable via the matrix or a repository variable:
- PLAYWRIGHT_WORKERS: "4" + PLAYWRIGHT_WORKERS: ${{ matrix.playwright-workers || vars.PLAYWRIGHT_WORKERS || '4' }}Then you can optionally set
playwright-workersper matrix entry incompute-integration-matrix.mjs, or use a globalPLAYWRIGHT_WORKERSrepository variable, falling back to'4'as the default.
📜 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 (1)
.github/workflows/ci.yml(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
297-297: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
318-318: missing input "turbo-team" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
318-318: missing input "turbo-token" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
335-335: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
352-352: missing input "turbo-team" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
352-352: missing input "turbo-token" which is required by action "Setup Action (Blacksmith)" defined at "./.github/actions/init-blacksmith". all required inputs are "turbo-team", "turbo-token"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
.github/workflows/ci.yml (5)
331-378: Double build is intentional for RQ variant testing.The workflow builds packages twice (default and RQ variants) which increases CI time but is necessary to test both
CLERK_USE_RQconfigurations. The--forceflag ensures the RQ build bypasses cache and rebuilds with the environment variable.This approach centralizes artifact creation, allowing downstream integration tests to download pre-built tarballs rather than building per-matrix-job, which should improve overall pipeline efficiency.
488-488: LGTM: Smart retry-based debug enablement.Enabling
E2E_DEBUGonly on retry attempts (whengithub.run_attempt > 1) is an excellent approach that provides detailed logging for flaky tests without cluttering the output on first runs.
379-395: LGTM: Well-structured dynamic matrix implementation.The job correctly:
- Depends on
integration-artifactsandintegration-matrixto ensure tarballs are built and the matrix is computed- Gates execution on
has-tests == 'true'to skip when no integration tests are affected- Uses
fromJson()to parse the dynamic matrix outputThis pattern enables efficient CI by running only affected integration test suites rather than a static set.
421-427: LGTM: Correct tarball variant selection.The conditional logic correctly selects between the RQ and default tarball directories based on the matrix parameter, aligning with the two build variants created in the
integration-artifactsjob.
320-329: The workflow already has proper error handling through GitHub Actions' default bash configuration. By default, bash runs with set -e, so it will terminate on the first non-zero exit code that isn't handled. Thecompute-integration-matrix.mjsscript exists and includes error handling with a try-catch block. No additional error handling is needed in the workflow step.Likely an incorrect or invalid review comment.
| - name: Resolve tarball paths | ||
| id: tarballs | ||
| run: | | ||
| echo "BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz | head -1)" >> "$GITHUB_OUTPUT" | ||
| echo "CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz | head -1)" >> "$GITHUB_OUTPUT" | ||
| echo "UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz | head -1)" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Add validation for tarball path resolution.
The ls ... | head -1 pattern will fail silently if no tarballs match, resulting in empty paths being set in GITHUB_OUTPUT. Subsequent install steps would then fail with unclear "file not found" errors, making debugging difficult.
Apply this diff to add validation:
- name: Resolve tarball paths
id: tarballs
run: |
- echo "BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz | head -1)" >> "$GITHUB_OUTPUT"
- echo "CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz | head -1)" >> "$GITHUB_OUTPUT"
- echo "UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz | head -1)" >> "$GITHUB_OUTPUT"
+ set -e
+ BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz 2>/dev/null | head -1)
+ CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz 2>/dev/null | head -1)
+ UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz 2>/dev/null | head -1)
+
+ if [ -z "$BACKEND_TGZ" ] || [ -z "$CLERK_JS_TGZ" ] || [ -z "$UI_TGZ" ]; then
+ echo "Error: Failed to resolve one or more tarball paths"
+ echo "BACKEND_TGZ: $BACKEND_TGZ"
+ echo "CLERK_JS_TGZ: $CLERK_JS_TGZ"
+ echo "UI_TGZ: $UI_TGZ"
+ echo "Available files:"
+ ls -la "$E2E_CLERK_TARBALLS_DIR"
+ exit 1
+ fi
+
+ echo "BACKEND_TGZ=$BACKEND_TGZ" >> "$GITHUB_OUTPUT"
+ echo "CLERK_JS_TGZ=$CLERK_JS_TGZ" >> "$GITHUB_OUTPUT"
+ echo "UI_TGZ=$UI_TGZ" >> "$GITHUB_OUTPUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Resolve tarball paths | |
| id: tarballs | |
| run: | | |
| echo "BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz | head -1)" >> "$GITHUB_OUTPUT" | |
| echo "CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz | head -1)" >> "$GITHUB_OUTPUT" | |
| echo "UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz | head -1)" >> "$GITHUB_OUTPUT" | |
| - name: Resolve tarball paths | |
| id: tarballs | |
| run: | | |
| set -e | |
| BACKEND_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-backend-*.tgz 2>/dev/null | head -1) | |
| CLERK_JS_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-clerk-js-*.tgz 2>/dev/null | head -1) | |
| UI_TGZ=$(ls "$E2E_CLERK_TARBALLS_DIR"/clerk-ui-*.tgz 2>/dev/null | head -1) | |
| if [ -z "$BACKEND_TGZ" ] || [ -z "$CLERK_JS_TGZ" ] || [ -z "$UI_TGZ" ]; then | |
| echo "Error: Failed to resolve one or more tarball paths" | |
| echo "BACKEND_TGZ: $BACKEND_TGZ" | |
| echo "CLERK_JS_TGZ: $CLERK_JS_TGZ" | |
| echo "UI_TGZ: $UI_TGZ" | |
| echo "Available files:" | |
| ls -la "$E2E_CLERK_TARBALLS_DIR" | |
| exit 1 | |
| fi | |
| echo "BACKEND_TGZ=$BACKEND_TGZ" >> "$GITHUB_OUTPUT" | |
| echo "CLERK_JS_TGZ=$CLERK_JS_TGZ" >> "$GITHUB_OUTPUT" | |
| echo "UI_TGZ=$UI_TGZ" >> "$GITHUB_OUTPUT" |
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 429 to 434: the step that resolves tarball
paths currently uses `ls ... | head -1` which can produce empty outputs
silently; change it to capture each glob result into a variable, validate that
the variable is non-empty, and if empty emit a clear error to stderr and exit
non-zero so the workflow fails fast; only after successful validation write the
variables to GITHUB_OUTPUT. Ensure this is done for BACKEND_TGZ, CLERK_JS_TGZ,
and UI_TGZ.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Tests
CI / Workflows
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.