Backend code #62
Backend code #62
Conversation
|
Caution Review failedFailed to post review comments WalkthroughThis PR introduces a comprehensive application overhaul including a unified dual-mode authentication system supporting both NextAuth sessions and JWT Bearer tokens, a complete UI component library with 60+ styled primitives, extensive API route implementations for workspace/team/user/billing management, redesigned authentication and dashboard pages, and comprehensive testing documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Auth as Auth System
participant NextAuth
participant JWT
participant DB
Client->>Auth: getAuthUser(request)
Auth->>NextAuth: getServerSession()
alt Session Found
NextAuth-->>Auth: session { user }
Auth->>DB: fetch user by id
DB-->>Auth: user data
Auth-->>Client: AuthUser
else No Session
Auth->>JWT: extract Bearer token
alt Bearer Token Valid
JWT->>DB: decode & validate token
DB-->>JWT: user id
JWT-->>Auth: verified user id
Auth->>DB: fetch user by id
DB-->>Auth: user data
Auth-->>Client: AuthUser
else No Token or Invalid
Auth-->>Client: 401 Unauthorized
end
end
sequenceDiagram
participant UI as Auth UI
participant API as API Route
participant Auth as Auth System
participant DB as Database
UI->>API: POST /api/auth/me (with session)
API->>Auth: requireAuth(request)
Auth-->>API: AuthUser
API->>DB: SELECT user WHERE id = ?
alt User Found
DB-->>API: user record
API-->>UI: { success: true, data: user }
else User Not Found
API-->>UI: { success: false, error: USER_NOT_FOUND } (404)
end
Note over UI,DB: Alternatively with JWT Bearer
UI->>API: GET /api/teams (Authorization: Bearer token)
API->>Auth: requireAuth(request)
Auth->>JWT: decode(token)
JWT-->>Auth: user id
Auth->>DB: SELECT user WHERE id = ?
DB-->>Auth: user record
Auth-->>API: AuthUser
API->>DB: SELECT teams WHERE user_id = ?
DB-->>API: teams list
API-->>UI: { teams, pagination }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 72
🧹 Nitpick comments (62)
apps/web/app/components/theme-provider.tsx (1)
1-10: LGTM! Clean wrapper implementation.This follows the standard pattern for wrapping third-party providers in Next.js applications. The
'use client'directive is correctly placed, type safety is maintained throughComponentProps, and the props forwarding is idiomatic.Optional enhancement for debugging:
Consider adding a
displayNamefor better identification in React DevTools:export function ThemeProvider({ children, ...props }: Props) { return <NextThemesProvider {...props}>{children}</NextThemesProvider> } + +ThemeProvider.displayName = 'ThemeProvider'apps/web/app/components/ui/alert.tsx (1)
6-20: Consider extracting long utility string for readability.Line 7 contains a very long utility class string (150+ characters) that could be split across multiple lines for better maintainability.
Consider this refactor:
const alertVariants = cva( - 'relative w-full rounded-lg border px-4 py-3 text-sm grid has-[>svg]:grid-cols-[calc(var(--spacing)*4)_1fr] grid-cols-[0_1fr] has-[>svg]:gap-x-3 gap-y-0.5 items-start [&>svg]:size-4 [&>svg]:translate-y-0.5 [&>svg]:text-current', + [ + 'relative w-full rounded-lg border px-4 py-3 text-sm grid', + 'has-[>svg]:grid-cols-[calc(var(--spacing)*4)_1fr] grid-cols-[0_1fr]', + 'has-[>svg]:gap-x-3 gap-y-0.5 items-start', + '[&>svg]:size-4 [&>svg]:translate-y-0.5 [&>svg]:text-current', + ].join(' '), { variants: {apps/web/app/components/ui/avatar.tsx (1)
8-22: Consider adding size variants for greater flexibility.The Avatar is currently fixed at
size-8(2rem). If the design system requires multiple avatar sizes, consider adding a variant prop.Example implementation with size variants:
import { cva, type VariantProps } from 'class-variance-authority' const avatarVariants = cva( 'relative flex shrink-0 overflow-hidden rounded-full', { variants: { size: { sm: 'size-6', md: 'size-8', lg: 'size-10', xl: 'size-12', }, }, defaultVariants: { size: 'md', }, }, ) function Avatar({ className, size, ...props }: React.ComponentProps<typeof AvatarPrimitive.Root> & VariantProps<typeof avatarVariants>) { return ( <AvatarPrimitive.Root data-slot="avatar" className={cn(avatarVariants({ size }), className)} {...props} /> ) }apps/web/app/components/ui/toggle.tsx (1)
31-47: LGTM!The Toggle component implementation is clean and correctly forwards props to the Radix UI primitive while applying variant styles. The TypeScript typing is proper, and the component follows the established pattern in the UI library.
Optional enhancements to consider:
If ref forwarding is needed, you could wrap the component with
React.forwardRef:-function Toggle({ +const Toggle = React.forwardRef< + React.ElementRef<typeof TogglePrimitive.Root>, + React.ComponentPropsWithoutRef<typeof TogglePrimitive.Root> & + VariantProps<typeof toggleVariants> +>(({ className, variant, size, ...props -}: React.ComponentProps<typeof TogglePrimitive.Root> & - VariantProps<typeof toggleVariants>) { +}, ref) => { return ( <TogglePrimitive.Root + ref={ref} data-slot="toggle" className={cn(toggleVariants({ variant, size, className }))} {...props} /> ) -} +}) +Toggle.displayName = TogglePrimitive.Root.displayNameAdditionally, JSDoc comments could improve discoverability:
/** * A two-state button that can be toggled on or off. * * @example * <Toggle variant="outline" size="sm"> * <Icon /> * </Toggle> */apps/web/app/components/ui/aspect-ratio.tsx (1)
5-9: Verify:data-slotattribute can be overridden by props.The
data-slot="aspect-ratio"attribute is placed before{...props}, which means anydata-slotpassed via props will override it. If the UI library requires consistent, non-overridable slot identifiers (as suggested in the PR summary), consider moving the spread before the attribute.Apply this diff if
data-slotshould be fixed:- return <AspectRatioPrimitive.Root data-slot="aspect-ratio" {...props} /> + return <AspectRatioPrimitive.Root {...props} data-slot="aspect-ratio" />Minor note: The parameter destructuring
{ ...props }is valid but unconventional;props: React.ComponentProps<typeof AspectRatioPrimitive.Root>would be more idiomatic.apps/web/app/components/ui/resizable.tsx (1)
25-29: Consider adding className support for API consistency.Unlike
ResizablePanelGroupandResizableHandle, this component doesn't accept aclassNameprop. While this may be intentional, supportingclassNamewould provide a more consistent API across all three components.Apply this diff if you'd like to add className support:
function ResizablePanel({ + className, ...props }: React.ComponentProps<typeof ResizablePrimitive.Panel>) { - return <ResizablePrimitive.Panel data-slot="resizable-panel" {...props} /> + return ( + <ResizablePrimitive.Panel + data-slot="resizable-panel" + className={className} + {...props} + /> + ) }apps/web/app/components/ui/switch.tsx (1)
8-11: Consider adding display name and return type annotation.For improved developer experience in React DevTools and stronger type safety, consider adding a display name and explicit return type.
Apply this diff:
-function Switch({ +function Switch({ className, ...props -}: React.ComponentProps<typeof SwitchPrimitive.Root>) { +}: React.ComponentProps<typeof SwitchPrimitive.Root>): React.ReactElement { return (And add after the function:
Switch.displayName = 'Switch'apps/web/app/components/ui/scroll-area.tsx (1)
19-24: Consider extracting the long className for better maintainability.The Viewport's className string on line 21 contains multiple utility classes that make it harder to read and maintain. Consider extracting it to a constant or using template literals with line breaks.
For example:
+const viewportClasses = cn( + 'focus-visible:ring-ring/50 size-full rounded-[inherit]', + 'transition-[color,box-shadow] outline-none', + 'focus-visible:ring-[3px] focus-visible:outline-1' +) + function ScrollArea({ className, children, ...props }: React.ComponentProps<typeof ScrollAreaPrimitive.Root>) { return ( <ScrollAreaPrimitive.Root data-slot="scroll-area" className={cn('relative', className)} {...props} > <ScrollAreaPrimitive.Viewport data-slot="scroll-area-viewport" - className="focus-visible:ring-ring/50 size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-[3px] focus-visible:outline-1" + className={viewportClasses} > {children} </ScrollAreaPrimitive.Viewport>apps/web/app/components/ui/collapsible.tsx (1)
5-31: Consider preventingdata-slotoverride.The
data-slotattributes are placed before{...props}, allowing consumers to override them. Ifdata-slotis meant to be a consistent internal identifier for styling or slotting, place it after the spread.Example for the Collapsible component:
function Collapsible({ ...props }: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { - return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} /> + return <CollapsiblePrimitive.Root {...props} data-slot="collapsible" /> }Apply the same pattern to
CollapsibleTriggerandCollapsibleContent.apps/web/app/api/templates/route.ts (2)
8-10: Consider more specific error handling.The generic catch block loses error context that could be valuable for debugging and providing better user feedback. Consider differentiating between JSON parsing errors, validation errors, and other exceptions.
} catch (err) { - return NextResponse.json({ ok: false, error: "Invalid request" }, { status: 400 }); + console.error("Template creation error:", err); + const message = err instanceof SyntaxError + ? "Invalid JSON" + : err instanceof Error + ? err.message + : "Invalid request"; + return NextResponse.json({ ok: false, error: message }, { status: 400 }); }
6-6: Address the TODO: implement database persistence.The handler currently echoes back the request body without persisting it. Based on
apps/web/prisma/seed.ts, you likely need to integrate with Prisma to save templates to the database.Would you like me to generate an implementation that persists templates using Prisma, or should I open an issue to track this task?
apps/web/app/api/account/delete/route.ts (1)
4-4: TODO: Implement real account deletion logic.The placeholder comment indicates missing implementation. Account deletion should handle user data removal, cascade deletions, and compliance requirements.
Do you want me to generate a more complete implementation that includes authentication, cascading deletions, and audit logging, or open a new issue to track this task?
apps/web/app/components/ui/table.tsx (2)
1-1: Reconsider the necessity of 'use client' directive.The file contains only presentational components without client-side hooks, state, or browser APIs. The
'use client'directive may be unnecessary here and could prevent server-side rendering optimizations unless these components are used exclusively in client contexts.
7-20: Table component structure looks good.The overflow wrapper pattern is appropriate for horizontal scrolling. Consider adding ref forwarding (
React.forwardRef) if consuming code needs direct access to the underlying table element.apps/web/app/components/ui/navigation-menu.tsx (2)
61-63: Consider splitting the long class string for readability.Line 62 contains over 450 characters of Tailwind classes in a single line. While this pattern is common in shadcn/ui components, consider splitting it into multiple lines or grouping related classes for improved maintainability.
Example refactor:
const navigationMenuTriggerStyle = cva( - 'group inline-flex h-9 w-max items-center justify-center rounded-md bg-background px-4 py-2 text-sm font-medium hover:bg-accent hover:text-accent-foreground focus:bg-accent focus:text-accent-foreground disabled:pointer-events-none disabled:opacity-50 data-[state=open]:hover:bg-accent data-[state=open]:text-accent-foreground data-[state=open]:focus:bg-accent data-[state=open]:bg-accent/50 focus-visible:ring-ring/50 outline-none transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1', + [ + 'group inline-flex h-9 w-max items-center justify-center rounded-md', + 'bg-background px-4 py-2 text-sm font-medium', + 'hover:bg-accent hover:text-accent-foreground', + 'focus:bg-accent focus:text-accent-foreground', + 'disabled:pointer-events-none disabled:opacity-50', + 'data-[state=open]:hover:bg-accent data-[state=open]:text-accent-foreground', + 'data-[state=open]:focus:bg-accent data-[state=open]:bg-accent/50', + 'focus-visible:ring-ring/50 focus-visible:ring-[3px] focus-visible:outline-1', + 'outline-none transition-[color,box-shadow]', + ].join(' '), )
93-95: Consider splitting extremely long class strings.Lines 93-94 contain over 1,300 characters combined, with line 94 alone exceeding 900 characters. This severely impacts code readability and maintainability.
apps/web/app/components/ui/toggle-group.tsx (1)
10-15: Consider aligning context defaults with actual usage.The initial context defaults (
size: 'default', variant: 'default') are never actually used because the Provider on line 36 immediately overrides them with{ variant, size }from props, which may be undefined. Consider either:
- Setting initial defaults to
undefinedfor clarity, or- Having the Provider merge with defaults:
value={{ variant: variant ?? 'default', size: size ?? 'default' }}apps/web/app/api/reporting/route.ts (1)
33-61: Synthetic data generation looks good for demo purposes.The approach of generating varied synthetic metrics and time-series data is well-suited for a demo/development endpoint. The logic correctly varies data based on the requested time range.
Optional: Consider adding error handling around the data generation and response:
try { // ... existing data generation code ... return NextResponse.json({ range, summary: { activeUsers, builds, errors, cpu, memory, network }, topProjects, timeseries, updatedAt: Date.now(), }); } catch (error) { console.error("Error generating reporting data:", error); return NextResponse.json( { error: "Failed to generate reporting data" }, { status: 500 } ); }apps/web/app/components/ui/badge.tsx (1)
28-44: Consider adding ref forwarding for better composability.The Badge component doesn't forward refs, which limits its flexibility when
asChild={false}. While Slot handles refs whenasChild={true}, consumers cannot attach refs to the rendered span element in the default case.Apply this diff to add ref forwarding:
-function Badge({ +const Badge = React.forwardRef< + HTMLSpanElement, + React.ComponentProps<'span'> & + VariantProps<typeof badgeVariants> & { asChild?: boolean } +>(function Badge({ className, variant, asChild = false, ...props -}: React.ComponentProps<'span'> & - VariantProps<typeof badgeVariants> & { asChild?: boolean }) { +}, ref) { const Comp = asChild ? Slot : 'span' return ( <Comp + ref={ref} data-slot="badge" className={cn(badgeVariants({ variant }), className)} {...props} /> ) -} +})apps/web/app/components/ui/use-mobile.tsx (1)
9-14: Prefermql.matchesoverwindow.innerWidthfor consistency.The code creates a
MediaQueryListbut then checkswindow.innerWidthdirectly. Usingmql.matchesis more idiomatic and avoids redundant window access.Apply this diff to use the media query result directly:
React.useEffect(() => { const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) } mql.addEventListener('change', onChange) - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) return () => mql.removeEventListener('change', onChange) }, [])apps/web/app/components/ui/card.tsx (1)
23-23: Clarify the[.border-b]:pb-6selector pattern.The selector
[.border-b]:pb-6applies padding when the element has theborder-bclass. This pattern may be unfamiliar to maintainers. Consider adding a comment explaining this conditional styling approach, or using a more explicit pattern.apps/web/app/components/ui/tabs.tsx (1)
37-51: Consider breaking up the long className for readability.The className on line 45 is quite lengthy (450+ characters). While the styling is comprehensive and correct, consider breaking it into logical groups using an array or template literal for better maintainability.
Example refactor:
- className={cn( - "data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", - className, - )} + className={cn( + // Layout & typography + "inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md px-2 py-1 text-sm font-medium whitespace-nowrap", + // Base styling + "border border-transparent text-foreground dark:text-muted-foreground", + // Active state + "data-[state=active]:bg-background data-[state=active]:shadow-sm", + "dark:data-[state=active]:text-foreground dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30", + // Focus state + "focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring focus-visible:ring-[3px] focus-visible:outline-1", + // Transitions & disabled + "transition-[color,box-shadow] disabled:pointer-events-none disabled:opacity-50", + // SVG children + "[&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + className, + )}apps/web/app/components/ui/accordion.tsx (1)
28-48: LGTM: Well-implemented trigger with good accessibility.The
AccordionTriggercomponent properly handles focus states, disabled states, and includes a chevron icon that rotates based on the accordion state. The implementation follows best practices for Radix UI wrappers.Optional: Consider extracting the className for readability.
The className on line 38 is quite long. For improved maintainability, you could extract it to a constant:
+const accordionTriggerClasses = + 'focus-visible:border-ring focus-visible:ring-ring/50 flex flex-1 items-start justify-between gap-4 rounded-md py-4 text-left text-sm font-medium transition-all outline-none hover:underline focus-visible:ring-[3px] disabled:pointer-events-none disabled:opacity-50 [&[data-state=open]>svg]:rotate-180' + function AccordionTrigger({ className, children, ...props }: React.ComponentProps<typeof AccordionPrimitive.Trigger>) { return ( <AccordionPrimitive.Header className="flex"> <AccordionPrimitive.Trigger data-slot="accordion-trigger" - className={cn( - 'focus-visible:border-ring focus-visible:ring-ring/50 flex flex-1 items-start justify-between gap-4 rounded-md py-4 text-left text-sm font-medium transition-all outline-none hover:underline focus-visible:ring-[3px] disabled:pointer-events-none disabled:opacity-50 [&[data-state=open]>svg]:rotate-180', - className, - )} + className={cn(accordionTriggerClasses, className)} {...props} >apps/web/app/api/ai/mcp-config/route.ts (1)
3-8: Document the in-memory store limitations and consider removing hardcoded demo data.The in-memory state will be lost on server restart and is not suitable for production use. The hardcoded recent entries should either be documented as demo data or removed.
Consider adding a comment:
+// In-memory store for demo purposes only. Replace with persistent storage (DB/Redis) for production. +// State is lost on server restart and shared across all users. let config = { url: "", apiKey: "" }; let recent: string[] = [ - "10 Oct - AWS Workspace - Auto Snapshot", - "09 Oct - GCP Workspace - Manual Backup", - "08 Oct - Azure VM - Auto Snapshot", + // Demo entries - remove in production ];apps/web/app/ai-agents/page.tsx (6)
46-64: Add abort controller and user-facing error feedback.The data fetching effect lacks cleanup (AbortController) and doesn't provide user feedback when errors occur, only logging to console.
Apply this diff:
useEffect(() => { async function fetchData() { + const abortController = new AbortController(); try { setLoadingAgents(true); const [a, c] = await Promise.all([ - fetch("/api/ai/agents").then((r) => r.json()), - fetch("/api/ai/mcp-config").then((r) => r.json()), + fetch("/api/ai/agents", { signal: abortController.signal }).then((r) => r.json()), + fetch("/api/ai/mcp-config", { signal: abortController.signal }).then((r) => r.json()), ]); setAgents(a.agents ?? []); setConfig({ url: c.url ?? "", apiKey: c.apiKey ?? "" }); setRecent(c.recent ?? []); } catch (e) { - console.error(e); + // Ignore abort errors + if (e instanceof Error && e.name !== 'AbortError') { + console.error(e); + // TODO: Show user-facing error message (toast notification) + } } finally { setLoadingAgents(false); } + return () => abortController.abort(); } if (mounted) fetchData(); }, [mounted]);
66-98: Add user-facing error feedback for action handlers.Both
toggleAgentandsaveConfigsilently fail when errors occur, providing no feedback to the user beyond console logs.Consider adding a toast notification system or error state:
// Add to state declarations const [error, setError] = useState<string | null>(null); // In toggleAgent catch (e) { console.error(e); setError("Failed to update agent. Please try again."); } // In saveConfig catch (e) { console.error(e); setError("Failed to save configuration. Please try again."); }Then display the error in the UI:
{error && ( <div className="bg-destructive/10 border border-destructive text-destructive px-4 py-2 rounded-md"> {error} </div> )}
100-103: Consider extracting StatusDot for reusability.The StatusDot component could be useful in other parts of the application displaying agent or service status.
If used elsewhere, extract to
components/ui/status-dot.tsx:export function StatusDot({ status }: { status: "connected" | "disconnected" | "warning" }) { const color = status === "connected" ? "bg-emerald-500" : status === "warning" ? "bg-amber-500" : "bg-rose-500"; return <span className={`inline-block h-2.5 w-2.5 rounded-full ${color}`} />; }
149-149: Loading placeholder approach is functional but could be improved.Creating dummy agents with sequential IDs works but could be replaced with a more explicit skeleton UI pattern.
Consider a clearer skeleton pattern:
{loadingAgents ? ( [1, 2, 3].map((n) => ( <div key={`skeleton-${n}`} className="flex items-center justify-between rounded-md border border-border bg-card px-4 py-3 animate-pulse"> <div className="flex items-center gap-3"> <div className="h-9 w-9 rounded-md bg-muted" /> <div className="h-4 w-32 bg-muted rounded" /> </div> <div className="h-8 w-20 bg-muted rounded" /> </div> )) ) : ( agents.map((agent) => ( // existing agent rendering )) )}
192-196: Consider adding password visibility toggle for the API key field.Using
type="password"prevents users from verifying their API key input, which could lead to configuration errors. A toggle button would improve UX.Add a visibility toggle:
const [showApiKey, setShowApiKey] = useState(false);<div className="relative"> <Input id="mcpKey" type={showApiKey ? "text" : "password"} placeholder="••••••••" value={config.apiKey} onChange={(e) => setConfig({ ...config, apiKey: e.target.value })} /> <button type="button" onClick={() => setShowApiKey(!showApiKey)} className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground" > {showApiKey ? <EyeOff className="h-4 w-4" /> : <Eye className="h-4 w-4" />} </button> </div>
134-136: Complete the header implementation or remove placeholder elements.The notification bell (🔔) and user avatar ("R") appear to be placeholders without functionality.
Are these placeholder elements intended to be functional? If so, they should:
- Link to actual notification and profile pages
- Display real user data (avatar, unread count)
- Have proper accessibility attributes
If they're decorative for now, consider adding a TODO comment or removing them to avoid confusion.
apps/web/app/components/ui/drawer.tsx (1)
8-122: Consider adding displayName to all components for better debugging.Adding
displayNameto each component function improves the developer experience in React DevTools by providing meaningful names instead of anonymous function labels.For example:
function Drawer({ ...props }: React.ComponentProps<typeof DrawerPrimitive.Root>) { return <DrawerPrimitive.Root data-slot="drawer" {...props} /> } +Drawer.displayName = 'Drawer' function DrawerTrigger({ ...props }: React.ComponentProps<typeof DrawerPrimitive.Trigger>) { return <DrawerPrimitive.Trigger data-slot="drawer-trigger" {...props} /> } +DrawerTrigger.displayName = 'DrawerTrigger'Repeat this pattern for all exported components.
apps/web/app/components/ui/hover-card.tsx (1)
29-29: Portal doesn't render a DOM node.The
data-slotattribute onHoverCardPrimitive.Portalwon't be applied anywhere since Radix's Portal component doesn't render its own DOM element—it only portals children to a different location in the DOM tree.If the data-slot pattern is purely for consistency across wrapper components, this is harmless. Otherwise, consider removing it:
- <HoverCardPrimitive.Portal data-slot="hover-card-portal"> + <HoverCardPrimitive.Portal>apps/web/app/components/ui/tooltip.tsx (2)
48-50: Consider extracting the className for maintainability.The className string is quite long (370+ characters), making it harder to read and maintain. Consider extracting it to a constant or breaking it into logical groups.
Example refactor:
+const tooltipContentClasses = [ + 'bg-primary text-primary-foreground', + 'animate-in fade-in-0 zoom-in-95', + 'data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95', + 'data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2', + 'data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2', + 'z-50 w-fit origin-(--radix-tooltip-content-transform-origin)', + 'rounded-md px-3 py-1.5 text-xs text-balance', +].join(' ') + function TooltipContent({ className, sideOffset = 0, children, ...props }: React.ComponentProps<typeof TooltipPrimitive.Content>) { return ( <TooltipPrimitive.Portal> <TooltipPrimitive.Content data-slot="tooltip-content" sideOffset={sideOffset} - className={cn( - 'bg-primary text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 w-fit origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-xs text-balance', - className, - )} + className={cn(tooltipContentClasses, className)} {...props} >
55-55: Potentially redundant styling on Arrow element.The Arrow element includes both
bg-primaryandfill-primary. Since Radix's Arrow is an SVG element,fill-primaryis likely the effective class, whilebg-primary(background-color) may have no effect on SVG fill color.Consider simplifying:
- <TooltipPrimitive.Arrow className="bg-primary fill-primary z-50 size-2.5 translate-y-[calc(-50%_-_2px)] rotate-45 rounded-[2px]" /> + <TooltipPrimitive.Arrow className="fill-primary z-50 size-2.5 translate-y-[calc(-50%_-_2px)] rotate-45 rounded-[2px]" />apps/web/app/api/workspaces/[id]/terminal/route.ts (1)
16-26: Keep workspace state timestamps in syncWe mutate the terminal buffer but never refresh
ws.lastUpdate, so other routes consuming that field will keep seeing stale timestamps. Consider updating it when you append lines.for (let i = 0; i < count; i++) { const idx = Math.floor(Math.random() * sampleLines.length); const line = sampleLines[idx] ?? ""; ws.terminal.push(line); } // keep last 120 lines if (ws.terminal.length > 120) ws.terminal = ws.terminal.slice(-120); + ws.lastUpdate = Date.now(); - return NextResponse.json({ lines: ws.terminal, updatedAt: Date.now() }); + const updatedAt = Date.now(); + return NextResponse.json({ lines: ws.terminal, updatedAt });apps/web/WORKSPACE_CREATION_FIX.md (1)
7-9: Polish fenced blocks and URLs for lint complianceMarkdownlint is flagging the unlabeled fenced blocks and the bare URL. Adding an explicit language (e.g.
```text) to the log snippets and wrapping the localhost link in<...>(or converting it to Markdown link text) will clear the warnings and keep the docs lint-clean.Also applies to: 190-196
apps/web/app/api/billing/invoice/route.ts (1)
6-6: Improve date formatting for filename.The ISO date slice creates a filename like
invoice-2025-10.txt. Consider a more readable format.- "Content-Disposition": `attachment; filename=invoice-${new Date().toISOString().slice(0, 7)}.txt`, + "Content-Disposition": `attachment; filename=invoice-${new Date().toLocaleDateString('en-US', { year: 'numeric', month: '2-digit' }).replace('/', '-')}.txt`,apps/web/app/api/auth/verify-email/route.ts (1)
27-31: Consider using findUnique with compound key.Using
findFirstwith a single field when a compound unique key exists is less efficient and less semantically clear.- const verificationToken = await prisma.verificationToken.findFirst({ - where: { - token, - }, - }); + const verificationToken = await prisma.verificationToken.findUnique({ + where: { + token, + }, + });Note: This assumes
tokenalone is unique in your schema. If not, keepfindFirst.apps/web/app/api/workspaces/[id]/metrics/route.ts (1)
8-12: Consider extracting metric jitter helper.The pattern of
Math.max(min, Math.min(max, Math.round(jitter(...))))is repeated for each metric. A helper function would reduce duplication.function clampedJitter(value: number, pct: number, min: number, max: number): number { return Math.max(min, Math.min(max, Math.round(jitter(value, pct)))); } // Usage: ws.metrics.cpu = clampedJitter(ws.metrics.cpu, 0.2, 1, 100); ws.metrics.memory.usedGb = clampedJitter(ws.metrics.memory.usedGb, 0.15, 1, ws.metrics.memory.totalGb); ws.metrics.disk.usedGb = clampedJitter(ws.metrics.disk.usedGb, 0.1, 5, ws.metrics.disk.totalGb); ws.metrics.network.inMb = clampedJitter(ws.metrics.network.inMb, 0.3, 10, Number.POSITIVE_INFINITY); ws.metrics.network.outMb = clampedJitter(ws.metrics.network.outMb, 0.3, 10, Number.POSITIVE_INFINITY);apps/web/app/api/auth/refresh/route.ts (1)
10-50: Consider adding security audit logging.Token refresh is a security-sensitive operation. Logging refresh attempts (successful and failed) helps with security monitoring and incident response.
export async function POST(request: NextRequest) { try { const body = await request.json(); const { refreshToken } = body; if (!refreshToken) { return NextResponse.json( createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, 'Refresh token required'), { status: 400 } ); } // Verify refresh token const payload = verifyToken(refreshToken); if (payload.type !== 'refresh') { + console.warn('Token refresh failed: invalid token type', { userId: payload.userId }); return NextResponse.json( createErrorResponse(401, ErrorCodes.TOKEN_INVALID, 'Invalid token type'), { status: 401 } ); } // Generate new access token const newAccessToken = generateAccessToken({ userId: payload.userId, email: payload.email, role: payload.role, }); + console.info('Token refreshed successfully', { userId: payload.userId }); + return NextResponse.json({ success: true, data: { accessToken: newAccessToken, }, }); } catch (error) { + console.error('Token refresh error:', error); const { response, status } = handleAPIError(error); return NextResponse.json(response, { status }); } }apps/web/app/api/workspaces/[id]/clone/route.ts (1)
55-55: Consider smarter name generation for clones.Appending " (Copy)" can result in names like "Workspace (Copy) (Copy)" if the original already has "(Copy)" in its name. Consider a numbered suffix approach.
// Generate unique clone name const baseName = original.name.replace(/ \(Copy( \d+)?\)$/, ''); const existingClones = await prisma.environment.count({ where: { userId: payload.userId, name: { startsWith: `${baseName} (Copy`, }, }, }); const cloneName = existingClones === 0 ? `${baseName} (Copy)` : `${baseName} (Copy ${existingClones})`;apps/web/app/api/workspaces/options/route.ts (1)
3-27: Consider externalizing configuration.The workspace options are hardcoded in the route handler. Moving them to a dedicated config file would improve maintainability and allow easier updates without code changes.
Create a config file at
apps/web/config/workspace-options.ts:export const WORKSPACE_OPTIONS = { providers: [ { id: "aws", name: "AWS" }, { id: "gcp", name: "GCP" }, { id: "azure", name: "Azure" }, { id: "local", name: "Local" }, ], // ... rest of options };Then import in the route:
import { NextResponse } from "next/server"; +import { WORKSPACE_OPTIONS } from "@/config/workspace-options"; -const OPTIONS = { - providers: [ - // ... - ], -}; - export async function GET() { - return NextResponse.json(OPTIONS); + return NextResponse.json(WORKSPACE_OPTIONS); }apps/web/app/api/auth/login/route.ts (1)
68-72: Remove commented-out code.The commented lastLoginAt tracking should either be implemented or removed. Dead code adds clutter and confusion.
Either implement it:
- // Update last login (optional - you may want to add this field to schema) - // await prisma.user.update({ - // where: { id: user.id }, - // data: { lastLoginAt: new Date() }, - // }); + // Update last login + await prisma.user.update({ + where: { id: user.id }, + data: { lastLoginAt: new Date() }, + });Or remove it entirely if not needed.
apps/web/app/api/auth/logout/route.ts (1)
12-17: Clarify the purpose of authentication check in logout.The
requireAuthcall verifies the token but its result is unused. Since JWT logout is client-side (token removal), this validation step seems unnecessary. If the intent is to verify the token is valid before acknowledging logout, the comment should clarify this.Either remove the check:
export async function POST(request: NextRequest) { try { - // Verify authentication (optional - just for validation) - await requireAuth(request); - // In a JWT-based system, logout is typically handled client-side // by removing the token. You could optionally implement a token // blacklist using Redis here.Or clarify its purpose:
- // Verify authentication (optional - just for validation) + // Verify token is valid before acknowledging logout + // This prevents logout of already-invalid tokens from appearing successful await requireAuth(request);apps/web/app/api/teams/invitations/accept/route.ts (1)
60-71: Potential for clearer error messaging.The check at line 66 conflates two distinct scenarios: (1) user doesn't exist, and (2) authenticated user doesn't match the invited email. Consider separating these for clearer diagnostics, though the current implementation correctly prevents email enumeration.
apps/web/QUICK_FIX_TEST.md (1)
115-132: Optional: Add language specifiers to code blocks for better rendering.The code blocks showing expected API responses would render better with
jsonlanguage specifiers.Apply this diff to improve syntax highlighting:
## 📊 Expected API Calls (Check Network Tab) When creating a workspace, you should see: -``` +```json POST /api/workspaces 201 Created Response: { "id": "some-uuid",When viewing workspaces: -``` +```json GET /api/workspaces 200 OK Response: [ { "id": "...", "name": "My First Workspace", ... }apps/web/TESTING_GUIDE.md (1)
36-39: Optional: Add language specifiers to code blocks.Several code blocks showing expected output lack language specifiers (lines 36, 49, 61), which would improve rendering. This aligns with the markdownlint hints.
Also applies to: 49-51, 61-68
apps/web/app/api/teams/[id]/workspaces/route.ts (1)
43-58: Consider clarifying the environment-to-workspace naming.The code queries
prisma.environmentbut returns it asworkspacesin the response (line 65). This naming inconsistency might reflect schema evolution, but consider whether the schema should be updated for clarity or if this mapping is intentional.apps/web/app/api/teams/route.ts (1)
75-78: Defensive handling for empty membership array.Line 76 uses optional chaining and fallback (
team.members[0]?.role || 'OWNER'), which is good defensive programming. However, since the create operation includes the membership creation,team.members[0]should always exist. Consider whether this defensive code is necessary or if it indicates a potential issue with the query.apps/web/Dev8-Postman-Collection.json (1)
82-102: Capture auth tokens after login.Running the collection starting from “Login” leaves
accessToken,refreshToken, anduserIdempty, so every downstream request fails authorization. Please mirror the register script and persist the tokens returned by the login response.Apply this diff:
{ "name": "1.2 Login User", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "if (pm.response.code === 200) {", + " const response = pm.response.json();", + " pm.collectionVariables.set('accessToken', response.tokens.accessToken);", + " pm.collectionVariables.set('refreshToken', response.tokens.refreshToken);", + " pm.collectionVariables.set('userId', response.user.id);", + " console.log('✅ Tokens refreshed from login response');", + "}" + ] + } + } + ], "request": {apps/web/app/billing-usage/page.tsx (1)
44-90: Handle non-OK billing responses before parsing JSON.If
/api/billingreturns 401/500 (or non-JSON),res.json()throws, we log, setloadingfalse, and the component rendersnull, giving users a blank screen. Checkres.okfirst and surface an actionable fallback (e.g., redirect, set error state).Apply this diff:
- const res = await fetch("/api/billing", { cache: "no-store" }); - const j = (await res.json()) as BillingData; - setData(j); + const res = await fetch("/api/billing", { cache: "no-store" }); + if (!res.ok) { + throw new Error(`Billing fetch failed with ${res.status}`); + } + const payload = (await res.json()) as BillingData; + setData(payload); + } catch (e) { + console.error(e); + setData(null); + return; } catch (e) { console.error(e);(Adjust the catch to expose a visible error state or redirect as appropriate.)
apps/web/app/api/workspaces/[id]/activity/route.ts (1)
55-61: Remove redundantworkspaceIdparameter.The
workspaceIdproperty in the object passed torecordActivity(id, {...})duplicates the first parameterid. This redundancy could lead to inconsistencies if they ever diverge.Apply this diff:
try { await recordActivity(id, { - workspaceId: id, cpuUsage: body.cpuUsage || 0, memoryUsage: body.memoryUsage || 0, diskUsage: body.diskUsage || 0, timestamp: new Date().toISOString(), });apps/web/AUTHENTICATION_FIX.md (1)
148-153: Format bare URLs to follow markdown linting rules.Lines 148, 151, 152, and 153 contain bare URLs that should be wrapped in angle brackets for proper markdown formatting.
Apply this diff:
2. Open http://localhost:3000 3. Test user flow: - - Sign up: http://localhost:3000/signup - - Log in: http://localhost:3000/signin - - Create workspace: http://localhost:3000/workspaces/new + - Sign up: <http://localhost:3000/signup> + - Log in: <http://localhost:3000/signin> + - Create workspace: <http://localhost:3000/workspaces/new> - **Expected**: No 401 errors, workspace created successfullyFor line 148, you could similarly wrap it:
-2. Open http://localhost:3000 +2. Open <http://localhost:3000>apps/web/app/api/users/search/route.ts (5)
14-14: Consider capturing the authentication payload for consistency.Other routes in the codebase (e.g.,
apps/web/app/api/users/me/route.ts) capture the returnedAuthUserpayload fromrequireAuth. While not strictly necessary here, storing it improves consistency and enables future enhancements like audit logging or user-specific rate limiting.Apply this diff:
- await requireAuth(request); + const payload = await requireAuth(request);
18-19: Add radix to parseInt and validate numeric bounds.The
parseIntcalls should specify base 10 explicitly, and the code should validate thatlimitandoffsetare non-negative to prevent unexpected behavior.Apply this diff:
- const limit = parseInt(searchParams.get('limit') || '20'); - const offset = parseInt(searchParams.get('offset') || '0'); + const limit = Math.max(1, parseInt(searchParams.get('limit') || '20', 10)); + const offset = Math.max(0, parseInt(searchParams.get('offset') || '0', 10));
21-21: Simplify validation condition.The
!qcheck is redundant since an empty string has length 0, which will be caught byq.length < 2.Apply this diff:
- if (!q || q.length < 2) { + if (q.length < 2) {
46-54: Extract duplicate query logic to reduce code duplication.The
whereclause is duplicated between the search query (lines 30-34) and the count query (lines 48-52). Extract it into a variable to follow DRY principles and reduce maintenance burden.Apply this diff:
+ const whereClause = { + OR: [ + { name: { contains: q, mode: 'insensitive' as const } }, + { email: { contains: q, mode: 'insensitive' as const } }, + ], + }; + // Search users const users = await prisma.user.findMany({ - where: { - OR: [ - { name: { contains: q, mode: 'insensitive' } }, - { email: { contains: q, mode: 'insensitive' } }, - ], - }, + where: whereClause, select: { id: true, name: true, email: true, image: true, }, take: Math.min(limit, 100), skip: offset, }); // Get total count - const total = await prisma.user.count({ - where: { - OR: [ - { name: { contains: q, mode: 'insensitive' } }, - { email: { contains: q, mode: 'insensitive' } }, - ], - }, - }); + const total = await prisma.user.count({ where: whereClause });
29-54: Consider performance implications of two-query pagination.The current implementation makes two separate database queries (findMany + count) for pagination. While this is a standard pattern, consider these alternatives for high-traffic scenarios:
- Cursor-based pagination: More efficient for large datasets, eliminates the need for count queries.
- Approximate counts: For very large result sets, consider caching or approximating total counts.
- Database transactions: If consistency between count and results is critical, wrap both queries in a transaction.
For most use cases, the current approach is acceptable.
apps/web/app/components/ui/pagination.tsx (2)
68-83: Consider allowing size customization.Both
PaginationPreviousandPaginationNexthardcodesize="default", which prevents users from customizing the button size via props. While this ensures consistent sizing, it limits flexibility.If size customization is desired, remove the hardcoded value:
function PaginationPrevious({ className, + size = 'default', ...props }: React.ComponentProps<typeof PaginationLink>) { return ( <PaginationLink aria-label="Go to previous page" - size="default" + size={size} className={cn('gap-1 px-2.5 sm:pl-2.5', className)} {...props} >Apply the same pattern to
PaginationNext.Also applies to: 85-100
102-117: Consider explicit aria-hidden value for clarity.While
aria-hiddenwithout a value is valid in React (treated astrue), being explicit improves code clarity.<span - aria-hidden + aria-hidden="true" data-slot="pagination-ellipsis" className={cn('flex size-9 items-center justify-center', className)}apps/web/app/components/ui/chart.tsx (1)
107-249: Consider decomposing for maintainability.The ChartTooltipContent component handles multiple responsibilities: label formatting, payload rendering, indicator styling, and nested label logic. While functionally correct, extracting helper functions or sub-components for tooltip label rendering and payload item rendering could improve readability and testability.
apps/web/app/components/ui/dropdown-menu.tsx (1)
34-52: Note: Embedded Portal in Content component.The
DropdownMenuContentcomponent wraps itself in aPortalfor convenience, which is a common pattern. However, sinceDropdownMenuPortalis also exported separately, this might be confusing for users. The exportedDropdownMenuPortalappears to be for advanced use cases only, not for use withDropdownMenuContent.Consider adding a JSDoc comment clarifying that
DropdownMenuContentincludes its own portal and thatDropdownMenuPortalis for advanced customization only.
| export async function POST() { | ||
| // TODO: hook into your real user deletion logic | ||
| // For now, just simulate success so the button works end-to-end | ||
| return NextResponse.json({ ok: true }, { status: 200 }); | ||
| } |
There was a problem hiding this comment.
Critical: Add authentication and authorization checks immediately.
This endpoint allows account deletion but has no authentication or authorization checks. Any unauthenticated request would receive a success response, and when real deletion logic is implemented, this could allow unauthorized account deletion.
Before implementing real deletion logic, add authentication:
import { NextResponse } from "next/server";
+import { getServerSession } from "next-auth";
+import { authOptions } from "@/lib/auth"; // adjust path as needed
-export async function POST() {
+export async function POST(req: Request) {
+ const session = await getServerSession(authOptions);
+ if (!session?.user) {
+ return NextResponse.json({ ok: false, error: "Unauthorized" }, { status: 401 });
+ }
+
// TODO: hook into your real user deletion logic
// For now, just simulate success so the button works end-to-end
return NextResponse.json({ ok: true }, { status: 200 });
}Additionally, when implementing real deletion:
- Verify the authenticated user matches the account to be deleted
- Add audit logging for compliance
- Implement GDPR-compliant data deletion (cascade to related data)
- Consider requiring a confirmation token to prevent accidental deletion
- Handle related resources (workspace memberships, billing, etc.)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/account/delete/route.ts around lines 3 to 7, the handler
returns success without any authentication or authorization; add an
authentication guard that extracts and validates the requesting user (return 401
if unauthenticated), verify the authenticated user's id matches the account
being deleted (return 403 if mismatched), and only then proceed to call the
deletion logic; also emit an audit log entry (who, when, target account, request
id) and require/validate a deletion confirmation token or explicit confirmation
flag before performing destructive operations to prevent accidental or
unauthorized deletes.
| export async function POST(req: Request) { | ||
| // This is a placeholder implementation; validate and change password in your auth system here | ||
| const { current, next } = await req.json(); | ||
| if (!next || next.length < 8) { | ||
| return NextResponse.json({ ok: false, error: "Password too short" }, { status: 400 }); | ||
| } | ||
| return NextResponse.json({ ok: true }); |
There was a problem hiding this comment.
Require auth and implement real password change handling
Right now any unauthenticated caller can POST here and immediately get { ok: true } because we never call requireAuth, never verify the provided current password, and never persist the new one. That defeats the whole point of the reset flow and opens the door to spoofed success responses. Please gate this route behind requireAuth, validate both current and next as strings, compare current against the stored hash, and write the new hash back to the user record before returning success.
-import { NextResponse } from "next/server";
+import { NextResponse } from "next/server";
+import { requireAuth } from "@/lib/auth";
-export async function POST(req: Request) {
- // This is a placeholder implementation; validate and change password in your auth system here
- const { current, next } = await req.json();
- if (!next || next.length < 8) {
- return NextResponse.json({ ok: false, error: "Password too short" }, { status: 400 });
- }
- return NextResponse.json({ ok: true });
+export async function POST(req: Request) {
+ const user = await requireAuth(req);
+ const { current, next } = await req.json();
+
+ if (typeof next !== "string" || next.length < 8) {
+ return NextResponse.json({ ok: false, error: "Password too short" }, { status: 400 });
+ }
+ if (typeof current !== "string" || current.length === 0) {
+ return NextResponse.json({ ok: false, error: "Current password required" }, { status: 400 });
+ }
+
+ // TODO: look up the user record (user.userId), verify `current`, hash `next`, and persist it.
+
+ return NextResponse.json({ ok: true });
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/account/password/route.ts around lines 3 to 9, the handler
currently allows unauthenticated callers to change passwords and returns success
without verification; update it to require authentication via requireAuth,
validate that both `current` and `next` exist and are strings (and that `next`
meets length/complexity), fetch the authenticated user's record, verify
`current` by comparing it against the stored password hash (using the same
hashing/compare utility), if verification fails return a 401/400 error,
otherwise hash the `next` password with the project's password-hashing utility,
persist the new hash to the user record in the database, and only then return `{
ok: true }`; include appropriate error responses for missing fields, failed
verification, and persistence errors.
| export async function GET() { | ||
| return NextResponse.json({ agents }); | ||
| } |
There was a problem hiding this comment.
Add authentication to protect the agents endpoint.
Similar to the MCP config endpoint, this route should require authentication to prevent unauthorized access to agent information.
Apply this diff:
+import { auth } from "@/lib/auth";
+
export async function GET() {
+ const session = await auth();
+ if (!session) {
+ return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
+ }
return NextResponse.json({ agents });
}🤖 Prompt for AI Agents
In apps/web/app/api/ai/agents/route.ts around lines 12-14, the GET handler
currently returns agents without authentication; update it to require
authentication like the MCP config endpoint by importing and invoking the same
auth/session check used elsewhere (e.g., getServerSession or your project's
requireAuth helper) at the start of the GET handler, return a 401/403
NextResponse if the user is unauthenticated, and only return NextResponse.json({
agents }) after the auth check succeeds; ensure you preserve existing response
shape and error status codes consistent with the MCP config route.
| export async function POST(req: Request) { | ||
| const { id, action } = await req.json(); | ||
| agents = agents.map((a) => | ||
| a.id === id | ||
| ? { | ||
| ...a, | ||
| status: action === "connect" ? "connected" : action === "disconnect" ? "disconnected" : a.status, | ||
| } | ||
| : a | ||
| ); | ||
| return NextResponse.json({ ok: true, agents }); | ||
| } |
There was a problem hiding this comment.
Add authentication and validate the request payload.
The POST endpoint lacks authentication and doesn't validate that the agent ID exists or that the action is valid. This allows unauthorized state changes and silent failures.
Apply this diff:
+import { auth } from "@/lib/auth";
+
export async function POST(req: Request) {
+ const session = await auth();
+ if (!session) {
+ return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
+ }
+
const { id, action } = await req.json();
+
+ // Validate action
+ if (!["connect", "disconnect"].includes(action)) {
+ return NextResponse.json({ error: "Invalid action" }, { status: 400 });
+ }
+
+ // Check if agent exists
+ const agentExists = agents.some((a) => a.id === id);
+ if (!agentExists) {
+ return NextResponse.json({ error: "Agent not found" }, { status: 404 });
+ }
+
agents = agents.map((a) =>
a.id === id
? {
...a,
status: action === "connect" ? "connected" : action === "disconnect" ? "disconnected" : a.status,
}
: a
);
return NextResponse.json({ ok: true, agents });
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/ai/agents/route.ts around lines 16 to 27, the POST handler
must enforce authentication and validate the incoming payload before mutating
agent state: extract and verify the caller is authenticated (return 401 if not),
parse and validate that req.json() contains a string id and an action that is
one of "connect" or "disconnect" (return 400 for missing/invalid fields), check
that an agent with the given id exists (return 404 if not), then update only
that agent's status accordingly and return the updated agents list; also ensure
proper error responses and avoid silently ignoring invalid actions.
| export async function GET() { | ||
| return NextResponse.json({ ...config, recent }); | ||
| } |
There was a problem hiding this comment.
Add authentication to protect sensitive configuration data.
This endpoint returns API keys without any authentication, exposing potentially sensitive credentials to unauthorized users.
Based on the PR context showing a unified authentication system in lib/auth.ts, apply authentication middleware or check session before returning data:
+import { auth } from "@/lib/auth";
+
export async function GET() {
+ const session = await auth();
+ if (!session) {
+ return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
+ }
return NextResponse.json({ ...config, recent });
}📝 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.
| export async function GET() { | |
| return NextResponse.json({ ...config, recent }); | |
| } | |
| import { auth } from "@/lib/auth"; | |
| export async function GET() { | |
| const session = await auth(); | |
| if (!session) { | |
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | |
| } | |
| return NextResponse.json({ ...config, recent }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/ai/mcp-config/route.ts around lines 10 to 12, the GET
handler currently returns sensitive config (including API keys) with no auth;
update it to require authentication by importing and using the unified auth from
lib/auth.ts (or the existing session middleware) to verify the request/session
before returning data, return a 401/403 when the user is not
authenticated/authorized, and ensure you sanitize the response by omitting or
redacting secrets unless the authenticated user has explicit permission to view
them.
| function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | ||
| return ( | ||
| <tfoot | ||
| data-slot="table-footer" | ||
| className={cn( | ||
| 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix invalid CSS selector syntax.
Line 47 contains [&>tr]:last:border-b-0 which uses invalid CSS syntax. The :last: portion is not a valid pseudo-class and will cause this selector to fail.
Apply this diff to correct the selector:
className={cn(
- 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0',
+ 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0',
className,
)}📝 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 TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | |
| return ( | |
| <tfoot | |
| data-slot="table-footer" | |
| className={cn( | |
| 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | |
| return ( | |
| <tfoot | |
| data-slot="table-footer" | |
| className={cn( | |
| 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/components/ui/table.tsx around lines 42 to 53, the class string
uses an invalid selector `[&>tr]:last:border-b-0`; replace that invalid
pseudo-class with a valid one such as `[&>tr:last-child]:border-b-0` so the
selector correctly targets the last tr child and removes its bottom border;
update the className entry accordingly.
| function Tabs({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof TabsPrimitive.Root>) { | ||
| return ( | ||
| <TabsPrimitive.Root | ||
| data-slot="tabs" | ||
| className={cn('flex flex-col gap-2', className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| function TabsList({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof TabsPrimitive.List>) { | ||
| return ( | ||
| <TabsPrimitive.List | ||
| data-slot="tabs-list" | ||
| className={cn( | ||
| 'bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-[3px]', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| function TabsTrigger({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof TabsPrimitive.Trigger>) { | ||
| return ( | ||
| <TabsPrimitive.Trigger | ||
| data-slot="tabs-trigger" | ||
| className={cn( | ||
| "data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| function TabsContent({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof TabsPrimitive.Content>) { | ||
| return ( | ||
| <TabsPrimitive.Content | ||
| data-slot="tabs-content" | ||
| className={cn('flex-1 outline-none', className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if ref forwarding is needed for these components.
None of the tab components use React.forwardRef, which means refs cannot be passed to the underlying Radix primitives. If consumers need to access the DOM elements for focus management, measurements, or testing, this could be a limitation.
Run the following script to check if refs are used with these tab components elsewhere in the codebase:
If refs are needed, consider wrapping each component with React.forwardRef:
const Tabs = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Root>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Root>
>(({ className, ...props }, ref) => {
return (
<TabsPrimitive.Root
ref={ref}
data-slot="tabs"
className={cn('flex flex-col gap-2', className)}
{...props}
/>
)
})
Tabs.displayName = 'Tabs'🏁 Script executed:
#!/bin/bash
# Description: Search for ref usage with Tabs components to determine if forwardRef is needed.
# Search for ref props being passed to any of the tab components
echo "=== Searching for ref usage with tab components ==="
rg -nP --type=tsx --type=ts -C3 '<Tab(s|sList|sTrigger|sContent)\b[^>]*\bref='
# Search for useRef hooks that might be assigned to tab components
echo -e "\n=== Searching for useRef with tab component variables ==="
rg -nP --type=tsx --type=ts -C3 'useRef.*tab|tab.*useRef' -i
# Search for React.createRef usage with tabs
echo -e "\n=== Searching for createRef with tabs ==="
rg -nP --type=tsx --type=ts -C3 'createRef.*tab|tab.*createRef' -iLength of output: 237
I need to adjust the search approach since the file type recognition failed. Let me run an updated verification:
🏁 Script executed:
#!/bin/bash
# Alternative approach: use fd to find files, then search for ref patterns
echo "=== Searching for Tabs component imports and usage ==="
fd -e tsx -e ts | head -30 | xargs rg -l "Tabs|TabsList|TabsTrigger|TabsContent" 2>/dev/null | head -20
echo -e "\n=== Searching for ref= patterns in component files ==="
fd -e tsx -e ts | xargs rg -C3 "ref=" 2>/dev/null | grep -A3 -B3 "Tabs\|tabs" | head -50
echo -e "\n=== Checking for useRef patterns related to tabs ==="
fd -e tsx -e ts | xargs rg -C2 "useRef" 2>/dev/null | grep -i "tab" | head -30
echo -e "\n=== Checking how TabsPrimitive.Root is typically used in Radix UI examples ==="
rg -C2 "data-slot=\"tabs\"" 2>/dev/nullLength of output: 554
Let me check if these components are exported and search more broadly:
🏁 Script executed:
#!/bin/bash
# Check if tabs are exported and their usage
echo "=== Checking for tabs.tsx export paths ==="
fd -e ts -e tsx -e js -e jsx | xargs rg "from.*tabs|import.*Tabs" 2>/dev/null | head -20
echo -e "\n=== Checking if tabs are exported from index files ==="
fd -name "index.ts" -o -name "index.tsx" | xargs rg -l "tabs\|Tabs" 2>/dev/null
echo -e "\n=== Checking package.json or barrel exports ==="
fd -e ts | xargs rg "export.*Tabs|export.*from.*tabs" 2>/dev/null | head -20
echo -e "\n=== Listing all files in ui directory ==="
fd -path "*/ui/*" -type f | head -20Length of output: 680
🏁 Script executed:
#!/bin/bash
# Check if tabs are exported and their usage
echo "=== Checking for tabs.tsx import statements ==="
fd -e ts -e tsx -e js -e jsx | xargs rg "from.*tabs|import.*Tabs" 2>/dev/null | head -20
echo -e "\n=== Checking if tabs are exported from index files ==="
fd -name "index.ts" -o -name "index.tsx" | xargs rg "tabs|Tabs" 2>/dev/null
echo -e "\n=== Checking for re-exports in ui directory ==="
fd -path "*/ui/*" -type f -name "*.ts" -o -name "*.tsx" | xargs rg "export.*from|export.*Tabs" 2>/dev/null | head -20
echo -e "\n=== Check the actual tabs.tsx file content for exports ==="
rg "^export" apps/web/app/components/ui/tabs.tsx 2>/dev/nullLength of output: 2352
Let me check if other UI components in the codebase use forwardRef to establish the pattern:
🏁 Script executed:
#!/bin/bash
# Check if other UI components use React.forwardRef
echo "=== Searching for forwardRef usage in UI components ==="
fd -path "*/ui/*.tsx" -o -path "*/ui/*.ts" | xargs rg "forwardRef|React.forwardRef" 2>/dev/null
echo -e "\n=== Checking structure of other UI component files ==="
fd -path "*/components/ui/*.tsx" | head -5 | xargs rg -A5 "^(function|export)" 2>/dev/null | head -60
echo -e "\n=== Checking for ref parameters in other component signatures ==="
fd -path "*/components/ui/*.tsx" | xargs rg "ref.*React\.ComponentProps|React\.ComponentPropsWithoutRef" 2>/dev/nullLength of output: 5718
Add React.forwardRef to all tab components to match established codebase patterns.
The codebase consistently uses React.forwardRef for UI wrapper components. Every similar component in apps/web/components/ui/ uses it (button.tsx, card.tsx, input.tsx, label.tsx, and toast.tsx all forward refs). The Tabs components should follow the same pattern, especially since they wrap Radix UI primitives that support ref forwarding.
Apply forwardRef to each component:
const Tabs = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Root>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Root>
>(({ className, ...props }, ref) => (
<TabsPrimitive.Root
ref={ref}
data-slot="tabs"
className={cn('flex flex-col gap-2', className)}
{...props}
/>
))
Tabs.displayName = 'Tabs'
const TabsList = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.List>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.List>
>(({ className, ...props }, ref) => (
<TabsPrimitive.List
ref={ref}
data-slot="tabs-list"
className={cn('bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-[3px]', className)}
{...props}
/>
))
TabsList.displayName = 'TabsList'
const TabsTrigger = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Trigger>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Trigger>
>(({ className, ...props }, ref) => (
<TabsPrimitive.Trigger
ref={ref}
data-slot="tabs-trigger"
className={cn("data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", className)}
{...props}
/>
))
TabsTrigger.displayName = 'TabsTrigger'
const TabsContent = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Content>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Content>
>(({ className, ...props }, ref) => (
<TabsPrimitive.Content
ref={ref}
data-slot="tabs-content"
className={cn('flex-1 outline-none', className)}
{...props}
/>
))
TabsContent.displayName = 'TabsContent'🤖 Prompt for AI Agents
In apps/web/app/components/ui/tabs.tsx around lines 8 to 64, the Tabs, TabsList,
TabsTrigger, and TabsContent components are plain functional components but
should use React.forwardRef to match the codebase pattern and to forward refs to
the underlying Radix primitives; update each component to be a const assigned to
React.forwardRef with the proper generic types (React.ElementRef<typeof
TabsPrimitive.X> and React.ComponentPropsWithoutRef<typeof TabsPrimitive.X>),
accept (props, ref), pass ref to the corresponding TabsPrimitive element, set
displayName for each (e.g., Tabs, TabsList, TabsTrigger, TabsContent), and keep
existing className, data-slot and props spreading unchanged.
| function TooltipProvider({ | ||
| delayDuration = 0, | ||
| ...props | ||
| }: React.ComponentProps<typeof TooltipPrimitive.Provider>) { | ||
| return ( | ||
| <TooltipPrimitive.Provider | ||
| data-slot="tooltip-provider" | ||
| delayDuration={delayDuration} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Consider increasing the default delay for better UX.
The delayDuration = 0 default causes tooltips to appear instantly on hover, which can lead to flickering or distracting tooltips when users move their cursor across the interface. A small delay (200-700ms) is typically recommended to prevent accidental triggers.
Apply this diff to use a more standard delay:
function TooltipProvider({
- delayDuration = 0,
+ delayDuration = 400,
...props
}: React.ComponentProps<typeof TooltipPrimitive.Provider>) {📝 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 TooltipProvider({ | |
| delayDuration = 0, | |
| ...props | |
| }: React.ComponentProps<typeof TooltipPrimitive.Provider>) { | |
| return ( | |
| <TooltipPrimitive.Provider | |
| data-slot="tooltip-provider" | |
| delayDuration={delayDuration} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| function TooltipProvider({ | |
| delayDuration = 400, | |
| ...props | |
| }: React.ComponentProps<typeof TooltipPrimitive.Provider>) { | |
| return ( | |
| <TooltipPrimitive.Provider | |
| data-slot="tooltip-provider" | |
| delayDuration={delayDuration} | |
| {...props} | |
| /> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/components/ui/tooltip.tsx around lines 8 to 19, the
TooltipProvider currently sets delayDuration = 0 causing tooltips to appear
instantly; change the default to a small UX-friendly delay (e.g., delayDuration
= 300) so tooltips wait ~300ms before showing to reduce accidental triggers and
flicker, keeping the rest of the props spread and behavior unchanged.
| function Tooltip({ | ||
| ...props | ||
| }: React.ComponentProps<typeof TooltipPrimitive.Root>) { | ||
| return ( | ||
| <TooltipProvider> | ||
| <TooltipPrimitive.Root data-slot="tooltip" {...props} /> | ||
| </TooltipProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Reconsider the Provider wrapping pattern.
Each Tooltip instance creates its own TooltipProvider, which deviates from the Radix UI recommended pattern where a single Provider wraps multiple tooltips at the app or section level. This approach:
- Prevents shared tooltip behavior (e.g., skip delay when moving between tooltips)
- Creates performance overhead with multiple provider instances
- May cause confusion since
TooltipProvideris also exported separately
Recommended approach:
Either remove the internal provider and require users to wrap their app in TooltipProvider, or document this convenience wrapper clearly if the behavior is intentional.
Apply this diff to follow the standard pattern:
function Tooltip({
...props
}: React.ComponentProps<typeof TooltipPrimitive.Root>) {
- return (
- <TooltipProvider>
- <TooltipPrimitive.Root data-slot="tooltip" {...props} />
- </TooltipProvider>
- )
+ return <TooltipPrimitive.Root data-slot="tooltip" {...props} />
}Then users would wrap their app/section with TooltipProvider:
// In app layout or page
<TooltipProvider>
<YourApp />
</TooltipProvider>🤖 Prompt for AI Agents
In apps/web/app/components/ui/tooltip.tsx around lines 21 to 29, the component
currently creates a new TooltipProvider per Tooltip instance which breaks
Radix's recommended single-provider pattern and causes shared-behavior and
performance issues; remove the internal <TooltipProvider> wrapper so Tooltip
simply forwards props to TooltipPrimitive.Root (leaving TooltipProvider exported
for app-level wrapping), and update any docs/examples to show wrapping the app
or section with TooltipProvider instead of relying on the component to provide
it.
| export function useIsMobile() { | ||
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) | ||
|
|
||
| React.useEffect(() => { | ||
| const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) | ||
| const onChange = () => { | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | ||
| } | ||
| mql.addEventListener('change', onChange) | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | ||
| return () => mql.removeEventListener('change', onChange) | ||
| }, []) | ||
|
|
||
| return !!isMobile | ||
| } |
There was a problem hiding this comment.
SSR/hydration mismatch will cause warnings on mobile devices.
In Next.js (app directory), this hook will return false during server-side rendering and initial client render, then switch to true after the effect runs on mobile viewports. This creates a hydration mismatch that triggers React warnings and may cause layout shifts.
Root cause: The server cannot access window, so isMobile remains undefined (coerced to false), while the client updates to true post-mount on mobile devices.
Apply this diff to prevent hydration mismatches:
export function useIsMobile() {
- const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined)
+ const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined)
+ const [mounted, setMounted] = React.useState(false)
React.useEffect(() => {
+ setMounted(true)
const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`)
const onChange = () => {
setIsMobile(window.innerWidth < MOBILE_BREAKPOINT)
}
mql.addEventListener('change', onChange)
setIsMobile(window.innerWidth < MOBILE_BREAKPOINT)
return () => mql.removeEventListener('change', onChange)
}, [])
- return !!isMobile
+ // Return undefined until mounted to prevent hydration mismatch
+ return mounted ? isMobile ?? false : undefined
}Note: Callers must handle undefined (e.g., const isMobile = useIsMobile(); if (isMobile === undefined) return null;) or accept that the initial render assumes non-mobile.
📝 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.
| export function useIsMobile() { | |
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) | |
| React.useEffect(() => { | |
| const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) | |
| const onChange = () => { | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | |
| } | |
| mql.addEventListener('change', onChange) | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | |
| return () => mql.removeEventListener('change', onChange) | |
| }, []) | |
| return !!isMobile | |
| } | |
| export function useIsMobile() { | |
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) | |
| const [mounted, setMounted] = React.useState(false) | |
| React.useEffect(() => { | |
| setMounted(true) | |
| const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) | |
| const onChange = () => { | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | |
| } | |
| mql.addEventListener('change', onChange) | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) | |
| return () => mql.removeEventListener('change', onChange) | |
| }, []) | |
| // Return undefined until mounted to prevent hydration mismatch | |
| return mounted ? isMobile ?? false : undefined | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/components/ui/use-mobile.tsx around lines 5 to 19, the hook
currently coerces isMobile to a boolean (return !!isMobile) causing SSR/client
hydration mismatches; change the hook to preserve undefined during SSR by (a)
initializing state without coercion and using a safe window check (e.g., set
initial state to window.innerWidth < MOBILE_BREAKPOINT only when typeof window
!== 'undefined', otherwise undefined), (b) keep the effect to update state on
mount and media changes, and (c) return the raw isMobile (possibly undefined)
instead of forcing a boolean so callers can handle the undefined initial state.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 34
♻️ Duplicate comments (33)
apps/web/app/components/ui/tabs.tsx (1)
8-64: [Unresolved] Missing React.forwardRef on all tab components.This critical issue was previously flagged but remains unresolved. All four tab components (
Tabs,TabsList,TabsTrigger,TabsContent) still use plain function syntax instead ofReact.forwardRef, which breaks the established codebase pattern where every UI wrapper component (button.tsx, card.tsx, input.tsx, label.tsx, toast.tsx) forwards refs.Without ref forwarding, consumers cannot access the underlying DOM elements for focus management, measurements, or testing.
Please apply the forwardRef pattern as detailed in the previous review comment above.
apps/web/app/components/ui/progress.tsx (1)
14-20: Critical: The previously reported accessibility issue remains unfixed.The
valueprop is still not passed toProgressPrimitive.Root, which prevents Radix from setting proper accessibility attributes (aria-valuenow,data-value,data-state) for screen readers. This critical issue was flagged in a previous review but has not been addressed.Apply this diff to fix the accessibility issue:
<ProgressPrimitive.Root data-slot="progress" className={cn( 'bg-primary/20 relative h-2 w-full overflow-hidden rounded-full', className, )} + value={value} {...props} >apps/web/app/components/ui/use-mobile.tsx (1)
5-19: Critical SSR/hydration issue remains unaddressed from previous review.This hook still causes hydration mismatches on mobile devices as flagged in the previous review. During SSR,
isMobileis coerced tofalse(line 18), but switches totrueafter mount on mobile viewports, triggering React hydration warnings and potential layout shifts.Please implement the fix suggested in the previous review before merging this PR.
apps/web/app/components/ui/collapsible.tsx (1)
1-3: Missing React import for type annotations.The file uses
React.ComponentPropson lines 7, 13, and 24 but does not import React, which will cause TypeScript errors.apps/web/app/components/ui/context-menu.tsx (1)
80-112: Tailwind CSS v3.4.0 incompatibility already flagged.This critical issue with the parentheses syntax for CSS custom properties on lines 88 and 105 has already been identified in the previous review. The codebase uses Tailwind v3.4.0, which requires square bracket syntax:
origin-[var(--radix-context-menu-content-transform-origin)]andmax-h-[var(--radix-context-menu-content-available-height)].Please apply the fixes suggested in the previous review comment before merging.
apps/web/app/components/ui/table.tsx (1)
47-49: Fix invalid selector pseudo-class.Line 47 uses
[&>tr]:last:border-b-0, but:lastis not a valid pseudo-class, so the selector never matches and the final row keeps its border. Replace it with:last-childso the intended styling applies.className={cn( - 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', + 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0', className, )}apps/web/app/api/templates/route.ts (2)
3-11: Critical: Authentication still missing.The authentication issue flagged in the previous review remains unaddressed. Without authentication, this endpoint is publicly accessible, allowing anyone to create templates.
As previously recommended, add the authentication check at the start of the handler:
export async function POST(req: Request) { + const payload = await requireAuth(req); try { const body = await req.json();
5-7: Major: Input validation still missing.The input validation issue flagged in the previous review remains unaddressed. The handler still accepts any JSON payload without validating against the expected template schema (name, displayName, description, baseImage, defaultCPU, defaultMemory, category, tags, etc.).
As previously recommended, add schema validation before persisting:
+import { z } from "zod"; + +const templateSchema = z.object({ + name: z.string().min(1), + displayName: z.string().min(1), + description: z.string(), + baseImage: z.string(), + defaultCPU: z.number().positive(), + defaultMemory: z.number().positive(), + category: z.string(), + tags: z.array(z.string()), + // ... other required fields +}); + export async function POST(req: Request) { try { const body = await req.json(); + const validatedData = templateSchema.parse(body); // TODO: persist to database/service - return NextResponse.json({ ok: true, template: body }, { status: 201 }); + return NextResponse.json({ ok: true, template: validatedData }, { status: 201 });apps/web/app/components/ui/popover.tsx (1)
20-40: Fix the Tailwind class names (duplicate issue).The className on line 33 still contains the two issues previously identified:
outline-hiddenis not a valid Tailwind utility class. Useoutline-noneinstead.origin-(--radix-popover-content-transform-origin)has incorrect syntax for Tailwind arbitrary values. It should beorigin-[var(--radix-popover-content-transform-origin)].Apply this diff to fix the class names:
className={cn( - 'bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 w-72 origin-(--radix-popover-content-transform-origin) rounded-md border p-4 shadow-md outline-hidden', + 'bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 w-72 origin-[var(--radix-popover-content-transform-origin)] rounded-md border p-4 shadow-md outline-none', className, )}apps/web/app/components/ui/carousel.tsx (2)
78-88: Restore vertical arrow-key navigation.
When orientation isvertical, ArrowUp/ArrowDown still do nothing, so vertical carousels remain non-keyboard-accessible. Branch on orientation to map the correct key pair and includeorientationin the callback deps.const handleKeyDown = React.useCallback( (event: React.KeyboardEvent<HTMLDivElement>) => { - if (event.key === 'ArrowLeft') { - event.preventDefault() - scrollPrev() - } else if (event.key === 'ArrowRight') { - event.preventDefault() - scrollNext() - } + if (orientation === 'horizontal') { + if (event.key === 'ArrowLeft') { + event.preventDefault() + scrollPrev() + } else if (event.key === 'ArrowRight') { + event.preventDefault() + scrollNext() + } + } else if (orientation === 'vertical') { + if (event.key === 'ArrowUp') { + event.preventDefault() + scrollPrev() + } else if (event.key === 'ArrowDown') { + event.preventDefault() + scrollNext() + } + } }, - [scrollPrev, scrollNext], + [orientation, scrollPrev, scrollNext], )
96-105: Unregister thereInithandler on cleanup.
We add bothreInitandselectlisteners, but only removeselect. After each Embla re-creation, anotherreInitcallback stacks up. Please unregister both.api.on('reInit', onSelect) api.on('select', onSelect) return () => { - api?.off('select', onSelect) + api?.off('reInit', onSelect) + api?.off('select', onSelect) }apps/web/app/components/ui/drawer.tsx (1)
54-54: Redundant data-slot attribute remains.This issue was already flagged in a previous review. The
DrawerPortalcomponent setsdata-slot="drawer-portal"internally at line 23, so passing it again here is unnecessary.apps/web/app/api/reporting/route.ts (2)
12-15: Refactor: Code duplication issue remains unresolved.This duplication was previously flagged. The jitter function in
apps/web/app/api/_state/workspaces.tsprovides better safety bounds and precision handling.
5-10: Critical: Global mutable seed still causes race conditions.This issue was previously flagged and remains unresolved. The global
seedvariable creates race conditions across concurrent requests in the same runtime instance.apps/web/app/components/ui/breadcrumb.tsx (1)
52-63: Remove incorrect link semantics from BreadcrumbPage.This issue was already flagged in a previous review. The
role="link"andaria-disabled="true"attributes on a non-interactive<span>create confusing semantics for screen readers. The current page node should simply usearia-current="page"on a non-interactive element without the role and aria-disabled attributes.apps/web/app/api/workspaces/estimate/route.ts (1)
19-25: Unsafe provider and size lookups can cause runtime errors.This is the same issue flagged in the previous review: the type casts on lines 19-20 don't validate the runtime values, so
BASE_PRICING[provider][size]on line 23 can throw when given unexpected input (e.g.,provider: "digitalocean"), returning a 500 instead of a proper 4xx validation error.The previous review provided a detailed fix. Alternatively, the Zod schema approach suggested above would also resolve this issue by validating the values before they're used.
apps/web/app/components/ui/command.tsx (1)
55-55: Fix invalid CSS selector syntax.The className contains
**:data-[slot=command-input-wrapper]:h-12which is invalid CSS syntax and will not apply the intended height styling. This was previously flagged but remains unresolved.Apply this diff to fix:
- <Command className="[&_[cmdk-group-heading]]:text-muted-foreground **:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5"> + <Command className="[&_[cmdk-group-heading]]:text-muted-foreground [&_[data-slot=command-input-wrapper]]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5">apps/web/app/components/ui/dialog.tsx (1)
58-58: Remove redundant data-slot attribute.Line 58 passes
data-slot="dialog-portal"to DialogPortal, but DialogPortal already sets this attribute internally at line 24. This creates unnecessary redundancy.Apply this diff:
- <DialogPortal data-slot="dialog-portal"> + <DialogPortal>apps/web/app/components/ui/menubar.tsx (2)
59-59: Tailwind v4 utilities require configuration or upgrade.The code uses
outline-hiddenandrounded-xsutilities which are Tailwind v4 features, but the project is on v3.4.0. These utilities need to be either:
- Added as custom utilities in
apps/web/tailwind.config.js, or- Resolved by upgrading to Tailwind v4
This is a project-wide issue affecting multiple UI components.
The
origin-(--radix-*)syntax is valid arbitrary value syntax and will work correctly.Also applies to: 82-82, 106-106, 124-124, 149-149, 251-251
232-232: Inconsistent outline utility class.Line 232 uses
outline-nonewhile other similar components useoutline-hidden(lines 59, 106, 124, 149). Once the Tailwind configuration issue is resolved, this should be updated for consistency.Apply this diff after resolving the Tailwind config:
- 'focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-none select-none data-[inset]:pl-8', + 'focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[inset]:pl-8',apps/web/app/components/ui/tooltip.tsx (1)
21-28: Remove the per-instance provider override.Each
Tooltipinstantiates its ownTooltipProviderwithdelayDuration=0, which overrides any ancestor provider configuration (shared delay, skip delay,disableHoverableContent, etc.). That breaks Radix’s shared tooltip behavior and prevents consumers from tuning timing at the app level. Please drop this inner provider so the exportedTooltipProviderremains the single context.function Tooltip({ ...props }: React.ComponentProps<typeof TooltipPrimitive.Root>) { - return ( - <TooltipProvider> - <TooltipPrimitive.Root data-slot="tooltip" {...props} /> - </TooltipProvider> - ) + return <TooltipPrimitive.Root data-slot="tooltip" {...props} /> }apps/web/app/components/ui/slider.tsx (1)
16-24: Fix fallback thumb count to match Radix state.When neither
valuenordefaultValueis provided,_valuesbecomes[min, max], so you render two thumbs while Radix only tracks a single value[min]. The second thumb is stuck/out of sync. Align the fallback to a single value so the thumb count matches Radix’s managed state.const _values = React.useMemo( () => Array.isArray(value) ? value : Array.isArray(defaultValue) ? defaultValue - : [min, max], - [value, defaultValue, min, max], + : [min], + [value, defaultValue, min], )apps/web/app/api/billing/invoice/route.ts (1)
1-9: CRITICAL: Authentication and PII issues remain unaddressed.The previously flagged critical and major security issues are still present:
- No authentication check allows unauthorized invoice downloads
- Hardcoded sensitive data (amount 4820.00 INR) in placeholder
Please address the authentication and PII concerns from the previous review.
apps/web/app/api/account/delete/route.ts (1)
3-7: CRITICAL: Authentication issue remains unaddressed.The previously flagged critical security issue is still present: this endpoint has no authentication or authorization checks, allowing any request to receive a success response. Please address the authentication concerns from the previous review before implementing real deletion logic.
apps/web/app/api/ai/mcp-config/route.ts (2)
10-12: CRITICAL: Authentication issue remains unaddressed.The previously flagged critical security issue is still present: this GET endpoint returns sensitive configuration (including API keys) without any authentication. Please address the authentication concerns from the previous review.
14-24: CRITICAL: Multiple security issues remain unaddressed.The previously flagged critical and major security issues are still present:
- No authentication on PUT endpoint
- No input validation for URL and API key fields
- API key exposure in response (line 23)
Please address these concerns from the previous review.
apps/web/app/api/billing/route.ts (1)
19-58: CRITICAL: Authentication and PII issues remain unaddressed.The previously flagged critical security issue is still present: this endpoint exposes billing data and hardcoded PII (line 48: "[email protected]") without any authentication. Please address the authentication and data sanitization concerns from the previous review.
apps/web/app/api/account/password/route.ts (1)
3-9: Authentication and persistence are missingLine 3 currently lets any caller POST arbitrary data, never checks the authenticated user, never verifies
current, and still returns{ ok: true }, so it both advertises a successful change and leaves the real password untouched. Please block unauthenticated access withrequireAuth, validate both fields, comparecurrentagainst the stored hash, and persist the hashednextpassword before responding. The diff below sketches the intended flow—wire it up to the existing auth utilities in this repo.-import { NextResponse } from "next/server"; - -export async function POST(req: Request) { - // This is a placeholder implementation; validate and change password in your auth system here - const { current, next } = await req.json(); - if (!next || next.length < 8) { - return NextResponse.json({ ok: false, error: "Password too short" }, { status: 400 }); - } - return NextResponse.json({ ok: true }); +import { NextResponse } from "next/server"; +import { requireAuth } from "@/lib/auth"; +import { prisma } from "@/lib/prisma"; +import { hashPassword, verifyPassword } from "@/lib/jwt"; + +export async function POST(req: Request) { + const session = await requireAuth(req); + const { current, next } = await req.json(); + + if (typeof current !== "string" || !current.length) { + return NextResponse.json({ ok: false, error: "Current password required" }, { status: 400 }); + } + if (typeof next !== "string" || next.length < 8) { + return NextResponse.json({ ok: false, error: "Password too short" }, { status: 400 }); + } + + const user = await prisma.user.findUnique({ where: { id: session.userId } }); + if (!user || !(await verifyPassword(current, user.password))) { + return NextResponse.json({ ok: false, error: "Invalid current password" }, { status: 401 }); + } + + await prisma.user.update({ + where: { id: user.id }, + data: { password: await hashPassword(next) }, + }); + + return NextResponse.json({ ok: true });Replace
verifyPasswordwith whichever password-compare helper this codebase already exposes.apps/web/app/api/auth/login/route.ts (2)
32-53: Stop leaking which emails exist.Returning 404 for unknown users while invalid passwords get a 401 lets an attacker enumerate valid accounts just by watching status codes. OWASP guidance is clear: authentication flows must always respond with the same generic error when credentials fail so the caller cannot learn whether a username exists.(cheatsheetseries.owasp.org)
Please collapse the “user not found” and “password missing” branches into the same 401 Invalid Credentials response so every failure path looks identical.
- if (!user) { - return NextResponse.json( - createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'), - { status: 404 } - ); - } - - // Verify password - if (!user.password) { + if (!user || !user.password) { return NextResponse.json( createErrorResponse(401, ErrorCodes.INVALID_CREDENTIALS, 'Invalid credentials'), { status: 401 } ); }
55-66: Respect the role stored in the database.Both access and refresh tokens hardcode
role: 'USER', so an admin will authenticate as an ordinary user. That breaks role-based authorization and creates escalation vectors if downstream services assume the token’s claim is authoritative. Authorization best practice is to propagate the actual role that was persisted for the account.(cheatsheetseries.owasp.org)Use the value from
user.role(with a safe fallback) instead of a literal.- role: 'USER', + role: user.role ?? 'USER',apps/web/app/api/workspaces/[id]/metrics/route.ts (1)
4-16: CRITICAL: Missing authentication and authorization.This endpoint exposes workspace metrics without any authentication or authorization checks. Any user can access sensitive operational data (CPU, memory, disk, network usage) for any workspace by guessing workspace IDs. This is a significant security vulnerability.
As noted in the previous review, add authentication using
requireAuthand verify workspace ownership before returning metrics. The suggested fix in the past review comment provides the complete solution.apps/web/app/api/ai/agents/route.ts (2)
12-14: Require authentication before returning agent inventory.Reiterating the earlier review: this handler still exposes the agent list without any auth check, so anyone can enumerate internal agents. Gate it with the same
requireAuthflow used across the new APIs before responding.-import { NextResponse } from "next/server"; +import { NextRequest, NextResponse } from "next/server"; +import { requireAuth } from "@/lib/auth"; … -export async function GET() { - return NextResponse.json({ agents }); -} +export async function GET(request: NextRequest) { + await requireAuth(request); + return NextResponse.json({ agents }); +}
16-26: Authenticate and validate agent mutations.POST still skips auth and accepts arbitrary
{ id, action }, so unauthenticated callers can flip statuses or send nonsense without feedback. Please require auth, validate the payload, and return 400/404 for bad requests before mutating state.-export async function POST(req: Request) { - const { id, action } = await req.json(); +export async function POST(request: NextRequest) { + await requireAuth(request); + const { id, action } = await request.json(); + if (typeof id !== "string" || (action !== "connect" && action !== "disconnect")) { + return NextResponse.json({ error: "Invalid request" }, { status: 400 }); + } + const agentExists = agents.some((a) => a.id === id); + if (!agentExists) { + return NextResponse.json({ error: "Agent not found" }, { status: 404 }); + } agents = agents.map((a) => a.id === id ? { ...a, status: action === "connect" ? "connected" : action === "disconnect" ? "disconnected" : a.status, } : a ); return NextResponse.json({ ok: true, agents }); }
| // Store reset token in database | ||
| // Note: You'll need to add these fields to your User model or create a separate PasswordReset model | ||
| await prisma.user.update({ | ||
| where: { id: user.id }, | ||
| data: { | ||
| // Add these fields to your schema: | ||
| // resetToken: String? | ||
| // resetTokenExpiry: DateTime? | ||
| // resetToken, | ||
| // resetTokenExpiry, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Non-functional database update.
The Prisma update call has an empty data object with all fields commented out. This means the generated reset token and expiry (lines 41-42) are never stored in the database, making the password reset flow non-functional.
Either:
- Add the
resetTokenandresetTokenExpiryfields to your User schema and uncomment lines 52-53 - Create a separate PasswordReset table as suggested in the comment
- Remove this update call entirely until the schema is ready
Calling an update with no data changes is wasteful and creates a false impression that the implementation is complete.
🤖 Prompt for AI Agents
In apps/web/app/api/auth/forgot-password/route.ts around lines 44 to 55 the
Prisma update call runs with an empty data object (all fields commented out), so
the generated resetToken and resetTokenExpiry are never stored and the password
reset flow is non-functional; fix by either (A) add resetToken: String? and
resetTokenExpiry: DateTime? to the User schema and update the call to set
resetToken and resetTokenExpiry to the generated values, (B) implement a
separate PasswordReset model and replace this update with a create call that
stores userId, token, and expiry, or (C) remove the no-op update until the
schema/model is implemented to avoid misleading, and ensure whichever option you
choose also updates any types/imports and runs prisma migrate when adding schema
fields.
| export async function GET(request: NextRequest) { | ||
| try { | ||
| // Verify authentication | ||
| const payload = await requireAuth(request); | ||
|
|
||
| // Fetch user | ||
| const user = await prisma.user.findUnique({ | ||
| where: { id: payload.userId }, | ||
| select: { | ||
| id: true, | ||
| email: true, | ||
| name: true, | ||
| image: true, | ||
| emailVerified: true, | ||
| createdAt: true, | ||
| updatedAt: true, | ||
| }, | ||
| }); | ||
|
|
||
| if (!user) { | ||
| return NextResponse.json( | ||
| createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'), | ||
| { status: 404 } | ||
| ); | ||
| } | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| data: user, | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| const { response, status } = handleAPIError(error); | ||
| return NextResponse.json(response, { status }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate endpoint: /api/auth/me vs /api/users/me.
This route duplicates the functionality of /api/users/me (shown in the relevant code snippets). Both endpoints fetch and return the same user profile data with identical logic.
Consider:
- Keeping only one endpoint (typically
/api/users/meis more RESTful for user resources) - If both are needed for backward compatibility, have one redirect to the other
- Document why both endpoints exist if there's a specific reason
Having duplicate endpoints increases maintenance burden and can lead to inconsistencies.
🤖 Prompt for AI Agents
In apps/web/app/api/auth/me/route.ts around lines 11 to 46, this endpoint
duplicates the logic of /api/users/me; either remove this handler and route
consumers to the canonical /api/users/me, or implement a lightweight
redirect/forward to /api/users/me for backward compatibility, and add a short
code comment and README entry documenting the chosen behavior; ensure any
auth/response shapes remain identical if delegating so clients are not broken.
| const user = await prisma.user.findFirst({ | ||
| where: { | ||
| // resetToken: token, | ||
| // resetTokenExpiry: { | ||
| // gte: new Date(), | ||
| // }, | ||
| }, | ||
| }); | ||
|
|
||
| if (!user) { | ||
| return NextResponse.json( | ||
| createErrorResponse(400, ErrorCodes.TOKEN_INVALID, 'Invalid or expired reset token'), | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // Hash new password | ||
| const hashedPassword = await hashPassword(password); | ||
|
|
||
| // Update password and clear reset token | ||
| await prisma.user.update({ | ||
| where: { id: user.id }, | ||
| data: { | ||
| password: hashedPassword, | ||
| // resetToken: null, | ||
| // resetTokenExpiry: null, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Reset token is ignored
Line 38 fetches a user with an empty where {} clause, so the first user in the table is returned even when the provided token is missing or invalid. The subsequent update at Line 58 will therefore reset the wrong account every time. Reinstate the token filters before merging—store the reset token (and expiry) on the user record and query against them so only valid tokens succeed.
- const user = await prisma.user.findFirst({
- where: {
- // resetToken: token,
- // resetTokenExpiry: {
- // gte: new Date(),
- // },
- },
- });
+ const user = await prisma.user.findFirst({
+ where: {
+ resetToken: token,
+ resetTokenExpiry: {
+ gte: new Date(),
+ },
+ },
+ });Be sure to add the corresponding resetToken fields to the Prisma schema before enabling this route.
📝 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.
| const user = await prisma.user.findFirst({ | |
| where: { | |
| // resetToken: token, | |
| // resetTokenExpiry: { | |
| // gte: new Date(), | |
| // }, | |
| }, | |
| }); | |
| if (!user) { | |
| return NextResponse.json( | |
| createErrorResponse(400, ErrorCodes.TOKEN_INVALID, 'Invalid or expired reset token'), | |
| { status: 400 } | |
| ); | |
| } | |
| // Hash new password | |
| const hashedPassword = await hashPassword(password); | |
| // Update password and clear reset token | |
| await prisma.user.update({ | |
| where: { id: user.id }, | |
| data: { | |
| password: hashedPassword, | |
| // resetToken: null, | |
| // resetTokenExpiry: null, | |
| }, | |
| }); | |
| const user = await prisma.user.findFirst({ | |
| where: { | |
| resetToken: token, | |
| resetTokenExpiry: { | |
| gte: new Date(), | |
| }, | |
| }, | |
| }); | |
| if (!user) { | |
| return NextResponse.json( | |
| createErrorResponse(400, ErrorCodes.TOKEN_INVALID, 'Invalid or expired reset token'), | |
| { status: 400 } | |
| ); | |
| } | |
| // Hash new password | |
| const hashedPassword = await hashPassword(password); | |
| // Update password and clear reset token | |
| await prisma.user.update({ | |
| where: { id: user.id }, | |
| data: { | |
| password: hashedPassword, | |
| // resetToken: null, | |
| // resetTokenExpiry: null, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/app/api/auth/reset-password/route.ts around lines 38 to 66, the user
lookup uses an empty where clause so the first user is returned and the wrong
account is updated; restore the reset token checks by querying
prisma.user.findFirst({ where: { resetToken: token, resetTokenExpiry: { gte: new
Date() } } }) so only a matching, not-yet-expired token succeeds, and when
updating the password clear resetToken and resetTokenExpiry (set to null) in the
update call; also ensure the Prisma schema includes resetToken and
resetTokenExpiry fields before enabling this route.
| export async function GET(req: Request) { | ||
| const { searchParams } = new URL(req.url); | ||
| const range = (searchParams.get("range") as Range) || "last_7d"; | ||
|
|
||
| const points = range === "last_24h" ? 24 : range === "last_7d" ? 7 * 24 : 30 * 24; | ||
|
|
||
| const activeUsers = jitter(1280, 0.12); | ||
| const builds = jitter(420, 0.2); | ||
| const errors = jitter(18, 0.4); | ||
| const cpu = Math.min(100, Math.max(3, Math.round(30 + rnd() * 50))); | ||
| const memory = Math.min(100, Math.max(8, Math.round(40 + rnd() * 40))); | ||
| const network = jitter(320, 0.25); // Mbps | ||
|
|
||
| const topProjects = [ | ||
| { name: "ai-search-service", usage: jitter(34, 0.3) }, | ||
| { name: "web-frontend", usage: jitter(28, 0.3) }, | ||
| { name: "worker-queue", usage: jitter(22, 0.3) }, | ||
| { name: "analytics-pipeline", usage: jitter(16, 0.3) }, | ||
| ]; | ||
|
|
||
| const timeseries = { | ||
| cpu: makeTimeseries(points, 55, 0.25), | ||
| mem: makeTimeseries(points, 60, 0.2), | ||
| net: makeTimeseries(points, 300, 0.35), | ||
| builds: makeTimeseries(points, 18, 0.5), | ||
| errors: makeTimeseries(points, 1.2, 0.8), | ||
| }; | ||
|
|
||
| return NextResponse.json({ | ||
| range, | ||
| summary: { activeUsers, builds, errors, cpu, memory, network }, | ||
| topProjects, | ||
| timeseries, | ||
| updatedAt: Date.now(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify authentication requirements for this endpoint.
The handler generates synthetic reporting data without authentication or authorization checks. Confirm whether this endpoint should be publicly accessible or requires authentication.
Run the following script to check if other reporting/API routes implement authentication:
🏁 Script executed:
#!/bin/bash
# Check if other API routes in this directory use auth middleware or session checks
rg -n -C3 --type=ts 'getServerSession|auth|authenticate|requireAuth' apps/web/app/api/Length of output: 38132
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check the full reporting route file for any comments indicating public/demo access
cat apps/web/app/api/reporting/route.tsLength of output: 1954
Add authentication to the reporting endpoint.
This endpoint returns sensitive application metrics without requiring authentication. All similar data endpoints in the codebase (/api/users/me/usage, /api/teams/[id]/usage, /api/teams/[id]/activity) consistently use requireAuth(). Import and call requireAuth(req) at the start of the GET handler.
Additionally, the global seed mutation in rnd() (line 5) causes concurrent requests to interfere with each other's PRNG state. Fixing the authentication will allow you to add per-request seeding if needed.
| const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 100); | ||
| const offset = parseInt(searchParams.get('offset') || '0'); | ||
|
|
There was a problem hiding this comment.
Sanitize pagination params before handing them to Prisma.
parseInt returns NaN for junk input (and negative numbers slip through), so take/skip ends up as NaN or <0, which Prisma rejects with a 500 instead of the expected 4xx. Guard and clamp these values before calling the client.
- const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 100);
- const offset = parseInt(searchParams.get('offset') || '0');
+ const limitParam = parseInt(searchParams.get('limit') ?? '', 10);
+ const limit = Number.isNaN(limitParam) ? 50 : Math.min(Math.max(limitParam, 1), 100);
+ const offsetParam = parseInt(searchParams.get('offset') ?? '', 10);
+ const offset = Number.isNaN(offsetParam) || offsetParam < 0 ? 0 : offsetParam;📝 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.
| const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 100); | |
| const offset = parseInt(searchParams.get('offset') || '0'); | |
| const limitParam = parseInt(searchParams.get('limit') ?? '', 10); | |
| const limit = Number.isNaN(limitParam) ? 50 : Math.min(Math.max(limitParam, 1), 100); | |
| const offsetParam = parseInt(searchParams.get('offset') ?? '', 10); | |
| const offset = Number.isNaN(offsetParam) || offsetParam < 0 ? 0 : offsetParam; |
| @@ -0,0 +1,35 @@ | |||
| 'use client' | |||
|
|
|||
| import { useToast } from '@/hooks/use-toast' | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify the import path for useToast hook.
The import uses @/hooks/use-toast, but the relevant code snippets indicate the file is located at apps/web/app/components/ui/use-toast.ts. Please verify that this import path is correct and that the path alias @/hooks is properly configured in your build setup (e.g., tsconfig.json).
Run the following script to verify the actual location of the use-toast file:
🏁 Script executed:
#!/bin/bash
# Find the use-toast file location
fd -t f "use-toast.ts"Length of output: 39
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all use-toast files
echo "=== All use-toast files ==="
fd -t f "use-toast"
# Check tsconfig.json for path aliases
echo -e "\n=== Path aliases in tsconfig.json ==="
find . -maxdepth 3 -name "tsconfig.json" -type f | head -5 | xargs grep -A 10 "baseUrl\|paths" || echo "No path configuration found"
# Check if there's a hooks directory with use-toast
echo -e "\n=== Checking for hooks directory structure ==="
fd -t d "hooks" | head -10
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Find tsconfig files in the app structure
echo "=== Finding all tsconfig.json files ==="
find . -name "tsconfig.json" -type f
# Check for next.config in apps/web
echo -e "\n=== Checking for build config files ==="
find ./apps/web -maxdepth 2 -name "*.config.*" -o -name "tsconfig*.json"
# Search for imports using @/hooks pattern
echo -e "\n=== Searching for @/hooks imports in the codebase ==="
rg "@/hooks" -A 1 -B 1 | head -30
# Check the actual toaster.tsx file
echo -e "\n=== Content of toaster.tsx around the import ==="
fd -t f "toaster.tsx" -x cat {}Length of output: 1770
🏁 Script executed:
#!/bin/bash
# Check tsconfig.json for path aliases
echo "=== Content of apps/web/tsconfig.json ==="
cat ./apps/web/tsconfig.json
# Check if there's supposed to be a hooks directory
echo -e "\n=== Checking app directory structure ==="
ls -la ./apps/web/app/
# Look for any other use-* files that might be in @/hooks location
echo -e "\n=== Looking for use-* files in the codebase ==="
fd -t f "use-.*\.ts" | head -20Length of output: 1949
Fix the incorrect import path for the useToast hook.
The import path @/hooks/use-toast does not match the actual file location at apps/web/app/components/ui/use-toast.ts. Based on the tsconfig.json configuration where @/* resolves to ./* (apps/web root), the correct import should be @/app/components/ui/use-toast.
Change line 3 from:
import { useToast } from '@/hooks/use-toast'
to:
import { useToast } from '@/app/components/ui/use-toast'
🤖 Prompt for AI Agents
In apps/web/app/components/ui/toaster.tsx around line 3, the import path for
useToast is incorrect; update the import from "@/hooks/use-toast" to the correct
path "@/app/components/ui/use-toast" (matching the actual file at
apps/web/app/components/ui/use-toast.ts) so the module resolves correctly under
the project tsconfig path mapping.
|
|
||
| --- | ||
|
|
||
| **Date Fixed**: October 31, 2024 |
There was a problem hiding this comment.
Correct the date in the documentation.
The date shows "October 31, 2024" but should be "October 31, 2025" based on the current date.
Apply this diff:
-**Date Fixed**: October 31, 2024
+**Date Fixed**: October 31, 2025📝 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.
| **Date Fixed**: October 31, 2024 | |
| **Date Fixed**: October 31, 2025 |
🤖 Prompt for AI Agents
In apps/web/AUTHENTICATION_FIX.md around line 223, the documented "Date Fixed"
is wrong (shows October 31, 2024); update the date string to "October 31, 2025"
so the document reflects the correct current date.
|
|
||
| **Project:** Dev8 - Cloud Development Environment Platform | ||
| **Date:** October 28, 2025 | ||
| **Status:** ✅ ALL 40 ENDPOINTS COMPLETED & VERIFIED |
There was a problem hiding this comment.
Documentation overstates production readiness.
The documentation claims all endpoints are "COMPLETED & VERIFIED" (line 5), "DEPLOYMENT READY" (line 273), and "production-ready" (line 326). However, several API routes in this PR lack authentication guards, exposing sensitive operations (account deletion, billing data, API key configuration) to unauthorized access.
Consider updating the documentation to reflect the current implementation state and clearly note which endpoints still require authentication implementation before production deployment.
Also applies to: 275-286, 326-328
🤖 Prompt for AI Agents
In apps/web/BACKEND_API_COMPLETE_SUMMARY.md around lines 5, 275-286, and
326-328, the document overstates production readiness by claiming all endpoints
are "COMPLETED & VERIFIED", "DEPLOYMENT READY", and "production-ready" despite
several routes lacking authentication; update those lines to remove absolute
production claims, add a clear disclaimer that some endpoints still require
authentication before production, and insert a short, enumerated list or
reference to the specific routes (account deletion, billing, API key
configuration, etc.) that currently lack auth along with links to the
corresponding PRs or tickets and an expected owner/ETA for completing the auth
work. Ensure the summary reflects per-endpoint status (e.g., "implemented",
"needs auth", "needs tests") rather than a blanket completed label.
|
|
||
| ```bash | ||
| cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web" | ||
| pnpm dev | ||
| ``` |
There was a problem hiding this comment.
Remove hardcoded absolute path.
The guide contains a hardcoded Windows-specific path that won't work for other developers.
Apply this diff:
### Step 1: Start the Server
```bash
-cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web"
+# From repository root
+cd apps/web
pnpm dev
<details>
<summary>🤖 Prompt for AI Agents</summary>
In apps/web/QUICK_TEST_GUIDE.md around lines 10 to 14, the quick-start shows a
hardcoded Windows absolute path; replace it with a relative repository-root
instruction (e.g., "From repository root: cd apps/web") so the guide works
cross-platform and for other contributors, then run the existing "pnpm dev" step
unchanged.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| #### Option A: Automated Test Script ⚡ (Recommended) | ||
|
|
||
| ```bash | ||
| cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web" | ||
| ./test-all-apis.sh | ||
| ``` |
There was a problem hiding this comment.
Remove hardcoded absolute path.
Same hardcoded path issue as in Step 1.
Apply this diff:
#### Option A: Automated Test Script ⚡ (Recommended)
```bash
-cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web"
+# From repository root
+cd apps/web
./test-all-apis.sh
<details>
<summary>🤖 Prompt for AI Agents</summary>
In apps/web/QUICK_TEST_GUIDE.md around lines 99 to 104, replace the hardcoded
absolute Windows path with a relative path from the repository root; change the
cd command to "cd apps/web" (optionally include a short comment like "# From
repository root" above it) and keep the subsequent "./test-all-apis.sh"
invocation unchanged so the guide works for other developers and CI.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
ritesh301
force-pushed
the
backend-code
branch
from
da8b965 to
9df47ed
Compare
2 participants
🚀 Pull Request
📋 Description
Brief description of what this PR does.
🎯 Type of Change
🔗 Related Issue
Fixes #(issue number)
🧪 Testing
📸 Screenshots/Videos
If applicable, add screenshots or videos demonstrating the changes.
📋 Checklist
🌍 Environment Tested
📝 Additional Notes
Any additional information that reviewers should know.
Summary by CodeRabbit
New Features
Documentation
UI/Style