From 121e3b5dfd58a743439660dba2c89917a71d7218 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Tue, 6 May 2025 19:18:49 +0200 Subject: [PATCH] agent: Handle context servers that do not provide a configuration in MCP setup dialog (#30023) image image Release Notes: - N/A --- .../add_context_server_modal.rs | 81 ++--- .../configure_context_server_modal.rs | 276 ++++++++++++------ .../agent/src/context_server_configuration.rs | 69 +++-- crates/ui/src/components/modal.rs | 2 +- 4 files changed, 279 insertions(+), 149 deletions(-) diff --git a/crates/agent/src/assistant_configuration/add_context_server_modal.rs b/crates/agent/src/assistant_configuration/add_context_server_modal.rs index 6109b2d513..47ba57b035 100644 --- a/crates/agent/src/assistant_configuration/add_context_server_modal.rs +++ b/crates/agent/src/assistant_configuration/add_context_server_modal.rs @@ -147,47 +147,50 @@ impl Render for AddContextServerModal { ), ) .footer( - ModalFooter::new() - .start_slot( - Button::new("cancel", "Cancel") - .key_binding( - KeyBinding::for_action_in( - &menu::Cancel, - &focus_handle, - window, - cx, + ModalFooter::new().end_slot( + h_flex() + .gap_2() + .child( + Button::new("cancel", "Cancel") + .key_binding( + KeyBinding::for_action_in( + &menu::Cancel, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), ) - .map(|kb| kb.size(rems_from_px(12.))), - ) - .on_click(cx.listener(|this, _event, _window, cx| { - this.cancel(&menu::Cancel, cx) - })), - ) - .end_slot( - Button::new("add-server", "Add Server") - .disabled(is_name_empty || is_command_empty) - .key_binding( - KeyBinding::for_action_in( - &menu::Confirm, - &focus_handle, - window, - cx, + .on_click(cx.listener(|this, _event, _window, cx| { + this.cancel(&menu::Cancel, cx) + })), + ) + .child( + Button::new("add-server", "Add Server") + .disabled(is_name_empty || is_command_empty) + .key_binding( + KeyBinding::for_action_in( + &menu::Confirm, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), ) - .map(|kb| kb.size(rems_from_px(12.))), - ) - .map(|button| { - if is_name_empty { - button.tooltip(Tooltip::text("Name is required")) - } else if is_command_empty { - button.tooltip(Tooltip::text("Command is required")) - } else { - button - } - }) - .on_click(cx.listener(|this, _event, _window, cx| { - this.confirm(&menu::Confirm, cx) - })), - ), + .map(|button| { + if is_name_empty { + button.tooltip(Tooltip::text("Name is required")) + } else if is_command_empty { + button.tooltip(Tooltip::text("Command is required")) + } else { + button + } + }) + .on_click(cx.listener(|this, _event, _window, cx| { + this.confirm(&menu::Confirm, cx) + })), + ), + ), ), ) } diff --git a/crates/agent/src/assistant_configuration/configure_context_server_modal.rs b/crates/agent/src/assistant_configuration/configure_context_server_modal.rs index 8365fb577d..29c14d2662 100644 --- a/crates/agent/src/assistant_configuration/configure_context_server_modal.rs +++ b/crates/agent/src/assistant_configuration/configure_context_server_modal.rs @@ -19,18 +19,24 @@ use project::{ }; use settings::{Settings as _, update_settings_file}; use theme::ThemeSettings; -use ui::{KeyBinding, Modal, ModalFooter, ModalHeader, Section, prelude::*}; +use ui::{KeyBinding, Modal, ModalFooter, ModalHeader, Section, Tooltip, prelude::*}; use util::ResultExt; use workspace::{ModalView, Workspace}; pub(crate) struct ConfigureContextServerModal { workspace: WeakEntity, - context_servers_to_setup: Vec, + focus_handle: FocusHandle, + context_servers_to_setup: Vec, context_server_store: Entity, } -struct ConfigureContextServer { - id: ContextServerId, +#[allow(clippy::large_enum_variant)] +enum Configuration { + NotAvailable, + Required(ConfigurationRequiredState), +} + +struct ConfigurationRequiredState { installation_instructions: Entity, settings_validator: Option, settings_editor: Entity, @@ -38,64 +44,91 @@ struct ConfigureContextServer { waiting_for_context_server: bool, } +struct ContextServerSetup { + id: ContextServerId, + repository_url: Option, + configuration: Configuration, +} + impl ConfigureContextServerModal { pub fn new( - configurations: impl Iterator, + configurations: impl Iterator, context_server_store: Entity, jsonc_language: Option>, language_registry: Arc, workspace: WeakEntity, window: &mut Window, - cx: &mut App, - ) -> Option { + cx: &mut Context, + ) -> Self { let context_servers_to_setup = configurations - .map(|(id, manifest)| { - let jsonc_language = jsonc_language.clone(); - let settings_validator = jsonschema::validator_for(&manifest.settings_schema) - .context("Failed to load JSON schema for context server settings") - .log_err(); - ConfigureContextServer { - id: id.clone(), - installation_instructions: cx.new(|cx| { - Markdown::new( - manifest.installation_instructions.clone().into(), - Some(language_registry.clone()), - None, - cx, - ) - }), - settings_validator, - settings_editor: cx.new(|cx| { - let mut editor = Editor::auto_height(16, window, cx); - editor.set_text(manifest.default_settings.trim(), window, cx); - editor.set_show_gutter(false, cx); - editor.set_soft_wrap_mode(language::language_settings::SoftWrap::None, cx); - if let Some(buffer) = editor.buffer().read(cx).as_singleton() { - buffer.update(cx, |buffer, cx| buffer.set_language(jsonc_language, cx)) - } - editor - }), - waiting_for_context_server: false, - last_error: None, + .map(|config| match config { + crate::context_server_configuration::Configuration::NotAvailable( + context_server_id, + repository_url, + ) => ContextServerSetup { + id: context_server_id, + repository_url, + configuration: Configuration::NotAvailable, + }, + crate::context_server_configuration::Configuration::Required( + context_server_id, + repository_url, + config, + ) => { + let jsonc_language = jsonc_language.clone(); + let settings_validator = jsonschema::validator_for(&config.settings_schema) + .context("Failed to load JSON schema for context server settings") + .log_err(); + let state = ConfigurationRequiredState { + installation_instructions: cx.new(|cx| { + Markdown::new( + config.installation_instructions.clone().into(), + Some(language_registry.clone()), + None, + cx, + ) + }), + settings_validator, + settings_editor: cx.new(|cx| { + let mut editor = Editor::auto_height(16, window, cx); + editor.set_text(config.default_settings.trim(), window, cx); + editor.set_show_gutter(false, cx); + editor.set_soft_wrap_mode( + language::language_settings::SoftWrap::None, + cx, + ); + if let Some(buffer) = editor.buffer().read(cx).as_singleton() { + buffer.update(cx, |buffer, cx| { + buffer.set_language(jsonc_language, cx) + }) + } + editor + }), + waiting_for_context_server: false, + last_error: None, + }; + ContextServerSetup { + id: context_server_id, + repository_url, + configuration: Configuration::Required(state), + } } }) .collect::>(); - if context_servers_to_setup.is_empty() { - return None; - } - - Some(Self { + Self { workspace, + focus_handle: cx.focus_handle(), context_servers_to_setup, context_server_store, - }) + } } } impl ConfigureContextServerModal { pub fn confirm(&mut self, cx: &mut Context) { if self.context_servers_to_setup.is_empty() { + self.dismiss(cx); return; } @@ -103,7 +136,18 @@ impl ConfigureContextServerModal { return; }; - let configuration = &mut self.context_servers_to_setup[0]; + let id = self.context_servers_to_setup[0].id.clone(); + let configuration = match &mut self.context_servers_to_setup[0].configuration { + Configuration::NotAvailable => { + self.context_servers_to_setup.remove(0); + if self.context_servers_to_setup.is_empty() { + self.dismiss(cx); + } + return; + } + Configuration::Required(state) => state, + }; + configuration.last_error.take(); if configuration.waiting_for_context_server { return; @@ -127,7 +171,7 @@ impl ConfigureContextServerModal { return; } } - let id = configuration.id.clone(); + let id = id.clone(); let settings_changed = ProjectSettings::get_global(cx) .context_servers @@ -156,9 +200,14 @@ impl ConfigureContextServerModal { this.complete_setup(id, cx); } Err(err) => { - if let Some(configuration) = this.context_servers_to_setup.get_mut(0) { - configuration.last_error = Some(err.into()); - configuration.waiting_for_context_server = false; + if let Some(setup) = this.context_servers_to_setup.get_mut(0) { + match &mut setup.configuration { + Configuration::NotAvailable => {} + Configuration::Required(state) => { + state.last_error = Some(err.into()); + state.waiting_for_context_server = false; + } + } } else { this.dismiss(cx); } @@ -267,8 +316,8 @@ fn wait_for_context_server( impl Render for ConfigureContextServerModal { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - let Some(configuration) = self.context_servers_to_setup.first() else { - return div().child("No context servers to setup"); + let Some(setup) = self.context_servers_to_setup.first() else { + return div().into_any_element(); }; let focus_handle = self.focus_handle(cx); @@ -277,6 +326,7 @@ impl Render for ConfigureContextServerModal { .elevation_3(cx) .w(rems(42.)) .key_context("ConfigureContextServerModal") + .track_focus(&focus_handle) .on_action(cx.listener(|this, _: &menu::Confirm, _window, cx| this.confirm(cx))) .on_action(cx.listener(|this, _: &menu::Cancel, _window, cx| this.dismiss(cx))) .capture_any_mouse_down(cx.listener(|this, _, window, cx| { @@ -284,9 +334,15 @@ impl Render for ConfigureContextServerModal { })) .child( Modal::new("configure-context-server", None) - .header(ModalHeader::new().headline(format!("Configure {}", configuration.id))) - .section( - Section::new() + .header(ModalHeader::new().headline(format!("Configure {}", setup.id))) + .section(match &setup.configuration { + Configuration::NotAvailable => Section::new().child( + Label::new( + "No configuration options available for this context server. Visit the Repository for any further instructions.", + ) + .color(Color::Muted), + ), + Configuration::Required(configuration) => Section::new() .child(div().pb_2().text_sm().child(MarkdownElement::new( configuration.installation_instructions.clone(), default_markdown_style(window, cx), @@ -370,45 +426,84 @@ impl Render for ConfigureContextServerModal { ), ) }), - ) + }) .footer( - ModalFooter::new().end_slot( - h_flex() - .gap_1() - .child( - Button::new("cancel", "Cancel") - .key_binding( - KeyBinding::for_action_in( - &menu::Cancel, - &focus_handle, - window, - cx, - ) - .map(|kb| kb.size(rems_from_px(12.))), - ) - .on_click(cx.listener(|this, _event, _window, cx| { - this.dismiss(cx) - })), + ModalFooter::new() + .when_some(setup.repository_url.clone(), |this, repository_url| { + this.start_slot( + h_flex().w_full().child( + Button::new("open-repository", "Open Repository") + .icon(IconName::ArrowUpRight) + .icon_color(Color::Muted) + .icon_size(IconSize::XSmall) + .tooltip({ + let repository_url = repository_url.clone(); + move |window, cx| { + Tooltip::with_meta( + "Open Repository", + None, + repository_url.clone(), + window, + cx, + ) + } + }) + .on_click(move |_, _, cx| cx.open_url(&repository_url)), + ), ) - .child( - Button::new("configure-server", "Configure MCP") - .disabled(configuration.waiting_for_context_server) - .key_binding( - KeyBinding::for_action_in( - &menu::Confirm, - &focus_handle, - window, - cx, - ) - .map(|kb| kb.size(rems_from_px(12.))), + }) + .end_slot(match &setup.configuration { + Configuration::NotAvailable => Button::new("dismiss", "Dismiss") + .key_binding( + KeyBinding::for_action_in( + &menu::Cancel, + &focus_handle, + window, + cx, ) - .on_click(cx.listener(|this, _event, _window, cx| { - this.confirm(cx) - })), - ), - ), + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click( + cx.listener(|this, _event, _window, cx| this.dismiss(cx)), + ) + .into_any_element(), + Configuration::Required(state) => h_flex() + .gap_2() + .child( + Button::new("cancel", "Cancel") + .key_binding( + KeyBinding::for_action_in( + &menu::Cancel, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click(cx.listener(|this, _event, _window, cx| { + this.dismiss(cx) + })), + ) + .child( + Button::new("configure-server", "Configure MCP") + .disabled(state.waiting_for_context_server) + .key_binding( + KeyBinding::for_action_in( + &menu::Confirm, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click(cx.listener(|this, _event, _window, cx| { + this.confirm(cx) + })), + ) + .into_any_element(), + }), ), - ) + ).into_any_element() } } @@ -446,9 +541,14 @@ impl EventEmitter for ConfigureContextServerModal {} impl Focusable for ConfigureContextServerModal { fn focus_handle(&self, cx: &App) -> FocusHandle { if let Some(current) = self.context_servers_to_setup.first() { - current.settings_editor.read(cx).focus_handle(cx) + match ¤t.configuration { + Configuration::NotAvailable => self.focus_handle.clone(), + Configuration::Required(configuration) => { + configuration.settings_editor.read(cx).focus_handle(cx) + } + } } else { - cx.focus_handle() + self.focus_handle.clone() } } } diff --git a/crates/agent/src/context_server_configuration.rs b/crates/agent/src/context_server_configuration.rs index ccf0204058..0daa04f463 100644 --- a/crates/agent/src/context_server_configuration.rs +++ b/crates/agent/src/context_server_configuration.rs @@ -2,7 +2,8 @@ use std::sync::Arc; use anyhow::Context as _; use context_server::ContextServerId; -use extension::ExtensionManifest; +use extension::{ContextServerConfiguration, ExtensionManifest}; +use gpui::Task; use language::LanguageRegistry; use project::context_server_store::registry::ContextServerDescriptorRegistry; use ui::prelude::*; @@ -54,6 +55,15 @@ pub(crate) fn init(language_registry: Arc, cx: &mut App) { .detach(); } +pub enum Configuration { + NotAvailable(ContextServerId, Option), + Required( + ContextServerId, + Option, + ContextServerConfiguration, + ), +} + fn show_configure_mcp_modal( language_registry: Arc, manifest: &Arc, @@ -62,6 +72,7 @@ fn show_configure_mcp_modal( cx: &mut Context<'_, Workspace>, ) { let context_server_store = workspace.project().read(cx).context_server_store(); + let repository: Option = manifest.repository.as_ref().map(|s| s.clone().into()); let registry = ContextServerDescriptorRegistry::default_global(cx).read(cx); let worktree_store = workspace.project().read(cx).worktree_store(); @@ -69,21 +80,37 @@ fn show_configure_mcp_modal( .context_servers .keys() .cloned() - .filter_map({ + .map({ |key| { - let descriptor = registry.context_server_descriptor(&key)?; - Some(cx.spawn({ + let Some(descriptor) = registry.context_server_descriptor(&key) else { + return Task::ready(Configuration::NotAvailable( + ContextServerId(key), + repository.clone(), + )); + }; + cx.spawn({ + let repository_url = repository.clone(); let worktree_store = worktree_store.clone(); async move |_, cx| { - descriptor + let configuration = descriptor .configuration(worktree_store.clone(), &cx) .await .context("Failed to resolve context server configuration") .log_err() - .flatten() - .map(|config| (ContextServerId(key), config)) + .flatten(); + + match configuration { + Some(config) => Configuration::Required( + ContextServerId(key), + repository_url, + config, + ), + None => { + Configuration::NotAvailable(ContextServerId(key), repository_url) + } + } } - })) + }) } }) .collect::>(); @@ -91,22 +118,22 @@ fn show_configure_mcp_modal( let jsonc_language = language_registry.language_for_name("jsonc"); cx.spawn_in(window, async move |this, cx| { - let descriptors = futures::future::join_all(configuration_tasks).await; + let configurations = futures::future::join_all(configuration_tasks).await; let jsonc_language = jsonc_language.await.ok(); this.update_in(cx, |this, window, cx| { - let modal = ConfigureContextServerModal::new( - descriptors.into_iter().flatten(), - context_server_store, - jsonc_language, - language_registry, - cx.entity().downgrade(), - window, - cx, - ); - if let Some(modal) = modal { - this.toggle_modal(window, cx, |_, _| modal); - } + let workspace = cx.entity().downgrade(); + this.toggle_modal(window, cx, |window, cx| { + ConfigureContextServerModal::new( + configurations.into_iter(), + context_server_store, + jsonc_language, + language_registry, + workspace, + window, + cx, + ) + }); }) }) .detach(); diff --git a/crates/ui/src/components/modal.rs b/crates/ui/src/components/modal.rs index 8266c7b3d2..7b2c8beb62 100644 --- a/crates/ui/src/components/modal.rs +++ b/crates/ui/src/components/modal.rs @@ -253,7 +253,7 @@ impl RenderOnce for ModalFooter { .mt_4() .p(DynamicSpacing::Base08.rems(cx)) .flex_none() - .justify_end() + .justify_between() .gap_1() .border_t_1() .border_color(cx.theme().colors().border_variant)