feat(e2b): transfer files via envd Filesystem REST API [2/3]

[chcodex]

Stacked PR 2/3

Split from #2196. Merge order: #3341 → #3343 → #3344.

Base is main, so this diff currently also carries #3341. That is unavoidable: GitHub rejects a PR base pointing at a fork branch, and #3341's hunk overlaps this one in E2bEnvdProcessClient (streaming parsing and file-transfer request builders sit adjacent), so splitting them would mean hand-resolving ~276 conflicting lines. Once #3341 merges, this diff shrinks to the 2 files below.

This PR adds the client-layer API only. Its sole production caller lands in #3344, so it is dead code until then — review it as the API contract, not as a shipped feature.

---

Closes #3339. Split from #2196.

Stacked on #3341 (A: envd process streaming parsing fix). Own change is the top commit only; diff shrinks to 2 files once #3341 merges.

Scope: add E2bEnvdProcessClient.uploadFile (multipart POST) and downloadFile (GET) against /files, routed via envdHost(); keep E2b-Sandbox-Id/Port/X-Access-Token/Authorization headers. Plus unit tests.

Checks: E2bEnvdProcessClientTest 36 tests pass; spotless:check passes.

[codecov]

Codecov Report

❌ Patch coverage is 82.05128% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...e/extensions/sandbox/e2b/E2bEnvdProcessClient.java	82.05%	3 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

[oss-maintainer]

Summary

Adds native envd Filesystem REST upload/download to E2bEnvdProcessClient (step 2 of the #3341 → #3343 → #3344 split): uploadFile / downloadFile against GET|POST {envd}/files?path=…&username=…, with the request construction refactored into a shared baseRequest so the Connect and REST paths keep identical auth headers. Direction is right — the base64-chunked shell writes in E2bSandbox.doHydrateWorkspace are the thing this replaces in #3344 — CLA signed, build (ubuntu-latest), License and Module Sync pass, windows-latest still running at review time.

The functional core is sound and the mocked-transport tests (uploadFileViaRestApiSendsMultipartPost, …ThrowsOnHttpError, downloadFileViaRestApiReturnsBytes, uploadFileWithDirectoryPathThrowsIllegalArgument) cover the happy and error paths. My concerns are all about operating envelope rather than logic: nothing bounds transfer duration, nothing bounds transfer size, the "absolute path" contract is documented but unenforced, and one documented envd behaviour is contradicted by how the sibling Kubernetes backend treats the same API.

Note for the series: the end-stream envelope in E2bEnvdProcessClientIntegrationTest still logs envdAccessToken, which was raised on #3341 and is unchanged in the rebase — I have repeated it here because this branch is the one that will land the file.

Findings

• [Warning] E2bEnvdProcessClient.java:381 — no writeTimeout/callTimeout for uploads; the shared client leaves OkHttp's 10 s write default while the exec path explicitly bounds calls.
• [Warning] E2bEnvdProcessClient.java:417 — res.body().bytes() buffers a whole remote file with no ceiling; this becomes the workspace-archive transport in #3344.
• [Warning] E2bEnvdProcessClient.java:373 — javadoc requires an absolute remotePath, but nothing validates it; relative paths silently resolve outside the intended directory.
• [Info] E2bEnvdProcessClient.java:366 — "creates parent directories automatically" is the opposite of what the Kubernetes backend assumes for its runtime file API; untested against real envd.
• [Info] E2bEnvdProcessClient.java:440 — shared baseRequest is a good de-duplication; the class javadoc/name ("Process" client) now understates what it does.

Not covered

No live envd endpoint was exercised (no E2B credentials here), so the REST contract claims (parent-directory creation, overwrite semantics, multipart field name) are verified against the mocks only.

---

Automated review by github-manager-bot

[oss-maintainer]

[Warning] The OkHttp client used here is built with connectTimeout and readTimeout only (constructor, lines 78-83) — writeTimeout keeps OkHttp's 10 s default, and there is no callTimeout for this path, unlike runShellCapture, which deliberately rebuilds the client per call with callTimeout(timeoutSeconds).

Multipart upload is exactly the operation that stress-tests the write side: with RequestBody.create(data, …) the whole payload is written to the socket, so an envd hiccup or a big archive trips a 10 s SocketTimeoutException (writeTimeout) that surfaces as a generic IOException rather than a SandboxException. Since #3344 routes the workspace tar through this method, please either set an explicit writeTimeout/callTimeout on the builder (or take a timeout argument here the way the exec path does) so large transfers have the same bound as exec.

[oss-maintainer]

[Warning] res.body().bytes() materializes the entire remote file on the JVM heap with no ceiling, and uploadFile has the mirror shape (byte[] data in, fully buffered). For the process path this class is careful — OUTPUT_TRUNCATE_BYTES bounds capture and the frame length is capped at 64 MiB — but here the archive is bounded only by whatever envd serves.

The consumer in #3344 (doPersistWorkspace → downloadFile(tarPath), doHydrateWorkspace → uploadFile(tarPath, all)) makes this the workspace-archive transport, so a workspace that grew past available heap turns a persist into an OOM instead of a clean error. Consider a streaming surface (downloadFileTo(state, path, OutputStream) / uploadFile(state, path, InputStream, long len)) or at least a documented maximum with a pre-flight Content-Length check that fails with SandboxErrorCode rather than an OutOfMemoryError.

[oss-maintainer]

[Warning] The javadoc states the destination is an "absolute path in the sandbox", but nothing enforces it: remotePath = "data/model.bin" passes the filename check (filenameFromPath yields model.bin) and goes out as path=data/model.bin, which envd resolves relative to the run user's home — the file silently lands somewhere other than where the caller thinks, and the later downloadFile("data/model.bin") reads it back, so the mistake is self-consistent and invisible.

#3344's E2bSandbox.supportsFileTransfer is where this actually matters (it only admits startsWith("/) paths, so a direct client call bypasses the guard). A cheap if (!remotePath.startsWith("/")) throw new IllegalArgumentException(...) in both methods makes the contract the code states, and lets the sibling check in the sandbox be a convenience rather than the only line of defence.

[oss-maintainer]

[Info] "Creates parent directories automatically and overwrites existing files" is an envd-side behaviour claim, and the sibling Kubernetes backend explicitly does not assume it — KubernetesSandbox.uploadFile runs mkdir -p first with the comment "The runtime file API is not required to create parent directories".

The new unit tests mock the HTTP layer (uploadFileViaRestApiSendsMultipartPost, …ThrowsOnHttpError), so this assumption is untested against a real envd. If it is intentional here, worth a line saying which envd/E2B API version guarantees it (and that the NATIVE_SNAPSHOT/older-template paths are covered); if it is not guaranteed, hydrate of a nested path will fail with an envd error that looks like a permissions problem.

[oss-maintainer]

[Info] Nice refactor — hoisting baseRequest out of buildEnvdRequest means the filesystem calls inherit E2b-Sandbox-Id, E2b-Sandbox-Port, the User-Agent and the X-Access-Token handling automatically, so the two transports cannot drift on auth headers. Two small notes:

• The class javadoc still describes the file as "Minimal Connect client for envd process.Process/Start (server streaming)", while it now also owns REST filesystem transfers; a one-line update (and possibly splitting the filesystem half into its own class) would keep the name honest.
• buildFilesystemRequest is a one-line alias of baseRequest; fine to keep for symmetry, just worth the comment saying it exists to make future filesystem-specific headers obvious at the call site.