From 9c3aa4b9fa5ad49ed3455de83ed772ef4a25aa20 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 16 Mar 2026 23:25:04 +0100 Subject: [PATCH] agent_ui: Remove special casing for previous built in agents (#51713) Now that the migration has happened, we can stop making these ever present Release Notes: - N/A --- crates/agent_servers/src/acp.rs | 2 +- crates/agent_servers/src/custom.rs | 33 +--- crates/agent_ui/src/agent_panel.rs | 245 ++--------------------------- crates/agent_ui/src/agent_ui.rs | 87 +--------- 4 files changed, 18 insertions(+), 349 deletions(-) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 54166f1d553b4ed1ef0f3642517125b30bc5fda8..a8c8e2f6ebf2b7efeb264aa412131881564ce3b3 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -324,7 +324,7 @@ impl AcpConnection { // TODO: Remove this override once Google team releases their official auth methods let auth_methods = if agent_id.0.as_ref() == GEMINI_ID { let mut args = command.args.clone(); - args.retain(|a| a != "--experimental-acp"); + args.retain(|a| a != "--experimental-acp" && a != "--acp"); let value = serde_json::json!({ "label": "gemini /auth", "command": command.path.to_string_lossy().into_owned(), diff --git a/crates/agent_servers/src/custom.rs b/crates/agent_servers/src/custom.rs index d9a4469aefa957033a583a1061656dcb090eeec1..30bfae3a34049116a523d971952f40b3f2269cbf 100644 --- a/crates/agent_servers/src/custom.rs +++ b/crates/agent_servers/src/custom.rs @@ -405,8 +405,6 @@ fn api_key_for_gemini_cli(cx: &mut App) -> Task> { fn is_registry_agent(agent_id: impl Into, cx: &App) -> bool { let agent_id = agent_id.into(); - let is_previous_built_in = - matches!(agent_id.0.as_ref(), CLAUDE_AGENT_ID | CODEX_ID | GEMINI_ID); let is_in_registry = project::AgentRegistryStore::try_global(cx) .map(|store| store.read(cx).agent(&agent_id).is_some()) .unwrap_or(false); @@ -421,7 +419,7 @@ fn is_registry_agent(agent_id: impl Into, cx: &App) -> bool { ) }) }); - is_previous_built_in || is_in_registry || is_settings_registry + is_in_registry || is_settings_registry } fn default_settings_for_agent( @@ -509,16 +507,6 @@ mod tests { }); } - #[gpui::test] - fn test_previous_builtins_are_registry(cx: &mut TestAppContext) { - init_test(cx); - cx.update(|cx| { - assert!(is_registry_agent(CLAUDE_AGENT_ID, cx)); - assert!(is_registry_agent(CODEX_ID, cx)); - assert!(is_registry_agent(GEMINI_ID, cx)); - }); - } - #[gpui::test] fn test_unknown_agent_is_not_registry(cx: &mut TestAppContext) { init_test(cx); @@ -581,25 +569,6 @@ mod tests { }); } - #[gpui::test] - fn test_default_settings_for_builtin_agent(cx: &mut TestAppContext) { - init_test(cx); - cx.update(|cx| { - assert!(matches!( - default_settings_for_agent(CODEX_ID, cx), - settings::CustomAgentServerSettings::Registry { .. } - )); - assert!(matches!( - default_settings_for_agent(CLAUDE_AGENT_ID, cx), - settings::CustomAgentServerSettings::Registry { .. } - )); - assert!(matches!( - default_settings_for_agent(GEMINI_ID, cx), - settings::CustomAgentServerSettings::Registry { .. } - )); - }); - } - #[gpui::test] fn test_default_settings_for_extension_agent(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index e6dc1b8148067503554fbdf71a414782f805b850..4a40f3303b8aab6f7ad5794e564d79e2c4943d98 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -16,10 +16,7 @@ use agent_servers::AgentServer; use collections::HashSet; use db::kvp::{Dismissable, KEY_VALUE_STORE}; use itertools::Itertools; -use project::{ - AgentId, - agent_server_store::{CLAUDE_AGENT_ID, CODEX_ID, GEMINI_ID}, -}; +use project::AgentId; use serde::{Deserialize, Serialize}; use settings::{LanguageModelProviderSetting, LanguageModelSelection}; @@ -645,7 +642,7 @@ enum WhichFontSize { } // TODO unify this with ExternalAgent -#[derive(Debug, Default, Clone, PartialEq, Serialize)] +#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub enum AgentType { #[default] NativeAgent, @@ -656,65 +653,6 @@ pub enum AgentType { }, } -// Custom impl handles legacy variant names from before the built-in agents were moved to -// the registry: "ClaudeAgent" -> Custom { name: "claude-acp" }, "Codex" -> Custom { name: -// "codex-acp" }, "Gemini" -> Custom { name: "gemini" }. -// Can be removed at some point in the future and go back to #[derive(Deserialize)]. -impl<'de> Deserialize<'de> for AgentType { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let value = serde_json::Value::deserialize(deserializer)?; - - if let Some(s) = value.as_str() { - return match s { - "NativeAgent" => Ok(Self::NativeAgent), - "TextThread" => Ok(Self::TextThread), - "ClaudeAgent" | "ClaudeCode" => Ok(Self::Custom { - id: CLAUDE_AGENT_ID.into(), - }), - "Codex" => Ok(Self::Custom { - id: CODEX_ID.into(), - }), - "Gemini" => Ok(Self::Custom { - id: GEMINI_ID.into(), - }), - other => Err(serde::de::Error::unknown_variant( - other, - &[ - "NativeAgent", - "TextThread", - "Custom", - "ClaudeAgent", - "ClaudeCode", - "Codex", - "Gemini", - ], - )), - }; - } - - if let Some(obj) = value.as_object() { - if let Some(inner) = obj.get("Custom") { - #[derive(Deserialize)] - struct CustomFields { - name: SharedString, - } - let fields: CustomFields = - serde_json::from_value(inner.clone()).map_err(serde::de::Error::custom)?; - return Ok(Self::Custom { - id: AgentId::new(fields.name), - }); - } - } - - Err(serde::de::Error::custom( - "expected a string variant or {\"Custom\": {\"name\": ...}}", - )) - } -} - impl AgentType { pub fn is_native(&self) -> bool { matches!(self, Self::NativeAgent) @@ -4045,10 +3983,8 @@ impl AgentPanel { .header("External Agents") .map(|mut menu| { let agent_server_store = agent_server_store.read(cx); - let registry_store = - project::AgentRegistryStore::try_global(cx); - let registry_store_ref = - registry_store.as_ref().map(|s| s.read(cx)); + let registry_store = project::AgentRegistryStore::try_global(cx); + let registry_store_ref = registry_store.as_ref().map(|s| s.read(cx)); struct AgentMenuItem { id: AgentId, @@ -4076,12 +4012,10 @@ impl AgentPanel { .collect::>(); for item in &agent_items { - let mut entry = - ContextMenuEntry::new(item.display_name.clone()); + let mut entry = ContextMenuEntry::new(item.display_name.clone()); - let icon_path = agent_server_store - .agent_icon(&item.id) - .or_else(|| { + let icon_path = + agent_server_store.agent_icon(&item.id).or_else(|| { registry_store_ref .as_ref() .and_then(|store| store.agent(&item.id)) @@ -4100,9 +4034,9 @@ impl AgentPanel { id: item.id.clone(), }), |this| { - this.action(Box::new( - NewExternalAgentThread { agent: None }, - )) + this.action(Box::new(NewExternalAgentThread { + agent: None, + })) }, ) .icon_color(Color::Muted) @@ -4137,111 +4071,14 @@ impl AgentPanel { menu }) .separator() - .map(|mut menu| { - let agent_server_store = agent_server_store.read(cx); - let registry_store = - project::AgentRegistryStore::try_global(cx); - let registry_store_ref = - registry_store.as_ref().map(|s| s.read(cx)); - - let previous_built_in_ids: &[AgentId] = - &[CLAUDE_AGENT_ID.into(), CODEX_ID.into(), GEMINI_ID.into()]; - - let promoted_items = previous_built_in_ids - .iter() - .filter(|id| { - !agent_server_store.external_agents.contains_key(*id) - }) - .filter_map(|id| { - let display_name = registry_store_ref - .as_ref() - .and_then(|store| store.agent(&id)) - .map(|a| a.name().clone())?; - Some((id.clone(), display_name)) - }) - .sorted_unstable_by_key(|(_, display_name)| display_name.to_lowercase()) - .collect::>(); - - for (agent_id, display_name) in &promoted_items { - let mut entry = - ContextMenuEntry::new(display_name.clone()); - - let icon_path = registry_store_ref - .as_ref() - .and_then(|store| store.agent(agent_id)) - .and_then(|a| a.icon_path().cloned()); - - if let Some(icon_path) = icon_path { - entry = entry.custom_icon_svg(icon_path); - } else { - entry = entry.icon(IconName::Sparkle); - } - - entry = entry - .icon_color(Color::Muted) - .disabled(is_via_collab) - .handler({ - let workspace = workspace.clone(); - let agent_id = agent_id.clone(); - move |window, cx| { - let fs = ::global(cx); - let agent_id_string = - agent_id.to_string(); - settings::update_settings_file( - fs, - cx, - move |settings, _| { - let agent_servers = settings - .agent_servers - .get_or_insert_default(); - agent_servers.entry(agent_id_string).or_insert_with(|| { - settings::CustomAgentServerSettings::Registry { - default_mode: None, - default_model: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - } - }); - }, - ); - - if let Some(workspace) = workspace.upgrade() { - workspace.update(cx, |workspace, cx| { - if let Some(panel) = - workspace.panel::(cx) - { - panel.update(cx, |panel, cx| { - panel.new_agent_thread( - AgentType::Custom { - id: agent_id.clone(), - }, - window, - cx, - ); - }); - } - }); - } - } - }); - - menu = menu.item(entry); - } - - menu - }) .item( ContextMenuEntry::new("Add More Agents") .icon(IconName::Plus) .icon_color(Color::Muted) .handler({ move |window, cx| { - window.dispatch_action( - Box::new(zed_actions::AcpRegistry), - cx, - ) + window + .dispatch_action(Box::new(zed_actions::AcpRegistry), cx) } }), ) @@ -5348,6 +5185,7 @@ mod tests { use fs::FakeFs; use gpui::{TestAppContext, VisualTestContext}; use project::Project; + use project::agent_server_store::CODEX_ID; use serde_json::json; use workspace::MultiWorkspace; @@ -6273,35 +6111,7 @@ mod tests { } #[test] - fn test_deserialize_legacy_agent_type_variants() { - assert_eq!( - serde_json::from_str::(r#""ClaudeAgent""#).unwrap(), - AgentType::Custom { - id: CLAUDE_AGENT_ID.into(), - }, - ); - assert_eq!( - serde_json::from_str::(r#""ClaudeCode""#).unwrap(), - AgentType::Custom { - id: CLAUDE_AGENT_ID.into(), - }, - ); - assert_eq!( - serde_json::from_str::(r#""Codex""#).unwrap(), - AgentType::Custom { - id: CODEX_ID.into(), - }, - ); - assert_eq!( - serde_json::from_str::(r#""Gemini""#).unwrap(), - AgentType::Custom { - id: GEMINI_ID.into(), - }, - ); - } - - #[test] - fn test_deserialize_current_agent_type_variants() { + fn test_deserialize_agent_type_variants() { assert_eq!( serde_json::from_str::(r#""NativeAgent""#).unwrap(), AgentType::NativeAgent, @@ -6471,31 +6281,4 @@ mod tests { "the new worktree workspace should use the same agent (Codex) that was selected in the original panel", ); } - - #[test] - fn test_deserialize_legacy_serialized_panel() { - let json = serde_json::json!({ - "width": 300.0, - "selected_agent": "ClaudeAgent", - "last_active_thread": { - "session_id": "test-session", - "agent_type": "Codex", - }, - }); - - let panel: SerializedAgentPanel = serde_json::from_value(json).unwrap(); - assert_eq!( - panel.selected_agent, - Some(AgentType::Custom { - id: CLAUDE_AGENT_ID.into(), - }), - ); - let thread = panel.last_active_thread.unwrap(); - assert_eq!( - thread.agent_type, - AgentType::Custom { - id: CODEX_ID.into(), - }, - ); - } } diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index dde70f15e8084144d9beb1d4fb9563cf12fb942e..2488bc1687cc01af47e1939a5feff8b49cce64dd 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -218,7 +218,7 @@ pub struct NewNativeAgentThreadFromSummary { } // TODO unify this with AgentType -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum Agent { NativeAgent, @@ -228,65 +228,6 @@ pub enum Agent { }, } -// Custom impl handles legacy variant names from before the built-in agents were moved to -// the registry: "claude_code" -> Custom { name: "claude-acp" }, "codex" -> Custom { name: -// "codex-acp" }, "gemini" -> Custom { name: "gemini" }. -// Can be removed at some point in the future and go back to #[derive(Deserialize)]. -impl<'de> serde::Deserialize<'de> for Agent { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - use project::agent_server_store::{CLAUDE_AGENT_ID, CODEX_ID, GEMINI_ID}; - - let value = serde_json::Value::deserialize(deserializer)?; - - if let Some(s) = value.as_str() { - return match s { - "native_agent" => Ok(Self::NativeAgent), - "claude_code" | "claude_agent" => Ok(Self::Custom { - id: CLAUDE_AGENT_ID.into(), - }), - "codex" => Ok(Self::Custom { - id: CODEX_ID.into(), - }), - "gemini" => Ok(Self::Custom { - id: GEMINI_ID.into(), - }), - other => Err(serde::de::Error::unknown_variant( - other, - &[ - "native_agent", - "custom", - "claude_agent", - "claude_code", - "codex", - "gemini", - ], - )), - }; - } - - if let Some(obj) = value.as_object() { - if let Some(inner) = obj.get("custom") { - #[derive(serde::Deserialize)] - struct CustomFields { - name: SharedString, - } - let fields: CustomFields = - serde_json::from_value(inner.clone()).map_err(serde::de::Error::custom)?; - return Ok(Self::Custom { - id: AgentId::new(fields.name), - }); - } - } - - Err(serde::de::Error::custom( - "expected a string variant or {\"custom\": {\"name\": ...}}", - )) - } -} - impl Agent { pub fn server( &self, @@ -759,31 +700,7 @@ mod tests { } #[test] - fn test_deserialize_legacy_external_agent_variants() { - use project::agent_server_store::{CLAUDE_AGENT_ID, CODEX_ID, GEMINI_ID}; - - assert_eq!( - serde_json::from_str::(r#""claude_code""#).unwrap(), - Agent::Custom { - id: CLAUDE_AGENT_ID.into(), - }, - ); - assert_eq!( - serde_json::from_str::(r#""codex""#).unwrap(), - Agent::Custom { - id: CODEX_ID.into(), - }, - ); - assert_eq!( - serde_json::from_str::(r#""gemini""#).unwrap(), - Agent::Custom { - id: GEMINI_ID.into(), - }, - ); - } - - #[test] - fn test_deserialize_current_external_agent_variants() { + fn test_deserialize_external_agent_variants() { assert_eq!( serde_json::from_str::(r#""native_agent""#).unwrap(), Agent::NativeAgent,