feat: support reusing prebuilt SDK sdists#2426
Merged
simonrosenberg merged 1 commit intomainfrom Mar 13, 2026
Merged
Conversation
Contributor
API breakage checks (Griffe)Result: Passed |
Contributor
Agent server REST API breakage checks (OpenAPI)Result: Passed |
all-hands-bot
approved these changes
Mar 13, 2026
Collaborator
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟢 Good taste - Clean, pragmatic solution
What This Does Right:
- Solves Real Problem: Eliminates redundant
uv build --sdistcalls in CI/CD pipelines where sdist is already built - Simple Design: Optional
prebuilt_sdistparameter with clean fallback to existing behavior - no special cases - Proper Data Flow: Either build OR reuse, with correct cleanup logic in both paths
- Backward Compatible: Default
Nonepreserves all existing behavior - Real Tests: Tests verify actual behavior (no uv build called, sdist reusable), not just mock assertions
Code Quality:
- No deep nesting (< 3 levels) ✅
- Clear error handling (
FileNotFoundErrorwith helpful message) ✅ - Proper resource cleanup (conditional
rmtreeforsdist_dir) ✅ - Good logging at decision points ✅
Testing Assessment:
The tests exercise real code paths and assert on outcomes:
test_make_build_context_reuses_prebuilt_sdist_without_running_uv_build: Verifies nouv buildcall AND content extraction workstest_build_with_prebuilt_sdist_preserves_tags_and_docker_args: Integration test verifying docker build command structuretest_build_can_reuse_same_prebuilt_sdist_multiple_times: Proves reusability claim
No mocking of the unit under test - just mocking infrastructure (docker commands). Good.
Verdict: ✅ LGTM - This is how features should be built: solving real problems with simple, testable code.
Contributor
Coverage Report •
|
||||||||||||||||||||
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
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
BuildOptions.prebuilt_sdistas a public API for reusing a prebuilt SDK sdist_make_build_context()and the CLI to use a provided sdist instead of always runninguv build --sdistScope
This intentionally carries only the original prebuilt-sdist feature from #2399. It does not include the later BuildKit cache export changes.
Validation
uv run pytest tests/agent_server/test_docker_build.pyAgent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:6c88624-pythonRun
All tags pushed for this build
About Multi-Architecture Support
6c88624-python) is a multi-arch manifest supporting both amd64 and arm646c88624-python-amd64) are also available if needed