Skip to content

Commit 96cafb4

Browse files
authored
feat(acp): Add Claude ACP support via @zed-industries/claude-code-acp (#58)
## Summary Generated with [Nori](https://www.npmjs.com/package/nori-ai) - Add Claude as an ACP provider alongside Gemini via `@zed-industries/claude-code-acp` package - Add Claude ModelPreset for UI model selection in `/model` command - Add unit tests for Claude ACP registry config and model family - Add live E2E tests for ACP models (opt-in with `--ignored` flag, requires ANTHROPIC_API_KEY/GEMINI_API_KEY) - Update documentation in docs.md files - Fix clippy warnings for uninlined format args (pre-existing) - Update snapshot tests for UI changes (pre-existing /review command addition) ## Test Plan - [x] Unit tests pass: `cargo test -p codex-acp` and `cargo test -p codex-core client_acp_tests --lib` - [x] E2E snapshot tests pass: `cargo insta test` - [x] Live ACP test with ANTHROPIC_API_KEY: `cargo test -p tui-pty-e2e -- --ignored test_claude_acp_live_response` - [x] CI passes Share Nori with your team: https://www.npmjs.com/package/nori-ai
1 parent 29d0543 commit 96cafb4

15 files changed

Lines changed: 200 additions & 23 deletions

codex-rs/acp/docs.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,18 @@ Path: @/codex-rs/acp
1818
### Model Registry
1919

2020
The ACP registry in `@/codex-rs/acp/src/registry.rs` is **model-centric** rather than provider-centric:
21-
- `get_agent_config()` accepts model names (e.g., "mock-model", "gemini-2.5-flash") instead of provider names
21+
- `get_agent_config()` accepts model names (e.g., "mock-model", "gemini-2.5-flash", "claude-acp") instead of provider names
2222
- Called from `@/codex-rs/core/src/client.rs` at the start of `stream()` to check if model is an ACP agent
2323
- Returns `AcpAgentConfig` containing:
24-
- `provider_slug`: Identifies which agent subprocess to spawn (e.g., "mock-acp", "gemini-acp")
24+
- `provider_slug`: Identifies which agent subprocess to spawn (e.g., "mock-acp", "gemini-acp", "claude-acp")
2525
- `command`: Executable path or command name
2626
- `args`: Arguments to pass to the subprocess
2727
- `provider_info`: Embedded `AcpProviderInfo` with provider configuration (name, retry settings, timeouts)
2828
- Model names are normalized to lowercase for case-insensitive matching (e.g., "Gemini-2.5-Flash" → "gemini-2.5-flash")
2929
- Uses exact matching only (no prefix matching) - each model must be explicitly registered
3030
- The `provider_slug` field enables future optimization to determine when existing subprocess can be reused vs when new one must be spawned when switching models
31+
- Claude ACP is registered for both "claude" and "claude-acp" model names, using `npx @zed-industries/claude-code-acp` command with no arguments
32+
- Unit test `test_get_claude_model_config()` verifies Claude ACP registry configuration
3133

3234
### Embedded Provider Info
3335

codex-rs/acp/src/connection.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@ use std::rc::Rc;
1515
use std::thread;
1616

1717
use agent_client_protocol as acp;
18-
use anyhow::{Context, Result};
18+
use anyhow::Context;
19+
use anyhow::Result;
1920
use futures::AsyncBufReadExt;
2021
use futures::io::BufReader;
21-
use tokio::process::{Child, Command};
22+
use tokio::process::Child;
23+
use tokio::process::Command;
2224
use tokio::sync::mpsc;
2325
use tokio::sync::oneshot;
24-
use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
25-
use tracing::{debug, warn};
26+
use tokio_util::compat::TokioAsyncReadCompatExt;
27+
use tokio_util::compat::TokioAsyncWriteCompatExt;
28+
use tracing::debug;
29+
use tracing::warn;
2630

2731
use crate::registry::AcpAgentConfig;
2832

@@ -80,7 +84,10 @@ impl AcpConnection {
8084

8185
// Spawn a dedicated thread with a single-threaded tokio runtime
8286
let worker_thread = thread::spawn(move || {
83-
#[expect(clippy::expect_used, reason = "Runtime creation in dedicated thread is infallible in practice")]
87+
#[expect(
88+
clippy::expect_used,
89+
reason = "Runtime creation in dedicated thread is infallible in practice"
90+
)]
8491
let rt = tokio::runtime::Builder::new_current_thread()
8592
.enable_all()
8693
.build()
@@ -407,8 +414,8 @@ impl acp::Client for ClientDelegate {
407414
arguments: acp::ReadTextFileRequest,
408415
) -> acp::Result<acp::ReadTextFileResponse> {
409416
// Read file content
410-
let content = std::fs::read_to_string(&arguments.path)
411-
.map_err(acp::Error::into_internal_error)?;
417+
let content =
418+
std::fs::read_to_string(&arguments.path).map_err(acp::Error::into_internal_error)?;
412419
Ok(acp::ReadTextFileResponse {
413420
content,
414421
meta: None,
@@ -537,8 +544,7 @@ mod tests {
537544
);
538545
assert!(
539546
messages.iter().any(|m| m.contains("Test message")),
540-
"Should contain test message, got: {:?}",
541-
messages
547+
"Should contain test message, got: {messages:?}"
542548
);
543549
assert_eq!(stop_reason, acp::StopReason::EndTurn);
544550
}

codex-rs/acp/src/registry.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,16 @@ mod tests {
154154
assert_eq!(config.provider_info.name, "Gemini ACP");
155155
}
156156

157+
#[test]
158+
fn test_get_claude_model_config() {
159+
let config = get_agent_config("claude-acp").expect("Should return config for claude-acp");
160+
161+
assert_eq!(config.provider_slug, "claude-acp");
162+
assert_eq!(config.command, "npx");
163+
assert_eq!(config.args, vec!["@zed-industries/claude-code-acp"]);
164+
assert_eq!(config.provider_info.name, "Claude ACP");
165+
}
166+
157167
#[test]
158168
fn test_get_unknown_model_returns_error() {
159169
let result = get_agent_config("unknown-model-xyz");

codex-rs/acp/src/translator.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
//! (Agent Client Protocol) data types and the codex internal data types.
55
66
use agent_client_protocol as acp;
7-
use codex_protocol::models::{ContentItem, ResponseItem};
7+
use codex_protocol::models::ContentItem;
8+
use codex_protocol::models::ResponseItem;
89

910
/// Convert codex ResponseItems to ACP ContentBlocks for prompting.
1011
///

codex-rs/acp/tests/tracing_test.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ fn test_file_tracing_comprehensive() {
3232
// Verify log file exists
3333
assert!(
3434
log_file_path.exists(),
35-
"Log file should exist at {:?}",
36-
log_file_path
35+
"Log file should exist at {log_file_path:?}"
3736
);
3837

3938
// Read and verify log file contents

codex-rs/common/docs.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ pub struct CliConfigOverrides {
5050

5151
**Model Presets:**
5252

53-
`model_presets` defines available models by provider with capabilities:
54-
- Default reasoning effort levels
53+
`model_presets` in `@/codex-rs/common/src/model_presets.rs` defines available models by provider with capabilities:
54+
- Default reasoning effort levels (set to Medium for all models)
5555
- Summary generation support
5656
- Tool capabilities
57+
- Claude ACP preset added with display_name "Claude" and description "Anthropic's Claude via Agent Context Protocol" to make Claude model visible in TUI model selection
5758

5859
**Approval Presets:**
5960

codex-rs/common/src/model_presets.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
6969
is_default: false,
7070
upgrade: None,
7171
},
72+
ModelPreset {
73+
id: "claude-acp",
74+
model: "claude-acp",
75+
display_name: "Claude",
76+
description: "Anthropic's Claude via Agent Context Protocol.",
77+
default_reasoning_effort: ReasoningEffort::Medium,
78+
supported_reasoning_efforts: &[],
79+
is_default: false,
80+
upgrade: None,
81+
},
7282
ModelPreset {
7383
id: "gpt-5.1-codex",
7484
model: "gpt-5.1-codex",

codex-rs/core/docs.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ The `client.rs` defines `ModelClient` trait implemented by:
8888

8989
Response streaming uses `ResponseStream` of `ResponseEvent` items.
9090

91-
For ACP providers (`wire_api: WireApi::Acp`), the client looks up subprocess configuration via `codex_acp::get_agent_config(self.config.model)` from `@/codex-rs/acp/src/registry.rs`. The registry is **model-centric**: it maps model names (e.g., "mock-model", "gemini-2.5-flash") to `AcpAgentConfig` structs containing provider identifier, command, and args. This differs from the provider-based approach used for HTTP APIs. ACP providers should not define `env_key` or `env_key_instructions` in their `ModelProviderInfo` entries, as they communicate via subprocess rather than HTTP APIs.
91+
For ACP providers (`wire_api: WireApi::Acp`), the client looks up subprocess configuration via `codex_acp::get_agent_config(self.config.model)` from `@/codex-rs/acp/src/registry.rs`. The registry is **model-centric**: it maps model names (e.g., "mock-model", "gemini-2.5-flash", "claude-acp") to `AcpAgentConfig` structs containing provider identifier, command, and args. This differs from the provider-based approach used for HTTP APIs. ACP providers should not define `env_key` or `env_key_instructions` in their `ModelProviderInfo` entries, as they communicate via subprocess rather than HTTP APIs. Unit test `test_claude_acp_model_has_family()` in `@/codex-rs/core/src/client_acp_tests.rs` verifies that Claude ACP models resolve to a valid model family.
9292

9393
**ACP Streaming Flow (`stream_acp` / `stream_acp_internal`):**
9494

codex-rs/core/src/client.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,10 +1105,13 @@ async fn stream_acp_internal(
11051105
tx: mpsc::Sender<Result<ResponseEvent>>,
11061106
) -> Result<()> {
11071107
use agent_client_protocol::SessionUpdate;
1108-
use codex_acp::translator::{TranslatedEvent, translate_session_update};
1109-
use codex_protocol::models::{ContentItem, ResponseItem};
1108+
use codex_acp::translator::TranslatedEvent;
1109+
use codex_acp::translator::translate_session_update;
1110+
use codex_protocol::models::ContentItem;
1111+
use codex_protocol::models::ResponseItem;
11101112
use std::sync::Arc;
1111-
use std::sync::atomic::{AtomicBool, Ordering};
1113+
use std::sync::atomic::AtomicBool;
1114+
use std::sync::atomic::Ordering;
11121115

11131116
// Spawn ACP connection
11141117
let connection = codex_acp::AcpConnection::spawn(config, cwd)

codex-rs/core/src/client_acp_tests.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,15 @@ mod tests {
5151
"gemini-acp model should have a model family"
5252
);
5353
}
54+
55+
#[test]
56+
fn test_claude_acp_model_has_family() {
57+
use crate::model_family::find_family_for_model;
58+
59+
let family = find_family_for_model("claude-acp");
60+
assert!(
61+
family.is_some(),
62+
"claude-acp model should have a model family"
63+
);
64+
}
5465
}

0 commit comments

Comments
 (0)