From f8b840389b8c453f8a16d011b2e9587631570ca0 Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 22:59:43 +0000 Subject: [PATCH] agent_servers: Use correct default settings (#51136) (cherry-pick to preview) (#51137) Cherry-pick of #51136 to preview ---- These are edge cases, but there are a few ways you can get into a state where you are setting favorites for registry agents and we don't have the setting yet. This prioritizes `type: registry` for agents that we have in the registry, especially the previous built-ins. Release Notes: - N/A Co-authored-by: Ben Brandt --- crates/agent_servers/src/custom.rs | 334 +++++++++++++++------ crates/project/src/agent_registry_store.rs | 16 + 2 files changed, 266 insertions(+), 84 deletions(-) diff --git a/crates/agent_servers/src/custom.rs b/crates/agent_servers/src/custom.rs index b0669d1fb69e110f0ba206a3579f16738de5e7e2..0a1830717217872868e66a8222902c49eeaabf9c 100644 --- a/crates/agent_servers/src/custom.rs +++ b/crates/agent_servers/src/custom.rs @@ -84,19 +84,12 @@ impl AgentServer for CustomAgentServer { let config_id = config_id.to_string(); let value_id = value_id.to_string(); - update_settings_file(fs, cx, move |settings, _| { + update_settings_file(fs, cx, move |settings, cx| { let settings = settings .agent_servers .get_or_insert_default() .entry(name.to_string()) - .or_insert_with(|| settings::CustomAgentServerSettings::Extension { - default_model: None, - default_mode: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }); + .or_insert_with(|| default_settings_for_agent(&name, cx)); match settings { settings::CustomAgentServerSettings::Custom { @@ -132,19 +125,12 @@ impl AgentServer for CustomAgentServer { fn set_default_mode(&self, mode_id: Option, fs: Arc, cx: &mut App) { let name = self.name(); - update_settings_file(fs, cx, move |settings, _| { + update_settings_file(fs, cx, move |settings, cx| { let settings = settings .agent_servers .get_or_insert_default() .entry(name.to_string()) - .or_insert_with(|| settings::CustomAgentServerSettings::Extension { - default_model: None, - default_mode: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }); + .or_insert_with(|| default_settings_for_agent(&name, cx)); match settings { settings::CustomAgentServerSettings::Custom { default_mode, .. } @@ -171,19 +157,12 @@ impl AgentServer for CustomAgentServer { fn set_default_model(&self, model_id: Option, fs: Arc, cx: &mut App) { let name = self.name(); - update_settings_file(fs, cx, move |settings, _| { + update_settings_file(fs, cx, move |settings, cx| { let settings = settings .agent_servers .get_or_insert_default() .entry(name.to_string()) - .or_insert_with(|| settings::CustomAgentServerSettings::Extension { - default_model: None, - default_mode: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }); + .or_insert_with(|| default_settings_for_agent(&name, cx)); match settings { settings::CustomAgentServerSettings::Custom { default_model, .. } @@ -222,19 +201,12 @@ impl AgentServer for CustomAgentServer { cx: &App, ) { let name = self.name(); - update_settings_file(fs, cx, move |settings, _| { + update_settings_file(fs, cx, move |settings, cx| { let settings = settings .agent_servers .get_or_insert_default() .entry(name.to_string()) - .or_insert_with(|| settings::CustomAgentServerSettings::Extension { - default_model: None, - default_mode: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }); + .or_insert_with(|| default_settings_for_agent(&name, cx)); let favorite_models = match settings { settings::CustomAgentServerSettings::Custom { @@ -282,19 +254,12 @@ impl AgentServer for CustomAgentServer { let name = self.name(); let config_id = config_id.to_string(); let value_id = value_id.map(|s| s.to_string()); - update_settings_file(fs, cx, move |settings, _| { + update_settings_file(fs, cx, move |settings, cx| { let settings = settings .agent_servers .get_or_insert_default() .entry(name.to_string()) - .or_insert_with(|| settings::CustomAgentServerSettings::Extension { - default_model: None, - default_mode: None, - env: Default::default(), - favorite_models: Vec::new(), - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }); + .or_insert_with(|| default_settings_for_agent(&name, cx)); match settings { settings::CustomAgentServerSettings::Custom { @@ -332,45 +297,27 @@ impl AgentServer for CustomAgentServer { .unwrap_or_else(|| name.clone()); let default_mode = self.default_mode(cx); let default_model = self.default_model(cx); - let is_previous_built_in = - matches!(name.as_ref(), CLAUDE_AGENT_NAME | CODEX_NAME | GEMINI_NAME); - let (default_config_options, is_registry_agent) = - cx.read_global(|settings: &SettingsStore, _| { - let agent_settings = settings - .get::(None) - .get(self.name().as_ref()); - - let is_registry = agent_settings - .map(|s| { - matches!( - s, - project::agent_server_store::CustomAgentServerSettings::Registry { .. } - ) - }) - .unwrap_or(false); - - let config_options = agent_settings - .map(|s| match s { - project::agent_server_store::CustomAgentServerSettings::Custom { - default_config_options, - .. - } - | project::agent_server_store::CustomAgentServerSettings::Extension { - default_config_options, - .. - } - | project::agent_server_store::CustomAgentServerSettings::Registry { - default_config_options, - .. - } => default_config_options.clone(), - }) - .unwrap_or_default(); - - (config_options, is_registry) - }); - - // Intermediate step to allow for previous built-ins to also be triggered if they aren't in settings yet. - let is_registry_agent = is_registry_agent || is_previous_built_in; + let is_registry_agent = is_registry_agent(&name, cx); + let default_config_options = cx.read_global(|settings: &SettingsStore, _| { + settings + .get::(None) + .get(self.name().as_ref()) + .map(|s| match s { + project::agent_server_store::CustomAgentServerSettings::Custom { + default_config_options, + .. + } + | project::agent_server_store::CustomAgentServerSettings::Extension { + default_config_options, + .. + } + | project::agent_server_store::CustomAgentServerSettings::Registry { + default_config_options, + .. + } => default_config_options.clone(), + }) + .unwrap_or_default() + }); if is_registry_agent { if let Some(registry_store) = project::AgentRegistryStore::try_global(cx) { @@ -458,3 +405,222 @@ fn api_key_for_gemini_cli(cx: &mut App) -> Task> { ) }) } + +fn is_registry_agent(name: &str, cx: &App) -> bool { + let is_previous_built_in = matches!(name, CLAUDE_AGENT_NAME | CODEX_NAME | GEMINI_NAME); + let is_in_registry = project::AgentRegistryStore::try_global(cx) + .map(|store| store.read(cx).agent(name).is_some()) + .unwrap_or(false); + let is_settings_registry = cx.read_global(|settings: &SettingsStore, _| { + settings + .get::(None) + .get(name) + .is_some_and(|s| { + matches!( + s, + project::agent_server_store::CustomAgentServerSettings::Registry { .. } + ) + }) + }); + is_previous_built_in || is_in_registry || is_settings_registry +} + +fn default_settings_for_agent(name: &str, cx: &App) -> settings::CustomAgentServerSettings { + if is_registry_agent(name, cx) { + settings::CustomAgentServerSettings::Registry { + default_model: None, + default_mode: None, + env: Default::default(), + favorite_models: Vec::new(), + default_config_options: Default::default(), + favorite_config_option_values: Default::default(), + } + } else { + settings::CustomAgentServerSettings::Extension { + default_model: None, + default_mode: None, + env: Default::default(), + favorite_models: Vec::new(), + default_config_options: Default::default(), + favorite_config_option_values: Default::default(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use collections::HashMap; + use gpui::TestAppContext; + use project::agent_registry_store::{ + AgentRegistryStore, RegistryAgent, RegistryAgentMetadata, RegistryNpxAgent, + }; + use settings::Settings as _; + + fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + }); + } + + fn init_registry_with_agents(cx: &mut TestAppContext, agent_ids: &[&str]) { + let agents: Vec = agent_ids + .iter() + .map(|id| { + let id = SharedString::from(id.to_string()); + RegistryAgent::Npx(RegistryNpxAgent { + metadata: RegistryAgentMetadata { + id: id.clone(), + name: id.clone(), + description: SharedString::from(""), + version: SharedString::from("1.0.0"), + repository: None, + icon_path: None, + }, + package: id, + args: Vec::new(), + env: HashMap::default(), + }) + }) + .collect(); + cx.update(|cx| { + AgentRegistryStore::init_test_global(cx, agents); + }); + } + + fn set_agent_server_settings( + cx: &mut TestAppContext, + entries: Vec<(&str, settings::CustomAgentServerSettings)>, + ) { + cx.update(|cx| { + AllAgentServersSettings::override_global( + project::agent_server_store::AllAgentServersSettings( + entries + .into_iter() + .map(|(name, settings)| (name.to_string(), settings.into())) + .collect(), + ), + cx, + ); + }); + } + + #[gpui::test] + fn test_previous_builtins_are_registry(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + assert!(is_registry_agent(CLAUDE_AGENT_NAME, cx)); + assert!(is_registry_agent(CODEX_NAME, cx)); + assert!(is_registry_agent(GEMINI_NAME, cx)); + }); + } + + #[gpui::test] + fn test_unknown_agent_is_not_registry(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + assert!(!is_registry_agent("my-custom-agent", cx)); + }); + } + + #[gpui::test] + fn test_agent_in_registry_store_is_registry(cx: &mut TestAppContext) { + init_test(cx); + init_registry_with_agents(cx, &["some-new-registry-agent"]); + cx.update(|cx| { + assert!(is_registry_agent("some-new-registry-agent", cx)); + assert!(!is_registry_agent("not-in-registry", cx)); + }); + } + + #[gpui::test] + fn test_agent_with_registry_settings_type_is_registry(cx: &mut TestAppContext) { + init_test(cx); + set_agent_server_settings( + cx, + vec![( + "agent-from-settings", + settings::CustomAgentServerSettings::Registry { + env: HashMap::default(), + default_mode: None, + default_model: None, + favorite_models: Vec::new(), + default_config_options: HashMap::default(), + favorite_config_option_values: HashMap::default(), + }, + )], + ); + cx.update(|cx| { + assert!(is_registry_agent("agent-from-settings", cx)); + }); + } + + #[gpui::test] + fn test_agent_with_extension_settings_type_is_not_registry(cx: &mut TestAppContext) { + init_test(cx); + set_agent_server_settings( + cx, + vec![( + "my-extension-agent", + settings::CustomAgentServerSettings::Extension { + env: HashMap::default(), + default_mode: None, + default_model: None, + favorite_models: Vec::new(), + default_config_options: HashMap::default(), + favorite_config_option_values: HashMap::default(), + }, + )], + ); + cx.update(|cx| { + assert!(!is_registry_agent("my-extension-agent", cx)); + }); + } + + #[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_NAME, cx), + settings::CustomAgentServerSettings::Registry { .. } + )); + assert!(matches!( + default_settings_for_agent(CLAUDE_AGENT_NAME, cx), + settings::CustomAgentServerSettings::Registry { .. } + )); + assert!(matches!( + default_settings_for_agent(GEMINI_NAME, cx), + settings::CustomAgentServerSettings::Registry { .. } + )); + }); + } + + #[gpui::test] + fn test_default_settings_for_extension_agent(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + assert!(matches!( + default_settings_for_agent("some-extension-agent", cx), + settings::CustomAgentServerSettings::Extension { .. } + )); + }); + } + + #[gpui::test] + fn test_default_settings_for_agent_in_registry(cx: &mut TestAppContext) { + init_test(cx); + init_registry_with_agents(cx, &["new-registry-agent"]); + cx.update(|cx| { + assert!(matches!( + default_settings_for_agent("new-registry-agent", cx), + settings::CustomAgentServerSettings::Registry { .. } + )); + assert!(matches!( + default_settings_for_agent("not-in-registry", cx), + settings::CustomAgentServerSettings::Extension { .. } + )); + }); + } +} diff --git a/crates/project/src/agent_registry_store.rs b/crates/project/src/agent_registry_store.rs index 155badc4ac7da22921b121428cc34a0d46f5b982..79d6e52097d17cadc0271cb09de4ab283c6d93b8 100644 --- a/crates/project/src/agent_registry_store.rs +++ b/crates/project/src/agent_registry_store.rs @@ -147,6 +147,22 @@ impl AgentRegistryStore { .map(|store| store.0.clone()) } + #[cfg(any(test, feature = "test-support"))] + pub fn init_test_global(cx: &mut App, agents: Vec) -> Entity { + let fs: Arc = fs::FakeFs::new(cx.background_executor().clone()); + let store = cx.new(|_cx| Self { + fs, + http_client: http_client::FakeHttpClient::with_404_response(), + agents, + is_fetching: false, + fetch_error: None, + pending_refresh: None, + last_refresh: None, + }); + cx.set_global(GlobalAgentRegistryStore(store.clone())); + store + } + pub fn agents(&self) -> &[RegistryAgent] { &self.agents }