From ab80ef18458417ac010f5b6de9c51be3bca9fac3 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Tue, 25 Nov 2025 17:03:21 +0100 Subject: [PATCH] mcp: Fix `source` property showing up as undefined in settings (#43417) Follow up to #39021. image - Add migration to remove `source` tag because `ContextServerSettings` is now untagged - Fix typos in context server modal - PR seems to have removed the `test_action_namespaces` test, which I brought back in this PR Release Notes: - Fixed an issue where the `source` property of MCP settings would show up as unrecognised --- crates/agent/src/tests/mod.rs | 2 +- .../configure_context_server_modal.rs | 11 +- .../src/wasm_host/wit/since_v0_6_0.rs | 2 +- crates/migrator/src/migrations.rs | 6 + .../src/migrations/m_2025_11_25/settings.rs | 17 +++ crates/migrator/src/migrator.rs | 55 +++++++- crates/project/src/context_server_store.rs | 14 +- crates/project/src/project_settings.rs | 38 +++--- .../settings/src/settings_content/project.rs | 4 +- crates/settings/src/vscode_import.rs | 2 +- crates/zed/src/zed.rs | 127 ++++++++++++++++++ docs/src/ai/mcp.md | 4 +- 12 files changed, 237 insertions(+), 45 deletions(-) create mode 100644 crates/migrator/src/migrations/m_2025_11_25/settings.rs diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index efba471f1a927446aa96b1c1426c60b42b725b89..b33080671980eb28c7900aea4bb0942d152a054a 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -2553,7 +2553,7 @@ fn setup_context_server( let mut settings = ProjectSettings::get_global(cx).clone(); settings.context_servers.insert( name.into(), - project::project_settings::ContextServerSettings::Custom { + project::project_settings::ContextServerSettings::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), diff --git a/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs b/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs index ebea8c25fb68a8a5055d4ccaa8b9068583c4b91c..a93df3839d98d95e2f91833078dbe96bc3fb8889 100644 --- a/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs +++ b/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs @@ -182,7 +182,7 @@ impl ConfigurationSource { parse_input(&editor.read(cx).text(cx)).map(|(id, command)| { ( id, - ContextServerSettings::Custom { + ContextServerSettings::Stdio { enabled: true, command, }, @@ -403,7 +403,7 @@ impl ConfigureContextServerModal { window.spawn(cx, async move |cx| { let target = match settings { - ContextServerSettings::Custom { + ContextServerSettings::Stdio { enabled: _, command, } => Some(ConfigurationTarget::Existing { @@ -635,7 +635,6 @@ impl ConfigureContextServerModal { } fn render_modal_content(&self, cx: &App) -> AnyElement { - // All variants now use single editor approach let editor = match &self.source { ConfigurationSource::New { editor, .. } => editor, ConfigurationSource::Existing { editor, .. } => editor, @@ -712,12 +711,12 @@ impl ConfigureContextServerModal { ) } else if let ConfigurationSource::New { is_http, .. } = &self.source { let label = if *is_http { - "Run command" + "Configure Local" } else { - "Connect via HTTP" + "Configure Remote" }; let tooltip = if *is_http { - "Configure an MCP serevr that runs on stdin/stdout." + "Configure an MCP server that runs on stdin/stdout." } else { "Configure an MCP server that you connect to over HTTP" }; diff --git a/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs b/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs index dd0548d9d182e4b81e8490476eef2420f0e6c13d..c96e5216c4703df2a73e1a0bc27c90d13adbb782 100644 --- a/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs +++ b/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs @@ -972,7 +972,7 @@ impl ExtensionImports for WasmState { }); match settings { - project::project_settings::ContextServerSettings::Custom { + project::project_settings::ContextServerSettings::Stdio { enabled: _, command, } => Ok(serde_json::to_string(&settings::ContextServerSettings { diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 8a481c734f9efcce4f6342789df6ff1d7fc01562..07b7d3f0afb141d4dde77b883ca97f4df67cdd6c 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -147,3 +147,9 @@ pub(crate) mod m_2025_11_20 { pub(crate) use settings::SETTINGS_PATTERNS; } + +pub(crate) mod m_2025_11_25 { + mod settings; + + pub(crate) use settings::remove_context_server_source; +} diff --git a/crates/migrator/src/migrations/m_2025_11_25/settings.rs b/crates/migrator/src/migrations/m_2025_11_25/settings.rs new file mode 100644 index 0000000000000000000000000000000000000000..944eee8a119714b7d9839e2ddf13ec61db4c18d2 --- /dev/null +++ b/crates/migrator/src/migrations/m_2025_11_25/settings.rs @@ -0,0 +1,17 @@ +use anyhow::Result; +use serde_json::Value; + +pub fn remove_context_server_source(settings: &mut Value) -> Result<()> { + if let Some(obj) = settings.as_object_mut() { + if let Some(context_servers) = obj.get_mut("context_servers") { + if let Some(servers) = context_servers.as_object_mut() { + for (_, server) in servers.iter_mut() { + if let Some(server_obj) = server.as_object_mut() { + server_obj.remove("source"); + } + } + } + } + } + Ok(()) +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index fd30bf24982d2625e4f40669aa2e0142b8634186..444ebadfb615628e91422ed62c351722d8cb9300 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -223,6 +223,7 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2025_11_20::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_11_20, ), + MigrationType::Json(migrations::m_2025_11_25::remove_context_server_source), ]; run_migrations(text, migrations) } @@ -1334,7 +1335,6 @@ mod tests { r#"{ "context_servers": { "some-mcp-server": { - "source": "custom", "command": { "path": "npx", "args": [ @@ -1354,7 +1354,6 @@ mod tests { r#"{ "context_servers": { "some-mcp-server": { - "source": "custom", "command": "npx", "args": [ "-y", @@ -1376,7 +1375,6 @@ mod tests { r#"{ "context_servers": { "server-with-extras": { - "source": "custom", "command": { "path": "/usr/bin/node", "args": ["server.js"] @@ -1389,7 +1387,6 @@ mod tests { r#"{ "context_servers": { "server-with-extras": { - "source": "custom", "command": "/usr/bin/node", "args": ["server.js"], "settings": {} @@ -1404,7 +1401,6 @@ mod tests { r#"{ "context_servers": { "simple-server": { - "source": "custom", "command": { "path": "simple-mcp-server" } @@ -1415,7 +1411,6 @@ mod tests { r#"{ "context_servers": { "simple-server": { - "source": "custom", "command": "simple-mcp-server" } } @@ -2311,4 +2306,52 @@ mod tests { ), ); } + + #[test] + fn test_remove_context_server_source() { + assert_migrate_settings( + &r#" + { + "context_servers": { + "extension_server": { + "source": "extension", + "settings": { + "foo": "bar" + } + }, + "custom_server": { + "source": "custom", + "command": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + } + } + "# + .unindent(), + Some( + &r#" + { + "context_servers": { + "extension_server": { + "settings": { + "foo": "bar" + } + }, + "custom_server": { + "command": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + } + } + "# + .unindent(), + ), + ); + } } diff --git a/crates/project/src/context_server_store.rs b/crates/project/src/context_server_store.rs index efc2bbf686a273fe18ca3a34f071176d07532981..342a59ab7d5530e8f2268f1c4b72ea44f302f807 100644 --- a/crates/project/src/context_server_store.rs +++ b/crates/project/src/context_server_store.rs @@ -122,7 +122,7 @@ impl ContextServerConfiguration { cx: &AsyncApp, ) -> Option { match settings { - ContextServerSettings::Custom { + ContextServerSettings::Stdio { enabled: _, command, } => Some(ContextServerConfiguration::Custom { command }), @@ -1003,7 +1003,7 @@ mod tests { ), ( server_2_id.0.clone(), - settings::ContextServerSettingsContent::Custom { + settings::ContextServerSettingsContent::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), @@ -1044,7 +1044,7 @@ mod tests { ), ( server_2_id.0.clone(), - settings::ContextServerSettingsContent::Custom { + settings::ContextServerSettingsContent::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), @@ -1127,7 +1127,7 @@ mod tests { json!({"code.rs": ""}), vec![( SERVER_1_ID.into(), - ContextServerSettings::Custom { + ContextServerSettings::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), @@ -1180,7 +1180,7 @@ mod tests { set_context_server_configuration( vec![( server_1_id.0.clone(), - settings::ContextServerSettingsContent::Custom { + settings::ContextServerSettingsContent::Stdio { enabled: false, command: ContextServerCommand { path: "somebinary".into(), @@ -1209,7 +1209,7 @@ mod tests { set_context_server_configuration( vec![( server_1_id.0.clone(), - settings::ContextServerSettingsContent::Custom { + settings::ContextServerSettingsContent::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), @@ -1328,7 +1328,7 @@ mod tests { } fn dummy_server_settings() -> ContextServerSettings { - ContextServerSettings::Custom { + ContextServerSettings::Stdio { enabled: true, command: ContextServerCommand { path: "somebinary".into(), diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index 05d5612f7db5b35e3c2fe6513cc45a05ddaac68c..b7dadc52f74f4800741f5cf537ac9f52c09643e3 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -117,7 +117,7 @@ pub struct GlobalLspSettings { #[derive(Deserialize, Serialize, Clone, PartialEq, Eq, JsonSchema, Debug)] #[serde(tag = "source", rename_all = "snake_case")] pub enum ContextServerSettings { - Custom { + Stdio { /// Whether the context server is enabled. #[serde(default = "default_true")] enabled: bool, @@ -125,16 +125,6 @@ pub enum ContextServerSettings { #[serde(flatten)] command: ContextServerCommand, }, - Extension { - /// Whether the context server is enabled. - #[serde(default = "default_true")] - enabled: bool, - /// The settings for this context server specified by the extension. - /// - /// Consult the documentation for the context server to see what settings - /// are supported. - settings: serde_json::Value, - }, Http { /// Whether the context server is enabled. #[serde(default = "default_true")] @@ -145,13 +135,23 @@ pub enum ContextServerSettings { #[serde(skip_serializing_if = "HashMap::is_empty", default)] headers: HashMap, }, + Extension { + /// Whether the context server is enabled. + #[serde(default = "default_true")] + enabled: bool, + /// The settings for this context server specified by the extension. + /// + /// Consult the documentation for the context server to see what settings + /// are supported. + settings: serde_json::Value, + }, } impl From for ContextServerSettings { fn from(value: settings::ContextServerSettingsContent) -> Self { match value { - settings::ContextServerSettingsContent::Custom { enabled, command } => { - ContextServerSettings::Custom { enabled, command } + settings::ContextServerSettingsContent::Stdio { enabled, command } => { + ContextServerSettings::Stdio { enabled, command } } settings::ContextServerSettingsContent::Extension { enabled, settings } => { ContextServerSettings::Extension { enabled, settings } @@ -171,8 +171,8 @@ impl From for ContextServerSettings { impl Into for ContextServerSettings { fn into(self) -> settings::ContextServerSettingsContent { match self { - ContextServerSettings::Custom { enabled, command } => { - settings::ContextServerSettingsContent::Custom { enabled, command } + ContextServerSettings::Stdio { enabled, command } => { + settings::ContextServerSettingsContent::Stdio { enabled, command } } ContextServerSettings::Extension { enabled, settings } => { settings::ContextServerSettingsContent::Extension { enabled, settings } @@ -200,17 +200,17 @@ impl ContextServerSettings { pub fn enabled(&self) -> bool { match self { - ContextServerSettings::Custom { enabled, .. } => *enabled, - ContextServerSettings::Extension { enabled, .. } => *enabled, + ContextServerSettings::Stdio { enabled, .. } => *enabled, ContextServerSettings::Http { enabled, .. } => *enabled, + ContextServerSettings::Extension { enabled, .. } => *enabled, } } pub fn set_enabled(&mut self, enabled: bool) { match self { - ContextServerSettings::Custom { enabled: e, .. } => *e = enabled, - ContextServerSettings::Extension { enabled: e, .. } => *e = enabled, + ContextServerSettings::Stdio { enabled: e, .. } => *e = enabled, ContextServerSettings::Http { enabled: e, .. } => *e = enabled, + ContextServerSettings::Extension { enabled: e, .. } => *e = enabled, } } } diff --git a/crates/settings/src/settings_content/project.rs b/crates/settings/src/settings_content/project.rs index ccad50ce8827f6d7d59a45b0fb2efd4abb5257b7..0076721228b3b8c6b8d5e6bfd85fc1d25f00c5e3 100644 --- a/crates/settings/src/settings_content/project.rs +++ b/crates/settings/src/settings_content/project.rs @@ -192,7 +192,7 @@ pub struct SessionSettingsContent { #[derive(Deserialize, Serialize, Clone, PartialEq, Eq, JsonSchema, MergeFrom, Debug)] #[serde(untagged, rename_all = "snake_case")] pub enum ContextServerSettingsContent { - Custom { + Stdio { /// Whether the context server is enabled. #[serde(default = "default_true")] enabled: bool, @@ -225,7 +225,7 @@ pub enum ContextServerSettingsContent { impl ContextServerSettingsContent { pub fn set_enabled(&mut self, enabled: bool) { match self { - ContextServerSettingsContent::Custom { + ContextServerSettingsContent::Stdio { enabled: custom_enabled, .. } => { diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index 22081727d8ff767b861a776f0a821e3b4a8d5fdf..7ba07395964266e303965733bdccda42ba7df60e 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -568,7 +568,7 @@ impl VsCodeSettings { .filter_map(|(k, v)| { Some(( k.clone().into(), - ContextServerSettingsContent::Custom { + ContextServerSettingsContent::Stdio { enabled: true, command: serde_json::from_value::(v.clone()) .ok() diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f6348a8cf22bda6441bca6d31abe8823c1d2215a..180f53a46b93eaa93ea355ece256807a16d03f43 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -4686,6 +4686,133 @@ mod tests { }); } + /// Checks that action namespaces are the expected set. The purpose of this is to prevent typos + /// and let you know when introducing a new namespace. + #[gpui::test] + async fn test_action_namespaces(cx: &mut gpui::TestAppContext) { + use itertools::Itertools; + + init_keymap_test(cx); + cx.update(|cx| { + let all_actions = cx.all_action_names(); + + let mut actions_without_namespace = Vec::new(); + let all_namespaces = all_actions + .iter() + .filter_map(|action_name| { + let namespace = action_name + .split("::") + .collect::>() + .into_iter() + .rev() + .skip(1) + .rev() + .join("::"); + if namespace.is_empty() { + actions_without_namespace.push(*action_name); + } + if &namespace == "test_only" || &namespace == "stories" { + None + } else { + Some(namespace) + } + }) + .sorted() + .dedup() + .collect::>(); + assert_eq!(actions_without_namespace, Vec::<&str>::new()); + + let expected_namespaces = vec![ + "action", + "activity_indicator", + "agent", + #[cfg(not(target_os = "macos"))] + "app_menu", + "assistant", + "assistant2", + "auto_update", + "bedrock", + "branches", + "buffer_search", + "channel_modal", + "cli", + "client", + "collab", + "collab_panel", + "command_palette", + "console", + "context_server", + "copilot", + "debug_panel", + "debugger", + "dev", + "diagnostics", + "edit_prediction", + "editor", + "feedback", + "file_finder", + "git", + "git_onboarding", + "git_panel", + "go_to_line", + "icon_theme_selector", + "journal", + "keymap_editor", + "keystroke_input", + "language_selector", + "line_ending_selector", + "lsp_tool", + "markdown", + "menu", + "notebook", + "notification_panel", + "onboarding", + "outline", + "outline_panel", + "pane", + "panel", + "picker", + "project_panel", + "project_search", + "project_symbols", + "projects", + "repl", + "rules_library", + "search", + "settings_editor", + "settings_profile_selector", + "snippets", + "stash_picker", + "supermaven", + "svg", + "syntax_tree_view", + "tab_switcher", + "task", + "terminal", + "terminal_panel", + "theme_selector", + "toast", + "toolchain", + "variable_list", + "vim", + "window", + "workspace", + "zed", + "zed_actions", + "zed_predict_onboarding", + "zeta", + ]; + assert_eq!( + all_namespaces, + expected_namespaces + .into_iter() + .map(|namespace| namespace.to_string()) + .sorted() + .collect::>() + ); + }); + } + #[gpui::test] fn test_bundled_settings_and_themes(cx: &mut App) { cx.text_system() diff --git a/docs/src/ai/mcp.md b/docs/src/ai/mcp.md index d8d2de2a014459ddeed0f2a0fe92c2cbe84045e4..956477a1c2872d9371f770c3a767e5a77bead9fa 100644 --- a/docs/src/ai/mcp.md +++ b/docs/src/ai/mcp.md @@ -40,12 +40,12 @@ You can connect them by adding their commands directly to your `settings.json`, ```json [settings] { "context_servers": { - "run-command": { + "local-mcp-server": { "command": "some-command", "args": ["arg-1", "arg-2"], "env": {} }, - "over-http": { + "remote-mcp-server": { "url": "custom", "headers": { "Authorization": "Bearer " } }