feat(acp): ACP proxy chain refactor — Conductor, HookProxy, zero-conversion passthrough

openspec/changes/archive/2026-07-08-acp-proxy-chain-refactor/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-07-07

openspec/changes/archive/2026-07-08-acp-proxy-chain-refactor/proposal.md

@@ -0,0 +1,44 @@
+## Why
+
+The current ACPAgent implementation was built before the ACP proxy chain concept existed. It conflates subprocess management, ACP client communication, and event conversion into a single monolithic class with three critical issues: (1) `ACPTurn` — the designed Turn abstraction — is non-functional dead code due to a missing adapter, (2) streaming uses a 50ms polling loop instead of async push, and (3) nesting ACP server + client causes ~1600 lines of bidirectional event conversion (ACP→native→ACP) that should be zero-copy passthrough. The proxy chain RFD (`docs/rfds/proxy-chains.mdx` in agent-client-protocol) defines a conductor pattern that directly solves these structural problems.
+
+## What Changes
+
+- **NEW**: `Conductor` class — manages proxy chain lifecycle, routes `proxy/successor` messages, spawns subprocesses
+- **NEW**: `Proxy` protocol (`typing.Protocol`) — defines `proxy_initialize()` + `proxy_successor()` per RFD
+- **NEW**: `ProxySideConnection` — wire-protocol wrapper for proxy components (analogous to `AgentSideConnection`/`ClientSideConnection`)
+- **NEW**: `ACPClientAdapter` — bridges `ACPAgentAPI` (blocking prompt + notification deque) to `ACPClientProtocol` (stream interface), making `ACPTurn` functional
+- **NEW**: `HookProxy` — wraps existing `CallableHook`/`CommandHook`/`PromptHook` as proxy chain components, reusing the entire hook system
+- **NEW**: Built-in proxy implementations: `ContextInjectionProxy`, `ToolProviderProxy` (reusing `AcpMcpTransport`), `PermissionHookProxy`
+- **NEW**: YAML `proxy_chain:` configuration section for defining ordered proxy chains
+- **REWRITE**: `ACPAgent` — split into Conductor (subprocess management) + ACPTurn (turn cycle). Delete `_stream_events()` inline logic, `poll_acp_events()`, `ACPSessionState` deque
+- **REWRITE**: `ACPClientHandler.session_update()` — push directly to async stream (eliminate `TimeoutableEvent` polling)
+- **FIX**: `ACPTurn` — remove `cast()` hack, use `ACPClientAdapter` for real `ACPClientProtocol` compliance
+- **FIX**: `ACPAgent` output type `str` → `ChatMessage[str]` for `MessageNode` contract compliance
+- **DELETE**: `poll_acp_events()` and 50ms timeout loop
+- **DELETE**: Legacy `ACPSession.process_prompt()` dual path (consolidate to `ACPProtocolHandler`)
+- **MIGRATE**: `ACPAgent` from `ToolManagerBridge` (deprecated) to `ResourceProvider`
+- **BREAKING**: `ACPAgent.create_turn()` now returns a functional `ACPTurn` (previously would crash at runtime)
+- **BREAKING**: `ACPAgent._stream_events()` signature changes — conductor-driven, no inline polling
+
+## Capabilities
+
+### New Capabilities
+- `acp-proxy-chain`: Conductor pattern, proxy/initialize + proxy/successor protocol, proxy chain lifecycle management
+- `acp-proxy-impls`: Built-in proxy implementations (context injection, tool provider, permission hooks) and HookProxy adapter for reusing existing hook system
+- `acp-client-adapter`: ACPClientAdapter bridging ACPAgentAPI to ACPClientProtocol, making ACPTurn functional with async push streaming
+
+### Modified Capabilities
+- `acp-server`: Server-side ACP agent (`AgentPoolACPAgent`) becomes terminal agent behind conductor; legacy `ACPSession.process_prompt()` dual path removed
+- `acp-single-execution-path`: ACPTurn becomes the single execution path for ACP agents (eliminates path A/B divergence between TurnRunner and run_stream)
+- `session-orchestration`: TurnRunner now works with ACP agents via functional ACPTurn (previously broken due to missing ACPClientProtocol implementation)
+
+## Impact
+
+- **`src/acp/`**: New `conductor.py`, `proxy/` package (protocol, connection, impls). Existing `Connection`, `AgentSideConnection`, `ClientSideConnection` unchanged (additive only).
+- **`src/agentpool/agents/acp_agent/`**: Major rewrite of `acp_agent.py`, `client_handler.py`. New `adapter.py`. Delete `turn.py` dead code patterns (ACPTurn moves to use adapter). Simplify `acp_converters.py` (passthrough eliminates most conversion).
+- **`src/agentpool_server/acp_server/`**: `AgentPoolACPAgent` refactored as terminal agent. `ACPProtocolHandler` unchanged (already works). `ACPEventConverter` becomes a proxy component.
+- **`src/agentpool/hooks/`**: No changes to hook implementations. New `HookProxy` adapter in `src/acp/proxy/impls/` wraps them.
+- **`src/agentpool/models/`**: New `ProxyChainConfig` model. `ACPAgentConfig` updated with optional `proxy_chain` field.
+- **YAML configs**: New `proxy_chain:` section. Existing configs unchanged (backward compatible — no proxy_chain = direct conductor→agent).
+- **Dependencies**: No new external dependencies. Reuses existing `anyio`, `pydantic`, `acp` library.

