From 5d556a2bdacef263efb10ed5a7cdce436931b0d1 Mon Sep 17 00:00:00 2001 From: cameron Date: Fri, 22 May 2026 13:13:56 +0100 Subject: [PATCH 01/16] Custom settings ui element --- crates/settings_ui/src/page_data.rs | 29 +++- crates/settings_ui/src/settings_ui.rs | 200 ++++++++++++++++++++------ 2 files changed, 180 insertions(+), 49 deletions(-) diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 3eb1ec94512d5c..315ee81f95cb96 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -10,8 +10,9 @@ use theme::SystemAppearance; use ui::IntoElement; use crate::{ - ActionLink, DynamicItem, PROJECT, SettingField, SettingItem, SettingsFieldMetadata, - SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names, + ActionLink, DynamicItem, NonJsonItem, PROJECT, SettingField, SettingItem, + SettingsFieldMetadata, SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, + all_language_names, pages::{ open_audio_test_window, render_edit_prediction_setup_page, render_skills_setup_page, render_tool_permissions_setup_page, @@ -131,6 +132,29 @@ fn developer_page() -> SettingsPage { } fn general_page(cx: &App) -> SettingsPage { + fn non_json_test_section() -> [SettingsPageItem; 2] { + [ + SettingsPageItem::SectionHeader("NonJson Test"), + SettingsPageItem::NonJson(NonJsonItem { + title: "Test NonJson Setting", + description: "A test non-JSON-backed setting that prints to stdout on every interaction.", + json_path: Some("test.non_json"), + files: USER, + can_reset: |_cx| { + println!("[NonJson test] can_reset called"); + true + }, + reset: |_window, _cx| { + println!("[NonJson test] reset called"); + }, + render_control: |_settings_window, _window, _cx| { + println!("[NonJson test] render_control called"); + ui::Label::new("NonJson control").into_any_element() + }, + }), + ] + } + fn general_settings_section(_cx: &App) -> Vec { vec![ SettingsPageItem::SectionHeader("General Settings"), @@ -418,6 +442,7 @@ fn general_page(cx: &App) -> SettingsPage { title: "General", items: concat_sections!( @vec, + non_json_test_section(), general_settings_section(cx), security_section(), workspace_restoration_section(), diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 2a49a95af2ae98..a9ffda72fec4a9 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -849,6 +849,7 @@ enum SettingsPageItem { SubPageLink(SubPageLink), DynamicItem(DynamicItem), ActionLink(ActionLink), + NonJson(NonJsonItem), } impl std::fmt::Debug for SettingsPageItem { @@ -867,6 +868,9 @@ impl std::fmt::Debug for SettingsPageItem { SettingsPageItem::ActionLink(action_link) => { write!(f, "ActionLink({})", action_link.title) } + SettingsPageItem::NonJson(non_json_item) => { + write!(f, "NonJson({})", non_json_item.title) + } } } } @@ -1157,23 +1161,38 @@ impl SettingsPageItem { ) .when(bottom_border, |this| this.child(Divider::horizontal())) .into_any_element(), + SettingsPageItem::NonJson(non_json_item) => { + let field = render_non_json_item(settings_window, non_json_item, window, cx); + let field_with_padding = apply_padding(field); + + v_flex() + .group("setting-item") + .px_8() + .child(field_with_padding) + .when(bottom_border, |this| this.child(Divider::horizontal())) + .into_any_element() + } } } } -fn render_settings_item( +/// Shared layout for both JSON-backed and non-JSON-backed setting items. +/// +/// Renders title + description on the left, control on the right, with +/// optional reset button and copy-link icon. +fn render_settings_item_layout( settings_window: &SettingsWindow, - setting_item: &SettingItem, - file: SettingsUiFile, + title: &'static str, + description: &'static str, control: AnyElement, + reset_fn: Option>, + modified_in: Option, + json_path: Option<&'static str>, sub_field: bool, cx: &mut Context<'_, SettingsWindow>, ) -> Stateful
{ - let (found_in_file, _) = setting_item.field.file_set_in(file.clone(), cx); - let file_set_in = SettingsUiFile::from_settings(found_in_file.clone()); - h_flex() - .id(setting_item.title) + .id(title) .min_w_0() .justify_between() .child( @@ -1186,47 +1205,28 @@ fn render_settings_item( h_flex() .w_full() .gap_1() - .child(Label::new(SharedString::new_static(setting_item.title))) - .when_some( - if sub_field { - None - } else { - setting_item - .field - .reset_to_default_fn(&file, &found_in_file, cx) - }, - |this, reset_to_default| { - this.child( - IconButton::new("reset-to-default-btn", IconName::Undo) - .icon_color(Color::Muted) - .icon_size(IconSize::Small) - .tooltip(Tooltip::text("Reset to Default")) - .on_click({ - move |_, window, cx| { - reset_to_default(window, cx); - } - }), - ) - }, - ) - .when_some( - file_set_in.filter(|file_set_in| file_set_in != &file), - |this, file_set_in| { - this.child( - Label::new(format!( - "— Modified in {}", - settings_window - .display_name(&file_set_in) - .expect("File name should exist") - )) + .child(Label::new(SharedString::new_static(title))) + .when_some(reset_fn, |this, reset_to_default| { + this.child( + IconButton::new("reset-to-default-btn", IconName::Undo) + .icon_color(Color::Muted) + .icon_size(IconSize::Small) + .tooltip(Tooltip::text("Reset to Default")) + .on_click(move |_, window, cx| { + reset_to_default(window, cx); + }), + ) + }) + .when_some(modified_in, |this, modified_in| { + this.child( + Label::new(format!("\u{2014} Modified in {modified_in}")) .color(Color::Muted) .size(LabelSize::Small), - ) - }, - ), + ) + }), ) .child( - Label::new(SharedString::new_static(setting_item.description)) + Label::new(SharedString::new_static(description)) .size(LabelSize::Small) .color(Color::Muted) .render_code_spans(), @@ -1235,8 +1235,8 @@ fn render_settings_item( .child(control) .when(settings_window.sub_page_stack.is_empty(), |this| { this.child(render_settings_item_link( - setting_item.description, - setting_item.field.json_path(), + description, + json_path, sub_field, settings_window, cx, @@ -1244,6 +1244,70 @@ fn render_settings_item( }) } +fn render_settings_item( + settings_window: &SettingsWindow, + setting_item: &SettingItem, + file: SettingsUiFile, + control: AnyElement, + sub_field: bool, + cx: &mut Context<'_, SettingsWindow>, +) -> Stateful
{ + let (found_in_file, _) = setting_item.field.file_set_in(file.clone(), cx); + let file_set_in = SettingsUiFile::from_settings(found_in_file.clone()); + + let reset_fn = if sub_field { + None + } else { + setting_item + .field + .reset_to_default_fn(&file, &found_in_file, cx) + }; + + let modified_in = file_set_in + .filter(|f| f != &file) + .and_then(|f| settings_window.display_name(&f)); + + render_settings_item_layout( + settings_window, + setting_item.title, + setting_item.description, + control, + reset_fn, + modified_in, + setting_item.field.json_path(), + sub_field, + cx, + ) +} + +fn render_non_json_item( + settings_window: &SettingsWindow, + item: &NonJsonItem, + window: &mut Window, + cx: &mut Context<'_, SettingsWindow>, +) -> Stateful
{ + let control = (item.render_control)(settings_window, window, cx); + + let reset_fn: Option> = if (item.can_reset)(cx) { + let reset = item.reset; + Some(Box::new(move |window, cx| reset(window, cx))) + } else { + None + }; + + render_settings_item_layout( + settings_window, + item.title, + item.description, + control, + reset_fn, + None, + item.json_path, + false, + cx, + ) +} + fn render_settings_item_link( id: impl Into, json_path: Option<&'static str>, @@ -1412,6 +1476,25 @@ impl PartialEq for ActionLink { } } +struct NonJsonItem { + title: &'static str, + description: &'static str, + /// A stable path identifier for deep-linking and search, even though this + /// setting is not stored in settings.json. + json_path: Option<&'static str>, + files: FileMask, + can_reset: fn(&App) -> bool, + reset: fn(&mut Window, &mut App), + render_control: + fn(&SettingsWindow, &mut Window, &mut Context) -> AnyElement, +} + +impl PartialEq for NonJsonItem { + fn eq(&self, other: &Self) -> bool { + self.title == other.title + } +} + fn all_language_names(cx: &App) -> Vec { let state = workspace::AppState::global(cx); state @@ -1903,7 +1986,8 @@ impl SettingsWindow { | SettingsPageItem::DynamicItem(DynamicItem { discriminant: SettingItem { files, .. }, .. - }) => { + }) + | SettingsPageItem::NonJson(NonJsonItem { files, .. }) => { if !files.contains(current_file) { page_filter[index] = false; } else { @@ -2165,6 +2249,28 @@ impl SettingsWindow { action_link.title.as_ref(), ); } + SettingsPageItem::NonJson(non_json_item) => { + json_path = non_json_item.json_path; + documents.push(SearchDocument { + id: key_index, + words: split_into_words(&[ + page.title, + header_str, + non_json_item.title, + non_json_item.description, + ]), + }); + push_candidates( + &mut fuzzy_match_candidates, + key_index, + non_json_item.title, + ); + push_candidates( + &mut fuzzy_match_candidates, + key_index, + non_json_item.description, + ); + } } push_candidates(&mut fuzzy_match_candidates, key_index, page.title); push_candidates(&mut fuzzy_match_candidates, key_index, header_str); From 40e45be685a662cabc89236da581e2818f509854 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 26 May 2026 13:51:29 +0100 Subject: [PATCH 02/16] add LLM provider page with dummy element --- crates/settings_ui/src/page_data.rs | 41 +++++--------- crates/settings_ui/src/pages.rs | 2 + .../src/pages/llm_providers_page.rs | 56 +++++++++++++++++++ crates/settings_ui/src/settings_ui.rs | 11 +++- 4 files changed, 79 insertions(+), 31 deletions(-) create mode 100644 crates/settings_ui/src/pages/llm_providers_page.rs diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 315ee81f95cb96..ee50d29a1b811e 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -10,11 +10,11 @@ use theme::SystemAppearance; use ui::IntoElement; use crate::{ - ActionLink, DynamicItem, NonJsonItem, PROJECT, SettingField, SettingItem, - SettingsFieldMetadata, SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, - all_language_names, + ActionLink, DynamicItem, PROJECT, SettingField, SettingItem, SettingsFieldMetadata, + SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names, pages::{ - open_audio_test_window, render_edit_prediction_setup_page, render_skills_setup_page, + open_audio_test_window, render_edit_prediction_setup_page, + render_llm_providers_page, render_skills_setup_page, render_tool_permissions_setup_page, }, }; @@ -132,29 +132,6 @@ fn developer_page() -> SettingsPage { } fn general_page(cx: &App) -> SettingsPage { - fn non_json_test_section() -> [SettingsPageItem; 2] { - [ - SettingsPageItem::SectionHeader("NonJson Test"), - SettingsPageItem::NonJson(NonJsonItem { - title: "Test NonJson Setting", - description: "A test non-JSON-backed setting that prints to stdout on every interaction.", - json_path: Some("test.non_json"), - files: USER, - can_reset: |_cx| { - println!("[NonJson test] can_reset called"); - true - }, - reset: |_window, _cx| { - println!("[NonJson test] reset called"); - }, - render_control: |_settings_window, _window, _cx| { - println!("[NonJson test] render_control called"); - ui::Label::new("NonJson control").into_any_element() - }, - }), - ] - } - fn general_settings_section(_cx: &App) -> Vec { vec![ SettingsPageItem::SectionHeader("General Settings"), @@ -442,7 +419,6 @@ fn general_page(cx: &App) -> SettingsPage { title: "General", items: concat_sections!( @vec, - non_json_test_section(), general_settings_section(cx), security_section(), workspace_restoration_section(), @@ -7509,6 +7485,15 @@ fn ai_page(cx: &App) -> SettingsPage { fn agent_configuration_section(_cx: &App) -> Box<[SettingsPageItem]> { let mut items = vec![ SettingsPageItem::SectionHeader("Agent Configuration"), + SettingsPageItem::SubPageLink(SubPageLink { + title: "LLM Providers".into(), + r#type: Default::default(), + json_path: Some("llm_providers"), + description: Some("Configure API keys and settings for LLM providers.".into()), + in_json: false, + files: USER, + render: render_llm_providers_page, + }), SettingsPageItem::SubPageLink(SubPageLink { title: "Skills".into(), r#type: Default::default(), diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index f2f8dab3c4c0b3..9b1ba629410694 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -2,6 +2,7 @@ mod audio_input_output_setup; mod audio_test_window; mod edit_prediction_provider_setup; mod feature_flags; +mod llm_providers_page; mod skills_setup; mod tool_permissions_setup; @@ -11,6 +12,7 @@ pub(crate) use audio_input_output_setup::{ pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; pub(crate) use feature_flags::render_feature_flags_page; +pub(crate) use llm_providers_page::render_llm_providers_page; pub(crate) use skills_setup::render_skills_setup_page; pub(crate) use tool_permissions_setup::render_tool_permissions_setup_page; diff --git a/crates/settings_ui/src/pages/llm_providers_page.rs b/crates/settings_ui/src/pages/llm_providers_page.rs new file mode 100644 index 00000000000000..037897d44079b4 --- /dev/null +++ b/crates/settings_ui/src/pages/llm_providers_page.rs @@ -0,0 +1,56 @@ +use std::sync::atomic::{AtomicU32, Ordering}; + +use gpui::{ScrollHandle, prelude::*}; +use ui::prelude::*; + +use crate::{NonJsonItem, SettingsWindow, USER, render_non_json_item}; + +static COUNTER: AtomicU32 = AtomicU32::new(0); + +pub(crate) fn render_llm_providers_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let test_item = NonJsonItem { + title: "Test Counter", + description: "A test non-JSON-backed setting with a counter.", + json_path: Some("test.counter"), + files: USER, + can_reset: |_cx| COUNTER.load(Ordering::SeqCst) != 0, + reset: |_window, _cx| { + COUNTER.store(0, Ordering::SeqCst); + }, + render_control: |_settings_window, _window, cx| { + let value = COUNTER.load(Ordering::SeqCst); + h_flex() + .gap_2() + .items_center() + .child(Label::new(format!("Count: {value}"))) + .child( + Button::new("increment", "Increment") + .tab_index(0_isize) + .style(ButtonStyle::Outlined) + .on_click(cx.listener(|_this, _, _window, cx| { + COUNTER.fetch_add(1, Ordering::SeqCst); + cx.notify(); + })), + ) + .into_any_element() + }, + }; + + let item = render_non_json_item(settings_window, &test_item, window, cx); + + v_flex() + .id("llm-providers-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child(item) + .into_any_element() +} diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index a9ffda72fec4a9..f1645d8440027c 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -1280,7 +1280,7 @@ fn render_settings_item( ) } -fn render_non_json_item( +pub(crate) fn render_non_json_item( settings_window: &SettingsWindow, item: &NonJsonItem, window: &mut Window, @@ -1476,7 +1476,7 @@ impl PartialEq for ActionLink { } } -struct NonJsonItem { +pub(crate) struct NonJsonItem { title: &'static str, description: &'static str, /// A stable path identifier for deep-linking and search, even though this @@ -1919,7 +1919,11 @@ impl SettingsWindow { move |this: &mut SettingsWindow, window: &mut Window, cx: &mut Context| { - this.open_and_scroll_to_navbar_entry(entry_index, None, false, window, cx); + if this.sub_page_stack.is_empty() { + this.open_and_scroll_to_navbar_entry( + entry_index, None, false, window, cx, + ); + } }, ); focus_subscriptions.push(subscription); @@ -2435,6 +2439,7 @@ impl SettingsWindow { let is_new_page = self.navbar_entries[self.navbar_entry].page_index != self.navbar_entries[navbar_entry].page_index; + self.navbar_entry = navbar_entry; // We only need to reset visible items when updating matches From 4f8f3c09a2a8178bbe4815f39133f9cbbcc0ec8e Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 26 May 2026 17:58:50 +0100 Subject: [PATCH 03/16] providers working --- Cargo.lock | 1 + crates/settings_ui/Cargo.toml | 1 + .../src/pages/llm_providers_page.rs | 192 ++++++++++++++---- crates/settings_ui/src/settings_ui.rs | 10 + 4 files changed, 168 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 43c2b961b86b23..5e4864b42f7c68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16252,6 +16252,7 @@ dependencies = [ "heck 0.5.0", "itertools 0.14.0", "language", + "language_model", "log", "menu", "paths", diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index ee725c0a1d99a6..3a4bf84be5561f 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -37,6 +37,7 @@ gpui.workspace = true heck.workspace = true itertools.workspace = true language.workspace = true +language_model.workspace = true log.workspace = true menu.workspace = true paths.workspace = true diff --git a/crates/settings_ui/src/pages/llm_providers_page.rs b/crates/settings_ui/src/pages/llm_providers_page.rs index 037897d44079b4..e9a5d2631e1e1d 100644 --- a/crates/settings_ui/src/pages/llm_providers_page.rs +++ b/crates/settings_ui/src/pages/llm_providers_page.rs @@ -1,11 +1,13 @@ -use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::Arc; -use gpui::{ScrollHandle, prelude::*}; -use ui::prelude::*; +use gpui::{AnyView, ScrollHandle, prelude::*}; +use language_model::{ + ConfigurationViewTargetAgent, IconOrSvg, LanguageModelProvider, LanguageModelProviderId, + LanguageModelRegistry, +}; +use ui::{Disclosure, Divider, DividerColor, prelude::*}; -use crate::{NonJsonItem, SettingsWindow, USER, render_non_json_item}; - -static COUNTER: AtomicU32 = AtomicU32::new(0); +use crate::SettingsWindow; pub(crate) fn render_llm_providers_page( settings_window: &SettingsWindow, @@ -13,35 +15,7 @@ pub(crate) fn render_llm_providers_page( window: &mut Window, cx: &mut Context, ) -> AnyElement { - let test_item = NonJsonItem { - title: "Test Counter", - description: "A test non-JSON-backed setting with a counter.", - json_path: Some("test.counter"), - files: USER, - can_reset: |_cx| COUNTER.load(Ordering::SeqCst) != 0, - reset: |_window, _cx| { - COUNTER.store(0, Ordering::SeqCst); - }, - render_control: |_settings_window, _window, cx| { - let value = COUNTER.load(Ordering::SeqCst); - h_flex() - .gap_2() - .items_center() - .child(Label::new(format!("Count: {value}"))) - .child( - Button::new("increment", "Increment") - .tab_index(0_isize) - .style(ButtonStyle::Outlined) - .on_click(cx.listener(|_this, _, _window, cx| { - COUNTER.fetch_add(1, Ordering::SeqCst); - cx.notify(); - })), - ) - .into_any_element() - }, - }; - - let item = render_non_json_item(settings_window, &test_item, window, cx); + let providers = LanguageModelRegistry::read_global(cx).visible_providers(); v_flex() .id("llm-providers-page") @@ -51,6 +25,152 @@ pub(crate) fn render_llm_providers_page( .pb_16() .track_scroll(scroll_handle) .overflow_y_scroll() - .child(item) + .children( + providers + .iter() + .map(|provider| { + render_provider_block(settings_window, provider, window, cx) + }) + .collect::>(), + ) + .into_any_element() +} + +fn render_provider_block( + settings_window: &SettingsWindow, + provider: &Arc, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let provider_id = provider.id(); + let provider_name = provider.name().0.clone(); + let disclosure_id = SharedString::from(format!("provider-disclosure-{}", provider_id.0)); + + let is_expanded = settings_window + .expanded_provider_configurations + .get(&provider_id) + .copied() + .unwrap_or(false); + + let configuration_view = if is_expanded { + Some(get_or_create_configuration_view( + settings_window, + &provider_id, + provider, + window, + cx, + )) + } else { + None + }; + + let is_authenticated = provider.is_authenticated(cx); + + v_flex() + .min_w_0() + .w_full() + .when(is_expanded, |this| this.mb_2()) + .child( + div() + .px_2() + .child(Divider::horizontal().color(DividerColor::BorderFaded)), + ) + .child( + h_flex() + .map(|this| { + if is_expanded { + this.mt_2().mb_1() + } else { + this.my_2() + } + }) + .w_full() + .justify_between() + .child( + h_flex() + .id(disclosure_id.clone()) + .px_2() + .py_0p5() + .w_full() + .justify_between() + .rounded_sm() + .hover(|hover| hover.bg(cx.theme().colors().element_hover)) + .child( + h_flex() + .w_full() + .gap_1p5() + .child( + match provider.icon() { + IconOrSvg::Svg(path) => Icon::from_external_svg(path), + IconOrSvg::Icon(name) => Icon::new(name), + } + .size(IconSize::Small) + .color(Color::Muted), + ) + .child( + h_flex() + .w_full() + .gap_1() + .child(Label::new(provider_name.clone())) + .when(is_authenticated && !is_expanded, |this| { + this.child( + Icon::new(IconName::Check).color(Color::Success), + ) + }), + ), + ) + .child( + Disclosure::new(disclosure_id, is_expanded) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronDown), + ) + .on_click(cx.listener({ + let provider_id = provider_id.clone(); + move |this, _event, _window, _cx| { + let is_expanded = this + .expanded_provider_configurations + .entry(provider_id.clone()) + .or_insert(false); + *is_expanded = !*is_expanded; + } + })), + ), + ) + .child( + v_flex() + .min_w_0() + .w_full() + .px_2() + .gap_1() + .when_some(configuration_view, |this, view| this.child(view)), + ) .into_any_element() } + +fn get_or_create_configuration_view( + settings_window: &SettingsWindow, + provider_id: &LanguageModelProviderId, + provider: &Arc, + window: &mut Window, + cx: &mut Context, +) -> AnyView { + if let Some(view) = settings_window.provider_configuration_views.get(provider_id) { + return view.clone(); + } + + let view = provider.configuration_view( + ConfigurationViewTargetAgent::ZedAgent, + window, + cx, + ); + + // Store the view for future renders by deferring a mutation + let provider_id = provider_id.clone(); + let view_clone = view.clone(); + cx.defer_in(window, move |this, _window, _cx| { + this.provider_configuration_views + .insert(provider_id, view_clone); + }); + + view +} diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index f1645d8440027c..b8a656fbd04cf8 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -772,6 +772,10 @@ pub struct SettingsWindow { pub(crate) hidden_deleted_skill_directory_paths: HashSet, pub(crate) regex_validation_error: Option, last_copied_link_path: Option<&'static str>, + pub(crate) expanded_provider_configurations: + HashMap, + pub(crate) provider_configuration_views: + HashMap, } struct SearchDocument { @@ -1801,6 +1805,8 @@ impl SettingsWindow { regex_validation_error: None, list_state, last_copied_link_path: None, + expanded_provider_configurations: HashMap::default(), + provider_configuration_views: HashMap::default(), }; this.fetch_files(window, cx); @@ -4688,6 +4694,8 @@ pub mod test { hidden_deleted_skill_directory_paths: HashSet::default(), regex_validation_error: None, last_copied_link_path: None, + expanded_provider_configurations: HashMap::default(), + provider_configuration_views: HashMap::default(), } } } @@ -4815,6 +4823,8 @@ pub mod test { hidden_deleted_skill_directory_paths: HashSet::default(), regex_validation_error: None, last_copied_link_path: None, + expanded_provider_configurations: HashMap::default(), + provider_configuration_views: HashMap::default(), }; settings_window.build_filter_table(); From aac166bda962e6a69ea563584b1bcc27c2b6b258 Mon Sep 17 00:00:00 2001 From: cameron Date: Wed, 27 May 2026 20:08:41 +0100 Subject: [PATCH 04/16] [wip] MCP servers page --- Cargo.lock | 3 +++ crates/settings_ui/Cargo.toml | 3 +++ crates/settings_ui/src/page_data.rs | 11 ++++++++++- crates/settings_ui/src/pages.rs | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 5e4864b42f7c68..cf819d43513ba7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16238,12 +16238,15 @@ dependencies = [ "audio", "codestral", "component", + "context_server", "copilot", "copilot_ui", "cpal", "edit_prediction", "edit_prediction_ui", "editor", + "extension", + "extension_host", "feature_flags", "fs", "futures 0.3.32", diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index 3a4bf84be5561f..20cdab103d78be 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -23,12 +23,15 @@ anyhow.workspace = true audio.workspace = true component.workspace = true codestral.workspace = true +context_server.workspace = true copilot.workspace = true copilot_ui.workspace = true cpal.workspace = true edit_prediction.workspace = true edit_prediction_ui.workspace = true editor.workspace = true +extension.workspace = true +extension_host.workspace = true fs.workspace = true feature_flags.workspace = true futures.workspace = true diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index ee50d29a1b811e..e0a4c522595a4c 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -14,7 +14,7 @@ use crate::{ SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names, pages::{ open_audio_test_window, render_edit_prediction_setup_page, - render_llm_providers_page, render_skills_setup_page, + render_llm_providers_page, render_mcp_servers_page, render_skills_setup_page, render_tool_permissions_setup_page, }, }; @@ -7512,6 +7512,15 @@ fn ai_page(cx: &App) -> SettingsPage { files: USER, render: render_tool_permissions_setup_page, }), + SettingsPageItem::SubPageLink(SubPageLink { + title: "MCP Servers".into(), + r#type: Default::default(), + json_path: Some("context_servers"), + description: Some("View, add, configure, and remove Model Context Protocol servers.".into()), + in_json: false, + files: USER, + render: render_mcp_servers_page, + }), ]; items.extend([ diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 9b1ba629410694..ec18bcd16cad88 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -3,6 +3,7 @@ mod audio_test_window; mod edit_prediction_provider_setup; mod feature_flags; mod llm_providers_page; +mod mcp_servers_page; mod skills_setup; mod tool_permissions_setup; @@ -13,6 +14,7 @@ pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; pub(crate) use feature_flags::render_feature_flags_page; pub(crate) use llm_providers_page::render_llm_providers_page; +pub(crate) use mcp_servers_page::render_mcp_servers_page; pub(crate) use skills_setup::render_skills_setup_page; pub(crate) use tool_permissions_setup::render_tool_permissions_setup_page; From a09637d87c06be970b834a5f81fc7567baa652c2 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 2 Jun 2026 13:32:56 +0100 Subject: [PATCH 05/16] actually git add lol --- .../settings_ui/src/pages/mcp_servers_page.rs | 587 ++++++++++++++++++ 1 file changed, 587 insertions(+) create mode 100644 crates/settings_ui/src/pages/mcp_servers_page.rs diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs new file mode 100644 index 00000000000000..807f330ec8fd8a --- /dev/null +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -0,0 +1,587 @@ +use std::sync::Arc; + +use context_server::ContextServerId; +use extension_host::ExtensionStore; +use gpui::{Action as _, Entity, ScrollHandle, prelude::*}; +use project::context_server_store::{ + ContextServerConfiguration, ContextServerStatus, ContextServerStore, +}; +use settings::ContextServerSettingsContent; +use ui::{ + AiSettingItem, AiSettingItemSource, AiSettingItemStatus, ContextMenu, Divider, DividerColor, + PopoverMenu, Switch, ToggleState, Tooltip, prelude::*, +}; +use util::ResultExt as _; + +use zed_actions::ExtensionCategoryFilter; + +use crate::SettingsWindow; + +pub(crate) fn render_mcp_servers_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + _window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let context_server_store = get_context_server_store(settings_window, cx); + + let server_list = if let Some(store) = context_server_store.as_ref() { + let server_ids = store.read(cx).server_ids().to_vec(); + + if server_ids.is_empty() { + render_empty_state(cx) + } else { + render_server_list(&server_ids, store, cx) + } + } else { + render_no_project_state(cx) + }; + + let add_server_popover = render_add_server_popover(settings_window, cx); + + v_flex() + .id("mcp-servers-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child( + h_flex() + .w_full() + .justify_between() + .items_center() + .mb_4() + .child( + v_flex() + .child(Label::new("MCP Servers").size(LabelSize::Large)) + .child( + Label::new("Manage Model Context Protocol servers connected directly or via extensions.") + .size(LabelSize::Small) + .color(Color::Muted), + ), + ) + .child(add_server_popover), + ) + .child(server_list) + .into_any_element() +} + +fn get_context_server_store( + settings_window: &SettingsWindow, + cx: &App, +) -> Option> { + let original_window = settings_window.original_window.as_ref()?; + let multi_workspace = original_window.read(cx).ok()?; + let workspace = multi_workspace.workspaces().next()?; + let project = workspace.read(cx).project().clone(); + Some(project.read(cx).context_server_store()) +} + +fn render_empty_state(cx: &App) -> AnyElement { + h_flex() + .p_4() + .justify_center() + .border_1() + .border_dashed() + .border_color(cx.theme().colors().border.opacity(0.6)) + .rounded_sm() + .child( + Label::new("No MCP servers added yet. Click \"Add Server\" to get started.") + .color(Color::Muted) + .size(LabelSize::Small), + ) + .into_any_element() +} + +fn render_no_project_state(cx: &App) -> AnyElement { + h_flex() + .p_4() + .justify_center() + .border_1() + .border_dashed() + .border_color(cx.theme().colors().border.opacity(0.6)) + .rounded_sm() + .child( + Label::new("No active project found. Open a workspace to manage MCP servers.") + .color(Color::Muted) + .size(LabelSize::Small), + ) + .into_any_element() +} + +fn render_server_list( + server_ids: &[ContextServerId], + store: &Entity, + cx: &mut Context, +) -> AnyElement { + v_flex() + .w_full() + .gap_1() + .children(itertools::intersperse_with( + server_ids.iter().map(|server_id| { + render_context_server(server_id, store, cx).into_any_element() + }), + || { + Divider::horizontal() + .color(DividerColor::BorderFaded) + .into_any_element() + }, + )) + .into_any_element() +} + +fn render_context_server( + context_server_id: &ContextServerId, + store: &Entity, + cx: &mut Context, +) -> impl IntoElement { + let server_status = store + .read(cx) + .status_for_server(context_server_id) + .unwrap_or(ContextServerStatus::Stopped); + let server_configuration = store.read(cx).configuration_for_server(context_server_id); + + let is_running = matches!(server_status, ContextServerStatus::Running); + let item_id = SharedString::from(context_server_id.0.to_string()); + + let provided_by_extension = server_configuration.as_ref().is_none_or(|config| { + matches!( + config.as_ref(), + ContextServerConfiguration::Extension { .. } + ) + }); + + let display_name = if provided_by_extension { + resolve_extension_display_name(context_server_id, cx).unwrap_or_else(|| item_id.clone()) + } else { + item_id.clone() + }; + + let source = if provided_by_extension { + AiSettingItemSource::Extension + } else { + AiSettingItemSource::Custom + }; + + let status = map_server_status(&server_status); + + let is_remote = server_configuration + .as_ref() + .map(|config| matches!(config.as_ref(), ContextServerConfiguration::Http { .. })) + .unwrap_or(false); + + let should_show_logout = server_configuration.as_ref().is_some_and(|config| { + matches!(config.as_ref(), ContextServerConfiguration::Http { .. }) + && !config.has_static_auth_header() + }); + + // ContextServerRegistry is per-project (not a global), so we skip tool count + // in the settings UI for now. + let tool_count = 0usize; + + let tool_label = if is_running && tool_count > 0 { + Some(if tool_count == 1 { + SharedString::from("1 tool") + } else { + SharedString::from(format!("{} tools", tool_count)) + }) + } else { + None + }; + + // Build gear menu + let gear_menu = render_gear_menu( + context_server_id, + store, + provided_by_extension, + should_show_logout, + is_remote, + ); + + // Build toggle switch + let toggle_switch = render_toggle_switch(context_server_id, store, is_running); + + // Build details (error/auth feedback) + let details = render_status_details( + &server_status, + context_server_id, + store, + should_show_logout, + ); + + AiSettingItem::new(item_id, display_name, status, source) + .action(gear_menu) + .action(toggle_switch) + .when_some(tool_label, |this, label| this.detail_label(label)) + .when_some(details, |this, details| this.details(details)) +} + +fn map_server_status(status: &ContextServerStatus) -> AiSettingItemStatus { + match status { + ContextServerStatus::Starting => AiSettingItemStatus::Starting, + ContextServerStatus::Running => AiSettingItemStatus::Running, + ContextServerStatus::Stopped => AiSettingItemStatus::Stopped, + ContextServerStatus::Error(_) => AiSettingItemStatus::Error, + ContextServerStatus::AuthRequired => AiSettingItemStatus::AuthRequired, + ContextServerStatus::ClientSecretRequired { .. } => { + AiSettingItemStatus::ClientSecretRequired + } + ContextServerStatus::Authenticating => AiSettingItemStatus::Authenticating, + } +} + +fn resolve_extension_display_name( + id: &ContextServerId, + cx: &App, +) -> Option { + ExtensionStore::global(cx) + .read(cx) + .installed_extensions() + .iter() + .find(|(_, entry)| entry.manifest.context_servers.contains_key(&id.0)) + .map(|(_, entry)| { + let name = entry.manifest.name.as_str(); + let stripped = name + .strip_suffix(" MCP Server") + .or_else(|| name.strip_suffix(" MCP")) + .or_else(|| name.strip_suffix(" Context Server")) + .unwrap_or(name); + SharedString::from(stripped.to_string()) + }) +} + +fn render_gear_menu( + context_server_id: &ContextServerId, + store: &Entity, + provided_by_extension: bool, + should_show_logout: bool, + _is_remote: bool, +) -> impl IntoElement { + let context_server_id = context_server_id.clone(); + let store = store.clone(); + + PopoverMenu::new(SharedString::from(format!( + "mcp-gear-{}", + context_server_id.0 + ))) + .trigger_with_tooltip( + IconButton::new( + SharedString::from(format!("mcp-gear-btn-{}", context_server_id.0)), + IconName::Settings, + ) + .icon_color(Color::Muted) + .icon_size(IconSize::Small), + Tooltip::text("Configure MCP Server"), + ) + .anchor(gpui::Anchor::TopRight) + .menu({ + move |window, cx| { + let context_server_id = context_server_id.clone(); + let store = store.clone(); + + Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { + menu.when(should_show_logout, |this| { + this.entry("Log Out", None, { + let store = store.clone(); + let context_server_id = context_server_id.clone(); + move |_window, cx| { + store.update(cx, |s, cx| { + s.logout_server(&context_server_id, cx).log_err(); + }); + } + }) + }) + .separator() + .entry("Uninstall", None, { + let context_server_id = context_server_id.clone(); + move |_, cx| { + uninstall_server(&context_server_id, provided_by_extension, cx); + } + }) + })) + } + }) +} + +fn render_toggle_switch( + context_server_id: &ContextServerId, + store: &Entity, + is_running: bool, +) -> impl IntoElement { + let context_server_id = context_server_id.clone(); + let store = store.clone(); + + Switch::new( + SharedString::from(format!("mcp-toggle-{}", context_server_id.0)), + if is_running { + ToggleState::Selected + } else { + ToggleState::Unselected + }, + ) + .on_click({ + move |state, _window, cx| { + let is_enabled = match state { + ToggleState::Unselected | ToggleState::Indeterminate => { + store.update(cx, |this, cx| { + this.stop_server(&context_server_id, cx).log_err(); + }); + false + } + ToggleState::Selected => { + store.update(cx, |this, cx| { + if let Some(server) = this.get_server(&context_server_id) { + this.start_server(server, cx); + } + }); + true + } + }; + + let fs = ::global(cx); + settings::update_settings_file(fs, cx, { + let context_server_id = context_server_id.clone(); + move |settings, _| { + settings + .project + .context_servers + .entry(context_server_id.0.clone()) + .or_insert_with(|| ContextServerSettingsContent::Extension { + enabled: is_enabled, + remote: false, + settings: serde_json::json!({}), + }) + .set_enabled(is_enabled); + } + }); + } + }) +} + +fn render_status_details( + server_status: &ContextServerStatus, + context_server_id: &ContextServerId, + store: &Entity, + should_show_logout: bool, +) -> Option { + let feedback_base = || h_flex().py_1().min_w_0().w_full().gap_1().justify_between(); + + match server_status { + ContextServerStatus::Error(error) => { + let store = store.clone(); + let context_server_id = context_server_id.clone(); + Some( + feedback_base() + .child( + h_flex() + .pr_4() + .min_w_0() + .w_full() + .gap_2() + .child( + Icon::new(IconName::XCircle) + .size(IconSize::XSmall) + .color(Color::Error), + ) + .child( + div().min_w_0().flex_1().child( + Label::new(error.to_string()) + .color(Color::Muted) + .size(LabelSize::Small), + ), + ), + ) + .when(should_show_logout, |this| { + this.child( + Button::new("error-logout", "Log Out") + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .on_click({ + let store = store.clone(); + let context_server_id = context_server_id.clone(); + move |_event, _window, cx| { + store.update(cx, |s, cx| { + s.logout_server(&context_server_id, cx).log_err(); + }); + } + }), + ) + }) + .into_any_element(), + ) + } + ContextServerStatus::AuthRequired => { + let store = store.clone(); + let context_server_id = context_server_id.clone(); + Some( + feedback_base() + .child( + h_flex() + .pr_4() + .min_w_0() + .w_full() + .gap_2() + .child( + Icon::new(IconName::Info) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child( + Label::new("Authenticate to connect this server") + .color(Color::Muted) + .size(LabelSize::Small), + ), + ) + .child( + Button::new("authenticate-server", "Authenticate") + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .on_click({ + move |_event, _window, cx| { + store.update(cx, |s, cx| { + s.authenticate_server(&context_server_id, cx).log_err(); + }); + } + }), + ) + .into_any_element(), + ) + } + ContextServerStatus::ClientSecretRequired { .. } => Some( + feedback_base() + .child( + h_flex() + .pr_4() + .min_w_0() + .w_full() + .gap_2() + .child( + Icon::new(IconName::Info) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child( + Label::new("A client secret is required to connect this server") + .color(Color::Muted) + .size(LabelSize::Small), + ), + ) + .into_any_element(), + ), + ContextServerStatus::Authenticating => Some( + h_flex() + .mt_1() + .pr_4() + .min_w_0() + .w_full() + .gap_2() + .child(div().size_3().flex_shrink_0()) + .child( + Label::new("Authenticating…") + .color(Color::Muted) + .size(LabelSize::Small), + ) + .into_any_element(), + ), + _ => None, + } +} + +fn render_add_server_popover( + settings_window: &SettingsWindow, + _cx: &App, +) -> impl IntoElement { + let original_window = settings_window.original_window; + + PopoverMenu::new("add-mcp-server-popover") + .trigger( + Button::new("add-mcp-server", "Add Server") + .style(ButtonStyle::Outlined) + .start_icon( + Icon::new(IconName::Plus) + .size(IconSize::Small) + .color(Color::Muted), + ) + .label_size(LabelSize::Small), + ) + .anchor(gpui::Anchor::TopRight) + .menu({ + move |window, cx| { + Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { + menu.entry("Install from Extensions", None, { + move |_window, cx| { + if let Some(original_window) = original_window.as_ref() { + original_window + .update(cx, |_, window, cx| { + window.activate_window(); + window.dispatch_action( + zed_actions::Extensions { + category_filter: Some( + ExtensionCategoryFilter::ContextServers, + ), + id: None, + } + .boxed_clone(), + cx, + ); + }) + .log_err(); + } + } + }) + })) + } + }) +} + +fn uninstall_server( + context_server_id: &ContextServerId, + provided_by_extension: bool, + cx: &mut App, +) { + if provided_by_extension { + if let Some((ext_id, manifest)) = resolve_extension_for_context_server(context_server_id, cx) + { + if extension_only_provides_context_server(&manifest) { + ExtensionStore::global(cx) + .update(cx, |store, cx| store.uninstall_extension(ext_id, cx)) + .detach_and_log_err(cx); + } + } + } + + let fs = ::global(cx); + let context_server_id = context_server_id.clone(); + settings::update_settings_file(fs, cx, move |settings, _| { + settings + .project + .context_servers + .remove(&context_server_id.0); + }); +} + +fn resolve_extension_for_context_server( + id: &ContextServerId, + cx: &App, +) -> Option<(Arc, Arc)> { + ExtensionStore::global(cx) + .read(cx) + .installed_extensions() + .iter() + .find(|(_, entry)| entry.manifest.context_servers.contains_key(&id.0)) + .map(|(id, entry)| (id.clone(), entry.manifest.clone())) +} + +fn extension_only_provides_context_server(manifest: &extension::ExtensionManifest) -> bool { + manifest.context_servers.len() == 1 + && manifest.themes.is_empty() + && manifest.icon_themes.is_empty() + && manifest.languages.is_empty() + && manifest.grammars.is_empty() + && manifest.language_servers.is_empty() + && manifest.slash_commands.is_empty() + && manifest.snippets.is_none() + && manifest.debug_locators.is_empty() +} From 90787c33efc4411a02ab9c8b9c178b69a9942917 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 2 Jun 2026 13:54:59 +0100 Subject: [PATCH 06/16] focus mcp server page --- crates/settings_ui/src/pages/mcp_servers_page.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs index 807f330ec8fd8a..759ea415c62281 100644 --- a/crates/settings_ui/src/pages/mcp_servers_page.rs +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -513,6 +513,7 @@ fn render_add_server_popover( menu.entry("Install from Extensions", None, { move |_window, cx| { if let Some(original_window) = original_window.as_ref() { + cx.activate(true); original_window .update(cx, |_, window, cx| { window.activate_window(); From 7f83cf2da8c2c86434fb69211dab5d7f9cb17aa7 Mon Sep 17 00:00:00 2001 From: cameron Date: Fri, 5 Jun 2026 15:06:11 +0100 Subject: [PATCH 07/16] mcp servers UI --- Cargo.lock | 2 + crates/project/src/context_server_store.rs | 73 ++ crates/settings_ui/Cargo.toml | 2 + crates/settings_ui/src/pages.rs | 2 +- .../src/pages/llm_providers_page.rs | 7 +- .../settings_ui/src/pages/mcp_servers_page.rs | 933 +++++++++++++++++- .../src/pages/tool_permissions_setup.rs | 1 + crates/settings_ui/src/settings_ui.rs | 30 +- 8 files changed, 1017 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4550704c874721..8defbdb340a27b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16536,6 +16536,7 @@ dependencies = [ "anyhow", "audio", "codestral", + "collections", "component", "context_server", "copilot", @@ -16577,6 +16578,7 @@ dependencies = [ "theme_settings", "title_bar", "ui", + "url", "util", "workspace", "zed_actions", diff --git a/crates/project/src/context_server_store.rs b/crates/project/src/context_server_store.rs index 35651a7ff4bd81..386d04f5049923 100644 --- a/crates/project/src/context_server_store.rs +++ b/crates/project/src/context_server_store.rs @@ -535,6 +535,79 @@ impl ContextServerStore { self.servers.get(id).map(|state| state.configuration()) } + /// Returns the configured settings for a server, if it is present in the user + /// or project settings. This is available regardless of whether the server is + /// currently running, unlike [`Self::configuration_for_server`]. + pub fn settings_for_server(&self, id: &ContextServerId) -> Option<&ContextServerSettings> { + self.context_server_settings.get(&id.0) + } + + /// Returns whether a server is provided by an extension (as opposed to a + /// custom Stdio/HTTP server configured directly in settings). + /// + /// This is derived from the configured settings rather than the runtime + /// configuration, so it stays correct even when a custom server is disabled + /// or has not been started yet (in which case it has no runtime state). + pub fn is_extension_provided(&self, id: &ContextServerId, cx: &App) -> bool { + match self.context_server_settings.get(&id.0) { + Some(ContextServerSettings::Stdio { .. } | ContextServerSettings::Http { .. }) => false, + Some(ContextServerSettings::Extension { .. }) => true, + // No custom settings entry: the server can only originate from an + // extension descriptor in the registry. + None => self + .registry + .read(cx) + .context_server_descriptor(&id.0) + .is_some(), + } + } + + /// Returns a configuration suitable for pre-filling the "edit custom server" + /// UI. Prefers the live runtime configuration, but falls back to building one + /// synchronously from the configured settings when the server has no runtime + /// state (e.g. it is disabled or has not been started this session). + /// + /// Returns `None` for extension-provided servers, which are not edited via + /// the custom-server form, and for HTTP servers whose configured URL cannot + /// be parsed. + pub fn editable_configuration_for_server( + &self, + id: &ContextServerId, + ) -> Option> { + if let Some(configuration) = self.configuration_for_server(id) { + return Some(configuration); + } + + match self.context_server_settings.get(&id.0)? { + ContextServerSettings::Stdio { + command, remote, .. + } => Some(Arc::new(ContextServerConfiguration::Custom { + command: command.clone(), + remote: *remote, + })), + ContextServerSettings::Http { + url, + headers, + timeout, + oauth, + .. + } => { + // Parse failures are expected here (e.g. an invalid URL that the + // user is still editing). This runs on every render, so fail + // silently rather than logging; the invalid URL is surfaced via + // the server's error status instead. + let url = url::Url::parse(url).ok()?; + Some(Arc::new(ContextServerConfiguration::Http { + url, + headers: headers.clone(), + timeout: *timeout, + oauth: oauth.clone(), + })) + } + ContextServerSettings::Extension { .. } => None, + } + } + /// Returns a sorted slice of available unique context server IDs. Within the /// slice, context servers which have `mcp-server-` as a prefix in their ID will /// appear after servers that do not have this prefix in their ID. diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index 20cdab103d78be..bfe50ba50d5275 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -23,6 +23,7 @@ anyhow.workspace = true audio.workspace = true component.workspace = true codestral.workspace = true +collections.workspace = true context_server.workspace = true copilot.workspace = true copilot_ui.workspace = true @@ -61,6 +62,7 @@ telemetry.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true +url.workspace = true util.workspace = true workspace.workspace = true zed_actions.workspace = true diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index ec18bcd16cad88..1f61a97eff64cc 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -14,7 +14,7 @@ pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; pub(crate) use feature_flags::render_feature_flags_page; pub(crate) use llm_providers_page::render_llm_providers_page; -pub(crate) use mcp_servers_page::render_mcp_servers_page; +pub(crate) use mcp_servers_page::{McpServerForm, render_mcp_servers_page}; pub(crate) use skills_setup::render_skills_setup_page; pub(crate) use tool_permissions_setup::render_tool_permissions_setup_page; diff --git a/crates/settings_ui/src/pages/llm_providers_page.rs b/crates/settings_ui/src/pages/llm_providers_page.rs index e9a5d2631e1e1d..788abadccfcbf5 100644 --- a/crates/settings_ui/src/pages/llm_providers_page.rs +++ b/crates/settings_ui/src/pages/llm_providers_page.rs @@ -22,8 +22,7 @@ pub(crate) fn render_llm_providers_page( .size_full() .pt_2p5() .px_8() - .pb_16() - .track_scroll(scroll_handle) + .pb_16() .track_scroll(scroll_handle) .overflow_y_scroll() .children( providers @@ -43,7 +42,7 @@ fn render_provider_block( cx: &mut Context, ) -> AnyElement { let provider_id = provider.id(); - let provider_name = provider.name().0.clone(); + let provider_name = provider.name().0; let disclosure_id = SharedString::from(format!("provider-disclosure-{}", provider_id.0)); let is_expanded = settings_window @@ -111,7 +110,7 @@ fn render_provider_block( h_flex() .w_full() .gap_1() - .child(Label::new(provider_name.clone())) + .child(Label::new(provider_name)) .when(is_authenticated && !is_expanded, |this| { this.child( Icon::new(IconName::Check).color(Color::Success), diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs index 759ea415c62281..ea68f183f4210a 100644 --- a/crates/settings_ui/src/pages/mcp_servers_page.rs +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -1,12 +1,14 @@ use std::sync::Arc; +use collections::HashMap; use context_server::ContextServerId; +use editor::Editor; use extension_host::ExtensionStore; -use gpui::{Action as _, Entity, ScrollHandle, prelude::*}; +use gpui::{Action as _, Entity, Focusable as _, ScrollHandle, WeakEntity, prelude::*}; use project::context_server_store::{ ContextServerConfiguration, ContextServerStatus, ContextServerStore, }; -use settings::ContextServerSettingsContent; +use settings::{ContextServerCommand, ContextServerSettingsContent, OAuthClientSettings}; use ui::{ AiSettingItem, AiSettingItemSource, AiSettingItemStatus, ContextMenu, Divider, DividerColor, PopoverMenu, Switch, ToggleState, Tooltip, prelude::*, @@ -20,7 +22,7 @@ use crate::SettingsWindow; pub(crate) fn render_mcp_servers_page( settings_window: &SettingsWindow, scroll_handle: &ScrollHandle, - _window: &mut Window, + window: &mut Window, cx: &mut Context, ) -> AnyElement { let context_server_store = get_context_server_store(settings_window, cx); @@ -37,7 +39,7 @@ pub(crate) fn render_mcp_servers_page( render_no_project_state(cx) }; - let add_server_popover = render_add_server_popover(settings_window, cx); + let add_server_popover = render_add_server_popover(settings_window, window, cx); v_flex() .id("mcp-servers-page") @@ -146,12 +148,11 @@ fn render_context_server( let is_running = matches!(server_status, ContextServerStatus::Running); let item_id = SharedString::from(context_server_id.0.to_string()); - let provided_by_extension = server_configuration.as_ref().is_none_or(|config| { - matches!( - config.as_ref(), - ContextServerConfiguration::Extension { .. } - ) - }); + // Determine the source from the configured settings rather than the runtime + // configuration: a custom (Stdio/HTTP) server that is disabled or not yet + // started has no runtime configuration, and must not be mistaken for an + // extension-provided server. + let provided_by_extension = store.read(cx).is_extension_provided(context_server_id, cx); let display_name = if provided_by_extension { resolve_extension_display_name(context_server_id, cx).unwrap_or_else(|| item_id.clone()) @@ -167,11 +168,6 @@ fn render_context_server( let status = map_server_status(&server_status); - let is_remote = server_configuration - .as_ref() - .map(|config| matches!(config.as_ref(), ContextServerConfiguration::Http { .. })) - .unwrap_or(false); - let should_show_logout = server_configuration.as_ref().is_some_and(|config| { matches!(config.as_ref(), ContextServerConfiguration::Http { .. }) && !config.has_static_auth_header() @@ -191,13 +187,19 @@ fn render_context_server( None }; - // Build gear menu + // Build gear menu. Use the editable configuration (which falls back to the + // configured settings) so "Configure Server" pre-fills correctly even when a + // custom server is disabled or has not been started this session. + let editable_configuration = store + .read(cx) + .editable_configuration_for_server(context_server_id); let gear_menu = render_gear_menu( context_server_id, store, + cx.entity().downgrade(), + editable_configuration, provided_by_extension, should_show_logout, - is_remote, ); // Build toggle switch @@ -255,9 +257,10 @@ fn resolve_extension_display_name( fn render_gear_menu( context_server_id: &ContextServerId, store: &Entity, + settings_window: WeakEntity, + configuration: Option>, provided_by_extension: bool, should_show_logout: bool, - _is_remote: bool, ) -> impl IntoElement { let context_server_id = context_server_id.clone(); let store = store.clone(); @@ -272,7 +275,8 @@ fn render_gear_menu( IconName::Settings, ) .icon_color(Color::Muted) - .icon_size(IconSize::Small), + .icon_size(IconSize::Small) + .tab_index(0isize), Tooltip::text("Configure MCP Server"), ) .anchor(gpui::Anchor::TopRight) @@ -280,9 +284,32 @@ fn render_gear_menu( move |window, cx| { let context_server_id = context_server_id.clone(); let store = store.clone(); + let settings_window = settings_window.clone(); + let configuration = configuration.clone(); Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { - menu.when(should_show_logout, |this| { + menu.when(!provided_by_extension, |this| { + this.entry("Configure Server", None, { + let settings_window = settings_window.clone(); + let context_server_id = context_server_id.clone(); + let configuration = configuration.clone(); + move |window, cx| { + let transport = match configuration.as_deref() { + Some(ContextServerConfiguration::Http { .. }) => McpTransport::Http, + _ => McpTransport::Stdio, + }; + let existing = configuration + .clone() + .map(|config| (context_server_id.clone(), config)); + settings_window + .update(cx, |this, cx| { + open_mcp_server_form(this, transport, existing, window, cx); + }) + .log_err(); + } + }) + }) + .when(should_show_logout, |this| { this.entry("Log Out", None, { let store = store.clone(); let context_server_id = context_server_id.clone(); @@ -293,7 +320,11 @@ fn render_gear_menu( } }) }) - .separator() + // Only show a divider when there is an entry above "Uninstall". + // Extension servers have neither "Configure Server" nor "Log Out". + .when(!provided_by_extension || should_show_logout, |this| { + this.separator() + }) .entry("Uninstall", None, { let context_server_id = context_server_id.clone(); move |_, cx| { @@ -321,6 +352,7 @@ fn render_toggle_switch( ToggleState::Unselected }, ) + .tab_index(0isize) .on_click({ move |state, _window, cx| { let is_enabled = match state { @@ -491,14 +523,31 @@ fn render_status_details( fn render_add_server_popover( settings_window: &SettingsWindow, - _cx: &App, + window: &mut Window, + cx: &mut Context, ) -> impl IntoElement { let original_window = settings_window.original_window; + // Stable handle so the button keeps focus state across renders and can show a + // focus ring even when the page is opened (and the button auto-focused) via a + // mouse click, where `focus_visible` styling is suppressed. + let focus_handle = settings_window + .mcp_add_server_focus_handle + .clone() + .tab_index(0) + .tab_stop(true); + let is_focused = focus_handle.is_focused(window); + let border_color = if is_focused { + cx.theme().colors().border_focused + } else { + gpui::transparent_black() + }; + let settings_window = cx.entity().downgrade(); - PopoverMenu::new("add-mcp-server-popover") + let popover = PopoverMenu::new("add-mcp-server-popover") .trigger( Button::new("add-mcp-server", "Add Server") .style(ButtonStyle::Outlined) + .track_focus(&focus_handle) .start_icon( Icon::new(IconName::Plus) .size(IconSize::Small) @@ -509,8 +558,42 @@ fn render_add_server_popover( .anchor(gpui::Anchor::TopRight) .menu({ move |window, cx| { + let settings_window = settings_window.clone(); Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { - menu.entry("Install from Extensions", None, { + menu.entry("Add Local Server", None, { + let settings_window = settings_window.clone(); + move |window, cx| { + settings_window + .update(cx, |this, cx| { + open_mcp_server_form( + this, + McpTransport::Stdio, + None, + window, + cx, + ); + }) + .log_err(); + } + }) + .entry("Add Remote Server", None, { + let settings_window = settings_window.clone(); + move |window, cx| { + settings_window + .update(cx, |this, cx| { + open_mcp_server_form( + this, + McpTransport::Http, + None, + window, + cx, + ); + }) + .log_err(); + } + }) + .separator() + .entry("Install from Extensions", None, { move |_window, cx| { if let Some(original_window) = original_window.as_ref() { cx.activate(true); @@ -534,7 +617,13 @@ fn render_add_server_popover( }) })) } - }) + }); + + div() + .rounded_md() + .border_1() + .border_color(border_color) + .child(popover) } fn uninstall_server( @@ -586,3 +675,797 @@ fn extension_only_provides_context_server(manifest: &extension::ExtensionManifes && manifest.snippets.is_none() && manifest.debug_locators.is_empty() } + +// === Custom (Stdio/HTTP) MCP server add/edit form === + +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum McpTransport { + /// Local server launched via stdin/stdout. + Stdio, + /// Remote server connected over HTTP. + Http, +} + +#[derive(Clone, Copy)] +enum McpKvKind { + Env, + Header, +} + +impl McpKvKind { + fn rows_mut(self, form: &mut McpServerForm) -> &mut Vec { + match self { + McpKvKind::Env => &mut form.env, + McpKvKind::Header => &mut form.headers, + } + } + + fn remove_id(self) -> &'static str { + match self { + McpKvKind::Env => "mcp-env-remove", + McpKvKind::Header => "mcp-header-remove", + } + } + + fn add_id(self) -> &'static str { + match self { + McpKvKind::Env => "mcp-env-add", + McpKvKind::Header => "mcp-header-add", + } + } +} + +struct KeyValueRow { + key: Entity, + value: Entity, +} + +/// Editor-backed state for the custom MCP server add/edit form. +pub(crate) struct McpServerForm { + transport: McpTransport, + /// `Some` when editing an existing server (used to remove the old entry on rename). + original_id: Option, + name: Entity, + command: Entity, + args: Entity, + url: Entity, + timeout: Entity, + oauth_client_id: Entity, + env: Vec, + headers: Vec, + error: Option, +} + +impl McpServerForm { + fn new( + transport: McpTransport, + existing: Option<(ContextServerId, Arc)>, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let original_id = existing.as_ref().map(|(id, _)| id.clone()); + let config = existing.map(|(_, config)| config); + let name_initial = original_id.as_ref().map(|id| id.0.to_string()); + + let mut command_initial = None; + let mut args_initial = None; + let mut url_initial = None; + let mut timeout_initial = None; + let mut oauth_initial = None; + let mut env = Vec::new(); + let mut headers = Vec::new(); + + if let Some(config) = config.as_deref() { + match config { + ContextServerConfiguration::Custom { command, .. } + | ContextServerConfiguration::Extension { command, .. } => { + command_initial = Some(command.path.to_string_lossy().to_string()); + if !command.args.is_empty() { + args_initial = Some(command.args.join(" ")); + } + timeout_initial = command.timeout.map(|timeout| timeout.to_string()); + if let Some(env_map) = &command.env { + for (key, value) in sorted_pairs(env_map) { + env.push(new_kv_row(Some(&key), Some(&value), window, cx)); + } + } + } + ContextServerConfiguration::Http { + url, + headers: header_map, + timeout, + oauth, + } => { + url_initial = Some(url.to_string()); + timeout_initial = timeout.map(|timeout| timeout.to_string()); + for (key, value) in sorted_pairs(header_map) { + headers.push(new_kv_row(Some(&key), Some(&value), window, cx)); + } + oauth_initial = oauth.as_ref().map(|oauth| oauth.client_id.clone()); + } + } + } + + Self { + transport, + original_id, + name: new_input("my-mcp-server", name_initial.as_deref(), window, cx), + command: new_input("/path/to/server", command_initial.as_deref(), window, cx), + args: new_input("--flag value", args_initial.as_deref(), window, cx), + url: new_input("https://example.com/mcp", url_initial.as_deref(), window, cx), + timeout: new_input("60", timeout_initial.as_deref(), window, cx), + oauth_client_id: new_input( + "Optional OAuth client ID", + oauth_initial.as_deref(), + window, + cx, + ), + env, + headers, + error: None, + } + } +} + +fn sorted_pairs(map: &HashMap) -> Vec<(String, String)> { + let mut pairs: Vec<(String, String)> = map + .iter() + .map(|(key, value)| (key.clone(), value.clone())) + .collect(); + pairs.sort_by(|a, b| a.0.cmp(&b.0)); + pairs +} + +fn new_input( + placeholder: &str, + initial: Option<&str>, + window: &mut Window, + cx: &mut Context, +) -> Entity { + let placeholder = placeholder.to_string(); + let initial = initial.map(|text| text.to_string()); + cx.new(|cx| { + let mut editor = Editor::single_line(window, cx); + editor.set_placeholder_text(placeholder.as_str(), window, cx); + if let Some(text) = initial { + editor.set_text(text, window, cx); + } + editor + }) +} + +fn new_kv_row( + key: Option<&str>, + value: Option<&str>, + window: &mut Window, + cx: &mut Context, +) -> KeyValueRow { + KeyValueRow { + key: new_input("Key", key, window, cx), + value: new_input("Value", value, window, cx), + } +} + +/// Creates the form state and pushes the form sub-page onto the stack. +pub(crate) fn open_mcp_server_form( + settings_window: &mut SettingsWindow, + transport: McpTransport, + existing: Option<(ContextServerId, Arc)>, + window: &mut Window, + cx: &mut Context, +) { + let is_edit = existing.is_some(); + settings_window.mcp_server_form = Some(McpServerForm::new(transport, existing, window, cx)); + + let title = if is_edit { + "Configure MCP Server" + } else { + match transport { + McpTransport::Stdio => "Add Local MCP Server", + McpTransport::Http => "Add Remote MCP Server", + } + }; + + settings_window.push_dynamic_sub_page( + title, + "Agent Configuration", + Some("context_servers"), + false, + render_mcp_server_form_page, + window, + cx, + ); +} + +fn render_mcp_server_form_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + _window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let Some(form) = settings_window.mcp_server_form.as_ref() else { + return div().into_any_element(); + }; + let transport = form.transport; + let error = form.error.clone(); + + let fields = v_flex() + .w_full() + .max_w(rems(36.)) + .gap_3() + .child(labeled_field("Server Name", true, &form.name, cx)) + .map(|this| match transport { + McpTransport::Stdio => this + .child(labeled_field("Command", true, &form.command, cx)) + .child(labeled_field("Arguments", false, &form.args, cx)) + .child(render_kv_section( + "Environment Variables", + &form.env, + McpKvKind::Env, + cx, + )) + .child(labeled_field("Timeout (seconds)", false, &form.timeout, cx)), + McpTransport::Http => this + .child(labeled_field("URL", true, &form.url, cx)) + .child(render_kv_section( + "Headers", + &form.headers, + McpKvKind::Header, + cx, + )) + .child(labeled_field("Timeout (seconds)", false, &form.timeout, cx)) + .child(labeled_field( + "OAuth Client ID", + false, + &form.oauth_client_id, + cx, + )), + }) + .when_some(error, |this, error| this.child(render_form_error(error))) + .child(render_form_actions(cx)); + + v_flex() + .id("mcp-server-form-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child(fields) + .into_any_element() +} + +fn field_label(label: &str, required: bool) -> impl IntoElement { + h_flex() + .gap_0p5() + .child( + Label::new(label.to_string()) + .size(LabelSize::Small) + .color(Color::Muted), + ) + .when(required, |this| { + this.child(Label::new("*").size(LabelSize::Small).color(Color::Error)) + }) +} + +fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { + let colors = cx.theme().colors(); + // All form inputs share tab index 0, so tab order follows render (insertion) + // order. Tracking the editor's focus handle makes the field a tab stop and + // routes keyboard focus into the editor when tabbed to. + let focus_handle = editor.focus_handle(cx).tab_index(0).tab_stop(true); + h_flex() + .w_full() + .min_w_0() + .py_1() + .px_2() + .h_8() + .rounded_md() + .border_1() + .border_color(colors.border) + .bg(colors.editor_background) + .track_focus(&focus_handle) + .focus(|style| style.border_color(colors.border_focused)) + .child(editor.clone()) +} + +fn labeled_field( + label: &str, + required: bool, + editor: &Entity, + cx: &App, +) -> impl IntoElement { + v_flex() + .w_full() + .gap_1() + .child(field_label(label, required)) + .child(input_box(editor, cx)) +} + +fn render_kv_section( + label: &str, + rows: &[KeyValueRow], + kind: McpKvKind, + cx: &mut Context, +) -> impl IntoElement { + v_flex() + .w_full() + .gap_1() + .child(field_label(label, false)) + .children(rows.iter().enumerate().map(|(ix, row)| { + h_flex() + .w_full() + .gap_1() + .items_center() + .child(div().flex_1().min_w_0().child(input_box(&row.key, cx))) + .child(div().flex_1().min_w_0().child(input_box(&row.value, cx))) + .child( + IconButton::new((kind.remove_id(), ix), IconName::Close) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tooltip(Tooltip::text("Remove")) + .on_click(cx.listener(move |this, _, _window, cx| { + if let Some(form) = this.mcp_server_form.as_mut() { + let rows = kind.rows_mut(form); + if ix < rows.len() { + rows.remove(ix); + } + } + cx.notify(); + })), + ) + })) + .child( + Button::new(kind.add_id(), "Add") + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .start_icon( + Icon::new(IconName::Plus) + .size(IconSize::Small) + .color(Color::Muted), + ) + .on_click(cx.listener(move |this, _, window, cx| { + let row = new_kv_row(None, None, window, cx); + if let Some(form) = this.mcp_server_form.as_mut() { + kind.rows_mut(form).push(row); + } + cx.notify(); + })), + ) +} + +fn render_form_error(error: SharedString) -> impl IntoElement { + h_flex() + .w_full() + .gap_2() + .items_start() + .child( + Icon::new(IconName::XCircle) + .size(IconSize::Small) + .color(Color::Error), + ) + .child( + Label::new(error) + .size(LabelSize::Small) + .color(Color::Error), + ) +} + +fn render_form_actions(cx: &mut Context) -> impl IntoElement { + h_flex() + .w_full() + .gap_2() + .justify_end() + .pt_2() + .child( + Button::new("mcp-form-cancel", "Cancel") + .style(ButtonStyle::Subtle) + .on_click(cx.listener(|this, _, window, cx| { + this.mcp_server_form = None; + this.pop_sub_page(window, cx); + })), + ) + .child( + Button::new("mcp-form-save", "Save") + .style(ButtonStyle::Filled) + .on_click(cx.listener(|this, _, window, cx| { + save_mcp_server_form(this, window, cx); + })), + ) +} + +fn save_mcp_server_form( + settings_window: &mut SettingsWindow, + window: &mut Window, + cx: &mut Context, +) { + let built = { + let Some(form) = settings_window.mcp_server_form.as_ref() else { + return; + }; + build_settings_from_form(form, cx) + }; + + let (id, original_id, content) = match built { + Ok(value) => value, + Err(error) => { + if let Some(form) = settings_window.mcp_server_form.as_mut() { + form.error = Some(error); + } + cx.notify(); + return; + } + }; + + // Reject names that would collide with a *different* existing server. This + // covers both adding a new server and renaming an existing one (where the new + // name must not clobber another server's configuration). + let collides_with_other_server = + get_context_server_store(settings_window, cx).is_some_and(|store| { + name_collides_with_other_server(&id, original_id.as_ref(), store.read(cx).server_ids()) + }); + if collides_with_other_server { + if let Some(form) = settings_window.mcp_server_form.as_mut() { + form.error = Some(format!("A server named \"{}\" already exists.", id.0).into()); + } + cx.notify(); + return; + } + + let fs = ::global(cx); + settings::update_settings_file(fs, cx, move |settings, _| { + if let Some(original_id) = &original_id + && original_id.0 != id.0 + { + settings.project.context_servers.remove(&original_id.0); + } + settings.project.context_servers.insert(id.0.clone(), content); + }); + + settings_window.mcp_server_form = None; + settings_window.pop_sub_page(window, cx); +} + +/// Plain (editor-free) snapshot of the form's contents, so the validation / +/// build logic can be exercised without a GPUI context. +struct McpServerFormValues { + transport: McpTransport, + original_id: Option, + name: String, + command: String, + args: String, + url: String, + timeout: String, + oauth_client_id: String, + env: Vec<(String, String)>, + headers: Vec<(String, String)>, +} + +fn build_settings_from_form( + form: &McpServerForm, + cx: &App, +) -> Result<(ContextServerId, Option, ContextServerSettingsContent), SharedString> { + let values = McpServerFormValues { + transport: form.transport, + original_id: form.original_id.clone(), + name: form.name.read(cx).text(cx), + command: form.command.read(cx).text(cx), + args: form.args.read(cx).text(cx), + url: form.url.read(cx).text(cx), + timeout: form.timeout.read(cx).text(cx), + oauth_client_id: form.oauth_client_id.read(cx).text(cx), + env: read_kv(&form.env, cx), + headers: read_kv(&form.headers, cx), + }; + build_settings_from_values(&values) +} + +fn read_kv(rows: &[KeyValueRow], cx: &App) -> Vec<(String, String)> { + rows.iter() + .map(|row| (row.key.read(cx).text(cx), row.value.read(cx).text(cx))) + .collect() +} + +fn build_settings_from_values( + values: &McpServerFormValues, +) -> Result<(ContextServerId, Option, ContextServerSettingsContent), SharedString> { + let name = values.name.trim().to_string(); + if name.is_empty() { + return Err("Server name is required.".into()); + } + + let timeout = parse_timeout(&values.timeout)?; + + let content = match values.transport { + McpTransport::Stdio => { + let command = values.command.trim().to_string(); + if command.is_empty() { + return Err("Command is required.".into()); + } + let args = values + .args + .split_whitespace() + .map(|arg| arg.to_string()) + .collect::>(); + let env = collect_kv(&values.env, "environment variable")?; + ContextServerSettingsContent::Stdio { + enabled: true, + remote: false, + command: ContextServerCommand { + path: command.into(), + args, + env: (!env.is_empty()).then_some(env), + timeout, + }, + } + } + McpTransport::Http => { + let url = values.url.trim().to_string(); + if url.is_empty() { + return Err("URL is required.".into()); + } + // Validate the URL on save (a deliberate action) rather than on every + // render, so a clearly invalid URL is reported to the user instead of + // being silently written and failing later when the server starts. + if let Err(error) = url::Url::parse(&url) { + return Err(format!("Invalid URL: {error}").into()); + } + let headers = collect_kv(&values.headers, "header")?; + let oauth_client_id = values.oauth_client_id.trim().to_string(); + let oauth = (!oauth_client_id.is_empty()).then(|| OAuthClientSettings { + client_id: oauth_client_id, + client_secret: None, + }); + ContextServerSettingsContent::Http { + enabled: true, + url, + headers, + timeout, + oauth, + } + } + }; + + Ok((ContextServerId(name.into()), values.original_id.clone(), content)) +} + +/// Returns whether saving under `id` would overwrite a *different* existing +/// server. Editing a server in place (`id == original_id`) is allowed. +fn name_collides_with_other_server( + id: &ContextServerId, + original_id: Option<&ContextServerId>, + existing_ids: &[ContextServerId], +) -> bool { + original_id.is_none_or(|original| original.0 != id.0) + && existing_ids.iter().any(|existing| existing.0 == id.0) +} + +fn parse_timeout(text: &str) -> Result, SharedString> { + let text = text.trim(); + if text.is_empty() { + return Ok(None); + } + text.parse::() + .map(Some) + .map_err(|_| "Timeout must be a positive whole number of seconds.".into()) +} + +fn collect_kv( + rows: &[(String, String)], + label: &str, +) -> Result, SharedString> { + let mut map = HashMap::default(); + for (key, value) in rows { + let key = key.trim().to_string(); + if key.is_empty() { + continue; + } + if map.contains_key(&key) { + return Err(format!("Duplicate {label} \"{key}\".").into()); + } + map.insert(key, value.clone()); + } + Ok(map) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn values(transport: McpTransport) -> McpServerFormValues { + McpServerFormValues { + transport, + original_id: None, + name: "my-server".into(), + command: String::new(), + args: String::new(), + url: String::new(), + timeout: String::new(), + oauth_client_id: String::new(), + env: Vec::new(), + headers: Vec::new(), + } + } + + fn id(name: &str) -> ContextServerId { + ContextServerId(name.into()) + } + + #[test] + fn parse_timeout_handles_empty_and_invalid() { + assert_eq!(parse_timeout(""), Ok(None)); + assert_eq!(parse_timeout(" "), Ok(None)); + assert_eq!(parse_timeout("60"), Ok(Some(60))); + assert_eq!(parse_timeout(" 90 "), Ok(Some(90))); + assert!(parse_timeout("abc").is_err()); + assert!(parse_timeout("-5").is_err()); + assert!(parse_timeout("1.5").is_err()); + } + + #[test] + fn requires_server_name() { + let mut values = values(McpTransport::Stdio); + values.name = " ".into(); + values.command = "/bin/server".into(); + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "Server name is required." + ); + } + + #[test] + fn requires_command_for_local_server() { + let values = values(McpTransport::Stdio); + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "Command is required." + ); + } + + #[test] + fn requires_url_for_remote_server() { + let values = values(McpTransport::Http); + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "URL is required." + ); + } + + #[test] + fn rejects_invalid_url() { + let mut values = values(McpTransport::Http); + values.url = "not a url".into(); + let error = build_settings_from_values(&values).unwrap_err(); + assert!(error.starts_with("Invalid URL"), "unexpected error: {error}"); + } + + #[test] + fn rejects_invalid_timeout() { + let mut values = values(McpTransport::Stdio); + values.command = "/bin/server".into(); + values.timeout = "soon".into(); + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "Timeout must be a positive whole number of seconds." + ); + } + + #[test] + fn rejects_duplicate_environment_variables() { + let mut values = values(McpTransport::Stdio); + values.command = "/bin/server".into(); + values.env = vec![("FOO".into(), "1".into()), ("FOO".into(), "2".into())]; + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "Duplicate environment variable \"FOO\"." + ); + } + + #[test] + fn rejects_duplicate_headers() { + let mut values = values(McpTransport::Http); + values.url = "https://example.com/mcp".into(); + values.headers = vec![ + ("Authorization".into(), "a".into()), + ("Authorization".into(), "b".into()), + ]; + assert_eq!( + build_settings_from_values(&values).unwrap_err().as_ref(), + "Duplicate header \"Authorization\"." + ); + } + + #[test] + fn builds_local_server() { + let mut values = values(McpTransport::Stdio); + values.name = " local ".into(); + values.command = "/usr/bin/server".into(); + values.args = "--flag value".into(); + values.timeout = "30".into(); + // Empty values are kept, but rows with a blank key are ignored. + values.env = vec![ + ("KEY".into(), "VALUE".into()), + ("EMPTY".into(), String::new()), + (" ".into(), "ignored".into()), + ]; + + let (id, original_id, content) = build_settings_from_values(&values).unwrap(); + assert_eq!(id.0.as_ref(), "local"); + assert_eq!(original_id, None); + + let expected_env = HashMap::from_iter([ + ("KEY".to_string(), "VALUE".to_string()), + ("EMPTY".to_string(), String::new()), + ]); + assert_eq!( + content, + ContextServerSettingsContent::Stdio { + enabled: true, + remote: false, + command: ContextServerCommand { + path: "/usr/bin/server".into(), + args: vec!["--flag".into(), "value".into()], + env: Some(expected_env), + timeout: Some(30), + }, + } + ); + } + + #[test] + fn builds_remote_server() { + let mut values = values(McpTransport::Http); + values.name = "remote".into(); + values.url = "https://example.com/mcp".into(); + values.oauth_client_id = "client-123".into(); + values.headers = vec![("Authorization".into(), "Bearer token".into())]; + + let (id, _, content) = build_settings_from_values(&values).unwrap(); + assert_eq!(id.0.as_ref(), "remote"); + + let expected_headers = + HashMap::from_iter([("Authorization".to_string(), "Bearer token".to_string())]); + assert_eq!( + content, + ContextServerSettingsContent::Http { + enabled: true, + url: "https://example.com/mcp".into(), + headers: expected_headers, + timeout: None, + oauth: Some(OAuthClientSettings { + client_id: "client-123".into(), + client_secret: None, + }), + } + ); + } + + #[test] + fn name_collision_covers_new_and_rename() { + let existing = vec![id("foo"), id("bar")]; + + // New server taking an existing name collides. + assert!(name_collides_with_other_server(&id("foo"), None, &existing)); + // New server with a free name is fine. + assert!(!name_collides_with_other_server(&id("baz"), None, &existing)); + // Editing a server in place is allowed even though the name "exists". + assert!(!name_collides_with_other_server( + &id("foo"), + Some(&id("foo")), + &existing + )); + // Renaming onto a different server's name collides. + assert!(name_collides_with_other_server( + &id("bar"), + Some(&id("foo")), + &existing + )); + // Renaming to a free name is fine. + assert!(!name_collides_with_other_server( + &id("baz"), + Some(&id("foo")), + &existing + )); + } +} diff --git a/crates/settings_ui/src/pages/tool_permissions_setup.rs b/crates/settings_ui/src/pages/tool_permissions_setup.rs index 8f010bc6b0565f..1aa78d74b80c58 100644 --- a/crates/settings_ui/src/pages/tool_permissions_setup.rs +++ b/crates/settings_ui/src/pages/tool_permissions_setup.rs @@ -288,6 +288,7 @@ fn render_tool_list_item( tool_name, "Tool Permissions", None, + true, render_fn, window, cx, diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 58b6455160dbc8..e7ee6e9dfc2c6f 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -54,7 +54,9 @@ use crate::components::{ SettingsSectionHeader, font_picker, icon_theme_picker, render_ollama_model_picker, theme_picker, }; -use crate::pages::{render_input_audio_device_dropdown, render_output_audio_device_dropdown}; +use crate::pages::{ + McpServerForm, render_input_audio_device_dropdown, render_output_audio_device_dropdown, +}; const NAVBAR_CONTAINER_TAB_INDEX: isize = 0; const NAVBAR_GROUP_TAB_INDEX: isize = 1; @@ -780,6 +782,12 @@ pub struct SettingsWindow { /// Directory path of the skill whose share link was most recently copied, /// used to show a transient "copied" checkmark on its share button. pub(crate) last_copied_skill_directory_path: Option, + /// State for the active "add/edit custom MCP server" form sub-page, if open. + pub(crate) mcp_server_form: Option, + /// Stable focus handle for the MCP "Add Server" button, so it can show a + /// focus ring when the page auto-focuses it on open (which happens via mouse, + /// where `focus_visible` styling would otherwise be suppressed). + pub(crate) mcp_add_server_focus_handle: FocusHandle, } struct SearchDocument { @@ -1812,6 +1820,8 @@ impl SettingsWindow { expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), last_copied_skill_directory_path: None, + mcp_server_form: None, + mcp_add_server_focus_handle: cx.focus_handle(), }; this.fetch_files(window, cx); @@ -3576,7 +3586,16 @@ impl SettingsWindow { .id("settings-ui-page") .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| { if !this.sub_page_stack.is_empty() { + // Keep Tab navigation within the sub-page content. Global + // `focus_next` would otherwise wrap past the last control to + // the navbar; instead, when focus leaves the content region we + // wrap back to the first content tab stop. + let content_handle = this.content_focus_handle.focus_handle(cx); window.focus_next(cx); + if !content_handle.contains_focused(window, cx) { + content_handle.focus(window, cx); + window.focus_next(cx); + } return; } for (logical_index, (actual_index, _)) in this.visible_page_items().enumerate() { @@ -3831,6 +3850,7 @@ impl SettingsWindow { title: impl Into, section_header: impl Into, json_path: Option<&'static str>, + in_json: bool, render: fn( &SettingsWindow, &ScrollHandle, @@ -3846,7 +3866,7 @@ impl SettingsWindow { r#type: SubPageType::default(), description: None, json_path, - in_json: true, + in_json, files: USER, render, }; @@ -3924,7 +3944,7 @@ impl SettingsWindow { false } - fn pop_sub_page(&mut self, window: &mut Window, cx: &mut Context) { + pub(crate) fn pop_sub_page(&mut self, window: &mut Window, cx: &mut Context) { self.regex_validation_error = None; self.sub_page_stack.pop(); self.content_focus_handle.focus_handle(cx).focus(window, cx); @@ -4706,6 +4726,8 @@ pub mod test { expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), last_copied_skill_directory_path: None, + mcp_server_form: None, + mcp_add_server_focus_handle: cx.focus_handle(), } } } @@ -4836,6 +4858,8 @@ pub mod test { expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), last_copied_skill_directory_path: None, + mcp_server_form: None, + mcp_add_server_focus_handle: cx.focus_handle(), }; settings_window.build_filter_table(); From e5e97eb82026dd814702f3969e127da04e177d2f Mon Sep 17 00:00:00 2001 From: cameron Date: Mon, 8 Jun 2026 13:06:43 +0100 Subject: [PATCH 08/16] gracefully handle invalid settings values in json --- crates/project/src/context_server_store.rs | 46 --------- .../settings_ui/src/pages/mcp_servers_page.rs | 94 +++++++++++++------ 2 files changed, 64 insertions(+), 76 deletions(-) diff --git a/crates/project/src/context_server_store.rs b/crates/project/src/context_server_store.rs index 386d04f5049923..4da04e08234074 100644 --- a/crates/project/src/context_server_store.rs +++ b/crates/project/src/context_server_store.rs @@ -562,52 +562,6 @@ impl ContextServerStore { } } - /// Returns a configuration suitable for pre-filling the "edit custom server" - /// UI. Prefers the live runtime configuration, but falls back to building one - /// synchronously from the configured settings when the server has no runtime - /// state (e.g. it is disabled or has not been started this session). - /// - /// Returns `None` for extension-provided servers, which are not edited via - /// the custom-server form, and for HTTP servers whose configured URL cannot - /// be parsed. - pub fn editable_configuration_for_server( - &self, - id: &ContextServerId, - ) -> Option> { - if let Some(configuration) = self.configuration_for_server(id) { - return Some(configuration); - } - - match self.context_server_settings.get(&id.0)? { - ContextServerSettings::Stdio { - command, remote, .. - } => Some(Arc::new(ContextServerConfiguration::Custom { - command: command.clone(), - remote: *remote, - })), - ContextServerSettings::Http { - url, - headers, - timeout, - oauth, - .. - } => { - // Parse failures are expected here (e.g. an invalid URL that the - // user is still editing). This runs on every render, so fail - // silently rather than logging; the invalid URL is surfaced via - // the server's error status instead. - let url = url::Url::parse(url).ok()?; - Some(Arc::new(ContextServerConfiguration::Http { - url, - headers: headers.clone(), - timeout: *timeout, - oauth: oauth.clone(), - })) - } - ContextServerSettings::Extension { .. } => None, - } - } - /// Returns a sorted slice of available unique context server IDs. Within the /// slice, context servers which have `mcp-server-` as a prefix in their ID will /// appear after servers that do not have this prefix in their ID. diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs index ea68f183f4210a..21f668b09c9adf 100644 --- a/crates/settings_ui/src/pages/mcp_servers_page.rs +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -8,6 +8,7 @@ use gpui::{Action as _, Entity, Focusable as _, ScrollHandle, WeakEntity, prelud use project::context_server_store::{ ContextServerConfiguration, ContextServerStatus, ContextServerStore, }; +use project::project_settings::ContextServerSettings; use settings::{ContextServerCommand, ContextServerSettingsContent, OAuthClientSettings}; use ui::{ AiSettingItem, AiSettingItemSource, AiSettingItemStatus, ContextMenu, Divider, DividerColor, @@ -187,17 +188,16 @@ fn render_context_server( None }; - // Build gear menu. Use the editable configuration (which falls back to the - // configured settings) so "Configure Server" pre-fills correctly even when a - // custom server is disabled or has not been started this session. - let editable_configuration = store - .read(cx) - .editable_configuration_for_server(context_server_id); + // Build gear menu. Pre-fill "Configure Server" from the raw configured + // settings (not the resolved runtime configuration) so the form is editable + // even when the settings contain invalid data (e.g. an unparseable URL) or + // the server is disabled / not yet started. + let server_settings = store.read(cx).settings_for_server(context_server_id).cloned(); let gear_menu = render_gear_menu( context_server_id, store, cx.entity().downgrade(), - editable_configuration, + server_settings.clone(), provided_by_extension, should_show_logout, ); @@ -205,13 +205,12 @@ fn render_context_server( // Build toggle switch let toggle_switch = render_toggle_switch(context_server_id, store, is_running); - // Build details (error/auth feedback) - let details = render_status_details( - &server_status, - context_server_id, - store, - should_show_logout, - ); + // Surface invalid settings (which prevent the server from starting at all) + // ahead of runtime status feedback, so the misconfiguration is visible. + let details = match settings_validation_error(server_settings.as_ref()) { + Some(error) => Some(render_form_error(error).into_any_element()), + None => render_status_details(&server_status, context_server_id, store, should_show_logout), + }; AiSettingItem::new(item_id, display_name, status, source) .action(gear_menu) @@ -258,7 +257,7 @@ fn render_gear_menu( context_server_id: &ContextServerId, store: &Entity, settings_window: WeakEntity, - configuration: Option>, + server_settings: Option, provided_by_extension: bool, should_show_logout: bool, ) -> impl IntoElement { @@ -285,22 +284,22 @@ fn render_gear_menu( let context_server_id = context_server_id.clone(); let store = store.clone(); let settings_window = settings_window.clone(); - let configuration = configuration.clone(); + let server_settings = server_settings.clone(); Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { menu.when(!provided_by_extension, |this| { this.entry("Configure Server", None, { let settings_window = settings_window.clone(); let context_server_id = context_server_id.clone(); - let configuration = configuration.clone(); + let server_settings = server_settings.clone(); move |window, cx| { - let transport = match configuration.as_deref() { - Some(ContextServerConfiguration::Http { .. }) => McpTransport::Http, + let transport = match &server_settings { + Some(ContextServerSettings::Http { .. }) => McpTransport::Http, _ => McpTransport::Stdio, }; - let existing = configuration + let existing = server_settings .clone() - .map(|config| (context_server_id.clone(), config)); + .map(|settings| (context_server_id.clone(), settings)); settings_window .update(cx, |this, cx| { open_mcp_server_form(this, transport, existing, window, cx); @@ -739,12 +738,12 @@ pub(crate) struct McpServerForm { impl McpServerForm { fn new( transport: McpTransport, - existing: Option<(ContextServerId, Arc)>, + existing: Option<(ContextServerId, ContextServerSettings)>, window: &mut Window, cx: &mut Context, ) -> Self { let original_id = existing.as_ref().map(|(id, _)| id.clone()); - let config = existing.map(|(_, config)| config); + let settings = existing.map(|(_, settings)| settings); let name_initial = original_id.as_ref().map(|id| id.0.to_string()); let mut command_initial = None; @@ -755,10 +754,12 @@ impl McpServerForm { let mut env = Vec::new(); let mut headers = Vec::new(); - if let Some(config) = config.as_deref() { - match config { - ContextServerConfiguration::Custom { command, .. } - | ContextServerConfiguration::Extension { command, .. } => { + // Pre-fill from the raw settings so invalid values (e.g. a malformed URL + // the user typed directly into settings.json) still load into the form + // for correction, rather than being dropped during resolution. + if let Some(settings) = settings.as_ref() { + match settings { + ContextServerSettings::Stdio { command, .. } => { command_initial = Some(command.path.to_string_lossy().to_string()); if !command.args.is_empty() { args_initial = Some(command.args.join(" ")); @@ -770,19 +771,21 @@ impl McpServerForm { } } } - ContextServerConfiguration::Http { + ContextServerSettings::Http { url, headers: header_map, timeout, oauth, + .. } => { - url_initial = Some(url.to_string()); + url_initial = Some(url.clone()); timeout_initial = timeout.map(|timeout| timeout.to_string()); for (key, value) in sorted_pairs(header_map) { headers.push(new_kv_row(Some(&key), Some(&value), window, cx)); } oauth_initial = oauth.as_ref().map(|oauth| oauth.client_id.clone()); } + ContextServerSettings::Extension { .. } => {} } } @@ -850,7 +853,7 @@ fn new_kv_row( pub(crate) fn open_mcp_server_form( settings_window: &mut SettingsWindow, transport: McpTransport, - existing: Option<(ContextServerId, Arc)>, + existing: Option<(ContextServerId, ContextServerSettings)>, window: &mut Window, cx: &mut Context, ) { @@ -1230,6 +1233,18 @@ fn build_settings_from_values( Ok((ContextServerId(name.into()), values.original_id.clone(), content)) } +/// Returns a human-readable error when a server's configured settings are +/// invalid in a way that prevents it from starting (currently: an HTTP server +/// whose URL cannot be parsed). Used to surface misconfiguration in the list. +fn settings_validation_error(settings: Option<&ContextServerSettings>) -> Option { + match settings? { + ContextServerSettings::Http { url, .. } if url::Url::parse(url).is_err() => { + Some("Invalid URL in settings.".into()) + } + _ => None, + } +} + /// Returns whether saving under `id` would overwrite a *different* existing /// server. Editing a server in place (`id == original_id`) is allowed. fn name_collides_with_other_server( @@ -1441,6 +1456,25 @@ mod tests { ); } + #[test] + fn flags_invalid_url_in_settings() { + let http = |url: &str| ContextServerSettings::Http { + enabled: true, + url: url.into(), + headers: HashMap::default(), + timeout: None, + oauth: None, + }; + assert_eq!( + settings_validation_error(Some(&http("not a url"))) + .unwrap() + .as_ref(), + "Invalid URL in settings." + ); + assert!(settings_validation_error(Some(&http("https://example.com/mcp"))).is_none()); + assert!(settings_validation_error(None).is_none()); + } + #[test] fn name_collision_covers_new_and_rename() { let existing = vec![id("foo"), id("bar")]; From 6fc1ffdff602f0996e5eab2af2844bff99077ef4 Mon Sep 17 00:00:00 2001 From: cameron Date: Mon, 8 Jun 2026 14:27:20 +0100 Subject: [PATCH 09/16] feature flag and external agents --- crates/agent_ui/src/agent_panel.rs | 18 +- crates/feature_flags/src/flags.rs | 16 + crates/settings_ui/src/page_data.rs | 47 +- crates/settings_ui/src/pages.rs | 2 + .../src/pages/external_agents_page.rs | 450 ++++++++++++++++++ 5 files changed, 522 insertions(+), 11 deletions(-) create mode 100644 crates/settings_ui/src/pages/external_agents_page.rs diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index ca6ac97b384fdf..4f354e2d4bfcf7 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -70,7 +70,9 @@ use cloud_api_types::Plan; use collections::HashMap; use editor::{Editor, MultiBuffer}; use extension_host::ExtensionStore; -use feature_flags::{CreateThreadToolFeatureFlag, FeatureFlagAppExt as _}; +use feature_flags::{ + AgentSettingsUiFeatureFlag, CreateThreadToolFeatureFlag, FeatureFlagAppExt as _, +}; use fs::Fs; use gpui::{ @@ -3625,6 +3627,20 @@ impl AgentPanel { } pub(crate) fn open_configuration(&mut self, window: &mut Window, cx: &mut Context) { + // When the agent settings have been moved into the settings UI, the + // panel no longer shows its own configuration overlay. Instead, route to + // the settings UI at the LLM providers page (where the model selector's + // "Configure" button expects to land). + if cx.has_flag::() { + window.dispatch_action( + Box::new(zed_actions::OpenSettingsAt { + path: "llm_providers".to_string(), + }), + cx, + ); + return; + } + if matches!(self.overlay_view, Some(OverlayView::Configuration)) { self.clear_overlay(true, window, cx); return; diff --git a/crates/feature_flags/src/flags.rs b/crates/feature_flags/src/flags.rs index 36e363ca40cec0..69e6cd108ca544 100644 --- a/crates/feature_flags/src/flags.rs +++ b/crates/feature_flags/src/flags.rs @@ -155,6 +155,22 @@ impl FeatureFlag for AgentThreadWorktreeLabelFlag { } register_feature_flag!(AgentThreadWorktreeLabelFlag); +/// Moves LLM provider and MCP server configuration out of the dedicated agent +/// panel page and into the settings UI. When enabled, the agent panel no longer +/// shows its configuration overlay and the settings UI exposes the "LLM +/// Providers" and "MCP Servers" sub-pages instead. +pub struct AgentSettingsUiFeatureFlag; + +impl FeatureFlag for AgentSettingsUiFeatureFlag { + const NAME: &'static str = "agent-settings-ui"; + type Value = PresenceFlag; + + fn enabled_for_staff() -> bool { + false + } +} +register_feature_flag!(AgentSettingsUiFeatureFlag); + pub struct AutoWatchFeatureFlag; impl FeatureFlag for AutoWatchFeatureFlag { diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index c207bd1c929723..78c75e02643d5d 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -13,7 +13,7 @@ use crate::{ ActionLink, DynamicItem, PROJECT, SettingField, SettingItem, SettingsFieldMetadata, SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names, pages::{ - open_audio_test_window, render_edit_prediction_setup_page, + open_audio_test_window, render_edit_prediction_setup_page, render_external_agents_page, render_llm_providers_page, render_mcp_servers_page, render_skills_setup_page, render_tool_permissions_setup_page, }, @@ -7499,10 +7499,17 @@ fn ai_page(cx: &App) -> SettingsPage { ] } - fn agent_configuration_section(_cx: &App) -> Box<[SettingsPageItem]> { - let mut items = vec![ - SettingsPageItem::SectionHeader("Agent Configuration"), - SettingsPageItem::SubPageLink(SubPageLink { + fn agent_configuration_section(cx: &App) -> Box<[SettingsPageItem]> { + use feature_flags::FeatureFlagAppExt as _; + + // The LLM provider and MCP server pages are gated behind a feature flag + // while their configuration is being moved out of the agent panel. + let agent_settings_ui_enabled = cx.has_flag::(); + + let mut items = vec![SettingsPageItem::SectionHeader("Agent Configuration")]; + + if agent_settings_ui_enabled { + items.push(SettingsPageItem::SubPageLink(SubPageLink { title: "LLM Providers".into(), r#type: Default::default(), json_path: Some("llm_providers"), @@ -7510,7 +7517,10 @@ fn ai_page(cx: &App) -> SettingsPage { in_json: false, files: USER, render: render_llm_providers_page, - }), + })); + } + + items.extend([ SettingsPageItem::SubPageLink(SubPageLink { title: "Skills".into(), r#type: Default::default(), @@ -7529,16 +7539,33 @@ fn ai_page(cx: &App) -> SettingsPage { files: USER, render: render_tool_permissions_setup_page, }), - SettingsPageItem::SubPageLink(SubPageLink { + ]); + + if agent_settings_ui_enabled { + items.push(SettingsPageItem::SubPageLink(SubPageLink { title: "MCP Servers".into(), r#type: Default::default(), json_path: Some("context_servers"), - description: Some("View, add, configure, and remove Model Context Protocol servers.".into()), + description: Some( + "View, add, configure, and remove Model Context Protocol servers.".into(), + ), in_json: false, files: USER, render: render_mcp_servers_page, - }), - ]; + })); + items.push(SettingsPageItem::SubPageLink(SubPageLink { + title: "External Agents".into(), + r#type: Default::default(), + json_path: Some("agent_servers"), + description: Some( + "View, add, and remove agents connected through the Agent Client Protocol." + .into(), + ), + in_json: false, + files: USER, + render: render_external_agents_page, + })); + } items.extend([ SettingsPageItem::SettingItem(SettingItem { diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 1f61a97eff64cc..3fda42713900ce 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -1,6 +1,7 @@ mod audio_input_output_setup; mod audio_test_window; mod edit_prediction_provider_setup; +mod external_agents_page; mod feature_flags; mod llm_providers_page; mod mcp_servers_page; @@ -12,6 +13,7 @@ pub(crate) use audio_input_output_setup::{ }; pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; +pub(crate) use external_agents_page::render_external_agents_page; pub(crate) use feature_flags::render_feature_flags_page; pub(crate) use llm_providers_page::render_llm_providers_page; pub(crate) use mcp_servers_page::{McpServerForm, render_mcp_servers_page}; diff --git a/crates/settings_ui/src/pages/external_agents_page.rs b/crates/settings_ui/src/pages/external_agents_page.rs new file mode 100644 index 00000000000000..f37b1cbc01834c --- /dev/null +++ b/crates/settings_ui/src/pages/external_agents_page.rs @@ -0,0 +1,450 @@ +use std::ops::Range; + +use anyhow::Result; +use collections::HashMap; +use editor::{Editor, MultiBufferOffset, SelectionEffects, scroll::Autoscroll}; +use gpui::{AsyncWindowContext, Entity, ScrollHandle, WeakEntity, WindowHandle, prelude::*}; +use itertools::Itertools as _; +use project::agent_server_store::{AgentId, AgentServerStore, ExternalAgentSource}; +use settings::{SettingsStore, update_settings_file}; +use ui::{ + AiSettingItem, AiSettingItemSource, AiSettingItemStatus, ContextMenu, ContextMenuEntry, + Divider, DividerColor, PopoverMenu, Tooltip, prelude::*, +}; +use util::ResultExt as _; +use workspace::{MultiWorkspace, Workspace, create_and_open_local_file}; +use zed_actions::OpenBrowser; + +use crate::SettingsWindow; + +pub(crate) fn render_external_agents_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let agent_server_store = get_agent_server_store(settings_window, cx); + + let agent_list = if let Some(store) = agent_server_store.as_ref() { + let agents = collect_agents(store, cx); + if agents.is_empty() { + render_empty_state(cx) + } else { + render_agent_list(agents, cx) + } + } else { + render_no_project_state(cx) + }; + + let add_agent_popover = render_add_agent_popover(settings_window, window, cx); + + v_flex() + .id("external-agents-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child( + h_flex() + .w_full() + .justify_between() + .items_center() + .mb_4() + .child( + v_flex() + .child(Label::new("External Agents").size(LabelSize::Large)) + .child( + Label::new("All agents connected through the Agent Client Protocol.") + .size(LabelSize::Small) + .color(Color::Muted), + ), + ) + .child(add_agent_popover), + ) + .child(agent_list) + .into_any_element() +} + +fn get_agent_server_store( + settings_window: &SettingsWindow, + cx: &App, +) -> Option> { + let original_window = settings_window.original_window.as_ref()?; + let multi_workspace = original_window.read(cx).ok()?; + let workspace = multi_workspace.workspaces().next()?; + let project = workspace.read(cx).project().clone(); + Some(project.read(cx).agent_server_store().clone()) +} + +/// An external agent listed on the page, paired with the data needed to render +/// its row: the optional extension-provided icon path, a human-readable name, +/// and where the agent came from. +type AgentRow = ( + AgentId, + Option, + SharedString, + ExternalAgentSource, +); + +fn collect_agents(store: &Entity, cx: &App) -> Vec { + let store = store.read(cx); + store + .external_agents() + .cloned() + .collect::>() + .into_iter() + .map(|name| { + let icon = store.agent_icon(&name); + let display_name = store + .agent_display_name(&name) + .unwrap_or_else(|| name.0.clone()); + let source = store.agent_source(&name).unwrap_or_default(); + (name, icon, display_name, source) + }) + .sorted_unstable_by_key(|(_, _, display_name, _)| display_name.to_lowercase()) + .collect() +} + +fn render_empty_state(cx: &App) -> AnyElement { + h_flex() + .p_4() + .justify_center() + .border_1() + .border_dashed() + .border_color(cx.theme().colors().border.opacity(0.6)) + .rounded_sm() + .child( + Label::new("No external agents added yet. Click \"Add Agent\" to get started.") + .color(Color::Muted) + .size(LabelSize::Small), + ) + .into_any_element() +} + +fn render_no_project_state(cx: &App) -> AnyElement { + h_flex() + .p_4() + .justify_center() + .border_1() + .border_dashed() + .border_color(cx.theme().colors().border.opacity(0.6)) + .rounded_sm() + .child( + Label::new("No active project found. Open a workspace to manage external agents.") + .color(Color::Muted) + .size(LabelSize::Small), + ) + .into_any_element() +} + +fn render_agent_list(agents: Vec, cx: &mut Context) -> AnyElement { + v_flex() + .w_full() + .gap_1() + .children(itertools::intersperse_with( + agents.into_iter().map(|(id, icon, display_name, source)| { + render_agent(id, icon, display_name, source, cx).into_any_element() + }), + || { + Divider::horizontal() + .color(DividerColor::BorderFaded) + .into_any_element() + }, + )) + .into_any_element() +} + +fn render_agent( + id: AgentId, + icon: Option, + display_name: SharedString, + source: ExternalAgentSource, + _cx: &mut Context, +) -> impl IntoElement { + let id_string = id.0.clone(); + + let icon = match icon { + Some(icon_path) => Icon::from_external_svg(icon_path), + None => Icon::new(IconName::Sparkle), + } + .size(IconSize::Small) + .color(Color::Muted); + + let source_kind = match source { + ExternalAgentSource::Registry => AiSettingItemSource::Registry, + ExternalAgentSource::Custom => AiSettingItemSource::Custom, + }; + + let remove_tooltip = match source { + ExternalAgentSource::Registry => "Remove Registry Agent", + ExternalAgentSource::Custom => "Remove Custom Agent", + }; + + let remove_button = IconButton::new( + SharedString::from(format!("uninstall-{}", id_string)), + IconName::Trash, + ) + .icon_color(Color::Muted) + .icon_size(IconSize::Small) + .tab_index(0isize) + .tooltip(Tooltip::text(remove_tooltip)) + .on_click(move |_event, _window, cx| { + remove_agent(&id, source, cx); + }); + + // The connection status of an external agent is tracked per agent-panel + // session (via the agent panel's `AgentConnectionStore`), which isn't + // available from the settings window. We therefore render a neutral status; + // the row still shows the agent's source and supports removal. + AiSettingItem::new( + id_string, + display_name, + AiSettingItemStatus::Stopped, + source_kind, + ) + .icon(icon) + .action(remove_button) +} + +fn remove_agent(id: &AgentId, source: ExternalAgentSource, cx: &mut App) { + let fs = ::global(cx); + let id = id.clone(); + update_settings_file(fs, cx, move |settings, _| { + let Some(agent_servers) = settings.agent_servers.as_mut() else { + return; + }; + // Only remove the entry if it still matches the source we rendered, so a + // stale row can't clobber an entry that was changed in the meantime. + let matches_source = agent_servers + .get(id.0.as_ref()) + .is_some_and(|entry| match source { + ExternalAgentSource::Registry => { + matches!(entry, settings::CustomAgentServerSettings::Registry { .. }) + } + ExternalAgentSource::Custom => { + matches!(entry, settings::CustomAgentServerSettings::Custom { .. }) + } + }); + if matches_source { + agent_servers.remove(id.0.as_ref()); + } + }); +} + +fn render_add_agent_popover( + settings_window: &SettingsWindow, + _window: &mut Window, + _cx: &mut Context, +) -> impl IntoElement { + let original_window = settings_window.original_window; + + PopoverMenu::new("add-agent-server-popover") + .trigger( + Button::new("add-agent", "Add Agent") + .style(ButtonStyle::Outlined) + .start_icon( + Icon::new(IconName::Plus) + .size(IconSize::Small) + .color(Color::Muted), + ) + .label_size(LabelSize::Small), + ) + .anchor(gpui::Anchor::TopRight) + .menu(move |window, cx| { + Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { + menu.entry("Install from Registry", None, move |_window, cx| { + if let Some(original_window) = original_window { + cx.activate(true); + original_window + .update(cx, |_, window, cx| { + window.activate_window(); + window.dispatch_action(Box::new(zed_actions::AcpRegistry), cx); + }) + .log_err(); + } + }) + .entry("Add Custom Agent", None, move |_window, cx| { + if let Some(original_window) = original_window { + open_new_custom_agent_in_settings(original_window, cx); + } + }) + .separator() + .header("Learn More") + .item( + ContextMenuEntry::new("ACP Docs") + .icon(IconName::ArrowUpRight) + .icon_color(Color::Muted) + .icon_position(IconPosition::End) + .handler(move |window, cx| { + window.dispatch_action( + Box::new(OpenBrowser { + url: "https://agentclientprotocol.com/".into(), + }), + cx, + ); + }), + ) + })) + }) +} + +/// Opens the user's `settings.json` in the original (editor) window, inserts a +/// scaffold `agent_servers` entry, and selects its name so the user can fill in +/// the executable path. Mirrors the agent panel's "Add Custom Agent" flow. +fn open_new_custom_agent_in_settings(original_window: WindowHandle, cx: &mut App) { + cx.activate(true); + original_window + .update(cx, |multi_workspace, window, cx| { + // Use the workspace handed to us by the update closure rather than + // `Workspace::for_window`, which would read the `MultiWorkspace` + // entity that this closure is already updating (a double borrow). + let Some(workspace) = multi_workspace.workspaces().next() else { + return; + }; + let workspace = workspace.downgrade(); + window.activate_window(); + window + .spawn(cx, async move |cx| { + add_custom_agent_settings_entry(workspace, cx).await + }) + .detach_and_log_err(cx); + }) + .log_err(); +} + +async fn add_custom_agent_settings_entry( + workspace: WeakEntity, + cx: &mut AsyncWindowContext, +) -> Result<()> { + let item = workspace + .update_in(cx, |_, window, cx| { + create_and_open_local_file(paths::settings_file(), window, cx, || { + settings::initial_user_settings_content().as_ref().into() + }) + })? + .await?; + + let Some(settings_editor) = item.downcast::() else { + return Ok(()); + }; + + settings_editor + .downgrade() + .update_in(cx, |item, window, cx| { + let text = item.buffer().read(cx).snapshot(cx).text(); + + let settings = cx.global::(); + + let mut unique_server_name = None; + let Some(edits) = settings + .edits_for_update(&text, |settings| { + let server_name: Option = (0..u8::MAX) + .map(|i| { + if i == 0 { + "your_agent".to_string() + } else { + format!("your_agent_{}", i) + } + }) + .find(|name| { + !settings + .agent_servers + .as_ref() + .is_some_and(|agent_servers| { + agent_servers.contains_key(name.as_str()) + }) + }); + if let Some(server_name) = server_name { + unique_server_name = Some(SharedString::from(server_name.clone())); + settings.agent_servers.get_or_insert_default().insert( + server_name, + settings::CustomAgentServerSettings::Custom { + path: "path_to_executable".into(), + args: vec![], + env: HashMap::default(), + default_mode: None, + default_config_options: Default::default(), + favorite_config_option_values: Default::default(), + }, + ); + } + }) + .log_err() + else { + return; + }; + + if edits.is_empty() { + return; + } + + let ranges = edits + .iter() + .map(|(range, _)| range.clone()) + .collect::>(); + + item.edit( + edits.into_iter().map(|(range, s)| { + ( + MultiBufferOffset(range.start)..MultiBufferOffset(range.end), + s, + ) + }), + cx, + ); + + if let Some((unique_server_name, buffer)) = + unique_server_name.zip(item.buffer().read(cx).as_singleton()) + { + let snapshot = buffer.read(cx).snapshot(); + if let Some(range) = + find_text_in_buffer(&unique_server_name, ranges[0].start, &snapshot) + { + item.change_selections( + SelectionEffects::scroll(Autoscroll::newest()), + window, + cx, + |selections| { + selections.select_ranges(vec![ + MultiBufferOffset(range.start)..MultiBufferOffset(range.end), + ]); + }, + ); + } + } + }) + .log_err(); + + Ok(()) +} + +fn find_text_in_buffer( + text: &str, + start: usize, + snapshot: &language::BufferSnapshot, +) -> Option> { + let chars = text.chars().collect::>(); + + let mut offset = start; + let mut char_offset = 0; + for c in snapshot.chars_at(start) { + if char_offset >= chars.len() { + break; + } + offset += 1; + + if c == chars[char_offset] { + char_offset += 1; + } else { + char_offset = 0; + } + } + + if char_offset == chars.len() { + Some(offset.saturating_sub(chars.len())..offset) + } else { + None + } +} From 3ad7863348de99407e799fa1337a4e6bd6916f27 Mon Sep 17 00:00:00 2001 From: cameron Date: Mon, 8 Jun 2026 21:09:27 +0100 Subject: [PATCH 10/16] external agents --- crates/settings_ui/src/pages.rs | 2 +- .../src/pages/external_agents_page.rs | 749 +++++++++++++++++- crates/settings_ui/src/settings_ui.rs | 13 +- 3 files changed, 747 insertions(+), 17 deletions(-) diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 3fda42713900ce..94b0466758d1d9 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -13,7 +13,7 @@ pub(crate) use audio_input_output_setup::{ }; pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; -pub(crate) use external_agents_page::render_external_agents_page; +pub(crate) use external_agents_page::{CustomAgentForm, render_external_agents_page}; pub(crate) use feature_flags::render_feature_flags_page; pub(crate) use llm_providers_page::render_llm_providers_page; pub(crate) use mcp_servers_page::{McpServerForm, render_mcp_servers_page}; diff --git a/crates/settings_ui/src/pages/external_agents_page.rs b/crates/settings_ui/src/pages/external_agents_page.rs index f37b1cbc01834c..65e341e70d621b 100644 --- a/crates/settings_ui/src/pages/external_agents_page.rs +++ b/crates/settings_ui/src/pages/external_agents_page.rs @@ -3,10 +3,13 @@ use std::ops::Range; use anyhow::Result; use collections::HashMap; use editor::{Editor, MultiBufferOffset, SelectionEffects, scroll::Autoscroll}; -use gpui::{AsyncWindowContext, Entity, ScrollHandle, WeakEntity, WindowHandle, prelude::*}; +use gpui::{ + AsyncWindowContext, Entity, FocusHandle, Focusable as _, ReadGlobal as _, ScrollHandle, + WeakEntity, WindowHandle, prelude::*, +}; use itertools::Itertools as _; use project::agent_server_store::{AgentId, AgentServerStore, ExternalAgentSource}; -use settings::{SettingsStore, update_settings_file}; +use settings::{CustomAgentServerSettings, SettingsStore, update_settings_file}; use ui::{ AiSettingItem, AiSettingItemSource, AiSettingItemStatus, ContextMenu, ContextMenuEntry, Divider, DividerColor, PopoverMenu, Tooltip, prelude::*, @@ -107,6 +110,19 @@ fn collect_agents(store: &Entity, cx: &App) -> Vec { .collect() } +/// Reads the raw, user-configured settings for a custom agent so the edit form +/// can be pre-filled. Reading the parsed settings (rather than the resolved +/// runtime server) keeps this resilient to malformed `settings.json`: the +/// settings layer drops individual bad fields instead of failing. +fn custom_agent_settings(id: &AgentId, cx: &App) -> Option { + SettingsStore::global(cx) + .get_content_for_file(settings::SettingsFile::User)? + .agent_servers + .as_ref()? + .get(id.0.as_ref()) + .cloned() +} + fn render_empty_state(cx: &App) -> AnyElement { h_flex() .p_4() @@ -161,7 +177,7 @@ fn render_agent( icon: Option, display_name: SharedString, source: ExternalAgentSource, - _cx: &mut Context, + cx: &mut Context, ) -> impl IntoElement { let id_string = id.0.clone(); @@ -177,6 +193,27 @@ fn render_agent( ExternalAgentSource::Custom => AiSettingItemSource::Custom, }; + // Only custom agents are editable here; registry agents are managed via the + // ACP registry and only support removal. + let configure_button = (source == ExternalAgentSource::Custom).then(|| { + IconButton::new( + SharedString::from(format!("configure-{}", id_string)), + IconName::Settings, + ) + .icon_color(Color::Muted) + .icon_size(IconSize::Small) + .tab_index(0isize) + .tooltip(Tooltip::text("Configure Agent")) + .on_click(cx.listener({ + let id = id.clone(); + move |this, _event, window, cx| { + let existing = + custom_agent_settings(&id, cx).map(|settings| (id.clone(), settings)); + open_custom_agent_form(this, existing, window, cx); + } + })) + }); + let remove_tooltip = match source { ExternalAgentSource::Registry => "Remove Registry Agent", ExternalAgentSource::Custom => "Remove Custom Agent", @@ -197,7 +234,7 @@ fn render_agent( // The connection status of an external agent is tracked per agent-panel // session (via the agent panel's `AgentConnectionStore`), which isn't // available from the settings window. We therefore render a neutral status; - // the row still shows the agent's source and supports removal. + // the row still shows the agent's source and supports configure/removal. AiSettingItem::new( id_string, display_name, @@ -205,6 +242,7 @@ fn render_agent( source_kind, ) .icon(icon) + .when_some(configure_button, |this, button| this.action(button)) .action(remove_button) } @@ -221,10 +259,10 @@ fn remove_agent(id: &AgentId, source: ExternalAgentSource, cx: &mut App) { .get(id.0.as_ref()) .is_some_and(|entry| match source { ExternalAgentSource::Registry => { - matches!(entry, settings::CustomAgentServerSettings::Registry { .. }) + matches!(entry, CustomAgentServerSettings::Registry { .. }) } ExternalAgentSource::Custom => { - matches!(entry, settings::CustomAgentServerSettings::Custom { .. }) + matches!(entry, CustomAgentServerSettings::Custom { .. }) } }); if matches_source { @@ -235,15 +273,26 @@ fn remove_agent(id: &AgentId, source: ExternalAgentSource, cx: &mut App) { fn render_add_agent_popover( settings_window: &SettingsWindow, - _window: &mut Window, - _cx: &mut Context, + window: &mut Window, + cx: &mut Context, ) -> impl IntoElement { let original_window = settings_window.original_window; + // Stable handle so the button keeps focus state across renders and can show a + // focus ring even when the page is opened (and the button auto-focused) via a + // mouse click, where `focus_visible` styling is suppressed. + let focus_handle = settings_window + .external_agent_add_focus_handle + .clone() + .tab_index(0) + .tab_stop(true); + let border_color = focus_ring_color(&focus_handle, window, cx); + let settings_window = cx.entity().downgrade(); - PopoverMenu::new("add-agent-server-popover") + let popover = PopoverMenu::new("add-agent-server-popover") .trigger( Button::new("add-agent", "Add Agent") .style(ButtonStyle::Outlined) + .track_focus(&focus_handle) .start_icon( Icon::new(IconName::Plus) .size(IconSize::Small) @@ -253,6 +302,7 @@ fn render_add_agent_popover( ) .anchor(gpui::Anchor::TopRight) .menu(move |window, cx| { + let settings_window = settings_window.clone(); Some(ContextMenu::build(window, cx, move |menu, _window, _cx| { menu.entry("Install from Registry", None, move |_window, cx| { if let Some(original_window) = original_window { @@ -265,10 +315,12 @@ fn render_add_agent_popover( .log_err(); } }) - .entry("Add Custom Agent", None, move |_window, cx| { - if let Some(original_window) = original_window { - open_new_custom_agent_in_settings(original_window, cx); - } + .entry("Add Custom Agent", None, move |window, cx| { + settings_window + .update(cx, |this, cx| { + open_custom_agent_form(this, None, window, cx); + }) + .log_err(); }) .separator() .header("Learn More") @@ -287,12 +339,545 @@ fn render_add_agent_popover( }), ) })) + }); + + div() + .rounded_md() + .border_1() + .border_color(border_color) + .child(popover) +} + +// === Custom external agent add/edit form === + +struct KeyValueRow { + key: Entity, + value: Entity, +} + +/// Editor-backed state for the custom external agent add/edit form. +pub(crate) struct CustomAgentForm { + /// `Some` when editing an existing agent (used to remove the old entry on rename). + original_id: Option, + name: Entity, + command: Entity, + args: Entity, + env: Vec, + /// Advanced fields not surfaced by the form. They're preserved verbatim so + /// editing the basic settings doesn't drop a user's hand-written config. + default_mode: Option, + default_config_options: HashMap, + favorite_config_option_values: HashMap>, + /// Stable handles for the Cancel/Save buttons so they can render a focus + /// ring. `Filled`/`Subtle` buttons only get a subtle `focus_visible` + /// background change otherwise, which is hard to see. + cancel_focus_handle: FocusHandle, + save_focus_handle: FocusHandle, + error: Option, +} + +impl CustomAgentForm { + fn new( + existing: Option<(AgentId, CustomAgentServerSettings)>, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let original_id = existing.as_ref().map(|(id, _)| id.clone()); + let name_initial = original_id.as_ref().map(|id| id.0.to_string()); + + let mut command_initial = None; + let mut args_initial = None; + let mut env = Vec::new(); + let mut default_mode = None; + let mut default_config_options = HashMap::default(); + let mut favorite_config_option_values = HashMap::default(); + + // Pre-fill from the raw settings so invalid values typed directly into + // settings.json still load into the form for correction. + if let Some((_, settings)) = existing.as_ref() { + match settings { + CustomAgentServerSettings::Custom { + path, + args, + env: env_map, + default_mode: mode, + default_config_options: config_options, + favorite_config_option_values: favorites, + } => { + command_initial = Some(path.to_string_lossy().to_string()); + if !args.is_empty() { + args_initial = Some(args.join(" ")); + } + for (key, value) in sorted_pairs(env_map) { + env.push(new_kv_row(Some(&key), Some(&value), window, cx)); + } + default_mode = mode.clone(); + default_config_options = config_options.clone(); + favorite_config_option_values = favorites.clone(); + } + CustomAgentServerSettings::Registry { + env: env_map, + default_mode: mode, + default_config_options: config_options, + favorite_config_option_values: favorites, + } => { + for (key, value) in sorted_pairs(env_map) { + env.push(new_kv_row(Some(&key), Some(&value), window, cx)); + } + default_mode = mode.clone(); + default_config_options = config_options.clone(); + favorite_config_option_values = favorites.clone(); + } + } + } + + Self { + original_id, + name: new_input("my-agent", name_initial.as_deref(), window, cx), + command: new_input("/path/to/agent", command_initial.as_deref(), window, cx), + args: new_input("--flag value", args_initial.as_deref(), window, cx), + env, + default_mode, + default_config_options, + favorite_config_option_values, + cancel_focus_handle: cx.focus_handle(), + save_focus_handle: cx.focus_handle(), + error: None, + } + } +} + +fn sorted_pairs(map: &HashMap) -> Vec<(String, String)> { + let mut pairs: Vec<(String, String)> = map + .iter() + .map(|(key, value)| (key.clone(), value.clone())) + .collect(); + pairs.sort_by(|a, b| a.0.cmp(&b.0)); + pairs +} + +fn new_input( + placeholder: &str, + initial: Option<&str>, + window: &mut Window, + cx: &mut Context, +) -> Entity { + let placeholder = placeholder.to_string(); + let initial = initial.map(|text| text.to_string()); + cx.new(|cx| { + let mut editor = Editor::single_line(window, cx); + editor.set_placeholder_text(placeholder.as_str(), window, cx); + if let Some(text) = initial { + editor.set_text(text, window, cx); + } + editor + }) +} + +fn new_kv_row( + key: Option<&str>, + value: Option<&str>, + window: &mut Window, + cx: &mut Context, +) -> KeyValueRow { + KeyValueRow { + key: new_input("Key", key, window, cx), + value: new_input("Value", value, window, cx), + } +} + +/// Creates the form state and pushes the form sub-page onto the stack. +pub(crate) fn open_custom_agent_form( + settings_window: &mut SettingsWindow, + existing: Option<(AgentId, CustomAgentServerSettings)>, + window: &mut Window, + cx: &mut Context, +) { + let is_edit = existing.is_some(); + settings_window.custom_agent_form = Some(CustomAgentForm::new(existing, window, cx)); + + let title = if is_edit { + "Configure External Agent" + } else { + "Add Custom Agent" + }; + + settings_window.push_dynamic_sub_page( + title, + "Agent Configuration", + Some("agent_servers"), + false, + render_custom_agent_form_page, + window, + cx, + ); +} + +fn render_custom_agent_form_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let Some(form) = settings_window.custom_agent_form.as_ref() else { + return div().into_any_element(); + }; + let error = form.error.clone(); + + let fields = v_flex() + .w_full() + .max_w(rems(36.)) + .gap_3() + .child(labeled_field("Agent Name", true, &form.name, cx)) + .child(labeled_field("Command", true, &form.command, cx)) + .child(labeled_field("Arguments", false, &form.args, cx)) + .child(render_env_section(&form.env, cx)) + .when_some(error, |this, error| this.child(render_form_error(error))) + .child(render_form_actions(form, window, cx)); + + v_flex() + .id("custom-agent-form-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child(fields) + .into_any_element() +} + +fn field_label(label: &str, required: bool) -> impl IntoElement { + h_flex() + .gap_0p5() + .child( + Label::new(label.to_string()) + .size(LabelSize::Small) + .color(Color::Muted), + ) + .when(required, |this| { + this.child(Label::new("*").size(LabelSize::Small).color(Color::Error)) }) } +fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { + let colors = cx.theme().colors(); + // All form inputs share tab index 0, so tab order follows render (insertion) + // order. Tracking the editor's focus handle makes the field a tab stop and + // routes keyboard focus into the editor when tabbed to. + let focus_handle = editor.focus_handle(cx).tab_index(0).tab_stop(true); + h_flex() + .w_full() + .min_w_0() + .py_1() + .px_2() + .h_8() + .rounded_md() + .border_1() + .border_color(colors.border) + .bg(colors.editor_background) + .track_focus(&focus_handle) + .focus(|style| style.border_color(colors.border_focused)) + .child(editor.clone()) +} + +fn labeled_field( + label: &str, + required: bool, + editor: &Entity, + cx: &App, +) -> impl IntoElement { + v_flex() + .w_full() + .gap_1() + .child(field_label(label, required)) + .child(input_box(editor, cx)) +} + +fn render_env_section(rows: &[KeyValueRow], cx: &mut Context) -> impl IntoElement { + v_flex() + .w_full() + .gap_1() + .child(field_label("Environment Variables", false)) + .children(rows.iter().enumerate().map(|(ix, row)| { + h_flex() + .w_full() + .gap_1() + .items_center() + .child(div().flex_1().min_w_0().child(input_box(&row.key, cx))) + .child(div().flex_1().min_w_0().child(input_box(&row.value, cx))) + .child( + IconButton::new(("custom-agent-env-remove", ix), IconName::Close) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tab_index(0isize) + .tooltip(Tooltip::text("Remove")) + .on_click(cx.listener(move |this, _, _window, cx| { + if let Some(form) = this.custom_agent_form.as_mut() + && ix < form.env.len() + { + form.env.remove(ix); + } + cx.notify(); + })), + ) + })) + .child( + Button::new("custom-agent-env-add", "Add") + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .tab_index(0isize) + .start_icon( + Icon::new(IconName::Plus) + .size(IconSize::Small) + .color(Color::Muted), + ) + .on_click(cx.listener(move |this, _, window, cx| { + let row = new_kv_row(None, None, window, cx); + // Focus the new key so the user can type immediately and tab + // through the new row (key -> value -> ... -> Add button). + let key_handle = row.key.focus_handle(cx); + if let Some(form) = this.custom_agent_form.as_mut() { + form.env.push(row); + } + key_handle.focus(window, cx); + cx.notify(); + })), + ) +} + +fn render_form_error(error: SharedString) -> impl IntoElement { + h_flex() + .w_full() + .gap_2() + .items_start() + .child( + Icon::new(IconName::XCircle) + .size(IconSize::Small) + .color(Color::Error), + ) + .child(Label::new(error).size(LabelSize::Small).color(Color::Error)) +} + +fn render_form_actions( + form: &CustomAgentForm, + window: &mut Window, + cx: &mut Context, +) -> impl IntoElement { + let cancel_handle = form.cancel_focus_handle.clone().tab_index(0).tab_stop(true); + let save_handle = form.save_focus_handle.clone().tab_index(0).tab_stop(true); + let cancel_border = focus_ring_color(&cancel_handle, window, cx); + let save_border = focus_ring_color(&save_handle, window, cx); + + h_flex() + .w_full() + .gap_2() + .justify_end() + .pt_2() + .child( + div() + .rounded_md() + .border_1() + .border_color(cancel_border) + .child( + Button::new("custom-agent-form-cancel", "Cancel") + .style(ButtonStyle::Subtle) + .track_focus(&cancel_handle) + .on_click(cx.listener(|this, _, window, cx| { + this.custom_agent_form = None; + this.pop_sub_page(window, cx); + })), + ), + ) + .child( + div() + .rounded_md() + .border_1() + .border_color(save_border) + .child( + Button::new("custom-agent-form-save", "Save") + .style(ButtonStyle::Filled) + .track_focus(&save_handle) + .on_click(cx.listener(|this, _, window, cx| { + save_custom_agent_form(this, window, cx); + })), + ), + ) +} + +/// Returns the border color for a button's focus ring: visible when focused +/// (keyboard or programmatic), transparent otherwise. +fn focus_ring_color(handle: &FocusHandle, window: &Window, cx: &App) -> gpui::Hsla { + if handle.is_focused(window) { + cx.theme().colors().border_focused + } else { + gpui::transparent_black() + } +} + +fn save_custom_agent_form( + settings_window: &mut SettingsWindow, + window: &mut Window, + cx: &mut Context, +) { + let built = { + let Some(form) = settings_window.custom_agent_form.as_ref() else { + return; + }; + build_settings_from_form(form, cx) + }; + + let (id, original_id, content) = match built { + Ok(value) => value, + Err(error) => { + if let Some(form) = settings_window.custom_agent_form.as_mut() { + form.error = Some(error); + } + cx.notify(); + return; + } + }; + + // Reject names that would collide with a *different* existing agent. This + // covers both adding a new agent and renaming an existing one. + let collides_with_other_agent = + get_agent_server_store(settings_window, cx).is_some_and(|store| { + let existing_ids = store + .read(cx) + .external_agents() + .cloned() + .collect::>(); + name_collides_with_other_agent(&id, original_id.as_ref(), &existing_ids) + }); + if collides_with_other_agent { + if let Some(form) = settings_window.custom_agent_form.as_mut() { + form.error = Some(format!("An agent named \"{}\" already exists.", id.0).into()); + } + cx.notify(); + return; + } + + let fs = ::global(cx); + update_settings_file(fs, cx, move |settings, _| { + let agent_servers = settings.agent_servers.get_or_insert_default(); + if let Some(original_id) = &original_id + && original_id.0 != id.0 + { + agent_servers.remove(original_id.0.as_ref()); + } + agent_servers.insert(id.0.to_string(), content); + }); + + settings_window.custom_agent_form = None; + settings_window.pop_sub_page(window, cx); +} + +/// Plain (editor-free) snapshot of the form's contents, so the validation / +/// build logic can be exercised without a GPUI context. +struct CustomAgentFormValues { + original_id: Option, + name: String, + command: String, + args: String, + env: Vec<(String, String)>, + default_mode: Option, + default_config_options: HashMap, + favorite_config_option_values: HashMap>, +} + +fn build_settings_from_form( + form: &CustomAgentForm, + cx: &App, +) -> Result<(AgentId, Option, CustomAgentServerSettings), SharedString> { + let values = CustomAgentFormValues { + original_id: form.original_id.clone(), + name: form.name.read(cx).text(cx), + command: form.command.read(cx).text(cx), + args: form.args.read(cx).text(cx), + env: read_kv(&form.env, cx), + default_mode: form.default_mode.clone(), + default_config_options: form.default_config_options.clone(), + favorite_config_option_values: form.favorite_config_option_values.clone(), + }; + build_settings_from_values(values) +} + +fn read_kv(rows: &[KeyValueRow], cx: &App) -> Vec<(String, String)> { + rows.iter() + .map(|row| (row.key.read(cx).text(cx), row.value.read(cx).text(cx))) + .collect() +} + +fn build_settings_from_values( + values: CustomAgentFormValues, +) -> Result<(AgentId, Option, CustomAgentServerSettings), SharedString> { + let name = values.name.trim().to_string(); + if name.is_empty() { + return Err("Agent name is required.".into()); + } + + let command = values.command.trim().to_string(); + if command.is_empty() { + return Err("Command is required.".into()); + } + + let args = values + .args + .split_whitespace() + .map(|arg| arg.to_string()) + .collect::>(); + let env = collect_kv(&values.env, "environment variable")?; + + let content = CustomAgentServerSettings::Custom { + path: command.into(), + args, + env, + default_mode: values.default_mode, + default_config_options: values.default_config_options, + favorite_config_option_values: values.favorite_config_option_values, + }; + + Ok((AgentId(name.into()), values.original_id, content)) +} + +/// Returns whether saving under `id` would overwrite a *different* existing +/// agent. Editing an agent in place (`id == original_id`) is allowed. +fn name_collides_with_other_agent( + id: &AgentId, + original_id: Option<&AgentId>, + existing_ids: &[AgentId], +) -> bool { + original_id.is_none_or(|original| original.0 != id.0) + && existing_ids.iter().any(|existing| existing.0 == id.0) +} + +fn collect_kv( + rows: &[(String, String)], + label: &str, +) -> Result, SharedString> { + let mut map = HashMap::default(); + for (key, value) in rows { + let key = key.trim().to_string(); + if key.is_empty() { + continue; + } + if map.contains_key(&key) { + return Err(format!("Duplicate {label} \"{key}\".").into()); + } + map.insert(key, value.clone()); + } + Ok(map) +} + +// === Open settings.json at the agent's position === +// +// Retained for an upcoming "Edit in settings.json" affordance that jumps the +// user to the relevant `agent_servers` entry. Not currently wired to any UI. + /// Opens the user's `settings.json` in the original (editor) window, inserts a /// scaffold `agent_servers` entry, and selects its name so the user can fill in -/// the executable path. Mirrors the agent panel's "Add Custom Agent" flow. +/// the executable path. +#[allow(dead_code)] fn open_new_custom_agent_in_settings(original_window: WindowHandle, cx: &mut App) { cx.activate(true); original_window @@ -314,6 +899,7 @@ fn open_new_custom_agent_in_settings(original_window: WindowHandle, cx: &mut AsyncWindowContext, @@ -360,7 +946,7 @@ async fn add_custom_agent_settings_entry( unique_server_name = Some(SharedString::from(server_name.clone())); settings.agent_servers.get_or_insert_default().insert( server_name, - settings::CustomAgentServerSettings::Custom { + CustomAgentServerSettings::Custom { path: "path_to_executable".into(), args: vec![], env: HashMap::default(), @@ -420,6 +1006,7 @@ async fn add_custom_agent_settings_entry( Ok(()) } +#[allow(dead_code)] fn find_text_in_buffer( text: &str, start: usize, @@ -448,3 +1035,135 @@ fn find_text_in_buffer( None } } + +#[cfg(test)] +mod tests { + use super::*; + + fn values() -> CustomAgentFormValues { + CustomAgentFormValues { + original_id: None, + name: "my-agent".into(), + command: "/usr/bin/agent".into(), + args: String::new(), + env: Vec::new(), + default_mode: None, + default_config_options: HashMap::default(), + favorite_config_option_values: HashMap::default(), + } + } + + fn id(name: &str) -> AgentId { + AgentId(name.into()) + } + + #[test] + fn requires_agent_name() { + let mut values = values(); + values.name = " ".into(); + assert_eq!( + build_settings_from_values(values).unwrap_err().as_ref(), + "Agent name is required." + ); + } + + #[test] + fn requires_command() { + let mut values = values(); + values.command = " ".into(); + assert_eq!( + build_settings_from_values(values).unwrap_err().as_ref(), + "Command is required." + ); + } + + #[test] + fn rejects_duplicate_environment_variables() { + let mut values = values(); + values.env = vec![("FOO".into(), "1".into()), ("FOO".into(), "2".into())]; + assert_eq!( + build_settings_from_values(values).unwrap_err().as_ref(), + "Duplicate environment variable \"FOO\"." + ); + } + + #[test] + fn builds_custom_agent() { + let mut values = values(); + values.name = " my-agent ".into(); + values.command = "/usr/bin/agent".into(); + values.args = "--flag value".into(); + // Empty values are kept, but rows with a blank key are ignored. + values.env = vec![ + ("KEY".into(), "VALUE".into()), + ("EMPTY".into(), String::new()), + (" ".into(), "ignored".into()), + ]; + + let (id, original_id, content) = build_settings_from_values(values).unwrap(); + assert_eq!(id.0.as_ref(), "my-agent"); + assert_eq!(original_id, None); + + let expected_env = HashMap::from_iter([ + ("KEY".to_string(), "VALUE".to_string()), + ("EMPTY".to_string(), String::new()), + ]); + assert_eq!( + content, + CustomAgentServerSettings::Custom { + path: "/usr/bin/agent".into(), + args: vec!["--flag".into(), "value".into()], + env: expected_env, + default_mode: None, + default_config_options: HashMap::default(), + favorite_config_option_values: HashMap::default(), + } + ); + } + + #[test] + fn preserves_advanced_fields() { + let mut values = values(); + values.default_mode = Some("ask".into()); + values.default_config_options = + HashMap::from_iter([("opt".to_string(), "val".to_string())]); + + let (_, _, content) = build_settings_from_values(values).unwrap(); + match content { + CustomAgentServerSettings::Custom { + default_mode, + default_config_options, + .. + } => { + assert_eq!(default_mode.as_deref(), Some("ask")); + assert_eq!( + default_config_options.get("opt").map(String::as_str), + Some("val") + ); + } + _ => panic!("expected a custom agent"), + } + } + + #[test] + fn name_collision_covers_new_and_rename() { + let existing = vec![id("foo"), id("bar")]; + + // New agent taking an existing name collides. + assert!(name_collides_with_other_agent(&id("foo"), None, &existing)); + // New agent with a free name is fine. + assert!(!name_collides_with_other_agent(&id("baz"), None, &existing)); + // Editing an agent in place is allowed even though the name "exists". + assert!(!name_collides_with_other_agent( + &id("foo"), + Some(&id("foo")), + &existing + )); + // Renaming onto a different agent's name collides. + assert!(name_collides_with_other_agent( + &id("bar"), + Some(&id("foo")), + &existing + )); + } +} diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index e7ee6e9dfc2c6f..5c25760103dbb5 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -55,7 +55,8 @@ use crate::components::{ theme_picker, }; use crate::pages::{ - McpServerForm, render_input_audio_device_dropdown, render_output_audio_device_dropdown, + CustomAgentForm, McpServerForm, render_input_audio_device_dropdown, + render_output_audio_device_dropdown, }; const NAVBAR_CONTAINER_TAB_INDEX: isize = 0; @@ -127,6 +128,11 @@ struct SettingField { json_path: Option<&'static str>, } +enum SettingsPath { + Json(&'static str), // a.b.c + Subpage(&'static str), // a/b/c +} + impl Clone for SettingField { fn clone(&self) -> Self { *self @@ -788,6 +794,8 @@ pub struct SettingsWindow { /// focus ring when the page auto-focuses it on open (which happens via mouse, /// where `focus_visible` styling would otherwise be suppressed). pub(crate) mcp_add_server_focus_handle: FocusHandle, + /// State for the active "add/edit custom external agent" form sub-page, if open. + pub(crate) custom_agent_form: Option, } struct SearchDocument { @@ -1822,6 +1830,7 @@ impl SettingsWindow { last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), + custom_agent_form: None, }; this.fetch_files(window, cx); @@ -4728,6 +4737,7 @@ pub mod test { last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), + custom_agent_form: None, } } } @@ -4860,6 +4870,7 @@ pub mod test { last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), + custom_agent_form: None, }; settings_window.build_filter_table(); From 1dc32684a0896f3c1769e2d47a722fe8d6143b84 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 9 Jun 2026 00:09:59 +0100 Subject: [PATCH 11/16] split out LLM providers into inline/subpages --- crates/language_model/src/language_model.rs | 27 ++ crates/language_models/src/api_key_editor.rs | 155 ++++++++++++ crates/language_models/src/language_models.rs | 2 + .../language_models/src/provider/anthropic.rs | 30 ++- crates/language_models/src/provider/cloud.rs | 14 +- .../src/provider/copilot_chat.rs | 14 +- .../language_models/src/provider/deepseek.rs | 29 ++- crates/language_models/src/provider/google.rs | 27 +- .../language_models/src/provider/mistral.rs | 29 ++- .../language_models/src/provider/open_ai.rs | 25 ++ .../src/provider/open_router.rs | 29 ++- .../src/provider/vercel_ai_gateway.rs | 29 ++- crates/language_models/src/provider/x_ai.rs | 29 ++- .../src/pages/llm_providers_page.rs | 232 ++++++++++-------- crates/settings_ui/src/settings_ui.rs | 25 +- 15 files changed, 574 insertions(+), 122 deletions(-) create mode 100644 crates/language_models/src/api_key_editor.rs diff --git a/crates/language_model/src/language_model.rs b/crates/language_model/src/language_model.rs index 7dc237a65dd768..1b3a17c0f4683b 100644 --- a/crates/language_model/src/language_model.rs +++ b/crates/language_model/src/language_model.rs @@ -290,6 +290,23 @@ pub trait LanguageModelProvider: 'static { ) -> AnyView; fn reset_credentials(&self, cx: &mut App) -> Task>; + /// Returns the provider's configuration UI together with how it prefers to + /// be presented: [`ProviderConfigurationView::Inline`] for a compact control + /// that can sit in a list row (e.g. a single API-key field), or + /// [`ProviderConfigurationView::SubPage`] for a richer view that needs its + /// own surface. + /// + /// The default reuses [`Self::configuration_view`] as a sub-page, so + /// providers only override this when they have a compact inline form. + fn configuration_view_v2( + &self, + target_agent: ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + ProviderConfigurationView::SubPage(self.configuration_view(target_agent, window, cx)) + } + /// Copy shown the first time a user enables fast mode for a model from /// this provider. Returning `None` skips the confirmation prompt and lets /// the toggle apply silently. @@ -298,6 +315,16 @@ pub trait LanguageModelProvider: 'static { } } +/// How a provider's configuration UI prefers to be presented by the settings UI. +#[derive(Clone)] +pub enum ProviderConfigurationView { + /// A compact control suitable for rendering inline in a list row, such as a + /// single API-key field. + Inline(AnyView), + /// A richer view that should be shown on its own dedicated sub-page. + SubPage(AnyView), +} + /// Provider-specific copy shown the first time a user enables fast mode. #[derive(Debug, Clone)] pub struct FastModeConfirmation { diff --git a/crates/language_models/src/api_key_editor.rs b/crates/language_models/src/api_key_editor.rs new file mode 100644 index 00000000000000..d6a9182a662b4b --- /dev/null +++ b/crates/language_models/src/api_key_editor.rs @@ -0,0 +1,155 @@ +use std::rc::Rc; + +use anyhow::Result; +use gpui::{App, Context, Entity, Subscription, Task, Window}; +use language_model::ApiKeyState; +use ui::{Tooltip, prelude::*}; +use ui_input::InputField; + +/// The current credential state of a single-API-key provider, as reported by the +/// provider when constructing an [`ApiKeyEditor`]. +pub enum ApiKeyStatus { + /// No key is configured; show the input field. + Unset, + /// A key is configured via the UI; show a "configured" row with a reset. + Configured, + /// The key comes from an environment variable and can't be edited here. + FromEnvVar(SharedString), +} + +/// Maps a provider's [`ApiKeyState`] to the [`ApiKeyStatus`] the editor renders. +/// Shared so the API-key providers don't each duplicate this mapping. +pub fn api_key_status(state: &ApiKeyState) -> ApiKeyStatus { + if state.is_from_env_var() { + ApiKeyStatus::FromEnvVar(state.env_var_name().clone()) + } else if state.has_key() { + ApiKeyStatus::Configured + } else { + ApiKeyStatus::Unset + } +} + +/// A compact, reusable control for editing a provider's single API key, intended +/// to be returned from `LanguageModelProvider::configuration_view_v2` as an +/// inline control. +/// +/// It is deliberately provider-agnostic: the provider supplies closures that +/// read the current [`ApiKeyStatus`] and store/clear the key against its own +/// state, so all credential knowledge stays in the provider. +pub struct ApiKeyEditor { + input: Entity, + api_key_url: SharedString, + status: Rc ApiKeyStatus>, + set_key: Rc Task>>, + reset_key: Rc Task>>, + _subscription: Subscription, +} + +impl ApiKeyEditor { + pub fn new( + state: Entity, + api_key_url: impl Into, + placeholder: &str, + status: impl Fn(&S, &App) -> ApiKeyStatus + 'static, + set_key: impl Fn(&Entity, String, &mut App) -> Task> + 'static, + reset_key: impl Fn(&Entity, &mut App) -> Task> + 'static, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let input = cx.new(|cx| { + InputField::new(window, cx, placeholder) + .masked(true) + .tab_index(0) + }); + let subscription = cx.observe(&state, |_, _, cx| cx.notify()); + + let status_state = state.clone(); + let set_state = state.clone(); + Self { + input, + api_key_url: api_key_url.into(), + status: Rc::new(move |cx| status(status_state.read(cx), cx)), + set_key: Rc::new(move |key, cx| set_key(&set_state, key, cx)), + reset_key: Rc::new(move |cx| reset_key(&state, cx)), + _subscription: subscription, + } + } + + fn save(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { + let key = self.input.read(cx).text(cx).trim().to_string(); + if key.is_empty() { + return; + } + self.input + .update(cx, |input, cx| input.set_text("", window, cx)); + (self.set_key.clone())(key, cx).detach_and_log_err(cx); + } + + fn reset(&mut self, cx: &mut Context) { + (self.reset_key.clone())(cx).detach_and_log_err(cx); + } + + fn render_where_to_find_key(&self) -> impl IntoElement { + let url = self.api_key_url.clone(); + let click_url = url.to_string(); + h_flex() + .id("where-to-find-key") + .gap_0p5() + .cursor_pointer() + .child( + Icon::new(IconName::Info) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child( + Label::new("Where to find key") + .size(LabelSize::Small) + .color(Color::Muted), + ) + .tooltip(Tooltip::text(format!("Create an API key at {url}"))) + .on_click(move |_, _window, cx| cx.open_url(&click_url)) + } +} + +impl Render for ApiKeyEditor { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + match (self.status)(cx) { + ApiKeyStatus::FromEnvVar(env_var_name) => Label::new(format!("Set via {env_var_name}")) + .size(LabelSize::Small) + .color(Color::Muted) + .into_any_element(), + ApiKeyStatus::Configured => h_flex() + .gap_2() + .items_center() + .child( + Icon::new(IconName::Check) + .size(IconSize::Small) + .color(Color::Success), + ) + .child( + Label::new("Configured") + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child( + Button::new("reset-api-key", "Reset") + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .tab_index(0isize) + .on_click(cx.listener(|this, _, _window, cx| this.reset(cx))), + ) + .into_any_element(), + ApiKeyStatus::Unset => v_flex() + .w_full() + .gap_1() + .child(self.render_where_to_find_key()) + .child( + div() + .w_full() + .on_action(cx.listener(Self::save)) + .child(self.input.clone()), + ) + .into_any_element(), + } + } +} diff --git a/crates/language_models/src/language_models.rs b/crates/language_models/src/language_models.rs index 97ac0b2c0abb63..e56eb80d72aa5c 100644 --- a/crates/language_models/src/language_models.rs +++ b/crates/language_models/src/language_models.rs @@ -10,10 +10,12 @@ use language_model::{ }; use provider::deepseek::DeepSeekLanguageModelProvider; +mod api_key_editor; pub mod extension; pub mod provider; mod settings; +pub use crate::api_key_editor::{ApiKeyEditor, ApiKeyStatus, api_key_status}; pub use crate::extension::init_proxy as init_extension_proxy; use crate::provider::anthropic::AnthropicLanguageModelProvider; diff --git a/crates/language_models/src/provider/anthropic.rs b/crates/language_models/src/provider/anthropic.rs index cb7f8b7aa114fb..c6372a6f2ab09e 100644 --- a/crates/language_models/src/provider/anthropic.rs +++ b/crates/language_models/src/provider/anthropic.rs @@ -8,11 +8,13 @@ use futures::{FutureExt, StreamExt, future::BoxFuture, stream::BoxStream}; use gpui::{AnyView, App, AsyncApp, Context, Entity, Task, TaskExt}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ANTHROPIC_PROVIDER_ID, ANTHROPIC_PROVIDER_NAME, ApiKeyState, AuthenticateError, + ANTHROPIC_PROVIDER_ID, ANTHROPIC_PROVIDER_NAME, ApiKeyState, + AuthenticateError, ConfigurationViewTargetAgent, EnvVar, FastModeConfirmation, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, - LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, RateLimiter, + LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, + ProviderConfigurationView, RateLimiter, env_var, }; use settings::{Settings, SettingsStore}; @@ -290,6 +292,30 @@ impl LanguageModelProvider for AnthropicLanguageModelProvider { .update(cx, |state, cx| state.set_api_key(None, cx)) } + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://console.anthropic.com/settings/keys", + "sk-ant-...", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } + fn fast_mode_confirmation(&self, _cx: &App) -> Option { Some(FastModeConfirmation { title: "Enable Fast Mode for Anthropic?".into(), diff --git a/crates/language_models/src/provider/cloud.rs b/crates/language_models/src/provider/cloud.rs index c8e85652e1af16..cb0a096750eb3a 100644 --- a/crates/language_models/src/provider/cloud.rs +++ b/crates/language_models/src/provider/cloud.rs @@ -11,7 +11,7 @@ use gpui::{AnyElement, AnyView, App, AppContext, Context, Entity, Subscription, use language_model::{ AuthenticateError, FastModeConfirmation, IconOrSvg, LanguageModel, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, - ZED_CLOUD_PROVIDER_ID, ZED_CLOUD_PROVIDER_NAME, + ProviderConfigurationView, ZED_CLOUD_PROVIDER_ID, ZED_CLOUD_PROVIDER_NAME, }; use language_models_cloud::{CloudLlmTokenProvider, CloudModelProvider}; use rand::{Rng as _, SeedableRng as _, rngs::StdRng}; @@ -355,6 +355,18 @@ impl LanguageModelProvider for CloudLanguageModelProvider { .into() } + fn configuration_view_v2( + &self, + target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + // The Zed sign-in/plan control is small enough that sending users to a + // dedicated sub-page just to reach it would be annoying, so render it + // inline even though it isn't an API-key field. + ProviderConfigurationView::Inline(self.configuration_view(target_agent, window, cx)) + } + fn reset_credentials(&self, _cx: &mut App) -> Task> { Task::ready(Ok(())) } diff --git a/crates/language_models/src/provider/copilot_chat.rs b/crates/language_models/src/provider/copilot_chat.rs index f42fad657c4052..01c203280fa8d9 100644 --- a/crates/language_models/src/provider/copilot_chat.rs +++ b/crates/language_models/src/provider/copilot_chat.rs @@ -25,7 +25,8 @@ use language_model::{ LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelRequestMessage, LanguageModelToolChoice, LanguageModelToolResultContent, LanguageModelToolSchemaFormat, - LanguageModelToolUse, MessageContent, RateLimiter, Role, StopReason, TokenUsage, + LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, + TokenUsage, }; use settings::SettingsStore; use ui::prelude::*; @@ -196,6 +197,17 @@ impl LanguageModelProvider for CopilotChatLanguageModelProvider { .into() } + fn configuration_view_v2( + &self, + target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + // GitHub Copilot's control is just a sign-in button, so render it inline + // rather than behind a sub-page. + ProviderConfigurationView::Inline(self.configuration_view(target_agent, window, cx)) + } + fn reset_credentials(&self, _cx: &mut App) -> Task> { Task::ready(Err(anyhow!( "Signing out of GitHub Copilot Chat is currently not supported." diff --git a/crates/language_models/src/provider/deepseek.rs b/crates/language_models/src/provider/deepseek.rs index a7da87f355c02c..f6e9ef122cbca1 100644 --- a/crates/language_models/src/provider/deepseek.rs +++ b/crates/language_models/src/provider/deepseek.rs @@ -8,11 +8,12 @@ use futures::{FutureExt, StreamExt, future::BoxFuture, stream::BoxStream}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, + LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, - LanguageModelToolResultContent, LanguageModelToolUse, MessageContent, RateLimiter, Role, + LanguageModelToolResultContent, LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, TokenUsage, env_var, }; pub use settings::DeepseekAvailableModel as AvailableModel; @@ -211,6 +212,30 @@ impl LanguageModelProvider for DeepSeekLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://platform.deepseek.com/api_keys", + "Paste your DeepSeek API key", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct DeepSeekLanguageModel { diff --git a/crates/language_models/src/provider/google.rs b/crates/language_models/src/provider/google.rs index 392a81454bbe11..87a6551480b56b 100644 --- a/crates/language_models/src/provider/google.rs +++ b/crates/language_models/src/provider/google.rs @@ -7,7 +7,8 @@ pub use google_ai::completion::{GoogleEventMapper, into_google}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - AuthenticateError, ConfigurationViewTargetAgent, EnvVar, LanguageModelCompletionError, + AuthenticateError, ConfigurationViewTargetAgent, EnvVar, + LanguageModelCompletionError, ProviderConfigurationView, LanguageModelCompletionEvent, LanguageModelToolChoice, LanguageModelToolSchemaFormat, }; use language_model::{ @@ -235,6 +236,30 @@ impl LanguageModelProvider for GoogleLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://aistudio.google.com/app/apikey", + "AIza...", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct GoogleLanguageModel { diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index 92c83342f3e94d..392db2bc55e96a 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -6,11 +6,12 @@ use futures::{FutureExt, Stream, StreamExt, future::BoxFuture, stream::BoxStream use gpui::{AnyView, App, AsyncApp, Context, Entity, Global, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, + LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolUse, MessageContent, RateLimiter, Role, StopReason, TokenUsage, env_var, + LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, TokenUsage, env_var, }; pub use mistral::{MISTRAL_API_URL, StreamResponse}; pub use settings::MistralAvailableModel as AvailableModel; @@ -235,6 +236,30 @@ impl LanguageModelProvider for MistralLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://console.mistral.ai/api-keys", + "Paste your Mistral API key", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct MistralLanguageModel { diff --git a/crates/language_models/src/provider/open_ai.rs b/crates/language_models/src/provider/open_ai.rs index 42200e349648e9..c62d2ad8736aa3 100644 --- a/crates/language_models/src/provider/open_ai.rs +++ b/crates/language_models/src/provider/open_ai.rs @@ -6,6 +6,7 @@ use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, use http_client::{CustomHeaders, HttpClient}; use language_model::{ ApiKeyState, AuthenticateError, EnvVar, FastModeConfirmation, IconOrSvg, LanguageModel, + ProviderConfigurationView, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, @@ -217,6 +218,30 @@ impl LanguageModelProvider for OpenAiLanguageModelProvider { .update(cx, |state, cx| state.set_api_key(None, cx)) } + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://platform.openai.com/api-keys", + "sk-...", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } + fn fast_mode_confirmation(&self, _cx: &App) -> Option { Some(FastModeConfirmation { title: "Enable Fast Mode for OpenAI?".into(), diff --git a/crates/language_models/src/provider/open_router.rs b/crates/language_models/src/provider/open_router.rs index ef434eed859992..1eebe583e60429 100644 --- a/crates/language_models/src/provider/open_router.rs +++ b/crates/language_models/src/provider/open_router.rs @@ -5,11 +5,12 @@ use futures::{FutureExt, Stream, StreamExt, future::BoxFuture}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, + LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, RateLimiter, Role, + LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, TokenUsage, env_var, }; use open_router::{ @@ -270,6 +271,30 @@ impl LanguageModelProvider for OpenRouterLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://openrouter.ai/keys", + "sk-or-...", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct OpenRouterLanguageModel { diff --git a/crates/language_models/src/provider/vercel_ai_gateway.rs b/crates/language_models/src/provider/vercel_ai_gateway.rs index 694972e48ae25a..e88dadfd6daa8c 100644 --- a/crates/language_models/src/provider/vercel_ai_gateway.rs +++ b/crates/language_models/src/provider/vercel_ai_gateway.rs @@ -7,10 +7,11 @@ use http_client::{ AsyncBody, CustomHeaders, HttpClient, Method, Request as HttpRequest, RequestBuilderExt, http, }; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, + LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, - LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolSchemaFormat, RateLimiter, + LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolSchemaFormat, ProviderConfigurationView, RateLimiter, env_var, }; use open_ai::ResponseStreamEvent; @@ -260,6 +261,30 @@ impl LanguageModelProvider for VercelAiGatewayLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%2Fapi-keys&title=Go+to+AI+Gateway", + "Paste your Vercel AI Gateway API key", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct VercelAiGatewayLanguageModel { diff --git a/crates/language_models/src/provider/x_ai.rs b/crates/language_models/src/provider/x_ai.rs index 8ac48213c0eb20..d1339b3b78068e 100644 --- a/crates/language_models/src/provider/x_ai.rs +++ b/crates/language_models/src/provider/x_ai.rs @@ -5,11 +5,12 @@ use futures::{FutureExt, StreamExt, future::BoxFuture}; use gpui::{AnyView, App, AsyncApp, Context, Entity, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, + LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, - LanguageModelToolSchemaFormat, RateLimiter, env_var, + LanguageModelToolSchemaFormat, ProviderConfigurationView, RateLimiter, env_var, }; use open_ai::ResponseStreamEvent; pub use settings::XaiAvailableModel as AvailableModel; @@ -207,6 +208,30 @@ impl LanguageModelProvider for XAiLanguageModelProvider { self.state .update(cx, |state, cx| state.set_api_key(None, cx)) } + + fn configuration_view_v2( + &self, + _target_agent: language_model::ConfigurationViewTargetAgent, + window: &mut Window, + cx: &mut App, + ) -> ProviderConfigurationView { + let state = self.state.clone(); + ProviderConfigurationView::Inline( + cx.new(|cx| { + crate::ApiKeyEditor::new( + state, + "https://console.x.ai/team/default/api-keys", + "xai-...", + |state, _cx| crate::api_key_status(&state.api_key_state), + |state, key, cx| state.update(cx, |state, cx| state.set_api_key(Some(key), cx)), + |state, cx| state.update(cx, |state, cx| state.set_api_key(None, cx)), + window, + cx, + ) + }) + .into(), + ) + } } pub struct XAiLanguageModel { diff --git a/crates/settings_ui/src/pages/llm_providers_page.rs b/crates/settings_ui/src/pages/llm_providers_page.rs index 788abadccfcbf5..d663dbe7d947c3 100644 --- a/crates/settings_ui/src/pages/llm_providers_page.rs +++ b/crates/settings_ui/src/pages/llm_providers_page.rs @@ -1,11 +1,11 @@ use std::sync::Arc; -use gpui::{AnyView, ScrollHandle, prelude::*}; +use gpui::{ScrollHandle, prelude::*}; use language_model::{ ConfigurationViewTargetAgent, IconOrSvg, LanguageModelProvider, LanguageModelProviderId, - LanguageModelRegistry, + LanguageModelRegistry, ProviderConfigurationView, }; -use ui::{Disclosure, Divider, DividerColor, prelude::*}; +use ui::{Divider, DividerColor, prelude::*}; use crate::SettingsWindow; @@ -22,20 +22,19 @@ pub(crate) fn render_llm_providers_page( .size_full() .pt_2p5() .px_8() - .pb_16() .track_scroll(scroll_handle) + .pb_16() + .track_scroll(scroll_handle) .overflow_y_scroll() .children( providers .iter() - .map(|provider| { - render_provider_block(settings_window, provider, window, cx) - }) + .map(|provider| render_provider_row(settings_window, provider, window, cx)) .collect::>(), ) .into_any_element() } -fn render_provider_block( +fn render_provider_row( settings_window: &SettingsWindow, provider: &Arc, window: &mut Window, @@ -43,32 +42,45 @@ fn render_provider_block( ) -> AnyElement { let provider_id = provider.id(); let provider_name = provider.name().0; - let disclosure_id = SharedString::from(format!("provider-disclosure-{}", provider_id.0)); - - let is_expanded = settings_window - .expanded_provider_configurations - .get(&provider_id) - .copied() - .unwrap_or(false); - - let configuration_view = if is_expanded { - Some(get_or_create_configuration_view( - settings_window, - &provider_id, - provider, - window, - cx, - )) - } else { - None - }; - let is_authenticated = provider.is_authenticated(cx); + let icon = match provider.icon() { + IconOrSvg::Svg(path) => Icon::from_external_svg(path), + IconOrSvg::Icon(name) => Icon::new(name), + } + .size(IconSize::Small) + .color(Color::Muted); + + let left = h_flex() + .flex_none() + .gap_1p5() + .child(icon) + .child(Label::new(provider_name)) + .when(is_authenticated, |this| { + this.child( + Icon::new(IconName::Check) + .size(IconSize::Small) + .color(Color::Success), + ) + }); + + // The provider tells us how it wants to be presented: a compact inline + // control, or a richer view that belongs on its own sub-page. + let control = + match get_or_create_configuration_view(settings_window, &provider_id, provider, window, cx) + { + ProviderConfigurationView::Inline(view) => v_flex() + .min_w_0() + .w_full() + .max_w(rems(24.)) + .child(view) + .into_any_element(), + ProviderConfigurationView::SubPage(_) => render_configure_button(&provider_id, cx), + }; + v_flex() .min_w_0() .w_full() - .when(is_expanded, |this| this.mb_2()) .child( div() .px_2() @@ -76,92 +88,112 @@ fn render_provider_block( ) .child( h_flex() - .map(|this| { - if is_expanded { - this.mt_2().mb_1() - } else { - this.my_2() - } - }) - .w_full() - .justify_between() - .child( - h_flex() - .id(disclosure_id.clone()) - .px_2() - .py_0p5() - .w_full() - .justify_between() - .rounded_sm() - .hover(|hover| hover.bg(cx.theme().colors().element_hover)) - .child( - h_flex() - .w_full() - .gap_1p5() - .child( - match provider.icon() { - IconOrSvg::Svg(path) => Icon::from_external_svg(path), - IconOrSvg::Icon(name) => Icon::new(name), - } - .size(IconSize::Small) - .color(Color::Muted), - ) - .child( - h_flex() - .w_full() - .gap_1() - .child(Label::new(provider_name)) - .when(is_authenticated && !is_expanded, |this| { - this.child( - Icon::new(IconName::Check).color(Color::Success), - ) - }), - ), - ) - .child( - Disclosure::new(disclosure_id, is_expanded) - .opened_icon(IconName::ChevronUp) - .closed_icon(IconName::ChevronDown), - ) - .on_click(cx.listener({ - let provider_id = provider_id.clone(); - move |this, _event, _window, _cx| { - let is_expanded = this - .expanded_provider_configurations - .entry(provider_id.clone()) - .or_insert(false); - *is_expanded = !*is_expanded; - } - })), - ), - ) - .child( - v_flex() - .min_w_0() .w_full() + .py_2() .px_2() - .gap_1() - .when_some(configuration_view, |this, view| this.child(view)), + .gap_6() + .justify_between() + .items_start() + .child(left) + .child(control), ) .into_any_element() } +fn render_configure_button( + provider_id: &LanguageModelProviderId, + cx: &mut Context, +) -> AnyElement { + let provider_id = provider_id.clone(); + Button::new( + SharedString::from(format!("configure-{}", provider_id.0)), + "Configure", + ) + .style(ButtonStyle::Outlined) + .label_size(LabelSize::Small) + .tab_index(0isize) + .on_click(cx.listener(move |this, _, window, cx| { + open_provider_configuration(this, provider_id.clone(), window, cx); + })) + .into_any_element() +} + +fn open_provider_configuration( + settings_window: &mut SettingsWindow, + provider_id: LanguageModelProviderId, + window: &mut Window, + cx: &mut Context, +) { + let title = LanguageModelRegistry::read_global(cx) + .provider(&provider_id) + .map(|provider| provider.name().0) + .unwrap_or_else(|| provider_id.0.clone()); + + settings_window.configuring_provider = Some(provider_id); + + settings_window.push_dynamic_sub_page( + title, + "Agent Configuration", + Some("llm_providers"), + false, + render_provider_config_sub_page, + window, + cx, + ); +} + +fn render_provider_config_sub_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let Some(provider_id) = settings_window.configuring_provider.clone() else { + return div().into_any_element(); + }; + let Some(provider) = LanguageModelRegistry::read_global(cx).provider(&provider_id) else { + return div().into_any_element(); + }; + + // A provider routed to a sub-page always provides a `SubPage` view; fall + // back to whatever view it returns otherwise. + let view = match get_or_create_configuration_view( + settings_window, + &provider_id, + &provider, + window, + cx, + ) { + ProviderConfigurationView::Inline(view) | ProviderConfigurationView::SubPage(view) => view, + }; + + v_flex() + .id("provider-config-sub-page") + .size_full() + .pt_2p5() + .px_8() + .pb_16() + .track_scroll(scroll_handle) + .overflow_y_scroll() + .child(view) + .into_any_element() +} + fn get_or_create_configuration_view( settings_window: &SettingsWindow, provider_id: &LanguageModelProviderId, provider: &Arc, window: &mut Window, cx: &mut Context, -) -> AnyView { - if let Some(view) = settings_window.provider_configuration_views.get(provider_id) { +) -> ProviderConfigurationView { + if let Some(view) = settings_window + .provider_configuration_views + .get(provider_id) + { return view.clone(); } - let view = provider.configuration_view( - ConfigurationViewTargetAgent::ZedAgent, - window, - cx, - ); + let view = provider.configuration_view_v2(ConfigurationViewTargetAgent::ZedAgent, window, cx); // Store the view for future renders by deferring a mutation let provider_id = provider_id.clone(); diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 5c25760103dbb5..5ed4762c7a5cd3 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -781,10 +781,14 @@ pub struct SettingsWindow { pub(crate) hidden_deleted_skill_directory_paths: HashSet, pub(crate) regex_validation_error: Option, last_copied_link_path: Option<&'static str>, - pub(crate) expanded_provider_configurations: - HashMap, - pub(crate) provider_configuration_views: - HashMap, + /// Cached configuration views per provider, created lazily. Holds the + /// provider's chosen presentation ([`Inline`] or [`SubPage`]). + pub(crate) provider_configuration_views: HashMap< + language_model::LanguageModelProviderId, + language_model::ProviderConfigurationView, + >, + /// The provider whose configuration sub-page is currently open, if any. + pub(crate) configuring_provider: Option, /// Directory path of the skill whose share link was most recently copied, /// used to show a transient "copied" checkmark on its share button. pub(crate) last_copied_skill_directory_path: Option, @@ -796,6 +800,10 @@ pub struct SettingsWindow { pub(crate) mcp_add_server_focus_handle: FocusHandle, /// State for the active "add/edit custom external agent" form sub-page, if open. pub(crate) custom_agent_form: Option, + /// Stable focus handle for the external agents "Add Agent" button, so it can + /// show a focus ring when the page auto-focuses it on open (which happens via + /// mouse, where `focus_visible` styling would otherwise be suppressed). + pub(crate) external_agent_add_focus_handle: FocusHandle, } struct SearchDocument { @@ -1825,12 +1833,13 @@ impl SettingsWindow { regex_validation_error: None, list_state, last_copied_link_path: None, - expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), + configuring_provider: None, last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), custom_agent_form: None, + external_agent_add_focus_handle: cx.focus_handle(), }; this.fetch_files(window, cx); @@ -4732,12 +4741,13 @@ pub mod test { hidden_deleted_skill_directory_paths: HashSet::default(), regex_validation_error: None, last_copied_link_path: None, - expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), + configuring_provider: None, last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), custom_agent_form: None, + external_agent_add_focus_handle: cx.focus_handle(), } } } @@ -4865,12 +4875,13 @@ pub mod test { hidden_deleted_skill_directory_paths: HashSet::default(), regex_validation_error: None, last_copied_link_path: None, - expanded_provider_configurations: HashMap::default(), provider_configuration_views: HashMap::default(), + configuring_provider: None, last_copied_skill_directory_path: None, mcp_server_form: None, mcp_add_server_focus_handle: cx.focus_handle(), custom_agent_form: None, + external_agent_add_focus_handle: cx.focus_handle(), }; settings_window.build_filter_table(); From 5c607876992b68a2f4cc86c194abe79a60c21915 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 16 Jun 2026 13:38:35 +0100 Subject: [PATCH 12/16] polish --- crates/feature_flags/src/flags.rs | 4 - .../src/pages/external_agents_page.rs | 149 ++++++----- .../settings_ui/src/pages/mcp_servers_page.rs | 243 ++++++++++++------ 3 files changed, 253 insertions(+), 143 deletions(-) diff --git a/crates/feature_flags/src/flags.rs b/crates/feature_flags/src/flags.rs index 69e6cd108ca544..41ff35447e306e 100644 --- a/crates/feature_flags/src/flags.rs +++ b/crates/feature_flags/src/flags.rs @@ -164,10 +164,6 @@ pub struct AgentSettingsUiFeatureFlag; impl FeatureFlag for AgentSettingsUiFeatureFlag { const NAME: &'static str = "agent-settings-ui"; type Value = PresenceFlag; - - fn enabled_for_staff() -> bool { - false - } } register_feature_flag!(AgentSettingsUiFeatureFlag); diff --git a/crates/settings_ui/src/pages/external_agents_page.rs b/crates/settings_ui/src/pages/external_agents_page.rs index 65e341e70d621b..3369b58be0bf8d 100644 --- a/crates/settings_ui/src/pages/external_agents_page.rs +++ b/crates/settings_ui/src/pages/external_agents_page.rs @@ -526,12 +526,50 @@ fn render_custom_agent_form_page( let fields = v_flex() .w_full() - .max_w(rems(36.)) - .gap_3() - .child(labeled_field("Agent Name", true, &form.name, cx)) - .child(labeled_field("Command", true, &form.command, cx)) - .child(labeled_field("Arguments", false, &form.args, cx)) - .child(render_env_section(&form.env, cx)) + .gap_4() + .child( + crate::render_settings_item_layout( + settings_window, + "Agent Name", + "Required. A unique name used to identify this agent.", + input_box(&form.name, cx).into_any_element(), + None, + None, + None, + false, + cx, + ) + .into_any_element(), + ) + .child( + crate::render_settings_item_layout( + settings_window, + "Command", + "Required. Path to the executable that launches the agent.", + input_box(&form.command, cx).into_any_element(), + None, + None, + None, + false, + cx, + ) + .into_any_element(), + ) + .child( + crate::render_settings_item_layout( + settings_window, + "Arguments", + "Space-separated arguments passed to the command.", + input_box(&form.args, cx).into_any_element(), + None, + None, + None, + false, + cx, + ) + .into_any_element(), + ) + .child(render_env_section(settings_window, &form.env, cx)) .when_some(error, |this, error| this.child(render_form_error(error))) .child(render_form_actions(form, window, cx)); @@ -547,19 +585,6 @@ fn render_custom_agent_form_page( .into_any_element() } -fn field_label(label: &str, required: bool) -> impl IntoElement { - h_flex() - .gap_0p5() - .child( - Label::new(label.to_string()) - .size(LabelSize::Small) - .color(Color::Muted), - ) - .when(required, |this| { - this.child(Label::new("*").size(LabelSize::Small).color(Color::Error)) - }) -} - fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { let colors = cx.theme().colors(); // All form inputs share tab index 0, so tab order follows render (insertion) @@ -567,8 +592,7 @@ fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { // routes keyboard focus into the editor when tabbed to. let focus_handle = editor.focus_handle(cx).tab_index(0).tab_stop(true); h_flex() - .w_full() - .min_w_0() + .min_w_64() .py_1() .px_2() .h_8() @@ -581,46 +605,39 @@ fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { .child(editor.clone()) } -fn labeled_field( - label: &str, - required: bool, - editor: &Entity, - cx: &App, +fn render_env_section( + settings_window: &SettingsWindow, + rows: &[KeyValueRow], + cx: &mut Context, ) -> impl IntoElement { - v_flex() - .w_full() - .gap_1() - .child(field_label(label, required)) - .child(input_box(editor, cx)) -} - -fn render_env_section(rows: &[KeyValueRow], cx: &mut Context) -> impl IntoElement { - v_flex() - .w_full() - .gap_1() - .child(field_label("Environment Variables", false)) + // The right-hand control column is narrower than a full row, so each + // variable stacks its key above its value (with the remove affordance next + // to the value) to stay readable. + let control = v_flex() + .min_w_64() + .gap_2() .children(rows.iter().enumerate().map(|(ix, row)| { - h_flex() - .w_full() - .gap_1() - .items_center() - .child(div().flex_1().min_w_0().child(input_box(&row.key, cx))) - .child(div().flex_1().min_w_0().child(input_box(&row.value, cx))) - .child( - IconButton::new(("custom-agent-env-remove", ix), IconName::Close) - .icon_size(IconSize::Small) - .icon_color(Color::Muted) - .tab_index(0isize) - .tooltip(Tooltip::text("Remove")) - .on_click(cx.listener(move |this, _, _window, cx| { - if let Some(form) = this.custom_agent_form.as_mut() - && ix < form.env.len() - { - form.env.remove(ix); - } - cx.notify(); - })), - ) + v_flex().gap_1().child(input_box(&row.key, cx)).child( + h_flex() + .gap_1() + .items_center() + .child(input_box(&row.value, cx)) + .child( + IconButton::new(("custom-agent-env-remove", ix), IconName::Close) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tab_index(0isize) + .tooltip(Tooltip::text("Remove")) + .on_click(cx.listener(move |this, _, _window, cx| { + if let Some(form) = this.custom_agent_form.as_mut() + && ix < form.env.len() + { + form.env.remove(ix); + } + cx.notify(); + })), + ), + ) })) .child( Button::new("custom-agent-env-add", "Add") @@ -644,6 +661,20 @@ fn render_env_section(rows: &[KeyValueRow], cx: &mut Context) -> cx.notify(); })), ) + .into_any_element(); + + crate::render_settings_item_layout( + settings_window, + "Environment Variables", + "Environment variables provided to the agent process.", + control, + None, + None, + None, + false, + cx, + ) + .into_any_element() } fn render_form_error(error: SharedString) -> impl IntoElement { diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs index 21f668b09c9adf..8a3aa033d27745 100644 --- a/crates/settings_ui/src/pages/mcp_servers_page.rs +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -123,9 +123,9 @@ fn render_server_list( .w_full() .gap_1() .children(itertools::intersperse_with( - server_ids.iter().map(|server_id| { - render_context_server(server_id, store, cx).into_any_element() - }), + server_ids + .iter() + .map(|server_id| render_context_server(server_id, store, cx).into_any_element()), || { Divider::horizontal() .color(DividerColor::BorderFaded) @@ -192,7 +192,10 @@ fn render_context_server( // settings (not the resolved runtime configuration) so the form is editable // even when the settings contain invalid data (e.g. an unparseable URL) or // the server is disabled / not yet started. - let server_settings = store.read(cx).settings_for_server(context_server_id).cloned(); + let server_settings = store + .read(cx) + .settings_for_server(context_server_id) + .cloned(); let gear_menu = render_gear_menu( context_server_id, store, @@ -233,10 +236,7 @@ fn map_server_status(status: &ContextServerStatus) -> AiSettingItemStatus { } } -fn resolve_extension_display_name( - id: &ContextServerId, - cx: &App, -) -> Option { +fn resolve_extension_display_name(id: &ContextServerId, cx: &App) -> Option { ExtensionStore::global(cx) .read(cx) .installed_extensions() @@ -631,7 +631,8 @@ fn uninstall_server( cx: &mut App, ) { if provided_by_extension { - if let Some((ext_id, manifest)) = resolve_extension_for_context_server(context_server_id, cx) + if let Some((ext_id, manifest)) = + resolve_extension_for_context_server(context_server_id, cx) { if extension_only_provides_context_server(&manifest) { ExtensionStore::global(cx) @@ -795,7 +796,12 @@ impl McpServerForm { name: new_input("my-mcp-server", name_initial.as_deref(), window, cx), command: new_input("/path/to/server", command_initial.as_deref(), window, cx), args: new_input("--flag value", args_initial.as_deref(), window, cx), - url: new_input("https://example.com/mcp", url_initial.as_deref(), window, cx), + url: new_input( + "https://example.com/mcp", + url_initial.as_deref(), + window, + cx, + ), timeout: new_input("60", timeout_initial.as_deref(), window, cx), oauth_client_id: new_input( "Optional OAuth client ID", @@ -894,32 +900,72 @@ fn render_mcp_server_form_page( let fields = v_flex() .w_full() - .max_w(rems(36.)) - .gap_3() - .child(labeled_field("Server Name", true, &form.name, cx)) + .gap_4() + .child(render_form_field( + settings_window, + "Server Name", + "Required. A unique name used to identify this MCP server.", + &form.name, + cx, + )) .map(|this| match transport { McpTransport::Stdio => this - .child(labeled_field("Command", true, &form.command, cx)) - .child(labeled_field("Arguments", false, &form.args, cx)) + .child(render_form_field( + settings_window, + "Command", + "Required. Path to the executable that launches the server.", + &form.command, + cx, + )) + .child(render_form_field( + settings_window, + "Arguments", + "Space-separated arguments passed to the command.", + &form.args, + cx, + )) .child(render_kv_section( + settings_window, "Environment Variables", + "Environment variables provided to the server process.", &form.env, McpKvKind::Env, cx, )) - .child(labeled_field("Timeout (seconds)", false, &form.timeout, cx)), + .child(render_form_field( + settings_window, + "Timeout (seconds)", + "How long to wait for the server to respond before timing out.", + &form.timeout, + cx, + )), McpTransport::Http => this - .child(labeled_field("URL", true, &form.url, cx)) + .child(render_form_field( + settings_window, + "URL", + "Required. The base URL of the remote MCP server.", + &form.url, + cx, + )) .child(render_kv_section( + settings_window, "Headers", + "HTTP headers sent with each request to the server.", &form.headers, McpKvKind::Header, cx, )) - .child(labeled_field("Timeout (seconds)", false, &form.timeout, cx)) - .child(labeled_field( + .child(render_form_field( + settings_window, + "Timeout (seconds)", + "How long to wait for the server to respond before timing out.", + &form.timeout, + cx, + )) + .child(render_form_field( + settings_window, "OAuth Client ID", - false, + "Optional OAuth client ID used to authenticate with the server.", &form.oauth_client_id, cx, )), @@ -939,19 +985,6 @@ fn render_mcp_server_form_page( .into_any_element() } -fn field_label(label: &str, required: bool) -> impl IntoElement { - h_flex() - .gap_0p5() - .child( - Label::new(label.to_string()) - .size(LabelSize::Small) - .color(Color::Muted), - ) - .when(required, |this| { - this.child(Label::new("*").size(LabelSize::Small).color(Color::Error)) - }) -} - fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { let colors = cx.theme().colors(); // All form inputs share tab index 0, so tab order follows render (insertion) @@ -959,8 +992,7 @@ fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { // routes keyboard focus into the editor when tabbed to. let focus_handle = editor.focus_handle(cx).tab_index(0).tab_stop(true); h_flex() - .w_full() - .min_w_0() + .min_w_64() .py_1() .px_2() .h_8() @@ -973,51 +1005,64 @@ fn input_box(editor: &Entity, cx: &App) -> impl IntoElement { .child(editor.clone()) } -fn labeled_field( - label: &str, - required: bool, +fn render_form_field( + settings_window: &SettingsWindow, + title: &'static str, + description: &'static str, editor: &Entity, - cx: &App, -) -> impl IntoElement { - v_flex() - .w_full() - .gap_1() - .child(field_label(label, required)) - .child(input_box(editor, cx)) + cx: &mut Context, +) -> AnyElement { + let control = input_box(editor, cx).into_any_element(); + crate::render_settings_item_layout( + settings_window, + title, + description, + control, + None, + None, + None, + false, + cx, + ) + .into_any_element() } fn render_kv_section( - label: &str, + settings_window: &SettingsWindow, + title: &'static str, + description: &'static str, rows: &[KeyValueRow], kind: McpKvKind, cx: &mut Context, -) -> impl IntoElement { - v_flex() - .w_full() - .gap_1() - .child(field_label(label, false)) +) -> AnyElement { + let control = v_flex() + .min_w_64() + .gap_2() .children(rows.iter().enumerate().map(|(ix, row)| { - h_flex() - .w_full() + v_flex() .gap_1() - .items_center() - .child(div().flex_1().min_w_0().child(input_box(&row.key, cx))) - .child(div().flex_1().min_w_0().child(input_box(&row.value, cx))) .child( - IconButton::new((kind.remove_id(), ix), IconName::Close) - .icon_size(IconSize::Small) - .icon_color(Color::Muted) - .tooltip(Tooltip::text("Remove")) - .on_click(cx.listener(move |this, _, _window, cx| { - if let Some(form) = this.mcp_server_form.as_mut() { - let rows = kind.rows_mut(form); - if ix < rows.len() { - rows.remove(ix); - } - } - cx.notify(); - })), + h_flex() + .gap_1() + .items_center() + .child(div().flex_1().min_w_0().child(input_box(&row.key, cx))) + .child( + IconButton::new((kind.remove_id(), ix), IconName::Close) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tooltip(Tooltip::text("Remove")) + .on_click(cx.listener(move |this, _, _window, cx| { + if let Some(form) = this.mcp_server_form.as_mut() { + let rows = kind.rows_mut(form); + if ix < rows.len() { + rows.remove(ix); + } + } + cx.notify(); + })), + ), ) + .child(input_box(&row.value, cx)) })) .child( Button::new(kind.add_id(), "Add") @@ -1036,6 +1081,20 @@ fn render_kv_section( cx.notify(); })), ) + .into_any_element(); + + crate::render_settings_item_layout( + settings_window, + title, + description, + control, + None, + None, + None, + false, + cx, + ) + .into_any_element() } fn render_form_error(error: SharedString) -> impl IntoElement { @@ -1048,11 +1107,7 @@ fn render_form_error(error: SharedString) -> impl IntoElement { .size(IconSize::Small) .color(Color::Error), ) - .child( - Label::new(error) - .size(LabelSize::Small) - .color(Color::Error), - ) + .child(Label::new(error).size(LabelSize::Small).color(Color::Error)) } fn render_form_actions(cx: &mut Context) -> impl IntoElement { @@ -1123,7 +1178,10 @@ fn save_mcp_server_form( { settings.project.context_servers.remove(&original_id.0); } - settings.project.context_servers.insert(id.0.clone(), content); + settings + .project + .context_servers + .insert(id.0.clone(), content); }); settings_window.mcp_server_form = None; @@ -1148,7 +1206,14 @@ struct McpServerFormValues { fn build_settings_from_form( form: &McpServerForm, cx: &App, -) -> Result<(ContextServerId, Option, ContextServerSettingsContent), SharedString> { +) -> Result< + ( + ContextServerId, + Option, + ContextServerSettingsContent, + ), + SharedString, +> { let values = McpServerFormValues { transport: form.transport, original_id: form.original_id.clone(), @@ -1172,7 +1237,14 @@ fn read_kv(rows: &[KeyValueRow], cx: &App) -> Vec<(String, String)> { fn build_settings_from_values( values: &McpServerFormValues, -) -> Result<(ContextServerId, Option, ContextServerSettingsContent), SharedString> { +) -> Result< + ( + ContextServerId, + Option, + ContextServerSettingsContent, + ), + SharedString, +> { let name = values.name.trim().to_string(); if name.is_empty() { return Err("Server name is required.".into()); @@ -1230,7 +1302,11 @@ fn build_settings_from_values( } }; - Ok((ContextServerId(name.into()), values.original_id.clone(), content)) + Ok(( + ContextServerId(name.into()), + values.original_id.clone(), + content, + )) } /// Returns a human-readable error when a server's configured settings are @@ -1352,7 +1428,10 @@ mod tests { let mut values = values(McpTransport::Http); values.url = "not a url".into(); let error = build_settings_from_values(&values).unwrap_err(); - assert!(error.starts_with("Invalid URL"), "unexpected error: {error}"); + assert!( + error.starts_with("Invalid URL"), + "unexpected error: {error}" + ); } #[test] @@ -1482,7 +1561,11 @@ mod tests { // New server taking an existing name collides. assert!(name_collides_with_other_server(&id("foo"), None, &existing)); // New server with a free name is fine. - assert!(!name_collides_with_other_server(&id("baz"), None, &existing)); + assert!(!name_collides_with_other_server( + &id("baz"), + None, + &existing + )); // Editing a server in place is allowed even though the name "exists". assert!(!name_collides_with_other_server( &id("foo"), From 0c04f7597b66ce7f4e2d21865011bc60c27a638c Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 16 Jun 2026 13:39:04 +0100 Subject: [PATCH 13/16] fmt --- .../language_models/src/provider/anthropic.rs | 6 ++---- crates/language_models/src/provider/deepseek.rs | 7 +++---- crates/language_models/src/provider/google.rs | 4 ++-- crates/language_models/src/provider/mistral.rs | 6 +++--- crates/language_models/src/provider/open_ai.rs | 4 ++-- .../language_models/src/provider/open_router.rs | 7 +++---- .../src/provider/vercel_ai_gateway.rs | 7 +++---- crates/language_models/src/provider/x_ai.rs | 3 +-- crates/settings_ui/src/settings_ui.rs | 17 ++++++----------- 9 files changed, 25 insertions(+), 36 deletions(-) diff --git a/crates/language_models/src/provider/anthropic.rs b/crates/language_models/src/provider/anthropic.rs index c6372a6f2ab09e..726de389fa90bf 100644 --- a/crates/language_models/src/provider/anthropic.rs +++ b/crates/language_models/src/provider/anthropic.rs @@ -8,14 +8,12 @@ use futures::{FutureExt, StreamExt, future::BoxFuture, stream::BoxStream}; use gpui::{AnyView, App, AsyncApp, Context, Entity, Task, TaskExt}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ANTHROPIC_PROVIDER_ID, ANTHROPIC_PROVIDER_NAME, ApiKeyState, - AuthenticateError, + ANTHROPIC_PROVIDER_ID, ANTHROPIC_PROVIDER_NAME, ApiKeyState, AuthenticateError, ConfigurationViewTargetAgent, EnvVar, FastModeConfirmation, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, - ProviderConfigurationView, RateLimiter, - env_var, + ProviderConfigurationView, RateLimiter, env_var, }; use settings::{Settings, SettingsStore}; use std::sync::{Arc, LazyLock}; diff --git a/crates/language_models/src/provider/deepseek.rs b/crates/language_models/src/provider/deepseek.rs index f6e9ef122cbca1..098777ad1a7c86 100644 --- a/crates/language_models/src/provider/deepseek.rs +++ b/crates/language_models/src/provider/deepseek.rs @@ -8,13 +8,12 @@ use futures::{FutureExt, StreamExt, future::BoxFuture, stream::BoxStream}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, - LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, - LanguageModelToolResultContent, LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, - StopReason, TokenUsage, env_var, + LanguageModelToolResultContent, LanguageModelToolUse, MessageContent, + ProviderConfigurationView, RateLimiter, Role, StopReason, TokenUsage, env_var, }; pub use settings::DeepseekAvailableModel as AvailableModel; use settings::{Settings, SettingsStore}; diff --git a/crates/language_models/src/provider/google.rs b/crates/language_models/src/provider/google.rs index 87a6551480b56b..7ea54a51ba3306 100644 --- a/crates/language_models/src/provider/google.rs +++ b/crates/language_models/src/provider/google.rs @@ -7,9 +7,9 @@ pub use google_ai::completion::{GoogleEventMapper, into_google}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - AuthenticateError, ConfigurationViewTargetAgent, EnvVar, - LanguageModelCompletionError, ProviderConfigurationView, + AuthenticateError, ConfigurationViewTargetAgent, EnvVar, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelToolChoice, LanguageModelToolSchemaFormat, + ProviderConfigurationView, }; use language_model::{ GOOGLE_PROVIDER_ID, GOOGLE_PROVIDER_NAME, IconOrSvg, LanguageModel, LanguageModelEffortLevel, diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index 392db2bc55e96a..6acb7ec2ddd62b 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -6,12 +6,12 @@ use futures::{FutureExt, Stream, StreamExt, future::BoxFuture, stream::BoxStream use gpui::{AnyView, App, AsyncApp, Context, Entity, Global, SharedString, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, - LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, TokenUsage, env_var, + LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, StopReason, + TokenUsage, env_var, }; pub use mistral::{MISTRAL_API_URL, StreamResponse}; pub use settings::MistralAvailableModel as AvailableModel; diff --git a/crates/language_models/src/provider/open_ai.rs b/crates/language_models/src/provider/open_ai.rs index c62d2ad8736aa3..6d66c2f070a484 100644 --- a/crates/language_models/src/provider/open_ai.rs +++ b/crates/language_models/src/provider/open_ai.rs @@ -6,11 +6,11 @@ use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt, use http_client::{CustomHeaders, HttpClient}; use language_model::{ ApiKeyState, AuthenticateError, EnvVar, FastModeConfirmation, IconOrSvg, LanguageModel, - ProviderConfigurationView, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, - LanguageModelToolChoice, OPEN_AI_PROVIDER_ID, OPEN_AI_PROVIDER_NAME, RateLimiter, env_var, + LanguageModelToolChoice, OPEN_AI_PROVIDER_ID, OPEN_AI_PROVIDER_NAME, ProviderConfigurationView, + RateLimiter, env_var, }; use menu; use open_ai::{ diff --git a/crates/language_models/src/provider/open_router.rs b/crates/language_models/src/provider/open_router.rs index 1eebe583e60429..857012d456750c 100644 --- a/crates/language_models/src/provider/open_router.rs +++ b/crates/language_models/src/provider/open_router.rs @@ -5,13 +5,12 @@ use futures::{FutureExt, Stream, StreamExt, future::BoxFuture}; use gpui::{AnyView, App, AsyncApp, Context, Entity, SharedString, Task, TaskExt}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, - LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, ProviderConfigurationView, RateLimiter, Role, - StopReason, TokenUsage, env_var, + LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, ProviderConfigurationView, + RateLimiter, Role, StopReason, TokenUsage, env_var, }; use open_router::{ Model, ModelMode as OpenRouterModelMode, OPEN_ROUTER_API_URL, ResponseStreamEvent, list_models, diff --git a/crates/language_models/src/provider/vercel_ai_gateway.rs b/crates/language_models/src/provider/vercel_ai_gateway.rs index e88dadfd6daa8c..21f0b7c7669d83 100644 --- a/crates/language_models/src/provider/vercel_ai_gateway.rs +++ b/crates/language_models/src/provider/vercel_ai_gateway.rs @@ -7,12 +7,11 @@ use http_client::{ AsyncBody, CustomHeaders, HttpClient, Method, Request as HttpRequest, RequestBuilderExt, http, }; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, - LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, - LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolSchemaFormat, ProviderConfigurationView, RateLimiter, - env_var, + LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolSchemaFormat, + ProviderConfigurationView, RateLimiter, env_var, }; use open_ai::ResponseStreamEvent; use serde::Deserialize; diff --git a/crates/language_models/src/provider/x_ai.rs b/crates/language_models/src/provider/x_ai.rs index d1339b3b78068e..f66e31aa8395c6 100644 --- a/crates/language_models/src/provider/x_ai.rs +++ b/crates/language_models/src/provider/x_ai.rs @@ -5,8 +5,7 @@ use futures::{FutureExt, StreamExt, future::BoxFuture}; use gpui::{AnyView, App, AsyncApp, Context, Entity, Task, TaskExt, Window}; use http_client::{CustomHeaders, HttpClient}; use language_model::{ - ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, - LanguageModelCompletionError, + ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 5ed4762c7a5cd3..92f09830de7ece 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -129,8 +129,8 @@ struct SettingField { } enum SettingsPath { - Json(&'static str), // a.b.c - Subpage(&'static str), // a/b/c + Json(&'static str), // a.b.c + Subpage(&'static str), // a/b/c } impl Clone for SettingField { @@ -783,10 +783,8 @@ pub struct SettingsWindow { last_copied_link_path: Option<&'static str>, /// Cached configuration views per provider, created lazily. Holds the /// provider's chosen presentation ([`Inline`] or [`SubPage`]). - pub(crate) provider_configuration_views: HashMap< - language_model::LanguageModelProviderId, - language_model::ProviderConfigurationView, - >, + pub(crate) provider_configuration_views: + HashMap, /// The provider whose configuration sub-page is currently open, if any. pub(crate) configuring_provider: Option, /// Directory path of the skill whose share link was most recently copied, @@ -1517,8 +1515,7 @@ pub(crate) struct NonJsonItem { files: FileMask, can_reset: fn(&App) -> bool, reset: fn(&mut Window, &mut App), - render_control: - fn(&SettingsWindow, &mut Window, &mut Context) -> AnyElement, + render_control: fn(&SettingsWindow, &mut Window, &mut Context) -> AnyElement, } impl PartialEq for NonJsonItem { @@ -1959,9 +1956,7 @@ impl SettingsWindow { window: &mut Window, cx: &mut Context| { if this.sub_page_stack.is_empty() { - this.open_and_scroll_to_navbar_entry( - entry_index, None, false, window, cx, - ); + this.open_and_scroll_to_navbar_entry(entry_index, None, false, window, cx); } }, ); From 6f790bdda4b864176882085f979064a196b92beb Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 16 Jun 2026 14:47:32 +0100 Subject: [PATCH 14/16] is that even a spelling mistake?? --- crates/settings_ui/src/pages/mcp_servers_page.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/settings_ui/src/pages/mcp_servers_page.rs b/crates/settings_ui/src/pages/mcp_servers_page.rs index 8a3aa033d27745..d31e158ce1e6b6 100644 --- a/crates/settings_ui/src/pages/mcp_servers_page.rs +++ b/crates/settings_ui/src/pages/mcp_servers_page.rs @@ -190,7 +190,7 @@ fn render_context_server( // Build gear menu. Pre-fill "Configure Server" from the raw configured // settings (not the resolved runtime configuration) so the form is editable - // even when the settings contain invalid data (e.g. an unparseable URL) or + // even when the settings contain invalid data (e.g. an unparsable URL) or // the server is disabled / not yet started. let server_settings = store .read(cx) From 7465dc3c3c35d8d97021dd0a72c8b1a7ea1ab706 Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 16 Jun 2026 15:08:51 +0100 Subject: [PATCH 15/16] clippy --- crates/settings_ui/src/settings_ui.rs | 91 +-------------------------- 1 file changed, 1 insertion(+), 90 deletions(-) diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 740fe33df3e568..a1b69c9a7beefe 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -137,11 +137,6 @@ struct SettingField { json_path: Option<&'static str>, } -enum SettingsPath { - Json(&'static str), // a.b.c - Subpage(&'static str), // a/b/c -} - impl Clone for SettingField { fn clone(&self) -> Self { *self @@ -993,7 +988,6 @@ enum SettingsPageItem { SubPageLink(SubPageLink), DynamicItem(DynamicItem), ActionLink(ActionLink), - NonJson(NonJsonItem), } impl std::fmt::Debug for SettingsPageItem { @@ -1012,9 +1006,6 @@ impl std::fmt::Debug for SettingsPageItem { SettingsPageItem::ActionLink(action_link) => { write!(f, "ActionLink({})", action_link.title) } - SettingsPageItem::NonJson(non_json_item) => { - write!(f, "NonJson({})", non_json_item.title) - } } } } @@ -1305,17 +1296,6 @@ impl SettingsPageItem { ) .when(bottom_border, |this| this.child(Divider::horizontal())) .into_any_element(), - SettingsPageItem::NonJson(non_json_item) => { - let field = render_non_json_item(settings_window, non_json_item, window, cx); - let field_with_padding = apply_padding(field); - - v_flex() - .group("setting-item") - .px_8() - .child(field_with_padding) - .when(bottom_border, |this| this.child(Divider::horizontal())) - .into_any_element() - } } } } @@ -1453,34 +1433,6 @@ fn render_settings_item( ) } -pub(crate) fn render_non_json_item( - settings_window: &SettingsWindow, - item: &NonJsonItem, - window: &mut Window, - cx: &mut Context<'_, SettingsWindow>, -) -> Stateful
{ - let control = (item.render_control)(settings_window, window, cx); - - let reset_fn: Option> = if (item.can_reset)(cx) { - let reset = item.reset; - Some(Box::new(move |window, cx| reset(window, cx))) - } else { - None - }; - - render_settings_item_layout( - settings_window, - item.title, - item.description, - control, - reset_fn, - None, - item.json_path, - false, - cx, - ) -} - fn render_settings_item_link( id: impl Into, json_path: Option<&'static str>, @@ -1650,24 +1602,6 @@ impl PartialEq for ActionLink { } } -pub(crate) struct NonJsonItem { - title: &'static str, - description: &'static str, - /// A stable path identifier for deep-linking and search, even though this - /// setting is not stored in settings.json. - json_path: Option<&'static str>, - files: FileMask, - can_reset: fn(&App) -> bool, - reset: fn(&mut Window, &mut App), - render_control: fn(&SettingsWindow, &mut Window, &mut Context) -> AnyElement, -} - -impl PartialEq for NonJsonItem { - fn eq(&self, other: &Self) -> bool { - self.title == other.title - } -} - fn all_language_names(cx: &App) -> Vec { let state = workspace::AppState::global(cx); state @@ -2169,8 +2103,7 @@ impl SettingsWindow { | SettingsPageItem::DynamicItem(DynamicItem { discriminant: SettingItem { files, .. }, .. - }) - | SettingsPageItem::NonJson(NonJsonItem { files, .. }) => { + }) => { if !files.contains(current_file) { page_filter[index] = false; } else { @@ -2432,28 +2365,6 @@ impl SettingsWindow { action_link.title.as_ref(), ); } - SettingsPageItem::NonJson(non_json_item) => { - json_path = non_json_item.json_path; - documents.push(SearchDocument { - id: key_index, - words: split_into_words(&[ - page.title, - header_str, - non_json_item.title, - non_json_item.description, - ]), - }); - push_candidates( - &mut fuzzy_match_candidates, - key_index, - non_json_item.title, - ); - push_candidates( - &mut fuzzy_match_candidates, - key_index, - non_json_item.description, - ); - } } push_candidates(&mut fuzzy_match_candidates, key_index, page.title); push_candidates(&mut fuzzy_match_candidates, key_index, header_str); From 84590f9a1b7352d3460d03ec094ced3c66814d4c Mon Sep 17 00:00:00 2001 From: cameron Date: Tue, 16 Jun 2026 15:48:08 +0100 Subject: [PATCH 16/16] clippy mac maybe???? --- crates/zed/src/visual_test_runner.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 609f5c180eb9a3..0f4e84abcfc9ce 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -2438,6 +2438,7 @@ fn run_tool_permissions_visual_tests( "Terminal", "Configure Tool Rules", None, + true, settings_ui::pages::render_terminal_tool_config, window, cx,