Bennet Bo Fenner created
Release Notes:
- agent: Automatically remove context server settings when uninstalling
MCP extensioncrates/agent/src/agent.rs | 2
crates/agent/src/context_server_configuration.rs | 29 +++
crates/extension/src/extension_events.rs | 1
crates/extension_host/src/extension_host.rs | 14 +
crates/project/src/lsp_store.rs | 1
crates/settings/src/settings_store.rs | 143 +++++++++++++++--
6 files changed, 171 insertions(+), 19 deletions(-)
@@ -162,7 +162,7 @@ pub fn init(
assistant_slash_command::init(cx);
thread_store::init(cx);
agent_panel::init(cx);
- context_server_configuration::init(language_registry, cx);
+ context_server_configuration::init(language_registry, fs.clone(), cx);
register_slash_commands(cx);
inline_assistant::init(
@@ -3,16 +3,21 @@ use std::sync::Arc;
use anyhow::Context as _;
use context_server::ContextServerId;
use extension::{ContextServerConfiguration, ExtensionManifest};
+use fs::Fs;
use gpui::Task;
use language::LanguageRegistry;
-use project::context_server_store::registry::ContextServerDescriptorRegistry;
+use project::{
+ context_server_store::registry::ContextServerDescriptorRegistry,
+ project_settings::ProjectSettings,
+};
+use settings::update_settings_file;
use ui::prelude::*;
use util::ResultExt;
use workspace::Workspace;
use crate::agent_configuration::ConfigureContextServerModal;
-pub(crate) fn init(language_registry: Arc<LanguageRegistry>, cx: &mut App) {
+pub(crate) fn init(language_registry: Arc<LanguageRegistry>, fs: Arc<dyn Fs>, cx: &mut App) {
cx.observe_new(move |_: &mut Workspace, window, cx| {
let Some(window) = window else {
return;
@@ -21,6 +26,7 @@ pub(crate) fn init(language_registry: Arc<LanguageRegistry>, cx: &mut App) {
if let Some(extension_events) = extension::ExtensionEvents::try_global(cx).as_ref() {
cx.subscribe_in(extension_events, window, {
let language_registry = language_registry.clone();
+ let fs = fs.clone();
move |workspace, _, event, window, cx| match event {
extension::Event::ExtensionInstalled(manifest) => {
show_configure_mcp_modal(
@@ -31,6 +37,13 @@ pub(crate) fn init(language_registry: Arc<LanguageRegistry>, cx: &mut App) {
cx,
);
}
+ extension::Event::ExtensionUninstalled(manifest) => {
+ remove_context_server_settings(
+ manifest.context_servers.keys().cloned().collect(),
+ fs.clone(),
+ cx,
+ );
+ }
extension::Event::ConfigureExtensionRequested(manifest) => {
if !manifest.context_servers.is_empty() {
show_configure_mcp_modal(
@@ -55,6 +68,18 @@ pub(crate) fn init(language_registry: Arc<LanguageRegistry>, cx: &mut App) {
.detach();
}
+fn remove_context_server_settings(
+ context_server_ids: Vec<Arc<str>>,
+ fs: Arc<dyn Fs>,
+ cx: &mut App,
+) {
+ update_settings_file::<ProjectSettings>(fs, cx, move |settings, _| {
+ settings
+ .context_servers
+ .retain(|server_id, _| !context_server_ids.contains(server_id));
+ });
+}
+
pub enum Configuration {
NotAvailable(ContextServerId, Option<SharedString>),
Required(
@@ -36,6 +36,7 @@ impl ExtensionEvents {
#[derive(Clone)]
pub enum Event {
ExtensionInstalled(Arc<ExtensionManifest>),
+ ExtensionUninstalled(Arc<ExtensionManifest>),
ExtensionsInstalledChanged,
ConfigureExtensionRequested(Arc<ExtensionManifest>),
}
@@ -132,6 +132,7 @@ pub enum Event {
ExtensionsUpdated,
StartedReloading,
ExtensionInstalled(Arc<str>),
+ ExtensionUninstalled(Arc<str>),
ExtensionFailedToLoad(Arc<str>),
}
@@ -842,6 +843,8 @@ impl ExtensionStore {
let work_dir = self.wasm_host.work_dir.join(extension_id.as_ref());
let fs = self.fs.clone();
+ let extension_manifest = self.extension_manifest_for_id(&extension_id).cloned();
+
match self.outstanding_operations.entry(extension_id.clone()) {
btree_map::Entry::Occupied(_) => return,
btree_map::Entry::Vacant(e) => e.insert(ExtensionOperation::Remove),
@@ -878,6 +881,17 @@ impl ExtensionStore {
)
.await?;
+ this.update(cx, |_, cx| {
+ cx.emit(Event::ExtensionUninstalled(extension_id.clone()));
+ if let Some(events) = ExtensionEvents::try_global(cx) {
+ if let Some(manifest) = extension_manifest {
+ events.update(cx, |this, cx| {
+ this.emit(extension::Event::ExtensionUninstalled(manifest.clone()), cx)
+ });
+ }
+ }
+ })?;
+
anyhow::Ok(())
})
.detach_and_log_err(cx)
@@ -3934,6 +3934,7 @@ impl LspStore {
) {
match evt {
extension::Event::ExtensionInstalled(_)
+ | extension::Event::ExtensionUninstalled(_)
| extension::Event::ConfigureExtensionRequested(_) => return,
extension::Event::ExtensionsInstalledChanged => {}
}
@@ -1349,16 +1349,23 @@ fn update_value_in_json_text<'a>(
if let (Value::Object(old_object), Value::Object(new_object)) = (old_value, new_value) {
for (key, old_sub_value) in old_object.iter() {
key_path.push(key);
- let new_sub_value = new_object.get(key).unwrap_or(&Value::Null);
- update_value_in_json_text(
- text,
- key_path,
- tab_size,
- old_sub_value,
- new_sub_value,
- preserved_keys,
- edits,
- );
+ if let Some(new_sub_value) = new_object.get(key) {
+ // Key exists in both old and new, recursively update
+ update_value_in_json_text(
+ text,
+ key_path,
+ tab_size,
+ old_sub_value,
+ new_sub_value,
+ preserved_keys,
+ edits,
+ );
+ } else {
+ // Key was removed from new object, remove the entire key-value pair
+ let (range, replacement) = replace_value_in_json_text(text, key_path, 0, None);
+ text.replace_range(range.clone(), &replacement);
+ edits.push((range, replacement));
+ }
key_path.pop();
}
for (key, new_sub_value) in new_object.iter() {
@@ -1385,7 +1392,8 @@ fn update_value_in_json_text<'a>(
if let Some(new_object) = new_value.as_object_mut() {
new_object.retain(|_, v| !v.is_null());
}
- let (range, replacement) = replace_value_in_json_text(text, key_path, tab_size, &new_value);
+ let (range, replacement) =
+ replace_value_in_json_text(text, key_path, tab_size, Some(&new_value));
text.replace_range(range.clone(), &replacement);
edits.push((range, replacement));
}
@@ -1395,7 +1403,7 @@ fn replace_value_in_json_text(
text: &str,
key_path: &[&str],
tab_size: usize,
- new_value: &Value,
+ new_value: Option<&Value>,
) -> (Range<usize>, String) {
static PAIR_QUERY: LazyLock<Query> = LazyLock::new(|| {
Query::new(
@@ -1461,16 +1469,64 @@ fn replace_value_in_json_text(
}
}
- // We found the exact key we want, insert the new value
+ // We found the exact key we want
if depth == key_path.len() {
- let new_val = to_pretty_json(&new_value, tab_size, tab_size * depth);
- (existing_value_range, new_val)
+ if let Some(new_value) = new_value {
+ let new_val = to_pretty_json(new_value, tab_size, tab_size * depth);
+ (existing_value_range, new_val)
+ } else {
+ let mut removal_start = first_key_start.unwrap_or(existing_value_range.start);
+ let mut removal_end = existing_value_range.end;
+
+ // Find the actual key position by looking for the key in the pair
+ // We need to extend the range to include the key, not just the value
+ if let Some(key_start) = text[..existing_value_range.start].rfind('"') {
+ if let Some(prev_key_start) = text[..key_start].rfind('"') {
+ removal_start = prev_key_start;
+ } else {
+ removal_start = key_start;
+ }
+ }
+
+ // Look backward for a preceding comma first
+ let preceding_text = text.get(0..removal_start).unwrap_or("");
+ if let Some(comma_pos) = preceding_text.rfind(',') {
+ // Check if there are only whitespace characters between the comma and our key
+ let between_comma_and_key = text.get(comma_pos + 1..removal_start).unwrap_or("");
+ if between_comma_and_key.trim().is_empty() {
+ removal_start = comma_pos;
+ }
+ } else {
+ // No preceding comma, check for trailing comma
+ if let Some(remaining_text) = text.get(existing_value_range.end..) {
+ let mut chars = remaining_text.char_indices();
+ while let Some((offset, ch)) = chars.next() {
+ if ch == ',' {
+ removal_end = existing_value_range.end + offset + 1;
+ // Also consume whitespace after the comma
+ while let Some((_, next_ch)) = chars.next() {
+ if next_ch.is_whitespace() {
+ removal_end += next_ch.len_utf8();
+ } else {
+ break;
+ }
+ }
+ break;
+ } else if !ch.is_whitespace() {
+ break;
+ }
+ }
+ }
+ }
+ (removal_start..removal_end, String::new())
+ }
} else {
// We have key paths, construct the sub objects
let new_key = key_path[depth];
// We don't have the key, construct the nested objects
- let mut new_value = serde_json::to_value(new_value).unwrap();
+ let mut new_value =
+ serde_json::to_value(new_value.unwrap_or(&serde_json::Value::Null)).unwrap();
for key in key_path[(depth + 1)..].iter().rev() {
new_value = serde_json::json!({ key.to_string(): new_value });
}
@@ -1775,6 +1831,61 @@ mod tests {
cx,
);
+ // entries removed
+ check_settings_update::<LanguageSettings>(
+ &mut store,
+ r#"{
+ "languages": {
+ "Rust": {
+ "language_setting_2": true
+ },
+ "JSON": {
+ "language_setting_1": false
+ }
+ }
+ }"#
+ .unindent(),
+ |settings| {
+ settings.languages.remove("JSON").unwrap();
+ },
+ r#"{
+ "languages": {
+ "Rust": {
+ "language_setting_2": true
+ }
+ }
+ }"#
+ .unindent(),
+ cx,
+ );
+
+ check_settings_update::<LanguageSettings>(
+ &mut store,
+ r#"{
+ "languages": {
+ "Rust": {
+ "language_setting_2": true
+ },
+ "JSON": {
+ "language_setting_1": false
+ }
+ }
+ }"#
+ .unindent(),
+ |settings| {
+ settings.languages.remove("Rust").unwrap();
+ },
+ r#"{
+ "languages": {
+ "JSON": {
+ "language_setting_1": false
+ }
+ }
+ }"#
+ .unindent(),
+ cx,
+ );
+
// weird formatting
check_settings_update::<UserSettings>(
&mut store,