Files
OpenHands/__tests__/scripts/acp-docker-env-sync.test.ts
T
b2ba5889d3 fix(examples): inherit acp-docker image from config/defaults.json (#1434)
* fix(examples): inherit acp-docker image from config/defaults.json

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.

* test(examples): harden acp-docker env-sync per review

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.

* fix(examples): guard acp-docker env-sync entrypoint against undefined argv[1]

The CLI entrypoint guard called pathToFileURL(process.argv[1]) unconditionally.
process.argv[1] is undefined in some ESM contexts (e.g. importing the module for
its exports via `node --input-type=module -e "import(...)"`), so the guard threw
ERR_INVALID_ARG_TYPE at import, before any exported helper was reachable.

Short-circuit on process.argv[1] before pathToFileURL so importing the module is
side-effect-free while the CLI path is unchanged. Add a regression test that
reproduces the bare-import context and asserts a clean exit.

Addresses the review finding on #1434.

* docs(acp-docker): trim verbose comments per review

Address all-hands-bot's review suggestions on #1434:
- test header describes the current invariant, not the prior-state history
  (that narration belonged in the PR description)
- docker-compose.yml: condense the image-pin comment to the how-to-override;
  the compatibility-floor / #3510 rationale already lives in README §1 + the test
- .env.example: 7-line pin explainer down to 2

Comment-only; env-sync test still 10/10 green, prettier clean.

* Clarify ACP Docker image version guidance

Co-authored-by: openhands <openhands@all-hands.dev>

---------

Co-authored-by: enyst <engel.nyst@gmail.com>
Co-authored-by: openhands <openhands@all-hands.dev>
2026-06-26 23:21:32 +00:00

141 lines
5.2 KiB
TypeScript

// @vitest-environment node
//
// Drift-detection for the examples/acp-docker quickstart. Invariants enforced:
// the generated pin must equal versions.agentServer from the single source of
// truth (config/defaults.json); the no-config compose fallback must stay on
// `latest-python`; and the pinned tag must never fall below the Canvas
// compatibility floor (compatibility.minimumAgentServer). Any of those drifting
// fails this test.
import { spawnSync } from "node:child_process";
import { readFileSync } from "node:fs";
import path from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { describe, expect, it } from "vitest";
import {
computeAgentServerImage,
renderEnvLine,
upsertEnvLine,
} from "../../scripts/gen-acp-docker-env.mjs";
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
"../..",
);
function read(rel: string): string {
return readFileSync(path.join(repoRoot, rel), "utf-8");
}
const config = JSON.parse(read("config/defaults.json")) as {
images: { agentServer: string };
versions: { agentServer: string };
compatibility: { minimumAgentServer: string };
};
const pinnedImage = `${config.images.agentServer}:${config.versions.agentServer}-python`;
// Numeric-semver comparison. Throws (rather than silently comparing NaN) if a
// pin carries a non-numeric segment — these defaults.json fields are dotted
// numeric version pins, so a sha or pre-release tag landing here is a config
// error the floor check should surface loudly.
function parseSemver(v: string): number[] {
if (!/^\d+(\.\d+)*$/.test(v)) {
throw new Error(`expected a dotted numeric version, got "${v}"`);
}
return v.split(".").map(Number);
}
function gte(a: string, b: string): boolean {
const pa = parseSemver(a);
const pb = parseSemver(b);
for (let i = 0; i < Math.max(pa.length, pb.length); i++) {
const da = pa[i] ?? 0;
const db = pb[i] ?? 0;
if (da !== db) return da > db;
}
return true;
}
describe("examples/acp-docker stays in sync with config/defaults.json", () => {
it("computeAgentServerImage derives the pinned SoT tag", () => {
expect(computeAgentServerImage(config)).toBe(pinnedImage);
});
it("renders the AGENT_SERVER_IMAGE line with the pinned tag", () => {
expect(renderEnvLine(config)).toBe(`AGENT_SERVER_IMAGE=${pinnedImage}`);
});
it("the pinned SoT version satisfies the Canvas compatibility floor", () => {
expect(
gte(config.versions.agentServer, config.compatibility.minimumAgentServer),
).toBe(true);
});
it("the no-config compose fallback uses the SoT registry with the latest-python tag", () => {
const compose = read("examples/acp-docker/docker-compose.yml");
const match = compose.match(/AGENT_SERVER_IMAGE:-([^}]+)\}/);
expect(match?.[1]).toBe(`${config.images.agentServer}:latest-python`);
});
});
describe("gen-acp-docker-env.mjs is safe to import", () => {
// The module exports helpers (imported above, and by future consumers), so
// importing it must not run main(). The entrypoint guard compares
// import.meta.url against process.argv[1] — but argv[1] is undefined in some
// ESM contexts (e.g. `node --input-type=module -e "import(...)"`), and an
// unguarded pathToFileURL(argv[1]) throws ERR_INVALID_ARG_TYPE at import,
// before any export is reachable. Reproduces that exact context.
const scriptPath = path.join(repoRoot, "scripts", "gen-acp-docker-env.mjs");
it("imports without throwing when process.argv[1] is undefined", () => {
const url = pathToFileURL(scriptPath).href;
const res = spawnSync(
process.execPath,
["--input-type=module", "-e", `import(${JSON.stringify(url)})`],
{ encoding: "utf-8" },
);
expect(res.stderr).not.toMatch(/ERR_INVALID_ARG_TYPE/);
expect(res.status).toBe(0);
});
});
describe("upsertEnvLine", () => {
const line =
"AGENT_SERVER_IMAGE=ghcr.io/openhands/agent-server:1.28.1-python";
it("appends the line to an empty file", () => {
expect(upsertEnvLine("", line)).toBe(`${line}\n`);
});
it("replaces an existing assignment in place, preserving other lines", () => {
const existing =
"SESSION_API_KEY=abc\n" +
"AGENT_SERVER_IMAGE=ghcr.io/openhands/agent-server:1.25.0-python\n" +
"CLAUDE_CODE_OAUTH_TOKEN=zzz\n";
expect(upsertEnvLine(existing, line)).toBe(
`SESSION_API_KEY=abc\n${line}\nCLAUDE_CODE_OAUTH_TOKEN=zzz\n`,
);
});
it("is idempotent — re-running yields one assignment, unchanged content", () => {
const once = upsertEnvLine("", line);
expect(upsertEnvLine(once, line)).toBe(once);
expect(once.match(/^AGENT_SERVER_IMAGE=/gm)?.length).toBe(1);
});
it("treats a commented template line as documentation and appends the real value", () => {
// .env.example ships `# AGENT_SERVER_IMAGE=...` as a documented knob; after
// `cp .env.example .env` the comment stays and the generator adds the value.
const existing =
"# AGENT_SERVER_IMAGE=ghcr.io/openhands/agent-server:latest-python\n";
expect(upsertEnvLine(existing, line)).toBe(
`${existing.trimEnd()}\n${line}\n`,
);
});
it("throws rather than rewrite every line when given a keyless line", () => {
expect(() => upsertEnvLine("FOO=bar\n", "novalue")).toThrow();
});
});