From 538c7c2d07e86ed12713e41e17df9af5c4d7514a Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 20:49:18 -0700 Subject: [PATCH] MCP: headless file tools pass relative paths to the engine as given Since #3971, open_file, save_file, export and screenshot made every path absolute against the server's working directory, headless as well as with --connect. Headless, the engine runs in the server's process and resolves a relative path against the same directory, so the conversion is only needed for a connected app. It also made a relative path fail headless when the absolute path is not UTF-8. The server now makes these paths absolute only for a connected app, as docs/mcp.md describes. Headless, it sends them as given. --- crates/mcp/src/tests.rs | 35 ++++++++++++++++++++++++++++++++++ crates/mcp/src/tests_fileio.rs | 5 +++-- crates/mcp/src/tools.rs | 21 +++++++++++--------- 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 0220455f7..eeeb15126 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -743,6 +743,41 @@ fn headless_file_tools_take_relative_paths() { assert_eq!(r["isError"], false, "{r}"); } +#[test] +fn headless_file_tools_send_paths_as_given() { + use std::cell::RefCell; + use std::rc::Rc; + + struct Recording(Rc>>); + impl Backend for Recording { + fn call(&mut self, _: &str, params: Value) -> Result { + self.0.borrow_mut().push(params); + Ok(json!({})) + } + fn has_ui(&self) -> bool { + false + } + fn describe(&self) -> String { + "test".into() + } + } + + let seen = Rc::new(RefCell::new(Vec::new())); + let mut s = Server::new(Box::new(Recording(seen.clone()))); + call(&mut s, 1, "open_file", json!({"path": "rel/in.svg"})); + call(&mut s, 2, "save_file", json!({"path": "rel/out.vectorcraft"})); + call(&mut s, 3, "export", json!({"path": "rel/out.png"})); + call(&mut s, 4, "export", json!({"format": "svg", "options": {"path": "rel/opt.svg"}})); + call(&mut s, 5, "screenshot", json!({"path": "rel/shot.png"})); + let seen = seen.borrow(); + assert_eq!(seen.len(), 5, "{seen:?}"); + assert_eq!(seen[0]["path"], "rel/in.svg"); + assert_eq!(seen[1]["path"], "rel/out.vectorcraft"); + assert_eq!(seen[2]["params"]["path"], "rel/out.png"); + assert_eq!(seen[3]["params"]["path"], "rel/opt.svg"); + assert_eq!(seen[4]["path"], "rel/shot.png"); +} + #[test] fn remote_rejects_uncorrelated_replies_without_replaying() { for reply in [ diff --git a/crates/mcp/src/tests_fileio.rs b/crates/mcp/src/tests_fileio.rs index 1788f558e..486f969cf 100644 --- a/crates/mcp/src/tests_fileio.rs +++ b/crates/mcp/src/tests_fileio.rs @@ -306,8 +306,9 @@ fn save_file_waits_for_the_file_in_the_app() { assert_eq!(r.is_error, ui, "the app's error is the call's: {r:?}"); let [(method, params)] = &b.seen[..] else { panic!("one call: {:?}", b.seen) }; assert_eq!(method, "app.save"); - // The tool sends the path made absolute (on Windows, against the current drive). - let path = std::path::absolute("/out/a.vectorcraft").unwrap().to_string_lossy().into_owned(); + // The tool sends the app the path made absolute (on Windows, against the current drive), and + // a headless session the path as given. + let path = if ui { std::path::absolute("/out/a.vectorcraft").unwrap().to_string_lossy().into_owned() } else { "/out/a.vectorcraft".into() }; let mut want = json!({"path": path}); if ui { want["wait"] = json!(true); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index a11bbb098..b2c459706 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -520,11 +520,11 @@ fn req_str<'a>(a: &'a Args, k: &str) -> Result<&'a str, String> { a.get(k).and_then(Value::as_str).filter(|s| !s.is_empty()).ok_or_else(|| format!("missing string argument `{k}`")) } -/// A file tool's path made absolute against this process's working directory. An empty path is -/// returned as given. -fn absolute_path(p: &str) -> Result { - if p.is_empty() { - return Ok(String::new()); +/// A file tool's path made absolute against this process's working directory when it goes to the app +/// (`--connect`). Headless, and for an empty path, it is returned as given. +fn absolute_path(b: &dyn Backend, p: &str) -> Result { + if p.is_empty() || !b.has_ui() { + return Ok(p.to_owned()); } let abs = std::path::absolute(p).map_err(|e| format!("{p}: {e}"))?; abs.into_os_string().into_string().map_err(|abs| format!("{p}: the absolute path {} is not UTF-8", abs.to_string_lossy())) @@ -838,7 +838,7 @@ fn transform(b: &mut dyn Backend, a: &Args) -> Result { } fn screenshot(b: &mut dyn Backend, a: &Args) -> Result { - let path = a.get("path").and_then(Value::as_str).map(absolute_path).transpose()?; + let path = a.get("path").and_then(Value::as_str).map(|p| absolute_path(b, p)).transpose()?; let path = path.as_deref(); let window = a.get("window").and_then(Value::as_bool) == Some(true); let max = max_side(a)?; @@ -1084,9 +1084,12 @@ fn dispatch(b: &mut dyn Backend, name: &str, a: &Args) -> Result screenshot(b, a), - "open_file" => j(b.call("app.open", json!({"path": absolute_path(req_str(a, "path")?)?}))?), + "open_file" => { + let path = absolute_path(b, req_str(a, "path")?)?; + j(b.call("app.open", json!({"path": path}))?) + } "save_file" => { - let mut p = json!({"path": a.get("path").and_then(Value::as_str).map(absolute_path).transpose()?}); + let mut p = json!({"path": a.get("path").and_then(Value::as_str).map(|p| absolute_path(b, p)).transpose()?}); for k in SAVE_FILE_OPTIONS { if let Some(v) = a.get(k).filter(|v| !v.is_null()) { p[k] = v.clone(); @@ -1121,7 +1124,7 @@ fn dispatch(b: &mut dyn Backend, name: &str, a: &Args) -> Result