Skip to content

feat: add dynamic MCP server infrastructure to McpPool #3869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bistack wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add dynamic MCP server infrastructure to McpPool #3869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 51 additions & 9 deletions crates/tui/src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ use std::collections::HashMap;
use std::fs;
use std::io::Read;
use std::path::{Component, Path, PathBuf};
use std::sync::Arc;
use std::sync::atomic::{AtomicU64, Ordering};
use std::time::Duration;

use anyhow::{Context, Result};
use parking_lot::RwLock;
use serde::{Deserialize, Serialize};

mod headers;
Expand Down Expand Up @@ -1432,6 +1434,9 @@ pub struct McpPool {
config_hash: u64,
/// Most recently observed mtime for `config_sources`.
last_mtimes: Vec<Option<std::time::SystemTime>>,
/// Dynamically added MCP servers (from tool calls at runtime).
/// These are not persisted to disk and live for the process lifetime.
pub(crate) dynamic_servers: Arc<RwLock<HashMap<String, McpServerConfig>>>,
}

impl McpPool {
Expand All @@ -1446,6 +1451,7 @@ impl McpPool {
workspace: None,
config_hash,
last_mtimes: Vec::new(),
dynamic_servers: Arc::new(RwLock::new(HashMap::new())),
}
}

Expand Down Expand Up @@ -1589,12 +1595,14 @@ impl McpPool {

self.drop_connection(server_name, "reconnect");

// Check static config first, then dynamic servers
let server_config = self
.config
.servers
.get(server_name)
.ok_or_else(|| anyhow::anyhow!("Failed to find MCP server: {server_name}"))?
.clone();
.cloned()
.or_else(|| self.dynamic_servers.read().get(server_name).cloned())
.ok_or_else(|| anyhow::anyhow!("Failed to find MCP server: {server_name}"))?;

if !server_config.is_enabled() {
anyhow::bail!("Failed to connect MCP server '{server_name}': server is disabled");
Expand Down Expand Up @@ -2084,14 +2092,48 @@ impl McpPool {
}
}

/// Get list of configured server names
/// Get list of configured server names (static + dynamic)
#[allow(dead_code)] // Public API for MCP consumers
pub fn server_names(&self) -> Vec<&str> {
self.config
.servers
.keys()
.map(std::string::String::as_str)
.collect()
pub fn server_names(&self) -> Vec<String> {
let mut names: Vec<String> = self.config.servers.keys().cloned().collect();
let dynamic = self.dynamic_servers.read();
for name in dynamic.keys() {
if !names.contains(name) {
names.push(name.clone());
}
}
names
}

/// Add a runtime server configuration (in-memory only, not persisted).
///
/// This is used for dynamically started MCP servers from chat context.
/// Stored in `dynamic_servers` so it doesn't interfere with file-based config reload.
///
/// Returns `Err` if a server with the same name already exists as a static config
/// or a dynamic config. The caller should surface the error to the LLM/user.
pub fn add_runtime_server_config(
&self,
name: String,
config: McpServerConfig,
) -> Result<(), String> {
if self.config.servers.contains_key(&name) {
return Err(format!(
"MCP server '{}' already exists in the config file. \
Remove it from the config first, or choose a different name.",
name
));
}
let mut dynamic = self.dynamic_servers.write();
if dynamic.contains_key(&name) {
return Err(format!(
"MCP server '{}' was already started earlier in this session. \
Choose a different name.",
name
));
}
dynamic.insert(name, config);
Ok(())
}

/// Get list of connected server names
Expand Down
91 changes: 78 additions & 13 deletions crates/tui/src/mcp/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ async fn workspace_mcp_pool_reload_picks_up_project_config_creation() {
.unwrap();

let mut pool = McpPool::from_config_path_with_workspace(&global_path, &workspace).unwrap();
assert_eq!(pool.server_names(), vec!["global"]);
assert_eq!(pool.server_names(), vec!["global".to_string()]);

fs::create_dir_all(&project_dir).unwrap();
fs::write(
Expand All @@ -775,8 +775,11 @@ async fn workspace_mcp_pool_reload_picks_up_project_config_creation() {
.unwrap();

assert!(pool.reload_if_config_changed().await.unwrap());
let names: std::collections::BTreeSet<_> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<_> = ["global", "project"].into_iter().collect();
let names: std::collections::BTreeSet<String> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<String> =
["global".to_string(), "project".to_string()]
.into_iter()
.collect();
assert_eq!(names, expected);
}

Expand All @@ -800,13 +803,16 @@ async fn workspace_mcp_pool_reload_picks_up_project_config_after_workspace_trust
.unwrap();

let mut pool = McpPool::from_config_path_with_workspace(&global_path, &workspace).unwrap();
assert_eq!(pool.server_names(), vec!["global"]);
assert_eq!(pool.server_names(), vec!["global".to_string()]);

write_workspace_trust_config(&trust_env.config_path, &workspace);

assert!(pool.reload_if_config_changed().await.unwrap());
let names: std::collections::BTreeSet<_> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<_> = ["global", "project"].into_iter().collect();
let names: std::collections::BTreeSet<String> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<String> =
["global".to_string(), "project".to_string()]
.into_iter()
.collect();
assert_eq!(names, expected);
}

Expand All @@ -830,14 +836,17 @@ async fn workspace_mcp_pool_reload_drops_project_config_after_workspace_trust_re
.unwrap();

let mut pool = McpPool::from_config_path_with_workspace(&global_path, &workspace).unwrap();
let names: std::collections::BTreeSet<_> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<_> = ["global", "project"].into_iter().collect();
let names: std::collections::BTreeSet<String> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<String> =
["global".to_string(), "project".to_string()]
.into_iter()
.collect();
assert_eq!(names, expected);

fs::remove_file(&trust.config_path).unwrap();

assert!(pool.reload_if_config_changed().await.unwrap());
assert_eq!(pool.server_names(), vec!["global"]);
assert_eq!(pool.server_names(), vec!["global".to_string()]);
}

#[tokio::test]
Expand All @@ -861,14 +870,17 @@ async fn workspace_mcp_pool_reload_drops_project_config_after_deletion() {
.unwrap();

let mut pool = McpPool::from_config_path_with_workspace(&global_path, &workspace).unwrap();
let names: std::collections::BTreeSet<_> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<_> = ["global", "project"].into_iter().collect();
let names: std::collections::BTreeSet<String> = pool.server_names().into_iter().collect();
let expected: std::collections::BTreeSet<String> =
["global".to_string(), "project".to_string()]
.into_iter()
.collect();
assert_eq!(names, expected);

fs::remove_file(project_path).unwrap();

assert!(pool.reload_if_config_changed().await.unwrap());
assert_eq!(pool.server_names(), vec!["global"]);
assert_eq!(pool.server_names(), vec!["global".to_string()]);
}

#[test]
Expand Down Expand Up @@ -1345,7 +1357,7 @@ async fn reload_if_config_changed_swaps_config_on_content_change() {
assert!(reloaded, "content-changed config must trigger reload");
let names = pool.server_names();
assert!(
names.contains(&"new"),
names.contains(&"new".to_string()),
"expected new server in pool after reload, got {names:?}"
);
}
Expand Down Expand Up @@ -3154,3 +3166,56 @@ async fn custom_headers_applied_to_get_preflight() {
"GET preflight must include user-configured custom headers"
);
}

// === add_runtime_server_config conflict tests ===

#[test]
fn add_runtime_server_config_rejects_static_conflict() {
let config: McpConfig = serde_json::from_str(
r#"{
"servers": {
"existing": {"command": "node server.js"}
}
}"#,
)
.unwrap();
let pool = McpPool::new(config);

let err = pool
.add_runtime_server_config(
"existing".to_string(),
serde_json::from_str(r#"{"command": "npx other"}"#).unwrap(),
)
.unwrap_err();
assert!(err.contains("already exists in the config file"));
}

#[test]
fn add_runtime_server_config_rejects_dynamic_duplicate() {
let pool = McpPool::new(McpConfig::default());

pool.add_runtime_server_config(
"my_server".to_string(),
serde_json::from_str(r#"{"command": "node a.js"}"#).unwrap(),
)
.unwrap();

let err = pool
.add_runtime_server_config(
"my_server".to_string(),
serde_json::from_str(r#"{"command": "node b.js"}"#).unwrap(),
)
.unwrap_err();
assert!(err.contains("already started earlier"));
}

#[test]
fn add_runtime_server_config_accepts_new_name() {
let pool = McpPool::new(McpConfig::default());

pool.add_runtime_server_config(
"brand_new".to_string(),
serde_json::from_str(r#"{"command": "node x.js"}"#).unwrap(),
)
.unwrap();
}