Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
34bfd35
feat: add restart sandbox button to toolbar
drfarrell Sep 15, 2025
804a1ea
fix: use reload icon and add toast notification for sandbox restart
drfarrell Sep 15, 2025
a3f45d2
refactor: address CodeRabbit review comments for restart sandbox feature
drfarrell Sep 15, 2025
fa834eb
feat: add loading state and delay to sandbox restart
drfarrell Sep 15, 2025
0025f42
feat: add error state indication for restart sandbox button
drfarrell Sep 15, 2025
da4ab4d
refactor: optimize error detection for massive performance improvement
drfarrell Sep 15, 2025
f897610
fix: remove non-existent hasTimedOut property to fix build error
drfarrell Sep 15, 2025
4cb140a
feat: improve spinner visibility and add startup grace period
drfarrell Sep 15, 2025
ab76b25
fix: use useMemo for proper memoization of hasSandboxError
drfarrell Sep 15, 2025
45456c8
fix: clear amber error state when sandbox successfully connects
drfarrell Sep 15, 2025
5d7cc0e
feat: detect actual 502 errors using 30-second timeout
drfarrell Sep 15, 2025
ccf112f
fix: reduce timeout detection to 5 seconds for faster user feedback
drfarrell Sep 15, 2025
cb0293e
perf: ultra-lightweight error detection with minimal resource usage
drfarrell Sep 15, 2025
eaaa718
fix: detect 502 errors on initial project load
drfarrell Sep 15, 2025
ebc7297
Merge branch 'main' into restart-sandbox-in-toolbar
Kitenite Sep 16, 2025
dcfada1
refactor and clean up
Kitenite Sep 16, 2025
367678f
finish loop
Kitenite Sep 16, 2025
1638260
clean up
Kitenite Sep 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 137 additions & 12 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,146 @@
# Claude Agent Instructions
## Onlook Agents Guide

Anthropic models must read and follow `onlook/AGENTS.md` before making any change.
Actionable rules for repo agents—keep diffs minimal, safe, token‑efficient.

This is a mandatory precondition: do not proceed until you have read and internalized the rules in `onlook/AGENTS.md`.
### Purpose & Scope

Key enforcement (see AGENTS.md for full details):
- Audience: automated coding agents working within this repository.
- Goal: small, correct diffs aligned with the project’s architecture.
- Non-goals: editing generated artifacts, lockfiles, or `node_modules`.

- Package manager: Bun only. Use Bun for all installs and scripts.
- `bun install` (not `npm install`)
- `bun add <pkg>` (not `npm install <pkg>`)
- `bun run <script>` (not `npm run <script>`)
- `bunx <cmd>` (not `npx <cmd>`)
### Repo Map

- Monorepo managed by Bun workspaces (see root `package.json`).
- App: `apps/web/client` (Next.js App Router + TailwindCSS).
- API routes: `apps/web/client/src/server/api/routers/*`, aggregated in
`apps/web/client/src/server/api/root.ts`.
- Shared utilities: `packages/*` (e.g., `packages/utility`).

### Stack & Runtimes

- UI: Next.js App Router, TailwindCSS.
- API: tRPC + Zod (`apps/web/client/src/server/api/*`).
- Package manager: Bun only — use Bun for all installs and scripts; do not use
npm, yarn, or pnpm.

### Agent Priorities

- Correctness first: minimal scope and targeted edits.
- Respect client/server boundaries in App Router.
- Prefer local patterns and existing abstractions; avoid one-off frameworks.
- Do not modify build outputs, generated files, or lockfiles.
- Use Bun for all scripts; do not introduce npm/yarn.
- Avoid running the local dev server in automation contexts.
- Follow the project’s structure and client/server boundaries.
- Respect type safety and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line "- Respect type safety and" appears to be incomplete. Please complete the thought or remove this line if it's not needed.

Suggested change
- Respect type safety and


### Next.js App Router

- Default to Server Components. Add `use client` when using events,
state/effects, browser APIs, or client-only libs.
- App structure: `apps/web/client/src/app/**` (`page.tsx`, `layout.tsx`,
`route.ts`).
- Client providers live behind a client boundary (e.g.,
`apps/web/client/src/trpc/react.tsx`).
- Example roots: `apps/web/client/src/app/layout.tsx` (RSC shell, providers
wired, scripts gated by env).
- Components using `mobx-react-lite`'s `observer` must be client components
(include `use client`).

### tRPC API

