Conversation
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive authentication system with passkeys and social login, establishes an RPC API layer with TanStack Query integration, massively expands the UI component library with 50+ new styled components, adds database schema for authentication entities, and scaffolds authentication pages and flows in the portal application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Portal as Portal App
participant Auth as Better-Auth
participant ORPC as ORPC API
participant DB as Database
User->>Portal: Navigate to /login
Portal->>Portal: Render LoginForm
User->>Portal: Submit email + password
Portal->>Auth: authClient.signIn.email()
Auth->>DB: Query user + validate
DB-->>Auth: User session
Auth-->>Portal: Session + redirect
Portal->>Portal: Show toast (success)
Portal->>Portal: Navigate to /
User->>Portal: Access protected route
Portal->>Portal: Call useIsLoggedIn hook
Portal->>Auth: authClient.useSession()
Auth->>DB: Fetch session
DB-->>Auth: Session data
Auth-->>Portal: {session, user}
Portal->>Portal: Render protected content
sequenceDiagram
participant Client as Client Component
participant ORPC_Client as ORPC Client
participant RPCRoute as /api/rpc Route
participant Handler as RPC Handler
participant Middleware as Error Middleware
participant Procedure as Server Procedure
Client->>ORPC_Client: Call procedure (deduplicated)
ORPC_Client->>RPCRoute: POST /api/rpc
RPCRoute->>Handler: handleRequest(req)
Handler->>Middleware: Create shared context
Middleware->>Procedure: Execute with context
Procedure-->>Middleware: Return data
Middleware-->>Handler: Log + wrap response
Handler->>RPCRoute: Response
RPCRoute-->>ORPC_Client: JSON result
ORPC_Client->>Client: Hydrate via TanStack Query
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (21)
packages/ui/src/components/textarea.tsx-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorThe
numbertype insizeprop is unused.The
sizeprop acceptsnumberas a valid type, but the className logic only handles the string values"sm","default", and"lg". If a numeric value is passed, it will be set ondata-sizebut won't affect any styling.Either remove
numberfrom the union type, or add handling for numeric sizes (e.g., dynamic row count or height).🔧 Option 1: Remove unused number type
- size?: "sm" | "default" | "lg" | number; + size?: "sm" | "default" | "lg";🔧 Option 2: Use number for rows attribute
<textarea className={cn( "field-sizing-content min-h-17.5 w-full rounded-[inherit] px-[calc(--spacing(3)-1px)] py-[calc(--spacing(1.5)-1px)] outline-none max-sm:min-h-20.5", size === "sm" && "min-h-16.5 px-[calc(--spacing(2.5)-1px)] py-[calc(--spacing(1)-1px)] max-sm:min-h-19.5", size === "lg" && "min-h-18.5 py-[calc(--spacing(2)-1px)] max-sm:min-h-21.5" )} data-slot="textarea" + rows={typeof size === "number" ? size : undefined} {...mergeProps(defaultProps, props)} />Also applies to: 33-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/textarea.tsx` at line 11, The size prop on the Textarea component includes number but the className switch (handling "sm" | "default" | "lg") ignores numeric values; either remove number from the size union type on the TextareaProps to keep only "sm"|"default"|"lg", or update the Textarea component (the size prop handling and className logic) to treat numeric sizes (e.g., when typeof size === "number" set the textarea rows attribute or apply an inline height style) so numeric inputs actually affect layout; update the type declaration and the render path in the Textarea component (where size is read and data-size/className are set) accordingly and ensure tests/usage reflect the chosen behavior.packages/ui/src/components/sheet.tsx-44-57 (1)
44-57:⚠️ Potential issue | 🟡 MinorCSS class conflict between
gridandflexfor left/right sides.Line 47 always applies the
gridclass, but forside === "left"andside === "right"(lines 50-51),flexis also added. Whileflexmay overridegridin some browsers due to CSS cascade, this is fragile and could cause inconsistent layout behavior. Additionally, lines 48-49 redundantly addgridwhen it's already applied on line 47.🛠️ Proposed fix
return ( <SheetPrimitive.Viewport className={cn( - "fixed inset-0 z-50 grid", - side === "bottom" && "grid grid-rows-[1fr_auto] pt-12", - side === "top" && "grid grid-rows-[auto_1fr] pb-12", - side === "left" && "flex justify-start", - side === "right" && "flex justify-end", + "fixed inset-0 z-50", + (side === "bottom" || side === "top") && "grid", + side === "bottom" && "grid-rows-[1fr_auto] pt-12", + side === "top" && "grid-rows-[auto_1fr] pb-12", + (side === "left" || side === "right") && "flex", + side === "left" && "justify-start", + side === "right" && "justify-end", variant === "inset" && "sm:p-4" )} data-slot="sheet-viewport" {...props} /> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/sheet.tsx` around lines 44 - 57, The SheetPrimitive.Viewport className currently always includes "grid" and then conditionally adds "flex" for side === "left" or "right", causing a grid/flex conflict; update the className logic (where cn is used in the SheetPrimitive.Viewport JSX) so that "grid" is applied only for top/bottom layouts (or when side is undefined) and "flex" is applied exclusively for left/right—also remove the redundant explicit "grid" additions for top/bottom and ensure variant === "inset" handling remains unchanged; in short, compute mutually exclusive layout classes based on the side prop before passing them to cn in the SheetPrimitive.Viewport.packages/ui/src/components/empty.tsx-77-88 (1)
77-88:⚠️ Potential issue | 🟡 MinorType and rendered element mismatch.
EmptyDescriptionis typed withReact.ComponentProps<"p">but renders a<div>. This inconsistency could cause confusion and may result in invalid props being passed (e.g., paragraph-specific attributes that don't apply to divs).🔧 Proposed fix — either render a `
` or type as `
`Option 1: Render a
<p>to match the type (semantically appropriate for description text):function EmptyDescription({ className, ...props }: React.ComponentProps<"p">) { return ( - <div + <p className={cn( "text-muted-foreground text-sm [&>a:hover]:text-primary [&>a]:underline [&>a]:underline-offset-4 [[data-slot=empty-title]+&]:mt-1", className )} data-slot="empty-description" {...props} - /> + /> ); }Option 2: Change the type to match the element:
-function EmptyDescription({ className, ...props }: React.ComponentProps<"p">) { +function EmptyDescription({ className, ...props }: React.ComponentProps<"div">) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/empty.tsx` around lines 77 - 88, The component EmptyDescription currently declares props as React.ComponentProps<"p"> but renders a <div>, causing a props/semantic mismatch; update EmptyDescription to either render a <p> element instead of the <div> (preferred for descriptive text) or change the declared props to React.ComponentProps<"div"> so the types match the rendered element—modify the JSX tag and/or the prop typing in the EmptyDescription function accordingly and keep the existing className, data-slot and spread props intact.packages/ui/src/components/select.tsx-194-202 (1)
194-202:⚠️ Potential issue | 🟡 MinorInconsistent className handling - should use
cn()to merge.Unlike other components in this file (SelectSeparator, SelectItem, SelectValue),
SelectGroupLabeldoesn't destructure and merge theclassNameprop. If a consumer passesclassName, it will completely replace the default styling instead of extending it.🐛 Proposed fix
-function SelectGroupLabel(props: SelectPrimitive.GroupLabel.Props) { +function SelectGroupLabel({ className, ...props }: SelectPrimitive.GroupLabel.Props) { return ( <SelectPrimitive.GroupLabel - className="px-2 py-1.5 font-medium text-muted-foreground text-xs" + className={cn("px-2 py-1.5 font-medium text-muted-foreground text-xs", className)} data-slot="select-group-label" {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/select.tsx` around lines 194 - 202, SelectGroupLabel currently spreads props directly so a consumer `className` would override defaults; update the SelectGroupLabel function to destructure `className` from its `props`, merge it with the existing default classes using the `cn()` utility (same pattern used by SelectSeparator/SelectItem/SelectValue), and pass the merged className plus the remaining props into <SelectPrimitive.GroupLabel /> so default styling is preserved and extendable.packages/ui/src/components/checkbox.tsx-23-49 (1)
23-49:⚠️ Potential issue | 🟡 MinorHide the indicator SVGs from assistive tech.
These icons are decorative inside an already-labeled checkbox. Leaving them exposed can add extra announcement noise for screen-reader users.
♿ Proposed fix
<svg + aria-hidden="true" className="size-3.5 sm:size-3" fill="none" + focusable="false" height="24" stroke="currentColor" strokeLinecap="round" @@ <svg + aria-hidden="true" className="size-3.5 sm:size-3" fill="none" + focusable="false" height="24" stroke="currentColor" strokeLinecap="round"As per coding guidelines, "Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (
<button>,<nav>,<main>)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/checkbox.tsx` around lines 23 - 49, The decorative SVGs rendered inside the checkbox component are exposed to assistive tech; update both SVG elements (the conditional branches that render the minus and the other icon) to be hidden from AT by adding aria-hidden="true", focusable="false" and role="presentation" attributes so screen readers ignore them while keeping the visuals intact.packages/ui/src/components/kbd.tsx-5-13 (1)
5-13:⚠️ Potential issue | 🟡 Minor
pointer-events-nonemakes part of the public<kbd>API unusable.Because
Kbdaccepts full native<kbd>props, consumers can reasonably passtitleor mouse handlers. The hard-codedpointer-events-noneprevents those from working and also makes the shortcut harder to select/copy.Proposed fix
<kbd className={cn( - "pointer-events-none inline-flex h-5 min-w-5 select-none items-center justify-center gap-1 rounded bg-muted px-1 font-medium font-sans text-muted-foreground text-xs [&_svg:not([class*='size-'])]:size-3", + "inline-flex h-5 min-w-5 select-none items-center justify-center gap-1 rounded bg-muted px-1 font-medium font-sans text-muted-foreground text-xs [&_svg:not([class*='size-'])]:size-3", className )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/kbd.tsx` around lines 5 - 13, The Kbd component currently hard-codes "pointer-events-none" which breaks native kbd props like title, onClick, and prevents selecting/copying; update the Kbd function to remove "pointer-events-none" from the className string and instead, if necessary, target only icon children (e.g. use a selector like "[&_svg]:pointer-events-none" or "[&>svg]:pointer-events-none") so icons keep disabled pointer events but the <kbd> element itself accepts mouse handlers and selection; update the class string in Kbd({ className, ...props }) accordingly.packages/ui/src/components/breadcrumb.tsx-47-54 (1)
47-54:⚠️ Potential issue | 🟡 MinorRemove the fake link semantics from the current page item.
role="link"makes assistive tech announce this as interactive even though it is not focusable or clickable. For the current breadcrumb item, plain text witharia-current="page"is the correct pattern;aria-disabledcan go too.♿ Suggested fix
- // biome-ignore lint(a11y/useFocusableInteractive): known <span aria-current="page" - aria-disabled="true" className={cn("font-normal text-foreground", className)} data-slot="breadcrumb-page" - role="link" {...props} />As per coding guidelines, "Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (
<button>,<nav>,<main>)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/breadcrumb.tsx` around lines 47 - 54, The current breadcrumb item in breadcrumb.tsx is using fake link semantics—remove role="link" and aria-disabled from the span that renders the current page (the element with aria-current="page", data-slot="breadcrumb-page", className handled via cn and {...props}) so it is announced as plain static text; keep aria-current="page" and the existing className/props handling but do not present it as interactive. Ensure any props passed that imply interactivity are not used for the current-page branch (adjust the rendering logic around the span if needed).packages/ui/src/components/breadcrumb.tsx-75-77 (1)
75-77:⚠️ Potential issue | 🟡 MinorThe screen-reader label is currently unreachable.
aria-hidden="true"on the wrapper removes the nestedsr-onlytext from the accessibility tree, so"More"is never announced. Either removearia-hiddenor drop the hidden label if this ellipsis is purely decorative.♿ Suggested fix if the label should be announced
- <span aria-hidden="true" className={className} data-slot="breadcrumb-ellipsis" role="presentation" {...props}> + <span className={className} data-slot="breadcrumb-ellipsis" {...props}> <MoreHorizontal className="size-4" /> <span className="sr-only">More</span> </span>As per coding guidelines, "Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (
<button>,<nav>,<main>)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/breadcrumb.tsx` around lines 75 - 77, The wrapper span with aria-hidden="true" prevents the nested sr-only "More" label from being announced; either remove aria-hidden so the label is exposed or, if the ellipsis is purely decorative, remove the sr-only text. Locate the element containing MoreHorizontal (the span with data-slot="breadcrumb-ellipsis" and className) and either delete the aria-hidden attribute and keep the inner <span className="sr-only">More</span> (or add an explicit aria-label="More" on the wrapper) to make it accessible, or remove the sr-only node entirely if you intend the icon to be non-interactive/decorative. Ensure any interactive behavior uses a semantic element (e.g., convert the span to a <button> if clickable) and update attributes accordingly.packages/ui/src/components/autocomplete.tsx-205-218 (1)
205-218:⚠️ Potential issue | 🟡 MinorDuplicate XIcon in AutocompleteClear.
AutocompleteClearrenders its ownXIcon(line 215), but when used withinAutocompleteInput(lines 69-79), the input also renders<XIcon />inside<AutocompleteClear>. This results in duplicate icons whenshowClearis true. The standaloneAutocompleteClearshould not include the icon internally, or the usage inAutocompleteInputshould not add another icon.🐛 Option 1: Remove icon from standalone AutocompleteClear
function AutocompleteClear({ className, ...props }: AutocompletePrimitive.Clear.Props) { return ( <AutocompletePrimitive.Clear className={cn( "absolute end-0.5 top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-[color,background-color,box-shadow,opacity] pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0", className )} data-slot="autocomplete-clear" {...props} - > - <XIcon /> - </AutocompletePrimitive.Clear> + /> ); }🐛 Option 2: Use primitive directly in AutocompleteInput
{showClear && ( - <AutocompleteClear + <AutocompletePrimitive.Clear className={cn( "absolute top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-colors pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 has-[+[data-slot=autocomplete-clear]]:hidden sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0", sizeValue === "sm" ? "end-0" : "end-0.5" )} + data-slot="autocomplete-clear" {...clearProps} > <XIcon /> - </AutocompleteClear> + </AutocompletePrimitive.Clear> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/autocomplete.tsx` around lines 205 - 218, The AutocompleteClear component currently renders a built-in XIcon which causes a duplicate when AutocompleteInput also supplies an XIcon; remove the internal <XIcon /> from the AutocompleteClear JSX so the Clear primitive becomes icon-agnostic and rely on callers (AutocompleteInput) to pass the icon as children; update AutocompleteClear (the function/component) to simply render children (preserving className, data-slot, and props) and verify AutocompleteInput still passes <XIcon /> into <AutocompleteClear />.packages/ui/src/components/toggle-group.tsx-77-90 (1)
77-90:⚠️ Potential issue | 🟡 Minor
ToggleGroupSeparatoronly defaults correctly for horizontal groups.When the parent group uses
orientation="vertical",<ToggleGroupSeparator />still renders with a vertical separator unless every caller overrides it manually. That leaves one of the two supported group orientations with the wrong default behavior.💡 Suggested fix
function ToggleGroupSeparator({ className, - orientation = "vertical", + orientation, ...props }: { className?: string; } & React.ComponentProps<typeof Separator>) { + const context = React.useContext(ToggleGroupContext); + const resolvedOrientation = + orientation ?? (context?.orientation === "vertical" ? "horizontal" : "vertical"); + return ( <Separator className={cn( "pointer-events-none relative bg-input before:absolute before:inset-0 dark:before:bg-input/32", className )} - orientation={orientation} + orientation={resolvedOrientation} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/toggle-group.tsx` around lines 77 - 90, ToggleGroupSeparator currently defaults its orientation to "vertical", causing incorrect rendering for horizontal parent groups; change the default orientation to "horizontal" in the ToggleGroupSeparator function signature (orientation = "horizontal") so the component defaults match the common horizontal group use, and ensure the orientation prop is still forwarded to the <Separator> component (function ToggleGroupSeparator).packages/ui/src/components/input-group.tsx-86-91 (1)
86-91:⚠️ Potential issue | 🟡 MinorKeep
unstylednon-overridable in the group wrappers.Both wrappers pass
unstyledbefore{...props}, sounstyled={false}from a caller re-enables standaloneInput/Textareachrome and breaks the group composition. If these wrappers are meant to guarantee group styling, setunstyledlast.Suggested change
function InputGroupInput({ className, ...props }: InputProps) { - return <Input className={className} unstyled {...props} />; + return <Input {...props} className={className} unstyled />; } function InputGroupTextarea({ className, ...props }: TextareaProps) { - return <Textarea className={className} unstyled {...props} />; + return <Textarea {...props} className={className} unstyled />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/input-group.tsx` around lines 86 - 91, The InputGroupInput and InputGroupTextarea wrappers currently pass unstyled before spreading {...props}, allowing callers to override it; change the prop order so the wrapper enforces unstyled by either spreading {...props} first and then specifying unstyled (or setting unstyled={true} after the spread) so callers cannot re-enable standalone chrome; update both InputGroupInput and InputGroupTextarea accordingly.packages/ui/src/components/input-group.tsx-56-69 (1)
56-69:⚠️ Potential issue | 🟡 MinorCompose the addon focus handler instead of letting props replace it.
Because
{...props}is spread after the inline handler, any consumer-supplied pointer/mouse handler overrides the focus-forwarding behavior entirely. Switch to a pointer handler to keep the same behavior on touch devices and properly compose by calling the consumer's handler first, then respectingdefaultPreventedbefore applying internal focus logic.Suggested change
function InputGroupAddon({ className, align = "inline-start", + onPointerDown, ...props }: React.ComponentProps<"div"> & VariantProps<typeof inputGroupAddonVariants>) { return ( <div className={cn(inputGroupAddonVariants({ align }), className)} data-align={align} data-slot="input-group-addon" - onMouseDown={(e) => { - const target = e.target as HTMLElement; + onPointerDown={(e) => { + onPointerDown?.(e); + if (e.defaultPrevented) return; + const target = e.target; + if (!(target instanceof Element)) return; const isInteractive = target.closest( "button, a, input, select, textarea, [role='button'], [role='combobox'], [role='listbox'], [data-slot='select-trigger']" ); if (isInteractive) return; e.preventDefault(); const parent = e.currentTarget.parentElement; const input = parent?.querySelector<HTMLInputElement | HTMLTextAreaElement>("input, textarea"); if (input && !parent?.querySelector("input:focus, textarea:focus")) { input.focus(); } }} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/input-group.tsx` around lines 56 - 69, Replace the current inline onMouseDown handler with a composed onPointerDown handler that calls any consumer-supplied pointer/mouse handler first and only runs the internal focus-forwarding when the event is not defaultPrevented; specifically, read the consumer handler from props (e.g., props.onPointerDown || props.onMouseDown), invoke it with the event, check if event.defaultPrevented and return early if so, then perform the existing interactive-element check, preventDefault, locate the parent input/textarea and focus it; ensure you attach this composed handler (onPointerDown) while still spreading {...props} so consumers can pass other attributes or handlers.packages/ui/src/components/pagination.tsx-91-98 (1)
91-98:⚠️ Potential issue | 🟡 Minor
aria-hiddenis hiding the only accessible label.Because the wrapper
spanisaria-hidden, the nestedsr-onlytext is removed from the accessibility tree too, so screen readers won't announce "More pages".💡 Proposed fix
- <span - aria-hidden - className={cn("flex min-w-7 justify-center", className)} - data-slot="pagination-ellipsis" - {...props} - > - <MoreHorizontalIcon className="size-5 sm:size-4" /> + <span + className={cn("flex min-w-7 justify-center", className)} + data-slot="pagination-ellipsis" + {...props} + > + <MoreHorizontalIcon aria-hidden="true" className="size-5 sm:size-4" /> <span className="sr-only">More pages</span> </span>As per coding guidelines, "Follow WCAG 2.1 AA accessibility standards" and "Use ARIA labels when semantic HTML elements are insufficient for accessibility."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/pagination.tsx` around lines 91 - 98, The wrapper span with data-slot="pagination-ellipsis" currently has aria-hidden which hides the nested sr-only label and prevents screen readers from announcing "More pages"; fix it by removing aria-hidden from the wrapper (or alternatively move aria-hidden to the visual icon element MoreHorizontalIcon and add an accessible label/aria-label on the wrapper span) so that the sr-only text or an explicit aria-label on the span is exposed to assistive technologies; update the element that uses className and data-slot="pagination-ellipsis" accordingly and ensure only the decorative MoreHorizontalIcon is aria-hidden while the wrapper remains accessible.packages/ui/src/components/pagination.tsx-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorFix type alias and accessibility issues in pagination components.
- Line 33: Change
type PaginationLinkPropstointerface PaginationLinkPropsper guidelines preferring interfaces for object shapes.- Lines 69, 84: Icon elements have explicit margin classes (
sm:-ms-1,sm:-me-1). Per guidelines, don't add margin to icons in buttons—the button component handles spacing automatically. Remove these classes unless intentional custom spacing is required.- Lines 91–99: The outer
<span aria-hidden>will suppress the innersr-onlytext from screen readers. Move thesr-onlyspan outside thearia-hiddenwrapper or removearia-hiddenif the icon needs accessibility context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/pagination.tsx` at line 9, Change the PaginationLinkProps type alias to an interface named PaginationLinkProps to follow the object-shape convention; remove the explicit icon margin utility classes (sm:-ms-1, sm:-me-1) from the icon elements inside the PaginationLink/Pagination components so the Button/buttonVariants spacing handles icon padding automatically; and fix the accessibility issue by moving the inner sr-only span out of the wrapper that has aria-hidden or by removing aria-hidden on the outer span so the screen-reader-only label is reachable (adjust in the PaginationLink/PaginationLinkProps render where the icon and the sr-only label are composed).packages/ui/src/components/menu.tsx-126-138 (1)
126-138:⚠️ Potential issue | 🟡 MinorHide the indicator SVGs from assistive tech.
The checkmark SVGs at Lines 126 and 162 are decorative. Without
aria-hiddenandfocusable="false", some screen readers announce extra graphics inside each menu item.As per coding guidelines, `**/*.tsx`: Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (``, ``, ``)♿ Suggested fix
<svg + aria-hidden="true" fill="none" + focusable="false" height="24"Apply the same attributes to both inline SVGs.
Also applies to: 162-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/menu.tsx` around lines 126 - 138, The inline checkmark SVGs used as decorative indicators in the menu component should be hidden from assistive tech; update both SVG elements (the checkmark icons inside the menu rendering in packages/ui/src/components/menu.tsx) to include aria-hidden="true" and focusable="false" so screen readers ignore them and they are not keyboard focusable.packages/ui/src/components/tabs.tsx-11-15 (1)
11-15:⚠️ Potential issue | 🟡 MinorMove
data-slotattributes after{...props}to make them non-overridable.The wrapper sets
data-slotbefore spreading props (lines 13, 37, 61, 68), allowing consumers to override slot names and silently break internal styling. The component relies on selectors like*:data-[slot=tabs-tab]:hover:bg-accent(line 34) that depend on exact slot values. Reorder to placedata-slotafter{...props}.Additionally,
TabsPanelremoves the outline withoutline-nonebut provides nofocus-visiblereplacement. This breaks keyboard navigation visibility. Addfocus-visible:ring-2 focus-visible:ring-ringor similar focus styling to matchTabsTab(line 58).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/tabs.tsx` around lines 11 - 15, Move the data-slot attributes so they are applied after the spread props to prevent consumers from overriding internal slot names: for each usage of TabsPrimitive.Root and other Tab wrapper elements (e.g., the element rendering TabsPrimitive.Root in this file and any elements that set data-slot="tabs-tab" / "tabs-panel" / etc.), reorder the JSX so {...props} comes before the data-slot attribute; ensure className and other attributes remain unchanged but data-slot is last. Also update the TabsPanel element (the component rendering the panel) to not only remove outline-none but add focus-visible styles consistent with TabsTab (e.g., include focus-visible:ring-2 and focus-visible:ring-ring or equivalent focus-visible classes) so keyboard focus is visually indicated.packages/ui/src/components/combobox.tsx-166-179 (1)
166-179:⚠️ Potential issue | 🟡 MinorUse a
divfor the popup shell.This wrapper is acting as the flex container for the popup, but
spanis phrasing content. Keeping it as adivavoids invalid HTML around the listbox.As per coding guidelines, Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (``, ``, ``).♻️ Proposed fix
- <span + <div className={cn( "relative flex max-h-full min-w-(--anchor-width) max-w-(--available-width) origin-(--transform-origin) rounded-lg border bg-popover not-dark:bg-clip-padding shadow-lg/5 transition-[scale,opacity] before:pointer-events-none before:absolute before:inset-0 before:rounded-[calc(var(--radius-lg)-1px)] before:shadow-[0_1px_--theme(--color-black/4%)] dark:before:shadow-[0_-1px_--theme(--color-white/6%)]", className )} > <ComboboxPrimitive.Popup className="flex max-h-[min(var(--available-height),23rem)] flex-1 flex-col text-foreground" data-slot="combobox-popup" {...props} > {children} </ComboboxPrimitive.Popup> - </span> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/combobox.tsx` around lines 166 - 179, The wrapper around ComboboxPrimitive.Popup currently uses a <span> which is inline/phrasing content and can produce invalid HTML for a block-level popup; replace that <span> with a <div> in the Combobox component so the shell is a proper block container, preserve the existing cn(...) className and children props, and ensure the JSX still renders <ComboboxPrimitive.Popup ...>{children}</ComboboxPrimitive.Popup> inside the new div (look for the span around ComboboxPrimitive.Popup in the Combobox component to locate the change).packages/ui/src/components/combobox.tsx-152-160 (1)
152-160:⚠️ Potential issue | 🟡 MinorUse the
multipleflag to guard the fallback anchor.Line 153 should only fall back to
chipsRefwhen the combobox is in multi-select mode. Currently,chipsRefis always passed even in single-select, where it's never attached to a DOM element. This overrides Base UI's default anchor behavior. Destructure themultipleflag from context and update the anchor logic to:const anchor = anchorProp ?? (multiple ? chipsRef : undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/combobox.tsx` around lines 152 - 160, Destructure the multiple flag from ComboboxContext and use it to guard the chipsRef fallback: instead of always using chipsRef for anchor (anchorProp ?? chipsRef), set anchor to anchorProp ?? (multiple ? chipsRef : undefined) so that chipsRef is only used in multi-select mode; update the anchor variable where it's declared and ensure ComboboxPrimitive.Positioner continues to receive the computed anchor.packages/ui/src/styles/globals.css-142-143 (1)
142-143:⚠️ Potential issue | 🟡 MinorRename
--code-foregroundto--color-code-foregroundto expose color utilities.In Tailwind CSS v4, the
--color-*namespace is required to generate color utilities (e.g.,text-code-foreground,bg-code-foreground). Line 143 uses--code-foregroundinstead, breaking the naming convention used throughout the rest of the theme.Suggested change
- --code-foreground: var(--code-foreground); + --color-code-foreground: var(--code-foreground);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/styles/globals.css` around lines 142 - 143, Rename the CSS variable --code-foreground to --color-code-foreground so it follows the Tailwind v4 color namespace; update the declaration (replace --code-foreground with --color-code-foreground) and change every usage of var(--code-foreground) to var(--color-code-foreground) (search for references in theme files/components) to restore color utility generation (the relevant symbols are the --code-foreground variable and any var(...) usages referencing it).packages/ui/src/components/number-field.tsx-52-79 (1)
52-79:⚠️ Potential issue | 🟡 MinorReadonly steppers still look clickable.
Line 56 and Line 71 always apply
cursor-pointerandhover:bg-accent, so the decrement/increment controls keep their active affordances even when the field is readonly. Add inert-state styling here so controls that can’t change the value don’t still read as interactive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/number-field.tsx` around lines 52 - 79, The decrement/increment controls (NumberFieldDecrement and NumberFieldIncrement) always apply interactive affordances (cursor-pointer and hover:bg-accent) even when the control is inert; update both components to conditionally include interactive classes only when not readonly/disabled. In the className composition for NumberFieldPrimitive.Decrement and NumberFieldPrimitive.Increment, detect props.readOnly || props.disabled || props["aria-disabled"] and when true replace/remove "cursor-pointer" and "hover:bg-accent" with non-interactive styling (e.g., cursor-default or no pointer, and reduced opacity/pointer-events-none) so readonly/disabled steppers no longer appear clickable. Ensure you reference NumberFieldDecrement and NumberFieldIncrement when making the change.packages/ui/src/components/number-field.tsx-82-91 (1)
82-91:⚠️ Potential issue | 🟡 MinorFix the small-size line-height typo.
Line 86 sets
sm:in-data-[size=sm]:leading-8.5while the same variant usessm:in-data-[size=sm]:h-6.5. That mismatch will push the compact input text off-center.✏️ Suggested fix
- "h-8.5 in-data-[size=lg]:h-9.5 in-data-[size=sm]:h-7.5 w-full min-w-0 grow bg-transparent in-data-[size=sm]:px-[calc(--spacing(2.5)-1px)] px-[calc(--spacing(3)-1px)] text-center tabular-nums in-data-[size=lg]:leading-9.5 in-data-[size=sm]:leading-7.5 leading-8.5 outline-none [transition:background-color_5000000s_ease-in-out_0s] sm:h-7.5 sm:in-data-[size=lg]:h-8.5 sm:in-data-[size=sm]:h-6.5 sm:in-data-[size=lg]:leading-8.5 sm:in-data-[size=sm]:leading-8.5 sm:leading-7.5", + "h-8.5 in-data-[size=lg]:h-9.5 in-data-[size=sm]:h-7.5 w-full min-w-0 grow bg-transparent in-data-[size=sm]:px-[calc(--spacing(2.5)-1px)] px-[calc(--spacing(3)-1px)] text-center tabular-nums in-data-[size=lg]:leading-9.5 in-data-[size=sm]:leading-7.5 leading-8.5 outline-none [transition:background-color_5000000s_ease-in-out_0s] sm:h-7.5 sm:in-data-[size=lg]:h-8.5 sm:in-data-[size=sm]:h-6.5 sm:in-data-[size=lg]:leading-8.5 sm:in-data-[size=sm]:leading-6.5 sm:leading-7.5",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/number-field.tsx` around lines 82 - 91, In NumberFieldInput, fix the small-size line-height typo so the small responsive variant matches its height: update the class token "sm:in-data-[size=sm]:leading-8.5" used in the NumberFieldPrimitive.Input JSX to the small variant line-height that corresponds to height "sm:in-data-[size=sm]:h-6.5" (e.g., "sm:in-data-[size=sm]:leading-6.5"), ensuring the sm + in-data[size=sm] combination keeps text vertically centered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46bf84bc-553e-4b85-b782-4f69fb219851
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (70)
.vscode/settings.jsonapps/portal/next.config.tsapps/portal/package.jsonapps/portal/src/app/api/auth/[...all]/route.tsapps/portal/src/app/api/rpc/[[...rest]]/route.tsapps/portal/src/app/layout.tsxapps/portal/src/components/providers.tsxapps/portal/src/lib/orpc/client.tsapps/portal/src/lib/orpc/globals.d.tsapps/portal/src/lib/orpc/query/client.tsapps/portal/src/lib/orpc/query/hydration.tsxapps/portal/src/lib/orpc/query/serializer.tsapps/portal/src/lib/orpc/server.tspackages/api/package.jsonpackages/ui/components.jsonpackages/ui/package.jsonpackages/ui/src/components/accordion.tsxpackages/ui/src/components/alert-dialog.tsxpackages/ui/src/components/alert.tsxpackages/ui/src/components/autocomplete.tsxpackages/ui/src/components/avatar.tsxpackages/ui/src/components/badge.tsxpackages/ui/src/components/breadcrumb.tsxpackages/ui/src/components/button.tsxpackages/ui/src/components/calendar.tsxpackages/ui/src/components/card.tsxpackages/ui/src/components/checkbox-group.tsxpackages/ui/src/components/checkbox.tsxpackages/ui/src/components/collapsible.tsxpackages/ui/src/components/combobox.tsxpackages/ui/src/components/command.tsxpackages/ui/src/components/dialog.tsxpackages/ui/src/components/empty.tsxpackages/ui/src/components/field.tsxpackages/ui/src/components/fieldset.tsxpackages/ui/src/components/form.tsxpackages/ui/src/components/frame.tsxpackages/ui/src/components/group.tsxpackages/ui/src/components/input-group.tsxpackages/ui/src/components/input.tsxpackages/ui/src/components/kbd.tsxpackages/ui/src/components/label.tsxpackages/ui/src/components/menu.tsxpackages/ui/src/components/meter.tsxpackages/ui/src/components/number-field.tsxpackages/ui/src/components/pagination.tsxpackages/ui/src/components/popover.tsxpackages/ui/src/components/preview-card.tsxpackages/ui/src/components/progress.tsxpackages/ui/src/components/radio-group.tsxpackages/ui/src/components/scroll-area.tsxpackages/ui/src/components/select.tsxpackages/ui/src/components/separator.tsxpackages/ui/src/components/sheet.tsxpackages/ui/src/components/sidebar.tsxpackages/ui/src/components/skeleton.tsxpackages/ui/src/components/slider.tsxpackages/ui/src/components/spinner.tsxpackages/ui/src/components/switch.tsxpackages/ui/src/components/table.tsxpackages/ui/src/components/tabs.tsxpackages/ui/src/components/textarea.tsxpackages/ui/src/components/toast.tsxpackages/ui/src/components/toggle-group.tsxpackages/ui/src/components/toggle.tsxpackages/ui/src/components/toolbar.tsxpackages/ui/src/components/tooltip.tsxpackages/ui/src/hooks/use-media-query.tspackages/ui/src/styles/globals.csspnpm-workspace.yaml
| async function handleRequest(req: NextRequest) { | ||
| // Create context once and reuse it for both handlers | ||
| // This prevents duplicate session retrieval calls and ensures consistency | ||
| // const context = await createContext(req, await headers()); | ||
| const context = await createContext(req); | ||
|
|
||
| const rpcResult = await rpcHandler.handle(req, { | ||
| prefix: "/api/rpc", | ||
| context, | ||
| }); | ||
| if (rpcResult.response) { | ||
| return rpcResult.response; | ||
| } | ||
|
|
||
| const apiResult = await apiHandler.handle(req, { | ||
| prefix: "/api/rpc/docs", | ||
| context, | ||
| }); | ||
| if (apiResult.response) { | ||
| return apiResult.response; | ||
| } | ||
|
|
||
| return new Response("Not found", { status: 404 }); |
There was a problem hiding this comment.
Wrap the route entrypoint in a top-level try/catch.
createContext(req) and the handler calls can throw before the onError interceptors run, so this route can fail without the contextual logging/error mapping the server rules expect.
Based on learnings, "Implement server-side error handling with try/catch blocks including context; return appropriate oRPC error codes (UNAUTHORIZED, NOT_FOUND, etc.)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/app/api/rpc/`[[...rest]]/route.ts around lines 30 - 52, Wrap
the entire body of handleRequest in a top-level try/catch so exceptions from
createContext(req), rpcHandler.handle, or apiHandler.handle are caught and
translated into proper HTTP/oRPC responses; in the catch, log the error together
with any available context (from createContext if it succeeded) and map known
error types to appropriate oRPC/HTTP status codes (e.g., Unauthorized → 401,
NotFound → 404) otherwise return a 500/internal oRPC error, and ensure any
existing rpcHandler.onError or api handler error-mapping logic is invoked or
mirrored so telemetry and server-side error rules still run.
Add route-level throttling before createContext.
This endpoint exposes both the public RPC surface and the docs handler, and the docs path sits outside any procedure-level middleware. Without a guard here, repeated requests can still fan out auth/session work before any per-procedure protection applies.
As per coding guidelines, "Apply rate limiting via @ziron/rate-limit on oRPC and public routes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/app/api/rpc/`[[...rest]]/route.ts around lines 30 - 52, The
handler currently calls createContext before any rate limiting, allowing
unauthenticated requests (including docs) to trigger expensive session/auth
work; add a route-level rate limiter from `@ziron/rate-limit` at the top of
handleRequest to guard both rpcHandler and apiHandler before calling
createContext. Specifically, import and instantiate the rate limiter and call
its consume/check using the request identifier (e.g., IP or x-forwarded-for) at
the start of handleRequest, returning a 429 response when limits are exceeded so
rpcHandler.handle, apiHandler.handle and createContext never run for throttled
requests.
| const link = new RPCLink({ | ||
| url: `${typeof window !== "undefined" ? window.location.origin : "http://localhost:3000"}/api/rpc`, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Imports of the server-side oRPC initializer ==="
rg -n --type=ts --type=tsx '(`@/lib/orpc/server`|/lib/orpc/server|["'"'"']\./server["'"'"'])'
echo
echo "=== Imports of the shared oRPC client ==="
rg -n --type=ts --type=tsx '(`@/lib/orpc/client`|/lib/orpc/client|["'"'"']\./client["'"'"'])' apps/portal/srcRepository: itzTedx/ZironTap
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Imports of server-side oRPC initializer ==="
rg -n '(orpc/server|lib/orpc/server)' apps/portal/src --type-list | head -20
echo
echo "=== Imports of shared oRPC client ==="
rg -n '(orpc/client|lib/orpc/client)' apps/portal/src --type-list | head -20
echo
echo "=== Client module content ==="
cat -n apps/portal/src/lib/orpc/client.ts
echo
echo "=== Server module content (if exists) ==="
if [ -f apps/portal/src/lib/orpc/server.ts ]; then cat -n apps/portal/src/lib/orpc/server.ts; else echo "File not found"; fiRepository: itzTedx/ZironTap
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Client module ==="
cat -n apps/portal/src/lib/orpc/client.ts 2>/dev/null || echo "File not found"
echo
echo "=== Server module ==="
cat -n apps/portal/src/lib/orpc/server.ts 2>/dev/null || echo "File not found"
echo
echo "=== Search for imports of /orpc/client ==="
rg 'orpc/client' apps/portal/src 2>/dev/null || echo "No matches"
echo
echo "=== Search for imports of /orpc/server ==="
rg 'orpc/server' apps/portal/src 2>/dev/null || echo "No matches"
echo
echo "=== Look for layout.tsx or other server entry points ==="
find apps/portal/src -name "layout.tsx" -o -name "page.tsx" -o -name "middleware.ts" | head -10Repository: itzTedx/ZironTap
Length of output: 3544
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Root layout.tsx ==="
cat -n apps/portal/src/app/layout.tsx 2>/dev/null | head -40
echo
echo "=== Root page.tsx ==="
cat -n apps/portal/src/app/page.tsx 2>/dev/null | head -40
echo
echo "=== Where is client.ts imported? ==="
rg 'from.*orpc/client' apps/portal/src 2>/dev/null
echo
echo "=== Where is server.ts imported? ==="
rg 'from.*orpc/server' apps/portal/src 2>/dev/null
echo
echo "=== Check globals.d.ts ==="
cat -n apps/portal/src/lib/orpc/globals.d.ts 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 3102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Providers component ==="
cat -n apps/portal/src/components/providers.tsx 2>/dev/null | head -50
echo
echo "=== Search for middleware files ==="
find apps/portal/src -name "middleware.ts" -o -name "middleware.js" 2>/dev/null | xargs cat -n 2>/dev/null || echo "No middleware found"
echo
echo "=== Search for instrumentation ==="
find apps/portal/src -name "instrumentation.*" 2>/dev/null | xargs cat -n 2>/dev/null || echo "No instrumentation found"
echo
echo "=== Actual usage of client export ==="
rg 'from.*client.*export\|import.*client' apps/portal/src 2>/dev/null
echo
echo "=== Search for any imports of client from the lib ==="
rg 'from ["\x27]@/lib/orpc/client' apps/portal/src 2>/dev/null || echo "No direct imports"
echo
echo "=== Check if server.ts needs to be explicitly imported ==="
echo "Searching for any patterns that might auto-initialize server..."
rg 'globalThis\.\$client' apps/portal/src 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 1019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Query client file ==="
cat -n apps/portal/src/lib/orpc/query/client.ts 2>/dev/null
echo
echo "=== All files in lib/orpc directory ==="
ls -la apps/portal/src/lib/orpc/
echo
echo "=== Search for any imports of orpc/client (more broadly) ==="
rg 'lib/orpc/client|@/lib/orpc/client' apps/portal/src 2>/dev/null
echo
echo "=== Check if any server routes import from lib ==="
cat -n apps/portal/src/app/api/rpc/\[\[..rest\]\]/route.ts 2>/dev/null | head -50
echo
echo "=== Search for uses of globalThis.$client ==="
rg 'globalThis.*\$client|\$client' apps/portal/src 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 2162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify if client.ts is used anywhere ==="
rg 'from.*client.*import|import.*client' apps/portal/src 2>/dev/null | grep -v node_modules || echo "No imports found"
echo
echo "=== Check if there are any server components/functions that might need the client ==="
rg '"use server"' apps/portal/src 2>/dev/null | head -5
echo
echo "=== File size and usage check ==="
wc -l apps/portal/src/lib/orpc/client.ts apps/portal/src/lib/orpc/server.tsRepository: itzTedx/ZironTap
Length of output: 1038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check for any route handlers in api/ directory ==="
find apps/portal/src/app/api -type f \( -name "*.ts" -o -name "*.js" \) | head -10 | xargs ls -la
echo
echo "=== Check RPC route handler ==="
cat -n apps/portal/src/app/api/rpc/\[\[..rest\]\]/route.ts 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check RPC route handler ==="
find apps/portal/src -path "*api/rpc*" -name "route.ts" 2>/dev/null | while read f; do
echo "File: $f"
cat -n "$f"
done
echo
echo "=== Auth route handler ==="
find apps/portal/src -path "*api/auth*" -name "route.ts" 2>/dev/null | while read f; do
echo "File: $f"
cat -n "$f"
doneRepository: itzTedx/ZironTap
Length of output: 2410
The server-side oRPC initializer is never imported, making the fallback URL architecture unsound.
The server.ts module sets globalThis.$client as a side effect, but it is never imported anywhere in the codebase. When client.ts is eventually used, globalThis.$client will never be initialized, and the client will permanently freeze to the HTTP fallback with a hardcoded localhost:3000 URL (line 10). This will fail outside local development.
The issue is currently latent because client.ts itself is not yet imported anywhere, but the architectural pattern is broken. Either ensure server.ts is imported early in your server initialization path, or resolve the client lazily instead of at module evaluation time.
Also applies to: 48-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/lib/orpc/client.ts` around lines 9 - 10, The RPCLink is being
constructed at module-evaluation time so the fallback URL always uses a
hardcoded localhost and never observes the server-side initializer (server.ts)
that sets globalThis.$client; fix by making client URL resolution lazy: replace
the direct RPCLink instantiation in apps/portal/src/lib/orpc/client.ts with a
factory/getter that computes the URL at runtime (e.g., a getRpcUrl or
createRpcLink function) which checks (1) if typeof window !== "undefined" then
use window.location.origin, (2) else look for globalThis.$client or another
server-side-provided value, and (3) finally fall back to a configurable env var
instead of hardcoded "http://localhost:3000"; alternatively, ensure server.ts is
imported early in your server bootstrap so globalThis.$client is initialized
before this module runs—refer to RPCLink, globalThis.$client, client.ts and
server.ts when making the change.
| queryCache: new QueryCache({ | ||
| onError: (error, query) => { | ||
| const id = toastManager.add({ | ||
| title: "Error", | ||
| description: `Error: ${error.message}`, | ||
|
|
||
| actionProps: { | ||
| children: "Retry", | ||
| onClick: () => { | ||
| toastManager.close(id); | ||
| query.invalidate(); | ||
| }, | ||
| }, | ||
|
|
||
| type: "error", | ||
| }); | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Keep toast side effects out of the shared query client.
createQueryClient() is also used from apps/portal/src/lib/orpc/query/hydration.tsx, so this onError path can run during server-side prefetches too. Calling toastManager.add() there couples SSR to a client UI API, and the raw error.message is sent straight to users. Guard the toast to the browser and log detailed errors server-side instead.
Suggested fix
queryCache: new QueryCache({
onError: (error, query) => {
+ if (typeof window === "undefined") {
+ console.error("Query failed", {
+ queryKey: query.queryKey,
+ error,
+ });
+ return;
+ }
+
const id = toastManager.add({
- title: "Error",
- description: `Error: ${error.message}`,
+ title: "Something went wrong",
+ description: "Please try again.",
actionProps: {
children: "Retry",As per coding guidelines, "Log errors with contextual information (userId, path, etc.) for debugging and monitoring".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| queryCache: new QueryCache({ | |
| onError: (error, query) => { | |
| const id = toastManager.add({ | |
| title: "Error", | |
| description: `Error: ${error.message}`, | |
| actionProps: { | |
| children: "Retry", | |
| onClick: () => { | |
| toastManager.close(id); | |
| query.invalidate(); | |
| }, | |
| }, | |
| type: "error", | |
| }); | |
| }, | |
| }), | |
| queryCache: new QueryCache({ | |
| onError: (error, query) => { | |
| if (typeof window === "undefined") { | |
| console.error("Query failed", { | |
| queryKey: query.queryKey, | |
| error, | |
| }); | |
| return; | |
| } | |
| const id = toastManager.add({ | |
| title: "Something went wrong", | |
| description: "Please try again.", | |
| actionProps: { | |
| children: "Retry", | |
| onClick: () => { | |
| toastManager.close(id); | |
| query.invalidate(); | |
| }, | |
| }, | |
| type: "error", | |
| }); | |
| }, | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/lib/orpc/query/client.ts` around lines 12 - 29, The onError
handler attached to QueryCache is invoking toastManager.add during SSR and
exposing raw error.message; update the QueryCache({ onError }) implementation to
guard UI side-effects by checking for browser execution (e.g., typeof window !==
'undefined') before calling toastManager.add and show a generic user-facing
message and Retry action that calls query.invalidate(); for non-browser (SSR)
paths, call your server logger (or console.error) with full contextual details
(error, query key, route/user context) instead of adding a toast so errors are
recorded without leaking messages to users.
| default: | ||
| "not-disabled:inset-shadow-[0_1px_--theme(--color-white/16%)] border-primary bg-primary text-primary-foreground shadow-primary/24 shadow-xs hover:bg-primary/90 data-pressed:bg-primary/90 [:active,[data-pressed]]:inset-shadow-[0_1px_--theme(--color-black/8%)] [:disabled,:active,[data-pressed]]:shadow-none", | ||
| destructive: | ||
| "not-disabled:inset-shadow-[0_1px_--theme(--color-white/16%)] border-destructive bg-destructive text-white shadow-destructive/24 shadow-xs hover:bg-destructive/90 data-pressed:bg-destructive/90 [:active,[data-pressed]]:inset-shadow-[0_1px_--theme(--color-black/8%)] [:disabled,:active,[data-pressed]]:shadow-none", | ||
| "destructive-outline": | ||
| "border-input bg-popover not-dark:bg-clip-padding text-destructive-foreground shadow-xs/5 not-disabled:not-active:not-data-pressed:before:shadow-[0_1px_--theme(--color-black/4%)] hover:border-destructive/32 hover:bg-destructive/4 data-pressed:border-destructive/32 data-pressed:bg-destructive/4 dark:bg-input/32 dark:not-disabled:before:shadow-[0_-1px_--theme(--color-white/2%)] dark:not-disabled:not-active:not-data-pressed:before:shadow-[0_-1px_--theme(--color-white/6%)] [:disabled,:active,[data-pressed]]:shadow-none", | ||
| ghost: "border-transparent text-foreground hover:bg-accent data-pressed:bg-accent", | ||
| link: "border-transparent underline-offset-4 hover:underline data-pressed:underline", | ||
| outline: | ||
| "border-border bg-background hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:border-input dark:bg-input/30 dark:hover:bg-input/50", | ||
| "border-input bg-popover not-dark:bg-clip-padding text-foreground shadow-xs/5 not-disabled:not-active:not-data-pressed:before:shadow-[0_1px_--theme(--color-black/4%)] hover:bg-accent/50 data-pressed:bg-accent/50 dark:bg-input/32 dark:data-pressed:bg-input/64 dark:hover:bg-input/64 dark:not-disabled:before:shadow-[0_-1px_--theme(--color-white/2%)] dark:not-disabled:not-active:not-data-pressed:before:shadow-[0_-1px_--theme(--color-white/6%)] [:disabled,:active,[data-pressed]]:shadow-none", | ||
| secondary: | ||
| "bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground", | ||
| ghost: "hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:hover:bg-muted/50", | ||
| destructive: | ||
| "bg-destructive/10 text-destructive hover:bg-destructive/20 focus-visible:border-destructive/40 focus-visible:ring-destructive/20 dark:bg-destructive/20 dark:focus-visible:ring-destructive/40 dark:hover:bg-destructive/30", | ||
| link: "text-primary underline-offset-4 hover:underline", | ||
| "border-transparent bg-secondary text-secondary-foreground hover:bg-secondary/90 data-pressed:bg-secondary/90 [:active,[data-pressed]]:bg-secondary/80", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "button.tsx" packages/ui/src/components/ -x wc -l {}Repository: itzTedx/ZironTap
Length of output: 100
🏁 Script executed:
cat -n packages/ui/src/components/button.tsxRepository: itzTedx/ZironTap
Length of output: 5474
🏁 Script executed:
rg "useRender|Button.*from.*base-ui" packages/ui/src/components/ -A 3 -B 1Repository: itzTedx/ZironTap
Length of output: 23094
🏁 Script executed:
rg "data-pressed" packages/ui/src/components/ --type tsxRepository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
git log --oneline -n 20 packages/ui/src/components/button.tsxRepository: itzTedx/ZironTap
Length of output: 120
🏁 Script executed:
fd "package.json" -e json | head -3 | xargs grep "@base-ui/react" -A 1Repository: itzTedx/ZironTap
Length of output: 44
🏁 Script executed:
fd "button\.(test|spec)" packages/ui/src/components/Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "data-pressed|useRender.*button" packages/ -l --type-list 2>/dev/null | head -10Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "onMouseDown|onKeyDown|onPress" packages/ui/src/components/button.tsxRepository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
cat package.json | grep -A 5 "@base-ui/react"Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "data-pressed" . --type-listRepository: itzTedx/ZironTap
Length of output: 449
🏁 Script executed:
rg "data-pressed" -g "*.tsx" -g "*.ts" -g "*.json" -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 7595
🏁 Script executed:
head -20 packages/ui/src/components/toggle.tsxRepository: itzTedx/ZironTap
Length of output: 1455
🏁 Script executed:
cat packages/ui/src/components/select.tsx | grep -A 10 "import.*Select"Repository: itzTedx/ZironTap
Length of output: 1730
🏁 Script executed:
web_searchRepository: itzTedx/ZironTap
Length of output: 110
🌐 Web query:
Base UI React @base-ui/react Button component data-pressed
💡 Result:
In Base UI (@base-ui/react) the plain Button component does not expose a data-pressed attribute—its documented state data attribute is data-disabled (present when disabled). [1]
If you need a “pressed/on” state with a data-pressed attribute, use Toggle (or ToggleGroup). Toggle renders data-pressed when it’s pressed. [2]
Sources:
[1] Base UI Button docs (attributes) https://base-ui.com/react/components/button
[2] Base UI Toggle docs (attributes) https://base-ui.com/react/components/toggle
Use Base UI's Button primitive or manually handle pressed/disabled/keyboard semantics.
The variant classes depend on data-pressed selectors (lines 34–45), but useRender without a Button primitive never sets this attribute. Base UI's Button doesn't expose data-pressed—that comes from Toggle. Additionally, line 57 sets type = undefined when render is provided, causing custom render targets inside forms to become unintended submit buttons. Custom render targets also lose keyboard button semantics (Enter/Space), disabled behavior, and semantic HTML meaning, violating WCAG 2.1 AA.
Either use Base UI's Button primitive directly or manually handle data-pressed, keyboard events, disabled attribute propagation, and type="button" for all render paths to preserve accessibility and form behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/button.tsx` around lines 34 - 45, The variant CSS
relies on a data-pressed attribute and proper button semantics but useRender and
the current render path never set it and also sets type = undefined (breaking
form/button behavior); update the Button component (references: useRender, prop
render, the code that sets type = undefined, and the variant classes that use
data-pressed) to either: 1) switch to Base UI’s Button primitive so pressed
state/attributes are provided, or 2) if keeping a custom render path, explicitly
manage accessibility by setting/propagating data-pressed when pressed,
forwarding disabled to the rendered element, ensuring keyboard handling for
Enter/Space (onKeyDown/onKeyUp) to trigger click behavior, and defaulting to
type="button" for custom render targets instead of type = undefined so custom
renders in forms don’t submit unintentionally.
| {showTrigger && ( | ||
| <ComboboxTrigger | ||
| className={cn( | ||
| "absolute top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-opacity pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 has-[+[data-slot=combobox-clear]]:hidden sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0", | ||
| sizeValue === "sm" ? "end-0" : "end-0.5" | ||
| )} | ||
| {...triggerProps} | ||
| > | ||
| <ComboboxPrimitive.Icon data-slot="combobox-icon"> | ||
| <ChevronsUpDownIcon /> | ||
| </ComboboxPrimitive.Icon> | ||
| </ComboboxTrigger> | ||
| )} | ||
| {showClear && ( | ||
| <ComboboxClear | ||
| className={cn( | ||
| "absolute top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-opacity pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 has-[+[data-slot=combobox-clear]]:hidden sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0", | ||
| sizeValue === "sm" ? "end-0" : "end-0.5" | ||
| )} | ||
| {...clearProps} | ||
| > | ||
| <XIcon /> | ||
| </ComboboxClear> | ||
| )} |
There was a problem hiding this comment.
Add visible focus styles to the icon buttons.
ComboboxTrigger and ComboboxClear both remove the default outline but never replace it, so keyboard users can land on them with no visible focus indicator.
♿ Proposed fix
- "absolute top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-opacity pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 has-[+[data-slot=combobox-clear]]:hidden sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0"
+ "absolute top-1/2 inline-flex size-8 shrink-0 -translate-y-1/2 cursor-pointer items-center justify-center rounded-md border border-transparent opacity-80 outline-none transition-opacity focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background pointer-coarse:after:absolute pointer-coarse:after:min-h-11 pointer-coarse:after:min-w-11 hover:opacity-100 has-[+[data-slot=combobox-clear]]:hidden sm:size-7 [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg]:pointer-events-none [&_svg]:shrink-0"Apply the same focus-visible classes to both button class strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/combobox.tsx` around lines 100 - 123,
ComboboxTrigger and ComboboxClear remove the default outline so keyboard users
lack a visible focus indicator; update the className strings on both
ComboboxTrigger and ComboboxClear (the elements using triggerProps and
clearProps) to include the same focus-visible utility classes (e.g.,
focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2
focus-visible:outline-primary or your design system equivalent) so they render a
clear focus ring when focused via keyboard.
| "use client"; | ||
|
|
||
| import { Dialog as SheetPrimitive } from "@base-ui/react/dialog"; | ||
| import { XIcon } from "lucide-react"; | ||
|
|
||
| import { Button } from "@ziron/ui/components/button"; | ||
| import { ScrollArea } from "@ziron/ui/components/scroll-area"; | ||
| import { cn } from "@ziron/ui/lib/utils"; |
There was a problem hiding this comment.
Missing React import for React.ComponentProps usage.
The file uses React.ComponentProps<"div"> on lines 113, 130, and 171, but React is not imported. This will cause a TypeScript compilation error.
🐛 Proposed fix
"use client";
+import * as React from "react";
+
import { Dialog as SheetPrimitive } from "@base-ui/react/dialog";
import { XIcon } from "lucide-react";Alternatively, use direct imports:
+import type { ComponentProps } from "react";Then replace React.ComponentProps<"div"> with ComponentProps<"div">.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "use client"; | |
| import { Dialog as SheetPrimitive } from "@base-ui/react/dialog"; | |
| import { XIcon } from "lucide-react"; | |
| import { Button } from "@ziron/ui/components/button"; | |
| import { ScrollArea } from "@ziron/ui/components/scroll-area"; | |
| import { cn } from "@ziron/ui/lib/utils"; | |
| "use client"; | |
| import * as React from "react"; | |
| import { Dialog as SheetPrimitive } from "@base-ui/react/dialog"; | |
| import { XIcon } from "lucide-react"; | |
| import { Button } from "@ziron/ui/components/button"; | |
| import { ScrollArea } from "@ziron/ui/components/scroll-area"; | |
| import { cn } from "@ziron/ui/lib/utils"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/sheet.tsx` around lines 1 - 8, The sheet.tsx file
is using React.ComponentProps<"div"> (seen on the
SheetRoot/SheetContent/SheetFooter props) but React is not imported; add an
import to fix TypeScript errors — either add `import React from "react"` at the
top so React.ComponentProps resolves, or import the type directly with `import {
ComponentProps } from "react"` and replace `React.ComponentProps<"div">` with
`ComponentProps<"div">` in the SheetRoot/SheetContent/SheetFooter (or any other
components referencing those types).
| "inline-flex h-[calc(var(--thumb-size)+2px)] w-[calc(var(--thumb-size)*2-2px)] shrink-0 items-center rounded-full p-px outline-none transition-[background-color,box-shadow] duration-200 [--thumb-size:--spacing(5)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-checked:bg-primary data-unchecked:bg-input data-disabled:opacity-64 sm:[--thumb-size:--spacing(4)]", | ||
| className | ||
| )} | ||
| data-slot="switch" | ||
| {...props} | ||
| > | ||
| <SwitchPrimitive.Thumb | ||
| className={cn( | ||
| "pointer-events-none block aspect-square h-full origin-left in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:not-data-disabled:scale-x-110 in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:rounded-[var(--thumb-size)/calc(var(--thumb-size)*1.1)] rounded-(--thumb-size) bg-background shadow-sm/5 will-change-transform [transition:translate_.15s,border-radius_.15s,scale_.1s_.1s,transform-origin_.15s] data-checked:origin-[var(--thumb-size)_50%] data-checked:translate-x-[calc(var(--thumb-size)-4px)]" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Do CSS calc() expressions require whitespace around + and - operators, and in Tailwind arbitrary values should those spaces be encoded with underscores (for example h-[calc(var(--x)_+_2px)])?
💡 Result:
Yes.
-
In CSS
calc(), when+or-is used as a binary operator (addition/subtraction), whitespace is required on both sides (e.g.calc(100% - 2rem)), per the CSS Values & Units spec;*and/don’t have this requirement. [1] (MDN notes the same rule.) [2] -
In Tailwind arbitrary values (
[...]), if the value needs spaces, you should write them as underscores (_) and Tailwind will convert them back to spaces at build time. Soh-[calc(var(--x)_+_2px)](and..._-_...for subtraction) is the correct pattern. [3]
Sources:
[1] https://www.w3.org/TR/css-values-4/
[2] https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/calc
[3] https://tailwindcss.com/docs/adding-custom-styles#handling-whitespace
Fix invalid calc() syntax in Tailwind arbitrary values.
The h-*, w-*, and translate-x-* classes use calc() without required whitespace around + and - operators. In Tailwind arbitrary values, spaces must be encoded with underscores; otherwise these utilities compile to invalid CSS and the switch sizing and thumb positioning breaks.
Proposed fix
- "inline-flex h-[calc(var(--thumb-size)+2px)] w-[calc(var(--thumb-size)*2-2px)] shrink-0 items-center rounded-full p-px outline-none transition-[background-color,box-shadow] duration-200 [--thumb-size:--spacing(5)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-checked:bg-primary data-unchecked:bg-input data-disabled:opacity-64 sm:[--thumb-size:--spacing(4)]",
+ "inline-flex h-[calc(var(--thumb-size)_+_2px)] w-[calc(var(--thumb-size)_*_2_-_2px)] shrink-0 items-center rounded-full p-px outline-none transition-[background-color,box-shadow] duration-200 [--thumb-size:--spacing(5)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-checked:bg-primary data-unchecked:bg-input data-disabled:opacity-64 sm:[--thumb-size:--spacing(4)]",
...
- "pointer-events-none block aspect-square h-full origin-left in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:not-data-disabled:scale-x-110 in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:rounded-[var(--thumb-size)/calc(var(--thumb-size)*1.1)] rounded-(--thumb-size) bg-background shadow-sm/5 will-change-transform [transition:translate_.15s,border-radius_.15s,scale_.1s_.1s,transform-origin_.15s] data-checked:origin-[var(--thumb-size)_50%] data-checked:translate-x-[calc(var(--thumb-size)-4px)]"
+ "pointer-events-none block aspect-square h-full origin-left in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:not-data-disabled:scale-x-110 in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:rounded-[var(--thumb-size)/calc(var(--thumb-size)*1.1)] rounded-(--thumb-size) bg-background shadow-sm/5 will-change-transform [transition:translate_.15s,border-radius_.15s,scale_.1s_.1s,transform-origin_.15s] data-checked:origin-[var(--thumb-size)_50%] data-checked:translate-x-[calc(var(--thumb-size)_-_4px)]"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "inline-flex h-[calc(var(--thumb-size)+2px)] w-[calc(var(--thumb-size)*2-2px)] shrink-0 items-center rounded-full p-px outline-none transition-[background-color,box-shadow] duration-200 [--thumb-size:--spacing(5)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-checked:bg-primary data-unchecked:bg-input data-disabled:opacity-64 sm:[--thumb-size:--spacing(4)]", | |
| className | |
| )} | |
| data-slot="switch" | |
| {...props} | |
| > | |
| <SwitchPrimitive.Thumb | |
| className={cn( | |
| "pointer-events-none block aspect-square h-full origin-left in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:not-data-disabled:scale-x-110 in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:rounded-[var(--thumb-size)/calc(var(--thumb-size)*1.1)] rounded-(--thumb-size) bg-background shadow-sm/5 will-change-transform [transition:translate_.15s,border-radius_.15s,scale_.1s_.1s,transform-origin_.15s] data-checked:origin-[var(--thumb-size)_50%] data-checked:translate-x-[calc(var(--thumb-size)-4px)]" | |
| "inline-flex h-[calc(var(--thumb-size)_+_2px)] w-[calc(var(--thumb-size)_*_2_-_2px)] shrink-0 items-center rounded-full p-px outline-none transition-[background-color,box-shadow] duration-200 [--thumb-size:--spacing(5)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-checked:bg-primary data-unchecked:bg-input data-disabled:opacity-64 sm:[--thumb-size:--spacing(4)]", | |
| className | |
| )} | |
| data-slot="switch" | |
| {...props} | |
| > | |
| <SwitchPrimitive.Thumb | |
| className={cn( | |
| "pointer-events-none block aspect-square h-full origin-left in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:not-data-disabled:scale-x-110 in-[[role=switch]:active,[data-slot=label]:active,[data-slot=field-label]:active]:rounded-[var(--thumb-size)/calc(var(--thumb-size)*1.1)] rounded-(--thumb-size) bg-background shadow-sm/5 will-change-transform [transition:translate_.15s,border-radius_.15s,scale_.1s_.1s,transform-origin_.15s] data-checked:origin-[var(--thumb-size)_50%] data-checked:translate-x-[calc(var(--thumb-size)_-_4px)]" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/switch.tsx` around lines 11 - 19, The Tailwind
arbitrary class values in the Switch component use calc() without encoded spaces
which produces invalid CSS; update the className strings used by the switch root
and SwitchPrimitive.Thumb so any calc() expressions encode spaces with
underscores (e.g., change calc(var(--thumb-size)+2px) to
calc(var(--thumb-size)_+_2px), calc(var(--thumb-size)-4px) to
calc(var(--thumb-size)_-_4px), and any other calc uses) so Tailwind compiles
valid CSS—locate the className in the switch root and the SwitchPrimitive.Thumb
definition and replace the calc() usages accordingly.
| function TabsPanel({ className, ...props }: TabsPrimitive.Panel.Props) { | ||
| return <TabsPrimitive.Panel className={cn("flex-1 outline-none", className)} data-slot="tabs-content" {...props} />; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/ui/src/components && cat -n tabs.tsx | head -80Repository: itzTedx/ZironTap
Length of output: 3304
🏁 Script executed:
rg "TabsPrimitive" packages/ui/src/components/tabs.tsx -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 2081
🏁 Script executed:
head -20 packages/ui/src/components/tabs.tsxRepository: itzTedx/ZironTap
Length of output: 500
🏁 Script executed:
rg "TabsPanel|TabsContent" --type tsx --type ts -B 2 -A 2 | head -50Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
rg "focus-visible" packages/ui/src/components/ --type tsx | grep -i panelRepository: itzTedx/ZironTap
Length of output: 87
🌐 Web query:
WCAG 2.1 AA tab panel focus indicator keyboard navigation accessibility
💡 Result:
For a WCAG 2.1 AA-conformant tab panel (tabs) component, focus on these areas:
1) Keyboard operation (required)
- All tab functionality must work by keyboard (no mouse required). (WCAG 2.1.1 Keyboard, Level A) [1]
- No keyboard trap: users must be able to move focus into and out of the tab set with standard keys (Tab / Shift+Tab). (WCAG 2.1.2, Level A—commonly evaluated alongside 2.1.1) [1]
2) Expected tab/tabs keyboard pattern (best practice; widely used)
Follow the WAI-ARIA Authoring Practices tabs (“tablist”) interaction model:
- Tab: moves focus into the tablist (typically to the selected tab), then out of the tablist to the next focusable element.
- Arrow keys: move focus between tabs (Left/Right or Up/Down depending on orientation).
- Home/End: move focus to first/last tab (commonly supported).
- Enter/Space: (commonly) activates the focused tab when using “manual activation”. [2][3]
(Using this pattern isn’t “a WCAG requirement” by itself, but it’s the standard behavior users and AT expect for tabs.) [2][3]
3) Focus order (required)
- Focus should move in an order that preserves meaning and operability (e.g., tablist → active tab → tabpanel content → next control). (WCAG 2.4.3 Focus Order, Level A) [4]
4) Visible focus indicator (required) + contrast (AA)
- Whatever has keyboard focus must show a visible focus indicator (e.g., outline/border) and it must not disappear. (WCAG 2.4.7 Focus Visible, Level AA) [5]
- Your focus indicator and key component visuals must meet 3:1 contrast against adjacent colors, including focused/selected states. (WCAG 1.4.11 Non-text Contrast, Level AA) [6]
5) ARIA / semantics (strongly recommended for tabs)
Typical accessible markup/behavior:
role="tablist"on the container.- Each tab:
role="tab",aria-selected="true|false", and (commonly) roving tabindex (only the active tab istabindex="0", others-1). [3] - Each panel:
role="tabpanel", linked viaaria-controls/aria-labelledby; non-active panels should be hidden (and not contain focusable elements reachable while hidden). [3]
If you share your current HTML/JS (or a link), I can map it to these checks and point out exactly what will fail/pass WCAG 2.1 AA.
🏁 Script executed:
rg "base-ui" packages/ui/src/components/ --type js --type ts -iRepository: itzTedx/ZironTap
Length of output: 5862
🏁 Script executed:
rg "outline-none" packages/ui/src/components/ -A 1 -B 1Repository: itzTedx/ZironTap
Length of output: 25516
Add a visible focus indicator to the tab panel.
The TabsPanel component removes the outline on line 68 but provides no focus-visible replacement. This creates an accessibility gap for keyboard users, as the panel can receive focus during tab navigation without any visible indication.
All other focusable components in the UI library (TabsTab, Button, Toggle, Switch, Radio, Checkbox, etc.) pair outline-none with focus-visible:ring-2 focus-visible:ring-ring. TabsPanel should follow the same pattern to comply with WCAG 2.1 AA requirements for visible focus indicators.
Suggested fix
function TabsPanel({ className, ...props }: TabsPrimitive.Panel.Props) {
- return <TabsPrimitive.Panel className={cn("flex-1 outline-none", className)} data-slot="tabs-content" {...props} />;
+ return (
+ <TabsPrimitive.Panel
+ className={cn("flex-1 outline-none focus-visible:ring-2 focus-visible:ring-ring", className)}
+ data-slot="tabs-content"
+ {...props}
+ />
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function TabsPanel({ className, ...props }: TabsPrimitive.Panel.Props) { | |
| return <TabsPrimitive.Panel className={cn("flex-1 outline-none", className)} data-slot="tabs-content" {...props} />; | |
| function TabsPanel({ className, ...props }: TabsPrimitive.Panel.Props) { | |
| return ( | |
| <TabsPrimitive.Panel | |
| className={cn("flex-1 outline-none focus-visible:ring-2 focus-visible:ring-ring", className)} | |
| data-slot="tabs-content" | |
| {...props} | |
| /> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/tabs.tsx` around lines 67 - 68, TabsPanel
currently strips the native outline but doesn't add a visible focus style;
update the className in the TabsPanel component (function TabsPanel) to pair
"outline-none" with the same focus-visible classes used across the UI library
(e.g. "focus-visible:ring-2 focus-visible:ring-ring") so keyboard users get a
clear focus indicator; ensure you merge with the existing cn call (cn("flex-1
outline-none", className)) by adding the focus-visible classes to that string so
the panel shows the ring when focused.
| {toast.actionProps && ( | ||
| <Toast.Action className={buttonVariants({ size: "xs" })} data-slot="toast-action"> | ||
| {toast.actionProps.children} | ||
| </Toast.Action> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/ui/src/components/toast.tsx | head -250Repository: itzTedx/ZironTap
Length of output: 13152
🏁 Script executed:
rg "actionProps" packages/ui/src/components/toast.tsx -A 3 -B 3Repository: itzTedx/ZironTap
Length of output: 650
🏁 Script executed:
rg "toast\." packages/ui/src/ -t ts -t tsx --max-count 50 | grep -i action | head -20Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
fd -e ts -e tsx | xargs grep -l "Toast\.create\|createToastManager\|toast.*action" | head -10Repository: itzTedx/ZironTap
Length of output: 96
🏁 Script executed:
rg "toastManager\|anchoredToastManager" packages/ui/src/components/toast.tsx -A 5 -B 2Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "show\(" packages/ -t ts -t tsx --max-count 30 | grep -i toast | head -15Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
grep -r "Toast\.Action" packages/ --include="*.ts" --include="*.tsx" -A 2 -B 2 | head -30Repository: itzTedx/ZironTap
Length of output: 1217
🌐 Web query:
Base UI React Toast.Action component props API
💡 Result:
Toast.Action (Base UI React) — props API
Toast.Action renders a <button> and therefore supports normal button props (e.g. onClick, disabled, type, etc.). In addition, Base UI documents these component-specific props: [1]
nativeButton?: boolean(default:true) — when you replace the element viarender, controls whether it should still behave as a native button; setfalseif your rendered element isn’t a button (e.g. adiv). [1]className?: string | (state: Toast.Action.State) => string | undefined— string or state-derived className. [1]style?: React.CSSProperties | (state: Toast.Action.State) => React.CSSProperties | undefined— inline styles or state-derived styles. [1]render?: ReactElement | (props: HTMLProps, state: Toast.Action.State) => ReactElement— replace/compose the underlying element (render prop). [1]
Also exposed on the rendered element:
data-type— the toast’stype. [1]
Source: Base UI Toast docs → API reference → Action. [1]
🏁 Script executed:
rg "actionProps" packages/ -t ts -t tsx --max-count 50 -B 3 -A 3Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
find packages -name "*.ts" -o -name "*.tsx" | xargs grep -l "show\|notify\|toast" | head -10Repository: itzTedx/ZironTap
Length of output: 334
🏁 Script executed:
rg "toastManager\.show\|anchoredToastManager\.show" packages/ -B 2 -A 5Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "toastManager\|anchoredToastManager" --type ts --type tsx -B 2 -A 3Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
cat -n package.json | grep -i "@base-ui\|dependencies"Repository: itzTedx/ZironTap
Length of output: 88
🏁 Script executed:
rg "\.show\(" packages/ --type ts --type tsx | head -20Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
rg "show\(" packages/ -A 3 | head -40Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
cat package.json | grep -A 5 "@base-ui"Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
rg "actionProps:" packages/ -B 5 -A 5Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
find . -maxdepth 2 -name "package.json" -type f | xargs grep "@base-ui"Repository: itzTedx/ZironTap
Length of output: 44
🏁 Script executed:
rg "actionProps" --no-binaryRepository: itzTedx/ZironTap
Length of output: 414
🏁 Script executed:
rg "Toast\." packages/ui/src/components/toast.tsx | head -30Repository: itzTedx/ZironTap
Length of output: 1656
🏁 Script executed:
cat -n apps/portal/src/lib/orpc/query/client.ts | grep -B 10 -A 10 "actionProps:"Repository: itzTedx/ZironTap
Length of output: 656
🏁 Script executed:
cat -n apps/portal/src/lib/orpc/query/client.ts | grep -B 5 -A 15 "actionProps:"Repository: itzTedx/ZironTap
Length of output: 646
🏁 Script executed:
rg "actionProps\s*:" packages/ apps/ --type ts --type tsx -A 5Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
rg "Toast\.Action" apps/ packages/ -B 2 -A 5Repository: itzTedx/ZironTap
Length of output: 1533
🌐 Web query:
Base UI Toast.Action onClick handler props API documentation
💡 Result:
In Base UI (base-ui.com) Toast, the click handler for the action button is documented under the toast actionProps option (used with toastManager.add(...), update(...), or promise(...)), not as a dedicated Toast.Action onClick prop.
- Type:
actionProps: React.ComponentPropsWithRef<'button'>(internally shown as button HTML props, omittingref) — so it includes standard button props likeonClick,type,disabled, etc. [1] - Example (from the docs pattern):
toastManager.add({ description: 'Saved', actionProps: { children: 'Undo', onClick() { // undo logic }, }, }); ``` [1]
Source: Base UI Toast docs (“Undo action” + API reference for actionProps). [1]
Spread the full actionProps object to Toast.Action to preserve click handlers and other button properties.
The code currently forwards only toast.actionProps.children, dropping the onClick handler and any other button props (e.g., disabled, type, aria-label). The button will render but won't respond to clicks.
Example fix:
{toast.actionProps && (
<Toast.Action
{...toast.actionProps}
className={cn(buttonVariants({ size: "xs" }), toast.actionProps.className)}
data-slot="toast-action"
/>
)}Apply to both toast renderers (lines 131–135 and 211–217).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/toast.tsx` around lines 131 - 135, The
Toast.Action is currently rendering only toast.actionProps.children and dropping
handlers/props; update both render locations where Toast.Action is used to
spread the entire toast.actionProps ({...toast.actionProps}) onto the
Toast.Action component, merge/append the existing className from
buttonVariants({ size: "xs" }) with toast.actionProps.className (e.g., via your
cn helper or string concat), and keep data-slot="toast-action" so click handlers
(onClick), disabled, aria-* and other props are preserved.
| const ToggleGroupContext = React.createContext<VariantProps<typeof toggleVariants>>({ | ||
| size: "default", | ||
| variant: "default", | ||
| }); |
There was a problem hiding this comment.
size/variant are effectively hard-wired to "default" here.
Because the context is created with concrete defaults, useContext() never represents “no group”, so a standalone <Toggle variant="outline" size="sm" /> will still resolve to "default". The same logic also makes per-item overrides inside a group unreachable.
💡 Suggested fix
-const ToggleGroupContext = React.createContext<VariantProps<typeof toggleVariants>>({
- size: "default",
- variant: "default",
-});
+const ToggleGroupContext = React.createContext<
+ VariantProps<typeof toggleVariants> | null
+>(null);
@@
- const resolvedVariant = context.variant || variant;
- const resolvedSize = context.size || size;
+ const resolvedVariant = variant ?? context?.variant ?? "default";
+ const resolvedSize = size ?? context?.size ?? "default";Also applies to: 46-46, 58-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/toggle-group.tsx` around lines 13 - 16, The
context is created with concrete defaults which hard-wires size/variant to
"default"; change ToggleGroupContext to use a nullable type (e.g.,
VariantProps<typeof toggleVariants> | undefined) and initialize it with
undefined (React.createContext<VariantProps<typeof toggleVariants> |
undefined>(undefined)); then update consumers (where
useContext(ToggleGroupContext) is read inside Toggle and ToggleItem logic) to
explicitly fall back to prop values or the component defaults when the context
is undefined so standalone <Toggle> props and per-item overrides inside a group
work correctly.
…nstack/react-form and react dependencies
…eadability and consistency
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (4)
apps/portal/src/lib/orpc/client.ts (1)
9-10:⚠️ Potential issue | 🟠 MajorDon't freeze the fallback RPC client at import time.
Lines 10 and 48 are both evaluated at module load, so if
globalThis.$clientis still unset the process permanently falls back tohttp://localhost:3000. That breaks SSR outside local dev, andorpcon Line 50 inherits the same stale client. Resolve the URL/client lazily, or guarantee the server initializer runs before this module is imported.Also applies to: 48-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/lib/orpc/client.ts` around lines 9 - 10, The RPC client and URL are being evaluated at module load which causes a permanent fallback to "http://localhost:3000" if globalThis.$client isn't set; to fix, make link and orpc lazy-initialized instead of top-level constants: create a getter function (e.g., getRPCLink/getOrpc) that computes the base URL using typeof window !== "undefined" ? window.location.origin : process/server-provided value and checks globalThis.$client at call time, then constructs a new RPCLink (symbol: RPCLink) and the orpc instance only when first requested; replace direct uses of the top-level link and orpc with calls to these getters so the server initializer can set globalThis.$client before the client is created.apps/portal/src/app/api/rpc/[[...rest]]/route.ts (2)
32-36:⚠️ Potential issue | 🟠 MajorThrottle the public route before building context.
Line 36 builds request context for every hit, including
/api/rpc/docs, before any abuse guard runs. Add route-level@ziron/rate-limitbeforecreateContextso burst traffic is rejected with 429 before it fans out into context work.As per coding guidelines, "Apply rate limiting via
@ziron/rate-limiton oRPC and public routes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app/api/rpc/`[[...rest]]/route.ts around lines 32 - 36, In handleRequest, apply the `@ziron/rate-limit` check before building the request context so high-volume requests (e.g. /api/rpc/docs) are rejected with 429 before createContext runs; modify handleRequest to invoke the rate limiter early (route-level usage) and return the 429 response on throttle, then only call createContext(req) after the limiter passes. Ensure you reference the zircon limiter import and use it at the top of handleRequest so public oRPC routes are protected.
32-55:⚠️ Potential issue | 🟠 MajorWrap
handleRequestin a top-leveltry/catch.
createContext(req)on Line 36 can fail before either interceptor is involved, and any uncaught failure inside the handler calls will skip the consistent logging/status mapping this route should provide. Catch once around the whole body and translate known auth/not-found cases before falling back to 500.As per coding guidelines, "Implement server-side error handling with try/catch blocks including context; return appropriate oRPC error codes (UNAUTHORIZED, NOT_FOUND, etc.)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app/api/rpc/`[[...rest]]/route.ts around lines 32 - 55, Wrap the entire body of the async function handleRequest in a top-level try/catch so failures in createContext(req) or in rpcHandler.handle / apiHandler.handle are caught; inside the catch map known errors (auth failures thrown by createContext or auth interceptors -> return an oRPC UNAUTHORIZED response, resource-missing errors -> return oRPC NOT_FOUND) and log the error, and for anything else return a 500 response while preserving consistent logging/status mapping for this route.apps/portal/src/lib/orpc/query/client.ts (1)
9-25:⚠️ Potential issue | 🟠 MajorKeep toast side effects out of the shared query client.
createQueryClient()is reused for server hydration, so thisonErrorpath can run outside the browser. CallingtoastManager.add()there couples SSR to a client UI API, and Line 13 sends rawerror.messageto users. Guard the toast to the browser and log context server-side instead.Suggested fix
queryCache: new QueryCache({ onError: (error, query) => { + if (typeof window === "undefined") { + console.error("Query failed", { + queryKey: query.queryKey, + error, + }); + return; + } + const id = toastManager.add({ - title: "Error", - description: `Error: ${error.message}`, + title: "Something went wrong", + description: "Please try again.",As per coding guidelines, "Log errors with contextual information (userId, path, etc.) for debugging and monitoring".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/lib/orpc/query/client.ts` around lines 9 - 25, The onError handler inside the QueryCache (the onError in new QueryCache({...})) should not call toastManager.add unguarded because createQueryClient() is used during SSR; change the onError to detect browser context (e.g. if (typeof window !== "undefined") or an isBrowser flag) before calling toastManager.add and ensure the user-facing toast shows a generic, non-sensitive message (not raw error.message); for server-side/SSR runs, emit a structured log with contextual info (include the error, the query identifier from the query object such as query.queryKey or query.meta, and any available user/path info) using the server logger (e.g. processLogger or console.error), and keep query.invalidate() behavior triggered only from the UI path after closing the toast.
🧹 Nitpick comments (10)
packages/ui/src/components/field.tsx (2)
182-182: Use strict equality (===) instead of loose equality (==).🔧 Suggested fix
- if (uniqueErrors?.length == 1) { + if (uniqueErrors?.length === 1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/field.tsx` at line 182, Change the loose equality check in the conditional that inspects uniqueErrors length to a strict equality: update the expression using uniqueErrors?.length == 1 to use === instead; locate the conditional around uniqueErrors (the if statement that tests uniqueErrors?.length) in the Field component in components/field.tsx and replace the == operator with === so the comparison is type-safe.
107-118: Consider using a distinctdata-slotforFieldTitle.Both
FieldTitleandFieldLabelusedata-slot="field-label". While this may be intentional for shared styling, it could cause unintended side effects with slot-based selectors (e.g.,*:data-[slot=field-label]) and makes it harder to target each component independently.💡 Suggested change
function FieldTitle({ className, ...props }: React.ComponentProps<"div">) { return ( <div className={cn( "flex w-fit items-center gap-2 font-medium text-sm leading-snug group-data-[disabled=true]/field:opacity-50", className )} - data-slot="field-label" + data-slot="field-title" {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/field.tsx` around lines 107 - 118, FieldTitle uses the same data-slot ("field-label") as FieldLabel which can cause selector collisions; change FieldTitle's data-slot to a distinct value (e.g., "field-title") in the FieldTitle component and update any CSS/JS selectors that currently target data-slot="field-label" when they should only target the label or title respectively so styles/behaviors remain correct; reference the FieldTitle component and ensure corresponding selectors that targeted data-slot="field-label" for titles are adjusted to data-slot="field-title".packages/db/src/schema/auth.ts (1)
72-93: Consider adding a unique constraint oncredentialIDand consistent timestamp handling.
The
credentialIDfield should likely be unique since WebAuthn credential IDs are globally unique identifiers. An index exists but not a uniqueness constraint.The
createdAtfield lacks.notNull()and a default value, unlike other tables that use thetimestamps()helper or explicit defaults.♻️ Proposed improvements
export const passkeys = pgTable( "passkeys", { id: id(), name: text("name"), publicKey: text("public_key").notNull(), userId: uuid("user_id") .notNull() .references(() => users.id, { onDelete: "cascade" }), - credentialID: text("credential_id").notNull(), + credentialID: text("credential_id").notNull().unique(), counter: integer("counter").notNull(), deviceType: text("device_type").notNull(), backedUp: boolean("backed_up").notNull(), transports: text("transports"), - createdAt: timestamp("created_at"), + createdAt: timestamp("created_at").defaultNow().notNull(), aaguid: text("aaguid"), }, (table) => [ index("passkeys_userId_idx").on(table.userId), - index("passkeys_credentialID_idx").on(table.credentialID), + uniqueIndex("passkeys_credentialID_uidx").on(table.credentialID), ] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/schema/auth.ts` around lines 72 - 93, Add a uniqueness constraint for WebAuthn credential IDs and make createdAt consistent with other tables: in the passkeys pgTable definition ensure credentialID (text("credential_id")) is declared unique (e.g., via a unique index or unique constraint instead of only index("passkeys_credentialID_idx")) and update createdAt (timestamp("created_at")) to be notNull with a sensible default timestamp (or replace it with the project timestamps() helper) so creation times are always present and consistent with other schema entries.pnpm-workspace.yaml (1)
27-27: Avoid duplicatingreactacross catalogs to maintain a single source of truth.Line 27 introduces
reactinto the default catalog whilereact-domremains only incatalogs.react19. This creates the potential for packages to inconsistently referencereactfromcatalog:whilereact-domcomes fromcatalog:react19. Current workspace packages all correctly usecatalog:react19for both, but the duplication is unnecessary and increases the risk of future drift.Remove the line at 27 and keep
react/react-domtogether inreact19:♻️ Proposed cleanup
- react: ^19.2.4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnpm-workspace.yaml` at line 27, Remove the duplicate entry "react: ^19.2.4" from the default catalog so React is only declared in catalogs.react19 alongside react-dom; ensure catalogs.react19 lists both react and react-dom together (and remove the standalone react line introduced in the default catalog), then re-run workspace install to validate packages still resolve via catalog:react19.apps/portal/src/app/(auth)/layout.tsx (1)
1-2: Use a<main>landmark for the auth shell.This wrapper is the primary page content for the auth routes, so a plain
divdrops a useful navigation landmark for assistive tech.As per coding guidelines, "Follow WCAG 2.1 AA accessibility standards; use semantic HTML elements (``, ``, ``)."♿ Suggested change
export default function AuthLayout({ children }: { children: React.ReactNode }) { - return <div className="flex h-screen w-screen items-center justify-center">{children}</div>; + return <main className="flex h-screen w-screen items-center justify-center">{children}</main>; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app/`(auth)/layout.tsx around lines 1 - 2, Replace the non-semantic wrapper div in the AuthLayout component with a semantic <main> element so the auth routes expose a primary content landmark; update the JSX returned by the export default function AuthLayout({ children }) to use <main className="flex h-screen w-screen items-center justify-center">{children}</main> (preserve the same className and children prop) to satisfy WCAG/semantic HTML guidelines.apps/portal/package.json (1)
34-36: Move the remaining direct version pins into the catalog.
lucide-reactandnext-themesare still using inline semver ranges here. Please add them topnpm-workspace.yamland reference them withcatalog:so this package stays aligned with the workspace version-management policy.As per coding guidelines, "Manage dependency versions only through the pnpm catalog in
pnpm-workspace.yamlusingcatalog:orcatalog:react19references in package.json."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/package.json` around lines 34 - 36, Package.json currently pins "lucide-react" and "next-themes" with semver ranges; move their versions into the pnpm catalog and reference them with "catalog:" in this package. Add entries for lucide-react and next-themes in pnpm-workspace.yaml (or update existing catalog group) with the desired version strings, then update apps/portal/package.json to replace the version values for "lucide-react" and "next-themes" with "catalog:" (matching the pattern used for "next": "catalog:"). Ensure you use the same catalog key style (e.g., catalog: or catalog:react19 if applicable) so the package adheres to the workspace version-management policy.apps/portal/src/features/auth/components/forms/test-form.tsx (1)
3-29: Use the shared form components here instead of hand-wiring the fields.This form is manually composing
form.Fieldwith low-levelField,Input, andInputGroupprimitives instead of the repo-standard building blocks inpackages/ui/src/components/form/. Please switch it to the shared form inputs and standard TanStack Form wiring before this pattern spreads to the other auth forms.As per coding guidelines, "Use TanStack Form with
@tanstack/zod-form-adapterand shadcn form components for all forms."Also applies to: 39-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/features/auth/components/forms/test-form.tsx` around lines 3 - 29, The BugReportForm currently hand-wires low-level Field, Input and InputGroup primitives; replace those with the shared form components and TanStack Zod adapter used across the repo: use the shared Form, FormField, FormItem, FormLabel, FormControl, FormMessage and FormDescription components (from the UI package's form module) and wire the form with `@tanstack/zod-form-adapter` using formSchema as the Zod schema; update the useForm invocation to use the adapter/validator pattern and replace occurrences where you directly referenced form.Field, Field, Input, InputGroup, FieldLabel, FieldDescription, FieldError, etc., with the corresponding shared Form* components and the form.control/field render helpers so validation and submit handling work consistently with other auth forms.packages/validators/src/auth/test.ts (1)
1-12: Don't publish this bug-report schema underauth/.This validator describes a generic title/description bug-report payload, but
packages/validators/src/index.tsnow re-exports it through./auth. Leaving it here hardens the wrong public API surface and makes future auth imports harder to reason about. Move it to a domain-specific module before other packages start depending on it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/validators/src/auth/test.ts` around lines 1 - 12, The formSchema defined in this file (exported as formSchema) is a generic bug-report title/description validator and must be moved out of the auth domain: create a domain-specific module (e.g., a new bug-report or reports schema file) and move the z.object(...) formSchema there, update its export name if desired, remove the validator from the auth module, and then update packages/validators/src/index.ts to export the schema from the new module instead of re-exporting it through ./auth; finally delete or repoint the original packages/validators/src/auth/test.ts to avoid publishing bug-report schema under auth.packages/validators/src/auth/login.ts (1)
10-10: Prefer an interface for this exported object shape.The repo guidelines ask for
interfaceovertypeon object-shaped TypeScript exports, so wrapping the inferred schema type in an interface would keep this file consistent with the rest of the codebase.♻️ Suggested change
-export type LoginType = z.infer<typeof loginSchema>; +export interface LoginType extends z.infer<typeof loginSchema> {}As per coding guidelines:
Prefer interface over type for defining object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/validators/src/auth/login.ts` at line 10, Replace the exported type alias LoginType with an exported interface per repo guidelines: change the declaration from "export type LoginType = z.infer<typeof loginSchema>;" to an interface form such as "export interface LoginType extends z.infer<typeof loginSchema> {}" so the exported object shape uses an interface while preserving the inferred schema shape from loginSchema.apps/portal/src/features/auth/components/forms/login-form.tsx (1)
24-31: Move these auth fields onto the shared TanStack/Zod form stack.This hand-wires raw
useForm,Field, andInputeven though the repo standard is@tanstack/zod-form-adapterplus the shared form primitives. That drift is already duplicated inregister-form.tsx.As per coding guidelines, "Use TanStack Form with
@tanstack/zod-form-adapterand shadcn form components for all forms" and "Use form components from packages/ui/src/components/form/."Also applies to: 98-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/features/auth/components/forms/login-form.tsx` around lines 24 - 31, The login form is using a raw useForm + Field/Input instead of the repo-standard TanStack Zod adapter and shared shadcn form primitives; update the component to use `@tanstack/zod-form-adapter` with the shared Form, FormField, FormItem, FormControl, and Input components from packages/ui, wiring loginSchema into the adapter (replace the current useForm(...) with the adapter-enabled Form using loginSchema) and refactor places where Field/Input are used (and similar logic in register-form.tsx) to use the shared FormField/FormControl/FormMessage components so validation and styling are consistent across forms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/plans/portal-folder-structure.plan.md:
- Around line 12-21: The fenced code blocks showing the folder trees are
unlabeled and causing markdownlint warnings; update each triple-backtick fence
for the examples (e.g., the block containing the apps/portal/src/ tree and the
other block at lines 43-60 mentioned in the comment) to use a language label
such as "text" (change ``` to ```text) so the Markdown is properly labeled while
preserving the existing content.
- Line 75: The example uses React Router prop `to` on the Link component which
is incorrect for Next.js; update the example in the View/ RegisterView snippet
so the Link uses the `href` prop instead (e.g., change `<Link to="/login">Sign
in</Link>` to use `href`) and ensure the surrounding symbols referenced
(AuthCard, RegisterForm, RegisterView, Link) remain unchanged.
In `@apps/portal/package.json`:
- Around line 8-14: The package scripts currently wrap Next.js commands with the
custom "with-env" script which invokes dotenv-cli; remove that wrapper so
Next.js can load env files using its own precedence. Update the "dev", "build",
and "start" script values to call "next dev -p 3000", "next build", and "next
start" respectively (replacing "pnpm with-env next ..."), and delete the
"with-env" script entry so dotenv-cli is no longer used.
In `@apps/portal/src/features/auth/api.ts`:
- Around line 7-12: Wrap the existing cached getSession async function in a
try/catch: call auth.api.getSession(...) inside the try (still passing
headers()), and if it succeeds return the session as before; on error catch the
exception, log contextual details (e.g., include that getSession failed and any
error.message/stack using your app logger or console.error), and return null so
server-rendered auth pages don’t crash; keep the cache(...) wrapper and function
name getSession unchanged.
In `@apps/portal/src/features/auth/components/forms/login-form.tsx`:
- Around line 32-86: The onSubmit handler calls authClient.signIn.email inside
startTransition without a surrounding try/catch, so promise rejections that
never hit fetchOptions.onError (e.g., network or parsing failures) are
unhandled; wrap the await authClient.signIn.email(...) call in a try/catch
inside the startTransition callback (and similarly update the onSubmit in
register-form.tsx and the passkey flow in passkey-button.tsx) and in the catch
block call toastManager.add with a generic error message and log the error (or
call router fallback) so all rejections are handled even when
fetchOptions.onError is not invoked.
In `@apps/portal/src/features/auth/components/forms/register-form.tsx`:
- Around line 50-82: The onError handler in register-form.tsx is using login
copy and log text; update the switch in the onError callback (the function
referenced as onError: (ctx) => { ... }) to use registration-appropriate
messages (e.g., replace "Invalid request. Please check your email and password."
with "Invalid signup request. Please check your email and password.", replace
"Invalid email or password. Please try again." with "Invalid email or password
for registration. Please try again.", and change the 403 description context to
reference verifying email for signup if needed) and replace the
console.error("Login error:", error) call with a registration-specific log like
console.error("Registration error:", error) so both user-facing copy and logs
correctly reflect registration flow.
In `@apps/portal/src/features/auth/components/forms/test-form.tsx`:
- Around line 47-56: The form inputs don't enforce the formSchema limits so
users can type past the title (32) and description (100) caps; update the Input
controls (the Input component instances using field.name / field.handleChange /
field.state.value) to include maxLength props that mirror the schema
(maxLength={32} for the title input and maxLength={100} for the description
input), and add the same change to the other corresponding Input block mentioned
(lines 71-85) so the UI prevents typing beyond the schema limits.
In `@apps/portal/src/features/auth/components/is-logged-in.tsx`:
- Around line 5-10: The IsLoggedIn component currently declares IsLoggedInProps
extends React.ComponentProps<"div"> but only uses children, so any passed div
props are dropped; either narrow the props to only accept children (e.g., use
React.PropsWithChildren or a single children prop in IsLoggedInProps) or accept
and forward the div props to a wrapper element: update IsLoggedIn signature to
accept ({ children, ...divProps }: IsLoggedInProps) and when rendering return a
wrapper like <div {...divProps}>{children}</div> only when
useIsLoggedIn().isLoggedIn is true; keep the component name IsLoggedIn and the
useIsLoggedIn call unchanged.
In `@apps/portal/src/features/auth/components/logout-button.tsx`:
- Around line 32-41: The rendered button lacks an explicit type which makes it
act as a submit button inside forms; update the default props passed into
useRender/mergeProps in the logout-button component to include type: "button"
(alongside aria-disabled, className, disabled, onClick: logout) so the element
created by useRender is explicitly a non-submit button.
In `@apps/portal/src/features/auth/components/passkey-button.tsx`:
- Around line 16-24: The conditional mediation probe is being treated
synchronously and may throw if PublicKeyCredential is undefined; wrap the logic
inside an async function called from useEffect, first check that global
PublicKeyCredential and its isConditionalMediationAvailable property exist
(e.g., typeof PublicKeyCredential !== 'undefined' && typeof
PublicKeyCredential.isConditionalMediationAvailable === 'function'), await
PublicKeyCredential.isConditionalMediationAvailable(), and only call
authClient.signIn.passkey({ autoFill: true }) when the awaited result is true;
ensure you await or handle the promise and handle failures (try/catch) so the
early return behavior works as intended.
In `@apps/portal/src/features/auth/views/login-view.tsx`:
- Line 6: Update the product name casing in the AuthCard title prop from "Sign
in to Zirontap" to "Sign in to ZironTap" so the displayed product name matches
the project branding; locate the AuthCard usage in login-view.tsx (the JSX
element with title="Sign in to Zirontap") and change the string accordingly.
In `@packages/api/src/middleware/require-auth.ts`:
- Line 1: The require-auth middleware currently uses RSC APIs (headers() and
cache()) — update it to match the existing auth.ts oRPC pattern: remove imports
of "react" and "next/headers", change the middleware function to accept the oRPC
context parameter (e.g., function requireAuth(context) or the same signature
used in auth.ts), read headers from context.request.headers, and preserve the
same middleware behavior and exported symbol(s) (e.g., requireAuth or
requireAuthMiddleware) so it works in API route/oRPC context and stays
consistent with auth.ts.
In `@packages/auth/src/index.ts`:
- Around line 82-86: The passkey call is passing env.BETTER_AUTH_URL directly as
origin and using new URL(...).hostname for rpID, which can include a path and
break WebAuthn; parse BETTER_AUTH_URL once into a URL object (e.g., const parsed
= new URL(env.BETTER_AUTH_URL)) and pass parsed.hostname to rpID and
parsed.origin to origin in the passkey(...) options to ensure the origin is
normalized to scheme://host[:port] and reuse the parsed URL object.
In `@packages/db/src/migrations/0001_ambitious_moira_mactaggert.sql`:
- Around line 15-18: The cards table migration is missing an organization_id
column required for tenant scoping; update the CREATE TABLE "cards" statement to
add organization_id uuid NOT NULL (with appropriate FK or index if desired) and
also add organizationId: 'uuid' (or the equivalent column/type) to the Card
schema definition in card.ts so the runtime schema matches the migration and
every card is scoped to an organization.
In `@packages/ui/package.json`:
- Line 34: The package currently lists "tw-animate-css" in devDependencies while
exporting "./globals.css" (mapped as "./globals.css":
"./src/styles/globals.css") that `@imports` runtime CSS, and the workspace catalog
is missing the entry; to fix, add a catalog entry for tw-animate-css with the
version ^1.4.0 in pnpm-workspace.yaml, and move the "tw-animate-css" entry from
devDependencies to dependencies in packages/ui/package.json replacing the
version string with "catalog:" so consumers of `@ziron/ui` can resolve the runtime
CSS.
In `@packages/validators/src/auth/register.ts`:
- Around line 6-13: The name field currently allows whitespace-only strings
because min(1) runs before trimming; update the schema for the name string (the
variable/property named "name" in the register validator) to trim input before
validation—use a z.string() transform to trim the value (or a refine that checks
trimmed length) so the subsequent .min(1) and .max(128) apply to the trimmed
name and store the trimmed result across callers.
---
Duplicate comments:
In `@apps/portal/src/app/api/rpc/`[[...rest]]/route.ts:
- Around line 32-36: In handleRequest, apply the `@ziron/rate-limit` check before
building the request context so high-volume requests (e.g. /api/rpc/docs) are
rejected with 429 before createContext runs; modify handleRequest to invoke the
rate limiter early (route-level usage) and return the 429 response on throttle,
then only call createContext(req) after the limiter passes. Ensure you reference
the zircon limiter import and use it at the top of handleRequest so public oRPC
routes are protected.
- Around line 32-55: Wrap the entire body of the async function handleRequest in
a top-level try/catch so failures in createContext(req) or in rpcHandler.handle
/ apiHandler.handle are caught; inside the catch map known errors (auth failures
thrown by createContext or auth interceptors -> return an oRPC UNAUTHORIZED
response, resource-missing errors -> return oRPC NOT_FOUND) and log the error,
and for anything else return a 500 response while preserving consistent
logging/status mapping for this route.
In `@apps/portal/src/lib/orpc/client.ts`:
- Around line 9-10: The RPC client and URL are being evaluated at module load
which causes a permanent fallback to "http://localhost:3000" if
globalThis.$client isn't set; to fix, make link and orpc lazy-initialized
instead of top-level constants: create a getter function (e.g.,
getRPCLink/getOrpc) that computes the base URL using typeof window !==
"undefined" ? window.location.origin : process/server-provided value and checks
globalThis.$client at call time, then constructs a new RPCLink (symbol: RPCLink)
and the orpc instance only when first requested; replace direct uses of the
top-level link and orpc with calls to these getters so the server initializer
can set globalThis.$client before the client is created.
In `@apps/portal/src/lib/orpc/query/client.ts`:
- Around line 9-25: The onError handler inside the QueryCache (the onError in
new QueryCache({...})) should not call toastManager.add unguarded because
createQueryClient() is used during SSR; change the onError to detect browser
context (e.g. if (typeof window !== "undefined") or an isBrowser flag) before
calling toastManager.add and ensure the user-facing toast shows a generic,
non-sensitive message (not raw error.message); for server-side/SSR runs, emit a
structured log with contextual info (include the error, the query identifier
from the query object such as query.queryKey or query.meta, and any available
user/path info) using the server logger (e.g. processLogger or console.error),
and keep query.invalidate() behavior triggered only from the UI path after
closing the toast.
---
Nitpick comments:
In `@apps/portal/package.json`:
- Around line 34-36: Package.json currently pins "lucide-react" and
"next-themes" with semver ranges; move their versions into the pnpm catalog and
reference them with "catalog:" in this package. Add entries for lucide-react and
next-themes in pnpm-workspace.yaml (or update existing catalog group) with the
desired version strings, then update apps/portal/package.json to replace the
version values for "lucide-react" and "next-themes" with "catalog:" (matching
the pattern used for "next": "catalog:"). Ensure you use the same catalog key
style (e.g., catalog: or catalog:react19 if applicable) so the package adheres
to the workspace version-management policy.
In `@apps/portal/src/app/`(auth)/layout.tsx:
- Around line 1-2: Replace the non-semantic wrapper div in the AuthLayout
component with a semantic <main> element so the auth routes expose a primary
content landmark; update the JSX returned by the export default function
AuthLayout({ children }) to use <main className="flex h-screen w-screen
items-center justify-center">{children}</main> (preserve the same className and
children prop) to satisfy WCAG/semantic HTML guidelines.
In `@apps/portal/src/features/auth/components/forms/login-form.tsx`:
- Around line 24-31: The login form is using a raw useForm + Field/Input instead
of the repo-standard TanStack Zod adapter and shared shadcn form primitives;
update the component to use `@tanstack/zod-form-adapter` with the shared Form,
FormField, FormItem, FormControl, and Input components from packages/ui, wiring
loginSchema into the adapter (replace the current useForm(...) with the
adapter-enabled Form using loginSchema) and refactor places where Field/Input
are used (and similar logic in register-form.tsx) to use the shared
FormField/FormControl/FormMessage components so validation and styling are
consistent across forms.
In `@apps/portal/src/features/auth/components/forms/test-form.tsx`:
- Around line 3-29: The BugReportForm currently hand-wires low-level Field,
Input and InputGroup primitives; replace those with the shared form components
and TanStack Zod adapter used across the repo: use the shared Form, FormField,
FormItem, FormLabel, FormControl, FormMessage and FormDescription components
(from the UI package's form module) and wire the form with
`@tanstack/zod-form-adapter` using formSchema as the Zod schema; update the
useForm invocation to use the adapter/validator pattern and replace occurrences
where you directly referenced form.Field, Field, Input, InputGroup, FieldLabel,
FieldDescription, FieldError, etc., with the corresponding shared Form*
components and the form.control/field render helpers so validation and submit
handling work consistently with other auth forms.
In `@packages/db/src/schema/auth.ts`:
- Around line 72-93: Add a uniqueness constraint for WebAuthn credential IDs and
make createdAt consistent with other tables: in the passkeys pgTable definition
ensure credentialID (text("credential_id")) is declared unique (e.g., via a
unique index or unique constraint instead of only
index("passkeys_credentialID_idx")) and update createdAt
(timestamp("created_at")) to be notNull with a sensible default timestamp (or
replace it with the project timestamps() helper) so creation times are always
present and consistent with other schema entries.
In `@packages/ui/src/components/field.tsx`:
- Line 182: Change the loose equality check in the conditional that inspects
uniqueErrors length to a strict equality: update the expression using
uniqueErrors?.length == 1 to use === instead; locate the conditional around
uniqueErrors (the if statement that tests uniqueErrors?.length) in the Field
component in components/field.tsx and replace the == operator with === so the
comparison is type-safe.
- Around line 107-118: FieldTitle uses the same data-slot ("field-label") as
FieldLabel which can cause selector collisions; change FieldTitle's data-slot to
a distinct value (e.g., "field-title") in the FieldTitle component and update
any CSS/JS selectors that currently target data-slot="field-label" when they
should only target the label or title respectively so styles/behaviors remain
correct; reference the FieldTitle component and ensure corresponding selectors
that targeted data-slot="field-label" for titles are adjusted to
data-slot="field-title".
In `@packages/validators/src/auth/login.ts`:
- Line 10: Replace the exported type alias LoginType with an exported interface
per repo guidelines: change the declaration from "export type LoginType =
z.infer<typeof loginSchema>;" to an interface form such as "export interface
LoginType extends z.infer<typeof loginSchema> {}" so the exported object shape
uses an interface while preserving the inferred schema shape from loginSchema.
In `@packages/validators/src/auth/test.ts`:
- Around line 1-12: The formSchema defined in this file (exported as formSchema)
is a generic bug-report title/description validator and must be moved out of the
auth domain: create a domain-specific module (e.g., a new bug-report or reports
schema file) and move the z.object(...) formSchema there, update its export name
if desired, remove the validator from the auth module, and then update
packages/validators/src/index.ts to export the schema from the new module
instead of re-exporting it through ./auth; finally delete or repoint the
original packages/validators/src/auth/test.ts to avoid publishing bug-report
schema under auth.
In `@pnpm-workspace.yaml`:
- Line 27: Remove the duplicate entry "react: ^19.2.4" from the default catalog
so React is only declared in catalogs.react19 alongside react-dom; ensure
catalogs.react19 lists both react and react-dom together (and remove the
standalone react line introduced in the default catalog), then re-run workspace
install to validate packages still resolve via catalog:react19.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3af2546-ea3c-460f-9daf-f386d2ae631b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.cursor/plans/portal-folder-structure.plan.md.vscode/extensions.json.vscode/tasks.jsonapps/portal/biome.jsonapps/portal/next.config.tsapps/portal/package.jsonapps/portal/src/app/(auth)/layout.tsxapps/portal/src/app/(auth)/login/page.tsxapps/portal/src/app/(auth)/register/page.tsxapps/portal/src/app/api/auth/[...all]/route.tsapps/portal/src/app/api/rpc/[[...rest]]/route.tsapps/portal/src/app/layout.tsxapps/portal/src/app/page.tsxapps/portal/src/components/providers.tsxapps/portal/src/components/theme-provider.tsxapps/portal/src/features/auth/api.tsapps/portal/src/features/auth/components/auth-card.tsxapps/portal/src/features/auth/components/forms/login-form.tsxapps/portal/src/features/auth/components/forms/register-form.tsxapps/portal/src/features/auth/components/forms/test-form.tsxapps/portal/src/features/auth/components/is-logged-in.tsxapps/portal/src/features/auth/components/logout-button.tsxapps/portal/src/features/auth/components/passkey-button.tsxapps/portal/src/features/auth/hooks/use-is-logged-in.tsapps/portal/src/features/auth/views/login-view.tsxapps/portal/src/features/auth/views/register-view.tsxapps/portal/src/instrumentation.tsapps/portal/src/lib/auth/client.tsapps/portal/src/lib/orpc/client.tsapps/portal/src/lib/orpc/query/client.tsapps/portal/src/lib/orpc/query/hydration.tsxapps/portal/tsconfig.jsonpackages/api/package.jsonpackages/api/src/middleware/require-auth.tspackages/auth/package.jsonpackages/auth/src/client.tspackages/auth/src/index.tspackages/db/src/helpers/index.tspackages/db/src/index.tspackages/db/src/migrate.tspackages/db/src/migrations/0001_ambitious_moira_mactaggert.sqlpackages/db/src/migrations/meta/0001_snapshot.jsonpackages/db/src/migrations/meta/_journal.jsonpackages/db/src/schema/auth.tspackages/ui/package.jsonpackages/ui/src/base-ui/csp-provider.tspackages/ui/src/base-ui/direction-provider.tspackages/ui/src/base-ui/merge-props.tspackages/ui/src/base-ui/use-render.tspackages/ui/src/components/field.tsxpackages/ui/src/styles/globals.csspackages/validators/src/auth/index.tspackages/validators/src/auth/login.tspackages/validators/src/auth/register.tspackages/validators/src/auth/test.tspackages/validators/src/index.tspnpm-workspace.yaml
💤 Files with no reviewable changes (2)
- .vscode/tasks.json
- .vscode/extensions.json
✅ Files skipped from review due to trivial changes (7)
- packages/auth/src/client.ts
- packages/ui/src/base-ui/use-render.ts
- apps/portal/biome.json
- packages/ui/src/base-ui/merge-props.ts
- apps/portal/tsconfig.json
- packages/db/src/migrations/meta/0001_snapshot.json
- apps/portal/src/components/theme-provider.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/portal/src/components/providers.tsx
- packages/api/package.json
- apps/portal/src/lib/orpc/query/hydration.tsx
| ``` | ||
| apps/portal/src/ | ||
| ├── app/ # Routes only — thin pages that delegate to features | ||
| ├── features/ # Domain/feature modules (auth, dashboard, cards, links, …) | ||
| ├── components/ # Shared UI: layout (header, sidebar), providers | ||
| ├── lib/ # Shared infra: orpc, auth client, env, utils | ||
| ├── hooks/ # Shared hooks (optional; feature hooks live in features) | ||
| ├── providers/ # App-level providers (optional; or keep under components) | ||
| └── types/ # Global app types only (prefer validators + feature types) | ||
| ``` |
There was a problem hiding this comment.
Add languages to the fenced examples.
Both blocks are currently unlabeled, and markdownlint is already flagging them. text is enough for the folder trees.
Also applies to: 43-60
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.cursor/plans/portal-folder-structure.plan.md around lines 12 - 21, The
fenced code blocks showing the folder trees are unlabeled and causing
markdownlint warnings; update each triple-backtick fence for the examples (e.g.,
the block containing the apps/portal/src/ tree and the other block at lines
43-60 mentioned in the comment) to use a language label such as "text" (change
``` to ```text) so the Markdown is properly labeled while preserving the
existing content.
| **Flow:** | ||
|
|
||
| - **Page** stays thin: `app/(auth)/register/page.tsx` → `<RegisterView />`. | ||
| - **View** composes layout (e.g. `AuthCard`) + form + links. Example: `RegisterView` → `<AuthCard title="Create account"><RegisterForm /><Link to="/login">Sign in</Link></AuthCard>`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "portal-folder-structure.plan.md" -type fRepository: itzTedx/ZironTap
Length of output: 107
🏁 Script executed:
# Check the content around line 75
cat -n ".cursor/plans/portal-folder-structure.plan.md" | sed -n '70,80p'Repository: itzTedx/ZironTap
Length of output: 644
🌐 Web query:
Next.js Link component href prop API documentation
💡 Result:
The official Next.js API docs for the <Link> component href prop are here:
- App Router (
app/): “Link Component” →href (required)(type: String or Object) with examples using{ pathname, query }. [1] - Pages Router (
pages/): “<Link>” →href (required)(type: String or Object) and “With URL Object” section (also notes it formats a URL object into a URL string). [2]
What href is (per docs): the path or URL to navigate to, required, and can be a string (e.g. "/dashboard") or an object (e.g. { pathname: "/about", query: { name: "test" } }). [1][2]
Example (object form):
import Link from "next/link";
<Link href={{ pathname: "/about", query: { name: "test" } }}>About</Link>[1][2]
Sources
[1] Next.js Docs (App Router) – Link Component API reference: href (required) (nextjs.org)
[2] Next.js Docs (Pages Router) – <Link> API reference: href (required) / “With URL Object” (nextjs.org)
Citations:
- 1: https://nextjs.org/docs/app/api-reference/components/link
- 2: https://nextjs.org/docs/14/pages/api-reference/components/link
Use href instead of to in the Link component example.
The code example at line 75 uses <Link to="/login">, which is React Router syntax. Next.js requires href as the prop name. This needs correction to prevent implementation errors.
Suggested fix
-- **View** composes layout (e.g. `AuthCard`) + form + links. Example: `RegisterView` → `<AuthCard title="Create account"><RegisterForm /><Link to="/login">Sign in</Link></AuthCard>`.
+- **View** composes layout (e.g. `AuthCard`) + form + links. Example: `RegisterView` → `<AuthCard title="Create account"><RegisterForm /><Link href="/login">Sign in</Link></AuthCard>`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **View** composes layout (e.g. `AuthCard`) + form + links. Example: `RegisterView` → `<AuthCard title="Create account"><RegisterForm /><Link to="/login">Sign in</Link></AuthCard>`. | |
| - **View** composes layout (e.g. `AuthCard`) + form + links. Example: `RegisterView` → `<AuthCard title="Create account"><RegisterForm /><Link href="/login">Sign in</Link></AuthCard>`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.cursor/plans/portal-folder-structure.plan.md at line 75, The example uses
React Router prop `to` on the Link component which is incorrect for Next.js;
update the example in the View/ RegisterView snippet so the Link uses the `href`
prop instead (e.g., change `<Link to="/login">Sign in</Link>` to use `href`) and
ensure the surrounding symbols referenced (AuthCard, RegisterForm, RegisterView,
Link) remain unchanged.
| "dev": "pnpm with-env next dev -p 3000", | ||
| "build": "pnpm with-env next build", | ||
| "start": "pnpm with-env next start", | ||
| "lint": "biome check .", | ||
| "format": "biome format --write .", | ||
| "typecheck": "tsc --noEmit" | ||
| "typecheck": "tsc --noEmit", | ||
| "with-env": "pnpm exec dotenv -e .env --" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me check the actual content of apps/portal/package.json
cat apps/portal/package.jsonRepository: itzTedx/ZironTap
Length of output: 1660
🏁 Script executed:
# Also check pnpm-workspace.yaml to understand the catalog setup
cat pnpm-workspace.yamlRepository: itzTedx/ZironTap
Length of output: 962
🏁 Script executed:
# Search for any environment configuration or .env files that might exist
find . -maxdepth 3 -name ".env*" -o -name "pnpm-workspace.yaml" | head -20Repository: itzTedx/ZironTap
Length of output: 81
🌐 Web query:
Next.js environment variables loading precedence order .env files
💡 Result:
Next.js resolves an environment variable by checking these locations in order (it stops at the first match): [1]
process.env.env.$(NODE_ENV).local(e.g..env.development.local).env.local(skipped whenNODE_ENV === "test").env.$(NODE_ENV)(e.g..env.development).env
So, for example in development, .env.development.local overrides .env. [1]
🏁 Script executed:
# Check if .env files exist in the repository
find . -maxdepth 3 -type f -name ".env*" 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check apps/portal directory structure for any env configuration
ls -la apps/portal/ | head -30Repository: itzTedx/ZironTap
Length of output: 844
Remove the dotenv-cli wrapper from Next.js lifecycle scripts.
Next.js's documented environment variable precedence is process.env → .env.$(NODE_ENV).local → .env.local → .env.$(NODE_ENV) → .env. The pnpm with-env next wrapper preloads only .env into process.env before Next.js starts, which prevents environment-specific and local files (.env.development.local, .env.local, etc.) from overriding base .env values, breaking the intended precedence hierarchy.
Suggested change
"scripts": {
- "dev": "pnpm with-env next dev -p 3000",
- "build": "pnpm with-env next build",
- "start": "pnpm with-env next start",
+ "dev": "next dev -p 3000",
+ "build": "next build",
+ "start": "next start",
"lint": "biome check .",
"format": "biome format --write .",
- "typecheck": "tsc --noEmit",
- "with-env": "pnpm exec dotenv -e .env --"
+ "typecheck": "tsc --noEmit"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "pnpm with-env next dev -p 3000", | |
| "build": "pnpm with-env next build", | |
| "start": "pnpm with-env next start", | |
| "lint": "biome check .", | |
| "format": "biome format --write .", | |
| "typecheck": "tsc --noEmit" | |
| "typecheck": "tsc --noEmit", | |
| "with-env": "pnpm exec dotenv -e .env --" | |
| "dev": "next dev -p 3000", | |
| "build": "next build", | |
| "start": "next start", | |
| "lint": "biome check .", | |
| "format": "biome format --write .", | |
| "typecheck": "tsc --noEmit" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/package.json` around lines 8 - 14, The package scripts currently
wrap Next.js commands with the custom "with-env" script which invokes
dotenv-cli; remove that wrapper so Next.js can load env files using its own
precedence. Update the "dev", "build", and "start" script values to call "next
dev -p 3000", "next build", and "next start" respectively (replacing "pnpm
with-env next ..."), and delete the "with-env" script entry so dotenv-cli is no
longer used.
| export const getSession = cache(async () => { | ||
| const session = await auth.api.getSession({ | ||
| headers: await headers(), | ||
| }); | ||
| return session; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read its content
cat -n apps/portal/src/features/auth/api.tsRepository: itzTedx/ZironTap
Length of output: 564
🏁 Script executed:
# Search for usages of getSession to understand scope
rg "getSession" apps/portal/src --type ts --type tsx -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
# Look for similar error handling patterns in other auth-related code
rg "auth\.api\." apps/portal/src --type ts --type tsx -B 2 -A 5 | head -100Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
# Check if Better Auth is used elsewhere with error handling
rg "try.*catch" apps/portal/src/features/auth --type ts --type tsxRepository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
# Search for getSession usage across the portal app
rg "getSession" apps/portal/src -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 2078
🏁 Script executed:
# Look for auth.api usage patterns with error handling
rg "auth\.api\." apps/ -A 3 -B 1 | head -150Repository: itzTedx/ZironTap
Length of output: 385
🏁 Script executed:
# Check for try/catch patterns in server-side auth code
rg "try \{" apps/*/src/features/auth -A 10 | head -100Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Search for error handling in Better Auth integration
rg "catch.*error" packages/auth -A 2 | head -80Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look at the Better Auth package implementation to understand error behavior
find packages/auth -type f -name "*.ts" | head -20Repository: itzTedx/ZironTap
Length of output: 142
🏁 Script executed:
# Check the Better Auth package exports and implementation
cat -n packages/auth/src/index.ts | head -50Repository: itzTedx/ZironTap
Length of output: 2113
🏁 Script executed:
# Search for auth.api usage in other apps to see if error handling is common
rg "auth\.api\." apps/ -B 2 -A 3Repository: itzTedx/ZironTap
Length of output: 423
🏁 Script executed:
# Look for how session failures are handled elsewhere
rg "session" packages/auth/src -A 2 -B 2 | head -100Repository: itzTedx/ZironTap
Length of output: 1047
🏁 Script executed:
# Check how Better Auth's getSession is typically used - look for error scenarios
rg "getSession" packages/auth/src -A 5 -B 2Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look for any example usage or tests showing error handling for auth.api
find packages/auth -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "getSession" 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 44
🏁 Script executed:
# Check if there are any error responses from auth.api elsewhere
rg "auth\.api\." apps/ -A 5 | grep -i "error\|catch\|throw" -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look at the actual calling context more carefully
cat -n apps/portal/src/app/\(auth\)/login/page.tsxRepository: itzTedx/ZironTap
Length of output: 467
🏁 Script executed:
# Look at how pages handle getSession to understand expected return type
rg "getSession\(\)" apps/portal/src/app -A 3 | head -50Repository: itzTedx/ZironTap
Length of output: 584
🏁 Script executed:
# Check if any other server-side auth helpers have error handling
rg "cache.*async" apps/portal/src/features/auth -B 2 -A 8Repository: itzTedx/ZironTap
Length of output: 883
🏁 Script executed:
# Search for any middleware or interceptors that might catch errors
rg "error\|catch\|throw" apps/portal/src/features/authRepository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check Next.js error handling - see if there's a global error handler
find apps/portal/src -name "error.tsx" -o -name "error.ts"Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look for Better Auth documentation or patterns in the codebase about session failures
rg "Better Auth\|better-auth" packages/auth/src/index.ts -A 20 | head -60Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check if isLoggedIn depends on getSession and how it handles potential null
cat -n apps/portal/src/features/auth/api.tsRepository: itzTedx/ZironTap
Length of output: 564
🏁 Script executed:
# Let me check if Better Auth might handle errors or return null by default
cat -n packages/auth/src/index.ts | head -100Repository: itzTedx/ZironTap
Length of output: 3404
🏁 Script executed:
# Look for any documentation comments or type definitions in the auth package
rg "getSession" packages/auth -B 3 -A 10Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Let me check the Better Auth library documentation inline to understand its error behavior
cat -n packages/auth/src/client.ts | head -50Repository: itzTedx/ZironTap
Length of output: 104
🏁 Script executed:
# Look for any error scenarios in Next.js pages that could happen during getSession
find apps/portal/src/app -name "layout.tsx" -o -name "error.tsx" | xargs cat -n 2>/dev/null | head -80Repository: itzTedx/ZironTap
Length of output: 1362
Add error handling to prevent session lookup failures from crashing server renders.
The getSession() helper is called from auth pages (login/register) without any error handling. If auth.api.getSession() throws, the exception will bubble up and 500 the page. Add try/catch to catch errors, log context, and return null instead.
Suggested fix
export const getSession = cache(async () => {
- const session = await auth.api.getSession({
- headers: await headers(),
- });
- return session;
+ try {
+ return await auth.api.getSession({
+ headers: await headers(),
+ });
+ } catch (error) {
+ console.error("Failed to resolve portal session", { error });
+ return null;
+ }
});This aligns with the coding guideline: "Implement server-side error handling with try/catch blocks including context" and ensures pages can gracefully handle auth failures since they already check session?.session defensively.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/features/auth/api.ts` around lines 7 - 12, Wrap the existing
cached getSession async function in a try/catch: call auth.api.getSession(...)
inside the try (still passing headers()), and if it succeeds return the session
as before; on error catch the exception, log contextual details (e.g., include
that getSession failed and any error.message/stack using your app logger or
console.error), and return null so server-rendered auth pages don’t crash; keep
the cache(...) wrapper and function name getSession unchanged.
| onSubmit: async ({ value }) => { | ||
| startTransition(async () => { | ||
| await authClient.signIn.email({ | ||
| email: value.email, | ||
| password: value.password, | ||
|
|
||
| callbackURL: "/", | ||
| fetchOptions: { | ||
| onSuccess: () => { | ||
| toastManager.add({ | ||
| title: "Logged in successfully.", | ||
| description: "You are now signed in.", | ||
| type: "success", | ||
|
|
||
| timeout: 3000, | ||
| }); | ||
| router.push("/"); | ||
| }, | ||
| onError: (ctx) => { | ||
| const { error } = ctx; | ||
| const status = error.status; | ||
| // const message = error.message; | ||
|
|
||
| // Handle specific status codes | ||
| switch (status) { | ||
| case 400: | ||
| toastManager.add({ | ||
| title: "Invalid request. Please check your email and password.", | ||
| type: "error", | ||
| }); | ||
| break; | ||
| case 401: | ||
| toastManager.add({ | ||
| title: "Invalid email or password. Please try again.", | ||
| type: "error", | ||
| }); | ||
| break; | ||
| case 403: | ||
| toastManager.add({ | ||
| title: "Access denied.", | ||
| description: "Please verify your email address", | ||
| type: "error", | ||
| }); | ||
| break; | ||
| default: | ||
| toastManager.add({ | ||
| title: "An unexpected error occurred. Please try again.", | ||
| type: "error", | ||
| }); | ||
| console.error("Login error:", error); | ||
| } | ||
| }, | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find apps/portal/src/features/auth/components/forms -type f -name "*.tsx" | head -20Repository: itzTedx/ZironTap
Length of output: 247
🏁 Script executed:
cat -n apps/portal/src/features/auth/components/forms/login-form.tsxRepository: itzTedx/ZironTap
Length of output: 5204
🏁 Script executed:
cat -n apps/portal/src/features/auth/components/forms/register-form.tsx 2>/dev/null || echo "File not found or check path"Repository: itzTedx/ZironTap
Length of output: 6089
🏁 Script executed:
# Search for similar signIn patterns in the codebase to understand error handling
rg "signIn\.email\(" -A 20 --type tsx -B 2Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
rg "signIn\.email" -A 20 -B 2 --type-list | head -5Repository: itzTedx/ZironTap
Length of output: 449
🏁 Script executed:
rg "startTransition" apps/portal/src/features/auth -A 15 -B 2Repository: itzTedx/ZironTap
Length of output: 9884
🏁 Script executed:
# Check if there are any other places where authClient methods are wrapped in try-catch
rg "try \{" apps/portal/src/features/auth -A 10 | head -50Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look for Better Auth client initialization and error handling patterns
find . -path ./node_modules -prune -o -type f -name "*auth*client*" -print 2>/dev/null | grep -v node_modules | head -10Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
cat -n apps/portal/src/lib/auth/client.ts 2>/dev/null || find . -path ./node_modules -prune -o -name "client.ts" -path "*/auth/*" -print | head -5Repository: itzTedx/ZironTap
Length of output: 897
🏁 Script executed:
# Check for error boundaries in the portal app
rg "ErrorBoundary|error\.tsx" apps/portal/src -B 2 -A 5Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look at the app structure for global error handling
find apps/portal/src/app -name "*.tsx" | head -20Repository: itzTedx/ZironTap
Length of output: 244
🏁 Script executed:
# Check for any async/await error handling patterns in similar code
rg "try \{" apps/portal/src -B 2 -A 8 | head -60Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Search for Better Auth documentation or usage patterns in the codebase
find . -path ./node_modules -prune -o -type f -name "*.md" -print | xargs grep -l "better-auth" 2>/dev/null | head -5Repository: itzTedx/ZironTap
Length of output: 171
🏁 Script executed:
# Check the package.json to see the Better Auth version
cat package.json | grep -A 5 "better-auth"Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look for any error handling tests or examples
find . -path ./node_modules -prune -o -path "*/test/*" -o -path "*/__tests__/*" -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "signIn\|fetchOptions" 2>/dev/null | head -5Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check if there are global error handlers or error boundaries
cat -n apps/portal/src/app/layout.tsxRepository: itzTedx/ZironTap
Length of output: 1163
🏁 Script executed:
# Let me check the Providers component to see if there are any error boundaries
cat -n apps/portal/src/components/providers.tsx 2>/dev/null || find apps/portal/src/components -name "providers.tsx" | head -1 | xargs cat -nRepository: itzTedx/ZironTap
Length of output: 911
🏁 Script executed:
# Check if Better Auth signIn can reject outside of fetchOptions
rg "signIn\.email|signUp\.email" -A 30 apps/portal/src | head -100Repository: itzTedx/ZironTap
Length of output: 5575
🏁 Script executed:
# Look at one more auth component to confirm the pattern
cat -n apps/portal/src/features/auth/components/passkey-button.tsx | head -60Repository: itzTedx/ZironTap
Length of output: 1989
🏁 Script executed:
# Let me check if there's any documentation about Better Auth's error handling behavior
cat -n ./.cursor/plans/04-auth.plan.md | head -100Repository: itzTedx/ZironTap
Length of output: 1579
Add try-catch to handle promise rejections outside fetchOptions.onError.
The await authClient.signIn.email() call inside startTransition lacks error handling. The fetchOptions.onError callback only runs when the API returns a response; if the promise rejects before that (network error, parsing failure, etc.), the error will be unhandled. startTransition does not provide error recovery.
This same gap exists in register-form.tsx and passkey-button.tsx.
Suggested fix
onSubmit: async ({ value }) => {
startTransition(async () => {
- await authClient.signIn.email({
+ try {
+ await authClient.signIn.email({
email: value.email,
password: value.password,
callbackURL: "/",
fetchOptions: {
onSuccess: () => {
toastManager.add({
title: "Logged in successfully.",
description: "You are now signed in.",
type: "success",
timeout: 3000,
});
router.push("/");
},
onError: (ctx) => {
const { error } = ctx;
const status = error.status;
switch (status) {
case 400:
toastManager.add({
title: "Invalid request. Please check your email and password.",
type: "error",
});
break;
case 401:
toastManager.add({
title: "Invalid email or password. Please try again.",
type: "error",
});
break;
case 403:
toastManager.add({
title: "Access denied.",
description: "Please verify your email address",
type: "error",
});
break;
default:
toastManager.add({
title: "An unexpected error occurred. Please try again.",
type: "error",
});
console.error("Login error:", error);
}
},
},
});
+ } catch (error) {
+ toastManager.add({
+ title: "An unexpected error occurred. Please try again.",
+ type: "error",
+ });
+ console.error("Login error:", error);
+ }
});
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onSubmit: async ({ value }) => { | |
| startTransition(async () => { | |
| await authClient.signIn.email({ | |
| email: value.email, | |
| password: value.password, | |
| callbackURL: "/", | |
| fetchOptions: { | |
| onSuccess: () => { | |
| toastManager.add({ | |
| title: "Logged in successfully.", | |
| description: "You are now signed in.", | |
| type: "success", | |
| timeout: 3000, | |
| }); | |
| router.push("/"); | |
| }, | |
| onError: (ctx) => { | |
| const { error } = ctx; | |
| const status = error.status; | |
| // const message = error.message; | |
| // Handle specific status codes | |
| switch (status) { | |
| case 400: | |
| toastManager.add({ | |
| title: "Invalid request. Please check your email and password.", | |
| type: "error", | |
| }); | |
| break; | |
| case 401: | |
| toastManager.add({ | |
| title: "Invalid email or password. Please try again.", | |
| type: "error", | |
| }); | |
| break; | |
| case 403: | |
| toastManager.add({ | |
| title: "Access denied.", | |
| description: "Please verify your email address", | |
| type: "error", | |
| }); | |
| break; | |
| default: | |
| toastManager.add({ | |
| title: "An unexpected error occurred. Please try again.", | |
| type: "error", | |
| }); | |
| console.error("Login error:", error); | |
| } | |
| }, | |
| }, | |
| }); | |
| }); | |
| onSubmit: async ({ value }) => { | |
| startTransition(async () => { | |
| try { | |
| await authClient.signIn.email({ | |
| email: value.email, | |
| password: value.password, | |
| callbackURL: "/", | |
| fetchOptions: { | |
| onSuccess: () => { | |
| toastManager.add({ | |
| title: "Logged in successfully.", | |
| description: "You are now signed in.", | |
| type: "success", | |
| timeout: 3000, | |
| }); | |
| router.push("/"); | |
| }, | |
| onError: (ctx) => { | |
| const { error } = ctx; | |
| const status = error.status; | |
| // Handle specific status codes | |
| switch (status) { | |
| case 400: | |
| toastManager.add({ | |
| title: "Invalid request. Please check your email and password.", | |
| type: "error", | |
| }); | |
| break; | |
| case 401: | |
| toastManager.add({ | |
| title: "Invalid email or password. Please try again.", | |
| type: "error", | |
| }); | |
| break; | |
| case 403: | |
| toastManager.add({ | |
| title: "Access denied.", | |
| description: "Please verify your email address", | |
| type: "error", | |
| }); | |
| break; | |
| default: | |
| toastManager.add({ | |
| title: "An unexpected error occurred. Please try again.", | |
| type: "error", | |
| }); | |
| console.error("Login error:", error); | |
| } | |
| }, | |
| }, | |
| }); | |
| } catch (error) { | |
| toastManager.add({ | |
| title: "An unexpected error occurred. Please try again.", | |
| type: "error", | |
| }); | |
| console.error("Login error:", error); | |
| } | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/features/auth/components/forms/login-form.tsx` around lines
32 - 86, The onSubmit handler calls authClient.signIn.email inside
startTransition without a surrounding try/catch, so promise rejections that
never hit fetchOptions.onError (e.g., network or parsing failures) are
unhandled; wrap the await authClient.signIn.email(...) call in a try/catch
inside the startTransition callback (and similarly update the onSubmit in
register-form.tsx and the passkey flow in passkey-button.tsx) and in the catch
block call toastManager.add with a generic error message and log the error (or
call router fallback) so all rejections are handled even when
fetchOptions.onError is not invoked.
| @@ -1,3 +1,5 @@ | |||
| import { cache } from "react"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how getSession is used across the codebase to understand invocation contexts
# Find all usages of getSession to verify contexts
echo "=== getSession usages ==="
rg -n -A 3 'getSession' --type ts -g '!node_modules'
# Check for any oRPC route handler definitions to understand invocation context
echo "=== oRPC route handlers ==="
rg -n 'createORPCHandler|orpcHandler' --type ts -g '!node_modules'Repository: itzTedx/ZironTap
Length of output: 3397
🏁 Script executed:
#!/bin/bash
# Check where requireAuth middleware is imported and used
echo "=== requireAuth imports/usages ==="
rg -n 'requireAuth|from.*require-auth' --type ts --type tsx
# Check how oRPC router middleware is typically implemented
echo "=== oRPC router setup ==="
rg -n 'router|procedure' packages/api/src/router.ts -A 2 -B 2 | head -50
# Look at the broader context of require-auth.ts
echo "=== Full require-auth.ts file ==="
cat -n packages/api/src/middleware/require-auth.tsRepository: itzTedx/ZironTap
Length of output: 1083
🏁 Script executed:
#!/bin/bash
# Check what 'base' is
echo "=== base import/definition ==="
cat -n packages/api/src/base.ts | head -30
# Check packages/api structure to understand oRPC setup
echo "=== packages/api directory structure ==="
find packages/api/src -type f -name "*.ts" | head -20
# Search for where oRPC is instantiated/configured
echo "=== oRPC configuration ==="
rg -l 'oRPC|procedure|router' packages/api/src --type ts | head -10
# Check if there's any reference to require-auth or middleware patterns
echo "=== Middleware patterns in codebase ==="
rg -n 'middleware' packages/api/src --type ts -B 2 -A 2Repository: itzTedx/ZironTap
Length of output: 2801
🏁 Script executed:
#!/bin/bash
# Check other middleware files to see the pattern
echo "=== auth.ts middleware ==="
cat -n packages/api/src/middleware/auth.ts | head -30
echo "=== db.ts middleware ==="
cat -n packages/api/src/middleware/db.ts | head -20
echo "=== context.ts usage ==="
cat -n packages/api/src/context.ts | head -25Repository: itzTedx/ZironTap
Length of output: 1768
🏁 Script executed:
#!/bin/bash
# Verify what headers() from next/headers returns in API context
echo "=== Next.js documentation check: headers() behavior ==="
# Check if there are any comments or docs about headers() usage
rg -n 'headers\(\)' packages/api/src --type ts -B 2 -A 2
# Check git history or comments about this pattern
echo "=== Check if this is intentional pattern ==="
rg -n 'require-auth|getSession.*cache' --type ts -B 3 -A 3
# Verify: context.ts and require-auth.ts are NOT both being used
echo "=== Check if both createContext and requireAuth are used ==="
rg -n 'createContext|requireAuth' packages/ --type ts | grep -E '(import|use|call)'Repository: itzTedx/ZironTap
Length of output: 2121
🏁 Script executed:
#!/bin/bash
# Final check: verify if oRPC middleware has access to context.request
echo "=== oRPC middleware context type ==="
cat -n packages/api/src/base.ts | grep -A 5 'ORPCContext'
# Check if protectedProcedure is actually used/exported
echo "=== Verify protectedProcedure export in index.ts ==="
cat -n packages/api/src/index.tsRepository: itzTedx/ZironTap
Length of output: 753
Align middleware pattern with existing auth.ts — avoid using RSC-specific APIs in oRPC context.
The require-auth middleware is using headers() from "next/headers" and React's cache(), which are React Server Component APIs. In oRPC middleware context (API routes), this is incorrect. The existing auth.ts middleware demonstrates the proper pattern: receive headers from the oRPC context.request parameter directly.
Update require-auth.ts to match the auth.ts pattern by receiving context as a parameter and accessing context.request.headers instead of calling the RSC headers() function. This ensures the middleware functions correctly in API route context and maintains consistency with other oRPC middleware in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api/src/middleware/require-auth.ts` at line 1, The require-auth
middleware currently uses RSC APIs (headers() and cache()) — update it to match
the existing auth.ts oRPC pattern: remove imports of "react" and "next/headers",
change the middleware function to accept the oRPC context parameter (e.g.,
function requireAuth(context) or the same signature used in auth.ts), read
headers from context.request.headers, and preserve the same middleware behavior
and exported symbol(s) (e.g., requireAuth or requireAuthMiddleware) so it works
in API route/oRPC context and stays consistent with auth.ts.
| passkey({ | ||
| rpID: new URL(env.BETTER_AUTH_URL).hostname, | ||
| rpName: "ZironTap", | ||
| origin: env.BETTER_AUTH_URL, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code at the specified lines
cat -n packages/auth/src/index.ts | sed -n '75,95p'Repository: itzTedx/ZironTap
Length of output: 676
🏁 Script executed:
# Search for BETTER_AUTH_URL usage in the codebase to understand how it's defined and used
rg "BETTER_AUTH_URL" -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 983
🏁 Script executed:
# Check if there are any Better Auth or passkey-related config/setup files
fd -e ts -e tsx | xargs grep -l "passkey\|rpID\|rpName" | head -20Repository: itzTedx/ZironTap
Length of output: 274
🏁 Script executed:
# Check the Better Auth passkey plugin imports and type definitions
rg "from.*passkey\|import.*passkey" packages/auth/src/index.ts -B 5 -A 5Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look for any Better Auth configuration documentation or comments about origin
rg "origin.*passkey|passkey.*origin" -iRepository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check if there are any tests or examples of passkey configuration in the repo
fd -e test -e spec | xargs grep -l "passkey" 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 44
🏁 Script executed:
# Examine the actual env var validation to understand what format BETTER_AUTH_URL can have
cat -n packages/env/src/server.ts | grep -A 5 -B 5 "BETTER_AUTH_URL"Repository: itzTedx/ZironTap
Length of output: 513
🏁 Script executed:
# Check the imports at the top of the auth file to understand where passkey comes from
head -30 packages/auth/src/index.tsRepository: itzTedx/ZironTap
Length of output: 1152
🏁 Script executed:
# Look for any environment variable usage patterns - check if BETTER_AUTH_URL is ever used with path stripping elsewhere
rg "BETTER_AUTH_URL|env\.BETTER_AUTH_URL" -B 3 -A 3Repository: itzTedx/ZironTap
Length of output: 1282
🏁 Script executed:
# Check if there's any documentation or comments about passkey configuration in the codebase
rg "passkey|rpID|rpName" -B 5 -A 5 | head -50Repository: itzTedx/ZironTap
Length of output: 2676
🏁 Script executed:
# Check if there's a type definition or README in Better Auth documentation about passkey origin
# First, let's see if there are any TypeScript type hints we can find
rg "type.*passkey|interface.*passkey" -A 10Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check if BETTER_AUTH_URL ever includes a path in configuration or documentation
rg "BETTER_AUTH_URL.*=" -B 2 -A 2Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Look at how trustedOrigins is used elsewhere to understand if raw URL strings are expected
rg "trustedOrigins" -B 3 -A 3Repository: itzTedx/ZironTap
Length of output: 416
🏁 Script executed:
# Check if there are any existing issues or TODOs about URL normalization
rg "TODO|FIXME" packages/auth/src/index.ts | head -20Repository: itzTedx/ZironTap
Length of output: 182
🏁 Script executed:
# Let's check if there are any examples or tests showing how passkey should be configured
fd -type f -name "*.md" | xargs grep -l "passkey\|rpID\|rpName" 2>/dev/nullRepository: itzTedx/ZironTap
Length of output: 232
🏁 Script executed:
# Check if there are any GitHub issues or documentation comments about WebAuthn origin
rg "WebAuthn|webauthn|origin.*passkey" -iRepository: itzTedx/ZironTap
Length of output: 586
🏁 Script executed:
# Let's verify what the env var format looks like in practice by checking any .env examples
fd -type f \( -name ".env*" -o -name "*.example" \) | xargs cat 2>/dev/null | grep BETTER_AUTH_URLRepository: itzTedx/ZironTap
Length of output: 230
🏁 Script executed:
# Check the Better Auth package.json or type definitions to understand origin parameter better
rg "@better-auth/passkey" --type json -A 5 -B 5Repository: itzTedx/ZironTap
Length of output: 1282
🏁 Script executed:
# Let's check the passkey-button component to see how it uses the auth configuration
cat packages/auth/src/index.ts | head -100 | tail -50Repository: itzTedx/ZironTap
Length of output: 1000
🏁 Script executed:
# Look at the full passkey configuration section to see all related settings
sed -n '77,100p' packages/auth/src/index.tsRepository: itzTedx/ZironTap
Length of output: 563
🏁 Script executed:
# Search for any validation or setup code that might normalize URLs
rg "\.origin\|\.hostname" packages/auth/src/index.ts -B 2 -A 2Repository: itzTedx/ZironTap
Length of output: 42
🏁 Script executed:
# Check if Better Auth documentation or source gives hints about origin normalization
# Look for any comments or configuration patterns in the auth setup
cat packages/auth/src/index.ts | grep -A 20 "betterAuth\|trustedOrigins"Repository: itzTedx/ZironTap
Length of output: 1799
Normalize the WebAuthn origin before passing it to passkey().
The origin parameter must be a URL origin (scheme://hostname[:port]) without any path component. While BETTER_AUTH_URL is validated as a URL, it can include paths. Extract the URL once and reuse both .hostname for rpID and .origin for origin to ensure WebAuthn compliance and prevent registration/assertion failures.
Suggested fix
+const betterAuthUrl = new URL(env.BETTER_AUTH_URL);
+
export const auth = betterAuth({
@@
passkey({
- rpID: new URL(env.BETTER_AUTH_URL).hostname,
+ rpID: betterAuthUrl.hostname,
rpName: "ZironTap",
- origin: env.BETTER_AUTH_URL,
+ origin: betterAuthUrl.origin,
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/auth/src/index.ts` around lines 82 - 86, The passkey call is passing
env.BETTER_AUTH_URL directly as origin and using new URL(...).hostname for rpID,
which can include a path and break WebAuthn; parse BETTER_AUTH_URL once into a
URL object (e.g., const parsed = new URL(env.BETTER_AUTH_URL)) and pass
parsed.hostname to rpID and parsed.origin to origin in the passkey(...) options
to ensure the origin is normalized to scheme://host[:port] and reuse the parsed
URL object.
| CREATE TABLE "cards" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "name" text NOT NULL | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for cards table schema definition to check for organizationId
# Look for cards table/schema definition
rg -n "cards" --type ts -g "packages/db/src/schema/**"
# Check if organizationId is referenced anywhere in cards-related code
ast-grep --pattern 'organizationId: $_'Repository: itzTedx/ZironTap
Length of output: 353
🏁 Script executed:
cat -n packages/db/src/schema/card.tsRepository: itzTedx/ZironTap
Length of output: 323
🏁 Script executed:
cat -n packages/db/src/schema/columns.tsRepository: itzTedx/ZironTap
Length of output: 1838
🏁 Script executed:
fd "\.ts$" packages/db/src/schema/ | head -20Repository: itzTedx/ZironTap
Length of output: 187
🏁 Script executed:
cat -n packages/db/src/schema/index.tsRepository: itzTedx/ZironTap
Length of output: 482
Add organization_id column to cards table.
Per coding guidelines, all resources (cards, links, QR, reviews) must be scoped to organizationId for proper multi-tenant isolation. The cards table in both the migration and schema definition is missing this required column.
Update the migration to add organization_id uuid NOT NULL and the schema definition in packages/db/src/schema/card.ts to include organizationId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/migrations/0001_ambitious_moira_mactaggert.sql` around lines
15 - 18, The cards table migration is missing an organization_id column required
for tenant scoping; update the CREATE TABLE "cards" statement to add
organization_id uuid NOT NULL (with appropriate FK or index if desired) and also
add organizationId: 'uuid' (or the equivalent column/type) to the Card schema
definition in card.ts so the runtime schema matches the migration and every card
is scoped to an organization.
| "@types/react-dom": "catalog:react19", | ||
| "@ziron/typescript-config": "workspace:*", | ||
| "tailwindcss": "catalog:", | ||
| "tw-animate-css": "^1.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking catalog entries in pnpm-workspace.yaml..."
fd '^pnpm-workspace\.yaml$' . --hidden --exclude .git --exclude node_modules -x sh -c '
echo "-- $1 --"
rg -n "tw-animate-css|react-day-picker" "$1" || true
' sh {}
echo
echo "Checking whether exported UI assets reference tw-animate-css..."
rg -n "tw-animate-css" packages/uiRepository: itzTedx/ZironTap
Length of output: 355
🏁 Script executed:
grep -A 5 -B 5 '"tw-animate-css"' packages/ui/package.jsonRepository: itzTedx/ZironTap
Length of output: 408
Add catalog entry and move to dependencies: this exported CSS requires runtime access to tw-animate-css.
Line 34 hardcodes tw-animate-css, breaking the workspace versioning rule. More critically, globals.css is exported ("./globals.css": "./src/styles/globals.css") and imports tw-animate-css via @import, yet the package is only in devDependencies. This breaks consumers of @ziron/ui at runtime—they cannot use the exported CSS without manually installing tw-animate-css.
Both issues must be fixed:
- Add
tw-animate-cssto the catalog inpnpm-workspace.yaml - Move
tw-animate-cssfromdevDependenciestodependencies
Required changes
In pnpm-workspace.yaml, add:
tw-animate-css: ^1.4.0In packages/ui/package.json, move tw-animate-css from devDependencies to dependencies and update to:
"tw-animate-css": "catalog:",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/package.json` at line 34, The package currently lists
"tw-animate-css" in devDependencies while exporting "./globals.css" (mapped as
"./globals.css": "./src/styles/globals.css") that `@imports` runtime CSS, and the
workspace catalog is missing the entry; to fix, add a catalog entry for
tw-animate-css with the version ^1.4.0 in pnpm-workspace.yaml, and move the
"tw-animate-css" entry from devDependencies to dependencies in
packages/ui/package.json replacing the version string with "catalog:" so
consumers of `@ziron/ui` can resolve the runtime CSS.
| name: z | ||
| .string() | ||
| .min(1, { | ||
| message: "What should we call you? Please enter your name.", | ||
| }) | ||
| .max(128, { | ||
| message: "That name is a bit long - please keep it under 128 characters.", | ||
| }), |
There was a problem hiding this comment.
Trim name before the length checks.
min(1) still accepts whitespace-only input, so " " will produce a blank-looking account name. Since this schema is shared, that leaks everywhere registration is used.
Suggested fix
name: z
.string()
+ .trim()
.min(1, {
message: "What should we call you? Please enter your name.",
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/validators/src/auth/register.ts` around lines 6 - 13, The name field
currently allows whitespace-only strings because min(1) runs before trimming;
update the schema for the name string (the variable/property named "name" in the
register validator) to trim input before validation—use a z.string() transform
to trim the value (or a refine that checks trimmed length) so the subsequent
.min(1) and .max(128) apply to the trimmed name and store the trimmed result
across callers.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/portal/src/features/auth/lib/handle-auth-error.ts (1)
5-17: Consider usinginterfacefor object shape definitions.Per coding guidelines, prefer
interfaceovertypefor defining object shapes. This is a minor style suggestion.♻️ Suggested refactor
-type AuthErrorContext = "login" | "register"; +type AuthErrorContext = "login" | "register"; // Union types remain as type -type AuthError = { +interface AuthError { status?: number; message?: string; [key: string]: unknown; -}; +} -type HandleAuthErrorParams = { +interface HandleAuthErrorParams { context: AuthErrorContext; email: string; error: AuthError; -}; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/features/auth/lib/handle-auth-error.ts` around lines 5 - 17, Replace the object-shaped type aliases with interfaces: change the type declarations AuthError and HandleAuthErrorParams into interface declarations preserving all properties (status?: number, message?: string, and the index signature {[key: string]: unknown} for AuthError; context: AuthErrorContext, email: string, error: AuthError for HandleAuthErrorParams), while leaving the AuthErrorContext union as-is; update any local references if necessary to the new interface names so code compiles.packages/logger/src/index.ts (1)
31-37: Prefer aninterfaceforLoggerContext.This is an object shape, and the repo standard asks for
interfacehere.As per coding guidelines, "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/logger/src/index.ts` around lines 31 - 37, Replace the exported type alias LoggerContext with an exported interface to match repo convention for object shapes; keep the same optional properties (requestId, userId, orgId, path, method) and their optional markers, and ensure any imports/uses still reference LoggerContext unchanged so consumers compile correctly.
🤖 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/portal/src/features/auth/lib/handle-auth-error.ts`:
- Around line 19-26: In handleAuthError, avoid logging raw user emails to
client-side console; remove the email property from the console.error call (or
replace it with a masked value derived from the email param, e.g., show only the
domain or first/last character) and ensure any full-email logging is done only
on the server side; update the function signature usage (HandleAuthErrorParams)
accordingly and adjust callers if they rely on client-side email logging.
In `@apps/portal/src/lib/auth/social-login.ts`:
- Line 7: The default callbackURL in signInWithProvider is set to "/login",
causing social logins to redirect back to the login page; change the default
parameter value of callbackURL in the signInWithProvider function to "/" (or
ensure callers pass "/" when invoking signInWithProvider) so social
authentication redirects match the email/password flows and send users to the
dashboard/home instead of the login page.
In `@packages/logger/package.json`:
- Around line 12-15: Update the pnpm catalog and the package dependency entry
for winston: add winston to the pnpm workspace catalog (pnpm-workspace.yaml)
following the same pattern used for `@types/node` and typescript, then change the
dependency in packages/logger package.json from the hardcoded semver "winston":
"^3.14.2" to reference the catalog as "winston": "catalog:" so the package uses
the workspace catalog version.
In `@packages/logger/src/index.ts`:
- Around line 14-17: Replace JSON.stringify in format.printf with util.inspect
to safely serialize meta (and preserve Error properties and avoid crashes on
circular refs): import util from 'util' and call util.inspect(meta, { depth:
null, colors: false, getters: true }) (include a leading space only when meta is
non-empty as before) inside the format.printf callback (the function passed to
format.printf). Also change the exported LoggerContext declaration from a type
to an interface named LoggerContext to match project conventions.
---
Nitpick comments:
In `@apps/portal/src/features/auth/lib/handle-auth-error.ts`:
- Around line 5-17: Replace the object-shaped type aliases with interfaces:
change the type declarations AuthError and HandleAuthErrorParams into interface
declarations preserving all properties (status?: number, message?: string, and
the index signature {[key: string]: unknown} for AuthError; context:
AuthErrorContext, email: string, error: AuthError for HandleAuthErrorParams),
while leaving the AuthErrorContext union as-is; update any local references if
necessary to the new interface names so code compiles.
In `@packages/logger/src/index.ts`:
- Around line 31-37: Replace the exported type alias LoggerContext with an
exported interface to match repo convention for object shapes; keep the same
optional properties (requestId, userId, orgId, path, method) and their optional
markers, and ensure any imports/uses still reference LoggerContext unchanged so
consumers compile correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5971ce9b-9191-4a07-a33b-7917af51c483
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/portal/package.jsonapps/portal/src/app/(auth)/login/page.tsxapps/portal/src/app/(auth)/register/page.tsxapps/portal/src/app/api/rpc/[[...rest]]/route.tsapps/portal/src/features/auth/components/auth-card.tsxapps/portal/src/features/auth/components/forms/login-form.tsxapps/portal/src/features/auth/components/forms/register-form.tsxapps/portal/src/features/auth/components/passkey-button.tsxapps/portal/src/features/auth/hooks/use-keep-me-logged-in.tsapps/portal/src/features/auth/lib/api.tsapps/portal/src/features/auth/lib/handle-auth-error.tsapps/portal/src/features/auth/views/login-view.tsxapps/portal/src/features/auth/views/register-view.tsxapps/portal/src/lib/auth/social-login.tspackages/auth/src/index.tspackages/env/src/server.tspackages/logger/package.jsonpackages/logger/src/index.tspackages/logger/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/portal/src/features/auth/views/register-view.tsx
- apps/portal/src/features/auth/components/passkey-button.tsx
- apps/portal/src/features/auth/components/forms/register-form.tsx
- apps/portal/src/features/auth/views/login-view.tsx
- apps/portal/package.json
- apps/portal/src/app/(auth)/login/page.tsx
- apps/portal/src/features/auth/components/auth-card.tsx
- apps/portal/src/app/api/rpc/[[...rest]]/route.ts
| export function handleAuthError({ context, email, error }: HandleAuthErrorParams) { | ||
| const status = error.status; | ||
|
|
||
| console.error(`${context} error`, { | ||
| email, | ||
| status, | ||
| error, | ||
| }); |
There was a problem hiding this comment.
Consider removing email from client-side error logs.
Logging the user's email to console.error on the client side exposes PII in browser developer tools and any client-side log aggregation. Consider removing or masking the email, or only logging it server-side where access is more controlled.
export function handleAuthError({ context, email, error }: HandleAuthErrorParams) {
const status = error.status;
console.error(`${context} error`, {
- email,
status,
error,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/features/auth/lib/handle-auth-error.ts` around lines 19 - 26,
In handleAuthError, avoid logging raw user emails to client-side console; remove
the email property from the console.error call (or replace it with a masked
value derived from the email param, e.g., show only the domain or first/last
character) and ensure any full-email logging is done only on the server side;
update the function signature usage (HandleAuthErrorParams) accordingly and
adjust callers if they rely on client-side email logging.
|
|
||
| type SocialProvider = "google" | "apple"; | ||
|
|
||
| export async function signInWithProvider(provider: SocialProvider, callbackURL = "/login") { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how signInWithProvider is called to see if callbackURL is always overridden
rg "signInWithProvider" apps/portal/src -A 2 -B 1
# Check for any OAuth redirect configuration
rg "callbackURL|redirectUrl" apps/portal/src --type ts -A 2 -B 2Repository: itzTedx/ZironTap
Length of output: 3003
Update the default callbackURL to match other authentication flows.
The default callbackURL = "/login" is inconsistent with email/password authentication (callbackURL: "/" in both login and register forms). After successful social authentication, users should be redirected to the dashboard or home page, not back to the login page. Either pass callbackURL: "/" when calling signInWithProvider, or change the function default to "/".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/lib/auth/social-login.ts` at line 7, The default callbackURL
in signInWithProvider is set to "/login", causing social logins to redirect back
to the login page; change the default parameter value of callbackURL in the
signInWithProvider function to "/" (or ensure callers pass "/" when invoking
signInWithProvider) so social authentication redirects match the email/password
flows and send users to the dashboard/home instead of the login page.
| "dependencies": { | ||
| "@ziron/env": "workspace:*", | ||
| "winston": "^3.14.2" | ||
| }, |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# Check if pnpm-workspace.yaml exists and contains catalog definitions
find . -name "pnpm-workspace.yaml" -o -name "pnpm-workspace.yml" | head -5Repository: itzTedx/ZironTap
Repository: itzTedx/ZironTap
Exit code: 0
stdout:
./pnpm-workspace.yaml
Script executed:
# Check the full packages/logger/package.json file
cat -n packages/logger/package.jsonRepository: itzTedx/ZironTap
Repository: itzTedx/ZironTap
Exit code: 0
stdout:
1 {
2 "name": "@ziron/logger",
3 "version": "0.0.0",
4 "type": "module",
5 "private": true,
6 "scripts": {
7 "build": "tsc",
8 "lint": "biome check .",
9 "format": "biome format --write .",
10 "typecheck": "tsc --noEmit"
11 },
12 "dependencies": {
13 "@ziron/env": "workspace:*",
14 "winston": "^3.14.2"
15 },
16 "devDependencies": {
17 "@ziron/typescript-config": "workspace:*",
18 "@types/node": "catalog:",
19 "typescript": "catalog:"
20 },
21 "exports": {
22 ".": {
23 "default": "./src/index.ts"
24 }
25 }
26 }
Script executed:
# Search for winston in the workspace catalog if it exists
rg -A 20 "catalog:" pnpm-workspace.yaml 2>/dev/null || echo "File not found or no catalog section"Repository: itzTedx/ZironTap
Repository: itzTedx/ZironTap
Exit code: 0
stdout:
catalog:
'@better-auth/api-key': ^1.5.4
'@better-auth/drizzle-adapter': ^1.5.4
'@better-auth/passkey': ^1.5.4
'@biomejs/biome': ^2.4.6
'@orpc/client': ^1.13.6
'@orpc/openapi': ^1.13.6
'@orpc/server': ^1.13.6
'@orpc/tanstack-query': ^1.13.6
'@orpc/zod': ^1.13.6
'@tailwindcss/postcss': ^4.2.1
'@tanstack/react-form': ^1.28.4
'@tanstack/react-query': ^5.90.21
'@tanstack/react-query-devtools': ^5.91.3
'@turbo/gen': ^2.8.15
'@types/node': ^25.4.0
babel-plugin-react-compiler: ^1.0.0
better-auth: ^1.5.4
dotenv: ^17.2.2
dotenv-cli: ^7.4.2
next: 16.1.6
Use the pnpm catalog for winston.
Line 14 hardcodes a semver range (^3.14.2), which bypasses the workspace catalog and makes version drift more likely across packages. Add winston to the pnpm catalog in pnpm-workspace.yaml, then reference it with "winston": "catalog:" in this file, matching the pattern already used for @types/node and typescript.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/logger/package.json` around lines 12 - 15, Update the pnpm catalog
and the package dependency entry for winston: add winston to the pnpm workspace
catalog (pnpm-workspace.yaml) following the same pattern used for `@types/node`
and typescript, then change the dependency in packages/logger package.json from
the hardcoded semver "winston": "^3.14.2" to reference the catalog as "winston":
"catalog:" so the package uses the workspace catalog version.
| format.printf(({ level, message, timestamp, ...meta }) => { | ||
| const metaJson = Object.keys(meta).length > 0 ? ` ${JSON.stringify(meta)}` : ""; | ||
| return `${timestamp as string} ${level}: ${String(message)}${metaJson}`; | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/logger/src/index.tsRepository: itzTedx/ZironTap
Length of output: 1424
🏁 Script executed:
# Check for logger usage patterns in the codebase
rg "withContext|logger\.(debug|info|warn|error)" --type ts --type tsx -A 2 | head -100Repository: itzTedx/ZironTap
Length of output: 87
🏁 Script executed:
# Check for logger usage patterns in the codebase
rg "withContext|logger\.(debug|info|warn|error)" -A 2 | head -150Repository: itzTedx/ZironTap
Length of output: 737
🏁 Script executed:
# Check what types of metadata are passed to logger calls
rg "logger\.(debug|info|warn|error)\(" -B 2 -A 5 | head -200Repository: itzTedx/ZironTap
Length of output: 1246
🏁 Script executed:
# Check if Error objects are logged and their structure
rg "err:" -B 3 -A 3 | head -100Repository: itzTedx/ZironTap
Length of output: 1689
🏁 Script executed:
# Verify Winston's handling of Error objects in metadata
node <<'EOF'
const error = new Error("Test error");
const metadata = { err: error };
try {
const result = JSON.stringify(metadata);
console.log("Result:", result);
} catch (e) {
console.log("Error caught:", e.message);
}
// Check what JSON.stringify actually does with Error
console.log("\nDirect Error stringify:", JSON.stringify(new Error("test")));
console.log("\nError in object:", JSON.stringify({ error: new Error("test") }));
// Check for circular references in error cause chains
const err1 = new Error("First error");
const err2 = new Error("Second error", { cause: err1 });
try {
console.log("\nError with cause:", JSON.stringify({ err: err2 }));
} catch (e) {
console.log("Circular reference error:", e.message);
}
EOFRepository: itzTedx/ZironTap
Length of output: 167
🏁 Script executed:
# Test more specific circular reference scenarios that might occur in practice
node <<'EOF'
const winston = require('winston');
// Test 1: Error with circular reference in metadata
const error = new Error("Test error");
const metadata = { err: error, ref: null };
metadata.ref = metadata; // Create circular reference
try {
const result = JSON.stringify(metadata);
console.log("Test 1 - Circular ref in metadata: Success (shouldn't happen)");
} catch (e) {
console.log("Test 1 - Circular ref in metadata: ERROR -", e.message);
}
// Test 2: Request-like object with circular reference
const req = {
url: "/api/test",
method: "POST",
headers: { "content-type": "application/json" },
};
req.headers.req = req; // Circular
try {
const result = JSON.stringify(req);
console.log("Test 2 - Request circular ref: Success (shouldn't happen)");
} catch (e) {
console.log("Test 2 - Request circular ref: ERROR -", e.message);
}
// Test 3: What Winston format.errors() does (it's included in the format chain)
console.log("\nTest 3 - Winston error handling:");
const logger = winston.createLogger({
format: winston.format.combine(
winston.format.errors({ stack: true }),
winston.format.json()
),
transports: [new winston.transports.Console()],
});
// This should handle the error properly
const errorWithCause = new Error("Main error", { cause: new Error("Cause error") });
console.log("Logging error with cause via Winston...");
// logger.error("Test", { err: errorWithCause });
EOFRepository: itzTedx/ZironTap
Length of output: 897
🏁 Script executed:
# Test circular reference scenarios directly
node <<'EOF'
// Test 1: Direct circular reference
console.log("Test 1: Circular reference in metadata");
const metadata = { level: "error", message: "test", timestamp: "2026-03-01" };
metadata.self = metadata; // Create circular reference
try {
const result = JSON.stringify(metadata);
console.log("Result: No error thrown");
} catch (e) {
console.log("ERROR:", e.message);
}
// Test 2: Request-like object (common in middleware logging)
console.log("\nTest 2: Request object with circular refs (common in Express/Node)");
const mockReq = {
url: "/api/test",
method: "POST",
headers: { "content-type": "application/json" },
};
// Express requests have circular references
mockReq.app = { request: mockReq }; // Simulating Express app reference
try {
const result = JSON.stringify(mockReq);
console.log("Result: No error thrown");
} catch (e) {
console.log("ERROR:", e.message);
}
// Test 3: Winston logger object
console.log("\nTest 3: Logger object with circular refs");
const loggerLike = {
name: "test",
level: "debug"
};
loggerLike.logger = loggerLike; // Circular
try {
const result = JSON.stringify(loggerLike);
console.log("Result: No error thrown");
} catch (e) {
console.log("ERROR:", e.message);
}
// Test 4: Error objects are empty when stringified but don't cause circular errors
console.log("\nTest 4: Error object handling");
const errorObj = new Error("Test error");
errorObj.context = { nested: { level: "deep" } };
try {
const result = JSON.stringify({ err: errorObj });
console.log("Result:", result);
console.log("Note: Error properties are lost!");
} catch (e) {
console.log("ERROR:", e.message);
}
// Test 5: util.inspect handles circular refs gracefully
console.log("\nTest 5: util.inspect with circular refs");
const { inspect } = require('util');
const circularObj = { a: 1 };
circularObj.self = circularObj;
console.log("util.inspect output:", inspect(circularObj, { depth: null, colors: false }));
EOFRepository: itzTedx/ZironTap
Length of output: 915
🏁 Script executed:
# Check coding guidelines requirement for type vs interface
grep -r "Prefer.*interface.*over.*type" --include="*.md" | head -5Repository: itzTedx/ZironTap
Length of output: 221
🏁 Script executed:
# Verify that util.inspect is available and what it would output
node <<'EOF'
const { inspect } = require('util');
// Test with error
const error = new Error("Test error");
console.log("Error with inspect:", inspect({ err: error }, { depth: null, colors: false, compact: true }));
// Test with circular ref
const circularMetadata = { level: "error", data: {} };
circularMetadata.data.self = circularMetadata;
console.log("\nCircular ref with inspect:", inspect(circularMetadata, { depth: null, colors: false, compact: true }));
// Test with request-like object
const mockMeta = {
requestId: "123",
userId: "user-1",
nested: { deep: { value: "test" } }
};
console.log("\nNormal metadata with inspect:", inspect(mockMeta, { depth: null, colors: false, compact: true }));
EOFRepository: itzTedx/ZironTap
Length of output: 821
Use util.inspect() instead of JSON.stringify() for log metadata to safely handle circular references.
Line 15 will throw when metadata contains circular structures (common in request objects and error chains), breaking logs exactly when debugging is critical. Additionally, JSON.stringify silently loses Error properties, rendering error objects as empty {}.
Fix
+import { inspect } from "node:util";
import { createLogger, format, transports } from "winston";
@@
format.printf(({ level, message, timestamp, ...meta }) => {
- const metaJson = Object.keys(meta).length > 0 ? ` ${JSON.stringify(meta)}` : "";
- return `${timestamp as string} ${level}: ${String(message)}${metaJson}`;
+ const metaText =
+ Object.keys(meta).length > 0
+ ? ` ${inspect(meta, { depth: null, colors: false, compact: true })}`
+ : "";
+ return `${timestamp as string} ${level}: ${String(message)}${metaText}`;
})Also, change LoggerContext (line 31) from type to interface per project conventions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| format.printf(({ level, message, timestamp, ...meta }) => { | |
| const metaJson = Object.keys(meta).length > 0 ? ` ${JSON.stringify(meta)}` : ""; | |
| return `${timestamp as string} ${level}: ${String(message)}${metaJson}`; | |
| }) | |
| format.printf(({ level, message, timestamp, ...meta }) => { | |
| const metaText = | |
| Object.keys(meta).length > 0 | |
| ? ` ${inspect(meta, { depth: null, colors: false, compact: true })}` | |
| : ""; | |
| return `${timestamp as string} ${level}: ${String(message)}${metaText}`; | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/logger/src/index.ts` around lines 14 - 17, Replace JSON.stringify in
format.printf with util.inspect to safely serialize meta (and preserve Error
properties and avoid crashes on circular refs): import util from 'util' and call
util.inspect(meta, { depth: null, colors: false, getters: true }) (include a
leading space only when meta is non-empty as before) inside the format.printf
callback (the function passed to format.printf). Also change the exported
LoggerContext declaration from a type to an interface named LoggerContext to
match project conventions.
…lity; add nodemailer integration
…s and improved parameter handling
2 participants
Summary by CodeRabbit
Release Notes
New Features
Infrastructure