Repository navigation
fix(commit0): drop the forked agent-layer Dockerfile, build through the shared phased pipeline - #778
Conversation
…he shared phased pipeline Dockerfile.agent-layer-commit0 re-implemented the SDK's base-image-minimal setup (runtime packages, user creation, ACP install) instead of inheriting it, and had silently drifted onto stale/dead ACP pins (codex-acp pointed at the abandoned @zed-industries pre-migration package). The SDK's base-image-minimal target already handles raw upstream apt-based images (exactly commit0's case) via its package-manager case statement — this is the same mechanism SWE-Bench already uses successfully against its own raw upstream (docker.io/swebench/*) base images. Route commit0 through the same three-phase pipeline as SWE-Bench: builder -> base-image-minimal -> Dockerfile.agent-layer. Image tags now use the shared get_phased_image_tag_prefix() scheme (content-hash-qualified) instead of a commit0-only Dockerfile-hash prefix. Relates to OpenHands/software-agent-sdk#4643 (opportunity 1). #4805's cross-repo Dockerfile/acp_providers.py parity test cannot reach into this repo, which is why deletion (not re-pinning) is the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Mirrors build-swebench-images.yml's existing max-retries input/env/CLI wiring. commit0's build_images.py already accepts --max-retries via the shared args parser (default 3); the workflow just never exposed it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Eval-run verification is currently blocked by an unrelated infra bug, not by this PR. Both eval runs dispatched to verify this PR ( Fix: OpenHands/evaluation#608 Will re-dispatch both commit0 eval runs once that merges and update this PR's Eval runs section with real results. |
juanmichelini
left a comment
There was a problem hiding this comment.
@simonrosenberg is commit0 broken on main ? I ask because commit0 is not a priority at the moment, and every change is likely to bring unexpected bugs.
|
@juanmichelini half-broken on main
This PR first exists for the version fix. It could be narrower. Bigger picture: I've been reworking the SDK's Docker build pipeline to make it easy to add new ACP harnesses to OpenHands (a shared, parent-independent ACP layer instead of installing it per benchmark). That work happens to make commit0's own approach obsolete. Why the old approach was bad: This PR deletes the fork and routes commit0 through that same shared path instead. Measured Benefits
So agreed: small risk. |
juanmichelini
left a comment
There was a problem hiding this comment.
Approving due acp versions drift despite simplifications.
For the future, let's prioritize battle tested code over simplifications.
Drop the forked commit0 agent-layer Dockerfile
Why
benchmarks/utils/Dockerfile.agent-layer-commit0re-implemented the SDK'sbase-image-minimalsetup (runtime packages, user creation, ACP install) instead of inheriting it, and had silently
drifted onto stale/dead ACP pins:
@agentclientprotocol/claude-agent-acp@0.30.00.63.0@google/gemini-cli@0.38.00.46.0@zed-industries/codex-acp@0.15.0(abandoned pre-migration package)@agentclientprotocol/codex-acp@1.1.7The SDK has a CI test (#4805) asserting its Dockerfile and
acp_providers.pyagree, but it can'treach across repos, so this fork stayed broken by construction.
Premise check (done before touching anything): the fork's header claims commit0's raw upstream
base images (Ubuntu 22.04-derived) "lack the runtime packages ... that the SDK's
base-image-minimal target normally provides." That's false today —
base-image-minimalhandlesapt/apk/microdnf/dnf/yum/zypper bases via a package-manager case statement, and SWE-Bench already
runs this exact path against its own raw upstream images (
docker.io/swebench/*) in production.commit0's own base images (
docker.io/wentingzhao/*) are structurally identical: a third-party,non-OpenHands-controlled Docker Hub namespace. There's no remaining reason for the fork.
What changed
Deleted
Dockerfile.agent-layer-commit0. commit0 now builds through the same three-phase pipelineas SWE-Bench:
benchmarks/commit0/build_images.py: replaced the customassemble_commit0_agent_images/COMMIT0_AGENT_LAYER_DOCKERFILEpath with calls into the sharedbuild_builder_image/build_all_base_images/assemble_all_agent_imageshelpers(
benchmarks/swebench/build_base_images.py), mirroringswebench/build_images.py'smain().benchmarks/commit0/run_infer.py: image tag lookup now uses the sharedget_phased_image_tag_prefix()scheme instead of a commit0-only Dockerfile-content-hash prefix.benchmarks/utils/version.py: updatedget_phased_image_tag_prefix()'s docstring — commit0moves from the "legacy" group to the phased-build group.
test_phased_build.py/test_multimodal_phased_build.py'sorchestration-test pattern.
Behavior changes (consequences of fixing the drift, not bugs)
acp-*runs (see table above).agent_type=defaultrunsare unaffected — confirmed below.
/opt/acp-node→/acp-node(SDK #4651). Verified no code inthis repo hardcodes
/opt/acp-node(benchmarks/utils/acp.pyuses bare command names via/usr/local/binwrappers, not the node dir path directly).OH_ENABLE_VNC=falsein the fork was vestigial — SDK #4792 removed the desktop stackentirely; the new path carries no VNC-related env at all.
/usr/local/binwrappers (claude-agent-acp,codex-acp,gemini) remain present andexecutable — verified live in the acp-claude eval run below (no npx fallback exists in this
repo's ACP invocation code, so a missing wrapper would fail the run outright).
Verification
Build latency
The most recent successful
build-commit0-images.ymlrun on record predates this PR by over fourmonths (2026-04-23, run 24861799673) and its build-log artifact has since expired (7-day
retention) — there was no usable stored baseline. Rather than compare against a run whose
BuildKit/registry cache state can't be characterized, I ran a fresh, same-SDK-commit,
same-instance-selection comparison today:
build-commit0-images.ymldispatched onmain(unmodified fork Dockerfile), run33508307526
33508335701
sdk-commit=c98ab175f4e9eb8b0777707c5739348109c6244c,repo-split=lite,n-limit=4(tinydb, simpy, deprecated, wcwidth),
force-build=true,max-workers=4Cache state: neither the old fork path nor the new phased path configures BuildKit
--cache-from/--cache-to— both builds are structurally cold on every run (confirmed empirically:manifest telemetry fields are null for both
manifest.jsonland the newbase-manifest.jsonl,because the phased-path build/assembly helpers don't wire up
_apply_sdk_telemetry; this is apre-existing characteristic of the shared phased helpers, not something this PR changes). So the
two runs are comparable on their own terms; I sourced wall-clock numbers from the GH Actions job
step timings instead of manifest fields.
The new path is ~1.9x slower wall-clock for this cold,
force-build=true, n=4 test — expected,since it adds a real phase (a separate
base-image-minimalbuild per instance) that the oldmonolithic fork Dockerfile didn't have. This test deliberately used
force-build=true, whichbypasses the
remote_image_existsskip checks that let repeat/incremental builds reusealready-built base and assembly images — so it does not exercise the cross-instance layer-reuse
win. That win is structural and is verified directly below instead.
Layer sharing (the structural win)
Built two commit0 instances from different upstream repos (
tinydb,simpy— both from the runabove) and compared layer digests via
docker buildx imagetools inspect --raw, cross-checkedagainst each image's build history (
created_bymetadata) to confirm layer identity, not justposition:
/acp-node(COPY /acp-node /acp-node), in theeval-baseimages:sha256:524d7b2107898cb1e5dbf3232da24210eb3f9c37d438d6b59e4716ea2296e2d7(324,386,244 bytes) —identical across
eval-base:1b2bb05-commit0-tinydbandeval-base:1b2bb05-commit0-simpy./agent-server(COPY --chown=openhands:openhands /agent-server /agent-server), in the finaleval-agent-serverimages:sha256:7917e10e071a59e5ef41fa7604eb4283c623347f5643fdddb44f329875ad407b(333,185,169 bytes) —identical across
eval-agent-server:c98ab17-1b2bb05-commit0-tinydb-source-minimaland...-commit0-simpy-source-minimal.Under the old fork this could never happen — everything (apt packages, ACP install, the
/agent-serverCOPY) was baked into per-instance layers directly on top of each uniqueBASE_IMAGEin one monolithic Dockerfile stage, so identical content still produced distinctlayer digests per instance. The new path builds the ~324MB ACP payload once, from a fixed
python:3.13-bookwormbase entirely decoupled from any commit0BASE_IMAGE(SDK #4651), and the/agent-serverpayload once via the shared builder image — both dedupe across every instance inthe registry.
Image size (unprompted finding)
Compared the full layer stack of the
tinydbimage, old fork vs. new pipeline, byte for byte:New pipeline is 120.0 MB (10.2%) smaller. Layers 0–11 (the raw upstream base image itself) are
byte-identical in both, as expected. The apt-packages,
uvbinary, and/agent-serverlayers areall within a few KB of each other between the two. The entire 120 MB delta is isolated to one
layer — the ACP + Node.js install:
RUNinstalling Node 22 +@agentclientprotocol/claude-agent-acp@0.30.0+@zed-industries/codex-acp@0.15.0+@google/gemini-cli@0.38.0.acp-providersstage installing the sameNode runtime +
@agentclientprotocol/claude-agent-acp@0.63.0+@agentclientprotocol/codex-acp@1.1.7+@google/gemini-cli@0.46.0.Fixing the stale pins — specifically dropping the abandoned
@zed-industries/codex-acppackage infavor of
@agentclientprotocol/codex-acp— incidentally shrinks the final image by ~120 MB perinstance. Smaller pulls for every runtime pod, on top of everything else.
Cache reuse across SDK commits (the realistic day-2 scenario)
commit0 gets rebuilt for a new
sdk_commitmany times a day (every SDK PR/eval dispatch pins adifferent one), almost always with an unchanged
base-image-minimalDockerfile. Built the same 4instances again for a different SDK commit
(
8d0a6d7762a3f5ddec12dd6e9155ab3621288e59, confirmed to have an unchanged Dockerfile relative tothe first commit) on both pipelines, this time without
force-build:33566632785): all 4 base
images hit
already exists. Skipping.— the content-hash-keyed tag doesn't depend onsdk_commitat all, so the entire apt+ACP install phase (131s on the first cold run) is now~0.5s. Only the assembly COPY re-runs (~123s for 4 instances in parallel).
Commit0 image assembly done. Built=4 Skipped=0 Failed=0— every instance's tag is keyed bysdk_commit, so the entire monolithic apt+ACP+copy layer rebuilds from scratch on every singleSDK commit bump, regardless of whether anything in it actually changed. Took 153.4s.
(Both runs' shared
eval-builder:<sdk-sha>also hit a skip — whichever ran first paid that cost;excluded from the comparison above since it's identical shared infrastructure, not something this
PR changes.)
New pipeline is 19.5% faster (123.5s vs. 153.3s) for this realistic repeat-build scenario, and
that gap only widens with more instances, since the ~131s apt+ACP cost the new pipeline skips is
per-build-invocation, not per-instance.
Eval runs
Both dispatched via
eval-job.ymlonmain(benchmarks_branch=drop-commit0-agent-layer-fork,sdk_commit=c98ab175f4e9eb8b0777707c5739348109c6244c,eval_limit=4, modelclaude-sonnet-4-5-20250929, instances tinydb/simpy/deprecated/wcwidth). Both completed cleanly,end-to-end (build → infer → eval harness), zero errors:
agent_type=default(run 33565651669)agent_type=acp-claude(run 33565655689)main, old fork,agent_type=default, run 24944288456, 2026-04-26, same modelagent_type=default: identical resolve/unresolved split against the historical baseline onevery overlapping instance. This is the bar the task set —
defaultnever touches ACP, so itshould be unaffected by this change, and it is.
agent_type=acp-claude: identical resolve/unresolved split toagent_type=defaulton thissame SDK commit — confirmed the ACP agent actually ran via
output.jsonl:acp_agent_name: "@agentclientprotocol/claude-agent-acp",acp_agent_version: "0.63.0"on all 4instances (the new pin, not the fork's stale 0.30.0). The
/usr/local/bin/claude-agent-acpwrapper worked correctly through the new shared build path — exactly what this check exists to
prove.
Getting here took three unrelated infra fixes/workarounds along the way, none of them caused
by this PR:
33510015034,33510035404) failed before reaching anycommit0-specific code:
apt-get install google-cloud-sdkin the eval-job orchestrationbootstrap broke when Google renamed the package to
google-cloud-cli. Root-caused via kubectl +Datadog, fixed and merged: OpenHands/evaluation#608
(validated against the real base image and a live pod before merge).
agent_type=acp-clauderetry (dispatched against the fix branch,run 33520184927) got as far
as real inference — the
deprecatedinstance's transcript showedclaude-agent-acpspawningand executing real ACP tool calls (
pytestruns, file reads) — before the LLM proxy(
llm-proxy.eval.all-hands.dev) started returning 500/503s mid-run and at LiteLLM virtual-keycreation for the other 3 instances.
claude-sonnet-4-6(ruling out a model-specific cause) failed at thepreflight connectivity check itself (
✗ Cannot reach proxy ...: Service Unavailable), confirmingthe proxy was generally down, not model-specific. It recovered on its own; the final clean runs
above were dispatched once ACP proxy_cost=0 for fast single-turn instances (timing race) #608 was merged and the proxy was healthy again.
Also included
build-commit0-images.ymlnow exposes--max-retries(default 5 on manual dispatch, matchingbuild-swebench-images.yml).commit0/build_images.pyalready accepted the flag via the sharedargs parser — the workflow just never wired it through.
Out of scope
/bin/uvplacement left as-is (separately tracked, ~169MB per benchmark set, cross-repoconsumers).
create_docker_workspace'sworkspace_type=dockerfallback (DockerDevWorkspace) tags its build output under a name thatnever matches the pre-checked
agent_server_imagetag, so local dev runs always rebuild fromscratch. This affects commit0 the same way it affects GAIA and all four
hybridgym_*benchmarks — it's a pre-existing gap in the shared
image_utils.pyhelper, not something thisPR's pipeline change touches. Filed as create_docker_workspace never reuses locally-built images (tag mismatch with DockerDevWorkspace) #779 rather than folded in here.
Relates to OpenHands/software-agent-sdk#4643 (opportunity 1 in its remaining list). #4805's
cross-repo Dockerfile/
acp_providers.pyparity test cannot reach into this repo, which is whydeletion (not re-pinning) is the fix.