Skip to content

Commit 0b5c485

Browse files
committed
refactor(acp): Prepare new ACP module implementation
1 parent 1a5ef77 commit 0b5c485

11 files changed

Lines changed: 20 additions & 1281 deletions

File tree

codex-rs/acp/docs.md

Lines changed: 9 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -2,114 +2,20 @@
22

33
Path: @/codex-rs/acp
44

5-
### Overview
5+
## Overview
66

77
- Implements Agent Context Protocol (ACP) for Codex to communicate with external AI agent subprocesses
8-
- Uses the official `agent-client-protocol` v0.7 library instead of custom JSON-RPC implementation
9-
- Includes `AcpModelClient` for high-level streaming interaction with ACP agents
10-
- Manages agent lifecycle, initialization handshake, and stderr capture for diagnostic logging
8+
- Uses the official `agent-client-protocol` v0.7 library instead of any custom JSON-RPC implementation
119
- Exports `init_file_tracing()` for file-based structured logging at DEBUG level
12-
- **Critical**: All ACP operations use !Send futures from `agent-client-protocol`, requiring `LocalSet` contexts
1310

1411
### How it fits into the larger codebase
1512

16-
- Used by `@/codex-rs/core/src/client.rs` to spawn and communicate with ACP-compliant agents via `WireApi::Acp` variant
17-
- `AcpModelClient` is designed to mirror the `ModelClient` interface for future core integration
18-
- Enables Codex to delegate AI operations to external providers (Claude, Gemini, etc.) that implement the ACP specification
19-
- Complements the existing OpenAI-style API path in core by providing an alternative subprocess-based agent model
13+
- Used by `@/codex-rs/core/src/client.rs` to communicate with ACP-compliant agents via `WireApi::Acp` variant
2014
- Uses channel-based streaming pattern (mpsc) consistent with core's `ResponseStream`
2115
- Provides structured error handling via library's typed error responses that core translates to user-facing messages
2216
- TUI and other clients can access captured stderr for displaying agent diagnostic output
23-
- Re-exports commonly used types from `agent-client-protocol` library for convenience (`Agent`, `Client`, `ClientSideConnection`, request/response types)
2417

