refactor agent server settings to store commands uniformly

Co-authored-by: Nia Espera <nia-e@haecceity.cc>

crates/agent_servers/src/agent_servers.rs

@@ -70,7 +70,7 @@ impl AgentServerDelegate {
         binary_name: SharedString,
         package_name: SharedString,
         entrypoint_path: PathBuf,
-        ignore_system_version: bool,
+        search_path: bool,
         minimum_version: Option<Version>,
         cx: &mut App,
     ) -> Task<Result<AgentServerCommand>> {
@@ -85,7 +85,7 @@ impl AgentServerDelegate {
         let new_version_available = self.new_version_available;
 
         cx.spawn(async move |cx| {
-            if !ignore_system_version {
+            if search_path {
                 if let Some(bin) = find_bin_in_path(binary_name.clone(), &project, cx).await {
                     return Ok(AgentServerCommand {
                         path: bin,

crates/agent_servers/src/claude.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use std::rc::Rc;
 use std::{any::Any, path::PathBuf};
 
-use anyhow::Result;
+use anyhow::{Result, bail};
 use gpui::{App, AppContext as _, SharedString, Task};
 
 use crate::{AgentServer, AgentServerDelegate, AllAgentServersSettings};
@@ -25,20 +25,23 @@ impl ClaudeCode {
         delegate: AgentServerDelegate,
         cx: &mut App,
     ) -> Task<Result<ClaudeCodeLoginCommand>> {
-        let settings = cx.read_global(|settings: &SettingsStore, _| {
-            settings.get::<AllAgentServersSettings>(None).claude.clone()
+        let custom_command = cx.read_global(|settings: &SettingsStore, _| {
+            settings
+                .get::<AllAgentServersSettings>(None)
+                .get("claude")
+                .cloned()
         });
 
         cx.spawn(async move |cx| {
-            let mut command = if let Some(settings) = settings {
-                settings.command
+            let mut command = if custom_command.is_some() {
+                bail!("Cannot construct login command because a custom command was specified for claude-code-acp in settings")
             } else {
                 cx.update(|cx| {
                     delegate.get_or_npm_install_builtin_agent(
                         Self::BINARY_NAME.into(),
                         Self::PACKAGE_NAME.into(),
                         "node_modules/@anthropic-ai/claude-code/cli.js".into(),
-                        true,
+                        false,
                         Some("0.2.5".parse().unwrap()),
                         cx,
                     )
@@ -77,20 +80,23 @@ impl AgentServer for ClaudeCode {
         let root_dir = root_dir.to_path_buf();
         let fs = delegate.project().read(cx).fs().clone();
         let server_name = self.name();
-        let settings = cx.read_global(|settings: &SettingsStore, _| {
-            settings.get::<AllAgentServersSettings>(None).claude.clone()
+        let custom_command = cx.read_global(|settings: &SettingsStore, _| {
+            settings
+                .get::<AllAgentServersSettings>(None)
+                .get("claude")
+                .cloned()
         });
 
         cx.spawn(async move |cx| {
-            let mut command = if let Some(settings) = settings {
-                settings.command
+            let mut command = if let Some(custom_command) = custom_command {
+                custom_command
             } else {
                 cx.update(|cx| {
                     delegate.get_or_npm_install_builtin_agent(
                         Self::BINARY_NAME.into(),
                         Self::PACKAGE_NAME.into(),
                         format!("node_modules/{}/dist/index.js", Self::PACKAGE_NAME).into(),
-                        true,
+                        false,
                         None,
                         cx,
                     )

crates/agent_servers/src/e2e_tests.rs

@@ -1,6 +1,6 @@
-use crate::{AgentServer, AgentServerDelegate};
 #[cfg(test)]
-use crate::{AgentServerCommand, CustomAgentServerSettings};
+use crate::AgentServerCommand;
+use crate::{AgentServer, AgentServerDelegate};
 use acp_thread::{AcpThread, AgentThreadEntry, ToolCall, ToolCallStatus};
 use agent_client_protocol as acp;
 use futures::{FutureExt, StreamExt, channel::mpsc, select};
@@ -473,15 +473,20 @@ pub async fn init_test(cx: &mut TestAppContext) -> Arc<FakeFs> {
         #[cfg(test)]
         crate::AllAgentServersSettings::override_global(
             crate::AllAgentServersSettings {
-                claude: Some(CustomAgentServerSettings {
-                    command: AgentServerCommand {
-                        path: "claude-code-acp".into(),
-                        args: vec![],
-                        env: None,
-                    },
-                }),
-                gemini: Some(crate::gemini::tests::local_command().into()),
-                custom: collections::HashMap::default(),
+                commands: [
+                    (
+                        "claude".into(),
+                        AgentServerCommand {
+                            path: "claude-code-acp".into(),
+                            args: vec![],
+                            env: None,
+                        },
+                    ),
+                    ("gemini".into(), crate::gemini::tests::local_command()),
+                ]
+                .into_iter()
+                .collect(),
+                gemini_is_system: false,
             },
             cx,
         );

crates/agent_servers/src/gemini.rs

@@ -2,15 +2,13 @@ use std::rc::Rc;
 use std::{any::Any, path::Path};
 
 use crate::acp::AcpConnection;
-use crate::{AgentServer, AgentServerDelegate};
+use crate::{AgentServer, AgentServerDelegate, AllAgentServersSettings};
 use acp_thread::{AgentConnection, LoadError};
 use anyhow::Result;
 use gpui::{App, AppContext as _, SharedString, Task};
 use language_models::provider::google::GoogleLanguageModelProvider;
 use settings::SettingsStore;
 
-use crate::AllAgentServersSettings;
-
 #[derive(Clone)]
 pub struct Gemini;
 
@@ -38,26 +36,25 @@ impl AgentServer for Gemini {
         let root_dir = root_dir.to_path_buf();
         let fs = delegate.project().read(cx).fs().clone();
         let server_name = self.name();
-        let settings = cx.read_global(|settings: &SettingsStore, _| {
-            settings.get::<AllAgentServersSettings>(None).gemini.clone()
+        let (custom_command, is_system) = cx.read_global(|settings: &SettingsStore, _| {
+            let s = settings.get::<AllAgentServersSettings>(None);
+            (
+                s.get("gemini").cloned(),
+                AllAgentServersSettings::is_system(s, "gemini"),
+            )
         });
 
         cx.spawn(async move |cx| {
-            let ignore_system_version = settings
-                .as_ref()
-                .and_then(|settings| settings.ignore_system_version)
-                .unwrap_or(true);
-            let mut command = if let Some(settings) = settings
-                && let Some(command) = settings.custom_command()
+            let mut command = if let Some(custom_command) = custom_command
             {
-                command
+                custom_command
             } else {
                 cx.update(|cx| {
                     delegate.get_or_npm_install_builtin_agent(
                         Self::BINARY_NAME.into(),
                         Self::PACKAGE_NAME.into(),
                         format!("node_modules/{}/dist/index.js", Self::PACKAGE_NAME).into(),
-                        ignore_system_version,
+                        is_system,
                         Some(Self::MINIMUM_VERSION.parse().unwrap()),
                         cx,
                     )

crates/agent_servers/src/settings.rs

@@ -14,13 +14,48 @@ pub fn init(cx: &mut App) {
 
 #[derive(Default, Deserialize, Serialize, Clone, JsonSchema, Debug, SettingsUi, SettingsKey)]
 #[settings_key(key = "agent_servers")]
-pub struct AllAgentServersSettings {
-    pub gemini: Option<BuiltinAgentServerSettings>,
-    pub claude: Option<CustomAgentServerSettings>,
-
+pub struct AllAgentServersSettingsContent {
+    gemini: Option<GeminiSettingsContent>,
+    claude: Option<AgentServerCommand>,
     /// Custom agent servers configured by the user
     #[serde(flatten)]
-    pub custom: HashMap<SharedString, CustomAgentServerSettings>,
+    pub custom: HashMap<SharedString, AgentServerCommand>,
+}
+
+#[derive(Clone, Debug, Default)]
+pub struct AllAgentServersSettings {
+    pub commands: HashMap<SharedString, AgentServerCommand>,
+    pub gemini_is_system: bool,
+}
+
+impl AllAgentServersSettings {
+    pub fn is_system(this: &Self, name: &str) -> bool {
+        if name == "gemini" {
+            this.gemini_is_system
+        } else {
+            false
+        }
+    }
+}
+
+impl std::ops::Deref for AllAgentServersSettings {
+    type Target = HashMap<SharedString, AgentServerCommand>;
+    fn deref(&self) -> &Self::Target {
+        &self.commands
+    }
+}
+
+impl std::ops::DerefMut for AllAgentServersSettings {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.commands
+    }
+}
+
+#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, PartialEq)]
+pub struct GeminiSettingsContent {
+    ignore_system_version: Option<bool>,
+    #[serde(flatten)]
+    inner: Option<AgentServerCommand>,
 }
 
 #[derive(Default, Deserialize, Serialize, Clone, JsonSchema, Debug, PartialEq)]
@@ -70,35 +105,40 @@ impl From<AgentServerCommand> for BuiltinAgentServerSettings {
 }
 
 #[derive(Deserialize, Serialize, Clone, JsonSchema, Debug, PartialEq)]
-pub struct CustomAgentServerSettings {
+pub struct AgentServerSettings {
     #[serde(flatten)]
     pub command: AgentServerCommand,
 }
 
 impl settings::Settings for AllAgentServersSettings {
-    type FileContent = Self;
+    type FileContent = AllAgentServersSettingsContent;
 
     fn load(sources: SettingsSources<Self::FileContent>, _: &mut App) -> Result<Self> {
         let mut settings = AllAgentServersSettings::default();
 
-        for AllAgentServersSettings {
+        for AllAgentServersSettingsContent {
             gemini,
             claude,
             custom,
         } in sources.defaults_and_customizations()
         {
-            if gemini.is_some() {
-                settings.gemini = gemini.clone();
+            if let Some(gemini) = gemini {
+                if let Some(ignore) = gemini.ignore_system_version {
+                    settings.gemini_is_system = !ignore;
+                }
+                if let Some(gemini) = gemini.inner.as_ref() {
+                    settings.insert("gemini".into(), gemini.clone());
+                }
             }
-            if claude.is_some() {
-                settings.claude = claude.clone();
+            if let Some(claude) = claude.clone() {
+                settings.insert("claude".into(), claude);
             }
 
             // Merge custom agents
-            for (name, config) in custom {
+            for (name, command) in custom {
                 // Skip built-in agent names to avoid conflicts
                 if name != "gemini" && name != "claude" {
-                    settings.custom.insert(name.clone(), config.clone());
+                    settings.commands.insert(name.clone(), command.clone());
                 }
             }
         }
@@ -108,3 +148,31 @@ impl settings::Settings for AllAgentServersSettings {
 
     fn import_from_vscode(_vscode: &settings::VsCodeSettings, _current: &mut Self::FileContent) {}
 }
+
+#[cfg(test)]
+mod tests {
+    use serde_json::json;
+
+    use crate::{AgentServerCommand, GeminiSettingsContent};
+
+    #[test]
+    fn test_deserialization() {
+        let value = json!({
+            "command": "foo",
+            "args": ["bar"],
+            "ignore_system_version": false
+        });
+        let settings = serde_json::from_value::<GeminiSettingsContent>(value).unwrap();
+        assert_eq!(
+            settings,
+            GeminiSettingsContent {
+                ignore_system_version: Some(false),
+                inner: Some(AgentServerCommand {
+                    path: "foo".into(),
+                    args: vec!["bar".into()],
+                    env: Default::default(),
+                })
+            }
+        )
+    }
+}

crates/agent_ui/src/agent_configuration.rs

@@ -5,7 +5,7 @@ mod tool_picker;
 
 use std::{ops::Range, sync::Arc};
 
-use agent_servers::{AgentServerCommand, AllAgentServersSettings, CustomAgentServerSettings};
+use agent_servers::{AgentServerCommand, AllAgentServersSettings};
 use agent_settings::AgentSettings;
 use anyhow::Result;
 use assistant_tool::{ToolSource, ToolWorkingSet};
@@ -20,6 +20,7 @@ use gpui::{
     Action, AnyView, App, AsyncWindowContext, Corner, Entity, EventEmitter, FocusHandle, Focusable,
     Hsla, ScrollHandle, Subscription, Task, WeakEntity,
 };
+use itertools::Itertools as _;
 use language::LanguageRegistry;
 use language_model::{
     LanguageModelProvider, LanguageModelProviderId, LanguageModelRegistry, ZED_CLOUD_PROVIDER_ID,
@@ -993,15 +994,16 @@ impl AgentConfiguration {
     fn render_agent_servers_section(&mut self, cx: &mut Context<Self>) -> impl IntoElement {
         let settings = AllAgentServersSettings::get_global(cx).clone();
         let user_defined_agents = settings
-            .custom
             .iter()
-            .map(|(name, settings)| {
+            .filter(|(name, _)| *name != "gemini" && *name != "claude")
+            .sorted_by(|(l, _), (r, _)| l.cmp(r))
+            .map(|(name, command)| {
                 self.render_agent_server(
                     IconName::Ai,
                     name.clone(),
                     ExternalAgent::Custom {
                         name: name.clone(),
-                        command: settings.command.clone(),
+                        command: command.clone(),
                     },
                     cx,
                 )
@@ -1280,6 +1282,7 @@ async fn open_new_agent_servers_entry_in_settings_editor(
             let settings = cx.global::<SettingsStore>();
 
             let mut unique_server_name = None;
+            // FIXME test that this still works
             let edits = settings.edits_for_update::<AllAgentServersSettings>(&text, |file| {
                 let server_name: Option<SharedString> = (0..u8::MAX)
                     .map(|i| {
@@ -1294,12 +1297,10 @@ async fn open_new_agent_servers_entry_in_settings_editor(
                     unique_server_name = Some(server_name.clone());
                     file.custom.insert(
                         server_name,
-                        CustomAgentServerSettings {
-                            command: AgentServerCommand {
-                                path: "path_to_executable".into(),
-                                args: vec![],
-                                env: Some(HashMap::default()),
-                            },
+                        AgentServerCommand {
+                            path: "path_to_executable".into(),
+                            args: vec![],
+                            env: Some(HashMap::default()),
                         },
                     );
                 }

crates/agent_ui/src/agent_panel.rs

@@ -8,6 +8,7 @@ use acp_thread::AcpThread;
 use agent_servers::AgentServerCommand;
 use agent2::{DbThreadMetadata, HistoryEntry};
 use db::kvp::{Dismissable, KEY_VALUE_STORE};
+use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use zed_actions::OpenBrowser;
 use zed_actions::agent::{OpenClaudeCodeOnboardingModal, ReauthenticateAgent};
@@ -2683,7 +2684,13 @@ impl AgentPanel {
                                 // Add custom agents from settings
                                 let settings =
                                     agent_servers::AllAgentServersSettings::get_global(cx);
-                                for (agent_name, agent_settings) in &settings.custom {
+                                for (agent_name, command) in
+                                    settings.iter().sorted_by(|(l, _), (r, _)| l.cmp(r))
+                                {
+                                    if agent_name == "gemini" || agent_name == "claude" {
+                                        continue;
+                                    }
+
                                     menu = menu.item(
                                         ContextMenuEntry::new(format!("New {} Thread", agent_name))
                                             .icon(IconName::Terminal)
@@ -2692,7 +2699,7 @@ impl AgentPanel {
                                             .handler({
                                                 let workspace = workspace.clone();
                                                 let agent_name = agent_name.clone();
-                                                let agent_settings = agent_settings.clone();
+                                                let command = command.clone();
                                                 move |window, cx| {
                                                     if let Some(workspace) = workspace.upgrade() {
                                                         workspace.update(cx, |workspace, cx| {
@@ -2704,8 +2711,7 @@ impl AgentPanel {
                                                                         AgentType::Custom {
                                                                             name: agent_name
                                                                                 .clone(),
-                                                                            command: agent_settings
-                                                                                .command
+                                                                            command: command
                                                                                 .clone(),
                                                                         },
                                                                         window,