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 (#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>
This commit is contained in:
committed by
GitHub
co-authored by
openhands
enyst
parent
3520cf1821
commit
b2ba5889d3
@@ -0,0 +1,107 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Generate examples/acp-docker/.env from the single source of truth.
|
||||
*
|
||||
* Reads the version pins in config/defaults.json and writes the
|
||||
* `AGENT_SERVER_IMAGE=` line into examples/acp-docker/.env, pinning the
|
||||
* example to the exact `versions.agentServer` release. This keeps the
|
||||
* reproducible quickstart path on the SoT version instead of a hardcoded
|
||||
* tag that silently drifts below the Canvas compatibility floor
|
||||
* (compatibility.minimumAgentServer) and renders "Disconnected".
|
||||
*
|
||||
* The no-config `docker compose up` path uses the compose fallback
|
||||
* (`latest-python`, always >= the floor); running this script first pins the
|
||||
* example to the reproducible SoT version instead.
|
||||
*
|
||||
* Idempotent: re-running upserts the AGENT_SERVER_IMAGE line and leaves any
|
||||
* other lines in an existing .env untouched.
|
||||
*
|
||||
* Usage:
|
||||
* node scripts/gen-acp-docker-env.mjs # or: npm run example:acp-docker:env
|
||||
*/
|
||||
import { readFileSync, writeFileSync } from "node:fs";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const projectRoot = join(__dirname, "..");
|
||||
|
||||
/**
|
||||
* @param {{ images: { agentServer: string }, versions: { agentServer: string } }} config
|
||||
* @returns {string} e.g. "ghcr.io/openhands/agent-server:1.28.1-python"
|
||||
*/
|
||||
export function computeAgentServerImage(config) {
|
||||
return `${config.images.agentServer}:${config.versions.agentServer}-python`;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {{ images: { agentServer: string }, versions: { agentServer: string } }} config
|
||||
* @returns {string} the `AGENT_SERVER_IMAGE=<image>` line
|
||||
*/
|
||||
export function renderEnvLine(config) {
|
||||
return `AGENT_SERVER_IMAGE=${computeAgentServerImage(config)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Upsert the AGENT_SERVER_IMAGE line into an existing .env body, preserving
|
||||
* every other line. Appends the line if absent.
|
||||
* @param {string} existing prior .env contents ("" if the file is absent)
|
||||
* @param {string} line the `AGENT_SERVER_IMAGE=...` line to set
|
||||
* @returns {string} the updated .env contents
|
||||
*/
|
||||
export function upsertEnvLine(existing, line) {
|
||||
const eq = line.indexOf("=");
|
||||
if (eq <= 0) {
|
||||
// A keyless line ("", "novalue", "=value") would make `key` empty and
|
||||
// match every line — refuse rather than silently rewrite the whole file.
|
||||
throw new Error(
|
||||
`upsertEnvLine: expected a "KEY=value" line, got "${line}"`,
|
||||
);
|
||||
}
|
||||
const key = line.slice(0, eq + 1); // "AGENT_SERVER_IMAGE="
|
||||
const lines = existing.length ? existing.replace(/\n+$/, "").split("\n") : [];
|
||||
let replaced = false;
|
||||
const next = lines.map((l) => {
|
||||
if (l.startsWith(key)) {
|
||||
replaced = true;
|
||||
return line;
|
||||
}
|
||||
return l;
|
||||
});
|
||||
if (!replaced) next.push(line);
|
||||
return next.join("\n") + "\n";
|
||||
}
|
||||
|
||||
function loadConfig() {
|
||||
return JSON.parse(
|
||||
readFileSync(join(projectRoot, "config", "defaults.json"), "utf-8"),
|
||||
);
|
||||
}
|
||||
|
||||
function main() {
|
||||
const config = loadConfig();
|
||||
const line = renderEnvLine(config);
|
||||
const envPath = join(projectRoot, "examples", "acp-docker", ".env");
|
||||
|
||||
let existing = "";
|
||||
try {
|
||||
existing = readFileSync(envPath, "utf-8");
|
||||
} catch {
|
||||
// no .env yet — create it
|
||||
}
|
||||
|
||||
const updated = upsertEnvLine(existing, line);
|
||||
writeFileSync(envPath, updated);
|
||||
console.log(`Wrote ${line} to examples/acp-docker/.env`);
|
||||
}
|
||||
|
||||
// Run main() only when invoked as a CLI. process.argv[1] is undefined in some
|
||||
// ESM contexts (e.g. `node --input-type=module -e "import(...)"`), so guard it
|
||||
// before pathToFileURL — otherwise importing this module for its exports throws
|
||||
// ERR_INVALID_ARG_TYPE.
|
||||
if (
|
||||
process.argv[1] &&
|
||||
import.meta.url === pathToFileURL(process.argv[1]).href
|
||||
) {
|
||||
main();
|
||||
}
|
||||
Reference in New Issue
Block a user