-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[Work-in-progress] Code Provider API with E2B #2711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Someone is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughIntroduces 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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
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 unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
api_sandbox_url(app); | ||
|
||
const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/url', { | ||
method: 'GET', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
method: 'GET', | |
method: 'POST', |
}>(); | ||
|
||
// Set up auth middleware first (required for client middleware) | ||
setupAuthJwtMiddleware(app); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setupAuthJwtMiddleware() is called without the required path argument. The function signature expects (app, path).
setupAuthJwtMiddleware(app); | |
setupAuthJwtMiddleware(app, env.URL_PATH_PREFIX + '/api'); |
setupAuthJwtMiddleware(app); | ||
|
||
// Set up client middleware | ||
setupClientMiddleware(app); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setupClientMiddleware() is called without providing the required path argument. Please add the appropriate path.
setupClientMiddleware(app); | |
setupClientMiddleware(app, env.URL_PATH_PREFIX); |
|
||
expect(mockClient._sandbox.files.remove).toHaveBeenCalledTimes(1); | ||
expect(mockClient._sandbox.files.remove).toHaveBeenCalledWith( | ||
"/test/file.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
"/test/file.txt" | |
"/code/test/file.txt" |
|
||
export const dbKind: DBKind = (process.env.DRIZZLE_DB as DBKind) || "sqlite"; | ||
|
||
export async function getDb() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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);
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, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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; |
import { Hono } from 'hono'; | ||
import { api_sandbox_create } from './index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/create', { | ||
method: 'POST', | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
expect(response.status).toBe(201); | ||
const body = await response.json(); | ||
expect(body).toEqual({}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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.
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.
import { Hono } from 'hono'; | ||
import { api_sandbox_file_copy } from './index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
-----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----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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., usinggit filter-repo
orgit 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 ofapps/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 onNODE_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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
"jsonwebtoken": "^9.0.2", | ||
"mysql2": "^3.9.7", | ||
"pg": "^8.11.5", | ||
"zod": "^4.0.17" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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:
- 1: https://github.com/honojs/middleware/issues/1177?utm_source=chatgpt.com
- 2: https://www.npmjs.com/package/%40hono/zod-openapi?activeTab=versions
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:
- Issue requesting Zod v4 support in @hono/zod-openapi v1.1.0: [@hono/zod-openapi] Supports Zod v4 honojs/middleware#1177
- @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.
"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.".`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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".`).
description: | ||
"Create a new sandbox. If the sandbox already exists, the system will resume the sandbox.", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
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; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- The empty object
{}
passed tosandbox.get({})
andsandbox.resume({})
may not provide the necessary context (e.g., sandbox ID). - 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".
- 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.
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!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
try { | ||
const session = await provider.createSession({ | ||
args: { | ||
id: shortenUuid(input.userId ?? uuidv4(), 20), | ||
}, | ||
}); | ||
await provider.destroy(); | ||
return session; | ||
} catch (error) { | ||
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
const provider = await createCodeProviderClient(CodeProvider.Coderouter, { | ||
providerOptions: { | ||
codesandbox: {}, | ||
coderouter: { | ||
url: env.CODEROUTER_HOST_URL, | ||
sandboxId: newSandboxId, | ||
userId: userId, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
await provider.destroy(); | ||
|
||
const forkedProvider = await createCodeProviderClient(CodeProvider.CodeSandbox, { | ||
providerOptions: { | ||
codesandbox: { | ||
sandboxId: project.id, | ||
userId, | ||
initClient: true, | ||
}, | ||
}, | ||
}); | ||
|
||
return { | ||
provider: forkedProvider, | ||
provider, | ||
sandboxId: project.id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
- [x] Start from text or image | ||
- [ ] Use prebuilt templates | ||
- [ ] Import from Figma | ||
- [ ] Start from GitHub repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
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>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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
with
export interface SandboxFileCopyInput {} export interface SandboxFileCopyOutput {}
export interface SandboxFileCopyInput { source: string; destination: string; overwrite: boolean; recursive: boolean; } export interface SandboxFileCopyOutput {} // remains empty
- Replace
with
export interface SandboxFileDownloadInput {} export interface SandboxFileDownloadOutput {}
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.
export interface SandboxFileCopyInput {} | ||
export interface SandboxFileCopyOutput {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
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.
export interface SandboxFileDownloadInput {} | ||
export interface SandboxFileDownloadOutput {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
protected fullpath(path: string): string { | ||
return this.folder + (path.startsWith("/") ? "" : "/") + path; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
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; | |
} |
export interface CoderouterApiAuthSignPost200Response { | ||
/** | ||
* The JWT token to send when interacting with the API as header "X-Auth-Jwt.". | ||
* @type {string} | ||
* @memberof CoderouterApiAuthSignPost200Response | ||
*/ | ||
jwt: string; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@http-teapot fyi key leakage
Description
Adding an API abstraction layer for code sandboxes.
TODO
Related Issues
Type of Change
Testing
cp apps/coderouter/.env.example apps/coderouter/.env
.env
and specifyCODEROUTER_API_KEY
(make something up) andE2B_API_KEY
(sign up on e2b.dev)apps/web/client/.env
and addbun run backend:start
127.0.0.1 onlook.internal
in your/etc/hosts
(sudo vim /etc/hosts
)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.
CoderouterProvider
inpackages/code-provider/src/providers/coderouter
for E2B integration.apps/coderouter/src/api
for sandbox operations like create, pause, resume, stop, and file operations.apps/coderouter/src/server.ts
to register new routes and middleware.apps/coderouter/src/util/auth.ts
.CODEROUTER_API_KEY
andCODEROUTER_HOST_URL
to environment variables inapps/web/client/src/env.ts
.docker-compose.yaml
andnginx
config for local development.packages/code-provider/src/providers/coderouter/codegen
.package.json
scripts to include Docker setup for backend.This description was created by
for e016242. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation
Chores