openspec/changes/archive/2026-07-08-acp-proxy-chain-refactor/specs/acp-client-adapter/spec.md

@@ -0,0 +1,77 @@
+## ADDED Requirements
+
+### Requirement: ACPClientProtocol SHALL be redefined for non-blocking semantics
+
+The `ACPClientProtocol` interface SHALL be modified to support non-blocking prompt semantics. The `prompt()` method SHALL return `None` (not `PromptResponse`). The `stream_events()` method SHALL take no `response` parameter and SHALL return an `AsyncIterator[SessionUpdate]`. A `stop_reason` property SHALL be added to expose the `PromptResponse.stop_reason` after streaming completes. This is an internal interface change — `ACPClientProtocol` is only implemented by `ACPClientAdapter` and consumed by `ACPTurn`.
+
+#### Scenario: prompt returns None
+- **WHEN** `ACPClientAdapter.prompt()` is called
+- **THEN** the adapter SHALL launch `api.prompt()` as a background asyncio task (fire-and-forget)
+- **AND** SHALL return `None` immediately without waiting for the prompt to complete
+- **AND** SHALL NOT block the calling coroutine
+
+#### Scenario: stream_events takes no arguments
+- **WHEN** `ACPClientAdapter.stream_events()` is called (with no arguments)
+- **THEN** the adapter SHALL return an async iterator that yields ACP session update notifications
+- **AND** notifications SHALL be pushed to the queue by `ACPClientHandler.session_update()` as they arrive
+- **AND** the iterator SHALL yield notifications in order
+- **AND** the iterator SHALL signal completion when the prompt background task completes
+
+#### Scenario: stop_reason available after streaming
+- **WHEN** the prompt background task completes
+- **THEN** the adapter SHALL store the `PromptResponse` internally
+- **AND** the `stop_reason` property SHALL return the `PromptResponse.stop_reason` value
+- **AND** accessing `stop_reason` before streaming completes SHALL raise `RuntimeError("stop_reason not available until streaming completes")`
+
+#### Scenario: Adapter get_messages retrieves history
+- **WHEN** `ACPClientAdapter.get_messages()` is called after the prompt completes
+- **THEN** the adapter SHALL call `api.get_messages()` and return the message history
+
+### Requirement: ACPClientHandler SHALL bifurcate state updates and stream data
+
+The `ACPClientHandler.session_update()` method SHALL process state updates (model, mode, config, commands) in-place and push only stream-data updates (text chunks, tool calls, thoughts) to the async queue. State updates (`CurrentModeUpdate`, `CurrentModelUpdate`, `ConfigOptionUpdate`, `AvailableCommandsUpdate`) SHALL NOT be pushed to the stream queue — they SHALL be processed by the handler directly, preserving the existing state tracking behavior. Stream-data updates (`AgentMessageChunk`, `ToolCallStart`, `ToolCallComplete`, etc.) SHALL be pushed to the async queue.
+
+#### Scenario: State update processed in-place
+- **WHEN** `ACPClientHandler.session_update()` receives a `CurrentModelUpdate` notification
+- **THEN** the handler SHALL update its internal model state directly
+- **AND** SHALL NOT push the update to the async queue
+
+#### Scenario: Stream data pushed to queue
+- **WHEN** `ACPClientHandler.session_update()` receives an `AgentMessageChunk` notification
+- **THEN** the handler SHALL push the notification to the async queue
+- **AND** SHALL NOT process it as a state update
+
+### Requirement: ACPClientAdapter async queue SHALL be bounded
+
+The async queue in `ACPClientAdapter` SHALL have a `max_buffer_size` of 1000 items to prevent unbounded memory growth. If the queue is full when a new notification arrives, the adapter SHALL apply backpressure by blocking the push until the consumer drains items.
+
+#### Scenario: Queue backpressure
+- **WHEN** the async queue has 1000 items and a new notification arrives
+- **THEN** the push operation SHALL block until the consumer dequeues at least one item
+- **AND** the ACP server SHALL be effectively throttled until the consumer catches up
+
+### Requirement: ACPClientAdapter SHALL reject concurrent prompts
+
+The `ACPClientAdapter` SHALL reject a new `prompt()` call while a previous prompt is still streaming. ACP sessions typically allow one active prompt at a time. This matches the current behavior where `ACPAgentAPI.prompt()` blocks until completion.
+
+#### Scenario: Concurrent prompt rejected
+- **WHEN** `adapter.prompt()` is called while a previous prompt's background task is still running
+- **THEN** the adapter SHALL raise `RuntimeError("Prompt already in progress")`
+- **AND** SHALL NOT launch a new background task
+
+### Requirement: ACPTurn SHALL use ACPClientAdapter instead of cast hack
+
+The `ACPAgent.create_turn()` method SHALL construct an `ACPClientAdapter` wrapping `self._api` and pass it to `ACPTurn`. The `cast("ACPClientProtocol", self._api)` hack SHALL be removed. `ACPTurn.execute()` SHALL call `adapter.prompt()`, then iterate `adapter.stream_events()`, then access `adapter.stop_reason`, and finally call `adapter.get_messages()`.
+
+#### Scenario: ACPTurn executes successfully
+- **WHEN** `ACPTurn.execute()` is called
+- **THEN** the turn SHALL call `adapter.prompt()` (returns None, non-blocking)
+- **AND** SHALL iterate `adapter.stream_events()` yielding each notification as a `RichAgentStreamEvent`
+- **AND** SHALL access `adapter.stop_reason` after the stream completes
+- **AND** SHALL call `adapter.get_messages()` after the stream completes
+- **AND** SHALL return the final `ChatMessage[str]` result
+
+#### Scenario: ACPTurn no longer uses cast
+- **WHEN** `ACPAgent.create_turn()` is called
+- **THEN** it SHALL construct `ACPClientAdapter(self._api)` 
+- **AND** SHALL NOT use `cast("ACPClientProtocol", self._api)`

