add social login to auth#28
Merged
Merged
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds Google/GitHub OAuth routes and callback handlers across Next/Nuxt/SvelteKit, refactors social callbacks to return structured results, centralizes response-cookie propagation via adapter/runtime appendResponseCookie hooks, and makes forms validation schema-aware for sensitive/password fields while updating tests and docs. ChangesOAuth Social Authentication & Cookie Handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/auth/tests/package.test.ts (1)
774-805: ⚡ Quick winConsider also exercising an async
appendResponseCookie.The new test validates the sync path. Since the contract allows
void | Promise<void>, a small additional assertion (or a second test) using an async implementation would lock in that the runtime awaits the hook before resolvingloginUsing/logout. Otherwise a regression that fires-and-forgets the hook would slip through.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth/tests/package.test.ts` around lines 774 - 805, The test only verifies the synchronous appendResponseCookie path; add a case that registers/configures the runtime with an async appendResponseCookie implementation (returns a Promise and pushes to appendedCookies after a small await) and then call register/loginUsing and logout to assert the test waits for the async hook to complete (i.e., appendedCookies contains cookies after loginUsing and logout). Update the existing test or add a new it block that uses configureAuthRuntime with context.appendResponseCookie as an async function and asserts appendedCookies matches established.cookies and then appendedCookies includes loggedOut.cookies after logout to ensure loginUsing/logout await the hook.apps/blog-sveltekit/src/routes/auth/google/+server.ts (1)
3-5: 💤 Low valueUse SvelteKit's typed
RequestHandlerfrom./$types.The inline type annotation
{ request: Request }works but bypasses SvelteKit's generated types. UsingRequestHandlerkeeps the signature in sync with route-specific params, cookies, and locals if this endpoint ever needs them.♻️ Suggested refactor
import { redirect } from '@holo-js/auth-social' +import type { RequestHandler } from './$types' -export function GET({ request }: { request: Request }): Promise<Response> { - return redirect('google', request) -} +export const GET: RequestHandler = async ({ request }) => redirect('google', request)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/blog-sveltekit/src/routes/auth/google/`+server.ts around lines 3 - 5, Change the GET handler to use SvelteKit's generated RequestHandler type from ./$types: import the type RequestHandler and update the exported handler signature to use RequestHandler (e.g., export const GET: RequestHandler = async ({ request, params, cookies, locals }) => ...) so the route stays in sync with route-specific params/cookies/locals and still calls the existing redirect(...) helper; keep the same return behavior (Promise<Response>) and ensure the handler destructures request from the provided argument as before.packages/adapter-sveltekit/src/index.ts (1)
43-57: ⚡ Quick winImplicit cross-package contract via
globalThis.__holoSvelteKitRequestEventStorekey.Both
packages/adapter-sveltekit/src/index.tsandpackages/auth/src/sveltekit/server.tsindependently definegetSvelteKitRequestEventStore()using the sameglobalThis.__holoSvelteKitRequestEventStorekey (but typed differently —AsyncLocalStorage<SvelteKitRequestEvent>here vs.AsyncLocalStorage<SvelteKitStoredRequestEvent>inserver.ts). The architecture is intentional (avoiding a circular package dependency), but the key is an unprotected implicit contract: renaming it in one file silently breaks response-cookie appending for all auth operations covered by the auth-server handle hooks, with no type or build error.Consider extracting the key name as a shared constant in an internal utility, or at minimum add a co-location comment in both files referencing the shared contract.
💡 Minimal approach: document the shared contract
+// NOTE: This key is shared with packages/auth/src/sveltekit/server.ts. +// Both files must use the same string to coordinate on the same ALS instance. type SvelteKitRuntimeGlobal = typeof globalThis & { __holoSvelteKitRequestEventStore?: AsyncLocalStorage<SvelteKitRequestEvent> }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapter-sveltekit/src/index.ts` around lines 43 - 57, The global key __holoSvelteKitRequestEventStore is an implicit cross-package contract used by getSvelteKitRequestEventStore() in this file and a similarly named function in packages/auth; to prevent silent breakage, extract that string into a shared internal constant (e.g., HoloSvelteKitRequestEventStoreKey) in a tiny cross-package utility or shared types module and import it here and in packages/auth, or at minimum add a prominent co-location comment in both getSvelteKitRequestEventStore() implementations documenting the exact key, its expected AsyncLocalStorage type (AsyncLocalStorage<SvelteKitRequestEvent> vs AsyncLocalStorage<SvelteKitStoredRequestEvent>), and that both files must stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/blog-sveltekit/src/routes/auth/google/callback/`+server.ts:
- Around line 19-20: The guest-only guard is not covering the OAuth callback
routes so runWithSvelteKitRequestEvent is never invoked and AsyncLocalStorage
isn’t established; update the guestOnly configuration in hooks.server.ts to
include '/auth/google/callback' and '/auth/github/callback' in the routes array
(e.g., add those strings to the routes passed to guestOnly) or alternatively
ensure the callback handler (the function that calls auth.guard(...).loginUsing
and redirect) is wrapped with runWithSvelteKitRequestEvent so
appendResponseCookies runs with a valid SvelteKit request event store.
In `@apps/docs/docs/auth/social-login.md`:
- Around line 216-218: Update the callback flow bullet that currently says the
callback “establishes a local authenticated session” so it covers both session
and token-based guards; change the wording to note that the callback either
establishes a local session (when using a session guard and loginUsing()) or
returns an authenticated user (which can be used to create an access token for
token guards), referencing the existing loginUsing() guidance and the
token-guard note for clarity.
In `@packages/adapter-next/src/config.ts`:
- Around line 81-84: The JSON.parse call in config resolution can throw on
malformed package.json; wrap the read+parse in a try/catch inside the function
that builds the parsed variable (the block assigning parsed from JSON.parse in
packages/adapter-next/src/config.ts) and on any error log/debug if appropriate
and return undefined so withHolo()/Next config loading degrades to “no optional
externals”; ensure you still validate that parsed is an object/ not an array
(the existing checks) after successful parse.
In `@tests/example-app-auth-flow.mjs`:
- Around line 157-165: The test's assertSocialRedirect currently asserts
result.response.status === 302 which is brittle; update the assert in
assertSocialRedirect to accept any 3xx response (e.g.,
assert.ok(result.response.status >= 300 && result.response.status < 400,
'Expected redirect status in 3xx range') ), and adjust the assertion message
accordingly; leave the rest of the function (location extraction,
authorizationUrl checks, and state assertion) unchanged so location,
authorizationUrl.origin/pathname, and state validation still run.
- Around line 303-316: The throttled-login uses Date.now() inside the retry loop
which changes the email each attempt and can avoid the IP+email throttle; fix by
computing a single stable email before the loop (e.g., const throttledEmail =
`${appName}-throttled-login-${Date.now()}@app.test`) and then call
fetchAuthJson('/api/login', { fields: { email: throttledEmail, password:
'wrong-password' }, ... }) inside the for loop, keeping the rest
(throttleLoginIp headers, allowFailure flag) unchanged so throttledLogin and
assertThrottleFailure() exercise a deterministic throttling scenario.
---
Nitpick comments:
In `@apps/blog-sveltekit/src/routes/auth/google/`+server.ts:
- Around line 3-5: Change the GET handler to use SvelteKit's generated
RequestHandler type from ./$types: import the type RequestHandler and update the
exported handler signature to use RequestHandler (e.g., export const GET:
RequestHandler = async ({ request, params, cookies, locals }) => ...) so the
route stays in sync with route-specific params/cookies/locals and still calls
the existing redirect(...) helper; keep the same return behavior
(Promise<Response>) and ensure the handler destructures request from the
provided argument as before.
In `@packages/adapter-sveltekit/src/index.ts`:
- Around line 43-57: The global key __holoSvelteKitRequestEventStore is an
implicit cross-package contract used by getSvelteKitRequestEventStore() in this
file and a similarly named function in packages/auth; to prevent silent
breakage, extract that string into a shared internal constant (e.g.,
HoloSvelteKitRequestEventStoreKey) in a tiny cross-package utility or shared
types module and import it here and in packages/auth, or at minimum add a
prominent co-location comment in both getSvelteKitRequestEventStore()
implementations documenting the exact key, its expected AsyncLocalStorage type
(AsyncLocalStorage<SvelteKitRequestEvent> vs
AsyncLocalStorage<SvelteKitStoredRequestEvent>), and that both files must stay
in sync.
In `@packages/auth/tests/package.test.ts`:
- Around line 774-805: The test only verifies the synchronous
appendResponseCookie path; add a case that registers/configures the runtime with
an async appendResponseCookie implementation (returns a Promise and pushes to
appendedCookies after a small await) and then call register/loginUsing and
logout to assert the test waits for the async hook to complete (i.e.,
appendedCookies contains cookies after loginUsing and logout). Update the
existing test or add a new it block that uses configureAuthRuntime with
context.appendResponseCookie as an async function and asserts appendedCookies
matches established.cookies and then appendedCookies includes loggedOut.cookies
after logout to ensure loginUsing/logout await the hook.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfe8ec65-eca1-46ec-9d47-c6373be814b1
📒 Files selected for processing (42)
apps/blog-next/app/api/login/route.tsapps/blog-next/app/api/logout/route.tsapps/blog-next/app/api/register/route.tsapps/blog-next/app/auth/github/callback/route.tsapps/blog-next/app/auth/github/route.tsapps/blog-next/app/auth/google/callback/route.tsapps/blog-next/app/auth/google/route.tsapps/blog-next/app/login/page.tsxapps/blog-nuxt/app/pages/login.vueapps/blog-nuxt/server/api/login.post.tsapps/blog-nuxt/server/api/logout.post.tsapps/blog-nuxt/server/api/register.post.tsapps/blog-nuxt/server/lib/request.tsapps/blog-nuxt/server/routes/auth/github.get.tsapps/blog-nuxt/server/routes/auth/github/callback.get.tsapps/blog-nuxt/server/routes/auth/google.get.tsapps/blog-nuxt/server/routes/auth/google/callback.get.tsapps/blog-sveltekit/src/routes/api/login/+server.tsapps/blog-sveltekit/src/routes/api/logout/+server.tsapps/blog-sveltekit/src/routes/api/register/+server.tsapps/blog-sveltekit/src/routes/auth/github/+server.tsapps/blog-sveltekit/src/routes/auth/github/callback/+server.tsapps/blog-sveltekit/src/routes/auth/google/+server.tsapps/blog-sveltekit/src/routes/auth/google/callback/+server.tsapps/blog-sveltekit/src/routes/login/+page.svelteapps/docs/docs/auth/social-login.mdpackages/adapter-next/src/config.tspackages/adapter-next/src/runtime.tspackages/adapter-next/tests/adapter.test.tspackages/adapter-nuxt/src/runtime/composables/index.tspackages/adapter-nuxt/tests/module.test.tspackages/adapter-sveltekit/src/index.tspackages/adapter-sveltekit/tests/runtime.test.tspackages/auth-social/src/index.tspackages/auth-social/tests/package.test.tspackages/auth/src/contracts.tspackages/auth/src/runtime.tspackages/auth/src/sveltekit/server.tspackages/auth/tests/package.test.tspackages/core/src/portable/holo.tsscripts/validate-framework-smoke.mjstests/example-app-auth-flow.mjs
💤 Files with no reviewable changes (4)
- apps/blog-nuxt/server/api/register.post.ts
- apps/blog-nuxt/server/api/login.post.ts
- apps/blog-next/app/api/register/route.ts
- apps/blog-sveltekit/src/routes/api/register/+server.ts
Comment on lines
+19
to
+20
| await auth.guard(result.guard).loginUsing(result.user) | ||
| throw redirect(303, '/admin') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the SvelteKit blog app's hooks.server.ts for runWithSvelteKitRequestEvent coverage
fd -e ts 'hooks.server' apps/blog-sveltekit --exec cat {}Repository: cobraprojects/holo-js
Length of output: 409
🏁 Script executed:
# Find the `@holo-js/auth` package implementation of guestOnly and authOnly
fd -type f -name '*.ts' | xargs rg -l 'export.*guestOnly|export.*authOnly' | head -20Repository: cobraprojects/holo-js
Length of output: 488
🏁 Script executed:
# Verify the exact file path of the callback handler
find apps/blog-sveltekit -type f -name '+server.ts' | grep -i callbackRepository: cobraprojects/holo-js
Length of output: 191
🏁 Script executed:
# Check if there are any other references to the callback route or auth routes in hooks
rg 'auth/google|callback' apps/blog-sveltekitRepository: cobraprojects/holo-js
Length of output: 753
🏁 Script executed:
# Read the guestOnly and authOnly implementations
cat packages/auth/src/sveltekit/server.tsRepository: cobraprojects/holo-js
Length of output: 6098
🏁 Script executed:
# Find the adapter-sveltekit implementation
fd -type f -name '*.ts' -path '*/adapter-sveltekit/*' | head -20Repository: cobraprojects/holo-js
Length of output: 236
🏁 Script executed:
# Search for appendResponseCookie implementation
rg -A 10 'appendResponseCookie' packages/auth/src/sveltekitRepository: cobraprojects/holo-js
Length of output: 47
🏁 Script executed:
# Search for appendResponseCookie more broadly
rg 'appendResponseCookie' --max-count=50Repository: cobraprojects/holo-js
Length of output: 2259
🏁 Script executed:
# Find all files in packages/auth
find packages/auth -type f -name '*.ts' | head -20Repository: cobraprojects/holo-js
Length of output: 708
🏁 Script executed:
# Read the adapter-sveltekit implementation
cat packages/adapter-sveltekit/src/index.tsRepository: cobraprojects/holo-js
Length of output: 5695
🏁 Script executed:
# Read the actual callback handler to confirm the flow
cat apps/blog-sveltekit/src/routes/auth/google/callback/+server.tsRepository: cobraprojects/holo-js
Length of output: 636
🏁 Script executed:
# Verify the loginUsing flow by checking auth/src/runtime.ts
rg -A 20 'async function appendResponseCookies' packages/auth/src/runtime.tsRepository: cobraprojects/holo-js
Length of output: 572
🏁 Script executed:
# Check if there's any other mechanism that might populate ALS for the callback route
rg 'runWithSvelteKitRequestEvent' apps/blog-sveltekitRepository: cobraprojects/holo-js
Length of output: 47
Add callback routes to guestOnly guard in hooks.server.ts.
The /auth/google/callback and /auth/github/callback routes are not covered by the guestOnly or authOnly handlers in hooks.server.ts. This means runWithSvelteKitRequestEvent is never called for these routes, so the AsyncLocalStorage context is not established. When loginUsing() calls appendResponseCookies(), the session cookie call reaches appendResponseCookie in the adapter, which silently returns without setting the cookie because getSvelteKitRequestEventStore().getStore() is undefined. The session is created server-side but the cookie is never sent to the client—the user is redirected to /admin but arrives unauthenticated.
Add both callback routes to the guestOnly routes list (or wrap the handler with runWithSvelteKitRequestEvent if they should be auth-required):
guestOnly({
routes: ['/login', '/register', '/forgot-password', '/reset-password', '/auth/google/callback', '/auth/github/callback'],
redirectTo: '/admin',
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/blog-sveltekit/src/routes/auth/google/callback/`+server.ts around lines
19 - 20, The guest-only guard is not covering the OAuth callback routes so
runWithSvelteKitRequestEvent is never invoked and AsyncLocalStorage isn’t
established; update the guestOnly configuration in hooks.server.ts to include
'/auth/google/callback' and '/auth/github/callback' in the routes array (e.g.,
add those strings to the routes passed to guestOnly) or alternatively ensure the
callback handler (the function that calls auth.guard(...).loginUsing and
redirect) is wrapped with runWithSvelteKitRequestEvent so appendResponseCookies
runs with a valid SvelteKit request event store.
apps/blog-sveltekit/src/hooks.server.ts: did not add /auth/google/callback and /auth/github/callback to guestOnly.routes. Reason: after checking the current framework implementation, packages/auth/src/sveltekit/server.ts wraps the entire guestOnly handler body in runWithSvelteKitRequestEvent(event, ...) before it checks matchesRoutes(...). That means even routes not matched by guestOnly.routes, including OAuth callbacks, still run inside the SvelteKit AsyncLocalStorage context when resolve(event) is called. The reported issue said ALS was not established for callbacks unless the callback routes were listed, but that is not true in the current code. apps/blog-sveltekit/src/routes/auth/google/callback/+server.ts: did not wrap the callback handler directly with runWithSvelteKitRequestEvent. Reason: same root cause as above. The framework-level SvelteKit handle already establishes the request-event store before route resolution reaches this callback handler. Adding a second manual wrapper in the route would duplicate framework integration logic in userland/example code, which goes against the repo’s API discipline for auth flows.
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/forms/src/client.ts (1)
133-166: ⚡ Quick win
normalizeFailureInputandnormalizeFailureErrorsare duplicated verbatim incontracts.tsBoth functions (lines 135–168 in
contracts.ts) are identical to this implementation. If one copy diverges — say, to handle a new edge case — server and client failure payloads will silently behave differently.
client.tsalready imports types fromcontracts.ts, so the simplest fix is to export these two utilities fromcontracts.ts(or a dedicated internal module) and import them here instead.♻️ Option A — export from
contracts.tsand re-importIn
contracts.ts, change the private declarations to exported ones:-function normalizeFailureInput(input: FormFailureInput, fallbackStatus: number): { +export function normalizeFailureInput(input: FormFailureInput, fallbackStatus: number): { readonly status: number readonly errors?: FormFailureErrors } { ... } -function normalizeFailureErrors( +export function normalizeFailureErrors( fallback: Record<string, readonly string[]>, override: FormFailureErrors | undefined, ): Record<string, readonly string[]> { ... }In
client.ts, remove the duplicate bodies and import fromcontracts.ts:import type { FormFailureErrors, FormFailureInput, ... } from './contracts' +import { normalizeFailureInput, normalizeFailureErrors } from './contracts'Then delete lines 133–166 of
client.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/forms/src/client.ts` around lines 133 - 166, Duplicate implementations of normalizeFailureInput and normalizeFailureErrors exist in client.ts and contracts.ts; remove the duplication by exporting these utilities from contracts.ts and importing them in client.ts. Modify contracts.ts to export normalizeFailureInput and normalizeFailureErrors (or move both into a shared internal module and export them), then in packages/forms/src/client.ts delete the local function bodies for normalizeFailureInput and normalizeFailureErrors and add imports that reference the exported symbols from contracts.ts (or the shared module) so both client and server use the single shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/adapter-sveltekit/src/index.ts`:
- Around line 27-35: Add support for the Partitioned cookie attribute by adding
a partitioned?: boolean field to the SvelteKitCookieOptions type and updating
the parseResponseCookie switch in parseResponseCookie to handle the
'partitioned' attribute (set options.partitioned = true or parse a boolean if a
value is provided) so the parsed cookie options are passed through to
event.cookies.set(); modify the switch inside the parseResponseCookie function
to include a 'partitioned' case that sets the partitioned flag on the options
object.
In `@packages/forms/src/sensitiveInput.ts`:
- Around line 103-109: The code in sanitizeFlashedInput creates only a shallow
clone (output = Object.fromEntries(...)) so deletePath(output, path) mutates
nested structures inside the original values; change to a deep clone before
deleting sensitive nested paths (e.g., use structuredClone(values) or your
project deepClone util) and then run deletePath against that deep-cloned output;
apply the same deep-clone fix to the second occurrence around the 114-119 block
so callers receive a non-mutating sanitized object (referencing
variables/functions: sanitizeFlashedInput, output, values, sensitivePaths, and
deletePath).
---
Nitpick comments:
In `@packages/forms/src/client.ts`:
- Around line 133-166: Duplicate implementations of normalizeFailureInput and
normalizeFailureErrors exist in client.ts and contracts.ts; remove the
duplication by exporting these utilities from contracts.ts and importing them in
client.ts. Modify contracts.ts to export normalizeFailureInput and
normalizeFailureErrors (or move both into a shared internal module and export
them), then in packages/forms/src/client.ts delete the local function bodies for
normalizeFailureInput and normalizeFailureErrors and add imports that reference
the exported symbols from contracts.ts (or the shared module) so both client and
server use the single shared implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4551257-3857-4392-9c76-e361b93e6560
📒 Files selected for processing (47)
apps/blog-next/app/api/forgot-password/route.tsapps/blog-next/app/api/login/route.tsapps/blog-next/app/api/register/route.tsapps/blog-next/app/api/reset-password/route.tsapps/blog-next/app/api/verify-email/route.tsapps/blog-next/lib/schemas/auth.tsapps/blog-nuxt/server/api/forgot-password.post.tsapps/blog-nuxt/server/api/login.post.tsapps/blog-nuxt/server/api/register.post.tsapps/blog-nuxt/server/api/reset-password.post.tsapps/blog-nuxt/server/api/verify-email.post.tsapps/blog-nuxt/shared/schemas/auth.tsapps/blog-sveltekit/src/lib/schemas/auth.tsapps/blog-sveltekit/src/routes/api/forgot-password/+server.tsapps/blog-sveltekit/src/routes/api/login/+server.tsapps/blog-sveltekit/src/routes/api/register/+server.tsapps/blog-sveltekit/src/routes/api/reset-password/+server.tsapps/blog-sveltekit/src/routes/api/verify-email/+server.tsapps/blog-sveltekit/src/routes/auth/google/+server.tsapps/docs/docs/auth/index.mdapps/docs/docs/auth/local-auth.mdapps/docs/docs/auth/social-login.mdapps/docs/docs/forms/client-usage.mdapps/docs/docs/forms/framework-integration.mdapps/docs/docs/forms/index.mdapps/docs/docs/forms/server-validation.mdapps/docs/docs/security.mdapps/docs/docs/validation/index.mdapps/docs/docs/validation/rules-and-errors.mdpackages/adapter-next/src/config.tspackages/adapter-next/tests/adapter.test.tspackages/adapter-sveltekit/src/index.tspackages/auth/src/sveltekit/server.tspackages/auth/tests/package.test.tspackages/forms/src/client.tspackages/forms/src/contracts.tspackages/forms/src/index.tspackages/forms/src/sensitiveInput.tspackages/forms/tests/client.test.tspackages/forms/tests/client.type.test.tspackages/forms/tests/contracts.test.tspackages/forms/tests/docs-examples.test.tspackages/validation/src/contracts-support.tspackages/validation/src/contracts-types.tspackages/validation/src/contracts.tspackages/validation/tests/contracts.test.tstests/example-app-auth-flow.mjs
✅ Files skipped from review due to trivial changes (3)
- apps/docs/docs/validation/rules-and-errors.md
- apps/docs/docs/forms/client-usage.md
- apps/docs/docs/auth/social-login.md
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/blog-nuxt/server/api/login.post.ts
- apps/blog-nuxt/server/api/register.post.ts
- apps/blog-sveltekit/src/routes/api/register/+server.ts
- apps/blog-next/app/api/login/route.ts
- apps/blog-sveltekit/src/routes/api/login/+server.ts
- packages/adapter-next/src/config.ts
- tests/example-app-auth-flow.mjs
- packages/auth/src/sveltekit/server.ts
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/forms/tests/contracts.test.ts`:
- Around line 283-299: Add a test that disambiguates replace-vs-merge semantics
by asserting that fail({ errors }) merges override errors with validation errors
rather than replacing them: when calling the same test helper used in this file
(failure.fail) simulate validation producing an error on a field the override
does not touch (e.g. token: ['Missing token']) and the override only setting
email errors, then assert the returned payload contains both token and email
errors. Update the assertion in packages/forms/tests/contracts.test.ts to
reference failure.fail and ensure normalizeFailureErrors (from contracts.ts)
preserves/merges existing validation errors into the final errors object rather
than dropping them.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0458add-21df-43e7-a343-8ad3a4a6b398
📒 Files selected for processing (6)
packages/adapter-sveltekit/src/index.tspackages/adapter-sveltekit/tests/runtime.test.tspackages/forms/src/client.tspackages/forms/src/contracts.tspackages/forms/src/sensitiveInput.tspackages/forms/tests/contracts.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/forms/src/sensitiveInput.ts
- packages/adapter-sveltekit/src/index.ts
- packages/forms/src/client.ts
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
This was referenced May 9, 2026
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 by CodeRabbit
New Features
Documentation
Refactor
Bug Fixes