feat: add Clerk authentication with E2E tests#7
Conversation
Figma Assets:
- Rename all assets to consistent `{theme}-{page}_{variant}` pattern
- Add auth screen variants with/without background (nobg/withbg)
- Standardise mobile nav naming (nav_open, nav_close)
Documentation:
- Add flexbox/grid layout explanation for auth layout
- Condense CLAUDE.md breaking changes section
- Add Next.js 16 proxy (middleware) rename note
- Add allowed bash commands (ls, tree) to settings
Naming convention change improves asset organisation and makes theme variants
immediately identifiable. Layout doc explains CSS concepts used in auth pages.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Route Groups: - Add (auth) group with centred layout and theme-aware background images - Add (root) group with navbar, move home page into it - Remove navbar from root layout (now in (root) group layout) Auth Pages: - Add placeholder sign-in and sign-up pages Dependencies: - Update React 19.2.1, Next.js 16.0.7, lucide-react, vitest, lefthook Route groups allow auth pages to have a distinct full-screen layout without the navbar, while main app pages retain navigation. Background images switch automatically based on theme. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Clerk Integration: - Install Clerk via shadcn registry (@clerk/nextjs, @clerk/themes) - Add ClerkProvider wrapper with shadcn theme integration - Replace placeholder auth pages with Clerk sign-in/sign-up components - Configure Next.js 16 proxy pattern for Clerk middleware - Add sample Header component with Clerk auth buttons - Implement catch-all routes for multi-step auth flows Package Updates: - Update next 16.0.7 → 16.0.8 (CVE-2025-66478 critical security fix) - Update all dependencies to latest stable versions Configuration: - Remove outdated ignore for renamed globals-old.css file - Fix linting issues in old.globals.css (remove !important, duplicate properties) Applies critical Next.js security patch for Remote Code Execution vulnerability (CVSS 10.0) in React Server Components. Clerk provides production-ready authentication with OAuth support whilst maintaining course route structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Root Layout: - Add Suspense boundary around ClerkProvider with null fallback - Resolves "Uncached data was accessed outside of <Suspense>" warning When cacheComponents is enabled, Next.js requires components accessing runtime data (cookies/headers) to be wrapped in Suspense. ClerkProvider reads cookies for auth state, triggering this requirement. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
E2E Testing: - Add `@clerk/testing` package for Playwright integration - Create global setup with `clerkSetup()` for testing tokens - Add auth smoke test covering sign-in, OTP verification, and sign-out - Configure Playwright projects with setup dependencies - Skip WebKit due to timing issues with Clerk OTP modal CI/CD: - Add Clerk env vars to both GitHub workflows - Include build step env vars for Next.js compilation UI Consolidation: - Move auth buttons from header.tsx into navbar component - Delete redundant header.tsx (Clerk boilerplate) - Update theme toggle icon sizing Config: - Enable typedRoutes in next.config.ts - Fix homepage test regex case sensitivity - Add vercel CLI permissions to Claude settings Adds automated verification that authentication works before deployments reach production. Test credentials are stored in GitHub Secrets; setup instructions documented in x_docs/own/setup-clerk-secrets.md. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Auth Pages: - Add `await connection()` to sign-in and sign-up pages - Converts pages from static to dynamic Server Components - Resolves "Render in Browser should be wrapped in suspense boundary" build error Git Hooks: - Add production build step to pre-push hook - Run build before Playwright to catch static generation errors locally - Set parallel: false to avoid build lock race conditions Docs: - Update README to clarify commit vs push hook stages Clerk's SignIn/SignUp components use browser APIs incompatible with Next.js 16 static prerendering. Using connection() is the idiomatic solution—it explicitly opts pages into dynamic rendering while keeping them as Server Components with PPR support. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughIntegrates Clerk authentication across the app: adds sign-in/sign-up routes and layouts, a ClerkProvider, middleware proxy, Playwright global setup and auth E2E tests, CI env vars, dependency updates, navbar auth UI, styling tweaks, docs, and ancillary config changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User Browser
participant Next as Next.js App
participant Clerk as Clerk Auth Service
participant Playwright as E2E Test Runner
Browser->>Next: GET /sign-in or /sign-up
Next->>Next: Render SignIn/SignUp inside ClerkProvider (server-side)
Playwright->>Next: Inject test token via global setup (before tests)
Browser->>Clerk: Submit email & password (Clerk component)
Clerk->>Clerk: Validate credentials (may trigger OTP)
alt OTP required
Browser->>Clerk: Enter OTP token
Clerk->>Clerk: Verify OTP
end
Clerk-->>Browser: Set session / auth cookies
Browser->>Next: Request authenticated UI
Next->>Next: Render SignedIn UI (UserButton)
Playwright->>Browser: Assert authenticated state and sign-out flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js,jsx,md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-12-01T13:45:33.949ZApplied to files:
🪛 LanguageToolx_docs/own/setup-clerk-secrets.md[uncategorized] ~63-~63: The official name of this software platform is spelled with a capital “H”. (GITHUB) ⏰ 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). (1)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/old.globals.css (1)
361-397: Markdown code/word-breaking tweaks look good; consider tightening selector scopeSwitching from aggressive
word-break: break-all–style behaviour toword-break: break-wordwith hyphenation keeps long links and code from overflowing whilst avoiding awkward mid-word breaks. The combination ofcolor: #ff7000for inline code andcolor: inheritfor.markdown pre codeneatly differentiates inline snippets from code blocks, and removing!importanton the lexical editor height/overflow should play better with upstream styles.One improvement you might consider while you are here: the selector
.markdown a, codemeans allcodeelements globally inherit these rules, not just those inside.markdown. If that is unintentional, scoping it to.markdown a, .markdown codewould avoid surprising styling in non-markdown contexts.CLAUDE.md (1)
26-41: Docs additions are helpful; minor Playwright typoThe extra Vercel CLI commands and Next.js 16 notes make this doc more actionable and align with the current project setup. One small nit while you are here: in the tech stack above, "PlayWright" should be "Playwright" for correct tool naming.
app/layout.tsx (1)
27-52: Add 'use client' directive or restructure ClerkProvider placement for root layout.The current structure wraps
<ClerkProvider>at the root layout level without a'use client'directive. Since the root layout is a server component by default andClerkProviderrequires client context, this configuration is invalid.Per Clerk's Next.js 16 + PPR best practices, either:
- Add
'use client'at the top ofapp/layout.tsxifClerkProviderremains here, or- Move
ClerkProviderto nested layouts that need authentication, wrapping each with<Suspense>and using<ClerkProvider dynamic>to keep the root layout static for PPR benefits.The second approach is recommended to avoid opting the entire application into dynamic rendering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (27)
package-lock.jsonis excluded by!**/package-lock.jsonx_docs/figma/auth/dark-signin1_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/dark-signin1_withbg.jpgis excluded by!**/*.jpgx_docs/figma/auth/dark-signin2_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/dark-signin2_withbg.jpgis excluded by!**/*.jpgx_docs/figma/auth/dark-signup_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/dark-signup_withbg.jpgis excluded by!**/*.jpgx_docs/figma/auth/light-signin1_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/light-signin1_withbg.jpgis excluded by!**/*.jpgx_docs/figma/auth/light-signin2_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/light-signin2_withbg.jpgis excluded by!**/*.jpgx_docs/figma/auth/light-signup_nobg.pngis excluded by!**/*.pngx_docs/figma/auth/light-signup_withbg.jpgis excluded by!**/*.jpgx_docs/figma/dark-bg.jpgis excluded by!**/*.jpgx_docs/figma/dark-home.pngis excluded by!**/*.pngx_docs/figma/dark-profile.pngis excluded by!**/*.pngx_docs/figma/dark-qna.pngis excluded by!**/*.pngx_docs/figma/dark-search.pngis excluded by!**/*.pngx_docs/figma/light-bg.jpgis excluded by!**/*.jpgx_docs/figma/light-home.pngis excluded by!**/*.pngx_docs/figma/light-profile.pngis excluded by!**/*.pngx_docs/figma/light-qna.pngis excluded by!**/*.pngx_docs/figma/light-search.pngis excluded by!**/*.pngx_docs/figma/mobile/dark-nav_close.pngis excluded by!**/*.pngx_docs/figma/mobile/dark-nav_open.pngis excluded by!**/*.pngx_docs/figma/mobile/light-nav_close.pngis excluded by!**/*.pngx_docs/figma/mobile/light-nav_open.pngis excluded by!**/*.png
📒 Files selected for processing (29)
.claude/settings.json(4 hunks).github/workflows/test-e2e-vercel.yml(1 hunks).github/workflows/test-e2e.yml(1 hunks).gitignore(1 hunks)CLAUDE.md(2 hunks)README.md(1 hunks)app/(auth)/layout.tsx(1 hunks)app/(auth)/sign-in/[[...sign-in]]/page.tsx(1 hunks)app/(auth)/sign-up/[[...sign-up]]/page.tsx(1 hunks)app/(root)/layout.tsx(1 hunks)app/globals.css(1 hunks)app/layout.tsx(2 hunks)app/old.globals.css(5 hunks)biome.json(1 hunks)components.json(1 hunks)components/clerk-provider.tsx(1 hunks)components/navigation/navbar/index.tsx(2 hunks)components/navigation/theme-toggle.tsx(1 hunks)e2e/auth.spec.ts(1 hunks)e2e/global.setup.ts(1 hunks)e2e/homepage.spec.ts(1 hunks)lefthook.yml(1 hunks)next.config.ts(1 hunks)package.json(1 hunks)playwright.config.ts(2 hunks)proxy.ts(1 hunks)x_docs/own/WORDS.md(1 hunks)x_docs/own/flex_grid_layout.md(1 hunks)x_docs/own/setup-clerk-secrets.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English spelling and conventions throughout the project
Files:
README.mde2e/homepage.spec.tsapp/(root)/layout.tsxapp/(auth)/sign-in/[[...sign-in]]/page.tsxx_docs/own/WORDS.mdCLAUDE.mdapp/(auth)/layout.tsxx_docs/own/setup-clerk-secrets.mdapp/(auth)/sign-up/[[...sign-up]]/page.tsxx_docs/own/flex_grid_layout.mdapp/layout.tsxnext.config.tscomponents/navigation/navbar/index.tsxplaywright.config.tse2e/auth.spec.tsproxy.tscomponents/clerk-provider.tsxe2e/global.setup.tscomponents/navigation/theme-toggle.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid manual useMemo/useCallback unless profiling shows need
Files:
e2e/homepage.spec.tsapp/(root)/layout.tsxapp/(auth)/sign-in/[[...sign-in]]/page.tsxapp/(auth)/layout.tsxapp/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsxnext.config.tscomponents/navigation/navbar/index.tsxplaywright.config.tse2e/auth.spec.tsproxy.tscomponents/clerk-provider.tsxe2e/global.setup.tscomponents/navigation/theme-toggle.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts instead of ./fonts)
Files:
e2e/homepage.spec.tsapp/(root)/layout.tsxapp/(auth)/sign-in/[[...sign-in]]/page.tsxapp/(auth)/layout.tsxapp/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsxnext.config.tscomponents/navigation/navbar/index.tsxplaywright.config.tse2e/auth.spec.tsproxy.tscomponents/clerk-provider.tsxe2e/global.setup.tscomponents/navigation/theme-toggle.tsx
app/**
📄 CodeRabbit inference engine (CLAUDE.md)
Use Next.js 16 App Router (not Pages Router) for routing
Files:
app/(root)/layout.tsxapp/old.globals.cssapp/(auth)/sign-in/[[...sign-in]]/page.tsxapp/(auth)/layout.tsxapp/globals.cssapp/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Only add "use client" directive when interactivity is needed
Files:
app/(root)/layout.tsxapp/(auth)/sign-in/[[...sign-in]]/page.tsxapp/(auth)/layout.tsxapp/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
**/*.{css,postcss}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind v4 @import syntax (@import "tailwindcss") instead of @tailwind directives
Files:
app/old.globals.cssapp/globals.css
app/**/page.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Dynamic route params is a Promise and must be awaited in page components
Files:
app/(auth)/sign-in/[[...sign-in]]/page.tsxapp/(auth)/sign-up/[[...sign-up]]/page.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Use Lefthook for Git hooks with pre-commit checks (lint, typecheck, unit tests) and pre-push E2E tests
Applied to files:
README.mdCLAUDE.mdlefthook.yml
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit tests
Applied to files:
README.mdCLAUDE.mdlefthook.ymlplaywright.config.tspackage.json
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/tests/e2e/** : Use Playwright for E2E tests
Applied to files:
CLAUDE.mdx_docs/own/setup-clerk-secrets.mdlefthook.yml.gitignoreplaywright.config.tse2e/auth.spec.ts.github/workflows/test-e2e.yml.github/workflows/test-e2e-vercel.ymle2e/global.setup.ts
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/*.{css,postcss} : Use Tailwind v4 import syntax (import "tailwindcss") instead of tailwind directives
Applied to files:
CLAUDE.mdapp/globals.css
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to app/** : Use Next.js 16 App Router (not Pages Router) for routing
Applied to files:
CLAUDE.mdnext.config.tsproxy.ts
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to app/**/page.{ts,tsx} : Dynamic route params is a Promise and must be awaited in page components
Applied to files:
CLAUDE.md
🧬 Code graph analysis (3)
app/(root)/layout.tsx (2)
app/layout.tsx (1)
RootLayout(27-52)components/navigation/navbar/index.tsx (1)
Navbar(13-52)
components/navigation/navbar/index.tsx (2)
components/navigation/theme-toggle.tsx (1)
ThemeToggle(8-32)components/ui/button.tsx (1)
Button(60-60)
components/navigation/theme-toggle.tsx (1)
components/ui/button.tsx (1)
Button(60-60)
🪛 LanguageTool
x_docs/own/setup-clerk-secrets.md
[uncategorized] ~61-~61: The official name of this software platform is spelled with a capital “H”.
Context: ..._TEST_OTP|424242 | The workflows (.github/workflows/test-e2e.ymlandtest-e2e-v...
(GITHUB)
x_docs/own/flex_grid_layout.md
[style] ~145-~145: Would you like to use the Oxford spelling “organizes”? The spelling ‘organises’ is also correct.
Context: ...lwind Docs? The Tailwind documentation organises utilities by the CSS concepts they cont...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~160-~160: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ne? - Is it fixedto the viewport? - Is itabsolute` and pulled out of normal ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~253-~253: This phrase is redundant. Consider writing “space”.
Context: ...e scales to cover entire container | No empty space, image may crop | | 🎯 bg-center | `b...
(EMPTY_HOLE)
[grammar] ~323-~323: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ariant | Via dark: prefix (covered in variants docs) | `dark:bg-[url('/images/auth-bg-...
(PLURAL_MODIFIER)
⏰ 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). (1)
- GitHub Check: Run E2E Tests
🔇 Additional comments (27)
x_docs/own/WORDS.md (1)
130-132: Approved. Documentation addition is clear and concise.The new "Misc" section properly documents the theme-aware background implementation approach for auth pages.
.gitignore (1)
44-45: Approved. Gitignore entries are properly formatted.The addition correctly ignores temporary documentation and maintains consistency with existing entries.
README.md (1)
21-21: Approved. README table update accurately reflects Lefthook configuration.The description correctly documents that Lefthook now runs build validation and E2E tests on pre-push.
.github/workflows/test-e2e.yml (1)
47-59: Approved. Clerk environment variables properly configured for CI workflow.Both build and test steps have appropriate Clerk credentials sourced from secrets. The variable names (E2E_TEST_EMAIL, E2E_TEST_PASSWORD, E2E_TEST_OTP) align with @clerk/testing requirements.
components.json (1)
21-23: Approved. Clerk registry properly configured for shadcn/ui integration.The registry entry enables Clerk components to be installed via the shadcn/ui CLI, supporting the Clerk authentication UI implementation.
.github/workflows/test-e2e-vercel.yml (1)
78-83: Approved. Clerk environment variables consistent with test-e2e.yml.Vercel preview E2E tests have the same Clerk credentials as local tests, ensuring consistent authentication testing across deployment environments.
lefthook.yml (1)
43-48: Approved. Pre-push build hook properly configured for Next.js 16 PPR compatibility.The sequential build check catches static generation errors (particularly missing Suspense boundaries around async components like ClerkProvider) before E2E tests run. The
parallel: falsesetting correctly prevents race conditions with file locks during the build process.playwright.config.ts (1)
59-105: Global setup project wiring for Clerk looks soundUsing a dedicated
setupproject withtestMatch: /global\.setup\.ts/and addingdependencies: ["setup"]for browser projects is a clean way to guarantee Clerk initialisation runs once before the rest of the suite, especially withfullyParallel: true. This aligns well with Playwright’s project-dependency model.Please double-check against the current Playwright docs that
dependenciessemantics have not changed in your installed Playwright version, to ensuresetupalways runs before any auth-dependent tests.app/old.globals.css (1)
327-355: Removal of!importantaround the MDX editor toolbar improves cascade controlDropping
!importantfrom the toolbar background and related styles while keeping explicit colours should make these rules easier to override from theme-specific layers, without changing the intended appearance. The.active-themefilter remains focused and self-contained.x_docs/own/setup-clerk-secrets.md (1)
1-112: Clear, end-to-end documentation for Clerk E2E setupThis document gives a coherent walkthrough of local, GitHub Actions, and Vercel preview configuration, with a sensible separation between app runtime secrets (on Vercel) and test credentials (in GitHub Secrets). The explanation of how
clerkSetup()/setupClerkTestingToken()work, plus the troubleshooting section, should make maintaining the auth smoke tests straightforward..claude/settings.json (1)
2-85: Permissions updates maintain good safety boundaries whilst enabling new toolingAdding the
playwright-skillmarketplace entry and allowing extra Playwright and Vercel-related Bash/WebFetch commands is useful for this repo’s workflow, and keeping"deny": ["Read(**/.env)", "Read(**/.envrc)"]preserves an important guardrail around environment secrets. The remaining allowed commands are operational and diagnostics-oriented, which is appropriate.app/globals.css (1)
1-52: Clerk shadcn theme import is well-placed; watch for variable collisionsImporting
@clerk/themes/shadcn.cssalongside your Tailwind and animation layers centralises global styling and keeps Clerk’s components visually aligned with the rest of the shadcn-based UI. With it imported aftertailwindcssandtw-animate-css, Clerk’s styles should layer on cleanly.It is worth sanity-checking in the browser that Clerk’s theme does not unintentionally override any of your own CSS variables (for example font or colour tokens) outside Clerk components, especially in dark mode.
next.config.ts (1)
10-13: typedRoutes is a good safety net for routing; ensure existing paths conformEnabling
typedRoutes: trueis a solid move: it will surface typos and invalid hrefs at build time without needing a separate routes constants module. Just be aware it can flag any ad hoc string routes that do not correspond to real files (including unusual groupings like(auth)and catch-all segments), so it is worth running a full type-check to confirm all existingLink/router.pushusages type-check cleanly.e2e/homepage.spec.ts (1)
3-6: Title assertion update makes the homepage smoke test more robustUsing a case-insensitive regex
/DevFlow/ifor the title assertion reduces brittleness to minor casing changes in the document title while still checking the key branding string is present.e2e/global.setup.ts (1)
1-10: Clerk global setup is correctly integrated with Playwright’s test runnerAliasing
testassetup, configuringsetup.describe.configure({ mode: "serial" }), and defining a single"global setup"test that awaitsclerkSetup()is idiomatic for project-based global setup in afullyParallelsuite. This should ensure the Clerk Testing Token is initialised once before dependent projects run.Please confirm against the current
@clerk/testing/playwrightdocumentation that no additional teardown or configuration steps are recommended for your version (for example, explicit cleanup of testing state), and that all requiredCLERK_*andE2E_TEST_*variables are present in CI.app/(auth)/sign-in/[[...sign-in]]/page.tsx (1)
1-12: Sign‑in page implementation looks soundAsync pattern with
connection()and the centred<SignIn />wrapper is straightforward and consistent with the rest of the auth flow; no issues from my side.app/(root)/layout.tsx (1)
1-12: Root group layout composition is cleanNavbar-first composition with children afterwards is clear and matches the overall layout structure; nothing blocking here.
components/navigation/theme-toggle.tsx (1)
15-27: Theme toggle size adjustments are fineShrinking the placeholder, button, and icon keeps behaviour unchanged while likely improving visual balance in the navbar; no further changes needed.
app/(auth)/layout.tsx (1)
1-9: Auth layout background wrapper looks appropriateFull‑height
<main>with light/dark background imagery cleanly scopes styling for all auth routes; implementation looks good.proxy.ts (1)
1-12: Alignproxy.tsexport with the documented Next 16 proxy patternCLAUDE.md says that with Next.js 16 we should move to
proxy.tsand exportproxy()rather thanmiddleware(), but this file currently does a default export ofclerkMiddleware(). That may still work, but it is worth confirming against the official Next 16 proxy docs and our own template guidelines.If we do want a named
proxyentry point, a minimal wrapper that keeps Clerk’s factory usage could look like:import { clerkMiddleware } from "@clerk/nextjs/server"; const handler = clerkMiddleware(); export function proxy(request: Request) { return handler(request); } export const config = { matcher: [ "/((?!_next|[^?]*\\.(?:html?|css|js(?!on)|jpe?g|webp|png|gif|svg|ttf|woff2?|ico|csv|docx?|xlsx?|zip|webmanifest)).*)", "/(api|trpc)(.*)", ], };Please verify which export shape Next 16 expects here and whether we want to standardise on this
proxyfunction pattern.components/clerk-provider.tsx (1)
1-22: The review comment is incorrect. Spreadingundefinedin JavaScript does not throw aTypeError—it is silently ignored. The existing code works correctly:appearance={{ theme: shadcn, ...appearance, // Safe when appearance is undefined }}When
appearanceisundefined(as in theapp/layout.tsxusage at line 34), the spread operator simply contributes no properties, leaving only{ theme: shadcn }. This is valid JavaScript and poses no runtime risk.The proposed fix using
...(appearance ?? {})is unnecessary and changes nothing functionally.components/navigation/navbar/index.tsx (1)
13-52: LGTM! Clean layout implementation.The refactoring from grid to flexbox with absolute positioning for the centered search placeholder is a solid approach. The flex layout provides better control for the three-section navbar structure (logo, centered search, auth controls).
e2e/auth.spec.ts (3)
8-11: LGTM! Appropriate handling of browser-specific flakiness.The WebKit skip with clear documentation is a pragmatic approach to handling timing issues with Clerk's OTP modal. The comment provides good context for future maintainers.
12-24: LGTM! Robust sign-in flow with accessible selectors.The test properly sets up Clerk testing context and uses semantic role-based selectors, which makes the test resilient to styling changes whilst ensuring accessibility compliance.
41-52: LGTM! Complete authentication cycle coverage.The test properly verifies both authenticated and unauthenticated states, with appropriate timeouts for redirects. The sign-out flow through the user menu ensures the full authentication lifecycle is tested.
x_docs/own/flex_grid_layout.md (1)
1-349: LGTM! Excellent documentation following British English conventions.This documentation provides comprehensive coverage of flexbox, grid, and layout concepts with clear examples and visual diagrams. The British English spelling ("organises", "centre", "centred") correctly follows the coding guidelines for the project.
The static analysis hints suggesting Oxford spelling can be safely ignored—British English is the documented standard for this codebase.
app/layout.tsx (1)
18-25: LGTM! Metadata improvements.The title capitalisation correction, improved description, and favicon addition are all solid improvements to the site metadata.
- Rename output from `z_comment.md` to `z_rabbit_comment.md` for clarity - Add note to write output to project root, not `/tmp/` Prevents confusion with other temporary comment files and ensures the output is accessible for evaluation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ute import Accessibility: - Add `aria-hidden="true"` to decorative Lucide icons on sign-up page Imports: - Change `./globals.css` to `@/app/globals.css` for consistency Decorative icons adjacent to text labels should be hidden from screen readers to avoid redundant announcements. Import path aligns with project convention. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.claude/commands/coderabbit.md(1 hunks)app/(auth)/sign-up/[[...sign-up]]/page.tsx(1 hunks)app/layout.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English spelling and conventions throughout the project
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
app/**
📄 CodeRabbit inference engine (CLAUDE.md)
Use Next.js 16 App Router (not Pages Router) for routing
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Only add "use client" directive when interactivity is needed
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid manual useMemo/useCallback unless profiling shows need
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts instead of ./fonts)
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
app/**/page.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Dynamic route params is a Promise and must be awaited in page components
Files:
app/(auth)/sign-up/[[...sign-up]]/page.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:46.607Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) exported from clerk/nextjs are designed to work in Server Components without requiring a "use client" directive in the consuming component, as they handle the client/server boundary internally with their own directives.
📚 Learning: 2025-12-10T20:20:46.607Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:46.607Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) from clerk/nextjs can be used inside Server Components without adding 'use client' in the consuming component. They manage client/server boundary internally. When reviewing code, prefer omitting 'use client' in server components that render these Clerk components and avoid introducing client directives solely for these components. This guideline helps maintain server/server boundary and reduce client bundle size.
Applied to files:
app/(auth)/sign-up/[[...sign-up]]/page.tsxapp/layout.tsx
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts instead of ./fonts)
Applied to files:
app/layout.tsx
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/*.{css,postcss} : Use Tailwind v4 import syntax (import "tailwindcss") instead of tailwind directives
Applied to files:
app/layout.tsx
📚 Learning: 2025-12-01T13:45:33.949Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T13:45:33.949Z
Learning: Applies to **/*.{ts,tsx,js,jsx,md} : Use British English spelling and conventions throughout the project
Applied to files:
app/layout.tsx
🧬 Code graph analysis (1)
app/layout.tsx (2)
components/clerk-provider.tsx (1)
ClerkProvider(6-22)components/theme-provider.tsx (1)
ThemeProvider(6-11)
🔇 Additional comments (4)
.claude/commands/coderabbit.md (1)
15-15: Consistent file naming update with clarity improvement.The changes rename the output file from
z_comment.mdtoz_rabbit_comment.mdacross the command documentation (lines 15, 18, 24), and add explicit guidance to write to the project root rather than/tmp/. This prevents potential execution errors and improves file discoverability. No remaining references to the old filename exist in the codebase.app/(auth)/sign-up/[[...sign-up]]/page.tsx (1)
1-57: Sign-up page implementation looks correct and consistent with the sign-in flow
- Server component without
"use client"matches the Clerk/Next.js pattern and project guideline.await connection()is in place to force dynamic rendering as per the PPR/Next.js 16 setup.- Two-column layout is clear and responsive; right-hand
<SignUp />usage is straightforward.- Lucide icons are now marked
aria-hidden="true", resolving the earlier accessibility nit so screen readers focus on the text.If you have not already, please re-run the Clerk E2E auth flow against this page (including mobile viewport) to confirm layout, theming, and the
connection()behaviour match expectations in your deployment environment.app/layout.tsx (2)
2-17: Geist font configuration and usage look soundThe Geist and Geist_Mono setup with CSS variables and the subsequent use on the
<body>classes are consistent and idiomatic for the current Next.js font API. No changes needed here.
19-24: Metadata update is clear and consistentThe revised
metadata(title capitalisation, streamlined description, and explicit favicon icon path) is coherent and aligns with the overall branding. No issues from a code or content perspective.
| <Suspense fallback={null}> | ||
| <ClerkProvider> | ||
| <html lang="en" className="h-full" suppressHydrationWarning> | ||
| <body | ||
| className={`${geistSans.variable} ${geistMono.variable} flex min-h-full flex-col antialiased`} | ||
| > | ||
| <ThemeProvider | ||
| attribute="class" | ||
| defaultTheme="system" | ||
| enableSystem | ||
| disableTransitionOnChange | ||
| > | ||
| {children} | ||
| </ThemeProvider> | ||
| </body> | ||
| </html> | ||
| </ClerkProvider> | ||
| </Suspense> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "layout.tsx" -path "*/app/*" | head -20Repository: michellepace/devflow
Length of output: 128
🏁 Script executed:
cat -n ./app/layout.tsxRepository: michellepace/devflow
Length of output: 1848
🌐 Web query:
Next.js 16 root layout Suspense html body top-level requirements
💡 Result:
Short answer — in Next.js (App Router / root layout):
- The app directory must include a root layout file (app/layout.js|ts|tsx). It is the top-most layout that applies to all routes. (nextjs.org)
- The root layout must explicitly render and (Next.js does not auto-create them). Those tags should be the top-level elements returned by the root layout. (nextjs.org)
- Root layouts are Server Components by default and cannot be a Client Component (i.e., don't use "use client" at the root layout). (nextjs.org)
- Do not manually manage contents inside the root layout; use the Metadata API (Next.js handles streaming / dedupe of head elements). (nextjs.org)
- You can use React Suspense inside layouts/pages. Newer React features allow suspending rendering while streaming; but the root layout still must include the top-level and elements (you can place Suspense around children or nested parts as needed). For the root-level constraints rely on the Next.js root-layout rules above when using Suspense. (nextjs.org)
If you want examples (root layout with Suspense or wrapping in Suspense), tell me and I’ll show a minimal code sample.
Citations:
- 1: https://nextjs.org/docs/13/app/building-your-application/routing/pages-and-layouts?utm_source=openai
- 2: https://nextjs.org/docs/13/app/building-your-application/routing/pages-and-layouts?utm_source=openai
- 3: https://nextjs.org/docs/13/app/building-your-application/routing/pages-and-layouts?utm_source=openai
- 4: https://nextjs.org/docs/app/api-reference/file-conventions/layout?utm_source=openai
- 5: https://nextjs.org/docs/app/api-reference/file-conventions/layout?utm_source=openai
🌐 Web query:
Next.js 16 root layout Suspense inside body ClerkProvider valid
💡 Result:
Short answer: Yes — you can put a React Suspense boundary inside the of your Next.js 16 root layout, and you can wrap your app with ClerkProvider there — but you must respect server/client component rules.
Key points and recommended patterns
- The root layout must return and and is a Server Component by default. Put global HTML there. [1].
- Suspense/fallbacks are supported in the App Router and may be used in the root layout to provide a fallback while server data or client boundaries load (or to avoid the "uncached data accessed outside " error). Use React.Suspense around components that need fallbacks or will suspend. [2][3].
- ClerkProvider is part of Clerk’s Next.js integration and is typically used to provide auth context for client usage. Clerk’s docs show wrapping layouts with ClerkProvider (often inside a Suspense) and explain options like using or marking the provider as a client component so it behaves correctly with the App Router streaming model. Follow Clerk’s examples (wrap in Suspense when needed, or use ClerkProvider.dynamic when you need to opt a layout into dynamic rendering). [4][5].
Practical guidance
- Keep app/layout.tsx as the root layout (server). If ClerkProvider is a client component:
- Create a small client wrapper (add "use client") that renders {children}, and import that wrapper into the server root layout; OR
- Use Clerk’s documented dynamic option (e.g. ) per Clerk’s guide to opt the layout/route into dynamic rendering and wrap it in a Suspense fallback. [4][5].
- Place Suspense around the client/provider boundary or components that suspend; fallback UI will render in while the client boundary hydrates. [2][3].
Sources
- Next.js App Router / root layout docs. [1].
- Next.js guidance on Suspense and providing fallbacks (uncached data message). [2][3].
- Clerk Next.js integration / rendering modes and examples (ClerkProvider + Suspense + dynamic). [4][5].
If you want, tell me whether you’re using Clerk’s latest package and I’ll show the exact minimal wrapper/layout code for your setup.
🏁 Script executed:
find . -name "*clerk*provider*" -type fRepository: michellepace/devflow
Length of output: 95
🏁 Script executed:
cat -n ./components/clerk-provider.tsxRepository: michellepace/devflow
Length of output: 684
Move <Suspense> inside <body> to maintain proper root layout structure
Next.js root layouts must return <html> and <body> as the top-level elements. Currently, <Suspense> wraps these tags, which violates this requirement and may cause validation errors or build failures.
Restructure to keep <html>/<body> at the root while wrapping ClerkProvider and nested content in Suspense:
return (
+ <html lang="en" className="h-full" suppressHydrationWarning>
+ <body
+ className={`${geistSans.variable} ${geistMono.variable} flex min-h-full flex-col antialiased`}
+ >
<Suspense fallback={null}>
<ClerkProvider>
- <html lang="en" className="h-full" suppressHydrationWarning>
- <body
- className={`${geistSans.variable} ${geistMono.variable} flex min-h-full flex-col antialiased`}
- >
<ThemeProvider
attribute="class"
defaultTheme="system"
enableSystem
disableTransitionOnChange
>
{children}
</ThemeProvider>
- </body>
- </html>
</ClerkProvider>
</Suspense>
+ </body>
+ </html>
);📝 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.
| <Suspense fallback={null}> | |
| <ClerkProvider> | |
| <html lang="en" className="h-full" suppressHydrationWarning> | |
| <body | |
| className={`${geistSans.variable} ${geistMono.variable} flex min-h-full flex-col antialiased`} | |
| > | |
| <ThemeProvider | |
| attribute="class" | |
| defaultTheme="system" | |
| enableSystem | |
| disableTransitionOnChange | |
| > | |
| {children} | |
| </ThemeProvider> | |
| </body> | |
| </html> | |
| </ClerkProvider> | |
| </Suspense> | |
| <html lang="en" className="h-full" suppressHydrationWarning> | |
| <body | |
| className={`${geistSans.variable} ${geistMono.variable} flex min-h-full flex-col antialiased`} | |
| > | |
| <Suspense fallback={null}> | |
| <ClerkProvider> | |
| <ThemeProvider | |
| attribute="class" | |
| defaultTheme="system" | |
| enableSystem | |
| disableTransitionOnChange | |
| > | |
| {children} | |
| </ThemeProvider> | |
| </ClerkProvider> | |
| </Suspense> | |
| </body> | |
| </html> |
|
Any chance you can update the step4 from the Personal Tutor Follow along on your blog (projects director/zsh). It doesn't up on clicking. Thanks. |
Clerk's Next.js SDK requires the NEXT_PUBLIC_ prefix for client-side env vars. The GitHub secret remains CLERK_PUBLISHABLE_KEY but is now mapped to the correct env var name for the build step. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Hello @sundhirajtry I am not entirely sure what "up on clicking" means within step 4. But if you drop your email I'll get in touch to understand your problem better. It will be better than continuing to comment here in my unrelated repo. |
2 participants
Summary
@clerk/testingTechnical Details
Route Groups:
(auth)group with dedicated layout featuring auth background images(root)group for authenticated app content[[...sign-in]]and[[...sign-up]]for Clerk's multi-step flowsNext.js 16 PPR Compatibility:
await connection()to force dynamic rendering on auth pages (fixes static generation errors)CI/CD Improvements:
npm run buildto pre-push hook to catch static generation errors locallyTest Plan
Note
E2E tests on Vercel preview currently fail because
repository_dispatchruns workflows from main, which lacks the Clerk env vars. This is a one-time bootstrap issue that resolves after merge.🤖 Generated with Claude Code