Skip to content

Commit 129a49e

Browse files
committed
fix(acp): Align all sample gemini ACP names
1 parent aa47c80 commit 129a49e

4 files changed

Lines changed: 20 additions & 18 deletions

File tree

codex-rs/acp/docs.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,13 @@ Agent callbacks are handled through a channel-based forwarding pattern:
112112
**Model Registry and Lookup Architecture:**
113113

114114
The ACP registry in `@/codex-rs/acp/src/registry.rs` is **model-centric** rather than provider-centric:
115-
- `get_agent_config()` accepts model names (e.g., "mock-model", "gemini-flash-2.0") instead of provider names
115+
- `get_agent_config()` accepts model names (e.g., "mock-model", "gemini-flash-2.5") instead of provider names
116116
- Called from `@/codex-rs/core/src/client.rs` with `self.config.model` when handling `WireApi::Acp`
117117
- Returns `AcpAgentConfig` containing three fields:
118118
- `provider`: Identifies which agent subprocess to spawn (e.g., "mock-acp", "gemini-acp")
119119
- `command`: Executable path or command name
120120
- `args`: Arguments to pass to the subprocess
121-
- Model names are normalized to lowercase for case-insensitive matching (e.g., "Gemini-Flash-2.0" → "gemini-flash-2.0")
121+
- Model names are normalized to lowercase for case-insensitive matching (e.g., "Gemini-Flash-2.5" → "gemini-flash-2.5")
122122
- Uses exact matching only (no prefix matching) - each model must be explicitly registered
123123
- The `provider` field enables future optimization to determine when existing subprocess can be reused vs when new one must be spawned when switching models
124124

codex-rs/acp/src/registry.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub struct AcpAgentConfig {
2020
/// Get ACP agent configuration for a given model name
2121
///
2222
/// # Arguments
23-
/// * `model_name` - The model identifier (e.g., "mock-model", "gemini-flash-2.0")
23+
/// * `model_name` - The model identifier (e.g., "mock-model", "gemini-flash-2.5")
2424
/// Names are normalized to lowercase for case-insensitive matching.
2525
///
2626
/// # Returns
@@ -38,14 +38,18 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
3838
// This handles both debug and release builds
3939
let exe_path = match std::env::current_exe() {
4040
Ok(p) => {
41-
let mock_path = p.parent()
41+
let mock_path = p
42+
.parent()
4243
.map(|parent| parent.join("mock_acp_agent"))
4344
.unwrap_or_else(|| std::path::PathBuf::from("mock_acp_agent"));
4445
tracing::debug!("Mock ACP agent path resolved to: {}", mock_path.display());
4546
mock_path
4647
}
4748
Err(e) => {
48-
tracing::warn!("Failed to get current_exe for mock-model: {}, falling back to 'mock_acp_agent'", e);
49+
tracing::warn!(
50+
"Failed to get current_exe for mock-model: {}, falling back to 'mock_acp_agent'",
51+
e
52+
);
4953
std::path::PathBuf::from("mock_acp_agent")
5054
}
5155
};
@@ -56,7 +60,7 @@ pub fn get_agent_config(model_name: &str) -> Result<AcpAgentConfig> {
5660
args: vec![],
5761
})
5862
}
59-
"gemini-flash-2.0" => Ok(AcpAgentConfig {
63+
"gemini-2.5-flash" | "gemini-acp" => Ok(AcpAgentConfig {
6064
provider: "gemini-acp".to_string(),
6165
command: "npx".to_string(),
6266
args: vec![
@@ -87,7 +91,8 @@ mod tests {
8791

8892
#[test]
8993
fn test_get_gemini_model_config() {
90-
let config = get_agent_config("gemini-flash-2.0").expect("Should return config for gemini-flash-2.0");
94+
let config = get_agent_config("gemini-2.5-flash")
95+
.expect("Should return config for gemini-2.5-flash");
9196

9297
assert_eq!(config.provider, "gemini-acp");
9398
assert_eq!(config.command, "npx");
@@ -110,19 +115,19 @@ mod tests {
110115
fn test_get_agent_config_normalizes_model_names() {
111116
// Should work with lowercase model names
112117
assert!(
113-
get_agent_config("gemini-flash-2.0").is_ok(),
114-
"Lowercase 'gemini-flash-2.0' should work"
118+
get_agent_config("gemini-2.5-flash").is_ok(),
119+
"Lowercase 'gemini-2.5-flash' should work"
115120
);
116121
assert!(
117122
get_agent_config("mock-model").is_ok(),
118123
"Lowercase 'mock-model' should work"
119124
);
120125

121126
// Should work with mixed case (normalized to lowercase)
122-
let gemini_result = get_agent_config("Gemini-Flash-2.0");
127+
let gemini_result = get_agent_config("Gemini-2.5-Flash");
123128
assert!(
124129
gemini_result.is_ok(),
125-
"Mixed case 'Gemini-Flash-2.0' should work"
130+
"Mixed case 'Gemini-2.5-Flash' should work"
126131
);
127132
assert_eq!(
128133
gemini_result.unwrap().provider,
@@ -140,10 +145,7 @@ mod tests {
140145

141146
// Should still reject unknown models
142147
let unknown_result = get_agent_config("unknown-model-xyz");
143-
assert!(
144-
unknown_result.is_err(),
145-
"Unknown model should return error"
146-
);
148+
assert!(unknown_result.is_err(), "Unknown model should return error");
147149
let err_msg = unknown_result.unwrap_err().to_string();
148150
assert!(
149151
err_msg.contains("unknown-model-xyz"),

codex-rs/common/src/model_presets.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
6060
upgrade: None,
6161
},
6262
ModelPreset {
63-
id: "gemini-2.0-flash-thinking-exp",
64-
model: "gemini-2.0-flash-thinking-exp",
63+
id: "gemini-2.5-flash",
64+
model: "gemini-2.5-flash",
6565
display_name: "Gemini 2.0 Flash Thinking",
6666
description: "Google's experimental thinking model.",
6767
default_reasoning_effort: ReasoningEffort::Medium,

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-flash-2.0") 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") 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.
9292

9393
**Session Recording:**
9494

0 commit comments

Comments
 (0)