Skip to content

Conversation

http-teapot
Copy link
Contributor

@http-teapot http-teapot commented Aug 20, 2025

Description

Adding an API abstraction layer for code sandboxes.

TODO

  1. Implement filewatcher
  2. Implement terminal
  3. Implement codesandbox in router (codesandbox does not work in this branch atm)
  4. Implement parameter to initialize project on a specific code provider
  5. Finish writing tests
  6. Update documentation

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

  1. cp apps/coderouter/.env.example apps/coderouter/.env
  2. Edit .env and specify CODEROUTER_API_KEY (make something up) and E2B_API_KEY (sign up on e2b.dev)
  3. Edit apps/web/client/.env and add
    E2B_DEFAULT_TEMPLATE_ID=8x0xlv0pmo0qtn5a469w # this template is on my account for now
    CODEROUTER_HOST_URL=http://localhost:4444
    CODEROUTER_API_KEY="{REPLACE WITH MADE-UP KEY ON STEP ABOVE}"
    
  4. Run bun run backend:start
  5. Add 127.0.0.1 onlook.internal in your /etc/hosts (sudo vim /etc/hosts)
  6. Go to https://onlook.internal

Screenshots (if applicable)

Additional Notes


Important

Introduces a new API abstraction layer for code sandboxes using E2B, replacing CodeSandbox, with updated routes, middleware, and environment configurations.

  • Behavior:
    • Introduces CoderouterProvider in packages/code-provider/src/providers/coderouter for E2B integration.
    • Adds new API routes in apps/coderouter/src/api for sandbox operations like create, pause, resume, stop, and file operations.
    • Updates apps/coderouter/src/server.ts to register new routes and middleware.
    • Implements JWT authentication in apps/coderouter/src/util/auth.ts.
  • Environment & Config:
    • Adds CODEROUTER_API_KEY and CODEROUTER_HOST_URL to environment variables in apps/web/client/src/env.ts.
    • Updates docker-compose.yaml and nginx config for local development.
  • Code Generation:
    • Uses OpenAPI generator to create TypeScript client in packages/code-provider/src/providers/coderouter/codegen.
  • Misc:
    • Updates package.json scripts to include Docker setup for backend.

This description was created by Ellipsis for e016242. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • New Features

    • Introduced “Coderouter” service with JWT auth, sandbox lifecycle (create/pause/resume/stop/url) and file operations (list/read/write/rename/copy/delete/download) APIs.
    • Web app now uses Coderouter as the code provider; supports environment-driven host URL, API key, and default E2B template.
    • OpenAPI client generation added for Coderouter.
  • Documentation

    • Updated README and docs to reflect feature hierarchy and E2B alternative.
    • Added setup guides for E2B templates.
  • Chores

    • Added local docker-compose with NGINX TLS reverse proxy and example env files.
    • Expanded env variables for n8n, E2B, and Coderouter.

Copy link

vercel bot commented Aug 20, 2025

Someone is attempting to deploy a commit to the Onlook Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

coderabbitai bot commented Aug 20, 2025

Walkthrough

Introduces a new “coderouter” service: a Bun/Hono OpenAPI server with JWT- and API key–based auth, E2B-backed sandbox client, middleware stack, and sandbox/file endpoints. Integrates nginx TLS reverse proxy and docker-compose for local dev. Updates web client to use the new coderouter provider. Adds OpenAPI codegen client and provider wiring. Minor docs/config tweaks.

Changes

