feat(e2e): add staging instance settings validation#8094
feat(e2e): add staging instance settings validation#8094jacekradko wants to merge 4 commits intomainfrom
Conversation
Compares FAPI /v1/environment responses between production and staging instance pairs to detect configuration drift (auth strategies, MFA, org settings, user requirements, etc.). Runs as a non-blocking warning step in the e2e-staging workflow before integration tests. Also runnable locally via: node scripts/validate-staging-instances.mjs
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: ba1e1b0 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 |
@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: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new "Validate Staging Instances" job to the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
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 @.github/workflows/e2e-staging.yml:
- Around line 57-59: The workflow's integration-tests job is missing an explicit
dependency on the validate-instances job, so add needs: [validate-instances] to
the integration-tests job definition (reference job name integration-tests and
validate-instances) to ensure validation runs beforehand; additionally, add
tests to cover the new validation behavior and workflow ordering—create/modify
CI tests that assert the validate-instances step is required before
integration-tests and add unit/integration tests for the validation script
behavior to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42251c79-33eb-4f74-a042-a5e40ebb8646
📒 Files selected for processing (2)
.github/workflows/e2e-staging.ymlscripts/validate-staging-instances.mjs
Group mismatches by section with aligned columns, collapse child fields when a parent attribute is disabled, show array diffs as missing/extra items instead of raw JSON, and collapse wholly missing social providers.
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 `@scripts/validate-staging-instances.mjs`:
- Around line 61-67: The fetchEnvironment function makes an unbounded network
call and the validation summary can falsely report all pairs matched even when
some fetches failed; modify fetchEnvironment to accept a timeout parameter and
implement AbortController (or equivalent) so the fetch is aborted after the
timeout and throws a clear error, then update the validation loop that currently
skips pairs on fetch errors to record failures separately (e.g., maintain
matchedCount and failedCount or a map of pair->status) so any fetch/validation
error marks that pair as "failed" rather than silently skipping it, and change
the final summary logic that prints "all N instance pair(s) matched" to only
claim all matched when failedCount is zero and matchedCount equals totalPairs,
otherwise report exact counts and exit non-zero if any failures occurred.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1b644e7-d86b-440e-a314-b6ddc1626cb1
📒 Files selected for processing (1)
scripts/validate-staging-instances.mjs
Add 10s fetch timeout via AbortSignal. Track fetch failures separately so the summary never falsely reports 'all matched' when fetches failed.
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 `@scripts/validate-staging-instances.mjs`:
- Around line 39-42: The loadKeys function currently
JSON.parse(process.env[envVar]) directly which can throw and abort the whole
run; change loadKeys to catch JSON.parse errors and validate each returned pair
has a string pair.*.pk, converting malformed JSON or invalid pairs into explicit
load/pair failures instead of throwing (e.g., return a result object {ok: false,
error: "..."} or an array where invalid entries are flagged), and update the
callers that consume loadKeys (and similar code paths referenced by the other
occurrences using the same logic) to handle these failure objects by recording a
per-pair validation error rather than letting the exception short-circuit the
script; ensure main().catch still exits non-zero only for fatal errors while
per-pair errors are reported in the summary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 066433e0-a7f5-44dc-b344-fe0da2802266
📒 Files selected for processing (1)
scripts/validate-staging-instances.mjs
| function loadKeys(envVar, filePath) { | ||
| if (process.env[envVar]) { | ||
| return JSON.parse(process.env[envVar]); | ||
| } |
There was a problem hiding this comment.
Don’t let malformed keys short-circuit the entire validation run.
A bad INTEGRATION_*_KEYS JSON value or a missing/non-string pair.*.pk throws before the per-pair recovery path. In CI that stops validation for every remaining pair, then main().catch exits 0, so the job stays green without an accurate summary. Please validate/catch these inputs locally and turn them into explicit load/pair failures instead of aborting the whole script.
Suggested fix
function loadKeys(envVar, filePath) {
if (process.env[envVar]) {
- return JSON.parse(process.env[envVar]);
+ try {
+ return JSON.parse(process.env[envVar]);
+ } catch (err) {
+ console.error(`Invalid ${envVar}: ${err.message}`);
+ return null;
+ }
}
try {
return JSON.parse(readFileSync(resolve(filePath), 'utf-8'));
} catch {
return null;
@@
function parseFapiDomain(pk) {
+ if (typeof pk !== 'string') {
+ throw new Error('Missing publishable key');
+ }
const parts = pk.split('_');
+ if (parts.length < 3) {
+ throw new Error(`Invalid publishable key: ${pk}`);
+ }
const encoded = parts.slice(2).join('_');
const decoded = Buffer.from(encoded, 'base64').toString('utf-8');
return decoded.replace(/\$$/, '');
}
@@
for (const pair of pairs) {
- const prodDomain = parseFapiDomain(pair.prod.pk);
- const stagingDomain = parseFapiDomain(pair.staging.pk);
-
let prodEnv, stagingEnv;
try {
+ const prodDomain = parseFapiDomain(pair.prod?.pk);
+ const stagingDomain = parseFapiDomain(pair.staging?.pk);
[prodEnv, stagingEnv] = await Promise.all([fetchEnvironment(prodDomain), fetchEnvironment(stagingDomain)]);
} catch (err) {
fetchFailCount++;
- console.log(`⚠️ ${pair.name}: failed to fetch environment`);
+ console.log(`⚠️ ${pair.name}: failed to load or fetch environment`);
console.log(` ${err.message}\n`);
continue;
}Also applies to: 52-57, 320-325, 349-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/validate-staging-instances.mjs` around lines 39 - 42, The loadKeys
function currently JSON.parse(process.env[envVar]) directly which can throw and
abort the whole run; change loadKeys to catch JSON.parse errors and validate
each returned pair has a string pair.*.pk, converting malformed JSON or invalid
pairs into explicit load/pair failures instead of throwing (e.g., return a
result object {ok: false, error: "..."} or an array where invalid entries are
flagged), and update the callers that consume loadKeys (and similar code paths
referenced by the other occurrences using the same logic) to handle these
failure objects by recording a per-pair validation error rather than letting the
exception short-circuit the script; ensure main().catch still exits non-zero
only for fatal errors while per-pair errors are reported in the summary.
1 participant
Summary
scripts/validate-staging-instances.mjs— compares FAPI/v1/environmentresponses between production and staging Clerk instance pairsvalidate-instancesjob to thee2e-staging.ymlworkflow that runs before integration testsWhat it compares
auth_config— session mode, reverification, first/second factorsuser_settings.attributes— enabled, required, factor settings for email/phone/username/password/web3/passkeyuser_settings.social— OAuth providers (only those enabled in at least one env)user_settings.sign_in— MFA settingsuser_settings.sign_up— mode, legal consentuser_settings.password_settings— length and complexity requirementsorganization_settings— enabled, force selectionWhat it skips
Example output
Local usage
Test plan
Summary by CodeRabbit