-
Notifications
You must be signed in to change notification settings - Fork 365
chore(repo): Fix nightly e2e tests #6262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis set of changes includes both stylistic and functional updates across workflow configurations, integration utilities, and test environments. Several GitHub Actions workflow files were reformatted for consistency, with some logic enhancements such as improved polling and error reporting in the Verdaccio action and enhanced Slack notifications in nightly checks. The Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
.github/workflows/ci.yml (1)
17-22
: 🚨 Down-stream jobs will all be skipped because their prerequisite is now skipped
check-permissions
is required by almost every other job vianeeds: [check-permissions]
.
Settingif: false
means the job finishes withskipped
, which in turn skips all dependent jobs, effectively disabling the whole CI pipeline.Either:
- Remove every
needs: [check-permissions]
, or- Keep the job and gate the sensitive steps only, or
- Guard with an env flag as suggested below.
- check-permissions: - runs-on: 'blacksmith-8vcpu-ubuntu-2204' - if: false + check-permissions: + if: ${{ env.DISABLE_PERMISSION_CHECK == 'true' }}
🧹 Nitpick comments (1)
.github/workflows/pr-title-linter.yml (1)
13-17
: Temporarily disabling PR-title linting degrades commit history qualityIf conventional-commit compliance is still required, prefer an env toggle instead of hard-coding
if: false
.
Otherwise, remove the workflow entirely to avoid “dead” files being formatted and re-committed by Prettier later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml
(1 hunks).github/workflows/major-version-check.yml
(1 hunks).github/workflows/nightly-checks.yml
(2 hunks).github/workflows/pr-title-linter.yml
(1 hunks).prettierignore
(0 hunks)
💤 Files with no reviewable changes (1)
- .prettierignore
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/major-version-check.yml
.github/workflows/ci.yml
.github/workflows/pr-title-linter.yml
.github/workflows/nightly-checks.yml
🧠 Learnings (5)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
.github/workflows/major-version-check.yml (2)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
.github/workflows/ci.yml (2)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
.github/workflows/pr-title-linter.yml (2)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
.github/workflows/nightly-checks.yml (12)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace on npm.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use Turbo for build orchestration and caching
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
4-7
: 👍 Additional PR triggers look goodExtending the trigger to run on PRs to
main
andrelease/v4
broadens test coverage without side effects.
.github/workflows/nightly-checks.yml
Outdated
- name: Debug registry configuration | ||
run: | | ||
echo "Current registry:" | ||
pnpm config get registry | ||
echo "Registry auth token set:" | ||
pnpm config get localhost:4873/:_authToken | ||
echo "Testing registry access:" | ||
curl -f http://localhost:4873/ || echo "Verdaccio not accessible" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔑 Printing Verdaccio auth token leaks credentials into public logs
pnpm config get localhost:4873/:_authToken
echoes the token in plain text, making it trivially accessible to anyone who can view the workflow logs.
Suggested fix – mask the value and avoid printing it outright:
- echo "Registry auth token set:"
- pnpm config get localhost:4873/:_authToken
+ echo "::add-mask::$(pnpm config get localhost:4873/:_authToken)"
+ echo "Registry auth token masked (hidden in logs)."
Alternatively, drop the line entirely if not strictly required.
📝 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: Debug registry configuration | |
run: | | |
echo "Current registry:" | |
pnpm config get registry | |
echo "Registry auth token set:" | |
pnpm config get localhost:4873/:_authToken | |
echo "Testing registry access:" | |
curl -f http://localhost:4873/ || echo "Verdaccio not accessible" | |
- name: Debug registry configuration | |
run: | | |
echo "Current registry:" | |
pnpm config get registry | |
- echo "Registry auth token set:" | |
- pnpm config get localhost:4873/:_authToken | |
+ echo "::add-mask::$(pnpm config get localhost:4873/:_authToken)" | |
+ echo "Registry auth token masked (hidden in logs)." | |
echo "Testing registry access:" | |
curl -f http://localhost:4873/ || echo "Verdaccio not accessible" |
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml around lines 46 to 53, the command
printing the Verdaccio auth token exposes sensitive credentials in the public
logs. To fix this, remove or comment out the line that runs `pnpm config get
localhost:4873/:_authToken` to prevent the token from being printed. If the
token value is needed for debugging, replace the direct print with a masked or
obfuscated output instead.
.github/workflows/nightly-checks.yml
Outdated
- name: Edit .npmrc [link-workspace-packages=false] | ||
run: sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' .npmrc | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
sed
will fail (and abort the job) when .npmrc
doesn’t contain the key
If .npmrc
is missing or lacks link-workspace-packages
, GNU sed exits 1, causing the step – and therefore the whole job – to fail.
- run: sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' .npmrc
+ run: |
+ if [ -f .npmrc ]; then
+ sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' .npmrc || true
+ fi
🏁 Script executed:
#!/usr/bin/env bash
set -eo pipefail
# 1) When .npmrc exists but has no matching key
printf 'registry=https://registry.npmjs.org/\n' > tmp.npmrc
sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' tmp.npmrc
echo "Exit code with no match: $?"
# 2) When .npmrc is missing
rm tmp.npmrc
if sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' tmp.npmrc; then
echo "Unexpected success on missing file"
else
echo "Failed on missing file with exit code $?"
fi
Length of output: 474
Ensure .npmrc
exists before running sed
Guard the sed
invocation to avoid a non-zero exit when the file is missing (GNU sed exits with code 2 if the target doesn’t exist):
File: .github/workflows/nightly-checks.yml (lines 43–45)
- run: sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' .npmrc
+ run: |
+ if [ -f .npmrc ]; then
+ sed -i -E 's/link-workspace-packages=(deep|true)/link-workspace-packages=false/' .npmrc
+ fi
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml around lines 43 to 45, the sed command
runs without checking if the .npmrc file exists, causing a non-zero exit if the
file is missing. Fix this by adding a conditional check before running sed to
ensure .npmrc exists, for example using a shell if statement or a test command,
so sed only runs when the file is present.
2c04bad
to
14a9140
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/actions/verdaccio/action.yml (1)
61-73
: Duplicate logic → extract into a reusable helperThe start-poll-configure block is repeated for both publish and install. Repetition increases maintenance cost and risk of divergence.
Consider factoring this into a small shell function (or a separate composite) that accepts the config path and re-uses the same readiness check & PID handling.
This will also guarantee that future fixes (e.g., the failure guard suggested above) apply to both call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/actions/verdaccio/action.yml
(2 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/major-version-check.yml
(1 hunks).github/workflows/nightly-checks.yml
(1 hunks).github/workflows/pr-title-linter.yml
(1 hunks).prettierignore
(0 hunks)
💤 Files with no reviewable changes (1)
- .prettierignore
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/major-version-check.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/nightly-checks.yml
- .github/workflows/ci.yml
- .github/workflows/pr-title-linter.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
.github/actions/verdaccio/action.yml (2)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: Analyze (javascript-typescript)
run: | | ||
if [ -n "$VERDACCIO_PID" ]; then | ||
kill -9 $VERDACCIO_PID || true | ||
sleep 2 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing symmetrical shutdown for the second Verdaccio instance
You stop the first Verdaccio (publish) instance but never terminate the second one started with the install config.
This can leave an orphaned background process in the runner, potentially contaminating parallel jobs.
Add a second “Stop Verdaccio (install)” step or trap the PID so both instances are always cleaned up.
🤖 Prompt for AI Agents
In .github/actions/verdaccio/action.yml around lines 52 to 56, the script stops
the first Verdaccio instance using its PID but does not stop the second
Verdaccio instance started with the install config, risking orphaned processes.
Modify the script to capture and store the PID of the second Verdaccio instance
as well, then add a corresponding kill command to terminate it, ensuring both
instances are properly shut down and cleaned up.
nohup ./node_modules/.bin/verdaccio --config ./verdaccio.publish.yaml & echo "VERDACCIO_PID=$!" >> $GITHUB_ENV | ||
sleep 5 | ||
# Wait for Verdaccio to start and verify it's responding | ||
for i in {1..10}; do | ||
if curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then | ||
echo "Verdaccio is up and running" | ||
break | ||
fi | ||
echo "Waiting for Verdaccio to start (attempt $i)..." | ||
sleep 2 | ||
done | ||
pnpm config set $(echo ${{ inputs.registry }} | sed -E 's/https?://')/:_authToken secretToken |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Add an explicit failure after the readiness-polling loop
If Verdaccio never comes up within the 10 attempts, the script silently proceeds to the publish step and will fail later with a much less obvious error. Abort early to surface the root cause:
for i in {1..10}; do
if curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then
echo "Verdaccio is up and running"
break
fi
echo "Waiting for Verdaccio to start (attempt $i)..."
sleep 2
done
+# Bail out if Verdaccio never became ready
+if ! curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then
+ echo "Verdaccio did not start after 10 attempts" >&2
+ exit 1
+fi
Enforce failure if Verdaccio isn’t ready
Add an explicit check after the readiness-poll loop in .github/actions/verdaccio/action.yml
so the workflow aborts early if Verdaccio never comes up:
for i in {1..10}; do
if curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then
echo "Verdaccio is up and running"
break
fi
echo "Waiting for Verdaccio to start (attempt $i)…"
sleep 2
done
+# Bail out if Verdaccio never became ready
+if ! curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then
+ echo "Verdaccio did not start after 10 attempts" >&2
+ exit 1
+fi
pnpm config set $(echo ${{ inputs.registry }} | sed -E 's/https?://')/:_authToken secretToken
📝 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.
nohup ./node_modules/.bin/verdaccio --config ./verdaccio.publish.yaml & echo "VERDACCIO_PID=$!" >> $GITHUB_ENV | |
sleep 5 | |
# Wait for Verdaccio to start and verify it's responding | |
for i in {1..10}; do | |
if curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then | |
echo "Verdaccio is up and running" | |
break | |
fi | |
echo "Waiting for Verdaccio to start (attempt $i)..." | |
sleep 2 | |
done | |
pnpm config set $(echo ${{ inputs.registry }} | sed -E 's/https?://')/:_authToken secretToken | |
nohup ./node_modules/.bin/verdaccio --config ./verdaccio.publish.yaml & echo "VERDACCIO_PID=$!" >> $GITHUB_ENV | |
# Wait for Verdaccio to start and verify it's responding | |
for i in {1..10}; do | |
if curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then | |
echo "Verdaccio is up and running" | |
break | |
fi | |
echo "Waiting for Verdaccio to start (attempt $i)..." | |
sleep 2 | |
done | |
# Bail out if Verdaccio never became ready | |
if ! curl -f ${{ inputs.registry }}/ > /dev/null 2>&1; then | |
echo "Verdaccio did not start after 10 attempts" >&2 | |
exit 1 | |
fi | |
pnpm config set $(echo ${{ inputs.registry }} | sed -E 's/https?://')/:_authToken secretToken |
🤖 Prompt for AI Agents
In .github/actions/verdaccio/action.yml around lines 34 to 44, after the loop
that waits for Verdaccio to start, add a conditional check to verify if
Verdaccio is actually up. If the service is not responding after all attempts,
exit the script with a non-zero status to fail the workflow early. This ensures
the action does not proceed when Verdaccio is not ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/nightly-checks.yml (3)
3-6
: Consider scoping thepull_request
trigger with apaths
filterRunning the nightly job for every PR that targets
main
orrelease/v4
can double the CI minutes (e.g. docs-only PRs).
If the intent is to verify only e2e-relevant changes, add an include filter:on: pull_request: branches: [main, release/v4] paths: - 'integration/**' - '.github/workflows/nightly-checks.yml'
88-97
:status
vsnotify_when
: the Slack step might silently skip notifications
notify_when: 'failure'
means the action posts only whenstatus
equals'failure'
.
Becausestatus
is computed inline, a typo or non-matching value will drop the message.
Recommend computing once and re-using:- status: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }} - notify_when: 'failure' + env: + JOB_STATUS: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }} + status: ${{ env.JOB_STATUS }} + notify_when: ${{ env.JOB_STATUS }}This guarantees the expression is evaluated once and the action’s filters stay in sync.
100-103
: Redundantcontinue-on-error
/ explicit exitSince the step above already marks the workflow as failed, you could drop
continue-on-error: true
on the test step and skip this explicitexit 1
, simplifying the flow:- - name: Run Integration Tests - continue-on-error: true + - name: Run Integration TestsLess indirection → clearer logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
.github/workflows/nightly-checks.yml (13)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to **/*.{test,spec}.{js,ts,tsx} : Unit tests should use Jest or Vitest as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
71-82
: Artifact upload path may exceed the 2 GB limit
integration/.next/
frequently weighs hundreds of megabytes per matrix run; three failures could exhaust the 2 GB quota and start pruning older artifacts.
Consider zipping the directory or restricting to.next/test-results/**
(or equivalent) instead of the full build.
.github/workflows/nightly-checks.yml
Outdated
id: integration_tests | ||
continue-on-error: true | ||
run: | | ||
# Capture the output and exit code | ||
OUTPUT_FILE="${{runner.temp}}/test-output.log" | ||
pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" | ||
echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT | ||
env: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
PIPESTATUS
relies on Bash – pin the step shell to avoid silent failures
The default shell for run:
on Ubuntu runners is bash, but on self-hosted or future images it could change (e.g. sh
).
Because ${PIPESTATUS[0]}
is Bash-only the expression would expand to an empty string and break downstream logic.
- - name: Run Integration Tests
+ - name: Run Integration Tests
id: integration_tests
continue-on-error: true
+ shell: bash
run: |
# Capture the output and exit code
📝 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.
id: integration_tests | |
continue-on-error: true | |
run: | | |
# Capture the output and exit code | |
OUTPUT_FILE="${{runner.temp}}/test-output.log" | |
pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" | |
echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT | |
env: | |
- name: Run Integration Tests | |
id: integration_tests | |
continue-on-error: true | |
shell: bash | |
run: | | |
# Capture the output and exit code | |
OUTPUT_FILE="${{runner.temp}}/test-output.log" | |
pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" | |
echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT | |
env: |
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml around lines 54 to 61, the use of
${PIPESTATUS[0]} relies on Bash-specific behavior which may not be available if
the default shell changes. To fix this, explicitly set the shell for this step
to bash by adding a shell: bash directive to ensure the PIPESTATUS array is
supported and the exit code is captured correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/nightly-checks.yml (1)
70-76
: Explicitly pin the step shell to Bash to keep${PIPESTATUS[0]}
reliable
PIPESTATUS
is a Bash-only array; if the runner default shell ever changes (self-hosted image, future GH update) this expansion becomes empty and the exit code logic breaks.
Addshell: bash
to the step so the script always runs in Bash.- - name: Run Integration Tests + - name: Run Integration Tests id: integration_tests continue-on-error: true + shell: bash # guarantees PIPESTATUS support run: | # Capture the output and exit code OUTPUT_FILE="${{runner.temp}}/test-output.log"
🧹 Nitpick comments (1)
.github/workflows/nightly-checks.yml (1)
88-98
: Fail-safe upload: ignore missing paths to avoid false negativesIf a directory (e.g.
integration/.next/
) is not generated before failure,actions/upload-artifact
errors and marks the step failed, overshadowing the real test failure context.
Addif-no-files-found: ignore
so uploads are best-effort.- name: Upload Test Artifacts if: steps.integration_tests.outputs.exit_code != '0' uses: actions/upload-artifact@v4 with: name: test-artifacts-${{ matrix.test-name }} path: | ${{runner.temp}}/test-output.log integration/test-results/ integration/.next/ ${{runner.temp}}/clerk-js/node_modules/ + if-no-files-found: ignore retention-days: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (14)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: Analyze (javascript-typescript)
working-directory: ${{runner.temp}} | ||
run: mkdir clerk-js && cd clerk-js && pnpm init && pnpm add @clerk/clerk-js | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pnpm init
is interactive – CI will hang
pnpm init
prompts for package-json answers unless --yes
(or -y
) is supplied.
On GitHub runners this blocks the job indefinitely.
-run: mkdir clerk-js && cd clerk-js && pnpm init && pnpm add @clerk/clerk-js
+run: mkdir clerk-js && cd clerk-js && pnpm init -y && pnpm add @clerk/clerk-js
📝 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.
working-directory: ${{runner.temp}} | |
run: mkdir clerk-js && cd clerk-js && pnpm init && pnpm add @clerk/clerk-js | |
working-directory: ${{ runner.temp }} | |
run: mkdir clerk-js && cd clerk-js && pnpm init -y && pnpm add @clerk/clerk-js |
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml at lines 58 to 60, the command `pnpm
init` is used without the `--yes` flag, causing it to run interactively and hang
the CI job. Modify the command to include `--yes` (or `-y`) so it runs
non-interactively, for example, change `pnpm init` to `pnpm init --yes` to
prevent the job from blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/nightly-checks.yml (3)
57-60
:pnpm init
is interactive – CI will hang
Identical issue flagged earlier: add-y
(--yes
) to run non-interactively.
61-67
: Same verbosity caveat as above
Listing the full tree a second time further increases log size; see preceding comment for a toggle pattern.
70-81
:${PIPESTATUS[0]}
requires Bash – pin the step shell
Withoutshell: bash
, this array is undefined onsh
-based runners, causingexit_code
to be blank and logic depending on it to mis-fire.
🧹 Nitpick comments (2)
.github/workflows/nightly-checks.yml (2)
49-56
: Consider gating verbose package listings behind a debug flag
pnpm list --json
dumps the entire dependency tree, which can easily bloat logs and breach the 4 MB per-step limit on larger workspaces.Add an environment toggle, e.g.
-if: always() +if: env.CI_DEBUG == 'true'and set
CI_DEBUG
only on demand.
109-117
: Hard-coded “Failure” in Slack title even on success
notification_title: 'Integration Test Failure - …'
fires for both outcomes because the action only sends on failure, yet a future config tweak could widennotify_when
. Make the title conditional or neutral (“Integration Test Result”).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (17)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
20-21
: 👍 Good call disablingfail-fast
for the matrix
Prevents an early-failing variant from cancelling the rest and retains full signal from the nightly run.
c0df9ad
to
ad108d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/nightly-checks.yml (2)
70-81
: Explicitly pin the step to Bash to guarantee PIPESTATUS worksThe script relies on the Bash-specific
${PIPESTATUS[0]}
array, but the step does not declareshell: bash
.
If the default shell ever switches (e.g. tosh
on self-hosted runners), this will silently break andexit_code
will be empty.- name: Run Integration Tests id: integration_tests continue-on-error: true + shell: bash run: |
45-48
: Makepnpm init
non-interactive to avoid CI hangs
pnpm init
prompts for package-json fields unless--yes/-y
is supplied.
On GitHub runners this blocks the job indefinitely.- run: pnpm init && pnpm add @clerk/backend + run: pnpm init -y && pnpm add @clerk/backend- run: mkdir clerk-js && cd clerk-js && pnpm init && pnpm add @clerk/clerk-js + run: mkdir clerk-js && cd clerk-js && pnpm init -y && pnpm add @clerk/clerk-jsAlso applies to: 57-60
🧹 Nitpick comments (1)
.github/workflows/nightly-checks.yml (1)
49-56
: Trimpnpm list
output to keep logs readable
pnpm list --json
prints the entire dependency tree, which can be thousands of lines and slows down log loading.
Unless deep trees are required for debugging, consider limiting depth:- pnpm list --json + pnpm list --depth 1 --jsonSame for the second listing block.
Also applies to: 61-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (17)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/nightly-checks.yml (2)
62-73
:PIPESTATUS
requires Bash – still missingshell: bash
Same concern as previously raised; without pinning the shell this will break on runners where the default issh
.
57-60
:pnpm init
is interactive – CI will hangAdd the non-interactive flag to prevent the job from blocking:
-run: mkdir clerk-js && cd clerk-js && pnpm init && pnpm add @clerk/clerk-js +run: mkdir clerk-js && cd clerk-js && pnpm init -y && pnpm add @clerk/clerk-js
🧹 Nitpick comments (3)
.github/workflows/nightly-checks.yml (3)
20-21
:fail-fast: false
has no effect with a single-item matrixThe matrix currently defines only one
test-name
. Unless additional entries will be added soon, this directive is a no-op and can be removed to reduce noise.
49-55
: Log spam risk – limitpnpm list
output depth
pnpm list --json
emits the full dependency tree, which can be thousands of lines and overwhelm workflow logs.
Consider:- pnpm list --json + pnpm list --depth 0 --jsonThis keeps diagnostics readable while still proving the installed versions.
101-109
: Slack title hard-codes “Failure” even on successBecause the step runs under
if: always()
, successful runs will still post a message titled Integration Test Failure. Use a dynamic title or conditionally skip on success:- notification_title: 'Integration Test Failure - ${{ matrix.test-name }}' + notification_title: '${{ steps.integration_tests.outputs.exit_code == \'0\' && \'Integration Test Success\' || \'Integration Test Failure\' }} - ${{ matrix.test-name }}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (18)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
3-6
: Validate necessity of running nightly workflow on every PR tomain
/release/v4
Triggering this resource-heavy job on all pull requests targeting the main release branches can slow CI and consume minutes unnecessarily.
Confirm this is intentional—otherwise consider gating on a label, path filter, or dedicated workflow.
3031990
to
e8a7e34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
.github/workflows/nightly-checks.yml (1)
45-48
:pnpm init
is still interactive – add--yes
to avoid CI hangsRunning
pnpm init
without the--yes
flag prompts for user input and will block the workflow.
Same issue as in theclerk-js
step, but this path (/integration
) hasn’t been addressed yet.- run: pnpm init && pnpm add @clerk/backend + run: pnpm init -y && pnpm add @clerk/backend
♻️ Duplicate comments (1)
.github/workflows/nightly-checks.yml (1)
58-60
: Interactivepnpm init
– duplicate of previously-raised issue
See earlier comment; the clerk-js install still needs-y
.
🧹 Nitpick comments (1)
.github/workflows/nightly-checks.yml (1)
98-110
: Artifact upload guard misses failures from the 2nd test stepThe condition
steps.integration_tests.outputs.exit_code != '0'
ignores the result ofintegration_tests2
.
If the first step passes and the second fails, artifacts won’t be uploaded. Consider aggregating both exit codes or basing the guard onfailure()
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/actions/verdaccio/action.yml
(2 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/major-version-check.yml
(1 hunks).github/workflows/nightly-checks.yml
(4 hunks).github/workflows/pr-title-linter.yml
(1 hunks).prettierignore
(0 hunks)integration/models/longRunningApplication.ts
(2 hunks)integration/playwright.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/major-version-check.yml
- .github/workflows/pr-title-linter.yml
- integration/playwright.config.ts
- integration/models/longRunningApplication.ts
- .github/workflows/ci.yml
- .github/actions/verdaccio/action.yml
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (17)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
.github/workflows/nightly-checks.yml
Outdated
- name: Run Integration Tests | ||
run: pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only | ||
id: integration_tests | ||
continue-on-error: true | ||
run: | | ||
# Capture the output and exit code | ||
OUTPUT_FILE="${{runner.temp}}/test-output.log" | ||
# Only run Typedoc tests for one matrix version | ||
if [ "${{ matrix.test-name }}" == "nextjs" ]; then | ||
E2E_APP_ID=quickstart.next.appRouter pnpm test:integration:base --grep @quickstart 2>&1 | tee "$OUTPUT_FILE" | ||
else | ||
pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" | ||
echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broken shell script: missing fi
and Bash-specific $PIPESTATUS
without pinning the shell
- The
if … else
block never closes – there is nofi
, so the step fails immediately. ${PIPESTATUS[0]}
only works in Bash; the step isn’t pinned toshell: bash
.
- run: |
+ shell: bash
+ run: |
# Capture the output and exit code
OUTPUT_FILE="${{runner.temp}}/test-output.log"
# Only run Typedoc tests for one matrix version
if [ "${{ matrix.test-name }}" == "nextjs" ]; then
E2E_APP_ID=quickstart.next.appRouter pnpm test:integration:base --grep @quickstart 2>&1 | tee "$OUTPUT_FILE"
else
pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE"
- echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT
+ fi
+ echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml around lines 61 to 72, the shell script
is missing a closing 'fi' for the if-else block, causing a syntax error, and it
uses the Bash-specific variable '${PIPESTATUS[0]}' without specifying the shell
as Bash. To fix this, add the missing 'fi' after the else block to properly
close the if statement, and set the step's shell to 'bash' by adding 'shell:
bash' to ensure '${PIPESTATUS[0]}' works correctly.
e8a7e34
to
66aae0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/nightly-checks.yml (3)
45-48
:pnpm init
is still interactive – CI will block
Earlier feedback highlighted thatpnpm init
prompts for input unless-y/--yes
is supplied. The command here hasn’t been updated, so the job will hang on the first package prompt.- run: pnpm init && pnpm add @clerk/backend + run: pnpm init -y && pnpm add @clerk/backend
61-74
: Bash-specific${PIPESTATUS[0]}
without pinning the shell
The default runner shell is currently Bash, but that isn’t guaranteed.${PIPESTATUS}
is Bash-only, so this step will silently break if the default changes or a self-hosted runner usessh
.
Addshell: bash
(or migrate to a POSIX-portable approach).- - name: Run Integration Tests + - name: Run Integration Tests id: integration_tests continue-on-error: true + shell: bash run: | # Capture the output and exit code OUTPUT_FILE="${{runner.temp}}/test-output.log"
84-88
:$OUTPUT_FILE
reused but never (re)defined
This second test step references$OUTPUT_FILE
, yet the variable is only set in the previous step. When the two steps run in separate shells, the variable is empty, sotee
writes only to stdout and the artifact upload can’t find the log.Either remove this duplicate run or define the variable:
- run: | - E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" + shell: bash + run: | + OUTPUT_FILE="${{runner.temp}}/test-output-2.log" + E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" + echo "exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT
🧹 Nitpick comments (1)
.github/workflows/nightly-checks.yml (1)
100-110
: Large artifact upload may exceed capacity & fail silently
Uploading a full Next.js.next/
build and an entirenode_modules
directory can easily exceed the 2 GB artifact limit, causing the step to error out and lose logs. Consider pruning these directories (e.g. only.next/.vercel/output
or specific log sub-folders) or zipping + excluding heavy binaries before upload.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (18)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
⏰ 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: semgrep/ci
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/nightly-checks.yml (1)
46-49
:pnpm init
will block the job – make it non-interactiveRunning
pnpm init
without-y/--yes
prompts for input and causes CI to hang.- run: pnpm init && pnpm add @clerk/backend + run: pnpm init -y && pnpm add @clerk/backend
♻️ Duplicate comments (3)
.github/workflows/nightly-checks.yml (3)
58-61
: Interactivepnpm init
persists
Same issue as previously flagged – please add-y
.
63-75
: Bash-specific${PIPESTATUS[0]}
withoutshell: bash
${PIPESTATUS[@]}
only exists in Bash. Pin the step shell to avoid silent failures on environments where/bin/sh
≠ Bash.- - name: Run Integration Tests + - name: Run Integration Tests id: integration_tests continue-on-error: true + shell: bash run: | # Capture the output and exit code
85-90
:OUTPUT_FILE
is referenced but never defined
tee "$OUTPUT_FILE"
will write to an empty path, producing “No such file or directory” and losing the logs.- E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" + OUTPUT_FILE="${{runner.temp}}/test-output-${{ matrix.test-name }}.log" + E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nightly-checks.yml
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (18)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
🪛 actionlint (1.7.7)
.github/workflows/nightly-checks.yml
14-14: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "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", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
14-15
: Unknown runner label – verify self-hosted availability
blacksmith-8vcpu-ubuntu-2204
is not one of GitHub’s default labels (see action-lint warning).
If this is meant to be a self-hosted runner, make sure it is registered in the organisation and online; otherwise the workflow will stay queued forever.
status: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }} | ||
notify_when: 'failure' | ||
notification_title: 'Integration Test Failure - ${{ matrix.test-name }}' | ||
message_format: | | ||
*Job:* ${{ github.workflow }} (${{ matrix.test-name }}) | ||
*Status:* ${{ steps.integration_tests.outputs.exit_code == '0' && 'Success' || 'Failed' }} | ||
*Commit:* ${{ github.sha }} | ||
*PR:* ${{ github.event.pull_request.html_url }} | ||
*Artifacts:* ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Slack notification ignores second test run – potential false green
status:
and message fields only look at integration_tests.exit_code
.
If the first run passes and the second fails, the workflow reports success.
Consider aggregating both steps, or emit a combined output and reference that:
- status: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }}
+ status: ${{ (steps.integration_tests.outputs.exit_code == '0' && steps.integration_tests2.outputs.exit_code == '0') && 'success' || 'failure' }}
📝 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.
status: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }} | |
notify_when: 'failure' | |
notification_title: 'Integration Test Failure - ${{ matrix.test-name }}' | |
message_format: | | |
*Job:* ${{ github.workflow }} (${{ matrix.test-name }}) | |
*Status:* ${{ steps.integration_tests.outputs.exit_code == '0' && 'Success' || 'Failed' }} | |
*Commit:* ${{ github.sha }} | |
*PR:* ${{ github.event.pull_request.html_url }} | |
*Artifacts:* ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | |
status: ${{ (steps.integration_tests.outputs.exit_code == '0' && steps.integration_tests2.outputs.exit_code == '0') && 'success' || 'failure' }} | |
notify_when: 'failure' | |
notification_title: 'Integration Test Failure - ${{ matrix.test-name }}' | |
message_format: | | |
*Job:* ${{ github.workflow }} (${{ matrix.test-name }}) | |
*Status:* ${{ steps.integration_tests.outputs.exit_code == '0' && 'Success' || 'Failed' }} | |
*Commit:* ${{ github.sha }} | |
*PR:* ${{ github.event.pull_request.html_url }} | |
*Artifacts:* ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} |
🤖 Prompt for AI Agents
In .github/workflows/nightly-checks.yml around lines 117 to 125, the Slack
notification only checks the exit code of the integration_tests step, ignoring
the second test run, which can cause false success reports. Modify the workflow
to aggregate the exit codes of both test runs into a combined output or status
variable, then update the notification fields to reference this combined result
to accurately reflect the overall test outcome.
eed3ea4
to
f9712d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/nightly-checks.yml (3)
85-89
:$OUTPUT_FILE
is undefined in second run step (repeat of earlier feedback)The variable is initialised only in the first test step, so
tee "$OUTPUT_FILE"
writes to stdout only and the artifact upload may miss logs.- run: | - E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE" + run: | + OUTPUT_FILE="${{runner.temp}}/test-output.log" + E2E_DEBUG=1 pnpm turbo test:integration:${{ matrix.test-name }} $TURBO_ARGS --only 2>&1 | tee "$OUTPUT_FILE"
117-125
: Slack notification & failure gate ignoreintegration_tests2
(repeat of earlier feedback)Only the first test run’s exit code is considered for Slack status and the final
Check Test Status
step, so a failure in the second run can report green.Aggregate both exit codes, e.g.:
- status: ${{ steps.integration_tests.outputs.exit_code == '0' && 'success' || 'failure' }} + status: ${{ (steps.integration_tests.outputs.exit_code == '0' && steps.integration_tests2.outputs.exit_code == '0') && 'success' || 'failure' }}…and mirror the same check in the final gate.
131-132
: Final gate should also includeintegration_tests2
exit code-if: steps.integration_tests.outputs.exit_code != '0' +if: (steps.integration_tests.outputs.exit_code != '0') || (steps.integration_tests2.outputs.exit_code != '0')
🧹 Nitpick comments (2)
.github/workflows/nightly-checks.yml (2)
50-56
: Diagnostics step useful for debugging but increases CI timeListing the full dependency tree on every nightly run adds ~4-5 s and a few hundred log lines.
Consider wrapping this inif: failure()
or guarding with a${{ env.DEBUG }}
flag so it only executes when you actually need the info.
62-75
: Pin shell to bash to guarantee${PIPESTATUS[0]}
support
${PIPESTATUS[@]}
is a Bash-specific array. While the default Linux shell is currently Bash, pinning avoids silent breakage on future image changes or alternative runners.- - name: Run Integration Tests + - name: Run Integration Tests + shell: bash id: integration_tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/actions/verdaccio/action.yml
(2 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/major-version-check.yml
(1 hunks).github/workflows/nightly-checks.yml
(3 hunks).github/workflows/pr-title-linter.yml
(1 hunks).prettierignore
(0 hunks)integration/models/longRunningApplication.ts
(2 hunks)integration/playwright.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/major-version-check.yml
- .github/workflows/pr-title-linter.yml
- .github/workflows/ci.yml
- integration/playwright.config.ts
- integration/models/longRunningApplication.ts
- .github/actions/verdaccio/action.yml
🧰 Additional context used
📓 Path-based instructions (1)
`.github/workflows/*.yml`: CI/CD must be set up with GitHub Actions.
.github/workflows/*.yml
: CI/CD must be set up with GitHub Actions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
.github/workflows/nightly-checks.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
.github/workflows/nightly-checks.yml (17)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Never commit sensitive keys or credentials
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to {pnpm-lock.yaml,turbo.json} : Use pnpm as the package manager and Turbo for build orchestration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm as the package manager for all dependency management and scripts
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test:integration:* to run framework-specific integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Framework integration templates and E2E tests must be located under the integration/ directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/* : End-to-end tests and integration templates must be located in the 'integration/' directory.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Use pnpm test to run unit tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Each framework integration has its own test suite
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Ensure all CI checks pass before merging
🪛 actionlint (1.7.7)
.github/workflows/nightly-checks.yml
14-14: label "blacksmith-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "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", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "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)
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Integration Tests (nextjs)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/nightly-checks.yml (1)
14-15
: Verify custom runner label is registered
blacksmith-8vcpu-ubuntu-2204
isn’t a standard GitHub-hosted label (action-lint warning).
If this is a self-hosted runner make sure it’s online in all required environments; otherwise the job will queue forever.
chore(ci): Update nightly checks and integration test configurations
…ate references in longRunningApps and tests
7e6a92d
to
0d99589
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/actions/verdaccio/action.yml
(2 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/major-version-check.yml
(1 hunks).github/workflows/nightly-checks.yml
(3 hunks).github/workflows/pr-title-linter.yml
(1 hunks).prettierignore
(0 hunks)integration/models/applicationConfig.ts
(1 hunks)integration/models/longRunningApplication.ts
(2 hunks)integration/models/stateFile.ts
(3 hunks)integration/presets/envs.ts
(2 hunks)integration/presets/longRunningApps.ts
(2 hunks)integration/templates/next-app-router/next.config.js
(0 hunks)integration/tests/protect-jwt-v2.test.ts
(1 hunks)integration/tests/protect.test.ts
(1 hunks)
💤 Files with no reviewable changes (2)
- integration/templates/next-app-router/next.config.js
- .prettierignore
✅ Files skipped from review due to trivial changes (3)
- integration/tests/protect.test.ts
- integration/models/applicationConfig.ts
- integration/presets/envs.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/major-version-check.yml
- .github/workflows/ci.yml
- .github/workflows/pr-title-linter.yml
- integration/models/longRunningApplication.ts
- .github/workflows/nightly-checks.yml
- .github/actions/verdaccio/action.yml
🧰 Additional context used
📓 Path-based instructions (7)
`**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's conf...
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Use Prettier for consistent code formatting
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
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Validate all inputs and sanitize outputs
Implement proper logging with different levels
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
integration/models/stateFile.ts
integration/presets/longRunningApps.ts
`**/*.{ts,tsx}`: Use proper TypeScript error types
**/*.{ts,tsx}
: Use proper TypeScript error types
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
integration/models/stateFile.ts
integration/presets/longRunningApps.ts
`integration/**/*`: End-to-end tests and integration templates must be located in the 'integration/' directory.
integration/**/*
: End-to-end tests and integration templates must be located in the 'integration/' directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
integration/models/stateFile.ts
integration/presets/longRunningApps.ts
`**/*.{test,spec}.{js,ts,tsx}`: Unit tests should use Jest or Vitest as the test runner.
**/*.{test,spec}.{js,ts,tsx}
: Unit tests should use Jest or Vitest as the test runner.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
`integration/**/*.{test,spec}.{js,ts,tsx}`: Integration tests should use Playwright as the test runner.
integration/**/*.{test,spec}.{js,ts,tsx}
: Integration tests should use Playwright as the test runner.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
`**/*.{ts,tsx}`: Always define explicit return types for functions, especially p...
**/*.{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
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
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
Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Define custom error classes for domain-specific errors
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining and nullish coalescing for safe property access
Let TypeScript infer types when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use readonly arrays and objects for immutability
Use immutable update patterns (spread, etc.) for objects and arrays
Use lazy loading for large types
Preferunknown
overany
for performance
Use type-only imports:import type { ... }
Use branded types for domain safety
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints in TypeScript generics
No unused type parameters in generics
Proper use of utility types instead of manual type construction
Type-only imports where possible for performance
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
integration/models/stateFile.ts
integration/presets/longRunningApps.ts
`**/*.ts`: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
**/*.ts
: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
integration/tests/protect-jwt-v2.test.ts
integration/models/stateFile.ts
integration/presets/longRunningApps.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .github/workflows/*.yml : CI/CD must be set up with GitHub Actions.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to verdaccio.yaml : Verdaccio must be used for local npm registry testing.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Verdaccio for local npm registry testing
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use immutable update patterns (spread, etc.) for objects and arrays
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Implement ISR (Incremental Static Regeneration) with revalidate option in Pages Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Test component interactions
integration/tests/protect-jwt-v2.test.ts (16)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use branded types for test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use Vitest for type-safe testing in TypeScript
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Create type-safe test builders/factories
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to **/*.{test,spec}.{js,ts,tsx} : Unit tests should use Jest or Vitest as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Implement proper mock types that match interfaces in tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test setup
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test data
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test assertions
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test queries
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Test component interactions
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Test component behavior, not implementation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test cleanup
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Jest for unit testing, Vitest for some packages
integration/models/stateFile.ts (10)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type definitions for props and state
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper state naming
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use `public` explicitly for clarity in public APIs
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper state normalization
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use branded types for test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper use of utility types instead of manual type construction
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use proper type annotations for variables and parameters where inference isn't clear
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper state persistence
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/api/**/*.ts : Document API endpoints with TypeScript interfaces
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper tree-shaking friendly exports
integration/presets/longRunningApps.ts (14)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use immutable update patterns (spread, etc.) for objects and arrays
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper tree-shaking friendly exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to packages/**/index.{js,ts} : Use tree-shaking friendly exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : No circular dependencies
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper state normalization
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use efficient data structures and algorithms
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Maintain comprehensive JSDoc comments for public APIs
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use ES6 imports/exports consistently
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Export components as named exports for better tree-shaking
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Create type-safe test builders/factories
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test data
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test setup
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/[[]*[]]/**/page.tsx : Use square brackets for dynamic segments: [id]/page.tsx
🧬 Code Graph Analysis (1)
integration/tests/protect-jwt-v2.test.ts (1)
integration/presets/index.ts (1)
appConfigs
(14-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
integration/models/stateFile.ts (1)
5-12
: LGTM - Type export improves reusability.Exporting the
AppParams
type allows other modules to reference it, improving type safety across the integration framework.integration/tests/protect-jwt-v2.test.ts (1)
9-9
: LGTM - Environment preset rename is consistent.The change from
withBillingStaging
towithBillingJwtV2
aligns with the broader environment preset renaming across the codebase.integration/presets/longRunningApps.ts (1)
20-75
: LGTM - Improved code organization and readability.The categorization with descriptive comments and the
prettier-ignore
directive significantly improve code readability and maintainability. The reorganization into logical groups (NextJS basic flows, Quickstart apps, Billing apps, etc.) makes the configuration easier to navigate.
const debug = () => { | ||
const json = read(); | ||
console.log('state file', JSON.stringify(json, null, 2)); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add explicit return type and JSDoc documentation.
The debug
method is missing an explicit return type and JSDoc documentation as required for public APIs.
+ /**
+ * Logs the current state file content in a pretty-printed format for debugging purposes.
+ */
- const debug = () => {
+ const debug = (): void => {
const json = read();
console.log('state file', JSON.stringify(json, null, 2));
};
📝 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.
const debug = () => { | |
const json = read(); | |
console.log('state file', JSON.stringify(json, null, 2)); | |
}; | |
/** | |
* Logs the current state file content in a pretty-printed format for debugging purposes. | |
*/ | |
const debug = (): void => { | |
const json = read(); | |
console.log('state file', JSON.stringify(json, null, 2)); | |
}; |
🤖 Prompt for AI Agents
In integration/models/stateFile.ts around lines 86 to 89, the debug function
lacks an explicit return type and JSDoc documentation. Add a JSDoc comment above
the debug function describing its purpose and specify that it returns void.
Also, explicitly declare the return type of the debug function as void in its
signature.
const idMatchesPattern = (id: string, pattern: string) => { | ||
if (pattern.includes('*')) { | ||
const [idStart, _] = pattern.split('*'); | ||
return id.startsWith(idStart); | ||
if (!pattern.includes('*')) { | ||
return id === pattern; | ||
} | ||
return id === pattern; | ||
// Convert glob pattern to regex by escaping special regex chars and replacing * with .* | ||
const escapedPattern = pattern | ||
.replace(/[.+?^${}()|[\]\\]/g, '\\$&') // Escape regex special chars except * | ||
.replace(/\*/g, '.*'); // Replace * with .* for regex matching | ||
return new RegExp(`^${escapedPattern}$`).test(id); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add JSDoc documentation and consider performance optimization.
The refactored idMatchesPattern
function now supports full wildcard matching, which is more flexible than the previous implementation. However, it needs proper documentation and could benefit from performance optimization.
+/**
+ * Checks if an application ID matches a given pattern with wildcard support.
+ * @param id - The application ID to test
+ * @param pattern - The pattern to match against (supports '*' wildcards)
+ * @returns True if the ID matches the pattern
+ * @example
+ * idMatchesPattern('react.vite.withEmailCodes', 'react.vite.*') // true
+ * idMatchesPattern('react.vite.withEmailCodes', '*.withEmailCodes') // true
+ */
-const idMatchesPattern = (id: string, pattern: string) => {
+const idMatchesPattern = (id: string, pattern: string): boolean => {
if (!pattern.includes('*')) {
return id === pattern;
}
- // Convert glob pattern to regex by escaping special regex chars and replacing * with .*
- const escapedPattern = pattern
- .replace(/[.+?^${}()|[\]\\]/g, '\\$&') // Escape regex special chars except *
- .replace(/\*/g, '.*'); // Replace * with .* for regex matching
- return new RegExp(`^${escapedPattern}$`).test(id);
+ // Cache regex patterns for better performance
+ const escapedPattern = pattern
+ .replace(/[.+?^${}()|[\]\\]/g, '\\$&')
+ .replace(/\*/g, '.*');
+ return new RegExp(`^${escapedPattern}$`).test(id);
};
Consider implementing a pattern cache if this function is called frequently with the same patterns.
📝 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.
const idMatchesPattern = (id: string, pattern: string) => { | |
if (pattern.includes('*')) { | |
const [idStart, _] = pattern.split('*'); | |
return id.startsWith(idStart); | |
if (!pattern.includes('*')) { | |
return id === pattern; | |
} | |
return id === pattern; | |
// Convert glob pattern to regex by escaping special regex chars and replacing * with .* | |
const escapedPattern = pattern | |
.replace(/[.+?^${}()|[\]\\]/g, '\\$&') // Escape regex special chars except * | |
.replace(/\*/g, '.*'); // Replace * with .* for regex matching | |
return new RegExp(`^${escapedPattern}$`).test(id); | |
}; | |
/** | |
* Checks if an application ID matches a given pattern with wildcard support. | |
* @param id - The application ID to test | |
* @param pattern - The pattern to match against (supports '*' wildcards) | |
* @returns True if the ID matches the pattern | |
* @example | |
* idMatchesPattern('react.vite.withEmailCodes', 'react.vite.*') // true | |
* idMatchesPattern('react.vite.withEmailCodes', '*.withEmailCodes') // true | |
*/ | |
const idMatchesPattern = (id: string, pattern: string): boolean => { | |
if (!pattern.includes('*')) { | |
return id === pattern; | |
} | |
// Cache regex patterns for better performance | |
const escapedPattern = pattern | |
.replace(/[.+?^${}()|[\]\\]/g, '\\$&') | |
.replace(/\*/g, '.*'); | |
return new RegExp(`^${escapedPattern}$`).test(id); | |
}; |
🤖 Prompt for AI Agents
In integration/presets/longRunningApps.ts around lines 113 to 122, add JSDoc
comments to the idMatchesPattern function explaining its purpose, parameters,
and return value. To optimize performance, implement a cache to store compiled
regex patterns keyed by the pattern string, so repeated calls with the same
pattern reuse the regex instead of recompiling it each time.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
Description
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores