diff --git a/crates/agent/src/agent_configuration.rs b/crates/agent/src/agent_configuration.rs index be76804bbf..1091e16a5b 100644 --- a/crates/agent/src/agent_configuration.rs +++ b/crates/agent/src/agent_configuration.rs @@ -586,7 +586,7 @@ impl AgentConfiguration { if let Some(server) = this.get_server(&context_server_id) { - this.start_server(server, cx).log_err(); + this.start_server(server, cx); } }) } diff --git a/crates/agent/src/agent_configuration/add_context_server_modal.rs b/crates/agent/src/agent_configuration/add_context_server_modal.rs index 47ba57b035..d9eff2a0b3 100644 --- a/crates/agent/src/agent_configuration/add_context_server_modal.rs +++ b/crates/agent/src/agent_configuration/add_context_server_modal.rs @@ -1,7 +1,6 @@ use context_server::ContextServerCommand; use gpui::{DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, WeakEntity, prelude::*}; -use project::project_settings::{ContextServerConfiguration, ProjectSettings}; -use serde_json::json; +use project::project_settings::{ContextServerSettings, ProjectSettings}; use settings::update_settings_file; use ui::{KeyBinding, Modal, ModalFooter, ModalHeader, Section, Tooltip, prelude::*}; use ui_input::SingleLineInput; @@ -81,13 +80,12 @@ impl AddContextServerModal { update_settings_file::(fs.clone(), cx, |settings, _| { settings.context_servers.insert( name.into(), - ContextServerConfiguration { - command: Some(ContextServerCommand { + ContextServerSettings::Custom { + command: ContextServerCommand { path, args, env: None, - }), - settings: Some(json!({})), + }, }, ); }); diff --git a/crates/agent/src/agent_configuration/configure_context_server_modal.rs b/crates/agent/src/agent_configuration/configure_context_server_modal.rs index 3bb4450132..923bb07992 100644 --- a/crates/agent/src/agent_configuration/configure_context_server_modal.rs +++ b/crates/agent/src/agent_configuration/configure_context_server_modal.rs @@ -15,7 +15,7 @@ use markdown::{Markdown, MarkdownElement, MarkdownStyle}; use notifications::status_toast::{StatusToast, ToastIcon}; use project::{ context_server_store::{ContextServerStatus, ContextServerStore}, - project_settings::{ContextServerConfiguration, ProjectSettings}, + project_settings::{ContextServerSettings, ProjectSettings}, }; use settings::{Settings as _, update_settings_file}; use theme::ThemeSettings; @@ -175,8 +175,9 @@ impl ConfigureContextServerModal { let settings_changed = ProjectSettings::get_global(cx) .context_servers .get(&id.0) - .map_or(true, |config| { - config.settings.as_ref() != Some(&settings_value) + .map_or(true, |settings| match settings { + ContextServerSettings::Custom { .. } => false, + ContextServerSettings::Extension { settings } => settings != &settings_value, }); let is_running = self.context_server_store.read(cx).status_for_server(&id) @@ -221,17 +222,12 @@ impl ConfigureContextServerModal { update_settings_file::(workspace.read(cx).app_state().fs.clone(), cx, { let id = id.clone(); |settings, _| { - if let Some(server_config) = settings.context_servers.get_mut(&id.0) { - server_config.settings = Some(settings_value); - } else { - settings.context_servers.insert( - id.0, - ContextServerConfiguration { - settings: Some(settings_value), - ..Default::default() - }, - ); - } + settings.context_servers.insert( + id.0, + ContextServerSettings::Extension { + settings: settings_value, + }, + ); } }); } 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 854ad460e5..75c964396d 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 @@ -938,24 +938,33 @@ impl ExtensionImports for WasmState { })?) } "context_servers" => { - let configuration = key + let settings = key .and_then(|key| { ProjectSettings::get(location, cx) .context_servers .get(key.as_str()) }) .cloned() - .unwrap_or_default(); - Ok(serde_json::to_string(&settings::ContextServerSettings { - command: configuration.command.map(|command| { - settings::CommandSettings { + .context("Failed to get context server configuration")?; + + match settings { + project::project_settings::ContextServerSettings::Custom { + command, + } => Ok(serde_json::to_string(&settings::ContextServerSettings { + command: Some(settings::CommandSettings { path: Some(command.path), arguments: Some(command.args), env: command.env.map(|env| env.into_iter().collect()), - } - }), - settings: configuration.settings, - })?) + }), + settings: None, + })?), + project::project_settings::ContextServerSettings::Extension { + settings, + } => Ok(serde_json::to_string(&settings::ContextServerSettings { + command: None, + settings: Some(settings), + })?), + } } _ => { bail!("Unknown settings category: {}", category); diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 3a79cd251e..281ae93123 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -75,3 +75,9 @@ pub(crate) mod m_2025_05_29 { pub(crate) use settings::SETTINGS_PATTERNS; } + +pub(crate) mod m_2025_06_16 { + mod settings; + + pub(crate) use settings::SETTINGS_PATTERNS; +} diff --git a/crates/migrator/src/migrations/m_2025_06_16/settings.rs b/crates/migrator/src/migrations/m_2025_06_16/settings.rs new file mode 100644 index 0000000000..0bb7f7454f --- /dev/null +++ b/crates/migrator/src/migrations/m_2025_06_16/settings.rs @@ -0,0 +1,152 @@ +use std::ops::Range; + +use tree_sitter::{Query, QueryMatch}; + +use crate::MigrationPatterns; + +pub const SETTINGS_PATTERNS: MigrationPatterns = &[ + ( + SETTINGS_CUSTOM_CONTEXT_SERVER_PATTERN, + migrate_custom_context_server_settings, + ), + ( + SETTINGS_EXTENSION_CONTEXT_SERVER_PATTERN, + migrate_extension_context_server_settings, + ), + ( + SETTINGS_EMPTY_CONTEXT_SERVER_PATTERN, + migrate_empty_context_server_settings, + ), +]; + +const SETTINGS_CUSTOM_CONTEXT_SERVER_PATTERN: &str = r#"(document + (object + (pair + key: (string (string_content) @context-servers) + value: (object + (pair + key: (string) + value: (object + (pair + key: (string (string_content) @previous-key) + value: (object) + )* + (pair + key: (string (string_content) @key) + value: (object) + ) + (pair + key: (string (string_content) @next-key) + value: (object) + )* + ) @server-settings + ) + ) + ) + ) + (#eq? @context-servers "context_servers") + (#eq? @key "command") + (#not-eq? @previous-key "source") + (#not-eq? @next-key "source") +)"#; + +fn migrate_custom_context_server_settings( + _contents: &str, + mat: &QueryMatch, + query: &Query, +) -> Option<(Range, String)> { + let server_settings_index = query.capture_index_for_name("server-settings")?; + let server_settings = mat.nodes_for_capture_index(server_settings_index).next()?; + // Move forward 1 to get inside the object + let start = server_settings.start_byte() + 1; + + Some(( + start..start, + r#" + "source": "custom","# + .to_string(), + )) +} + +const SETTINGS_EXTENSION_CONTEXT_SERVER_PATTERN: &str = r#"(document + (object + (pair + key: (string (string_content) @context-servers) + value: (object + (pair + key: (string) + value: (object + (pair + key: (string (string_content) @previous-key) + value: (object) + )* + (pair + key: (string (string_content) @key) + value: (object) + ) + (pair + key: (string (string_content) @next-key) + value: (object) + )* + ) @server-settings + ) + ) + ) + ) + (#eq? @context-servers "context_servers") + (#eq? @key "settings") + (#not-match? @previous-key "^command|source$") + (#not-match? @next-key "^command|source$") +)"#; + +fn migrate_extension_context_server_settings( + _contents: &str, + mat: &QueryMatch, + query: &Query, +) -> Option<(Range, String)> { + let server_settings_index = query.capture_index_for_name("server-settings")?; + let server_settings = mat.nodes_for_capture_index(server_settings_index).next()?; + // Move forward 1 to get inside the object + let start = server_settings.start_byte() + 1; + + Some(( + start..start, + r#" + "source": "extension","# + .to_string(), + )) +} + +const SETTINGS_EMPTY_CONTEXT_SERVER_PATTERN: &str = r#"(document + (object + (pair + key: (string (string_content) @context-servers) + value: (object + (pair + key: (string) + value: (object) @server-settings + ) + ) + ) + ) + (#eq? @context-servers "context_servers") + (#eq? @server-settings "{}") +)"#; + +fn migrate_empty_context_server_settings( + _contents: &str, + mat: &QueryMatch, + query: &Query, +) -> Option<(Range, String)> { + let server_settings_index = query.capture_index_for_name("server-settings")?; + let server_settings = mat.nodes_for_capture_index(server_settings_index).next()?; + + Some(( + server_settings.byte_range(), + r#"{ + "source": "extension", + "settings": {} + }"# + .to_string(), + )) +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index dabd0a7975..6c693235d7 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -148,6 +148,10 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2025_05_29::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_05_29, ), + ( + migrations::m_2025_06_16::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_06_16, + ), ]; run_migrations(text, migrations) } @@ -246,6 +250,10 @@ define_query!( SETTINGS_QUERY_2025_05_29, migrations::m_2025_05_29::SETTINGS_PATTERNS ); +define_query!( + SETTINGS_QUERY_2025_06_16, + migrations::m_2025_06_16::SETTINGS_PATTERNS +); // custom query static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new(|| { @@ -854,4 +862,152 @@ mod tests { ), ); } + + #[test] + fn test_mcp_settings_migration() { + assert_migrate_settings( + r#"{ + "context_servers": { + "empty_server": {}, + "extension_server": { + "settings": { + "foo": "bar" + } + }, + "custom_server": { + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + } + }, + "invalid_server": { + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "settings": { + "foo": "bar" + } + }, + "empty_server2": {}, + "extension_server2": { + "foo": "bar", + "settings": { + "foo": "bar" + }, + "bar": "foo" + }, + "custom_server2": { + "foo": "bar", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "bar": "foo" + }, + "invalid_server2": { + "foo": "bar", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "bar": "foo", + "settings": { + "foo": "bar" + } + } + } +}"#, + Some( + r#"{ + "context_servers": { + "empty_server": { + "source": "extension", + "settings": {} + }, + "extension_server": { + "source": "extension", + "settings": { + "foo": "bar" + } + }, + "custom_server": { + "source": "custom", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + } + }, + "invalid_server": { + "source": "custom", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "settings": { + "foo": "bar" + } + }, + "empty_server2": { + "source": "extension", + "settings": {} + }, + "extension_server2": { + "source": "extension", + "foo": "bar", + "settings": { + "foo": "bar" + }, + "bar": "foo" + }, + "custom_server2": { + "source": "custom", + "foo": "bar", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "bar": "foo" + }, + "invalid_server2": { + "source": "custom", + "foo": "bar", + "command": { + "path": "foo", + "args": ["bar"], + "env": { + "FOO": "BAR" + } + }, + "bar": "foo", + "settings": { + "foo": "bar" + } + } + } +}"#, + ), + ); + } } diff --git a/crates/project/src/context_server_store.rs b/crates/project/src/context_server_store.rs index 34d6abb96c..5ebfaadf83 100644 --- a/crates/project/src/context_server_store.rs +++ b/crates/project/src/context_server_store.rs @@ -5,14 +5,15 @@ use std::{path::Path, sync::Arc}; use anyhow::{Context as _, Result}; use collections::{HashMap, HashSet}; -use context_server::{ContextServer, ContextServerId}; +use context_server::{ContextServer, ContextServerCommand, ContextServerId}; +use futures::{FutureExt as _, future::join_all}; use gpui::{App, AsyncApp, Context, Entity, EventEmitter, Subscription, Task, WeakEntity, actions}; use registry::ContextServerDescriptorRegistry; use settings::{Settings as _, SettingsStore}; use util::ResultExt as _; use crate::{ - project_settings::{ContextServerConfiguration, ProjectSettings}, + project_settings::{ContextServerSettings, ProjectSettings}, worktree_store::WorktreeStore, }; @@ -81,6 +82,50 @@ impl ContextServerState { } } +#[derive(PartialEq, Eq)] +pub enum ContextServerConfiguration { + Custom { + command: ContextServerCommand, + }, + Extension { + command: ContextServerCommand, + settings: serde_json::Value, + }, +} + +impl ContextServerConfiguration { + pub fn command(&self) -> &ContextServerCommand { + match self { + ContextServerConfiguration::Custom { command } => command, + ContextServerConfiguration::Extension { command, .. } => command, + } + } + + pub async fn from_settings( + settings: ContextServerSettings, + id: ContextServerId, + registry: Entity, + worktree_store: Entity, + cx: &AsyncApp, + ) -> Option { + match settings { + ContextServerSettings::Custom { command } => { + Some(ContextServerConfiguration::Custom { command }) + } + ContextServerSettings::Extension { settings } => { + let descriptor = cx + .update(|cx| registry.read(cx).context_server_descriptor(&id.0)) + .ok() + .flatten()?; + + let command = descriptor.command(worktree_store, cx).await.log_err()?; + + Some(ContextServerConfiguration::Extension { command, settings }) + } + } + } +} + pub type ContextServerFactory = Box) -> Arc>; @@ -207,29 +252,37 @@ impl ContextServerStore { .collect() } - pub fn start_server( - &mut self, - server: Arc, - cx: &mut Context, - ) -> Result<()> { - let location = self - .worktree_store - .read(cx) - .visible_worktrees(cx) - .next() - .map(|worktree| settings::SettingsLocation { - worktree_id: worktree.read(cx).id(), - path: Path::new(""), - }); - let settings = ProjectSettings::get(location, cx); - let configuration = settings - .context_servers - .get(&server.id().0) - .context("Failed to load context server configuration from settings")? - .clone(); + pub fn start_server(&mut self, server: Arc, cx: &mut Context) { + cx.spawn(async move |this, cx| { + let this = this.upgrade().context("Context server store dropped")?; + let settings = this + .update(cx, |this, cx| { + this.context_server_settings(cx) + .get(&server.id().0) + .cloned() + }) + .ok() + .flatten() + .context("Failed to get context server settings")?; - self.run_server(server, Arc::new(configuration), cx); - Ok(()) + let (registry, worktree_store) = this.update(cx, |this, _| { + (this.registry.clone(), this.worktree_store.clone()) + })?; + let configuration = ContextServerConfiguration::from_settings( + settings, + server.id(), + registry, + worktree_store, + cx, + ) + .await + .context("Failed to create context server configuration")?; + + this.update(cx, |this, cx| { + this.run_server(server, Arc::new(configuration), cx) + }) + }) + .detach_and_log_err(cx); } pub fn stop_server(&mut self, id: &ContextServerId, cx: &mut Context) -> Result<()> { @@ -349,11 +402,6 @@ impl ContextServerStore { Ok(()) } - fn is_configuration_valid(&self, configuration: &ContextServerConfiguration) -> bool { - // Command must be some when we are running in stdio mode. - self.context_server_factory.as_ref().is_some() || configuration.command.is_some() - } - fn create_context_server( &self, id: ContextServerId, @@ -362,14 +410,29 @@ impl ContextServerStore { if let Some(factory) = self.context_server_factory.as_ref() { Ok(factory(id, configuration)) } else { - let command = configuration - .command - .clone() - .context("Missing command to run context server")?; - Ok(Arc::new(ContextServer::stdio(id, command))) + Ok(Arc::new(ContextServer::stdio( + id, + configuration.command().clone(), + ))) } } + fn context_server_settings<'a>( + &'a self, + cx: &'a App, + ) -> &'a HashMap, ContextServerSettings> { + let location = self + .worktree_store + .read(cx) + .visible_worktrees(cx) + .next() + .map(|worktree| settings::SettingsLocation { + worktree_id: worktree.read(cx).id(), + path: Path::new(""), + }); + &ProjectSettings::get(location, cx).context_servers + } + fn update_server_state( &mut self, id: ContextServerId, @@ -407,43 +470,39 @@ impl ContextServerStore { } async fn maintain_servers(this: WeakEntity, cx: &mut AsyncApp) -> Result<()> { - let mut desired_servers = HashMap::default(); - - let (registry, worktree_store) = this.update(cx, |this, cx| { - let location = this - .worktree_store - .read(cx) - .visible_worktrees(cx) - .next() - .map(|worktree| settings::SettingsLocation { - worktree_id: worktree.read(cx).id(), - path: Path::new(""), - }); - let settings = ProjectSettings::get(location, cx); - desired_servers = settings.context_servers.clone(); - - (this.registry.clone(), this.worktree_store.clone()) + let (mut configured_servers, registry, worktree_store) = this.update(cx, |this, cx| { + ( + this.context_server_settings(cx).clone(), + this.registry.clone(), + this.worktree_store.clone(), + ) })?; - for (id, descriptor) in + for (id, _) in registry.read_with(cx, |registry, _| registry.context_server_descriptors())? { - let config = desired_servers.entry(id.clone()).or_default(); - if config.command.is_none() { - if let Some(extension_command) = descriptor - .command(worktree_store.clone(), &cx) - .await - .log_err() - { - config.command = Some(extension_command); - } - } + configured_servers + .entry(id) + .or_insert(ContextServerSettings::Extension { + settings: serde_json::json!({}), + }); } - this.update(cx, |this, _| { - // Filter out configurations without commands, the user uninstalled an extension. - desired_servers.retain(|_, configuration| this.is_configuration_valid(configuration)); - })?; + let configured_servers = join_all(configured_servers.into_iter().map(|(id, settings)| { + let id = ContextServerId(id); + ContextServerConfiguration::from_settings( + settings, + id.clone(), + registry.clone(), + worktree_store.clone(), + cx, + ) + .map(|config| (id, config)) + })) + .await + .into_iter() + .filter_map(|(id, config)| config.map(|config| (id, config))) + .collect::>(); let mut servers_to_start = Vec::new(); let mut servers_to_remove = HashSet::default(); @@ -452,16 +511,13 @@ impl ContextServerStore { this.update(cx, |this, _cx| { for server_id in this.servers.keys() { // All servers that are not in desired_servers should be removed from the store. - // E.g. this can happen if the user removed a server from the configuration, - // or the user uninstalled an extension. - if !desired_servers.contains_key(&server_id.0) { + // This can happen if the user removed a server from the context server settings. + if !configured_servers.contains_key(&server_id) { servers_to_remove.insert(server_id.clone()); } } - for (id, config) in desired_servers { - let id = ContextServerId(id.clone()); - + for (id, config) in configured_servers { let existing_config = this.servers.get(&id).map(|state| state.configuration()); if existing_config.as_deref() != Some(&config) { let config = Arc::new(config); @@ -478,27 +534,28 @@ impl ContextServerStore { } })?; - for id in servers_to_stop { - this.update(cx, |this, cx| this.stop_server(&id, cx).ok())?; - } - - for id in servers_to_remove { - this.update(cx, |this, cx| this.remove_server(&id, cx).ok())?; - } - - for (server, config) in servers_to_start { - this.update(cx, |this, cx| this.run_server(server, config, cx)) - .log_err(); - } - - Ok(()) + this.update(cx, |this, cx| { + for id in servers_to_stop { + this.stop_server(&id, cx)?; + } + for id in servers_to_remove { + this.remove_server(&id, cx)?; + } + for (server, config) in servers_to_start { + this.run_server(server, config, cx); + } + anyhow::Ok(()) + })? } } #[cfg(test)] mod tests { use super::*; - use crate::{FakeFs, Project, project_settings::ProjectSettings}; + use crate::{ + FakeFs, Project, context_server_store::registry::ContextServerDescriptor, + project_settings::ProjectSettings, + }; use context_server::test::create_fake_transport; use gpui::{AppContext, TestAppContext, UpdateGlobal as _}; use serde_json::json; @@ -514,8 +571,8 @@ mod tests { cx, json!({"code.rs": ""}), vec![ - (SERVER_1_ID.into(), ContextServerConfiguration::default()), - (SERVER_2_ID.into(), ContextServerConfiguration::default()), + (SERVER_1_ID.into(), dummy_server_settings()), + (SERVER_2_ID.into(), dummy_server_settings()), ], ) .await; @@ -537,9 +594,7 @@ mod tests { Arc::new(create_fake_transport(SERVER_2_ID, cx.executor())), )); - store - .update(cx, |store, cx| store.start_server(server_1, cx)) - .unwrap(); + store.update(cx, |store, cx| store.start_server(server_1, cx)); cx.run_until_parked(); @@ -551,9 +606,7 @@ mod tests { assert_eq!(store.read(cx).status_for_server(&server_2_id), None); }); - store - .update(cx, |store, cx| store.start_server(server_2.clone(), cx)) - .unwrap(); + store.update(cx, |store, cx| store.start_server(server_2.clone(), cx)); cx.run_until_parked(); @@ -593,8 +646,8 @@ mod tests { cx, json!({"code.rs": ""}), vec![ - (SERVER_1_ID.into(), ContextServerConfiguration::default()), - (SERVER_2_ID.into(), ContextServerConfiguration::default()), + (SERVER_1_ID.into(), dummy_server_settings()), + (SERVER_2_ID.into(), dummy_server_settings()), ], ) .await; @@ -628,15 +681,11 @@ mod tests { cx, ); - store - .update(cx, |store, cx| store.start_server(server_1, cx)) - .unwrap(); + store.update(cx, |store, cx| store.start_server(server_1, cx)); cx.run_until_parked(); - store - .update(cx, |store, cx| store.start_server(server_2.clone(), cx)) - .unwrap(); + store.update(cx, |store, cx| store.start_server(server_2.clone(), cx)); cx.run_until_parked(); @@ -652,7 +701,7 @@ mod tests { let (_fs, project) = setup_context_server_test( cx, json!({"code.rs": ""}), - vec![(SERVER_1_ID.into(), ContextServerConfiguration::default())], + vec![(SERVER_1_ID.into(), dummy_server_settings())], ) .await; @@ -684,21 +733,11 @@ mod tests { cx, ); - store - .update(cx, |store, cx| { - store.start_server(server_with_same_id_1.clone(), cx) - }) - .unwrap(); - store - .update(cx, |store, cx| { - store.start_server(server_with_same_id_2.clone(), cx) - }) - .unwrap(); - cx.update(|cx| { - assert_eq!( - store.read(cx).status_for_server(&server_id), - Some(ContextServerStatus::Starting) - ); + store.update(cx, |store, cx| { + store.start_server(server_with_same_id_1.clone(), cx) + }); + store.update(cx, |store, cx| { + store.start_server(server_with_same_id_2.clone(), cx) }); cx.run_until_parked(); @@ -719,23 +758,28 @@ mod tests { let server_1_id = ContextServerId(SERVER_1_ID.into()); let server_2_id = ContextServerId(SERVER_2_ID.into()); + let fake_descriptor_1 = Arc::new(FakeContextServerDescriptor::new(SERVER_1_ID)); + let (_fs, project) = setup_context_server_test( cx, json!({"code.rs": ""}), vec![( SERVER_1_ID.into(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ + ContextServerSettings::Extension { + settings: json!({ "somevalue": true - })), + }), }, )], ) .await; let executor = cx.executor(); - let registry = cx.new(|_| ContextServerDescriptorRegistry::new()); + let registry = cx.new(|_| { + let mut registry = ContextServerDescriptorRegistry::new(); + registry.register_context_server_descriptor(SERVER_1_ID.into(), fake_descriptor_1); + registry + }); let store = cx.new(|cx| { ContextServerStore::test_maintain_server_loop( Box::new(move |id, _| { @@ -777,11 +821,10 @@ mod tests { set_context_server_configuration( vec![( server_1_id.0.clone(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ + ContextServerSettings::Extension { + settings: json!({ "somevalue": false - })), + }), }, )], cx, @@ -796,11 +839,10 @@ mod tests { set_context_server_configuration( vec![( server_1_id.0.clone(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ + ContextServerSettings::Extension { + settings: json!({ "somevalue": false - })), + }), }, )], cx, @@ -823,20 +865,58 @@ mod tests { vec![ ( server_1_id.0.clone(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ + ContextServerSettings::Extension { + settings: json!({ "somevalue": false - })), + }), }, ), ( server_2_id.0.clone(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ - "somevalue": true - })), + ContextServerSettings::Custom { + command: ContextServerCommand { + path: "somebinary".to_string(), + args: vec!["arg".to_string()], + env: None, + }, + }, + ), + ], + cx, + ); + + cx.run_until_parked(); + } + + // Ensure that mcp-2 is restarted once the args have changed + { + let _server_events = assert_server_events( + &store, + vec![ + (server_2_id.clone(), ContextServerStatus::Stopped), + (server_2_id.clone(), ContextServerStatus::Starting), + (server_2_id.clone(), ContextServerStatus::Running), + ], + cx, + ); + set_context_server_configuration( + vec![ + ( + server_1_id.0.clone(), + ContextServerSettings::Extension { + settings: json!({ + "somevalue": false + }), + }, + ), + ( + server_2_id.0.clone(), + ContextServerSettings::Custom { + command: ContextServerCommand { + path: "somebinary".to_string(), + args: vec!["anotherArg".to_string()], + env: None, + }, }, ), ], @@ -856,11 +936,10 @@ mod tests { set_context_server_configuration( vec![( server_1_id.0.clone(), - ContextServerConfiguration { - command: None, - settings: Some(json!({ + ContextServerSettings::Extension { + settings: json!({ "somevalue": false - })), + }), }, )], cx, @@ -875,7 +954,7 @@ mod tests { } fn set_context_server_configuration( - context_servers: Vec<(Arc, ContextServerConfiguration)>, + context_servers: Vec<(Arc, ContextServerSettings)>, cx: &mut TestAppContext, ) { cx.update(|cx| { @@ -909,6 +988,16 @@ mod tests { } } + fn dummy_server_settings() -> ContextServerSettings { + ContextServerSettings::Custom { + command: ContextServerCommand { + path: "somebinary".to_string(), + args: vec!["arg".to_string()], + env: None, + }, + } + } + fn assert_server_events( store: &Entity, expected_events: Vec<(ContextServerId, ContextServerStatus)>, @@ -953,7 +1042,7 @@ mod tests { async fn setup_context_server_test( cx: &mut TestAppContext, files: serde_json::Value, - context_server_configurations: Vec<(Arc, ContextServerConfiguration)>, + context_server_configurations: Vec<(Arc, ContextServerSettings)>, ) -> (Arc, Entity) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); @@ -972,4 +1061,36 @@ mod tests { (fs, project) } + + struct FakeContextServerDescriptor { + path: String, + } + + impl FakeContextServerDescriptor { + fn new(path: impl Into) -> Self { + Self { path: path.into() } + } + } + + impl ContextServerDescriptor for FakeContextServerDescriptor { + fn command( + &self, + _worktree_store: Entity, + _cx: &AsyncApp, + ) -> Task> { + Task::ready(Ok(ContextServerCommand { + path: self.path.clone(), + args: vec!["arg1".to_string(), "arg2".to_string()], + env: None, + })) + } + + fn configuration( + &self, + _worktree_store: Entity, + _cx: &AsyncApp, + ) -> Task>> { + Task::ready(Ok(None)) + } + } } diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index 8e84f3c73b..92879c75f1 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -55,7 +55,7 @@ pub struct ProjectSettings { /// Settings for context servers used for AI-related features. #[serde(default)] - pub context_servers: HashMap, ContextServerConfiguration>, + pub context_servers: HashMap, ContextServerSettings>, /// Configuration for Diagnostics-related features. #[serde(default)] @@ -84,17 +84,22 @@ pub struct DapSettings { pub binary: Option, } -#[derive(Deserialize, Serialize, Clone, PartialEq, Eq, JsonSchema, Debug, Default)] -pub struct ContextServerConfiguration { - /// The command to run this context server. - /// - /// This will override the command set by an extension. - pub command: Option, - /// The settings for this context server. - /// - /// Consult the documentation for the context server to see what settings - /// are supported. - pub settings: Option, +#[derive(Deserialize, Serialize, Clone, PartialEq, Eq, JsonSchema, Debug)] +#[serde(tag = "source", rename_all = "snake_case")] +pub enum ContextServerSettings { + Custom { + /// The command to run this context server. + /// + /// This will override the command set by an extension. + command: ContextServerCommand, + }, + Extension { + /// 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, + }, } #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, JsonSchema)] @@ -472,13 +477,12 @@ impl Settings for ProjectSettings { .extend(mcp.iter().filter_map(|(k, v)| { Some(( k.clone().into(), - ContextServerConfiguration { - command: Some( - serde_json::from_value::(v.clone()) - .ok()? - .into(), - ), - settings: None, + ContextServerSettings::Custom { + command: serde_json::from_value::( + v.clone(), + ) + .ok()? + .into(), }, )) })); diff --git a/docs/src/ai/mcp.md b/docs/src/ai/mcp.md index 3d356cddfd..8fbea0fef1 100644 --- a/docs/src/ai/mcp.md +++ b/docs/src/ai/mcp.md @@ -41,12 +41,12 @@ You can connect them by adding their commands directly to your `settings.json`, { "context_servers": { "some-context-server": { + "source": "custom", "command": { "path": "some-command", "args": ["arg-1", "arg-2"], "env": {} - }, - "settings": {} + } } } }