mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:38:34 +08:00
* 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>
141 lines
5.2 KiB
TypeScript
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();
|
|
});
|
|
});
|