From e71bf71376efe62b5f0abe7f73e52a560c7d0468 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 20 Oct 2026 20:09:37 -0700 Subject: [PATCH] MCP: the file tools send paths made absolute against the server's working directory With --connect, open_file, save_file, export and screenshot {path} sent a relative path to the app unchanged, and the app resolved it against its own working directory (/ when started from the Finder). A relative path then named a different file than it does headless, where the server resolves it against its own working directory. The server now makes these paths absolute against its working directory before it sends them, including export's options.path and the path a screenshot reply reports. Paths inside run_command and command_batch params go to the app as given. docs/mcp.md describes the rule. --- crates/mcp/src/tests.rs | 62 +++++++++++++++++++++++++++++++++++++++++ crates/mcp/src/tools.rs | 21 ++++++++++++-- docs/mcp.md | 4 +++ 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index e3e0f94c9..3777380ed 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -607,6 +607,68 @@ fn remote_does_not_replay_when_edit_reply_is_lost() { assert!(next.is_ok(), "the next call must reconnect: {next:?}"); } +#[test] +fn remote_file_tools_send_absolute_paths() { + let (addr, h) = fake_app(); + let mut s = Server::new(Box::new(Remote::connect(&addr).unwrap())); + let given = std::env::temp_dir().join("given.svg"); + let given = given.to_str().unwrap(); + call(&mut s, 1, "open_file", json!({"path": "rel/in.svg"})); + call(&mut s, 2, "open_file", json!({"path": given})); + call(&mut s, 3, "save_file", json!({"path": "rel/out.vectorcraft"})); + call(&mut s, 4, "export", json!({"path": "rel/out.png", "scale": 2})); + call(&mut s, 5, "export", json!({"format": "svg", "options": {"path": "rel/opt.svg"}})); + call(&mut s, 6, "screenshot", json!({"path": "rel/shot.png"})); + call(&mut s, 7, "run_command", json!({"command": "document.export", "params": {"path": "rel/cmd.svg"}})); + drop(s); + let seen = h.join().unwrap(); + let abs = |p: &str| std::path::absolute(p).unwrap().to_str().unwrap().to_string(); + let methods: Vec<&str> = seen.iter().map(|m| m["method"].as_str().unwrap()).collect(); + assert_eq!(methods, ["app.open", "app.open", "app.save", "engine.execute", "engine.execute", "ui.render", "engine.execute"]); + assert_eq!(seen[0]["params"]["path"], abs("rel/in.svg")); + assert_eq!(seen[1]["params"]["path"], given); + assert_eq!(seen[2]["params"]["path"], abs("rel/out.vectorcraft")); + assert_eq!(seen[3]["params"]["params"]["path"], abs("rel/out.png")); + // The format still comes from the extension the agent gave: a raster export keeps `scale`. + assert_eq!(seen[3]["params"]["params"]["scale"], 2); + assert_eq!(seen[4]["params"]["params"]["path"], abs("rel/opt.svg")); + assert_eq!(seen[5]["params"]["path"], abs("rel/shot.png")); + // Command params go to the app as given. + assert_eq!(seen[6]["params"]["params"]["path"], "rel/cmd.svg"); +} + +/// `p` written relative to this process's working directory, or None when the two are on +/// different drives. +fn relative_to_cwd(p: &std::path::Path) -> Option { + use std::path::Component; + let cwd = std::env::current_dir().unwrap(); + if cwd.components().next() != p.components().next() { + return None; + } + let mut rel = std::path::PathBuf::new(); + for _ in cwd.components().filter(|c| matches!(c, Component::Normal(_))) { + rel.push(".."); + } + for c in p.components().filter(|c| matches!(c, Component::Normal(_))) { + rel.push(c); + } + Some(rel) +} + +#[test] +fn headless_file_tools_take_relative_paths() { + let mut s = server(); + let file = tmp("relative.vectorcraft"); + let Some(rel) = relative_to_cwd(&file) else { return }; + let rel = rel.to_str().unwrap(); + assert!(std::path::Path::new(rel).is_relative()); + let r = call(&mut s, 1, "save_file", json!({"path": rel})); + assert_eq!(r["isError"], false, "{r}"); + assert!(file.is_file()); + let r = call(&mut s, 2, "open_file", json!({"path": rel})); + assert_eq!(r["isError"], false, "{r}"); +} + #[test] fn remote_rejects_uncorrelated_replies_without_replaying() { for reply in [ diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 021daa9b1..6781ddd35 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -502,6 +502,16 @@ 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()); + } + 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())) +} + fn exec(b: &mut dyn Backend, cmd: &str, params: Value) -> Result { b.call("engine.execute", json!({"command": cmd, "params": params})) } @@ -810,7 +820,8 @@ 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); + let path = a.get("path").and_then(Value::as_str).map(absolute_path).transpose()?; + let path = path.as_deref(); let window = a.get("window").and_then(Value::as_bool) == Some(true); // The backend writes the file, where its automation roots allow, and sends the image back too // (`data`): this process writes nothing. @@ -1010,8 +1021,8 @@ fn dispatch(b: &mut dyn Backend, name: &str, a: &Args) -> Result screenshot(b, a), - "open_file" => j(b.call("app.open", json!({"path": req_str(a, "path")?}))?), - "save_file" => j(b.call("app.save", json!({"path": a.get("path").and_then(Value::as_str)}))?), + "open_file" => j(b.call("app.open", json!({"path": absolute_path(req_str(a, "path")?)?}))?), + "save_file" => j(b.call("app.save", json!({"path": a.get("path").and_then(Value::as_str).map(absolute_path).transpose()?}))?), "export" => { // One params object for every backend; the named arguments win over `options` (a named // artboard or range replaces every artboard choice in `options`). @@ -1026,6 +1037,10 @@ fn dispatch(b: &mut dyn Backend, name: &str, a: &Args) -> Result