feat(envd): add composite file upload API#2043
Merged
mishushakov merged 21 commits intomainfrom Mar 12, 2026
Merged
Conversation
…c writes - Use sync.Map.LoadAndDelete to atomically claim upload sessions in Complete and Delete handlers, preventing concurrent requests from corrupting output files - Replace lexicographic sort with numeric sort for part filenames to handle part numbers crossing the 6-digit boundary correctly - Write assembled output to a temp file and rename on success to preserve any pre-existing file at the destination if assembly fails Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check json.Encode return values and add blank lines before return statements to satisfy errchkjson and nlreturn linters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a per-session RWMutex to prevent concurrent Complete/Delete from snapshotting or removing the parts directory while a PUT is still writing. PUTs hold an RLock; Complete/Delete acquire a write Lock that blocks until all in-flight PUTs finish, eliminating silent data loss. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When file.ReadFrom(r.Body) fails after partial write, the part file is left on disk with corrupt data. A subsequent Complete call would silently assemble this truncated part into the final file. Close and remove the part file in the error path to prevent this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two concurrent Complete calls targeting the same destination path would share the same tmpPath (derived only from meta.Path). The second caller would truncate the first's in-progress assembly via O_TRUNC, corrupting both. Include uploadId in the temp filename to make it unique. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When os.RemoveAll fails in the abort handler, the upload directory is orphaned on disk but the session is already removed from the in-memory map, so the files will never be cleaned up. Log the error and return 500 so the client knows the abort did not fully succeed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test checked for destPath + ".e2b-upload.tmp" which was never created, so the assertion always passed trivially. Update to use the actual temp file path that includes the uploadId. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claimUpload atomically removes the session from the sync.Map and the deferred os.RemoveAll unconditionally deletes all uploaded parts. If assembly fails at any subsequent step, both the session metadata and all part data are permanently destroyed — forcing the client to re-upload every part from scratch. On failure, re-register the session in the map and preserve the parts directory so the client can retry Complete without re-uploading. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mishushakov
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5b2c9c503
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
CI runs tests as root, which bypasses DAC permission checks. The test makes a directory read-only to trigger a compose failure, but root can still write to it. Skip the test when running as root. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Return 400 if any resolved source path matches the destination to prevent data loss from self-overwrite. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reject directories and device files early with 400 instead of letting them fail later in ReadFrom with a 500. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ComposeResponse with EntryInfo in the compose endpoint response. This aligns the response format with the upload endpoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jakubno
approved these changes
Mar 11, 2026
Member
There was a problem hiding this comment.
it should probably be compose.go
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
POST /files/compose) that concatenates pre-uploaded part files into a single destination file using zero-copy kernel transfers (copy_file_range)spec/envd.yaml) and generated code for the new endpointTest plan
POST /files/composecorrectly assembles parts into a single file and cleans up sourcesPOST /filesupload path still worksmake testinpackages/envd/🤖 Generated with Claude Code
Note
Medium Risk
Introduces a new file-writing endpoint that can delete source files, so bugs in path resolution/ownership or error handling could cause data loss or unexpected writes. Performance is generally good (zero-copy), but large compose requests still amplify disk I/O and require careful monitoring of ENOSPC and partial-failure cleanup.
Overview
Adds a new
POST /files/composeendpoint (and updates OpenAPI + generated clients) that concatenates multiple existing files into a single destination usingReadFrom/copy_file_range, writing via a temp file + rename, creating parent dirs/setting ownership, and deleting sources on success; includes comprehensive unit tests for success and failure cases and bumpsenvdversion to0.5.5.Written by Cursor Bugbot for commit ed73d5b. This will update automatically on new commits. Configure here.