Fix/e2b sdk 2.31 compat - #9
Open
benlavalley wants to merge 2 commits into
Open
Conversation
- bump e2b to ^2.31.0 and refresh deps (zod, hono, chanfana, dockerode, nanoid, vitest, zod-to-openapi); set version 26.5.0 and typescript ^6
- added version definition to package.json - was needed trying to install locally (without it got error with cli/bin/install.js unable to download the release because version was undefined)
- proxy: strip transfer-encoding header and buffer request body via arrayBuffer to avoid streaming/double-encoding issues - required after test failure test/integration/e2b-smoke.spec.ts on line 168 (await sbA.files.write("/tmp/isolated.txt", "sandbox-a”);)
- e2b-smoke: set validateApiKey: false for the integration client - required after e2b commit 4a4bb36839 (broke key validation) and 78c200a (supported validateApiKey: false to skip it)
- Generate API keys with `e2b_` prefix instead of `sk-sandbox-` in serve command - Update tests, docs, and skill guide to the new key format - Drop `validateApiKey: false` from the e2b smoke test now that keys pass SDK validation
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughRenames the generated API key prefix from ChangesAPI Key Rename, Proxy Fix, and Dependency Bumps
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:S
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
135
to
138
| body: | ||
| c.req.method !== "GET" && c.req.method !== "HEAD" | ||
| ? c.req.raw.body | ||
| ? await c.req.raw.arrayBuffer() | ||
| : undefined, |
There was a problem hiding this comment.
Suggestion: Buffering every non-GET/HEAD request body with arrayBuffer() forces the proxy to load the full payload into memory before forwarding, which breaks streaming request behavior and can cause high memory usage or OOMs on large uploads. Forward the original request stream instead of eagerly buffering the entire body. [performance]
Severity Level: Major ⚠️
❌ Large sandbox file uploads may exhaust proxy memory.
⚠️ Envd proxy cannot stream request bodies to sandboxes.
⚠️ All non-GET envd data-plane calls lose streaming.Steps of Reproduction ✅
1. Start the envd proxy server via `cli/src/server/index.ts:1-18`, which calls
`createApp()` and then `Bun.serve({ fetch: envdProxy.fetch })`, wiring all incoming
data-plane HTTP requests through the Hono app.
2. In `cli/src/server/app.ts:8-33`, observe that the app-level middleware routes all
data-plane requests to `handleProxyRequest(c, backends, sandboxId)` based on either the
Host header pattern, the `E2b-Sandbox-Id` header, or the `X-Access-Token` header, so any
non-GET/HEAD envd HTTP request will pass through `handleProxyRequest`.
3. In `cli/src/server/services/proxy.ts:65-140`, see that `handleProxyRequest` forwards
the request with `fetch(targetUrl, { method: c.req.method, headers, body: c.req.method !==
"GET" && c.req.method !== "HEAD" ? await c.req.raw.arrayBuffer() : undefined, duplex:
"half" })`, meaning every POST/PUT/etc. request body is fully read into an ArrayBuffer
before being sent to the sandbox container, rather than streaming the original request
body.
4. Use the E2B SDK in `cli/test/integration/e2b-smoke.spec.ts:12-45` as a concrete caller
pattern (e.g., `await sandbox.files.write("/tmp/test.txt", "hello e2b")`) and extend it to
a large payload (e.g., `sandbox.files.write("/tmp/big.bin", hugeString)`): the SDK sends a
non-GET request through envd, which hits `handleProxyRequest`; for such large bodies, the
`arrayBuffer()` call forces the proxy process to hold the entire payload in memory at
once, breaking streaming semantics and potentially exhausting memory for very large
uploads.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** cli/src/server/services/proxy.ts
**Line:** 135:138
**Comment:**
*Performance: Buffering every non-GET/HEAD request body with `arrayBuffer()` forces the proxy to load the full payload into memory before forwarding, which breaks streaming request behavior and can cause high memory usage or OOMs on large uploads. Forward the original request stream instead of eagerly buffering the entire body.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/test/unit/commands/serve.spec.ts (1)
38-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the full E2B key shape in these tests. The first assertion only checks the
e2b_prefix, and the fixtures here are shorter than the 40-hex key generated incli/src/commands/serve.ts. Use a full-shape regex and full-length fixtures so the suite catches regressions in key generation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/test/unit/commands/serve.spec.ts` around lines 38 - 64, The `serve.spec.ts` assertions only validate the `e2b_` prefix, so update the tests around `run()` to check the full E2B API key format instead of a partial prefix. Use a full-length regex matching the key shape generated in `cli/src/commands/serve.ts`, and replace the short `apiKey` fixtures in the `readConfigMock` cases with full-length values so the `writeConfigMock` and `process.env.API_KEYS` expectations cover the real key generation contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/server/services/proxy.ts`:
- Around line 131-138: The proxy request path in handleProxyRequest is fully
buffering non-GET/HEAD bodies via c.req.raw.arrayBuffer(), which can blow up
memory on uploads. Update the fetch call in proxy.ts to stream c.req.raw.body
directly when available, or add a strict request-size guard before any
buffering; keep the logic centered around handleProxyRequest and its
fetch(targetUrl, ...) body handling.
---
Outside diff comments:
In `@cli/test/unit/commands/serve.spec.ts`:
- Around line 38-64: The `serve.spec.ts` assertions only validate the `e2b_`
prefix, so update the tests around `run()` to check the full E2B API key format
instead of a partial prefix. Use a full-length regex matching the key shape
generated in `cli/src/commands/serve.ts`, and replace the short `apiKey`
fixtures in the `readConfigMock` cases with full-length values so the
`writeConfigMock` and `process.env.API_KEYS` expectations cover the real key
generation contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73d85780-8a06-4245-8d1d-2ff8fb6e6e75
⛔ Files ignored due to path filters (1)
cli/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
README.mdcli/package.jsoncli/src/commands/serve.tscli/src/server/services/proxy.tscli/test/global-setup.tscli/test/unit/commands/serve.spec.tsdocs/container-runtimes.mdskills/guide/SKILL.md
Comment on lines
131
to
138
| try { | ||
| const response = await fetch(targetUrl, { | ||
| method: c.req.method, | ||
| headers, | ||
| body: | ||
| c.req.method !== "GET" && c.req.method !== "HEAD" | ||
| ? c.req.raw.body | ||
| ? await c.req.raw.arrayBuffer() | ||
| : undefined, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## file map\n'
git ls-files 'cli/src/server/**' | sed -n '1,200p'
printf '\n## outline proxy.ts\n'
ast-grep outline cli/src/server/services/proxy.ts --view expanded || true
printf '\n## outline app.ts\n'
ast-grep outline cli/src/server/app.ts --view expanded || true
printf '\n## relevant search\n'
rg -n "arrayBuffer\(\)|handleProxyRequest|fetch\(targetUrl|raw\.body|ReadableStream|body:" cli/src/server -SRepository: circlesac/sandbox
Length of output: 2720
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## proxy.ts lines 1-220\n'
sed -n '1,220p' cli/src/server/services/proxy.ts | cat -n
printf '\n## app.ts lines 1-140\n'
sed -n '1,140p' cli/src/server/app.ts | cat -nRepository: circlesac/sandbox
Length of output: 10838
Avoid buffering proxy request bodies.
app.ts routes envd file traffic through handleProxyRequest, and this path turns every non-GET/HEAD request into await c.req.raw.arrayBuffer(). That fully buffers uploads in memory; stream c.req.raw.body into fetch or enforce a strict size cap before buffering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/server/services/proxy.ts` around lines 131 - 138, The proxy request
path in handleProxyRequest is fully buffering non-GET/HEAD bodies via
c.req.raw.arrayBuffer(), which can blow up memory on uploads. Update the fetch
call in proxy.ts to stream c.req.raw.body directly when available, or add a
strict request-size guard before any buffering; keep the logic centered around
handleProxyRequest and its fetch(targetUrl, ...) body handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Resolves issue #7 and #8
CodeAnt-AI Description
Use E2B-compatible API keys and keep proxied requests working
What Changed
e2b_...format instead of the oldsk-sandbox-...format.transfer-encodingheader and send buffered request bodies, which avoids broken uploads and double-encoding issues with the sandbox backend.Impact
✅ Fewer sandbox startup failures✅ Working API key validation with E2B✅ Fewer broken proxy uploads💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores