Merge pull request #4108 from dotCooCoo/fix/mcp-headless-relative-paths

MCP: headless file tools pass relative paths to the engine as given

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<RefCell<Vec<Value>>>);
+    impl Backend for Recording {
+        fn call(&mut self, _: &str, params: Value) -> Result<Value, String> {
+            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 [

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);

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<String, String> {
-    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<String, String> {
+    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<Value, String> {
 }
 
 fn screenshot(b: &mut dyn Backend, a: &Args) -> Result<ToolResult, String> {
-    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<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": 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<ToolResult, Str
                 }
             }
             if let Some(path) = params.get("path").and_then(Value::as_str) {
-                let path = absolute_path(path)?;
+                let path = absolute_path(b, 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" };