fix(e2e): stabilize auth suite + observability hardening [PID-SENTINEL]#69
Conversation
…re-throw [PID-SENTINEL] Root causes resolved: 1. playwright.config.ts — port 3333 + reuseExistingServer: false prevents Playwright from reusing a dev server loaded with .env.local production credentials. Explicit env block forwards Supabase vars to the child process. Reverted trace to 'on-first-retry' after debugging session. 2. prisma/seed-e2e.ts — purgeAuthUsers() deletes auth.users by email before re-seeding with fixed UUIDs, eliminating non-deterministic UUID collisions from prior Supabase sign-ups. 3. src/app/actions/auth.ts — wrap redirect() in try/catch that re-throws via isRedirectError so Next.js can inline-render the redirect target. Non-PGRST116 profile errors return a user-facing message instead of being swallowed. 4. tests/e2e/helpers/auth.ts — hard page.goto(expectedPath) after waitForURL forces a fresh GET where the browser includes the session cookie, working around the Next.js inline RSC render where Set-Cookie is not yet in Cookie. All 4 auth spec tests now pass (4 passed, 39s). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…captures, harden student portal - Extract src/lib/logger.ts (structured Logger wrapper, Sentry-aware) - Replace all console.error/log in client + server files with Logger.* - Add Sentry.captureException in alunos/financeiro/treinos server actions - Add data-testid="dashboard-welcome" to AlunoDashboardClient h1 - Defensive XP serialization in /aluno/dashboard page (null-safe, div-by-zero guard) - student-portal.spec.ts: switch to getByTestId selector for stability - instrumentation-client.ts: restructured for Next.js 15 (sentry.client.config.ts removed) - Fix form-matricula.test.tsx: simplify SelectTrigger/SelectValue mocks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ation-client.ts) Next.js 15 / @sentry/nextjs v10 uses instrumentation-client.ts instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a centralized Logger with Sentry integration, replaces many console.* calls with Logger/Sentry reporting, exposes a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- CURRENT-STATE: bump to 0.4.2, add Logger row, update E2E/Sentry entries, refresh quality gate timestamps, update tech debt notes, add PR #69 status - CHANGELOG: prepend [Unreleased] block with full PID-SENTINEL summary (env isolation, seed purge, redirect re-throw, ALUNO session fix, Logger) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
playwright.config.ts (1)
28-33: Consider logging a warning when required env vars are missing.The
?? ''fallback silently converts missing environment variables to empty strings. If.env.testis misconfigured, the app may fail with cryptic errors rather than clearly indicating which variable is missing.💡 Optional: Add validation for required env vars
+const requiredEnvVars = ['NEXT_PUBLIC_SUPABASE_URL', 'NEXT_PUBLIC_SUPABASE_ANON_KEY', 'DATABASE_URL']; +for (const envVar of requiredEnvVars) { + if (!process.env[envVar]) { + console.warn(`⚠️ E2E config: ${envVar} is not set in .env.test`); + } +} + export default defineConfig({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playwright.config.ts` around lines 28 - 33, The env object in playwright.config.ts currently uses `?? ''`, which hides missing variables; update the config to validate required vars (NEXT_PUBLIC_SUPABASE_URL, NEXT_PUBLIC_SUPABASE_ANON_KEY, DATABASE_URL, DIRECT_URL) before assigning them to the env map: compute a list of missing keys by checking process.env for each symbol, log a clear warning (e.g., console.warn or a project logger) listing the missing keys, and optionally fail fast by throwing an Error when critical vars are absent; then assign the validated values to the env properties instead of silently defaulting to empty strings.src/lib/logger.ts (1)
20-21: Add a sanitization boundary before sending arbitrary payloads to Sentry.
unknowncontext/error is forwarded directly to telemetry. A small sanitizer/redaction helper would reduce accidental sensitive-field ingestion and keep payloads predictable.Suggested refactor
export class Logger { private static isProduction = process.env.NODE_ENV === 'production'; + private static toSafeSentryData(value: unknown): Record<string, unknown> | undefined { + if (value == null) return undefined; + if (typeof value !== 'object') return { value }; + + return Object.fromEntries( + Object.entries(value as Record<string, unknown>).map(([k, v]) => { + const key = k.toLowerCase(); + const shouldRedact = + key.includes('password') || key.includes('token') || key.includes('secret'); + return [k, shouldRedact ? '[REDACTED]' : v]; + }), + ); + } static info(message: string, context?: unknown) { @@ Sentry.addBreadcrumb({ category: 'log', message, level: 'info', - data: context as Record<string, unknown>, + data: this.toSafeSentryData(context), }); } @@ Sentry.addBreadcrumb({ category: 'log', message, level: 'warning', - data: context as Record<string, unknown>, + data: this.toSafeSentryData(context), }); } @@ } else { Sentry.captureMessage(message, { level: 'error', - extra: { error }, + extra: this.toSafeSentryData(error), }); } }Also applies to: 31-32, 46-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/logger.ts` around lines 20 - 21, The telemetry code is sending raw unknown/context objects directly (e.g. the "data: context as Record<string, unknown>" payload) which risks leaking sensitive fields; add a sanitizer/redaction boundary (a small helper like sanitizePayload or redactSensitiveFields) and call it wherever context or error objects are forwarded to Sentry/telemetry (the spots around the "data: context as Record<string, unknown>" usage and the other occurrences noted at lines 31-32 and 46-47). The helper should whitelist or redact known sensitive keys, normalize types to primitives/arrays/objects, and be invoked immediately before any capture/send call so only sanitized data is passed to Sentry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/actions/auth.ts`:
- Line 5: Replace the internal import of isRedirectError with the public
(unstable) unstable_rethrow from next/navigation and call unstable_rethrow(err)
inside the catch where you currently check isRedirectError(err) to re-throw
Next.js navigation/redirect errors; update the import statement (remove
isRedirectError) and adjust the catch block in the auth action to call
unstable_rethrow(err) before returning your generic error response.
In `@src/lib/logger.ts`:
- Line 13: The logger drops valid falsy context/error values by using logical OR
(e.g., console.log(`[INFO] ${message}`, context || '')); update each logger
output (the console.log/console.error calls that format `[INFO] ${message}`,
`[ERROR] ${message}`, etc.) to use nullish coalescing (context ?? '' and error
?? '') instead of || so that 0, false, and other falsy-but-meaningful values are
preserved while only null/undefined fall back to an empty string.
---
Nitpick comments:
In `@playwright.config.ts`:
- Around line 28-33: The env object in playwright.config.ts currently uses `??
''`, which hides missing variables; update the config to validate required vars
(NEXT_PUBLIC_SUPABASE_URL, NEXT_PUBLIC_SUPABASE_ANON_KEY, DATABASE_URL,
DIRECT_URL) before assigning them to the env map: compute a list of missing keys
by checking process.env for each symbol, log a clear warning (e.g., console.warn
or a project logger) listing the missing keys, and optionally fail fast by
throwing an Error when critical vars are absent; then assign the validated
values to the env properties instead of silently defaulting to empty strings.
In `@src/lib/logger.ts`:
- Around line 20-21: The telemetry code is sending raw unknown/context objects
directly (e.g. the "data: context as Record<string, unknown>" payload) which
risks leaking sensitive fields; add a sanitizer/redaction boundary (a small
helper like sanitizePayload or redactSensitiveFields) and call it wherever
context or error objects are forwarded to Sentry/telemetry (the spots around the
"data: context as Record<string, unknown>" usage and the other occurrences noted
at lines 31-32 and 46-47). The helper should whitelist or redact known sensitive
keys, normalize types to primitives/arrays/objects, and be invoked immediately
before any capture/send call so only sanitized data is passed to Sentry.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 036010d9-3539-4eca-84ae-fcb50203a3c7
📒 Files selected for processing (19)
instrumentation-client.tsplaywright.config.tsprisma/seed-e2e.tssrc/app/actions/auth.tssrc/app/aluno/dashboard/dashboard-client.tsxsrc/app/aluno/dashboard/page.tsxsrc/app/aluno/meus-treinos/meus-treinos-client.tsxsrc/app/dashboard/dev/page.tsxsrc/app/dashboard/treinos/treinos-client.tsxsrc/components/dashboard/alunos/form-matricula.test.tsxsrc/lib/actions/alunos.tssrc/lib/actions/financeiro.tssrc/lib/actions/treinos.tssrc/lib/auth.tssrc/lib/data.tssrc/lib/logger.tssrc/lib/prisma.tstests/e2e/helpers/auth.tstests/e2e/specs/student-portal.spec.ts
|
|
||
| import { createClient } from '@/utils/supabase/server'; | ||
| import { redirect } from 'next/navigation'; | ||
| import { isRedirectError } from 'next/dist/client/components/redirect-error'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15 isRedirectError public export
💡 Result:
In Next.js 15, isRedirectError is an internal utility function that checks if an error is a redirect error thrown by redirect or permanentRedirect from next/navigation. It is defined in the Next.js source code at packages/next/src/client/components/redirect.ts (or redirect-error.ts in some branches). It is not a public export in the stable API. There is no official public export in next/navigation (a PR to add it was closed without merging). Official docs do not document it as a public API. To use it, import from the internal path commonly referenced in issues and discussions: import { isRedirectError } from 'next/dist/client/components/redirect'; Usage example (in a Client Component handling a Server Action): 'use client'; import { isRedirectError } from 'next/dist/client/components/redirect'; async function handleAction { try { await serverAction; } catch (error) { if (isRedirectError(error)) { // Re-throw to allow Next.js to handle the redirect throw error; } // Handle other errors console.error(error); } } This pattern is necessary because redirect throws a NEXT_REDIRECT error (with a specific digest format) that Next.js intercepts for navigation. Catching it without re-throwing prevents the redirect. Note: Internal imports like this are not guaranteed stable across Next.js versions and may break (e.g., webpack bundling issues reported in Next.js 15.1.0 with some libraries). For production, prefer calling redirect outside try/catch or use unstable_rethrow from next/navigation where applicable. Confirmed in Next.js 15 source code and GitHub issues/discussions (#73984, #74258, #57844).
Citations:
- 1: https://github.com/vercel/next.js/blob/c1f8d9317588e51a8a31240f6add36b5f2c9f9bf/packages/next/src/client/components/redirect.ts
- 2: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/redirect.ts
- 3: Webpack isRedirectError is not a function when using next-safe-actions in Nextjs v5.1.0 vercel/next.js#73984
- 4: Unexpected behaviour in redirect next/navigation function - Next.js 15 vercel/next.js#74258
- 5: https://stackoverflow.com/questions/78081043/error-next-redirect-using-redirect-in-nextjs-14-while-trying-to-redirect
- 6: Add
isRedirectErrorexport innavigationvercel/next.js#57844 - 7:
NEXT_REDIRECTinternal error making it to the client vercel/next.js#71831 - 8: redirect() error from action caught in try/catch on client vercel/next.js#71740
- 9: Next Redirect - Server action redirects do not execute when inside try / catch blocks vercel/next.js#55586
🏁 Script executed:
# Check the exact import statement and surrounding code
sed -n '1,60p' src/app/actions/auth.ts | cat -nRepository: EmiyaKiritsugu3/PWeb_Project
Length of output: 2392
🏁 Script executed:
# Search for redirect-related imports and patterns
rg "(isRedirectError|unstable_rethrow|from.*redirect)" src/ --max-count=20Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 238
🏁 Script executed:
# Search for unstable_rethrow or alternative redirect error handling
rg "unstable_rethrow|redirect.*catch|catch.*redirect" src/ --max-count=10Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 54
🏁 Script executed:
# Check if the redirect-error vs redirect path distinction matters
# by looking at what is actually exported/available
rg "redirect.ts|redirect-error" . --type=ts --type=tsx --max-count=5 2>/dev/null || echo "No matches in repo"Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 90
🏁 Script executed:
# Check for any Next.js patterns or documentation in the repo
find . -name "*.md" -type f | head -5 | xargs grep -l "redirect\|server.*action" 2>/dev/null || echo "No docs found"Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 91
🏁 Script executed:
# Check the errors documentation
cat .claude/errors.md 2>/dev/null | head -50Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 1662
🌐 Web query:
Next.js 15 unstable_rethrow server action redirect pattern
💡 Result:
In Next.js 15, the unstable_rethrow function from next/navigation enables server actions with try/catch blocks to properly handle both application errors and Next.js navigation errors like redirect. The pattern addresses the issue where redirect throws a NEXT_REDIRECT error that's caught by try/catch, preventing the redirect. unstable_rethrow(err) at the top of the catch block re-throws Next.js errors (including from redirect, notFound, etc.) so Next.js can handle them, while allowing custom handling of other errors. Example server action: 'use server' import { redirect, unstable_rethrow } from 'next/navigation' import { createOrder } from '@/lib' export async function createOrderAction(formData: FormData) { try { const res = await createOrder(formData) redirect(res.url) // Throws NEXT_REDIRECT if successful } catch (err) { unstable_rethrow(err) // Re-throws if Next.js error (e.g. redirect), else falls through // Handle application errors here (e.g. return error state for useActionState) console.error(err) return { error: 'Failed to create order' } } } Key points: - Import: import { unstable_rethrow } from 'next/navigation' - Call unstable_rethrow(err) first in catch block - Only needed when mixing app errors and Next.js APIs in same try block - Unstable API, subject to change - Works in Server Components and Server Actions - For expected errors, prefer useActionState returning error objects over throwing This resolves common issues like #59930, #58002 where redirects failed inside try/catch in server actions.
Citations:
- 1: https://nextjs.im/ar/docs/15/app/api-reference/functions/unstable_rethrow/
- 2: https://nextjs.org/docs/app/api-reference/functions/unstable_rethrow
- 3: RFC: `unstable_rethrow` API vercel/next.js#64076
- 4: https://nextjs.org/docs/app/api-reference/functions/redirect
🌐 Web query:
next/dist/client/components/redirect-error vs redirect path Next.js 15
💡 Result:
In Next.js 15 (App Router), next/dist/client/components/redirect-error refers to internal modules (redirect-error.ts and related files like redirect.ts) that define the RedirectError type and isRedirectError utility. These implement the redirect function from next/navigation, which throws a special NEXT_REDIRECT error (with a digest property encoding the target URL, type like 'push'/'replace', and status code like 307/308) to trigger navigation without rendering the full page. This contrasts with a plain "redirect path" (e.g., using redirect('/path') or next.config.js redirects), which performs an HTTP/meta redirect. The error-based mechanism: - In Server Components: Inserts a refresh tag or HTTP status (307 temporary, 308 permanent). - In Client Components (rendering only, not event handlers): Caught by Next.js's internal RedirectBoundary (error boundary in next/dist/client/components/redirect-boundary.tsx) to perform router.push/replace. - In Server Actions/Route Handlers: Serves HTTP redirect response. Key utilities: - isRedirectError(error): Checks if error is a redirect (digest starts with 'NEXT_REDIRECT'). - getURLFromRedirectError(error): Extracts URL from digest. - Must re-throw in try/catch: if (isRedirectError(e)) throw e; to propagate. No changes specific to Next.js 15; behavior stable since App Router introduction (v13+). Custom error.js/error.tsx boundaries catch but should re-throw redirect errors (or wrap in Next.js RedirectBoundary). Use redirect('/path') for programmatic redirects; avoid custom error boundaries conflicting with internal handling. For config-based paths: next.config.js redirects[] or Middleware NextResponse.redirect.
Citations:
- 1: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/redirect.ts
- 2: https://github.com/vercel/next.js/blob/c1f8d9317588e51a8a31240f6add36b5f2c9f9bf/packages/next/src/client/components/redirect.ts
- 3: https://nextjs.org/docs/app/api-reference/functions/redirect
- 4: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/redirect-boundary.tsx
- 5: https://github.com/vercel/next.js/blob/c1f8d9317588e51a8a31240f6add36b5f2c9f9bf/packages/next/src/client/components/redirect-boundary.tsx
- 6: https://nextjs.im/docs/15/app/guides/redirecting
- 7: https://nextjs.org/blog/next-15
- 8: Docs: Client side redirect() usage incompatible with error boundaries. vercel/next.js#62458
Use the public API alternative unstable_rethrow instead of internal import.
Replace the internal isRedirectError import with unstable_rethrow from next/navigation. The internal path next/dist/client/components/redirect-error is not part of Next.js's public API and may break on upgrades. Next.js 15 provides unstable_rethrow as the documented approach:
import { redirect, unstable_rethrow } from 'next/navigation';
// In catch block:
catch (err: unknown) {
unstable_rethrow(err); // Re-throws Next.js errors (e.g., redirect)
return { error: 'Ocorreu um erro inesperado. Por favor, tente novamente.' };
}This is equivalent to your current if (isRedirectError(err)) throw err; pattern but uses the public (though unstable) API instead of internal internals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/actions/auth.ts` at line 5, Replace the internal import of
isRedirectError with the public (unstable) unstable_rethrow from next/navigation
and call unstable_rethrow(err) inside the catch where you currently check
isRedirectError(err) to re-throw Next.js navigation/redirect errors; update the
import statement (remove isRedirectError) and adjust the catch block in the auth
action to call unstable_rethrow(err) before returning your generic error
response.
| static info(message: string, context?: unknown) { | ||
| if (!this.isProduction) { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`[INFO] ${message}`, context || ''); |
There was a problem hiding this comment.
Preserve falsy diagnostic values in log output.
context || '' / error || '' drops valid falsy values (e.g., 0, false), which can hide useful debugging signals.
Suggested fix
- console.log(`[INFO] ${message}`, context || '');
+ console.log(`[INFO] ${message}`, context ?? '');
- console.warn(`[WARN] ${message}`, context || '');
+ console.warn(`[WARN] ${message}`, context ?? '');
- console.error(`[ERROR] ${message}`, error || '');
+ console.error(`[ERROR] ${message}`, error ?? '');
- console.debug(`[DEBUG] ${message}`, context || '');
+ console.debug(`[DEBUG] ${message}`, context ?? '');Also applies to: 26-26, 37-37, 54-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/logger.ts` at line 13, The logger drops valid falsy context/error
values by using logical OR (e.g., console.log(`[INFO] ${message}`, context ||
'')); update each logger output (the console.log/console.error calls that format
`[INFO] ${message}`, `[ERROR] ${message}`, etc.) to use nullish coalescing
(context ?? '' and error ?? '') instead of || so that 0, false, and other
falsy-but-meaningful values are preserved while only null/undefined fall back to
an empty string.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/CURRENT-STATE.md (2)
76-76: Minor terminology inconsistency: "purge-on-reseed" vs "purge-on-seed".Line 26 uses "purge-on-seed" while line 76 uses "purge-on-reseed" to describe the same behavior. Consider using consistent terminology across the document for clarity.
✏️ Proposed fix for consistency
-| `prisma/seed-e2e.ts` | E2E seed (4 users, fixed UUIDs, purge-on-reseed) | +| `prisma/seed-e2e.ts` | E2E seed (4 users, fixed UUIDs, purge-on-seed) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CURRENT-STATE.md` at line 76, The document uses two different terms for the same behavior; standardize to a single phrase (pick either "purge-on-seed" or "purge-on-reseed") throughout the file and update the entry that references `prisma/seed-e2e.ts` (currently showing "purge-on-reseed") to match the term used at line 26 ("purge-on-seed"); ensure any other occurrences in CURRENT-STATE.md use the same chosen term for consistency.
3-3: Update the "Last Updated" date for consistency.The documentation shows
2026-04-17, but the PR was created on2026-04-18. Update to match or exceed the PR creation date to ensure the timestamp reflects the current state.📅 Proposed fix
-**Last Updated**: 2026-04-17 +**Last Updated**: 2026-04-18🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CURRENT-STATE.md` at line 3, Update the "Last Updated" timestamp string in docs/CURRENT-STATE.md by changing the value after **Last Updated** from 2026-04-17 to 2026-04-18 (or a later date) so the document reflects the PR creation date; edit the literal line containing "**Last Updated**: 2026-04-17" to the new date.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/CURRENT-STATE.md`:
- Line 76: The document uses two different terms for the same behavior;
standardize to a single phrase (pick either "purge-on-seed" or
"purge-on-reseed") throughout the file and update the entry that references
`prisma/seed-e2e.ts` (currently showing "purge-on-reseed") to match the term
used at line 26 ("purge-on-seed"); ensure any other occurrences in
CURRENT-STATE.md use the same chosen term for consistency.
- Line 3: Update the "Last Updated" timestamp string in docs/CURRENT-STATE.md by
changing the value after **Last Updated** from 2026-04-17 to 2026-04-18 (or a
later date) so the document reflects the PR creation date; edit the literal line
containing "**Last Updated**: 2026-04-17" to the new date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83b3569c-351e-40e6-9a9d-98dd489ba3b5
📒 Files selected for processing (3)
CHANGELOG.mddocs/CURRENT-STATE.mddocs/operations/RUNBOOK.md
✅ Files skipped from review due to trivial changes (2)
- docs/operations/RUNBOOK.md
- CHANGELOG.md
- dev-errors: add ERR-019 (redirect swallowed by try/catch), ERR-020 (ALUNO session cookie inline RSC render timing), ERR-021 (E2E server reusing .env.local production credentials) - README: fix env var name (ANON_KEY → PUBLISHABLE_DEFAULT_KEY), port 3000 → 3001, Node 18 → 20+, add quality gates section, replace broken arquitetura.md link with real doc links Local-only (gitignored): MILESTONES.md, PDR-001, ADR-004, RUNBOOK Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
133-136: Evite contagens fixas nos Quality Gates.As métricas
18/18e15/15na Line 135-136 tendem a ficar desatualizadas rápido; melhor manter os comandos e o critério de sucesso.📌 Sugestão de ajuste no README
-npm run test # Vitest — 18/18 testes -npm run e2e # Playwright — 15/15 cenários (requer Docker + supabase start) +npm run test # Vitest — todos os testes devem passar +npm run e2e # Playwright — todos os cenários devem passar (requer Docker + supabase start)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 133 - 136, Remova as contagens fixas "18/18" e "15/15" na seção que lista os comandos e substitua por critérios de sucesso genéricos (ex.: "todos os testes passam" para "npm run test" e "cenários concluídos com sucesso" para "npm run e2e"), atualizando as strings correspondentes no README.md onde aparecem essas linhas; mantenha os comandos "npm run test" e "npm run e2e" e acrescente, se quiser, observação sobre requisitos (ex.: "requer Docker + supabase start") em texto que não dependa de números estáticos.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-errors.md`:
- Around line 314-317: The env snippet is missing the explicit Supabase
publishable key variable; update the env object that currently contains
NEXT_PUBLIC_SUPABASE_URL to also include
NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY (set to
process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY ?? '') so the
documentation shows the correct, current key name and avoids accidental
reintroduction of the old key name.
In `@README.md`:
- Around line 115-117: Replace the no-op local migration instruction that
currently runs "npx prisma migrate deploy" in README.md with the CI-safe command
used by the workflow ("npx prisma db push --accept-data-loss") so local setup
matches CI; alternatively, if you intend to use migrations, update the README to
instruct running "npx prisma migrate dev --name <description>" and ensure the
prisma/migrations directory is generated and committed.
---
Nitpick comments:
In `@README.md`:
- Around line 133-136: Remova as contagens fixas "18/18" e "15/15" na seção que
lista os comandos e substitua por critérios de sucesso genéricos (ex.: "todos os
testes passam" para "npm run test" e "cenários concluídos com sucesso" para "npm
run e2e"), atualizando as strings correspondentes no README.md onde aparecem
essas linhas; mantenha os comandos "npm run test" e "npm run e2e" e acrescente,
se quiser, observação sobre requisitos (ex.: "requer Docker + supabase start")
em texto que não dependa de números estáticos.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8dd4d9dd-a995-40f9-b2b3-249290973cfa
📒 Files selected for processing (4)
README.mddocs/dev-errors.mddocs/pdr/MILESTONES.mddocs/pdr/PDR-001-core-system.md
✅ Files skipped from review due to trivial changes (2)
- docs/pdr/PDR-001-core-system.md
- docs/pdr/MILESTONES.md
| env: { | ||
| NEXT_PUBLIC_SUPABASE_URL: process.env.NEXT_PUBLIC_SUPABASE_URL ?? '', | ||
| // ... forward explícito de todas as vars do .env.test | ||
| }, |
There was a problem hiding this comment.
Torne explícita a variável de chave Supabase no snippet do ERR-021.
No trecho da Line 314-317, a documentação fala em forward explícito, mas não mostra NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY. Isso pode levar alguém a reintroduzir o nome antigo por engano.
✍️ Sugestão de ajuste no snippet
env: {
NEXT_PUBLIC_SUPABASE_URL: process.env.NEXT_PUBLIC_SUPABASE_URL ?? '',
- // ... forward explícito de todas as vars do .env.test
+ NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY:
+ process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY ?? '',
+ DATABASE_URL: process.env.DATABASE_URL ?? '',
+ DIRECT_URL: process.env.DIRECT_URL ?? '',
},📝 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.
| env: { | |
| NEXT_PUBLIC_SUPABASE_URL: process.env.NEXT_PUBLIC_SUPABASE_URL ?? '', | |
| // ... forward explícito de todas as vars do .env.test | |
| }, | |
| env: { | |
| NEXT_PUBLIC_SUPABASE_URL: process.env.NEXT_PUBLIC_SUPABASE_URL ?? '', | |
| NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY: | |
| process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY ?? '', | |
| DATABASE_URL: process.env.DATABASE_URL ?? '', | |
| DIRECT_URL: process.env.DIRECT_URL ?? '', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/dev-errors.md` around lines 314 - 317, The env snippet is missing the
explicit Supabase publishable key variable; update the env object that currently
contains NEXT_PUBLIC_SUPABASE_URL to also include
NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY (set to
process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY ?? '') so the
documentation shows the correct, current key name and avoids accidental
reintroduction of the old key name.
| # Aplicar migrations | ||
| npx prisma migrate deploy | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar se há migrations reais e qual fluxo Prisma está ativo no repositório
set -euo pipefail
echo "== Prisma migrations directory =="
fd -HI '^migrations$' prisma || true
echo
echo "== Arquivos dentro de prisma/migrations =="
if [ -d "prisma/migrations" ]; then
find prisma/migrations -maxdepth 2 -type f | sed 's#^# - #'
else
echo " - (diretório prisma/migrations não encontrado)"
fi
echo
echo "== Referências de workflow Prisma no repo =="
rg -n "prisma (migrate deploy|db push|migrate dev)" README.md docs package.json .github || true
echo
echo "== Scripts npm relevantes =="
cat package.json | jq '.scripts | with_entries(select(.key|test("prisma|db|migrate|seed")))'Repository: EmiyaKiritsugu3/PWeb_Project
Length of output: 1231
Align local migration setup with CI workflow.
Line 116 uses npx prisma migrate deploy, but the prisma/migrations directory doesn't exist, making this a no-op during local setup. This issue is documented in docs/dev-errors.md:168-174 (ERR-014). The CI pipeline correctly uses npx prisma db push --accept-data-loss instead (.github/workflows/ci.yml:124).
Change Line 116 to match the working CI approach:
npx prisma db push --accept-data-loss
Alternatively, if migrations are planned, use npx prisma migrate dev --name <description> and ensure prisma/migrations is committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 115 - 117, Replace the no-op local migration
instruction that currently runs "npx prisma migrate deploy" in README.md with
the CI-safe command used by the workflow ("npx prisma db push
--accept-data-loss") so local setup matches CI; alternatively, if you intend to
use migrations, update the README to instruct running "npx prisma migrate dev
--name <description>" and ensure the prisma/migrations directory is generated
and committed.
Port was set to 3001 but playwright.config.ts webServer starts Next.js on 3333 (dedicated E2E port). All tests hit ERR_CONNECTION_REFUSED. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
reuseExistingServer: falseinplaywright.config.tsprevents reuse of a dev server carrying.env.localproduction credentials; explicitenvblock forwards Supabase vars to the webServer child processpurgeAuthUsers()inseed-e2e.tsdeletesauth.usersby email before re-seeding, eliminating UUID collisions from prior sign-upsisRedirectErrorguard insrc/app/actions/auth.tsensures Next.js can inline-render the redirect target instead of swallowing it in a catch blockpage.goto(expectedPath)afterwaitForURLinloginAs()forces a GET with the session cookie, working around the Next.js RSC inline render whereSet-Cookieis not yet inCookierequest headerssrc/lib/logger.ts, replacedconsole.*calls withLogger.*, addedSentry.captureExceptionin server actions; migrated toinstrumentation-client.ts(Next.js 15)data-testid="dashboard-welcome"on dashboard h1; defensive XP serialization (null-safe, div-by-zero guard); updated E2E selectorQuality gates
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Chores
Tests
Documentation