From 53a40301c1b9f6af543f67cdf2aed6840e91a866 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 19 Feb 2026 16:27:54 -0300 Subject: [PATCH 01/10] feat: Add ACPAgent for Agent Client Protocol integration Add ACPAgent, an AgentBase subclass that delegates to ACP-compatible servers (Claude Code, Gemini CLI, etc.) instead of direct LLM calls. The ACP server manages its own LLM, tools, and execution lifecycle. Closes #590 Co-Authored-By: Claude Opus 4.6 --- .../01_standalone_sdk/36_acp_agent_example.py | 34 ++ openhands-sdk/openhands/sdk/agent/__init__.py | 9 + .../openhands/sdk/agent/acp_agent.py | 503 ++++++++++++++++ openhands-sdk/pyproject.toml | 1 + tests/sdk/agent/test_acp_agent.py | 562 ++++++++++++++++++ uv.lock | 21 +- 6 files changed, 1129 insertions(+), 1 deletion(-) create mode 100644 examples/01_standalone_sdk/36_acp_agent_example.py create mode 100644 openhands-sdk/openhands/sdk/agent/acp_agent.py create mode 100644 tests/sdk/agent/test_acp_agent.py diff --git a/examples/01_standalone_sdk/36_acp_agent_example.py b/examples/01_standalone_sdk/36_acp_agent_example.py new file mode 100644 index 0000000000..42afab961b --- /dev/null +++ b/examples/01_standalone_sdk/36_acp_agent_example.py @@ -0,0 +1,34 @@ +"""Example: Using ACPAgent with Claude Code ACP server. + +This example shows how to use an ACP-compatible server (claude-code-acp) +as the agent backend instead of direct LLM calls. + +Prerequisites: + - Node.js / npx available + - Claude Code CLI authenticated (or CLAUDE_API_KEY set) + - pip install agent-client-protocol>=0.8.1 + +Usage: + uv run python examples/01_standalone_sdk/36_acp_agent_example.py +""" + +import os + +from openhands.sdk.agent import ACPAgent +from openhands.sdk.conversation import Conversation + +agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) + +cwd = os.getcwd() +conversation = Conversation(agent=agent, workspace=cwd) + +conversation.send_message( + "List the Python source files under openhands-sdk/openhands/sdk/agent/, " + "then read the __init__.py and summarize what agent classes are exported." +) +conversation.run() + +# Clean up the ACP server subprocess +agent.close() + +print("Done!") diff --git a/openhands-sdk/openhands/sdk/agent/__init__.py b/openhands-sdk/openhands/sdk/agent/__init__.py index 6fd3a74487..8502579f99 100644 --- a/openhands-sdk/openhands/sdk/agent/__init__.py +++ b/openhands-sdk/openhands/sdk/agent/__init__.py @@ -2,7 +2,16 @@ from openhands.sdk.agent.base import AgentBase +def __getattr__(name: str): + if name == "ACPAgent": + from openhands.sdk.agent.acp_agent import ACPAgent + + return ACPAgent + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + __all__ = [ "Agent", "AgentBase", + "ACPAgent", ] diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py new file mode 100644 index 0000000000..ee4793b0a8 --- /dev/null +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -0,0 +1,503 @@ +"""ACPAgent — an AgentBase subclass that delegates to an ACP server. + +The Agent Client Protocol (ACP) lets OpenHands power conversations using +ACP-compatible servers (Claude Code, Gemini CLI, etc.) instead of direct +LLM calls. The ACP server manages its own LLM, tools, and execution; +the ACPAgent simply relays user messages and collects the response. + +See https://agentclientprotocol.com/protocol/overview +""" + +from __future__ import annotations + +import os +from collections.abc import Generator +from typing import TYPE_CHECKING, Any + +from pydantic import Field, PrivateAttr + +from openhands.sdk.agent.base import AgentBase +from openhands.sdk.conversation.state import ConversationExecutionStatus +from openhands.sdk.event import MessageEvent, SystemPromptEvent +from openhands.sdk.llm import LLM, Message, TextContent +from openhands.sdk.logger import get_logger +from openhands.sdk.tool import Tool # noqa: TC002 + + +if TYPE_CHECKING: + from openhands.sdk.conversation import ( + ConversationCallbackType, + ConversationState, + ConversationTokenCallbackType, + LocalConversation, + ) + + +logger = get_logger(__name__) + + +def _make_sentinel_llm() -> LLM: + """Create a sentinel LLM that should never be called.""" + return LLM(model="acp-managed") + + +# --------------------------------------------------------------------------- +# ACP Client implementation +# --------------------------------------------------------------------------- + + +async def _filter_jsonrpc_lines( + source: Any, dest: Any +) -> None: + """Read lines from *source* and forward only JSON-RPC lines to *dest*. + + Some ACP servers (e.g. ``claude-code-acp`` v0.1.x) emit log messages + like ``[ACP] ...`` to stdout alongside JSON-RPC traffic. This coroutine + strips those non-protocol lines so the JSON-RPC connection is not confused. + """ + try: + while True: + line = await source.readline() + if not line: + dest.feed_eof() + break + # JSON-RPC messages are single-line JSON objects containing + # "jsonrpc". Filter out multi-line pretty-printed JSON from + # debug logs that also start with '{'. + stripped = line.lstrip() + if stripped.startswith(b"{") and b'"jsonrpc"' in line: + dest.feed_data(line) + else: + # Log non-JSON lines at debug level + try: + logger.debug( + "ACP stdout (non-JSON): %s", + line.decode(errors="replace").rstrip(), + ) + except Exception: + pass + except Exception: + dest.feed_eof() + + +class _OpenHandsACPClient: + """ACP Client that accumulates session updates and emits OpenHands events. + + Implements the ``Client`` protocol from ``agent_client_protocol``. + """ + + def __init__(self) -> None: + self.accumulated_text: list[str] = [] + self.accumulated_thoughts: list[str] = [] + self.on_token: Any = None # ConversationTokenCallbackType | None + + def reset(self) -> None: + self.accumulated_text.clear() + self.accumulated_thoughts.clear() + self.on_token = None + + # -- Client protocol methods ------------------------------------------ + + async def session_update( + self, + session_id: str, # noqa: ARG002 + update: Any, + **kwargs: Any, # noqa: ARG002 + ) -> None: + from acp.schema import ( + AgentMessageChunk, + AgentThoughtChunk, + TextContentBlock, + ToolCallProgress, + ToolCallStart, + ) + + if isinstance(update, AgentMessageChunk): + if isinstance(update.content, TextContentBlock): + text = update.content.text + self.accumulated_text.append(text) + if self.on_token is not None: + try: + self.on_token(text) + except Exception: + pass + elif isinstance(update, AgentThoughtChunk): + if isinstance(update.content, TextContentBlock): + self.accumulated_thoughts.append(update.content.text) + elif isinstance(update, (ToolCallStart, ToolCallProgress)): + logger.debug("ACP tool call event: %s", type(update).__name__) + else: + logger.debug("ACP session update: %s", type(update).__name__) + + async def request_permission( + self, + options: list[Any], + session_id: str, # noqa: ARG002 + tool_call: Any, # noqa: ARG002 + **kwargs: Any, # noqa: ARG002 + ) -> Any: + """Auto-approve all permission requests from the ACP server.""" + from acp.schema import AllowedOutcome, RequestPermissionResponse + + # Pick the first option (usually "allow once") + option_id = options[0].option_id if options else "allow_once" + return RequestPermissionResponse( + result=AllowedOutcome(outcome="selected", option_id=option_id), + ) + + # fs/terminal methods — raise NotImplementedError; ACP server handles its own + async def write_text_file(self, **kwargs: Any) -> None: + raise NotImplementedError("ACP server handles file operations") + + async def read_text_file(self, **kwargs: Any) -> Any: + raise NotImplementedError("ACP server handles file operations") + + async def create_terminal(self, **kwargs: Any) -> Any: + raise NotImplementedError("ACP server handles terminal operations") + + async def terminal_output(self, **kwargs: Any) -> Any: + raise NotImplementedError("ACP server handles terminal operations") + + async def release_terminal(self, **kwargs: Any) -> None: + raise NotImplementedError("ACP server handles terminal operations") + + async def wait_for_terminal_exit(self, **kwargs: Any) -> Any: + raise NotImplementedError("ACP server handles terminal operations") + + async def kill_terminal(self, **kwargs: Any) -> None: + raise NotImplementedError("ACP server handles terminal operations") + + async def ext_method( + self, + method: str, # noqa: ARG002 + params: dict[str, Any], # noqa: ARG002 + ) -> dict[str, Any]: + return {} + + async def ext_notification( + self, + method: str, # noqa: ARG002 + params: dict[str, Any], # noqa: ARG002 + ) -> None: + pass + + def on_connect(self, conn: Any) -> None: # noqa: ARG002 + pass + + +# --------------------------------------------------------------------------- +# ACPAgent +# --------------------------------------------------------------------------- + + +class ACPAgent(AgentBase): + """Agent that delegates to an ACP (Agent Client Protocol) server. + + Instead of calling an LLM directly, this agent spawns an ACP-compatible + server (e.g. ``claude-code-acp``) as a subprocess and communicates with + it via the ACP protocol. The server manages its own LLM, tools, and + execution lifecycle. + + Example:: + + from openhands.sdk.agent import ACPAgent + from openhands.sdk.conversation import Conversation + + agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) + conversation = Conversation(agent=agent, workspace="./workspace") + conversation.send_message("Hello! What is 2+2?") + conversation.run() + """ + + # Override required fields with ACP-appropriate defaults + llm: LLM = Field(default_factory=_make_sentinel_llm) + tools: list[Tool] = Field(default_factory=list) + include_default_tools: list[str] = Field(default_factory=list) + + # ACP-specific configuration + acp_command: list[str] = Field( + ..., + description=( + "Command to start the ACP server, " + "e.g. ['npx', '-y', 'claude-code-acp']" + ), + ) + acp_args: list[str] = Field( + default_factory=list, + description="Additional arguments for the ACP server command", + ) + acp_env: dict[str, str] = Field( + default_factory=dict, + description="Additional environment variables for the ACP server process", + ) + + # Private runtime state + _executor: Any = PrivateAttr(default=None) + _conn: Any = PrivateAttr(default=None) # ClientSideConnection + _session_id: str | None = PrivateAttr(default=None) + _process: Any = PrivateAttr(default=None) # asyncio subprocess + _client: Any = PrivateAttr(default=None) # _OpenHandsACPClient + _filtered_reader: Any = PrivateAttr(default=None) # StreamReader + _closed: bool = PrivateAttr(default=False) + + # -- Override base properties to be no-ops for ACP --------------------- + + @property + def system_message(self) -> str: + return "ACP-managed agent" + + def get_all_llms(self) -> Generator[LLM, None, None]: + yield from () + + # -- Lifecycle --------------------------------------------------------- + + def init_state( + self, + state: ConversationState, + on_event: ConversationCallbackType, + ) -> None: + """Spawn the ACP server and initialize a session.""" + # Lazy import guard + try: + from acp import spawn_agent_process # noqa: F401 + except ImportError: + raise ImportError( + "The 'agent-client-protocol' package is required for ACPAgent. " + "Install it with: pip install 'openhands-sdk[acp]' or " + "pip install agent-client-protocol" + ) from None + + # Validate no unsupported features + if self.tools: + raise NotImplementedError( + "ACPAgent does not support custom tools; " + "the ACP server manages its own tools" + ) + if self.mcp_config: + raise NotImplementedError( + "ACPAgent does not support mcp_config; " + "configure MCP on the ACP server instead" + ) + if self.condenser is not None: + raise NotImplementedError( + "ACPAgent does not support condenser; " + "the ACP server manages its own context" + ) + if self.critic is not None: + raise NotImplementedError( + "ACPAgent does not support critic; " + "the ACP server manages its own evaluation" + ) + if self.agent_context is not None: + raise NotImplementedError( + "ACPAgent does not support agent_context; " + "configure the ACP server directly" + ) + + from openhands.sdk.utils.async_executor import AsyncExecutor + + self._executor = AsyncExecutor() + + try: + self._start_acp_server(state) + except Exception as e: + logger.error("Failed to start ACP server: %s", e) + self._cleanup() + raise + + # Emit a minimal SystemPromptEvent + event = SystemPromptEvent( + source="agent", + system_prompt=TextContent(text="ACP-managed agent"), + tools=[], + ) + on_event(event) + self._initialized = True + + def _start_acp_server(self, state: ConversationState) -> None: + """Start the ACP subprocess and initialize the session.""" + import asyncio + + from acp.client.connection import ClientSideConnection + from acp.transports import default_environment + + client = _OpenHandsACPClient() + self._client = client + + # Build environment: inherit current env + ACP extras + env = default_environment() + env.update(os.environ) + env.update(self.acp_env) + + command = self.acp_command[0] + args = list(self.acp_command[1:]) + list(self.acp_args) + + working_dir = str(state.workspace.working_dir) + + async def _init() -> tuple[Any, Any, Any, str]: + # Spawn the subprocess directly so we can install a + # filtering reader that skips non-JSON-RPC lines some + # ACP servers (e.g. claude-code-acp v0.1.x) write to + # stdout. + process = await asyncio.create_subprocess_exec( + command, + *args, + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + env=env, + ) + assert process.stdin is not None + assert process.stdout is not None + + # Wrap the subprocess stdout in a filtering reader that + # only passes lines starting with '{' (JSON-RPC messages). + filtered_reader = asyncio.StreamReader() + asyncio.get_event_loop().create_task( + _filter_jsonrpc_lines(process.stdout, filtered_reader) + ) + + conn = ClientSideConnection( + client, + process.stdin, # write to subprocess + filtered_reader, # read filtered output + ) + + # Initialize the protocol + await conn.initialize(protocol_version=1) + + # Create a new session + response = await conn.new_session(cwd=working_dir) + session_id = response.session_id + + return conn, process, filtered_reader, session_id + + result = self._executor.run_async(_init) + self._conn, self._process, self._filtered_reader, self._session_id = result + + def step( + self, + conversation: LocalConversation, + on_event: ConversationCallbackType, + on_token: ConversationTokenCallbackType | None = None, + ) -> None: + """Send the latest user message to the ACP server and emit the response.""" + state = conversation.state + + # Find the latest user message + user_message = None + for event in reversed(list(state.events)): + if isinstance(event, MessageEvent) and event.source == "user": + # Extract text from the message + for content in event.llm_message.content: + if isinstance(content, TextContent) and content.text.strip(): + user_message = content.text + break + if user_message: + break + + if user_message is None: + logger.warning("No user message found; finishing conversation") + state.execution_status = ConversationExecutionStatus.FINISHED + return + + # Reset client accumulators + self._client.reset() + self._client.on_token = on_token + + try: + import asyncio + + from acp.helpers import text_block + + async def _prompt() -> None: + await self._conn.prompt( + [text_block(user_message)], + self._session_id, + ) + # Allow pending session_update notifications to be + # processed before we read accumulated text. The ACP + # server sends notifications and responses through the + # same connection, but notification handlers run as + # tasks and may not have completed when prompt() returns. + await asyncio.sleep(0.1) + + # Send prompt to ACP server + self._executor.run_async(_prompt) + + # Build response message + response_text = "".join(self._client.accumulated_text) + thought_text = "".join(self._client.accumulated_thoughts) + + if not response_text: + response_text = "(No response from ACP server)" + + message = Message( + role="assistant", + content=[TextContent(text=response_text)], + reasoning_content=thought_text if thought_text else None, + ) + + msg_event = MessageEvent( + source="agent", + llm_message=message, + ) + on_event(msg_event) + state.execution_status = ConversationExecutionStatus.FINISHED + + except Exception as e: + logger.error("ACP prompt failed: %s", e, exc_info=True) + # Emit error as an agent message since AgentErrorEvent requires + # tool context we don't have + error_message = Message( + role="assistant", + content=[TextContent(text=f"ACP error: {e}")], + ) + error_event = MessageEvent( + source="agent", + llm_message=error_message, + ) + on_event(error_event) + state.execution_status = ConversationExecutionStatus.ERROR + + def close(self) -> None: + """Terminate the ACP subprocess and clean up resources.""" + if self._closed: + return + self._closed = True + self._cleanup() + + def _cleanup(self) -> None: + """Internal cleanup of ACP resources.""" + # Close the connection first + if self._conn is not None and self._executor is not None: + try: + self._executor.run_async(self._conn.close()) + except Exception as e: + logger.debug("Error closing ACP connection: %s", e) + self._conn = None + + # Terminate the subprocess + if self._process is not None: + try: + self._process.terminate() + except Exception: + pass + try: + self._process.kill() + except Exception: + pass + self._process = None + + if self._executor is not None: + try: + self._executor.close() + except Exception: + pass + self._executor = None + + def __del__(self) -> None: + try: + self.close() + except Exception: + pass diff --git a/openhands-sdk/pyproject.toml b/openhands-sdk/pyproject.toml index 3817f9c769..f4d3debfa2 100644 --- a/openhands-sdk/pyproject.toml +++ b/openhands-sdk/pyproject.toml @@ -26,6 +26,7 @@ Documentation = "https://docs.openhands.dev/sdk" [project.optional-dependencies] boto3 = ["boto3>=1.35.0"] +acp = ["agent-client-protocol>=0.8.1"] [build-system] requires = ["setuptools>=61.0", "wheel"] diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py new file mode 100644 index 0000000000..d3f16ba386 --- /dev/null +++ b/tests/sdk/agent/test_acp_agent.py @@ -0,0 +1,562 @@ +"""Tests for ACPAgent.""" + +from __future__ import annotations + +import asyncio +import json +import uuid +from unittest.mock import MagicMock, patch + +import pytest + +from openhands.sdk.agent.acp_agent import ACPAgent, _OpenHandsACPClient +from openhands.sdk.agent.base import AgentBase +from openhands.sdk.conversation.state import ( + ConversationExecutionStatus, + ConversationState, +) +from openhands.sdk.event import MessageEvent, SystemPromptEvent +from openhands.sdk.llm import Message, TextContent +from openhands.sdk.workspace.local import LocalWorkspace + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_agent(**kwargs) -> ACPAgent: + return ACPAgent(acp_command=["echo", "test"], **kwargs) + + +def _make_state(tmp_path) -> ConversationState: + agent = _make_agent() + workspace = LocalWorkspace(working_dir=str(tmp_path)) + return ConversationState.create( + id=uuid.uuid4(), + agent=agent, + workspace=workspace, + ) + + +# --------------------------------------------------------------------------- +# Instantiation +# --------------------------------------------------------------------------- + + +class TestACPAgentInstantiation: + def test_creates_with_sentinel_llm(self): + agent = _make_agent() + assert agent.llm.model == "acp-managed" + + def test_creates_with_empty_tools(self): + agent = _make_agent() + assert agent.tools == [] + + def test_creates_with_empty_default_tools(self): + agent = _make_agent() + assert agent.include_default_tools == [] + + def test_requires_acp_command(self): + with pytest.raises(Exception): + ACPAgent() # type: ignore[call-arg] + + def test_acp_command_stored(self): + agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) + assert agent.acp_command == ["npx", "-y", "claude-code-acp"] + + def test_acp_args_default_empty(self): + agent = _make_agent() + assert agent.acp_args == [] + + def test_acp_env_default_empty(self): + agent = _make_agent() + assert agent.acp_env == {} + + def test_system_message_returns_acp_managed(self): + agent = _make_agent() + assert agent.system_message == "ACP-managed agent" + + def test_get_all_llms_yields_nothing(self): + agent = _make_agent() + assert list(agent.get_all_llms()) == [] + + def test_agent_is_frozen(self): + agent = _make_agent() + with pytest.raises(Exception): + agent.acp_command = ["other"] # type: ignore[misc] + + +# --------------------------------------------------------------------------- +# Serialization +# --------------------------------------------------------------------------- + + +class TestACPAgentSerialization: + def test_kind_is_acp_agent(self): + agent = _make_agent() + data = json.loads(agent.model_dump_json()) + assert data["kind"] == "ACPAgent" + + def test_roundtrip_serialization(self): + agent = ACPAgent( + acp_command=["npx", "-y", "claude-code-acp"], + acp_args=["--verbose"], + acp_env={"FOO": "bar"}, + ) + dumped = agent.model_dump_json() + restored = AgentBase.model_validate_json(dumped) + assert isinstance(restored, ACPAgent) + assert restored.acp_command == agent.acp_command + assert restored.acp_args == agent.acp_args + assert restored.acp_env == agent.acp_env + + def test_deserialization_from_dict(self): + data = { + "kind": "ACPAgent", + "acp_command": ["echo", "test"], + } + agent = AgentBase.model_validate(data) + assert isinstance(agent, ACPAgent) + assert agent.acp_command == ["echo", "test"] + + +# --------------------------------------------------------------------------- +# Feature validation (init_state guards) +# --------------------------------------------------------------------------- + + +class TestACPAgentValidation: + """Test that unsupported features raise NotImplementedError in init_state.""" + + def _init_with_patches(self, agent, tmp_path): + """Call init_state with ACP SDK mocked out.""" + state = _make_state(tmp_path) + events = [] + with ( + patch("openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server"), + patch( + "openhands.sdk.utils.async_executor.AsyncExecutor", + return_value=MagicMock(), + ), + ): + agent.init_state(state, on_event=events.append) + return events + + def test_rejects_mcp_config(self, tmp_path): + agent = ACPAgent( + acp_command=["echo"], + mcp_config={"mcpServers": {"test": {"command": "echo"}}}, + ) + with pytest.raises(NotImplementedError, match="mcp_config"): + self._init_with_patches(agent, tmp_path) + + +# --------------------------------------------------------------------------- +# init_state +# --------------------------------------------------------------------------- + + +class TestACPAgentInitState: + def test_emits_system_prompt_event(self, tmp_path): + agent = _make_agent() + state = _make_state(tmp_path) + events: list = [] + + with ( + patch( + "openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server" + ), + ): + agent.init_state(state, on_event=events.append) + + assert len(events) == 1 + assert isinstance(events[0], SystemPromptEvent) + assert events[0].system_prompt.text == "ACP-managed agent" + assert events[0].tools == [] + + def test_missing_acp_sdk_raises_import_error(self, tmp_path): + agent = _make_agent() + state = _make_state(tmp_path) + + with patch.dict("sys.modules", {"acp": None}): + with pytest.raises(ImportError, match="agent-client-protocol"): + agent.init_state(state, on_event=lambda _: None) + + +# --------------------------------------------------------------------------- +# _OpenHandsACPClient +# --------------------------------------------------------------------------- + + +class TestOpenHandsACPClient: + def test_reset_clears_state(self): + client = _OpenHandsACPClient() + client.accumulated_text.append("hello") + client.accumulated_thoughts.append("thinking") + client.on_token = lambda _: None + + client.reset() + + assert client.accumulated_text == [] + assert client.accumulated_thoughts == [] + assert client.on_token is None + + @pytest.mark.asyncio + async def test_session_update_accumulates_text(self): + client = _OpenHandsACPClient() + client.accumulated_text.append("Hello") + client.accumulated_text.append(" World") + assert "".join(client.accumulated_text) == "Hello World" + + @pytest.mark.asyncio + async def test_session_update_accumulates_thoughts(self): + client = _OpenHandsACPClient() + client.accumulated_thoughts.append("Let me think") + client.accumulated_thoughts.append(" about this") + assert "".join(client.accumulated_thoughts) == "Let me think about this" + + def test_on_token_callback(self): + client = _OpenHandsACPClient() + tokens: list[str] = [] + client.on_token = tokens.append + + # Simulate what session_update would do + text = "chunk1" + client.accumulated_text.append(text) + if client.on_token is not None: + client.on_token(text) + + assert tokens == ["chunk1"] + + @pytest.mark.asyncio + async def test_fs_methods_raise(self): + client = _OpenHandsACPClient() + with pytest.raises(NotImplementedError): + await client.write_text_file() + with pytest.raises(NotImplementedError): + await client.read_text_file() + + @pytest.mark.asyncio + async def test_terminal_methods_raise(self): + client = _OpenHandsACPClient() + with pytest.raises(NotImplementedError): + await client.create_terminal() + with pytest.raises(NotImplementedError): + await client.terminal_output() + with pytest.raises(NotImplementedError): + await client.release_terminal() + with pytest.raises(NotImplementedError): + await client.wait_for_terminal_exit() + with pytest.raises(NotImplementedError): + await client.kill_terminal() + + @pytest.mark.asyncio + async def test_ext_method_returns_empty_dict(self): + client = _OpenHandsACPClient() + result = await client.ext_method("test", {}) + assert result == {} + + @pytest.mark.asyncio + async def test_ext_notification_is_noop(self): + client = _OpenHandsACPClient() + await client.ext_notification("test", {}) # Should not raise + + +# --------------------------------------------------------------------------- +# step +# --------------------------------------------------------------------------- + + +class TestACPAgentStep: + def _make_conversation_with_message(self, tmp_path, text="Hello"): + """Create a mock conversation with a user message.""" + state = _make_state(tmp_path) + state.events.append( + SystemPromptEvent( + source="agent", + system_prompt=TextContent(text="ACP-managed agent"), + tools=[], + ) + ) + state.events.append( + MessageEvent( + source="user", + llm_message=Message( + role="user", content=[TextContent(text=text)] + ), + ) + ) + + conversation = MagicMock() + conversation.state = state + return conversation + + def test_step_emits_message_event(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + events: list = [] + + # Set up mocked runtime state — populate text *after* reset + # (step() calls client.reset() then run_async which populates text) + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("The answer is 4") + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + agent.step(conversation, on_event=events.append) + + assert len(events) == 1 + assert isinstance(events[0], MessageEvent) + assert events[0].source == "agent" + assert events[0].llm_message.content[0].text == "The answer is 4" + + def test_step_includes_reasoning(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + events: list = [] + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("4") + mock_client.accumulated_thoughts.append("I need to add 2+2") + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + agent.step(conversation, on_event=events.append) + + msg = events[0].llm_message + assert msg.reasoning_content == "I need to add 2+2" + + def test_step_sets_finished(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("done") + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + agent.step(conversation, on_event=lambda _: None) + + assert ( + conversation.state.execution_status + == ConversationExecutionStatus.FINISHED + ) + + def test_step_no_user_message_finishes(self, tmp_path): + agent = _make_agent() + state = _make_state(tmp_path) + # No user message added + + conversation = MagicMock() + conversation.state = state + + agent._client = _OpenHandsACPClient() + + agent.step(conversation, on_event=lambda _: None) + + assert state.execution_status == ConversationExecutionStatus.FINISHED + + def test_step_error_sets_error_status(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + events: list = [] + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + mock_executor = MagicMock() + mock_executor.run_async = MagicMock(side_effect=RuntimeError("boom")) + agent._executor = mock_executor + + agent.step(conversation, on_event=events.append) + + assert ( + conversation.state.execution_status + == ConversationExecutionStatus.ERROR + ) + assert len(events) == 1 + assert "ACP error: boom" in events[0].llm_message.content[0].text + + def test_step_no_response_text_fallback(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + events: list = [] + + mock_client = _OpenHandsACPClient() + # accumulated_text stays empty — run_async is a no-op + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + mock_executor = MagicMock() + mock_executor.run_async = lambda _coro: None + agent._executor = mock_executor + + agent.step(conversation, on_event=events.append) + + assert "(No response from ACP server)" in events[0].llm_message.content[0].text + + def test_step_passes_on_token(self, tmp_path): + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("ok") + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + on_token = MagicMock() + + agent.step(conversation, on_event=lambda _: None, on_token=on_token) + + # Verify on_token was passed to the client + assert mock_client.on_token == on_token + + +# --------------------------------------------------------------------------- +# Cleanup +# --------------------------------------------------------------------------- + + +class TestACPAgentCleanup: + def test_close_terminates_process(self): + agent = _make_agent() + mock_process = MagicMock() + agent._process = mock_process + agent._executor = MagicMock() + agent._conn = None + + agent.close() + + mock_process.terminate.assert_called_once() + mock_process.kill.assert_called_once() + + def test_close_is_idempotent(self): + agent = _make_agent() + mock_process = MagicMock() + agent._process = mock_process + agent._executor = MagicMock() + agent._conn = None + + agent.close() + agent.close() # Second call should be a no-op + + # terminate/kill should only be called once + mock_process.terminate.assert_called_once() + + def test_close_closes_executor(self): + agent = _make_agent() + mock_executor = MagicMock() + agent._executor = mock_executor + agent._process = None + agent._conn = None + + agent.close() + + mock_executor.close.assert_called_once() + + def test_close_handles_errors_gracefully(self): + agent = _make_agent() + mock_process = MagicMock() + mock_process.terminate.side_effect = OSError("already dead") + mock_process.kill.side_effect = OSError("already dead") + agent._process = mock_process + agent._executor = MagicMock() + agent._conn = None + + # Should not raise + agent.close() + + +# --------------------------------------------------------------------------- +# _filter_jsonrpc_lines +# --------------------------------------------------------------------------- + + +class TestFilterJsonrpcLines: + @pytest.mark.asyncio + async def test_passes_jsonrpc_lines(self): + from openhands.sdk.agent.acp_agent import _filter_jsonrpc_lines + + source = asyncio.StreamReader() + dest = asyncio.StreamReader() + + jsonrpc_line = b'{"jsonrpc":"2.0","method":"test"}\n' + source.feed_data(jsonrpc_line) + source.feed_eof() + + await _filter_jsonrpc_lines(source, dest) + + result = await dest.readline() + assert result == jsonrpc_line + + @pytest.mark.asyncio + async def test_filters_non_jsonrpc_lines(self): + from openhands.sdk.agent.acp_agent import _filter_jsonrpc_lines + + source = asyncio.StreamReader() + dest = asyncio.StreamReader() + + source.feed_data(b"[ACP] Starting server...\n") + source.feed_data(b'{"jsonrpc":"2.0","id":1}\n') + source.feed_data(b"Some debug output\n") + source.feed_eof() + + await _filter_jsonrpc_lines(source, dest) + + result = await dest.readline() + assert b'"jsonrpc"' in result + + # Should get EOF next (non-JSON lines were filtered) + result2 = await dest.readline() + assert result2 == b"" + + @pytest.mark.asyncio + async def test_filters_pretty_printed_json(self): + from openhands.sdk.agent.acp_agent import _filter_jsonrpc_lines + + source = asyncio.StreamReader() + dest = asyncio.StreamReader() + + # Pretty-printed JSON starts with { but doesn't contain "jsonrpc" + source.feed_data(b"{\n") + source.feed_data(b' "type": "message"\n') + source.feed_data(b"}\n") + source.feed_eof() + + await _filter_jsonrpc_lines(source, dest) + + # Should only get EOF + result = await dest.readline() + assert result == b"" diff --git a/uv.lock b/uv.lock index 82ede5906f..95575a85ca 100644 --- a/uv.lock +++ b/uv.lock @@ -42,6 +42,18 @@ dev = [ { name = "tabulate", specifier = ">=0.9.0" }, ] +[[package]] +name = "agent-client-protocol" +version = "0.8.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pydantic" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/1b/7b/7cdac86db388809d9e3bc58cac88cc7dfa49b7615b98fab304a828cd7f8a/agent_client_protocol-0.8.1.tar.gz", hash = "sha256:1bbf15663bf51f64942597f638e32a6284c5da918055d9672d3510e965143dbd", size = 68866, upload-time = "2026-02-13T15:34:54.567Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4b/f3/219eeca0ad4a20843d4b9eaac5532f87018b9d25730a62a16f54f6c52d1a/agent_client_protocol-0.8.1-py3-none-any.whl", hash = "sha256:9421a11fd435b4831660272d169c3812d553bb7247049c138c3ca127e4b8af8e", size = 54529, upload-time = "2026-02-13T15:34:53.344Z" }, +] + [[package]] name = "aiofiles" version = "25.1.0" @@ -1191,6 +1203,7 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/44/69/9b804adb5fd0671f367781560eb5eb586c4d495277c93bde4307b9e28068/greenlet-3.2.4-cp312-cp312-macosx_11_0_universal2.whl", hash = "sha256:3b67ca49f54cede0186854a008109d6ee71f66bd57bb36abd6d0a0267b540cdd", size = 274079, upload-time = "2025-08-07T13:15:45.033Z" }, { url = "https://files.pythonhosted.org/packages/46/e9/d2a80c99f19a153eff70bc451ab78615583b8dac0754cfb942223d2c1a0d/greenlet-3.2.4-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:ddf9164e7a5b08e9d22511526865780a576f19ddd00d62f8a665949327fde8bb", size = 640997, upload-time = "2025-08-07T13:42:56.234Z" }, { url = "https://files.pythonhosted.org/packages/3b/16/035dcfcc48715ccd345f3a93183267167cdd162ad123cd93067d86f27ce4/greenlet-3.2.4-cp312-cp312-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:f28588772bb5fb869a8eb331374ec06f24a83a9c25bfa1f38b6993afe9c1e968", size = 655185, upload-time = "2025-08-07T13:45:27.624Z" }, + { url = "https://files.pythonhosted.org/packages/31/da/0386695eef69ffae1ad726881571dfe28b41970173947e7c558d9998de0f/greenlet-3.2.4-cp312-cp312-manylinux2014_s390x.manylinux_2_17_s390x.whl", hash = "sha256:5c9320971821a7cb77cfab8d956fa8e39cd07ca44b6070db358ceb7f8797c8c9", size = 649926, upload-time = "2025-08-07T13:53:15.251Z" }, { url = "https://files.pythonhosted.org/packages/68/88/69bf19fd4dc19981928ceacbc5fd4bb6bc2215d53199e367832e98d1d8fe/greenlet-3.2.4-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:c60a6d84229b271d44b70fb6e5fa23781abb5d742af7b808ae3f6efd7c9c60f6", size = 651839, upload-time = "2025-08-07T13:18:30.281Z" }, { url = "https://files.pythonhosted.org/packages/19/0d/6660d55f7373b2ff8152401a83e02084956da23ae58cddbfb0b330978fe9/greenlet-3.2.4-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:3b3812d8d0c9579967815af437d96623f45c0f2ae5f04e366de62a12d83a8fb0", size = 607586, upload-time = "2025-08-07T13:18:28.544Z" }, { url = "https://files.pythonhosted.org/packages/8e/1a/c953fdedd22d81ee4629afbb38d2f9d71e37d23caace44775a3a969147d4/greenlet-3.2.4-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:abbf57b5a870d30c4675928c37278493044d7c14378350b3aa5d484fa65575f0", size = 1123281, upload-time = "2025-08-07T13:42:39.858Z" }, @@ -1201,6 +1214,7 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/49/e8/58c7f85958bda41dafea50497cbd59738c5c43dbbea5ee83d651234398f4/greenlet-3.2.4-cp313-cp313-macosx_11_0_universal2.whl", hash = "sha256:1a921e542453fe531144e91e1feedf12e07351b1cf6c9e8a3325ea600a715a31", size = 272814, upload-time = "2025-08-07T13:15:50.011Z" }, { url = "https://files.pythonhosted.org/packages/62/dd/b9f59862e9e257a16e4e610480cfffd29e3fae018a68c2332090b53aac3d/greenlet-3.2.4-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:cd3c8e693bff0fff6ba55f140bf390fa92c994083f838fece0f63be121334945", size = 641073, upload-time = "2025-08-07T13:42:57.23Z" }, { url = "https://files.pythonhosted.org/packages/f7/0b/bc13f787394920b23073ca3b6c4a7a21396301ed75a655bcb47196b50e6e/greenlet-3.2.4-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:710638eb93b1fa52823aa91bf75326f9ecdfd5e0466f00789246a5280f4ba0fc", size = 655191, upload-time = "2025-08-07T13:45:29.752Z" }, + { url = "https://files.pythonhosted.org/packages/f2/d6/6adde57d1345a8d0f14d31e4ab9c23cfe8e2cd39c3baf7674b4b0338d266/greenlet-3.2.4-cp313-cp313-manylinux2014_s390x.manylinux_2_17_s390x.whl", hash = "sha256:c5111ccdc9c88f423426df3fd1811bfc40ed66264d35aa373420a34377efc98a", size = 649516, upload-time = "2025-08-07T13:53:16.314Z" }, { url = "https://files.pythonhosted.org/packages/7f/3b/3a3328a788d4a473889a2d403199932be55b1b0060f4ddd96ee7cdfcad10/greenlet-3.2.4-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:d76383238584e9711e20ebe14db6c88ddcedc1829a9ad31a584389463b5aa504", size = 652169, upload-time = "2025-08-07T13:18:32.861Z" }, { url = "https://files.pythonhosted.org/packages/ee/43/3cecdc0349359e1a527cbf2e3e28e5f8f06d3343aaf82ca13437a9aa290f/greenlet-3.2.4-cp313-cp313-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:23768528f2911bcd7e475210822ffb5254ed10d71f4028387e5a99b4c6699671", size = 610497, upload-time = "2025-08-07T13:18:31.636Z" }, { url = "https://files.pythonhosted.org/packages/b8/19/06b6cf5d604e2c382a6f31cafafd6f33d5dea706f4db7bdab184bad2b21d/greenlet-3.2.4-cp313-cp313-musllinux_1_1_aarch64.whl", hash = "sha256:00fadb3fedccc447f517ee0d3fd8fe49eae949e1cd0f6a611818f4f6fb7dc83b", size = 1121662, upload-time = "2025-08-07T13:42:41.117Z" }, @@ -1211,6 +1225,7 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/22/5c/85273fd7cc388285632b0498dbbab97596e04b154933dfe0f3e68156c68c/greenlet-3.2.4-cp314-cp314-macosx_11_0_universal2.whl", hash = "sha256:49a30d5fda2507ae77be16479bdb62a660fa51b1eb4928b524975b3bde77b3c0", size = 273586, upload-time = "2025-08-07T13:16:08.004Z" }, { url = "https://files.pythonhosted.org/packages/d1/75/10aeeaa3da9332c2e761e4c50d4c3556c21113ee3f0afa2cf5769946f7a3/greenlet-3.2.4-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:299fd615cd8fc86267b47597123e3f43ad79c9d8a22bebdce535e53550763e2f", size = 686346, upload-time = "2025-08-07T13:42:59.944Z" }, { url = "https://files.pythonhosted.org/packages/c0/aa/687d6b12ffb505a4447567d1f3abea23bd20e73a5bed63871178e0831b7a/greenlet-3.2.4-cp314-cp314-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:c17b6b34111ea72fc5a4e4beec9711d2226285f0386ea83477cbb97c30a3f3a5", size = 699218, upload-time = "2025-08-07T13:45:30.969Z" }, + { url = "https://files.pythonhosted.org/packages/dc/8b/29aae55436521f1d6f8ff4e12fb676f3400de7fcf27fccd1d4d17fd8fecd/greenlet-3.2.4-cp314-cp314-manylinux2014_s390x.manylinux_2_17_s390x.whl", hash = "sha256:b4a1870c51720687af7fa3e7cda6d08d801dae660f75a76f3845b642b4da6ee1", size = 694659, upload-time = "2025-08-07T13:53:17.759Z" }, { url = "https://files.pythonhosted.org/packages/92/2e/ea25914b1ebfde93b6fc4ff46d6864564fba59024e928bdc7de475affc25/greenlet-3.2.4-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:061dc4cf2c34852b052a8620d40f36324554bc192be474b9e9770e8c042fd735", size = 695355, upload-time = "2025-08-07T13:18:34.517Z" }, { url = "https://files.pythonhosted.org/packages/72/60/fc56c62046ec17f6b0d3060564562c64c862948c9d4bc8aa807cf5bd74f4/greenlet-3.2.4-cp314-cp314-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:44358b9bf66c8576a9f57a590d5f5d6e72fa4228b763d0e43fee6d3b06d3a337", size = 657512, upload-time = "2025-08-07T13:18:33.969Z" }, { url = "https://files.pythonhosted.org/packages/23/6e/74407aed965a4ab6ddd93a7ded3180b730d281c77b765788419484cdfeef/greenlet-3.2.4-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:2917bdf657f5859fbf3386b12d68ede4cf1f04c90c3a6bc1f013dd68a22e2269", size = 1612508, upload-time = "2025-11-04T12:42:23.427Z" }, @@ -2275,12 +2290,16 @@ dependencies = [ ] [package.optional-dependencies] +acp = [ + { name = "agent-client-protocol" }, +] boto3 = [ { name = "boto3" }, ] [package.metadata] requires-dist = [ + { name = "agent-client-protocol", marker = "extra == 'acp'", specifier = ">=0.8.1" }, { name = "boto3", marker = "extra == 'boto3'", specifier = ">=1.35.0" }, { name = "deprecation", specifier = ">=2.1.0" }, { name = "fastmcp", specifier = ">=2.11.3" }, @@ -2294,7 +2313,7 @@ requires-dist = [ { name = "tenacity", specifier = ">=9.1.2" }, { name = "websockets", specifier = ">=12" }, ] -provides-extras = ["boto3"] +provides-extras = ["boto3", "acp"] [[package]] name = "openhands-tools" From 0d2058ef0d1744fd5581de7903303e9ad656b606 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 19 Feb 2026 17:34:34 -0300 Subject: [PATCH 02/10] Add LLM telemetry to ACPAgent and address PR review feedback - Record per-turn token usage from PromptResponse.usage in step() - Record incremental cost from UsageUpdate notifications in session_update() - Yield sentinel LLM from get_all_llms() for telemetry pipeline - Wire _llm_ref on client for dynamic metrics/telemetry access - Trigger stats callback after each step for GUI updates - Extract notification drain delay to configurable constant - Add audit logging for auto-approved permission requests - Add debug logging in cleanup for process/executor errors - Wrap example in try/finally for robust cleanup - Add 8 telemetry tests in TestACPAgentTelemetry Co-Authored-By: Claude Opus 4.6 --- .../01_standalone_sdk/36_acp_agent_example.py | 24 +- .../openhands/sdk/agent/acp_agent.py | 75 +++++- tests/sdk/agent/test_acp_agent.py | 244 +++++++++++++++++- 3 files changed, 318 insertions(+), 25 deletions(-) diff --git a/examples/01_standalone_sdk/36_acp_agent_example.py b/examples/01_standalone_sdk/36_acp_agent_example.py index 42afab961b..3d7fc927e9 100644 --- a/examples/01_standalone_sdk/36_acp_agent_example.py +++ b/examples/01_standalone_sdk/36_acp_agent_example.py @@ -17,18 +17,20 @@ from openhands.sdk.agent import ACPAgent from openhands.sdk.conversation import Conversation -agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) - -cwd = os.getcwd() -conversation = Conversation(agent=agent, workspace=cwd) -conversation.send_message( - "List the Python source files under openhands-sdk/openhands/sdk/agent/, " - "then read the __init__.py and summarize what agent classes are exported." -) -conversation.run() +agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) -# Clean up the ACP server subprocess -agent.close() +try: + cwd = os.getcwd() + conversation = Conversation(agent=agent, workspace=cwd) + + conversation.send_message( + "List the Python source files under openhands-sdk/openhands/sdk/agent/, " + "then read the __init__.py and summarize what agent classes are exported." + ) + conversation.run() +finally: + # Clean up the ACP server subprocess + agent.close() print("Done!") diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index ee4793b0a8..dbb396cd31 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -35,6 +35,10 @@ logger = get_logger(__name__) +# Seconds to wait after prompt() for pending session_update notifications +# to be processed. Increase if using a slow or remote ACP server. +_NOTIFICATION_DRAIN_DELAY: float = 0.1 + def _make_sentinel_llm() -> LLM: """Create a sentinel LLM that should never be called.""" @@ -90,11 +94,17 @@ def __init__(self) -> None: self.accumulated_text: list[str] = [] self.accumulated_thoughts: list[str] = [] self.on_token: Any = None # ConversationTokenCallbackType | None + # Telemetry state from UsageUpdate (persists across turns) + self._last_cost: float = 0.0 # last cumulative cost seen + self._context_window: int = 0 # context window size from ACP + self._llm_ref: Any = None # reference to the sentinel LLM def reset(self) -> None: self.accumulated_text.clear() self.accumulated_thoughts.clear() self.on_token = None + # Note: telemetry state (_last_cost, _context_window, etc.) + # is intentionally NOT cleared — it accumulates across turns. # -- Client protocol methods ------------------------------------------ @@ -110,6 +120,7 @@ async def session_update( TextContentBlock, ToolCallProgress, ToolCallStart, + UsageUpdate, ) if isinstance(update, AgentMessageChunk): @@ -124,6 +135,15 @@ async def session_update( elif isinstance(update, AgentThoughtChunk): if isinstance(update.content, TextContentBlock): self.accumulated_thoughts.append(update.content.text) + elif isinstance(update, UsageUpdate): + # Update context window size + self._context_window = update.size + # Record incremental cost + if update.cost is not None and self._llm_ref is not None: + delta = update.cost.amount - self._last_cost + if delta > 0: + self._llm_ref.metrics.add_cost(delta) + self._last_cost = update.cost.amount elif isinstance(update, (ToolCallStart, ToolCallProgress)): logger.debug("ACP tool call event: %s", type(update).__name__) else: @@ -133,7 +153,7 @@ async def request_permission( self, options: list[Any], session_id: str, # noqa: ARG002 - tool_call: Any, # noqa: ARG002 + tool_call: Any, **kwargs: Any, # noqa: ARG002 ) -> Any: """Auto-approve all permission requests from the ACP server.""" @@ -141,6 +161,11 @@ async def request_permission( # Pick the first option (usually "allow once") option_id = options[0].option_id if options else "allow_once" + logger.info( + "ACP auto-approving permission: %s (option: %s)", + tool_call, + option_id, + ) return RequestPermissionResponse( result=AllowedOutcome(outcome="selected", option_id=option_id), ) @@ -247,7 +272,7 @@ def system_message(self) -> str: return "ACP-managed agent" def get_all_llms(self) -> Generator[LLM, None, None]: - yield from () + yield self.llm # -- Lifecycle --------------------------------------------------------- @@ -322,6 +347,7 @@ def _start_acp_server(self, state: ConversationState) -> None: from acp.transports import default_environment client = _OpenHandsACPClient() + client._llm_ref = self.llm self._client = client # Build environment: inherit current env + ACP extras @@ -410,8 +436,8 @@ def step( from acp.helpers import text_block - async def _prompt() -> None: - await self._conn.prompt( + async def _prompt() -> Any: + response = await self._conn.prompt( [text_block(user_message)], self._session_id, ) @@ -420,10 +446,35 @@ async def _prompt() -> None: # server sends notifications and responses through the # same connection, but notification handlers run as # tasks and may not have completed when prompt() returns. - await asyncio.sleep(0.1) + await asyncio.sleep(_NOTIFICATION_DRAIN_DELAY) + return response # Send prompt to ACP server - self._executor.run_async(_prompt) + response = self._executor.run_async(_prompt) + + # Record per-turn token usage from PromptResponse + if ( + response is not None + and hasattr(response, "usage") + and response.usage is not None + ): + usage = response.usage + self.llm.metrics.add_token_usage( + prompt_tokens=usage.input_tokens, + completion_tokens=usage.output_tokens, + cache_read_tokens=usage.cached_read_tokens or 0, + cache_write_tokens=usage.cached_write_tokens or 0, + reasoning_tokens=usage.thought_tokens or 0, + context_window=self._client._context_window, + response_id=self._session_id or "", + ) + + # Notify stats callback + if self.llm.telemetry._stats_update_callback is not None: + try: + self.llm.telemetry._stats_update_callback() + except Exception: + pass # Build response message response_text = "".join(self._client.accumulated_text) @@ -481,19 +532,19 @@ def _cleanup(self) -> None: if self._process is not None: try: self._process.terminate() - except Exception: - pass + except Exception as e: + logger.debug("Error terminating ACP process: %s", e) try: self._process.kill() - except Exception: - pass + except Exception as e: + logger.debug("Error killing ACP process: %s", e) self._process = None if self._executor is not None: try: self._executor.close() - except Exception: - pass + except Exception as e: + logger.debug("Error closing executor: %s", e) self._executor = None def __del__(self) -> None: diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index d3f16ba386..86be43c419 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -77,9 +77,11 @@ def test_system_message_returns_acp_managed(self): agent = _make_agent() assert agent.system_message == "ACP-managed agent" - def test_get_all_llms_yields_nothing(self): + def test_get_all_llms_yields_sentinel(self): agent = _make_agent() - assert list(agent.get_all_llms()) == [] + llms = list(agent.get_all_llms()) + assert len(llms) == 1 + assert llms[0].model == "acp-managed" def test_agent_is_frozen(self): agent = _make_agent() @@ -560,3 +562,241 @@ async def test_filters_pretty_printed_json(self): # Should only get EOF result = await dest.readline() assert result == b"" + + +# --------------------------------------------------------------------------- +# Telemetry +# --------------------------------------------------------------------------- + + +class TestACPAgentTelemetry: + def _make_conversation_with_message(self, tmp_path, text="Hello"): + """Create a mock conversation with a user message.""" + state = _make_state(tmp_path) + state.events.append( + SystemPromptEvent( + source="agent", + system_prompt=TextContent(text="ACP-managed agent"), + tools=[], + ) + ) + state.events.append( + MessageEvent( + source="user", + llm_message=Message( + role="user", content=[TextContent(text=text)] + ), + ) + ) + + conversation = MagicMock() + conversation.state = state + return conversation + + def test_get_all_llms_yields_sentinel(self): + """get_all_llms() yields the sentinel LLM for telemetry.""" + agent = _make_agent() + llms = list(agent.get_all_llms()) + assert len(llms) == 1 + assert llms[0] is agent.llm + assert llms[0].model == "acp-managed" + + def test_step_records_token_usage(self, tmp_path): + """step() records per-turn token usage from PromptResponse.usage.""" + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + + mock_client = _OpenHandsACPClient() + mock_client._context_window = 200000 + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + # Build a mock PromptResponse with usage + mock_usage = MagicMock() + mock_usage.input_tokens = 100 + mock_usage.output_tokens = 50 + mock_usage.cached_read_tokens = 10 + mock_usage.cached_write_tokens = 5 + mock_usage.thought_tokens = 20 + + mock_response = MagicMock() + mock_response.usage = mock_usage + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("response text") + return mock_response + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + agent.step(conversation, on_event=lambda _: None) + + # Verify token usage was recorded + metrics = agent.llm.metrics + assert len(metrics.token_usages) == 1 + usage = metrics.token_usages[0] + assert usage.prompt_tokens == 100 + assert usage.completion_tokens == 50 + assert usage.cache_read_tokens == 10 + assert usage.cache_write_tokens == 5 + assert usage.reasoning_tokens == 20 + assert usage.context_window == 200000 + + def test_step_handles_no_usage(self, tmp_path): + """step() handles PromptResponse with no usage gracefully.""" + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + mock_response = MagicMock() + mock_response.usage = None + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("response") + return mock_response + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + agent.step(conversation, on_event=lambda _: None) + + # No token usage should be recorded + assert len(agent.llm.metrics.token_usages) == 0 + + @pytest.mark.asyncio + async def test_usage_update_records_cost(self): + """UsageUpdate with cost records incremental cost via metrics.""" + from acp.schema import UsageUpdate + + from openhands.sdk.llm import LLM + + client = _OpenHandsACPClient() + llm = LLM(model="acp-managed") + client._llm_ref = llm + client._last_cost = 0.0 + + update = MagicMock(spec=UsageUpdate) + update.size = 128000 + update.cost = MagicMock() + update.cost.amount = 0.05 + + await client.session_update("sess-1", update) + + assert llm.metrics.accumulated_cost == pytest.approx(0.05) + assert client._last_cost == 0.05 + assert client._context_window == 128000 + + @pytest.mark.asyncio + async def test_usage_update_incremental_cost(self): + """UsageUpdate cost tracking is incremental (delta from last seen).""" + from acp.schema import UsageUpdate + + from openhands.sdk.llm import LLM + + client = _OpenHandsACPClient() + llm = LLM(model="acp-managed") + client._llm_ref = llm + + # First update: cost 0.05 + update1 = MagicMock(spec=UsageUpdate) + update1.size = 128000 + update1.cost = MagicMock() + update1.cost.amount = 0.05 + + await client.session_update("sess-1", update1) + assert llm.metrics.accumulated_cost == pytest.approx(0.05) + + # Second update: cumulative cost 0.12 → delta should be 0.07 + update2 = MagicMock(spec=UsageUpdate) + update2.size = 130000 + update2.cost = MagicMock() + update2.cost.amount = 0.12 + + await client.session_update("sess-1", update2) + assert llm.metrics.accumulated_cost == pytest.approx(0.12) + assert client._last_cost == 0.12 + + @pytest.mark.asyncio + async def test_usage_update_updates_context_window(self): + """UsageUpdate.size updates the client's _context_window.""" + from acp.schema import UsageUpdate + + client = _OpenHandsACPClient() + + update = MagicMock(spec=UsageUpdate) + update.size = 200000 + update.cost = None + + await client.session_update("sess-1", update) + + assert client._context_window == 200000 + + def test_stats_callback_invoked(self, tmp_path): + """After step(), the sentinel LLM's stats callback is invoked.""" + agent = _make_agent() + conversation = self._make_conversation_with_message(tmp_path) + + mock_client = _OpenHandsACPClient() + agent._client = mock_client + agent._conn = MagicMock() + agent._session_id = "test-session" + + mock_response = MagicMock() + mock_response.usage = None + + def _fake_run_async(_coro): + mock_client.accumulated_text.append("ok") + return mock_response + + mock_executor = MagicMock() + mock_executor.run_async = _fake_run_async + agent._executor = mock_executor + + # Set up a stats callback + callback = MagicMock() + agent.llm.telemetry._stats_update_callback = callback + + agent.step(conversation, on_event=lambda _: None) + + callback.assert_called_once() + + def test_start_acp_server_wires_llm_ref(self, tmp_path): + """_start_acp_server wires _llm_ref on the client.""" + agent = _make_agent() + state = _make_state(tmp_path) + + with patch( + "openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server" + ) as mock_start: + def fake_start(s): + client = _OpenHandsACPClient() + client._llm_ref = agent.llm + agent._client = client + mock_start.side_effect = fake_start + agent.init_state(state, on_event=lambda _: None) + + assert agent._client._llm_ref is agent.llm + + def test_reset_preserves_telemetry_state(self): + """reset() clears text/thoughts but preserves telemetry state.""" + client = _OpenHandsACPClient() + client._last_cost = 1.23 + client._context_window = 128000 + client._llm_ref = MagicMock() + client.accumulated_text.append("hello") + client.accumulated_thoughts.append("thinking") + + client.reset() + + assert client.accumulated_text == [] + assert client.accumulated_thoughts == [] + assert client._last_cost == 1.23 + assert client._context_window == 128000 + assert client._llm_ref is not None From 364fca1a15af129c2ab5cfc38fe716ecd6744c55 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 19 Feb 2026 17:44:25 -0300 Subject: [PATCH 03/10] Make notification drain delay configurable and document the race condition Add ACP_NOTIFICATION_DRAIN_DELAY env var override, yield to the event loop before the timed sleep, and add a TODO for protocol-level fix. Co-Authored-By: Claude Opus 4.6 --- .../openhands/sdk/agent/acp_agent.py | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index dbb396cd31..e0ac391138 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -36,8 +36,16 @@ logger = get_logger(__name__) # Seconds to wait after prompt() for pending session_update notifications -# to be processed. Increase if using a slow or remote ACP server. -_NOTIFICATION_DRAIN_DELAY: float = 0.1 +# to be processed. This is a best-effort workaround: the ACP protocol does +# not currently signal when all notifications for a turn have been delivered, +# so we yield to the event loop and then sleep briefly to allow in-flight +# handlers to finish. Override via ACP_NOTIFICATION_DRAIN_DELAY for slow or +# remote servers. +# TODO: Replace with protocol-level synchronization once ACP supports a +# "turn complete" sentinel notification. +_NOTIFICATION_DRAIN_DELAY: float = float( + os.environ.get("ACP_NOTIFICATION_DRAIN_DELAY", "0.1") +) def _make_sentinel_llm() -> LLM: @@ -441,11 +449,10 @@ async def _prompt() -> Any: [text_block(user_message)], self._session_id, ) - # Allow pending session_update notifications to be - # processed before we read accumulated text. The ACP - # server sends notifications and responses through the - # same connection, but notification handlers run as - # tasks and may not have completed when prompt() returns. + # Drain pending session_update notification handlers. + # First yield lets already-queued handlers run, then a + # short sleep covers handlers still arriving over IO. + await asyncio.sleep(0) await asyncio.sleep(_NOTIFICATION_DRAIN_DELAY) return response From db2b482647a4bdfe71c36bfbc65e0cf97e4e4e99 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 19 Feb 2026 19:53:45 -0300 Subject: [PATCH 04/10] Fix pre-commit: pyright type errors and ruff formatting - Fix RequestPermissionResponse constructor (outcome, not result) - Add proper signatures to Client protocol stub methods - Narrow content[0] type in tests with isinstance checks - Pass required args in fs/terminal stub tests Co-Authored-By: Claude Opus 4.6 --- .../openhands/sdk/agent/acp_agent.py | 51 ++++++++++++++----- tests/sdk/agent/test_acp_agent.py | 48 +++++++++-------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index e0ac391138..17ed58ee6b 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -58,9 +58,7 @@ def _make_sentinel_llm() -> LLM: # --------------------------------------------------------------------------- -async def _filter_jsonrpc_lines( - source: Any, dest: Any -) -> None: +async def _filter_jsonrpc_lines(source: Any, dest: Any) -> None: """Read lines from *source* and forward only JSON-RPC lines to *dest*. Some ACP servers (e.g. ``claude-code-acp`` v0.1.x) emit log messages @@ -175,29 +173,55 @@ async def request_permission( option_id, ) return RequestPermissionResponse( - result=AllowedOutcome(outcome="selected", option_id=option_id), + outcome=AllowedOutcome(outcome="selected", option_id=option_id), ) # fs/terminal methods — raise NotImplementedError; ACP server handles its own - async def write_text_file(self, **kwargs: Any) -> None: + async def write_text_file( + self, content: str, path: str, session_id: str, **kwargs: Any + ) -> None: raise NotImplementedError("ACP server handles file operations") - async def read_text_file(self, **kwargs: Any) -> Any: + async def read_text_file( + self, + path: str, + session_id: str, + limit: int | None = None, + line: int | None = None, + **kwargs: Any, + ) -> Any: raise NotImplementedError("ACP server handles file operations") - async def create_terminal(self, **kwargs: Any) -> Any: + async def create_terminal( + self, + command: str, + session_id: str, + args: list[str] | None = None, + cwd: str | None = None, + env: Any = None, + output_byte_limit: int | None = None, + **kwargs: Any, + ) -> Any: raise NotImplementedError("ACP server handles terminal operations") - async def terminal_output(self, **kwargs: Any) -> Any: + async def terminal_output( + self, session_id: str, terminal_id: str, **kwargs: Any + ) -> Any: raise NotImplementedError("ACP server handles terminal operations") - async def release_terminal(self, **kwargs: Any) -> None: + async def release_terminal( + self, session_id: str, terminal_id: str, **kwargs: Any + ) -> None: raise NotImplementedError("ACP server handles terminal operations") - async def wait_for_terminal_exit(self, **kwargs: Any) -> Any: + async def wait_for_terminal_exit( + self, session_id: str, terminal_id: str, **kwargs: Any + ) -> Any: raise NotImplementedError("ACP server handles terminal operations") - async def kill_terminal(self, **kwargs: Any) -> None: + async def kill_terminal( + self, session_id: str, terminal_id: str, **kwargs: Any + ) -> None: raise NotImplementedError("ACP server handles terminal operations") async def ext_method( @@ -251,8 +275,7 @@ class ACPAgent(AgentBase): acp_command: list[str] = Field( ..., description=( - "Command to start the ACP server, " - "e.g. ['npx', '-y', 'claude-code-acp']" + "Command to start the ACP server, e.g. ['npx', '-y', 'claude-code-acp']" ), ) acp_args: list[str] = Field( @@ -393,7 +416,7 @@ async def _init() -> tuple[Any, Any, Any, str]: conn = ClientSideConnection( client, - process.stdin, # write to subprocess + process.stdin, # write to subprocess filtered_reader, # read filtered output ) diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index 86be43c419..c8ffafcc54 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -166,9 +166,7 @@ def test_emits_system_prompt_event(self, tmp_path): events: list = [] with ( - patch( - "openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server" - ), + patch("openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server"), ): agent.init_state(state, on_event=events.append) @@ -235,23 +233,23 @@ def test_on_token_callback(self): async def test_fs_methods_raise(self): client = _OpenHandsACPClient() with pytest.raises(NotImplementedError): - await client.write_text_file() + await client.write_text_file("c", "/f", "s1") with pytest.raises(NotImplementedError): - await client.read_text_file() + await client.read_text_file("/f", "s1") @pytest.mark.asyncio async def test_terminal_methods_raise(self): client = _OpenHandsACPClient() with pytest.raises(NotImplementedError): - await client.create_terminal() + await client.create_terminal("bash", "s1") with pytest.raises(NotImplementedError): - await client.terminal_output() + await client.terminal_output("s1", "t1") with pytest.raises(NotImplementedError): - await client.release_terminal() + await client.release_terminal("s1", "t1") with pytest.raises(NotImplementedError): - await client.wait_for_terminal_exit() + await client.wait_for_terminal_exit("s1", "t1") with pytest.raises(NotImplementedError): - await client.kill_terminal() + await client.kill_terminal("s1", "t1") @pytest.mark.asyncio async def test_ext_method_returns_empty_dict(self): @@ -284,9 +282,7 @@ def _make_conversation_with_message(self, tmp_path, text="Hello"): state.events.append( MessageEvent( source="user", - llm_message=Message( - role="user", content=[TextContent(text=text)] - ), + llm_message=Message(role="user", content=[TextContent(text=text)]), ) ) @@ -318,7 +314,9 @@ def _fake_run_async(_coro): assert len(events) == 1 assert isinstance(events[0], MessageEvent) assert events[0].source == "agent" - assert events[0].llm_message.content[0].text == "The answer is 4" + content_block = events[0].llm_message.content[0] + assert isinstance(content_block, TextContent) + assert content_block.text == "The answer is 4" def test_step_includes_reasoning(self, tmp_path): agent = _make_agent() @@ -362,8 +360,7 @@ def _fake_run_async(_coro): agent.step(conversation, on_event=lambda _: None) assert ( - conversation.state.execution_status - == ConversationExecutionStatus.FINISHED + conversation.state.execution_status == ConversationExecutionStatus.FINISHED ) def test_step_no_user_message_finishes(self, tmp_path): @@ -396,12 +393,11 @@ def test_step_error_sets_error_status(self, tmp_path): agent.step(conversation, on_event=events.append) - assert ( - conversation.state.execution_status - == ConversationExecutionStatus.ERROR - ) + assert conversation.state.execution_status == ConversationExecutionStatus.ERROR assert len(events) == 1 - assert "ACP error: boom" in events[0].llm_message.content[0].text + content_block = events[0].llm_message.content[0] + assert isinstance(content_block, TextContent) + assert "ACP error: boom" in content_block.text def test_step_no_response_text_fallback(self, tmp_path): agent = _make_agent() @@ -420,7 +416,9 @@ def test_step_no_response_text_fallback(self, tmp_path): agent.step(conversation, on_event=events.append) - assert "(No response from ACP server)" in events[0].llm_message.content[0].text + content_block = events[0].llm_message.content[0] + assert isinstance(content_block, TextContent) + assert "(No response from ACP server)" in content_block.text def test_step_passes_on_token(self, tmp_path): agent = _make_agent() @@ -583,9 +581,7 @@ def _make_conversation_with_message(self, tmp_path, text="Hello"): state.events.append( MessageEvent( source="user", - llm_message=Message( - role="user", content=[TextContent(text=text)] - ), + llm_message=Message(role="user", content=[TextContent(text=text)]), ) ) @@ -775,10 +771,12 @@ def test_start_acp_server_wires_llm_ref(self, tmp_path): with patch( "openhands.sdk.agent.acp_agent.ACPAgent._start_acp_server" ) as mock_start: + def fake_start(s): client = _OpenHandsACPClient() client._llm_ref = agent.llm agent._client = client + mock_start.side_effect = fake_start agent.init_state(state, on_event=lambda _: None) From 14b89856acc21112bdb5430a826bef0bc9d67749 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 20 Feb 2026 08:16:18 -0300 Subject: [PATCH 05/10] Fix CI failures: duplicate example, pyright errors, missing acp in tests - Renumber example 36 -> 40 to avoid conflict with 36_event_json_to_openai_messages.py - Add type: ignore[reportMissingImports] to all acp imports (optional dep) - Add TYPE_CHECKING import in __init__.py to satisfy pyright __all__ check - Add @requires_acp skip marker for tests needing the acp package Co-Authored-By: Claude Opus 4.6 --- ...ent_example.py => 40_acp_agent_example.py} | 2 +- openhands-sdk/openhands/sdk/agent/__init__.py | 8 +++++ .../openhands/sdk/agent/acp_agent.py | 21 +++++++++---- tests/sdk/agent/test_acp_agent.py | 31 +++++++++++++++++-- 4 files changed, 52 insertions(+), 10 deletions(-) rename examples/01_standalone_sdk/{36_acp_agent_example.py => 40_acp_agent_example.py} (93%) diff --git a/examples/01_standalone_sdk/36_acp_agent_example.py b/examples/01_standalone_sdk/40_acp_agent_example.py similarity index 93% rename from examples/01_standalone_sdk/36_acp_agent_example.py rename to examples/01_standalone_sdk/40_acp_agent_example.py index 3d7fc927e9..89e0f3e1cb 100644 --- a/examples/01_standalone_sdk/36_acp_agent_example.py +++ b/examples/01_standalone_sdk/40_acp_agent_example.py @@ -9,7 +9,7 @@ - pip install agent-client-protocol>=0.8.1 Usage: - uv run python examples/01_standalone_sdk/36_acp_agent_example.py + uv run python examples/01_standalone_sdk/40_acp_agent_example.py """ import os diff --git a/openhands-sdk/openhands/sdk/agent/__init__.py b/openhands-sdk/openhands/sdk/agent/__init__.py index 8502579f99..e734c3cca9 100644 --- a/openhands-sdk/openhands/sdk/agent/__init__.py +++ b/openhands-sdk/openhands/sdk/agent/__init__.py @@ -1,7 +1,15 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from openhands.sdk.agent.agent import Agent from openhands.sdk.agent.base import AgentBase +if TYPE_CHECKING: + from openhands.sdk.agent.acp_agent import ACPAgent + + def __getattr__(name: str): if name == "ACPAgent": from openhands.sdk.agent.acp_agent import ACPAgent diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 17ed58ee6b..04f7de4843 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -120,7 +120,7 @@ async def session_update( update: Any, **kwargs: Any, # noqa: ARG002 ) -> None: - from acp.schema import ( + from acp.schema import ( # type: ignore[reportMissingImports] AgentMessageChunk, AgentThoughtChunk, TextContentBlock, @@ -163,7 +163,10 @@ async def request_permission( **kwargs: Any, # noqa: ARG002 ) -> Any: """Auto-approve all permission requests from the ACP server.""" - from acp.schema import AllowedOutcome, RequestPermissionResponse + from acp.schema import ( # type: ignore[reportMissingImports] + AllowedOutcome, + RequestPermissionResponse, + ) # Pick the first option (usually "allow once") option_id = options[0].option_id if options else "allow_once" @@ -315,7 +318,9 @@ def init_state( """Spawn the ACP server and initialize a session.""" # Lazy import guard try: - from acp import spawn_agent_process # noqa: F401 + from acp import ( + spawn_agent_process, # type: ignore[reportMissingImports] # noqa: F401 + ) except ImportError: raise ImportError( "The 'agent-client-protocol' package is required for ACPAgent. " @@ -374,8 +379,12 @@ def _start_acp_server(self, state: ConversationState) -> None: """Start the ACP subprocess and initialize the session.""" import asyncio - from acp.client.connection import ClientSideConnection - from acp.transports import default_environment + from acp.client.connection import ( + ClientSideConnection, # type: ignore[reportMissingImports] + ) + from acp.transports import ( + default_environment, # type: ignore[reportMissingImports] + ) client = _OpenHandsACPClient() client._llm_ref = self.llm @@ -465,7 +474,7 @@ def step( try: import asyncio - from acp.helpers import text_block + from acp.helpers import text_block # type: ignore[reportMissingImports] async def _prompt() -> Any: response = await self._conn.prompt( diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index c8ffafcc54..c6248e4be2 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -20,6 +20,18 @@ from openhands.sdk.workspace.local import LocalWorkspace +try: + import acp # type: ignore[reportMissingImports] # noqa: F401 + + _acp_available = True +except ImportError: + _acp_available = False + +requires_acp = pytest.mark.skipif( + not _acp_available, reason="acp package not installed" +) + + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -128,6 +140,7 @@ def test_deserialization_from_dict(self): # --------------------------------------------------------------------------- +@requires_acp class TestACPAgentValidation: """Test that unsupported features raise NotImplementedError in init_state.""" @@ -160,6 +173,7 @@ def test_rejects_mcp_config(self, tmp_path): class TestACPAgentInitState: + @requires_acp def test_emits_system_prompt_event(self, tmp_path): agent = _make_agent() state = _make_state(tmp_path) @@ -290,6 +304,7 @@ def _make_conversation_with_message(self, tmp_path, text="Hello"): conversation.state = state return conversation + @requires_acp def test_step_emits_message_event(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -318,6 +333,7 @@ def _fake_run_async(_coro): assert isinstance(content_block, TextContent) assert content_block.text == "The answer is 4" + @requires_acp def test_step_includes_reasoning(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -341,6 +357,7 @@ def _fake_run_async(_coro): msg = events[0].llm_message assert msg.reasoning_content == "I need to add 2+2" + @requires_acp def test_step_sets_finished(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -377,6 +394,7 @@ def test_step_no_user_message_finishes(self, tmp_path): assert state.execution_status == ConversationExecutionStatus.FINISHED + @requires_acp def test_step_error_sets_error_status(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -399,6 +417,7 @@ def test_step_error_sets_error_status(self, tmp_path): assert isinstance(content_block, TextContent) assert "ACP error: boom" in content_block.text + @requires_acp def test_step_no_response_text_fallback(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -597,6 +616,7 @@ def test_get_all_llms_yields_sentinel(self): assert llms[0] is agent.llm assert llms[0].model == "acp-managed" + @requires_acp def test_step_records_token_usage(self, tmp_path): """step() records per-turn token usage from PromptResponse.usage.""" agent = _make_agent() @@ -666,10 +686,11 @@ def _fake_run_async(_coro): # No token usage should be recorded assert len(agent.llm.metrics.token_usages) == 0 + @requires_acp @pytest.mark.asyncio async def test_usage_update_records_cost(self): """UsageUpdate with cost records incremental cost via metrics.""" - from acp.schema import UsageUpdate + from acp.schema import UsageUpdate # type: ignore[reportMissingImports] from openhands.sdk.llm import LLM @@ -689,10 +710,11 @@ async def test_usage_update_records_cost(self): assert client._last_cost == 0.05 assert client._context_window == 128000 + @requires_acp @pytest.mark.asyncio async def test_usage_update_incremental_cost(self): """UsageUpdate cost tracking is incremental (delta from last seen).""" - from acp.schema import UsageUpdate + from acp.schema import UsageUpdate # type: ignore[reportMissingImports] from openhands.sdk.llm import LLM @@ -719,10 +741,11 @@ async def test_usage_update_incremental_cost(self): assert llm.metrics.accumulated_cost == pytest.approx(0.12) assert client._last_cost == 0.12 + @requires_acp @pytest.mark.asyncio async def test_usage_update_updates_context_window(self): """UsageUpdate.size updates the client's _context_window.""" - from acp.schema import UsageUpdate + from acp.schema import UsageUpdate # type: ignore[reportMissingImports] client = _OpenHandsACPClient() @@ -734,6 +757,7 @@ async def test_usage_update_updates_context_window(self): assert client._context_window == 200000 + @requires_acp def test_stats_callback_invoked(self, tmp_path): """After step(), the sentinel LLM's stats callback is invoked.""" agent = _make_agent() @@ -763,6 +787,7 @@ def _fake_run_async(_coro): callback.assert_called_once() + @requires_acp def test_start_acp_server_wires_llm_ref(self, tmp_path): """_start_acp_server wires _llm_ref on the client.""" agent = _make_agent() From e9cbac066c27fae9006717f9c61c526c03deb871 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 20 Feb 2026 08:19:54 -0300 Subject: [PATCH 06/10] Make agent-client-protocol a required dependency Move acp from optional to required deps, removing: - Lazy import guard in init_state - type: ignore[reportMissingImports] comments - @requires_acp skip markers on tests - test_missing_acp_sdk_raises_import_error test The __getattr__ in __init__.py is kept to avoid a circular import. Co-Authored-By: Claude Opus 4.6 --- .../01_standalone_sdk/40_acp_agent_example.py | 1 - .../openhands/sdk/agent/acp_agent.py | 22 +++-------- openhands-sdk/pyproject.toml | 2 +- tests/sdk/agent/test_acp_agent.py | 39 ++----------------- 4 files changed, 9 insertions(+), 55 deletions(-) diff --git a/examples/01_standalone_sdk/40_acp_agent_example.py b/examples/01_standalone_sdk/40_acp_agent_example.py index 89e0f3e1cb..e8d2b0932f 100644 --- a/examples/01_standalone_sdk/40_acp_agent_example.py +++ b/examples/01_standalone_sdk/40_acp_agent_example.py @@ -6,7 +6,6 @@ Prerequisites: - Node.js / npx available - Claude Code CLI authenticated (or CLAUDE_API_KEY set) - - pip install agent-client-protocol>=0.8.1 Usage: uv run python examples/01_standalone_sdk/40_acp_agent_example.py diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 04f7de4843..82351db626 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -120,7 +120,7 @@ async def session_update( update: Any, **kwargs: Any, # noqa: ARG002 ) -> None: - from acp.schema import ( # type: ignore[reportMissingImports] + from acp.schema import ( AgentMessageChunk, AgentThoughtChunk, TextContentBlock, @@ -163,7 +163,7 @@ async def request_permission( **kwargs: Any, # noqa: ARG002 ) -> Any: """Auto-approve all permission requests from the ACP server.""" - from acp.schema import ( # type: ignore[reportMissingImports] + from acp.schema import ( AllowedOutcome, RequestPermissionResponse, ) @@ -316,18 +316,6 @@ def init_state( on_event: ConversationCallbackType, ) -> None: """Spawn the ACP server and initialize a session.""" - # Lazy import guard - try: - from acp import ( - spawn_agent_process, # type: ignore[reportMissingImports] # noqa: F401 - ) - except ImportError: - raise ImportError( - "The 'agent-client-protocol' package is required for ACPAgent. " - "Install it with: pip install 'openhands-sdk[acp]' or " - "pip install agent-client-protocol" - ) from None - # Validate no unsupported features if self.tools: raise NotImplementedError( @@ -380,10 +368,10 @@ def _start_acp_server(self, state: ConversationState) -> None: import asyncio from acp.client.connection import ( - ClientSideConnection, # type: ignore[reportMissingImports] + ClientSideConnection, ) from acp.transports import ( - default_environment, # type: ignore[reportMissingImports] + default_environment, ) client = _OpenHandsACPClient() @@ -474,7 +462,7 @@ def step( try: import asyncio - from acp.helpers import text_block # type: ignore[reportMissingImports] + from acp.helpers import text_block async def _prompt() -> Any: response = await self._conn.prompt( diff --git a/openhands-sdk/pyproject.toml b/openhands-sdk/pyproject.toml index f4d3debfa2..12660d05eb 100644 --- a/openhands-sdk/pyproject.toml +++ b/openhands-sdk/pyproject.toml @@ -5,6 +5,7 @@ description = "OpenHands SDK - Core functionality for building AI agents" requires-python = ">=3.12" dependencies = [ + "agent-client-protocol>=0.8.1", "deprecation>=2.1.0", "fastmcp>=2.11.3", "filelock>=3.20.1", @@ -26,7 +27,6 @@ Documentation = "https://docs.openhands.dev/sdk" [project.optional-dependencies] boto3 = ["boto3>=1.35.0"] -acp = ["agent-client-protocol>=0.8.1"] [build-system] requires = ["setuptools>=61.0", "wheel"] diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index c6248e4be2..1b0b8527f0 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -20,18 +20,6 @@ from openhands.sdk.workspace.local import LocalWorkspace -try: - import acp # type: ignore[reportMissingImports] # noqa: F401 - - _acp_available = True -except ImportError: - _acp_available = False - -requires_acp = pytest.mark.skipif( - not _acp_available, reason="acp package not installed" -) - - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -140,7 +128,6 @@ def test_deserialization_from_dict(self): # --------------------------------------------------------------------------- -@requires_acp class TestACPAgentValidation: """Test that unsupported features raise NotImplementedError in init_state.""" @@ -173,7 +160,6 @@ def test_rejects_mcp_config(self, tmp_path): class TestACPAgentInitState: - @requires_acp def test_emits_system_prompt_event(self, tmp_path): agent = _make_agent() state = _make_state(tmp_path) @@ -189,14 +175,6 @@ def test_emits_system_prompt_event(self, tmp_path): assert events[0].system_prompt.text == "ACP-managed agent" assert events[0].tools == [] - def test_missing_acp_sdk_raises_import_error(self, tmp_path): - agent = _make_agent() - state = _make_state(tmp_path) - - with patch.dict("sys.modules", {"acp": None}): - with pytest.raises(ImportError, match="agent-client-protocol"): - agent.init_state(state, on_event=lambda _: None) - # --------------------------------------------------------------------------- # _OpenHandsACPClient @@ -304,7 +282,6 @@ def _make_conversation_with_message(self, tmp_path, text="Hello"): conversation.state = state return conversation - @requires_acp def test_step_emits_message_event(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -333,7 +310,6 @@ def _fake_run_async(_coro): assert isinstance(content_block, TextContent) assert content_block.text == "The answer is 4" - @requires_acp def test_step_includes_reasoning(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -357,7 +333,6 @@ def _fake_run_async(_coro): msg = events[0].llm_message assert msg.reasoning_content == "I need to add 2+2" - @requires_acp def test_step_sets_finished(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -394,7 +369,6 @@ def test_step_no_user_message_finishes(self, tmp_path): assert state.execution_status == ConversationExecutionStatus.FINISHED - @requires_acp def test_step_error_sets_error_status(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -417,7 +391,6 @@ def test_step_error_sets_error_status(self, tmp_path): assert isinstance(content_block, TextContent) assert "ACP error: boom" in content_block.text - @requires_acp def test_step_no_response_text_fallback(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) @@ -616,7 +589,6 @@ def test_get_all_llms_yields_sentinel(self): assert llms[0] is agent.llm assert llms[0].model == "acp-managed" - @requires_acp def test_step_records_token_usage(self, tmp_path): """step() records per-turn token usage from PromptResponse.usage.""" agent = _make_agent() @@ -686,11 +658,10 @@ def _fake_run_async(_coro): # No token usage should be recorded assert len(agent.llm.metrics.token_usages) == 0 - @requires_acp @pytest.mark.asyncio async def test_usage_update_records_cost(self): """UsageUpdate with cost records incremental cost via metrics.""" - from acp.schema import UsageUpdate # type: ignore[reportMissingImports] + from acp.schema import UsageUpdate from openhands.sdk.llm import LLM @@ -710,11 +681,10 @@ async def test_usage_update_records_cost(self): assert client._last_cost == 0.05 assert client._context_window == 128000 - @requires_acp @pytest.mark.asyncio async def test_usage_update_incremental_cost(self): """UsageUpdate cost tracking is incremental (delta from last seen).""" - from acp.schema import UsageUpdate # type: ignore[reportMissingImports] + from acp.schema import UsageUpdate from openhands.sdk.llm import LLM @@ -741,11 +711,10 @@ async def test_usage_update_incremental_cost(self): assert llm.metrics.accumulated_cost == pytest.approx(0.12) assert client._last_cost == 0.12 - @requires_acp @pytest.mark.asyncio async def test_usage_update_updates_context_window(self): """UsageUpdate.size updates the client's _context_window.""" - from acp.schema import UsageUpdate # type: ignore[reportMissingImports] + from acp.schema import UsageUpdate client = _OpenHandsACPClient() @@ -757,7 +726,6 @@ async def test_usage_update_updates_context_window(self): assert client._context_window == 200000 - @requires_acp def test_stats_callback_invoked(self, tmp_path): """After step(), the sentinel LLM's stats callback is invoked.""" agent = _make_agent() @@ -787,7 +755,6 @@ def _fake_run_async(_coro): callback.assert_called_once() - @requires_acp def test_start_acp_server_wires_llm_ref(self, tmp_path): """_start_acp_server wires _llm_ref on the client.""" agent = _make_agent() From f04ca711332ebbfb18f0bae6351d5ca458b03e8d Mon Sep 17 00:00:00 2001 From: simonrosenberg <157206163+simonrosenberg@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:19:10 -0300 Subject: [PATCH 07/10] Update openhands-sdk/openhands/sdk/agent/acp_agent.py Co-authored-by: Engel Nyst --- openhands-sdk/openhands/sdk/agent/acp_agent.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 82351db626..390d120330 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -335,7 +335,6 @@ def init_state( if self.critic is not None: raise NotImplementedError( "ACPAgent does not support critic; " - "the ACP server manages its own evaluation" ) if self.agent_context is not None: raise NotImplementedError( From 2ae9b9b88767a764fb90f0ba91b5a8b07ee2984b Mon Sep 17 00:00:00 2001 From: simonrosenberg <157206163+simonrosenberg@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:20:16 -0300 Subject: [PATCH 08/10] Update openhands-sdk/openhands/sdk/agent/acp_agent.py Co-authored-by: Engel Nyst --- openhands-sdk/openhands/sdk/agent/acp_agent.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 390d120330..68c0d3e3f6 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -257,16 +257,6 @@ class ACPAgent(AgentBase): server (e.g. ``claude-code-acp``) as a subprocess and communicates with it via the ACP protocol. The server manages its own LLM, tools, and execution lifecycle. - - Example:: - - from openhands.sdk.agent import ACPAgent - from openhands.sdk.conversation import Conversation - - agent = ACPAgent(acp_command=["npx", "-y", "claude-code-acp"]) - conversation = Conversation(agent=agent, workspace="./workspace") - conversation.send_message("Hello! What is 2+2?") - conversation.run() """ # Override required fields with ACP-appropriate defaults From 506a50a936442fbe55406fac046d4725ecb58bcc Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 20 Feb 2026 15:31:20 -0300 Subject: [PATCH 09/10] Address PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename _make_sentinel_llm → _make_dummy_llm - Move inline acp imports to top-level - Add GitHub issue link to TODO comment - Simplify _filter_jsonrpc_lines exception handling - Add debug logging for on_token and stats callback failures - Remove chatty Example from class docstring - Remove critic validation check (ACP server manages its own evaluation) - Add comment explaining lazy import in __init__.py (avoids circular dependency) Co-Authored-By: Claude Opus 4.6 --- openhands-sdk/openhands/sdk/agent/__init__.py | 4 + .../openhands/sdk/agent/acp_agent.py | 80 +++++++------------ tests/sdk/agent/test_acp_agent.py | 50 ++++++------ uv.lock | 8 +- 4 files changed, 63 insertions(+), 79 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/__init__.py b/openhands-sdk/openhands/sdk/agent/__init__.py index e734c3cca9..5e8b4ea54d 100644 --- a/openhands-sdk/openhands/sdk/agent/__init__.py +++ b/openhands-sdk/openhands/sdk/agent/__init__.py @@ -10,6 +10,10 @@ from openhands.sdk.agent.acp_agent import ACPAgent +# Lazy import: eagerly importing ACPAgent registers it in the +# DiscriminatedUnionMixin, which makes `kind` required in Agent payloads +# that previously defaulted. It also avoids a circular import through +# tool/builtins/finish.py if ACPAgent is imported before Agent. def __getattr__(name: str): if name == "ACPAgent": from openhands.sdk.agent.acp_agent import ACPAgent diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 68c0d3e3f6..5dbc213ae3 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -10,10 +10,24 @@ from __future__ import annotations +import asyncio import os from collections.abc import Generator from typing import TYPE_CHECKING, Any +from acp.client.connection import ClientSideConnection +from acp.helpers import text_block +from acp.schema import ( + AgentMessageChunk, + AgentThoughtChunk, + AllowedOutcome, + RequestPermissionResponse, + TextContentBlock, + ToolCallProgress, + ToolCallStart, + UsageUpdate, +) +from acp.transports import default_environment from pydantic import Field, PrivateAttr from openhands.sdk.agent.base import AgentBase @@ -41,15 +55,16 @@ # so we yield to the event loop and then sleep briefly to allow in-flight # handlers to finish. Override via ACP_NOTIFICATION_DRAIN_DELAY for slow or # remote servers. -# TODO: Replace with protocol-level synchronization once ACP supports a -# "turn complete" sentinel notification. +# TODO(https://github.com/agentclientprotocol/agent-client-protocol/issues/554): +# Replace with protocol-level synchronization once ACP supports a +# "turn complete" notification. _NOTIFICATION_DRAIN_DELAY: float = float( os.environ.get("ACP_NOTIFICATION_DRAIN_DELAY", "0.1") ) -def _make_sentinel_llm() -> LLM: - """Create a sentinel LLM that should never be called.""" +def _make_dummy_llm() -> LLM: + """Create a dummy LLM that should never be called directly.""" return LLM(model="acp-managed") @@ -78,20 +93,17 @@ async def _filter_jsonrpc_lines(source: Any, dest: Any) -> None: if stripped.startswith(b"{") and b'"jsonrpc"' in line: dest.feed_data(line) else: - # Log non-JSON lines at debug level - try: - logger.debug( - "ACP stdout (non-JSON): %s", - line.decode(errors="replace").rstrip(), - ) - except Exception: - pass + logger.debug( + "ACP stdout (non-JSON): %s", + line.decode(errors="replace").rstrip(), + ) except Exception: + logger.debug("_filter_jsonrpc_lines stopped", exc_info=True) dest.feed_eof() -class _OpenHandsACPClient: - """ACP Client that accumulates session updates and emits OpenHands events. +class _OpenHandsACPBridge: + """Bridge between OpenHands and ACP that accumulates session updates. Implements the ``Client`` protocol from ``agent_client_protocol``. """ @@ -120,15 +132,6 @@ async def session_update( update: Any, **kwargs: Any, # noqa: ARG002 ) -> None: - from acp.schema import ( - AgentMessageChunk, - AgentThoughtChunk, - TextContentBlock, - ToolCallProgress, - ToolCallStart, - UsageUpdate, - ) - if isinstance(update, AgentMessageChunk): if isinstance(update.content, TextContentBlock): text = update.content.text @@ -137,7 +140,7 @@ async def session_update( try: self.on_token(text) except Exception: - pass + logger.debug("on_token callback failed", exc_info=True) elif isinstance(update, AgentThoughtChunk): if isinstance(update.content, TextContentBlock): self.accumulated_thoughts.append(update.content.text) @@ -163,11 +166,6 @@ async def request_permission( **kwargs: Any, # noqa: ARG002 ) -> Any: """Auto-approve all permission requests from the ACP server.""" - from acp.schema import ( - AllowedOutcome, - RequestPermissionResponse, - ) - # Pick the first option (usually "allow once") option_id = options[0].option_id if options else "allow_once" logger.info( @@ -260,7 +258,7 @@ class ACPAgent(AgentBase): """ # Override required fields with ACP-appropriate defaults - llm: LLM = Field(default_factory=_make_sentinel_llm) + llm: LLM = Field(default_factory=_make_dummy_llm) tools: list[Tool] = Field(default_factory=list) include_default_tools: list[str] = Field(default_factory=list) @@ -285,7 +283,7 @@ class ACPAgent(AgentBase): _conn: Any = PrivateAttr(default=None) # ClientSideConnection _session_id: str | None = PrivateAttr(default=None) _process: Any = PrivateAttr(default=None) # asyncio subprocess - _client: Any = PrivateAttr(default=None) # _OpenHandsACPClient + _client: Any = PrivateAttr(default=None) # _OpenHandsACPBridge _filtered_reader: Any = PrivateAttr(default=None) # StreamReader _closed: bool = PrivateAttr(default=False) @@ -322,10 +320,6 @@ def init_state( "ACPAgent does not support condenser; " "the ACP server manages its own context" ) - if self.critic is not None: - raise NotImplementedError( - "ACPAgent does not support critic; " - ) if self.agent_context is not None: raise NotImplementedError( "ACPAgent does not support agent_context; " @@ -354,16 +348,7 @@ def init_state( def _start_acp_server(self, state: ConversationState) -> None: """Start the ACP subprocess and initialize the session.""" - import asyncio - - from acp.client.connection import ( - ClientSideConnection, - ) - from acp.transports import ( - default_environment, - ) - - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client._llm_ref = self.llm self._client = client @@ -449,9 +434,6 @@ def step( self._client.on_token = on_token try: - import asyncio - - from acp.helpers import text_block async def _prompt() -> Any: response = await self._conn.prompt( @@ -490,7 +472,7 @@ async def _prompt() -> Any: try: self.llm.telemetry._stats_update_callback() except Exception: - pass + logger.debug("Stats update callback failed", exc_info=True) # Build response message response_text = "".join(self._client.accumulated_text) diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index 1b0b8527f0..a87341f084 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -9,7 +9,7 @@ import pytest -from openhands.sdk.agent.acp_agent import ACPAgent, _OpenHandsACPClient +from openhands.sdk.agent.acp_agent import ACPAgent, _OpenHandsACPBridge from openhands.sdk.agent.base import AgentBase from openhands.sdk.conversation.state import ( ConversationExecutionStatus, @@ -177,13 +177,13 @@ def test_emits_system_prompt_event(self, tmp_path): # --------------------------------------------------------------------------- -# _OpenHandsACPClient +# _OpenHandsACPBridge # --------------------------------------------------------------------------- class TestOpenHandsACPClient: def test_reset_clears_state(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client.accumulated_text.append("hello") client.accumulated_thoughts.append("thinking") client.on_token = lambda _: None @@ -196,20 +196,20 @@ def test_reset_clears_state(self): @pytest.mark.asyncio async def test_session_update_accumulates_text(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client.accumulated_text.append("Hello") client.accumulated_text.append(" World") assert "".join(client.accumulated_text) == "Hello World" @pytest.mark.asyncio async def test_session_update_accumulates_thoughts(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client.accumulated_thoughts.append("Let me think") client.accumulated_thoughts.append(" about this") assert "".join(client.accumulated_thoughts) == "Let me think about this" def test_on_token_callback(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() tokens: list[str] = [] client.on_token = tokens.append @@ -223,7 +223,7 @@ def test_on_token_callback(self): @pytest.mark.asyncio async def test_fs_methods_raise(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() with pytest.raises(NotImplementedError): await client.write_text_file("c", "/f", "s1") with pytest.raises(NotImplementedError): @@ -231,7 +231,7 @@ async def test_fs_methods_raise(self): @pytest.mark.asyncio async def test_terminal_methods_raise(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() with pytest.raises(NotImplementedError): await client.create_terminal("bash", "s1") with pytest.raises(NotImplementedError): @@ -245,13 +245,13 @@ async def test_terminal_methods_raise(self): @pytest.mark.asyncio async def test_ext_method_returns_empty_dict(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() result = await client.ext_method("test", {}) assert result == {} @pytest.mark.asyncio async def test_ext_notification_is_noop(self): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() await client.ext_notification("test", {}) # Should not raise @@ -289,7 +289,7 @@ def test_step_emits_message_event(self, tmp_path): # Set up mocked runtime state — populate text *after* reset # (step() calls client.reset() then run_async which populates text) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -315,7 +315,7 @@ def test_step_includes_reasoning(self, tmp_path): conversation = self._make_conversation_with_message(tmp_path) events: list = [] - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -337,7 +337,7 @@ def test_step_sets_finished(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -363,7 +363,7 @@ def test_step_no_user_message_finishes(self, tmp_path): conversation = MagicMock() conversation.state = state - agent._client = _OpenHandsACPClient() + agent._client = _OpenHandsACPBridge() agent.step(conversation, on_event=lambda _: None) @@ -374,7 +374,7 @@ def test_step_error_sets_error_status(self, tmp_path): conversation = self._make_conversation_with_message(tmp_path) events: list = [] - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -396,7 +396,7 @@ def test_step_no_response_text_fallback(self, tmp_path): conversation = self._make_conversation_with_message(tmp_path) events: list = [] - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() # accumulated_text stays empty — run_async is a no-op agent._client = mock_client agent._conn = MagicMock() @@ -416,7 +416,7 @@ def test_step_passes_on_token(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -594,7 +594,7 @@ def test_step_records_token_usage(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() mock_client._context_window = 200000 agent._client = mock_client agent._conn = MagicMock() @@ -637,7 +637,7 @@ def test_step_handles_no_usage(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -665,7 +665,7 @@ async def test_usage_update_records_cost(self): from openhands.sdk.llm import LLM - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() llm = LLM(model="acp-managed") client._llm_ref = llm client._last_cost = 0.0 @@ -688,7 +688,7 @@ async def test_usage_update_incremental_cost(self): from openhands.sdk.llm import LLM - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() llm = LLM(model="acp-managed") client._llm_ref = llm @@ -716,7 +716,7 @@ async def test_usage_update_updates_context_window(self): """UsageUpdate.size updates the client's _context_window.""" from acp.schema import UsageUpdate - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() update = MagicMock(spec=UsageUpdate) update.size = 200000 @@ -731,7 +731,7 @@ def test_stats_callback_invoked(self, tmp_path): agent = _make_agent() conversation = self._make_conversation_with_message(tmp_path) - mock_client = _OpenHandsACPClient() + mock_client = _OpenHandsACPBridge() agent._client = mock_client agent._conn = MagicMock() agent._session_id = "test-session" @@ -765,7 +765,7 @@ def test_start_acp_server_wires_llm_ref(self, tmp_path): ) as mock_start: def fake_start(s): - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client._llm_ref = agent.llm agent._client = client @@ -776,7 +776,7 @@ def fake_start(s): def test_reset_preserves_telemetry_state(self): """reset() clears text/thoughts but preserves telemetry state.""" - client = _OpenHandsACPClient() + client = _OpenHandsACPBridge() client._last_cost = 1.23 client._context_window = 128000 client._llm_ref = MagicMock() diff --git a/uv.lock b/uv.lock index 95575a85ca..2f41905924 100644 --- a/uv.lock +++ b/uv.lock @@ -2276,6 +2276,7 @@ name = "openhands-sdk" version = "1.12.0" source = { editable = "openhands-sdk" } dependencies = [ + { name = "agent-client-protocol" }, { name = "deprecation" }, { name = "fastmcp" }, { name = "filelock" }, @@ -2290,16 +2291,13 @@ dependencies = [ ] [package.optional-dependencies] -acp = [ - { name = "agent-client-protocol" }, -] boto3 = [ { name = "boto3" }, ] [package.metadata] requires-dist = [ - { name = "agent-client-protocol", marker = "extra == 'acp'", specifier = ">=0.8.1" }, + { name = "agent-client-protocol", specifier = ">=0.8.1" }, { name = "boto3", marker = "extra == 'boto3'", specifier = ">=1.35.0" }, { name = "deprecation", specifier = ">=2.1.0" }, { name = "fastmcp", specifier = ">=2.11.3" }, @@ -2313,7 +2311,7 @@ requires-dist = [ { name = "tenacity", specifier = ">=9.1.2" }, { name = "websockets", specifier = ">=12" }, ] -provides-extras = ["boto3", "acp"] +provides-extras = ["boto3"] [[package]] name = "openhands-tools" From 6e97751908b5aeb599733b80f200c9b29c5e2af7 Mon Sep 17 00:00:00 2001 From: simonrosenberg <157206163+simonrosenberg@users.noreply.github.com> Date: Fri, 20 Feb 2026 17:49:22 -0300 Subject: [PATCH 10/10] Apply suggestion from @simonrosenberg --- openhands-sdk/openhands/sdk/agent/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/__init__.py b/openhands-sdk/openhands/sdk/agent/__init__.py index 5e8b4ea54d..666921be92 100644 --- a/openhands-sdk/openhands/sdk/agent/__init__.py +++ b/openhands-sdk/openhands/sdk/agent/__init__.py @@ -12,8 +12,7 @@ # Lazy import: eagerly importing ACPAgent registers it in the # DiscriminatedUnionMixin, which makes `kind` required in Agent payloads -# that previously defaulted. It also avoids a circular import through -# tool/builtins/finish.py if ACPAgent is imported before Agent. +# that previously defaulted. def __getattr__(name: str): if name == "ACPAgent": from openhands.sdk.agent.acp_agent import ACPAgent