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