agent: Handle context servers that do not provide a configuration in MCP setup dialog (#30023)

Bennet Bo Fenner created

<img width="674" alt="image"
src="https://github.com/user-attachments/assets/0ccb89e2-1dc1-4caf-88a7-49159f43979f"
/>
<img width="675" alt="image"
src="https://github.com/user-attachments/assets/790e5d45-905e-45da-affa-04ddd1d33c65"
/>

Release Notes:

- N/A

Change summary

crates/agent/src/assistant_configuration/add_context_server_modal.rs       |  81 
crates/agent/src/assistant_configuration/configure_context_server_modal.rs | 276 
crates/agent/src/context_server_configuration.rs                           |  69 
crates/ui/src/components/modal.rs                                          |   2 
4 files changed, 279 insertions(+), 149 deletions(-)

Detailed changes

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)
+                                        })),
+                                ),
+                        ),
                     ),
             )
     }

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<Workspace>,
-    context_servers_to_setup: Vec<ConfigureContextServer>,
+    focus_handle: FocusHandle,
+    context_servers_to_setup: Vec<ContextServerSetup>,
     context_server_store: Entity<ContextServerStore>,
 }
 
-struct ConfigureContextServer {
-    id: ContextServerId,
+#[allow(clippy::large_enum_variant)]
+enum Configuration {
+    NotAvailable,
+    Required(ConfigurationRequiredState),
+}
+
+struct ConfigurationRequiredState {
     installation_instructions: Entity<markdown::Markdown>,
     settings_validator: Option<jsonschema::Validator>,
     settings_editor: Entity<Editor>,
@@ -38,64 +44,91 @@ struct ConfigureContextServer {
     waiting_for_context_server: bool,
 }
 
+struct ContextServerSetup {
+    id: ContextServerId,
+    repository_url: Option<SharedString>,
+    configuration: Configuration,
+}
+
 impl ConfigureContextServerModal {
     pub fn new(
-        configurations: impl Iterator<Item = (ContextServerId, extension::ContextServerConfiguration)>,
+        configurations: impl Iterator<Item = crate::context_server_configuration::Configuration>,
         context_server_store: Entity<ContextServerStore>,
         jsonc_language: Option<Arc<Language>>,
         language_registry: Arc<LanguageRegistry>,
         workspace: WeakEntity<Workspace>,
         window: &mut Window,
-        cx: &mut App,
-    ) -> Option<Self> {
+        cx: &mut Context<Self>,
+    ) -> 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::<Vec<_>>();
 
-        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<Self>) {
         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<Self>) -> 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<DismissEvent> 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 &current.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()
         }
     }
 }

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<LanguageRegistry>, cx: &mut App) {
     .detach();
 }
 
+pub enum Configuration {
+    NotAvailable(ContextServerId, Option<SharedString>),
+    Required(
+        ContextServerId,
+        Option<SharedString>,
+        ContextServerConfiguration,
+    ),
+}
+
 fn show_configure_mcp_modal(
     language_registry: Arc<LanguageRegistry>,
     manifest: &Arc<ExtensionManifest>,
@@ -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<SharedString> = 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::<Vec<_>>();
@@ -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();

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)