chore(examples): update demo user interfaces#145
Merged
Conversation
Contributor
|
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 16 minutes and 10 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR standardizes authentication API contracts across Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant Browser as Browser
participant Server as Server/API
participant Database as Session/Database
User->>Browser: Submit credentials (username/password)
Browser->>Server: POST /api/auth/signInCredentials
Server->>Server: Validate credentials
Server->>Server: Generate sub via createSecretValue
Server->>Database: Store session with user identity
Database-->>Server: Session created
Server-->>Browser: Return success + redirectURL or signInURL
alt Redirect Enabled (redirect: true)
Browser->>Browser: Redirect to redirectURL
else Manual Redirect (redirect: false)
Browser->>Server: Check response.signInURL
Browser->>Browser: Handle redirect programmatically
end
User->>Browser: Edit profile (username/email)
Browser->>Server: POST updateSession with FormData
Server->>Server: Extract username/email from FormData
Server->>Database: Update session.user.name/email
Database-->>Server: Session updated
Server-->>Browser: Return success + optional redirectURL
Browser->>Browser: Refresh session state
User->>Browser: Click Sign Out
Browser->>Server: POST signOut with CSRF token
Server->>Database: Invalidate session token
Database-->>Server: Session cleared
Server-->>Browser: Return success + redirectURL
Browser->>Browser: Redirect and clear auth state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 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: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/nuxt/app/components/header.vue (1)
24-45:⚠️ Potential issue | 🟡 MinorInconsistent
rel="noopener noreferrer"on external links.Only the Discord anchor received
rel="noopener noreferrer"; the Documentation (Line 24-30) and Repository (Line 31-37) anchors still usetarget="_blank"without it, which is the same security/performance concern the PR intends to address. Apply consistently.Proposed fix
<a href="https://aura-stack-auth.vercel.app/docs" class="text-sm text-muted-foreground hover:text-white transition-colors" target="_blank" + rel="noopener noreferrer" > Documentation </a> <a href="https://github.com/aura-stack-ts/auth" class="text-sm text-muted-foreground hover:text-white transition-colors" target="_blank" + rel="noopener noreferrer" > Repository </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nuxt/app/components/header.vue` around lines 24 - 45, The Documentation and Repository anchor tags in header.vue open external links with target="_blank" but lack rel="noopener noreferrer"; update the two anchors whose inner text is "Documentation" and "Repository" to include rel="noopener noreferrer" (matching the existing Discord anchor) to prevent window.opener security issues and improve performance.packages/next/src/lib/api.ts (1)
19-26:⚠️ Potential issue | 🟠 MajorPreserve
Set-Cookieattributes when syncing auth cookies.
applyCookiescurrently keeps onlyname=value, droppingHttpOnly,Secure,SameSite,Path,Expires, andMax-Age. That can weaken session-cookie security and can also make sign-out unreliable when the auth API sends expiry cookies.Proposed safer parsing shape
async function applyCookies(responseHeaders: Headers) { const cookieStore = await cookies() const setCookies = responseHeaders.getSetCookie() for (const cookieStr of setCookies) { - const [nameValue] = cookieStr.split(";").map((s) => s.trim()) - const [name, value] = nameValue.split("=") - cookieStore.set(name, value, {}) + const [nameValue, ...attributes] = cookieStr.split(";").map((s) => s.trim()) + const separatorIndex = nameValue.indexOf("=") + if (separatorIndex === -1) continue + + const name = nameValue.slice(0, separatorIndex) + const value = nameValue.slice(separatorIndex + 1) + const options = {} as { + path?: string + domain?: string + expires?: Date + maxAge?: number + httpOnly?: boolean + secure?: boolean + sameSite?: "lax" | "strict" | "none" + } + + for (const attribute of attributes) { + const [rawKey, ...rawValue] = attribute.split("=") + const key = rawKey.toLowerCase() + const attrValue = rawValue.join("=") + + if (key === "path") options.path = attrValue + else if (key === "domain") options.domain = attrValue + else if (key === "expires") options.expires = new Date(attrValue) + else if (key === "max-age") options.maxAge = Number(attrValue) + else if (key === "httponly") options.httpOnly = true + else if (key === "secure") options.secure = true + else if (key === "samesite") options.sameSite = attrValue.toLowerCase() as "lax" | "strict" | "none" + } + + cookieStore.set(name, value, options) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/src/lib/api.ts` around lines 19 - 26, applyCookies currently strips all Set-Cookie attributes and only writes name/value to cookieStore, losing HttpOnly/Secure/SameSite/Path/Expires/Max-Age which breaks security and sign-out; update applyCookies to parse each cookie string (in applyCookies) into name, value and attribute map and pass the appropriate options into cookieStore.set (e.g., httpOnly, secure, path, sameSite, expires or maxAge) so attributes are preserved when syncing auth cookies; you can implement this by splitting on ";" after the first name=value, normalizing attribute names and values, converting Expires to a Date and Max-Age to seconds, and mapping SameSite to the expected enum before calling cookieStore.set(name, value, options).apps/tanstack-start/src/components/footer.tsx (1)
63-91:⚠️ Potential issue | 🟡 MinorWrap footer links in list items.
The
<ul>elements currently render<a>as direct children. Use<li>wrappers so the footer keeps valid list semantics for assistive technologies.Proposed fix
{resources.map(({ href, title }, i) => ( - <a - key={i} - className="w-max py-1 text-xs text-muted-foreground duration-200 hover:underline" - href={href} - target="_blank" - rel="noopener noreferrer" - > - {title} - </a> + <li key={i}> + <a + className="w-max py-1 text-xs text-muted-foreground duration-200 hover:underline" + href={href} + target="_blank" + rel="noopener noreferrer" + > + {title} + </a> + </li> ))} @@ {community.map(({ href, title }, i) => ( - <a - key={i} - className="w-max py-1 text-xs text-muted-foreground duration-200 hover:underline" - href={href} - target="_blank" - rel="noopener noreferrer" - > - {title} - </a> + <li key={i}> + <a + className="w-max py-1 text-xs text-muted-foreground duration-200 hover:underline" + href={href} + target="_blank" + rel="noopener noreferrer" + > + {title} + </a> + </li> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/components/footer.tsx` around lines 63 - 91, The footer currently renders anchors directly inside <ul> for the resources and community lists (mapping over resources and community); wrap each <a> in an <li> instead to preserve valid list semantics and assistive tech support: move the key={i} to the <li> (e.g., <li key={i}>) and keep the existing className, href, target, rel and inner {title} on the <a> inside the <li>, ensuring the <ul className="flex flex-col gap-1"> children are <li> elements rather than anchors.
🟠 Major comments (17)
packages/core/package.json-30-30 (1)
30-30:⚠️ Potential issue | 🟠 MajorRemove
sideEffects: true—this package has no top-level side effects.The current setting disables tree-shaking for all consumers. Analysis shows all
windowandsetTimeoutusages are inside function bodies, not at module initialization. Since the core package has no intentional top-level side effects, change this to:- "sideEffects": true, + "sideEffects": false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/package.json` at line 30, Update the package.json "sideEffects" setting for the core package: replace the current "sideEffects": true entry with "sideEffects": false (or remove the field entirely) so bundlers can safely tree-shake this package; locate the "sideEffects" key in packages/core/package.json and change its value accordingly.apps/nextjs/pages-router/src/components/edit-profile.tsx-55-60 (1)
55-60:⚠️ Potential issue | 🟠 MajorCancel button submits the form instead of closing the dialog.
Both buttons are inside a
<form>element (line 31) withaction={updateSession}. Without an explicittypeattribute,<button>defaults totype="submit"inside a form. This means clicking "Cancel" will trigger the form submission beforesetIsOpen(false)runs. Settype="button"on Cancel andtype="submit"on Edit for explicit intent.Proposed fix
- <Button className="w-full mt-6" variant="default"> + <Button type="submit" className="w-full mt-6" variant="default"> Edit </Button> - <Button className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> + <Button type="button" className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> Cancel </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/components/edit-profile.tsx` around lines 55 - 60, The Cancel button is submitting the form because it lacks an explicit type; update the JSX for the two Button components inside the form (the Edit button and the Cancel button rendered by Button) to be explicit: set the Edit Button to type="submit" and the Cancel Button to type="button" (the Cancel handler uses setIsOpen(false)). Locate the Button components (named Button) inside the form that uses action={updateSession} and add the type attributes accordingly so Cancel does not trigger the updateSession submission.apps/tanstack-start/src/components/edit-profile.tsx-54-61 (1)
54-61:⚠️ Potential issue | 🟠 MajorCancel button will submit the form.
Inside a
<form>, buttons default totype="submit". The CancelButtonon Line 58 has no explicit type, so clicking it will triggerupdateSession(submitting the form) in addition to closing the panel. Settype="button"on Cancel (and ideally make the Edit button explicitlytype="submit").Proposed fix
- <Button className="w-full mt-6" variant="default"> + <Button type="submit" className="w-full mt-6" variant="default"> Edit </Button> - <Button className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> + <Button type="button" className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> Cancel </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/components/edit-profile.tsx` around lines 54 - 61, The Cancel Button inside the form is missing an explicit type so it defaults to type="submit" and unintentionally triggers updateSession; update the JSX for the two buttons in edit-profile (the Button components around the Edit and Cancel labels) to set the Cancel button to type="button" and explicitly set the Edit button to type="submit" so Cancel only calls setIsOpen(false) and Edit performs the form submission.packages/next/src/lib/api.ts-65-76 (1)
65-76:⚠️ Potential issue | 🟠 MajorTie the generic to the input parameter and honor
redirect: false.Line 66 should declare
options: Oinstead ofoptions: SignInCredentialsAPIOptions. The genericOis constrained but not applied to the parameter, so the actual options type doesn't propagate to the return typeNextSignInCredentials<O>, causing type drift.Additionally, line 74 only checks
options?.redirectTobefore redirecting. It should also checkoptions.redirect !== false, matching the pattern used in React Router (packages/react-router/src/lib/api.ts:57) and respecting the contract ofNextSignInCredentials<O>, which returnsneverwhenredirect: trueis passed.Proposed adjustment
export const signInCredentials = <DefaultUser extends User = User>({ api }: AuthInstance<DefaultUser>) => { return async <O extends SignInCredentialsAPIOptions>( - options: SignInCredentialsAPIOptions + options: O ): Promise<NextSignInCredentials<O>> => { const signIn = await api.signInCredentials({ headers: await headers(), ...options, payload: options.payload, }) await applyCookies(signIn.headers) - if (signIn.success && options?.redirectTo) { + if (signIn.success && options.redirect !== false && signIn.redirectURL) { redirect(signIn.redirectURL) } return signIn as NextSignInCredentials<O> } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/src/lib/api.ts` around lines 65 - 76, Change the parameter type of the returned async function from SignInCredentialsAPIOptions to the generic O (i.e. options: O) so the input options propagate to the return type NextSignInCredentials<O>; keep the constraint O extends SignInCredentialsAPIOptions. After receiving signIn and applying applyCookies(signIn.headers), only call redirect(signIn.redirectURL) when options.redirect !== false AND options?.redirectTo is truthy (mirror the react-router pattern) so cases with redirect: false are honored. Ensure references in this function to api.signInCredentials, headers(), applyCookies, redirectTo, and redirect remain unchanged except for the type and conditional redirect check.apps/react-router/app/components/edit-profile.tsx-54-60 (1)
54-60:⚠️ Potential issue | 🟠 MajorSet explicit form button types.
Inside a form, an untyped native button submits by default. The
Buttoncomponent spreads props without applying a default type, so "Cancel" will callupdateSessionand update the session instead of only closing the form. Make both buttons explicit.🐛 Proposed fix
- <Button className="w-full mt-6" variant="default"> + <Button className="w-full mt-6" variant="default" type="submit"> Edit </Button> - <Button className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> + <Button className="w-full mt-6" variant="secondary" type="button" onClick={() => setIsOpen(false)}> Cancel </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/components/edit-profile.tsx` around lines 54 - 60, The two Button instances in the EditProfile component are missing explicit button types so the "Cancel" button (which calls setIsOpen(false)) will act as a submit; update both Button usages to include explicit type props: set the Edit Button (which triggers the form submit handler like updateSession) to type="submit" and the Cancel Button to type="button" to prevent implicit submission.apps/nextjs/app-router/src/components/edit-profile.tsx-55-60 (1)
55-60:⚠️ Potential issue | 🟠 MajorAdd explicit form button types.
Inside a form, an untyped native button submits by default. The
Buttoncomponent spreads props without a defaulttype, so the Cancel button will callupdateSessioninstead of only closing the form.Proposed fix
- <Button className="w-full mt-6" variant="default"> + <Button className="w-full mt-6" variant="default" type="submit"> Edit </Button> - <Button className="w-full mt-6" variant="secondary" onClick={() => setIsOpen(false)}> + <Button className="w-full mt-6" variant="secondary" type="button" onClick={() => setIsOpen(false)}> Cancel </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/components/edit-profile.tsx` around lines 55 - 60, The Cancel button in the form is missing an explicit type so it defaults to submit and triggers updateSession; update the two buttons inside the grid so the primary "Edit" Button has type="submit" and the Cancel Button (the one with onClick={() => setIsOpen(false)}) has type="button" (ensure the Button component accepts/passes through the type prop if needed) to prevent accidental form submission.apps/tanstack-start/src/routes/index.tsx-69-77 (1)
69-77:⚠️ Potential issue | 🟠 MajorUse a native anchor for the external card link.
The
flow[0].hrefis an absolute URL (https://aura-stack-auth.vercel.app/), but the code renders it through TanStack Router'sLinkcomponent. TheLinkcomponent'stoprop does not support absolute external URLs and will produce TypeScript errors. Use a native<a href>for external navigation instead. Keep the routerLinkfor internal routes like/serverand/client.🐛 Proposed fix
<Button className="w-min mt-6" variant="outline" asChild> - <Link - to={item.href} - target={item.external ? "_blank" : undefined} - rel={item.external ? "noopener noreferrer" : undefined} - > - Try Now - </Link> + {item.external ? ( + <a href={item.href} target="_blank" rel="noopener noreferrer"> + Try Now + </a> + ) : ( + <Link to={item.href}>Try Now</Link> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/routes/index.tsx` around lines 69 - 77, Replace the TanStack Router Link usage for absolute/external URLs: inside the Button/asChild block, check item.external (or specifically flow[0].href when applicable) and when true render a native anchor element with href={item.href} and proper target={item.external ? "_blank" : undefined} and rel={item.external ? "noopener noreferrer" : undefined}; otherwise keep using the router Link with to={item.href}. Update the conditional rendering around Button -> Link/anchor to avoid passing absolute URLs into the Link component (which causes TypeScript errors).apps/nextjs/pages-router/src/pages/client/index.tsx-79-87 (1)
79-87:⚠️ Potential issue | 🟠 MajorPrevent the sign-out button from submitting the form.
Inside a form, a button defaults to
type="submit". This can trigger a native form submission/reload whilesignOut()is still running.Proposed fix
-<Button className="w-20" variant="default" onClick={handleSignOut}> +<Button className="w-20" variant="default" type="button" onClick={handleSignOut}> Sign Out </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/client/index.tsx` around lines 79 - 87, The Sign Out Button inside the form defaults to submitting the form; modify the Button usage in pages/client/index.tsx (the Button that calls handleSignOut) to explicitly set type="button" so clicking it does not trigger form submission or page reload while signOut() runs; ensure you update the Button element that currently calls handleSignOut to include the type attribute and keep the onClick handler unchanged.apps/tanstack-start/src/routes/server.tsx-31-47 (1)
31-47:⚠️ Potential issue | 🟠 MajorRefresh loader-backed auth state after credentials sign-in/sign-out.
This page derives
isAuthenticatedfrom loader data, but these server mutations do not invalidate the route. If the server function returns without a full redirect, the UI can keep showing the stale pre-mutation session.Proposed fix
const handleSignOut = async () => { await signOut() + await router.invalidate() } const handleSignInCredentials = async (event: FormEvent<HTMLFormElement>) => { event.preventDefault() const formData = new FormData(event.currentTarget) @@ }, }) + await router.invalidate() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/routes/server.tsx` around lines 31 - 47, The loader-backed auth state isn't refreshed after calling signOut() or signInCredentials(), so update handleSignOut and handleSignInCredentials to trigger a loader revalidation once the async auth call completes (e.g., get a revalidator via useRevalidator() or the equivalent route revalidation hook and call revalidator.revalidate() after the await). Specifically, after the await signOut() in handleSignOut and after the await signInCredentials(...) in handleSignInCredentials, invoke the route revalidation method so the loader-derived isAuthenticated value is refreshed.apps/nextjs/pages-router/src/pages/client/index.tsx-57-64 (1)
57-64:⚠️ Potential issue | 🟠 MajorConfigure
images.remotePatternsfor OAuth avatar hosts or use a plain<img>tag.The app supports GitHub, Gitlab, and Bitbucket OAuth providers, which return external avatar URLs from
github.com,gitlab.com, andbitbucket.com. Withoutimages.remotePatternsorimages.domainsinnext.config.ts, thenext/imagecomponent will throw at runtime when attempting to load these external avatars.Either configure the image hosts in
next.config.ts:{ images: { remotePatterns: [ { protocol: 'https', hostname: 'avatars.githubusercontent.com' }, { protocol: 'https', hostname: 'gitlab.com' }, { protocol: 'https', hostname: 'bitbucket.org' }, ], }, }Or replace the
next/imagecomponent with a standard<img>tag for user-provided URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/client/index.tsx` around lines 57 - 64, The Next.js Image usage in the client page (the Image component rendering session.user.image in pages/client/index.tsx) will fail for external OAuth avatars from github.com, gitlab.com, and bitbucket.com unless those hosts are whitelisted; update the Next.js image config by adding images.remotePatterns (or images.domains) in next.config.js to include avatars.githubusercontent.com, gitlab.com, and bitbucket.org, or replace the next/image usage in the component that renders session.user.image with a plain <img> tag to accept external URLs; locate the Image usage in this file (the JSX that checks session?.user?.image) and apply one of these fixes.apps/nuxt/app/composables/useAuthClient.ts-14-16 (1)
14-16:⚠️ Potential issue | 🟠 MajorDo not ignore a provided
initialSession.If the shared
aura-sessionstate already contains stale data, a server-providednullor updated session is skipped andonMountedalso avoids refreshing. That can render an authenticated UI after the server determined there is no session.Proposed fix
- if (initialSession !== undefined && session.value === null) { + if (initialSession !== undefined) { session.value = initialSession } @@ onMounted(async () => { - if (!session.value) { + if (initialSession === undefined && !session.value) { await refresh() } })Also applies to: 73-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nuxt/app/composables/useAuthClient.ts` around lines 14 - 16, The code currently only assigns initialSession when initialSession !== undefined AND session.value === null, which ignores server-provided nulls or updated sessions and leaves stale state; change the logic to assign session.value whenever initialSession !== undefined (so that null overwrites stale session), i.e. replace the conditional that checks session.value === null with a simple initialSession !== undefined check for the assignment; apply the same change to the other occurrence (the block around lines 73-76) so both spots that reference initialSession and session.value use the corrected condition and ensure any onMounted refresh logic relies on this updated session value.apps/nextjs/pages-router/src/pages/server/index.tsx-10-19 (1)
10-19:⚠️ Potential issue | 🟠 MajorReturn the fetched session as
initialSession.
getServerSidePropscallsapi.getSession(...)but then discards the result, so the SSR page always hydrates as unauthenticated.Proposed fix
return { props: { - initialSession: null, + initialSession: session, }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/server/index.tsx` around lines 10 - 19, getServerSideProps calls api.getSession(...) but always returns initialSession: null; update the return to pass the fetched session so the page hydrates authenticated. Specifically, in getServerSideProps replace the hardcoded initialSession: null with initialSession: session (the value returned by api.getSession) so the server-provided session is included in props for hydration.apps/nextjs/app-router/src/app/server/page.tsx-17-45 (1)
17-45:⚠️ Potential issue | 🟠 MajorRedirect back to the
/serverroute, not/ssr.This page is mounted at
/server, but sign-out, credentials sign-in, and session update redirect to/ssr, which makes the flow land on the wrong route.Proposed fix
await api.signOut({ - redirectTo: "/ssr", + redirectTo: "/server", }) @@ payload: { username, password, }, - redirectTo: "/ssr", + redirectTo: "/server", }) @@ - redirect("/ssr") + redirect("/server")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/app/server/page.tsx` around lines 17 - 45, The redirect targets in signOutAction, signInCredentialsAction, and updateSessionAction are pointing to "/ssr" but this page is mounted at "/server"; update each call to use the correct route by changing redirectTo: "/ssr" to redirectTo: "/server" in api.signOut and api.signInCredentials, and change redirect("/ssr") to redirect("/server") in updateSessionAction so all flows return to the /server route.apps/react-router/app/routes/client/index.tsx-77-87 (1)
77-87:⚠️ Potential issue | 🟠 MajorAdd
type="button"to prevent the button from submitting the surrounding form.The button on line 84 is nested inside a
<form>element. Without an explicittypeattribute, it defaults totype="submit", which will trigger a form submission in addition to executingonClick={handleSignOut}. The Button component properly forwards the native HTMLtypeprop to the underlying<button>element, so the fix is straightforward:Proposed fix
- <Button className="w-20" variant="default" onClick={handleSignOut}> + <Button className="w-20" variant="default" type="button" onClick={handleSignOut}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/routes/client/index.tsx` around lines 77 - 87, The Sign Out Button inside the surrounding <form> is missing an explicit type and thus defaults to type="submit", causing an unwanted form submission when clicked; update the Button instance (the one with className="w-20" variant="default" onClick={handleSignOut}) to include type="button" so it only invokes handleSignOut and does not submit the form (the Button component already forwards native button props).apps/nextjs/pages-router/src/pages/server/index.tsx-93-103 (1)
93-103:⚠️ Potential issue | 🟠 MajorSet the sign-out button to
type="button".The button defaults to
type="submit"within the form, which can trigger form submission and cause a page reload whilesignOut()is executing. Since the Button component forwards props to the underlying button element, addingtype="button"will prevent unwanted form submission behavior.Proposed fix
- <Button className="w-20" variant="default" onClick={handleSignOut}> + <Button className="w-20" variant="default" type="button" onClick={handleSignOut}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/server/index.tsx` around lines 93 - 103, The Sign Out Button inside the form is missing an explicit type, causing it to act as a submit button and potentially trigger form submission when handleSignOut runs; update the Button component usage (the Button element that calls handleSignOut in pages/server/index.tsx) to include type="button" so it does not submit the surrounding <form> and will only invoke the handleSignOut handler.apps/tanstack-start/src/routes/client.tsx-83-93 (1)
83-93:⚠️ Potential issue | 🟠 MajorAdd
type="button"to prevent form submission.Without an explicit
typeattribute, the button defaults totype="submit"and will submit the parent form. Since the form has noonSubmithandler, the submission will trigger a full page reload, interrupting thehandleSignOut()execution.Proposed fix
- <Button className="w-20" variant="default" onClick={handleSignOut}> + <Button className="w-20" variant="default" type="button" onClick={handleSignOut}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/routes/client.tsx` around lines 83 - 93, The Sign Out Button inside the form is being treated as a submit button by the browser, causing a full page reload and interrupting handleSignOut; update the Button component (the one rendering Sign Out with onClick={handleSignOut}) to include type="button" so it does not submit the parent form (alternatively convert the surrounding <form> to a <div>, but preferred fix is adding the type prop to the Button).apps/nextjs/app-router/src/app/client/page.tsx-80-90 (1)
80-90:⚠️ Potential issue | 🟠 MajorAdd
type="button"to prevent accidental form submission.The sign-out button sits inside a form without specifying
type="button", causing it to default totype="submit". This triggers form submission and can reload the page whilehandleSignOut()is running.Proposed fix
- <Button className="w-20" variant="default" onClick={handleSignOut}> + <Button className="w-20" variant="default" type="button" onClick={handleSignOut}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/app/client/page.tsx` around lines 80 - 90, The Sign Out button inside the form is missing an explicit type and defaults to submitting the form; update the Button component usage in the form that contains handleSignOut to include type="button" so clicking it invokes handleSignOut without triggering form submission or a page reload; locate the JSX form and the Button element (the one with onClick={handleSignOut}) and add the type prop to that Button.
🟡 Minor comments (10)
apps/nextjs/app-router/src/components/footer.tsx-36-36 (1)
36-36:⚠️ Potential issue | 🟡 MinorAlign the footer bottom row with the widened wrapper.
Line 36 widens the footer shell to
max-w-6xl, but Line 84 still caps the copyright row atmax-w-5xl, unlike the matching footer updates in the other examples. This can leave the bottom row visually misaligned with the new border/header width.🎨 Proposed alignment fix
- <div className="max-w-5xl py-4 flex flex-col justify-between gap-2"> + <div className="max-w-6xl py-4 flex flex-col justify-between gap-2">Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/components/footer.tsx` at line 36, The footer's outer wrapper was widened to max-w-6xl but the bottom copyright row still uses max-w-5xl, causing misalignment; in the Footer component locate the element whose className contains "max-w-5xl" (the copyright/ bottom row) and change it to "max-w-6xl" so the bottom row matches the widened wrapper's width and border alignment.apps/nextjs/pages-router/src/lib/auth.ts-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorSame locale-aware lowercasing issue as in the app-router
auth.ts.Switch
toLocaleLowerCase()totoLowerCase()to avoid locale-dependent casing (e.g., TurkishI→ı) when building the email.Proposed fix
- email: `${username.toLocaleLowerCase()}@example.com`, + email: `${username.toLowerCase()}@example.com`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/lib/auth.ts` at line 20, Replace the locale-aware casing call used when constructing the email: change the use of username.toLocaleLowerCase() in the auth email construction (the line setting email: `${username.toLocaleLowerCase()}@example.com`) to username.toLowerCase() so the generated email uses locale-independent lowercasing; update the occurrence inside the auth builder/function in auth.ts accordingly.apps/nextjs/app-router/src/lib/auth.ts-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorUse
toLowerCase()instead oftoLocaleLowerCase()for email normalization.Email local-parts are ASCII and locale-sensitive lowercasing can yield unexpected results (e.g., Turkish locale maps
I→ı). For normalizing identifiers like emails, prefertoLowerCase().Proposed fix
- email: `${username.toLocaleLowerCase()}@example.com`, + email: `${username.toLowerCase()}@example.com`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/lib/auth.ts` at line 20, Replace the locale-aware lowercasing call on the username used for generating the email — change the template string that currently uses username.toLocaleLowerCase() to use username.toLowerCase() instead so email normalization uses locale-independent lowercasing; locate the occurrence in the email template (the template string with `${username.toLocaleLowerCase()}@example.com`) in auth.ts and update it to `${username.toLowerCase()}@example.com`.apps/tanstack-start/src/components/edit-profile.tsx-16-18 (1)
16-18:⚠️ Potential issue | 🟡 Minor
htmlFor="signout"points to a non-existent element.The label targets
id="signout"but there is no such input in this component (this is the Edit Profile section). Either removehtmlForor point it to the intended control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/components/edit-profile.tsx` around lines 16 - 18, The label in the Edit Profile component uses htmlFor="signout" which doesn't match any element id; update the label to either remove the htmlFor attribute or change it to the correct target id for the intended control (e.g., the input/button related to editing profile), locating the label element rendered in the Edit Profile component (the <label className="font-medium block" htmlFor="signout"> element) and ensure the referenced id exists on the corresponding input/button or remove htmlFor if no explicit control should be associated.apps/tanstack-start/src/lib/auth-server.ts-89-98 (1)
89-98:⚠️ Potential issue | 🟡 MinorRename the input field from
usernametonamefor consistency with the session schema.The code correctly maps to
user.nameas required by the session schema (which defines only anamefield, notusername). However, the input validator acceptingusernameis inconsistent with the output. Rename the parameter tonameto align with the session's actual user structure:Current state
.inputValidator((data: { username?: string; email?: string }) => data) .handler(async ({ data }) => { const response = await api .updateSession({ session: { user: { name: data.username, email: data.email, },The session user object expects
name(notusername), so the input parameter should reflect this naming for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/lib/auth-server.ts` around lines 89 - 98, Update the input shape used by the route so it uses name instead of username: change the inputValidator signature/type from { username?: string; email?: string } to { name?: string; email?: string } and in the handler update the mapping passed to api.updateSession so it assigns user.name = data.name (and user.email = data.email remains unchanged); update any references inside the same handler/function (e.g., the .inputValidator and .handler that call updateSession) to use data.name for consistency with the session schema.apps/tanstack-start/src/styles.css-5-17 (1)
5-17:⚠️ Potential issue | 🟡 MinorFix the stylelint failures in the font stack.
Stylelint is flagging the changed
bodyblock for the declaration spacing and unnecessary quotes around single-word font family names.Proposed stylelint-compatible adjustment
body { `@apply` m-0; + font-family: - -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", - "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif; + -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen, Ubuntu, + Cantarell, "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/styles.css` around lines 5 - 17, The body and code font-family declarations violate stylelint rules: remove unnecessary quotes from single-word font family names and normalize spacing/commas; in the body selector (font-family) unquote single-word names like "Roboto" and "sans-serif" and ensure each family is comma + space separated and the property uses a single contiguous value (you can keep multi-word names quoted, e.g., "Segoe UI", "Helvetica Neue"); do the same for the code selector (unquote source-code-pro and other single-word names) so both font-family properties conform to stylelint spacing and quoting rules.apps/nuxt/server/api/auth/session.get.ts-4-6 (1)
4-6:⚠️ Potential issue | 🟡 MinorThe type cast masks potential
undefinedheader values from h3'sgetHeaders().h3's
getHeaders(event)returnsRecord<string, string | string[] | undefined>, but the cast toRecord<string, string>suppresses the possibility ofundefinedvalues and multi-value headers. While theHeadersconstructor likely tolerates this in practice, the cast hides a real type mismatch and makes intent unclear.🔧 Suggested approach
Either pass headers directly (as other frameworks do) if the core API tolerates the h3 type, or filter undefined values:
- const response = await api.getSession({ - headers: getHeaders(event) as Record<string, string>, - }) + const headers = Object.fromEntries( + Object.entries(getHeaders(event)).filter(([, v]) => v !== undefined) + ) as Record<string, string> + const response = await api.getSession({ headers })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nuxt/server/api/auth/session.get.ts` around lines 4 - 6, The getHeaders(event) call is being unsafely cast to Record<string,string> before passing to api.getSession; remove the cast and either pass the raw headers through if api.getSession accepts Record<string,string|string[]|undefined>, or normalize them first by converting string[] to a single string (e.g., join with ",") and stripping undefined values into a Record<string,string> before calling api.getSession; update the call site using getHeaders and api.getSession to perform this normalization (or pass raw headers) so the type mismatch is resolved and multi-value/undefined headers are handled explicitly.apps/nextjs/pages-router/src/pages/client/index.tsx-3-14 (1)
3-14:⚠️ Potential issue | 🟡 MinorAdd
typekeyword to the import statement.
SubmitEventis properly exported from React 19.2.10+ (@types/react19.2.14) and correctly used throughout the codebase. However, type-only imports should use thetypekeyword for clarity and to avoid unnecessary runtime bundling.Proposed fix
-import { SubmitEvent } from "react" +import type { SubmitEvent } from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/client/index.tsx` around lines 3 - 14, The import of SubmitEvent should be a type-only import to avoid runtime bundling; update the import statement that currently reads "import { SubmitEvent } from 'react'" to use the type keyword (import type { SubmitEvent } from 'react') so that the type-only symbol used in handleSignInCredentials (and elsewhere in this file) is erased at compile time and does not produce runtime import overhead.apps/nuxt/app/composables/useAuthClient.ts-18-24 (1)
18-24:⚠️ Potential issue | 🟡 MinorMake
refresh()drive the loading state used bystatus.Initial client refreshes currently leave
isLoadingasfalse, sostatusreports"unauthenticated"until the request finishes instead of"loading".Proposed fix
const refresh = async () => { + isLoading.value = true try { session.value = await authClient.getSession() } catch { session.value = null + } finally { + isLoading.value = false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nuxt/app/composables/useAuthClient.ts` around lines 18 - 24, The refresh() function should drive the isLoading state so status reflects "loading" during the fetch: set isLoading (the reactive flag used by status) to true immediately before calling await authClient.getSession(), then set isLoading back to false in a finally block after the await; update session.value with the result on success and set it to null on error (catch) as before, ensuring isLoading is always cleared even if authClient.getSession() throws. Target symbols: refresh(), isLoading, status, session.value, authClient.getSession().apps/react-router/app/routes/server.tsx-28-41 (1)
28-41:⚠️ Potential issue | 🟡 MinorValidate
providerand handle credentials failures.
provideris read from untrusted form input and passed straight toapi.signInwithout validating it against the allowed list (github/gitlab/bitbucket). Constrain it to a known set to avoid unexpected provider lookups or misleading errors.api.signInCredentialshas no try/catch and no way to surface failure to the UI — invalid credentials will either throw (500 to the client) or silently no-op. Consider returning an error payload that the page can render.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/routes/server.tsx` around lines 28 - 41, Validate the untrusted provider value before calling api.signIn: when actionType === "signIn" check formData.get("provider") (provider) against an explicit allowlist (e.g., "github","gitlab","bitbucket") and return a controlled error/response for invalid values instead of forwarding them to api.signIn; and for the "signInCredentials" branch wrap the api.signInCredentials call in a try/catch (using the username and password extracted from formData) and return a structured error payload (e.g., { success: false, error: "Invalid credentials" } or a 400/401 response) so the UI can render credential failures rather than letting exceptions bubble as 500s or silently no-op.
🧹 Nitpick comments (9)
apps/tanstack-start/src/components/auth-client.tsx (1)
1-12: Remove the unusedAuthClientcomponent.The component is not imported anywhere in the tanstack-start app—only
AuthClientPage(a separate route component) andauthClient(the auth library client instance) are used. Keeping this stub adds dead code with no benefit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/components/auth-client.tsx` around lines 1 - 12, Remove the dead exported React component AuthClient from apps/tanstack-start/src/components/auth-client.tsx: delete the entire AuthClient function and its default/ named export so the unused component and its useAuth import are eliminated; if other files expect this module, instead export nothing or refactor imports to use AuthClientPage or authClient, and run a project-wide search for "AuthClient" to ensure all references are removed.apps/nuxt/app/components/header.vue (1)
67-73:NuxtLinkused for external URL.
NuxtLinkis intended for internal routing. Forhttps://aura-stack-auth.vercel.app/docs, use a plain<a>withtarget="_blank"andrel="noopener noreferrer"to match the other external links in this header and avoid relying on NuxtLink's external-URL handling.Proposed fix
- <NuxtLink - to="https://aura-stack-auth.vercel.app/docs" + <a + href="https://aura-stack-auth.vercel.app/docs" class="text-sm text-muted-foreground hover:text-white transition-colors py-2" + target="_blank" + rel="noopener noreferrer" `@click`="closeMenu" > Documentation - </NuxtLink> + </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nuxt/app/components/header.vue` around lines 67 - 73, Replace the external URL usage of NuxtLink with a normal anchor: change the <NuxtLink ...> element that points to "https://aura-stack-auth.vercel.app/docs" to an <a> element, keep the same classes and `@click`="closeMenu" handler, and add target="_blank" rel="noopener noreferrer" so the link behaves like the other external links in this header component and avoids using NuxtLink for external navigation.apps/tanstack-start/src/components/header.tsx (1)
113-117: Consider disabling the sign-in button while pending/submitting.Currently the button is hidden while
isPending, which can cause layout shift on load. Consider keeping it rendered butdisabledduringisPendingfor a smoother UX, and also preventing double-submits whilesignIn("github")is in flight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/components/header.tsx` around lines 113 - 117, Render the Sign in with GitHub Button even when isPending instead of hiding it to avoid layout shift: change the conditional that uses isPending and isAuthenticated so the Button is rendered whenever !isAuthenticated and pass disabled={isPending} (and optionally show a spinner) to prevent clicks while pending; also guard handleSignIn (or the signIn("github") call) so it no-ops if isPending to prevent double-submits (use the existing isPending flag or a local submitting flag inside handleSignIn).apps/tanstack-start/src/lib/auth-server.ts (1)
86-88:inputValidatordoes not validate.The validator is a pass-through and will accept any payload shape, defeating the purpose of
inputValidatorand allowing non-string fields to flow intoapi.updateSession. Add shape/type checks similar tosignInCredentialsFn.Proposed fix
- .inputValidator((data: { username?: string; email?: string }) => data) + .inputValidator((data: { username?: string; email?: string }) => { + if (!data || typeof data !== "object") { + throw new Error("update session payload is invalid") + } + if (data.username !== undefined && typeof data.username !== "string") { + throw new Error("username must be a string") + } + if (data.email !== undefined && typeof data.email !== "string") { + throw new Error("email must be a string") + } + return data + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tanstack-start/src/lib/auth-server.ts` around lines 86 - 88, The inputValidator for updateSessionFn currently just returns the input and accepts any shape; change it to validate that data is an object with optional username and email that, if present, are strings (mirroring the signInCredentialsFn validation) and throw or return an error for invalid types so only string values reach api.updateSession; update the inputValidator used by updateSessionFn to enforce these shape/type checks and reject non-string fields.apps/nextjs/app-router/next.config.ts (1)
5-12: LGTM!
remotePatternsforavatars.githubusercontent.comis the correct approach for Next.js 15 image optimization. Consider also settingpathname: "/u/**"to narrow the allowed paths if only GitHub user avatars are rendered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/next.config.ts` around lines 5 - 12, The Next.js images.remotePatterns currently allows avatars.githubusercontent.com broadly; to restrict to GitHub user avatars, update the remotePatterns entry (the images.remotePatterns array in your next config) to include a pathname filter like "/u/**" so only paths under /u/ are permitted; locate the images config in next.config.ts and add the pathname field to the existing object alongside protocol and hostname.apps/nextjs/app-router/src/app/layout.tsx (1)
26-26: Remove the commented-out<Footer />placeholder.Dead/commented code tends to rot. If the Footer will be reintroduced, track it via an issue/TODO; otherwise drop the comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/app/layout.tsx` at line 26, Remove the commented-out "<Footer />" placeholder from the JSX in layout.tsx (the commented line containing {/* <Footer /> */}); either delete the comment outright or replace it with a brief TODO reference to an issue number if the Footer will be reintroduced, ensuring no dead/commented JSX remains.packages/core/test/api/signOut.test.ts (1)
39-44: Assert that sign-out actually clears the session cookie.These tests currently accept any
Headersobject, so a regression that returnssuccess: truewithout expiringaura-auth.session_tokenwould still pass.Suggested assertion to add to each success case
expect(out).toEqual({ success: true, redirectURL: "/", headers: expect.any(Headers), toResponse: expect.any(Function), }) + +const setCookie = out.headers.get("set-cookie") +expect(setCookie).toContain("aura-auth.session_token=") +expect(setCookie).toMatch(/(?:Max-Age=0|Expires=)/i)Also applies to: 61-66, 84-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/api/signOut.test.ts` around lines 39 - 44, The tests currently only assert success and accept any Headers, so update each success-case assertion (the expects that currently check headers: expect.any(Headers) in signOut.test.ts) to explicitly verify the Set-Cookie header clears the session cookie: read the Set-Cookie from out.headers (e.g., out.headers.get('set-cookie')) and assert it contains the cookie name "aura-auth.session_token" and an expiry/clear directive such as "Max-Age=0" or an expired date (e.g., "Expires=Thu, 01 Jan 1970") or the cookie value being empty; apply the same explicit Set-Cookie assertion to the other two success blocks (the assertions around lines 61-66 and 84-89) so a regression that returns success without expiring the session_token will fail.apps/react-router/app/routes/server.tsx (2)
162-189: AddautoCompleteandrequiredon credential inputs.The username/password inputs lack
autoCompletehints (username,current-password) andrequired, which hurts password-manager integration and lets empty submissions hit the server. Minor UX/a11y polish.♻️ Proposed fix
- <input - type="text" - id="username" - name="username" + <input + type="text" + id="username" + name="username" + autoComplete="username" + required className="..." /> @@ - <input - type="password" - id="password" - name="password" + <input + type="password" + id="password" + name="password" + autoComplete="current-password" + required className="..." />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/routes/server.tsx` around lines 162 - 189, The username and password inputs in the Form used by handleSignInCredentials are missing autoComplete hints and required attributes, allowing empty submissions and hindering password managers; update the <input id="username" name="username"> to include required and autoComplete="username" and update the <input id="password" name="password"> to include required and autoComplete="current-password" so the Form handler (handleSignInCredentials) receives validated values and browsers/password managers can autofill correctly (keep the existing ids/names and Button unchanged).
8-14: Drop theas anycast on the loader return.Casting to
anydiscards the inferred session type and propagatesanyintoloaderDatain the component, defeating React Router's typed route data. Return the object directly and let the type flow.♻️ Proposed fix
- return { session } as any + return { session }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/routes/server.tsx` around lines 8 - 14, Remove the unnecessary "as any" cast in the loader so the inferred session type is preserved; in the export const loader async function that takes ({ request }: Route.LoaderArgs) and calls api.getSession, return { session } directly (or explicitly type the loader return if you prefer) instead of "return { session } as any" so the session type flows into loaderData in consuming components.
…146) * feat(packages): enhance API type definitions and response structures * chore: update README.md files
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/core/src/api/getSession.ts (1)
16-24:⚠️ Potential issue | 🟡 MinorInclude the new error metadata in
toResponse().The returned object now has
error, but the generated 401 response drops it, so programmatic and HTTP consumers see different failure shapes.Proposed fix
+ const unauthorizedError = { + code: "GET_SESSION_FAILED", + message: "Failed to retrieve session. The session token may be missing, expired, or invalid.", + } as const const unauthorized: GetSessionAPIReturn<DefaultUser> = { session: null, headers, success: false, - error: { - code: "GET_SESSION_FAILED", - message: "Failed to retrieve session. The session token may be missing, expired, or invalid.", - }, - toResponse: () => Response.json({ success: false, session: null }, { status: 401, headers }), + error: unauthorizedError, + toResponse: () => + Response.json({ success: false, session: null, error: unauthorizedError }, { status: 401, headers }), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/api/getSession.ts` around lines 16 - 24, The toResponse() for the unauthorized GetSessionAPIReturn in getSession.ts currently returns { success:false, session:null } only; update the unauthorized object's toResponse implementation (the toResponse method on the unauthorized constant) to include the error metadata (error.code and error.message and/or the entire error object) in the JSON body so the HTTP 401 response shape matches the returned object (e.g., return Response.json({ success: false, session: null, error }, { status: 401, headers })).packages/core/test/actions/signOut/signOut.test.ts (1)
145-184:⚠️ Potential issue | 🟠 MajorDo not assert
redirect: falsefor 302 responses.Lines 156 and 178 expect a redirect status, and Line 164 also checks the
Locationheader, but the JSON body is asserted asredirect: false. That locks in a contradictory contract for clients.Either make these responses non-redirecting, or update the response contract/tests so 302 responses report
redirect: true.Proposed test expectation if 302 is intended
expect(await request.json()).toMatchObject({ success: true, - redirect: false, + redirect: true, redirectURL: "/custom-logout-page", }) @@ expect(await request.json()).toMatchObject({ - redirect: false, + redirect: true, success: true, redirectURL: "/", })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/actions/signOut/signOut.test.ts` around lines 145 - 184, The tests "valid sessionToken cookie with redirectTo parameter" and "valid sessionToken cookie with invalid redirectTo parameter" currently assert a 302 status but also assert the JSON body has redirect: false, which is contradictory; update these tests to reflect that a 302 response should report redirect: true (modify the assertions in those two tests to expect redirect: true and keep the redirectURL checks) and ensure the first test still asserts the Location header equals "/custom-logout-page"; alternatively, if the intended behavior is no HTTP redirect, change the response status to 200 in the code under test and keep redirect: false—adjust whichever side (test assertions or response status in the signOut handler) to make the status, Location header, and JSON redirect flag consistent.packages/core/src/api/credentials.ts (1)
49-78:⚠️ Potential issue | 🟠 MajorExtract code/message from
AuthValidationErrorin addition toAuthInternalError.
isAuthInternalError(error)only recognizesAuthInternalErrorinstances, but the credentials sign-in endpoint throwsAuthValidationError("INVALID_CREDENTIALS", ...)on line 31. When caught, the check on line 52 fails, leavingcodeandmessageat their generic defaults. The response then returnscode: "CREDENTIALS_SIGN_IN_ERROR"instead of the specific"INVALID_CREDENTIALS", even though theerror instanceof AuthValidationErrorbranch on line 66 knows exactly what error occurred. Both error classes have identical structure withcodeandmessageproperties, so they can be handled together:🛠️ Proposed fix
let code = "CREDENTIALS_SIGN_IN_ERROR" let message = "An error occurred during credentials sign-in." - if (isAuthInternalError(error)) { + if (isAuthInternalError(error) || error instanceof AuthValidationError) { code = error.code message = error.message }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/api/credentials.ts` around lines 49 - 78, The catch block in the credentials sign-in flow sets generic code/message because isAuthInternalError(error) doesn't recognize AuthValidationError; update the catch to extract error.code and error.message for AuthValidationError as well (or use a type-guard that checks for both structures) before building the SignInCredentialsAPIReturn; specifically adjust the logic around isAuthInternalError(error) in the catch so it also handles error instanceof AuthValidationError (or a shared hasCodeAndMessage guard) and assigns code = error.code and message = error.message prior to creating invalidCredentials, keeping the existing logger branches (logger?.log(...)) intact.packages/core/src/client/client.ts (1)
111-139:⚠️ Potential issue | 🟠 Major
options.sessionis dereferenced without a guard.Line 119-120 does
const { session } = options; const user = session.user ?? {}.UpdateSessionOptions<DefaultUser>requiressessionat the type level, but a consumer passing an untyped object (common in server-action contexts whereoptionsis built from FormData, e.g.apps/react-router/app/routes/server/index.tsxat lines 47-52 where{ user: {...} }is passed at the top level instead of{ session: { user: {...} } }) will hit a runtimeCannot destructure property 'session' of .../Cannot read properties of undefined (reading 'user'). Consider validating and returning the typed error shape early:Proposed fix
- const { session } = options - const user = session.user ?? {} + const { session } = options ?? {} + if (!session) { + return { success: false, session: null } as unknown as UpdateSessionReturn<Options, DefaultUser> + } + const user = session.user ?? {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/client/client.ts` around lines 111 - 139, The updateSession function dereferences options.session without guarding against missing/invalid input; add an early validation at the top of updateSession (the function named updateSession and the type UpdateSessionOptions) that checks whether options and options.session exist and are proper objects, and if not return the expected UpdateSessionReturn shaped failure (e.g. { success: false, session: null } or include an error field) instead of continuing to destructure; update the destructuring to use a safe access pattern (avoid const { session } = options when options may be undefined) and log or throw an AuthClientError consistent with other error paths so callers get the typed error shape immediately.
🧹 Nitpick comments (5)
packages/react-router/tsdown.config.ts (1)
16-19: Spread shareddepsdefaults defensively, even though there's no current impact.The top-level spread of
tsdownConfigdoes not deep-merge nested properties. When you redefinedepson line 16-19, it completely replacestsdownConfig.depsrather than extending it.However, the shared config (
@aura-stack/tsdown-config) currently only definesonlyBundle: falseindeps, and your local override also includesonlyBundle: false, so nothing is actually being dropped today. The risk is future-facing: if shared defaults grow to include additional dependency-resolution fields, they'd be silently lost.Defensively, merge the shared defaults:
deps: { + ...tsdownConfig.deps, onlyBundle: false, neverBundle: ["react", "react-dom", "react-router"], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/tsdown.config.ts` around lines 16 - 19, The current override of deps replaces the shared tsdownConfig.deps entirely; update the override to defensively merge the nested deps (e.g., set deps: { ...tsdownConfig.deps, onlyBundle: false, neverBundle: ["react","react-dom","react-router"] }) so any future fields added to the shared tsdownConfig.deps are preserved; reference the top-level tsdownConfig and the deps object when making this change.packages/next/README.md (1)
79-93: Add type annotations to the client setup example.The example component lacks TypeScript types for
children, which diminishes the "Type-safe by design" claim. Consider adding proper typing to demonstrate best practices.📝 Suggested improvement with types
-export const App = ({ children }) => { +export const App = ({ children }: { children: React.ReactNode }) => { return <AuthProvider client={client}>{children}</AuthProvider> }Additionally, consider replacing the vague
/* your config */comment with either a minimal example or a reference to the configuration documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/README.md` around lines 79 - 93, The example for createAuthClient/AuthProvider lacks TypeScript typings for the component props; update the App component to use explicit types for children (e.g., React.ReactNode) and annotate the client variable with the proper auth client type returned by createAuthClient so the snippet demonstrates type-safe usage; also replace the placeholder "/* your config */" with either a minimal config example or a short comment pointing to the configuration docs to clarify what belongs there.apps/react-router/app/lib/auth.ts (1)
14-25: UsetoLowerCase()(nottoLocaleLowerCase()) when constructing an email address.
toLocaleLowerCase()varies with the runtime locale (notably TurkishI→ı), which can produce surprising results when the demo is run in a non-en locale. Emails should be normalized with ASCII-safetoLowerCase().🛠️ Proposed fix
- email: `${username.toLocaleLowerCase()}@example.com`, + email: `${username.toLowerCase()}@example.com`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/react-router/app/lib/auth.ts` around lines 14 - 25, The authorize function in the credentials object uses username.toLocaleLowerCase() when building the email which can yield locale-specific results; change that call to username.toLowerCase() in the authorize implementation (the block that returns { sub, name: username, email: `${username.toLocaleLowerCase()}@example.com` }) so the email is normalized in an ASCII-safe way; no other behavior changes needed (leave createSecretValue and returned fields intact).packages/react-router/src/lib/api.ts (1)
42-55: Redundantpayloadre-assignment after spread.
{ ...options, payload: options.payload }is equivalent to{ ...options }— the explicit re-assignment is a no-op. Drop it for clarity:Proposed refactor
- const signIn = await api.signInCredentials({ - ...options, - payload: options.payload, - }) + const signIn = await api.signInCredentials({ ...options })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/lib/api.ts` around lines 42 - 55, In signInCredentials (the exported function) remove the redundant payload re-assignment inside the call to api.signInCredentials — replace the object spread { ...options, payload: options.payload } with simply { ...options } so you pass the same options without the no-op override; update the call site at api.signInCredentials(...) accordingly in the signInCredentials function.packages/next/src/lib/api.ts (1)
105-117:signOutparameter widened toPartial<Options>for no clear reason.Changing from
OptionstoPartial<Options>makes every field (including required ones likeheadersin the coreSignOutAPIOptions) optional at the call site. That's fine in Next whereheadersis auto-injected, but it also turnsredirect/redirectTointounknown-ish properties and degrades inference for the conditional return type. Prefer keeping the public parameter asPartial<Pick<Options, "headers">> & Omit<Options, "headers">— or justOmit<Options, "headers"> & { headers?: HeadersInit }— so the generic stays useful and onlyheadersis optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/src/lib/api.ts` around lines 105 - 117, The public parameter type for signOut was overly widened to Partial<Options>, which makes required fields (like headers in SignOutAPIOptions) optional and breaks inference for redirect/redirectTo and the conditional NextSignOutReturn; update the signOut parameter type in the signOut function so only headers are optional—e.g. use Omit<Options, "headers"> & { headers?: HeadersInit } or Partial<Pick<Options, "headers">> & Omit<Options, "headers">—so the generic Options remains useful, keep the body using api.signOut({ headers: await headers(), ...options }) and preserve applyCookies/redirect behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/react-router/app/routes/api.auth`.$.tsx:
- Around line 8-14: The action function currently does a manual method dispatch
(inspecting request.method and calling handlers.PATCH or handlers.POST), which
lets unsupported HTTP methods fall through to POST; replace that manual dispatch
with the framework-aware handler by invoking handlers.ALL(request) from the
action function so the framework’s method validation runs and unsupported
methods return 405. Specifically, update the action implementation to call
handlers.ALL(request) instead of the current if/else using handlers.PATCH and
handlers.POST to ensure proper method validation.
In `@apps/react-router/app/routes/server/index.tsx`:
- Around line 33-44: The sign-in branch currently awaits
api.signInCredentials(...) and discards its redirect Response, causing the
action to fall through to return null; update the signInCredentials branch in
the route action handler to return the Response produced by
api.signInCredentials (or its .toResponse()) instead of awaiting and dropping it
so the redirect is sent to the browser (locate the actionType ===
"signInCredentials" block and return the call's Response).
- Around line 46-54: The call to api.updateSession is using the wrong payload
shape and omits the request: wrap the user data inside a session object to match
UpdateSessionOptions (i.e., pass { session: { user: { name, email } } }) and
forward the incoming request so the wrapper can access options.request.headers
(include the request variable from the route handler when invoking
api.updateSession). Update the invocation at the actionType === "updateSession"
block (the api.updateSession call) to build { session: { user: { ... } } } and
include request in the options passed to api.updateSession.
In `@packages/core/CHANGELOG.md`:
- Line 13: In packages/core/CHANGELOG.md remove the standalone dash that creates
an empty list item under the "### Added" section—delete the lone "-" line so the
changelog no longer renders a blank bullet (look for the "### Added" header and
the single "-" list entry and remove that entry).
In `@packages/core/src/`@types/api.ts:
- Around line 59-68: The interface APIOptionsWithRedirectTo currently documents
redirect as defaulting to true but signOut (and other APIs) may default it to
false; update the APIOptionsWithRedirectTo JSDoc to be action-neutral (remove
the "@default `true`" and any wording that claims a universal default) so
defaults are specified per-API (e.g., in signOut's returned redirect logic like
redirect ?? false) and avoid contradictory shared documentation; reference
APIOptionsWithRedirectTo and signOut (the function that sets redirect ?? false)
when making the change.
- Around line 18-23: AuthActionAPIReturn's failure branch currently declares an
error property but its toResponse return type (toResponse: () =>
AuthResponse<Exclude<Body, { success: true }>>) doesn't include that error,
causing a type mismatch with handlers that serialize error (e.g., the
credentials and signIn handlers). Fix by making the failure return type include
the error property (update the failure branch of AuthActionAPIReturn / the
toResponse return type to include error: { code: ErrorCodes; message: string })
so types match implementations, or alternatively remove error serialization from
the handlers (credentials handler and signIn handler) to match the existing
type—choose one consistent approach and update AuthActionAPIReturn and the
toResponse signature (and/or the handlers) accordingly.
- Around line 145-147: The conditional return type for SignInReturn is inverted:
change the conditional from Options extends { redirect: true } to Options
extends { redirect: false } so that the default (omitted redirect, which
defaults to true) yields void, while explicit redirect: false yields the data
object; apply the same inversion to the equivalent conditional types at the
other occurrences referenced (the similar conditionals around the blocks
currently at the later locations that return void vs data) so all SignIn-related
conditional return types use Options extends { redirect: false } instead of
Options extends { redirect: true } and update any related type aliases
accordingly (e.g., the other three matching conditional types in the file).
In `@packages/core/src/api/updateSession.ts`:
- Around line 15-66: The error branch's toResponse currently serializes an empty
body; update the error return in updateSession.ts (inside the catch of the
refreshSession call) so toResponse returns Response.json({ success: false,
redirectURL: null, error: { code, message } }, { status: 400, headers }) instead
of an empty object, and ensure the runtime error object (code/message) is the
one serialized. Also resolve the redirectURL type mismatch: make the success
return's redirectURL nullable (string | null) rather than forcing it to string —
adjust the UpdateSessionAPIReturn usage or the local return signature so
redirectURL is string | null (or alternatively always set redirectURL to ""),
and remove the misleading cast that hides the type mismatch.
In `@packages/core/src/client/client.ts`:
- Around line 141-166: The signOut function is forcing a default redirectTo of
"/" which overwrites server defaults; update the client.post call in signOut
(the function using getCSRFToken and client.post) to omit the redirectTo key
from searchParams when options?.redirectTo is undefined (only include redirectTo
when options?.redirectTo is explicitly provided), so the server receives no
redirectTo and can apply its configured default; leave token_type_hint and
X-CSRF-Token handling unchanged and keep the existing redirect logic that uses
json.redirectURL.
In `@packages/core/test/actions/signIn/signIn.test.ts`:
- Around line 367-371: The test assertion for the untrusted-origin sign-in
response is missing checks for standardized error metadata; update the
expect(await request.json()).toMatchObject({...}) in signIn.test.ts to include
an error object asserting error.code === 'UNTRUSTED_ORIGIN' and error.message
contains or equals the same message (e.g., error: { code: 'UNTRUSTED_ORIGIN',
message: expect.stringContaining('UNTRUSTED_ORIGIN') }) so the response's
error.code and error.message are validated alongside success, signInURL, and
redirect.
In `@packages/next/README.md`:
- Around line 103-112: The Profile component accesses session.user.name without
checking for a missing session; update the Profile function that calls
useSession() to handle the unauthenticated case after the loading state (i.e.,
when status !== "loading" but session is null/undefined). Add a null check for
session (or use optional chaining) and return a safe fallback UI such as a
sign-in prompt or a message like "Not signed in" instead of trying to read
session.user.name; ensure the change is made in the Profile component and
references the existing useSession, status === "loading", and session.user.name
usage.
In `@packages/next/src/`@types/api.ts:
- Around line 32-36: Update the NextSignInCredentials conditional type to match
the runtime implementation: change the constraint from SignInAPIOptions to
SignInCredentialsAPIOptions (import SignInCredentialsAPIOptions from
`@aura-stack/react/types`) and update the branch for Options extends { redirectTo:
string } to return never (not false) so the type reflects that redirect() throws
and does not return; keep the other branches unchanged.
In `@packages/next/src/lib/api.ts`:
- Around line 61-67: The handlers signIn, signInCredentials, and updateSession
currently only redirect when both signIn.success and options?.redirectTo are
truthy, which breaks the previous default automatic redirect; change the logic
so that if options?.redirect === false you return the raw result, otherwise if
signIn.success perform a redirect using options.redirectTo if provided, falling
back to the server-provided URL (e.g., signIn.signInURL or equivalent), and then
return the result when not redirecting; update the conditional in the signIn,
signInCredentials, and updateSession flows to implement this (check
options?.redirect first, then if signIn.success call redirect(options.redirectTo
?? signIn.signInURL)).
- Around line 71-86: The generic Options in signInCredentials is never inferred
because the parameter is typed as the constraint type; change the inner function
signature in signInCredentials to accept options: Options (not
SignInCredentialsAPIOptions) so TypeScript can infer the caller's literal type
and preserve the conditional NextSignInCredentials<Options> return narrowing;
update any usages inside (e.g., payload: options.payload and
options?.redirectTo) to remain compatible with Options.
In `@packages/react-router/README.md`:
- Around line 91-99: The example Profile component does not guard against a null
session after loading; update the Profile component that uses useSession so it
checks session (or status === "authenticated") before accessing
session.user.name and render a safe fallback (e.g., a sign-in prompt or null)
when session is null/unauthenticated to avoid runtime errors and TypeScript
failures.
- Line 116: Remove the stray standalone closing code fence (the lone ``` at EOF)
from the README so it no longer opens an unterminated Markdown code block;
locate the trailing triple-backtick token and delete it, leaving the footer and
document tail as plain Markdown.
In `@packages/react-router/src/`@types/index.ts:
- Around line 64-66: Replace the incorrect session field type in the exported
ReactRouterUpdateSessionAPIOptions type: the session currently uses
UpdateSessionOptions<DefaultUser> (client-side hook options) but must be the
core session shape. Update the definition of ReactRouterUpdateSessionAPIOptions
to use DeepPartial<Session<DefaultUser>> (or equivalently
UpdateSessionAPIOptions<DefaultUser>["session"]) for the session property so
server-side callers can pass the actual partial session object; keep the rest of
the type (Partial<UpdateSessionAPIOptions<DefaultUser>> & { request: Request })
the same and only change the session property's type in the type alias.
In `@packages/react-router/src/lib/api.ts`:
- Around line 57-83: In updateSession and signOut, callers can override the
injected headers because you spread ...options after setting headers; change the
call to construct the payload by spreading options first (but excluding the
request property) and then explicitly set headers from options.request.headers
so the injected headers cannot be overridden, and stop forwarding the request
object to api.updateSession/api.signOut (i.e., build an args object that omits
request, spreads remaining options, then sets headers: options.request.headers);
update the functions updateSession and signOut accordingly.
In `@packages/react/src/hooks.ts`:
- Around line 65-68: The JSDoc above useUpdateSession is stale/garbled: update
the comment to fix spacing and wording and remove reference to removed
skipRefresh option; instead document the current behavior and options (e.g.,
mention that redirect: false can be used to avoid a follow-up getSession),
correct the missing noun after "Default", and make the summary clear that this
patches session user/expiry and merges fields per call; update the block above
the useUpdateSession function accordingly.
---
Outside diff comments:
In `@packages/core/src/api/credentials.ts`:
- Around line 49-78: The catch block in the credentials sign-in flow sets
generic code/message because isAuthInternalError(error) doesn't recognize
AuthValidationError; update the catch to extract error.code and error.message
for AuthValidationError as well (or use a type-guard that checks for both
structures) before building the SignInCredentialsAPIReturn; specifically adjust
the logic around isAuthInternalError(error) in the catch so it also handles
error instanceof AuthValidationError (or a shared hasCodeAndMessage guard) and
assigns code = error.code and message = error.message prior to creating
invalidCredentials, keeping the existing logger branches (logger?.log(...))
intact.
In `@packages/core/src/api/getSession.ts`:
- Around line 16-24: The toResponse() for the unauthorized GetSessionAPIReturn
in getSession.ts currently returns { success:false, session:null } only; update
the unauthorized object's toResponse implementation (the toResponse method on
the unauthorized constant) to include the error metadata (error.code and
error.message and/or the entire error object) in the JSON body so the HTTP 401
response shape matches the returned object (e.g., return Response.json({
success: false, session: null, error }, { status: 401, headers })).
In `@packages/core/src/client/client.ts`:
- Around line 111-139: The updateSession function dereferences options.session
without guarding against missing/invalid input; add an early validation at the
top of updateSession (the function named updateSession and the type
UpdateSessionOptions) that checks whether options and options.session exist and
are proper objects, and if not return the expected UpdateSessionReturn shaped
failure (e.g. { success: false, session: null } or include an error field)
instead of continuing to destructure; update the destructuring to use a safe
access pattern (avoid const { session } = options when options may be undefined)
and log or throw an AuthClientError consistent with other error paths so callers
get the typed error shape immediately.
In `@packages/core/test/actions/signOut/signOut.test.ts`:
- Around line 145-184: The tests "valid sessionToken cookie with redirectTo
parameter" and "valid sessionToken cookie with invalid redirectTo parameter"
currently assert a 302 status but also assert the JSON body has redirect: false,
which is contradictory; update these tests to reflect that a 302 response should
report redirect: true (modify the assertions in those two tests to expect
redirect: true and keep the redirectURL checks) and ensure the first test still
asserts the Location header equals "/custom-logout-page"; alternatively, if the
intended behavior is no HTTP redirect, change the response status to 200 in the
code under test and keep redirect: false—adjust whichever side (test assertions
or response status in the signOut handler) to make the status, Location header,
and JSON redirect flag consistent.
---
Nitpick comments:
In `@apps/react-router/app/lib/auth.ts`:
- Around line 14-25: The authorize function in the credentials object uses
username.toLocaleLowerCase() when building the email which can yield
locale-specific results; change that call to username.toLowerCase() in the
authorize implementation (the block that returns { sub, name: username, email:
`${username.toLocaleLowerCase()}@example.com` }) so the email is normalized in
an ASCII-safe way; no other behavior changes needed (leave createSecretValue and
returned fields intact).
In `@packages/next/README.md`:
- Around line 79-93: The example for createAuthClient/AuthProvider lacks
TypeScript typings for the component props; update the App component to use
explicit types for children (e.g., React.ReactNode) and annotate the client
variable with the proper auth client type returned by createAuthClient so the
snippet demonstrates type-safe usage; also replace the placeholder "/* your
config */" with either a minimal config example or a short comment pointing to
the configuration docs to clarify what belongs there.
In `@packages/next/src/lib/api.ts`:
- Around line 105-117: The public parameter type for signOut was overly widened
to Partial<Options>, which makes required fields (like headers in
SignOutAPIOptions) optional and breaks inference for redirect/redirectTo and the
conditional NextSignOutReturn; update the signOut parameter type in the signOut
function so only headers are optional—e.g. use Omit<Options, "headers"> & {
headers?: HeadersInit } or Partial<Pick<Options, "headers">> & Omit<Options,
"headers">—so the generic Options remains useful, keep the body using
api.signOut({ headers: await headers(), ...options }) and preserve
applyCookies/redirect behavior.
In `@packages/react-router/src/lib/api.ts`:
- Around line 42-55: In signInCredentials (the exported function) remove the
redundant payload re-assignment inside the call to api.signInCredentials —
replace the object spread { ...options, payload: options.payload } with simply {
...options } so you pass the same options without the no-op override; update the
call site at api.signInCredentials(...) accordingly in the signInCredentials
function.
In `@packages/react-router/tsdown.config.ts`:
- Around line 16-19: The current override of deps replaces the shared
tsdownConfig.deps entirely; update the override to defensively merge the nested
deps (e.g., set deps: { ...tsdownConfig.deps, onlyBundle: false, neverBundle:
["react","react-dom","react-router"] }) so any future fields added to the shared
tsdownConfig.deps are preserved; reference the top-level tsdownConfig and the
deps object when making this change.
🪄 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: e05f8e27-b6bf-401c-9b2f-635dd8eae16d
📒 Files selected for processing (44)
apps/astro/src/lib/server.tsapps/nextjs/app-router/src/app/globals.cssapps/nuxt/app/assets/css/tailwind.cssapps/nuxt/app/components/header.vueapps/react-router/app/lib/auth.tsapps/react-router/app/routes.tsapps/react-router/app/routes/api.auth.$.tsxapps/react-router/app/routes/client/index.tsxapps/react-router/app/routes/server/index.tsxpackages/core/CHANGELOG.mdpackages/core/README.mdpackages/core/src/@types/api.tspackages/core/src/@types/index.tspackages/core/src/@types/session.tspackages/core/src/api/createApi.tspackages/core/src/api/credentials.tspackages/core/src/api/getSession.tspackages/core/src/api/signIn.tspackages/core/src/api/signOut.tspackages/core/src/api/updateSession.tspackages/core/src/client/client.tspackages/core/src/shared/identity.tspackages/core/test/actions/signIn/signIn.test.tspackages/core/test/actions/signOut/signOut.test.tspackages/core/test/actions/updateSession/updateSession.test.tspackages/core/test/api/signIn.test.tspackages/core/test/api/signOut.test.tspackages/core/test/api/updateSession.test.tspackages/core/test/types.test-d.tspackages/next/CHANGELOG.mdpackages/next/README.mdpackages/next/src/@types/api.tspackages/next/src/lib/api.tspackages/react-router/CHANGELOG.mdpackages/react-router/README.mdpackages/react-router/src/@types/index.tspackages/react-router/src/lib/api.tspackages/react-router/tsdown.config.tspackages/react/CHANGELOG.mdpackages/react/README.mdpackages/react/src/@types/types.tspackages/react/src/context.tsxpackages/react/src/hooks.tspackages/react/test/hooks.test.tsx
✅ Files skipped from review due to trivial changes (9)
- packages/next/CHANGELOG.md
- apps/nuxt/app/components/header.vue
- packages/core/README.md
- packages/core/test/types.test-d.ts
- packages/react/CHANGELOG.md
- packages/react/README.md
- packages/core/src/api/createApi.ts
- packages/core/test/actions/updateSession/updateSession.test.ts
- packages/react-router/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/nuxt/app/assets/css/tailwind.css
- apps/nextjs/app-router/src/app/globals.css
- packages/core/test/api/signOut.test.ts
- packages/core/src/api/signIn.ts
- apps/react-router/app/routes/client/index.tsx
- apps/react-router/app/routes.ts
halvaradop
added
enhancement
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
apps/nextjs/app-router/src/app/client/page.tsx (1)
8-8:⚠️ Potential issue | 🔴 CriticalStill importing non-existent
SubmitEventfromreact(flagged in a previous review).This was raised on the prior commit but remains unaddressed. Use
FormEvent<HTMLFormElement>(or switch toSubmitEventHandlerfor the handler type).Proposed fix
-import type { SubmitEvent } from "react" +import type { FormEvent } from "react" @@ - const handleSignInCredentials = async (event: SubmitEvent<HTMLFormElement>) => { + const handleSignInCredentials = async (event: FormEvent<HTMLFormElement>) => {Also applies to: 14-14
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/app/client/page.tsx` at line 8, The import is using a non-existent SubmitEvent from React; replace it with the correct React types and update any handler signatures that reference it (e.g., change import type { SubmitEvent } from "react" to import type { FormEvent } from "react" and use FormEvent<HTMLFormElement> for the form handler parameter, or alternatively use SubmitEventHandler<HTMLFormElement> as the handler type); locate and update the handler function(s) (e.g., handleSubmit) and their parameter annotations to use the chosen correct type.apps/nextjs/pages-router/src/pages/client/index.tsx (1)
6-6:⚠️ Potential issue | 🔴 Critical
SubmitEventis not exported byreact— same issue asapps/react-router/app/routes/server/index.tsxandapps/nextjs/app-router/src/app/client/page.tsx. UseFormEvent<HTMLFormElement>instead.Proposed fix
-import type { SubmitEvent } from "react" +import type { FormEvent } from "react" @@ - const handleSignInCredentials = async (event: SubmitEvent<HTMLFormElement>) => { + const handleSignInCredentials = async (event: FormEvent<HTMLFormElement>) => {Also applies to: 12-12
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/pages-router/src/pages/client/index.tsx` at line 6, Replace the invalid import of SubmitEvent from "react" with the correct type FormEvent<HTMLFormElement>; remove the line importing SubmitEvent and update any usage sites (e.g., onSubmit handlers or functions named handleSubmit / submitForm in this file) to type their event parameter as FormEvent<HTMLFormElement> instead of SubmitEvent so the code compiles and uses the supported React type.packages/next/src/lib/api.ts (1)
61-67:⚠️ Potential issue | 🟠 MajorDefault-redirect behavior still requires explicit
redirectTo— contradicts the JSDoc.Previously flagged and still present across all three handlers: the redirect fires only when
options?.redirectTois truthy. Callers invoking with no options (the previous "default redirect" path) — and, notably, the demo pages callingsignOut()bare (e.g.,apps/nextjs/app-router/src/app/client/page.tsxline 40) — now silently get the raw data object instead of a navigation. This also directly contradicts the JSDoc block added at lines 127–130: "By default the redirect is automatic, but it can be disabled by setting theredirectoption tofalse."Recommend restoring the default: redirect whenever
redirect !== falseand a redirect URL is available (falling back to the server-provided URL).Proposed fix for signIn (apply same pattern to signInCredentials and updateSession)
- if (options?.redirect === false) { - return signIn as NextSignInReturn<Options> - } - if (signIn.success && options?.redirectTo) { - return redirect(signIn.signInURL) - } - return signIn as NextSignInReturn<Options> + if (options?.redirect !== false && signIn.success && signIn.signInURL) { + return redirect(signIn.signInURL) + } + return signIn as NextSignInReturn<Options>Also applies to: 79-82, 96-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/src/lib/api.ts` around lines 61 - 67, The redirect logic currently only fires when options.redirectTo is provided, contradicting the JSDoc; update the handlers (signIn, signInCredentials, updateSession) so they perform a redirect whenever signIn.success (or equivalent result.success) is true and options.redirect !== false, using options.redirectTo if present otherwise falling back to the server-provided URL (e.g., signIn.signInURL); if a resolved URL exists call redirect(url) and otherwise return the original response object. Ensure you change the condition surrounding redirect in the signIn, signInCredentials and updateSession handlers (the blocks that currently check options?.redirectTo) to the new logic.
🧹 Nitpick comments (3)
packages/core/src/client/client.ts (1)
111-142:updateSessionfetches a CSRF token before checking whether there's anything to update.When
options.sessionis missing the function returns{ success: false, session: null }, but only after a full network round-trip to/csrfToken. Hoisting the early-return abovegetCSRFToken()avoids the wasted request and makes the no-op path obvious.🔧 Proposed fix
const updateSession = async <Options extends UpdateSessionOptions<DefaultUser>>( options: Options ): Promise<UpdateSessionReturn<Options, DefaultUser>> => { try { + const { session } = options ?? {} + if (!session) { + return { success: false, session: null } as UpdateSessionReturn<Options, DefaultUser> + } const csrfToken = await getCSRFToken() if (!csrfToken) { throw new AuthClientError("Failed to fetch CSRF token for session update.") } - const { session } = options ?? {} - if (!session) { - return { success: false, session: null } as UpdateSessionReturn<Options, DefaultUser> - } const user = session.user ?? {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/client/client.ts` around lines 111 - 142, The updateSession function currently calls getCSRFToken() before checking options.session, causing an unnecessary network request; reorder the logic in updateSession so you first extract const { session } = options ?? {} and return { success: false, session: null } as UpdateSessionReturn<Options, DefaultUser> if !session, and only then call getCSRFToken() and proceed with client.patch("/session") and the rest of the flow (preserving existing error handling and redirect logic).packages/next/src/lib/api.ts (1)
73-77: Redundant key after...optionsspread.
payload: options.payloadon line 76 (andsession: options.sessionon line 92) duplicates what...optionsalready carries. Harmless, but noisy; dropping the explicit keys makes the forwarding intent clear.const signIn = await api.signInCredentials({ headers: await headers(), ...options, - payload: options.payload, }) @@ const updated = await api.updateSession({ ...options, - session: options.session, headers: await headers(), })Also applies to: 90-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next/src/lib/api.ts` around lines 73 - 77, Remove the redundant explicit properties that duplicate the spreaded options: in the call to api.signInCredentials (the signIn variable) drop the explicit payload: options.payload because ...options already includes it; likewise remove session: options.session in the other API call that spreads ...options (the call around line 92) so the forwarded options are not repeated and the intent to forward all options via the spread is clear.apps/nextjs/app-router/src/app/client/page.tsx (1)
44-44: Inconsistent container height utility with sibling demo pages.Sibling pages (
apps/nextjs/pages-router/.../client/index.tsx,apps/nuxt/app/pages/client.vue,apps/react-router/.../server/index.tsx) all usemin-h-container, but this page usesmin-h-[calc(100vh-3.8rem)] mt-[3.8rem]. Ifmin-h-containerwas introduced as the shared utility for this exact purpose (per the AI summary), prefer it here for consistency.- <main className="w-11/12 min-h-[calc(100vh-3.8rem)] max-w-5xl mt-[3.8rem] mx-auto content-center"> + <main className="w-11/12 min-h-container max-w-5xl mx-auto content-center">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nextjs/app-router/src/app/client/page.tsx` at line 44, The main element's className uses custom min-h and top margin utilities instead of the shared utility; update the <main> element (className on the main tag in page.tsx) to replace "min-h-[calc(100vh-3.8rem)] mt-[3.8rem]" with the shared "min-h-container" utility so it matches sibling demo pages and keeps consistent layout behavior across apps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/nuxt/app/composables/useAuthClient.ts`:
- Line 45: The signInCredentials function is typed incorrectly: it accepts
options?: SignInOptions (OAuth sign-in) but builds an object with payload:
credentials for the credentials flow; change the options parameter type to
Omit<SignInCredentialsOptions, "payload"> so callers must provide the correct
credential-flow options shape and the function can safely spread options and add
payload; update the function signature for signInCredentials and any related
callers/exports to use Omit<SignInCredentialsOptions, "payload"> and ensure
payload remains the credentials object.
In `@apps/nuxt/app/pages/client.vue`:
- Around line 23-30: handleUpdateSession is currently a no-op because its body
is commented out; restore and wire it to call updateSession with the correct
payload: read username and email from the FormData (formData.get("username"),
formData.get("email")), build the UpdateSessionOptions-shaped payload under
session.user (matching UpdateSessionOptions/session.user), await
updateSession(...) inside handleUpdateSession, handle/await errors and update
local UI state or refetch the session as needed so the EditProfile flow actually
updates on the client; reference the function name handleUpdateSession and the
updateSession API used in server.vue/nextjs example to match shape.
In `@apps/react-router/app/components/edit-profile.tsx`:
- Around line 14-28: The label element currently uses htmlFor="signout" which
points to a non-existent id and is inappropriate because the adjacent control is
a Button; remove the htmlFor attribute (or replace the <label> with a non-label
element) in the Edit Profile block so the text is not tied to a missing form
control—edit the <label> with text "Edit Profile" (or convert it to a <span> or
<p>) near the Button component and keep the Button (data-open={isOpen},
onClick={() => setIsOpen(true)}) unchanged to preserve behavior.
In `@apps/react-router/app/routes/server/index.tsx`:
- Line 5: The import of SubmitEvent from "react" is invalid; replace that import
and any type annotations using SubmitEvent with a React-compatible event type:
import/remove the incorrect SubmitEvent import and update handlers/annotations
(e.g., the function or variable named in the file that currently types
parameters as SubmitEvent) to use FormEvent<HTMLFormElement> from "react" or use
the DOM global SubmitEvent by typing the parameter as SubmitEvent and casting
currentTarget as HTMLFormElement where needed; ensure to update both occurrences
(the import at the top and the second occurrence around the handler at line ~69)
so types compile correctly.
In `@apps/tanstack-start/src/lib/auth.ts`:
- Around line 11-22: The authorize implementation currently creates a new random
sub on every sign-in and accepts any non-empty username/password; update
authorize to (1) be marked with a clear "// demo only" comment, (2) stop
generating a random identity each call by deriving a stable sub from the
username (e.g., hash or deterministic encoding of username) instead of
createSecretValue(10) so the same credentials map to the same user, and (3) add
a simple placeholder credential check (e.g., compare against a hard-coded demo
password or call a stubbed verifyCredentials function) to make the acceptance
behavior explicit; refer to the authorize function and the createSecretValue(10)
usage when applying these changes.
In `@apps/tanstack-start/src/routes/server.tsx`:
- Around line 100-110: The sign-out UI incorrectly uses a <form> as a layout
container and a <label htmlFor="signout"> that points to no control; replace the
<form> with a non-semantic container (e.g., a div) so Enter-key submissions
can't occur, and change the <label htmlFor="signout"> to a plain text element
(e.g., a div/span) or associate it with a real control id if you intend it to be
a form label; keep the Button (type="button") and its onClick={handleSignOut}
intact so the existing handleSignOut handler continues to run.
- Around line 78-92: The img alt value can end up undefined or "null" because
session.user.name is string | null | undefined; update the <img> usage (the
block that checks session?.user?.image and renders an img) to always pass a
non-null string by coalescing session?.user?.name into a sensible default (e.g.,
"User avatar" or "Unknown user") so alt never receives null/undefined; locate
the img element that uses session.user.image and change its alt expression
accordingly.
In `@packages/core/src/api/updateSession.ts`:
- Around line 23-46: The code computes redirectURL regardless of whether refresh
produced a session; change the logic in updateSession (around redirectURL,
redirectToInit, requestInit, and createRedirectTo) so you only call
createRedirectTo and set redirectURL when session is non-null (i.e., after
refreshSession succeeded). Ensure the returned object and toResponse continue to
reflect success based on session and that redirectURL remains null when session
is null.
In `@packages/core/test/actions/session/session.test.ts`:
- Around line 49-56: The test is incorrectly asserting an error object on the
success branch of the GetSessionAPIReturn discriminated union; update the test
expectations in session.test.ts (the expect(await request.json()) assertions
around lines showing success true) to only assert { success: true, session: {
user: sessionPayload, expires: expect.any(String) } } and remove the error
object check, and ensure packages/core/src/api/getSession.ts continues to return
the strict discriminated union (success branch must not include an error field)
so tests and implementation remain consistent.
---
Duplicate comments:
In `@apps/nextjs/app-router/src/app/client/page.tsx`:
- Line 8: The import is using a non-existent SubmitEvent from React; replace it
with the correct React types and update any handler signatures that reference it
(e.g., change import type { SubmitEvent } from "react" to import type {
FormEvent } from "react" and use FormEvent<HTMLFormElement> for the form handler
parameter, or alternatively use SubmitEventHandler<HTMLFormElement> as the
handler type); locate and update the handler function(s) (e.g., handleSubmit)
and their parameter annotations to use the chosen correct type.
In `@apps/nextjs/pages-router/src/pages/client/index.tsx`:
- Line 6: Replace the invalid import of SubmitEvent from "react" with the
correct type FormEvent<HTMLFormElement>; remove the line importing SubmitEvent
and update any usage sites (e.g., onSubmit handlers or functions named
handleSubmit / submitForm in this file) to type their event parameter as
FormEvent<HTMLFormElement> instead of SubmitEvent so the code compiles and uses
the supported React type.
In `@packages/next/src/lib/api.ts`:
- Around line 61-67: The redirect logic currently only fires when
options.redirectTo is provided, contradicting the JSDoc; update the handlers
(signIn, signInCredentials, updateSession) so they perform a redirect whenever
signIn.success (or equivalent result.success) is true and options.redirect !==
false, using options.redirectTo if present otherwise falling back to the
server-provided URL (e.g., signIn.signInURL); if a resolved URL exists call
redirect(url) and otherwise return the original response object. Ensure you
change the condition surrounding redirect in the signIn, signInCredentials and
updateSession handlers (the blocks that currently check options?.redirectTo) to
the new logic.
---
Nitpick comments:
In `@apps/nextjs/app-router/src/app/client/page.tsx`:
- Line 44: The main element's className uses custom min-h and top margin
utilities instead of the shared utility; update the <main> element (className on
the main tag in page.tsx) to replace "min-h-[calc(100vh-3.8rem)] mt-[3.8rem]"
with the shared "min-h-container" utility so it matches sibling demo pages and
keeps consistent layout behavior across apps.
In `@packages/core/src/client/client.ts`:
- Around line 111-142: The updateSession function currently calls getCSRFToken()
before checking options.session, causing an unnecessary network request; reorder
the logic in updateSession so you first extract const { session } = options ??
{} and return { success: false, session: null } as UpdateSessionReturn<Options,
DefaultUser> if !session, and only then call getCSRFToken() and proceed with
client.patch("/session") and the rest of the flow (preserving existing error
handling and redirect logic).
In `@packages/next/src/lib/api.ts`:
- Around line 73-77: Remove the redundant explicit properties that duplicate the
spreaded options: in the call to api.signInCredentials (the signIn variable)
drop the explicit payload: options.payload because ...options already includes
it; likewise remove session: options.session in the other API call that spreads
...options (the call around line 92) so the forwarded options are not repeated
and the intent to forward all options via the spread is clear.
🪄 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: 45ef0285-9799-42db-9f81-fe4194e64640
📒 Files selected for processing (48)
apps/nextjs/app-router/next.config.tsapps/nextjs/app-router/src/app/client/page.tsxapps/nextjs/app-router/src/app/layout.tsxapps/nextjs/app-router/src/app/server/page.tsxapps/nextjs/app-router/src/components/edit-profile.tsxapps/nextjs/app-router/src/lib/auth.tsapps/nextjs/pages-router/next.config.tsapps/nextjs/pages-router/src/components/edit-profile.tsxapps/nextjs/pages-router/src/lib/auth.tsapps/nextjs/pages-router/src/pages/client/index.tsxapps/nextjs/pages-router/src/pages/server/index.tsxapps/nuxt/app/components/header.vueapps/nuxt/app/composables/useAuthClient.tsapps/nuxt/app/pages/client.vueapps/nuxt/app/pages/index.vueapps/react-router/app/components/auth-client.tsxapps/react-router/app/components/edit-profile.tsxapps/react-router/app/lib/auth.tsapps/react-router/app/routes/api.auth.$.tsxapps/react-router/app/routes/client/index.tsxapps/react-router/app/routes/server/index.tsxapps/tanstack-start/src/components/auth-client.tsxapps/tanstack-start/src/components/auth-server.tsxapps/tanstack-start/src/components/edit-profile.tsxapps/tanstack-start/src/components/footer.tsxapps/tanstack-start/src/lib/auth-server.tsapps/tanstack-start/src/lib/auth.tsapps/tanstack-start/src/routes/client.tsxapps/tanstack-start/src/routes/server.tsxpackages/core/CHANGELOG.mdpackages/core/src/@types/api.tspackages/core/src/api/credentials.tspackages/core/src/api/getSession.tspackages/core/src/api/signIn.tspackages/core/src/api/updateSession.tspackages/core/src/client/client.tspackages/core/src/shared/errors.tspackages/core/test/actions/session/session.test.tspackages/core/test/actions/signIn/signIn.test.tspackages/next/README.mdpackages/next/src/@types/api.tspackages/next/src/lib/api.tspackages/react-router/README.mdpackages/react-router/src/@types/index.tspackages/react-router/src/lib/api.tspackages/react-router/tsdown.config.tspackages/react/README.mdpackages/react/src/hooks.ts
💤 Files with no reviewable changes (3)
- apps/tanstack-start/src/components/auth-client.tsx
- apps/react-router/app/components/auth-client.tsx
- apps/tanstack-start/src/components/auth-server.tsx
✅ Files skipped from review due to trivial changes (10)
- apps/nextjs/app-router/src/app/layout.tsx
- apps/nextjs/pages-router/next.config.ts
- apps/nextjs/app-router/next.config.ts
- packages/core/test/actions/signIn/signIn.test.ts
- apps/nuxt/app/components/header.vue
- packages/react/README.md
- packages/react-router/README.md
- packages/core/src/api/signIn.ts
- packages/core/CHANGELOG.md
- packages/core/src/@types/api.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/react-router/app/routes/api.auth.$.tsx
- packages/react-router/tsdown.config.ts
- apps/nextjs/app-router/src/lib/auth.ts
- apps/nextjs/pages-router/src/lib/auth.ts
- apps/nextjs/app-router/src/components/edit-profile.tsx
- apps/tanstack-start/src/components/footer.tsx
- apps/nextjs/pages-router/src/components/edit-profile.tsx
- packages/core/src/api/credentials.ts
- apps/tanstack-start/src/components/edit-profile.tsx
- apps/nextjs/pages-router/src/pages/server/index.tsx
- apps/react-router/app/routes/client/index.tsx
- apps/nextjs/app-router/src/app/server/page.tsx
- packages/next/README.md
- packages/react-router/src/@types/index.ts
- packages/core/src/api/getSession.ts
- apps/tanstack-start/src/routes/client.tsx
- packages/react/src/hooks.ts
- apps/tanstack-start/src/lib/auth-server.ts
- packages/react-router/src/lib/api.ts
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.
Description
This pull request updates the demo integration examples with a redesigned UI and improved structure. The new design provides a clear overview of available demos and includes direct navigation to dedicated rendering strategies such as server-side rendering (SSR) and client-side rendering (CSR), depending on framework support.
The visual style remains consistent with the existing design philosophy, focusing on minimalism using lines, boxes, and simple layouts.
Additionally, due to the scope and size of related changes, part of the implementation was moved to a separate package and later merged into this PR to keep concerns modular and manageable.
Next.js Demo
The following images showcase the updated design introduced in this PR:
Key Changes
Related PRs
Summary by CodeRabbit
Release Notes
New Features
Changes
Documentation