| 1 | //! A real MCP client that speaks JSON-RPC to a spawned child process. |
| 2 | //! |
| 3 | //! `codewhale mcp-server` used to wire every configured server to an |
| 4 | //! in-memory stub, so a user's `command`/`args`/`env` were never executed and |
| 5 | //! every health probe answered `{"status": "ok"}` from a hardcoded literal |
| 6 | //! (#4727). A fabricated success is the worst possible answer here: it is |
| 7 | //! indistinguishable from a working integration. This module replaces it with |
| 8 | //! an actual subprocess connection, and every failure path below returns an |
| 9 | //! error naming the server rather than a plausible-looking value. |
| 10 | |
| 11 | use std::io::{BufRead, BufReader, Write}; |
| 12 | use std::process::{Child, ChildStdin, Command, Stdio}; |
| 13 | use std::sync::Mutex; |
| 14 | use std::sync::mpsc::{Receiver, RecvTimeoutError, channel}; |
| 15 | use std::thread; |
| 16 | use std::time::{Duration, Instant}; |
| 17 | |
| 18 | use anyhow::{Context, Result, anyhow, bail}; |
| 19 | use serde_json::{Value, json}; |
| 20 | |
| 21 | use crate::{McpManagedClient, McpResourceDescriptor, McpServerConfig, McpToolDescriptor}; |
| 22 | |
| 23 | /// Protocol revision advertised during the handshake. Matches the revision the |
| 24 | /// TUI's MCP pool negotiates (`crates/tui/src/mcp.rs`), so a server that works |
| 25 | /// in the TUI works here. |
| 26 | const PROTOCOL_VERSION: &str = "2024-11-05"; |
| 27 | |
| 28 | /// Budget for spawn + `initialize` + `notifications/initialized`. Generous |
| 29 | /// because a first `npx`/`uvx` launch may download the server package. |
| 30 | const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(30); |
| 31 | |
| 32 | /// Budget for a single request once the server is up. |
| 33 | const REQUEST_TIMEOUT: Duration = Duration::from_secs(120); |
| 34 | |
| 35 | /// How long a dropped client waits for a graceful exit after closing stdin |
| 36 | /// before it kills the child. |
| 37 | const SHUTDOWN_GRACE: Duration = Duration::from_millis(500); |
| 38 | |
| 39 | /// How long a failure path waits for a dying child's exit status before giving |
| 40 | /// up and reporting the failure without one. |
| 41 | const EXIT_STATUS_GRACE: Duration = Duration::from_millis(200); |
| 42 | |
| 43 | /// Upper bound on `*/list` pagination follow-ups, so a server that keeps |
| 44 | /// echoing the same cursor cannot pin us in a loop. |
| 45 | const MAX_LIST_PAGES: usize = 100; |
| 46 | |
| 47 | /// What the server said it supports in its `initialize` response. |
| 48 | /// |
| 49 | /// `None` means the server sent no `capabilities` object at all. Those are |
| 50 | /// treated as legacy servers and probed optimistically; an explicit |
| 51 | /// capabilities object is honoured, because a tools-only server answers |
| 52 | /// `resources/list` with a "method not found" error that would otherwise fail |
| 53 | /// the whole aggregated listing. |
| 54 | #[derive(Debug, Clone, Copy)] |
| 55 | struct ServerCapabilities { |
| 56 | tools: bool, |
| 57 | resources: bool, |
| 58 | } |
| 59 | |
| 60 | /// A live connection to one MCP server subprocess. |
| 61 | pub struct ChildProcessMcpClient { |
| 62 | server_name: String, |
| 63 | capabilities: Option<ServerCapabilities>, |
| 64 | connection: Mutex<Connection>, |
| 65 | request_timeout: Duration, |
| 66 | } |
| 67 | |
| 68 | impl std::fmt::Debug for ChildProcessMcpClient { |
| 69 | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { |
| 70 | f.debug_struct("ChildProcessMcpClient") |
| 71 | .field("server_name", &self.server_name) |
| 72 | .finish_non_exhaustive() |
| 73 | } |
| 74 | } |
| 75 | |
| 76 | impl ChildProcessMcpClient { |
| 77 | /// Spawn `config.command` with `config.args`/`config.env` and complete the |
| 78 | /// MCP handshake. |
| 79 | /// |
| 80 | /// Returns `Err` — never a degraded-but-usable client — when the command |
| 81 | /// cannot be executed, exits immediately, or does not answer `initialize` |
| 82 | /// within `HANDSHAKE_TIMEOUT`. |
| 83 | pub fn spawn(config: &McpServerConfig) -> Result<Self> { |
| 84 | Self::spawn_with_timeouts(config, HANDSHAKE_TIMEOUT, REQUEST_TIMEOUT) |
| 85 | } |
| 86 | |
| 87 | fn spawn_with_timeouts( |
| 88 | config: &McpServerConfig, |
| 89 | handshake_timeout: Duration, |
| 90 | request_timeout: Duration, |
| 91 | ) -> Result<Self> { |
| 92 | let server_name = config.name.clone(); |
| 93 | if config.command.trim().is_empty() { |
| 94 | bail!("MCP server '{server_name}' has no command configured"); |
| 95 | } |
| 96 | |
| 97 | let mut command = Command::new(&config.command); |
| 98 | command |
| 99 | .args(&config.args) |
| 100 | .envs(&config.env) |
| 101 | .stdin(Stdio::piped()) |
| 102 | .stdout(Stdio::piped()) |
| 103 | // The child's diagnostics belong on our stderr: stdout is the |
| 104 | // JSON-RPC channel and must not be polluted, and swallowing the |
| 105 | // child's stderr is how a misconfigured server becomes a silent |
| 106 | // one. |
| 107 | .stderr(Stdio::inherit()); |
| 108 | |
| 109 | let mut child = command.spawn().with_context(|| { |
| 110 | format!( |
| 111 | "MCP server '{server_name}': failed to spawn command '{}'", |
| 112 | config.command |
| 113 | ) |
| 114 | })?; |
| 115 | |
| 116 | let stdin = child |
| 117 | .stdin |
| 118 | .take() |
| 119 | .with_context(|| format!("MCP server '{server_name}': child stdin unavailable"))?; |
| 120 | let stdout = child |
| 121 | .stdout |
| 122 | .take() |
| 123 | .with_context(|| format!("MCP server '{server_name}': child stdout unavailable"))?; |
| 124 | |
| 125 | // A dedicated reader thread keeps `recv_timeout` able to bound a wait |
| 126 | // that a blocking `read_line` on the child would not. |
| 127 | let (sender, responses) = channel(); |
| 128 | thread::spawn(move || { |
| 129 | for line in BufReader::new(stdout).lines() { |
| 130 | match line { |
| 131 | Ok(line) => { |
| 132 | if sender.send(line).is_err() { |
| 133 | break; |
| 134 | } |
| 135 | } |
| 136 | Err(_) => break, |
| 137 | } |
| 138 | } |
| 139 | }); |
| 140 | |
| 141 | let mut connection = Connection { |
| 142 | child, |
| 143 | stdin: Some(stdin), |
| 144 | responses, |
| 145 | next_id: 1, |
| 146 | }; |
| 147 | |
| 148 | let initialize = connection.request( |
| 149 | &server_name, |
| 150 | "initialize", |
| 151 | json!({ |
| 152 | "protocolVersion": PROTOCOL_VERSION, |
| 153 | "clientInfo": { |
| 154 | "name": "codewhale-mcp-server", |
| 155 | "version": env!("CARGO_PKG_VERSION") |
| 156 | }, |
| 157 | "capabilities": { |
| 158 | "tools": {}, |
| 159 | "resources": {} |
| 160 | } |
| 161 | }), |
| 162 | handshake_timeout, |
| 163 | )?; |
| 164 | |
| 165 | connection |
| 166 | .send(&json!({ |
| 167 | "jsonrpc": "2.0", |
| 168 | "method": "notifications/initialized" |
| 169 | })) |
| 170 | .with_context(|| { |
| 171 | format!("MCP server '{server_name}': failed to confirm initialization") |
| 172 | })?; |
| 173 | |
| 174 | let capabilities = initialize |
| 175 | .get("capabilities") |
| 176 | .and_then(Value::as_object) |
| 177 | .map(|caps| ServerCapabilities { |
| 178 | tools: caps.contains_key("tools"), |
| 179 | resources: caps.contains_key("resources"), |
| 180 | }); |
| 181 | |
| 182 | Ok(Self { |
| 183 | server_name, |
| 184 | capabilities, |
| 185 | connection: Mutex::new(connection), |
| 186 | request_timeout, |
| 187 | }) |
| 188 | } |
| 189 | |
| 190 | fn supports_tools(&self) -> bool { |
| 191 | self.capabilities.is_none_or(|caps| caps.tools) |
| 192 | } |
| 193 | |
| 194 | fn supports_resources(&self) -> bool { |
| 195 | self.capabilities.is_none_or(|caps| caps.resources) |
| 196 | } |
| 197 | |
| 198 | fn request(&self, method: &str, params: Value) -> Result<Value> { |
| 199 | let mut connection = self.connection.lock().map_err(|_| { |
| 200 | anyhow!( |
| 201 | "MCP server '{}': connection poisoned by an earlier panic", |
| 202 | self.server_name |
| 203 | ) |
| 204 | })?; |
| 205 | connection.request(&self.server_name, method, params, self.request_timeout) |
| 206 | } |
| 207 | |
| 208 | /// Drive a paginated `*/list` method to exhaustion, collecting `field`. |
| 209 | fn list_paginated(&self, method: &str, field: &str) -> Result<Vec<Value>> { |
| 210 | let mut items = Vec::new(); |
| 211 | let mut cursor: Option<String> = None; |
| 212 | for _ in 0..MAX_LIST_PAGES { |
| 213 | let params = match &cursor { |
| 214 | Some(cursor) => json!({ "cursor": cursor }), |
| 215 | None => json!({}), |
| 216 | }; |
| 217 | let page = self.request(method, params)?; |
| 218 | if let Some(values) = page.get(field).and_then(Value::as_array) { |
| 219 | items.extend(values.iter().cloned()); |
| 220 | } |
| 221 | let next = page |
| 222 | .get("nextCursor") |
| 223 | .and_then(Value::as_str) |
| 224 | .map(str::to_string); |
| 225 | match next { |
| 226 | // A repeated cursor is a server bug; stop rather than spin. |
| 227 | Some(next) if Some(&next) != cursor.as_ref() => cursor = Some(next), |
| 228 | _ => break, |
| 229 | } |
| 230 | } |
| 231 | Ok(items) |
| 232 | } |
| 233 | } |
| 234 | |
| 235 | impl McpManagedClient for ChildProcessMcpClient { |
| 236 | fn list_tools(&self) -> Result<Vec<McpToolDescriptor>> { |
| 237 | if !self.supports_tools() { |
| 238 | return Ok(Vec::new()); |
| 239 | } |
| 240 | let tools = self.list_paginated("tools/list", "tools")?; |
| 241 | Ok(tools |
| 242 | .iter() |
| 243 | .filter_map(|tool| { |
| 244 | let tool_name = tool.get("name")?.as_str()?.to_string(); |
| 245 | Some(McpToolDescriptor { |
| 246 | server_name: self.server_name.clone(), |
| 247 | // The manager owns qualification; report the raw name and |
| 248 | // let it build `mcp__server__tool`. |
| 249 | qualified_name: tool_name.clone(), |
| 250 | tool_name, |
| 251 | description: tool |
| 252 | .get("description") |
| 253 | .and_then(Value::as_str) |
| 254 | .map(str::to_string), |
| 255 | }) |
| 256 | }) |
| 257 | .collect()) |
| 258 | } |
| 259 | |
| 260 | fn call_tool(&self, tool_name: &str, arguments: Value) -> Result<Value> { |
| 261 | // The server's result is returned verbatim, including an `isError` |
| 262 | // content payload: reinterpreting it here would replace what the |
| 263 | // server actually said with our guess about it. |
| 264 | self.request( |
| 265 | "tools/call", |
| 266 | json!({ |
| 267 | "name": tool_name, |
| 268 | "arguments": arguments |
| 269 | }), |
| 270 | ) |
| 271 | } |
| 272 | |
| 273 | fn list_resources(&self) -> Result<Vec<McpResourceDescriptor>> { |
| 274 | if !self.supports_resources() { |
| 275 | return Ok(Vec::new()); |
| 276 | } |
| 277 | let resources = self.list_paginated("resources/list", "resources")?; |
| 278 | Ok(resources |
| 279 | .iter() |
| 280 | .filter_map(|resource| { |
| 281 | Some(McpResourceDescriptor { |
| 282 | server_name: self.server_name.clone(), |
| 283 | uri: resource.get("uri")?.as_str()?.to_string(), |
| 284 | description: resource |
| 285 | .get("description") |
| 286 | .and_then(Value::as_str) |
| 287 | .map(str::to_string), |
| 288 | }) |
| 289 | }) |
| 290 | .collect()) |
| 291 | } |
| 292 | |
| 293 | fn read_resource(&self, uri: &str) -> Result<Value> { |
| 294 | self.request("resources/read", json!({ "uri": uri })) |
| 295 | } |
| 296 | } |
| 297 | |
| 298 | struct Connection { |
| 299 | child: Child, |
| 300 | stdin: Option<ChildStdin>, |
| 301 | responses: Receiver<String>, |
| 302 | next_id: u64, |
| 303 | } |
| 304 | |
| 305 | impl Connection { |
| 306 | fn send(&mut self, message: &Value) -> Result<()> { |
| 307 | let stdin = self |
| 308 | .stdin |
| 309 | .as_mut() |
| 310 | .context("connection stdin already closed")?; |
| 311 | let mut line = serde_json::to_string(message)?; |
| 312 | line.push('\n'); |
| 313 | stdin.write_all(line.as_bytes())?; |
| 314 | stdin.flush()?; |
| 315 | Ok(()) |
| 316 | } |
| 317 | |
| 318 | fn request( |
| 319 | &mut self, |
| 320 | server: &str, |
| 321 | method: &str, |
| 322 | params: Value, |
| 323 | timeout: Duration, |
| 324 | ) -> Result<Value> { |
| 325 | let id = self.next_id; |
| 326 | self.next_id += 1; |
| 327 | if let Err(err) = self.send(&json!({ |
| 328 | "jsonrpc": "2.0", |
| 329 | "id": id, |
| 330 | "method": method, |
| 331 | "params": params |
| 332 | })) { |
| 333 | // A child that has already exited leaves us racing two symptoms of |
| 334 | // the same fact: either the reader thread sees EOF first, or our |
| 335 | // write loses the race and returns EPIPE. Which one wins is |
| 336 | // platform- and timing-dependent (macOS reliably reports the write |
| 337 | // error where Linux reports the EOF), so both report the death the |
| 338 | // same way rather than leaking a bare "Broken pipe". |
| 339 | if is_broken_pipe(&err) { |
| 340 | bail!( |
| 341 | "MCP server '{server}': process closed stdin before answering {method}{}", |
| 342 | self.exit_note() |
| 343 | ); |
| 344 | } |
| 345 | return Err(err) |
| 346 | .with_context(|| format!("MCP server '{server}': failed to send {method}")); |
| 347 | } |
| 348 | |
| 349 | let deadline = Instant::now() + timeout; |
| 350 | loop { |
| 351 | let remaining = deadline.saturating_duration_since(Instant::now()); |
| 352 | if remaining.is_zero() { |
| 353 | bail!("MCP server '{server}': {method} timed out after {timeout:?}"); |
| 354 | } |
| 355 | let line = match self.responses.recv_timeout(remaining) { |
| 356 | Ok(line) => line, |
| 357 | Err(RecvTimeoutError::Timeout) => { |
| 358 | bail!("MCP server '{server}': {method} timed out after {timeout:?}"); |
| 359 | } |
| 360 | Err(RecvTimeoutError::Disconnected) => { |
| 361 | bail!( |
| 362 | "MCP server '{server}': process closed stdout before answering {method}{}", |
| 363 | self.exit_note() |
| 364 | ); |
| 365 | } |
| 366 | }; |
| 367 | |
| 368 | // Servers occasionally emit banners or log lines on stdout, and |
| 369 | // notifications carry no id. Both are skipped; only the matching |
| 370 | // response ends the wait. |
| 371 | let Ok(message) = serde_json::from_str::<Value>(&line) else { |
| 372 | continue; |
| 373 | }; |
| 374 | if message.get("id").and_then(Value::as_u64) != Some(id) { |
| 375 | continue; |
| 376 | } |
| 377 | if let Some(error) = message.get("error") { |
| 378 | bail!("MCP server '{server}': {method} failed: {error}"); |
| 379 | } |
| 380 | return Ok(message.get("result").cloned().unwrap_or(Value::Null)); |
| 381 | } |
| 382 | } |
| 383 | |
| 384 | /// The child's exit status, when it has one, for appending to a failure |
| 385 | /// message. Polls briefly because both callers run at the moment the child |
| 386 | /// is dying: the write can return EPIPE, or stdout can hit EOF, before the |
| 387 | /// kernel has finished reaping the process. Bounded and error-path-only, |
| 388 | /// so the cost buys a real diagnostic ("exited with status 127" is the |
| 389 | /// difference between a crashed server and a missing one). |
| 390 | fn exit_note(&mut self) -> String { |
| 391 | let deadline = Instant::now() + EXIT_STATUS_GRACE; |
| 392 | loop { |
| 393 | match self.child.try_wait() { |
| 394 | Ok(Some(status)) => return format!(" (process exited with {status})"), |
| 395 | Ok(None) if Instant::now() < deadline => { |
| 396 | thread::sleep(Duration::from_millis(5)); |
| 397 | } |
| 398 | _ => return String::new(), |
| 399 | } |
| 400 | } |
| 401 | } |
| 402 | } |
| 403 | |
| 404 | /// Whether an error chain bottoms out in a broken-pipe I/O error, i.e. we wrote |
| 405 | /// to a child that had already closed its end. |
| 406 | fn is_broken_pipe(err: &anyhow::Error) -> bool { |
| 407 | err.chain().any(|cause| { |
| 408 | cause |
| 409 | .downcast_ref::<std::io::Error>() |
| 410 | .is_some_and(|io| io.kind() == std::io::ErrorKind::BrokenPipe) |
| 411 | }) |
| 412 | } |
| 413 | |
| 414 | impl Drop for Connection { |
| 415 | fn drop(&mut self) { |
| 416 | // Closing stdin is the protocol-level shutdown signal for a stdio MCP |
| 417 | // server; kill only the ones that ignore it, so servers get a chance |
| 418 | // to flush state. |
| 419 | self.stdin.take(); |
| 420 | let deadline = Instant::now() + SHUTDOWN_GRACE; |
| 421 | loop { |
| 422 | match self.child.try_wait() { |
| 423 | Ok(Some(_)) => return, |
| 424 | Ok(None) if Instant::now() < deadline => { |
| 425 | thread::sleep(Duration::from_millis(10)); |
| 426 | } |
| 427 | _ => break, |
| 428 | } |
| 429 | } |
| 430 | let _ = self.child.kill(); |
| 431 | let _ = self.child.wait(); |
| 432 | } |
| 433 | } |
| 434 | |
| 435 | #[cfg(test)] |
| 436 | mod tests { |
| 437 | use std::collections::HashMap; |
| 438 | |
| 439 | use super::*; |
| 440 | |
| 441 | fn config(command: &str, args: &[&str]) -> McpServerConfig { |
| 442 | McpServerConfig { |
| 443 | name: "probe".to_string(), |
| 444 | command: command.to_string(), |
| 445 | args: args.iter().map(|arg| (*arg).to_string()).collect(), |
| 446 | env: HashMap::new(), |
| 447 | enabled: true, |
| 448 | } |
| 449 | } |
| 450 | |
| 451 | #[test] |
| 452 | fn spawn_fails_loudly_when_the_command_does_not_exist() { |
| 453 | let err = ChildProcessMcpClient::spawn(&config( |
| 454 | "codewhale-nonexistent-mcp-server-binary", |
| 455 | &["--stdio"], |
| 456 | )) |
| 457 | .unwrap_err(); |
| 458 | let message = format!("{err:#}"); |
| 459 | assert!( |
| 460 | message.contains("failed to spawn command"), |
| 461 | "spawn failure must name the command, got: {message}" |
| 462 | ); |
| 463 | assert!( |
| 464 | message.contains("probe"), |
| 465 | "spawn failure must name the server, got: {message}" |
| 466 | ); |
| 467 | } |
| 468 | |
| 469 | #[test] |
| 470 | fn spawn_rejects_an_empty_command() { |
| 471 | let err = ChildProcessMcpClient::spawn(&config(" ", &[])).unwrap_err(); |
| 472 | assert!( |
| 473 | format!("{err:#}").contains("no command configured"), |
| 474 | "unexpected error: {err:#}" |
| 475 | ); |
| 476 | } |
| 477 | |
| 478 | #[cfg(unix)] |
| 479 | #[test] |
| 480 | fn spawn_fails_when_the_child_exits_without_answering_initialize() { |
| 481 | let err = ChildProcessMcpClient::spawn(&config("/bin/sh", &["-c", "exit 0"])).unwrap_err(); |
| 482 | let message = format!("{err:#}"); |
| 483 | // Which end reports the death first is a race the OS arbitrates — |
| 484 | // stdout EOF on Linux, an EPIPE write on macOS — so assert on what is |
| 485 | // actually contractual: the server is named, the failure is attributed |
| 486 | // to the child dying before it answered, and no raw io::Error leaks. |
| 487 | assert!( |
| 488 | message.contains("probe") && message.contains("before answering initialize"), |
| 489 | "unexpected error: {message}" |
| 490 | ); |
| 491 | assert!( |
| 492 | !message.contains("Broken pipe"), |
| 493 | "a dead child must not surface as a raw pipe error: {message}" |
| 494 | ); |
| 495 | } |
| 496 | |
| 497 | #[cfg(unix)] |
| 498 | #[test] |
| 499 | fn a_child_that_dies_mid_handshake_reports_its_exit_status() { |
| 500 | // The child closes both pipes and exits nonzero: the diagnostic has to |
| 501 | // carry the status, because "exited with 127" is what distinguishes a |
| 502 | // crashed server from a missing one. |
| 503 | let err = |
| 504 | ChildProcessMcpClient::spawn(&config("/bin/sh", &["-c", "exit 127"])).unwrap_err(); |
| 505 | let message = format!("{err:#}"); |
| 506 | assert!( |
| 507 | message.contains("before answering initialize") && message.contains("127"), |
| 508 | "unexpected error: {message}" |
| 509 | ); |
| 510 | } |
| 511 | |
| 512 | #[cfg(unix)] |
| 513 | #[test] |
| 514 | fn handshake_times_out_when_the_child_never_answers() { |
| 515 | let err = ChildProcessMcpClient::spawn_with_timeouts( |
| 516 | &config("/bin/sh", &["-c", "sleep 30"]), |
| 517 | Duration::from_millis(250), |
| 518 | Duration::from_millis(250), |
| 519 | ) |
| 520 | .unwrap_err(); |
| 521 | assert!( |
| 522 | format!("{err:#}").contains("initialize timed out"), |
| 523 | "unexpected error: {err:#}" |
| 524 | ); |
| 525 | } |
| 526 | |
| 527 | #[cfg(unix)] |
| 528 | #[test] |
| 529 | fn a_real_child_answers_tools_list_and_tools_call() { |
| 530 | let script = crate::test_support::write_fake_mcp_server("stdio_client_roundtrip"); |
| 531 | let client = ChildProcessMcpClient::spawn(&config( |
| 532 | "/bin/sh", |
| 533 | &[script.path().to_str().expect("utf-8 script path")], |
| 534 | )) |
| 535 | .expect("fake MCP server should complete the handshake"); |
| 536 | |
| 537 | let tools = client.list_tools().unwrap(); |
| 538 | assert_eq!(tools.len(), 1); |
| 539 | assert_eq!(tools[0].tool_name, "add"); |
| 540 | |
| 541 | let result = client.call_tool("add", json!({"a": 2, "b": 3})).unwrap(); |
| 542 | assert_eq!( |
| 543 | result["content"][0]["text"], "5", |
| 544 | "the answer must come from the child process: {result}" |
| 545 | ); |
| 546 | |
| 547 | // The stub's fabricated tools must be gone. |
| 548 | assert!(client.call_tool("health", json!({})).is_err()); |
| 549 | } |
| 550 | } |
| 551 |