25-
### Core Implementation
26-
27-
**High-Level API:** `AcpModelClient::stream()` in `@/codex-rs/acp/src/acp_client.rs`
28-
29-
- Encapsulates the full flow: spawn, initialize, session/new, session/prompt, stream notifications, complete
30-
- Returns an `AcpStream` implementing the futures `Stream` trait for async iteration
31-
- Events are delivered via `AcpEvent` enum: `TextDelta`, `ReasoningDelta`, `Completed`, `Error`
32-
- Uses mpsc channel (capacity 16) for backpressure-aware event delivery
33-
- **Spawns dedicated thread with LocalSet** because agent-client-protocol futures are !Send
34-
35-
**Low-Level Entry Point:** `AgentProcess::spawn()` in `@/codex-rs/acp/src/agent.rs`
36-
37-
- Creates a tokio subprocess with piped stdin/stdout/stderr
38-
- Wraps stdio streams with tokio-util compat layer to bridge tokio's AsyncRead/Write with futures crate traits
39-
- Creates `ClientSideConnection` from agent-client-protocol library, passing `spawn_local` callback for !Send futures
40-
- Spawns detached tokio task to asynchronously read stderr lines into a thread-safe buffer
41-
- Returns `AgentProcess` wrapping the connection, child process, and event channels
42-
43-
**Protocol Flow (via AcpModelClient and Library):**
44-
45-
```
46-
AcpModelClient AgentProcess ClientSideConnection Agent Subprocess
47-
| | | |
48-
|--- stream() -------->| | |
49-
| (spawns thread + | | |
50-
| LocalSet) | | |
51-
| |--- spawn() -------------->|--- Command::spawn() -->|
52-
| | | |
53-
| |--- initialize() -------->|--- Agent::initialize -->|
54-
| | |<-- InitializeResponse --|
55-
| | | |
56-
| |--- new_session() ------->|--- Agent::new_session ->|
57-
| | |<-- NewSessionResponse --|
58-
| | | |
59-
| |--- prompt() ------------>|--- Agent::prompt ------>|
60-
| | | |
61-
| |<-- ClientEvent::SessionUpdate <-- Client::session_notification()
62-
|<-- AcpEvent::TextDelta | |
63-
| |<-- ClientEvent::SessionUpdate <-- Client::session_notification()
64-
|<-- AcpEvent::TextDelta | |
65-
| | |<-- PromptResponse ------|
66-
|<-- AcpEvent::Completed | |
67-
| |--- kill() -------------->|--- SIGKILL ------------>|
68-
```
69-
70-
**Key Components:**
71-
72-
- `AcpModelClient` in `@/codex-rs/acp/src/acp_client.rs` - High-level client for streaming prompt responses, spawns dedicated thread with LocalSet
73-
- `AcpStream` in `@/codex-rs/acp/src/acp_client.rs` - Futures-compatible stream wrapping mpsc receiver
74-
- `AgentProcess` in `@/codex-rs/acp/src/agent.rs` - Wraps ClientSideConnection, manages subprocess lifecycle and stderr capture
75-
- `AcpClientHandler` in `@/codex-rs/acp/src/client_handler.rs` - Implements Client trait for handling agent callbacks (permission requests, session updates)
76-
- `ClientEvent` enum in `@/codex-rs/acp/src/client_handler.rs` - Forwarding type for client callbacks sent through mpsc channel
77-
- `ClientSideConnection` from `agent-client-protocol` library - Implements Agent trait for sending requests to subprocess
78-
- `AcpSession` in `@/codex-rs/acp/src/session.rs` - Session state management placeholder
79-
- `get_agent_config()` in `@/codex-rs/acp/src/registry.rs` - Maps model names to subprocess commands, args, and provider identifier
80-
- `AcpAgentConfig` in `@/codex-rs/acp/src/registry.rs` - Configuration struct containing provider, command, and args for spawning agent subprocess
81-
- `init_file_tracing()` in `@/codex-rs/acp/src/tracing_setup.rs` - Initializes file-based tracing subscriber
82-
83-
### Things to Know
84-
85-
**LocalSet Requirement and !Send Futures:**
86-
87-
The `agent-client-protocol` library uses !Send futures, requiring all ACP operations to run within a `LocalSet`:
88-
- `AcpModelClient::stream()` spawns a dedicated thread with single-threaded runtime + LocalSet to isolate !Send futures
89-
- All tests wrap execution in `LocalSet::run_until()`
90-
- `AgentProcess::spawn()` provides `spawn_local` callback to ClientSideConnection for spawning !Send tasks
91-
- This isolation prevents !Send futures from leaking into the main Codex runtime which uses multi-threaded tokio
92-
- The thread boundary acts as an isolation layer: Send channel messages cross thread boundaries, !Send futures stay contained
93-
94-
**Compat Layer Requirement:**
95-
96-
Bridging tokio and futures crate async traits requires tokio-util compat layer:
97-
- tokio provides `tokio::io::{AsyncRead, AsyncWrite}`
98-
- agent-client-protocol expects `futures::io::{AsyncRead, AsyncWrite}`
99-
- `TokioAsyncReadCompatExt::compat()` and `TokioAsyncWriteCompatExt::compat_write()` convert between them
100-
- Applied to child process stdin/stdout before passing to ClientSideConnection
101-
102-
**Client Callback Architecture:**
103-
104-
Agent callbacks are handled through a channel-based forwarding pattern:
105-
- `AcpClientHandler` implements the `Client` trait from agent-client-protocol
106-
- Callbacks (`request_permission`, `session_notification`) forward to mpsc channel as `ClientEvent` enum
107-
- `AgentProcess::next_client_event()` exposes the receiver for consuming callbacks
108-
- `AcpModelClient` processes `ClientEvent::SessionUpdate` to emit `AcpEvent::{TextDelta, ReasoningDelta}`
109-
- Permission requests currently auto-cancel (TODO: implement proper permission handling)
110-
- File operations and terminal operations return `method_not_found` errors
111-
112-
**Model Registry and Lookup Architecture:**
18+
### Model Registry
11319

