diff --git a/crates/egui_kittest_mcp/src/bridge.rs b/crates/egui_kittest_mcp/src/bridge.rs index 95dd2c4c4..f66250cc8 100644 --- a/crates/egui_kittest_mcp/src/bridge.rs +++ b/crates/egui_kittest_mcp/src/bridge.rs @@ -1,9 +1,10 @@ -//! Bridge between the MCP server and a running kittest harness child process. +//! Bridge between the MCP server and a running egui peer (a spawned kittest harness or an +//! attached live app), both reached over the same `egui_inspection` local socket. //! //! Lifecycle: -//! 1. [`Bridge::launch`] binds a local socket, spawns the target binary with -//! [`crate::HANDSHAKE_ENV_VAR`] + `KITTEST_INSPECTOR=1` + -//! `KITTEST_INSPECTOR_PATH=`, and waits for the shim to connect. +//! 1. [`Bridge::launch`] (kittest harness) / [`Bridge::prepare_attach`] + [`Bridge::accept_pending`] +//! (live app) bind a local socket and point the peer at it via +//! [`egui_inspection::INSPECTION_SOCKET_ENV_VAR`]; the peer dials in directly. //! 2. A reader task decodes [`HarnessMessage`]s from the socket and updates [`SharedState`]. //! 3. A writer task drains [`InspectorCommand`]s queued by MCP tool handlers and writes //! them to the socket. @@ -29,6 +30,10 @@ use tokio::time::timeout; /// Hard cap matching `inspector_api::MAX_MESSAGE_BYTES` so framing-level DoS is bounded. const MAX_MESSAGE_BYTES: usize = 256 * 1024 * 1024; +/// Accept timeout for [`Bridge::launch`]. Generous because the spawned `cargo test` / `cargo +/// run` child typically compiles before its harness dials in. +const LAUNCH_ACCEPT_TIMEOUT_SECS: u64 = 120; + /// One in-flight peer (a spawned kittest harness or an attached live app) + the tasks /// that talk to it. pub struct Bridge { @@ -141,34 +146,26 @@ impl SharedState { } impl Bridge { + /// Spawn a kittest harness binary and bridge to it. Binds a local socket, spawns the + /// child with [`egui_inspection::INSPECTION_SOCKET_ENV_VAR`] pointed at it, and accepts + /// the harness's inbound connection — the same mechanism as [`Self::prepare_attach`] + + /// [`Self::accept_pending`], which it reuses. pub async fn launch( bin: PathBuf, args: Vec, env: Vec<(String, String)>, cwd: Option, ) -> anyhow::Result { - let self_path = std::env::current_exe() - .context("get current_exe for KITTEST_INSPECTOR_PATH")?; - - let socket_target = - generate_socket_target().context("allocate handshake socket")?; - let name = socket_name(&socket_target.name) - .with_context(|| format!("parse socket name {}", socket_target.name))?; - let listener = ListenerOptions::new() - .name(name) - .create_tokio() - .with_context(|| format!("bind {}", socket_target.name))?; + let (listener, socket_target) = Self::prepare_attach().await?; let mut cmd = Command::new(&bin); cmd.args(&args) - .env("KITTEST_INSPECTOR", "1") - .env("KITTEST_INSPECTOR_PATH", &self_path) - .env(crate::HANDSHAKE_ENV_VAR, &socket_target.name) + .env(egui_inspection::INSPECTION_SOCKET_ENV_VAR, &socket_target.name) .stdin(std::process::Stdio::null()) - // Harness inspector path: the child's stdout/stderr aren't ours — they get - // captured by the shim. We don't need them in the MCP server. .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) + // Inherit stderr so harness panics and cargo build errors surface where the + // operator can see them, instead of being silently swallowed. + .stderr(std::process::Stdio::inherit()) .kill_on_drop(true); for (k, v) in &env { cmd.env(k, v); @@ -177,47 +174,18 @@ impl Bridge { cmd.current_dir(d); } - let mut child = cmd + let child = cmd .spawn() .with_context(|| format!("spawn {}", bin.display()))?; let pid = child.id().unwrap_or(0); - // Accept with a short timeout. If the binary fails to start, exits early, or - // doesn't have the inspector wired up, we surface that instead of hanging forever. - let stream = match timeout(Duration::from_secs(10), listener.accept()).await { - Ok(Ok(stream)) => stream, - Ok(Err(e)) => { - let _ = child.kill().await; - bail!("accept on handshake socket: {e}"); - } - Err(_) => { - let _ = child.kill().await; - // Try to report the child's exit status if it died early. - let status_hint = match child.try_wait() { - Ok(Some(s)) => format!(" (child exited {s})"), - _ => String::new(), - }; - bail!("timed out waiting for inspector handshake{status_hint}"); - } - }; - - let (reader, writer) = stream.split(); - let state = SharedState::new(); - let (cmd_tx, cmd_rx) = mpsc::unbounded_channel(); - let child_arc: Arc>> = Arc::new(Mutex::new(Some(child))); - - let reader_task = tokio::spawn(read_loop(reader, state.clone(), child_arc.clone())); - let writer_task = tokio::spawn(write_loop(writer, cmd_rx)); - - Ok(Self { - state, - cmd_tx, - _reader_task: reader_task, - _writer_task: writer_task, - child: child_arc, - _socket_target: socket_target, - peer_info: PeerInfo::Launched { bin, args, pid }, - }) + // The child usually compiles before it runs (cargo test/run), so allow a generous + // window before giving up on the handshake. + let accept_timeout = Duration::from_secs(LAUNCH_ACCEPT_TIMEOUT_SECS); + let mut bridge = + Self::accept_pending(listener, socket_target, Some(child), accept_timeout).await?; + bridge.peer_info = PeerInfo::Launched { bin, args, pid }; + Ok(bridge) } /// Bind a local socket and return it immediately. The caller is responsible for @@ -454,15 +422,15 @@ mod tests { use interprocess::local_socket::prelude::*; /// Full cross-platform transport round-trip: a tokio `interprocess` listener (the bridge - /// side) accepts a connection from a sync `interprocess` client (the plugin / shim side), - /// and a framed `Hello` is decoded into shared state. Runs against whatever local-socket + /// side) accepts a connection from a sync `interprocess` client (the egui peer side), and + /// a framed `Hello` is decoded into shared state. Runs against whatever local-socket /// backend the host uses (unix domain socket on unix, named pipe on Windows). #[tokio::test] async fn handshake_roundtrip() { let (listener, target) = Bridge::prepare_attach().await.unwrap(); let name = target.name.clone(); - // Connect + write from a blocking thread, mirroring how the plugin/shim dial in. + // Connect + write from a blocking thread, mirroring how an egui peer dials in. let client = std::thread::spawn(move || { let n = socket_name(&name).unwrap(); let mut stream = Stream::connect(n).unwrap(); diff --git a/crates/egui_kittest_mcp/src/main.rs b/crates/egui_kittest_mcp/src/main.rs index d47c77fc8..de44534b9 100644 --- a/crates/egui_kittest_mcp/src/main.rs +++ b/crates/egui_kittest_mcp/src/main.rs @@ -1,36 +1,22 @@ -//! `kittest-mcp` — dual-role binary. +//! `kittest-mcp` — an MCP server. //! -//! Default role: **MCP server**. Speaks MCP JSON-RPC over stdio to an agent. Exposes a -//! `launch` tool that spawns a target egui kittest binary with the inspector protocol -//! pointed back at this same executable in shim mode. -//! -//! Shim role: activated when [`HANDSHAKE_ENV_VAR`] is set. The target binary's -//! [`egui_kittest::InspectorPlugin`] thinks it's talking to the regular `kittest_inspector` -//! over stdio; in reality it's talking to us, and we relay the bytes to the MCP server -//! over a unix domain socket. +//! Speaks MCP JSON-RPC over stdio to an agent. Exposes a `launch` tool that binds a local +//! socket, spawns a target egui kittest binary with [`egui_inspection::INSPECTION_SOCKET_ENV_VAR`] +//! pointed at it, and accepts the harness's inbound connection — the same mechanism as +//! `attach` for live apps. The harness's [`egui_kittest::InspectorPlugin`] dials the socket +//! directly, so there's no relaying middleman. mod bridge; mod server; -mod shim; mod tools; mod tree; -/// Env var carrying the unix socket path the shim should connect to. -pub const HANDSHAKE_ENV_VAR: &str = "KITTEST_MCP_HANDSHAKE"; - fn main() -> anyhow::Result<()> { - if let Ok(socket_path) = std::env::var(HANDSHAKE_ENV_VAR) { - // Shim role: relay bytes between harness stdio and the MCP server's socket. - // No tokio runtime — keep the dependency surface tiny and the relay deterministic. - shim::run(&socket_path) - } else { - // Server role: MCP over stdio. - init_tracing(); - let rt = tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build()?; - rt.block_on(server::run()) - } + init_tracing(); + let rt = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build()?; + rt.block_on(server::run()) } fn init_tracing() { diff --git a/crates/egui_kittest_mcp/src/shim.rs b/crates/egui_kittest_mcp/src/shim.rs deleted file mode 100644 index 9da9710a8..000000000 --- a/crates/egui_kittest_mcp/src/shim.rs +++ /dev/null @@ -1,51 +0,0 @@ -//! Inspector shim role. -//! -//! Connects to the MCP server's local socket and relays bytes in both directions between -//! the harness's stdio and that socket. -//! -//! From the harness's perspective we're an ordinary `kittest_inspector` (msgpack framed -//! messages on stdin/stdout). The MCP server sees the same framed bytes on the other end of -//! the socket. We don't parse or interpret anything here — pure byte relay keeps the shim -//! independent of protocol revisions. - -use std::io::{Read as _, Write as _}; -use std::thread; - -use egui_inspection::transport::socket_name; -use interprocess::local_socket::{Stream, prelude::*}; - -pub fn run(socket: &str) -> anyhow::Result<()> { - let name = socket_name(socket).map_err(|e| anyhow::anyhow!("socket name {socket}: {e}"))?; - let stream = Stream::connect(name).map_err(|e| anyhow::anyhow!("connect {socket}: {e}"))?; - let (mut reader, mut stdin_to_socket) = stream.split(); - - // Thread A: stdin (from harness) → socket (to MCP server). - let t_in = thread::Builder::new() - .name("kittest-mcp-shim-stdin".into()) - .spawn(move || { - let mut stdin = std::io::stdin().lock(); - let _ = std::io::copy(&mut stdin, &mut stdin_to_socket); - // EOF on stdin or write error → drop the send half so the peer sees EOF on the - // write direction. - drop(stdin_to_socket); - })?; - - // Thread B: socket (from MCP server) → stdout (to harness). - // Runs on main thread so the process exits when stdout closes. - let mut stdout = std::io::stdout().lock(); - let mut buf = vec![0u8; 64 * 1024]; - loop { - match reader.read(&mut buf) { - Ok(0) | Err(_) => break, - Ok(n) => { - if stdout.write_all(&buf[..n]).is_err() { - break; - } - let _ = stdout.flush(); - } - } - } - - let _ = t_in.join(); - Ok(()) -} diff --git a/crates/egui_kittest_mcp/src/tools.rs b/crates/egui_kittest_mcp/src/tools.rs index 9ca801a72..60dfbc2a6 100644 --- a/crates/egui_kittest_mcp/src/tools.rs +++ b/crates/egui_kittest_mcp/src/tools.rs @@ -392,8 +392,9 @@ pub struct BatchAction { impl Server { #[tool( description = "Spawn a kittest harness binary as a child process. The binary must \ - link `egui_kittest` and call `Harness::run()` — `InspectorPlugin` \ - auto-attaches via the `KITTEST_INSPECTOR` env var this tool sets." + link `egui_kittest` (with the `inspector` feature) and call \ + `Harness::run()` — `InspectorPlugin` auto-connects to the \ + `EGUI_INSPECTION_SOCKET` this tool binds and sets." )] async fn launch( &self,