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.
This commit is contained in:
dotCooCoo
2026-10-24 17:29:33 +07:00
committed by bflatastic
parent 58827952b7
commit e71bf71376
3 changed files with 84 additions and 3 deletions
+62
View File
@@ -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<std::path::PathBuf> {
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 [
+18 -3
View File
@@ -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<String, String> {
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<Value, String> {
b.call("engine.execute", json!({"command": cmd, "params": params}))
}
@@ -810,7 +820,8 @@ fn transform(b: &mut dyn Backend, a: &Args) -> Result<Value, String> {
}
fn screenshot(b: &mut dyn Backend, a: &Args) -> Result<ToolResult, String> {
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<ToolResult, Str
j(b.call("ui.set", json!({"panel": req_str(a, "panel")?}))?)
}
"screenshot" => 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<ToolResult, Str
params.insert((*k).into(), v.clone());
}
}
if let Some(path) = params.get("path").and_then(Value::as_str) {
let path = absolute_path(path)?;
params.insert("path".into(), json!(path));
}
let cmd = if a.get("selection").and_then(Value::as_bool) == Some(true) { "document.exportSelection" } else { "document.export" };
j(exec(b, cmd, Value::Object(params))?)
}
+4
View File
@@ -107,6 +107,10 @@ the suite) keep the agent to the folders you give it:
The result is compared with the root by whole folder names: `/work2/x` is not inside `/work`. On
Windows case is ignored, UNC paths (`\\server\share\…`) are resolved like drive-letter ones, and
device names (`CON`, `NUL`, `COM1`…) are refused.
- With `--connect`, the server makes the paths of `open_file`, `save_file`, `export` (`path` or
`options.path`) and `screenshot {path}` absolute against its own working directory before it sends
them to the app. Paths inside `run_command` and `command_batch` params go to the app as given, and the
app resolves them against its own working directory.
- A refused path is an ordinary tool error the agent can read and correct, for example
`automation path rejected: outside the write root /work/project/out: /etc/x.png` (with several,
`outside the write roots /work/project/out, /tmp/scratch: …`) or