chore(mcp): drop unused legacy sync API (340 LOC dead code)
The `// === Backward Compatibility - Sync API (Legacy) ===` block in
`mcp.rs` was tagged `TODO(integrate): Wire legacy sync API into CLI
subcommands or remove` and had zero callers — the actual CLI flows
went through the async `add_server_config` / `remove_server_config`
helpers months ago. Delete the unused structs (`McpServerInput`,
`LegacyMcpServer`, `LegacyMcpConfig`), pub fns (`list`, `add`,
`remove`, `call_tool`), private helpers (`load_legacy`,
`save_legacy`, `parse_env`, `send_request_sync`,
`read_response_with_timeout`, `read_response_sync`, `next_id`), and
the unix-only test that only exercised the dead timeout helper.
Module doc loses the "backward compatibility with existing sync
API" bullet. `std::io::{BufRead, BufReader, Write}`,
`std::process::{Command, Stdio}`, `std::sync::{Arc, Mutex}`, and
`std::time::{SystemTime, UNIX_EPOCH}` are no longer needed at the
top level (the async path uses the tokio versions and only
`Duration` from `std::time`).
This commit is contained in:
+1
-341
@@ -4,16 +4,12 @@
|
||||
//! - Connection pooling for server reuse
|
||||
//! - Automatic tool discovery via `tools/list`
|
||||
//! - Configurable timeouts per-server and globally
|
||||
//! - Backward compatibility with existing sync API
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::path::Path;
|
||||
use std::process::{Command, Stdio};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::{Duration, SystemTime, UNIX_EPOCH};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
@@ -1769,315 +1765,6 @@ pub fn format_tool_result(result: &serde_json::Value) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
// === Backward Compatibility - Sync API (Legacy) ===
|
||||
// TODO(integrate): Wire legacy sync API into CLI subcommands or remove
|
||||
|
||||
/// Legacy input struct for adding MCP servers
|
||||
#[derive(Debug, Clone)]
|
||||
#[allow(dead_code)] // Legacy sync API, not yet wired into CLI subcommands
|
||||
pub struct McpServerInput {
|
||||
pub name: String,
|
||||
pub command: String,
|
||||
pub args: Vec<String>,
|
||||
pub env: Vec<String>,
|
||||
}
|
||||
|
||||
/// Legacy MCP server struct for internal use
|
||||
#[derive(Debug, Serialize, Deserialize, Default)]
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
struct LegacyMcpServer {
|
||||
command: String,
|
||||
args: Vec<String>,
|
||||
env: HashMap<String, String>,
|
||||
#[serde(default)]
|
||||
connect_timeout: Option<u64>,
|
||||
#[serde(default)]
|
||||
execute_timeout: Option<u64>,
|
||||
#[serde(default)]
|
||||
read_timeout: Option<u64>,
|
||||
}
|
||||
|
||||
/// Legacy config wrapper for backward compatibility
|
||||
#[derive(Debug, Serialize, Deserialize, Default)]
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
struct LegacyMcpConfig {
|
||||
#[serde(default, alias = "mcpServers")]
|
||||
servers: HashMap<String, LegacyMcpServer>,
|
||||
#[serde(default)]
|
||||
timeouts: McpTimeouts,
|
||||
}
|
||||
|
||||
/// List configured MCP servers (sync, for CLI)
|
||||
#[allow(dead_code)] // Legacy sync API, not yet wired into CLI subcommands
|
||||
pub fn list(path: &Path) -> Result<()> {
|
||||
let config = load_legacy(path)?;
|
||||
if config.servers.is_empty() {
|
||||
println!("No MCP servers configured.");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
for (name, server) in config.servers {
|
||||
println!("{} -> {} {}", name, server.command, server.args.join(" "));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Add an MCP server to configuration (sync, for CLI)
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
pub fn add(path: &Path, input: McpServerInput) -> Result<()> {
|
||||
let mut config = load_legacy(path)?;
|
||||
let env = parse_env(&input.env)?;
|
||||
config.servers.insert(
|
||||
input.name.clone(),
|
||||
LegacyMcpServer {
|
||||
command: input.command,
|
||||
args: input.args,
|
||||
env,
|
||||
connect_timeout: None,
|
||||
execute_timeout: None,
|
||||
read_timeout: None,
|
||||
},
|
||||
);
|
||||
save_legacy(path, &config)?;
|
||||
println!("Added MCP server: {}", input.name);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Remove an MCP server from configuration (sync, for CLI)
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
pub fn remove(path: &Path, name: &str) -> Result<()> {
|
||||
let mut config = load_legacy(path)?;
|
||||
if config.servers.remove(name).is_some() {
|
||||
save_legacy(path, &config)?;
|
||||
println!("Removed MCP server: {name}");
|
||||
} else {
|
||||
println!("No MCP server named {name}.");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Call an MCP tool (sync, for backward compatibility)
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
pub fn call_tool(
|
||||
path: &Path,
|
||||
server: &str,
|
||||
tool: &str,
|
||||
args: &serde_json::Value,
|
||||
) -> Result<String> {
|
||||
let config = load_legacy(path)?;
|
||||
let Some(server_cfg) = config.servers.get(server) else {
|
||||
anyhow::bail!("Failed to find MCP server: {server}");
|
||||
};
|
||||
let timeouts = config.timeouts;
|
||||
let connect_timeout = server_cfg
|
||||
.connect_timeout
|
||||
.unwrap_or(timeouts.connect_timeout);
|
||||
let execute_timeout = server_cfg
|
||||
.execute_timeout
|
||||
.unwrap_or(timeouts.execute_timeout);
|
||||
let read_timeout = server_cfg.read_timeout.unwrap_or(timeouts.read_timeout);
|
||||
|
||||
let mut cmd = Command::new(&server_cfg.command);
|
||||
cmd.args(&server_cfg.args)
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped());
|
||||
|
||||
for (key, value) in &server_cfg.env {
|
||||
cmd.env(key, value);
|
||||
}
|
||||
|
||||
let mut child = cmd.spawn().with_context(|| "Failed to spawn MCP server")?;
|
||||
let mut stdin = child.stdin.take().context("Failed to open MCP stdin")?;
|
||||
let stdout = child.stdout.take().context("Failed to open MCP stdout")?;
|
||||
let reader = Arc::new(Mutex::new(BufReader::new(stdout)));
|
||||
let child = Arc::new(Mutex::new(child));
|
||||
|
||||
let init_id = next_id();
|
||||
let init_payload = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": init_id,
|
||||
"method": "initialize",
|
||||
"params": {
|
||||
"protocolVersion": "2024-11-05",
|
||||
"clientInfo": { "name": "deepseek-tui", "version": env!("CARGO_PKG_VERSION") },
|
||||
"capabilities": {}
|
||||
}
|
||||
});
|
||||
send_request_sync(&mut stdin, &init_payload)?;
|
||||
if let Err(e) = read_response_with_timeout(
|
||||
&reader,
|
||||
&child,
|
||||
init_id,
|
||||
Duration::from_secs(connect_timeout),
|
||||
read_timeout,
|
||||
) {
|
||||
if let Ok(mut child_guard) = child.lock() {
|
||||
let _ = child_guard.kill();
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
let initialized_payload = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"method": "initialized",
|
||||
"params": {}
|
||||
});
|
||||
send_request_sync(&mut stdin, &initialized_payload)?;
|
||||
|
||||
let call_id = next_id();
|
||||
let call_payload = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": call_id,
|
||||
"method": "tools/call",
|
||||
"params": {
|
||||
"name": tool,
|
||||
"arguments": args
|
||||
}
|
||||
});
|
||||
send_request_sync(&mut stdin, &call_payload)?;
|
||||
let response = match read_response_with_timeout(
|
||||
&reader,
|
||||
&child,
|
||||
call_id,
|
||||
Duration::from_secs(execute_timeout),
|
||||
read_timeout,
|
||||
) {
|
||||
Ok(result) => result,
|
||||
Err(e) => {
|
||||
if let Ok(mut child_guard) = child.lock() {
|
||||
let _ = child_guard.kill();
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
};
|
||||
|
||||
if let Ok(mut child_guard) = child.lock() {
|
||||
let _ = child_guard.kill();
|
||||
}
|
||||
|
||||
if let Some(result) = response.get("result") {
|
||||
return Ok(serde_json::to_string_pretty(result)?);
|
||||
}
|
||||
if let Some(error) = response.get("error") {
|
||||
return Ok(serde_json::to_string_pretty(error)?);
|
||||
}
|
||||
Ok(serde_json::to_string_pretty(&response)?)
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn load_legacy(path: &Path) -> Result<LegacyMcpConfig> {
|
||||
if path.exists() {
|
||||
let contents = fs::read_to_string(path)
|
||||
.with_context(|| format!("Failed to read {}", path.display()))?;
|
||||
let config = serde_json::from_str(&contents)
|
||||
.with_context(|| format!("Failed to parse {}", path.display()))?;
|
||||
Ok(config)
|
||||
} else {
|
||||
Ok(LegacyMcpConfig::default())
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn save_legacy(path: &Path, config: &LegacyMcpConfig) -> Result<()> {
|
||||
if let Some(parent) = path.parent() {
|
||||
fs::create_dir_all(parent)?;
|
||||
}
|
||||
let contents = serde_json::to_string_pretty(config)?;
|
||||
write_atomic(path, contents.as_bytes())?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn parse_env(items: &[String]) -> Result<HashMap<String, String>> {
|
||||
let mut env = HashMap::new();
|
||||
for item in items {
|
||||
let parts: Vec<&str> = item.splitn(2, '=').collect();
|
||||
if parts.len() != 2 {
|
||||
anyhow::bail!("Failed to parse MCP env var '{item}': expected KEY=VALUE");
|
||||
}
|
||||
env.insert(parts[0].to_string(), parts[1].to_string());
|
||||
}
|
||||
Ok(env)
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn send_request_sync(stdin: &mut impl Write, payload: &serde_json::Value) -> Result<()> {
|
||||
let line = serde_json::to_string(payload)?;
|
||||
stdin
|
||||
.write_all(format!("{line}\n").as_bytes())
|
||||
.with_context(|| "Failed to write MCP request")?;
|
||||
stdin.flush()?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn read_response_with_timeout(
|
||||
reader: &Arc<Mutex<BufReader<std::process::ChildStdout>>>,
|
||||
child: &Arc<Mutex<std::process::Child>>,
|
||||
id: u64,
|
||||
timeout: Duration,
|
||||
read_timeout: u64,
|
||||
) -> Result<serde_json::Value> {
|
||||
let effective_timeout = Duration::from_secs(timeout.as_secs().min(read_timeout));
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
|
||||
let reader_clone = Arc::clone(reader);
|
||||
std::thread::spawn(move || {
|
||||
let result = read_response_sync(&reader_clone, id);
|
||||
let _ = tx.send(result);
|
||||
});
|
||||
|
||||
if let Ok(result) = rx.recv_timeout(effective_timeout) {
|
||||
result
|
||||
} else {
|
||||
if let Ok(mut child_guard) = child.lock() {
|
||||
let _ = child_guard.kill();
|
||||
}
|
||||
anyhow::bail!(
|
||||
"Failed to read MCP response: timed out after {}s",
|
||||
effective_timeout.as_secs()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn read_response_sync(
|
||||
reader: &Arc<Mutex<BufReader<std::process::ChildStdout>>>,
|
||||
id: u64,
|
||||
) -> Result<serde_json::Value> {
|
||||
let mut line = String::new();
|
||||
loop {
|
||||
line.clear();
|
||||
let read = {
|
||||
let mut guard = reader
|
||||
.lock()
|
||||
.map_err(|_| anyhow::anyhow!("MCP reader lock poisoned"))?;
|
||||
guard.read_line(&mut line)?
|
||||
};
|
||||
if read == 0 {
|
||||
anyhow::bail!("Failed to read MCP response: server closed output before responding.");
|
||||
}
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if let Ok(value) = serde_json::from_str::<serde_json::Value>(trimmed)
|
||||
&& value.get("id").and_then(serde_json::Value::as_u64) == Some(id)
|
||||
{
|
||||
return Ok(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Legacy sync API
|
||||
fn next_id() -> u64 {
|
||||
let micros = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_micros();
|
||||
u64::try_from(micros).unwrap_or(u64::MAX)
|
||||
}
|
||||
|
||||
// === Unit Tests ===
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -2253,33 +1940,6 @@ mod tests {
|
||||
assert!(formatted.contains("[image content]"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(unix)]
|
||||
fn test_read_response_timeout_kills_child() {
|
||||
let mut child = Command::new("sh")
|
||||
.arg("-c")
|
||||
.arg("sleep 5")
|
||||
.stdout(Stdio::piped())
|
||||
.spawn()
|
||||
.expect("spawn sleep");
|
||||
let stdout = child.stdout.take().expect("stdout");
|
||||
let reader = Arc::new(Mutex::new(BufReader::new(stdout)));
|
||||
let child = Arc::new(Mutex::new(child));
|
||||
|
||||
let result = read_response_with_timeout(&reader, &child, 1, Duration::from_secs(1), 1);
|
||||
|
||||
assert!(result.is_err());
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(err.contains("timed out"));
|
||||
|
||||
let status = child
|
||||
.lock()
|
||||
.expect("lock child")
|
||||
.wait()
|
||||
.expect("wait child");
|
||||
assert!(!status.success());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_mcp_pool_empty_config() {
|
||||
let pool = McpPool::new(McpConfig::default());
|
||||
|
||||
Reference in New Issue
Block a user