From 94a03de2b85a7e620cc93aad6969d3b7130b2d3f Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 1 Jun 2026 01:04:28 +0800 Subject: [PATCH] fix(settings): dedupe auto named providers --- crates/op-editor-core/src/agent_settings.rs | 43 ++++++++++++++++--- .../src/tests_agent_settings_draft.rs | 14 ++++++ crates/op-host-desktop/src/settings_io.rs | 40 ++++++++++++++++- 3 files changed, 91 insertions(+), 6 deletions(-) diff --git a/crates/op-editor-core/src/agent_settings.rs b/crates/op-editor-core/src/agent_settings.rs index d179dd2d4..1e3369d83 100644 --- a/crates/op-editor-core/src/agent_settings.rs +++ b/crates/op-editor-core/src/agent_settings.rs @@ -217,6 +217,35 @@ impl BuiltinAgentConfig { && self.base_url.trim().trim_end_matches('/') == base_url.trim().trim_end_matches('/') } + pub fn matches_add_candidate( + &self, + display_name: &str, + api_key: &str, + model: &str, + kind: BuiltinAgentKind, + base_url: &str, + ) -> bool { + if !self.matches_backend(api_key, model, kind, base_url) { + return false; + } + self.display_name.trim() == display_name.trim() + || is_auto_builtin_agent_name(&self.display_name) + || is_auto_builtin_agent_name(display_name) + } + + fn matches_backend( + &self, + api_key: &str, + model: &str, + kind: BuiltinAgentKind, + base_url: &str, + ) -> bool { + self.kind == kind + && self.api_key.trim() == api_key.trim() + && self.model.trim() == model.trim() + && self.base_url.trim().trim_end_matches('/') == base_url.trim().trim_end_matches('/') + } + pub fn apply_preset(&mut self, key: BuiltinAgentPresetKey) { let preset = builtin_agent_preset(key); self.preset = preset.key; @@ -252,6 +281,12 @@ impl BuiltinAgentConfig { } } +fn is_auto_builtin_agent_name(name: &str) -> bool { + name.trim() + .strip_prefix("Built-in Agent ") + .is_some_and(|suffix| suffix.parse::().is_ok()) +} + /// ACP-compatible agent connection style mirrored from the TS /// `AcpAgentConfig.connectionType` union. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -556,11 +591,9 @@ impl AgentSettings { let api_key = api_key.into(); let model = model.into(); let base_url = base_url.into(); - if let Some(existing) = self - .builtin_agents - .iter() - .find(|agent| agent.matches_config(&display_name, &api_key, &model, kind, &base_url)) - { + if let Some(existing) = self.builtin_agents.iter().find(|agent| { + agent.matches_add_candidate(&display_name, &api_key, &model, kind, &base_url) + }) { return existing.id.clone(); } let id = format!("builtin-{}", self.next_builtin_agent_id.max(1)); diff --git a/crates/op-editor-core/src/tests_agent_settings_draft.rs b/crates/op-editor-core/src/tests_agent_settings_draft.rs index 764a6a819..a9bbf54aa 100644 --- a/crates/op-editor-core/src/tests_agent_settings_draft.rs +++ b/crates/op-editor-core/src/tests_agent_settings_draft.rs @@ -13,6 +13,20 @@ fn duplicate_builtin_agent_config_reuses_existing_provider() { assert_eq!(s.next_builtin_agent_id, 2); } +#[test] +fn duplicate_auto_named_builtin_agent_config_reuses_existing_provider() { + let mut s = AgentSettings::default(); + + let first = + s.add_builtin_agent_with_defaults("Built-in Agent 5", "sk-test", "claude-sonnet-4-5"); + let second = + s.add_builtin_agent_with_defaults("Built-in Agent 6", "sk-test", "claude-sonnet-4-5"); + + assert_eq!(second, first); + assert_eq!(s.builtin_agents.len(), 1); + assert_eq!(s.builtin_agents[0].display_name, "Built-in Agent 5"); +} + #[test] fn builtin_agent_draft_does_not_persist_until_save() { let mut s = AgentSettings::default(); diff --git a/crates/op-host-desktop/src/settings_io.rs b/crates/op-host-desktop/src/settings_io.rs index 73793e210..759958148 100644 --- a/crates/op-host-desktop/src/settings_io.rs +++ b/crates/op-host-desktop/src/settings_io.rs @@ -448,7 +448,7 @@ fn dedupe_builtin_agents(agents: Vec) -> Vec = Vec::new(); for agent in agents { let is_duplicate = deduped.iter().any(|existing| { - existing.matches_config( + existing.matches_add_candidate( &agent.display_name, &agent.api_key, &agent.model, @@ -735,6 +735,44 @@ mod tests { assert_eq!(dst.editor_ui.agent_settings.next_builtin_agent_id, 2); } + #[test] + fn duplicate_auto_named_builtin_agents_are_deduped_on_load() { + let settings = r#"{ + "version": 1, + "builtin_agents": [ + { + "id": "builtin-5", + "display_name": "Built-in Agent 5", + "kind": "anthropic", + "api_key": "sk-test", + "model": "claude-sonnet-4-5", + "base_url": "https://api.anthropic.com", + "enabled": true + }, + { + "id": "builtin-6", + "display_name": "Built-in Agent 6", + "kind": "anthropic", + "api_key": "sk-test", + "model": "claude-sonnet-4-5", + "base_url": "https://api.anthropic.com", + "enabled": true + } + ] + }"#; + let payload: SettingsPayload = serde_json::from_str(settings).unwrap(); + let mut dst = EditorState::new(); + + apply_payload(&mut dst, payload); + + assert_eq!(dst.editor_ui.agent_settings.builtin_agents.len(), 1); + assert_eq!( + dst.editor_ui.agent_settings.builtin_agents[0].display_name, + "Built-in Agent 5" + ); + assert_eq!(dst.editor_ui.agent_settings.next_builtin_agent_id, 6); + } + #[test] fn acp_agents_round_trip_through_payload() { let mut src = EditorState::new();