diff --git a/src/mcp/client.rs b/src/mcp/client.rs index 9e319ab..0f4f440 100644 --- a/src/mcp/client.rs +++ b/src/mcp/client.rs @@ -7,7 +7,7 @@ //! - Dynamically registers MCP tools via the Tool trait adapter use std::collections::HashMap; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use tokio::sync::RwLock; use rmcp::{ @@ -18,10 +18,13 @@ use rmcp::{ transport::streamable_http_client::{StreamableHttpClientTransport, StreamableHttpClientTransportConfig}, }; use http::{HeaderName, HeaderValue}; +use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; use crate::mcp::config::{McpServerConfig, McpTransportConfig}; use std::env; +use std::path::PathBuf; +use std::process::Stdio; /// Resolve ${ENV_VAR} placeholders in a value string fn resolve_env_placeholders_in_value(value: &str) -> String { @@ -33,6 +36,65 @@ fn resolve_env_placeholders_in_value(value: &str) -> String { .to_string() } +/// Resolve a command name to an executable path. +/// +/// On Windows, if the command has no file extension, searches PATH for +/// `.exe`, `.cmd`, `.bat` variants (Rust's `Command::new` only finds `.exe`). +/// On non-Windows platforms, returns `None` (defers to OS default behavior). +fn resolve_command_path(command: &str) -> Option { + #[cfg(windows)] + { + let path = PathBuf::from(command); + + // If already an absolute path with extension, use directly + if path.is_absolute() { + if path.is_file() { + return Some(path); + } + // Absolute path but doesn't exist — try appending extensions if no ext + if path.extension().is_none() { + for ext in &["exe", "cmd", "bat"] { + let candidate = path.with_extension(ext); + if candidate.is_file() { + return Some(candidate); + } + } + } + return None; + } + + // If it has a path separator (relative path), don't search PATH + if path.parent().map(|p| !p.as_os_str().is_empty()).unwrap_or(false) { + return None; + } + + // Bare command name — search PATH with extensions + let extensions: &[&str] = if path.extension().is_some() { + &[""] // already has extension, try as-is + } else { + &[".exe", ".cmd", ".bat"] + }; + + let path_env = std::env::var_os("PATH")?; + for dir in std::env::split_paths(&path_env) { + for ext in extensions { + let candidate = dir.join(format!("{}{}", command, ext)); + if candidate.is_file() { + return Some(candidate); + } + } + } + None + } + + #[cfg(not(windows))] + { + // On non-Windows, defer to OS default behavior + let _ = command; + None + } +} + /// Type alias for the MCP client service pub type McpClient = RunningService; @@ -183,7 +245,7 @@ impl McpClientManager { let transport = config.transport().map_err(|e| anyhow::anyhow!("{}", e))?; let client = match transport { McpTransportConfig::Stdio { command, args, env, cwd } => { - self.connect_stdio(&command, &args, &env, &cwd).await? + self.connect_stdio(key, &command, &args, &env, &cwd).await? } McpTransportConfig::Http { url, headers } => { self.connect_http(&url, &headers).await? @@ -219,12 +281,54 @@ impl McpClientManager { /// Connect via stdio transport (spawn child process) async fn connect_stdio( &self, + server_key: &str, command: &str, args: &[String], env: &HashMap, - _cwd: &Option, + cwd: &Option, ) -> anyhow::Result { - let mut cmd = Command::new(command); + // Resolve command path (Windows extension handling) + let resolved_command = resolve_command_path(command); + let effective_command: PathBuf = match &resolved_command { + Some(p) => p.clone(), + None => PathBuf::from(command), + }; + + // Pre-flight check on Windows: if command is bare name and not resolved, give friendly error + #[cfg(windows)] + if resolved_command.is_none() { + let path = std::path::Path::new(command); + let is_absolute = path.is_absolute(); + let has_separator = path.parent().map(|p| !p.as_os_str().is_empty()).unwrap_or(false); + if !is_absolute && !has_separator && path.extension().is_none() { + // Bare name not found on Windows + let path_env = std::env::var("PATH").unwrap_or_default(); + return Err(anyhow::anyhow!( + "Command '{}' not found in PATH (tried .exe, .cmd, .bat). \ + Current PATH: {}. \ + Suggestion: use the full absolute path to the executable, \ + or ensure the tool is installed and its directory is in PATH.", + command, path_env + )); + } + } + + let env_keys: Vec<&String> = env.keys().collect(); + let cwd_display = cwd + .as_ref() + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "".to_string()); + + tracing::info!( + server_key = %server_key, + command = %effective_command.display(), + args = ?args, + env_keys = ?env_keys, // only keys, NOT values (avoid leaking secrets) + cwd = %cwd_display, + "Spawning MCP stdio child process" + ); + + let mut cmd = Command::new(&effective_command); cmd.args(args); // Set environment variables @@ -232,10 +336,70 @@ impl McpClientManager { cmd.env(key, value); } - let transport = TokioChildProcess::new(cmd)?; + // Set working directory if specified + if let Some(cwd_path) = cwd { + cmd.current_dir(cwd_path); + } + + // Shared buffer to collect stderr lines for error reporting + let stderr_lines: Arc>> = Arc::new(Mutex::new(Vec::new())); + let stderr_lines_for_task = stderr_lines.clone(); + + // Use builder to capture stderr (default is inherit) + let (transport, stderr_opt) = TokioChildProcess::builder(cmd) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| { + anyhow::anyhow!( + "Failed to spawn MCP stdio child process '{}': {}", + effective_command.display(), + e + ) + })?; + + // Spawn a background task to read stderr and log it + if let Some(child_stderr) = stderr_opt { + let server_key_owned = server_key.to_string(); + tokio::spawn(async move { + let reader = BufReader::new(child_stderr); + let mut lines = reader.lines(); + while let Ok(Some(line)) = lines.next_line().await { + tracing::warn!( + server_key = %server_key_owned, + stderr = %line, + "MCP child process stderr" + ); + // Also collect into the shared buffer (cap at 50 lines) + if let Ok(mut buf) = stderr_lines_for_task.lock() { + if buf.len() < 50 { + buf.push(line); + } + } + } + }); + } // Use default client handler (empty tuple) - let client = ().serve(transport).await?; + let client = ().serve(transport).await.map_err(|e| { + // Include stderr summary in error if available + let stderr_summary = stderr_lines + .lock() + .ok() + .map(|buf| { + if buf.is_empty() { + String::new() + } else { + format!("\nstderr:\n {}", buf.join("\n ")) + } + }) + .unwrap_or_default(); + anyhow::anyhow!( + "Failed to establish MCP stdio connection '{}': {}{}", + effective_command.display(), + e, + stderr_summary + ) + })?; // Track that we have a stdio (child process) connection self.stdio_client_count.fetch_add(1, std::sync::atomic::Ordering::SeqCst); @@ -598,4 +762,57 @@ impl McpInitializer { } Ok(()) } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_resolve_command_path_returns_none_for_nonexistent_bare_name() { + // A bare name that definitely doesn't exist + let result = resolve_command_path("this_binary_definitely_does_not_exist_xyz123"); + // On all platforms, this should return None (either because it's not found, + // or because non-Windows always returns None) + #[cfg(windows)] + assert!(result.is_none(), "Expected None for nonexistent command on Windows"); + #[cfg(not(windows))] + assert!(result.is_none(), "Expected None on non-Windows (always returns None)"); + } + + #[test] + fn test_resolve_command_path_absolute_path_existing() { + // Use an absolute path to a known existing executable + #[cfg(windows)] + let cmd = "C:\\Windows\\System32\\cmd.exe"; + #[cfg(not(windows))] + let cmd = "/bin/ls"; + + let result = resolve_command_path(cmd); + #[cfg(windows)] + assert!(result.is_some(), "Expected to find {} on Windows", cmd); + #[cfg(not(windows))] + assert!(result.is_none(), "Expected None on non-Windows for absolute path"); + } + + #[test] + fn test_resolve_command_path_absolute_path_nonexistent() { + let result = resolve_command_path("/definitely/not/a/real/path/binary123"); + assert!(result.is_none()); + } + + #[test] + #[cfg(windows)] + fn test_resolve_command_path_finds_known_windows_binary() { + // cmd.exe should always be in C:\Windows\System32 which is in PATH + let result = resolve_command_path("cmd"); + assert!(result.is_some(), "Expected to find cmd.exe via PATH on Windows"); + if let Some(p) = result { + assert!( + p.to_string_lossy().to_lowercase().ends_with("cmd.exe"), + "Expected resolved path to end with cmd.exe, got: {}", + p.display() + ); + } + } } \ No newline at end of file diff --git a/web/src/components/Settings/ConfigPage.tsx b/web/src/components/Settings/ConfigPage.tsx index b5f0860..8cb058a 100644 --- a/web/src/components/Settings/ConfigPage.tsx +++ b/web/src/components/Settings/ConfigPage.tsx @@ -26,6 +26,7 @@ interface McpServerConfig { command?: string args?: string[] env?: Record + cwd?: string base_url?: string headers?: Record description?: string @@ -746,8 +747,9 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) { updMcp(name, { description: e.target.value || undefined })} className={inputCls} placeholder="可选描述" /> {s.type === 'stdio' && ( <> - updMcp(name, { command: e.target.value })} className={inputCls} placeholder="npx" /> + updMcp(name, { command: e.target.value })} className={inputCls} placeholder="npx" /> updMcp(name, { args: e.target.value ? e.target.value.split(/\s+/) : [] })} className={inputCls} placeholder="-y @modelcontextprotocol/server-filesystem /tmp" /> + updMcp(name, { cwd: e.target.value || undefined })} className={inputCls} placeholder="E:\code_project\my-mcp-server" />