diff --git a/openhands-agent-server/openhands/agent_server/docker/Dockerfile b/openhands-agent-server/openhands/agent_server/docker/Dockerfile index b857f7b221..72020191e4 100644 --- a/openhands-agent-server/openhands/agent_server/docker/Dockerfile +++ b/openhands-agent-server/openhands/agent_server/docker/Dockerfile @@ -93,14 +93,40 @@ RUN set -eux; \ rm -rf /var/lib/apt/lists/* # Pre-install ACP servers for ACPAgent support (Claude Code, Codex, Gemini CLI) -# Install Node.js/npm if not present (SWE-bench base images may lack them) +# Install Node.js 22 to a dedicated prefix so ACP packages get a modern runtime +# WITHOUT overwriting the repo-specific Node.js that test suites depend on. +# SWE-bench images ship NVM/apt-managed Node 8-14 which cannot run ACP packages. +ENV ACP_NODE_DIR=/opt/acp-node RUN set -eux; \ - if ! command -v npm >/dev/null 2>&1; then \ - curl -fsSL https://deb.nodesource.com/setup_22.x | bash - && \ - apt-get install -y --no-install-recommends nodejs && \ - rm -rf /var/lib/apt/lists/*; \ - fi; \ - npm install -g @zed-industries/claude-agent-acp @zed-industries/codex-acp @google/gemini-cli + ARCH=$(dpkg --print-architecture); \ + case "$ARCH" in \ + amd64) NARCH=x64; NODE_SHA256=69b09dba5c8dcb05c4e4273a4340db1005abeafe3927efda2bc5b249e80437ec;; \ + arm64) NARCH=arm64; NODE_SHA256=08bfbf538bad0e8cbb0269f0173cca28d705874a67a22f60b57d99dc99e30050;; \ + *) echo "Unsupported architecture for ACP Node install: $ARCH" >&2; exit 1;; \ + esac; \ + NODE_TARBALL="/tmp/node-v22.14.0-linux-${NARCH}.tar.xz"; \ + mkdir -p "$ACP_NODE_DIR"; \ + curl -fsSL "https://nodejs.org/dist/v22.14.0/node-v22.14.0-linux-${NARCH}.tar.xz" -o "$NODE_TARBALL"; \ + echo "$NODE_SHA256 $NODE_TARBALL" | sha256sum -c -; \ + tar -xJ --strip-components=1 -C "$ACP_NODE_DIR" -f "$NODE_TARBALL"; \ + rm -f "$NODE_TARBALL"; \ + PATH="$ACP_NODE_DIR/bin:$PATH"; \ + node --version; \ + npm install -g \ + @zed-industries/claude-agent-acp \ + @zed-industries/codex-acp \ + @google/gemini-cli; \ + # Create wrappers in /usr/local/bin that prepend ACP's Node 22 to PATH. + # This ensures the ACP binary's #!/usr/bin/env node shebang resolves to + # Node 22, while the repo's own node (NVM/system) stays untouched for tests. + for bin in claude-agent-acp codex-acp gemini; do \ + if [ -e "$ACP_NODE_DIR/bin/$bin" ]; then \ + printf '#!/bin/sh\nPATH="%s/bin:$PATH" exec "%s/bin/%s" "$@"\n' \ + "$ACP_NODE_DIR" "$ACP_NODE_DIR" "$bin" \ + > /usr/local/bin/"$bin"; \ + chmod +x /usr/local/bin/"$bin"; \ + fi; \ + done # Configure Claude Code managed settings for headless operation: # Allow all tool permissions (no human in the loop to approve). diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index 9113c4ecd9..a171a3576f 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -3,6 +3,7 @@ from collections.abc import Mapping from pathlib import Path +from openhands.sdk.agent.acp_agent import ACPAgent from openhands.sdk.agent.base import AgentBase from openhands.sdk.context.prompts.prompt import render_template from openhands.sdk.conversation.base import BaseConversation @@ -484,6 +485,16 @@ def _ensure_agent_ready(self) -> None: self._agent_ready = True + def _should_initialize_agent_on_send_message(self) -> bool: + """Return whether send_message() should eagerly initialize the agent. + + ACPAgent startup is substantially heavier than regular agent + initialization because it launches and handshakes with an external ACP + subprocess. Deferring that work to run() keeps send_message() fast and + avoids HTTP client read timeouts on the remote conversation endpoint. + """ + return not isinstance(self.agent, ACPAgent) + def switch_profile(self, profile_name: str) -> None: """Switch the agent's LLM to a named profile. @@ -520,8 +531,12 @@ def send_message(self, message: str | Message, sender: str | None = None) -> Non one agent delegates to another, the sender can be set to identify which agent is sending the message. """ - # Ensure agent is fully initialized (loads plugins and initializes agent) - self._ensure_agent_ready() + # ACPAgent startup can take much longer than a normal send_message() + # round-trip because it launches and initializes a subprocess-backed + # session. Defer that work to run() so enqueueing the user message + # remains fast for remote callers. + if self._should_initialize_agent_on_send_message(): + self._ensure_agent_ready() if isinstance(message, str): message = Message(role="user", content=[TextContent(text=message)]) diff --git a/tests/sdk/conversation/local/test_conversation_send_message.py b/tests/sdk/conversation/local/test_conversation_send_message.py index e19f87c334..b43c4c7e7b 100644 --- a/tests/sdk/conversation/local/test_conversation_send_message.py +++ b/tests/sdk/conversation/local/test_conversation_send_message.py @@ -1,8 +1,14 @@ +from unittest.mock import patch + from pydantic import SecretStr +from openhands.sdk.agent.acp_agent import ACPAgent from openhands.sdk.agent.base import AgentBase from openhands.sdk.conversation import Conversation, LocalConversation -from openhands.sdk.conversation.state import ConversationState +from openhands.sdk.conversation.state import ( + ConversationExecutionStatus, + ConversationState, +) from openhands.sdk.conversation.types import ( ConversationCallbackType, ConversationTokenCallbackType, @@ -153,3 +159,45 @@ def test_send_message_with_message_object(): assert len(user_event.llm_message.content) == 1 assert isinstance(user_event.llm_message.content[0], TextContent) assert user_event.llm_message.content[0].text == test_text + + +def test_acp_send_message_defers_initialization_until_run(tmp_path): + """ACP conversations should enqueue messages before starting ACP bootstrap.""" + + agent = ACPAgent(acp_command=["echo", "test"]) + conversation = LocalConversation(agent=agent, workspace=str(tmp_path)) + test_text = "Hello from ACP" + + def _finish_immediately(self, conv, on_event, on_token=None): + conv.state.execution_status = ConversationExecutionStatus.FINISHED + + with ( + patch.object(ACPAgent, "init_state", autospec=True) as mock_init_state, + patch.object( + ACPAgent, + "step", + autospec=True, + side_effect=_finish_immediately, + ) as mock_step, + ): + conversation.send_message(test_text) + + assert mock_init_state.call_count == 0 + assert mock_step.call_count == 0 + assert len(conversation.state.events) == 1 + user_event = conversation.state.events[-1] + assert isinstance(user_event, MessageEvent) + assert user_event.source == "user" + assert user_event.llm_message.role == "user" + assert len(user_event.llm_message.content) == 1 + assert isinstance(user_event.llm_message.content[0], TextContent) + assert user_event.llm_message.content[0].text == test_text + + conversation.run() + + assert mock_init_state.call_count == 1 + assert mock_step.call_count == 1 + assert ( + conversation.state.execution_status == ConversationExecutionStatus.FINISHED + ) + assert conversation.state.events[-1] == user_event