Skip to content

Commit 0c75ad8

Browse files
CSResselclaude
andauthored
feat(acp): Update ACP registry with Codex adapter (#89)
- Add codex-acp agent with @zed-industries/codex-acp package - Use cfg(debug_assertions) to only include mock agents in non-release builds - Update display names to not include model versions (Claude, Codex, Gemini) - Add E2E test to verify agent picker shows correct agents in debug builds (5 agents: Mock ACP, Mock ACP Alt, Claude, Codex, Gemini) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 431dfab commit 0c75ad8

2 files changed

Lines changed: 214 additions & 54 deletions

File tree

codex-rs/acp/src/registry.rs

Lines changed: 118 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,46 @@ pub struct AcpAgentInfo {
2121

2222
/// Get list of all available ACP agents for the agent picker
2323
pub fn list_available_agents() -> Vec<AcpAgentInfo> {
24-
vec![
25-
AcpAgentInfo {
24+
let mut agents = Vec::new();
25+
26+
// Mock agents are only available in debug builds (for testing)
27+
#[cfg(debug_assertions)]
28+
{
29+
agents.push(AcpAgentInfo {
2630
model_name: "mock-model".to_string(),
2731
display_name: "Mock ACP".to_string(),
2832
description: "Mock agent for testing".to_string(),
2933
provider_slug: "mock-acp".to_string(),
30-
},
31-
AcpAgentInfo {
34+
});
35+
agents.push(AcpAgentInfo {
3236
model_name: "mock-model-alt".to_string(),
3337
display_name: "Mock ACP Alt".to_string(),
3438
description: "Alternate mock agent for testing".to_string(),
3539
provider_slug: "mock-acp-alt".to_string(),
36-
},
37-
AcpAgentInfo {
38-
model_name: "gemini-2.5-flash".to_string(),
39-
display_name: "Gemini 2.5 Flash".to_string(),
40-
description: "Google Gemini via ACP".to_string(),
41-
provider_slug: "gemini-acp".to_string(),
42-
},
43-
AcpAgentInfo {
44-
model_name: "claude-4.5".to_string(),
45-
display_name: "Claude 4.5".to_string(),
46-
description: "Anthropic Claude via ACP".to_string(),
47-
provider_slug: "claude-acp".to_string(),
48-
},
49-
]
40+
});
41+
}
42+
43+
// Production agents (always available)
44+
agents.push(AcpAgentInfo {
45+
model_name: "claude-acp".to_string(),
46+
display_name: "Claude".to_string(),
47+
description: "Anthropic Claude via ACP".to_string(),
48+
provider_slug: "claude-acp".to_string(),
49+
});
50+
agents.push(AcpAgentInfo {
51+
model_name: "codex-acp".to_string(),
52+
display_name: "Codex".to_string(),
53+
description: "OpenAI Codex via ACP".to_string(),
54+
provider_slug: "codex-acp".to_string(),
55+
});
56+
agents.push(AcpAgentInfo {
57+
model_name: "gemini-acp".to_string(),
58+
display_name: "Gemini".to_string(),
59+
description: "Google Gemini via ACP".to_string(),
60+
provider_slug: "gemini-acp".to_string(),
61+
});
62+
63+
agents
5064
}
5165

5266
/// Default idle timeout for ACP streaming (5 minutes)
@@ -106,7 +120,52 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
106120
// Normalize model name: lowercase
107121
let normalized = model_name.to_lowercase();
108122

123+
// Try mock agents first (only available in debug builds)
124+
#[cfg(debug_assertions)]
125+
if let Some(config) = get_mock_agent_config(&normalized) {
126+
return Ok(config);
127+
}
128+
129+
// Production agents
109130
match normalized.as_str() {
131+
"gemini-2.5-flash" | "gemini-acp" => Ok(AcpAgentConfig {
132+
provider_slug: "gemini-acp".to_string(),
133+
command: "npx".to_string(),
134+
args: vec![
135+
"@google/gemini-cli".to_string(),
136+
"--experimental-acp".to_string(),
137+
],
138+
provider_info: AcpProviderInfo {
139+
name: "Gemini ACP".to_string(),
140+
..Default::default()
141+
},
142+
}),
143+
"claude-4.5" | "claude-acp" => Ok(AcpAgentConfig {
144+
provider_slug: "claude-acp".to_string(),
145+
command: "npx".to_string(),
146+
args: vec!["@zed-industries/claude-code-acp".to_string()],
147+
provider_info: AcpProviderInfo {
148+
name: "Claude ACP".to_string(),
149+
..Default::default()
150+
},
151+
}),
152+
"codex-acp" => Ok(AcpAgentConfig {
153+
provider_slug: "codex-acp".to_string(),
154+
command: "npx".to_string(),
155+
args: vec!["@zed-industries/codex-acp".to_string()],
156+
provider_info: AcpProviderInfo {
157+
name: "Codex ACP".to_string(),
158+
..Default::default()
159+
},
160+
}),
161+
_ => anyhow::bail!("Unknown ACP model: {model_name}"),
162+
}
163+
}
164+
165+
/// Get mock agent configuration (only available in debug builds)
166+
#[cfg(debug_assertions)]
167+
fn get_mock_agent_config(normalized: &str) -> Option<AcpAgentConfig> {
168+
match normalized {
110169
"mock-model" => {
111170
// Resolve path to mock_acp_agent binary.
112171
//
@@ -144,7 +203,7 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
144203
mock_path
145204
};
146205

147-
Ok(AcpAgentConfig {
206+
Some(AcpAgentConfig {
148207
provider_slug: "mock-acp".to_string(),
149208
command: exe_path.to_string_lossy().to_string(),
150209
args: vec![],
@@ -184,7 +243,7 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
184243
mock_path
185244
};
186245

187-
Ok(AcpAgentConfig {
246+
Some(AcpAgentConfig {
188247
provider_slug: "mock-acp-alt".to_string(),
189248
command: exe_path.to_string_lossy().to_string(),
190249
args: vec![],
@@ -194,28 +253,7 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
194253
},
195254
})
196255
}
197-
"gemini-2.5-flash" | "gemini-acp" => Ok(AcpAgentConfig {
198-
provider_slug: "gemini-acp".to_string(),
199-
command: "npx".to_string(),
200-
args: vec![
201-
"@google/gemini-cli".to_string(),
202-
"--experimental-acp".to_string(),
203-
],
204-
provider_info: AcpProviderInfo {
205-
name: "Gemini ACP".to_string(),
206-
..Default::default()
207-
},
208-
}),
209-
"claude-4.5" | "claude-acp" => Ok(AcpAgentConfig {
210-
provider_slug: "claude-acp".to_string(),
211-
command: "npx".to_string(),
212-
args: vec!["@zed-industries/claude-code-acp".to_string()],
213-
provider_info: AcpProviderInfo {
214-
name: "Claude ACP".to_string(),
215-
..Default::default()
216-
},
217-
}),
218-
_ => anyhow::bail!("Unknown ACP model: {model_name}"),
256+
_ => None,
219257
}
220258
}
221259

@@ -224,6 +262,7 @@ mod tests {
224262
use super::*;
225263

226264
#[test]
265+
#[cfg(debug_assertions)]
227266
fn test_get_mock_model_config() {
228267
let config = get_agent_config("mock-model").expect("Should return config for mock-model");
229268

@@ -240,6 +279,7 @@ mod tests {
240279
}
241280

242281
#[test]
282+
#[cfg(debug_assertions)]
243283
fn test_get_mock_model_alt_config() {
244284
let config =
245285
get_agent_config("mock-model-alt").expect("Should return config for mock-model-alt");
@@ -278,6 +318,16 @@ mod tests {
278318
assert_eq!(config.provider_info.name, "Claude ACP");
279319
}
280320

321+
#[test]
322+
fn test_get_codex_model_config() {
323+
let config = get_agent_config("codex-acp").expect("Should return config for codex-acp");
324+
325+
assert_eq!(config.provider_slug, "codex-acp");
326+
assert_eq!(config.command, "npx");
327+
assert_eq!(config.args, vec!["@zed-industries/codex-acp"]);
328+
assert_eq!(config.provider_info.name, "Codex ACP");
329+
}
330+
281331
#[test]
282332
fn test_get_unknown_model_returns_error() {
283333
let result = get_agent_config("unknown-model-xyz");
@@ -294,10 +344,6 @@ mod tests {
294344
get_agent_config("gemini-2.5-flash").is_ok(),
295345
"Lowercase 'gemini-2.5-flash' should work"
296346
);
297-
assert!(
298-
get_agent_config("mock-model").is_ok(),
299-
"Lowercase 'mock-model' should work"
300-
);
301347

302348
// Should work with mixed case (normalized to lowercase)
303349
let gemini_result = get_agent_config("Gemini-2.5-Flash");
@@ -311,14 +357,6 @@ mod tests {
311357
"Should resolve to gemini-acp provider"
312358
);
313359

314-
let mock_result = get_agent_config("Mock-Model");
315-
assert!(mock_result.is_ok(), "Mixed case 'Mock-Model' should work");
316-
assert_eq!(
317-
mock_result.unwrap().provider_slug,
318-
"mock-acp",
319-
"Should resolve to mock-acp provider"
320-
);
321-
322360
// Should still reject unknown models
323361
let unknown_result = get_agent_config("unknown-model-xyz");
324362
assert!(unknown_result.is_err(), "Unknown model should return error");
@@ -328,4 +366,30 @@ mod tests {
328366
"Error message should contain original input"
329367
);
330368
}
369+
370+
#[test]
371+
#[cfg(debug_assertions)]
372+
fn test_list_available_agents_debug_build() {
373+
let agents = list_available_agents();
374+
// Debug build should have 5 agents: mock, mock-alt, claude, codex, gemini
375+
assert_eq!(agents.len(), 5, "Debug build should have 5 agents");
376+
377+
let names: Vec<&str> = agents.iter().map(|a| a.display_name.as_str()).collect();
378+
assert!(names.contains(&"Mock ACP"), "Should have Mock ACP");
379+
assert!(names.contains(&"Mock ACP Alt"), "Should have Mock ACP Alt");
380+
assert!(names.contains(&"Claude"), "Should have Claude");
381+
assert!(names.contains(&"Codex"), "Should have Codex");
382+
assert!(names.contains(&"Gemini"), "Should have Gemini");
383+
}
384+
385+
#[test]
386+
fn test_list_available_agents_contains_production_agents() {
387+
let agents = list_available_agents();
388+
let names: Vec<&str> = agents.iter().map(|a| a.display_name.as_str()).collect();
389+
390+
// Production agents should always be present
391+
assert!(names.contains(&"Claude"), "Should have Claude");
392+
assert!(names.contains(&"Codex"), "Should have Codex");
393+
assert!(names.contains(&"Gemini"), "Should have Gemini");
394+
}
331395
}

codex-rs/tui-pty-e2e/tests/agent_switching.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,102 @@ fn test_agent_switch_message_flow_mock_to_mock_alt() {
853853
);
854854
}
855855

856+
// ============================================================================
857+
// Test: Agent Picker Shows Correct Agents (Debug Build)
858+
// ============================================================================
859+
860+
/// Test that the agent picker shows all 5 agents in debug build.
861+
///
862+
/// In debug builds, the agent picker should show:
863+
/// - Mock ACP (mock agent for testing)
864+
/// - Mock ACP Alt (alternate mock agent for testing)
865+
/// - Claude (Anthropic Claude via ACP)
866+
/// - Codex (OpenAI Codex via ACP)
867+
/// - Gemini (Google Gemini via ACP)
868+
///
869+
/// Note: In release builds, only the 3 production agents (Claude, Codex, Gemini)
870+
/// would be shown. This test validates the debug build behavior.
871+
#[test]
872+
#[cfg(target_os = "linux")]
873+
#[cfg(debug_assertions)]
874+
fn test_agent_picker_shows_five_agents_in_debug_build() {
875+
let config = SessionConfig::new().with_model("mock-model".to_string());
876+
877+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
878+
879+
session
880+
.wait_for_text("›", TIMEOUT)
881+
.expect("TUI should start");
882+
std::thread::sleep(TIMEOUT_INPUT);
883+
884+
// Open agent picker with /agent command
885+
session.send_str("/agent").unwrap();
886+
std::thread::sleep(TIMEOUT_INPUT);
887+
session.send_key(Key::Enter).unwrap();
888+
std::thread::sleep(TIMEOUT_INPUT);
889+
890+
// Wait for agent picker to appear
891+
session
892+
.wait_for(
893+
|screen| screen.contains("Select Agent"),
894+
Duration::from_secs(3),
895+
)
896+
.expect("Agent picker should appear with title");
897+
898+
// Get screen contents to verify all agents are present
899+
let screen = session.screen_contents();
900+
901+
// Verify all 5 agents are shown in debug build
902+
// The display names should NOT include model versions (e.g., "Claude" not "Claude 4.5")
903+
assert!(
904+
screen.contains("Mock ACP"),
905+
"Agent picker should show 'Mock ACP', got: {}",
906+
screen
907+
);
908+
assert!(
909+
screen.contains("Mock ACP Alt"),
910+
"Agent picker should show 'Mock ACP Alt', got: {}",
911+
screen
912+
);
913+
assert!(
914+
screen.contains("Claude") && !screen.contains("Claude 4.5"),
915+
"Agent picker should show 'Claude' without model version, got: {}",
916+
screen
917+
);
918+
assert!(
919+
screen.contains("Codex"),
920+
"Agent picker should show 'Codex', got: {}",
921+
screen
922+
);
923+
assert!(
924+
screen.contains("Gemini") && !screen.contains("Gemini 2.5"),
925+
"Agent picker should show 'Gemini' without model version, got: {}",
926+
screen
927+
);
928+
929+
// Count agents by looking for unique agent entries
930+
// Each agent line should be distinct in the picker
931+
let agent_count = ["Mock ACP Alt", "Claude", "Codex", "Gemini"]
932+
.iter()
933+
.filter(|name| screen.contains(*name))
934+
.count()
935+
+ if screen.contains("Mock ACP") && !screen.contains("Mock ACP Alt") {
936+
1
937+
} else if screen.contains("Mock ACP") {
938+
1 // Mock ACP is present (Mock ACP Alt counted separately)
939+
} else {
940+
0
941+
};
942+
943+
// We should see all 5 agents
944+
assert!(
945+
agent_count >= 4, // At minimum Claude, Codex, Gemini, and one of the Mocks
946+
"Expected at least 4 distinct agents in picker, found approximately: {}. Screen: {}",
947+
agent_count,
948+
screen
949+
);
950+
}
951+
856952
/// Test that verifies the expected sequence of operations when switching agents
857953
/// This is a more focused test that checks specific message ordering
858954
#[test]

0 commit comments

Comments
 (0)