- Routers live in `apps/web/client/src/server/api/routers/**` and must be
exported from `apps/web/client/src/server/api/root.ts`.
- Use `publicProcedure`/`protectedProcedure` from
`apps/web/client/src/server/api/trpc.ts`; validate inputs with Zod.
- Serialization handled by SuperJSON; return plain objects/arrays.
- Client usage via `apps/web/client/src/trpc/react.tsx` (React Query + tRPC
links).

### Auth & Supabase

- Server-side client: `apps/web/client/src/utils/supabase/server.ts` (uses Next
headers/cookies). Use in server components, actions, and routes.
- Browser client: `apps/web/client/src/utils/supabase/client/index.ts` for
client components.
- Never pass server-only clients into client code.

### Env & Config

- Define/validate env vars in `apps/web/client/src/env.ts` via
`@t3-oss/env-nextjs`.
- Expose browser vars with `NEXT_PUBLIC_*` and declare in the `client` schema.
- Prefer `env` from `@/env`. In server-only helpers (e.g., base URL in
`src/trpc/helpers.ts`), read `process.env` only for deployment vars like
`VERCEL_URL`/`PORT`. Never use `process.env` in client code; in shared
modules, guard with `typeof window === 'undefined'`.
- Import `./src/env` in `apps/web/client/next.config.ts` to enforce validation.

### Imports & Paths

- Use path aliases: `@/*` and `~/*` map to `apps/web/client/src/*` (see
`apps/web/client/tsconfig.json`).
- Do not import server-only modules into client components. Limited exception:
editor modules that already use `path`; reuse only there. Never import
`process` in client code.
- Split code by environment if needed (server file vs client file).

### MobX + React Stores

- Create store instances with `useState(() => new Store())` for stability across
renders.
- Keep active store in `useRef`; clean up async with
`setTimeout(() => storeRef.current?.clear(), 0)` to avoid route-change races.
- Avoid `useMemo` for store instances; React may drop memoized values leading to
data loss.
- Avoid putting the store instance in effect deps if it loops; split concerns
(e.g., project vs branch).
- `observer` components are client-only. Place one client boundary at the
feature entry; child observers need not include `use client` (e.g.,
`apps/web/client/src/app/project/[id]/_components/main.tsx`).
- Example store: `apps/web/client/src/components/store/editor/engine.ts:1` (uses
`makeAutoObservable`).

### Styling & UI

- TailwindCSS-first styling; global styles are already imported in
`apps/web/client/src/app/layout.tsx`.
- Prefer existing UI components from `@onlook/ui` and local patterns.
- Preserve dark theme defaults via `ThemeProvider` usage in layout.

### Internationalization

- `next-intl` is configured; provider lives in
`apps/web/client/src/app/layout.tsx`.
- Strings live in `apps/web/client/messages/*`. Add/modify keys there; avoid
hardcoded user-facing text.
- Keep keys stable; prefer additions over breaking renames.

### Common Pitfalls

- Missing `use client` where needed (events/browser APIs) causes unbound events;
a single boundary at the feature root is sufficient.
- New tRPC routers not exported in `src/server/api/root.ts` (endpoints
unreachable).
- Env vars not typed/exposed in `src/env.ts` cause runtime/edge failures. Prefer
`env`; avoid new `process.env` reads in client code.
- Importing server-only code into client components (bundling/runtime errors).
Note: `path` is already used in specific client code-editor modules; avoid
expanding Node API usage beyond those areas.
- Bypassing i18n by hardcoding strings instead of using message files/hooks.
- Avoid `useMemo` to create MobX stores (risk of lost references); avoid
synchronous cleanup on route change (race conditions).

### Context Discipline (for Agents)

If any instruction here or in `onlook/AGENTS.md` conflicts with your defaults, prefer `onlook/AGENTS.md`.
- Search narrowly with ripgrep; open only files you need.
- Read small sections; avoid `node_modules`, `.next`, large assets.
- Propose minimal diffs aligned with existing conventions; avoid wide refactors.

When in doubt, stop and re‑read `onlook/AGENTS.md` before acting.
### Notes

- Unit tests can be run with `bun test`
- Run type checking with `bun run typecheck`
- Apply database updates to local dev with `bun run db:push`
- Refrain from running the dev server
- DO NOT run `db:gen`. This is reserved for the maintainer.
- DO NOT use any type unless necessary
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
'use client';

import { useEditorEngine } from '@/components/store/editor';
import { Icons } from '@onlook/ui/icons';
import { toast } from '@onlook/ui/sonner';
import { Tooltip, TooltipContent, TooltipTrigger } from '@onlook/ui/tooltip';
import { cn } from '@onlook/ui/utils';
import { observer } from 'mobx-react-lite';
import { useCallback, useEffect, useRef, useState } from 'react';

