feat: add start_mcp_server tool for dynamic MCP server startup

Allow LLM to start MCP servers from conversation context when a user
provides a command (e.g. npx ...) or URL. The tool connects to the
server and returns fully qualified tool names (mcp_{server}_{tool})
that the model discovers via tool_search.

McpPool infrastructure:
- Add dynamic_servers (parking_lot::RwLock) for runtime server configs
- add_runtime_server_config() rejects duplicate names (static or dynamic)
- get_or_connect() checks dynamic servers before static config
- server_names() includes both static and dynamic servers

Engine integration:
- Lazy pool initialization via ensure_mcp_pool() with network_policy wiring
- No eager McpPool construction in Engine::new
- start_mcp_server gated behind Feature::Mcp via tool_setup registration
- Dynamic always_load injection ensures the tool is eager (not deferred)

Security:
- ApprovalRequirement::Required — requires user approval before spawning
- Non-bypassable approval — YOLO mode cannot skip (registered_tool_requires_non_bypassable_approval)
- Reject shell wrapper commands (bash, sh, zsh, cmd, powershell)
- Reject shell metacharacters in args (redirects, pipes, chaining, $, backticks)
- Allowlist of permitted runtimes (npx, node, python, uvx, deno, ruby, cargo, etc.)
- Underscores in server names auto-converted to hyphens to prevent tool name collision

Tool implementation:
- Shell-words parsing for quoted arguments
- Name inference for npm/pnpm/node/python/uvx and Windows cmd /c
- McpAction approval classification
- with_runtime_mcp_tool() builder avoids McpToolAdapter duplication

Author: bistack

Cargo.lock

@@ -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"

crates/tui/Cargo.toml

@@ -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::{
@@ -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);
@@ -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 {
@@ -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());
         }

crates/tui/src/core/engine.rs

@@ -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
     }
 }

crates/tui/src/core/engine/tool_setup.rs

@@ -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 {

crates/tui/src/core/engine/turn_loop.rs

@@ -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;
 
@@ -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 {
@@ -1446,6 +1451,7 @@ impl McpPool {
             workspace: None,
             config_hash,
             last_mtimes: Vec::new(),
+            dynamic_servers: Arc::new(RwLock::new(HashMap::new())),
         }
     }
 
@@ -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");
@@ -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

crates/tui/src/mcp.rs

@@ -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(
@@ -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);
 }
 
@@ -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);
 }
 
@@ -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]
@@ -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]
@@ -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:?}"
     );
 }
@@ -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();
+}

crates/tui/src/mcp/tests.rs

@@ -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;

crates/tui/src/tools/mod.rs

@@ -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