Cohort / File(s) Summary
Docs updates
README.md, docs/content/docs/developers/appendix.mdx, apps/nginx/ssl/README.md, apps/web/template/README.md
README list restructured; docs mention E2B alternative; warnings for local TLS; add E2B run instructions.
Supabase auth redirects
apps/backend/supabase/config.toml
Append onlook.internal URLs to additional_redirect_urls.
Coderouter app scaffolding/config
apps/coderouter/.env.example, .gitignore, Dockerfile.dev, README.md, biome.json, bunfig.toml, codecov.yml, drizzle.config.ts, package.json, renovate.json, scripts/migrate.ts, tsconfig.json, typedoc.json, site/index.html
Add Bun/TypeScript app template, tooling configs, Drizzle setup, scripts, and basic site/docs.
Coderouter server and auth/middleware
apps/coderouter/src/server.ts, src/util/auth.ts, src/middleware/* and *.test.ts
Implement OpenAPI Hono server, JWT encode/decode, API key verify; add error/client/auth/sandbox-id/before-call middlewares with tests.
Coderouter provider definitions
apps/coderouter/src/provider/definition/...
Define abstract Client, Sandbox, and SandboxFile APIs, error codes/mapping.
E2B provider implementation
apps/coderouter/src/provider/e2b/... and .../file/*.test.ts
Implement E2BClient and E2BSandbox(+file ops), with lifecycle and file methods; add tests.
Coderouter API endpoints
apps/coderouter/src/api/** (auth/sign, sandbox/{create,pause,resume,stop,url}, sandbox/file/{copy,delete,download,list,read,rename,stat,write}+*.test.ts`)
Add OpenAPI-documented routes secured by JWT/API key; implement handlers delegating to client; add unit tests.
Nginx reverse proxy & TLS assets
apps/nginx/conf.d/server.conf, apps/nginx/bin/genkeys.sh, apps/nginx/ssl/*
Add HTTPS reverse proxy to coderouter and web, self-signed cert/key, key generation script, OpenSSL config.
Local orchestration
apps/docker-compose.yaml, package.json
Compose coderouter + nginx for local dev; backend:start chains docker-compose up in apps.
Web client: env/config
apps/web/client/.env.example, apps/web/client/src/env.ts
Add E2B/coderouter-related env vars and server schema mappings.
Web client: switch to coderouter
apps/web/client/src/app/projects/import/local/_context/index.tsx, src/components/store/editor/sandbox/session.ts, src/server/api/routers/project/sandbox.ts, src/server/api/routers/publish/helpers/fork.ts, src/components/store/editor/code/index.ts, src/components/store/editor/sandbox/terminal.ts
Replace CodeSandbox provider with coderouter; add session/JWT flow, template/user handling, URL usage; terminal write override for redraws; minor formatting.
Web template (E2B)
apps/web/template/e2b.Dockerfile, apps/web/template/e2b.toml.example, apps/web/template/.gitignore
Add E2B build files and ignore e2b.toml.
Code provider: add coderouter
packages/code-provider/src/index.ts, src/providers.ts
Add CodeProvider.Coderouter and factory wiring with options.
OpenAPI codegen for coderouter
packages/code-provider/openapitools.json, package.json, src/providers/coderouter/codegen/**
Add generator config, script, and generated TS Fetch client (apis, models, runtime).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant WebClient as Web Client (App)
  participant Coderouter as Coderouter API (Hono)
  participant Middleware as Auth/Client Middlewares
  participant E2B as E2B Sandbox

  User->>WebClient: Start sandbox workflow
  WebClient->>Coderouter: POST /coderouter/api/auth/sign (Authorization: Bearer <API_KEY>, {sandboxId?, userId?})
  Coderouter-->>WebClient: 200 { jwt }

  User->>WebClient: Perform file write
  WebClient->>Coderouter: POST /coderouter/api/sandbox/file/write (Authorization: Bearer <jwt>, body)
  activate Coderouter
  Coderouter->>Middleware: onRequest (error -> auth -> requireSandboxId -> client -> beforeSandboxCall)
  Middleware->>Middleware: decode JWT, attach auth, make E2BClient
  Middleware->>E2B: beforeSandboxCall/connect
  Coderouter->>E2B: sandbox.file.write(...)
  E2B-->>Coderouter: {}
  Coderouter-->>WebClient: 200 {}
  deactivate Coderouter
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Suggested reviewers

  • Kitenite

Poem

In cables and carrots I deftly burrow,
New routes hop in through Coderouter’s furrow.
JWT in paw, I squeak “All clear!”
E2B burrows spin up near.
Nginx moonlight, docks afloat—
I ship my bits with a twitching note. 🥕✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

api_sandbox_url(app);

const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/url', {
method: 'GET',
Copy link
Contributor

Choose a reason for hiding this comment

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

The test is using GET while the corresponding route is defined with a POST method. Please update the test to use POST (or change the route) for consistency.

Suggested change
method: 'GET',
method: 'POST',

}>();

// Set up auth middleware first (required for client middleware)
setupAuthJwtMiddleware(app);
Copy link
Contributor

Choose a reason for hiding this comment

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

setupAuthJwtMiddleware() is called without the required path argument. The function signature expects (app, path).

Suggested change
setupAuthJwtMiddleware(app);
setupAuthJwtMiddleware(app, env.URL_PATH_PREFIX + '/api');

setupAuthJwtMiddleware(app);

// Set up client middleware
setupClientMiddleware(app);
Copy link
Contributor

Choose a reason for hiding this comment

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

setupClientMiddleware() is called without providing the required path argument. Please add the appropriate path.

Suggested change
setupClientMiddleware(app);
setupClientMiddleware(app, env.URL_PATH_PREFIX);


expect(mockClient._sandbox.files.remove).toHaveBeenCalledTimes(1);
expect(mockClient._sandbox.files.remove).toHaveBeenCalledWith(
"/test/file.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

Test assertions for file operations do not account for the '/code' folder prefix added by fullpath(). For example, delete() should be called with '/code/test/file.txt' when given '/test/file.txt'. Update expected paths accordingly.

Suggested change
"/test/file.txt"
"/code/test/file.txt"


export const dbKind: DBKind = (process.env.DRIZZLE_DB as DBKind) || "sqlite";

export async function getDb() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The getDb() function is entirely commented out and does not return a database instance. Either implement the function or throw a clear error if it's not supported.

@@ -5,4 +5,5 @@ export enum CodeProvider {
VercelSandbox = 'vercel_sandbox',
Modal = 'modal',
NodeFs = 'node_fs',
Coderouter = 'coderouter',
Copy link
Contributor

Choose a reason for hiding this comment

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

Typographical note: The enum member 'Coderouter' might be intended to follow the camel case naming convention used by other members (e.g., 'VercelSandbox' and 'NodeFs'). Consider renaming it to 'CodeRouter' (and adjusting the string value to 'code_router' if applicable) for consistency.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 85

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
apps/nginx/ssl/onlook-internal.key (1)

1-29: Private key committed to the repo — remove immediately and regenerate.

This is sensitive material. Even for local dev, committed keys are often copied elsewhere. Rotate any deployments that used this key, purge from history, and ignore the path going forward. Generate on the fly instead.

Suggested actions:

  • git rm this file; add apps/nginx/ssl/*.key to .gitignore
  • Provide a generation script (OpenSSL/mkcert) and mount the result in docker-compose
  • Document “dev-only, do not reuse” in README and ensure server.conf points to the mounted path
apps/web/client/src/components/store/editor/sandbox/session.ts (1)

75-80: Guard against missing task to prevent runtime throw.

getTask might return an object without task; add a nullish guard.

-        if (result) {
-            return await result.task.open();
-        }
+        if (result?.task) {
+            return await result.task.open();
+        }
         return 'Dev server not found';
apps/web/client/src/server/api/routers/project/sandbox.ts (1)

108-114: Pass userId to getProvider to keep metadata consistent.

You’re passing userId to createProject, but E2B metadata construction in the provider likely reads userId from client options (set when constructing the provider). Not passing it here means the metadata may omit userId, causing later lookups to mismatch filtering by userId.

Apply this diff:

-                    const provider = await getProvider(input.sandbox.id);
+                    const provider = await getProvider(input.sandbox.id, input.userId);

Comment on lines +1 to +15
import "dotenv/config";

export default {
out: "./drizzle",
schema: "./src/db/schema.ts",
dialect:
process.env.DRIZZLE_DB === "postgres"
? "postgresql"
: process.env.DRIZZLE_DB === "mysql"
? "mysql"
: "sqlite",
dbCredentials: {
connectionString: process.env.DATABASE_URL,
},
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

drizzle-kit config uses the wrong credential key; use url (not connectionString) and add typing

drizzle-kit expects dbCredentials.url. Using connectionString will break migrations. Adding typing guards against config drift.

 import "dotenv/config";
+import type { Config } from "drizzle-kit";

-export default {
+export default {
 	out: "./drizzle",
 	schema: "./src/db/schema.ts",
 	dialect:
 		process.env.DRIZZLE_DB === "postgres"
 			? "postgresql"
 			: process.env.DRIZZLE_DB === "mysql"
 				? "mysql"
 				: "sqlite",
 	dbCredentials: {
-		connectionString: process.env.DATABASE_URL,
+		url: process.env.DATABASE_URL!,
 	},
-};
+} satisfies Config;
📝 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.

Suggested change
import "dotenv/config";
export default {
out: "./drizzle",
schema: "./src/db/schema.ts",
dialect:
process.env.DRIZZLE_DB === "postgres"
? "postgresql"
: process.env.DRIZZLE_DB === "mysql"
? "mysql"
: "sqlite",
dbCredentials: {
connectionString: process.env.DATABASE_URL,
},
};
import "dotenv/config";
import type { Config } from "drizzle-kit";
export default {
out: "./drizzle",
schema: "./src/db/schema.ts",
dialect:
process.env.DRIZZLE_DB === "postgres"
? "postgresql"
: process.env.DRIZZLE_DB === "mysql"
? "mysql"
: "sqlite",
dbCredentials: {
url: process.env.DATABASE_URL!,
},
} satisfies Config;

Comment on lines +2 to +3
import { Hono } from 'hono';
import { api_sandbox_create } from './index';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Test uses Hono without OpenAPI support; app.openapi won’t exist → route won’t mount

The route implementation registers via app.openapi(...). Using Hono here will throw at runtime. Use OpenAPIHono and provide the required context vars (client, auth) via middleware.

Apply this diff to convert the test to a working harness with stubs and correct assertions:

-import { describe, expect, it } from 'bun:test';
-import { Hono } from 'hono';
-import { api_sandbox_create } from './index';
+import { describe, expect, it } from 'bun:test';
+import { OpenAPIHono } from '@hono/zod-openapi';
+import { api_sandbox_create } from './index';

 describe('sandbox create endpoints', () => {
-    it('POST /coderouter/api/sandbox/create returns empty object', async () => {
-        const app = new Hono();
-        api_sandbox_create(app);
-
-        const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/create', {
-            method: 'POST',
-        });
-
-        expect(response.status).toBe(201);
-        const body = await response.json();
-        expect(body).toEqual({});
-    });
+    it('POST /coderouter/api/sandbox/create returns id', async () => {
+        const app = new OpenAPIHono();
+        // Provide required context for the handler
+        app.use('*', async (c, next) => {
+            c.set('client', {
+                sandbox: {
+                    get: async () => ({}),
+                    resume: async () => ({}),
+                    create: async () => ({}),
+                },
+            });
+            c.set('auth', { sandboxId: 'test-sandbox' });
+            await next();
+        });
+        api_sandbox_create(app);
+
+        const response = await app.request('/coderouter/api/sandbox/create', {
+            method: 'POST',
+            headers: { 'content-type': 'application/json' },
+            body: JSON.stringify({}),
+        });
+
+        expect(response.status).toBe(200);
+        const body = await response.json();
+        expect(body).toEqual({ id: 'test-sandbox' });
+    });
 });

Also applies to: 7-9

🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/create/index.test.ts around lines 2-9, the
test instantiates Hono which lacks OpenAPI support so app.openapi(...) in the
route will fail; replace Hono with OpenAPIHono from the same library, add
middleware to inject the required context variables (stub `client` and `auth`)
before mounting api_sandbox_create, and ensure the test mounts the route against
the OpenAPIHono app and calls the correct path; update assertions to expect the
stubbed behavior (e.g., status and response body) and remove any direct
assumptions that rely on Hono-only behavior.

Comment on lines +10 to +12
const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/create', {
method: 'POST',
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

env is undefined in this test context

env.URL_PATH_PREFIX isn’t imported or defined. Either import the same env used by the router or inline the prefix literal. The diff above inlines '/coderouter' to de-risk config coupling.

🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/create/index.test.ts around lines 10–12, the
test references env.URL_PATH_PREFIX but env is not defined; replace
env.URL_PATH_PREFIX with the literal '/coderouter' or import the same env module
used by the router. Update the test to either (a) add the appropriate import for
env from its module and use env.URL_PATH_PREFIX, or (b) inline the path prefix
string '/coderouter' in the request URL to remove the undefined variable.

Comment on lines +14 to +16
expect(response.status).toBe(201);
const body = await response.json();
expect(body).toEqual({});
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Status code and response shape mismatch with implementation

Implementation returns 200 with { id: string }, not 201 with {}. The proposed diff aligns the test to the current contract.

To confirm the current response contract across the codebase, run:


🏁 Script executed:

#!/bin/bash
# Inspect sandbox create handler for status and body shape
rg -n -C2 --type=ts "api_sandbox_create|sandboxId|c\.json\s*\(" apps/coderouter/src

Length of output: 44302


Update sandbox create test to match implementation

The test currently expects a 201 status and an empty body, but the handler returns 200 with an { id: string } payload. Please adjust the assertions in apps/coderouter/src/api/sandbox/create/index.test.ts to:

• Expect a 200 status:

- expect(response.status).toBe(201);
+ expect(response.status).toBe(200);

• Expect the response body to contain an id string:

- expect(body).toEqual({});
+ expect(body).toEqual({ id: expect.any(String) });

Make sure the test supplies a mock auth.sandboxId in the context (e.g. via middleware or c.set("auth", { sandboxId: "test-id" })) so that c.get("auth").sandboxId is defined.

📝 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.

Suggested change
expect(response.status).toBe(201);
const body = await response.json();
expect(body).toEqual({});
expect(response.status).toBe(200);
const body = await response.json();
expect(body).toEqual({ id: expect.any(String) });
🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/create/index.test.ts around lines 14 to 16,
the test asserts a 201 status and an empty body but the handler returns 200 with
a payload { id: string }; update the assertions to expect response.status
toBe(200) and to assert that the parsed body has an id property that is a string
(e.g. expect(typeof body.id).toBe("string") or
expect(body).toHaveProperty("id")); also ensure the test sets a mock auth
context before invoking the handler (for example c.set("auth", { sandboxId:
"test-id" })) so c.get("auth").sandboxId is defined during the request.

Comment on lines +2 to +3
import { Hono } from 'hono';
import { api_sandbox_file_copy } from './index';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Harness missing OpenAPI app and context stubs

Same issue as other tests. Provide OpenAPIHono, and set client/auth.

Suggested fix:

-import { describe, expect, it } from 'bun:test';
-import { Hono } from 'hono';
-import { api_sandbox_file_copy } from './index';
+import { describe, expect, it } from 'bun:test';
+import { OpenAPIHono } from '@hono/zod-openapi';
+import { api_sandbox_file_copy } from './index';

 describe('sandbox files copy endpoints', () => {
     it('POST /coderouter/api/sandbox/file/copy returns empty object', async () => {
-        const app = new Hono();
-        api_sandbox_file_copy(app);
+        const app = new OpenAPIHono();
+        app.use('*', async (c, next) => {
+            c.set('client', { sandbox: { file: { copy: async () => ({}) } } });
+            c.set('auth', { sandboxId: 'test-sandbox' });
+            await next();
+        });
+        api_sandbox_file_copy(app);
 
-        const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/file/copy', {
-            method: 'POST',
-        });
+        const response = await app.request('/coderouter/api/sandbox/file/copy', {
+            method: 'POST',
+            headers: { 'content-type': 'application/json' },
+            body: JSON.stringify({
+                source: '/a.txt',
+                destination: '/b.txt',
+                overwrite: false,
+                recursive: false,
+            }),
+        });
 
         expect(response.status).toBe(200);
         const body = await response.json();
         expect(body).toEqual({});
     });
 });

Also applies to: 7-9

🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/file/copy/index.test.ts around lines 2-3 (and
also apply same change to lines 7-9), the test harness is missing the OpenAPI
app and context stubs; update the test to construct and use an OpenAPIHono
instance (import it), mount the api_sandbox_file_copy handler onto that app, and
set up the request context with a mock client and auth (e.g., client: { ... }
and auth: { user: ... } or equivalent stubs used elsewhere) so the handler has
the expected app and ctx values during the test.

Comment on lines +1 to +24
-----BEGIN CERTIFICATE-----
MIIEBDCCAuygAwIBAgIURypApXyi6zML9QoB5/0BYkPr5TowDQYJKoZIhvcNAQEL
BQAwgZIxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQH
DA1TYW4gRnJhbmNpc2NvMQ8wDQYDVQQKDAZPbmxvb2sxDDAKBgNVBAsMA0VuZzEY
MBYGA1UEAwwPb25sb29rLmludGVybmFsMR0wGwYJKoZIhvcNAQkBFg5lbmdAb25s
b29rLmNvbTAeFw0yNTA4MDcxNzQ0NDVaFw0yNjA4MDcxNzQ0NDVaMIGSMQswCQYD
VQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5j
aXNjbzEPMA0GA1UECgwGT25sb29rMQwwCgYDVQQLDANFbmcxGDAWBgNVBAMMD29u
bG9vay5pbnRlcm5hbDEdMBsGCSqGSIb3DQEJARYOZW5nQG9ubG9vay5jb20wggEi
MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDCkpqbfeXnSorbz8ciQ0q5ydN2
UDsK6eqnJDXE6FO0WpbQlWEApORFML59zELaPx2sALTfnjfJuwFREuerSwMLqAl7
UfHWrGtur1Sn99Xr2/gjToOnlIew0IY8t9hmY7Gg0lTlU1Xf7lNGjyiDj03prBQh
5txa2SzeqewmMbPKQ6hdac4kjga55UQP29MpPgNjW7HIr3SherR8JHKv03ZvnRR+
SXJ00znocEbQaCLBuSO7ZNj3oNT0pn3WuhLTj47breaCC8mWADbYC1Ltss5tIKPT
ogjRqxkZbm3sfd2cq1HL0x4FwlboLPt9wo5UvlOs/qts1iP8XVKTy/uj9bcbAgMB
AAGjUDBOMC0GA1UdEQQmMCSCD29ubG9vay5pbnRlcm5hbIIRKi5vbmxvb2suaW50
ZXJuYWwwHQYDVR0OBBYEFB3RhNcRNIcIqjVyhsEL2DFQtNLUMA0GCSqGSIb3DQEB
CwUAA4IBAQCWz0xa40patIChU0I4Y1MqfJ1ic+iy7iP8aLmSRdoDp/8JSf7LYpnd
I9NRGKtpQp6uv5bmGMoK8A+J1BfZp2TZ0dMxsIEANEYYiq9aej095isQueFaK2s7
6ji4pR3ahhljdcwpsMDYicdJBnkl9xXWlaGxXb0ZerYfPh4+e0CP/5hJNQDZttww
sFe1ksupoWzKWJIYB80S+HDQo7FvHvTXm+VOqfNTIxait3I+KEnt8ovRbSu4PsMw
kAdvwDhb7QBxEIriZgxjgLEX0XaGuC84oA0fttjFuwdhOhy4o5aBnhUsWWFdHPoi
4vdIjPk+l06LeK3YdploPhnVv6ATpavZ
-----END CERTIFICATE-----
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Do not commit TLS certificates (even for local dev) to VCS.

Keeping certs in-repo risks accidental reuse and trust issues. Prefer generating at build/startup (e.g., mkcert or OpenSSL) and mounting via volume; commit only scripts and README. If you must keep them, add explicit warnings and ensure they’re never used outside local docker-compose.

Please:

  • Remove this file and its private key from git history, add the path to .gitignore.
  • Confirm docker/nginx loads certs from a mounted path generated by a script (e.g., apps/nginx/bin/genkeys.sh).
  • If retaining for tests, rename to onlook-internal.dev.crt and gate use behind NODE_ENV=development.

🏁 Script executed:

#!/bin/bash
set -eux

echo "1) Searching for references to onlook-internal.crt in the repo..."
rg -n "onlook-internal\.crt" || echo "No direct references found."

echo "2) Checking for ssl_certificate and ssl_certificate_key directives in nginx configs..."
rg -n "ssl_certificate(_key)?" -g "**/apps/nginx/**/*.conf" -C2 || echo "No ssl_certificate directives in apps/nginx."

echo "3) Looking for a genkeys.sh script under apps/nginx..."
find apps/nginx -type f -name "genkeys.sh" || echo "genkeys.sh not found."

echo "4) Verifying .gitignore for ssl cert entries..."
grep -E "apps/nginx/ssl|onlook-internal" .gitignore || echo "No .gitignore entries for apps/nginx/ssl or onlook-internal."

Length of output: 1866


Remove committed TLS certificates and mount generated ones at runtime

Please address the following critical security fixes:

  • Delete the checked-in files under apps/nginx/ssl/
    onlook-internal.crt
    onlook-internal.key
    – Purge them from Git history (e.g., using git filter-repo or git filter-branch).
  • Add these paths (or the entire apps/nginx/ssl/ directory) to .gitignore to prevent future commits.
  • Update apps/docker-compose.yaml (currently mounting ./nginx/ssl/onlook-internal.crt) to instead mount the directory where your build/startup script places the certs (e.g., the output location of apps/nginx/bin/genkeys.sh) into /etc/nginx/ssl.
  • Ensure apps/nginx/bin/genkeys.sh is invoked before NGINX starts (for example, as part of the container’s entrypoint), so that certificates are generated on-demand rather than stored in VCS.
  • Note that apps/nginx/conf.d/server.conf uses:
    ssl_certificate /etc/nginx/ssl/onlook-internal.crt;
    ssl_certificate_key /etc/nginx/ssl/onlook-internal.key;
    Confirm these paths align with your new volume mount.
  • If you still need in-repo certs for CI/tests, rename them to onlook-internal.dev.crt/.key, gate their use on NODE_ENV=development, and document the caveat in your README.
