feat: MCP stdio 命令解析与错误诊断增强
- 新增 resolve_command_path：Windows 上搜索 .exe/.cmd/.bat 后缀 - 启动失败时给出友好错误信息（含 PATH 和建议） - 捕获子进程 stderr 并在连接失败时回显 - ConfigPage 新增 cwd 工作目录字段 - 添加单元测试覆盖路径解析逻辑
This commit is contained in:
oudecheng
parent
1309fa28da
commit
76abdbd1de
@ -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<PathBuf> {
|
||||
#[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<RoleClient, ()>;
|
||||
|
||||
@ -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<String, String>,
|
||||
_cwd: &Option<std::path::PathBuf>,
|
||||
cwd: &Option<PathBuf>,
|
||||
) -> anyhow::Result<McpClient> {
|
||||
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(|| "<inherit>".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<Mutex<Vec<String>>> = 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()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -26,6 +26,7 @@ interface McpServerConfig {
|
||||
command?: string
|
||||
args?: string[]
|
||||
env?: Record<string, string>
|
||||
cwd?: string
|
||||
base_url?: string
|
||||
headers?: Record<string, string>
|
||||
description?: string
|
||||
@ -746,8 +747,9 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
|
||||
<Field label="描述"><input value={s.description ?? ''} onChange={e => updMcp(name, { description: e.target.value || undefined })} className={inputCls} placeholder="可选描述" /></Field>
|
||||
{s.type === 'stdio' && (
|
||||
<>
|
||||
<Field label="命令" hint="如 npx, node, cargo"><input value={s.command ?? ''} onChange={e => updMcp(name, { command: e.target.value })} className={inputCls} placeholder="npx" /></Field>
|
||||
<Field label="命令" hint="如 npx, node, cargo, uv"><input value={s.command ?? ''} onChange={e => updMcp(name, { command: e.target.value })} className={inputCls} placeholder="npx" /></Field>
|
||||
<Field label="参数" hint="空格分隔"><input value={(s.args ?? []).join(' ')} onChange={e => updMcp(name, { args: e.target.value ? e.target.value.split(/\s+/) : [] })} className={inputCls} placeholder="-y @modelcontextprotocol/server-filesystem /tmp" /></Field>
|
||||
<Field label="工作目录 (cwd)" hint="可选。子进程运行目录，常用于 uv/python 项目解析 pyproject.toml 或 venv"><input value={s.cwd ?? ''} onChange={e => updMcp(name, { cwd: e.target.value || undefined })} className={inputCls} placeholder="E:\code_project\my-mcp-server" /></Field>
|
||||
<Field label="环境变量" hint="KEY=VALUE，每行一个">
|
||||
<textarea
|
||||
value={Object.entries(s.env ?? {}).map(([k, v]) => `${k}=${v}`).join('\n')}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user