Uh oh!
There was an error while loading. Please reload this page.
fix(e2e): more robust user deletion - #9375
Conversation
🦋 Changeset detectedLatest commit: 417bb3b 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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@clerk/expo-google-signin@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/vuecommit: |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntegration tests now receive a unique run key from CI. Generated users include a derived E2E marker in identifiers and private metadata. Cleanup discovers and deletes users for the active run, verifies that no marked users remain, and reports run-scoped failures. Turbo passes the run key through the environment. Two changeset files were added. Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration/testUtils/usersService.ts`:
- Around line 205-207: Update the empty-user branch in the user cleanup flow to
replace the interpolated opts.email and opts.phoneNumber values with a generic
cleanup message, ensuring no supplied user identifier is written to logs while
preserving the early return.
In `@turbo.json`:
- Line 37: Update the Turbo integration task configuration containing
INTEGRATION_TEST_RUN_KEY so CI integration tasks do not reuse cached results:
set cache to false for those tasks, or move INTEGRATION_TEST_RUN_KEY into
globalEnv to include it in the cache key.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a309c6ba-20fe-4646-87e4-e7652685a849
📒 Files selected for processing (7)
.changeset/fair-llamas-shout.md.changeset/silver-lands-cut.md.github/workflows/ci.ymlintegration/cleanup/cleanup.setup.tsintegration/testUtils/e2eRun.tsintegration/testUtils/usersService.tsturbo.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
| if (ids.size === 0) { | ||
| console.log(`User "${opts.email || opts.phoneNumber}" does not exist!`); | ||
| return; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Do not log the supplied email address or phone number.
Line 206 writes a user identifier to CI logs. Log a generic cleanup message instead.
Proposed fix
- console.log(`User "${opts.email || opts.phoneNumber}" does not exist!`);+ console.log('No user exists for the supplied cleanup criteria.');📝 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.
| if(ids.size===0){ | |
| console.log(`User "${opts.email||opts.phoneNumber}" does not exist!`); | |
| return; | |
| if(ids.size===0){ | |
| console.log('No user exists for the supplied cleanup criteria.'); | |
| return; |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 205-205: Avoid logging sensitive data
Context: console.log(User "${opts.email || opts.phoneNumber}" does not exist!)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration/testUtils/usersService.ts` around lines 205 - 207, Update the
empty-user branch in the user cleanup flow to replace the interpolated
opts.email and opts.phoneNumber values with a generic cleanup message, ensuring
no supplied user identifier is written to logs while preserving the early
return.
Sources: Coding guidelines, Linters/SAST tools
| "globalPassThroughEnv": [ | ||
| "AWS_SECRET_KEY", | ||
| "GITHUB_TOKEN", | ||
| "INTEGRATION_TEST_RUN_KEY", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
# Every integration task must report cache=false.
jq -r ' (.tasks // {}) | to_entries[] | select(.key | test("test:integration")) | "\(.key): cache=\(.value.cache // true)"' turbo.jsonRepository: clerk/javascript
Length of output: 1212
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
printf'%s\n''--- turbo.json ---'
cat -n turbo.json | sed -n '1,90p'printf'%s\n''--- Turbo version and integration scripts ---'
rg -n '"turbo"|test:integration|INTEGRATION_TEST_RUN_KEY' package.json pnpm-lock.yaml packages apps examples .github 2>/dev/null | head -240Repository: clerk/javascript
Length of output: 7965
🌐 Web query:
Turborepo globalPassThroughEnv cache key behavior passThroughEnv cacheable task
💡 Result:
In Turborepo, both globalPassThroughEnv and passThroughEnv serve to expose environment variables to task runtimes without including them in the task's cache key [1][2][3]. Because these variables are intentionally omitted from the hashing process, changes to their values will not trigger a cache miss, even if the change would logically affect the output of the task [1][4]. The fundamental behaviors for these configurations are: 1. Cache Key Exclusion: Variables listed in either globalPassThroughEnv or passThroughEnv do not contribute to the task hash [2][3]. Consequently, Turborepo may serve a cached result even if the underlying environment variable has changed [1][4]. This is why these keys should only be used for variables that do not affect the build output, such as authentication tokens (e.g., SENTRY_AUTH_TOKEN), CI metadata, or credentials consumed at runtime after the build process [2][4]. 2. Runtime Exposure: By including a variable in either key, you satisfy Turborepo's Strict Environment Mode, ensuring the task has access to that variable at runtime [1][5][3]. If a task attempts to access an environment variable that is not accounted for in env, globalEnv, passThroughEnv, or globalPassThroughEnv while in Strict Mode, the task may fail [1][5]. 3. Scope Differences: - globalPassThroughEnv (or global.passThroughEnv when using the global configuration flag) applies the passthrough behavior to all tasks within the repository [2][6]. - passThroughEnv applies only to the specific task where it is configured [3]. If you need a change in an environment variable to invalidate the cache and trigger a rebuild, you must include that variable in env or globalEnv instead of the passthrough keys [1][3].
Citations:
- 1: https://turborepo.dev/docs/crafting-your-repository/using-environment-variables
- 2: https://github.com/vercel/turborepo/blob/main/skills/turborepo/references/environment/RULE.md
- 3: https://turborepo.dev/docs/reference/configuration
- 4: https://github.com/vercel/turborepo/blob/main/skills/turborepo/references/environment/gotchas.md
- 5: https://github.com/vercel/turbo/blob/main/skills/turborepo/references/environment/modes.md
- 6: https://github.com/vercel/turborepo/blob/v2.9.3/skills/turborepo/references/configuration/global-options.md
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
printf'%s\n''--- integration task definitions ---'
rg -n -C 3 '"[`#/`]?test:integration|test:integration' turbo.json
printf'%s\n''--- CI Turbo arguments and integration invocation ---'
sed -n '400,535p' .github/workflows/ci.yml
sed -n '260,300p' .github/workflows/e2e-staging.yml
printf'%s\n''--- exact task keys parsed from turbo.json ---'
jq -r '(.tasks // {}) | to_entries[] | select(.key | contains("test:integration")) | [.key, (.value.cache // true | tostring)] | `@tsv`' turbo.jsonRepository: clerk/javascript
Length of output: 16346
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
python3 - <<'PY'import jsonfrom pathlib import Pathconfig = json.loads(Path("turbo.json").read_text())tasks = config.get("tasks", {})matches = [(name, task.get("cache", True)) for name, task in tasks.items() if "test:integration" in name]print("matching task count:", len(matches))for name, cache in matches: print(f"{name}: cache={cache}")PYRepository: clerk/javascript
Length of output: 1236
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
printf'%s\n''--- all Turbo force/argument settings ---'
rg -n -C 4 'TURBO_(ARGS|FORCE)|pnpm turbo test:integration|pnpm turbo run test:integration' .github/workflows package.json
printf'%s\n''--- CI workflow environment and matrix context ---'
sed -n '1,120p' .github/workflows/ci.yml
sed -n '380,445p' .github/workflows/ci.yml
sed -n '495,535p' .github/workflows/ci.yml
printf'%s\n''--- staging workflow context ---'
sed -n '1,110p' .github/workflows/e2e-staging.yml
sed -n '270,320p' .github/workflows/e2e-staging.ymlRepository: clerk/javascript
Length of output: 30087
Disable caching for CI integration tasks.
The CI integration job does not set TURBO_FORCE, and all 25 integration tasks inherit cache: true. globalPassThroughEnv does not affect the cache key. Set cache: false for these tasks, or move INTEGRATION_TEST_RUN_KEY to globalEnv.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@turbo.json` at line 37, Update the Turbo integration task configuration
containing INTEGRATION_TEST_RUN_KEY so CI integration tasks do not reuse cached
results: set cache to false for those tasks, or move INTEGRATION_TEST_RUN_KEY
into globalEnv to include it in the cache key.
wobsoriano
left a comment
There was a problem hiding this comment.
Extra changesets but everything looks good!
Uh oh!
There was an error while loading. Please reload this page.
Description
This PR updates the logic we use to create fake users to embed a run key, which is a hash of the unique data for a single test run. After integration tests finish, we manually clean up users that were created with the run key in their username, email address, or metadata.
This also includes a fix for
deleteIfExiststhat will delete all matching users rather than just the first. This handles the case were we generate the same matching email/phone number across multiple tests. This shouldn't be as much of an issue now that we're storing the run key in the identifier, but it doesn't hurt!Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change