diff --git a/ROADMAP.md b/ROADMAP.md index b04c4b69..d9dd8231 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -137,6 +137,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-28 | MCP accepts repeated `--automation-read-root` and `--automation-write-root` flags for access to multiple folders (#1154) | | 2026-10-25 | Windows installer: the Start menu shortcut takes the icon embedded in `deckcraft.exe` instead of an MSI ``, so a taskbar pin keeps its icon after an update (#1114); a pin that already lost it has to be unpinned and pinned again once | | 2026-10-24 | AutoFormat as you type: a paragraph that starts with `*`, `-`, `--`, `>`, `<>`, `1.`, `1)`, `I.`, `i.`, `A.`, `a.` or `a)` and a space or tab becomes a bulleted or numbered list in that style; Backspace right after puts the typed text back, and Tools ▸ AutoCorrect and `text.autocorrect {lists}` turn it off | | 2026-10-24 | Tables: while the caret is in a cell, dragging into another cell selects that range of cells (shaded, merged cells taken in whole), and Shift+arrow grows or shrinks it; formatting applies to the selected cells only, and Delete clears their text and keeps the table | @@ -294,6 +295,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-28 | trivial | Progress log: MCP accepts multiple automation read and write roots (#1154). Scores and milestone names unchanged | | 2026-10-25 | trivial | Progress log: Windows shortcut icon survives updates (#1114). Scores and milestone names unchanged | | 2026-10-24 | trivial | Progress log: selecting a range of table cells by dragging. Scores and milestone names unchanged | | 2026-10-24 | trivial | Progress log: measurement units in centimetres (#1326). Scores unchanged | diff --git a/apps/deckcraft-cli/src/automation_args.rs b/apps/deckcraft-cli/src/automation_args.rs index 8f0cabf8..a83d1fa0 100644 --- a/apps/deckcraft-cli/src/automation_args.rs +++ b/apps/deckcraft-cli/src/automation_args.rs @@ -4,29 +4,24 @@ use std::path::PathBuf; pub struct AutomationArgs { - pub read_root: Option, - pub write_root: Option, - read_given: bool, - write_given: bool, + pub read_roots: Vec, + pub write_roots: Vec, } impl AutomationArgs { pub fn new(read_root: Option, write_root: Option) -> Self { - Self { read_root, write_root, read_given: false, write_given: false } + Self { read_roots: read_root.into_iter().collect(), write_roots: write_root.into_iter().collect() } } - /// Returns whether this argument is a root flag. A second occurrence of a flag is an error, - /// including when the two spellings (`--flag DIR`, `--flag=DIR`) are mixed. + /// Returns whether this argument is a root flag. Both spellings (`--flag DIR` and + /// `--flag=DIR`) may be repeated. pub fn consume(&mut self, arg: &str, args: &mut impl Iterator) -> Result { let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value))); - let (root, given) = match flag { - "--automation-read-root" => (&mut self.read_root, &mut self.read_given), - "--automation-write-root" => (&mut self.write_root, &mut self.write_given), + let roots = match flag { + "--automation-read-root" => &mut self.read_roots, + "--automation-write-root" => &mut self.write_roots, _ => return Ok(false), }; - if *given { - return Err(format!("{flag} given twice")); - } let value = match inline { Some(value) => value.to_owned(), None => { @@ -40,8 +35,7 @@ impl AutomationArgs { if value.is_empty() { return Err(format!("{flag} needs a non-empty folder")); } - *root = Some(value.into()); - *given = true; + roots.push(value.into()); Ok(true) } } @@ -51,17 +45,16 @@ mod tests { use super::*; #[test] - fn roots_are_taken_once_and_other_arguments_are_left() { + fn roots_are_repeated_and_other_arguments_are_left() { let mut roots = AutomationArgs::new(None, None); let mut args = ["a folder".to_owned(), "deck.pptx".to_owned()].into_iter(); assert!(roots.consume("--automation-read-root", &mut args).unwrap()); + assert!(roots.consume("--automation-read-root=other", &mut args).unwrap()); assert!(roots.consume("--automation-write-root=out", &mut args).unwrap()); assert!(!roots.consume("--sample", &mut args).unwrap()); - assert_eq!(roots.read_root, Some("a folder".into())); - assert_eq!(roots.write_root, Some("out".into())); + assert_eq!(roots.read_roots, [PathBuf::from("a folder"), PathBuf::from("other")]); + assert_eq!(roots.write_roots, [PathBuf::from("out")]); assert_eq!(args.next().as_deref(), Some("deck.pptx")); - assert!(roots.consume("--automation-read-root=other", &mut args).is_err()); - assert_eq!(roots.read_root, Some("a folder".into())); } #[test] @@ -71,8 +64,8 @@ mod tests { let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into())); let mut args = operand.map(str::to_owned).into_iter(); assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}"); - assert_eq!(roots.read_root, Some("read".into())); - assert_eq!(roots.write_root, Some("write".into())); + assert_eq!(roots.read_roots, [PathBuf::from("read")]); + assert_eq!(roots.write_roots, [PathBuf::from("write")]); } let mut roots = AutomationArgs::new(None, None); assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err()); @@ -84,7 +77,7 @@ mod tests { let mut roots = AutomationArgs::new(None, None); assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap()); assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap()); - assert_eq!(roots.read_root, Some("-folder".into())); - assert_eq!(roots.write_root, Some("./-folder".into())); + assert_eq!(roots.read_roots, [PathBuf::from("-folder")]); + assert_eq!(roots.write_roots, [PathBuf::from("./-folder")]); } } diff --git a/apps/deckcraft-cli/src/help.rs b/apps/deckcraft-cli/src/help.rs index 4227905f..dfc2bd66 100644 --- a/apps/deckcraft-cli/src/help.rs +++ b/apps/deckcraft-cli/src/help.rs @@ -127,16 +127,16 @@ and prints its answer as JSON. COMMAND [PARAMS] runs a command, as run --cmd doe }, Subcommand { name: "mcp", - usage: "mcp [--connect PORT] [--sample] [--automation-read-root DIR] [--automation-write-root DIR]", + usage: "mcp [--connect PORT] [--sample] [--automation-read-root DIR]... [--automation-write-root DIR]...", summary: "serve the commands to an agent over MCP on stdio", details: "Runs a Model Context Protocol server on stdin and stdout. It works on its own presentation without a window, or with --connect PORT (or host:port) on the DeckCraft app started with --control PORT. --sample opens the sample presentation. --automation-read-root DIR and --automation-write-root DIR confine every file the agent's commands read -(open, pictures, audio, video) and write (save, export, PDF) to those folders, links followed; a root left -out grants none of its access. They run the in-process server, so they conflict with --connect (a running -app is not confined by them).", +(open, pictures, audio, video) and write (save, export, PDF) to those folders, links followed. Repeat either +flag to grant access to more folders; a root left out grants none of its access. They run the in-process +server, so they conflict with --connect (a running app is not confined by them).", }, Subcommand { name: "bench", diff --git a/apps/deckcraft-cli/src/main.rs b/apps/deckcraft-cli/src/main.rs index c2c38e10..9c53eaf5 100644 --- a/apps/deckcraft-cli/src/main.rs +++ b/apps/deckcraft-cli/src/main.rs @@ -624,7 +624,7 @@ fn mcp(args: &[String]) -> Result<(), String> { other => return Err(format!("unknown mcp option `{other}`")), } } - let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?; + let roots = AutomationRoots::new(&roots_args.read_roots, &roots_args.write_roots)?; if roots.is_some() && connect.is_some() { return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \ so drop --connect (or use --connect without the roots)" @@ -641,8 +641,12 @@ so drop --connect (or use --connect without the roots)" if let Some(roots) = roots { // Every thread of this process, not only the server's: nothing reaches past the roots. file_access::confine_process(roots.clone()); - let show = |root: Option<&std::path::Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string()); - eprintln!("deckcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root())); + let show = |roots: Vec| if roots.is_empty() { "none".to_string() } else { roots.join(", ") }; + eprintln!( + "deckcraft-cli: files confined: read roots {}, write roots {}", + show(roots.read_roots().map(|r| r.display().to_string()).collect()), + show(roots.write_roots().map(|r| r.display().to_string()).collect()) + ); h = h.with_automation_roots(Some(roots)); } Box::new(h) diff --git a/apps/deckcraft-cli/tests/it/mcp_roots.rs b/apps/deckcraft-cli/tests/it/mcp_roots.rs index f42782c4..c80f8fe3 100644 --- a/apps/deckcraft-cli/tests/it/mcp_roots.rs +++ b/apps/deckcraft-cli/tests/it/mcp_roots.rs @@ -87,6 +87,45 @@ fn mcp_automation_roots_confine_the_server() { let _ = std::fs::remove_dir_all(base); } +#[test] +fn mcp_accepts_multiple_read_and_write_roots() { + let base = tmpdir("multiple-roots"); + let read1 = base.join("read1"); + let read2 = base.join("read2"); + let write1 = base.join("write1"); + let write2 = base.join("write2"); + for dir in [&read1, &read2, &write1, &write2] { + std::fs::create_dir_all(dir).unwrap(); + } + let (input1, input2) = (read1.join("one.txt"), read2.join("two.txt")); + std::fs::write(&input1, "one").unwrap(); + std::fs::write(&input2, "two").unwrap(); + let (output1, output2) = (write1.join("one.deckcraft"), write2.join("two.deckcraft")); + let args = [ + "--automation-read-root".to_owned(), + read1.to_string_lossy().into_owned(), + format!("--automation-read-root={}", read2.display()), + "--automation-write-root".to_owned(), + write1.to_string_lossy().into_owned(), + format!("--automation-write-root={}", write2.display()), + ]; + let args = args.iter().map(String::as_str).collect::>(); + let calls = [ + ("open_presentation", serde_json::json!({"path": input1})), + ("open_presentation", serde_json::json!({"path": input2})), + ("save_presentation", serde_json::json!({"path": output1})), + ("save_presentation", serde_json::json!({"path": output2})), + ]; + let (success, stderr, replies) = mcp_session(&args, &calls); + assert!(success, "{stderr}"); + for reply in &replies { + assert_eq!(reply["result"]["isError"], false, "{reply}"); + } + assert!(output1.exists()); + assert!(output2.exists()); + let _ = std::fs::remove_dir_all(base); +} + #[test] fn mcp_automation_root_flags_are_checked_up_front() { let base = tmpdir("roots-args"); @@ -103,8 +142,12 @@ fn mcp_automation_root_flags_are_checked_up_front() { // The flags confine the in-process server, so they cannot be combined with --connect. fails(&["--connect", "7979", "--automation-read-root", dir], "--connect"); fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect"); - fails(&["--automation-read-root", dir, "--automation-read-root", dir], "given twice"); - fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice"); + for flag in ["--automation-read-root", "--automation-write-root"] { + let mut full = vec!["mcp", flag, dir, flag]; + full.push(dir); + let out = run(&full); + assert!(out.status.success(), "{flag}: {}", String::from_utf8_lossy(&out.stderr)); + } fails(&["--automation-read-root"], "needs a folder"); fails(&["--automation-write-root", "--sample"], "needs a folder"); fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs index c2fb8a3c..303e4c78 100644 --- a/crates/engine/src/file_access.rs +++ b/crates/engine/src/file_access.rs @@ -35,8 +35,8 @@ pub enum Access { /// The folders automation may read and write; a root left out grants no access of its kind. #[derive(Debug)] pub struct AutomationRoots { - read: Option, - write: Option, + read: Vec, + write: Vec, } #[derive(Debug)] @@ -62,46 +62,47 @@ impl Root { } impl AutomationRoots { - /// The roots `read` and `write`, which must be existing folders; `None` when neither is given + /// The read and write roots, which must be existing folders; `None` when both are empty /// (nothing is confined then). - pub fn new(read: Option<&Path>, write: Option<&Path>) -> Result>, String> { - if read.is_none() && write.is_none() { + pub fn new(read: &[PathBuf], write: &[PathBuf]) -> Result>, String> { + if read.is_empty() && write.is_empty() { return Ok(None); } - let read = read.map(|p| Root::open(p, Access::Read)).transpose()?; - let write = write.map(|p| Root::open(p, Access::Write)).transpose()?; + let read = read.iter().map(|p| Root::open(p, Access::Read)).collect::>()?; + let write = write.iter().map(|p| Root::open(p, Access::Write)).collect::>()?; Ok(Some(Arc::new(Self { read, write }))) } - /// The read root, absolute with its links followed. - pub fn read_root(&self) -> Option<&Path> { - self.read.as_ref().map(|r| r.shown.as_path()) + /// The read roots, absolute with their links followed. + pub fn read_roots(&self) -> impl Iterator { + self.read.iter().map(|r| r.shown.as_path()) } - /// The write root, absolute with its links followed. - pub fn write_root(&self) -> Option<&Path> { - self.write.as_ref().map(|r| r.shown.as_path()) + /// The write roots, absolute with their links followed. + pub fn write_roots(&self) -> impl Iterator { + self.write.iter().map(|r| r.shown.as_path()) } /// May the file or folder at `path` be read (or written)? `Err` says why not. pub fn check(&self, path: &str, access: Access) -> Result<(), String> { - let root = match access { - Access::Read => self.read.as_ref(), - Access::Write => self.write.as_ref(), + let roots = match access { + Access::Read => &self.read, + Access::Write => &self.write, }; - let Some(Root { resolved: folder, shown }) = root else { + if roots.is_empty() { return match access { Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {path}")), Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {path}")), }; - }; - if inside(&resolve(path)?, folder) { + } + let resolved = resolve(path)?; + if roots.iter().any(|root| inside(&resolved, &root.resolved)) { return Ok(()); } - let root = shown.display(); + let roots = roots.iter().map(|root| root.shown.display().to_string()).collect::>().join(", "); match access { - Access::Read => Err(format!("automation path rejected: outside the read root {root}: {path}")), - Access::Write => Err(format!("automation path rejected: outside the write root {root}: {path}")), + Access::Read => Err(format!("automation path rejected: outside the read roots {roots}: {path}")), + Access::Write => Err(format!("automation path rejected: outside the write roots {roots}: {path}")), } } } @@ -276,7 +277,7 @@ mod tests { } std::fs::write(base.join("in/a.deckcraft"), "x").unwrap(); std::fs::write(base.join("outside/secret.deckcraft"), "x").unwrap(); - let roots = AutomationRoots::new(Some(&base.join("in")), Some(&base.join("out"))).unwrap().unwrap(); + let roots = AutomationRoots::new(&[base.join("in")], &[base.join("out")]).unwrap().unwrap(); (base, roots) } @@ -300,6 +301,17 @@ mod tests { let _ = std::fs::remove_dir_all(base); } + #[test] + fn any_configured_root_grants_access() { + let (base, _) = folders("multiple"); + let roots = AutomationRoots::new(&[base.join("in"), base.join("in2")], &[base.join("out"), base.join("outside")]).unwrap().unwrap(); + assert_eq!(roots.check(&s(base.join("in/a.deckcraft")), Access::Read), Ok(())); + assert_eq!(roots.check(&s(base.join("in2/b.deckcraft")), Access::Read), Ok(())); + assert_eq!(roots.check(&s(base.join("out/new.deckcraft")), Access::Write), Ok(())); + assert_eq!(roots.check(&s(base.join("outside/new.deckcraft")), Access::Write), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + #[test] fn a_sibling_with_the_same_prefix_is_outside() { let (base, roots) = folders("prefix"); @@ -336,18 +348,18 @@ mod tests { #[test] fn a_missing_root_is_an_error_and_none_is_no_confinement() { let (base, _) = folders("missing"); - let e = AutomationRoots::new(Some(&base.join("nope")), None).unwrap_err(); + let e = AutomationRoots::new(&[base.join("nope")], &[]).unwrap_err(); assert!(e.starts_with("cannot open the automation read root"), "{e}"); - let e = AutomationRoots::new(None, Some(&base.join("in/a.deckcraft"))).unwrap_err(); + let e = AutomationRoots::new(&[], &[base.join("in/a.deckcraft")]).unwrap_err(); assert!( e.starts_with("cannot open the automation write root") && e.ends_with(&std::io::Error::from(std::io::ErrorKind::NotADirectory).to_string()), "{e}" ); - assert!(AutomationRoots::new(Some(Path::new("")), None).unwrap_err().starts_with("cannot open the automation read root ``")); - assert!(AutomationRoots::new(None, None).unwrap().is_none()); + assert!(AutomationRoots::new(&[PathBuf::new()], &[]).unwrap_err().starts_with("cannot open the automation read root ``")); + assert!(AutomationRoots::new(&[], &[]).unwrap().is_none()); // One root alone grants only its own access. - let read_only = AutomationRoots::new(Some(&base.join("in")), None).unwrap().unwrap(); + let read_only = AutomationRoots::new(&[base.join("in")], &[]).unwrap().unwrap(); let e = read_only.check(&s(base.join("in/x.deckcraft")), Access::Write).unwrap_err(); assert!(e.contains("write authority is absent"), "{e}"); let _ = std::fs::remove_dir_all(base); @@ -384,7 +396,7 @@ mod tests { assert_eq!(roots.check(&s(base.join("in/alias/x.deckcraft")), Access::Read), Ok(())); // A root given through a link is the folder it leads to. assert!(made(&base.join("in"), &base.join("in-link"))); - let linked = AutomationRoots::new(Some(&base.join("in-link")), None).unwrap().unwrap(); + let linked = AutomationRoots::new(&[base.join("in-link")], &[]).unwrap().unwrap(); assert_eq!(linked.check(&s(base.join("in/a.deckcraft")), Access::Read), Ok(())); let _ = std::fs::remove_dir_all(base); } diff --git a/crates/mcp/tests/it/automation_roots.rs b/crates/mcp/tests/it/automation_roots.rs index 75229987..ba5734c5 100644 --- a/crates/mcp/tests/it/automation_roots.rs +++ b/crates/mcp/tests/it/automation_roots.rs @@ -31,7 +31,9 @@ fn confined(tag: &str, read: bool, write: bool) -> (Server, PathBuf) { std::fs::write(base.join("outside/pic.png"), PNG).unwrap(); std::fs::write(base.join("in2/c.txt"), "Sibling").unwrap(); let (in_dir, out_dir) = (base.join("in"), base.join("out")); - let roots = AutomationRoots::new(read.then_some(in_dir.as_path()), write.then_some(out_dir.as_path())).unwrap(); + let read = read.then(|| in_dir.to_path_buf()).into_iter().collect::>(); + let write = write.then(|| out_dir.to_path_buf()).into_iter().collect::>(); + let roots = AutomationRoots::new(&read, &write).unwrap(); let mut server = Server::new(Box::new(Headless::with_document().with_automation_roots(roots))); ok(&mut server, "new_presentation", json!({})); (server, base) @@ -215,7 +217,7 @@ fn without_roots_nothing_is_confined() { let _ = std::fs::remove_dir_all(&base); std::fs::create_dir_all(&base).unwrap(); std::fs::write(base.join("a.txt"), "Anywhere").unwrap(); - let mut server = Server::new(Box::new(Headless::with_document().with_automation_roots(AutomationRoots::new(None, None).unwrap()))); + let mut server = Server::new(Box::new(Headless::with_document().with_automation_roots(AutomationRoots::new(&[], &[]).unwrap()))); ok(&mut server, "open_presentation", json!({"path": s(&base.join("a.txt"))})); ok(&mut server, "save_presentation", json!({"path": s(&base.join("b.deckcraft"))})); assert!(base.join("b.deckcraft").exists()); diff --git a/docs/mcp.md b/docs/mcp.md index 0b3b3a3d..a42d1fb8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -36,8 +36,8 @@ deckcraft-cli mcp --automation-read-root /work/project --automation-write-root / "args": ["mcp", "--automation-read-root", "/work/project", "--automation-write-root", "/work/project/out"]}}} ``` -The two flags (VectorCraft, PhotoCraft and the other Craft apps take the same ones, so one client -config works across the suite) keep the agent to the folders you give it: +The two flags keep the agent to the folders you give it. Repeat either flag to allow access to more +folders: - Every file a command reads must lie inside the **read root**: `open_presentation` and `file.open`, and `insert_picture`, `insert.picture`, `picture.change`, `insert.audio` and @@ -59,8 +59,8 @@ config works across the suite) keep the agent to the folders you give it: - 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.pptx` or `automation filesystem access is not granted: write authority is absent: /work/project/out/x.pptx`. -- The roots must be existing folders; a missing one stops the server with an error. Each flag is - given once (`--flag ` or `--flag=`). +- The roots must be existing folders; a missing one stops the server with an error. Repeat either flag + to allow access to another folder; both `--flag ` and `--flag=` forms work. The flags confine the in-process server, so `--connect` together with them is an error: the server would otherwise talk to an app that isn't confined. DeckCraft's desktop app has no equivalent flags