feat(e2e): add E2E_STAGING flag for staging env auto-swap#8060
Open
jacekradko wants to merge 17 commits intomainfrom
Open
feat(e2e): add E2E_STAGING flag for staging env auto-swap#8060jacekradko wants to merge 17 commits intomainfrom
jacekradko wants to merge 17 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: ed9dc5f The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds staging support across integration tests: CI workflow matrix includes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-staging.yml:
- Around line 89-90: The branch allowlist currently permits personal branches
via the regex check on REF (^(main|release/.*|jacek/.*)$); remove the
personal-branch entry by changing the regex to only allow main and release/*
(e.g. ^(main|release/.*)$) and update the error message that references allowed
refs so it matches the tightened allowlist.
In `@integration/presets/longRunningApps.ts`:
- Around line 22-23: The current logic around configs collapses invalid app
patterns and legitimately empty staging keys into the same empty-array return
when E2E_STAGING is set, hiding typos in E2E_APP_ID; change the flow so you
first build and keep an unfiltered configs list (the initial configs array) for
validation, then apply isStagingReady as a separate filter only for deciding
which configs to run, and return [] only when the requested pattern matches a
known app that was filtered out by isStagingReady; adjust the function that
currently returns [] (the block referencing E2E_STAGING/E2E_APP_ID around the
configs handling) to validate pattern existence against the unfiltered list and
only treat missing staging keys as a filtered-out case, and add a regression
test that distinguishes “missing staging key” from “invalid pattern” (i.e., test
both the stale/typo E2E_APP_ID case and the known-app-without-staging-keys case)
before merging.
In `@scripts/1password-keys.mjs`:
- Around line 62-67: The fallback branch that runs when stagingKeysItem is falsy
should remove any existing integration/.keys.staging.json so stale staging keys
aren't reused; update the else path that currently only logs to attempt to
delete the file (e.g., using fs.unlink or fs.rm wrapped in a try/catch or check
with fs.existsSync) for the target path produced by join(process.cwd(),
'integration', '.keys.staging.json'), and only suppress errors if the file
doesn't exist while still logging removal failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbce25c2-375f-4d13-88c2-75cbe289178a
📒 Files selected for processing (8)
.github/workflows/e2e-staging.yml.gitignoreintegration/constants.tsintegration/presets/envs.tsintegration/presets/longRunningApps.tspackage.jsonscripts/1password-keys.mjsturbo.json
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
jacekradko
added a commit
that referenced
this pull request
Mar 13, 2026
- Separate invalid pattern detection from staging-filtered apps in getByPattern - Delete stale .keys.staging.json when 1Password staging keys are missing - Add regression tests for getByPattern staging behavior
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/presets/longRunningApps.ts`:
- Around line 92-93: The filter call that builds stagingReadyConfigs calls
isStagingReady(c.env) without ensuring c.env exists, so guard the env before
calling it: change the predicate used on allConfigs (the arrow passed to filter
that calls isStagingReady) to first check c.env (or c.env != null) and only call
isStagingReady(c.env) when present; update the stagingReadyConfigs assignment
that currently uses allConfigs.filter(...) and ensure longRunningApplication
only receives configs with a valid env to prevent runtime crashes when
env.privateVariables is accessed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30cb8655-4310-4ebe-aaaf-8d6e59a4a069
📒 Files selected for processing (4)
integration/presets/__tests__/longRunningApps.test.tsintegration/presets/longRunningApps.tsintegration/vitest.config.mtsscripts/1password-keys.mjs
jacekradko
changed the title
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
integration/presets/longRunningApps.ts (1)
99-100:⚠️ Potential issue | 🔴 CriticalGuard
envbeforeisStagingReadyto avoid staging-mode runtime crashes.At Line 99,
isStagingReady(c.env)can receive a missing env config in staging mode, andisStagingReadydereferencesenv.privateVariables. This turns a “skip missing staging keys” case into a hard failure.Suggested fix
- const stagingReadyConfigs = allConfigs.filter(c => isStagingReady(c.env)); + const stagingReadyConfigs = allConfigs.filter(c => c.env != null && isStagingReady(c.env)); const apps = stagingReadyConfigs.map(longRunningApplication);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration/presets/longRunningApps.ts` around lines 99 - 100, stagingReadyConfigs currently calls isStagingReady(c.env) without ensuring c.env exists, which can cause crashes when env or env.privateVariables is missing; update the filter to first guard that c.env (and/or c.env.privateVariables if appropriate) is defined before calling isStagingReady, e.g. change the predicate used when building stagingReadyConfigs (the usage of allConfigs.filter and symbol isStagingReady) to check c.env presence, and ensure longRunningApplication is only mapped over configs that passed that guard..github/workflows/e2e-staging.yml (1)
89-90:⚠️ Potential issue | 🔴 CriticalRemove the personal-branch allowlist before merge.
Line 89 still permits
jacek/*in a workflow that later injectsINTEGRATION_*, certificate, Turbo, and Slack secrets. Git refs are not user-scoped, so any writer can create a matching branch and run unreviewed code with those secrets; tighten this back tomainandrelease/*, and update Line 90 to match.Suggested fix
- if [[ ! "$REF" =~ ^(main|release/.*|jacek/.*)$ ]]; then - echo "::error::Ref '$REF' is not allowed. Only 'main', 'release/*', and 'jacek/*' branches are permitted." + if [[ ! "$REF" =~ ^(main|release/.*)$ ]]; then + echo "::error::Ref '$REF' is not allowed. Only 'main' and 'release/*' branches are permitted."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-staging.yml around lines 89 - 90, The workflow currently allows branches matching jacek/* in the REF check; remove that pattern from the conditional that evaluates REF (the if [[ ! "$REF" =~ ^(main|release/.*|jacek/.*)$ ]] test) so it only permits main and release/*, and update the accompanying echo message (the echo "::error::Ref '$REF' is not allowed..." string) to reflect only 'main' and 'release/*' as allowed branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/e2e-staging.yml:
- Around line 89-90: The workflow currently allows branches matching jacek/* in
the REF check; remove that pattern from the conditional that evaluates REF (the
if [[ ! "$REF" =~ ^(main|release/.*|jacek/.*)$ ]] test) so it only permits main
and release/*, and update the accompanying echo message (the echo "::error::Ref
'$REF' is not allowed..." string) to reflect only 'main' and 'release/*' as
allowed branches.
In `@integration/presets/longRunningApps.ts`:
- Around line 99-100: stagingReadyConfigs currently calls isStagingReady(c.env)
without ensuring c.env exists, which can cause crashes when env or
env.privateVariables is missing; update the filter to first guard that c.env
(and/or c.env.privateVariables if appropriate) is defined before calling
isStagingReady, e.g. change the predicate used when building stagingReadyConfigs
(the usage of allConfigs.filter and symbol isStagingReady) to check c.env
presence, and ensure longRunningApplication is only mapped over configs that
passed that guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: e936aa13-0158-4d7a-84d5-e6146d1a0120
📒 Files selected for processing (3)
.github/workflows/e2e-staging.ymlintegration/presets/longRunningApps.tspackage.json
When E2E_STAGING=1 is set, env configs transparently swap PK/SK to staging keys and add CLERK_API_URL for clerkstage.dev. No new env config objects, long-running app entries, or test file changes needed. Adding a new staging instance only requires adding keys — the withStagingSupport wrapper picks them up automatically.
Configs without staging keys are now marked with a private env var instead of being null. This prevents crashes in test files that directly access env configs (e.g. email-code.test.ts accesses envs.withEmailCodes). Long-running apps filter by the marker.
The __STAGING_UNAVAILABLE__ env var marker was being written to the app's .env file, causing side effects. Instead, use CLERK_API_URL presence to determine staging readiness — configs with staging keys have the staging API URL set, configs without don't.
- Separate invalid pattern detection from staging-filtered apps in getByPattern - Delete stale .keys.staging.json when 1Password staging keys are missing - Add regression tests for getByPattern staging behavior
jacekradko
force-pushed
the
jacek/staging-env-swap
branch
from
3595d36 to
664d80e
Compare
brkalow
reviewed
Mar 13, 2026
…NG_INSTANCE_KEYS to base turbo tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
withStagingSupport()wrapper inenvs.tsthat auto-swaps PK/SK to staging keys whenE2E_STAGING=1is set:stagingscript variants and turbo tasks for all ~16 integration test suitesgeneric:stagingAdding a new staging instance requires only adding keys — no code changes needed.
How it works
withStagingSupport(envConfig, 'with-email-codes')checks forclerkstage-with-email-codesininstanceKeys. If found, swaps PK/SK and setsCLERK_API_URL=https://api.clerkstage.dev. If not found, returnsnull(config excluded from exports, long-running apps filtered out, tests run zero cases).Test plan
E2E_STAGING=1swaps env configs correctly via inline scriptgeneric:stagingin CI via e2e-staging workflow dispatchsessions:stagingandhandshake:stagingstill pass (no regression)E2E_STAGINGunset → all configs present)Summary by CodeRabbit