🤖 Prompt for AI Agents
In apps/nginx/ssl/onlook-internal.crt lines 1-24, a private TLS certificate is
committed to the repo; remove the tracked files
apps/nginx/ssl/onlook-internal.crt and apps/nginx/ssl/onlook-internal.key, purge
them from Git history (e.g., git filter-repo or filter-branch), add
apps/nginx/ssl/ (or those filenames) to .gitignore, update
apps/docker-compose.yaml to mount the runtime-generated cert directory (the
output of apps/nginx/bin/genkeys.sh) into /etc/nginx/ssl instead of mounting the
checked-in file, ensure the container entrypoint or startup invokes
apps/nginx/bin/genkeys.sh before NGINX starts so certs are generated at runtime,
confirm apps/nginx/conf.d/server.conf ssl_certificate and ssl_certificate_key
paths match /etc/nginx/ssl, and if you need repo certs for CI rename them to
onlook-internal.dev.crt/.key and gate their use on NODE_ENV=development with
documentation in the README.

@@ -0,0 +1 @@
Do not use this private and public key in prod. Those are designed for local usage only.
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid committing private keys; generate at dev-time instead

Even for local use, keeping a private key in-repo is risky (accidental reuse in non-local contexts, leaking via forks/mirrors, security scanners noise). Prefer generating on developer machines or during docker-compose up with a script, and git-ignore the outputs. If you must retain artifacts, ensure they are invalid for production and never mounted in non-local stacks.

Suggested actions:

  • Remove committed key/cert, add them to .gitignore, and wire docker-compose to run apps/nginx/bin/genkeys.sh at startup or document a one-time generate step.
  • If keeping them, move to a path excluded from published artifacts and add a pre-commit/CI check preventing use outside dev.
  • Add explicit “DEVELOPMENT ONLY” notes in server.conf and docker-compose to avoid accidental promotion.

I can propose updated compose/nginx config to generate and mount the keys at runtime—want me to draft that?

🤖 Prompt for AI Agents
In apps/nginx/ssl/README.md around lines 1 to 1: the repo currently contains
committed SSL key/cert material intended only for local use; remove the
committed private/public keys from the repo, add their file paths to .gitignore,
and replace them with a documented developer workflow (or docker-compose entry)
that generates keys at dev-time via a script (e.g., apps/nginx/bin/genkeys.sh)
executed during docker-compose up or as a one-time documented step; ensure any
retained example files are clearly labeled “DEVELOPMENT ONLY” in server.conf and
docker-compose.yml, and add a CI/pre-commit check to block accidental promotion
or inclusion of real keys in non-local stacks.

@@ -28,6 +28,7 @@ yarn-error.log*

# local env files
.env*.local
e2b.toml
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify the example file exists and docs instruct copying

Ensure e2b.toml.example is present and referenced in developer docs so contributors know to copy it to /e2b.toml locally.

Run this to verify the example file and show its path:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -n -H -a 'e2b.toml.example' -C1

Length of output: 285


Let’s retry locating the example file with a corrected fd invocation:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Search hidden files and all directories for e2b.toml.example
fd -H -a 'e2b.toml.example'

Length of output: 101


Let’s check the project’s Markdown docs for any references to e2b.toml or the example file:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Search all markdown files for e2b.toml references
rg -n --glob '*.md' 'e2b\.toml'

Length of output: 53


Ensure e2b.toml.example is documented for contributors

We’ve confirmed the example file exists at
apps/web/template/e2b.toml.example

However, there are no references in the project’s Markdown docs instructing developers to copy it to e2b.toml before running the app. Please add a note in your developer guide or README:

• In your root README.md (or a dedicated docs file), include a step such as:
“Copy apps/web/template/e2b.toml.example to /e2b.toml at the project root and customize as needed.”

• If there’s a README under apps/web/template, add a similar copy instruction there.

This will ensure new contributors know how to initialize their local configuration.

Comment on lines +11 to +15
RUN curl -fsSL https://bun.com/install | bash
RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.3/install.sh | bash && \
. $NVM_DIR/nvm.sh && \
nvm install $NODE_VERSION && \
nvm use $NODE_VERSION
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Bun is on PATH or start_cmd will fail.

The E2B start_cmd uses bun, but the Dockerfile doesn’t export Bun to PATH. Without this, bun dev won’t be found inside the sandbox.

Apply this diff to export Bun’s bin dir:

 RUN curl -fsSL https://bun.com/install | bash
+ENV PATH="/root/.bun/bin:${PATH}"
🤖 Prompt for AI Agents
In apps/web/template/e2b.Dockerfile around lines 11 to 15, Bun is installed but
its bin directory is not added to PATH, so the E2B start_cmd (bun dev) will fail
to find the executable; after the Bun install line, export Bun’s bin directory
into PATH (e.g., append the typical Bun bin path to PATH or create an ENV/PATH
update so that subsequent layers and the container runtime can find bun) and
ensure that the export is in the same RUN layer or expressed as ENV so the PATH
change persists for later commands and the container runtime.

# const sandbox = await Sandbox.create('#########')

team_id = "27b622d8-2199-4c46-9e8c-7cb1b00da325"
start_cmd = "(cd /code && bun dev)"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