11420
The ACP registry in `@/codex-rs/acp/src/registry.rs` is **model-centric** rather than provider-centric:
11521
- `get_agent_config()` accepts model names (e.g., "mock-model", "gemini-flash-2.5") instead of provider names
@@ -122,33 +28,15 @@ The ACP registry in `@/codex-rs/acp/src/registry.rs` is **model-centric** rather
12228
- Uses exact matching only (no prefix matching) - each model must be explicitly registered
12329
- The `provider` field enables future optimization to determine when existing subprocess can be reused vs when new one must be spawned when switching models
12430

125-
**Session Lifecycle:**
126-
127-
Each `AcpModelClient::stream()` call spawns a fresh agent process:
128-
- Agent is initialized with hardcoded capabilities: `fs.readTextFile`, `fs.writeTextFile`, `terminal`
129-
- Session created via `session/new` with `cwd` and empty `mcpServers`
130-
- Prompt sent via `session/prompt` with text content block format
131-
- Library's `Agent::prompt()` returns when final response received; session updates arrive via Client callbacks
132-
- Agent is killed after stream completes or errors
13331

134-
**Typed Request/Response Pattern:**
32+
### Stderr Capture Implementation
13533

136-
The agent-client-protocol library provides typed request/response structs:
137-
- `InitializeRequest`/`InitializeResponse` - Protocol handshake with capability negotiation
138-
- `NewSessionRequest`/`NewSessionResponse` - Session creation with cwd and MCP servers
139-
- `PromptRequest`/`PromptResponse` - Prompt submission with content blocks
140-
- `SessionNotification` - Async notifications for session updates (agent message/thought chunks)
141-
- Eliminates manual JSON-RPC message construction and parsing
142-
- Provides compile-time type safety for protocol conformance
143-
144-
**Stderr Capture Implementation:**
145-
146-
- Buffer uses `Arc<Mutex<Vec<String>>>` for thread-safe access between reader task and caller
34+
- Buffer lines per session for access between reader task and caller
14735
- Bounded at 500 lines with FIFO eviction when full
14836
- Individual lines truncated to 10KB
14937
- Reader task runs until EOF or error, logging warnings via tracing
15038

151-
**File-Based Tracing:**
39+
### File-Based Tracing
15240

15341
The `init_file_tracing()` function in `@/codex-rs/acp/src/tracing_setup.rs` provides structured file logging:
15442
- Sets global tracing subscriber that writes to a user-specified file path
@@ -160,20 +48,8 @@ The `init_file_tracing()` function in `@/codex-rs/acp/src/tracing_setup.rs` prov
16048
- ANSI colors disabled for clean file output
16149
- Automatically initialized by the CLI (`@/codex-rs/cli/src/main.rs`) at startup, writing to `.codex-acp.log` in the current working directory
16250

163-
**Test coverage:**
164-
165-
- Thin slice integration tests in `@/codex-rs/acp/tests/thin_slice.rs` verify end-to-end streaming with mock agent, wrapped in LocalSet
166-
- Unit tests in `agent.rs` use shell commands to test stderr capture, buffer overflow, and line truncation - all wrapped in LocalSet
167-
- Integration tests in `@/codex-rs/acp/tests/integration.rs` test protocol handshake with typed requests/responses from library
168-
- Tracing integration test in `@/codex-rs/acp/tests/tracing_test.rs` validates file creation, log level filtering, and re-initialization error handling
169-
- TUI black-box tests in `@/codex-rs/tui-integration-tests` exercise full application flow including ACP protocol
170-
171-
**Removed Custom Implementation (~250 lines eliminated):**
51+
### Core Implementation
17252

173-
Prior to this refactoring, the crate contained custom JSON-RPC implementations that have been replaced by the agent-client-protocol library:
174-
- `protocol.rs` (~120 lines) - Custom `JsonRpcRequest`, `JsonRpcResponse`, `JsonRpcNotification`, `JsonRpcError` types
175-
- `transport.rs` (~130 lines) - Custom `StdioTransport` with manual JSON-RPC serialization/deserialization
176-
- Eliminated manual JSON parsing, error handling, and protocol conformance issues
177-
- Library provides spec compliance, type safety, and automatic protocol updates
53+
TODO!
17854

17955
Created and maintained by Nori.

0 commit comments

Comments
 (0)