export const RestartSandboxButton = observer(({
className,
}: {
className?: string;
}) => {
const editorEngine = useEditorEngine();
const branches = editorEngine.branches;
const [restarting, setRestarting] = useState(false);
const timeoutIdRef = useRef<NodeJS.Timeout | null>(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using 'number' instead of 'NodeJS.Timeout' for the timeout ref in client-side code.

Suggested change
const timeoutIdRef = useRef<NodeJS.Timeout | null>(null);
const timeoutIdRef = useRef<number | null>(null);

const [hasSandboxError, setHasSandboxError] = useState(false);
const mountTimeRef = useRef<number>(Date.now());
const checkInterval = 10000;

// Extract error checking logic with proper dependencies
const checkForError = useCallback(() => {
const activeBranch = branches.activeBranch;
if (!activeBranch) {
setHasSandboxError(false);
return false; // Stop checking
}

const branchData = branches.getBranchDataById(activeBranch.id);
const sandbox = branchData?.sandbox;

if (!sandbox?.session) {
setHasSandboxError(false);
return false; // Stop checking if no session
}

if (sandbox.session.provider) {
setHasSandboxError(false);
} else {
// Only show error after initial grace period
const timeSinceMount = Date.now() - mountTimeRef.current;
if (timeSinceMount >= 5000) {
setHasSandboxError(true);
}
}

return true; // Continue checking
}, [branches]);

// TODO: iFrame should also detect 502 errors and set hasSandboxError to true
useEffect(() => {
// Clear any existing timer first
if (timeoutIdRef.current) {
clearTimeout(timeoutIdRef.current);
timeoutIdRef.current = null;
}

const scheduleNextCheck = () => {
const shouldContinue = checkForError();
if (shouldContinue) {
timeoutIdRef.current = setTimeout(scheduleNextCheck, checkInterval);
}
};

// Initial delay for grace period if needed
const timeSinceMount = Date.now() - mountTimeRef.current;
const initialDelay = timeSinceMount < 5000 ? 5000 - timeSinceMount : 0;

timeoutIdRef.current = setTimeout(scheduleNextCheck, initialDelay);

return () => {
if (timeoutIdRef.current) {
clearTimeout(timeoutIdRef.current);
timeoutIdRef.current = null;
}
};
}, [checkForError, checkInterval]); // Re-run when checker or interval changes

const handleRestartSandbox = async () => {
try {
if (restarting) {
return;
}
const activeBranch = branches.activeBranch;
if (!activeBranch) return;
if (restarting) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant check for 'restarting' found. The flag is checked before and after fetching activeBranch, making the second check unnecessary.

return;
}
Comment on lines +84 to +91
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a redundant check for the restarting state at lines 89-91 that duplicates the earlier check at lines 84-86. The second check can be safely removed to improve code clarity and avoid unnecessary conditional evaluation.

Suggested change
if (restarting) {
return;
}
const activeBranch = branches.activeBranch;
if (!activeBranch) return;
if (restarting) {
return;
}
if (restarting) {
return;
}
const activeBranch = branches.activeBranch;
if (!activeBranch) return;

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


setRestarting(true);
setHasSandboxError(false);
// Reset mount time for grace period after restart
mountTimeRef.current = Date.now();
const sandbox = branches.getSandboxById(activeBranch.id);
if (!sandbox?.session) {
toast.error('Sandbox session not available');
setRestarting(false);
Comment on lines +99 to +100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Externalize user‑facing strings via next‑intl.

Avoid hardcoded UI text per repo guidelines. Use next-intl and add keys under apps/web/client/messages/*.

Apply this patch and I can follow up with message keys:

@@
-import { Icons } from '@onlook/ui/icons';
+import { Icons } from '@onlook/ui/icons';
+import { useTranslations } from 'next-intl';
@@
 export const RestartSandboxButton = observer(({ className }: { className?: string }) => {
     const editorEngine = useEditorEngine();
+    const t = useTranslations('toolbar'); // namespace suggestion
@@
-                toast.error('Sandbox session not available');
+                toast.error(t('sandbox.sessionUnavailable'));
@@
-                toast.loading('Restarting sandbox...');
+                const toastId = toast.loading(t('sandbox.restarting'));
@@
-                    toast.success('Sandbox restarted successfully', {
+                    toast.success(t('sandbox.restarted'), {
                         icon: <Icons.Cube className="h-4 w-4" />,
+                        id: toastId,
                     });
@@
-                toast.error('Failed to restart sandbox');
+                toast.error(t('sandbox.restartFailed'));
@@
-            toast.error('An error occurred while restarting the sandbox');
+            toast.error(t('sandbox.restartError'));
@@
-            <TooltipContent sideOffset={5} hideArrow>Restart Sandbox</TooltipContent>
+            <TooltipContent sideOffset={5} hideArrow>{t('sandbox.restartTooltip')}</TooltipContent>

Proposed keys:

  • toolbar.sandbox.sessionUnavailable
  • toolbar.sandbox.restarting
  • toolbar.sandbox.restarted
  • toolbar.sandbox.restartFailed
  • toolbar.sandbox.restartError
  • toolbar.sandbox.restartTooltip

Also applies to: 82-83, 93-95, 99-104, 140-141

return;
}

const success = await sandbox.session.restartDevServer();
if (success) {
// Wait 5 seconds before refreshing webviews to avoid 502 errors
setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setTimeout inside handleRestartSandbox isn’t cleared on unmount. Consider storing its ID in a ref and clearing it in a cleanup function to avoid potential memory leaks.

const frames = editorEngine.frames.getByBranchId(activeBranch.id);
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');
}
} catch (error) {
console.error('Error restarting sandbox:', error);
toast.error('An error occurred while restarting the sandbox');
setRestarting(false);
}
};

const disabled = !branches.activeBranch || restarting;

return (
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={handleRestartSandbox}
disabled={disabled}
className={cn(
"h-9 w-9 flex items-center justify-center rounded-md border border-transparent transition-colors",
hasSandboxError
? "bg-amber-900 text-amber-200 hover:bg-amber-800 hover:text-amber-100"
: restarting
? "text-foreground-tertiary bg-accent/30"
: !disabled
? "hover:text-foreground-hover text-foreground-tertiary hover:bg-accent/50"
: "text-foreground-disabled cursor-not-allowed opacity-50",
className
)}
>
{restarting ? (
<Icons.LoadingSpinner className="h-4 w-4 animate-spin" />
) : (
<Icons.RestartSandbox className={cn(
"h-4 w-4",
hasSandboxError && "text-amber-200"
)} />
)}
</button>
</TooltipTrigger>
<TooltipContent sideOffset={5} hideArrow>Restart Sandbox</TooltipContent>
</Tooltip>
);
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

import { useEditorEngine } from '@/components/store/editor';
import { Icons } from '@onlook/ui/icons';
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@onlook/ui/tabs';
Expand All @@ -6,6 +8,7 @@ import { cn } from '@onlook/ui/utils';
import { observer } from 'mobx-react-lite';
import { motion } from 'motion/react';
import { useState } from 'react';
import { RestartSandboxButton } from './restart-sandbox-button';
import { Terminal } from './terminal';

export const TerminalArea = observer(({ children }: { children: React.ReactNode }) => {
Expand Down Expand Up @@ -53,6 +56,7 @@ export const TerminalArea = observer(({ children }: { children: React.ReactNode
{terminalHidden ? (
<motion.div layout className="flex items-center gap-1">
{children}
<RestartSandboxButton />
<Tooltip>
<TooltipTrigger asChild>
<button
Expand Down Expand Up @@ -81,6 +85,7 @@ export const TerminalArea = observer(({ children }: { children: React.ReactNode
</motion.span>
<div className="flex items-center gap-1">
<motion.div layout>{/* <RunButton /> */}</motion.div>
<RestartSandboxButton />
<Tooltip>
<TooltipTrigger asChild>
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import type { IFrameView } from '@/app/project/[id]/_components/canvas/frame/vie
import { api } from '@/trpc/client';
import { toDbFrame, toDbPartialFrame } from '@onlook/db';
import { type Frame } from '@onlook/models';
import { calculateNonOverlappingPosition } from '@onlook/utility';
import { debounce } from 'lodash';
import { makeAutoObservable } from 'mobx';
import { v4 as uuid } from 'uuid';
import type { EditorEngine } from '../engine';
import { roundDimensions } from './dimension';
import { FrameNavigationManager } from './navigation';
import { calculateNonOverlappingPosition } from '@onlook/utility';

export interface FrameData {
frame: Frame;
Expand Down Expand Up @@ -51,6 +51,10 @@ export class FramesManager {
return Array.from(this._frameIdToData.values());
}

getByBranchId(branchId: string): FrameData[] {
return Array.from(this._frameIdToData.values()).filter((w) => w.frame.branchId === branchId);
}

get(id: string): FrameData | null {
return this._frameIdToData.get(id) ?? null;
}
Expand Down
Loading