From 2bae44417e73a196c3d094dcdb804723f7a4546d Mon Sep 17 00:00:00 2001 From: Fini Date: Sun, 31 May 2026 21:48:50 +0800 Subject: [PATCH] fix(agent): parse ACP args like web settings --- crates/op-editor-core/src/agent_settings.rs | 182 +----------------- crates/op-editor-core/src/lib.rs | 2 + .../src/tests_agent_settings.rs | 178 +++++++++++++++++ .../src/tests_agent_settings_draft.rs | 25 +++ crates/op-host-native/src/widget_host.rs | 2 + .../widget_host/agent_settings_acp_tests.rs | 144 ++++++++++++++ .../src/widget_host/agent_settings_tests.rs | 98 ---------- 7 files changed, 353 insertions(+), 278 deletions(-) create mode 100644 crates/op-editor-core/src/tests_agent_settings.rs create mode 100644 crates/op-host-native/src/widget_host/agent_settings_acp_tests.rs diff --git a/crates/op-editor-core/src/agent_settings.rs b/crates/op-editor-core/src/agent_settings.rs index e539458a6..288e0aa1a 100644 --- a/crates/op-editor-core/src/agent_settings.rs +++ b/crates/op-editor-core/src/agent_settings.rs @@ -281,7 +281,7 @@ impl AcpAgentConfig { } pub fn args_text(&self) -> String { - self.args.join(" ") + self.args.join(", ") } pub fn env_text(&self) -> String { @@ -294,7 +294,7 @@ impl AcpAgentConfig { pub fn set_args_text(&mut self, text: &str) { self.args = text - .split_whitespace() + .split(',') .map(str::trim) .filter(|part| !part.is_empty()) .map(str::to_string) @@ -704,181 +704,3 @@ impl AgentSettings { pub enum AgentSettingsDrag { Reserved, } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn default_settings_are_quiescent() { - let s = AgentSettings::default(); - assert_eq!(s.tab, AgentSettingsTab::Agents); - assert_eq!(s.connected, [false; 5]); - assert!(s.builtin_agents.is_empty()); - assert!(s.builtin_agent_draft.is_none()); - assert!(s.acp_agent_draft.is_none()); - assert!(s.image_gen_profiles.is_empty()); - assert!(s.active_image_gen_profile_id.is_none()); - assert!(!s.images_advanced_open); - assert!(s.openverse_client_id.is_empty()); - assert!(s.openverse_client_secret.is_empty()); - assert_eq!(s.mcp_server.port, 3100); - assert!(s.auto_update_enabled); - assert!(s.focus.is_none()); - assert_eq!(s.hover_provider, usize::MAX); - assert_eq!(s.hover_builtin_agent, usize::MAX); - assert_eq!(s.hover_acp_agent, usize::MAX); - } - - #[test] - fn tab_and_cli_arrays_cover_all_variants() { - assert_eq!(AgentSettingsTab::ALL.len(), 4); - assert_eq!(McpCli::ALL.len(), 6); - } - - #[test] - fn image_generation_profiles_follow_ts_lifecycle() { - let mut s = AgentSettings::default(); - - let first = s.add_image_gen_profile(); - assert_eq!(s.image_gen_profiles.len(), 1); - assert_eq!( - s.active_image_gen_profile_id.as_deref(), - Some(first.as_str()) - ); - assert_eq!(s.image_gen_profiles[0].name, "Config 1"); - assert_eq!(s.image_gen_profiles[0].provider, ImageGenProvider::OpenAi); - - let second = s.add_image_gen_profile(); - assert_eq!(s.image_gen_profiles.len(), 2); - assert_eq!( - s.active_image_gen_profile_id.as_deref(), - Some(first.as_str()) - ); - - assert!(s.set_active_image_gen_profile(&second)); - assert_eq!( - s.active_image_gen_profile_id.as_deref(), - Some(second.as_str()) - ); - - assert!(s.remove_image_gen_profile(&second)); - assert_eq!( - s.active_image_gen_profile_id.as_deref(), - Some(first.as_str()) - ); - - assert!(s.remove_image_gen_profile(&first)); - assert!(s.image_gen_profiles.is_empty()); - assert!(s.active_image_gen_profile_id.is_none()); - } - - #[test] - fn add_builtin_agent_prefills_ts_provider_presets_first() { - let mut s = AgentSettings::default(); - - for _ in 0..4 { - s.add_builtin_agent(); - } - - let summary: Vec<_> = s - .builtin_agents - .iter() - .map(|agent| { - ( - agent.display_name.as_str(), - agent.kind, - agent.model.as_str(), - agent.base_url.as_str(), - agent.api_key.as_str(), - ) - }) - .collect(); - - assert_eq!( - summary, - vec![ - ( - "Anthropic", - BuiltinAgentKind::Anthropic, - "claude-sonnet-4-6-20250916", - "https://api.anthropic.com", - "", - ), - ( - "OpenAI", - BuiltinAgentKind::OpenAiCompat, - "gpt-5.4", - "https://api.openai.com/v1", - "", - ), - ( - "OpenRouter", - BuiltinAgentKind::OpenAiCompat, - "anthropic/claude-sonnet-4.6", - "https://openrouter.ai/api/v1", - "", - ), - ( - "DeepSeek", - BuiltinAgentKind::OpenAiCompat, - "deepseek-v4-pro", - "https://api.deepseek.com/v1", - "", - ), - ] - ); - } - - #[test] - fn pure_builtin_presets_do_not_toggle_api_format() { - let mut anthropic = BuiltinAgentConfig { - id: "anthropic".into(), - preset: BuiltinAgentPresetKey::Anthropic, - display_name: "Anthropic".into(), - kind: BuiltinAgentKind::Anthropic, - api_key: String::new(), - model: "claude-sonnet-4-6-20250916".into(), - base_url: "https://api.anthropic.com".into(), - enabled: true, - }; - anthropic.toggle_kind_for_preset(); - assert_eq!(anthropic.kind, BuiltinAgentKind::Anthropic); - assert_eq!(anthropic.base_url, "https://api.anthropic.com"); - - let mut openai = BuiltinAgentConfig { - id: "openai".into(), - preset: BuiltinAgentPresetKey::OpenAi, - display_name: "OpenAI".into(), - kind: BuiltinAgentKind::OpenAiCompat, - api_key: String::new(), - model: "gpt-5.4".into(), - base_url: "https://api.openai.com/v1".into(), - enabled: true, - }; - openai.toggle_kind_for_preset(); - assert_eq!(openai.kind, BuiltinAgentKind::OpenAiCompat); - assert_eq!(openai.base_url, "https://api.openai.com/v1"); - } - - #[test] - fn add_acp_agent_assigns_id_and_defaults_to_local_config() { - let mut s = AgentSettings::default(); - - let first = s.add_acp_agent(); - let second = s.add_acp_agent(); - - assert_eq!(first, "acp-1"); - assert_eq!(second, "acp-2"); - assert_eq!(s.acp_agents.len(), 2); - assert_eq!(s.next_acp_agent_id, 3); - assert_eq!(s.acp_agents[0].display_name, "ACP Agent 1"); - assert_eq!(s.acp_agents[0].connection_type, AcpConnectionType::Local); - assert!(s.acp_agents[0].command.is_empty()); - assert!(s.acp_agents[0].args.is_empty()); - assert!(s.acp_agents[0].env.is_empty()); - assert!(s.acp_agents[0].url.is_none()); - assert!(s.acp_agents[0].enabled); - assert!(!s.acp_agents[0].connected); - } -} diff --git a/crates/op-editor-core/src/lib.rs b/crates/op-editor-core/src/lib.rs index eb9d1ec64..356d105bd 100644 --- a/crates/op-editor-core/src/lib.rs +++ b/crates/op-editor-core/src/lib.rs @@ -63,6 +63,8 @@ mod svg_import_tests; #[cfg(test)] mod test_support; #[cfg(test)] +mod tests_agent_settings; +#[cfg(test)] mod tests_agent_settings_draft; #[cfg(test)] mod tests_geometry; diff --git a/crates/op-editor-core/src/tests_agent_settings.rs b/crates/op-editor-core/src/tests_agent_settings.rs new file mode 100644 index 000000000..50c7d7f9a --- /dev/null +++ b/crates/op-editor-core/src/tests_agent_settings.rs @@ -0,0 +1,178 @@ +use crate::agent_settings::{ + AcpConnectionType, AgentSettings, AgentSettingsTab, BuiltinAgentConfig, BuiltinAgentKind, + ImageGenProvider, McpCli, +}; +use crate::agent_settings_builtin_presets::BuiltinAgentPresetKey; + +#[test] +fn default_settings_are_quiescent() { + let s = AgentSettings::default(); + assert_eq!(s.tab, AgentSettingsTab::Agents); + assert_eq!(s.connected, [false; 5]); + assert!(s.builtin_agents.is_empty()); + assert!(s.builtin_agent_draft.is_none()); + assert!(s.acp_agent_draft.is_none()); + assert!(s.image_gen_profiles.is_empty()); + assert!(s.active_image_gen_profile_id.is_none()); + assert!(!s.images_advanced_open); + assert!(s.openverse_client_id.is_empty()); + assert!(s.openverse_client_secret.is_empty()); + assert_eq!(s.mcp_server.port, 3100); + assert!(s.auto_update_enabled); + assert!(s.focus.is_none()); + assert_eq!(s.hover_provider, usize::MAX); + assert_eq!(s.hover_builtin_agent, usize::MAX); + assert_eq!(s.hover_acp_agent, usize::MAX); +} + +#[test] +fn tab_and_cli_arrays_cover_all_variants() { + assert_eq!(AgentSettingsTab::ALL.len(), 4); + assert_eq!(McpCli::ALL.len(), 6); +} + +#[test] +fn image_generation_profiles_follow_ts_lifecycle() { + let mut s = AgentSettings::default(); + + let first = s.add_image_gen_profile(); + assert_eq!(s.image_gen_profiles.len(), 1); + assert_eq!( + s.active_image_gen_profile_id.as_deref(), + Some(first.as_str()) + ); + assert_eq!(s.image_gen_profiles[0].name, "Config 1"); + assert_eq!(s.image_gen_profiles[0].provider, ImageGenProvider::OpenAi); + + let second = s.add_image_gen_profile(); + assert_eq!(s.image_gen_profiles.len(), 2); + assert_eq!( + s.active_image_gen_profile_id.as_deref(), + Some(first.as_str()) + ); + + assert!(s.set_active_image_gen_profile(&second)); + assert_eq!( + s.active_image_gen_profile_id.as_deref(), + Some(second.as_str()) + ); + + assert!(s.remove_image_gen_profile(&second)); + assert_eq!( + s.active_image_gen_profile_id.as_deref(), + Some(first.as_str()) + ); + + assert!(s.remove_image_gen_profile(&first)); + assert!(s.image_gen_profiles.is_empty()); + assert!(s.active_image_gen_profile_id.is_none()); +} + +#[test] +fn add_builtin_agent_prefills_ts_provider_presets_first() { + let mut s = AgentSettings::default(); + + for _ in 0..4 { + s.add_builtin_agent(); + } + + let summary: Vec<_> = s + .builtin_agents + .iter() + .map(|agent| { + ( + agent.display_name.as_str(), + agent.kind, + agent.model.as_str(), + agent.base_url.as_str(), + agent.api_key.as_str(), + ) + }) + .collect(); + + assert_eq!( + summary, + vec![ + ( + "Anthropic", + BuiltinAgentKind::Anthropic, + "claude-sonnet-4-6-20250916", + "https://api.anthropic.com", + "", + ), + ( + "OpenAI", + BuiltinAgentKind::OpenAiCompat, + "gpt-5.4", + "https://api.openai.com/v1", + "", + ), + ( + "OpenRouter", + BuiltinAgentKind::OpenAiCompat, + "anthropic/claude-sonnet-4.6", + "https://openrouter.ai/api/v1", + "", + ), + ( + "DeepSeek", + BuiltinAgentKind::OpenAiCompat, + "deepseek-v4-pro", + "https://api.deepseek.com/v1", + "", + ), + ] + ); +} + +#[test] +fn pure_builtin_presets_do_not_toggle_api_format() { + let mut anthropic = BuiltinAgentConfig { + id: "anthropic".into(), + preset: BuiltinAgentPresetKey::Anthropic, + display_name: "Anthropic".into(), + kind: BuiltinAgentKind::Anthropic, + api_key: String::new(), + model: "claude-sonnet-4-6-20250916".into(), + base_url: "https://api.anthropic.com".into(), + enabled: true, + }; + anthropic.toggle_kind_for_preset(); + assert_eq!(anthropic.kind, BuiltinAgentKind::Anthropic); + assert_eq!(anthropic.base_url, "https://api.anthropic.com"); + + let mut openai = BuiltinAgentConfig { + id: "openai".into(), + preset: BuiltinAgentPresetKey::OpenAi, + display_name: "OpenAI".into(), + kind: BuiltinAgentKind::OpenAiCompat, + api_key: String::new(), + model: "gpt-5.4".into(), + base_url: "https://api.openai.com/v1".into(), + enabled: true, + }; + openai.toggle_kind_for_preset(); + assert_eq!(openai.kind, BuiltinAgentKind::OpenAiCompat); + assert_eq!(openai.base_url, "https://api.openai.com/v1"); +} + +#[test] +fn add_acp_agent_assigns_id_and_defaults_to_local_config() { + let mut s = AgentSettings::default(); + + let first = s.add_acp_agent(); + let second = s.add_acp_agent(); + + assert_eq!(first, "acp-1"); + assert_eq!(second, "acp-2"); + assert_eq!(s.acp_agents.len(), 2); + assert_eq!(s.next_acp_agent_id, 3); + assert_eq!(s.acp_agents[0].display_name, "ACP Agent 1"); + assert_eq!(s.acp_agents[0].connection_type, AcpConnectionType::Local); + assert!(s.acp_agents[0].command.is_empty()); + assert!(s.acp_agents[0].args.is_empty()); + assert!(s.acp_agents[0].env.is_empty()); + assert!(s.acp_agents[0].url.is_none()); + assert!(s.acp_agents[0].enabled); + assert!(!s.acp_agents[0].connected); +} 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 883538f6f..764a6a819 100644 --- a/crates/op-editor-core/src/tests_agent_settings_draft.rs +++ b/crates/op-editor-core/src/tests_agent_settings_draft.rs @@ -97,3 +97,28 @@ fn acp_agent_draft_can_cancel_or_save() { assert!(s.acp_agent_draft.is_none()); assert_eq!(s.acp_agents[0].command, "op-agent"); } + +#[test] +fn acp_agent_args_text_uses_comma_separated_values_like_ts() { + let mut s = AgentSettings::default(); + let id = s.add_acp_agent_config( + "Local ACP", + crate::AcpConnectionType::Local, + "op-agent", + vec!["--stdio".into(), "--workspace /tmp".into()], + Default::default(), + None, + true, + ); + let agent = s + .acp_agents + .iter_mut() + .find(|agent| agent.id == id) + .expect("agent exists"); + + assert_eq!(agent.args_text(), "--stdio, --workspace /tmp"); + + agent.set_args_text(" --stdio, --workspace /tmp, , --verbose "); + + assert_eq!(agent.args, vec!["--stdio", "--workspace /tmp", "--verbose"]); +} diff --git a/crates/op-host-native/src/widget_host.rs b/crates/op-host-native/src/widget_host.rs index abc18d157..aff0eab10 100644 --- a/crates/op-host-native/src/widget_host.rs +++ b/crates/op-host-native/src/widget_host.rs @@ -34,6 +34,8 @@ use op_editor_ui::widgets::SelectionHandle; use op_editor_ui::{Rect, Theme}; +#[cfg(test)] +mod agent_settings_acp_tests; mod agent_settings_draft_dispatch; #[cfg(test)] mod agent_settings_tests; diff --git a/crates/op-host-native/src/widget_host/agent_settings_acp_tests.rs b/crates/op-host-native/src/widget_host/agent_settings_acp_tests.rs new file mode 100644 index 000000000..9c0368bf9 --- /dev/null +++ b/crates/op-host-native/src/widget_host/agent_settings_acp_tests.rs @@ -0,0 +1,144 @@ +use super::WidgetHostNative; +use op_editor_core::agent_settings::{AcpAgentField, SettingsFocus}; +use op_editor_ui::widgets::agent_settings_panel::AgentSettingsPanel; + +fn agent_settings_content_metrics(host: &WidgetHostNative) -> (f32, f32, f32) { + let panel = AgentSettingsPanel::for_editor(host.editor_state()); + let rect = panel.rect(1200.0, 800.0); + ( + rect.origin.x + 200.0 + 24.0, + rect.origin.y + 24.0, + rect.size.x - 200.0 - 48.0, + ) +} + +fn acp_card_y(content_y: f32) -> f32 { + content_y + 12.0 + 120.0 + 28.0 + 28.0 + 28.0 +} + +#[test] +fn acp_agent_command_field_accepts_text_and_commits() { + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .editor_ui + .agent_settings + .add_acp_agent(); + host.editor_state_mut() + .editor_ui + .agent_settings + .hover_acp_agent = 0; + + let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); + let card_y = acp_card_y(content_y); + assert!(host.dispatch_agent_settings_press( + content_x + content_w - 156.0, + card_y + 30.0, + 1200.0, + 800.0 + )); + + assert!(host.dispatch_agent_settings_press( + content_x + 92.0, + card_y + 154.0 + 14.0, + 1200.0, + 800.0 + )); + for c in "op-agent".chars() { + assert!(host.apply_text(c)); + } + assert!(host.apply_send()); + + let settings = &host.editor_state().editor_ui.agent_settings; + assert_eq!(settings.acp_agents[0].command, "op-agent"); + assert!(settings.focus.is_none()); + assert!(host + .editor_state() + .editor_ui + .settings_input_draft + .is_empty()); +} + +#[test] +fn acp_agent_args_field_commits_comma_separated_args() { + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .editor_ui + .agent_settings + .add_acp_agent(); + host.editor_state_mut().editor_ui.agent_settings.focus = Some(SettingsFocus::AcpAgent { + index: 0, + field: AcpAgentField::Args, + }); + host.editor_state_mut().editor_ui.settings_input_draft = + " --stdio, --workspace /tmp, , --verbose ".into(); + + assert!(host.apply_send()); + + let settings = &host.editor_state().editor_ui.agent_settings; + assert_eq!( + settings.acp_agents[0].args, + vec!["--stdio", "--workspace /tmp", "--verbose"] + ); + assert!(settings.focus.is_none()); + assert!(host + .editor_state() + .editor_ui + .settings_input_draft + .is_empty()); +} + +#[test] +fn acp_agent_remove_press_deletes_agent_and_clears_focus() { + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .editor_ui + .agent_settings + .add_acp_agent(); + host.editor_state_mut() + .editor_ui + .agent_settings + .hover_acp_agent = 0; + + let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); + let card_y = acp_card_y(content_y); + assert!(host.dispatch_agent_settings_press( + content_x + content_w - 128.0, + card_y + 30.0, + 1200.0, + 800.0 + )); + + let settings = &host.editor_state().editor_ui.agent_settings; + assert!(settings.acp_agents.is_empty()); + assert!(settings.focus.is_none()); + assert!(host + .editor_state() + .editor_ui + .settings_input_draft + .is_empty()); +} + +#[test] +fn acp_agent_connect_press_toggles_connected_state() { + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .editor_ui + .agent_settings + .add_acp_agent(); + host.editor_state_mut().editor_ui.agent_settings.acp_agents[0].command = "op-agent".into(); + + let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); + let card_y = acp_card_y(content_y); + let button_x = content_x + content_w - 60.0; + assert!(host.dispatch_agent_settings_press(button_x, card_y + 30.0, 1200.0, 800.0)); + assert!( + host.editor_state().editor_ui.agent_settings.acp_agents[0].connected, + "configured local ACP agent should become connected after pressing Connect" + ); + + assert!(host.dispatch_agent_settings_press(button_x, card_y + 30.0, 1200.0, 800.0)); + assert!( + !host.editor_state().editor_ui.agent_settings.acp_agents[0].connected, + "connected ACP agent should become disconnected after pressing Disconnect" + ); +} diff --git a/crates/op-host-native/src/widget_host/agent_settings_tests.rs b/crates/op-host-native/src/widget_host/agent_settings_tests.rs index 020ae41e1..51aa77367 100644 --- a/crates/op-host-native/src/widget_host/agent_settings_tests.rs +++ b/crates/op-host-native/src/widget_host/agent_settings_tests.rs @@ -378,104 +378,6 @@ fn acp_agent_compact_edit_focuses_display_name_form() { ); } -#[test] -fn acp_agent_command_field_accepts_text_and_commits() { - let mut host = WidgetHostNative::new(); - host.editor_state_mut() - .editor_ui - .agent_settings - .add_acp_agent(); - host.editor_state_mut() - .editor_ui - .agent_settings - .hover_acp_agent = 0; - - let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); - let card_y = acp_card_y(content_y); - assert!(host.dispatch_agent_settings_press( - content_x + content_w - 156.0, - card_y + 30.0, - 1200.0, - 800.0 - )); - - assert!(host.dispatch_agent_settings_press( - content_x + 92.0, - card_y + 154.0 + 14.0, - 1200.0, - 800.0 - )); - for c in "op-agent".chars() { - assert!(host.apply_text(c)); - } - assert!(host.apply_send()); - - let settings = &host.editor_state().editor_ui.agent_settings; - assert_eq!(settings.acp_agents[0].command, "op-agent"); - assert!(settings.focus.is_none()); - assert!(host - .editor_state() - .editor_ui - .settings_input_draft - .is_empty()); -} - -#[test] -fn acp_agent_remove_press_deletes_agent_and_clears_focus() { - let mut host = WidgetHostNative::new(); - host.editor_state_mut() - .editor_ui - .agent_settings - .add_acp_agent(); - host.editor_state_mut() - .editor_ui - .agent_settings - .hover_acp_agent = 0; - - let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); - let card_y = acp_card_y(content_y); - assert!(host.dispatch_agent_settings_press( - content_x + content_w - 128.0, - card_y + 30.0, - 1200.0, - 800.0 - )); - - let settings = &host.editor_state().editor_ui.agent_settings; - assert!(settings.acp_agents.is_empty()); - assert!(settings.focus.is_none()); - assert!(host - .editor_state() - .editor_ui - .settings_input_draft - .is_empty()); -} - -#[test] -fn acp_agent_connect_press_toggles_connected_state() { - let mut host = WidgetHostNative::new(); - host.editor_state_mut() - .editor_ui - .agent_settings - .add_acp_agent(); - host.editor_state_mut().editor_ui.agent_settings.acp_agents[0].command = "op-agent".into(); - - let (content_x, content_y, content_w) = agent_settings_content_metrics(&host); - let card_y = acp_card_y(content_y); - let button_x = content_x + content_w - 60.0; - assert!(host.dispatch_agent_settings_press(button_x, card_y + 30.0, 1200.0, 800.0)); - assert!( - host.editor_state().editor_ui.agent_settings.acp_agents[0].connected, - "configured local ACP agent should become connected after pressing Connect" - ); - - assert!(host.dispatch_agent_settings_press(button_x, card_y + 30.0, 1200.0, 800.0)); - assert!( - !host.editor_state().editor_ui.agent_settings.acp_agents[0].connected, - "connected ACP agent should become disconnected after pressing Disconnect" - ); -} - #[test] fn starting_mcp_server_commits_port_draft_and_clears_focus() { let mut host = WidgetHostNative::new();