Fix all cyclical dependencies in repo, resolve all typecheck issues in all packages, set up sherif to avoid future issues due to package mismatches #2966
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughConsolidates chat types into a local UI module, migrates many import paths to internal aliases, tightens TypeScript configs and deps, removes the legacy AI package/penpal exports, adds penpal/iframe safe fallbacks, and adds defensive null-safety guards in UI and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Window as Main Window
participant Preload as Preload Script
participant Iframe as IFrame Child
Note over Window,Preload: initialization & binding
Window->>Preload: register penpalParent (typed PromisifiedPenpalParentMethods)
Preload->>Iframe: attach channel / handshake
Iframe->>Preload: call method (e.g., captureScreenshot)
alt remote method exists
Preload->>Window: invoke real method -> Promise resolves
Window-->>Iframe: result
else remote method missing
Preload-->>Iframe: return safeFallback response (handled error, no crash)
end
Note right of Preload: promisified wrappers catch errors and return safe responses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 52
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
apps/web/client/src/app/callback/github/install/page.tsx (2)
24-78: Critical: Fix React exhaustive-deps violation.The useEffect has an empty dependency array but references
searchParams,handleInstallationCallback,setState, andsetMessage. This violates React's exhaustive-deps rule and can cause stale closures or missed updates.Apply this diff to fix the dependency array:
- }, []); + }, [searchParams, handleInstallationCallback]);Note:
setStateandsetMessageare stable fromuseStateand don't need to be included.
16-220: Avoid hardcoded user-facing text; use next-intl.All user-facing strings (titles, descriptions, button labels, error messages) are hardcoded, violating the coding guidelines for this path pattern. Use next-intl messages/hooks for i18n support.
As per coding guidelines for
apps/web/client/src/**/*.tsx: "Avoid hardcoded user-facing text; use next-intl messages/hooks instead."Examples of hardcoded text that should be externalized:
- Line 37:
'Missing installation_id parameter'- Line 43:
'Missing setup_action parameter'- Line 49:
'Missing state parameter'- Lines 166-167:
'Connecting to GitHub',"We're setting up your integration"- Lines 177-178:
'All set!','Your GitHub account is now connected'- Lines 188-189:
'Something went wrong'- Lines 198, 207: Button labels
'Try Again','Return to Import'apps/web/client/src/app/_components/landing-page/feature-blocks/responsive-website.tsx (1)
75-167: Replace hardcoded user-facing text with next-intl messages.The component contains extensive hardcoded user-facing text throughout (e.g., "Ceramix", "Shop", "Contact", "About", "Le Fidgette", "VIEW WORK", feature descriptions, and the "Instantly responsive" section). Replace these strings with next-intl message keys for proper internationalization support.
As per coding guidelines
Example implementation using next-intl:
import { useTranslations } from 'next-intl'; export function ResponsiveWebsiteBlock() { const t = useTranslations('landingPage.responsiveWebsite'); // ... rest of component // Then use like: <h2>{t('brandName')}</h2> <span>{t('nav.shop')}</span> // etc. }apps/web/client/src/app/about/page.tsx (1)
92-875: Replace hardcoded user-facing text with next-intl messages.The component contains extensive hardcoded user-facing text throughout (titles, descriptions, button labels, etc.). Per coding guidelines, all user-facing text should use next-intl messages/hooks for internationalization support.
Examples of hardcoded text that should be replaced:
- Lines 102-104: "Design deserves better tools"
- Lines 119-120: "Stars on GitHub"
- Lines 149-163: Description paragraphs
- Lines 274, 475, 540, 663, 739: Button text "Read more on Substack", "Claim your desk", "Browse open roles", "Take the leap"
- Lines 290-299, 339-346: Founder descriptions
- And many more throughout the component
Consider creating a messages file for the about page and using the
useTranslationshook:import { useTranslations } from 'next-intl'; export default function AboutPage() { const t = useTranslations('about'); // ... return ( // ... <h1>{t('hero.title')}</h1> // ... ); }As per coding guidelines
apps/web/client/src/app/_components/landing-page/feature-blocks/revision-history.tsx (1)
58-62: Replace hardcoded user-facing text with next-intl messages.Multiple user-facing strings are hardcoded throughout this component, violating the coding guideline: "Avoid hardcoded user-facing text; use next-intl messages/hooks instead." This prevents internationalization of the landing page.
Hardcoded text includes:
- Line 58-62: Version titles and subtitles in
todayVersionsarray- Line 108: "Versions"
- Line 112: "Today"
- Line 125: "Yesterday"
- Line 150: "Revision history"
- Line 155: "Never lose your progress – revert when you need to"
Example refactor using next-intl:
+import { useTranslations } from 'next-intl'; + export function RevisionHistory() { + const t = useTranslations('landing.revisionHistory'); + // ... const todayVersions = [ - { title: 'New typography and layout', subtitle: 'Alessandro · 3h ago' }, - { title: 'Update colors', subtitle: 'Jonathan · 10h ago' }, + { title: t('versions.0.title'), subtitle: t('versions.0.subtitle') }, + { title: t('versions.1.title'), subtitle: t('versions.1.subtitle') }, // ... ]; // ... - <p className="...">Versions</p> + <p className="...">{t('versionsHeader')}</p> - <div className="...">Today</div> + <div className="...">{t('today')}</div> - <span className="...">Revision history</span> + <span className="...">{t('title')}</span> - <p className="...">Never lose your progress – revert when you need to</p> + <p className="...">{t('description')}</p>As per coding guidelines.
Also applies to: 108-108, 112-112, 125-125, 150-150, 155-155
apps/web/client/src/app/project/[id]/_components/bottom-bar/restart-sandbox-button.tsx (1)
106-121: Guard possible undefined frames and reset state on failed restartAvoid runtime error if no frames; also clear restarting when restartDevServer returns false.
- setTimeout(() => { - const frames = editorEngine.frames.getByBranchId(activeBranch.id); - frames.forEach((frame) => { + setTimeout(() => { + const frames = editorEngine.frames.getByBranchId(activeBranch.id); + if (frames) { + frames.forEach((frame) => { try { editorEngine.frames.reloadView(frame.frame.id); } catch (frameError) { console.error('Failed to reload frame:', frame.frame.id, frameError); } - }); + }); + } toast.success('Sandbox restarted successfully', { icon: <Icons.Cube className="h-4 w-4" />, }); setRestarting(false); }, 5000); } else { - toast.error('Failed to restart sandbox'); + toast.error('Failed to restart sandbox'); + setRestarting(false); }apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/measurement.tsx (1)
92-94: Use adjusted rects consistently and remove unnecessary scaling
- Replace the
useMemocalls at lines 92–94 to usefromRectAdjusted/toRectAdjustedinstead of rawfromRect/toRect- At line 127, change
const scaleValue = (value: number) => adaptValueToCanvas(Math.abs(value), true);
to
const scaleValue = (value: number) => Math.abs(value);- In the SVG (around lines 389–409), update the
<rect>props to usefromRectAdjusted/toRectAdjustedapps/web/client/src/app/_components/landing-page/feature-blocks/layers.tsx (1)
83-139: Make interactive rows accessibleDon’t use
as a button. Use (or add role="button" + keyboard handlers). Provide visible focus styles.- <div + <button + type="button" key={layer.id} className={cn( - 'flex h-5.5 cursor-pointer items-center px-2 text-xs transition-colors select-none', + 'flex h-5.5 items-center px-2 text-xs transition-colors select-none focus:outline-none focus-visible:ring-2 focus-visible:ring-foreground-primary/40 focus-visible:ring-offset-2 ring-offset-background', /* ...rest unchanged... */ )} onMouseEnter={() => setHoveredId(layer.id)} onMouseLeave={() => setHoveredId(null)} onClick={() => setSelectedId(layer.id)} style={{ userSelect: 'none' }} > <div style={{ width: `${layer.level * 16}px` }} /> <NodeIcon iconClass="w-3.5 h-3.5 mr-1.5" tagName={layer.tagName} /> <span className="truncate">{layer.name}</span> - </div> + </button>apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
1-345: Missing 'use client' directive for hooks usage.This component uses React hooks (
useState,useEffect,useCallback,useRef) but lacks the'use client'directive at the top of the file. Server Components cannot use hooks, which will cause a runtime error.Apply this diff to add the required directive:
+' use client'; + import React, { useCallback, useEffect, useRef, useState } from 'react';As per coding guidelines
| <div className="xs:text-lg text-foreground-secondary my-2 px-2 text-center text-base font-light"> | ||
| Onlook is optimized for larger screens | ||
| </div> | ||
| <Button | ||
| size="sm" | ||
| onClick={handleShowEmailForm} | ||
| className="bg-foreground-primary text-background-primary w-full hover:bg-foreground-hover hover:text-background-primary h-auto py-3 text-base xs:text-lg whitespace-normal leading-tight" | ||
| className="bg-foreground-primary text-background-primary hover:bg-foreground-hover hover:text-background-primary xs:text-lg h-auto w-full py-3 text-base leading-tight whitespace-normal" | ||
| > | ||
| Email me a link for later | ||
| </Button> |
There was a problem hiding this comment.
Replace hardcoded user-facing strings with next-intl translations.
Use next-intl per app guidelines. Example:
- import React, { useCallback, useEffect, useRef, useState } from 'react';
+ import React, { useCallback, useEffect, useRef, useState } from 'react';
+ import { useTranslations } from 'next-intl';
...
export function MobileEmailCapture() {
+ const t = useTranslations('hero.emailCapture');
...
- setError('Please fill in all fields');
+ setError(t('errors.missingFields'));
...
- setError('Please enter a valid email address');
+ setError(t('errors.invalidEmail'));
...
- if (error.name === 'AbortError') { setError('Request timed out. Please check your connection and try again.'); }
+ if (error.name === 'AbortError') { setError(t('errors.timeout')); }
...
- Onlook is optimized for larger screens
+ {t('optimizedForLargeScreens')}
...
- Email me a link for later
+ {t('emailLinkForLater')}
...
- Thanks, an email to use Onlook has been sent to you!
+ {t('success')}
...
- Email me a link to Onlook
+ {t('title')}
...
- Name
+ {t('name.label')}
...
- placeholder="Pablo Picasso"
+ placeholder={t('name.placeholder')}
...
- Email
+ {t('email.label')}
...
- placeholder="Enter your email"
+ placeholder={t('email.placeholder')}
...
- {isSubmitting ? 'Submitting...' : 'Email me a link'}
+ {isSubmitting ? t('buttons.submitting') : t('buttons.emailMeALink')}Add corresponding messages in your locale JSON. As per coding guidelines.
Also applies to: 373-375, 379-383, 391-399, 412-420, 482-483, 107-115, 171-179, 181-182
| @@ -1,110 +1,134 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
Add 'use client' directive.
This component uses useState (line 60) and onClick event handlers (line 82), which require client-side execution. According to the coding guidelines for files in apps/web/client/src/app/**/*.tsx, you must add the 'use client' directive at the top of the file when using events, state/effects, or browser APIs.
As per coding guidelines.
Apply this diff:
+'use client';
+
import React, { useState } from 'react';📝 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.
| import React, { useState } from 'react'; | |
| 'use client'; | |
| import React, { useState } from 'react'; |
🤖 Prompt for AI Agents
apps/web/client/src/app/_components/landing-page/code-one-to-one-section.tsx
lines 1-1: This component uses React state and client event handlers but is
missing the required 'use client' directive; add a single line containing 'use
client' (exactly that text) as the very first line of the file before any
imports so the file runs on the client and React hooks/events work correctly.
| Truly one-to-one | ||
| <br /> | ||
| with code |
There was a problem hiding this comment.
Replace hardcoded text with next-intl messages.
The user-facing text "Truly one-to-one" and "with code" is hardcoded. According to the coding guidelines, you should avoid hardcoded user-facing text and use next-intl messages/hooks instead for proper internationalization support.
As per coding guidelines.
🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/landing-page/code-one-to-one-section.tsx
around lines 66 to 68, the user-facing strings "Truly one-to-one" and "with
code" are hardcoded; replace them with next-intl translation lookups: import and
call the useTranslations hook at the top of the component (e.g., const t =
useTranslations('Landing.CodeOneToOne')), replace the static JSX text with
t('titleLine1') and t('titleLine2') (or a single t('title') that includes a
newline/linebreak), and add corresponding keys and localized values to the
next-intl message files for all supported locales; ensure the JSX still renders
the <br/> where needed and update any tests or snapshots if present.
| <div className="mb-1 grid cursor-pointer grid-cols-6 gap-0.5"> | ||
| {colorClasses.slice(0, 6).map((cls, i) => ( | ||
| <div key={cls + i} className={`w-8 h-8 rounded-md border-[1px] border-foreground-primary/20 hover:border-foreground-primary/50 ${cls}`} /> | ||
| <div | ||
| key={cls + i} | ||
| className={`border-foreground-primary/20 hover:border-foreground-primary/50 h-8 w-8 rounded-md border-[1px] ${cls}`} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Consider a more stable key and clarify cursor behavior.
Two minor observations:
-
Key construction:
cls + iconcatenates the class name with the index. IfcolorClassescontains duplicate values, this could produce identical keys. Since you're slicing a fixed portion of the array, using justiori + 6(for the second grid) would be simpler and equally stable. -
Cursor pointer without handler: The grid container has
cursor-pointerbut noonClickhandler. If individual swatches should be clickable, movecursor-pointerto the swatch divs. If the entire grid is meant to be interactive, add the corresponding handler.
Consider this diff for the key:
- key={cls + i}
+ key={i}🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx
around lines 12 to 18, the current key uses `cls + i` which can collide if class
names repeat and the grid container has `cursor-pointer` without any click
handler. Replace the key with the stable index (e.g., `i` or `i + offset` for
the second row) to ensure uniqueness, and move `cursor-pointer` from the
container to each swatch div if individual swatches are meant to be clickable;
otherwise remove `cursor-pointer` from the container and add a single onClick
handler to the container if the whole grid should be interactive.
| import type { IframeHTMLAttributes } from 'react'; | ||
| import { forwardRef, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react'; | ||
| import { observer } from 'mobx-react-lite'; | ||
| import { connect, WindowMessenger } from 'penpal'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Align penpal imports to a single source
You import runtime APIs from 'penpal' and types/constants from '@onlook/penpal'. This can cause version/shape mismatches.
- If connect/WindowMessenger are provided by our internal penpal package, import them from '@onlook/penpal'.
- If you intend to use the external 'penpal' lib, also take types from it for consistency.
Run to confirm sources:
Also applies to: 9-15
🏁 Script executed:
#!/bin/bash
rg -nP "from ['\"]penpal['\"]|from ['\"]@onlook/penpal['\"]" -C2Length of output: 1803
Align penpal imports to a single source
In both apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx (line 6) and apps/web/preload/script/index.ts (line 2), you import { connect, WindowMessenger } from 'penpal' but types/constants from '@onlook/penpal'. Replace the runtime import with:
import { connect, WindowMessenger } from '@onlook/penpal';to ensure API and type consistency.
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx around
line 6, the runtime import pulls connect and WindowMessenger from 'penpal' while
project types/constants use '@onlook/penpal'; update the import source to
'@onlook/penpal' (i.e., replace the 'penpal' module specifier with
'@onlook/penpal') and verify there are no other imports from 'penpal' in this
file so runtime API and types remain consistent.
| const messenger = new WindowMessenger({ | ||
| remoteWindow: iframeRef.current.contentWindow, | ||
| allowedOrigins: ['*'], | ||
| }); | ||
|
|
There was a problem hiding this comment.
Avoid wildcard origins in WindowMessenger (security posture)
allowedOrigins: ['*'] weakens postMessage origin checks. Prefer an explicit allowlist (e.g., the origin of frame.url).
Example:
- const messenger = new WindowMessenger({
- remoteWindow: iframeRef.current.contentWindow,
- allowedOrigins: ['*'],
- });
+ const origin = (() => {
+ try {
+ return new URL(frame.url, window.location.href).origin;
+ } catch {
+ return undefined;
+ }
+ })();
+ const messenger = new WindowMessenger({
+ remoteWindow: iframeRef.current.contentWindow,
+ allowedOrigins: origin ? [origin] : [],
+ });🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx around
lines 119 to 123, the WindowMessenger is currently created with allowedOrigins:
['*'] which is insecure; replace the wildcard with an explicit origin derived
from the iframe URL (e.g., parse frame.url with new URL(frame.url).origin) and
pass that single-origin string to allowedOrigins, ensuring iframeRef.current and
frame.url exist before creating the messenger; also handle URL parsing errors
(throw or log and abort messenger creation) and fall back to a safe default or
refuse to instantiate if the origin cannot be determined.
| onDomProcessed: (data: { | ||
| layerMap: Record<string, any>; | ||
| rootNode: any; | ||
| }) => { | ||
| editorEngine.frameEvent.handleDomProcessed(frame.id, data); | ||
| }, | ||
| } satisfies PenpalParentMethods, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any with concrete types in onDomProcessed payload
Avoid any. Use the LayerNode type used by PenpalParentMethods.
Apply:
- onDomProcessed: (data: {
- layerMap: Record<string, any>;
- rootNode: any;
- }) => {
+ onDomProcessed: (data: {
+ layerMap: Record<string, LayerNode>;
+ rootNode: LayerNode;
+ }) => {
editorEngine.frameEvent.handleDomProcessed(frame.id, data);
},Add import:
-import { PENPAL_PARENT_CHANNEL } from '@onlook/penpal';
+import { PENPAL_PARENT_CHANNEL, type LayerNode } from '@onlook/penpal';As per coding guidelines.
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx around
lines 135-141, the onDomProcessed payload currently uses any for layerMap and
rootNode; replace those anys with the concrete LayerNode type used by
PenpalParentMethods (e.g. layerMap: Record<string, LayerNode>; rootNode:
LayerNode). Add an import for LayerNode from the same module that exports
PenpalParentMethods (replace the placeholder path with the actual path where
PenpalParentMethods is defined), and ensure the signature still satisfies
PenpalParentMethods.
| const promisifyMethod = <T extends (...args: any[]) => any>( | ||
| method: T | undefined, | ||
| ): ((...args: Parameters<T>) => Promise<ReturnType<T>>) => { | ||
| return async (...args: Parameters<T>) => { | ||
| try { | ||
| if (!method) throw new Error('Method not initialized'); | ||
| return method(...args); | ||
| } catch (error) { | ||
| console.error( | ||
| `${PENPAL_PARENT_CHANNEL} (${frame.id}) - Failed to setup penpal connection:`, | ||
| `${PENPAL_PARENT_CHANNEL} (${frame.id}) - Method failed:`, | ||
| error, | ||
| ); | ||
| retrySetupPenpalConnection(error); | ||
| }); | ||
| } catch (error) { | ||
| isConnecting.current = false; | ||
| console.error('Failed to setup penpal connection', error); | ||
| retrySetupPenpalConnection(error as Error); | ||
| } | ||
| }; | ||
|
|
||
| const promisifyMethod = <T extends (...args: any[]) => any>( | ||
| method: T | undefined, | ||
| ): ((...args: Parameters<T>) => Promise<ReturnType<T>>) => { | ||
| return async (...args: Parameters<T>) => { | ||
| try { | ||
| if (!method) throw new Error('Method not initialized'); | ||
| return method(...args); | ||
| } catch (error) { | ||
| console.error(`${PENPAL_PARENT_CHANNEL} (${frame.id}) - Method failed:`, error); | ||
| } | ||
| } | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Don’t swallow errors in promisifyMethod; return type currently lies
promisifyMethod logs and returns undefined on error, violating the declared Promise<ReturnType> and causing silent failures.
Prefer rethrow for upstream handling:
return async (...args: Parameters<T>) => {
try {
if (!method) throw new Error('Method not initialized');
return method(...args);
} catch (error) {
console.error(
`${PENPAL_PARENT_CHANNEL} (${frame.id}) - Method failed:`,
error,
);
- }
+ throw error;
+ }
};🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx around
lines 189 to 203, the promisifyMethod helper catches errors, logs them and then
returns undefined which violates the declared Promise<ReturnType<T>> and hides
failures; change the implementation to not swallow errors — either remove the
try/catch so errors propagate, or after logging rethrow the caught error (or
return Promise.reject(error)) so callers receive the rejection; keep the method
existence check but if it throws, rethrow the Error('Method not initialized')
instead of returning undefined.
| import React, { memo, useMemo } from 'react'; | ||
|
|
There was a problem hiding this comment.
Missing 'use client' — hooks used in an app/ component
This component uses hooks and client-only store; mark it as a Client Component.
As per coding guidelines
+'use client';
+
import React, { memo, useMemo } from 'react';📝 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.
| import React, { memo, useMemo } from 'react'; | |
| 'use client'; | |
| import React, { memo, useMemo } from 'react'; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/measurement.tsx
lines 1-2, the component uses React hooks and client-only stores but is not
marked as a Client Component; add the "use client" directive as the very first
line of the file (before any imports) so Next.js treats this file as a client
component, and verify any imports used are client-safe.
| if (draft) { | ||
| try { | ||
| const { prompt, images } = draft; | ||
| // Only restore if draft is less than 1 hour old | ||
| setInputValue(prompt); | ||
| setSelectedImages(images); | ||
|
|
||
| // Clear the draft after restoring | ||
| await localforage.removeItem(SAVED_INPUT_KEY); | ||
| } catch (error) { | ||
| console.error('Error restoring draft:', error); | ||
| // Clear the draft after restoring | ||
| await localforage.removeItem(SAVED_INPUT_KEY); | ||
| } catch (error) { | ||
| console.error('Error restoring draft:', error); | ||
| } |
There was a problem hiding this comment.
Implement the 1‑hour draft age check (comment says so, code doesn’t)
Currently restores all drafts and clears them. Respect the age threshold.
- if (draft) {
+ if (draft) {
try {
- const { prompt, images } = draft;
- // Only restore if draft is less than 1 hour old
- setInputValue(prompt);
- setSelectedImages(images);
-
- // Clear the draft after restoring
- await localforage.removeItem(SAVED_INPUT_KEY);
+ const { prompt, images, timestamp } = draft;
+ const ONE_HOUR = 60 * 60 * 1000;
+ // Only restore if draft is less than 1 hour old
+ if (Date.now() - timestamp < ONE_HOUR) {
+ setInputValue(prompt);
+ setSelectedImages(images);
+ }
+ // Always clear stale draft
+ await localforage.removeItem(SAVED_INPUT_KEY);
} catch (error) {
console.error('Error restoring draft:', error);
}
}| const handleSubmit = async () => { | ||
| if (isInputInvalid) { | ||
| console.warn('Input is too short'); | ||
| return; | ||
| } | ||
| createProject(inputValue, selectedImages); | ||
| }; |
There was a problem hiding this comment.
Guard against duplicate submissions (Enter can bypass button disabled)
Pressing Enter can trigger multiple creates while already creating. Add a guard and await the call.
- const handleSubmit = async () => {
- if (isInputInvalid) {
+ const handleSubmit = async () => {
+ if (isCreatingProject || isInputInvalid) {
console.warn('Input is too short');
return;
}
- createProject(inputValue, selectedImages);
+ await createProject(inputValue, selectedImages);
};Also applies to: 321-326
🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/hero/create.tsx around lines 80-86 (and
similarly at 321-326), the submit handler can be invoked multiple times (Enter
key bypasses disabled button) causing duplicate creates; add a reentrancy guard
by checking a local/state "isCreating" flag at the top of the handler and
returning early if true, set isCreating = true before calling createProject and
await the async call, wrap the await in try/catch/finally to handle errors and
ensure isCreating is reset to false in finally, and ensure any UI disabled state
is driven by that same isCreating flag so Enter cannot trigger a second
submission while a create is in progress.
| const container = e.currentTarget.closest('.bg-background-secondary'); | ||
| if (container) { | ||
| container.setAttribute('data-dragging-image', 'false'); | ||
| } |
There was a problem hiding this comment.
Fix drag highlight: broken selector for data attribute; bind via JSX instead
closest('.bg-background-secondary') will not match 'bg-background-secondary/80', so the data attribute never toggles. Bind data-dragging-image directly and gate state via handleDragStateChange.
- // Find and reset the container's data attribute
- const container = e.currentTarget.closest('.bg-background-secondary');
- if (container) {
- container.setAttribute('data-dragging-image', 'false');
- }
+ // State drives highlight; attribute bound in JSX
- const hasImage =
+ const hasImage =
e.dataTransfer.types.length > 0 &&
Array.from(e.dataTransfer.items).some(
(item) =>
item.type.startsWith('image/') ||
(item.type === 'Files' && e.dataTransfer.types.includes('public.file-url')),
);
if (hasImage) {
setIsDragging(isDragging);
- // Find the container div with the bg-background-secondary class
- const container = e.currentTarget.closest('.bg-background-secondary');
- if (container) {
- container.setAttribute('data-dragging-image', isDragging.toString());
- }
}
- <div
+ <div
className={cn(
'flex cursor-text flex-col gap-3 rounded p-0 transition-colors duration-200',
- 'bg-background-secondary/80 backdrop-blur-sm',
- '[&[data-dragging-image=true]]:bg-teal-500/40',
- isDragging && 'cursor-copy bg-teal-500/40',
+ 'bg-background-secondary/80 backdrop-blur-sm',
+ // data-dragging-image is bound below; only add cursor change via state
+ isDragging && 'cursor-copy',
)}
+ data-dragging-image={isDragging}
onClick={handleContainerClick}
- onDragOver={handleDragOver}
- onDragLeave={handleDragLeave}
+ onDragOver={(e) => {
+ e.preventDefault();
+ handleDragStateChange(true, e);
+ }}
+ onDragLeave={(e) => {
+ e.preventDefault();
+ handleDragStateChange(false, e);
+ }}
onDrop={handleDrop}
>Also applies to: 226-231, 271-278
| @@ -1,93 +1,113 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify necessity of 'use client' directive.
This component doesn't use any hooks (useState, useEffect, etc.) or event handlers directly. While it imports client components (AiChatInteractive, DirectEditingInteractive), the parent doesn't require 'use client' unless it performs client-side operations.
Consider removing the directive to allow this to be a Server Component, letting Next.js automatically create client boundaries at the child components.
🏁 Script executed:
#!/bin/bash
# Check if BenefitsSection uses any client-side features
rg -A5 -B5 "useState|useEffect|useCallback|useMemo|useRef|onClick|onChange|onSubmit" apps/web/client/src/app/_components/landing-page/benefits-section.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if imported components are client components
for comp in AiChatInteractive DirectEditingInteractive TailwindColorEditorMockup; do
echo "=== $comp ==="
file=$(rg -l "$comp" -g "apps/web/client/src/**/*.tsx" | head -n1)
[[ -z "$file" ]] && { echo "Component file for $comp not found"; continue; }
echo "File: $file"
rg -n "use client" "$file" || echo "No 'use client' directive found"
doneLength of output: 385
Remove unnecessary client boundary and add missing one
- Remove the
'use client'directive fromapps/web/client/src/app/_components/landing-page/benefits-section.tsx(no direct client-only features). - Add a
'use client'directive toapps/web/client/src/app/_components/shared/mockups/direct-editing-interactive.tsxto mark that interactive component as a client component.
🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/landing-page/benefits-section.tsx around
line 1, remove the unnecessary 'use client' directive because this component has
no client-only features; simply delete that top-line directive. In
apps/web/client/src/app/_components/shared/mockups/direct-editing-interactive.tsx
add a 'use client' directive as the very first line of the file to mark this
interactive mockup as a client component so it can use hooks and browser APIs.
| <div key={faq.question} className="px-0 py-6"> | ||
| <button | ||
| className="flex items-center justify-between w-full text-left text-foreground-primary text-lg focus:outline-none cursor-pointer py-2" | ||
| className="text-foreground-primary flex w-full cursor-pointer items-center justify-between py-2 text-left text-lg focus:outline-none" |
There was a problem hiding this comment.
Keyboard focus is invisible (focus:outline-none on a button)
Don't remove the native focus ring without providing an alternative. Add a visible focus style (e.g., focus-visible:ring).
Example:
- className="text-foreground-primary flex w/full cursor-pointer items-center justify-between py-2 text-left text-lg focus:outline-none"
+ className="text-foreground-primary flex w/full cursor-pointer items-center justify-between py-2 text-left text-lg focus:outline-none focus-visible:ring-2 focus-visible:ring-foreground-primary/40 focus-visible:ring-offset-2 ring-offset-background"As per coding guidelines
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/landing-page/faq-dropdown.tsx around line
22, the button has class "focus:outline-none" which removes the browser's native
focus indicator; replace or augment it with an accessible visible focus style
(for example remove focus:outline-none or replace it with a focus-visible
utility such as "focus-visible:ring focus-visible:ring-[color]
focus-visible:ring-offset-2" and optionally "focus-visible:ring-offset-[bg]" to
ensure a clear high-contrast ring for keyboard users) so keyboard focus is
visible and meets accessibility guidelines.
| import { useState } from 'react'; | ||
| import { observer } from 'mobx-react-lite'; | ||
|
|
There was a problem hiding this comment.
Add 'use client' directive.
This component uses state/observer within app dir; must be a client component.
As per coding guidelines
+'use client';📝 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.
| import { useState } from 'react'; | |
| import { observer } from 'mobx-react-lite'; | |
| 'use client'; | |
| import { useState } from 'react'; | |
| import { observer } from 'mobx-react-lite'; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/canvas/frame/top-bar/branch.tsx
around lines 1-3, this component uses useState/observer and must be a client
component; add the "use client" directive as the very first line of the file
(before any imports), then keep the existing imports and code intact so React
client behavior is enabled.
| import type { ReactNode } from 'react'; | ||
| import { useHotkeys } from 'react-hotkeys-hook'; | ||
|
|
There was a problem hiding this comment.
Missing 'use client' — hooks used in app route component
This file uses hooks (useHotkeys) under app/* without a client directive. Must mark as a client component to avoid runtime/build errors.
+'use client';
+
import type { ReactNode } from 'react';
import { useHotkeys } from 'react-hotkeys-hook';As per coding guidelines.
📝 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.
| import type { ReactNode } from 'react'; | |
| import { useHotkeys } from 'react-hotkeys-hook'; | |
| 'use client'; | |
| import type { ReactNode } from 'react'; | |
| import { useHotkeys } from 'react-hotkeys-hook'; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/canvas/hotkeys/index.tsx
lines 1-3, the file imports and uses React hooks (useHotkeys) inside an app
route component but is missing the required "use client" directive; add a
top-of-file "use client" directive as the very first line so the component is
treated as a client component, then ensure any other client-only imports/usage
are compatible and rebuild.
| @@ -1,163 +1,174 @@ | |||
| import { useEditorEngine } from '@/components/store/editor'; | |||
| import { transKeys } from '@/i18n/keys'; | |||
| import type { SetStateAction } from 'react'; | |||
There was a problem hiding this comment.
Add 'use client' directive (required for hooks/observer).
This component uses React hooks, mobx observer, and next-intl. In app directory, it must be a client component.
As per coding guidelines
Apply this diff at the top:
+'use client';🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsx
at the top of the file (before any imports, around line 1), this component is
using hooks/observer/next-intl and must be a client component; add the "use
client" directive as the very first line of the file to mark it as a client
component so hooks and observer work correctly.
| const handleSubmit = async () => { | ||
| toast.promise( | ||
| async () => { | ||
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | ||
| void editorEngine.chat.sendMessage(inputState.value, ChatType.EDIT); | ||
| }, | ||
| { | ||
| loading: 'Sending message...', | ||
| success: 'Message sent', | ||
| error: 'Failed to send message', | ||
| }, | ||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn( | ||
| 'rounded-xl backdrop-blur-lg transition-all duration-300', | ||
| 'shadow-xl shadow-background-secondary/50', | ||
| inputState.isInputting | ||
| ? 'bg-background/80 border shadow-xl shadow-background-secondary/50 p-1' | ||
| : 'bg-background-secondary/85 dark:bg-background/85 border-foreground-secondary/20 hover:border-foreground-secondary/50 p-0.5', | ||
| 'border flex relative', | ||
| )} | ||
| > | ||
| {!inputState.isInputting ? ( | ||
| // Chat Button | ||
| <button | ||
| onClick={() => setInputState((prev) => ({ ...prev, isInputting: true }))} | ||
| className="rounded-lg hover:text-foreground-primary transition-colors px-2.5 py-1.5 flex flex-row items-center gap-2 w-full" | ||
| > | ||
| <Icons.Sparkles className="w-4 h-4" /> | ||
| <span className="text-mini !font-medium whitespace-nowrap"> | ||
| {t(transKeys.editor.panels.edit.tabs.chat.miniChat.button)} | ||
| </span> | ||
| </button> | ||
| ) : ( | ||
| // Input Field | ||
| <div className="flex flex-row items-center gap-1 w-[280px] relative"> | ||
| <Button | ||
| size="icon" | ||
| onClick={() => | ||
| setInputState((prev) => ({ | ||
| ...prev, | ||
| isInputting: false, | ||
| value: '', | ||
| })) | ||
| } | ||
| className={cn( | ||
| 'group h-6 w-6 absolute left-1 top-1 z-10 border-none shadow-none bg-transparent hover:bg-transparent', | ||
| 'transition-all duration-200', | ||
| inputState.value.trim().length >= DIMENSIONS.minCharsToSubmit | ||
| ? 'opacity-0 -translate-x-2 scale-75 pointer-events-none' | ||
| : 'opacity-100 translate-x-0 scale-100 pointer-events-auto', | ||
| )} | ||
| disabled={inputState.isSubmitting} | ||
| setInputState(DEFAULT_INPUT_STATE); | ||
| }; |
There was a problem hiding this comment.
Fix toast.promise flow: await sendMessage and handle submitting state.
Currently, sendMessage isn’t awaited inside toast.promise, so success/errors may be misreported; isSubmitting is never set.
As per coding guidelines
- const handleSubmit = async () => {
- toast.promise(
- async () => {
- editorEngine.state.rightPanelTab = EditorTabValue.CHAT;
- void editorEngine.chat.sendMessage(inputState.value, ChatType.EDIT);
- },
- {
- loading: 'Sending message...',
- success: 'Message sent',
- error: 'Failed to send message',
- },
- );
-
- setInputState(DEFAULT_INPUT_STATE);
- };
+ const handleSubmit = async () => {
+ if (inputState.isSubmitting) return;
+ setInputState((prev) => ({ ...prev, isSubmitting: true }));
+ editorEngine.state.rightPanelTab = EditorTabValue.CHAT;
+ const promise = editorEngine.chat.sendMessage(inputState.value, ChatType.EDIT);
+ try {
+ await toast.promise(promise, {
+ loading: t(transKeys.editor.panels.edit.tabs.chat.toast.loading),
+ success: t(transKeys.editor.panels.edit.tabs.chat.toast.success),
+ error: t(transKeys.editor.panels.edit.tabs.chat.toast.error),
+ });
+ } finally {
+ setInputState(DEFAULT_INPUT_STATE);
+ }
+ };📝 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 handleSubmit = async () => { | |
| toast.promise( | |
| async () => { | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | |
| void editorEngine.chat.sendMessage(inputState.value, ChatType.EDIT); | |
| }, | |
| { | |
| loading: 'Sending message...', | |
| success: 'Message sent', | |
| error: 'Failed to send message', | |
| }, | |
| ); | |
| return ( | |
| <div | |
| className={cn( | |
| 'rounded-xl backdrop-blur-lg transition-all duration-300', | |
| 'shadow-xl shadow-background-secondary/50', | |
| inputState.isInputting | |
| ? 'bg-background/80 border shadow-xl shadow-background-secondary/50 p-1' | |
| : 'bg-background-secondary/85 dark:bg-background/85 border-foreground-secondary/20 hover:border-foreground-secondary/50 p-0.5', | |
| 'border flex relative', | |
| )} | |
| > | |
| {!inputState.isInputting ? ( | |
| // Chat Button | |
| <button | |
| onClick={() => setInputState((prev) => ({ ...prev, isInputting: true }))} | |
| className="rounded-lg hover:text-foreground-primary transition-colors px-2.5 py-1.5 flex flex-row items-center gap-2 w-full" | |
| > | |
| <Icons.Sparkles className="w-4 h-4" /> | |
| <span className="text-mini !font-medium whitespace-nowrap"> | |
| {t(transKeys.editor.panels.edit.tabs.chat.miniChat.button)} | |
| </span> | |
| </button> | |
| ) : ( | |
| // Input Field | |
| <div className="flex flex-row items-center gap-1 w-[280px] relative"> | |
| <Button | |
| size="icon" | |
| onClick={() => | |
| setInputState((prev) => ({ | |
| ...prev, | |
| isInputting: false, | |
| value: '', | |
| })) | |
| } | |
| className={cn( | |
| 'group h-6 w-6 absolute left-1 top-1 z-10 border-none shadow-none bg-transparent hover:bg-transparent', | |
| 'transition-all duration-200', | |
| inputState.value.trim().length >= DIMENSIONS.minCharsToSubmit | |
| ? 'opacity-0 -translate-x-2 scale-75 pointer-events-none' | |
| : 'opacity-100 translate-x-0 scale-100 pointer-events-auto', | |
| )} | |
| disabled={inputState.isSubmitting} | |
| setInputState(DEFAULT_INPUT_STATE); | |
| }; | |
| const handleSubmit = async () => { | |
| if (inputState.isSubmitting) return; | |
| setInputState((prev) => ({ ...prev, isSubmitting: true })); | |
| editorEngine.state.rightPanelTab = EditorTabValue.CHAT; | |
| const promise = editorEngine.chat.sendMessage(inputState.value, ChatType.EDIT); | |
| try { | |
| await toast.promise(promise, { | |
| loading: t(transKeys.editor.panels.edit.tabs.chat.toast.loading), | |
| success: t(transKeys.editor.panels.edit.tabs.chat.toast.success), | |
| error: t(transKeys.editor.panels.edit.tabs.chat.toast.error), | |
| }); | |
| } finally { | |
| setInputState(DEFAULT_INPUT_STATE); | |
| } | |
| }; |
| @@ -1,8 +1,10 @@ | |||
| import { useEditorEngine } from '@/components/store/editor'; | |||
| import { observer } from 'mobx-react-lite'; | |||
There was a problem hiding this comment.
Add 'use client' directive.
Hooks/observer usage in app dir requires client component.
As per coding guidelines
+'use client';📝 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.
| import { observer } from 'mobx-react-lite'; | |
| 'use client'; | |
| import { observer } from 'mobx-react-lite'; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/code.tsx
around line 1, this component uses hooks/observer and must be a client
component; add the "use client" directive as the very first line of the file
(before any imports) so React treats it as a client component and the
mobx-react-lite observer/hooks will work correctly.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| {/* Right: Copyright */} | ||
| <div className="text-foreground-tertiary text-small w-full md:w-auto flex justify-center md:justify-end">© {new Date().getFullYear()} On Off, Inc.</div> | ||
| <div className="text-foreground-tertiary text-small flex w-full justify-center md:w-auto md:justify-end"> | ||
| © {new Date().getFullYear()} On Off, Inc. |
There was a problem hiding this comment.
Typographical error: The copyright text reads "On Off, Inc.". Should this be "Onlook, Inc." to match the brand consistency?
| © {new Date().getFullYear()} On Off, Inc. | |
| © {new Date().getFullYear()} Onlook, Inc. |
| @@ -1,10 +1,18 @@ | |||
| import type type { RefObject } from 'react'; | |||
There was a problem hiding this comment.
Typo: Duplicate 'type' keyword in the import statement. Please update import type type { RefObject } from 'react'; to import type { RefObject } from 'react';.
| import type type { RefObject } from 'react'; | |
| import type { RefObject } from 'react'; |
| @@ -1,7 +1,10 @@ | |||
| import type type { RefObject } from 'react'; | |||
There was a problem hiding this comment.
There appears to be a typographical error on line 1: "import type type { RefObject } from 'react';". The duplicate 'type' keyword should be removed to read "import type { RefObject } from 'react';".
| import type type { RefObject } from 'react'; | |
| import type { RefObject } from 'react'; |
| @@ -1,5 +1,4 @@ | |||
| import { MessageContextType, type MessageContext } from '@onlook/models'; | |||
| import type { ComponentType } from 'react'; | |||
| import type { MessageContext, MessageContextType, MessageContextType } from '@onlook/models'; | |||
There was a problem hiding this comment.
There is a duplicate entry for 'MessageContextType' in the import statement. Please remove the duplicate entry to avoid confusion.
| import type { MessageContext, MessageContextType, MessageContextType } from '@onlook/models'; | |
| import type { MessageContext, MessageContextType } from '@onlook/models'; |
| Command, | ||
| Sandbox, | ||
| SandboxBrowserSession, | ||
| SandboxBrowserSession, |
There was a problem hiding this comment.
Typographical issue: Duplicate import of 'SandboxBrowserSession' detected. Please remove one of them.
| SandboxBrowserSession, |
| Task, | ||
| Terminal, | ||
| Watcher, | ||
| Watcher, |
There was a problem hiding this comment.
Typographical issue: Duplicate import of 'Watcher' detected. Please remove one of them.
| Watcher, |
saddlepaddle
force-pushed
the
run-lint-on-repo
branch
from
c84a82e to
181c483
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tooling/eslint/base.js (1)
45-50: Critical: Add Prettier flat config to disable conflicting formatting rules
- tooling/eslint/base.js: import
eslint-config-prettier/flatand append...eslintConfigPrettierlast in theextendsarray to disable overlapping ESLint formatting rules.- Clarify the comment above
import/consistent-type-specifier-style; Prettier doesn’t sort imports natively—reference the actual import-sorting tool or remove the misleading note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/client/tsconfig.json(1 hunks)packages/db/tsconfig.json(1 hunks)packages/fonts/tsconfig.json(1 hunks)packages/growth/tsconfig.json(1 hunks)packages/image-server/tsconfig.json(1 hunks)packages/ui/tsconfig.json(1 hunks)tooling/eslint/base.js(1 hunks)tooling/prettier/index.js(0 hunks)
💤 Files with no reviewable changes (1)
- tooling/prettier/index.js
✅ Files skipped from review due to trivial changes (1)
- packages/fonts/tsconfig.json
🔇 Additional comments (3)
apps/web/client/tsconfig.json (1)
13-19: tsconfig.json test path is valid
Thetestdirectory exists in apps/web/client, so updating the path from"tests"to"test"is correct.packages/growth/tsconfig.json (1)
6-6: Verifiedtestsdirectory exists. No changes to tsconfig.json required.tooling/eslint/base.js (1)
53-75: TypeScript rule improvements look good with one note.The stricter TypeScript rules improve code quality:
no-unused-varswith underscore-prefix ignoring is best practiceno-misused-promiseswithattributes: falseis appropriate for React event handlersno-unnecessary-conditionwithallowConstantLoopConditions: truecatches logic errors while allowing common loop patternsno-non-null-assertionenforces proper null checkingOne minor consideration: The change from
separate-type-importstoinline-type-imports(line 63) is a stylistic preference. Both are valid; inline is more compact but may be less explicit about type-only imports. Ensure this aligns with your team's preferences.
saddlepaddle
force-pushed
the
run-lint-on-repo
branch
from
181c483 to
01e7dac
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/client/src/components/ui/avatar-dropdown/index.tsx (1)
39-39: Consider extracting the URL construction pattern to a helper.This duplicates the URL construction logic reviewed in
apps/web/client/src/app/invitation/[id]/_components/main.tsx(line 38). See the refactoring suggestion in that file's review.apps/web/client/src/app/invitation/[id]/_components/auth.tsx (1)
15-15: Consider extracting the URL construction pattern to a helper.This duplicates the URL construction logic reviewed in
apps/web/client/src/app/invitation/[id]/_components/main.tsx(line 38). See the refactoring suggestion in that file's review.apps/web/client/next.config.ts (1)
5-5: Type-only import is correct.The change to
import type { NextConfig }is appropriate sinceNextConfigis only used as a type annotation on line 10. This follows best practices for type-only imports.Note: The past review comment about
pathbeing imported as type-only appears to be outdated. The current code correctly importspathas a regular runtime import on line 7, which is necessary for its usage inpath.join()on line 19.
🧹 Nitpick comments (1)
apps/web/client/src/app/invitation/[id]/_components/main.tsx (1)
38-38: Consider extracting the URL construction pattern to a helper.This URL construction logic (checking
searchParamsexistence and conditionally appending the query string) is duplicated across multiple files:
apps/web/client/src/components/ui/avatar-dropdown/index.tsx(line 39)apps/web/client/src/app/invitation/[id]/_components/auth.tsx(line 15)Extract to a utility function, for example:
// In apps/web/client/src/utils/url.ts export function buildUrlWithSearchParams( pathname: string, searchParams: ReadonlyURLSearchParams | null ): string { return `${pathname}${searchParams && searchParams.toString() ? `?${searchParams.toString()}` : ''}`; }Then use it consistently:
-const currentUrl = `${pathname}${searchParams && searchParams.toString() ? `?${searchParams.toString()}` : ''}`; +const currentUrl = buildUrlWithSearchParams(pathname, searchParams);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/web/client/next.config.ts(1 hunks)apps/web/client/src/app/callback/github/install/page.tsx(1 hunks)apps/web/client/src/app/invitation/[id]/_components/auth.tsx(1 hunks)apps/web/client/src/app/invitation/[id]/_components/main.tsx(2 hunks)apps/web/client/src/app/login/page.tsx(1 hunks)apps/web/client/src/components/ui/avatar-dropdown/index.tsx(1 hunks)apps/web/client/test/cache/file-cache.test.ts(3 hunks)apps/web/client/test/messages.test.ts(2 hunks)apps/web/client/test/pages/helper.test.ts(6 hunks)apps/web/client/tsconfig.json(1 hunks)apps/web/package.json(1 hunks)apps/web/preload/package.json(1 hunks)apps/web/preload/tsconfig.json(1 hunks)packages/db/tsconfig.json(1 hunks)packages/fonts/tsconfig.json(1 hunks)packages/growth/tsconfig.json(1 hunks)packages/image-server/tsconfig.json(1 hunks)packages/ui/tsconfig.json(1 hunks)tooling/eslint/base.js(1 hunks)tooling/prettier/index.js(0 hunks)
💤 Files with no reviewable changes (1)
- tooling/prettier/index.js
✅ Files skipped from review due to trivial changes (2)
- apps/web/package.json
- apps/web/preload/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/growth/tsconfig.json
- apps/web/client/tsconfig.json
- packages/ui/tsconfig.json
- packages/fonts/tsconfig.json
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/components/ui/avatar-dropdown/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/components/ui/avatar-dropdown/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/test/cache/file-cache.test.tsapps/web/client/test/pages/helper.test.tsapps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/next.config.tsapps/web/client/test/messages.test.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/invitation/[id]/_components/auth.tsxapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/login/page.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/test/cache/file-cache.test.tsapps/web/client/test/pages/helper.test.tsapps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/next.config.tsapps/web/client/test/messages.test.ts
apps/web/client/next.config.ts
📄 CodeRabbit inference engine (AGENTS.md)
Import ./src/env in Next.js config to enforce env validation at build time
Import ./src/env in next.config.ts to enforce env validation at build time
Files:
apps/web/client/next.config.ts
🧠 Learnings (4)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in Next.js config to enforce env validation at build time
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in next.config.ts to enforce env validation at build time
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables in src/env.ts via t3-oss/env-nextjs
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables via t3-oss/env-nextjs in apps/web/client/src/env.ts
Applied to files:
apps/web/client/next.config.ts
🧬 Code graph analysis (2)
apps/web/client/src/app/login/page.tsx (1)
apps/web/client/src/utils/constants/index.ts (1)
LocalForageKeys(48-50)
apps/web/client/test/cache/file-cache.test.ts (2)
apps/web/client/src/components/store/editor/cache/file-cache.ts (1)
FileCacheManager(6-218)packages/models/src/sandbox/files.ts (1)
SandboxFile(17-17)
🔇 Additional comments (18)
apps/web/client/src/app/callback/github/install/page.tsx (1)
23-24: LGTM! Defensive guard prevents potential NPE.The early return when
searchParamsis falsy prevents accessing properties onnullorundefinedin subsequent lines. This aligns with the defensive coding pattern applied consistently across authentication flows in this PR.apps/web/client/src/app/login/page.tsx (1)
18-19: LGTM! Guarded searchParams access is safe.The optional chaining and nullish coalescing operator ensure that
returnUrlsafely defaults tonullwhensearchParamsis unavailable, preventing runtime errors.apps/web/client/src/app/invitation/[id]/_components/main.tsx (1)
18-18: LGTM! Safe token extraction.The optional chaining with nullish coalescing prevents errors when
searchParamsis unavailable and ensurestokenis explicitlynullrather thanundefined.tooling/eslint/base.js (6)
53-60: LGTM! Standard unused variable pattern.The configuration properly enforces unused variable checking while allowing the conventional underscore prefix for intentionally unused parameters and variables.
61-64: LGTM! Inline type imports align with modern practices.The change to
inline-type-importsis appropriate and aligns with current TypeScript best practices for import organization.
65-68: LGTM! Appropriate for React event handlers.The
checksVoidReturn: { attributes: false }configuration appropriately allows async functions in JSX event handlers, which is a common pattern in React applications.
69-74: LGTM! Good strictness improvement.The
no-unnecessary-conditionrule will catch logical errors from always-true or always-false conditions while sensibly allowing constant loop conditions likewhile (true).
75-75: LGTM! Enforces safer null handling.Prohibiting the non-null assertion operator encourages proper null checking and type narrowing, reducing potential runtime errors.
76-78: Plugin reference verified —@ianvs/prettier-plugin-sort-importsexists in tooling/prettier/package.json, so the rule-disable comment is accurate.apps/web/preload/tsconfig.json (1)
9-9: Consider the implications ofskipLibCheck.While
skipLibCheck: truespeeds up compilation and is commonly used, it can hide type errors in dependencies. This is acceptable for this use case but be aware it may mask issues in third-party packages.packages/image-server/tsconfig.json (1)
6-6: LGTM!Adding the test directory ensures test files are properly type-checked, aligning with the broader pattern of test coverage improvements across the repository.
packages/db/tsconfig.json (1)
12-13: LGTM!Adding both the test directory and drizzle.config.ts to the TypeScript compilation ensures comprehensive type coverage for the database package configuration and tests.
apps/web/client/test/pages/helper.test.ts (2)
19-19: Good type safety improvement withas constassertions.Using
as constfor thetypefield ensures TypeScript infers the literal type ('file' or 'directory') rather than the widerstringtype, which provides better type checking against theReaddirEntryinterface.Also applies to: 41-41, 59-60, 91-91
34-35: Defensive optional chaining is a good practice.While the test expectations ensure
result[0]exists (viatoHaveLength(1)), using optional chaining (result[0]?.name) is defensive programming that prevents potential runtime errors if the test logic changes.Also applies to: 52-53, 103-104
apps/web/client/test/messages.test.ts (1)
231-231: Type guard ensures robustness.The
.filter((p): p is string => p !== undefined)type guard defensively removes any potential undefined values from regex capture groups before building the Set. While the regex pattern should always capturem[1], this makes the code more resilient to edge cases and satisfies TypeScript's strict type checking.Also applies to: 254-254
apps/web/client/test/cache/file-cache.test.ts (3)
21-21: Constructor correctly updated to match implementation.The test now properly instantiates
FileCacheManagerwith the requiredprojectIdandbranchIdparameters, aligning with the constructor signature change insrc/components/store/editor/cache/file-cache.ts.
76-87: Test semantics improved.The test name and content change clarify the distinction between empty content (empty string
'') and null content. This better represents the actual behavior where text files can have empty string content while remaining "loaded."
140-144: Binary file type better represents unloaded state.Changing to
type: 'binary'with a.pngextension andcontent: nullmore accurately represents an unloaded file scenario. Binary files are more commonly lazy-loaded, and the null content correctly triggers theisFileLoadedcheck to return false.
saddlepaddle
force-pushed
the
run-lint-on-repo
branch
from
01e7dac to
fb139f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/callback/github/install/page.tsx (1)
22-74: Add missing dependencies to the useEffect hook.The useEffect references
searchParams(line 16) andhandleInstallationCallback(line 20) but the dependency array on line 74 is empty. This violates the rules of hooks and causes stale closures—the effect will only run once with the initial values and won't react to changes.Apply this diff to fix the dependency array:
- }, []); + }, [searchParams, handleInstallationCallback]);Note: If
handleInstallationCallbackcauses the effect to re-run unnecessarily, you can wrap it withuseCallbackor destructure only themutatefunction from the mutation hook to stabilize the reference.
♻️ Duplicate comments (1)
apps/web/client/src/app/callback/github/install/page.tsx (1)
29-29: Remove or replace console logging in production code.These console statements expose internal state and reduce production observability. A structured logger should be used instead.
This issue was already flagged in a previous review.
Also applies to: 60-60, 70-70
🧹 Nitpick comments (3)
apps/web/client/test/messages.test.ts (1)
254-254: LGTM! Consistent type-safe handling.Same defensive approach as Line 231, correctly handling the type narrowing.
Optional: Consider extracting the duplicated pattern.
Lines 231 and 254 use identical logic to extract and filter placeholders. You could reduce duplication with a small helper:
function extractPlaceholders(text: string, regex: RegExp): Set<string> { const matches = [...text.matchAll(regex)]; return new Set(matches.map((m) => m[1]).filter((p): p is string => p !== undefined)); }Then use:
-const placeholders = new Set(matches.map((m) => m[1]).filter((p): p is string => p !== undefined)); +const placeholders = extractPlaceholders(value, placeholderRegex);And similarly for Line 254.
tooling/eslint/base.js (1)
65-68: Use string literal for consistency.The severity is set to numeric
2instead of the string'error', which is inconsistent with other rules in this file (lines 54, 70, 75, 76).Apply this diff for consistency:
- '@typescript-eslint/no-misused-promises': [ - 2, - { checksVoidReturn: { attributes: false } }, - ], + '@typescript-eslint/no-misused-promises': [ + 'error', + { checksVoidReturn: { attributes: false } }, + ],apps/web/client/test/cache/file-cache.test.ts (1)
17-18: Type the mock functions instead of usingany.The
anytype should be avoided per coding guidelines. These mocks have known signatures that can be properly typed.Apply this diff to add proper types:
- let mockReadFile: any; - let mockWriteFile: any; + let mockReadFile: (path: string) => Promise<SandboxFile | null>; + let mockWriteFile: (path: string, content: string | Uint8Array) => Promise<boolean>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/web/client/next.config.ts(1 hunks)apps/web/client/src/app/callback/github/install/page.tsx(1 hunks)apps/web/client/src/app/invitation/[id]/_components/auth.tsx(1 hunks)apps/web/client/src/app/invitation/[id]/_components/main.tsx(2 hunks)apps/web/client/src/app/login/page.tsx(1 hunks)apps/web/client/src/components/ui/avatar-dropdown/index.tsx(1 hunks)apps/web/client/test/cache/file-cache.test.ts(3 hunks)apps/web/client/test/messages.test.ts(2 hunks)apps/web/client/test/pages/helper.test.ts(6 hunks)apps/web/client/tsconfig.json(1 hunks)apps/web/package.json(1 hunks)apps/web/preload/package.json(1 hunks)apps/web/preload/tsconfig.json(1 hunks)packages/db/tsconfig.json(1 hunks)packages/fonts/tsconfig.json(1 hunks)packages/growth/tsconfig.json(1 hunks)packages/image-server/tsconfig.json(1 hunks)packages/ui/tsconfig.json(1 hunks)tooling/eslint/base.js(1 hunks)tooling/prettier/index.js(0 hunks)
💤 Files with no reviewable changes (1)
- tooling/prettier/index.js
✅ Files skipped from review due to trivial changes (1)
- apps/web/preload/package.json
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/web/client/src/app/invitation/[id]/_components/auth.tsx
- packages/image-server/tsconfig.json
- packages/growth/tsconfig.json
- packages/fonts/tsconfig.json
- apps/web/client/src/app/login/page.tsx
- apps/web/client/test/pages/helper.test.ts
- apps/web/preload/tsconfig.json
- packages/db/tsconfig.json
- packages/ui/tsconfig.json
- apps/web/client/src/components/ui/avatar-dropdown/index.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/test/cache/file-cache.test.tsapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/test/messages.test.tsapps/web/client/next.config.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/test/cache/file-cache.test.tsapps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/test/messages.test.tsapps/web/client/next.config.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/invitation/[id]/_components/main.tsxapps/web/client/src/app/callback/github/install/page.tsx
apps/web/client/next.config.ts
📄 CodeRabbit inference engine (AGENTS.md)
Import ./src/env in Next.js config to enforce env validation at build time
Import ./src/env in next.config.ts to enforce env validation at build time
Files:
apps/web/client/next.config.ts
🧠 Learnings (5)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Applied to files:
apps/web/client/tsconfig.json
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in Next.js config to enforce env validation at build time
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in next.config.ts to enforce env validation at build time
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables in src/env.ts via t3-oss/env-nextjs
Applied to files:
apps/web/client/next.config.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables via t3-oss/env-nextjs in apps/web/client/src/env.ts
Applied to files:
apps/web/client/next.config.ts
🧬 Code graph analysis (1)
apps/web/client/test/cache/file-cache.test.ts (2)
apps/web/client/src/components/store/editor/cache/file-cache.ts (1)
FileCacheManager(6-218)packages/models/src/sandbox/files.ts (1)
SandboxFile(17-17)
🔇 Additional comments (14)
apps/web/package.json (1)
25-25: No functional change — newline/EOL adjustment only.JSON remains valid; nothing else changed. LGTM.
apps/web/client/test/messages.test.ts (1)
231-231: LGTM! Type-safe handling of regex capture groups.The type guard correctly narrows
string | undefinedtostring, satisfying TypeScript's strict type checking. While the regex pattern guaranteesm[1]is never undefined at runtime, this defensive approach avoids disallowed non-null assertions.tooling/eslint/base.js (3)
53-60: Enhanced unused vars configuration looks good.The updated rule correctly enforces errors for unused variables while allowing underscore-prefixed names to be ignored, which is a widely adopted convention for intentionally unused parameters.
69-74: Good addition of no-unnecessary-condition rule.The
allowConstantLoopConditionsoption appropriately prevents false positives in legitimate infinite loops likewhile (true).
75-75: Enforcing no-non-null-assertion improves type safety.Disallowing the non-null assertion operator (
!) encourages proper null checks and improves type safety across the codebase.apps/web/client/src/app/callback/github/install/page.tsx (1)
23-24: LGTM! Defensive guard prevents null reference errors.The early return when searchParams is falsy prevents potential runtime errors when accessing search parameter methods below.
apps/web/client/src/app/invitation/[id]/_components/main.tsx (2)
18-18: Optional: Simplify redundant null coalescing.The
?? nullis redundant sincesearchParams?.get('token')already returnsstring | nullwhen searchParams is defined, orundefinedwhen it's not (which is then coerced tonullby the nullish coalescing). While not harmful, removing it simplifies the code slightly.Apply this diff if you prefer to simplify:
- const token = searchParams?.get('token') ?? null; + const token = searchParams?.get('token') ?? null;Actually, on second thought, the current code is fine as-is since it makes the intent explicit that token should be
string | nullrather thanstring | null | undefined. Feel free to keep it.
38-38: LGTM! Defensive currentUrl construction.The guard correctly handles the case when searchParams is undefined or when toString() returns an empty string, preventing malformed URLs.
apps/web/client/test/cache/file-cache.test.ts (3)
21-21: LGTM!Constructor arguments correctly match the updated signature with project and branch parameters.
76-80: LGTM!The test now correctly validates empty string content for text files. The distinction between empty content (
'') and unloaded content (null) is now clearer, with null content appropriately tested only for binary files.
140-143: LGTM!The unloaded file fixture is now more appropriate as a binary file, which aligns with the
writeEmptyFileAPI that only supports binary types for null content.apps/web/client/next.config.ts (1)
5-5: LGTM! Type-only import correctly applied.Converting
NextConfigto a type-only import is appropriate since it's only used for the type annotation on line 10, not at runtime. This change aligns with the@typescript-eslint/consistent-type-importsrule mentioned in the PR description.apps/web/client/tsconfig.json (2)
7-9: LGTM! Formatting improvements applied.The path aliases and exclude list have been reformatted to single-line arrays, improving consistency without any semantic changes.
Also applies to: 20-20
14-18: Include paths validated
testdirectory exists as expectednext-env.d.tsandnext.config.tsinclusions are correct
| '@typescript-eslint/consistent-type-imports': [ | ||
| 'warn', | ||
| { prefer: 'type-imports', fixStyle: 'separate-type-imports' }, | ||
| { prefer: 'type-imports', fixStyle: 'inline-type-imports' }, | ||
| ], |
There was a problem hiding this comment.
Critical conflict: Inline vs. top-level type imports.
Lines 61-64 configure @typescript-eslint/consistent-type-imports with fixStyle: 'inline-type-imports', which prefers:
import { type Foo } from './foo' // inlineHowever, line 76 sets import/consistent-type-specifier-style to 'prefer-top-level', which prefers:
import type { Foo } from './foo' // top-levelThese rules are in direct conflict and will cause linting inconsistencies. Based on the PR description stating that import/consistent-type-specifier-style should be disabled due to a conflict with Prettier, you should either:
- Remove or disable the
import/consistent-type-specifier-stylerule (preferred, as stated in PR description), or - Align both rules to use the same style
Apply this diff to disable the conflicting rule:
- '@typescript-eslint/no-non-null-assertion': 'error',
- 'import/consistent-type-specifier-style': ['error', 'prefer-top-level'],
+ '@typescript-eslint/no-non-null-assertion': 'error',
+ // Disabled due to conflict with @typescript-eslint/consistent-type-imports inline style
+ 'import/consistent-type-specifier-style': 'off',Also applies to: 76-76
🤖 Prompt for AI Agents
In tooling/eslint/base.js around lines 61-64 and line 76, the
@typescript-eslint/consistent-type-imports rule is set to use
inline-type-imports while import/consistent-type-specifier-style is set to
prefer-top-level, causing a direct conflict; remove or disable the
import/consistent-type-specifier-style rule at line 76 (preferably turn it off
per the PR description) so the two rules no longer conflict, or alternatively
align both rules to the same style if you prefer top-level imports.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/client/src/lib/ai/tools/classes/bash-read.ts (1)
68-74: Critical: Command validation uses incorrect substring matching.The validation logic checks if any allowed command is a substring of the base command, which is backwards. For example, if
baseCommandis"lsof"and"ls"is in the allowed list, the command would incorrectly pass validation because"lsof".includes("ls")istrue.This allows execution of unauthorized commands that contain allowed command names as substrings.
Apply this diff to fix the validation logic:
- if (!readOnlyCommands.some((cmd: string) => baseCommand.includes(cmd))) { + if (!readOnlyCommands.includes(baseCommand)) { return { output: '', success: false,apps/web/client/src/components/tools/tools.ts (1)
15-20: Replace hardcoded user-facing text with i18n messages.The toast notification and error message contain hardcoded English text. As per coding guidelines for this path, use
next-intlmessages/hooks instead for proper internationalization support.As per coding guidelines.
Consider refactoring to use message keys:
+import { useTranslations } from 'next-intl'; + +// In component or pass translations as parameter +const t = useTranslations('tools'); + toast.error( - `Tool "${toolName}" not available in ask mode`, + t('toolNotAvailable', { toolName }), { - description: `Switch to build mode to use this tool.`, + description: t('switchToBuildMode'), duration: 2000, } ); -throw new Error(`Tool "${toolName}" is not available in ${currentChatMode} mode`); +throw new Error(t('toolNotAvailableInMode', { toolName, mode: currentChatMode }));Note: Since this is a non-component function, you may need to pass the translation function as a parameter or handle errors differently.
apps/web/client/src/lib/ai/prompt/provider.ts (1)
1-5: Missing export for ChatMessage in @onlook/db
packages/db/src/types/chat.tsexports onlyMessageContextTypeandMessageContext;ChatMessageis not defined or re-exported. Add and re-export theChatMessagetype inpackages/db/src/types/chat.ts(and ensurepackages/db/src/index.tsre-exports it), or remove it from the import inapps/web/client/src/lib/ai/prompt/provider.ts.
🧹 Nitpick comments (19)
apps/web/client/src/lib/ai/tools/classes/bash-edit.ts (1)
78-78: Replaceanytype withunknownfor better type safety.The catch clause uses
anytype, which is discouraged by the coding guidelines. Usingunknownis the recommended practice for error handling in modern TypeScript as it maintains type safety while still allowing you to handle any thrown value.As per coding guidelines
Apply this diff:
- } catch (error: any) { + } catch (error: unknown) { return { output: '', success: false, - error: error.message || error.toString() + error: error instanceof Error ? error.message : String(error) }; }apps/web/client/src/lib/ai/tools/classes/scrape-url.ts (1)
61-70: Consider internationalizing user-facing labels.The
getLabelmethod contains hardcoded user-facing text ("Visiting URL", "Visiting "). Consider using next-intl messages/hooks for better i18n support in the future.As per coding guidelines.
apps/web/client/src/lib/ai/tools/classes/terminal-command.ts (1)
29-29: Consider using i18n for user-facing text.The hardcoded strings "Terminal" (line 37) and the error message template (line 29) appear to be user-facing. Per the coding guidelines, consider using next-intl messages/hooks instead of hardcoded text to support internationalization.
Based on coding guidelines.
Also applies to: 37-37
apps/web/client/src/lib/ai/tools/classes/list-files.ts (2)
94-97: Avoidanytype for better type safety.The
fileparameter uses theanytype, which violates the coding guideline. Consider defining a proper interface or using the inferred type fromsandbox.readDir().Apply this diff to improve type safety:
+ type FileEntry = { name: string; type: 'file' | 'directory' }; return fallbackResult.map((file: any) => ({ - return fallbackResult.map((file: any) => ({ + return fallbackResult.map((file: FileEntry) => ({ path: file.name, type: file.type, }));As per coding guidelines.
122-128: Avoidanytype in sort function.The sort function parameters use the
anytype, which violates the coding guideline. The types can be inferred from the mapped result above.Apply this diff to improve type safety:
- .sort((a: any, b: any) => { + .sort((a, b) => { // Sort directories first, then files, then alphabetically if (a.type !== b.type) { return a.type === 'directory' ? -1 : 1; } return a.path.localeCompare(b.path); });TypeScript will automatically infer the correct types from the preceding map operation.
As per coding guidelines.
apps/web/client/src/lib/ai/tools/classes/web-search.ts (2)
5-5: Consider using a path alias for consistency.While the relative import works correctly, consider replacing it with a path alias (
@/lib/ai/tools/models/client) to align with the coding guidelines that recommend using@/*and~/*for imports mapping tosrc/*.Apply this diff to use a path alias:
-import { ClientTool } from '../models/client'; +import { ClientTool } from '@/lib/ai/tools/models/client';Based on coding guidelines.
27-27: Minor: Inconsistent semicolon usage.Line 27 is missing a semicolon while most other statements in the file include them. Consider adding it for consistency.
Apply this diff:
- return res + return res;apps/web/client/src/lib/ai/tools/classes/typecheck.ts (4)
9-9: Consider using next-intl for user-facing text.The description string appears to be user-facing and should be internationalized using next-intl messages/hooks per the coding guidelines for this directory.
As per coding guidelines.
27-27: Consider using next-intl for error messages.These error messages are user-facing and should be internationalized using next-intl messages/hooks per the coding guidelines for this directory.
As per coding guidelines.
Also applies to: 41-41
44-44: Replaceanywithunknownfor type-safe error handling.The coding guidelines specify avoiding the
anytype unless absolutely necessary. Useunknowninstead, which maintains type safety while allowing you to handle any error type.- } catch (error: any) { + } catch (error: unknown) { return { success: false, - error: error.message || error.toString() + error: error instanceof Error ? error.message : String(error) }; }As per coding guidelines.
52-52: Consider using next-intl for the label text.The label string is user-facing and should be internationalized using next-intl messages/hooks per the coding guidelines for this directory.
As per coding guidelines.
apps/web/client/src/components/store/create/manager.ts (1)
3-5: Consolidate imports from@onlook/db.Lines 3 and 5 both import from
@onlook/db. Consider consolidating them into a single import statement for cleaner code organization.Apply this diff to consolidate the imports:
-import { createDefaultProject } from '@onlook/db'; import { CreateRequestContextType } from '@onlook/models'; -import { type ImageMessageContext } from '@onlook/db'; +import { createDefaultProject, type ImageMessageContext } from '@onlook/db';apps/web/client/src/lib/ai/tools/classes/bash-read.ts (1)
82-82: Consider usingunknowninstead ofanyfor stricter typing.While using
anyfor error types in catch blocks is common, usingunknownprovides better type safety and aligns with the coding guidelines to avoidanyunless necessary.Apply this diff:
- } catch (error: any) { + } catch (error: unknown) { return { output: '', success: false, - error: error.message || error.toString() + error: error instanceof Error ? error.message : String(error) };apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx (2)
214-214: Consider internationalizing the success message.The hardcoded success message
'✅ Typecheck passed!'should use next-intl for internationalization support. As per coding guidelines, avoid hardcoded user-facing text in favor of next-intl messages/hooks.Example refactor:
import { useTranslations } from 'next-intl'; // In component: const t = useTranslations('chat.toolCall'); // ... defaultStdOut={(result?.success ? t('typecheckPassed') : result?.error) ?? null}Note: This would require adding the 'use client' directive if the component doesn't already have a client boundary above it in the component tree.
1-1: Internationalize success message Replace'✅ Typecheck passed!'at line 214 with a next-intl formatted message to avoid hardcoded user-facing text.apps/web/client/src/components/tools/tools.ts (2)
22-26: Consider removingas anytype assertion.While the input is validated on line 23, the
as anytype assertion on line 26 bypasses TypeScript's type safety. Consider defining a proper type constraint for the tool's handle method or using the validated input's inferred type directly.As per coding guidelines.
Possible approaches:
- If
tool.parameters.parse()returns a properly typed result, use that type directly- Define a generic constraint that ensures type safety between parameters and the handle method
- Use a type guard or conditional type to narrow the type safely
Example with generic constraint:
// If the tool interface could be typed like: interface Tool<T> { parameters: z.ZodSchema<T>; handle(input: T, engine: EditorEngine): Promise<unknown>; } // Then validatedInput would already have the correct type output = await toolInstance.handle(validatedInput, editorEngine);
27-29: Improve error message formatting in catch block.The current error handling concatenates the error object directly to a string, which may produce unhelpful output like
"error handling tool call [object Object]". Consider extracting the error message and potentially preserving stack trace information.Apply this diff to improve error formatting:
} catch (error) { - output = 'error handling tool call ' + error; + const errorMessage = error instanceof Error ? error.message : String(error); + output = `Error handling tool call: ${errorMessage}`; + console.error('Tool call failed:', error); }apps/web/client/package.json (1)
69-93: Update dependency versions to latest published releases
No known security advisories affect these packages, but the following can be bumped to their latest minor/patch releases:
• @mendable/firecrawl-js → ^4.4.0
• exa-js → ^1.10.2
• gpt-tokenizer → ^3.2.0
• marked → ^16.4.0
• openai → ^6.3.0
• zod → ^4.1.12packages/db/src/types/chat.ts (1)
85-89: Consider standardizing enum string values.The AgentType enum uses lowercase string values ("root", "user") while MessageCheckpointType uses uppercase ("GIT"). For consistency across the codebase, consider adopting a uniform convention.
Example with uppercase:
export enum AgentType { - ROOT = "root", - USER = "user", + ROOT = "ROOT", + USER = "USER", }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (107)
apps/web/client/package.json(2 hunks)apps/web/client/src/app/api/chat/route.ts(1 hunks)apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/sent-context-pill.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsx(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.ts(1 hunks)apps/web/client/src/components/store/create/manager.ts(1 hunks)apps/web/client/src/components/store/editor/chat/context.ts(2 hunks)apps/web/client/src/components/store/editor/chat/index.ts(1 hunks)apps/web/client/src/components/tools/tools.ts(1 hunks)apps/web/client/src/lib/ai/agents/root.ts(1 hunks)apps/web/client/src/lib/ai/chat/mappers/conversation.ts(1 hunks)apps/web/client/src/lib/ai/chat/mappers/message.ts(1 hunks)apps/web/client/src/lib/ai/chat/providers.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/agent-rule.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/branch.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/error.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/file.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/highlight.ts(1 hunks)apps/web/client/src/lib/ai/contexts/classes/image.ts(1 hunks)apps/web/client/src/lib/ai/contexts/helpers.ts(1 hunks)apps/web/client/src/lib/ai/contexts/models/base.ts(1 hunks)apps/web/client/src/lib/ai/models/chat.ts(1 hunks)apps/web/client/src/lib/ai/prompt/provider.ts(1 hunks)apps/web/client/src/lib/ai/stream/index.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/branch-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/error-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/file-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/highlight-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/image-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/index.test.ts(1 hunks)apps/web/client/src/lib/ai/test/prompt/prompt.test.ts(1 hunks)apps/web/client/src/lib/ai/test/stream/convert.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tokens.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tools/edit.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tools/read.test.ts(1 hunks)apps/web/client/src/lib/ai/tokens/index.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/bash-edit.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/bash-read.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/check-errors.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/fuzzy-edit-file.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/glob.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/grep.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/list-branches.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/list-files.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/onlook-instructions.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/read-file.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/read-style-guide.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/sandbox.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/scrape-url.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/search-replace-edit.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/search-replace-multi-edit.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/terminal-command.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/typecheck.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/upload-image.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/web-search.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/write-file.ts(1 hunks)apps/web/client/src/lib/ai/tools/models/client.ts(1 hunks)apps/web/client/src/lib/ai/tools/shared/helpers/files.ts(1 hunks)apps/web/client/src/lib/ai/tools/toolset.ts(1 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(1 hunks)apps/web/client/src/server/api/routers/chat/message.ts(1 hunks)apps/web/client/src/server/api/routers/chat/suggestion.ts(1 hunks)apps/web/client/src/server/api/routers/code.ts(1 hunks)apps/web/client/src/server/api/routers/project/project.ts(1 hunks)package.json(1 hunks)packages/ai/eslint.config.js(0 hunks)packages/ai/package.json(0 hunks)packages/ai/tsconfig.json(0 hunks)packages/db/src/defaults/conversation.ts(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/mappers/chat/index.ts(0 hunks)packages/db/src/mappers/index.ts(0 hunks)packages/db/src/schema/chat/conversation.ts(1 hunks)packages/db/src/schema/chat/message.ts(2 hunks)packages/db/src/seed/db.ts(2 hunks)packages/db/src/types/chat.ts(2 hunks)packages/models/src/chat/conversation/index.ts(0 hunks)packages/models/src/chat/index.ts(0 hunks)packages/models/src/chat/message/checkpoint.ts(0 hunks)packages/models/src/chat/message/code.ts(0 hunks)packages/models/src/chat/message/index.ts(0 hunks)packages/models/src/chat/message/message.ts(0 hunks)packages/models/src/chat/message/queue.ts(0 hunks)packages/models/src/chat/request.ts(0 hunks)packages/models/src/chat/response.ts(0 hunks)packages/models/src/chat/suggestion.ts(0 hunks)packages/models/src/chat/summary.ts(0 hunks)
⛔ Files not processed due to max files limit (2)
- packages/models/src/chat/type.ts
- packages/models/src/index.ts
💤 Files with no reviewable changes (16)
- packages/db/src/mappers/chat/index.ts
- packages/models/src/chat/message/code.ts
- packages/models/src/chat/response.ts
- packages/models/src/chat/conversation/index.ts
- packages/db/src/mappers/index.ts
- packages/models/src/chat/message/checkpoint.ts
- packages/models/src/chat/message/queue.ts
- packages/models/src/chat/message/message.ts
- packages/ai/eslint.config.js
- packages/models/src/chat/request.ts
- packages/ai/tsconfig.json
- packages/models/src/chat/suggestion.ts
- packages/models/src/chat/summary.ts
- packages/models/src/chat/message/index.ts
- packages/ai/package.json
- packages/models/src/chat/index.ts
✅ Files skipped from review due to trivial changes (16)
- apps/web/client/src/lib/ai/contexts/classes/image.ts
- apps/web/client/src/lib/ai/test/stream/convert.test.ts
- apps/web/client/src/lib/ai/test/contexts/error-context.test.ts
- apps/web/client/src/lib/ai/tools/classes/grep.ts
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsx
- apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsx
- apps/web/client/src/server/api/routers/code.ts
- apps/web/client/src/lib/ai/tools/classes/read-file.ts
- apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
- apps/web/client/src/lib/ai/chat/providers.ts
- apps/web/client/src/lib/ai/agents/root.ts
- apps/web/client/src/lib/ai/contexts/classes/error.ts
- apps/web/client/src/lib/ai/tools/classes/write-file.ts
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/sent-context-pill.tsx
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsx
- apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/lib/ai/contexts/helpers.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/lib/ai/tools/classes/typecheck.tsapps/web/client/src/lib/ai/tools/classes/sandbox.tsapps/web/client/src/lib/ai/tools/classes/scrape-url.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/lib/ai/tools/classes/list-branches.tsapps/web/client/src/lib/ai/tools/classes/onlook-instructions.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/lib/ai/tools/models/client.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/lib/ai/models/chat.tsapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/lib/ai/contexts/classes/file.tsapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/tokens.test.tsapps/web/client/src/lib/ai/contexts/classes/branch.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/lib/ai/tools/classes/web-search.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/lib/ai/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/lib/ai/tools/classes/read-style-guide.tsapps/web/client/src/lib/ai/tools/classes/bash-read.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/tools/classes/terminal-command.tsapps/web/client/src/lib/ai/test/contexts/index.test.tsapps/web/client/src/lib/ai/tools/classes/fuzzy-edit-file.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-edit.tsapps/web/client/src/lib/ai/contexts/classes/agent-rule.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-multi-edit.tsapps/web/client/src/lib/ai/test/tools/edit.test.tsapps/web/client/src/lib/ai/test/contexts/image-context.test.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/lib/ai/chat/mappers/message.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/lib/ai/contexts/models/base.tsapps/web/client/src/lib/ai/test/tools/read.test.tsapps/web/client/src/lib/ai/tools/classes/bash-edit.tsapps/web/client/src/lib/ai/tools/shared/helpers/files.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/lib/ai/tools/classes/check-errors.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/lib/ai/contexts/classes/highlight.tsapps/web/client/src/lib/ai/tools/classes/list-files.tsapps/web/client/src/lib/ai/tools/classes/upload-image.tsapps/web/client/src/lib/ai/tools/classes/glob.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/chat/mappers/conversation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/lib/ai/contexts/helpers.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/lib/ai/tools/classes/typecheck.tsapps/web/client/src/lib/ai/tools/classes/sandbox.tsapps/web/client/src/lib/ai/tools/classes/scrape-url.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/lib/ai/tools/classes/list-branches.tsapps/web/client/src/lib/ai/tools/classes/onlook-instructions.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tspackages/db/src/index.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/lib/ai/tools/models/client.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/lib/ai/models/chat.tsapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/lib/ai/contexts/classes/file.tsapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/tokens.test.tsapps/web/client/src/lib/ai/contexts/classes/branch.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/lib/ai/tools/classes/web-search.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/lib/ai/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/lib/ai/tools/classes/read-style-guide.tsapps/web/client/src/lib/ai/tools/classes/bash-read.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/tools/classes/terminal-command.tsapps/web/client/src/lib/ai/test/contexts/index.test.tsapps/web/client/src/lib/ai/tools/classes/fuzzy-edit-file.tspackages/db/src/seed/db.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-edit.tsapps/web/client/src/lib/ai/contexts/classes/agent-rule.tspackages/db/src/schema/chat/conversation.tspackages/db/src/defaults/conversation.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-multi-edit.tsapps/web/client/src/lib/ai/test/tools/edit.test.tsapps/web/client/src/lib/ai/test/contexts/image-context.test.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/lib/ai/chat/mappers/message.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/lib/ai/contexts/models/base.tsapps/web/client/src/lib/ai/test/tools/read.test.tsapps/web/client/src/lib/ai/tools/classes/bash-edit.tsapps/web/client/src/lib/ai/tools/shared/helpers/files.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxpackages/db/src/schema/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/lib/ai/tools/classes/check-errors.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/lib/ai/contexts/classes/highlight.tsapps/web/client/src/lib/ai/tools/classes/list-files.tsapps/web/client/src/lib/ai/tools/classes/upload-image.tsapps/web/client/src/lib/ai/tools/classes/glob.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/chat/mappers/conversation.tspackages/db/src/types/chat.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/lib/ai/contexts/helpers.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/lib/ai/tools/classes/typecheck.tsapps/web/client/src/lib/ai/tools/classes/sandbox.tsapps/web/client/src/lib/ai/tools/classes/scrape-url.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/lib/ai/tools/classes/list-branches.tsapps/web/client/src/lib/ai/tools/classes/onlook-instructions.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tspackages/db/src/index.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/lib/ai/tools/models/client.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/lib/ai/models/chat.tsapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/lib/ai/contexts/classes/file.tsapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/tokens.test.tsapps/web/client/src/lib/ai/contexts/classes/branch.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/lib/ai/tools/classes/web-search.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/lib/ai/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/lib/ai/tools/classes/read-style-guide.tsapps/web/client/src/lib/ai/tools/classes/bash-read.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/tools/classes/terminal-command.tsapps/web/client/src/lib/ai/test/contexts/index.test.tsapps/web/client/src/lib/ai/tools/classes/fuzzy-edit-file.tspackages/db/src/seed/db.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-edit.tsapps/web/client/src/lib/ai/contexts/classes/agent-rule.tspackages/db/src/schema/chat/conversation.tspackages/db/src/defaults/conversation.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/lib/ai/tools/classes/search-replace-multi-edit.tsapps/web/client/src/lib/ai/test/tools/edit.test.tsapps/web/client/src/lib/ai/test/contexts/image-context.test.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/lib/ai/chat/mappers/message.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/lib/ai/contexts/models/base.tsapps/web/client/src/lib/ai/test/tools/read.test.tsapps/web/client/src/lib/ai/tools/classes/bash-edit.tsapps/web/client/src/lib/ai/tools/shared/helpers/files.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxpackages/db/src/schema/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/lib/ai/tools/classes/check-errors.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/lib/ai/contexts/classes/highlight.tsapps/web/client/src/lib/ai/tools/classes/list-files.tsapps/web/client/src/lib/ai/tools/classes/upload-image.tsapps/web/client/src/lib/ai/tools/classes/glob.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/chat/mappers/conversation.tspackages/db/src/types/chat.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/api/chat/route.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-mode-toggle.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/queue-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/chat/conversation.ts
🧠 Learnings (4)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Applied to files:
apps/web/client/src/lib/ai/tools/models/client.tsapps/web/client/src/lib/ai/tools/classes/web-search.tsapps/web/client/src/lib/ai/tools/classes/read-style-guide.tsapps/web/client/src/lib/ai/tools/classes/terminal-command.tsapps/web/client/src/lib/ai/tools/shared/helpers/files.tsapps/web/client/src/lib/ai/tools/classes/list-files.tsapps/web/client/src/lib/ai/tools/classes/glob.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Use path aliases @/* and ~/* for imports mapping to src/*
Applied to files:
apps/web/client/src/lib/ai/tools/models/client.tsapps/web/client/src/lib/ai/tools/classes/web-search.tsapps/web/client/src/lib/ai/tools/classes/read-style-guide.tsapps/web/client/src/lib/ai/tools/classes/terminal-command.tsapps/web/client/src/lib/ai/tools/shared/helpers/files.tsapps/web/client/src/lib/ai/tools/classes/list-files.tsapps/web/client/src/lib/ai/tools/classes/glob.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
🧬 Code graph analysis (4)
apps/web/client/src/lib/ai/models/chat.ts (2)
packages/db/src/types/chat.ts (1)
MessageCheckpoints(83-83)apps/web/client/src/lib/ai/tools/toolset.ts (1)
ChatTools(73-73)
packages/db/src/schema/chat/message.ts (1)
packages/db/src/types/chat.ts (1)
MessagePart(105-105)
apps/web/client/src/lib/ai/chat/mappers/conversation.ts (1)
packages/db/src/types/chat.ts (1)
ChatSuggestion(92-95)
packages/db/src/types/chat.ts (1)
apps/web/client/src/lib/ai/models/chat.ts (1)
MessageCheckpointType(36-36)
| import { type MessageContext, MessageContextType } from '@onlook/db'; | ||
| import { Icons } from '@onlook/ui/icons'; | ||
| import { cn } from '@onlook/ui/utils'; | ||
| import { motion } from 'motion/react'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify client directive presence for motion library.
This component uses motion/react which is a client-only animation library. Per coding guidelines, components using client-only libraries require a 'use client' directive. If this file doesn't have one at the top (not visible in the provided code), verify that a parent component establishes the client boundary.
🏁 Script executed:
#!/bin/bash
# Check for 'use client' directive in this file and parent components
echo "=== Checking for 'use client' directive ==="
# Check this file
rg -n "^['\"]use client['\"]" apps/web/client/src/app/project/\[id\]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx
# Check parent context-pills directory
rg -n "^['\"]use client['\"]" apps/web/client/src/app/project/\[id\]/_components/right-panel/chat-tab/context-pills/
# Check chat-tab parent
rg -n "^['\"]use client['\"]" apps/web/client/src/app/project/\[id\]/_components/right-panel/chat-tab/Length of output: 623
Add 'use client' directive
This component imports motion/react (a client-only library); add 'use client' at the top to mark it as a client component.
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx
around line 4, the component imports a client-only library (motion/react) but
lacks the 'use client' directive; add the exact string 'use client' as the first
non-comment line of the file (above all imports) so the component is treated as
a client component and can safely import motion/react.
| import { createDefaultProject } from '@onlook/db'; | ||
| import { CreateRequestContextType } from '@onlook/models'; | ||
| import { type ImageMessageContext } from '@onlook/models/chat'; | ||
| import { type ImageMessageContext } from '@onlook/db'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify ImageMessageContext export from @onlook/db.
The import location for ImageMessageContext has changed from @onlook/models/chat to @onlook/db. Ensure that:
- The type is properly exported from
@onlook/dbwith the same structure (includingcontentandmimeTypeproperties used on lines 65-66). - All other files that use
ImageMessageContexthave been updated to import from the new location.
Run the following script to verify the type export and find any remaining imports from the old location:
🏁 Script executed:
#!/bin/bash
# Verify ImageMessageContext is exported from @onlook/db and check for old imports
echo "=== Checking if ImageMessageContext is exported from @onlook/db ==="
ast-grep --pattern $'export $_ ImageMessageContext'
echo ""
echo "=== Searching for any remaining imports from old location ==="
rg -n "import.*ImageMessageContext.*from.*['\"]@onlook/models" -g '!*.md'Length of output: 274
Export and update imports for ImageMessageContext.
- Add
export type ImageMessageContextto@onlook/db, ensuring it includescontentandmimeType. - Replace the import in apps/web/client/src/app/_components/hero/create.tsx:8 to import
ImageMessageContextfrom@onlook/dbinstead of@onlook/models.
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/create/manager.ts around line 5 and in
apps/web/client/src/app/_components/hero/create.tsx around line 8: add an
exported type ImageMessageContext to the @onlook/db package (export in its
types/index or barrel file) with the shape { content: string; mimeType: string }
and then change the import in
apps/web/client/src/app/_components/hero/create.tsx to import
ImageMessageContext from '@onlook/db' instead of '@onlook/models'; ensure the
new exported type is exported as a type-only export and update any places that
referenced the old import path to use the new one.
| @@ -1,4 +1,4 @@ | |||
| import { MessageContextType, type FileMessageContext, type HighlightMessageContext } from '@onlook/models'; | |||
| import { MessageContextType, type FileMessageContext, type HighlightMessageContext } from '@onlook/db'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify type exports from @onlook/db package.
The import path change from @onlook/models to @onlook/db aligns with the PR's objective to consolidate types. The use of the type keyword for type-only imports is correct.
Run the following script to verify the types are properly exported and check for consistency:
🏁 Script executed:
#!/bin/bash
# Verify that the types are exported from @onlook/db
echo "=== Checking @onlook/db package exports ==="
fd -e ts -e tsx "index.ts" packages/db | head -5 | xargs cat | grep -E "(FileMessageContext|HighlightMessageContext|MessageContextType)"
echo -e "\n=== Checking for remaining @onlook/models imports ==="
rg -nP --type=ts --type=tsx -C2 "from ['\"]@onlook/models['\"]" -g '!node_modules/**' -g '!dist/**' -g '!build/**'
echo -e "\n=== Verifying new @onlook/db imports are consistent ==="
rg -nP --type=ts --type=tsx -C1 "(FileMessageContext|HighlightMessageContext)" -g '!node_modules/**' -g '!dist/**' -g '!build/**' | head -30Length of output: 211
🏁 Script executed:
#!/bin/bash
echo "=== Locating index.ts in packages/db ==="
fd index.ts packages/db || find packages/db -type f -name "index.ts"
echo -e "\n=== Searching for type exports in packages/db ==="
rg -n "export.*FileMessageContext" -g 'packages/db/**/*.ts' || echo "No direct export for FileMessageContext"
rg -n "export.*HighlightMessageContext" -g 'packages/db/**/*.ts' || echo "No direct export for HighlightMessageContext"
rg -n "export.*MessageContextType" -g 'packages/db/**/*.ts' || echo "No direct export for MessageContextType"
echo -e "\n=== Checking for remaining @onlook/models imports ==="
rg -n "from ['\"]@onlook/models['\"]" -g '*.ts' -g '*.tsx' -g '!node_modules/**' -g '!dist/**' -g '!build/**' || echo "No remaining @onlook/models imports"
echo -e "\n=== Verifying @onlook/db imports across repo ==="
rg -n "from ['\"]@onlook/db['\"]" -g '*.ts' -g '*.tsx' -g '!node_modules/**' -g '!dist/**' -g '!build/**' | head -30Length of output: 31840
Verify public exports and migrate legacy imports
- Confirm
FileMessageContext,HighlightMessageContextandMessageContextTypeare re-exported from@onlook/db’s entrypoint (packages/db/src/index.ts). - Replace all remaining
from '@onlook/models'imports withfrom '@onlook/db'across the repo.
| @@ -1,5 +1,4 @@ | |||
| import type { ChatMessage } from "@onlook/models"; | |||
| import { type MessageCheckpoints, type MessageContext } from "@onlook/models"; | |||
| import type { MessageCheckpoints, MessageContext, MessagePart } from "../../types/chat"; | |||
There was a problem hiding this comment.
MessagePart type is defined as any - type safety compromised.
The MessagePart type imported from ../../types/chat is currently defined as any (see packages/db/src/types/chat.ts:104). This violates the coding guideline that states: "Do not use the any type unless necessary."
Using any defeats TypeScript's type safety for message parts, which could be text, images, tool calls, or other data types. This should be properly typed with a discriminated union.
As per coding guidelines.
Consider defining MessagePart with a proper discriminated union type:
export type MessagePart =
| { type: 'text'; text: string }
| { type: 'image'; image: string | URL }
| { type: 'tool-call'; toolCallId: string; toolName: string; args: unknown }
| { type: 'tool-result'; toolCallId: string; toolName: string; result: unknown };Then update the import in this file accordingly.
packages/db/src/types/chat.ts
Outdated
| // Message parts type (generic - actual structure defined by AI SDK) | ||
| export type MessagePart = any; |
There was a problem hiding this comment.
Avoid using the any type.
The MessagePart type is defined as any, which bypasses TypeScript's type checking and violates the project's coding guidelines.
As per coding guidelines
If the AI SDK provides a specific type for message parts, import and use it:
+import type { MessagePart as AIMessagePart } from 'ai';
+
-export type MessagePart = any;
+export type MessagePart = AIMessagePart;If the structure is truly unknown or varies significantly, consider using unknown with type guards, or define a more specific union type based on the expected shapes.
🤖 Prompt for AI Agents
In packages/db/src/types/chat.ts around lines 104-105, MessagePart is declared
as `any`, which bypasses type checking; replace `any` by importing and using the
AI SDK's provided message-part type if available (e.g., import { MessagePartType
} from 'ai-sdk' and use that), or if no SDK type exists, change to `unknown` and
add appropriate type guards where MessagePart is consumed, or define a precise
union/interface representing expected shapes (e.g., TextPart | ImagePart) to
capture the real structure instead of `any`.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx (1)
34-51: LGTM! Styling updates from lint/prettier.The class name reordering and adjustments appear to be from the lint/prettier configuration updates mentioned in the PR objectives.
Note: A previous review already flagged that this component uses
motion/reactbut lacks a'use client'directive (see past review comments). That concern remains unresolved.
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
163-163: Consider using next-intl for user-facing messages.Multiple user-facing error and success messages are hardcoded. Per coding guidelines, prefer next-intl messages/hooks for all user-facing text to support internationalization.
Examples:
- Line 163: "Failed to send message. Please try again."
- Line 170: "Ask a question about your project..."
- Line 219: "Failed to get branch data"
- Line 242: "Image added to chat"
- Line 305: "No active frame available for screenshot"
- Line 341-343: Screenshot-related messages
Note that line 172 already uses
t(transKeys.editor.panels.edit.tabs.chat.input.placeholder), demonstrating the preferred pattern.Based on coding guidelines
Also applies to: 170-170, 219-219, 225-225, 242-242, 305-305, 341-343
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(1 hunks)apps/web/client/src/lib/ai/models/chat.ts(1 hunks)packages/db/package.json(1 hunks)packages/db/src/schema/chat/message.ts(2 hunks)packages/models/src/chat/index.ts(1 hunks)packages/models/src/chat/message/checkpoint.ts(1 hunks)packages/models/src/chat/message/context.ts(1 hunks)packages/models/src/chat/message/index.ts(1 hunks)packages/models/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx
- apps/web/client/src/lib/ai/models/chat.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxpackages/models/src/index.tspackages/models/src/chat/index.tspackages/db/src/schema/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxpackages/models/src/chat/message/checkpoint.tspackages/models/src/chat/message/index.tspackages/models/src/chat/message/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxpackages/models/src/index.tspackages/models/src/chat/index.tspackages/db/src/schema/chat/message.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxpackages/models/src/chat/message/checkpoint.tspackages/models/src/chat/message/index.tspackages/models/src/chat/message/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
🧬 Code graph analysis (2)
packages/db/src/schema/chat/message.ts (1)
packages/models/src/chat/message/index.ts (1)
MessagePart(5-5)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
getContextIcon(19-33)
🔇 Additional comments (11)
packages/models/src/index.ts (1)
4-4: LGTM! Expanding public API surface.The addition of the branching module export is consistent with the existing export pattern and properly extends the public API.
packages/models/src/chat/message/checkpoint.ts (1)
13-13: LGTM! Documentation cleanup.The comment removal doesn't affect the field's behavior or type signature. The
branchIdremains an optional string as intended.packages/models/src/chat/message/context.ts (1)
1-1: LGTM! More specific import path.The change to import
Branchfrom the more specific module path'../../project/branch'is good practice and aligns with the broader refactoring toward granular module imports.packages/models/src/chat/index.ts (1)
3-4: LGTM! Consistent export style.The change to directory-style exports (without
.tsextensions) is cleaner and consistent with the other exports in this file.packages/db/package.json (1)
30-30: LGTM! Necessary dependency addition.Adding
@onlook/modelsas a dependency is required for the schema type imports. The wildcard version"*"is appropriate for monorepo workspace dependencies.packages/db/src/schema/chat/message.ts (1)
1-1: Import consolidation is good, but type safety concern remains.The change to import
MessagePartfrom@onlook/modelsconsolidates the type source, which is an improvement for maintainability. However,MessagePartis currently defined asanyinpackages/models/src/chat/message/index.ts, which compromises type safety for thepartscolumn.Ensure that the
MessageParttype inpackages/models/src/chat/message/index.tsis updated with proper typing to maintain type safety for message parts stored in the database.Also applies to: 20-20
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
9-11: LGTM! Import path refactoring aligns with PR objectives.The import changes correctly move AI-related types from external packages to internal aliases (
@/lib/ai/models/chatand@onlook/models), consistent with the repository-wide lint and refactoring effort.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
1-3: LGTM! Import paths correctly updated.The import path changes align with the repository-wide refactoring to consolidate AI-related types under internal aliases.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (2)
2-3: LGTM! Import refactoring is correct.The type-only imports for
ImageMessageContextandMessageContext, and the value import forMessageContextTypefrom@onlook/modelsalign with the PR's import path consolidation.
39-82: No changes required: this observer component lives under the client boundary inapps/web/client/src/app/project/[id]/_components/right-panel/index.tsx, so child observers don’t need their ownuse clientdirective.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx (1)
1-5: LGTM! Import refactoring is correct.The addition of React and motion imports, along with the consolidation of
MessageContextandMessageContextTypeimports to@onlook/models, aligns with the PR's objectives.
| // MessagePart type (generic - actual structure defined by AI SDK) | ||
| export type MessagePart = any; |
There was a problem hiding this comment.
MessagePart type defined as any violates coding guidelines.
Defining MessagePart as any defeats TypeScript's type safety and violates the guideline: "Do not use the any type unless necessary." While the comment mentions the AI SDK defines the structure, proper typing is still achievable.
As per coding guidelines.
Consider defining MessagePart with a discriminated union that reflects the AI SDK's actual message part types:
// MessagePart type (generic - actual structure defined by AI SDK)
export type MessagePart =
| { type: 'text'; text: string }
| { type: 'image'; image: string | URL }
| { type: 'tool-call'; toolCallId: string; toolName: string; args: unknown }
| { type: 'tool-result'; toolCallId: string; toolName: string; result: unknown };If the AI SDK exports a proper type, import and re-export it instead of using any.
🤖 Prompt for AI Agents
In packages/models/src/chat/message/index.ts around lines 4 to 5, MessagePart is
currently defined as any which breaks TypeScript safety; replace it with a
discriminated union covering the SDK's expected part shapes (for example types
like 'text' with text: string, 'image' with image: string | URL, 'tool-call'
with toolCallId/toolName/args: unknown, and 'tool-result' with
toolCallId/toolName/result: unknown) and export that union; if the AI SDK
already exports a MessagePart type, import and re-export that type instead of
declaring any.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (1)
134-136: Use i18n for user-facing error messages.The toast error messages contain hardcoded English strings. Per coding guidelines, user-facing text should use next-intl messages/hooks for proper internationalization.
As per coding guidelines.
Consider defining these messages in your i18n message files and using them via next-intl hooks. Example:
import { useTranslations } from 'next-intl'; // Inside component: const t = useTranslations('Project'); // Then use: toast.error(t('createRequestFailed'), { description: error instanceof Error ? error.message : t('unknownError'), });Also applies to: 141-143
♻️ Duplicate comments (4)
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx (4)
20-20: Align penpal imports to a single source.This import still pulls from
'penpal'while types/constants come from'@onlook/penpal', which can cause version/shape mismatches. Replace with:-import { WindowMessenger, connect } from 'penpal'; +import { WindowMessenger, connect } from '@onlook/penpal';
103-106: Avoid wildcard origins in WindowMessenger (security posture).
allowedOrigins: ['*']weakens postMessage origin checks. Derive an explicit origin fromframe.urlinstead:+ const origin = (() => { + try { + return new URL(frame.url, window.location.href).origin; + } catch { + return undefined; + } + })(); const messenger = new WindowMessenger({ remoteWindow: iframeRef.current.contentWindow, - allowedOrigins: ['*'], + allowedOrigins: origin ? [origin] : [], });
119-122: Replaceanywith concrete types in onDomProcessed payload.The
layerMapandrootNodeshould use theLayerNodetype from your penpal types:- onDomProcessed: (data: { layerMap: Record<string, any>; rootNode: any }) => { + onDomProcessed: (data: { layerMap: Record<string, LayerNode>; rootNode: LayerNode }) => { editorEngine.frameEvent.handleDomProcessed(frame.id, data); },Add the import:
import { PENPAL_PARENT_CHANNEL, type PenpalParentMethods, + type LayerNode, } from '@onlook/penpal';As per coding guidelines.
173-184: Don't swallow errors in promisifyMethod; rethrow for upstream handling.The catch block currently logs and returns
undefined, violating the declaredPromise<ReturnType<T>>and causing silent failures:return async (...args: Parameters<T>) => { try { if (!method) throw new Error('Method not initialized'); return method(...args); } catch (error) { console.error(`${PENPAL_PARENT_CHANNEL} (${frame.id}) - Method failed:`, error); + throw error; } };
🧹 Nitpick comments (3)
packages/utility/test/tw-merge.test.ts (1)
8-36: Consider removing duplicate test cases and clarifying descriptions.Several test cases have duplicate implementations or misleading descriptions:
- Lines 8-11 and 13-16 are identical tests with slightly different descriptions
- Lines 23-26 and 28-31 are identical tests
- Multiple test descriptions say "should keep the last background color class when passed as separate arguments" but some actually test single-string inputs or vice versa
Consider consolidating these tests and ensuring descriptions accurately reflect what each test is validating.
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx (1)
44-44: Preferunknown[]overany[]for fallback args.- return async (..._args: any[]) => { + return async (..._args: unknown[]) => {
unknown[]provides the same flexibility while maintaining type safety by preventing accidental unsafe operations on the arguments.As per coding guidelines.
packages/utility/src/color.ts (1)
1-1: Prefer community types over suppressing errors
Install@types/color-namerand remove the// @ts-expect-errordirective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
apps/web/client/package.json(4 hunks)apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(1 hunks)apps/web/client/src/components/store/editor/api/index.ts(1 hunks)apps/web/preload/package.json(3 hunks)apps/web/preload/script/index.ts(1 hunks)apps/web/server/package.json(1 hunks)docs/package.json(1 hunks)packages/code-provider/package.json(1 hunks)packages/constants/package.json(1 hunks)packages/constants/src/csb.ts(1 hunks)packages/db/package.json(1 hunks)packages/db/src/client.ts(1 hunks)packages/db/src/defaults/canvas.ts(1 hunks)packages/db/src/defaults/conversation.ts(1 hunks)packages/db/src/defaults/frame.ts(1 hunks)packages/db/src/defaults/project-settings.ts(1 hunks)packages/db/src/defaults/project.ts(1 hunks)packages/db/src/defaults/user-canvas.ts(1 hunks)packages/db/src/defaults/user-settings.ts(1 hunks)packages/db/src/seed/db.ts(2 hunks)packages/db/src/seed/stripe/backfill-subscriptions.ts(1 hunks)packages/db/src/seed/stripe/legacy-subscription.ts(1 hunks)packages/db/src/seed/stripe/stripe.ts(1 hunks)packages/email/package.json(1 hunks)packages/file-system/package.json(1 hunks)packages/fonts/package.json(1 hunks)packages/fonts/test/class-utils.test.ts(1 hunks)packages/fonts/test/font-extractors.test.ts(3 hunks)packages/fonts/test/validators.test.ts(2 hunks)packages/fonts/tsconfig.json(1 hunks)packages/models/package.json(2 hunks)packages/parser/package.json(1 hunks)packages/parser/tsconfig.json(1 hunks)packages/penpal/package.json(1 hunks)packages/penpal/src/child.ts(0 hunks)packages/penpal/src/index.ts(0 hunks)packages/stripe/package.json(1 hunks)packages/ui/package.json(1 hunks)packages/utility/package.json(1 hunks)packages/utility/src/color.ts(2 hunks)packages/utility/test/tw-merge.test.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/penpal/src/child.ts
- packages/penpal/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- packages/db/src/defaults/canvas.ts
- packages/file-system/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/fonts/tsconfig.json
- packages/db/package.json
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/constants/src/csb.tspackages/db/src/seed/stripe/legacy-subscription.tspackages/utility/test/tw-merge.test.tspackages/db/src/defaults/project-settings.tspackages/db/src/seed/stripe/backfill-subscriptions.tsapps/web/client/src/components/store/editor/api/index.tspackages/db/src/client.tspackages/fonts/test/font-extractors.test.tspackages/db/src/seed/stripe/stripe.tspackages/db/src/defaults/user-settings.tspackages/db/src/seed/db.tspackages/db/src/defaults/project.tspackages/fonts/test/class-utils.test.tsapps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxpackages/fonts/test/validators.test.tspackages/db/src/defaults/user-canvas.tsapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxpackages/db/src/defaults/frame.tspackages/utility/src/color.tspackages/db/src/defaults/conversation.tsapps/web/preload/script/index.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/constants/src/csb.tspackages/db/src/seed/stripe/legacy-subscription.tspackages/utility/test/tw-merge.test.tspackages/db/src/defaults/project-settings.tspackages/db/src/seed/stripe/backfill-subscriptions.tsapps/web/client/src/components/store/editor/api/index.tspackages/db/src/client.tspackages/fonts/test/font-extractors.test.tspackages/db/src/seed/stripe/stripe.tspackages/db/src/defaults/user-settings.tspackages/db/src/seed/db.tspackages/db/src/defaults/project.tspackages/fonts/test/class-utils.test.tsapps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxpackages/fonts/test/validators.test.tspackages/db/src/defaults/user-canvas.tsapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxpackages/db/src/defaults/frame.tspackages/utility/src/color.tspackages/db/src/defaults/conversation.tsapps/web/preload/script/index.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/store/editor/api/index.tsapps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
🧬 Code graph analysis (3)
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx (1)
apps/web/preload/script/api/index.ts (1)
PenpalChildMethods(104-104)
packages/fonts/test/validators.test.ts (2)
packages/fonts/src/helpers/validators.ts (1)
isValidLocalFontDeclaration(52-66)packages/parser/src/packages.ts (1)
t(10-10)
apps/web/preload/script/index.ts (2)
packages/penpal/src/parent.ts (2)
PenpalParentMethods(3-12)PromisifiedPenpalParentMethods(15-19)packages/models/src/element/layers.ts (1)
LayerNode(12-25)
🔇 Additional comments (25)
packages/parser/tsconfig.json (1)
7-7:test/dataexclusion remains recursiveExplicitly listing the directory keeps TypeScript from touching any of the nested fixtures while aligning with the new lint style. No further action needed.
packages/db/src/defaults/conversation.ts (2)
1-1: LGTM! Import refactoring aligns with architectural improvements.The change from package self-reference (
@onlook/db) to local relative path (../schema) is a sound pattern that avoids circular dependencies and improves module structure.
5-15: LGTM! Function implementation is correct.The factory function correctly initializes all required fields for a default conversation with appropriate defaults (UUID, timestamps, default name, empty suggestions array, and ROOT agent type).
apps/web/client/src/components/store/editor/api/index.ts (1)
4-4: LGTM! Import path correctly updated.The type import has been successfully migrated to the new internal AI models module using the
@/path alias. The use of thetypekeyword is appropriate for type-only imports.packages/utility/test/tw-merge.test.ts (1)
1-1: LGTM! Bun test framework import is correct.The import statement correctly brings in the necessary test functions from Bun's test framework, and all imported items (
describe,it,expect) are used throughout the file. This aligns with the PR's objective of applying repository-wide test framework updates.packages/fonts/test/font-extractors.test.ts (2)
8-8: LGTM: Type-only import follows best practices.The type-only import for
Tis correct since it's used exclusively for type annotations (T.ObjectExpression) throughout the file. This improves build performance and aligns with TypeScript best practices.
223-223: LGTM: Optional chaining provides type safety.While line 222 asserts that
fontshas exactly one element, TypeScript's type system doesn't narrow array access types based on runtime assertions. The optional chaining ensures type safety and prevents potential TypeScript errors when accessing thevariableproperty.packages/db/src/seed/stripe/legacy-subscription.ts (1)
2-2: LGTM! Import path refactoring is correct.The change from absolute module specifier to relative path is correct (
../../clientfromseed/stripe/resolves properly tosrc/client) and aligns with the broader monorepo refactoring for lint compliance.packages/db/src/seed/stripe/backfill-subscriptions.ts (1)
7-7: LGTM! Consistent import path refactoring.The relative path change matches the pattern established in related stripe seed files and correctly resolves to the db client module.
packages/db/src/defaults/frame.ts (1)
1-1: LGTM! Type import consolidated to local schema.The type import refactoring from
@onlook/dbto../schemais correct and aligns with the broader pattern of consolidating type definitions to local schema sources across the db package defaults.packages/db/src/defaults/user-canvas.ts (1)
2-2: LGTM! Type import follows consistent refactoring pattern.The type import change mirrors the pattern in other defaults files (e.g., frame.ts, canvas.ts) and correctly consolidates type definitions to the local schema module.
packages/db/src/seed/db.ts (2)
30-39: LGTM! Import consolidation aligns with monorepo refactoring.The import path changes correctly consolidate schema, client, and default creator imports to relative local paths while preserving public model imports from
@onlook/models. The path resolution is correct for all relative imports from the seed directory.
134-134: LGTM! Added required agentType field to seed data.The addition of
agentType: nullto the conversation seed data aligns with the updatedConversationtype requirements, ensuring the seed data satisfies the schema.packages/fonts/test/class-utils.test.ts (1)
4-4: LGTM! Proper type-only import.The change correctly separates the
Ttype namespace from the runtimetnamespace by using a type-only import. All usages ofTin this file are type annotations (lines 20, 21, 34, 40, 113), making this an appropriate and beneficial refactor for tree-shaking and code clarity.apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (2)
6-6: LGTM! Import path updated correctly.The import source for
ChatTypehas been correctly updated to use the internal alias@/lib/ai/models/chat, aligning with the PR's refactoring goals to consolidate chat-related types. The usage on line 125 remains unchanged and appropriate.
57-61: Verify dependency array completeness.The effect uses
editorEngine.projectIdanduser?.idbut they are not included in the dependency array. This could potentially cause the reconnect to be called with stale values if these change.Please verify whether
editorEngine.projectIdandusershould be included in the dependency array. IfeditorEngineis a stable reference from a store/context andprojectIddoesn't change after mount, and ifuser?.idis expected to remain stable during tab reactivation, the current implementation may be acceptable. Otherwise, consider adding them to the dependencies.packages/constants/src/csb.ts (1)
1-4: Approve SandboxTemplate relocation and import cleanup
No stale@onlook/modelsimports remain; interface definition and usage are correct.apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsx (3)
9-16: LGTM! Clean type definition for promisified methods.The import source change to
@onlook/web-preload/script/apiand the localPromisifiedPendpalChildMethodstype provide clear, type-safe async wrappers for RPC calls.
186-234: LGTM! Comprehensive promisified method mapping.The
remoteMethodsmemo correctly wraps all child methods with promise-based error handling and provides a safe fallback when the connection isn't ready. The dependency onpenpalChildensures proper re-computation when the connection state changes.
236-280: LGTM! Robust imperative handle with proper fallbacks.The implementation provides safe defaults when the iframe or penpal connection isn't available, correctly registers the view with the editor engine, and properly combines sync and async methods. The dependency array is accurate.
packages/fonts/test/validators.test.ts (2)
2-2: LGTM! Type imports improve tree-shaking and clarity.Converting
TandNodePathto type-only imports follows TypeScript best practices and aligns with the PR objective of applying stricter lint rules. This makes the intent explicit and enables better tree-shaking.
62-99: Good defensive pattern for type safety.The addition of
expect(decl).toBeDefined()assertions combined withif (decl)guards correctly handles TypeScript's strict null checking for array access. This pattern:
- Ensures tests fail explicitly if the AST structure is unexpected
- Satisfies TypeScript's type narrowing requirements
- Prevents potential runtime errors
The pattern is consistently applied across all affected test cases, which is excellent for maintainability.
packages/utility/package.json (1)
33-36: LGTM! Dependency additions align with code usage.The newly added dependencies are appropriate:
color-namer,css-color-names, andparse-css-colorare directly imported and used incolor.ts- Type definitions (
@types/bun,@types/lodash,@types/node) support TypeScript development- Other dependencies (
lodash,nanoid,@onlook/models) may be used elsewhere in the packageThe versions specified align with current stable releases.
Based on learnings.
Also applies to: 43-43, 45-46, 50-50, 52-52, 54-54
apps/web/preload/script/index.ts (2)
1-1: LGTM!The
LayerNodeimport is correctly used in the type definitions below.
55-55: Verify necessity of the double cast or supply proper Penpal types
Penpal’s TypeScript definitions aren’t present, so TS can’t validateparentasPromisifiedPenpalParentMethods. Either install/import Penpal’s types (e.g.@types/penpalor built-in.d.ts), define the missing interface for theconnectAPI, and cast directly, or document whyas unknown asis unavoidable.
| "exports": { | ||
| "./script/api": { | ||
| "types": "./script/api/index.ts", | ||
| "default": "./script/api/index.ts" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Restore the default package export.
Introducing exports without defining "." hides the package’s main entry, so any existing import '@onlook/web-preload' now throws ERR_PACKAGE_PATH_NOT_EXPORTED. Add a "." condition that points at script/index.ts (matching module) before shipping this.
Apply this diff:
- "exports": {
- "./script/api": {
- "types": "./script/api/index.ts",
- "default": "./script/api/index.ts"
- }
- },
+ "exports": {
+ ".": {
+ "types": "./script/index.ts",
+ "default": "./script/index.ts"
+ },
+ "./script/api": {
+ "types": "./script/api/index.ts",
+ "default": "./script/api/index.ts"
+ }
+ },📝 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.
| "exports": { | |
| "./script/api": { | |
| "types": "./script/api/index.ts", | |
| "default": "./script/api/index.ts" | |
| } | |
| }, | |
| "exports": { | |
| ".": { | |
| "types": "./script/index.ts", | |
| "default": "./script/index.ts" | |
| }, | |
| "./script/api": { | |
| "types": "./script/api/index.ts", | |
| "default": "./script/api/index.ts" | |
| } | |
| }, |
🤖 Prompt for AI Agents
In apps/web/preload/package.json around lines 6 to 11, the added "exports" block
omits the package's default entry (the "." export), causing imports like import
'@onlook/web-preload' to fail; restore the default package export by adding a
"." condition that points to "script/index.ts" (matching the existing "module")
before shipping so the package root resolves correctly.
| const PENPAL_CHILD_CHANNEL = 'PENPAL_CHILD'; | ||
|
|
||
| // Type for parent window methods (matches @onlook/penpal PenpalParentMethods) | ||
| type PenpalParentMethods = { | ||
| getFrameId: () => string; | ||
| getBranchId: () => string; | ||
| onWindowMutated: (data: { | ||
| added: Record<string, LayerNode>; | ||
| removed: Record<string, LayerNode>; | ||
| }) => void; | ||
| onWindowResized: () => void; | ||
| onDomProcessed: (data: { layerMap: Record<string, LayerNode>; rootNode: LayerNode }) => void; | ||
| }; | ||
|
|
||
| type PromisifiedPenpalParentMethods = { | ||
| [K in keyof PenpalParentMethods]: ( | ||
| ...args: Parameters<PenpalParentMethods[K]> | ||
| ) => Promise<ReturnType<PenpalParentMethods[K]>>; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Remove duplicate type definitions and import from the penpal package.
These type definitions are exact duplicates of types exported from packages/penpal/src/parent.ts. This creates a maintenance burden and violates the DRY principle. If the types in the penpal package are updated, this file must also be manually updated, risking type mismatches.
Apply this diff to import the types instead:
-const PENPAL_CHILD_CHANNEL = 'PENPAL_CHILD';
-
-// Type for parent window methods (matches @onlook/penpal PenpalParentMethods)
-type PenpalParentMethods = {
- getFrameId: () => string;
- getBranchId: () => string;
- onWindowMutated: (data: {
- added: Record<string, LayerNode>;
- removed: Record<string, LayerNode>;
- }) => void;
- onWindowResized: () => void;
- onDomProcessed: (data: { layerMap: Record<string, LayerNode>; rootNode: LayerNode }) => void;
-};
-
-type PromisifiedPenpalParentMethods = {
- [K in keyof PenpalParentMethods]: (
- ...args: Parameters<PenpalParentMethods[K]>
- ) => Promise<ReturnType<PenpalParentMethods[K]>>;
-};
+import {
+ PENPAL_CHILD_CHANNEL,
+ type PenpalParentMethods,
+ type PromisifiedPenpalParentMethods
+} from '@onlook/penpal';Note: If these exports are not available from @onlook/penpal, verify whether they should be added to the package's exports or if there's a specific reason for the local duplication.
Run the following script to verify the exports from the penpal package:
🏁 Script executed:
#!/bin/bash
# Description: Check if types are exported from @onlook/penpal package
# Check the penpal package exports
echo "=== Checking penpal package index.ts ==="
fd -t f "index.ts" packages/penpal/src/ --exec cat {}
echo -e "\n=== Checking parent.ts exports ==="
cat packages/penpal/src/parent.ts
echo -e "\n=== Searching for PENPAL_CHILD_CHANNEL in penpal package ==="
rg -n "PENPAL_CHILD_CHANNEL" packages/penpal/Length of output: 877
Replace local type definitions with imports
Remove the duplicate PenpalParentMethods and PromisifiedPenpalParentMethods definitions in apps/web/preload/script/index.ts and import them from @onlook/penpal. Retain the PENPAL_CHILD_CHANNEL constant locally (it isn’t exported by the package).
-import const PENPAL_CHILD_CHANNEL = 'PENPAL_CHILD';
-
-// Type for parent window methods (matches @onlook/penpal PenpalParentMethods)
-type PenpalParentMethods = {
- getFrameId: () => string;
- getBranchId: () => string;
- onWindowMutated: (data: {
- added: Record<string, LayerNode>;
- removed: Record<string, LayerNode>;
- }) => void;
- onWindowResized: () => void;
- onDomProcessed: (data: { layerMap: Record<string, LayerNode>; rootNode: LayerNode }) => void;
-};
-
-type PromisifiedPenpalParentMethods = {
- [K in keyof PenpalParentMethods]: (
- ...args: Parameters<PenpalParentMethods[K]>
- ) => Promise<ReturnType<PenpalParentMethods[K]>>;
-};
+const PENPAL_CHILD_CHANNEL = 'PENPAL_CHILD';
+import type { PenpalParentMethods, PromisifiedPenpalParentMethods } from '@onlook/penpal';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/preload/script/index.ts around lines 6 to 24, remove the duplicate
local type definitions for PenpalParentMethods and
PromisifiedPenpalParentMethods and instead import those types from the
@onlook/penpal package; keep the PENPAL_CHILD_CHANNEL constant defined locally.
Replace the local type blocks with a type-only import (e.g., import type {
PenpalParentMethods, PromisifiedPenpalParentMethods } from '@onlook/penpal'),
update any usages to reference the imported types, and ensure the file still
exports or uses PENPAL_CHILD_CHANNEL as before.
| if (this._name) { | ||
| return this._name; | ||
| } | ||
| const ntcResult = colorNamer(this.toHex6()).ntc?.[0]; | ||
| return ntcResult?.name?.toLowerCase().replace(' ', '-') ?? ''; |
There was a problem hiding this comment.
Fix string replacement to handle all spaces.
The replace(' ', '-') on Line 384 only replaces the first space in the color name. Color names with multiple spaces (e.g., "Light Steel Blue") will result in incorrect output like "light-steel blue" instead of "light-steel-blue".
Apply this diff to replace all spaces:
- return ntcResult?.name?.toLowerCase().replace(' ', '-') ?? '';
+ return ntcResult?.name?.toLowerCase().replace(/\s+/g, '-') ?? '';🤖 Prompt for AI Agents
In packages/utility/src/color.ts around lines 380 to 384, the name normalization
uses replace(' ', '-') which only replaces the first space; change it to replace
all whitespace (e.g., use .replace(/\s+/g, '-') or .replaceAll(' ', '-') ) so
multi-word names like "Light Steel Blue" become "light-steel-blue"; keep the
toLowerCase() and fallback behavior unchanged.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/lib/ai/test/prompt/prompt.test.ts (1)
17-17: SetSHOULD_UPDATE_DATAtofalseto avoid overwriting test expectations.With
SHOULD_UPDATE_DATAset totrue, tests will overwrite their expected output files on every run, causing them to always pass regardless of correctness. This defeats the purpose of regression testing and could allow bugs to slip through.Apply this diff:
- const SHOULD_UPDATE_DATA = true; + const SHOULD_UPDATE_DATA = false;
♻️ Duplicate comments (1)
apps/web/client/src/components/store/create/manager.ts (1)
5-5: Verify the import source forImageMessageContext.The import was updated to
'@onlook/models', but a previous review suggested importing from'@onlook/db'instead. Please confirm whether:
- The type is properly exported from
'@onlook/models'with the required structure (contentandmimeTypeproperties used on lines 65-66), or- The import should be updated to
'@onlook/db'as originally recommended.Run the following script to verify the export location:
#!/bin/bash # Verify where ImageMessageContext is exported from echo "=== Checking exports from @onlook/models ===" fd -e ts -e tsx 'index.ts$' packages/models | xargs rg -n "export.*ImageMessageContext" echo "" echo "=== Checking exports from @onlook/db ===" fd -e ts -e tsx 'index.ts$' packages/db | xargs rg -n "export.*ImageMessageContext" echo "" echo "=== Checking type definition of ImageMessageContext ===" ast-grep --pattern 'type ImageMessageContext = { $$$ }'
🧹 Nitpick comments (6)
apps/web/client/src/lib/ai/test/apply.test.ts (1)
2-2: Use path alias for the import.The import should use the path alias
@/lib/ai/applyinstead of the relative path../applyto align with the coding guidelines for this repository.As per coding guidelines.
Apply this diff to use the path alias:
-import { applyCodeChange } from '../apply'; +import { applyCodeChange } from '@/lib/ai/apply';apps/web/client/src/lib/ai/test/tools/edit.test.ts (1)
1-3: LGTM! Import path refactoring looks good.The updated import paths resolve correctly and maintain functionality. For consistency with line 3 and the coding guidelines that prefer path aliases, you could optionally refactor lines 1-2 to use the
@/alias:-import { SearchReplaceEditTool } from '../../tools/classes/search-replace-edit'; -import { SearchReplaceMultiEditFileTool } from '../../tools/classes/search-replace-multi-edit'; +import { SearchReplaceEditTool } from '@/lib/ai/tools/classes/search-replace-edit'; +import { SearchReplaceMultiEditFileTool } from '@/lib/ai/tools/classes/search-replace-multi-edit'; import type { EditorEngine } from '@/components/store/editor/engine';However, relative paths within the same module subtree are also acceptable, so this is a minor consistency suggestion rather than a requirement.
As per coding guidelines
apps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.ts (1)
3-3: Consider using path aliases for imports withinsrc.The import path correctly resolves to the target file, but per the coding guidelines, imports in
apps/web/client/src/**/*.{ts,tsx}should use path aliases (@/*or~/*) instead of relative paths when referencing files within thesrcdirectory.As per coding guidelines.
Apply this diff to use a path alias:
-import { AgentRuleContext } from '../../contexts/classes/agent-rule'; +import { AgentRuleContext } from '@/lib/ai/contexts/classes/agent-rule';(Assuming
@maps toapps/web/client/src. If~is the configured alias, use that instead.)apps/web/client/src/lib/ai/test/contexts/file-context.test.ts (1)
3-3: Consider using path alias for consistency.The import uses a relative path, but could use the path alias
@/lib/ai/contexts/classes/fileto align with the coding guideline that recommends using@/*and~/*aliases for imports mapping toapps/web/client/src/*. However, relative imports within the same module are also a common practice, so this is optional.As per coding guidelines.
-import { FileContext } from '../../contexts/classes/file'; +import { FileContext } from '@/lib/ai/contexts/classes/file';apps/web/client/src/lib/ai/test/stream/convert.test.ts (1)
131-131: Consider reducing reliance onas anycasts.While
as anyis commonly used in test files to create fixtures with invalid or incomplete data, the pervasive usage throughout this file conflicts with the coding guideline to "avoid using theanytype unless absolutely necessary."Consider alternatives such as:
- Using
Partial<ChatMessage['parts']>for incomplete test data- Creating test-specific factory functions with optional parameters
- Using type guards or more specific type assertions where possible
This would improve type safety and make the tests more maintainable without sacrificing the ability to test edge cases.
As per coding guidelines
Also applies to: 141-141, 146-146, 151-151, 158-158, 163-163, 186-186, 226-226, 272-272, 314-314, 335-335, 356-356, 395-395, 404-404, 432-432, 441-441, 475-475, 485-485, 492-492
apps/web/client/src/lib/ai/prompt/provider.ts (1)
5-5: Use path alias instead of relative import.Replace the relative import with a path alias to align with coding guidelines.
As per coding guidelines
Apply this diff:-import type { ChatMessage } from '../models/chat'; +import type { ChatMessage } from '@/lib/ai/models/chat';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
apps/web/client/package.json(4 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/sent-context-pill.tsx(1 hunks)apps/web/client/src/components/store/create/manager.ts(1 hunks)apps/web/client/src/components/store/editor/chat/context.ts(2 hunks)apps/web/client/src/components/store/editor/state/index.ts(1 hunks)apps/web/client/src/lib/ai/chat/mappers/conversation.ts(1 hunks)apps/web/client/src/lib/ai/prompt/provider.ts(1 hunks)apps/web/client/src/lib/ai/stream/index.ts(1 hunks)apps/web/client/src/lib/ai/test/apply.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/branch-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/error-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/file-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/highlight-context.test.ts(1 hunks)apps/web/client/src/lib/ai/test/contexts/image-context.test.ts(2 hunks)apps/web/client/src/lib/ai/test/contexts/index.test.ts(4 hunks)apps/web/client/src/lib/ai/test/prompt/prompt.test.ts(1 hunks)apps/web/client/src/lib/ai/test/stream/convert.test.ts(7 hunks)apps/web/client/src/lib/ai/test/tokens.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tools/edit.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tools/helpers.test.ts(1 hunks)apps/web/client/src/lib/ai/test/tools/read.test.ts(1 hunks)apps/web/client/src/lib/ai/tokens/index.ts(1 hunks)apps/web/client/src/lib/ai/tools/classes/upload-image.ts(1 hunks)apps/web/client/src/server/api/routers/chat/suggestion.ts(1 hunks)apps/web/client/test/frame/navigation.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/client/src/lib/ai/test/contexts/error-context.test.ts
- apps/web/client/src/server/api/routers/chat/suggestion.ts
- apps/web/client/src/lib/ai/test/contexts/image-context.test.ts
- apps/web/client/src/lib/ai/test/contexts/index.test.ts
- apps/web/client/src/lib/ai/test/tokens.test.ts
- apps/web/client/src/lib/ai/tools/classes/upload-image.ts
- apps/web/client/src/components/store/editor/chat/context.ts
- apps/web/client/src/lib/ai/test/tools/read.test.ts
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/sent-context-pill.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/lib/ai/test/apply.test.tsapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsxapps/web/client/src/lib/ai/test/tools/helpers.test.tsapps/web/client/src/components/store/editor/state/index.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/test/stream/convert.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/lib/ai/chat/mappers/conversation.tsapps/web/client/src/lib/ai/test/tools/edit.test.ts
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/lib/ai/test/apply.test.tsapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsxapps/web/client/src/lib/ai/test/tools/helpers.test.tsapps/web/client/src/components/store/editor/state/index.tsapps/web/client/test/frame/navigation.test.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/test/stream/convert.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/lib/ai/chat/mappers/conversation.tsapps/web/client/src/lib/ai/test/tools/edit.test.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsxapps/web/client/src/lib/ai/test/contexts/branch-context.test.tsapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsxapps/web/client/src/lib/ai/test/apply.test.tsapps/web/client/src/lib/ai/tokens/index.tsapps/web/client/src/lib/ai/stream/index.tsapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsxapps/web/client/src/lib/ai/test/tools/helpers.test.tsapps/web/client/src/components/store/editor/state/index.tsapps/web/client/test/frame/navigation.test.tsapps/web/client/src/lib/ai/test/prompt/prompt.test.tsapps/web/client/src/lib/ai/test/contexts/file-context.test.tsapps/web/client/src/lib/ai/test/stream/convert.test.tsapps/web/client/src/lib/ai/test/contexts/highlight-context.test.tsapps/web/client/src/lib/ai/test/contexts/agent-rule-context.test.tsapps/web/client/src/lib/ai/prompt/provider.tsapps/web/client/src/components/store/create/manager.tsapps/web/client/src/lib/ai/chat/mappers/conversation.tsapps/web/client/src/lib/ai/test/tools/edit.test.ts
🧬 Code graph analysis (2)
apps/web/client/src/lib/ai/test/stream/convert.test.ts (1)
apps/web/client/src/lib/ai/stream/index.ts (1)
ensureToolCallResults(54-88)
apps/web/client/src/lib/ai/chat/mappers/conversation.ts (1)
packages/models/src/chat/suggestion.ts (1)
ChatSuggestion(9-12)
🔇 Additional comments (14)
apps/web/client/src/lib/ai/test/apply.test.ts (1)
4-58: LGTM!The test structure and logic are correct. The conditional test execution based on environment variables is appropriate, and the test case properly validates the code change application with expected results.
apps/web/client/test/frame/navigation.test.ts (1)
1-1: LGTM! Clean migration to Bun's test framework.The import statement correctly adopts Bun's native test runner. All imported utilities (
describe,it,expect,beforeEach) are used throughout the test suite, and the change aligns with the PR's broader tooling updates.apps/web/client/src/components/store/editor/state/index.ts (1)
8-8: LGTM — import uses correct alias andChatType.EDITis exported.apps/web/client/src/lib/ai/test/contexts/branch-context.test.ts (1)
3-3: Import path updated and validated. The new import correctly points to../../contexts/classes/branch, and theBranchContextmodule exists at that location.apps/web/client/src/lib/ai/tokens/index.ts (1)
15-15: Tool input serialization is correct. It aligns with existing tests (which useJSON.stringify(invocation)) to generate the expected token content, so you can remove the TODO.apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/sidebar/file-tree.tsx (1)
191-191: LGTM! Type assertion aligns useResizeObserver ref with React's type expectations.The type assertion
as React.Ref<HTMLDivElement>correctly bridges the type gap between useResizeObserver's return type and React's ref prop expectations. Since the ref is applied to a div element, HTMLDivElement is the appropriate type.apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/page-tab/index.tsx (1)
155-155: LGTM! Consistent type assertion pattern.The type assertion follows the same pattern applied across other components in this PR (file-tree.tsx, layers-tab/index.tsx). This maintains type safety while satisfying React's ref prop requirements.
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/layers-tab/index.tsx (1)
133-133: LGTM! Completes the consistent ref typing pattern.This change aligns with the ref type assertions added to file-tree.tsx (line 191) and page-tab/index.tsx (line 155), ensuring consistent type safety across the left-panel components.
apps/web/client/src/lib/ai/test/contexts/file-context.test.ts (1)
5-278: Excellent test coverage and structure.The test suite is comprehensive and well-organized, covering:
- Static properties validation
- Prompt generation with various file types and edge cases
- Label extraction with different path formats
- File content generation for single and multiple files
- Truncated content generation
- Edge cases including unicode, special characters, and empty values
The mock creation utilities are clean and reusable, and all assertions are meaningful and correct.
apps/web/client/src/lib/ai/test/stream/convert.test.ts (2)
1-1: LGTM! Import paths correctly updated.The import path changes follow the coding guidelines by using the
@/*path alias and reflect the updated repository structure.Also applies to: 7-7
344-344: LGTM! State name improved for clarity.The change from
'error'to'output-error'provides more specificity about the error state and aligns with the implementation's state naming conventions.Also applies to: 364-364
apps/web/client/src/lib/ai/test/contexts/highlight-context.test.ts (1)
5-312: Well-structured and comprehensive test suite.The test suite provides thorough coverage of the HighlightContext class functionality, including edge cases, special characters, and boundary conditions.
apps/web/client/package.json (2)
107-107: No changes needed for Next.js version rangeVersion 15.5.3 is available on npm, so
"next": ">=15.5.3"is valid.
121-127: Peer dependencies accept React 19
All key libraries allow React >=16.8 (mobx-react-lite even includes ^19) and Next.js 15.5.3 exists, so no reversion is needed.
| "@types/react": "^19.1.13", | ||
| "@types/react-dom": "^19.1.13", |
There was a problem hiding this comment.
Align React type packages with the supported React major
Once we stay on React 18, the @types/react and @types/react-dom packages should also remain on the 18.x line; the 19.x typings include breaking type-level changes meant for the unreleased runtime.
Apply:
- "@types/react": "^19.1.13",
- "@types/react-dom": "^19.1.13",
+ "@types/react": "^18.2.49",
+ "@types/react-dom": "^18.2.19",📝 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.
| "@types/react": "^19.1.13", | |
| "@types/react-dom": "^19.1.13", | |
| "@types/react": "^18.2.49", | |
| "@types/react-dom": "^18.2.19", |
🤖 Prompt for AI Agents
In apps/web/client/package.json around lines 153-154, the project currently
depends on @types/react and @types/react-dom v19.x which are incompatible with
our React 18 runtime; change both entries to the 18.x type definitions (e.g.,
^18.2.0 or the latest 18.x patch), update the lockfile by running your package
manager install (npm/yarn/pnpm) and verify types compile locally.
| export type ChatConversation = { | ||
| id: string; | ||
| agentType: AgentType; | ||
| title: string | null; | ||
| projectId: string; | ||
| createdAt: Date; | ||
| updatedAt: Date; | ||
| suggestions: ChatSuggestion[]; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify no duplicate type definitions and consistent import updates.
Ensure that this is the only definition of ChatConversation in the codebase and that all imports have been updated to reference this local definition rather than @onlook/models.
Run the following script to verify:
🏁 Script executed:
#!/bin/bash
# Description: Check for duplicate ChatConversation type definitions and verify import updates
echo "=== Searching for ChatConversation type definitions ==="
ast-grep --pattern $'export type ChatConversation = {
$$$
}'
echo ""
echo "=== Searching for ChatConversation interface definitions ==="
ast-grep --pattern $'export interface ChatConversation {
$$$
}'
echo ""
echo "=== Searching for imports of ChatConversation from @onlook/models ==="
rg -nP --type=ts --type=tsx 'import.*ChatConversation.*from.*@onlook/models' -C2
echo ""
echo "=== Searching for all ChatConversation imports ==="
rg -nP --type=ts --type=tsx 'import.*ChatConversation' -C2Length of output: 1679
🏁 Script executed:
#!/bin/bash
echo "=== Searching for imports of ChatConversation from @onlook/models in client code ==="
rg -nP "import\s*\{\s*ChatConversation\s*\}\s*from\s*['\"]@onlook/models['\"]" -C2 --glob 'apps/web/client/src/**/*.{ts,tsx}'
echo ""
echo "=== Searching for any ChatConversation usage outside conversation.ts ==="
rg -n "ChatConversation" -C2 --glob 'apps/web/client/src/**/*.{ts,tsx}' | grep -v 'apps/web/client/src/lib/ai/chat/mappers/conversation.ts'Length of output: 4278
Update remaining model imports
Replace the ChatConversation import in apps/web/client/src/components/store/editor/chat/conversation.ts (currently from @onlook/models) with:
import type { ChatConversation } from '@/lib/ai/chat/mappers/conversation';🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/chat/conversation.ts around the
import section (lines referenced in the review), replace the existing
ChatConversation import from "@onlook/models" with a local type-only import:
import type { ChatConversation } from '@/lib/ai/chat/mappers/conversation'; and
remove the old import to ensure the file uses the updated local model mapping.
| @@ -1,4 +1,4 @@ | |||
| import { type ChatMessage } from '@onlook/models'; | |||
| import { type ChatMessage } from '../models/chat'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use path aliases instead of relative imports.
The coding guidelines require using path aliases @/* or ~/* for imports mapping to src/* in web client files. Replace relative imports with path aliases for consistency.
As per coding guidelines.
Apply this diff to use path aliases:
-import { type ChatMessage } from '../models/chat';
+import { type ChatMessage } from '@/lib/ai/models/chat';
import { convertToModelMessages, type ModelMessage, type ToolUIPart } from 'ai';
-import { getHydratedUserMessage, type HydrateMessageOptions } from '../prompt';
+import { getHydratedUserMessage, type HydrateMessageOptions } from '@/lib/ai/prompt';Also applies to: 3-3
🤖 Prompt for AI Agents
In apps/web/client/src/lib/ai/stream/index.ts around lines 1-3, replace the
relative import(s) like ../models/chat with the project path alias that maps to
src (e.g. @/lib/ai/models/chat) so the file uses the required @/* or ~/* alias
convention; update any other similar relative imports in this file (line 3) to
their corresponding alias paths and save.
| import { MessageContextType, type HighlightMessageContext } from '@onlook/models'; | ||
| import { describe, expect, test } from 'bun:test'; | ||
| import { HighlightContext } from '../../src/contexts/classes/highlight'; | ||
| import { HighlightContext } from '../../contexts/classes/highlight'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use path alias instead of relative import.
Replace the relative import with a path alias to comply with the project's coding guidelines.
As per coding guidelines.
Apply this diff:
-import { HighlightContext } from '../../contexts/classes/highlight';
+import { HighlightContext } from '@/lib/ai/contexts/classes/highlight';📝 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.
| import { HighlightContext } from '../../contexts/classes/highlight'; | |
| import { HighlightContext } from '@/lib/ai/contexts/classes/highlight'; |
🤖 Prompt for AI Agents
In apps/web/client/src/lib/ai/test/contexts/highlight-context.test.ts around
line 3, the file uses a relative import ("../../contexts/classes/highlight");
replace it with the project's configured path alias (e.g. import from the alias
that maps to src/lib/ai/contexts/classes/highlight — check tsconfig/paths or
webpack config for the exact alias like "@/lib/..." or "lib/...") so the test
follows the coding guidelines; update the import to use that alias and run the
test/build to ensure the path mapping is correct.
| import { describe, expect, test } from 'bun:test'; | ||
| import path from 'path'; | ||
| import { FileContext, HighlightContext } from '../../src/contexts/classes'; | ||
| import { FileContext, HighlightContext } from '../../contexts/classes'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use path aliases instead of relative imports.
The coding guidelines require using path aliases @/* or ~/* for imports within apps/web/client/src/**/*.{ts,tsx} files. Replace relative imports with path aliases for consistency and maintainability.
As per coding guidelines.
Apply this diff:
-import { FileContext, HighlightContext } from '../../contexts/classes';
+import { FileContext, HighlightContext } from '@/lib/ai/contexts/classes';
import {
type HydrateMessageOptions,
getCreatePageSystemPrompt,
getHydratedUserMessage,
getSummaryPrompt,
getSystemPrompt,
-} from '../../prompt/provider';
+} from '@/lib/ai/prompt/provider';Also applies to: 11-11
🤖 Prompt for AI Agents
In apps/web/client/src/lib/ai/test/prompt/prompt.test.ts around lines 4 and 11,
replace the relative import '../../contexts/classes' with the project path alias
'@/lib/ai/contexts/classes' to comply with coding guidelines; update both import
statements to use the alias.
| escapeForShell, | ||
| isPathExcluded | ||
| } from '@onlook/ai/src/tools/shared/helpers/cli'; | ||
| } from '../../tools/shared/helpers/cli'; |
There was a problem hiding this comment.
Use path alias instead of relative import.
The import should use the path alias @/ instead of a relative path to comply with the coding guidelines for files in apps/web/client/src/**/*.{ts,tsx}.
Apply this diff to use the path alias:
-} from '../../tools/shared/helpers/cli';
+} from '@/lib/ai/tools/shared/helpers/cli';As per coding guidelines.
📝 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.
| } from '../../tools/shared/helpers/cli'; | |
| } from '@/lib/ai/tools/shared/helpers/cli'; |
🤖 Prompt for AI Agents
In apps/web/client/src/lib/ai/test/tools/helpers.test.ts around line 11, the
import uses a relative path ("../../tools/shared/helpers/cli"); replace it with
the project path alias by changing the import to use
"@/tools/shared/helpers/cli" so the file follows the coding guidelines for
apps/web/client/src/**/*.{ts,tsx}; ensure any surrounding imports remain valid
and update only this import path.
| @@ -1,4 +1,4 @@ | |||
| import { type ChatMessage } from '@onlook/models'; | |||
| import { type ChatMessage } from '../models/chat'; | |||
There was a problem hiding this comment.
Use path alias instead of relative import.
The import should use the @/ path alias to align with the coding guidelines for this directory pattern.
As per coding guidelines
Apply this diff:
-import { type ChatMessage } from '../models/chat';
+import { type ChatMessage } from '@/lib/ai/models/chat';📝 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.
| import { type ChatMessage } from '../models/chat'; | |
| import { type ChatMessage } from '@/lib/ai/models/chat'; |
🤖 Prompt for AI Agents
In apps/web/client/src/lib/ai/tokens/index.ts around line 1, the import uses a
relative path; replace the relative import with the project path alias. Change
the import to use "@/lib/ai/models/chat" (i.e., import { type ChatMessage } from
'@/lib/ai/models/chat') so it conforms to the codebase aliasing and directory
pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/preload/package.json (1)
6-11: Restore the package root export.Line 6 defines an
exportsmap without a"."entry, soimport '@onlook/web-preload'will now throwERR_PACKAGE_PATH_NOT_EXPORTED. Please reintroduce the root condition pointing toscript/index.tsbefore shipping."exports": { + ".": { + "types": "./script/index.ts", + "default": "./script/index.ts" + }, "./script/api": { "types": "./script/api/index.ts", "default": "./script/api/index.ts" } },
🧹 Nitpick comments (2)
packages/scripts/test/simple.test.ts (1)
66-71: Type assertion is appropriate for test context.The
as keyof typeof responsesassertion correctly addresses TypeScript's strict index signature checking. SinceAPI_KEYSandresponseshave aligned keys in this test, the assertion is safe. The|| ''fallback provides additional safety.For production code, consider defining an explicit shared type to eliminate the need for assertions:
type TestKeys = 'TEST_KEY1' | 'TEST_KEY2'; const API_KEYS: Record<TestKeys, { required: boolean }> = { ... }; const responses: Record<TestKeys, string> = { ... };However, for test files, the current approach is pragmatic and acceptable.
packages/image-server/test/image.test.ts (1)
390-398: Consider adding a defensive check forfindIndex.While the logic ensures
findIndexwill always succeed (sinceskippedResultsis filtered fromresultsand object references are preserved), adding a defensive check for-1would make the code more robust and eliminate the need for the non-null assertion:skippedResults.forEach((result, index) => { const skippedIndex = results.findIndex((r) => r === result); + if (skippedIndex === -1) return; - const fileName = allImages[skippedIndex]!; + const fileName = allImages[skippedIndex]; const extension = path.extname(fileName).toLowerCase();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
apps/backend/package.json(0 hunks)apps/web/client/package.json(3 hunks)apps/web/package.json(1 hunks)apps/web/preload/package.json(2 hunks)apps/web/server/package.json(2 hunks)docs/package.json(1 hunks)package.json(2 hunks)packages/code-provider/package.json(1 hunks)packages/constants/package.json(1 hunks)packages/db/package.json(1 hunks)packages/email/package.json(1 hunks)packages/file-system/package.json(1 hunks)packages/fonts/package.json(1 hunks)packages/git/package.json(1 hunks)packages/github/package.json(1 hunks)packages/growth/package.json(1 hunks)packages/growth/tests/inject.test.ts(1 hunks)packages/image-server/package.json(1 hunks)packages/image-server/test/image.test.ts(2 hunks)packages/models/package.json(2 hunks)packages/parser/package.json(1 hunks)packages/penpal/package.json(1 hunks)packages/rpc/package.json(1 hunks)packages/scripts/package.json(1 hunks)packages/scripts/test/integration.test.ts(2 hunks)packages/scripts/test/simple.test.ts(1 hunks)packages/scripts/tsconfig.json(0 hunks)packages/stripe/package.json(1 hunks)packages/types/package.json(1 hunks)packages/ui/package.json(3 hunks)packages/ui/src/components/progress-with-interval.tsx(1 hunks)packages/utility/package.json(1 hunks)tooling/eslint/package.json(1 hunks)tooling/prettier/package.json(1 hunks)tooling/typescript/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/backend/package.json
- packages/scripts/tsconfig.json
✅ Files skipped from review due to trivial changes (1)
- packages/growth/tests/inject.test.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/db/package.json
- packages/email/package.json
- packages/constants/package.json
- docs/package.json
- packages/fonts/package.json
- packages/models/package.json
- apps/web/package.json
- packages/file-system/package.json
- packages/parser/package.json
- packages/ui/package.json
- apps/web/server/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/scripts/test/integration.test.tspackages/image-server/test/image.test.tspackages/scripts/test/simple.test.tspackages/ui/src/components/progress-with-interval.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/scripts/test/integration.test.tspackages/image-server/test/image.test.tspackages/scripts/test/simple.test.tspackages/ui/src/components/progress-with-interval.tsx
🔇 Additional comments (5)
packages/scripts/test/integration.test.ts (1)
169-171: Consistent type assertion pattern applied correctly.Both instances use
as keyof typeofto satisfy TypeScript's strict index signature checking when accessing dynamically keyed objects. The pattern is consistent with the approach insimple.test.tsand is safe in these test contexts where key alignment is guaranteed.The truthiness checks (
!responses[...]) safely handle any undefined values.Also applies to: 181-183
packages/image-server/test/image.test.ts (1)
374-387: LGTM: Non-null assertion justified by length check.The non-null assertion at line 375 is safe because the test explicitly verifies that
results.lengthequalsallImages.lengthat line 367, guaranteeing that every index in theresultsarray is valid forallImages.packages/scripts/package.json (1)
22-24: zod v4.x is published and safe to use. Stable 4.x releases (4.1.8 through 4.1.12) are available on npm.packages/stripe/package.json (1)
36-40: Dependency versions published
dotenv@^17.2.1anduuid@^11.1.0are available on npm; ensure the lockfile reflects these updates before merging.packages/utility/package.json (1)
33-39: No issues found: TypeScript and @types/node versions existBoth
typescript@5.8.2and@types/node@24.7.2are published on npm.
| }: ProgressWithIntervalProps) => { | ||
| const [progress, setProgress] = useState(0); | ||
| const progressInterval = useRef<Timer | null>(null); | ||
| const progressInterval = useRef<NodeJS.Timeout | null>(null); |
There was a problem hiding this comment.
Use cross-platform timer type instead of NodeJS.Timeout.
UI components typically run in browsers where setInterval returns number, not NodeJS.Timeout. Using NodeJS.Timeout creates a type mismatch for browser environments.
Apply this diff for cross-platform compatibility:
- const progressInterval = useRef<NodeJS.Timeout | null>(null);
+ const progressInterval = useRef<ReturnType<typeof setInterval> | null>(null);This ensures correct typing across both Node.js (e.g., SSR) and browser contexts.
📝 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 progressInterval = useRef<NodeJS.Timeout | null>(null); | |
| const progressInterval = useRef<ReturnType<typeof setInterval> | null>(null); |
🤖 Prompt for AI Agents
In packages/ui/src/components/progress-with-interval.tsx around line 28, the ref
is typed as NodeJS.Timeout which mismatches browser timers; change the type to a
cross-platform timer type such as ReturnType<typeof setInterval> | null (or
number | null) and update any clearInterval/assignments accordingly so the ref
works in both browser and Node (SSR) environments.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/next-sitemap.config.js (1)
2-6: Revert next-sitemap.config.js to CommonJS exports
next-sitemap requires a CommonJS config. Changeexport default {…}back tomodule.exports = {…}(or rename the file to.cjs) to ensure sitemap generation works.apps/web/client/package.json (1)
47-65: Use workspace protocol instead of "*" for internal packages."*" can resolve to published registry versions and cause mismatches. Prefer explicit workspace ranges.
Apply:
- "@onlook/code-provider": "*", - "@onlook/constants": "*", - "@onlook/db": "*", - "@onlook/email": "*", - "@onlook/file-system": "*", - "@onlook/fonts": "*", - "@onlook/git": "*", - "@onlook/github": "*", - "@onlook/growth": "*", - "@onlook/image-server": "*", - "@onlook/models": "*", - "@onlook/parser": "*", - "@onlook/penpal": "*", - "@onlook/rpc": "*", - "@onlook/stripe": "*", - "@onlook/ui": "*", - "@onlook/utility": "*", - "@onlook/web-preload": "*", + "@onlook/code-provider": "workspace:*", + "@onlook/constants": "workspace:*", + "@onlook/db": "workspace:*", + "@onlook/email": "workspace:*", + "@onlook/file-system": "workspace:*", + "@onlook/fonts": "workspace:*", + "@onlook/git": "workspace:*", + "@onlook/github": "workspace:*", + "@onlook/growth": "workspace:*", + "@onlook/image-server": "workspace:*", + "@onlook/models": "workspace:*", + "@onlook/parser": "workspace:*", + "@onlook/penpal": "workspace:*", + "@onlook/rpc": "workspace:*", + "@onlook/stripe": "workspace:*", + "@onlook/ui": "workspace:*", + "@onlook/utility": "workspace:*", + "@onlook/web-preload": "workspace:*",
🧹 Nitpick comments (1)
apps/web/client/package.json (1)
156-157: Consider removing @types/react with React 19.*React 19 bundles its own types; the DefinitelyTyped packages may be redundant/conflicting. If you keep them, ensure they’re required by tooling.
Option to remove:
- "@types/react": "^19.1.13", - "@types/react-dom": "^19.1.13",Would you like a follow-up PR to clean these up repo-wide after verifying?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/web/client/package.json(3 hunks)docs/next-sitemap.config.js(1 hunks)docs/package.json(1 hunks)docs/public/sitemap-0.xml(1 hunks)tooling/eslint/package.json(2 hunks)
🔇 Additional comments (5)
tooling/eslint/package.json (1)
19-19: Keep and configure Prettier plugin and config
eslint-plugin-prettieris imported in tooling/eslint/base.js and theprettier/prettierrule is applied, so remove only if you drop that rule.
@onlook/prettierprovides theprettierConfigused at runtime—consider promoting it to a peerDependency if consumers supply their own Prettier config; otherwise leave it as a dependency.docs/package.json (2)
23-24: React 19 alignment looks goodDocs now match repo-wide React 19; resolves prior downgrade issue.
29-34: Compatibility verified for React 19/Next.js 15
fumadocs-ui@15.6.9 and fumadocs-mdx@11.7.4 declare peerDependencies targeting Next.js 15 (React 19+); no changes needed.apps/web/client/package.json (2)
123-129: React 19 upgrade looks good; confirm framework compatibility.Ensure your Next version fully supports React 19 features used in this app.
113-115: Prune unused DB drivers in web client
No imports or requires ofpgorpostgresdetected inapps/web/client; remove whichever (or both) is not needed to shrink install size.
| "dependencies": { | ||
| "@onlook/ui": "*", | ||
| "fumadocs-core": "^15.6.9", | ||
| "fumadocs-mdx": "^11.7.4", | ||
| "fumadocs-ui": "^15.6.9", | ||
| "next": ">=15.4.7", | ||
| "fumadocs-core": "15.6.9", | ||
| "fumadocs-mdx": "11.7.4", | ||
| "fumadocs-ui": "15.6.9", | ||
| "next": ">=15.5.3", | ||
| "next-sitemap": "^4.2.3", | ||
| "react": "^19.1.0", | ||
| "react-dom": "^19.1.0" | ||
| "react": "^19.1.13", | ||
| "react-dom": "^19.1.13" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use workspace protocol for internal packages
To prevent registry drift and align with sherif’s guarantees, link internal deps via workspace protocol.
- "@onlook/ui": "*",
+ "@onlook/ui": "workspace:*",📝 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.
| "dependencies": { | |
| "@onlook/ui": "*", | |
| "fumadocs-core": "^15.6.9", | |
| "fumadocs-mdx": "^11.7.4", | |
| "fumadocs-ui": "^15.6.9", | |
| "next": ">=15.4.7", | |
| "fumadocs-core": "15.6.9", | |
| "fumadocs-mdx": "11.7.4", | |
| "fumadocs-ui": "15.6.9", | |
| "next": ">=15.5.3", | |
| "next-sitemap": "^4.2.3", | |
| "react": "^19.1.0", | |
| "react-dom": "^19.1.0" | |
| "react": "^19.1.13", | |
| "react-dom": "^19.1.13" | |
| }, | |
| "dependencies": { | |
| "@onlook/ui": "workspace:*", | |
| "fumadocs-core": "15.6.9", | |
| "fumadocs-mdx": "11.7.4", | |
| "fumadocs-ui": "15.6.9", | |
| "next": ">=15.5.3", | |
| "next-sitemap": "^4.2.3", | |
| "react": "^19.1.13", | |
| "react-dom": "^19.1.13" | |
| }, |
🤖 Prompt for AI Agents
In docs/package.json around lines 16 to 25, internal packages are pinned to
literal versions which can cause registry drift; replace internal dependency
versions with the workspace protocol (e.g., "workspace:*" or "workspace:./path"
as appropriate) for @onlook/ui, fumadocs-core, fumadocs-mdx, and fumadocs-ui so
the monorepo links local workspaces instead of registry versions; update those
dependency entries to use workspace protocol and keep external deps (next,
react, etc.) unchanged.
| "fumadocs-core": "15.6.9", | ||
| "fumadocs-mdx": "11.7.4", | ||
| "fumadocs-ui": "15.6.9", | ||
| "next": ">=15.5.3", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid open-ended Next range; pin to major with caret
Using ">=15.5.3" can silently jump to Next 16+. Prefer "^15.5.3" for safer upgrades.
Apply:
- "next": ">=15.5.3",
+ "next": "^15.5.3",Run to confirm peer ranges and React 19 compatibility:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Next@15.5.3 peerDependencies:"
curl -s https://registry.npmjs.org/next/15.5.3 | jq -r '.peerDependencies'
echo -e "\nfumadocs-ui@15.6.9 peerDependencies:"
curl -s https://registry.npmjs.org/fumadocs-ui/15.6.9 | jq -r '.peerDependencies'
echo -e "\nfumadocs-core@15.6.9 peerDependencies:"
curl -s https://registry.npmjs.org/fumadocs-core/15.6.9 | jq -r '.peerDependencies'
echo -e "\nfumadocs-mdx@11.7.4 peerDependencies:"
curl -s https://registry.npmjs.org/fumadocs-mdx/11.7.4 | jq -r '.peerDependencies'Length of output: 962
Pin Next dependency to a caret range
Change "next": ">=15.5.3" to avoid unintended major upgrades; verified peerDependencies support Next 15.x.x with React 19.
- "next": ">=15.5.3",
+ "next": "^15.5.3",📝 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.
| "next": ">=15.5.3", | |
| "next": "^15.5.3", |
🤖 Prompt for AI Agents
In docs/package.json around line 21 change the Next dependency from an open
lower-bound (">=15.5.3") to a pinned caret range that prevents accidental major
upgrades; replace the value with a caret range like "^15.5.3" in package.json,
then reinstall/update the lockfile (npm/yarn/pnpm) so the lockfile reflects the
new constraint.
| "@onlook/eslint": "*", | ||
| "@types/node": "22.15.12", | ||
| "@types/react": "^19.1.3", | ||
| "@types/react-dom": "^19.1.3", | ||
| "eslint": "^9.0.0", | ||
| "typescript": "^5.5.4", | ||
| "@types/mdx": "^2.0.13", | ||
| "@tailwindcss/postcss": "^4.1.5", | ||
| "@types/mdx": "^2.0.13", | ||
| "@types/node": "^24.7.2", | ||
| "@types/react": "^19.1.13", | ||
| "@types/react-dom": "^19.1.13", | ||
| "eslint": "^9.0.0", | ||
| "postcss": "^8.5.3", | ||
| "tailwindcss": "^4.1.5", | ||
| "postcss": "^8.5.3" | ||
| "typescript": "^5.8.2" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Also link internal devDeps via workspace protocol
Same rationale for @onlook/eslint.
- "@onlook/eslint": "*",
+ "@onlook/eslint": "workspace:*",📝 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.
| "@onlook/eslint": "*", | |
| "@types/node": "22.15.12", | |
| "@types/react": "^19.1.3", | |
| "@types/react-dom": "^19.1.3", | |
| "eslint": "^9.0.0", | |
| "typescript": "^5.5.4", | |
| "@types/mdx": "^2.0.13", | |
| "@tailwindcss/postcss": "^4.1.5", | |
| "@types/mdx": "^2.0.13", | |
| "@types/node": "^24.7.2", | |
| "@types/react": "^19.1.13", | |
| "@types/react-dom": "^19.1.13", | |
| "eslint": "^9.0.0", | |
| "postcss": "^8.5.3", | |
| "tailwindcss": "^4.1.5", | |
| "postcss": "^8.5.3" | |
| "typescript": "^5.8.2" | |
| "@onlook/eslint": "workspace:*", | |
| "@tailwindcss/postcss": "^4.1.5", | |
| "@types/mdx": "^2.0.13", | |
| "@types/node": "^24.7.2", | |
| "@types/react": "^19.1.13", | |
| "@types/react-dom": "^19.1.13", | |
| "eslint": "^9.0.0", | |
| "postcss": "^8.5.3", | |
| "tailwindcss": "^4.1.5", | |
| "typescript": "^5.8.2" |
🤖 Prompt for AI Agents
In docs/package.json around lines 27 to 36, the internal devDependency
"@onlook/eslint" should be referenced via the workspace protocol like the other
internal packages; change its version spec from "*" to "workspace:*" (or the
appropriate workspace range used in the repo) so package managers link the local
package instead of pulling from registry, then run an install to verify the
workspace link resolves.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:news="http://www.google.com/schemas/sitemap-news/0.9" xmlns:xhtml="http://www.w3.org/1999/xhtml" xmlns:mobile="http://www.google.com/schemas/sitemap-mobile/1.0" xmlns:image="http://www.google.com/schemas/sitemap-image/1.1" xmlns:video="http://www.google.com/schemas/sitemap-video/1.1"> | ||
| <url><loc>https://docs.onlook.dev/robots.txt</loc><lastmod>2025-08-13T18:21:43.820Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/appendix</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/architecture</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/running-locally</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/faq</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/core-features</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/first-project</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/ui-overview</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/migrations/electron-to-web-migration</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/tutorials/figma-to-onlook</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/tutorials/importing-templates</loc><lastmod>2025-08-13T18:21:43.823Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/robots.txt</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/appendix</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/architecture</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/running-locally</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/developers/troubleshooting</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/enterprise</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/faq</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/core-features</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/first-project</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/getting-started/ui-overview</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/migrations/electron-to-web-migration</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/admin-dashboard</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/cloud-deployment</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/docker-compose</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/external-services</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/oauth-setup</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/self-hosting/single-machine</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/tutorials/figma-to-onlook</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| <url><loc>https://docs.onlook.dev/tutorials/importing-templates</loc><lastmod>2025-10-14T05:38:54.762Z</lastmod><changefreq>daily</changefreq><priority>0.7</priority></url> | ||
| </urlset> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify if this generated file should be committed.
Sitemap files are typically auto-generated by tools like next-sitemap during the build process. Committing generated files can lead to:
- Unnecessary merge conflicts when multiple developers regenerate the sitemap
- Repository bloat with frequent timestamp updates
- Potential inconsistencies if the file gets out of sync with the actual site structure
Please verify the project's convention for sitemap files:
🏁 Script executed:
#!/bin/bash
# Check if sitemap files are in .gitignore
echo "=== Checking .gitignore for sitemap patterns ==="
rg -n "sitemap" .gitignore docs/.gitignore 2>/dev/null || echo "No sitemap patterns found in .gitignore"
echo ""
echo "=== Checking next-sitemap config for output location ==="
cat docs/next-sitemap.config.js
echo ""
echo "=== Checking if sitemap generation is part of build process ==="
rg -n "next-sitemap" docs/package.jsonLength of output: 478
Remove generated sitemap files from version control and update .gitignore
- Delete
docs/public/sitemap-*.xmland add/docs/public/sitemap-*.xmlto.gitignore - Rely on the existing
postbuildscript (next-sitemap) to regenerate sitemaps during builds
🤖 Prompt for AI Agents
In docs/public/sitemap-0.xml (lines 1-25) the generated sitemap is checked into
Git; delete docs/public/sitemap-*.xml from the repo and add the pattern
/docs/public/sitemap-*.xml to .gitignore so future builds don’t re-add them;
remove the tracked files (git rm --cached or delete and commit) and commit the
.gitignore change, and ensure the existing postbuild next-sitemap step remains
in package.json so sitemaps are regenerated during build.
| }, | ||
| "dependencies": { | ||
| "@eslint/compat": "^1.2.9", | ||
| "@eslint/js": "^9.37.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Align ESLint core with @eslint/js to avoid mismatch.
@eslint/js is ^9.37.0 while eslint is ^9.0.0; ensure the resolved eslint version matches @eslint/js to prevent config skew. Consider bumping eslint to ^9.37.0 here for consistency.
Run to inspect versions across the repo:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f package.json | while read -r f; do
jq -r --arg f "$f" '
{
eslint: (.dependencies.eslint // .devDependencies.eslint // .peerDependencies.eslint),
eslint_js: (.dependencies["\u0040eslint/js"] // .devDependencies["\u0040eslint/js"] // .peerDependencies["\u0040eslint/js"])
} | select(.eslint or .eslint_js)
| "\($f): eslint=\(.eslint // "N/A"), @eslint/js=\(.eslint_js // "N/A")"
' "$f"
done | sortLength of output: 1382
Align ESLint core version with @eslint/js
In tooling/eslint/package.json eslint is ^9.0.0 while @eslint/js is ^9.37.0; bump eslint to ^9.37.0 to keep versions in sync.
🤖 Prompt for AI Agents
In tooling/eslint/package.json around line 17, the root "eslint" dependency is
at ^9.0.0 while @eslint/js is ^9.37.0; update the eslint version to ^9.37.0 (or
match the @eslint/js semver) so both core eslint and @eslint/js are aligned,
then run npm/yarn install and update lockfile.
| "eslint": "^9.0.0", | ||
| "prettier": "^3.3.3", | ||
| "typescript": "^5.5.4" | ||
| "typescript": "^5.8.2" |
There was a problem hiding this comment.
🧩 Analysis chain
TS 5.8 looks good; ensure monorepo alignment to avoid duplicate TS.
Verify all packages converge on ^5.8.x or the intended single major.
List TypeScript versions across packages:
🏁 Script executed:
#!/bin/bash
fd -HI -t f package.json | while read -r f; do
jq -r --arg f "$f" '
. as $r
| {ts: (.dependencies.typescript // .devDependencies.typescript // .peerDependencies.typescript)}
| select(.ts) | "\($f): typescript=\(.ts)"
' "$f"
done | sortLength of output: 1277
Align TypeScript versions across monorepo
Update the following to "^5.8.2" to match all other packages:
- apps/backend/package.json (typescript "^5.0.0")
- apps/web/package.json (typescript "^5")
🤖 Prompt for AI Agents
In tooling/eslint/package.json around line 35, update TypeScript version
alignment across the monorepo: change apps/backend/package.json (currently
"typescript": "^5.0.0") and apps/web/package.json (currently "typescript": "^5")
to use "typescript": "^5.8.2" so all packages match; update the version strings
in those two package.json files and run the repo's install command (yarn or npm
install) to ensure the lockfile and node_modules are consistent.
saddlepaddle
force-pushed
the
run-lint-on-repo
branch
from
e2463cc to
fb14321
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/client/package.json (1)
107-107: Pin Next to a stable major (avoid '>=').
'>=' can auto-upgrade to breaking majors. Use a caret within the supported major.- "next": ">=15.5.3", + "next": "^15.5.3",
🧹 Nitpick comments (2)
apps/web/client/package.json (2)
47-65: Use workspace protocol for internal packages instead of "*"Avoids pulling from registry and enforces local workspace linkage/version discipline (pairs well with sherif).
Apply:
- "@onlook/code-provider": "*", - "@onlook/constants": "*", - "@onlook/db": "*", - "@onlook/email": "*", - "@onlook/file-system": "*", - "@onlook/fonts": "*", - "@onlook/git": "*", - "@onlook/github": "*", - "@onlook/growth": "*", - "@onlook/image-server": "*", - "@onlook/models": "*", - "@onlook/parser": "*", - "@onlook/penpal": "*", - "@onlook/rpc": "*", - "@onlook/stripe": "*", - "@onlook/ui": "*", - "@onlook/utility": "*", - "@onlook/web-preload": "*", + "@onlook/code-provider": "workspace:*", + "@onlook/constants": "workspace:*", + "@onlook/db": "workspace:*", + "@onlook/email": "workspace:*", + "@onlook/file-system": "workspace:*", + "@onlook/fonts": "workspace:*", + "@onlook/git": "workspace:*", + "@onlook/github": "workspace:*", + "@onlook/growth": "workspace:*", + "@onlook/image-server": "workspace:*", + "@onlook/models": "workspace:*", + "@onlook/parser": "workspace:*", + "@onlook/penpal": "workspace:*", + "@onlook/rpc": "workspace:*", + "@onlook/stripe": "workspace:*", + "@onlook/ui": "workspace:*", + "@onlook/utility": "workspace:*", + "@onlook/web-preload": "workspace:*",
117-117: Move prettier to devDependenciesPrettier is a dev tool; keeping it in dependencies increases prod install surface.
"posthog-node": "^4.17.2", - "prettier": "^3.3.3", "prosemirror-commands": "^1.7.1","devDependencies": { @@ "eslint": "^9.0.0", "postcss": "^8.5.3", "tailwindcss": "^4.1.5", "type-fest": "^4.41.0", "typescript": "^5.8.2" + ,"prettier": "^3.3.3" },Also applies to: 149-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
apps/web/client/package.json(3 hunks)packages/stripe/src/client.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/stripe/src/client.ts
🔇 Additional comments (3)
apps/web/client/package.json (3)
113-114: Verify need for both 'pg' and 'postgres'Both drivers are present; most projects use one. Keeping both increases bundle size and risk of split usage.
Would you confirm both are required? If not, remove the unused one and update imports accordingly.
124-130: Confirm React 19 compatibility with your Next versionRuntime is React 19 and types are 19.x. Ensure your Next version and ESLint config fully support React 19 to avoid subtle type/runtime issues.
Please verify build/test against Next 15.5.x release notes and ensure no peer-dep warnings at install time.
Also applies to: 157-158
156-156: Align Node types with your deployment runtime
apps/web/client/package.json declares @types/node ^24.7.2 but no engines.node; if your client runs on Node 20/22, add an engines.node field and downgrade @types/node (or constrain TS libs) to avoid using unavailable APIs.
Description
Moves some code around to avoid these cyclical dependencies:
Cycle 1:
@onlook/web-client@onlook/db@onlook/ai@onlook/modelsThis one was kinda gnarly, since the execute blocks in the tool definitions depend on the actual business logic code in web-client, the ai package needs the db types and the db needs the ai types, etc.
There's probably three ways to solve it, I opted for the least clunky one in my opinion:
Cycle 2:
@onlook/web-preload@onlook/penpalCycle 3
@onlook/models@onlook/constantsIMPORTANT: To avoid future dependency mismatches, upgraded to bun@1.3.0 (context here: https://bun.com/blog/bun-v1.3#isolated-installs-are-now-the-default-for-workspaces).
Fixed typecheck in every package. Some of the issues were pretty rough due to package mismatches, to avoid this in the future I installed sherif, which'll make bun fail if someone attempts to install invalid package versions.
Related Issues
Type of Change
Testing
Used the product thoroughly to ensure no regressions occurred as a result of this pr.
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores
Important
Resolved cyclical dependencies and typecheck issues across packages, introduced
sheriffor dependency consistency, and updated tooling configurations.@onlook/web-client,@onlook/db,@onlook/ai, and@onlook/modelsby reorganizing type definitions and dependencies.@onlook/web-clientand adjusted dependencies inpackage.jsonfiles across multiple packages.@onlook/aipackage and integrated its functionality into other packages.sherifto enforce consistent package versions and prevent dependency mismatches.bun@1.3.0for isolated installs in workspaces.tooling/eslint/base.jsto improve code quality and consistency.sherifas a postinstall script inpackage.jsonto ensure dependency consistency.This description was created by
for e2463cc. You can customize this summary. It will automatically update as commits are pushed.