Skip to content

fix(commit0): drop the forked agent-layer Dockerfile, build through the shared phased pipeline - #778

Merged
simonrosenberg merged 2 commits into
mainfrom
drop-commit0-agent-layer-fork
Sep 2, 2026
Merged

simonrosenberg merged 2 commits into
mainfrom
drop-commit0-agent-layer-fork

Conversation

@simonrosenberg

@simonrosenberg simonrosenberg commented Sep 1, 2026 •

Copy link
Copy Markdown
Member

Drop the forked commit0 agent-layer Dockerfile

Why

benchmarks/utils/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:

Provider commit0 pins (before) SDK pins today
claude @agentclientprotocol/claude-agent-acp@0.30.0 0.63.0
gemini @google/gemini-cli@0.38.0 0.46.0
codex @zed-industries/codex-acp@0.15.0 (abandoned pre-migration package) @agentclientprotocol/codex-acp@1.1.7

The SDK has a CI test (#4805) asserting its Dockerfile and acp_providers.py agree, but it can't
reach 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-minimal handles
apt/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 pipeline
as SWE-Bench:

eval-builder:<sdk-sha>  →  eval-base:<instance-tag>  →  eval-agent-server:<tag>
(SDK builder target)       (SDK base-image-minimal,     (Dockerfile.agent-layer,
                            BASE_IMAGE=<commit0 image>)   one COPY)
  • benchmarks/commit0/build_images.py: replaced the custom assemble_commit0_agent_images /
    COMMIT0_AGENT_LAYER_DOCKERFILE path with calls into the shared
    build_builder_image / build_all_base_images / assemble_all_agent_images helpers
    (benchmarks/swebench/build_base_images.py), mirroring swebench/build_images.py's main().
  • benchmarks/commit0/run_infer.py: image tag lookup now uses the shared
    get_phased_image_tag_prefix() scheme instead of a commit0-only Dockerfile-content-hash prefix.
  • benchmarks/utils/version.py: updated get_phased_image_tag_prefix()'s docstring — commit0
    moves from the "legacy" group to the phased-build group.
  • Tests rewritten to mirror test_phased_build.py / test_multimodal_phased_build.py's
    orchestration-test pattern.

Behavior changes (consequences of fixing the drift, not bugs)

  • ACP provider versions change for acp-* runs (see table above). agent_type=default runs
    are unaffected — confirmed below.
  • The ACP payload path moves /opt/acp-node → /acp-node (SDK #4651). Verified no code in
    this repo hardcodes /opt/acp-node (benchmarks/utils/acp.py uses bare command names via
    /usr/local/bin wrappers, not the node dir path directly).
  • OH_ENABLE_VNC=false in the fork was vestigial — SDK #4792 removed the desktop stack
    entirely; the new path carries no VNC-related env at all.
  • The /usr/local/bin wrappers (claude-agent-acp, codex-acp, gemini) remain present and
    executable — 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.yml run on record predates this PR by over four
months (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:

  • Baseline: build-commit0-images.yml dispatched on main (unmodified fork Dockerfile), run
    33508307526
  • New: same dispatch on this branch, run
    33508335701
  • Both: sdk-commit=c98ab175f4e9eb8b0777707c5739348109c6244c, repo-split=lite, n-limit=4
    (tinydb, simpy, deprecated, wcwidth), force-build=true, max-workers=4

Cache 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.jsonl and the new base-manifest.jsonl,
because the phased-path build/assembly helpers don't wire up _apply_sdk_telemetry; this is a
pre-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.

Baseline (main, old fork) New (this branch)
"Build and push Commit0 images" step 209s (3m29s) 392s (6m32s)
Builder phase (fresh in both — no skip) ~72s ~78s
Base-image phase (new phase; doesn't exist in old fork) n/a ~131s (4 images, parallel)
Assembly/COPY phase ~121s (4 images, parallel) ~163s (4 images, parallel)

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-minimal build per instance) that the old
monolithic fork Dockerfile didn't have. This test deliberately used force-build=true, which
bypasses the remote_image_exists skip checks that let repeat/incremental builds reuse
already-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 run
above) and compared layer digests via docker buildx imagetools inspect --raw, cross-checked
against each image's build history (created_by metadata) to confirm layer identity, not just
position:

  • /acp-node (COPY /acp-node /acp-node), in the eval-base images:
    sha256:524d7b2107898cb1e5dbf3232da24210eb3f9c37d438d6b59e4716ea2296e2d7 (324,386,244 bytes) —
    identical across eval-base:1b2bb05-commit0-tinydb and eval-base:1b2bb05-commit0-simpy.
  • /agent-server (COPY --chown=openhands:openhands /agent-server /agent-server), in the final
    eval-agent-server images:
    sha256:7917e10e071a59e5ef41fa7604eb4283c623347f5643fdddb44f329875ad407b (333,185,169 bytes) —
    identical across eval-agent-server:c98ab17-1b2bb05-commit0-tinydb-source-minimal and
    ...-commit0-simpy-source-minimal.

Under the old fork this could never happen — everything (apt packages, ACP install, the
/agent-server COPY) was baked into per-instance layers directly on top of each unique
BASE_IMAGE in one monolithic Dockerfile stage, so identical content still produced distinct
layer digests per instance. The new path builds the ~324MB ACP payload once, from a fixed
python:3.13-bookworm base entirely decoupled from any commit0 BASE_IMAGE (SDK #4651), and the
/agent-server payload once via the shared builder image — both dedupe across every instance in
the registry.

Image size (unprompted finding)

Compared the full layer stack of the tinydb image, old fork vs. new pipeline, byte for byte:

Old fork (main) New pipeline (this branch)
Layers 16 19
Total compressed size 1,176,119,448 B (1176.1 MB) 1,056,070,934 B (1056.1 MB)

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, uv binary, and /agent-server layers are
all 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:

  • Old fork: 445,129,553 B (445.1 MB) — one combined RUN installing Node 22 +
    @agentclientprotocol/claude-agent-acp@0.30.0 + @zed-industries/codex-acp@0.15.0 +
    @google/gemini-cli@0.38.0.
  • New pipeline: 324,386,244 B (324.4 MB) — the shared acp-providers stage installing the same
    Node 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-acp package in
favor of @agentclientprotocol/codex-acp — incidentally shrinks the final image by ~120 MB per
instance. 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_commit many times a day (every SDK PR/eval dispatch pins a
different one), almost always with an unchanged base-image-minimal Dockerfile. Built the same 4
instances again for a different SDK commit
(8d0a6d7762a3f5ddec12dd6e9155ab3621288e59, confirmed to have an unchanged Dockerfile relative to
the first commit) on both pipelines, this time without force-build:

  • New pipeline (run
    33566632785): all 4 base
    images hit already exists. Skipping. — the content-hash-keyed tag doesn't depend on
    sdk_commit at 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).
  • Old fork (run 33566635331):
    Commit0 image assembly done. Built=4 Skipped=0 Failed=0 — every instance's tag is keyed by
    sdk_commit, so the entire monolithic apt+ACP+copy layer rebuilds from scratch on every single
    SDK 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.yml on main (benchmarks_branch=drop-commit0-agent-layer-fork,
sdk_commit=c98ab175f4e9eb8b0777707c5739348109c6244c, eval_limit=4, model
claude-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) Baseline: main, old fork, agent_type=default, run 24944288456, 2026-04-26, same model
Instances tinydb, simpy, deprecated, wcwidth tinydb, simpy, deprecated, wcwidth same 4 + voluptuous (5 total)
Resolved deprecated, tinydb (2/4) deprecated, tinydb (2/4) deprecated, tinydb (2/5)
Unresolved wcwidth, simpy wcwidth, simpy wcwidth, simpy, voluptuous
Errors 0 0 0
Tests passed 539/551 532/551 657/700
  • agent_type=default: identical resolve/unresolved split against the historical baseline on
    every overlapping instance. This is the bar the task set — default never touches ACP, so it
    should be unaffected by this change, and it is.
  • agent_type=acp-claude: identical resolve/unresolved split to agent_type=default on this
    same 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 4
    instances (the new pin, not the fork's stale 0.30.0). The /usr/local/bin/claude-agent-acp
    wrapper 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:

  1. First attempt at both runs (33510015034, 33510035404) failed before reaching any
    commit0-specific code: apt-get install google-cloud-sdk in the eval-job orchestration
    bootstrap 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).
  2. Before that fix merged, a agent_type=acp-claude retry (dispatched against the fix branch,
    run 33520184927) got as far
    as real inference — the deprecated instance's transcript showed claude-agent-acp spawning
    and executing real ACP tool calls (pytest runs, file reads) — before the LLM proxy
    (llm-proxy.eval.all-hands.dev) started returning 500/503s mid-run and at LiteLLM virtual-key
    creation for the other 3 instances.
  3. A further retry with claude-sonnet-4-6 (ruling out a model-specific cause) failed at the
    preflight connectivity check itself (✗ Cannot reach proxy ...: Service Unavailable), confirming
    the 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.yml now exposes --max-retries (default 5 on manual dispatch, matching
    build-swebench-images.yml). commit0/build_images.py already accepted the flag via the shared
    args parser — the workflow just never wired it through.

Out of scope

  • No SDK pin or SDK Dockerfile changes — this is a benchmarks-repo-only change.
  • No other benchmark's build path touched.
  • /bin/uv placement left as-is (separately tracked, ~169MB per benchmark set, cross-repo
    consumers).
  • While comparing against SWE-Bench's pattern, found that create_docker_workspace's
    workspace_type=docker fallback (DockerDevWorkspace) tags its build output under a name that
    never matches the pre-checked agent_server_image tag, so local dev runs always rebuild from
    scratch. 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.py helper, not something this
    PR'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.py parity test cannot reach into this repo, which is why
deletion (not re-pinning) is the fix.

…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>
@simonrosenberg simonrosenberg self-assigned this Sep 1, 2026
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>
@simonrosenberg

Copy link
Copy Markdown
Member Author

Eval-run verification is currently blocked by an unrelated infra bug, not by this PR.

Both eval runs dispatched to verify this PR (agent_type=default run 33510015034, agent_type=acp-claude run 33510035404) failed. Root-caused via kubectl + Datadog: every eval-job.yml orchestration pod is currently failing at bootstrap (apt-get install google-cloud-sdk — Google renamed the package to google-cloud-cli, and the current base image can't resolve the old name). This kills the pod ~9 seconds in, before any benchmark-specific code — including this PR's image/build changes — is ever reached. It's org-wide: every benchmark/model/agent_type dispatched right now hits this.

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.

@simonrosenberg
simonrosenberg marked this pull request as ready for review September 1, 2026 15:18

@juanmichelini juanmichelini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@simonrosenberg

simonrosenberg commented Sep 2, 2026 •

Copy link
Copy Markdown
Member Author

@juanmichelini half-broken on main

  • commit0 evals run fine today
  • acp versions were hardcoded so there's a drift that I forgot to fix.

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: Dockerfile.agent-layer-commit0 is a private fork that duplicates packages/user-setup/ACP-install instead of reusing the SDK's shared build path.
SWE-Bench already did it the correct way. This PR makes commit0 do it the same way as SWEBench.

This PR deletes the fork and routes commit0 through that same shared path instead.

Measured Benefits

  • ~10% smaller final image
  • ~20% faster incremental rebuilds once base image cached
  • Real cross-instance layer dedup, structurally impossible under the old fork
  • Verified end-to-end via real eval runs (not just a build check): default agent is byte-identical to main, acp-claude works correctly with the corrected pins
  • better code +160/-470

So agreed: small risk.
BUT: big code simplification (+160/-470), performance optimization + reuses same docker logic as swebench swtbench.
So overall I think it's good to merge. Worse case scenario if we run into a bug an agent can easily revert the commit0 docker logic and try it from here.
But this has been tested on e2e benchmarks jobs so...

@juanmichelini juanmichelini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving due acp versions drift despite simplifications.
For the future, let's prioritize battle tested code over simplifications.

@simonrosenberg
simonrosenberg merged commit 405bae7 into main Sep 2, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants