Skip to content

fix(examples): inherit acp-docker image from config/defaults.json#1434

Open
georgeglarson wants to merge 2 commits into
OpenHands:mainfrom
georgeglarson:feat-acp-docker-env-sync
Open

fix(examples): inherit acp-docker image from config/defaults.json#1434
georgeglarson wants to merge 2 commits into
OpenHands:mainfrom
georgeglarson:feat-acp-docker-env-sync

Conversation

@georgeglarson

@georgeglarson georgeglarson commented Jun 19, 2026

Copy link
Copy Markdown

HUMAN:

  • Verified locally: docker compose config resolves latest-python (zero-config) and 1.28.1-python (after npm run example:acp-docker:env); npx vitest run __tests__/scripts/acp-docker-env-sync.test.ts → 9 passed.
  • A human has tested these changes.

AGENT:


Why

examples/acp-docker/docker-compose.yml hardcoded agent-server:1.25.0-python. Canvas enforces compatibility.minimumAgentServer (1.28.0) from config/defaults.json, so the example's default image fell below the floor and rendered "Disconnected — requires 1.28.0 or newer" — a user following the quickstart as written never reached the feature. examples/acp-docker was the only in-repo file hardcoding a version instead of inheriting from the single source of truth.

Summary

  • New scripts/gen-acp-docker-env.mjs (+ npm run example:acp-docker:env) pins AGENT_SERVER_IMAGE to ${images.agentServer}:${versions.agentServer}-python from config/defaults.json (idempotent upsert; mirrors scripts/docker-build.mjs).
  • Compose no-config fallback 1.25.0-pythonlatest-python (always ≥ the floor; zero-config docker compose up never shows "Disconnected"). .env.example + README.md document both paths and correct the version narrative (floor = the defaults.json compatibility pin; #3510 is the deeper functional floor at/below it).
  • New __tests__/scripts/acp-docker-env-sync.test.ts (mirrors docs-version-sync.test.ts) — the guard that makes "can't silently drift" true.

Issue Number

#1433

How to Test

npm ci

# zero-config path → latest-python (always >= the compatibility floor)
cd examples/acp-docker && docker compose config | grep 'image:'
#     image: ghcr.io/openhands/agent-server:latest-python

# reproducible pinned path → SoT version from config/defaults.json
cd ../.. && npm run example:acp-docker:env
cd examples/acp-docker && docker compose config | grep 'image:'
#     image: ghcr.io/openhands/agent-server:1.28.1-python

# the drift guard
cd ../.. && npx vitest run __tests__/scripts/acp-docker-env-sync.test.ts   # 4 passed

Verified end-to-end on Linux: docker compose config resolves both images as shown above; full __tests__/scripts suite green (168 passed); npm run typecheck clean.

Type

  • Bug fix

Notes

examples/acp-docker/.env is gitignored; the generator writes it. Separate concern from #1244/#1430 (the ACP auth banner) — its own issue + PR.

examples/acp-docker/docker-compose.yml hardcoded the agent-server image at
`1.25.0-python`. Canvas enforces `compatibility.minimumAgentServer` (1.28.0)
from the repo's single source of truth, so the example default fell below the
floor and rendered "Disconnected — requires 1.28.0 or newer" — a reviewer
following the quickstart as written never reached the feature.

examples/acp-docker was the lone in-repo file hardcoding a version instead of
inheriting from config/defaults.json (14 other files read it; check-sdk-version
-sync only validates the released PyPI package, not in-repo files).

- scripts/gen-acp-docker-env.mjs: read defaults.json, pin AGENT_SERVER_IMAGE to
  `${images.agentServer}:${versions.agentServer}-python` in examples/acp-docker
  /.env (idempotent upsert; mirrors scripts/docker-build.mjs).
- package.json: `npm run example:acp-docker:env`.
- docker-compose.yml: no-config fallback `1.25.0-python` -> `latest-python`,
  always >= the compatibility floor, so zero-config `docker compose up` never
  shows "Disconnected"; the generated .env overrides with the pinned SoT
  version for the reproducible path.
- .env.example / README.md: document both paths; correct the version narrative
  (floor is the defaults.json compatibility pin; #3510 is the deeper functional
  floor at/below it).
- __tests__/scripts/acp-docker-env-sync.test.ts: assert the generator's tag
  matches defaults.json, the pin satisfies the floor, and the compose fallback
  stays `latest-python`. Mirrors docs-version-sync.test.ts — the guard that
  makes "can't silently drift" true.
@vercel

vercel Bot commented Jun 19, 2026

Copy link
Copy Markdown

@georgeglarson is attempting to deploy a commit to the openhands Team on Vercel.

A member of the Team first needs to authorize it.

Addresses the cli-review-panel findings worth acting on (the rest were
cosmetic or matched the no-validation idiom of scripts/docker-build.mjs):

- gte() in the test guarded with parseSemver — a non-numeric pin (sha /
  pre-release) now fails the floor check loudly instead of silently
  comparing NaN. The floor check is a CI gate; its one piece of logic
  shouldn't mis-compare in silence.
- compose-fallback assertion derives the registry from config.images
  .agentServer instead of hardcoding ghcr.io/openhands/... — a registry
  change no longer false-fails a test that only cares about the latest-python
  tag.
- upsertEnvLine now has unit tests (append / replace-in-place+preserve /
  idempotent / commented-template-line / keyless-line guard), making the
  "idempotent upsert" claim defensible. It was the one untested piece of real
  logic.
- upsertEnvLine guards a keyless line (no "=") with a clear throw, instead of
  an empty key matching every line and rewriting the whole file.
@georgeglarson georgeglarson marked this pull request as ready for review June 19, 2026 18:15
Copilot AI review requested due to automatic review settings June 19, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes drift in the examples/acp-docker quickstart by removing a hardcoded, below-minimum agent-server image tag and instead deriving the pinned image version from the repo’s single source of truth (config/defaults.json). This aligns the example with the frontend’s enforced minimum agent-server compatibility floor and prevents users from hitting a “Disconnected — requires … or newer” banner when following the docs.

Changes:

  • Add a small generator script (scripts/gen-acp-docker-env.mjs) plus an npm script to write/update examples/acp-docker/.env with AGENT_SERVER_IMAGE=<registry>:<versions.agentServer>-python from config/defaults.json.
  • Update the compose no-config fallback image to ghcr.io/openhands/agent-server:latest-python and refresh example docs/templates to explain the pinned vs. zero-config paths.
  • Add a Vitest drift-guard ensuring the compose fallback and generated pinned image stay in sync with config/defaults.json (and that the pinned version satisfies the compatibility floor).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/gen-acp-docker-env.mjs New generator that upserts AGENT_SERVER_IMAGE in examples/acp-docker/.env using config/defaults.json.
package.json Adds example:acp-docker:env script to run the generator.
examples/acp-docker/README.md Updates quickstart to default to latest-python and documents the reproducible pinned .env flow.
examples/acp-docker/docker-compose.yml Switches compose fallback from a hardcoded version to latest-python and documents the SoT pin workflow.
examples/acp-docker/.env.example Updates comments/template to reflect latest-python default and the generator-based pinned approach.
__tests__/scripts/acp-docker-env-sync.test.ts New drift-detection tests enforcing SoT version inheritance and compatibility-floor satisfaction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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