From d63909c598d7960da8c54903c7b4db0ea52f7d11 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Mon, 30 Jun 2025 10:05:52 +0200 Subject: [PATCH] agent: Use standardized MCP configuration format in settings (#33539) Changes our MCP settings from: ```json { "context_servers": { "some-mcp-server": { "source": "custom", "command": { "path": "npx", "args": [ "-y", "@supabase/mcp-server-supabase@latest", "--read-only", "--project-ref=", ], "env": { "SUPABASE_ACCESS_TOKEN": "", }, }, }, }, } ``` to: ```json { "context_servers": { "some-mcp-server": { "source": "custom", "command": "npx", "args": [ "-y", "@supabase/mcp-server-supabase@latest", "--read-only", "--project-ref=", ], "env": { "SUPABASE_ACCESS_TOKEN": "", }, }, }, } ``` Which seems to be somewhat of a standard now (VSCode, Cursor, Windsurf, ...) Release Notes: - agent: Use standardised format for configuring MCP Servers --- .../configure_context_server_modal.rs | 19 ++- crates/context_server/src/context_server.rs | 1 + crates/migrator/src/migrations.rs | 6 + .../src/migrations/m_2025_06_27/settings.rs | 133 ++++++++++++++++++ crates/migrator/src/migrator.rs | 128 ++++++++++++++++- crates/project/src/project_settings.rs | 5 +- 6 files changed, 276 insertions(+), 16 deletions(-) create mode 100644 crates/migrator/src/migrations/m_2025_06_27/settings.rs 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 af08eaf935..299f3cee34 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 @@ -180,7 +180,7 @@ impl ConfigurationSource { } fn context_server_input(existing: Option<(ContextServerId, ContextServerCommand)>) -> String { - let (name, path, args, env) = match existing { + let (name, command, args, env) = match existing { Some((id, cmd)) => { let args = serde_json::to_string(&cmd.args).unwrap(); let env = serde_json::to_string(&cmd.env.unwrap_or_default()).unwrap(); @@ -198,14 +198,12 @@ fn context_server_input(existing: Option<(ContextServerId, ContextServerCommand) r#"{{ /// The name of your MCP server "{name}": {{ - "command": {{ - /// The path to the executable - "path": "{path}", - /// The arguments to pass to the executable - "args": {args}, - /// The environment variables to set for the executable - "env": {env} - }} + /// The command which runs the MCP server + "command": "{command}", + /// The arguments to pass to the MCP server + "args": {args}, + /// The environment variables to set + "env": {env} }} }}"# ) @@ -439,8 +437,7 @@ fn parse_input(text: &str) -> Result<(ContextServerId, ContextServerCommand)> { let object = value.as_object().context("Expected object")?; anyhow::ensure!(object.len() == 1, "Expected exactly one key-value pair"); let (context_server_name, value) = object.into_iter().next().unwrap(); - let command = value.get("command").context("Expected command")?; - let command: ContextServerCommand = serde_json::from_value(command.clone())?; + let command: ContextServerCommand = serde_json::from_value(value.clone())?; Ok((ContextServerId(context_server_name.clone().into()), command)) } diff --git a/crates/context_server/src/context_server.rs b/crates/context_server/src/context_server.rs index f774deef17..905435fcce 100644 --- a/crates/context_server/src/context_server.rs +++ b/crates/context_server/src/context_server.rs @@ -29,6 +29,7 @@ impl Display for ContextServerId { #[derive(Deserialize, Serialize, Clone, PartialEq, Eq, JsonSchema)] pub struct ContextServerCommand { + #[serde(rename = "command")] pub path: String, pub args: Vec, pub env: Option>, diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index d43521faa9..4e3839358b 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -87,3 +87,9 @@ pub(crate) mod m_2025_06_25 { pub(crate) use settings::SETTINGS_PATTERNS; } + +pub(crate) mod m_2025_06_27 { + mod settings; + + pub(crate) use settings::SETTINGS_PATTERNS; +} diff --git a/crates/migrator/src/migrations/m_2025_06_27/settings.rs b/crates/migrator/src/migrations/m_2025_06_27/settings.rs new file mode 100644 index 0000000000..6156308fce --- /dev/null +++ b/crates/migrator/src/migrations/m_2025_06_27/settings.rs @@ -0,0 +1,133 @@ +use std::ops::Range; +use tree_sitter::{Query, QueryMatch}; + +use crate::MigrationPatterns; + +pub const SETTINGS_PATTERNS: MigrationPatterns = &[( + SETTINGS_CONTEXT_SERVER_PATTERN, + flatten_context_server_command, +)]; + +const SETTINGS_CONTEXT_SERVER_PATTERN: &str = r#"(document + (object + (pair + key: (string (string_content) @context-servers) + value: (object + (pair + key: (string (string_content) @server-name) + value: (object + (pair + key: (string (string_content) @source-key) + value: (string (string_content) @source-value) + ) + (pair + key: (string (string_content) @command-key) + value: (object) @command-object + ) @command-pair + ) @server-settings + ) + ) + ) + ) + (#eq? @context-servers "context_servers") + (#eq? @source-key "source") + (#eq? @source-value "custom") + (#eq? @command-key "command") +)"#; + +fn flatten_context_server_command( + contents: &str, + mat: &QueryMatch, + query: &Query, +) -> Option<(Range, String)> { + let command_pair_index = query.capture_index_for_name("command-pair")?; + let command_pair = mat.nodes_for_capture_index(command_pair_index).next()?; + + let command_object_index = query.capture_index_for_name("command-object")?; + let command_object = mat.nodes_for_capture_index(command_object_index).next()?; + + let server_settings_index = query.capture_index_for_name("server-settings")?; + let _server_settings = mat.nodes_for_capture_index(server_settings_index).next()?; + + // Parse the command object to extract path, args, and env + let mut path_value = None; + let mut args_value = None; + let mut env_value = None; + + let mut cursor = command_object.walk(); + for child in command_object.children(&mut cursor) { + if child.kind() == "pair" { + if let Some(key_node) = child.child_by_field_name("key") { + if let Some(string_content) = key_node.child(1) { + let key = &contents[string_content.byte_range()]; + if let Some(value_node) = child.child_by_field_name("value") { + let value_range = value_node.byte_range(); + match key { + "path" => path_value = Some(&contents[value_range]), + "args" => args_value = Some(&contents[value_range]), + "env" => env_value = Some(&contents[value_range]), + _ => {} + } + } + } + } + } + } + + let path = path_value?; + + // Get the proper indentation from the command pair + let command_pair_start = command_pair.start_byte(); + let line_start = contents[..command_pair_start] + .rfind('\n') + .map(|pos| pos + 1) + .unwrap_or(0); + let indent = &contents[line_start..command_pair_start]; + + // Build the replacement string + let mut replacement = format!("\"command\": {}", path); + + // Add args if present - need to reduce indentation + if let Some(args) = args_value { + replacement.push_str(",\n"); + replacement.push_str(indent); + replacement.push_str("\"args\": "); + let reduced_args = reduce_indentation(args, 4); + replacement.push_str(&reduced_args); + } + + // Add env if present - need to reduce indentation + if let Some(env) = env_value { + replacement.push_str(",\n"); + replacement.push_str(indent); + replacement.push_str("\"env\": "); + replacement.push_str(&reduce_indentation(env, 4)); + } + + let range_to_replace = command_pair.byte_range(); + Some((range_to_replace, replacement)) +} + +fn reduce_indentation(text: &str, spaces: usize) -> String { + let lines: Vec<&str> = text.lines().collect(); + let mut result = String::new(); + + for (i, line) in lines.iter().enumerate() { + if i > 0 { + result.push('\n'); + } + + // Count leading spaces + let leading_spaces = line.chars().take_while(|&c| c == ' ').count(); + + if leading_spaces >= spaces { + // Reduce indentation + result.push_str(&line[spaces..]); + } else { + // Keep line as is if it doesn't have enough indentation + result.push_str(line); + } + } + + result +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index bcd41836e6..be32b2734e 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -156,6 +156,10 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2025_06_25::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_06_25, ), + ( + migrations::m_2025_06_27::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_06_27, + ), ]; run_migrations(text, migrations) } @@ -262,6 +266,10 @@ define_query!( SETTINGS_QUERY_2025_06_25, migrations::m_2025_06_25::SETTINGS_PATTERNS ); +define_query!( + SETTINGS_QUERY_2025_06_27, + migrations::m_2025_06_27::SETTINGS_PATTERNS +); // custom query static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new(|| { @@ -286,6 +294,15 @@ mod tests { pretty_assertions::assert_eq!(migrated.as_deref(), output); } + fn assert_migrate_settings_with_migrations( + migrations: &[(MigrationPatterns, &Query)], + input: &str, + output: Option<&str>, + ) { + let migrated = run_migrations(input, migrations).unwrap(); + pretty_assertions::assert_eq!(migrated.as_deref(), output); + } + #[test] fn test_replace_array_with_single_string() { assert_migrate_keymap( @@ -873,7 +890,11 @@ mod tests { #[test] fn test_mcp_settings_migration() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[( + migrations::m_2025_06_16::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_06_16, + )], r#"{ "context_servers": { "empty_server": {}, @@ -1058,7 +1079,14 @@ mod tests { } } }"#; - assert_migrate_settings(settings, None); + assert_migrate_settings_with_migrations( + &[( + migrations::m_2025_06_16::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_06_16, + )], + settings, + None, + ); } #[test] @@ -1131,4 +1159,100 @@ mod tests { None, ); } + + #[test] + fn test_flatten_context_server_command() { + assert_migrate_settings( + r#"{ + "context_servers": { + "some-mcp-server": { + "source": "custom", + "command": { + "path": "npx", + "args": [ + "-y", + "@supabase/mcp-server-supabase@latest", + "--read-only", + "--project-ref=" + ], + "env": { + "SUPABASE_ACCESS_TOKEN": "" + } + } + } + } +}"#, + Some( + r#"{ + "context_servers": { + "some-mcp-server": { + "source": "custom", + "command": "npx", + "args": [ + "-y", + "@supabase/mcp-server-supabase@latest", + "--read-only", + "--project-ref=" + ], + "env": { + "SUPABASE_ACCESS_TOKEN": "" + } + } + } +}"#, + ), + ); + + // Test with additional keys in server object + assert_migrate_settings( + r#"{ + "context_servers": { + "server-with-extras": { + "source": "custom", + "command": { + "path": "/usr/bin/node", + "args": ["server.js"] + }, + "settings": {} + } + } +}"#, + Some( + r#"{ + "context_servers": { + "server-with-extras": { + "source": "custom", + "command": "/usr/bin/node", + "args": ["server.js"], + "settings": {} + } + } +}"#, + ), + ); + + // Test command without args or env + assert_migrate_settings( + r#"{ + "context_servers": { + "simple-server": { + "source": "custom", + "command": { + "path": "simple-mcp-server" + } + } + } +}"#, + Some( + r#"{ + "context_servers": { + "simple-server": { + "source": "custom", + "command": "simple-mcp-server" + } + } +}"#, + ), + ); + } } diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index d2a4e5126c..f45393fd5c 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -97,9 +97,8 @@ pub enum ContextServerSettings { /// Whether the context server is enabled. #[serde(default = "default_true")] enabled: bool, - /// The command to run this context server. - /// - /// This will override the command set by an extension. + + #[serde(flatten)] command: ContextServerCommand, }, Extension {