From f4d838bb148e6a878b496e807302fe4e2f51e82e Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 4 Sep 2025 18:12:27 -0400 Subject: [PATCH] refactor agent server settings to store commands uniformly Co-authored-by: Nia Espera --- crates/agent_servers/src/agent_servers.rs | 4 +- crates/agent_servers/src/claude.rs | 28 ++++--- crates/agent_servers/src/e2e_tests.rs | 27 ++++--- crates/agent_servers/src/gemini.rs | 23 +++--- crates/agent_servers/src/settings.rs | 94 +++++++++++++++++++--- crates/agent_ui/src/agent_configuration.rs | 21 ++--- crates/agent_ui/src/agent_panel.rs | 14 +++- 7 files changed, 147 insertions(+), 64 deletions(-) diff --git a/crates/agent_servers/src/agent_servers.rs b/crates/agent_servers/src/agent_servers.rs index e214dabfc763c2a46f1f4665c3d1f881d5ce406e..269d9889406f8211c3f8830b0d1d4af48188ba82 100644 --- a/crates/agent_servers/src/agent_servers.rs +++ b/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, cx: &mut App, ) -> Task> { @@ -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, diff --git a/crates/agent_servers/src/claude.rs b/crates/agent_servers/src/claude.rs index 48d3e33775d98dfe89801813c6926ff40f48ed87..0699e3e89ef0838939239ed03cf8046e1df63654 100644 --- a/crates/agent_servers/src/claude.rs +++ b/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> { - let settings = cx.read_global(|settings: &SettingsStore, _| { - settings.get::(None).claude.clone() + let custom_command = cx.read_global(|settings: &SettingsStore, _| { + settings + .get::(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::(None).claude.clone() + let custom_command = cx.read_global(|settings: &SettingsStore, _| { + settings + .get::(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, ) diff --git a/crates/agent_servers/src/e2e_tests.rs b/crates/agent_servers/src/e2e_tests.rs index f801ef246807f93c4bbdc26a1ff3bd478cc476d0..164047d2d10f872b8f8910218549b60a957401dd 100644 --- a/crates/agent_servers/src/e2e_tests.rs +++ b/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 { #[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, ); diff --git a/crates/agent_servers/src/gemini.rs b/crates/agent_servers/src/gemini.rs index b58ad703cda496c4413f30decbfa5e0b1d1b0735..05c40781225b0233ed4e367efc0f3f0d77e99703 100644 --- a/crates/agent_servers/src/gemini.rs +++ b/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::(None).gemini.clone() + let (custom_command, is_system) = cx.read_global(|settings: &SettingsStore, _| { + let s = settings.get::(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, ) diff --git a/crates/agent_servers/src/settings.rs b/crates/agent_servers/src/settings.rs index 167753296a1a489128ba916f114f4895c15afcf9..9dc1043d630d778814f690be5740e7507e27b5ea 100644 --- a/crates/agent_servers/src/settings.rs +++ b/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 AllAgentServersSettingsContent { + gemini: Option, + claude: Option, + /// Custom agent servers configured by the user + #[serde(flatten)] + pub custom: HashMap, +} + +#[derive(Clone, Debug, Default)] pub struct AllAgentServersSettings { - pub gemini: Option, - pub claude: Option, + pub commands: HashMap, + pub gemini_is_system: bool, +} - /// Custom agent servers configured by the user +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; + 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, #[serde(flatten)] - pub custom: HashMap, + inner: Option, } #[derive(Default, Deserialize, Serialize, Clone, JsonSchema, Debug, PartialEq)] @@ -70,35 +105,40 @@ impl From 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, _: &mut App) -> Result { 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::(value).unwrap(); + assert_eq!( + settings, + GeminiSettingsContent { + ignore_system_version: Some(false), + inner: Some(AgentServerCommand { + path: "foo".into(), + args: vec!["bar".into()], + env: Default::default(), + }) + } + ) + } +} diff --git a/crates/agent_ui/src/agent_configuration.rs b/crates/agent_ui/src/agent_configuration.rs index 5981a3c52bf52ff4549b2f73a6322e308725750d..f9da45451f447fd5d3072e6759af363c4f571565 100644 --- a/crates/agent_ui/src/agent_configuration.rs +++ b/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) -> 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::(); let mut unique_server_name = None; + // FIXME test that this still works let edits = settings.edits_for_update::(&text, |file| { let server_name: Option = (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()), }, ); } diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index d021eaefb5ebff43fad1fe4822b3758550a0179f..8676bcff1aa9ee6b27347ceee742725cd062c35d 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/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,