fix: always install Node 22 for ACP packages in agent-server images#2652
Open
simonrosenberg wants to merge 4 commits intomainfrom
Open
fix: always install Node 22 for ACP packages in agent-server images#2652simonrosenberg wants to merge 4 commits intomainfrom
simonrosenberg wants to merge 4 commits intomainfrom
Conversation
SWE-bench base images often ship with NVM-managed old Node.js versions
(v8–v14) that already have `npm` in PATH. The previous conditional
install (`if ! command -v npm`) would skip Node 22, causing ACP packages
(claude-agent-acp, codex-acp, gemini-cli) to be installed into the old
Node.js where they crash with SyntaxError on modern ES module syntax.
Remove the conditional and always install Node 22 from nodesource so ACP
subprocess servers can start reliably regardless of the base image.
Fixes: OpenHands/runtime-api#458
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Collaborator
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Eliminates broken conditional that failed on old Node versions. Fix is correct: always installing Node 22 ensures ACP packages work on all base images.
✅ Worth merging - Fixes real production bug with sound solution.
Instead of replacing the system Node.js with nodesource, download a Node 22 binary tarball to /opt/acp-node/ and install ACP packages there. Wrapper scripts in /usr/local/bin/ prepend the ACP Node to PATH only for the ACP subprocess, so the repo's own Node.js (needed for test suites) stays untouched. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The POST /api/conversations request triggers lazy agent initialization (ACP subprocess startup, model config loading, tool registration) which routinely takes >60s on heavy SWE-bench multimodal images. The 60s default caused httpx.ReadTimeout on 4/5 test instances even after the Node.js fix landed. Align with the parent RemoteWorkspace.read_timeout default (600s) and the legacy code which used 120-600s for this value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit d6da034.
This was referenced Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
if ! command -v npmchecknpmin PATH, causing the conditional to skip Node 22 installationclaude-agent-acp,codex-acp,gemini-cli) installed into old Node.js crash withSyntaxErroron modern ES module syntax (e.g.import ... with { type: "json" })Root cause analysis
The Dockerfile had:
if ! command -v npm >/dev/null 2>&1; then # install Node 22 fi npm install -g @zed-industries/claude-agent-acp ...SWE-bench images are built from repos that often include
.nvmrcor NVM-managed Node.js versions:Since
npmexists in PATH via the old Node.js, the conditional skipped Node 22 installation. ACP packages were installed into the old Node.js and crashed immediately on startup.Chart.js (Node 21.6.2) worked because its Node.js version is new enough.
Related
Test plan
node --versionshows v22.x inside the built imageclaude-agent-acpstarts without SyntaxError🤖 Generated with Claude Code
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:612ef5a-pythonRun
All tags pushed for this build
About Multi-Architecture Support
612ef5a-python) is a multi-arch manifest supporting both amd64 and arm64612ef5a-python-amd64) are also available if needed