Frontend changes - #77
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Updated backend URL to https://dev8-dev.onrender.com - Fixed transaction timeout issues (5s -> 120s) - Fixed quota check to exclude deleted workspaces - Added dynamic VSCode URL integration - Fixed VSCode URL connection timeout by stripping :8080 port - Azure Container Apps ingress only supports standard HTTPS (443) - Removed Browser button from workspace UI - All 4 workspace APIs tested and working (CREATE, START, STOP, DELETE) - Region configured to Central India (centralindia) - Health check now accepts degraded (503) status
WalkthroughThis PR introduces comprehensive documentation for API verification and integration status, unified authentication supporting NextAuth sessions and JWT Bearer tokens, 100+ new UI components wrapping Radix UI primitives and other libraries, extensive API route implementations for workspaces, teams, users, billing, and AI agents, authentication and workspace management pages, and foundational state/utility modules for demo functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Frontend as Frontend<br/>(Next.js)
participant AuthAPI as Auth<br/>API Routes
participant WorkspaceAPI as Workspace<br/>API Routes
participant AgentAPI as Agent API<br/>(Go Backend)
participant DB as Database
rect rgb(200, 220, 255)
Note over Browser,AuthAPI: Authentication Flow
Browser->>Frontend: Sign In / Sign Up
Frontend->>AuthAPI: POST /api/auth/* or<br/>NextAuth Session
AuthAPI->>AuthAPI: getAuthUser() checks<br/>Session then JWT
AuthAPI->>DB: Validate credentials
DB-->>AuthAPI: User confirmed
AuthAPI-->>Frontend: Auth token + Session
Frontend-->>Browser: Redirect /dashboard
end
rect rgb(220, 255, 220)
Note over Browser,AgentAPI: Workspace Provisioning
Browser->>Frontend: Create Workspace
Frontend->>WorkspaceAPI: POST /api/workspaces<br/>with payload<br/>(name, region, size...)
WorkspaceAPI->>DB: Create Environment<br/>(STOPPED status)
WorkspaceAPI->>WorkspaceAPI: isAgentIntegrationEnabled()?
alt Agent Enabled
WorkspaceAPI->>AgentAPI: POST /provision<br/>with camelCase payload<br/>(cpuCores, memoryGB...)<br/>[300s timeout]
AgentAPI->>AgentAPI: Health check<br/>[8s, accepts 200/503]
AgentAPI-->>WorkspaceAPI: Container created<br/>+ URLs (VSCode, SSH)
WorkspaceAPI->>DB: Update Environment<br/>(RUNNING, URLs, metadata)
else Agent Disabled/Failed
WorkspaceAPI->>DB: Keep created record<br/>(agentProvisioned: false)
end
WorkspaceAPI-->>Frontend: { ok, environment,<br/>agentProvisioned }
Frontend-->>Browser: Show workspace dashboard
end
rect rgb(255, 230, 200)
Note over Browser,AgentAPI: Workspace Actions
Browser->>Frontend: Start/Stop/Pause
Frontend->>WorkspaceAPI: POST /api/workspaces/[id]/start<br/>(or stop/pause)
WorkspaceAPI->>DB: Load Environment
WorkspaceAPI->>WorkspaceAPI: Validate ownership<br/>& state transitions
alt Action Permitted
WorkspaceAPI->>AgentAPI: POST /action<br/>{ action: START/STOP/PAUSE }<br/>[90s timeout for actions]
AgentAPI-->>WorkspaceAPI: Status updated
WorkspaceAPI->>DB: Update Environment status
else Validation Failed
WorkspaceAPI-->>Frontend: 400 Validation Error
end
WorkspaceAPI-->>Frontend: { ok, environment, message }
Frontend-->>Browser: Show updated status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring focused review:
Possibly related PRs
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}
| const accounts = await prisma.account.findMany({ | ||
| where: { userId: session.user.id }, | ||
| select: { provider: true }, | ||
| }); | ||
| connected = new Set(accounts.map((a) => a.provider.toLowerCase())); |
There was a problem hiding this comment.
Resolve TS7006 (a implicitly has an any type) in accounts.map
The callback parameter in accounts.map((a) => a.provider.toLowerCase()) is triggering TS7006 under your current compiler settings. Add an explicit type (or cast the accounts array) so the callback parameter is strongly typed, e.g.:
- const accounts = await prisma.account.findMany({
- where: { userId: session.user.id },
- select: { provider: true },
- });
- connected = new Set(accounts.map((a) => a.provider.toLowerCase()));
+ const accounts = await prisma.account.findMany({
+ where: { userId: session.user.id },
+ select: { provider: true },
+ });
+ connected = new Set(
+ accounts.map((account: { provider: string }) =>
+ account.provider.toLowerCase(),
+ ),
+ );This should clear the CI TypeScript error while keeping the behavior unchanged.
📝 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 accounts = await prisma.account.findMany({ | |
| where: { userId: session.user.id }, | |
| select: { provider: true }, | |
| }); | |
| connected = new Set(accounts.map((a) => a.provider.toLowerCase())); | |
| const accounts = await prisma.account.findMany({ | |
| where: { userId: session.user.id }, | |
| select: { provider: true }, | |
| }); | |
| connected = new Set( | |
| accounts.map((account: { provider: string }) => | |
| account.provider.toLowerCase(), | |
| ), | |
| ); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 24-24: TypeScript error TS7006: Parameter 'a' implicitly has an 'any' type.
🤖 Prompt for AI Agents
In apps/web/app/api/account/connections/route.ts around lines 20 to 24, the
callback parameter in accounts.map is implicitly typed (TS7006); fix by giving
the callback an explicit type or by typing the accounts variable: either
annotate the result of prisma.account.findMany as an array with a provider
string (e.g., const accounts: { provider: string }[] = await ...) or change the
map callback to accounts.map((a: { provider: string }) =>
a.provider.toLowerCase()); this will remove the implicit any error without
changing runtime behavior.
| 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: Missing authentication and error handling.
This endpoint is missing authentication checks, allowing any unauthenticated user to call it. Additionally, there's no error handling for potential failures.
Apply this diff to add authentication and error handling:
+import { NextRequest } from "next/server";
+import { requireAuth } from "@/lib/auth";
+import { handleAPIError } from "@/lib/errors";
-export async function POST() {
+export async function POST(request: NextRequest) {
+ try {
+ await requireAuth(request);
+
// 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 });
+ } catch (error) {
+ return handleAPIError(error);
+ }
}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 endpoint
currently lacks authentication and error handling; update it to: verify the
caller is authenticated (e.g., validate session/token and fetch the user id) and
return 401 if unauthenticated, then wrap the user-deletion logic in a try/catch
so failures are caught, log the error, and return an appropriate error response
(500 for server errors or a descriptive status/message). Ensure success returns
a 200/204 only after the real deletion completes and never expose internal error
details in client responses.
| export async function POST(req: Request) { | ||
| // This is a placeholder implementation; validate and change password in your auth system here | ||
| const { 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.
🧩 Analysis chain
Critical: Missing authentication and misleading parameter name.
This endpoint has several critical issues:
-
Security vulnerability: No authentication check allows anyone to call this endpoint. Password change routes must use
requireAuth(as seen in the complete implementation atapps/web/app/api/auth/change-password/route.ts). -
Misleading parameter name: The field
nextis confusing for a password parameter. It should be namednewPasswordor similar. -
Duplicate endpoint: There's already a complete, secure implementation at
/api/auth/change-password/route.ts(lines 12-77) that includes authentication, password strength validation, current password verification, and database updates.
Consider whether this placeholder endpoint is necessary. If it is, apply authentication and proper validation. Otherwise, remove this file and use the existing /api/auth/change-password route.
🏁 Script executed:
#!/bin/bash
# Description: Check if this placeholder endpoint is actually used in the codebase
# Search for references to /api/account/password
rg -n --type ts --type tsx --type js --type jsx '/api/account/password' -g '!route.ts'
# Also check for any fetch/axios calls to this endpoint
rg -n --type ts --type tsx --type js --type jsx 'account/password' -C 3Length of output: 55
🏁 Script executed:
#!/bin/bash
# Description: Check if /api/account/password endpoint is actually used in the codebase
# Search for references to /api/account/password (without type restrictions)
echo "=== Searching for references to /api/account/password ==="
rg -n '/api/account/password' -g '!route.ts'
# Search for fetch/axios calls to account/password
echo -e "\n=== Searching for account/password references ==="
rg -n 'account/password' -C 2
# Check what files exist in the api/account directory
echo -e "\n=== Contents of apps/web/app/api/account directory ==="
find apps/web/app/api/account -type f -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -20
# Search for any imports or usage of the password route handler
echo -e "\n=== Searching for password route imports ==="
rg -n 'from.*account/password|import.*account/password'Length of output: 864
🏁 Script executed:
#!/bin/bash
# Description: Get more context on the usage of /api/account/password endpoint
# View the full change-password page to see the request payload
echo "=== Full context of change-password/page.tsx (around line 21) ==="
cat apps/web/app/settings/change-password/page.tsx
# Check if there's any authentication middleware on the route
echo -e "\n=== Check for middleware in apps/web/app/api/account/password/route.ts ==="
cat apps/web/app/api/account/password/route.ts
# Check for any middleware configuration at the api level
echo -e "\n=== Check for middleware.ts or auth configuration ==="
find apps/web -name "middleware.ts" -o -name "middleware.js" | head -10Length of output: 3147
🏁 Script executed:
#!/bin/bash
# Description: Check middleware to see if authentication is handled
echo "=== Content of apps/web/middleware.ts ==="
cat apps/web/middleware.ts
# Check if there are any route-specific auth handlers
echo -e "\n=== Search for requireAuth usage in api routes ==="
rg -n 'requireAuth' apps/web/app/api/ -A 2Length of output: 18498
Add authentication check to password endpoint.
This endpoint lacks authentication and will allow any unauthenticated user to call it. Unlike the page accessing it (which may be protected), API routes are explicitly excluded from middleware protection (see matcher config in middleware.ts). All other protected API endpoints use requireAuth() at the start of their handlers.
Additionally, the handler extracts only the next parameter and ignores current (sent from the client), which is incorrect for password change logic.
Required fixes at apps/web/app/api/account/password/route.ts:
- Import and call
requireAuth(req)as the first operation - Handle both
currentandnextparameters properly - Consider consolidating with the complete implementation at
/api/auth/change-password/route.tsif this duplicate endpoint is unnecessary
🤖 Prompt for AI Agents
In apps/web/app/api/account/password/route.ts around lines 3 to 10, the handler
is missing authentication and only reads the new password; update it to import
and immediately call requireAuth(req) as the first operation to enforce the same
protection used by other protected APIs, then parse both current and next from
req.json() and validate them (ensure next meets length/strength rules and
current is provided), verify the current password with your auth layer (or call
the shared change-password service used by /api/auth/change-password/route.ts),
perform the password update only after successful verification, and return clear
JSON responses and appropriate HTTP status codes on success/failure; if this
endpoint is redundant, consolidate by routing calls to or reusing the
implementation at /api/auth/change-password/route.ts instead of duplicating
logic.
| export async function GET() { | ||
| const content = `Invoice\nPlan: Pro Developer Plan\nAmount: 4820.00 INR\nDate: ${new Date().toISOString()}\n(This is a placeholder invoice. Replace with PDF generation.)`; | ||
| return new Response(content, { | ||
| headers: { | ||
| "Content-Type": "text/plain", | ||
| "Content-Disposition": `attachment; filename=invoice-${new Date().toISOString().slice(0, 7)}.txt`, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
CRITICAL: Missing authentication for sensitive billing data.
This endpoint exposes invoice data without authentication checks, allowing any user to access billing information.
Apply this diff to add authentication and error handling:
+import { NextRequest } from "next/server";
+import { requireAuth } from "@/lib/auth";
+import { handleAPIError } from "@/lib/errors";
+
-export async function GET() {
+export async function GET(request: NextRequest) {
+ try {
+ await requireAuth(request);
+
const content = `Invoice\nPlan: Pro Developer Plan\nAmount: 4820.00 INR\nDate: ${new Date().toISOString()}\n(This is a placeholder invoice. Replace with PDF generation.)`;
return new Response(content, {
headers: {
"Content-Type": "text/plain",
"Content-Disposition": `attachment; filename=invoice-${new Date().toISOString().slice(0, 7)}.txt`,
},
});
+ } catch (error) {
+ return handleAPIError(error);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/billing/invoice/route.ts around lines 1 to 9, the GET
handler returns invoice data with no authentication or error handling; add an
authentication check that validates the request/session (e.g., verify JWT or
NextAuth session) and return 401/403 if the user is not
authenticated/authorized, wrap the invoice generation and response creation in
try/catch to return a 500 on unexpected errors and log them (without leaking
sensitive details), and ensure the Content-Disposition filename is safely
derived (sanitize or use a fixed pattern) so only authorized users receive the
invoice response.
| plan: "Pro Developer Plan", | ||
| accountEmail: "ritesh@cloudidex.com", | ||
| payment: "Visa **** 4872", | ||
| nextInvoice: new Date(today.getFullYear(), today.getMonth() + 1, 1).toLocaleDateString("en-US", { | ||
| month: "short", | ||
| day: "numeric", | ||
| year: "numeric", | ||
| }), | ||
| }, |
There was a problem hiding this comment.
Avoid hard-coded account identity details in a shared billing endpoint.
accountEmail and payment are hard-coded to a specific-looking user/card, which won’t match the authenticated user and may be undesirable from a PII/compliance perspective if this ever runs outside a local demo. Consider either deriving these fields from the logged-in account or clearly marking and scoping this route as mock/demo-only with generic placeholder values.
🤖 Prompt for AI Agents
In apps/web/app/api/billing/route.ts around lines 47 to 55, the billing response
contains hard-coded PII-like fields (accountEmail and payment); replace these
with either values derived from the authenticated user (pull email and masked
payment info from the current session/account service) or use generic non-PII
placeholders and clearly mark this route as mock/demo-only (e.g., comments and a
guard/env flag) so no real user data is returned in shared or production
environments.
| @@ -0,0 +1,1602 @@ | |||
| # 🚀 Complete Postman Testing Guide for Dev8 Backend APIs | |||
|
|
|||
| **Date:** October 28, 2025 | |||
There was a problem hiding this comment.
Update the documentation date for accuracy.
The guide shows "October 28, 2025" but the PR was created on November 20, 2025. Please update to reflect the correct date.
-**Date:** October 28, 2025
+**Date:** November 20, 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:** October 28, 2025 | |
| **Date:** November 20, 2025 |
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...ev8 Backend APIs Date: October 28, 2025 Total APIs: 40 Endpoints **Base...
(MISSING_COMMA_AFTER_YEAR)
🤖 Prompt for AI Agents
In apps/web/POSTMAN_TESTING_GUIDE.md around line 3, the document header date is
incorrect (shows "October 28, 2025"); update that date to "November 20, 2025" to
match the PR creation date and any other relevant metadata fields in the file so
the guide reflects the correct publication date.
| ### Step 1: Start the Server | ||
|
|
||
| ```bash | ||
| cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web" |
There was a problem hiding this comment.
Replace hard-coded Windows paths with portable placeholders.
The same hard-coded path issue exists here. Replace with portable placeholders or relative path instructions for cross-platform compatibility.
-cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web"
+# Navigate to the apps/web directory from your project root
+cd apps/webAlso applies to: 102-102
🤖 Prompt for AI Agents
In apps/web/QUICK_TEST_GUIDE.md around lines 12 and 102, the guide includes a
hard-coded Windows absolute path ("c:/Users/.../apps/web"); replace these with a
portable placeholder or a relative path example such as ./apps/web or
<project-root>/apps/web and add a short note showing how to cd into the project
cross-platform (e.g., using quotes only if necessary) so the instruction works
on Windows, macOS, and Linux.
| @@ -0,0 +1,1930 @@ | |||
| # 🧪 Complete Application Testing Guide | |||
|
|
|||
| **Date:** October 28, 2025 | |||
There was a problem hiding this comment.
Verify the date - appears to be in the future.
The date shows "October 28, 2025" which is in the future. This is likely a typo and should be corrected to reflect the actual testing date.
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...on Testing Guide Date: October 28, 2025 Branch: backend-code **Applicat...
(MISSING_COMMA_AFTER_YEAR)
🤖 Prompt for AI Agents
In apps/web/TESTING_GUIDE.md at line 3, the header date is set to "October 28,
2025" which is in the future; update that date to the correct/testing date
(e.g., today's actual date or the intended past date), ensuring the file header
reflects the real testing date and commit the change.
| ### Step 2: Verify Database Schema | ||
|
|
||
| ```bash | ||
| cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web" |
There was a problem hiding this comment.
Replace hard-coded Windows paths with portable placeholders.
Multiple commands contain hard-coded Windows-style paths with a specific username (c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web). This makes the documentation non-portable and will fail for other users.
Replace with a portable placeholder:
-cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web"
+cd /path/to/Dev8.dev/apps/webOr use a relative path instruction:
-cd "c:/Users/RITESH PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web"
+# Navigate to your Dev8.dev/apps/web directory
+cd apps/webAlso applies to: 56-56, 102-102, 572-572, 1573-1573
🤖 Prompt for AI Agents
In apps/web/TESTING_GUIDE.md around lines 44, 56, 102, 572, and 1573 replace the
hard-coded Windows absolute path "c:/Users/RITESH
PRADHAN/OneDrive/Desktop/FINAL_YEAR_PROJECT/Dev8.dev/apps/web" with a portable
placeholder or relative path; update each command to use something like
"<PROJECT_ROOT>/apps/web" or "cd ../apps/web" (or instruct the reader to run
commands from the project root) so the documentation works across different
systems and user accounts.
| ### Validation Schema (lib/validations.ts) | ||
|
|
||
| The backend validation requires: | ||
| ```typescript | ||
| export const createWorkspaceSchema = z.object({ | ||
| name: z.string().min(1).max(100), | ||
| cloudRegion: z.string().min(1), // Required | ||
| cpuCores: z.number().min(1).max(4), // Required | ||
| memoryGB: z.number().min(2).max(16), // Required | ||
| storageGB: z.number().min(10).max(100), // Required | ||
| baseImage: z.string().default('node'), // Required | ||
| // ... optional fields | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Align documented validation limits with size configuration (CPU cores).
The schema snippet documents cpuCores: z.number().min(1).max(4), while the size configuration later defines large as cpu: 8. As written, that payload would fail validation. Either adjust the documented schema (and actual backend, if needed) to allow 8 cores or update the size table/example so all sizes comply with the stated max(4) limit.
Also applies to: 147-156
🤖 Prompt for AI Agents
In apps/web/WORKSPACE_CREATION_FIX.md around lines 37 to 50 (and also apply same
fix to lines 147-156), the documented validation schema limits cpuCores to
max(4) but the sizes table defines a `large` size with cpu: 8 causing an
inconsistency; update the documentation and the backend validation to be
consistent by either lowering the `large` size to ≤4 in the size
configuration/examples or increasing the schema limit to max(8) (and change
actual backend zod schema to match), then run a quick validation test to ensure
payloads for all documented sizes pass.
| async function toggleAgent(agent: Agent) { | ||
| setSavingAgentId(agent.id); | ||
| try { | ||
| const res = await fetch("/api/ai/agents", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ id: agent.id, action: agent.status === "connected" ? "disconnect" : "connect" }), | ||
| }); | ||
| const data = await res.json(); | ||
| setAgents(data.agents); | ||
| } catch (e) { | ||
| console.error(e); | ||
| } finally { | ||
| setSavingAgentId(null); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard against failed agent updates to avoid crashing the agents list.
toggleAgent always does setAgents(data.agents) without checking res.ok or validating the shape. If the API returns an error payload without an agents array, agents becomes undefined, and the subsequent (loadingAgents ? ... : agents).map(...) will throw at runtime.
Harden this flow by checking the response and falling back to the previous state when data.agents is missing:
async function toggleAgent(agent: Agent) {
setSavingAgentId(agent.id);
try {
const res = await fetch("/api/ai/agents", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ id: agent.id, action: agent.status === "connected" ? "disconnect" : "connect" }),
});
- const data = await res.json();
- setAgents(data.agents);
+ if (!res.ok) {
+ console.error("Failed to update agent", res.status, res.statusText);
+ return;
+ }
+ const data = await res.json();
+ setAgents((prev) =>
+ Array.isArray((data as any).agents) ? (data as any).agents : prev
+ );
} catch (e) {
console.error(e);
} finally {
setSavingAgentId(null);
}
}Also applies to: 150-181
🤖 Prompt for AI Agents
In apps/web/app/ai-agents/page.tsx around lines 68-83 (and similarly 150-181),
the toggleAgent flow unconditionally calls setAgents(data.agents) which can set
agents to undefined if the API failed or returned a malformed payload; update
the code to first check res.ok, safely parse JSON (guarding against non-JSON
responses), verify that data.agents is an array (e.g.,
Array.isArray(data.agents)), and only call setAgents when that check passes; if
the response is not ok or data.agents is invalid, log the error and leave the
existing agents state unchanged (optionally show an error toast), and ensure
finally still clears setSavingAgentId.
| const validation = changePasswordSchema.safeParse(body); | ||
| if (!validation.success) { | ||
| return NextResponse.json( | ||
| createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, JSON.stringify(validation.error.issues)), | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Do not wrap createErrorResponse inside NextResponse.json
createErrorResponse already returns a Response; passing it into NextResponse.json nests a Response as JSON data and will break error responses. Return createErrorResponse directly instead.
Apply this kind of change in all error branches:
- if (!validation.success) {
- return NextResponse.json(
- createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, JSON.stringify(validation.error.issues)),
- { status: 400 }
- );
- }
+ if (!validation.success) {
+ return createErrorResponse(
+ 400,
+ ErrorCodes.VALIDATION_ERROR,
+ JSON.stringify(validation.error.issues),
+ );
+ }
@@
- if (!passwordValidation.valid) {
- return NextResponse.json(
- createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, passwordValidation.errors.join(', ')),
- { status: 400 }
- );
- }
+ if (!passwordValidation.valid) {
+ return createErrorResponse(
+ 400,
+ ErrorCodes.VALIDATION_ERROR,
+ passwordValidation.errors.join(', '),
+ );
+ }
@@
- if (!user || !user.password) {
- return NextResponse.json(
- createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'),
- { status: 404 }
- );
- }
+ if (!user || !user.password) {
+ return createErrorResponse(
+ 404,
+ ErrorCodes.USER_NOT_FOUND,
+ 'User not found',
+ );
+ }
@@
- const isValid = await verifyPassword(currentPassword, user.password);
- if (!isValid) {
- return NextResponse.json(
- createErrorResponse(401, ErrorCodes.INVALID_CREDENTIALS, 'Current password is incorrect'),
- { status: 401 }
- );
- }
+ const isValid = await verifyPassword(currentPassword, user.password);
+ if (!isValid) {
+ return createErrorResponse(
+ 401,
+ ErrorCodes.INVALID_CREDENTIALS,
+ 'Current password is incorrect',
+ );
+ }Also applies to: 33-38, 45-50, 53-59
🏁 Script executed:
#!/bin/bash
# Find createErrorResponse function definition to verify its return type
rg -nP "function createErrorResponse|const createErrorResponse|export.*createErrorResponse" --type=ts --type=tsx -A 10Length of output: 27
🏁 Script executed:
#!/bin/bash
# Inspect the target file and lines mentioned in the review
cat apps/web/app/api/auth/change-password/route.tsLength of output: 2273
🏁 Script executed:
#!/bin/bash
# Search for all usages of createErrorResponse wrapped in NextResponse.json
rg -nP "NextResponse\.json\s*\(\s*createErrorResponse" --type=ts --type=tsx -C 3Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find the errors module (createErrorResponse definition)
fd -e ts -e js "errors\." . | head -20Length of output: 24
🏁 Script executed:
#!/bin/bash
# Search for createErrorResponse definition without problematic file type
rg -nP "createErrorResponse" lib/errors --type ts -A 5 -B 2Length of output: 50
🏁 Script executed:
#!/bin/bash
# Alternative: find all files in lib/errors path
find . -path "*/lib/errors*" -type f | head -20Length of output: 24
🏁 Script executed:
#!/bin/bash
# Search for Response type returns related to createErrorResponse
rg -nP "createErrorResponse.*Response|function createErrorResponse" --type ts -A 8Length of output: 393
🏁 Script executed:
#!/bin/bash
# Get full createErrorResponse function to confirm complete return statement
cat apps/web/lib/errors.ts | sed -n '25,40p'Length of output: 227
Return createErrorResponse directly without wrapping it in NextResponse.json
Verification confirms the issue: createErrorResponse at apps/web/lib/errors.ts returns Response.json(..., { status: statusCode }), which is already a Response object. Wrapping it inside NextResponse.json(..., { status: 400 }) nests a Response as JSON data, breaking error responses.
All four error branches in the route require fixes:
- Line 21-27: validation error
- Line 33-38: password validation error
- Line 45-50: user not found (404)
- Line 53-59: invalid credentials (401)
Apply the suggested changes: return createErrorResponse(...) directly without wrapping it in NextResponse.json.
🤖 Prompt for AI Agents
In apps/web/app/api/auth/change-password/route.ts around lines 21-27, 33-38,
45-50 and 53-59 the route is wrapping createErrorResponse(...) inside
NextResponse.json(...), which nests an existing Response object as JSON; replace
each wrapped return with a direct return of createErrorResponse(...) so the
existing Response produced by createErrorResponse is returned unchanged (i.e.,
remove NextResponse.json(...) and return createErrorResponse(...) directly for
the validation error, password validation error, 404 user-not-found, and 401
invalid-credentials branches).
| 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. | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| message: 'Logged out successfully', | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| return handleAPIError(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove authentication requirement from logout endpoint.
Requiring authentication for logout is counterintuitive - if a user's token is invalid or expired, they cannot logout. Since JWT logout is client-side (as noted in the comments), this endpoint should succeed regardless of authentication state.
Apply this diff:
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.📝 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 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. | |
| return NextResponse.json({ | |
| success: true, | |
| message: 'Logged out successfully', | |
| }); | |
| } catch (error) { | |
| return handleAPIError(error); | |
| } | |
| } | |
| export async function POST(request: NextRequest) { | |
| try { | |
| // 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. | |
| return NextResponse.json({ | |
| success: true, | |
| message: 'Logged out successfully', | |
| }); | |
| } catch (error) { | |
| return handleAPIError(error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/auth/logout/route.ts around lines 10 to 27, the logout
endpoint currently calls requireAuth which blocks requests with missing/expired
tokens; remove that authentication requirement so the endpoint always returns a
successful logout response regardless of auth state. Specifically, delete the
await requireAuth(request) line and any logic that depends on it, keep the
optional comment about token blacklist/cleanup, and ensure the handler returns
NextResponse.json({ success: true, message: 'Logged out successfully' }) inside
try and still calls handleAPIError in catch so the endpoint succeeds even when
no valid token is present.
| if (!user) { | ||
| return NextResponse.json( | ||
| createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'), | ||
| { status: 404 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Fix incorrect response construction.
createErrorResponse already returns a Response.json() object, so wrapping it in NextResponse.json() creates a double-wrapped response.
Apply this diff to fix:
if (!user) {
- return NextResponse.json(
- createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'),
- { status: 404 }
- );
+ return createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found');
}📝 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.
| if (!user) { | |
| return NextResponse.json( | |
| createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'), | |
| { status: 404 } | |
| ); | |
| } | |
| if (!user) { | |
| return createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User not found'); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/auth/me/route.ts around lines 30 to 35, the code wraps
createErrorResponse (which already returns a Response.json()) in
NextResponse.json(), causing a double-wrapped response; replace the nested
NextResponse.json(...) call by returning the createErrorResponse(...) result
directly (so return createErrorResponse(404, ErrorCodes.USER_NOT_FOUND, 'User
not found')) and remove the extra status override to let the helper control the
response.
| let seed = Date.now() % 1000; | ||
| function rnd() { | ||
| // simple deterministic PRNG for jitter | ||
| seed = (seed * 9301 + 49297) % 233280; | ||
| return seed / 233280; | ||
| } |
There was a problem hiding this comment.
Race condition: Global mutable seed causes non-deterministic behavior under concurrent requests.
The global seed variable is mutated by rnd(), causing concurrent requests to interfere with each other's pseudo-random sequences. This defeats the purpose of deterministic generation and produces unpredictable results under load.
Move the seed into request-local scope:
-let seed = Date.now() % 1000;
-function rnd() {
- // simple deterministic PRNG for jitter
- seed = (seed * 9301 + 49297) % 233280;
- return seed / 233280;
-}
+function createRng(initialSeed = Date.now() % 1000) {
+ let seed = initialSeed;
+ return function rnd() {
+ seed = (seed * 9301 + 49297) % 233280;
+ return seed / 233280;
+ };
+}Then in the GET handler:
export async function GET(req: Request) {
+ const rnd = createRng();
const { searchParams } = new URL(req.url);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/reporting/route.ts around lines 5 to 10, the global mutable
`seed` and `rnd()` cause a race when concurrent requests share state; make the
seed request-local by removing the module-level `seed` and instead create a
per-request seed inside the GET handler, then define `rnd()` as a closure that
captures and updates that local seed (so each request has its own deterministic
PRNG). Replace all uses of the global `rnd()` with the request-local closure so
concurrent handlers do not interfere with each other's sequence.
| const status = searchParams.get('status'); | ||
| const region = searchParams.get('region'); | ||
| const limit = parseInt(searchParams.get('limit') || '20'); | ||
| const offset = parseInt(searchParams.get('offset') || '0'); | ||
| const sort = searchParams.get('sort') || 'createdAt'; | ||
| const order = searchParams.get('order') || 'desc'; | ||
|
|
There was a problem hiding this comment.
Harden pagination and sorting query params to avoid Prisma runtime errors.
limit, offset, sort, and order are taken directly from searchParams and passed into Prisma:
parseInton invalid strings yieldsNaN;Math.min(NaN, 100)isNaN, sotake/skipcan becomeNaN.- Arbitrary
sort/ordervalues will be forwarded toorderBy, which Prisma will reject at runtime.
Consider normalizing these values before using them:
const rawLimit = Number.parseInt(searchParams.get("limit") || "20", 10);
const limit = Number.isFinite(rawLimit) && rawLimit > 0 ? Math.min(rawLimit, 100) : 20;
const rawOffset = Number.parseInt(searchParams.get("offset") || "0", 10);
const offset = Number.isFinite(rawOffset) && rawOffset >= 0 ? rawOffset : 0;
const allowedSortFields = new Set(["createdAt", "updatedAt", "name", "status"]);
const sort = allowedSortFields.has(searchParams.get("sort") || "")
? (searchParams.get("sort") as string)
: "createdAt";
const order = searchParams.get("order") === "asc" ? "asc" : "desc";Then use these sanitized values in orderBy, take, and skip to keep the endpoint robust against bad query strings.
Also applies to: 53-70
🤖 Prompt for AI Agents
In apps/web/app/api/workspaces/route.ts around lines 29-35 (and similarly
53-70), the pagination and sorting query params are taken raw and can produce
NaN or invalid values for Prisma; parse limit and offset with a radix, validate
Number.isFinite and enforce defaults (limit default 20, clamp positive limit to
a max e.g. 100; offset default 0 and must be >= 0), validate sort against a
whitelist of allowed fields (e.g. createdAt, updatedAt, name, status) and
fallback to createdAt if invalid, and normalize order to only "asc" or "desc"
with "desc" as default; use these sanitized values for Prisma's take, skip, and
orderBy to avoid runtime errors.
| // Validate request | ||
| const validation = createWorkspaceSchema.safeParse(body); | ||
| if (!validation.success) { | ||
| return NextResponse.json( | ||
| createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, JSON.stringify(validation.error.issues)), | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid wrapping createErrorResponse in NextResponse.json in POST handler.
As in other routes, createErrorResponse returns a Response. Passing it into NextResponse.json breaks the error body.
Update the POST error paths to return the Response directly:
- const validation = createWorkspaceSchema.safeParse(body);
- if (!validation.success) {
- return NextResponse.json(
- createErrorResponse(400, ErrorCodes.VALIDATION_ERROR, JSON.stringify(validation.error.issues)),
- { status: 400 }
- );
- }
+ const validation = createWorkspaceSchema.safeParse(body);
+ if (!validation.success) {
+ return createErrorResponse(
+ 400,
+ ErrorCodes.VALIDATION_ERROR,
+ JSON.stringify(validation.error.issues),
+ );
+ }
...
- if (existingCount >= 10) {
- return NextResponse.json(
- createErrorResponse(402, ErrorCodes.QUOTA_EXCEEDED, 'Maximum workspace limit reached'),
- { status: 402 }
- );
- }
+ if (existingCount >= 10) {
+ return createErrorResponse(
+ 402,
+ ErrorCodes.QUOTA_EXCEEDED,
+ 'Maximum workspace limit reached',
+ );
+ }Also applies to: 124-137
🤖 Prompt for AI Agents
In apps/web/app/api/workspaces/route.ts around lines 113-121 (and similarly for
lines 124-137), the POST handler currently wraps the Response produced by
createErrorResponse inside NextResponse.json which corrupts the error body;
update the error paths to return the Response returned by createErrorResponse
directly (i.e., remove the NextResponse.json(...) wrapper) so the status and
JSON body from createErrorResponse are preserved, and apply the same change to
the other POST error branches mentioned.
| const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => { | ||
| const colorConfig = Object.entries(config).filter( | ||
| ([, config]) => config.theme || config.color, | ||
| ) | ||
|
|
||
| if (!colorConfig.length) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <style | ||
| dangerouslySetInnerHTML={{ | ||
| __html: Object.entries(THEMES) | ||
| .map( | ||
| ([theme, prefix]) => ` | ||
| ${prefix} [data-chart=${id}] { | ||
| ${colorConfig | ||
| .map(([key, itemConfig]) => { | ||
| const color = | ||
| itemConfig.theme?.[theme as keyof typeof itemConfig.theme] || | ||
| itemConfig.color | ||
| return color ? ` --color-${key}: ${color};` : null | ||
| }) | ||
| .join('\n')} | ||
| } | ||
| `, | ||
| ) | ||
| .join('\n'), | ||
| }} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Be explicit about trust boundaries when using dangerouslySetInnerHTML in ChartStyle
ChartStyle builds a <style> block with dangerouslySetInnerHTML from ChartConfig keys and color/theme values. That’s fine for purely static, internal config, but it becomes an XSS/style-injection risk if any of those values (keys, color, or theme[...]) can be influenced by user input.
At minimum, I’d recommend:
- Documenting that
ChartConfigmust only contain trusted, non-user-controlled strings, and - Optionally constraining colors/keys (e.g. validating keys against
/^[a-zA-Z0-9_-]+$/and colors against a safe pattern) before emitting them into CSS.
If you expect config to be built from user-provided data, introduce a sanitizer layer instead of passing it straight through to CSS.
I'll verify the XSS/style-injection concerns by examining the codebase structure and implementation details.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find ChartConfig type definition
rg -nP 'type\s+ChartConfig|interface\s+ChartConfig' --type=ts --type=tsx
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find ChartStyle imports and usages
rg -nP 'ChartStyle' --type=ts --type=tsx -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check the full context of chart.tsx file
cat apps/web/app/components/ui/chart.tsx | head -150Length of output: 4058
🏁 Script executed:
#!/bin/bash
# Look for how THEMES is defined
rg -nP 'const\s+THEMES|export\s+const\s+THEMES' --type=ts --type=tsx -A5Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for where ChartConfig might be built or passed
rg -nP 'ChartConfig' --type=ts --type=tsx -B2 -A2 | head -100Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if there are any validation utilities in the project
fd -e ts -e tsx | xargs grep -l 'sanitize\|validate.*color\|escape' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any CSS-related validation
rg -nP 'css|style.*valid' --type=ts --type=tsx apps/web/app/components/ui/ -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for user input or form submissions that might build chart configs
rg -nP 'user.*color|form.*chart|input.*color' --type=ts --type=tsx -i -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check exports from chart.tsx to see public API
rg -nP 'export' apps/web/app/components/ui/chart.tsx -A3Length of output: 202
🏁 Script executed:
#!/bin/bash
# Look for any existing sanitization in dangerouslySetInnerHTML usages
rg -nP 'dangerouslySetInnerHTML' --type=tsx -B5 -A5Length of output: 27
I need to search without the problematic --type flag and trace how ChartConfig is used throughout the codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for ChartContainer usages to see what config is passed
rg -n 'ChartContainer' -C3 | head -150
Length of output: 1116
🏁 Script executed:
#!/bin/bash
# Look for any sanitization or validation of keys/colors
rg -n 'sanitize|escape|validate' apps/web/app/components/ui/chart.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if id parameter is sanitized before use
rg -n 'uniqueId|chartId' apps/web/app/components/ui/chart.tsx -C2Length of output: 1099
🏁 Script executed:
#!/bin/bash
# Search for where ChartConfig objects are created/populated
rg -n 'ChartConfig|chart.*:.*{' -C2 | head -200Length of output: 109
🏁 Script executed:
#!/bin/bash
# Look for any CSS utility libraries or sanitizers in package.json
fd -name 'package.json' -exec grep -l 'sanitize\|escape\|xss' {} \;Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check if there's a schema or validation for ChartConfig
rg -n 'zod\|yup\|joi\|schema' apps/web/app/components/ui/chart.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any comments or documentation about ChartConfig safety
rg -n 'ChartConfig' apps/web/app/components/ui/chart.tsx -B3 -A3Length of output: 968
🏁 Script executed:
#!/bin/bash
# Search for how color/theme values are typically provided
rg -n 'theme.*:|color.*:' apps/web/ --include='*.tsx' --include='*.ts' | head -50Length of output: 441
🏁 Script executed:
#!/bin/bash
# Check if recharts or any chart library handles injection
fd -name 'package.json' -exec cat {} \; | grep -A10 'recharts\|chart'Length of output: 230
🏁 Script executed:
#!/bin/bash
# Look broadly at how ChartContainer is being used in examples or tests
fd -name '*.tsx' -o -name '*.ts' | xargs grep -l 'ChartContainer' | head -10Length of output: 230
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where config objects are actually created/instantiated with data
rg -n 'config.*=' apps/web/ | grep -i chart | head -30
Length of output: 708
🏁 Script executed:
#!/bin/bash
# Look for any usages in pages or components that might show data flow
fd '\.tsx$' -o -f '\.ts$' | xargs grep 'ChartContainer' 2>/dev/null | head -20Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check if there are tests that show how ChartConfig is supposed to be used
fd -name 'test' -o -name 'spec' | xargs grep -l 'ChartContainer' 2>/dev/null | head -5Length of output: 230
🏁 Script executed:
#!/bin/bash
# Look at the actual property name/value injection more carefully - check if CSS parsing protects us
cat apps/web/app/components/ui/chart.tsx | sed -n '72,103p'Length of output: 704
🏁 Script executed:
#!/bin/bash
# Check if there are actual dashboard/page implementations using ChartContainer
rg -n 'ChartContainer' apps/web/app/ -A 5 | head -60Length of output: 1012
Document trust boundary and add validation for ChartConfig keys and color values in ChartStyle
The code at lines 72-103 does have XSS/style-injection risks. Three unescaped data sources are interpolated directly into CSS:
idin the selector:[data-chart=${id}](line 50 passes it without escaping)- Config keys in properties:
--color-${key} - Color/theme values in declarations:
${color}
If any of these contain special characters (e.g., id="]; malicious-css {, or color="red; background:...", or key="foo]="x"), CSS injection occurs. The id prop is particularly at risk since it's passed from external callers with no sanitization beyond the React.useId() fallback.
Recommendations:
- Add documentation stating
ChartConfigmust contain only trusted, application-controlled strings - Validate
idparameter against/^[a-zA-Z0-9_-]+$/before use - Validate config keys against
/^[a-zA-Z0-9_-]+$/ - Validate color values against a safe color pattern (hex, rgb, named colors)
- If config can originate from user input, introduce a sanitizer layer instead of passing it directly to CSS
🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 82-82: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
[error] 83-83: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
| function ChartTooltipContent({ | ||
| active, | ||
| payload, | ||
| className, | ||
| indicator = 'dot', | ||
| hideLabel = false, | ||
| hideIndicator = false, | ||
| label, | ||
| labelFormatter, | ||
| labelClassName, | ||
| formatter, | ||
| color, | ||
| nameKey, | ||
| labelKey, | ||
| }: React.ComponentProps<typeof RechartsPrimitive.Tooltip> & | ||
| React.ComponentProps<'div'> & { | ||
| hideLabel?: boolean | ||
| hideIndicator?: boolean | ||
| indicator?: 'line' | 'dot' | 'dashed' | ||
| nameKey?: string | ||
| labelKey?: string | ||
| }) { | ||
| const { config } = useChart() | ||
|
|
||
| const tooltipLabel = React.useMemo(() => { | ||
| if (hideLabel || !payload?.length) { | ||
| return null | ||
| } | ||
|
|
||
| const [item] = payload | ||
| const key = `${labelKey || item?.dataKey || item?.name || 'value'}` | ||
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | ||
| const value = | ||
| !labelKey && typeof label === 'string' | ||
| ? config[label as keyof typeof config]?.label || label | ||
| : itemConfig?.label | ||
|
|
||
| if (labelFormatter) { | ||
| return ( | ||
| <div className={cn('font-medium', labelClassName)}> | ||
| {labelFormatter(value, payload)} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| if (!value) { | ||
| return null | ||
| } | ||
|
|
||
| return <div className={cn('font-medium', labelClassName)}>{value}</div> | ||
| }, [ | ||
| label, | ||
| labelFormatter, | ||
| payload, | ||
| hideLabel, | ||
| labelClassName, | ||
| config, | ||
| labelKey, | ||
| ]) | ||
|
|
||
| if (!active || !payload?.length) { | ||
| return null | ||
| } | ||
|
|
||
| const nestLabel = payload.length === 1 && indicator !== 'dot' | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn( | ||
| 'border-border/50 bg-background grid min-w-[8rem] items-start gap-1.5 rounded-lg border px-2.5 py-1.5 text-xs shadow-xl', | ||
| className, | ||
| )} | ||
| > | ||
| {!nestLabel ? tooltipLabel : null} | ||
| <div className="grid gap-1.5"> | ||
| {payload.map((item, index) => { | ||
| const key = `${nameKey || item.name || item.dataKey || 'value'}` | ||
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | ||
| const indicatorColor = color || item.payload.fill || item.color | ||
|
|
||
| return ( | ||
| <div | ||
| key={item.dataKey} | ||
| className={cn( | ||
| '[&>svg]:text-muted-foreground flex w-full flex-wrap items-stretch gap-2 [&>svg]:h-2.5 [&>svg]:w-2.5', | ||
| indicator === 'dot' && 'items-center', | ||
| )} | ||
| > | ||
| {formatter && item?.value !== undefined && item.name ? ( | ||
| formatter(item.value, item.name, item, index, item.payload) | ||
| ) : ( | ||
| <> | ||
| {itemConfig?.icon ? ( | ||
| <itemConfig.icon /> | ||
| ) : ( | ||
| !hideIndicator && ( | ||
| <div | ||
| className={cn( | ||
| 'shrink-0 rounded-[2px] border-(--color-border) bg-(--color-bg)', | ||
| { | ||
| 'h-2.5 w-2.5': indicator === 'dot', | ||
| 'w-1': indicator === 'line', | ||
| 'w-0 border-[1.5px] border-dashed bg-transparent': | ||
| indicator === 'dashed', | ||
| 'my-0.5': nestLabel && indicator === 'dashed', | ||
| }, | ||
| )} | ||
| style={ | ||
| { | ||
| '--color-bg': indicatorColor, | ||
| '--color-border': indicatorColor, | ||
| } as React.CSSProperties | ||
| } | ||
| /> | ||
| ) | ||
| )} | ||
| <div | ||
| className={cn( | ||
| 'flex flex-1 justify-between leading-none', | ||
| nestLabel ? 'items-end' : 'items-center', | ||
| )} | ||
| > | ||
| <div className="grid gap-1.5"> | ||
| {nestLabel ? tooltipLabel : null} | ||
| <span className="text-muted-foreground"> | ||
| {itemConfig?.label || item.name} | ||
| </span> | ||
| </div> | ||
| {item.value && ( | ||
| <span className="text-foreground font-mono font-medium tabular-nums"> | ||
| {item.value.toLocaleString()} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </> | ||
| )} | ||
| </div> | ||
| ) | ||
| })} | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Tooltip suppresses zero values due to item.value && check
In the default rendering branch you use:
{item.value && (
<span className="text-foreground font-mono font-medium tabular-nums">
{item.value.toLocaleString()}
</span>
)}This hides legitimate 0 values because 0 is falsy. Suggest changing the condition to allow zero:
- {item.value && (
+ {item.value !== undefined && item.value !== null && (
<span className="text-foreground font-mono font-medium tabular-nums">
{item.value.toLocaleString()}
</span>
)}This keeps null/undefined hidden but renders numeric zero correctly.
🤖 Prompt for AI Agents
In apps/web/app/components/ui/chart.tsx around lines 107 to 249, the tooltip
rendering uses a falsy check `item.value && ...` which suppresses legitimate
zero values; update the condition to explicitly check for null/undefined (e.g.,
`item.value !== null && item.value !== undefined`) or test the value type (e.g.,
`typeof item.value === 'number' || item.value !== null`) so numeric 0 is
rendered while still hiding null/undefined.
| // This is the internal state of the sidebar. | ||
| // We use openProp and setOpenProp for control from outside the component. | ||
| const [_open, _setOpen] = React.useState(defaultOpen) | ||
| const open = openProp ?? _open | ||
| const setOpen = React.useCallback( | ||
| (value: boolean | ((value: boolean) => boolean)) => { | ||
| const openState = typeof value === 'function' ? value(open) : value | ||
| if (setOpenProp) { | ||
| setOpenProp(openState) | ||
| } else { | ||
| _setOpen(openState) | ||
| } | ||
|
|
||
| // This sets the cookie to keep the sidebar state. | ||
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` | ||
| }, | ||
| [setOpenProp, open], | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Align setOpen with React state setter semantics
setOpen accepts either a boolean or an updater function, but it always calls the updater with the captured open value:
const setOpen = React.useCallback(
(value: boolean | ((value: boolean) => boolean)) => {
const openState = typeof value === 'function' ? value(open) : value
...
_setOpen(openState)
},
[setOpenProp, open],
)This behaves like setOpen(!open), not like React’s functional updates, and can misbehave if setOpen(prev => !prev) is used multiple times in a single render cycle. You can fix this by basing the computation on the previous state:
- const setOpen = React.useCallback(
- (value: boolean | ((value: boolean) => boolean)) => {
- const openState = typeof value === 'function' ? value(open) : value
- if (setOpenProp) {
- setOpenProp(openState)
- } else {
- _setOpen(openState)
- }
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`
- },
- [setOpenProp, open],
- )
+ const setOpen = React.useCallback(
+ (value: boolean | ((value: boolean) => boolean)) => {
+ _setOpen((prev) => {
+ const next = typeof value === 'function' ? value(prev) : value
+ if (setOpenProp) {
+ setOpenProp(next)
+ }
+ document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`
+ return next
+ })
+ },
+ [setOpenProp],
+ )This preserves external control while matching React’s updater semantics.
|
thanks |
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
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.