test(e2e): confirm Next.js 16.2.0 breaks cache-components tests#8122
Open
jacekradko wants to merge 5 commits intomainfrom
Open
test(e2e): confirm Next.js 16.2.0 breaks cache-components tests#8122jacekradko wants to merge 5 commits intomainfrom
jacekradko wants to merge 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@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: |
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:
📝 WalkthroughWalkthroughPinned 🚥 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: 2
🤖 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/tests/cache-components.test.ts`:
- Around line 68-72: The diagnostic try/catch around passwordInput.waitFor(...)
is allowing the test to continue when the password step never appears, masking
upstream render failures as the regression under test; modify the blocks that
call passwordInput.waitFor (the ones around the "Continue" click and the later
similar blocks at lines referenced) to throw or explicitly fail the test when
the wait times out instead of only logging—i.e., replace the silent catch with a
test.fail/assertion (or rethrow) so the test stops immediately if
passwordInput.waitFor(...) does not resolve, preventing later waitForSession
timeouts from being misattributed to the targeted window.Clerk?.session
regression.
- Around line 74-75: Logs currently print sensitive auth data (formHTML and
document.cookie). Update the logging around
page.locator('.cl-signIn-root').innerHTML() and any console.log of
document.cookie (and the other instances at the ranges noted) to redact or mask
sensitive fields before writing to CI: truncate large HTML blobs, remove or
replace cookie values and any user identifiers with placeholders (e.g.,
"<REDACTED_COOKIE>" or "<TRUNCATED_HTML>"), and log only non-sensitive
diagnostics such as presence/length or a hashed/partial fingerprint if needed;
ensure all console.log calls referencing formHTML or document.cookie are changed
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: b21079f1-d7a2-42f3-a7c8-1c10d70a32a5
📒 Files selected for processing (1)
integration/tests/cache-components.test.ts
Comment on lines
+68
to
+72
| try { | ||
| await passwordInput.waitFor({ state: 'visible', timeout: 5000 }); | ||
| console.log('[DIAG] password field appeared after filling identifier'); | ||
| } catch { | ||
| console.log('[DIAG] password field did NOT appear after 5s'); |
Contributor
There was a problem hiding this comment.
Don’t let a pre-sign-in failure masquerade as the target regression.
If the password step never renders, this test still clicks Continue and can later throw waitForSession timed out. That makes an upstream form/rendering failure indistinguishable from the specific post-sign-in window.Clerk?.session regression this PR is supposed to isolate.
Suggested fix
try {
await passwordInput.waitFor({ state: 'visible', timeout: 5000 });
console.log('[DIAG] password field appeared after filling identifier');
} catch {
console.log('[DIAG] password field did NOT appear after 5s');
// Capture what the form looks like
const formHTML = await page.locator('.cl-signIn-root').innerHTML();
console.log('[DIAG] sign-in form HTML:', formHTML.substring(0, 3000));
+ throw new Error('Password step never rendered; aborting before session diagnostics');
}
- const isPasswordVisible = await passwordInput.isVisible();
- console.log(`[DIAG] password visible: ${isPasswordVisible}`);
- if (isPasswordVisible) {
- await passwordInput.fill(fakeUser.password, { force: true });
- }
+ await passwordInput.fill(fakeUser.password, { force: true });Also applies to: 79-99, 119-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/tests/cache-components.test.ts` around lines 68 - 72, The
diagnostic try/catch around passwordInput.waitFor(...) is allowing the test to
continue when the password step never appears, masking upstream render failures
as the regression under test; modify the blocks that call passwordInput.waitFor
(the ones around the "Continue" click and the later similar blocks at lines
referenced) to throw or explicitly fail the test when the wait times out instead
of only logging—i.e., replace the silent catch with a test.fail/assertion (or
rethrow) so the test stops immediately if passwordInput.waitFor(...) does not
resolve, preventing later waitForSession timeouts from being misattributed to
the targeted window.Clerk?.session regression.
Comment on lines
+74
to
+75
| const formHTML = await page.locator('.cl-signIn-root').innerHTML(); | ||
| console.log('[DIAG] sign-in form HTML:', formHTML.substring(0, 3000)); |
Contributor
There was a problem hiding this comment.
Redact auth diagnostics before writing them to CI logs.
formHTML and document.cookie are being logged verbatim here. Those dumps can expose user identifiers, CSRF/session cookies, or other auth state in persisted Actions logs, which is unsafe to merge.
Suggested fix
- const formHTML = await page.locator('.cl-signIn-root').innerHTML();
- console.log('[DIAG] sign-in form HTML:', formHTML.substring(0, 3000));
+ const formShape = await page.locator('.cl-signIn-root').evaluate(root => ({
+ inputNames: Array.from(root.querySelectorAll('input')).map(input => input.getAttribute('name')),
+ buttonLabels: Array.from(root.querySelectorAll('button')).map(
+ button => button.textContent?.trim() ?? '',
+ ),
+ }));
+ console.log('[DIAG] sign-in form shape:', JSON.stringify(formShape));
const diagAfterWait = await page.evaluate(() => {
return {
url: window.location.href,
clerkLoaded: !!(window as any).Clerk?.loaded,
hasSession: !!(window as any).Clerk?.session,
hasUser: !!(window as any).Clerk?.user,
- cookies: document.cookie,
+ cookieNames: document.cookie
+ .split(';')
+ .map(cookie => cookie.trim().split('=')[0])
+ .filter(Boolean),
signInCardClass: document.querySelector('.cl-cardBox')?.className ?? 'NOT_FOUND',
};
});
@@
const finalState = await page.evaluate(() => ({
url: window.location.href,
hasSession: !!(window as any).Clerk?.session,
- cookies: document.cookie,
+ cookieNames: document.cookie
+ .split(';')
+ .map(cookie => cookie.trim().split('=')[0])
+ .filter(Boolean),
}));Also applies to: 104-117, 123-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/tests/cache-components.test.ts` around lines 74 - 75, Logs
currently print sensitive auth data (formHTML and document.cookie). Update the
logging around page.locator('.cl-signIn-root').innerHTML() and any console.log
of document.cookie (and the other instances at the ranges noted) to redact or
mask sensitive fields before writing to CI: truncate large HTML blobs, remove or
replace cookie values and any user identifiers with placeholders (e.g.,
"<REDACTED_COOKIE>" or "<TRUNCATED_HTML>"), and log only non-sensitive
diagnostics such as presence/length or a hashed/partial fingerprint if needed;
ensure all console.log calls referencing formHTML or document.cookie are changed
accordingly.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
integration/tests/cache-components.test.ts (2)
68-75:⚠️ Potential issue | 🔴 CriticalFail fast if the password step never renders.
This catch only logs and then continues without a password, so an upstream render/form failure becomes indistinguishable from the missing
attempt_first_factor/ session progression regression this PR is supposed to isolate.Suggested fix
try { await passwordInput.waitFor({ state: 'visible', timeout: 5000 }); console.log('[DIAG] password field appeared after filling identifier'); } catch { console.log('[DIAG] password field did NOT appear after 5s'); + throw new Error('Password step never rendered; aborting before session diagnostics'); } @@ - const isPasswordVisible = await passwordInput.isVisible(); - console.log(`[DIAG] password visible: ${isPasswordVisible}`); - if (isPasswordVisible) { - await passwordInput.fill(fakeUser.password, { force: true }); - } + await passwordInput.fill(fakeUser.password, { force: true });Also applies to: 109-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration/tests/cache-components.test.ts` around lines 68 - 75, The catch block after awaiting passwordInput.waitFor currently only logs diagnostics and lets the test continue without a password step; change it to fail the test immediately by throwing an Error (or using test.fail/assert) that includes the diagnostic details (e.g., the console message and the captured formHTML from page.locator('.cl-signIn-root')), and apply the same change to the similar block around the code handling the second occurrence (lines 109-113) so any missing password-step render causes a hard test failure rather than silently continuing.
73-74:⚠️ Potential issue | 🔴 CriticalRedact auth diagnostics before writing them to CI logs.
innerHTML(),pwDiag.domValue, anddocument.cookiecan dump identifiers, passwords, and session cookies into persisted job logs. That is unsafe to merge.Suggested fix
- const formHTML = await page.locator('.cl-signIn-root').innerHTML(); - console.log('[DIAG] sign-in form HTML:', formHTML.substring(0, 3000)); + const formShape = await page.locator('.cl-signIn-root').evaluate(root => ({ + inputNames: Array.from(root.querySelectorAll('input')).map(input => input.getAttribute('name')), + buttonLabels: Array.from(root.querySelectorAll('button')).map( + button => button.textContent?.trim() ?? '', + ), + })); + console.log('[DIAG] sign-in form shape:', JSON.stringify(formShape)); @@ - domValue: pwInput?.value ?? 'NOT_FOUND', domValueLength: pwInput?.value?.length ?? 0, @@ - cookies: document.cookie, + cookieNames: document.cookie + .split(';') + .map(cookie => cookie.trim().split('=')[0]) + .filter(Boolean),Also applies to: 116-135, 180-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration/tests/cache-components.test.ts` around lines 73 - 74, The test is logging sensitive auth data (formHTML from page.locator('.cl-signIn-root').innerHTML(), pwDiag.domValue, and document.cookie) to CI; replace those direct dumps with safe diagnostics by removing or redacting sensitive fields before logging (e.g., mask input values and cookie contents or only log non-sensitive metadata), or avoid logging innerHTML/domValue/cookies entirely and instead log a fixed-safe message or specific non-secret attributes (use element IDs/data-test attributes) in the test functions where formHTML, pwDiag.domValue, or document.cookie are referenced (also update similar logging at the other occurrences noted around the same file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@integration/tests/cache-components.test.ts`:
- Around line 68-75: The catch block after awaiting passwordInput.waitFor
currently only logs diagnostics and lets the test continue without a password
step; change it to fail the test immediately by throwing an Error (or using
test.fail/assert) that includes the diagnostic details (e.g., the console
message and the captured formHTML from page.locator('.cl-signIn-root')), and
apply the same change to the similar block around the code handling the second
occurrence (lines 109-113) so any missing password-step render causes a hard
test failure rather than silently continuing.
- Around line 73-74: The test is logging sensitive auth data (formHTML from
page.locator('.cl-signIn-root').innerHTML(), pwDiag.domValue, and
document.cookie) to CI; replace those direct dumps with safe diagnostics by
removing or redacting sensitive fields before logging (e.g., mask input values
and cookie contents or only log non-sensitive metadata), or avoid logging
innerHTML/domValue/cookies entirely and instead log a fixed-safe message or
specific non-secret attributes (use element IDs/data-test attributes) in the
test functions where formHTML, pwDiag.domValue, or document.cookie are
referenced (also update similar logging at the other occurrences noted around
the same file).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: f898c158-862b-43a7-bbc8-09eeb563b1bb
📒 Files selected for processing (1)
integration/tests/cache-components.test.ts
…ctive On Next.js 16.2.0 with cacheComponents, the invalidateCacheAction server action (which calls cookies() from next/headers) hangs indefinitely, blocking setActive() from completing after sign-in. The sign-in API returns complete but the session is never set because setActive is stuck awaiting the server action. Skip the server action on Next.js 16 and rely on router.refresh() in onAfterSetActive for cache invalidation instead.
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
1 participant
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
This PR is a root cause analysis branch — not a proposed solution. It exists to diagnose and confirm why the
cache-componentstest suite breaks on Next.js 16.2.0. The immediate CI unblock is #8121 (pins to 16.1.6).Diagnostic Findings
Instrumented the failing test across 3 rounds to trace exactly where the sign-in breaks. Results are 100% consistent across all retries.
Round 1: Basic state capture
clerkLoaded: true, version 6.3.1, statusready)__client_uat=0— no authentication occurswindow.Clerk.sessionstaysnullRound 2: Form interaction tracing
200 POST /v1/client/sign_insRound 3: Event tracing + API response body
focusandinputevents (trusted), DOM value set correctlyopacity: 1,pointerEvents: autostatus=complete— the sign-in succeeded at the API levelwindow.Clerk.sessionremainsnull,__client_uat=0, URL stays on/sign-inConfirmed root cause
The sign-in completes successfully at the API level, but
setActive()never finishes:invalidateCacheActionis a server action that callscookies()to force router cache invalidation. This works on Next.js ≤16.1.6 but hangs on 16.2.0 withcacheComponents: true.Verification
To confirm the root cause, the last commit skips
invalidateCacheActionon Next.js 16 — and the tests pass. This proves the server action hang is the sole blocker; the actual fix approach needs further discussion.Context
"next": "^16.0.0-canary.0"with no lockfile — every CI run gets the latest versioninvalidateCacheActionon Next.js 15+/16+ (line 64 in ClerkProvider.tsx)router.refresh()is already called inonAfterSetActiveas a secondary cache invalidation mechanismcacheComponents+ Activity breakage: vercel/next.js#86577Test plan
invalidateCacheActionskipped → passes (confirms root cause)