Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/tui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ tar = "0.4"
flate2 = "1.1"
sha2 = "0.11"
rust-i18n = "4.1.0"
shell-words = "1.1.1"

[dev-dependencies]
cucumber = "0.23.0"
Expand Down
18 changes: 15 additions & 3 deletions crates/tui/src/core/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::config::{ApiProvider, Config, DEFAULT_MAX_SUBAGENTS, DEFAULT_TEXT_MOD
use crate::error_taxonomy::{ErrorCategory, ErrorEnvelope, StreamError};
use crate::features::{Feature, Features};
use crate::llm_client::LlmClient;
use crate::mcp::McpPool;
use crate::mcp::{McpConfig, McpPool};
#[cfg(test)]
use crate::models::ToolCaller;
use crate::models::{
Expand Down Expand Up @@ -2415,6 +2415,11 @@ impl Engine {
let plan_state = self.config.plan_state.clone();

let tool_context = self.build_tool_context(input_policy.mode, input_policy.auto_approve);
// Ensure MCP pool is initialized before building the tool registry,
// so start_mcp_server can be registered when Feature::Mcp is enabled.
if self.config.features.enabled(Feature::Mcp) {
let _ = self.ensure_mcp_pool().await;
}
let builder = self
.build_turn_tool_registry_builder(input_policy.mode, todo_list, plan_state)
.with_dynamic_tools(&dynamic_tools);
Expand Down Expand Up @@ -2571,11 +2576,15 @@ impl Engine {
self.api_config.api_provider(),
&self.config.model,
);
let mut always_load = self.config.tools_always_load.clone();
if self.config.features.enabled(Feature::Mcp) {
always_load.insert("start_mcp_server".to_string());
}
let mut catalog = build_model_tool_catalog_with_surface(
registry.to_api_tools_with_cache(true),
mcp_tools,
input_policy.mode,
&self.config.tools_always_load,
&always_load,
capability.tool_surface_budget,
);
for tool in &mut catalog {
Expand Down Expand Up @@ -3113,7 +3122,10 @@ impl Engine {
&self.session.mcp_config_path,
&self.session.workspace,
)
.map_err(|e| ToolError::execution_failed(format!("Failed to load MCP config: {e}")))?;
.unwrap_or_else(|e| {
tracing::debug!("No MCP config: {e}");
McpPool::new(McpConfig::default())
});
if let Some(decider) = self.config.network_policy.as_ref() {
pool = pool.with_network_policy(decider.clone());
}
Expand Down
7 changes: 7 additions & 0 deletions crates/tui/src/core/engine/tool_setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ impl Engine {
// so there's no failure mode worth gating on.
builder = builder.with_notify_tool();

// Register the start_mcp_server tool so LLM can dynamically start
// MCP servers from conversation context. Only when the pool has been
// initialized (lazy via ensure_mcp_pool).
if let Some(ref pool) = self.mcp_pool {
builder = builder.with_runtime_mcp_tool(Arc::clone(pool));
}

builder
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/tui/src/core/engine/turn_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ fn normalize_domain_candidate(value: &str) -> Option<String> {
}

fn registered_tool_requires_non_bypassable_approval(tool_name: &str) -> bool {
matches!(tool_name, "rlm_eval")
matches!(tool_name, "rlm_eval" | "start_mcp_server")
}

impl Engine {
Expand Down
60 changes: 51 additions & 9 deletions crates/tui/src/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
//! - Automatic tool discovery via `tools/list`
//! - Configurable timeouts per-server and globally

use parking_lot::RwLock;
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;

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
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();
}
1 change: 1 addition & 0 deletions crates/tui/src/tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub mod remember;
pub mod revert_turn;
pub mod review;
pub mod rlm;
pub mod runtime_mcp;
pub mod schema_canonicalize;
pub mod schema_sanitize;
pub mod search;
Expand Down
15 changes: 15 additions & 0 deletions crates/tui/src/tools/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,21 @@ impl ToolRegistryBuilder {
self
}

/// Register the `start_mcp_server` tool for dynamically adding MCP servers
/// from conversation context. Does not register MCP tool adapters — those
/// are returned by `pool.to_api_tools()` in `engine.mcp_tools()`.
#[must_use]
pub fn with_runtime_mcp_tool(
mut self,
mcp_pool: std::sync::Arc<tokio::sync::Mutex<crate::mcp::McpPool>>,
) -> Self {
self.tools
.push(Arc::new(super::runtime_mcp::StartRuntimeMcpServer::new(
mcp_pool,
)));
self
}

/// Include all agent tools (file tools + shell + note + search).
///
/// Web and patch tools are NOT registered here — callers must add them
Expand Down
Loading