-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[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. |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a new "coderouter" service (Bun + Hono) with OpenAPI endpoints, JWT/API-key auth, E2B sandbox integration, provider/client middleware and codegen client; wires web client to use coderouter/E2B; adds nginx TLS reverse proxy, docker-compose for local dev, many tests, docs, and tooling/config files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Web as Web Client
participant API as Coderouter API
participant MW as Middleware
participant E2B as E2B Sandbox
User->>Web: Trigger sandbox action (start/write/etc.)
Web->>API: POST /coderouter/api/auth/sign (Bearer API_KEY, {sandboxId?, userId?})
API-->>Web: 200 { jwt }
Web->>API: POST /coderouter/api/sandbox/file/write (Authorization: Bearer <jwt>, body)
activate API
API->>MW: enter middleware chain (error -> auth -> requireSandboxId -> client -> beforeSandboxCall)
MW->>E2B: instantiate/attach client (uses E2B_API_KEY & auth)
API->>E2B: sandbox.file.write(body)
E2B-->>API: {} (operation result)
API-->>Web: 200 {}
deactivate API
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
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.
|
||
# 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.
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
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: 11
♻️ Duplicate comments (2)
apps/coderouter/src/provider/e2b/sandbox/file/index.ts (2)
96-104
: Fix: don’t treat empty file contents as “not found”.Empty strings are valid file contents. Use a null/undefined check instead.
Apply this diff:
- const data = await this.client._sandbox.files.read(this.fullpath(input.path)); - if (!data) { + const data = await this.client._sandbox.files.read(this.fullpath(input.path)); + if (data == null) { throw new ClientError(ClientErrorCode.FileNotFound, 'File not found', false); } return { data, };
218-220
: Harden path resolution to prevent traversal outside /code.Normalize with POSIX semantics and enforce the /code prefix.
Apply this diff:
- protected fullpath(path: string): string { - return this.folder + (path.startsWith('/') ? '' : '/') + path; - } + protected fullpath(p: string): string { + const joined = `${this.folder}/${p.replace(/^\/+/, '')}`; + const normalized = path.posix.normalize(joined); + if (!(normalized === this.folder || normalized.startsWith(this.folder + '/'))) { + throw new ClientError( + ClientErrorCode.ImplementationError, + 'Path traversal detected', + false, + ); + } + return normalized; + }
🧹 Nitpick comments (26)
apps/coderouter/src/api/sandbox/file/watch/index.ts (4)
9-11
: Fix description: “write” → “watch”.- description: 'The path of the file to write.', + description: 'The path to watch for file events.',
12-19
: Add sane defaults for recursive and excludePaths to ease client usage.- recursive: z.boolean().openapi({ + recursive: z.boolean().default(false).openapi({ description: 'Whether to watch the file recursively.', example: false, }), - excludePaths: z.array(z.string()).openapi({ + excludePaths: z.array(z.string()).default([]).openapi({ description: 'The paths to exclude from the watch.', example: ['/path/to/exclude'], }),
33-33
: Guard against undefined URL_PATH_PREFIX to avoid “undefined/api/...” paths.- path: env.URL_PATH_PREFIX + '/api/sandbox/file/watch', + path: (env.URL_PATH_PREFIX ?? '') + '/api/sandbox/file/watch',
51-52
: Response description grammar.- description: 'Watch a file to the sandbox.', + description: 'Watch files in the sandbox.',apps/coderouter/src/server.ts (4)
24-24
: Remove unused import “serve”.-import { env, serve } from 'bun'; +import { env } from 'bun';
35-37
: Enable request logging since logger is imported.const app: LocalHono = new OpenAPIHono<{ Variables: Variables }>(); +// basic request logging +app.use('*', logger());
49-49
: Avoid “undefined/api” when URL_PATH_PREFIX is not set.-const basePath = env.URL_PATH_PREFIX + '/api'; +const basePath = (env.URL_PATH_PREFIX ?? '') + '/api';
59-59
: Clarify comment: client middleware ordering.-// must be last +// must precede any handlers/middleware that use c.get('client')packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPostRequest.ts (2)
23-27
: Docstring mismatch: “write” → “watch” (source OpenAPI).This file is generated. Update the OpenAPI schema (in apps/coderouter/src/api/sandbox/file/watch/index.ts) and re-generate to fix the description.
15-15
: Unused import (mapValues).Since linting is disabled for generated files, no action is strictly needed. It will disappear on re-generation if templates stop emitting it.
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200Response.ts (1)
15-15
: Unused import (mapValues).Harmless in generated code; will be resolved on next codegen pass.
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200ResponseEventsInner.ts (2)
15-15
: Remove unused import.
mapValues
is imported but not used. Delete to avoid dead code and quiet linters.-import { mapValues } from '../runtime';
50-56
: Harden type guard against non-objects.Ensure safe guards before using the
in
operator; widen param tounknown
for a proper type guard.-export function instanceOfCoderouterApiSandboxFileWatchPost200ResponseEventsInner( - value: object, -): value is CoderouterApiSandboxFileWatchPost200ResponseEventsInner { - if (!('path' in value) || value['path'] === undefined) return false; - if (!('type' in value) || value['type'] === undefined) return false; - return true; -} +export function instanceOfCoderouterApiSandboxFileWatchPost200ResponseEventsInner( + value: unknown, +): value is CoderouterApiSandboxFileWatchPost200ResponseEventsInner { + if (value == null || typeof value !== 'object') return false; + if (!('path' in value) || (value as any)['path'] === undefined) return false; + if (!('type' in value) || (value as any)['type'] === undefined) return false; + return true; +}packages/code-provider/src/providers/coderouter/index.ts (8)
383-389
: Don’t send empty X-Auth-Jwt; omit header when unauthenticatedPrevents confusing 401s with an empty header.
- return { - headers: { - 'content-type': 'application/json', - 'X-Auth-Jwt': this.jwt ?? '', - }, - }; + return { + headers: { + 'content-type': 'application/json', + ...(this.jwt ? { 'X-Auth-Jwt': this.jwt } : {}), + }, + };
360-366
: Wire pause/stop to API endpointsThese are stubs; call the generated endpoints.
async pauseProject(input: PauseProjectInput): Promise<PauseProjectOutput> { - return {}; + await this.api.coderouterApiSandboxPausePost(this.requestInitOverrides()); + return {}; } @@ async stopProject(input: StopProjectInput): Promise<StopProjectOutput> { - return {}; + await this.api.coderouterApiSandboxStopPost(this.requestInitOverrides()); + return {}; }If the endpoints require bodies, I can adjust to the exact request shape.
224-236
: Remove unnecessary any on downloadFilesUse the typed response from the generated client.
- const res: any = await this.api.coderouterApiSandboxFileDownloadPost( + const res = await this.api.coderouterApiSandboxFileDownloadPost(
149-151
: Consider mapping additional stat fields if availablePropagate size/times/symlink when provided by API for richer UX.
- return { - type: res.type, - }; + return { + type: res.type, + size: (res as any).size, + mtime: (res as any).mtime ? new Date((res as any).mtime) : undefined, + ctime: (res as any).ctime ? new Date((res as any).ctime) : undefined, + atime: (res as any).atime ? new Date((res as any).atime) : undefined, + isSymlink: (res as any).isSymlink ?? false, + };
185-191
: Preserve isSymlink from API if presentHardcoding
false
loses information; map through when available.- isSymlink: false, + isSymlink: (file as any).isSymlink ?? false,
89-102
: Fail fast when JWT is absent (or auto-create session)Most methods rely on JWT; consider throwing a clear error or lazily creating one if
getSession
/createSession
isn’t provided.
467-469
: Abort in-flight watch request on stopAvoids dangling requests and ensures the poll loop halts promptly.
async stop(): Promise<void> { this.off = true; + this.abortController?.abort(); + this.abortController = null; }
459-461
: Make poll cadence configurable200ms can be noisy; consider a
pollMs
option with a sane default (e.g., 500–1000ms).apps/coderouter/src/provider/e2b/sandbox/file/index.ts (5)
67-69
: Use POSIX-normalized fullpath consistently (avoid host-OS path.normalize).With a hardened fullpath, extra normalize here is unnecessary and can introduce Windows backslashes during local dev.
Apply this diff:
- const p = path.normalize(this.fullpath(input.path)); + const p = this.fullpath(input.path); const files = await this.client._sandbox.files.list(p);
211-215
: Return watcher event paths relative to the sandbox root for consistency.Other APIs return paths relative to “/code”.
Apply this diff:
- events: events.map((event) => ({ - path: event.path, + events: events.map((event) => ({ + path: event.path.replace(this.folder, ''), type: event.type, })),
266-271
: Exclude filtering: now that excludes are normalized, drop the fragile '' trimming.**Use direct prefix check against absolute paths.
Apply this diff:
- if ( - input.excludePaths.some((exclude) => - event.name.startsWith(exclude.replace('**', '')), - ) - ) { + if (input.excludePaths.some((exclude) => event.name.startsWith(exclude))) { return; }
154-165
: Simplify write loop; input.files is already an array.Removes redundant Array.isArray guard.
Apply this diff:
- for (const file of Array.isArray(input.files) ? input.files : [input.files]) { + for (const file of input.files) { if (!file.overwrite) { const exists = await this.client._sandbox.files.exists(this.fullpath(file.path)); if (exists) { continue; } } files.push({ path: this.fullpath(file.path), data: file.data, }); }
58-86
: Reduce repetition: extract a small helper to ensure sandbox is instantiated.This trims boilerplate and centralizes the error message.
Example (apply across methods):
private ensureSandbox(): NonNullable<E2BClient["_sandbox"]> { const sb = this.client._sandbox; if (!sb) { throw new ClientError( ClientErrorCode.ImplementationError, 'Sandbox is not instantiated. Call start() or resume() first.', false ); } return sb; }Then replace occurrences of
if (!this.client._sandbox) { ... }
withconst sb = this.ensureSandbox();
and usesb
thereafter.Also applies to: 88-111, 113-143, 145-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/coderouter/src/api/sandbox/file/watch/index.test.ts
(1 hunks)apps/coderouter/src/api/sandbox/file/watch/index.ts
(1 hunks)apps/coderouter/src/provider/definition/sandbox/file/index.ts
(1 hunks)apps/coderouter/src/provider/e2b/sandbox/file/index.ts
(1 hunks)apps/coderouter/src/server.ts
(1 hunks)apps/web/client/src/components/store/editor/sandbox/index.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/.openapi-generator/FILES
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200ResponseEventsInner.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/index.ts
(1 hunks)packages/code-provider/src/providers/coderouter/index.ts
(1 hunks)packages/code-provider/src/providers/codesandbox/utils/utils.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- packages/code-provider/src/providers/codesandbox/utils/utils.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/client/src/components/store/editor/sandbox/index.ts
- apps/coderouter/src/provider/definition/sandbox/file/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/code-provider/src/providers/coderouter/codegen/.openapi-generator/FILES
- packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts
🧰 Additional context used
🧬 Code graph analysis (6)
apps/coderouter/src/api/sandbox/file/watch/index.test.ts (2)
apps/coderouter/src/api/sandbox/file/write/index.ts (1)
api_sandbox_file_write
(57-63)apps/web/client/src/env.ts (1)
env
(4-153)
apps/coderouter/src/api/sandbox/file/watch/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/file/index.ts (2)
SandboxFileWatchInput
(54-58)SandboxFileWatchOutput
(59-64)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(33-33)
apps/coderouter/src/server.ts (21)
apps/coderouter/src/util/auth.ts (1)
AuthJwtPayload
(16-19)apps/coderouter/src/middleware/error.ts (1)
setupErrorMiddleware
(6-27)apps/coderouter/src/middleware/auth.ts (1)
setupAuthJwtMiddleware
(4-23)apps/coderouter/src/middleware/requireSandboxId.ts (1)
setupRequiredSandboxIdMiddleware
(3-16)apps/coderouter/src/middleware/client.ts (1)
setupClientMiddleware
(5-29)apps/coderouter/src/middleware/beforeSandboxCall.ts (1)
setupBeforeSandboxCallMiddleware
(3-15)apps/coderouter/src/api/auth/sign/index.ts (1)
api_auth_sign
(64-81)apps/coderouter/src/api/sandbox/create/index.ts (1)
api_sandbox_create
(53-76)apps/coderouter/src/api/sandbox/pause/index.ts (1)
api_sandbox_pause
(43-54)apps/coderouter/src/api/sandbox/resume/index.ts (1)
api_sandbox_resume
(43-54)apps/coderouter/src/api/sandbox/stop/index.ts (1)
api_sandbox_stop
(43-54)apps/coderouter/src/api/sandbox/url/index.ts (1)
api_sandbox_url
(43-54)apps/coderouter/src/api/sandbox/file/copy/index.ts (1)
api_sandbox_file_copy
(58-64)apps/coderouter/src/api/sandbox/file/delete/index.ts (1)
api_sandbox_file_delete
(45-51)apps/coderouter/src/api/sandbox/file/download/index.ts (1)
api_sandbox_file_download
(45-51)apps/coderouter/src/api/sandbox/file/list/index.ts (1)
api_sandbox_file_list
(53-59)apps/coderouter/src/api/sandbox/file/read/index.ts (1)
api_sandbox_file_read
(47-53)apps/coderouter/src/api/sandbox/file/rename/index.ts (1)
api_sandbox_file_rename
(49-55)apps/coderouter/src/api/sandbox/file/stat/index.ts (1)
api_sandbox_file_stat
(47-53)apps/coderouter/src/api/sandbox/file/watch/index.ts (1)
api_sandbox_file_watch
(57-63)apps/coderouter/src/api/sandbox/file/write/index.ts (1)
api_sandbox_file_write
(57-63)
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200Response.ts (1)
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200ResponseEventsInner.ts (3)
CoderouterApiSandboxFileWatchPost200ResponseEventsInner
(21-34)CoderouterApiSandboxFileWatchPost200ResponseEventsInnerFromJSON
(58-62)CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSON
(77-81)
packages/code-provider/src/providers/coderouter/index.ts (4)
packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts (1)
DefaultApi
(137-918)packages/code-provider/src/providers/coderouter/codegen/runtime.ts (1)
ConfigurationParameters
(17-31)packages/code-provider/src/types.ts (33)
WriteFileInput
(8-14)WriteFileOutput
(15-17)StatFileInput
(19-23)StatFileOutput
(25-34)DeleteFilesInput
(69-74)DeleteFilesOutput
(75-75)ListFilesInput
(44-48)ListFilesOutput
(55-57)ReadFileInput
(59-63)ReadFileOutput
(65-67)DownloadFilesInput
(77-81)DownloadFilesOutput
(82-84)CopyFilesInput
(86-93)CopyFileOutput
(94-94)WatchFilesInput
(101-108)WatchFilesOutput
(109-111)CreateTerminalInput
(113-113)CreateTerminalOutput
(114-116)GetTaskInput
(118-122)GetTaskOutput
(123-125)TerminalCommandInput
(127-131)TerminalCommandOutput
(132-135)TerminalBackgroundCommandInput
(137-141)TerminalBackgroundCommandOutput
(142-144)CreateSessionInput
(195-199)CreateSessionOutput
(200-202)CreateProjectInput
(158-166)CreateProjectOutput
(167-169)ListProjectsInput
(177-177)ListProjectsOutput
(178-186)GetProjectUrlInput
(188-190)GetProjectUrlOutput
(191-193)WatchEvent
(96-99)apps/web/client/src/trpc/react.tsx (1)
api
(23-23)
apps/coderouter/src/provider/e2b/sandbox/file/index.ts (3)
apps/coderouter/src/provider/e2b/index.ts (1)
E2BClient
(5-16)apps/coderouter/src/provider/definition/sandbox/file/index.ts (18)
SandboxFileCopyInput
(3-3)SandboxFileCopyOutput
(4-4)SandboxFileDeleteInput
(6-8)SandboxFileDeleteOutput
(9-9)SandboxFileDownloadInput
(11-11)SandboxFileDownloadOutput
(12-12)SandboxFileListInput
(14-16)SandboxFileListOutput
(17-23)SandboxFileReadInput
(25-27)SandboxFileReadOutput
(28-30)SandboxFileRenameInput
(32-35)SandboxFileRenameOutput
(36-36)SandboxFileStatInput
(38-40)SandboxFileStatOutput
(41-43)SandboxFileWriteInput
(45-51)SandboxFileWriteOutput
(52-52)SandboxFileWatchInput
(54-58)SandboxFileWatchOutput
(59-64)apps/coderouter/src/provider/definition/index.ts (1)
ClientError
(32-48)
🔇 Additional comments (4)
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200ResponseEventsInner.ts (2)
39-46
: Enum shape looks good.
as const
yields a precise union fortype
; matches expected event kinds.
77-81
: Approve ToJSON wrapper signature fix.Verified that there are no internal callers of
CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSON
in the repository (only the function definition itself was found), so updating its signature is safe and won’t break existing code.• File:
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxFileWatchPost200ResponseEventsInner.ts
• Lines: 77–81Proposed diff:
-export function CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSON( - json: any, -): CoderouterApiSandboxFileWatchPost200ResponseEventsInner { - return CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSONTyped(json, false); -} +export function CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSON( + value?: CoderouterApiSandboxFileWatchPost200ResponseEventsInner | null, +): any { + return CoderouterApiSandboxFileWatchPost200ResponseEventsInnerToJSONTyped(value, false); +}packages/code-provider/src/providers/coderouter/codegen/models/index.ts (1)
3-24
: Barrel exports are comprehensive and consistent.All related models (including the new FileWatch inner type) are exported from a single entry point. No issues spotted.
packages/code-provider/src/providers/coderouter/index.ts (1)
372-375
: No request body required for coderouterApiSandboxUrlPostThe generated client’s
coderouterApiSandboxUrlPost
method signature inpackages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts
only accepts an optionalinitOverrides
parameter and returns aCoderouterApiSandboxUrlPost200Response
. Therefore, calling it with justthis.requestInitOverrides()
is correct—no request body is needed.
@@ -0,0 +1,18 @@ | |||
import { describe, expect, it } from 'bun:test'; | |||
import { Hono } from 'hono'; | |||
import { api_sandbox_file_write } 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.
Wrong endpoint tested + missing env import + no client stub + missing request body (schema-required).
This test file is under "watch" but imports/calls "write", lacks env import, doesn’t stub c.get('client'), and doesn’t send the required JSON body. Fix as below.
-import { Hono } from 'hono';
-import { api_sandbox_file_write } from './index';
+import { Hono } from 'hono';
+import { env } from 'bun';
+import { api_sandbox_file_watch } from './index';
-describe('sandbox files write endpoints', () => {
- it('POST /coderouter/api/sandbox/file/write returns empty object', async () => {
+describe('sandbox files watch endpoints', () => {
+ it('POST /coderouter/api/sandbox/file/watch returns events array', async () => {
const app = new Hono();
- api_sandbox_file_write(app);
+ api_sandbox_file_watch(app);
+ // stub provider client expected by the route handler
+ app.use('*', async (c, next) => {
+ c.set('client', {
+ sandbox: { file: { watch: async () => ({ events: [] }) } },
+ } as any);
+ return next();
+ });
- const response = await app.request(env.URL_PATH_PREFIX + '/api/sandbox/file/write', {
- method: 'POST',
- });
+ const response = await app.request((env.URL_PATH_PREFIX ?? '') + '/api/sandbox/file/watch', {
+ method: 'POST',
+ headers: { 'content-type': 'application/json' },
+ body: JSON.stringify({ path: '/tmp', recursive: false, excludePaths: [] }),
+ });
expect(response.status).toBe(200);
const body = await response.json();
- expect(body).toEqual({});
+ expect(body).toEqual({ events: [] });
});
});
Also applies to: 5-6, 8-8, 10-12, 14-16
🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/file/watch/index.test.ts around lines 3 (also
applies to lines 5-6, 8, 10-12, 14-16), the test imports and calls the wrong
endpoint (write) and is missing required setup: import of the env, a stub for
c.get('client'), and the JSON request body required by the route schema. Change
the import to api_sandbox_file_watch, add the env import used by the tested
handler, stub c.get('client') to return a minimal client mock before invoking
the handler, and include a valid JSON body matching the route schema when
calling the handler; ensure assertions reflect the watch endpoint behavior
instead of write.
async delete(input: SandboxFileDeleteInput): Promise<SandboxFileDeleteOutput> { | ||
if (!this.client._sandbox) { | ||
throw new ClientError( | ||
ClientErrorCode.ImplementationError, | ||
'Sandbox is not instantiated. Call start() or resume() first.', | ||
false, | ||
); | ||
} | ||
await this.client._sandbox.files.remove(this.fullpath(input.path)); | ||
return {}; | ||
} |
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.
Map NotFoundError on delete and guard against deleting the sandbox root.
Return FileNotFound for missing targets and refuse to delete “/code”.
Apply this diff:
- await this.client._sandbox.files.remove(this.fullpath(input.path));
- return {};
+ const target = this.fullpath(input.path);
+ if (target === this.folder) {
+ throw new ClientError(
+ ClientErrorCode.ImplementationError,
+ 'Refusing to delete sandbox root',
+ false,
+ );
+ }
+ try {
+ await this.client._sandbox.files.remove(target);
+ return {};
+ } catch (e) {
+ if (e instanceof NotFoundError) {
+ throw new ClientError(ClientErrorCode.FileNotFound, 'File not found', false);
+ }
+ throw e;
+ }
📝 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.
async delete(input: SandboxFileDeleteInput): Promise<SandboxFileDeleteOutput> { | |
if (!this.client._sandbox) { | |
throw new ClientError( | |
ClientErrorCode.ImplementationError, | |
'Sandbox is not instantiated. Call start() or resume() first.', | |
false, | |
); | |
} | |
await this.client._sandbox.files.remove(this.fullpath(input.path)); | |
return {}; | |
} | |
async delete(input: SandboxFileDeleteInput): Promise<SandboxFileDeleteOutput> { | |
if (!this.client._sandbox) { | |
throw new ClientError( | |
ClientErrorCode.ImplementationError, | |
'Sandbox is not instantiated. Call start() or resume() first.', | |
false, | |
); | |
} | |
const target = this.fullpath(input.path); | |
if (target === this.folder) { | |
throw new ClientError( | |
ClientErrorCode.ImplementationError, | |
'Refusing to delete sandbox root', | |
false, | |
); | |
} | |
try { | |
await this.client._sandbox.files.remove(target); | |
return {}; | |
} catch (e) { | |
if (e instanceof NotFoundError) { | |
throw new ClientError( | |
ClientErrorCode.FileNotFound, | |
'File not found', | |
false, | |
); | |
} | |
throw e; | |
} | |
} |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 42-52,
add two guards: first, before calling files.remove, normalize/resolve the
input.path and if it equals the sandbox root string "/code" throw a ClientError
refusing to delete the sandbox root; second, wrap the await
this.client._sandbox.files.remove(...) in a try/catch that maps the storage
NotFoundError to a ClientError with ClientErrorCode.FileNotFound (preserving a
clear message), rethrowing other errors unchanged; keep the existing check that
this.client._sandbox exists and return {} on success.
await this.client._sandbox.files.rename( | ||
this.fullpath(input.oldPath), | ||
this.fullpath(input.newPath), | ||
); | ||
return {}; | ||
} |
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
Map NotFoundError on rename for consistent error semantics.
Surface FileNotFound when either path is missing.
Apply this diff:
- await this.client._sandbox.files.rename(
- this.fullpath(input.oldPath),
- this.fullpath(input.newPath),
- );
- return {};
+ const oldP = this.fullpath(input.oldPath);
+ const newP = this.fullpath(input.newPath);
+ try {
+ await this.client._sandbox.files.rename(oldP, newP);
+ return {};
+ } catch (e) {
+ if (e instanceof NotFoundError) {
+ throw new ClientError(ClientErrorCode.FileNotFound, 'File not found', false);
+ }
+ throw e;
+ }
📝 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.
await this.client._sandbox.files.rename( | |
this.fullpath(input.oldPath), | |
this.fullpath(input.newPath), | |
); | |
return {}; | |
} | |
const oldP = this.fullpath(input.oldPath); | |
const newP = this.fullpath(input.newPath); | |
try { | |
await this.client._sandbox.files.rename(oldP, newP); | |
return {}; | |
} catch (e) { | |
if (e instanceof NotFoundError) { | |
throw new ClientError( | |
ClientErrorCode.FileNotFound, | |
'File not found', | |
false | |
); | |
} | |
throw e; | |
} |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 121-126,
the rename call can throw a NotFoundError from the sandbox client but the code
currently returns {} without translating that to the repository's FileNotFound
semantics; wrap the await this.client._sandbox.files.rename(...) in a try/catch,
detect the client's NotFoundError (by error.name or error.code as used
elsewhere), and re-throw or return the project's FileNotFound error when either
the oldPath or newPath is missing; otherwise re-throw the original error for
other cases.
if (!watchers.has(sandboxId)) { | ||
watchers.set(sandboxId, new Watcher(this.client)); | ||
} | ||
|
||
const watcher = watchers.get(sandboxId); | ||
|
||
// obviously this should never happen |
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 watcher leaks across sandbox lifecycles (keyed Map never cleaned).
Store sandboxId in Watcher and remove it from the Map on stop.
Apply this diff:
- if (!watchers.has(sandboxId)) {
- watchers.set(sandboxId, new Watcher(this.client));
- }
+ if (!watchers.has(sandboxId)) {
+ watchers.set(sandboxId, new Watcher(this.client, sandboxId));
+ }
And update the Watcher constructor (see separate diff).
📝 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.
if (!watchers.has(sandboxId)) { | |
watchers.set(sandboxId, new Watcher(this.client)); | |
} | |
const watcher = watchers.get(sandboxId); | |
// obviously this should never happen | |
if (!watchers.has(sandboxId)) { | |
watchers.set(sandboxId, new Watcher(this.client, sandboxId)); | |
} | |
const watcher = watchers.get(sandboxId); | |
// obviously this should never happen |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 189-195,
the watchers Map is never cleaned up which can leak Watcher instances across
sandbox lifecycles; modify Watcher to accept and store the sandboxId in its
constructor, and when Watcher.stop() (or equivalent shutdown path) runs it must
call watchers.delete(this.sandboxId) to remove itself from the Map; update the
code that creates the Watcher to pass the sandboxId into the constructor and
ensure any async/exceptional stop code also performs the Map deletion so the
keyed Map is cleaned when a sandbox stops.
await watcher.start({ | ||
path: this.fullpath(input.path), | ||
recursive: input.recursive, | ||
excludePaths: input.excludePaths, | ||
}); | ||
} |
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
Normalize excludePaths to absolute sandbox paths before starting watcher.
Relative excludes won’t match event names reliably.
Apply this diff:
- if (watcher.off) {
- await watcher.start({
- path: this.fullpath(input.path),
- recursive: input.recursive,
- excludePaths: input.excludePaths,
- });
- }
+ if (watcher.off) {
+ const excludes = input.excludePaths.map((p) => this.fullpath(p));
+ await watcher.start({
+ path: this.fullpath(input.path),
+ recursive: input.recursive,
+ excludePaths: excludes,
+ });
+ }
📝 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.
await watcher.start({ | |
path: this.fullpath(input.path), | |
recursive: input.recursive, | |
excludePaths: input.excludePaths, | |
}); | |
} | |
if (watcher.off) { | |
const excludes = input.excludePaths.map((p) => this.fullpath(p)); | |
await watcher.start({ | |
path: this.fullpath(input.path), | |
recursive: input.recursive, | |
excludePaths: excludes, | |
}); | |
} |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 201 to
206, the watcher is started with input.excludePaths which may contain relative
paths and thus won't reliably match event names; before calling watcher.start
map/normalize excludePaths to absolute sandbox paths using this.fullpath for
each non-empty entry (handle undefined/null), and pass the resulting array to
watcher.start (do not mutate the original input object unless intended) so the
watcher receives absolute paths that will match events correctly.
// avoid memory leak | ||
this.events = this.events.slice(0, this.maxEvents - 1); | ||
|
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 event capping logic (off-by-one and wrong end of buffer).
Keep the newest maxEvents entries.
Apply this diff:
- // avoid memory leak
- this.events = this.events.slice(0, this.maxEvents - 1);
+ // cap buffer to latest `maxEvents`
+ if (this.events.length > this.maxEvents) {
+ this.events = this.events.slice(-this.maxEvents);
+ }
📝 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.
// avoid memory leak | |
this.events = this.events.slice(0, this.maxEvents - 1); | |
// cap buffer to latest `maxEvents` | |
if (this.events.length > this.maxEvents) { | |
this.events = this.events.slice(-this.maxEvents); | |
} |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 283-285,
the current capping uses this.events.slice(0, this.maxEvents - 1) which drops
the newest entry and slices the wrong end; change it to keep the newest
maxEvents entries by taking the last N items (e.g. replace with a slice that
returns the tail: this.events = this.maxEvents > 0 ?
this.events.slice(-this.maxEvents) : []; this ensures no off-by-one and handles
non-positive maxEvents by clearing the buffer).
async stop(): Promise<void> { | ||
if (this._watchHandle) { | ||
await this._watchHandle.stop(); | ||
this._watchHandle = null; | ||
} | ||
|
||
this._off = true; | ||
this.events = []; | ||
} | ||
} |
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
Ensure watcher Map is cleaned on stop.
Prevents dangling watcher instances and memory leaks.
Apply this diff:
async stop(): Promise<void> {
if (this._watchHandle) {
await this._watchHandle.stop();
this._watchHandle = null;
}
this._off = true;
this.events = [];
+ // remove from global registry
+ watchers.delete(this.sandboxId);
}
📝 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.
async stop(): Promise<void> { | |
if (this._watchHandle) { | |
await this._watchHandle.stop(); | |
this._watchHandle = null; | |
} | |
this._off = true; | |
this.events = []; | |
} | |
} | |
async stop(): Promise<void> { | |
if (this._watchHandle) { | |
await this._watchHandle.stop(); | |
this._watchHandle = null; | |
} | |
this._off = true; | |
this.events = []; | |
// remove from global registry | |
watchers.delete(this.sandboxId); | |
} | |
} |
🤖 Prompt for AI Agents
In apps/coderouter/src/provider/e2b/sandbox/file/index.ts around lines 343 to
352, the stop() method currently stops the watch handle and clears events but
doesn't clear the Map that stores watcher instances, leaving dangling watchers
and a memory leak; update stop() to also clear the watcher Map (e.g., if the Map
is this._watchers or this.watchers, call its clear() method or reassign it to a
new Map()/null) after stopping the handle and before returning, ensuring no
leftover references remain.
data: | ||
input.args.content instanceof Uint8Array | ||
? Buffer.from(input.args.content).toString('utf-8') | ||
: input.args.content, | ||
overwrite: input.args.overwrite ?? true, |
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 Buffer in browser; use TextDecoder fallback to prevent runtime errors
Buffer
is Node-only; this will break in the web client. Decode Uint8Array
with TextDecoder
, falling back to Buffer
if available.
- data:
- input.args.content instanceof Uint8Array
- ? Buffer.from(input.args.content).toString('utf-8')
- : input.args.content,
+ data:
+ typeof input.args.content === 'string'
+ ? input.args.content
+ : (typeof TextDecoder !== 'undefined'
+ ? new TextDecoder().decode(input.args.content)
+ : typeof (globalThis as any).Buffer !== 'undefined'
+ ? (globalThis as any).Buffer.from(input.args.content).toString('utf-8')
+ : (() => { throw new Error('[coderouter] cannot decode Uint8Array: missing TextDecoder/Buffer'); })()),
📝 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.
data: | |
input.args.content instanceof Uint8Array | |
? Buffer.from(input.args.content).toString('utf-8') | |
: input.args.content, | |
overwrite: input.args.overwrite ?? true, | |
// packages/code-provider/src/providers/coderouter/index.ts | |
data: | |
typeof input.args.content === 'string' | |
? input.args.content | |
: (typeof TextDecoder !== 'undefined' | |
? new TextDecoder().decode(input.args.content) | |
: typeof (globalThis as any).Buffer !== 'undefined' | |
? (globalThis as any).Buffer.from(input.args.content).toString('utf-8') | |
: (() => { throw new Error('[coderouter] cannot decode Uint8Array: missing TextDecoder/Buffer'); })()), | |
overwrite: input.args.overwrite ?? true, |
🤖 Prompt for AI Agents
In packages/code-provider/src/providers/coderouter/index.ts around lines 111 to
115, the code uses Node's Buffer to decode a Uint8Array which breaks in browser
environments; replace the Buffer usage with TextDecoder (e.g. new
TextDecoder('utf-8').decode(input.args.content)) and only fall back to
Buffer.from(...).toString('utf-8') when TextDecoder is not available (or when
running in Node), ensuring the decoded value is a string; keep the overwrite
line as-is.
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> { | ||
// called in the backend | ||
const res = await this.api.coderouterApiAuthSignPost( | ||
{ | ||
coderouterApiAuthSignPostRequest: { | ||
sandboxId: this.options.sandboxId, | ||
userId: this.options.userId, | ||
}, | ||
}, | ||
{ | ||
headers: { | ||
'content-type': 'application/json', | ||
authorization: `Bearer ${process?.env?.CODEROUTER_API_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.
💡 Verification agent
🧩 Analysis chain
Guard server-only createSession to avoid leaking CODEROUTER_API_KEY
Ensure this never runs in the browser and use canonical header casing.
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> {
- // called in the backend
+ // server-only: contains secret usage
+ if (typeof window !== 'undefined') {
+ throw new Error('createSession must run on the server (uses CODEROUTER_API_KEY).');
+ }
const res = await this.api.coderouterApiAuthSignPost(
@@
- headers: {
- 'content-type': 'application/json',
- authorization: `Bearer ${process?.env?.CODEROUTER_API_KEY}`,
- },
+ headers: {
+ 'content-type': 'application/json',
+ Authorization: `Bearer ${process?.env?.CODEROUTER_API_KEY}`,
+ },
},
);
Run to verify no client imports call this:
🏁 Script executed:
#!/bin/bash
# Ensure createSession isn't referenced from the web client
rg -nP -C2 '\bcreateSession\s*\(' apps/web/client
# Check for direct provider usage in client
rg -nP -C2 '\bnew\s+CoderouterProvider\s*\(' apps/web/client
Length of output: 1251
Add server-only guard in createSession and fix header casing
Verified that createSession
is only invoked within server API routers (apps/web/client/src/server/api/routers/project/sandbox.ts
) and not in any browser-executed code (script output above). To prevent accidental client usage—since it relies on the secret CODEROUTER_API_KEY
—and to use canonical header casing, please apply the following changes:
packages/code-provider/src/providers/coderouter/index.ts
@@ -310,6 +310,10 @@
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> {
+ // server-only: contains secret usage
+ if (typeof window !== 'undefined') {
+ throw new Error('createSession must run on the server (uses CODEROUTER_API_KEY).');
+ }
const res = await this.api.coderouterApiAuthSignPost(
{
coderouterApiAuthSignPostRequest: {
@@ -318,7 +322,7 @@
sandboxId: this.options.sandboxId,
userId: this.options.userId,
},
},
- { headers: { 'content-type': 'application/json', authorization: `Bearer ${process?.env?.CODEROUTER_API_KEY}` } },
+ { headers: { 'content-type': 'application/json', Authorization: `Bearer ${process.env.CODEROUTER_API_KEY}` } },
},
);
📝 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.
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> { | |
// called in the backend | |
const res = await this.api.coderouterApiAuthSignPost( | |
{ | |
coderouterApiAuthSignPostRequest: { | |
sandboxId: this.options.sandboxId, | |
userId: this.options.userId, | |
}, | |
}, | |
{ | |
headers: { | |
'content-type': 'application/json', | |
authorization: `Bearer ${process?.env?.CODEROUTER_API_KEY}`, | |
}, | |
}, | |
); | |
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> { | |
// called in the backend | |
// server-only: contains secret usage | |
if (typeof window !== 'undefined') { | |
throw new Error('createSession must run on the server (uses CODEROUTER_API_KEY).'); | |
} | |
const res = await this.api.coderouterApiAuthSignPost( | |
{ | |
coderouterApiAuthSignPostRequest: { | |
sandboxId: this.options.sandboxId, | |
userId: this.options.userId, | |
}, | |
}, | |
{ | |
headers: { | |
'content-type': 'application/json', | |
Authorization: `Bearer ${process.env.CODEROUTER_API_KEY}`, | |
}, | |
}, | |
); |
🤖 Prompt for AI Agents
In packages/code-provider/src/providers/coderouter/index.ts around lines
310-325, add a server-only guard and fix header casing: first assert the call
runs on the server (e.g., if (typeof window !== 'undefined') throw new
Error('createSession must run on the server') or check for missing
CODEROUTER_API_KEY and throw), then use canonical HTTP header names
('Content-Type' and 'Authorization') and read the secret only from process.env
(after the server guard) to avoid leaking or referencing process.env in browser
builds; ensure you throw a clear error when the API key is absent.
return { | ||
id: input.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.
Return the server-created sandbox id (not input.id)
Returning input.id
desynchronizes client from the actual sandbox id.
- return {
- id: input.id,
- };
+ return {
+ id: (res as { id?: string })?.id ?? input.id,
+ };
📝 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.
return { | |
id: input.id, | |
}; | |
return { | |
id: (res as { id?: string })?.id ?? input.id, | |
}; |
🤖 Prompt for AI Agents
In packages/code-provider/src/providers/coderouter/index.ts around lines 355 to
357, the function currently returns id: input.id which desynchronizes the client
from the server-created sandbox id; change the returned id to the actual id
produced by the server (e.g., id: sandbox.id or id: createdSandbox.id depending
on the local variable used to hold the server response), ensure the variable
name matches the one assigned when the server response was received, and update
any return type/interface if necessary so the client receives the
server-generated sandbox id instead of input.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.
Actionable comments posted: 22
♻️ Duplicate comments (4)
apps/coderouter/src/provider/e2b/sandbox/index.ts (1)
34-34
: Pass API key toconnect
(repeat).Ensure per-client creds; avoid relying on process env.
- this.client._sandbox = await _E2BSandbox.connect(e2bSandboxId); + this.client._sandbox = await _E2BSandbox.connect(e2bSandboxId, { + apiKey: this.client.apiKey, + });packages/code-provider/src/providers/coderouter/index.ts (3)
114-116
: Buffer breaks in browsers; decode Uint8Array safelyUsing Node Buffer here throws in the web client. Use TextDecoder with a Buffer fallback, returning strings only.
- data: - input.args.content instanceof Uint8Array - ? Buffer.from(input.args.content).toString('utf-8') - : input.args.content, + data: + typeof input.args.content === 'string' + ? input.args.content + : (typeof TextDecoder !== 'undefined' + ? new TextDecoder().decode(input.args.content) + : typeof (globalThis as any).Buffer !== 'undefined' + ? (globalThis as any).Buffer.from(input.args.content).toString('utf-8') + : (() => { throw new Error('[coderouter] cannot decode Uint8Array: missing TextDecoder/Buffer'); })()),
314-329
: Server-only guard + canonical header casing + missing key checkPrevent client-side execution (secrets), use proper header casing, and fail fast when the API key is absent.
async createSession(input: CreateSessionInput): Promise<CreateSessionOutput> { - // called in the backend + // server-only: contains secret usage + if (typeof window !== 'undefined') { + throw new Error('createSession must run on the server (uses CODEROUTER_API_KEY).'); + } + if (!process.env.CODEROUTER_API_KEY) { + throw new Error('Missing CODEROUTER_API_KEY.'); + } const res = await this.api.coderouterApiAuthSignPost( @@ - { - headers: { - 'content-type': 'application/json', - authorization: `Bearer ${process?.env?.CODEROUTER_API_KEY}`, - }, - }, + { + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${process.env.CODEROUTER_API_KEY}`, + }, + }, );
359-361
: Return server-created sandbox idAvoid client/server ID desync by using the API response.
- return { - id: input.id, - }; + return { + id: (res as { id?: string })?.id ?? input.id, + };
🧹 Nitpick comments (36)
apps/web/client/src/components/store/editor/sandbox/index.ts (1)
121-123
: Remove or gate the ad‑hoc debug logUse consistent logger and guard for dev to avoid noisy prod logs.
- console.log('----> watch files called?'); + if (isDev) console.debug('[SandboxManager] watchFiles() invoked');apps/coderouter/src/api/sandbox/terminal/kill/index.ts (1)
24-28
: Add 404 handling and normalize URL_PATH_PREFIX.
- Normalize the prefix to avoid accidental double slashes when env.URL_PATH_PREFIX ends with '/'.
- Document and map "not found" to 404. Without this, client errors are indistinguishable from 5xx.
+const PREFIX = (env.URL_PATH_PREFIX || '').replace(/\/+$/, ''); + const route = createRoute({ method: 'post', - path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/kill', + path: PREFIX + '/api/sandbox/terminal/kill', security: [{ jwt: [] }], request: { @@ responses: { @@ description: 'Kill a terminal in the sandbox.', }, + 404: { + content: { + 'application/json': { schema: z.object({ error: z.string() }) }, + }, + description: 'Terminal not found.', + }, ...JwtAuthResponses, }, }); export function api_sandbox_terminal_kill(app: LocalHono) { app.openapi(route, async (c) => { - const body = await c.req.valid('json'); - const result = await c.get('client').sandbox.terminal.kill(body); - return c.json(result, 200); + const body = await c.req.valid('json'); + try { + const result = await c.get('client').sandbox.terminal.kill(body); + return c.json(result, 200); + } catch (err: any) { + if (err?.status === 404 || err?.code === 'NOT_FOUND') { + return c.json({ error: err?.message ?? 'Terminal not found' }, 404); + } + throw err; + } }); }Also applies to: 50-55
apps/coderouter/src/api/sandbox/terminal/write/index.ts (2)
47-48
: Fix wording in route description.Say “Write to a terminal” (preposition missing).
- description: 'Write a terminal in the sandbox.', + description: 'Write to a terminal in the sandbox.',
15-19
: Validate input, add 404 handling, and normalize URL_PATH_PREFIX.
- Guard against empty writes with .min(1).
- Normalize prefix to avoid double slashes.
- Document and map 404.
input: z.string().openapi({ - description: 'The input to the write command.', + description: 'The input to the write command.', example: 'ls -la', - }), + }).min(1), @@ -const route = createRoute({ +const PREFIX = (env.URL_PATH_PREFIX || '').replace(/\/+$/, ''); +const route = createRoute({ method: 'post', - path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/write', + path: PREFIX + '/api/sandbox/terminal/write', @@ responses: { @@ - ...JwtAuthResponses, + ...JwtAuthResponses, + 404: { + content: { + 'application/json': { schema: z.object({ error: z.string() }) }, + }, + description: 'Terminal not found.', + }, }, }); @@ export function api_sandbox_terminal_write(app: LocalHono) { app.openapi(route, async (c) => { const body = await c.req.valid('json'); - const result = await c.get('client').sandbox.terminal.write(body); - return c.json(result, 200); + try { + const result = await c.get('client').sandbox.terminal.write(body); + return c.json(result, 200); + } catch (err: any) { + if (err?.status === 404 || err?.code === 'NOT_FOUND') { + return c.json({ error: err?.message ?? 'Terminal not found' }, 404); + } + throw err; + } }); }Also applies to: 27-31, 40-50, 53-58
packages/code-provider/package.json (2)
18-20
: Harden codegen workflow: pre-clean and validate spec.
- Add a pre-step to avoid stale code in ./codegen.
- Validate the OpenAPI spec rather than skipping validation.
"scripts": { @@ - "typecheck": "tsc --noEmit", - "generate-coderouter": "openapi-generator-cli generate -i http://localhost:4444/openapi.json -g typescript-fetch -o ./src/providers/coderouter/codegen --skip-validate-spec" + "typecheck": "tsc --noEmit", + "pre-generate-coderouter": "rm -rf ./src/providers/coderouter/codegen", + "validate-openapi": "openapi-generator-cli validate -i http://localhost:4444/openapi.json", + "generate-coderouter": "openapi-generator-cli generate -i http://localhost:4444/openapi.json -g typescript-fetch -o ./src/providers/coderouter/codegen" },
41-41
: Move prettier to devDependencies.Prettier is a dev tool; keeping it in runtime deps bloats consumer installs.
apps/coderouter/src/api/sandbox/terminal/command/index.ts (2)
10-15
: Validate non-empty command.Empty commands should be rejected early.
- command: z.string().openapi({ + command: z.string().min(1).openapi({ description: 'The command to run.', example: 'ls -la', }),
40-44
: Normalize URL_PATH_PREFIX and add 404 mapping.Path join safety and clearer error signaling.
-const route = createRoute({ +const PREFIX = (env.URL_PATH_PREFIX || '').replace(/\/+$/, ''); +const route = createRoute({ method: 'post', - path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/command', + path: PREFIX + '/api/sandbox/terminal/command', @@ responses: { @@ - ...JwtAuthResponses, + ...JwtAuthResponses, + 404: { + content: { + 'application/json': { schema: z.object({ error: z.string() }) }, + }, + description: 'Sandbox resource not found.', + }, }, }); @@ export function api_sandbox_terminal_command(app: LocalHono) { app.openapi(route, async (c) => { const body = await c.req.valid('json'); - const result = await c.get('client').sandbox.terminal.command(body); - return c.json(result, 200); + try { + const result = await c.get('client').sandbox.terminal.command(body); + return c.json(result, 200); + } catch (err: any) { + if (err?.status === 404 || err?.code === 'NOT_FOUND') { + return c.json({ error: err?.message ?? 'Not found' }, 404); + } + throw err; + } }); }Also applies to: 53-64, 66-71
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalWritePostRequest.ts (1)
15-15
: Unused import in generated file.mapValues is unused; harmless in generated code. No action required unless you customize templates.
apps/coderouter/src/api/sandbox/terminal/create/index.ts (1)
32-35
: Normalize URL_PATH_PREFIX.Avoid double slashes when prefix ends with '/'.
-const route = createRoute({ +const PREFIX = (env.URL_PATH_PREFIX || '').replace(/\/+$/, ''); +const route = createRoute({ method: 'post', - path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/create', + path: PREFIX + '/api/sandbox/terminal/create',apps/coderouter/src/api/sandbox/terminal/open/index.ts (3)
55-55
: Avoid logging terminal output (may leak secrets).- console.log(res.output); + // Consider enabling via DEBUG flag if needed: + // if (env.DEBUG_TERMINAL_OUTPUT) console.debug(res.output);
22-22
: Make path resilient when URL_PATH_PREFIX is unset.- path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/open', + path: `${env.URL_PATH_PREFIX ?? ''}/api/sandbox/terminal/open`,
37-38
: Fix description: this is SSE, not long-polling.- description: 'Return the output of a terminal in the sandbox. Design for long-polling', + description: 'Return the output of a terminal in the sandbox via Server-Sent Events (SSE).',packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalWritePost200Response.ts (2)
15-15
: Remove unused import.-import { mapValues } from '../runtime'; +// (no imports)
58-62
: Fix ToJSON signature and parameter naming.The helper should accept a typed value and return JSON, not the other way around.
-export function CoderouterApiSandboxTerminalWritePost200ResponseToJSON( - json: any, -): CoderouterApiSandboxTerminalWritePost200Response { - return CoderouterApiSandboxTerminalWritePost200ResponseToJSONTyped(json, false); -} +export function CoderouterApiSandboxTerminalWritePost200ResponseToJSON( + value: CoderouterApiSandboxTerminalWritePost200Response, +): any { + return CoderouterApiSandboxTerminalWritePost200ResponseToJSONTyped(value, false); +}packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalKillPostRequest.ts (2)
15-15
: Remove unused import.-import { mapValues } from '../runtime'; +// (no imports)
58-62
: Fix ToJSON signature and parameter naming.Align with typical OpenAPI TS generators: accept typed value, return JSON.
-export function CoderouterApiSandboxTerminalKillPostRequestToJSON( - json: any, -): CoderouterApiSandboxTerminalKillPostRequest { - return CoderouterApiSandboxTerminalKillPostRequestToJSONTyped(json, false); -} +export function CoderouterApiSandboxTerminalKillPostRequestToJSON( + value: CoderouterApiSandboxTerminalKillPostRequest, +): any { + return CoderouterApiSandboxTerminalKillPostRequestToJSONTyped(value, false); +}packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalOpenPost200Response.ts (2)
15-15
: Remove unused import.
mapValues
is not used.Apply:
-import { mapValues } from '../runtime';
33-38
: Harden type guard to avoid indexingobject
.Current code may fail under strict TS because
object
lacks an index signature.Apply:
-export function instanceOfCoderouterApiSandboxTerminalOpenPost200Response( - value: object, -): value is CoderouterApiSandboxTerminalOpenPost200Response { - if (!('output' in value) || value['output'] === undefined) return false; - return true; -} +export function instanceOfCoderouterApiSandboxTerminalOpenPost200Response( + value: unknown, +): value is CoderouterApiSandboxTerminalOpenPost200Response { + return !!value && typeof value === 'object' && 'output' in (value as any); +}packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalOpenPostRequest.ts (2)
15-15
: Remove unused import.
mapValues
is not used.Apply:
-import { mapValues } from '../runtime';
33-38
: Fix type guard to avoid unsafe index access onobject
.Safer under strict settings.
-export function instanceOfCoderouterApiSandboxTerminalOpenPostRequest( - value: object, -): value is CoderouterApiSandboxTerminalOpenPostRequest { - if (!('terminalId' in value) || value['terminalId'] === undefined) return false; - return true; -} +export function instanceOfCoderouterApiSandboxTerminalOpenPostRequest( + value: unknown, +): value is CoderouterApiSandboxTerminalOpenPostRequest { + return !!value && typeof value === 'object' && 'terminalId' in (value as any); +}apps/coderouter/src/provider/e2b/sandbox/index.ts (2)
35-37
: Comment vs. behavior mismatch (5 min vs 30 min).Align the comment with the actual 30-minute timeout.
- // bump the timeout to 5 minutes - // this.client._sandbox.setTimeout(1000 * 60 * 5); + // bump the timeout to 30 minutes + // this.client._sandbox.setTimeout(1000 * 60 * 5);
116-126
: Clear local sandbox handle after kill.Avoid stale references post-stop.
await this.client._sandbox.kill(); + this.client._sandbox = undefined; return {};
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalRunPost200Response.ts (2)
15-15
: Remove unused import.
mapValues
is not used.-import { mapValues } from '../runtime';
33-38
: Harden the runtime type guard.Avoid
in
on arbitrary values; also assert the expected primitive type.-export function instanceOfCoderouterApiSandboxTerminalRunPost200Response( - value: object, -): value is CoderouterApiSandboxTerminalRunPost200Response { - if (!('output' in value) || value['output'] === undefined) return false; - return true; -} +export function instanceOfCoderouterApiSandboxTerminalRunPost200Response( + value: unknown, +): value is CoderouterApiSandboxTerminalRunPost200Response { + return typeof value === 'object' && value !== null && typeof (value as any).output === 'string'; +}apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts (4)
256-259
: Ensure cleanup completes before returning from start().Await
stop()
in finally.- } finally { - console.log('stopping terminal watcher'); - this.stop(); - } + } finally { + console.log('stopping terminal watcher'); + await this.stop(); + }
275-281
: Clarify error message for missing sandbox.Message currently repeats “Terminal not initialized.”
- if (!this.client._sandbox) { - throw new ClientError( - ClientErrorCode.ImplementationError, - 'Terminal not initialized. Call start() first.', - false, - ); - } + if (!this.client._sandbox) { + throw new ClientError( + ClientErrorCode.ImplementationError, + 'Sandbox is not instantiated. Call start() or resume() first.', + false, + ); + }
186-190
: Remove empty sandbox maps to prevent leaks.Clean up outer map when the last terminal is removed.
- if (watcher) { - await watcher.stop(); - terminalWatchers.get(this.client._sandbox.sandboxId)?.delete(input.terminalId); - } + if (watcher) { + await watcher.stop(); + const inner = terminalWatchers.get(this.client._sandbox.sandboxId); + inner?.delete(input.terminalId); + if (inner && inner.size === 0) { + terminalWatchers.delete(this.client._sandbox.sandboxId); + } + }
229-239
: Make PTY dimensions/timeouts configurable.Hard-coding cols/rows and
watchTimeout=0
reduces usability; accept these via create/open inputs or config.packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalCommandPostRequest.ts (2)
15-15
: Remove unused import.-import { mapValues } from '../runtime';
33-38
: Harden the runtime type guard.-export function instanceOfCoderouterApiSandboxTerminalCommandPostRequest( - value: object, -): value is CoderouterApiSandboxTerminalCommandPostRequest { - if (!('command' in value) || value['command'] === undefined) return false; - return true; -} +export function instanceOfCoderouterApiSandboxTerminalCommandPostRequest( + value: unknown, +): value is CoderouterApiSandboxTerminalCommandPostRequest { + return typeof value === 'object' && value !== null && typeof (value as any).command === 'string'; +}apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
6-12
: Prefer camelCase for TS DTOs; keep wire names at the boundaryThese fields are snake_case while the rest of the file uses camelCase (e.g., terminalId). Consider camelCase in types and map at I/O boundaries to avoid inconsistent access in TS.
export interface SandboxTerminalCommandOutput { - is_error: boolean; - output: string; - stdout?: string; - stderr?: string; - exit_code?: number; + isError: boolean; + output: string; + stdout?: string; + stderr?: string; + exitCode?: number; }
61-65
: Specify close-contract expectationsDocument whether the returned
close()
must be idempotent and if it also terminates the sandbox-side process or only the client stream. This affects implementers.packages/code-provider/src/providers/coderouter/index.ts (2)
463-465
: Polling interval is aggressive200ms may be wasteful under load. Consider 500–1000ms or making it configurable via provider options.
124-127
: Propagate actual write success/failureCurrently always returns success: true regardless of API outcome. Consider returning based on API result or catching errors.
packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts (1)
196-209
: Token wiring: prefer Configuration.accessToken over manual headersYou’re mixing custom headers (
X-Auth-Jwt
) in the provider with bearer auth here. Decide on one contract (recommended: Authorization: Bearer) and supply tokens viaconfiguration.accessToken
to avoid per-call header drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (31)
apps/coderouter/package.json
(1 hunks)apps/coderouter/src/api/sandbox/terminal/command/index.ts
(1 hunks)apps/coderouter/src/api/sandbox/terminal/create/index.ts
(1 hunks)apps/coderouter/src/api/sandbox/terminal/kill/index.ts
(1 hunks)apps/coderouter/src/api/sandbox/terminal/open/index.ts
(1 hunks)apps/coderouter/src/api/sandbox/terminal/run/index.ts
(1 hunks)apps/coderouter/src/api/sandbox/terminal/write/index.ts
(1 hunks)apps/coderouter/src/provider/definition/sandbox/index.ts
(1 hunks)apps/coderouter/src/provider/definition/sandbox/terminal/index.ts
(1 hunks)apps/coderouter/src/provider/e2b/index.ts
(1 hunks)apps/coderouter/src/provider/e2b/sandbox/index.ts
(1 hunks)apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts
(1 hunks)apps/coderouter/src/server.ts
(1 hunks)apps/nginx/conf.d/server.conf
(1 hunks)apps/web/client/src/components/store/editor/sandbox/index.ts
(2 hunks)packages/code-provider/package.json
(2 hunks)packages/code-provider/src/providers/coderouter/codegen/.openapi-generator/FILES
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalCommandPost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalCommandPostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalCreatePostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalKillPost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalKillPostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalOpenPost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalOpenPostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalRunPost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalRunPostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalWritePost200Response.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalWritePostRequest.ts
(1 hunks)packages/code-provider/src/providers/coderouter/codegen/models/index.ts
(1 hunks)packages/code-provider/src/providers/coderouter/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalRunPostRequest.ts
- packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalKillPost200Response.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/coderouter/src/provider/e2b/index.ts
- apps/coderouter/package.json
- apps/coderouter/src/provider/definition/sandbox/index.ts
- apps/coderouter/src/server.ts
- packages/code-provider/src/providers/coderouter/codegen/.openapi-generator/FILES
- apps/nginx/conf.d/server.conf
🧰 Additional context used
🧬 Code graph analysis (11)
apps/coderouter/src/api/sandbox/terminal/write/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
SandboxTerminalWriteInput
(31-34)SandboxTerminalWriteOutput
(35-37)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)
apps/coderouter/src/api/sandbox/terminal/command/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
SandboxTerminalCommandInput
(3-5)SandboxTerminalCommandOutput
(6-12)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)
apps/coderouter/src/api/sandbox/terminal/create/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
SandboxTerminalCreateInput
(14-17)SandboxTerminalCreateOutput
(18-21)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)
apps/coderouter/src/api/sandbox/terminal/kill/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
SandboxTerminalKillInput
(39-41)SandboxTerminalKillOutput
(42-44)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)
apps/coderouter/src/api/sandbox/terminal/run/index.ts (3)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
SandboxTerminalRunInput
(46-49)SandboxTerminalRunOutput
(50-52)apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)
apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (2)
apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts (1)
onOutput
(262-264)packages/code-provider/src/providers/coderouter/index.ts (3)
onOutput
(621-624)onOutput
(656-658)onOutput
(682-684)
apps/coderouter/src/provider/e2b/sandbox/index.ts (5)
apps/coderouter/src/provider/e2b/index.ts (1)
E2BClient
(5-19)apps/coderouter/src/provider/e2b/sandbox/file/index.ts (1)
E2BSandboxFile
(29-221)apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts (1)
E2BSandboxTerminal
(24-196)apps/coderouter/src/provider/definition/sandbox/index.ts (12)
SandboxCreateInput
(5-12)SandboxCreateOutput
(13-15)SandboxGetInput
(17-20)SandboxGetOutput
(21-24)SandboxPauseInput
(26-29)SandboxPauseOutput
(30-30)SandboxResumeInput
(32-35)SandboxResumeOutput
(36-36)SandboxStopInput
(38-41)SandboxStopOutput
(42-42)SandboxUrlInput
(44-47)SandboxUrlOutput
(48-50)apps/coderouter/src/provider/definition/index.ts (1)
ClientError
(32-48)
apps/coderouter/src/api/sandbox/terminal/open/index.ts (4)
apps/coderouter/src/util/auth.ts (1)
JwtAuthResponses
(5-14)apps/coderouter/src/server.ts (1)
LocalHono
(39-39)apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts (2)
onOutput
(262-264)open
(92-125)apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (1)
SandboxTerminalOpenOutput
(26-29)
apps/coderouter/src/provider/e2b/sandbox/terminal/index.ts (3)
apps/coderouter/src/provider/e2b/index.ts (1)
E2BClient
(5-19)apps/coderouter/src/provider/definition/sandbox/terminal/index.ts (12)
SandboxTerminalCommandInput
(3-5)SandboxTerminalCommandOutput
(6-12)SandboxTerminalCreateInput
(14-17)SandboxTerminalCreateOutput
(18-21)SandboxTerminalOpenInput
(23-25)SandboxTerminalOpenOutput
(26-29)SandboxTerminalWriteInput
(31-34)SandboxTerminalWriteOutput
(35-37)SandboxTerminalRunInput
(46-49)SandboxTerminalRunOutput
(50-52)SandboxTerminalKillInput
(39-41)SandboxTerminalKillOutput
(42-44)apps/coderouter/src/provider/definition/index.ts (1)
ClientError
(32-48)
packages/code-provider/src/providers/coderouter/index.ts (3)
packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts (1)
DefaultApi
(188-1301)packages/code-provider/src/providers/coderouter/codegen/runtime.ts (1)
ConfigurationParameters
(17-31)packages/code-provider/src/types.ts (42)
InitializeInput
(152-152)InitializeOutput
(153-153)WriteFileInput
(8-14)WriteFileOutput
(15-17)RenameFileInput
(36-41)RenameFileOutput
(42-42)StatFileInput
(19-23)StatFileOutput
(25-34)DeleteFilesInput
(69-74)DeleteFilesOutput
(75-75)ListFilesInput
(44-48)ListFilesOutput
(55-57)ReadFileInput
(59-63)ReadFileOutput
(65-67)DownloadFilesInput
(77-81)DownloadFilesOutput
(82-84)CopyFilesInput
(86-93)CopyFileOutput
(94-94)WatchFilesInput
(101-108)WatchFilesOutput
(109-111)CreateTerminalInput
(113-113)CreateTerminalOutput
(114-116)GetTaskInput
(118-122)GetTaskOutput
(123-125)TerminalCommandInput
(127-131)TerminalCommandOutput
(132-135)TerminalBackgroundCommandInput
(137-141)TerminalBackgroundCommandOutput
(142-144)GitStatusInput
(146-146)GitStatusOutput
(148-150)SetupInput
(155-155)SetupOutput
(156-156)CreateSessionInput
(195-199)CreateSessionOutput
(200-202)CreateProjectInput
(158-166)CreateProjectOutput
(167-169)PauseProjectInput
(171-171)StopProjectInput
(174-174)ListProjectsInput
(177-177)GetProjectUrlInput
(188-190)GetProjectUrlOutput
(191-193)WatchEvent
(96-99)
packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts (5)
packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiAuthSignPostRequest.ts (2)
CoderouterApiAuthSignPostRequest
(21-34)CoderouterApiAuthSignPostRequestToJSON
(64-68)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxCreatePostRequest.ts (2)
CoderouterApiSandboxCreatePostRequest
(21-34)CoderouterApiSandboxCreatePostRequestToJSON
(66-70)packages/code-provider/src/providers/coderouter/codegen/runtime.ts (3)
InitOverrideFunction
(344-347)ApiResponse
(455-458)HTTPHeaders
(324-324)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiAuthSignPost200Response.ts (2)
CoderouterApiAuthSignPost200Response
(21-28)CoderouterApiAuthSignPost200ResponseFromJSON
(40-44)packages/code-provider/src/providers/coderouter/codegen/models/CoderouterApiSandboxCreatePost200Response.ts (2)
CoderouterApiSandboxCreatePost200Response
(21-28)CoderouterApiSandboxCreatePost200ResponseFromJSON
(40-44)
🔇 Additional comments (6)
apps/coderouter/src/api/sandbox/terminal/run/index.ts (2)
29-33
: Harden prefix handling and add OpenAPI metadata- path: env.URL_PATH_PREFIX + '/api/sandbox/terminal/run', + path: (env.URL_PATH_PREFIX ?? '').replace(/\/+$/, '') + '/api/sandbox/terminal/run', + summary: 'Run a command in a sandbox terminal session', + operationId: 'sandboxTerminalRun', + tags: ['sandbox','terminal'],Apply the same
.replace(/\/+$/,'')
guard to all otherenv.URL_PATH_PREFIX + …
usages so stray or undefined slashes can’t break any routes.
55-61
: Remove per-route try/catch boilerplate: global error middleware handles exceptions and client injection + JWT auth are already applied.apps/coderouter/src/api/sandbox/terminal/open/index.ts (1)
54-61
: Watcher callback cleanup is already implemented:TerminalWatcher.stop()
resetsthis._onOutputCallbacks
to an empty array, preventing memory‐leak concerns.packages/code-provider/src/providers/coderouter/codegen/models/index.ts (1)
1-33
: LGTM: barrel exports are clear and usefulCentralized model re-exports improve DX; disabling lint is fine for generated code.
packages/code-provider/src/providers/coderouter/index.ts (1)
389-393
: Unify auth header; drop content-type from overrides
Remove'content-type': 'application/json'
fromrequestInitOverrides()
—the generated OpenAPI client already setsContent-Type
on JSON POSTs, and this override pollutes SSE/GET calls. AddAuthorization: Bearer ${this.jwt}
alongsideX-Auth-Jwt
until the server contract is finalized.packages/code-provider/src/providers/coderouter/codegen/apis/DefaultApi.ts (1)
982-1019
: Update OpenAPI spec for/coderouter/api/sandbox/terminal/create
to specify a proper 200 responseThe spec’s response for this operation currently points to the request model (
CoderouterApiSandboxTerminalCreatePostRequest
), so the generated client method returns the request type instead of an actual response (orvoid
). Define an appropriate response schema (orvoid
) in the spec and regenerate the client.
const { close } = await c.get('client').sandbox.terminal.open(query, onOutput); | ||
|
||
let open = true; | ||
stream.onAbort(() => { | ||
close(); | ||
open = false; | ||
}); | ||
|
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
Handle provider.open errors and avoid dangling close calls.
If sandbox.terminal.open()
throws (e.g., terminal missing), the stream ends with a 500 and no structured SSE error. Also, guard close()
if open failed.
- const { close } = await c.get('client').sandbox.terminal.open(query, onOutput);
-
- let open = true;
- stream.onAbort(() => {
- close();
- open = false;
- });
+ let closeFn: (() => void) | null = null;
+ try {
+ const { close } = await c.get('client').sandbox.terminal.open(query, onOutput);
+ closeFn = close;
+ } catch (err) {
+ await stream.writeSSE({
+ id: uuid(),
+ event: 'error',
+ data: JSON.stringify({ error: err instanceof Error ? err.message : String(err) }),
+ });
+ return;
+ }
+
+ let open = true;
+ stream.onAbort(() => {
+ closeFn?.();
+ open = false;
+ });
📝 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 { close } = await c.get('client').sandbox.terminal.open(query, onOutput); | |
let open = true; | |
stream.onAbort(() => { | |
close(); | |
open = false; | |
}); | |
let closeFn: (() => void) | null = null; | |
try { | |
const { close } = await c.get('client').sandbox.terminal.open(query, onOutput); | |
closeFn = close; | |
} catch (err) { | |
await stream.writeSSE({ | |
id: uuid(), | |
event: 'error', | |
data: JSON.stringify({ error: err instanceof Error ? err.message : String(err) }), | |
}); | |
return; | |
} | |
let open = true; | |
stream.onAbort(() => { | |
closeFn?.(); | |
open = false; | |
}); |
🤖 Prompt for AI Agents
In apps/coderouter/src/api/sandbox/terminal/open/index.ts around lines 63 to 70,
wrap the await c.get('client').sandbox.terminal.open(...) call in a try/catch so
provider.open errors are caught; only set open = true after the call succeeds;
in the catch write a structured SSE error to the stream (an error event with
JSON data) and end/close the stream gracefully; keep the existing stream.onAbort
handler but guard close() by checking the open flag so close() is only called
when open is true.
...er/src/providers/coderouter/codegen/models/CoderouterApiSandboxTerminalRunPost200Response.ts
Show resolved
Hide resolved
…upgrading e2b sdk to allow pausing
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