start_cmd will fail unless Bun is on PATH; either fix Dockerfile PATH or use absolute bun path here.

Given the current Dockerfile, bun isn’t on PATH. You can unblock execution by using the absolute path, though fixing PATH in the Dockerfile is preferred.

-start_cmd = "(cd /code && bun dev)"
+start_cmd = "(cd /code && /root/.bun/bin/bun dev)"

Also ensure your dev server binds to 0.0.0.0:3000 inside the container so E2B can forward it.

🤖 Prompt for AI Agents
In apps/web/template/e2b.toml.example around line 16, start_cmd uses "bun" which
will fail because Bun is not on PATH in the current Dockerfile; either update
the Dockerfile to add Bun's install directory to PATH (e.g., export
PATH=/path/to/bun/bin:$PATH) or change start_cmd to call Bun by its absolute
path in the image (replace "bun" with the full path to the bun binary), and
ensure the app's dev server is configured to bind to 0.0.0.0:3000 inside the
container so E2B can forward the port.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review continued from previous batch...

"jsonwebtoken": "^9.0.2",
"mysql2": "^3.9.7",
"pg": "^8.11.5",
"zod": "^4.0.17"
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify zod v4 compatibility with @hono/zod-openapi v1.1.0

There have been breaking changes in Zod v4; many libraries still target Zod v3. Confirm compatibility to avoid subtle type/runtime issues. If incompatible, pin Zod to ^3.23.x or upgrade @hono/zod-openapi accordingly.


🌐 Web query:

Does @hono/zod-openapi version 1.1.0 support Zod v4, or does it require Zod v3?

💡 Result:

Short answer: It still expects Zod v3 — @hono/zod-openapi v1.1.0 does not (yet) support Zod v4.

