-
Notifications
You must be signed in to change notification settings - Fork 51
Daily branch 2025 08 17 #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces WorkOS config check with a mode-based isWorkOSEnabled, adds client/server auth utilities and auth routes, introduces per-user rate limiting via Upstash, adds a typed error framework and fetch wrapper, refactors chat streaming and analytics capture, adds header/icon/UI components, updates environment examples, middleware, README, and minor formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (Chat UI)
participant API as /api/chat
participant Auth as getUserID()
participant RL as checkRateLimit()
participant Model as streamText(model)
participant Tools as createTools(...)
participant PH as PostHog
UI->>API: POST messages
API->>Auth: getUserID(req)
Auth-->>API: userID | "anonymous"
API->>RL: checkRateLimit(userID)
RL-->>API: ok or throw rate_limit
API->>PH: capture(hackerai-<mode>)
API->>Tools: createTools(userID, writer, mode, execMode)
par Stream response
API->>Model: streamText({systemPrompt, messages, tools})
Model-->>API: tokens/tool calls
API->>PH: capture(hackerai-<toolName>)
API-->>UI: UI stream events
end
API-->>UI: Stream finished / error (ChatSDKError.toResponse)
sequenceDiagram
participant User as Browser
participant RLogin as /login
participant RSignup as /signup
participant RLogout as /logout
participant Gate as isWorkOSEnabled()
participant WorkOS as AuthKit
User->>RLogin: GET
RLogin->>Gate: check
alt enabled
RLogin->>WorkOS: getSignInUrl
RLogin-->>User: redirect to WorkOS
else
RLogin-->>User: redirect "/"
end
User->>RSignup: GET
RSignup->>Gate: check
alt enabled
RSignup->>WorkOS: getSignUpUrl
RSignup-->>User: redirect to WorkOS
else
RSignup-->>User: redirect "/"
end
User->>RLogout: GET
RLogout->>Gate: check
alt enabled
RLogout->>WorkOS: signOut()
RLogout-->>User: response
else
RLogout-->>User: redirect "/"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (31)
lib/ai/tools/multi-edit.ts (2)
20-21: Unify contradictory guidance about file_path (absolute vs relative).The long-form description says “must be absolute, not relative” (Lines 20–21), while the inputSchema description allows relative paths (Lines 55–56). This inconsistency can confuse tool users.
Apply this diff to align both sections to allow relative or absolute paths:
- - file_path: The absolute path to the file to modify (must be absolute, not relative) + - file_path: The path to the file to modify. You can use either a relative path in the workspace or an absolute path.Also applies to: 55-56
24-24: Fix typos: “occurences” → “occurrences”.Minor copy correction in user-facing docs.
- - replace_all: Replace all occurences of old_string. This parameter is optional and defaults to false. + - replace_all: Replace all occurrences of old_string. This parameter is optional and defaults to false. @@ - .describe("Replace all occurences of old_string (default false)"), + .describe("Replace all occurrences of old_string (default false)"),Also applies to: 70-70
.env.local.example (2)
36-36: Clarify allowed values for NEXT_PUBLIC_AUTH_MODE.Make it explicit that both “workos” and “anonymous” are supported to reduce setup ambiguity.
-# NEXT_PUBLIC_AUTH_MODE=workos +# NEXT_PUBLIC_AUTH_MODE=workos +# Valid values: "workos" or "anonymous"
52-52: Add trailing newline to satisfy dotenv linters.dotenv-linter flags the missing ending blank line.
-# RATE_LIMIT_REQUESTS=10 +# RATE_LIMIT_REQUESTS=10 +lib/ai/tools/utils/local-file-operations.ts (1)
257-259: Minor nit: avoid resolving the same path twiceYou compute resolvedPath and then call checkLocalFileExists with filePath, which internally resolves again. Pass the resolvedPath to avoid duplicate resolution (very minor).
Apply this minimal change:
- const resolvedPath = resolve(filePath); - const fileExists = await checkLocalFileExists(filePath); + const resolvedPath = resolve(filePath); + const fileExists = await checkLocalFileExists(resolvedPath);components/icons/hackerai-svg.tsx (2)
1-8: Broaden props and add accessibility defaultsForward standard SVG props and add accessible labeling. This makes the icon more reusable (e.g., className, onClick) and screen-reader friendly when needed while defaulting to decorative.
Apply:
-import type { FC } from "react"; +import type { FC, SVGProps } from "react"; -interface HackerAISVGProps { - theme: "dark" | "light"; - scale?: number; -} +interface HackerAISVGProps extends SVGProps<SVGSVGElement> { + theme: "dark" | "light"; + scale?: number; + ariaLabel?: string; +} -export const HackerAISVG: FC<HackerAISVGProps> = ({ theme, scale = 1 }) => { +export const HackerAISVG: FC<HackerAISVGProps> = ({ + theme, + scale = 1, + ariaLabel, + ...props +}) => { const fillColor = theme === "dark" ? "#fff" : "#000";And on the SVG element:
- <svg + <svg + role="img" + aria-hidden={ariaLabel ? undefined : true} + aria-label={ariaLabel} width={189 * scale} height={194 * scale} viewBox="0 0 512 512" fill="none" xmlns="http://www.w3.org/2000/svg" + {...props} >
8-19: Consider using currentColor to integrate with themingInstead of hardcoding "#fff"/"#000", consider using CSS currentColor and set color on the parent. This makes the icon theme-adaptive without extra props.
Example:
- Replace fill={fillColor} with fill="currentColor".
- Let consumers control color via className or style (e.g., className="text-foreground").
components/ui/alert-dialog.tsx (3)
47-64: Forward refs for Radix primitives (Content)Radix components expect ref-forwarding for composition and focus management. Wrapping with forwardRef preserves ref access and aligns with common shadcn/ui patterns.
Apply:
-function AlertDialogContent({ - className, - ...props -}: React.ComponentProps<typeof AlertDialogPrimitive.Content>) { - return ( - <AlertDialogPortal> - <AlertDialogOverlay /> - <AlertDialogPrimitive.Content - data-slot="alert-dialog-content" - className={cn( - "bg-background 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 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg", - className, - )} - {...props} - /> - </AlertDialogPortal> - ); -} +const AlertDialogContent = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Content>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Content> +>(({ className, ...props }, ref) => ( + <AlertDialogPortal> + <AlertDialogOverlay /> + <AlertDialogPrimitive.Content + ref={ref} + data-slot="alert-dialog-content" + className={cn( + "bg-background 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 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg", + className, + )} + {...props} + /> + </AlertDialogPortal> +)); +AlertDialogContent.displayName = AlertDialogPrimitive.Content.displayName;
121-131: Forward refs and add data-slot for ActionMaintains ref access (e.g., focusing programmatically) and aligns slot usage with other parts.
Apply:
-function AlertDialogAction({ - className, - ...props -}: React.ComponentProps<typeof AlertDialogPrimitive.Action>) { - return ( - <AlertDialogPrimitive.Action - className={cn(buttonVariants(), className)} - {...props} - /> - ); -} +const AlertDialogAction = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Action>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Action> +>(({ className, ...props }, ref) => ( + <AlertDialogPrimitive.Action + ref={ref} + data-slot="alert-dialog-action" + className={cn(buttonVariants(), className)} + {...props} + /> +)); +AlertDialogAction.displayName = AlertDialogPrimitive.Action.displayName;
133-143: Forward refs and add data-slot for CancelSame rationale as Action: preserve ref semantics and consistent slotting.
Apply:
-function AlertDialogCancel({ - className, - ...props -}: React.ComponentProps<typeof AlertDialogPrimitive.Cancel>) { - return ( - <AlertDialogPrimitive.Cancel - className={cn(buttonVariants({ variant: "outline" }), className)} - {...props} - /> - ); -} +const AlertDialogCancel = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Cancel>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Cancel> +>(({ className, ...props }, ref) => ( + <AlertDialogPrimitive.Cancel + ref={ref} + data-slot="alert-dialog-cancel" + className={cn(buttonVariants({ variant: "outline" }), className)} + {...props} + /> +)); +AlertDialogCancel.displayName = AlertDialogPrimitive.Cancel.displayName;app/signup/route.ts (1)
11-14: Harden against upstream signup URL fetch failuresWrap getSignUpUrl in try/catch to avoid unhandled rejections and provide a safe fallback.
Apply:
- const signUpUrl = await getSignUpUrl(); - - return redirect(signUpUrl); + try { + const signUpUrl = await getSignUpUrl(); + return redirect(signUpUrl); + } catch (err) { + console.error("Failed to build WorkOS sign-up URL", err); + // Fallback to home to avoid leaving the user stranded on error + return redirect("/"); + }app/login/route.ts (2)
6-9: Nit: Update the comment to reflect “enabled” vs “configured”Small wording mismatch with the new API name.
- // If WorkOS is not configured, redirect to home page + // If WorkOS is not enabled, redirect to home page
5-14: Gracefully handle getSignInUrl failuresIf WorkOS env/config is incomplete while auth mode is enabled, getSignInUrl() may throw. Consider catching and redirecting with a user-friendly message/log.
export const GET = async () => { - if (!isWorkOSEnabled()) { - // If WorkOS is not configured, redirect to home page - return redirect("/"); - } - - const signInUrl = await getSignInUrl(); - - return redirect(signInUrl); + if (!isWorkOSEnabled()) { + // If WorkOS is not enabled, redirect to home page + return redirect("/"); + } + try { + const signInUrl = await getSignInUrl(); + return redirect(signInUrl); + } catch (err) { + console.error("Failed to obtain WorkOS sign-in URL:", err); + return redirect("/?error=signin_unavailable"); + } };app/logout/route.ts (2)
5-9: Nit: Update the comment to match the new naming (“enabled”)Keep the wording consistent with isWorkOSEnabled().
- if (!isWorkOSEnabled()) { - // If WorkOS is not configured, redirect to home page + if (!isWorkOSEnabled()) { + // If WorkOS is not enabled, redirect to home page return redirect("/"); }
5-14: Optional: Consider making logout a POST to reduce CSRF riskLogging out via GET is common but can be CSRF’d. Switching to POST (and updating the UI to submit a form or fetch) is a small hardening step.
app/layout.tsx (1)
45-49: Provider gating LGTM; minor consistency suggestionThis gating mirrors the middleware/route gating. If you want to avoid mixing redirect styles across routes, consider standardizing on either next/navigation’s redirect or NextResponse.redirect everywhere (purely a consistency nit).
middleware.ts (1)
7-22: Optional: centralize unauthenticated pathsTo avoid drift across middleware and routes/pages, consider exporting a shared constant (e.g., from lib/auth-utils or a new config module).
Example within this file:
-const middleware = isWorkOSEnabled() - ? authkitMiddleware({ - middlewareAuth: { - enabled: true, - unauthenticatedPaths: [ - "/", - "/login", - "/signup", - "/logout", - "/callback", - "/privacy-policy", - "/terms-of-service", - ], - }, - }) +const UNAUTHENTICATED_PATHS = [ + "/", + "/login", + "/signup", + "/logout", + "/callback", + "/privacy-policy", + "/terms-of-service", +]; + +const middleware = isWorkOSEnabled() + ? authkitMiddleware({ + middlewareAuth: { + enabled: true, + unauthenticatedPaths: UNAUTHENTICATED_PATHS, + }, + }) : (request: NextRequest) => { // No authentication required, just pass through return NextResponse.next(); };app/callback/route.ts (1)
7-15: Module-level gating is fine; consider consistency with other routesOther routes gate inside the handler; here you gate at module init. Both work; choosing one style across route files improves maintainability.
lib/utils.ts (1)
9-12: Add explicit return type for clarity and consistencyExplicitly annotating the return type improves readability and avoids accidental type widening.
-export async function fetchWithErrorHandlers( +export async function fetchWithErrorHandlers( input: RequestInfo | URL, init?: RequestInit, -) { +): Promise<Response> {app/components/Header.tsx (2)
12-22: Use Next.js router instead of window.location for navigation; remove unnecessary async
window.location.hrefcauses a full reload and bypasses client-side transitions. Usingnext/navigationimproves UX. Also,handleSignOutis markedasyncbut does not await anything.+import { useRouter } from "next/navigation"; @@ -const Header: React.FC = () => { - const { user, loading } = useAppAuth(); +const Header: React.FC = () => { + const { user, loading } = useAppAuth(); + const router = useRouter(); @@ - const handleSignIn = () => { - window.location.href = "/login"; - }; + const handleSignIn = () => { + router.push("/login"); + }; @@ - const handleSignUp = () => { - window.location.href = "/signup"; - }; + const handleSignUp = () => { + router.push("/signup"); + }; @@ - const handleSignOut = async () => { - window.location.href = "/logout"; - }; + const handleSignOut = () => { + router.push("/logout"); + };
41-61: Unify min-width classes for visual consistencyTwo different min-widths are used for similar buttons (
min-w-[74px]vsmin-w-16). Standardizing reduces visual jitter.- className="min-w-[74px] rounded-[10px]" + className="min-w-16 rounded-[10px]" @@ - className="min-w-[74px] rounded-[10px]" + className="min-w-16 rounded-[10px]"app/components/Messages.tsx (2)
151-152: Show the actual error message for non-rate-limit errorsFalling back to a generic string hides useful diagnostics. Rendering
error.messageis safe as plain text.- <p>An error occurred.</p> + <p>{error.message || "An error occurred."}</p>
140-159: Avoid repeated instanceof/type checksMinor readability nit: compute
isRateLimitonce and reuse it in both content and actions.If you like, I can send a small refactor to introduce
const isRateLimit = error instanceof ChatSDKError && error.type === "rate_limit";and reference it where needed.Also applies to: 160-174
app/page.tsx (3)
24-24: Auth gate on submit: consider SPA navigation and loading-state guardThe WorkOS-only auth check is correct. Two small UX refinements:
- Use Next.js client navigation instead of window.location for a smoother transition.
- Optionally guard against a transient “user not loaded yet” state to avoid flicker-redirects (depends on what useAppAuth returns in WorkOS mode).
Apply this minimal change for SPA navigation within the selected range:
- if (isWorkOSEnabled() && !user) { - window.location.href = "/login"; - return; - } + if (isWorkOSEnabled() && !user) { + router.push("/login"); + return; + }Additionally, add the router and (optionally) a loading-aware guard outside the selected range:
// imports import { useRouter } from "next/navigation"; // inside component const router = useRouter(); // Optional, only if useAppAuth exposes a loading flag in WorkOS mode: // if (isWorkOSEnabled() && loading) return; // prevent premature redirect while auth is resolvingAlso applies to: 66-71
47-55: Nit: avoid variable shadowing and provide a fallback toast for unknown errorsThe parameter name error shadows the outer error from useChat’s return. Not harmful but slightly confusing. Also, consider a fallback toast for unexpected non-ChatSDKError errors to avoid silent failures.
Suggested tweak:
- onError: (error) => { - if (error instanceof ChatSDKError) { + onError: (err) => { + if (err instanceof ChatSDKError) { // For rate limit errors, let them flow to the Messages component // For other errors, show toast - if (error.type !== "rate_limit") { - toast.error(error.message); + if (err.type !== "rate_limit") { + toast.error(err.message); } + } else { + toast.error("Something went wrong. Please try again."); } },
110-116: Avoid hard-coding header height in layout calculationsUsing h-[calc(100vh-58px)] couples the layout to a magic number and can break across breakpoints or future header changes. Prefer a flex column layout where the header is a sibling above the chat area, and the chat container uses flex-1 without manual height math. For example: make the page container a flex-col, render the header (sticky if needed), and give the chat area flex-1 h-full overflow-hidden. This removes the need for the calc.
If you want a minimal change here, consider replacing the calc with a padding-top equal to the header height on the chat container (e.g., pt-14) and leaving the container at h-full, but the flex approach is more robust.
app/api/chat/route.ts (3)
25-31: Parse/validate request body defensivelyYou trust the client-supplied mode field via a type assertion. A lightweight runtime validation (e.g., zod union for "agent" | "ask") would prevent bad modes from propagating.
Example sketch:
// outside selected lines import { z } from "zod"; const BodySchema = z.object({ messages: z.array(z.any()), // or tighten to your UIMessage shape mode: z.union([z.literal("agent"), z.literal("ask")]), }); // replace req.json() parse within the try const { messages, mode } = BodySchema.parse(await req.json());
35-38: Sanitize TERMINAL_EXECUTION_MODE to known valuesCasting the env var to ExecutionMode can let unexpected values leak in at runtime. Normalize to "local" when not "sandbox" or "local".
Apply this diff:
- const executionMode: ExecutionMode = - (process.env.TERMINAL_EXECUTION_MODE as ExecutionMode) || "local"; + const rawMode = process.env.TERMINAL_EXECUTION_MODE; + const executionMode: ExecutionMode = + rawMode === "sandbox" || rawMode === "local" ? rawMode : "local";
110-124: Consider error code for unexpected failuresMapping all unexpected errors to offline:chat may mislead users when the service is up but something else failed. You might prefer a more general api or chat error (e.g., bad_request:api or bad_request:chat) to better reflect the state, while still hiding internals.
Minimal change:
- const unexpectedError = new ChatSDKError( - "offline:chat", + const unexpectedError = new ChatSDKError( + "bad_request:api", error instanceof Error ? error.message : "Unknown error occurred", );lib/errors.ts (2)
36-51: Declare cause and set error name for better typing and diagnosticscause is assigned but not declared, which can weaken typing. Also set the error name for clearer stack traces.
Apply this diff:
export class ChatSDKError extends Error { public type: ErrorType; public surface: Surface; public statusCode: number; + public cause?: string; constructor(errorCode: ErrorCode, cause?: string) { - super(); + super(); + this.name = "ChatSDKError"; const [type, surface] = errorCode.split(":"); this.type = type as ErrorType; this.cause = cause; this.surface = surface as Surface; this.message = getMessageByErrorCode(errorCode); this.statusCode = getStatusCodeByType(this.type); }
76-113: Default messages provide good coverage; consider adding more surfaces as neededCurrent mappings are sensible. If you expect stream/history/vote/suggestions errors to surface to users, you can extend getMessageByErrorCode with tailored messages later. No action needed now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.env.local.example(1 hunks)app/api/chat/route.ts(1 hunks)app/callback/route.ts(1 hunks)app/components/Header.tsx(1 hunks)app/components/Messages.tsx(2 hunks)app/hooks/useAppAuth.ts(1 hunks)app/layout.tsx(2 hunks)app/login/route.ts(1 hunks)app/logout/route.ts(1 hunks)app/page.tsx(4 hunks)app/signup/route.ts(1 hunks)app/terms-of-service/page.tsx(1 hunks)components/icons/hackerai-svg.tsx(1 hunks)components/ui/alert-dialog.tsx(1 hunks)components/ui/sonner.tsx(1 hunks)lib/ai/tools/multi-edit.ts(2 hunks)lib/ai/tools/utils/local-file-operations.ts(2 hunks)lib/auth-utils.ts(1 hunks)lib/errors.ts(1 hunks)lib/rate-limit.ts(1 hunks)lib/utils.ts(1 hunks)middleware.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (14)
app/hooks/useAppAuth.ts (1)
lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/logout/route.ts (4)
app/login/route.ts (1)
GET(5-14)app/callback/route.ts (1)
GET(7-15)app/signup/route.ts (1)
GET(5-14)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/components/Header.tsx (4)
app/hooks/useAppAuth.ts (1)
useAppAuth(8-15)components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)components/ui/button.tsx (1)
Button(59-59)
app/signup/route.ts (4)
app/login/route.ts (1)
GET(5-14)app/callback/route.ts (1)
GET(7-15)app/logout/route.ts (1)
GET(5-14)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
lib/utils.ts (1)
lib/errors.ts (2)
ChatSDKError(36-74)ErrorCode(20-20)
lib/rate-limit.ts (1)
lib/errors.ts (1)
ChatSDKError(36-74)
app/components/Messages.tsx (2)
lib/errors.ts (1)
ChatSDKError(36-74)app/components/MemoizedMarkdown.tsx (2)
MemoizedMarkdown(117-129)p(89-93)
components/ui/alert-dialog.tsx (2)
lib/utils.ts (1)
cn(5-7)components/ui/button.tsx (1)
buttonVariants(59-59)
app/login/route.ts (4)
app/callback/route.ts (1)
GET(7-15)app/signup/route.ts (1)
GET(5-14)app/logout/route.ts (1)
GET(5-14)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
middleware.ts (1)
lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/callback/route.ts (4)
app/login/route.ts (1)
GET(5-14)app/signup/route.ts (1)
GET(5-14)app/logout/route.ts (1)
GET(5-14)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/page.tsx (5)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)app/hooks/useAppAuth.ts (1)
useAppAuth(8-15)lib/utils.ts (1)
fetchWithErrorHandlers(9-29)lib/errors.ts (1)
ChatSDKError(36-74)lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/layout.tsx (1)
lib/auth-utils.ts (1)
isWorkOSEnabled(13-14)
app/api/chat/route.ts (12)
types/chat.ts (1)
ChatMode(1-1)lib/auth-utils.ts (1)
getUserID(20-30)lib/rate-limit.ts (1)
checkRateLimit(6-63)types/agent.ts (1)
ExecutionMode(20-20)lib/token-utils.ts (1)
truncateMessagesToTokenLimit(24-44)lib/ai/providers.ts (1)
myProvider(4-13)lib/ai/tools/index.ts (1)
createTools(13-51)lib/actions/index.ts (1)
generateTitleFromUserMessage(17-44)lib/system-prompt.ts (1)
systemPrompt(11-95)lib/ai/tools/utils/sandbox-manager.ts (1)
getSandbox(16-37)lib/ai/tools/utils/sandbox.ts (1)
pauseSandbox(152-169)lib/errors.ts (1)
ChatSDKError(36-74)
🪛 Biome (2.1.2)
app/hooks/useAppAuth.ts
[error] 10-10: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 52-52: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (20)
app/terms-of-service/page.tsx (1)
84-84: LGTM – whitespace-only formatting change.No functional or visual impact. Safe to merge.
lib/ai/tools/multi-edit.ts (1)
104-104: LGTM – minor whitespace normalization.No logic changes; readability is fine.
Also applies to: 118-118
package.json (1)
18-23: Deps additions align with new features (alert-dialog, Upstash RL/Redis).The versions look reasonable and match the PR’s new UI and rate-limiting capabilities.
.env.local.example (1)
44-52: LGTM – helpful Upstash rate-limiting samples.The placeholders and comments are clear and match the new rate-limit feature.
lib/ai/tools/utils/local-file-operations.ts (3)
265-267: Good readability improvement on error message formattingSwitching to multiline throw improves diff legibility and keeps long messages readable. No behavioral change.
275-277: Multiline assignment is clearerExpanding the ternary across lines makes intent explicit without changing behavior.
294-296: Consistent error formattingAligns with the earlier throw formatting; consistent and easier to scan.
app/signup/route.ts (1)
5-14: Gated signup flow looks correctAuth-mode check with redirect fallback mirrors login route; WorkOS path delegates to getSignUpUrl.
app/login/route.ts (1)
3-3: Import migration to isWorkOSEnabled looks goodThis aligns with the new centralized auth-mode gating.
app/layout.tsx (1)
8-8: isWorkOSEnabled import swap is consistent with the new auth-modeMatches the rest of the PR’s gating approach.
middleware.ts (2)
7-25: Middleware gating is straightforward and clearConditional middleware based on isWorkOSEnabled() is clean and easy to reason about. Good inclusion of unauthenticatedPaths.
14-15: Adding /signup and /logout to unauthenticatedPaths makes sensePrevents middleware from blocking these flows. Good call.
app/callback/route.ts (1)
3-3: Import swap to isWorkOSEnabled is consistent with the new APIThis matches the rest of the codebase.
lib/utils.ts (1)
16-19: Use a valid ErrorCode fallbackThe
ErrorCodetype is defined as${ErrorType}:${Surface}and your fallback"unexpected:chat"isn’t valid becauseErrorTypeonly includes:
"bad_request""unauthorized""forbidden""not_found""rate_limit"Please choose one of the following remedies:
• Add
"unexpected"to theErrorTypeunion inlib/errors.tsif you intend to keepunexpected:chatas your generic fallback.
• Or swap out"unexpected:chat"for an existing code, for example"bad_request:chat"or"rate_limit:chat", to satisfy theErrorCodetype.Location to update:
- lib/utils.ts around the fallback throw (formerly
"unexpected:chat")- throw new ChatSDKError( - "unexpected:chat", - `${response.status} ${response.statusText}`, - ); + // Either add "unexpected" to ErrorType in lib/errors.ts, + // or swap in an existing type, e.g., "bad_request:chat" + throw new ChatSDKError( + "bad_request:chat", + `${response.status} ${response.statusText}`, + );Likely an incorrect or invalid review comment.
lib/rate-limit.ts (1)
17-26: Cache the Ratelimit Instance & Simplify Reset Handling
- Instantiate
RedisandRatelimitonce at module-scope (singleton) to avoid per-call overhead.- The
resetvalue returned by@upstash/ratelimitis already a Unix timestamp in milliseconds, so you can drop the seconds-to-ms fallback.- Compute the time until reset directly and round up the remaining minutes for a friendly message.
Suggested diff (in lib/rate-limit.ts):
- if (!success) { - // Normalize `reset` to milliseconds (handle seconds vs ms) - const resetMs = reset > 1e12 ? reset : reset * 1000; - const msUntilReset = Math.max(0, resetMs - Date.now()); - const hours = Math.floor(msUntilReset / (1000 * 60 * 60)); - const minutes = Math.ceil((msUntilReset % (1000 * 60 * 60)) / (1000 * 60)); // round up remaining minutes + if (!success) { + // `reset` is in milliseconds since epoch + const msUntilReset = Math.max(0, reset - Date.now()); + const hours = Math.floor(msUntilReset / (1000 * 60 * 60)); + const minutes = Math.ceil((msUntilReset % (1000 * 60 * 60)) / (1000 * 60));optional_refactors_recommended
app/page.tsx (1)
14-16: Networking and error-handling integration looks solidInjecting fetchWithErrorHandlers into DefaultChatTransport and using ChatSDKError with toast for UI feedback is a clean way to centralize error semantics and UX. This will ensure consistent handling of server responses and offline scenarios.
Also applies to: 29-29
app/api/chat/route.ts (3)
14-14: User ID centralization and per-user rate limiting: LGTMUsing getUserID(req) with an anonymous fallback aligns well with your WorkOS/anonymous mode. Placing checkRateLimit(userID) early ensures work is avoided for over-quota users and harmonizes with the frontend error UX.
Also applies to: 32-34
55-77: Non-blocking, transient title generation with ordering guarantees: LGTMRunning generateTitleFromUserMessage in parallel, emitting a transient data-title event, and awaiting the promise in both onError and onFinish to preserve ordering is a thoughtful pattern that keeps the primary stream resilient.
79-104: Cleanup symmetry on error/finish is correctPausing the sandbox in both onError and onFinish prevents resource leaks. The use of smoothStream and stepCountIs is coherent with your UX.
lib/errors.ts (1)
72-73: Response structure is consistent with visibility policytoResponse returning sanitized bodies for log-only surfaces and including code/message/cause for others matches the intended UX and security posture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/layout.tsx (1)
45-49: Refactor useAppAuth to avoid unguarded useAuth invocationThis hook currently calls useAuth() regardless of whether is mounted, causing a runtime crash in anonymous mode. Switch to a module-scope implementation: when WorkOS is disabled, export a no-op stub instead of calling useAuth(). Verified there are no other direct useAuth() calls outside this file.
• File: app/hooks/useAppAuth.ts (lines 8–12)
– Remove the unconditionalconst workosAuth = useAuth()
– Introduce module-scope branching:-import { useAuth as realUseAuth } from '@workos/auth-kit'; +import { useAuth as realUseAuth } from '@workos/auth-kit'; // Always call useAuth to comply with rules of hooks -export const useAppAuth = () => { - const workosAuth = useAuth(); - - if (isWorkOSEnabled()) { - return workosAuth; - } - - return { user: null, login: () => {}, logout: () => {} }; -}; +export const useAppAuth = isWorkOSEnabled() + ? realUseAuth + : () => ({ user: null, login: () => {}, logout: () => {} });
♻️ Duplicate comments (1)
app/hooks/useAppAuth.ts (1)
8-18: Runtime crash risk: useAuth called without provider in anonymous modeIn anonymous mode, RootLayout does not render AuthKitProvider. Calling useAuth unconditionally will likely throw (or at best return invalid context). Solve by selecting the implementation at module scope so that useAuth is never invoked when WorkOS is disabled. This also satisfies the Rules of Hooks by keeping hook calls unconditional per environment.
Apply:
-export const useAppAuth = () => { - // Always call useAuth to comply with rules of hooks - const workosAuth = useAuth(); - - if (isWorkOSEnabled()) { - return workosAuth; - } - - // Anonymous mode - no authentication required - return { user: null, loading: false }; -}; +// Resolve the auth mode once at module load to keep hook call order stable +const WORKOS_ENABLED = isWorkOSEnabled(); + +type AppAuth = Pick<ReturnType<typeof useAuth>, "user" | "loading">; + +export const useAppAuth: () => AppAuth = + WORKOS_ENABLED + ? () => useAuth() + : () => ({ user: null, loading: false });
🧹 Nitpick comments (4)
lib/auth-config.ts (1)
15-16: Default to anonymous when env is unset or malformedGuard against undefined or misconfigured env by providing a safe default. Behavior remains identical when correctly configured.
Apply:
-export const isWorkOSEnabled = (): boolean => - process.env.NEXT_PUBLIC_AUTH_MODE === "workos"; +export const isWorkOSEnabled = (): boolean => + (process.env.NEXT_PUBLIC_AUTH_MODE ?? "anonymous") === "workos";app/hooks/useAppAuth.ts (1)
1-1: Mark this hook module as clientThis file defines and uses React hooks; explicitly mark it as a client module to avoid accidental server imports and to align with Next.js conventions.
Apply:
+ "use client"; import { useAuth } from "@workos-inc/authkit-nextjs/components"; import { isWorkOSEnabled } from "@/lib/auth-config";lib/auth-utils.ts (2)
16-17: Avoid logging raw errors that may include tokensLogging the full error object could leak sensitive details. Consider downgrading to a concise message or guard by environment.
Apply:
- console.error("Failed to get user session:", error); + if (process.env.NODE_ENV !== "production") { + console.warn("Failed to get user session", error); + } else { + console.warn("Failed to get user session"); + }
1-3: Optional: Hardeninglib/auth-utils.tsfor client safetyAlthough
lib/auth-utils.tsis currently only imported in server‐side code (e.g.,app/api/chat/route.ts), you can prevent future accidental client–side bundling by making its server dependencies dynamic or type‐only:• In
lib/auth-utils.ts, change:-import { authkit } from "@workos-inc/authkit-nextjs"; -import { NextRequest } from "next/server"; +import type { NextRequest } from "next/server"; import { isWorkOSEnabled } from "./auth-config";• And at the call site within the same file (e.g., inside your
tryblock):- const { session } = await authkit(req); + const { authkit } = await import("@workos-inc/authkit-nextjs"); + const { session } = await authkit(req);This refactor keeps
lib/auth-utils.tsclient‐safe even if it’s ever (inadvertently) imported into a client bundle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
app/callback/route.ts(1 hunks)app/components/Header.tsx(1 hunks)app/hooks/useAppAuth.ts(1 hunks)app/layout.tsx(2 hunks)app/login/route.ts(1 hunks)app/logout/route.ts(1 hunks)app/page.tsx(4 hunks)app/signup/route.ts(1 hunks)lib/auth-config.ts(1 hunks)lib/auth-utils.ts(1 hunks)middleware.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- app/logout/route.ts
- app/signup/route.ts
- app/login/route.ts
- app/page.tsx
- app/components/Header.tsx
- middleware.ts
- app/callback/route.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/hooks/useAppAuth.ts (1)
lib/auth-config.ts (1)
isWorkOSEnabled(15-16)
app/layout.tsx (1)
lib/auth-config.ts (1)
isWorkOSEnabled(15-16)
lib/auth-utils.ts (1)
lib/auth-config.ts (1)
isWorkOSEnabled(15-16)
🔇 Additional comments (1)
lib/auth-config.ts (1)
15-16: LGTM: client-safe gating via NEXT_PUBLICUsing a NEXT_PUBLIC_ var makes this safe on both client and server and centralizes the source of truth for WorkOS enablement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/hooks/useAppAuth.ts (1)
8-18: Fix: Calling useAuth unconditionally will throw in anonymous mode (no provider). Use module-scope gating instead.In anonymous mode, isn’t mounted, so unconditionally invoking useAuth() will likely throw (most context-based hooks do). Keep the hook-call order stable by deciding the implementation at module load and avoid calling useAuth() when WorkOS is disabled.
Apply this diff to refactor:
+ "use client"; import { useAuth } from "@workos-inc/authkit-nextjs/components"; import { isWorkOSEnabled } from "@/lib/auth-config"; /** * Custom hook that handles authentication for both WorkOS and anonymous modes * Returns consistent user/loading state regardless of auth mode */ -export const useAppAuth = () => { - // Always call useAuth to comply with rules of hooks - const workosAuth = useAuth(); - - if (isWorkOSEnabled()) { - return workosAuth; - } - - // Anonymous mode - no authentication required - return { user: null, loading: false }; -}; +// Resolve auth mode once at module load to keep hook call order stable +const WORKOS_ENABLED = isWorkOSEnabled(); + +type AppAuthState = { user: unknown | null; loading: boolean }; + +export const useAppAuth: () => AppAuthState = + WORKOS_ENABLED + ? () => { + const { user, isLoading } = useAuth(); + // Normalize to a consistent shape + return { user: user ?? null, loading: Boolean(isLoading) }; + } + : () => ({ user: null, loading: false } as const);app/api/chat/route.ts (1)
18-18: Ensure barrel exports for types resolve correctlyThe import from "@/types" requires ChatMode and ExecutionMode to be re-exported from a types barrel (e.g., types/index.ts). If not present, this import will fail.
Run this to verify the barrel re-exports exist and are correct:
#!/bin/bash set -euo pipefail echo "Look for ChatMode and ExecutionMode type definitions:" rg -n -C1 -g 'types/**' '\bexport\s+type\s+ChatMode\b|\bexport\s+type\s+ExecutionMode\b' echo -e "\nLook for barrel re-exports under types/ (e.g., index.ts):" rg -n -C1 -g 'types/**' $'export\\s+(type\\s+)?\\{?\\s*ChatMode\\s*(,\\s*ExecutionMode)?\\s*\\}?\\s+from'
🧹 Nitpick comments (6)
app/hooks/useAppAuth.ts (2)
1-3: Mark as a client module to prevent accidental server imports.This hook depends on client-only APIs. Adding "use client" avoids accidental server-side imports and clarifies intent.
+ "use client"; import { useAuth } from "@workos-inc/authkit-nextjs/components"; import { isWorkOSEnabled } from "@/lib/auth-config";
12-17: Ensure consistent return shape: align “loading” vs potential “isLoading”.You return the entire WorkOS auth object in WorkOS mode and a { user, loading } stub in anonymous mode. If WorkOS uses isLoading (common), consumers may see different shapes across modes. Normalize the return to { user, loading } as shown in the refactor above.
Do your consumers rely on loading vs isLoading? If yes, confirm the WorkOS hook returns isLoading and normalize in this hook.
app/posthog.js (1)
3-17: Make PostHog client a singleton, add Edge-runtime guard, and prefer server-side env vars with sensible defaults
- Avoid instantiating a new client per request; reuse a single instance to reduce socket/GC churn.
- posthog-node isn’t Edge-compatible; guard to return null on Edge.
- Prefer server-side env names (POSTHOG_API_KEY/POSTHOG_HOST) with fallback to NEXT_PUBLIC_ vars. Also default host to https://app.posthog.com so host isn’t mandatory.
Apply this diff:
-export default function PostHogClient() { - if ( - !process.env.NEXT_PUBLIC_POSTHOG_KEY || - !process.env.NEXT_PUBLIC_POSTHOG_HOST - ) { - return null; - } - - const posthogClient = new PostHog(process.env.NEXT_PUBLIC_POSTHOG_KEY, { - host: process.env.NEXT_PUBLIC_POSTHOG_HOST, - flushAt: 1, - flushInterval: 0, - }); - return posthogClient; -} +export default function PostHogClient() { + // Guard: posthog-node is not supported in Edge runtime + if (process.env.NEXT_RUNTIME === "edge") return null; + + // Prefer server-side vars, fall back to NEXT_PUBLIC_* if provided + const apiKey = + process.env.POSTHOG_API_KEY || process.env.NEXT_PUBLIC_POSTHOG_KEY; + if (!apiKey) return null; + + const host = + process.env.POSTHOG_HOST || + process.env.NEXT_PUBLIC_POSTHOG_HOST || + "https://app.posthog.com"; + + // Reuse a single instance across invocations + const g = globalThis; + if (!g.__posthogClient) { + g.__posthogClient = new PostHog(apiKey, { + host, + flushAt: 1, + flushInterval: 0, + }); + } + return g.__posthogClient; +}.env.local.example (1)
58-61: Add server-side PostHog envs and end with a newline
- Include server-side variables used by posthog-node to avoid relying on NEXT_PUBLIC_* (which are intended for client).
- Add a trailing newline to satisfy dotenv linters.
Apply this diff:
# PostHog analytics configuration # Get started at: https://posthog.com/ # NEXT_PUBLIC_POSTHOG_KEY="phc_your_project_key_here" # NEXT_PUBLIC_POSTHOG_HOST="https://app.posthog.com" +# +# Server-side (Node) PostHog configuration (used by posthog-node) +# POSTHOG_API_KEY="your_project_api_key" +# POSTHOG_HOST="https://app.posthog.com" +app/api/chat/route.ts (2)
37-39: Harden executionMode parsing to only accept known valuesCasting the env var to ExecutionMode will accept any string at runtime. Explicitly map to "sandbox" | "local" to avoid unexpected values.
Apply this diff:
- const executionMode: ExecutionMode = - (process.env.TERMINAL_EXECUTION_MODE as ExecutionMode) || "local"; + const envExec = process.env.TERMINAL_EXECUTION_MODE; + const executionMode: ExecutionMode = + envExec === "sandbox" ? "sandbox" : "local";
27-35: Validate request payload (mode/messages) before usereq.json() is untyped at runtime. Consider validating:
- mode ∈ {"agent","ask"} (fallback to "agent" if invalid)
- messages is an array with expected shape (or reject with a 400)
This prevents malformed input from affecting analytics names and tool selection.
I can add a small zod schema here and wire it to respond 400 on validation errors. Want me to draft it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.env.local.example(1 hunks).gitignore(1 hunks)app/api/chat/route.ts(1 hunks)app/hooks/useAppAuth.ts(1 hunks)app/page.tsx(4 hunks)app/posthog.js(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- app/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/hooks/useAppAuth.ts (1)
lib/auth-config.ts (1)
isWorkOSEnabled(15-16)
app/api/chat/route.ts (13)
types/chat.ts (1)
ChatMode(1-1)lib/auth-utils.ts (1)
getUserID(9-19)lib/rate-limit.ts (1)
checkRateLimit(6-63)types/agent.ts (1)
ExecutionMode(20-20)lib/token-utils.ts (1)
truncateMessagesToTokenLimit(24-44)lib/ai/providers.ts (1)
myProvider(4-13)app/posthog.js (1)
PostHogClient(3-17)lib/ai/tools/index.ts (1)
createTools(13-51)lib/actions/index.ts (1)
generateTitleFromUserMessage(17-44)lib/system-prompt.ts (1)
systemPrompt(11-95)lib/ai/tools/utils/sandbox-manager.ts (1)
getSandbox(16-37)lib/ai/tools/utils/sandbox.ts (1)
pauseSandbox(152-169)lib/errors.ts (1)
ChatSDKError(36-74)
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 61-61: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (3)
app/hooks/useAppAuth.ts (1)
1-2: Confirmed correct import path foruseAuthThe official WorkOS AuthKit Next.js docs specify that the
useAuthhook must be imported from@workos-inc/authkit-nextjs/components, not the package root. Your current import is accurate and no change is needed.Reference: https://workos.com/docs/authkit/nextjs
app/api/chat/route.ts (2)
54-128: LGTM: Streaming pipeline, tool wiring, and cleanup look solid
- createTools integrates writer and execution mode cleanly.
- Parallel title generation is isolated and awaited in both onError/onFinish to avoid leaks.
- Sandbox pause on both error and finish is good hygiene.
45-52: Non-blocking analytics is fine; ensure misconfig doesn’t affect chat flowCalls to posthog.capture are guarded and non-blocking. With the singleton client and immediate flush config, this should be reliable in Node runtime. No action required.
Also applies to: 98-105
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (5)
3-3: Fix MD036: use a heading instead of emphasis for the subtitleThe subtitle is currently italicized; markdownlint flags this (MD036). Make it a heading.
-_Your AI-Powered Penetration Testing Assistant_ +### Your AI-Powered Penetration Testing Assistant
9-13: Add a brief note for users without pnpm installedMany users won’t have pnpm preinstalled. A quick Corepack snippet reduces setup friction.
### 1. Install Dependencies ```bash pnpm install+> Note: If pnpm isn’t installed, enable it via Corepack:
+>
+>bash +> corepack enable +> corepack prepare pnpm@latest --activate +>--- `15-27`: **Clarify environment configuration beyond OpenRouter (point to other optional variables)** Make it explicit that .env.local.example lists additional optional variables (auth mode, sandbox, rate limiting, analytics). ```diff -Add your OpenRouter API key: +Add your OpenRouter API key (required). You can also configure optional variables such as auth mode, sandbox, rate limiting, and analytics (see `.env.local.example` for the full list): ```env OPENROUTER_API_KEY=your_openrouter_api_key_here--- `53-61`: **Add a security disclaimer for local command execution** Since the default runs terminal commands locally, explicitly warn users about the risks. ```diff > 💡 **Default Behavior**: Terminal commands execute locally on your machine > 🔒 **Sandbox Mode**: Add E2B key for isolated container execution +> ⚠️ **Security Note**: Local execution can modify your system. Only run commands you understand and trust. Use Sandbox Mode for isolation when in doubt. #### Enable Sandbox Mode
39-62: Document optional configuration for auth mode, rate limiting, and analyticsGiven the broader feature set, consider adding concise optional sections so users discover these capabilities without digging.
## 🔑 API Configuration ### Required @@ #### Enable Sandbox Mode @@ E2B_API_KEY=your_e2b_api_key_here
+### Optional – Authentication Mode
+Switch between anonymous and WorkOS-based auth:
+env +NEXT_PUBLIC_AUTH_MODE=anonymous # or "workos" +NEXT_PUBLIC_WORKOS_REDIRECT_URI=http://localhost:3000/auth/callback +
+
+### Optional – Rate Limiting (Upstash)
+Enable per-user rate limiting:
+env +UPSTASH_REDIS_REST_URL=... +UPSTASH_REDIS_REST_TOKEN=... +
+
+### Optional – Analytics (PostHog)
+Enable product analytics:
+env +NEXT_PUBLIC_POSTHOG_KEY=... +NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com +To ensure these variables are actually supported in this branch and present in `.env.local.example`, please verify before merging. If you’d like, I can provide a quick script to scan the repo for usage and the example file entries. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d4aaed0f6ec1cc1503fbfc76e06b28b219497c11 and 0bb37f98763e97712497bb3a006d77d0799fdcc1. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `README.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [grammar] ~7-~7: There might be a mistake here. Context: ...sting Assistant_ --- ## 🚀 Quick Start ### 1. Install Dependencies ```bash pnpm ins... (QB_NEW_EN) --- [grammar] ~35-~35: There might be a mistake here. Context: ...art your penetration testing journey! 🎯 --- ## 🔑 API Configuration ### Required | Ser... (QB_NEW_EN) --- [grammar] ~39-~39: There might be a mistake here. Context: ...ourney! 🎯 --- ## 🔑 API Configuration ### Required | Service | Purpose ... (QB_NEW_EN) --- [grammar] ~49-~49: There might be a mistake here. Context: ... | Get API Key | | ------- | ------------------------- | ... (QB_NEW_EN) --- [grammar] ~50-~50: There might be a mistake here. Context: ...-------- | --------------------------- | | **E2B** | Secure isolated execution | ... (QB_NEW_EN) --- [grammar] ~54-~54: There might be a mistake here. Context: ...nds execute locally on your machine > 🔒 **Sandbox Mode**: Add E2B key for isola... (QB_NEW_EN) --- [grammar] ~56-~56: There might be a mistake here. Context: ... for isolated container execution #### Enable Sandbox Mode ```env TERMINAL_EXECU... (QB_NEW_EN) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 3-3: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>README.md (1)</summary> `31-35`: **Quick Start flow reads well** Install → configure → run is straightforward, and the launch instructions are clear. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/hooks/useAppAuth.ts (1)
8-18: Bug: unconditional useAuth() can crash in anonymous mode (no provider)Even though this satisfies the Rules of Hooks, calling useAuth when WorkOS is disabled can throw if the AuthKit provider isn’t mounted. Gate the hook at module scope so it’s never called in anonymous mode. Also mark the file as a client module.
Apply:
+ "use client"; import { useAuth } from "@workos-inc/authkit-nextjs/components"; import { isWorkOSEnabled } from "@/lib/auth/client"; /** * Custom hook that handles authentication for both WorkOS and anonymous modes * Returns consistent user/loading state regardless of auth mode */ -export const useAppAuth = () => { - // Always call useAuth to comply with rules of hooks - const workosAuth = useAuth(); - - if (isWorkOSEnabled()) { - return workosAuth; - } - - // Anonymous mode - no authentication required - return { user: null, loading: false }; -}; +// Resolve mode once at module load to keep hook call order stable and avoid calling hooks in anonymous mode +const WORKOS_ENABLED = isWorkOSEnabled(); + +export const useAppAuth = + WORKOS_ENABLED + ? () => useAuth() + : () => ({ user: null, loading: false } as const);Follow-up: verify the returned shape matches your consumers. If useAuth returns { user, isLoading }, consider mapping to { user, loading: isLoading } for consistency across modes.
app/logout/route.ts (1)
5-12: LGTM: returning signOut() preserves Set-Cookie + redirectGood fix. Returning the Response from signOut avoids overwriting cookie-clearing behavior. The WorkOS gate + fallback redirect are clear.
🧹 Nitpick comments (2)
lib/auth/client.ts (1)
9-16: Optional: expose a typed mode getter and a constant for tree-shakingThis avoids repeated env checks and gives a single source of truth for the mode. It also enables module-scope gating elsewhere without re-evaluating.
Apply:
export type AuthMode = "workos" | "anonymous"; /** * Check if WorkOS authentication is enabled * Works on both server and client side */ -export const isWorkOSEnabled = (): boolean => - process.env.NEXT_PUBLIC_AUTH_MODE === "workos"; +export const getAuthMode = (): AuthMode => + process.env.NEXT_PUBLIC_AUTH_MODE === "workos" ? "workos" : "anonymous"; + +export const WORKOS_ENABLED: boolean = getAuthMode() === "workos"; + +export const isWorkOSEnabled = (): boolean => WORKOS_ENABLED;lib/auth/server.ts (1)
16-28: Optional: derive a per-request anonymous ID (hashed IP) for fair rate limitingReturning the same "anonymous" ID collapses all unauthenticated users into one bucket, weakening per-user rate limits and observability. Consider deriving a privacy-preserving, stable anonymous ID per client (e.g., a short SHA-256 of the IP).
Apply:
import type { NextRequest } from "next/server"; import { isWorkOSEnabled } from "@/lib/auth/client"; +import { createHash } from "crypto"; @@ export const getUserID = async (req: NextRequest): Promise<string> => { - if (!isWorkOSEnabled()) return "anonymous"; + if (!isWorkOSEnabled()) return getAnonymousUserID(req); @@ } catch (error) { console.error("Failed to get user session:", error); - return "anonymous"; + return getAnonymousUserID(req); } }; + +const getAnonymousUserID = (req: NextRequest): string => { + const ip = + // Next/Vercel: NextRequest.ip may be populated; fall back to standard headers + // Note: header values can contain multiple IPs — use the first + // Avoid storing raw IP; hash it for privacy + // If none available, use "unknown" to keep a stable, low-privilege bucket + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (req as any).ip || + req.headers.get("x-forwarded-for")?.split(",")[0]?.trim() || + req.headers.get("x-real-ip") || + "unknown"; + const hash = createHash("sha256").update(ip).digest("hex").slice(0, 16); + return `anon_${hash}`; +};If strict privacy requirements exist, consider salting the hash with a secret and rotating periodically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
app/api/chat/route.ts(1 hunks)app/callback/route.ts(1 hunks)app/components/Header.tsx(1 hunks)app/hooks/useAppAuth.ts(1 hunks)app/layout.tsx(2 hunks)app/login/route.ts(1 hunks)app/logout/route.ts(1 hunks)app/page.tsx(4 hunks)app/signup/route.ts(1 hunks)lib/auth-utils.ts(0 hunks)lib/auth/client.ts(1 hunks)lib/auth/server.ts(1 hunks)middleware.ts(1 hunks)
💤 Files with no reviewable changes (1)
- lib/auth-utils.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- app/signup/route.ts
- app/login/route.ts
- app/layout.tsx
- middleware.ts
- app/components/Header.tsx
- app/api/chat/route.ts
- app/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
lib/auth/server.ts (1)
lib/auth/client.ts (1)
isWorkOSEnabled(15-16)
app/hooks/useAppAuth.ts (1)
lib/auth/client.ts (1)
isWorkOSEnabled(15-16)
app/callback/route.ts (4)
app/login/route.ts (1)
GET(5-14)app/logout/route.ts (1)
GET(5-12)app/signup/route.ts (1)
GET(5-14)lib/auth/client.ts (1)
isWorkOSEnabled(15-16)
app/logout/route.ts (4)
app/callback/route.ts (1)
GET(7-15)app/login/route.ts (1)
GET(5-14)app/signup/route.ts (1)
GET(5-14)lib/auth/client.ts (1)
isWorkOSEnabled(15-16)
🔇 Additional comments (2)
lib/auth/client.ts (1)
15-16: LGTM: simple, client-safe gating worksUsing NEXT_PUBLIC env to gate WorkOS usage is correct and safe for client imports.
app/callback/route.ts (1)
7-15: LGTM: module-scope gating keeps the handler staticChoosing the handler at module load via isWorkOSEnabled() is clean and avoids per-request branching. Redirect fallback looks correct.
Summary by CodeRabbit
New Features
Refactor
Documentation
Style
Chores