openspec/changes/archive/2026-07-08-acp-proxy-chain-refactor/specs/acp-proxy-chain/spec.md

@@ -0,0 +1,119 @@
+## ADDED Requirements
+
+### Requirement: Conductor SHALL manage proxy chain lifecycle
+
+The `Conductor` class SHALL manage the lifecycle of a proxy chain, including spawning the terminal agent subprocess, initializing proxies via `proxy/initialize`, and routing messages bidirectionally via `proxy/successor`. The Conductor SHALL inherit from `MessageNode[ChatMessage, ChatMessage[str]]` and expose a `_step` property for graph-based execution.
+
+#### Scenario: Conductor initializes proxy chain
+- **WHEN** a Conductor is created with a list of proxy configs and a terminal agent config
+- **THEN** the Conductor SHALL spawn the terminal agent subprocess
+- **AND** SHALL call `proxy/initialize` on each proxy in order from client toward terminal agent (P1 first, P2 next, ..., terminal agent last)
+- **AND** SHALL establish `proxy/successor` forwarding between adjacent proxies
+- **AND** SHALL send `initialize` (standard ACP method) to the terminal agent (last component)
+- **AND** SHALL return a ready signal when the chain is fully initialized
+
+#### Scenario: Conductor with zero proxies
+- **WHEN** a Conductor is created with no proxy configs (empty `proxy_chain` list)
+- **THEN** the Conductor SHALL connect directly to the terminal agent
+- **AND** SHALL NOT send any `proxy/initialize` or `proxy/successor` messages
+
+#### Scenario: Conductor cleanup on shutdown
+- **WHEN** the Conductor is shut down (async context manager exit)
+- **THEN** the Conductor SHALL terminate the terminal agent subprocess
+- **AND** SHALL clean up all proxy connections in reverse order
+- **AND** SHALL ensure no orphaned subprocesses remain
+
+### Requirement: Proxy protocol SHALL define proxy/initialize and proxy/successor
+
+The `Proxy` protocol SHALL define two methods following the ACP proxy chain RFD:
+- `proxy_initialize()`: Called during chain setup to signal that a successor exists. Returns proxy capabilities including `intercepted_methods` list (message types the proxy intercepts).
+- `proxy_successor(method, params, meta)`: Called to forward a message to the successor (next proxy or terminal agent). The proxy MAY inspect, modify, or block the message before forwarding.
+
+#### Scenario: Proxy receives proxy/initialize
+- **WHEN** the Conductor calls `proxy/initialize` on a proxy
+- **THEN** the proxy SHALL return its capabilities including `intercepted_methods` (list of ACP method names it intercepts)
+- **AND** SHALL prepare its internal state for chain operation
+
+#### Scenario: Proxy forwards message via proxy/successor
+- **WHEN** a proxy receives a `proxy/successor` call with method, params, and meta
+- **THEN** the proxy MAY inspect the method and params
+- **AND** if the proxy has interception logic for this message type (declared in `intercepted_methods`), it SHALL apply the interception
+- **AND** SHALL forward the (possibly modified) message to its successor
+- **OR** SHALL return a blocking response if the interception denies the message
+
+#### Scenario: Proxy passthrough for unregistered message types
+- **WHEN** a proxy receives a `proxy/successor` call for a message type not in its `intercepted_methods` list
+- **THEN** the proxy SHALL forward the raw message to its successor without deserializing the params
+- **AND** SHALL NOT pay any serialization/deserialization cost
+
+### Requirement: ProxySideConnection SHALL wrap proxy wire communication
+
+The `ProxySideConnection` class SHALL wrap a `Connection` instance to provide proxy-specific message handling. It SHALL listen for `proxy/initialize` and `proxy/successor` requests and dispatch them to the `Proxy` implementation. It SHALL be analogous to `AgentSideConnection` and `ClientSideConnection`.
+
+#### Scenario: ProxySideConnection receives proxy/successor
+- **WHEN** a `ProxySideConnection` receives a `proxy/successor` JSON-RPC request
+- **THEN** it SHALL dispatch the method, params, and meta to the Proxy implementation
+- **AND** SHALL return the Proxy's response to the caller
+
+### Requirement: Conductor SHALL determine terminal vs proxy by chain position
+
+The Conductor SHALL determine which components are proxies vs terminal agent based on **chain position** from configuration. The last component in the chain is the terminal agent; all others are proxies. The Conductor sends `proxy/initialize` to all proxy components and `initialize` to the terminal agent (last component). The Conductor does NOT detect terminal vs proxy status from responses — it knows from configuration.
+
+#### Scenario: Terminal agent receives initialize
+- **WHEN** the Conductor initializes the chain and the component is the last in the chain (terminal agent)
+- **THEN** the Conductor SHALL send `initialize` (standard ACP method)
+- **AND** SHALL NOT send `proxy/initialize` or `proxy/successor` to it
+- **AND** SHALL send standard ACP methods (`session/prompt`, `session/update`) directly
+
+#### Scenario: Proxy receives proxy/initialize
+- **WHEN** the Conductor initializes the chain and the component is not the last (proxy)
+- **THEN** the Conductor SHALL send `proxy/initialize`
+- **AND** SHALL route subsequent messages through `proxy/successor`
+
+### Requirement: YAML proxy_chain configuration
+
+The system SHALL support a `proxy_chain` section in ACP agent configuration. Each entry SHALL have a `type` field that maps to a registered proxy implementation. When `proxy_chain` is omitted, the Conductor SHALL run with zero proxies.
+
+#### Scenario: Agent with proxy chain
+- **WHEN** an ACP agent config includes a `proxy_chain` section with one or more proxy entries
+- **THEN** the Conductor SHALL instantiate each proxy in order
+- **AND** SHALL initialize the chain with the terminal agent at the end
+
+#### Scenario: Agent without proxy chain
+- **WHEN** an ACP agent config does not include a `proxy_chain` section
+- **THEN** the Conductor SHALL connect directly to the terminal agent with no proxies
+
+### Requirement: Conductor SHALL use structured concurrency for subprocess management
+
+The Conductor SHALL use anyio task groups for structured concurrency when spawning the terminal agent subprocess and managing proxy connections. Subprocess cleanup SHALL run in a `finally` block to prevent orphaned processes.
+
+#### Scenario: Subprocess crash during operation
+- **WHEN** the terminal agent subprocess crashes during operation
+- **THEN** the Conductor SHALL detect the crash via the connection's task supervisor
+- **AND** SHALL clean up all proxy connections
+- **AND** SHALL raise an appropriate error to the caller
+
+### Requirement: Proxy chain error propagation
+
+Errors in a proxy SHALL produce a JSON-RPC error response forwarded back through the chain to the client. The Conductor SHALL NOT silently skip failed proxies — a security hook proxy failing silently is dangerous.
+
+#### Scenario: Proxy exception during proxy/successor
+- **WHEN** a proxy raises an exception during `proxy/successor` processing
+- **THEN** the Conductor SHALL construct a JSON-RPC error response with the exception details
+- **AND** SHALL forward the error response back through the chain to the predecessor
+- **AND** SHALL NOT skip the proxy or continue with default behavior
+
+#### Scenario: Proxy crash during initialization
+- **WHEN** a proxy crashes during `proxy/initialize`
+- **THEN** the Conductor SHALL abort chain initialization
+- **AND** SHALL clean up all already-initialized proxies and the terminal agent
+- **AND** SHALL raise an initialization error to the caller
+
+### Requirement: Proxy hot-swap is out of scope
+
+The Conductor SHALL NOT support hot-swapping proxies at runtime (adding/removing proxies without restarting the chain). This is explicitly out of scope for this change. The design should not preclude it, but it will not be implemented.
+
+#### Scenario: Hot-swap not supported
+- **WHEN** a user attempts to modify the proxy chain at runtime
+- **THEN** the system SHALL raise `NotImplementedError("Proxy hot-swap is not supported")`
+- **AND** the proxy chain SHALL remain unchanged