Evidence:

  • The repo issue opened to request Zod v4 support (shows the package wasn't updated for v4 as of May 27, 2025). (github.com)
  • The published package (v1.1.0) README/npm listing continues to show the older zod-openapi usage pattern (no mention of zod/v4 support). (npmjs.com)

If you want, I can:

  • fetch the v1.1.0 package.json/peerDependencies to confirm exactly which zod range it declares, or
  • suggest workarounds (use zod v3, or switch to hono-openapi / other tooling that supports zod v4). Which would you prefer?

Citations:


Fix Zod compatibility and pin Bun engine

Your current setup uses Zod v4 alongside @hono/zod-openapi v1.1.0, which still expects Zod v3. To prevent runtime/type errors, you must downgrade Zod to the peer range declared by @hono/zod-openapi (v3.x) or upgrade to a middleware version that supports Zod v4 once available. Also, declaring an engines field for Bun will lock the runtime version for consistent environments.

• apps/coderouter/package.json:

  "dependencies": {
-   "zod": "^4.0.17",
+   "zod": "^3.23.0",
    // …other deps…
  },
  "scripts": {
    "dev": "bun run --hot src/server.ts",
    "start": "bun run src/server.ts",
    "db:migrate": "bun run scripts/migrate.ts"
  },
+ "engines": {
+   "bun": ">=1.1.0"
+ }

References:

  1. Issue requesting Zod v4 support in @hono/zod-openapi v1.1.0: [@hono/zod-openapi] Supports Zod v4 honojs/middleware#1177
  2. @hono/zod-openapi v1.1.0 npm listing (no Zod v4 peer): https://www.npmjs.com/package/%40hono/zod-openapi?activeTab=versions
📝 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.

Suggested change
"zod": "^4.0.17"
"dependencies": {
"zod": "^3.23.0",
// …other deps…
},
"scripts": {
"dev": "bun run --hot src/server.ts",
"start": "bun run src/server.ts",
"db:migrate": "bun run scripts/migrate.ts"
},
"engines": {
"bun": ">=1.1.0"
}
🤖 Prompt for AI Agents
In apps/coderouter/package.json around line 28, Zod is declared as "^4.0.17"
while @hono/zod-openapi v1.1.0 expects Zod v3.x, causing incompatibility; update
package.json to use a Zod v3 peer (e.g., "zod": "^3.0.0") or upgrade
@hono/zod-openapi to a version that officially supports Zod v4, and add an
"engines" field to pin Bun (e.g., "engines": { "bun": ">=1.0.0 <2.0.0" } or your
target Bun version) to ensure consistent runtime; after changing, run install
and verify types/tests.


const ResponseSchema = z.object({
jwt: z.string().openapi({
description: `The JWT token to send when interacting with the API as header "X-Auth-Jwt.".`,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in JWT header description.

The description mentions "X-Auth-Jwt." with an extra period at the end.

-    description: `The JWT token to send when interacting with the API as header "X-Auth-Jwt.".`,
+    description: `The JWT token to send when interacting with the API as header "X-Auth-Jwt".`,

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/coderouter/src/api/auth/sign/index.ts around line 26, the description
string contains a typo: the header name is written as "X-Auth-Jwt." with an
extra period inside the quotes; remove the trailing period so the header reads
"X-Auth-Jwt" (i.e., update the description text to `The JWT token to send when
interacting with the API as header "X-Auth-Jwt".`).

Comment on lines +50 to +52
description:
"Create a new sandbox. If the sandbox already exists, the system will resume the sandbox.",
},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect OpenAPI description for the auth/sign endpoint.

The response description refers to creating/resuming a sandbox, but this endpoint only issues JWT tokens for authentication.

-      description:
-        "Create a new sandbox. If the sandbox already exists, the system will resume the sandbox.",
+      description:
+        "Issue a JWT token for API authentication.",
📝 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.

Suggested change
description:
"Create a new sandbox. If the sandbox already exists, the system will resume the sandbox.",
},
description:
"Issue a JWT token for API authentication.",
},
🤖 Prompt for AI Agents
In apps/coderouter/src/api/auth/sign/index.ts around lines 50 to 52, the OpenAPI
response description wrongly mentions creating/resuming a sandbox; replace that
description with an accurate one stating that the endpoint issues/returns a JWT
access token (or authentication token) for the authenticated user, and ensure
the wording matches any response schema (e.g., "Returns a JWT access token for
authentication") so docs reflect the endpoint's true behavior.

Comment on lines +56 to +68
try {
await c.get("client").sandbox.get({});
await c.get("client").sandbox.resume({});
} catch (e) {
if (
e instanceof ClientError &&
e.code === ClientErrorCode.SandboxNotFound
) {
await c.get("client").sandbox.create(body);
} else {
throw e;
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reconsider the create-or-resume logic.

The current implementation attempts to get and resume an existing sandbox before creating a new one. This approach has several issues:

  1. The empty object {} passed to sandbox.get({}) and sandbox.resume({}) may not provide the necessary context (e.g., sandbox ID).
  2. The logic seems to assume that if a sandbox exists, it should be resumed instead of creating a new one, but the endpoint is explicitly named "create".
  3. This violates the principle of least surprise - a create endpoint should create, not resume.

Consider simplifying the logic to just create:

 export function api_sandbox_create(app: LocalHono) {
   app.openapi(route, async (c) => {
     const body = await c.req.valid("json");
-    try {
-      await c.get("client").sandbox.get({});
-      await c.get("client").sandbox.resume({});
-    } catch (e) {
-      if (
-        e instanceof ClientError &&
-        e.code === ClientErrorCode.SandboxNotFound
-      ) {
-        await c.get("client").sandbox.create(body);
-      } else {
-        throw e;
-      }
-    }
+    await c.get("client").sandbox.create(body);
     return c.json(
       {
         id: c.get("auth").sandboxId!,
       },
       200
     );
   });
 }

If you need a create-or-resume endpoint, consider naming it appropriately (e.g., /api/sandbox/ensure or /api/sandbox/create-or-resume).

📝 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.

Suggested change
try {
await c.get("client").sandbox.get({});
await c.get("client").sandbox.resume({});
} catch (e) {
if (
e instanceof ClientError &&
e.code === ClientErrorCode.SandboxNotFound
) {
await c.get("client").sandbox.create(body);
} else {
throw e;
}
}
export function api_sandbox_create(app: LocalHono) {
app.openapi(route, async (c) => {
const body = await c.req.valid("json");
await c.get("client").sandbox.create(body);
return c.json(
{
id: c.get("auth").sandboxId!,
},
200
);
});
}
🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/create/index.ts around lines 56 to 68, the
create endpoint currently calls sandbox.get({}) and sandbox.resume({}) with
empty objects (missing sandbox context) and treats existing sandboxes by
resuming them, which is surprising for a "create" endpoint; change the
implementation to call await c.get("client").sandbox.create(body) directly and
return its result (remove the get/resume try/catch), and if you actually need
create-or-resume semantics implement a separate endpoint with a clear name
(e.g., /ensure or /create-or-resume) that accepts the required sandbox
identifier before attempting resume.

}
return c.json(
{
id: c.get("auth").sandboxId!,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Non-null assertion on sandboxId could cause runtime errors.

Similar to the pause endpoint, using c.get("auth").sandboxId! with a non-null assertion is risky.

Add validation:

+    const sandboxId = c.get("auth").sandboxId;
+    if (!sandboxId) {
+      return c.json({ error: "Sandbox ID not found in auth context" }, 401);
+    }
     return c.json(
       {
-        id: c.get("auth").sandboxId!,
+        id: sandboxId,
       },
       200
     );
📝 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.

Suggested change
id: c.get("auth").sandboxId!,
const sandboxId = c.get("auth").sandboxId;
if (!sandboxId) {
return c.json({ error: "Sandbox ID not found in auth context" }, 401);
}
return c.json(
{
id: sandboxId,
},
200
);
🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/create/index.ts around line 71, the code uses
a non-null assertion c.get("auth").sandboxId! which can throw at runtime;
replace it with an explicit validation: retrieve auth = c.get("auth"), check if
auth and auth.sandboxId exist, and if not return an appropriate error response
(e.g., 400 Bad Request or a typed error) before proceeding; once validated, use
the validated sandboxId variable (no !) for the id field and ensure callers/logs
reflect the validation result.

- '4444:4444'
volumes:
- ./coderouter:/app
command: [/usr/local/bin/bun/bin/bun, dev]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Container command likely invalid; will fail to start

Path /usr/local/bin/bun/bin/bun is unusual and bun dev won’t run your package.json script. Use bun run dev so the script is honored.

Apply this diff:

-        command: [/usr/local/bin/bun/bin/bun, dev]
+        command: ["bun", "run", "dev"]
📝 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.

Suggested change
command: [/usr/local/bin/bun/bin/bun, dev]
command: ["bun", "run", "dev"]
🤖 Prompt for AI Agents
In apps/docker-compose.yaml around line 14, the container command uses an
unusual path (/usr/local/bin/bun/bin/bun) and invokes "bun dev" which won't run
the package.json script; replace it with the correct bun binary path (e.g.
/usr/local/bin/bun or just bun if in PATH) and invoke the run subcommand so the
package script is executed, e.g. set the command to call the bun binary with
arguments ["run","dev"] instead of the current value.

Comment on lines +54 to +64
try {
const session = await provider.createSession({
args: {
id: shortenUuid(input.userId ?? uuidv4(), 20),
},
});
await provider.destroy();
return session;
} catch (error) {
throw error;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure provider is destroyed on both success and error paths.

Currently, destroy() is only called on success, leaking resources on errors.

Apply this diff:

-            try {
-                const session = await provider.createSession({
-                    args: {
-                        id: shortenUuid(input.userId ?? uuidv4(), 20),
-                    },
-                });
-                await provider.destroy();
-                return session;
-            } catch (error) {
-                throw error;
-            }
+            try {
+                const session = await provider.createSession({
+                    args: {
+                        id: shortenUuid(input.userId ?? uuidv4(), 20),
+                    },
+                });
+                return session;
+            } finally {
+                // Best-effort cleanup
+                await provider.destroy().catch(() => {});
+            }
📝 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.

Suggested change
try {
const session = await provider.createSession({
args: {
id: shortenUuid(input.userId ?? uuidv4(), 20),
},
});
await provider.destroy();
return session;
} catch (error) {
throw error;
}
try {
const session = await provider.createSession({
args: {
id: shortenUuid(input.userId ?? uuidv4(), 20),
},
});
return session;
} finally {
// Best-effort cleanup
await provider.destroy().catch(() => {});
}
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/project/sandbox.ts around lines 54 to
64, provider.destroy() is only called on the success path which leaks resources
when createSession throws; change the flow to ensure provider.destroy() is
always awaited by moving the destroy call into a finally block (or equivalent)
so that after attempting to createSession you always await provider.destroy(),
rethrowing the original error if createSession failed and returning the session
on success.

Comment on lines +11 to 18
const provider = await createCodeProviderClient(CodeProvider.Coderouter, {
providerOptions: {
codesandbox: {},
coderouter: {
url: env.CODEROUTER_HOST_URL,
sandboxId: newSandboxId,
userId: userId,
},
},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for required environment variables.

The code assumes env.CODEROUTER_HOST_URL is defined, but it's marked as optional in the env schema. This could cause runtime errors if the environment variable is not set.

Add validation at the beginning of the function:

 export async function forkBuildSandbox(
     sandboxId: string,
     userId: string,
     deploymentId: string,
 ): Promise<{ provider: Provider; sandboxId: string }> {
+    if (!env.CODEROUTER_HOST_URL) {
+        throw new Error('CODEROUTER_HOST_URL environment variable is required');
+    }
     const newSandboxId = uuidv4();
     const provider = await createCodeProviderClient(CodeProvider.Coderouter, {
📝 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.

Suggested change
const provider = await createCodeProviderClient(CodeProvider.Coderouter, {
providerOptions: {
codesandbox: {},
coderouter: {
url: env.CODEROUTER_HOST_URL,
sandboxId: newSandboxId,
userId: userId,
},
},
export async function forkBuildSandbox(
sandboxId: string,
userId: string,
deploymentId: string,
): Promise<{ provider: Provider; sandboxId: string }> {
if (!env.CODEROUTER_HOST_URL) {
throw new Error('CODEROUTER_HOST_URL environment variable is required');
}
const newSandboxId = uuidv4();
const provider = await createCodeProviderClient(CodeProvider.Coderouter, {
providerOptions: {
coderouter: {
url: env.CODEROUTER_HOST_URL,
sandboxId: newSandboxId,
userId: userId,
},
},
// …rest of the client options…
});
// …remaining function body…
}
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/publish/helpers/fork.ts around lines
11 to 18, the code uses env.CODEROUTER_HOST_URL which is optional in the schema;
add an explicit check at the start of the function to validate that
env.CODEROUTER_HOST_URL is defined (and non-empty), and if not, throw a clear
error or return a rejected Promise with a message explaining the missing
environment variable; then use the validated value when constructing the
provider options to avoid runtime undefined URL issues.

Comment on lines 30 to 34
await provider.destroy();

const forkedProvider = await createCodeProviderClient(CodeProvider.CodeSandbox, {
providerOptions: {
codesandbox: {
sandboxId: project.id,
userId,
initClient: true,
},
},
});

return {
provider: forkedProvider,
provider,
sandboxId: project.id,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Provider lifecycle management issue.

The function destroys the provider and then returns it. This could lead to issues if the caller tries to use the destroyed provider.

Either don't destroy the provider here (let the caller manage its lifecycle), or create a new provider instance to return:

-    await provider.destroy();
-
     return {
         provider,
         sandboxId: project.id,
     };

Or if you need to destroy it:

     await provider.destroy();
 
+    // Return a new provider instance for the forked sandbox
+    const forkedProvider = await createCodeProviderClient(CodeProvider.Coderouter, {
+        providerOptions: {
+            coderouter: {
+                url: env.CODEROUTER_HOST_URL!,
+                sandboxId: project.id,
+                userId: userId,
+            },
+        },
+    });
+
     return {
-        provider,
+        provider: forkedProvider,
         sandboxId: project.id,
     };

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/publish/helpers/fork.ts around lines
30–34, the function currently awaits provider.destroy() and then returns the
destroyed provider which can cause callers to use an unusable instance; fix this
by removing the await provider.destroy() from this function and let the caller
manage provider lifecycle (i.e., return the live provider and document that the
caller must call destroy when done), OR if you must clean up here instead, stop
returning the provider and return only sandboxId (or a fresh provider instance
if appropriate) so callers do not receive a destroyed object.

Comment on lines +72 to +75
- [x] Start from text or image
- [ ] Use prebuilt templates
- [ ] Import from Figma
- [ ] Start from GitHub repo
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix nested list indentation and a split bullet to satisfy markdownlint (MD007) and readability

The nested checklist items are indented with 4 spaces; your repo’s lint expects 2. Also, the “Detect and use Components” bullet is split across two lines, breaking the markdown structure.

Apply the following diffs:

-    - [x] Start from text or image
-    - [ ] Use prebuilt templates
-    - [ ] Import from Figma
-    - [ ] Start from GitHub repo
+  - [x] Start from text or image
+  - [ ] Use prebuilt templates
+  - [ ] Import from Figma
+  - [ ] Start from GitHub repo
-    - [x] Use Figma-like UI
-    - [x] Preview your app in real-time
-    - [x] Manage brand assets and tokens
-    - [x] Create and navigate to Pages
-    - [x] Browse layers
-    - [x] Manage project Images
-    - [ ] Detect and use Components – _Previously in
-          [Onlook Desktop](https://github.com/onlook-dev/desktop)_
+  - [x] Use Figma-like UI
+  - [x] Preview your app in real-time
+  - [x] Manage brand assets and tokens
+  - [x] Create and navigate to Pages
+  - [x] Browse layers
+  - [x] Manage project Images
+  - [ ] Detect and use Components – _Previously in [Onlook Desktop](https://github.com/onlook-dev/desktop)_
-    - [x] Real-time code editor
-    - [x] Save and restore from checkpoints
-    - [x] Run commands via CLI
-    - [x] Connect with app marketplace
+  - [x] Real-time code editor
+  - [x] Save and restore from checkpoints
+  - [x] Run commands via CLI
+  - [x] Connect with app marketplace
-    - [x] Generate sharable links
-    - [x] Link your custom domain
+  - [x] Generate shareable links
+  - [x] Link your custom domain
-    - [ ] Real-time editing
-    - [ ] Leave comments
+  - [ ] Real-time editing
+  - [ ] Leave comments

Also applies to: 77-85, 87-90, 93-97

🧰 Tools
🪛 LanguageTool

[grammar] ~72-~72: There might be a mistake here.
Context: ...conds - [x] Start from text or image - [ ] Use prebuilt templates - [ ] Imp...

(QB_NEW_EN)


[grammar] ~73-~73: There might be a mistake here.
Context: ...r image - [ ] Use prebuilt templates - [ ] Import from Figma - [ ] Start fr...

(QB_NEW_EN)


[grammar] ~74-~74: There might be a mistake here.
Context: ...lt templates - [ ] Import from Figma - [ ] Start from GitHub repo - [x] Visuall...

(QB_NEW_EN)


[grammar] ~75-~75: There might be a mistake here.
Context: ...m Figma - [ ] Start from GitHub repo - [x] Visually edit your app - [x] Use...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

72-72: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


73-73: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


74-74: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


75-75: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

🤖 Prompt for AI Agents
In README.md around lines 72-75 (and also apply same changes to ranges 77-85,
87-90, 93-97), the nested checklist items are indented with 4 spaces and one
checklist bullet ("Detect and use Components") is split across two lines; change
nested list indentation to 2 spaces to satisfy MD007 and merge the split bullet
into a single line so each checklist item is a single continuous line; ensure
all nested checklist lines in the listed ranges use 2-space indentation and no
checklist text is broken across multiple lines.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review continued from previous batch...

Comment on lines +1 to +71
import { Client } from "../../index";

export interface SandboxFileCopyInput {}
export interface SandboxFileCopyOutput {}

export interface SandboxFileDeleteInput {
path: string;
}
export interface SandboxFileDeleteOutput {}

export interface SandboxFileDownloadInput {}
export interface SandboxFileDownloadOutput {}

export interface SandboxFileListInput {
path: string;
}
export interface SandboxFileListOutput {
files: Array<{
name: string;
path: string;
type: "file" | "directory";
}>;
}

export interface SandboxFileReadInput {
path: string;
}
export interface SandboxFileReadOutput {
data: string;
}

export interface SandboxFileRenameInput {
oldPath: string;
newPath: string;
}
export interface SandboxFileRenameOutput {}

export interface SandboxFileStatInput {
path: string;
}
export interface SandboxFileStatOutput {
type: "file" | "directory";
}

export interface SandboxFileWriteInput {
files: Array<{
path: string;
data: string;
overwrite: boolean;
}>;
}
export interface SandboxFileWriteOutput {}

export abstract class SandboxFile<T extends Client> {
constructor(protected readonly client: T) {}

abstract copy(input: SandboxFileCopyInput): Promise<SandboxFileCopyOutput>;
abstract delete(
input: SandboxFileDeleteInput
): Promise<SandboxFileDeleteOutput>;
abstract download(
input: SandboxFileDownloadInput
): Promise<SandboxFileDownloadOutput>;
abstract list(input: SandboxFileListInput): Promise<SandboxFileListOutput>;
abstract read(input: SandboxFileReadInput): Promise<SandboxFileReadOutput>;
abstract rename(
input: SandboxFileRenameInput
): Promise<SandboxFileRenameOutput>;
abstract stat(input: SandboxFileStatInput): Promise<SandboxFileStatOutput>;
abstract write(input: SandboxFileWriteInput): Promise<SandboxFileWriteOutput>;
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Quick repo-wide sanity check to find current usage assumptions for copy/download.

Run this to locate all call sites and confirm expected shapes before updating the interfaces:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Searching for SandboxFile copy/download signatures and usages..."
rg -n -C2 -g '!**/node_modules/**' -P '\bSandboxFileCopyInput\b|\bcopy\s*\('
rg -n -C2 -g '!**/node_modules/**' -P '\bSandboxFileDownload(Input|Output)\b|\bdownload\s*\('

echo "Searching for router codegen copy/download request usage..."
rg -n -C2 -g '!**/node_modules/**' -P '\bCoderouterApiSandboxFileCopyPostRequest\b|\bCoderouterApiSandboxFileDownloadPostRequest\b'

Length of output: 83213


Update SandboxFile copy/download interfaces to match usage

Based on call-sites in the router, codegen models and the CodeSandbox provider, the empty interfaces in apps/coderouter/src/provider/definition/sandbox/file/index.ts need to be expanded to reflect the actual request and response shapes:

• File: apps/coderouter/src/provider/definition/sandbox/file/index.ts

  • Replace
    export interface SandboxFileCopyInput {}
    export interface SandboxFileCopyOutput {}
    with
    export interface SandboxFileCopyInput {
      source: string;
      destination: string;
      overwrite: boolean;
      recursive: boolean;
    }
    export interface SandboxFileCopyOutput {}  // remains empty
  • Replace
    export interface SandboxFileDownloadInput {}
    export interface SandboxFileDownloadOutput {}
    with
    export interface SandboxFileDownloadInput {
      path: string;
    }
    export interface SandboxFileDownloadOutput {
      url: string;
    }

These fields align with:

  • the Zod schemas in apps/coderouter/src/api/sandbox/file/{copy,download}/index.ts
  • the generated CoderouterApiSandboxFile{Copy,Download}PostRequest models
  • the CodeSandbox provider’s download implementation (returns { url: string })
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/definition/sandbox/file/index.ts lines 1-71, the
SandboxFile copy and download interfaces are empty but callers and schemas
expect specific fields; update SandboxFileCopyInput to include source (string),
destination (string), overwrite (boolean) and recursive (boolean) while leaving
SandboxFileCopyOutput empty, and update SandboxFileDownloadInput to include path
(string) and SandboxFileDownloadOutput to include url (string); ensure the
method signatures remain unchanged but use the new types so implementations and
generated models align with the Zod schemas and provider behavior.

Comment on lines +3 to +5
export interface SandboxFileCopyInput {}
export interface SandboxFileCopyOutput {}

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

File copy input is empty; define required fields (source, destination, overwrite, recursive).

This API surface is currently unusable without parameters and diverges from your router’s OpenAPI (which expects these fields). Recommend aligning now:

-export interface SandboxFileCopyInput {}
+export interface SandboxFileCopyInput {
+  source: string;
+  destination: string;
+  overwrite?: boolean; // defaults to false
+  recursive?: boolean; // defaults to false
+}
 export interface SandboxFileCopyOutput {}
📝 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.

Suggested change
export interface SandboxFileCopyInput {}
export interface SandboxFileCopyOutput {}
export interface SandboxFileCopyInput {
source: string;
destination: string;
overwrite?: boolean; // defaults to false
recursive?: boolean; // defaults to false
}
export interface SandboxFileCopyOutput {}
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/definition/sandbox/file/index.ts around lines 3
to 5, the SandboxFileCopyInput/SandboxFileCopyOutput interfaces are empty;
define SandboxFileCopyInput with required fields source: string and destination:
string and optional overwrite?: boolean and recursive?: boolean (and any needed
metadata like permissions or timeout if your OpenAPI requires them), and define
SandboxFileCopyOutput to include status: 'success' | 'failure', and an optional
message?: string and errorCode?: string; update any imports/usages and ensure
the interfaces match the router’s OpenAPI schema so callers and type-checking
align.

Comment on lines +11 to +13
export interface SandboxFileDownloadInput {}
export interface SandboxFileDownloadOutput {}

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Download input/output shapes are unspecified; at minimum, include path.

The router’s OpenAPI requires a path for download. Also, consider defining the output shape to make implementations interoperable (base64 for binary is a pragmatic default).

-export interface SandboxFileDownloadInput {}
-export interface SandboxFileDownloadOutput {}
+export interface SandboxFileDownloadInput {
+  path: string;
+}
+export interface SandboxFileDownloadOutput {
+  // Raw file payload as base64 to support binary and text uniformly
+  data: string;
+  encoding: "base64";
+  // Optional metadata
+  filename?: string;
+  mimeType?: string;
+}

If you intend to return a signed URL instead, define it explicitly:

export type SandboxFileDownloadOutput =
  | { kind: "inline"; data: string; encoding: "base64"; filename?: string; mimeType?: string }
  | { kind: "url"; url: string; expiresAt?: string };
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/definition/sandbox/file/index.ts around lines 11
to 13, the SandboxFileDownloadInput and SandboxFileDownloadOutput are empty;
update the input to include at least a required path string and update the
output to a defined shape so implementations interoperate—for example, use a
discriminated union for inline vs url responses (inline should include
base64-encoded data, encoding, optional filename and mimeType; url should
include the signed URL and optional expiration), or otherwise document the exact
fields and types you expect (strings, ISO timestamps) so callers and providers
agree.

Comment on lines +200 to +202
protected fullpath(path: string): string {
return this.folder + (path.startsWith("/") ? "" : "/") + path;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden path handling to prevent traversal outside the sandbox root.

Currently fullpath simply concatenates strings. Normalize and enforce the /code prefix to keep all operations scoped to the sandbox folder.

Apply this diff:

-  protected fullpath(path: string): string {
-    return this.folder + (path.startsWith("/") ? "" : "/") + path;
-  }
+  protected fullpath(p: string): string {
+    // Ensure POSIX semantics inside the Linux sandbox
+    const joined = `${this.folder}/${p.replace(/^\/+/, "")}`;
+    const normalized = path.posix.normalize(joined);
+    // Allow exactly "/code" or any subpath under it
+    if (!(normalized === this.folder || normalized.startsWith(this.folder + "/"))) {
+      throw new ClientError(
+        ClientErrorCode.ImplementationError,
+        "Path traversal detected",
+        false
+      );
+    }
+    return normalized;
+  }
📝 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.

Suggested change
protected fullpath(path: string): string {
return this.folder + (path.startsWith("/") ? "" : "/") + path;
}
protected fullpath(p: string): string {
// Ensure POSIX semantics inside the Linux sandbox
const joined = `${this.folder}/${p.replace(/^\/+/, "")}`;
const normalized = path.posix.normalize(joined);
// Allow exactly "/code" or any subpath under it
if (!(normalized === this.folder || normalized.startsWith(this.folder + "/"))) {
throw new ClientError(
ClientErrorCode.ImplementationError,
"Path traversal detected",
false
);
}
return normalized;
}

Comment on lines +21 to +28
export interface CoderouterApiAuthSignPost200Response {
/**
* The JWT token to send when interacting with the API as header "X-Auth-Jwt.".
* @type {string}
* @memberof CoderouterApiAuthSignPost200Response
*/
jwt: string;
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify header name consistency for JWT propagation

JSDoc mentions header "X-Auth-Jwt". Ensure the server, client, and middleware consistently use this header (vs. Authorization: Bearer).

Run:


🏁 Script executed:

#!/bin/bash
# Check usage of X-Auth-Jwt vs Authorization across the repo.
rg -n -C2 -i '\bX-Auth-Jwt\b|\bAuthorization\b'
# Check coderouter middleware/routes for expected header name.
rg -n -C3 -i 'auth|jwt|x-auth-jwt' apps/coderouter 2>/dev/null || true

Length of output: 116073


Unify JWT header across server and client

The server middleware strictly reads “X-Auth-Jwt”, but our generated client (DefaultApi.ts) injects the JWT via Authorization: Bearer <token>. This will break authentication at runtime.

Key locations:

  • apps/coderouter/src/middleware/auth.ts → c.req.header("X-Auth-Jwt")
  • packages/code-provider/src/providers/coderouter/index.ts → uses
    authorization: Bearer <CODEROUTER_API_KEY> for API key auth
    X-Auth-Jwt: this.jwt for JWT calls
  • packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts → dozens of methods doing
    headerParameters['Authorization'] = \Bearer ${tokenString}``

Action items:

  • Patch or configure the OpenAPI generator so that JWT-protected endpoints use:
    headerParameters['X-Auth-Jwt'] = tokenString
  • Leave the API-key calls on Authorization: Bearer <CODEROUTER_API_KEY> as-is
  • Run end-to-end tests to confirm the server accepts the new header and rejects missing/mismatched ones
🤖 Prompt for AI Agents
In
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiAuthSignPost200Response.ts
(lines 21-28) the client/server JWT header is inconsistent: the server expects
X-Auth-Jwt but the generated client injects Authorization: Bearer <token>. Fix
by updating the OpenAPI spec so the JWT security scheme is declared as an apiKey
in header with name "X-Auth-Jwt" (keep the existing API-key/bearer scheme for
Authorization unchanged), then re-run the OpenAPI generator (or adjust the
generator template) to regenerate DefaultApi.ts so JWT-protected endpoints set
headerParameters['X-Auth-Jwt'] = tokenString; finally run end-to-end tests to
verify the server accepts X-Auth-Jwt and rejects missing/mismatched headers.

@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
Copy link
Contributor

Choose a reason for hiding this comment

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

@http-teapot fyi key leakage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants