From fe162f8f2326c89e8adf0d6743fd24ecfa44e0dc Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sat, 10 Oct 2026 19:04:03 +0200 Subject: [PATCH] Report an error when file commands get no file in MCP and CLI (#135) --- apps/gridcraft-cli/src/main.rs | 3 +-- crates/engine/src/lib.rs | 19 +++++++++++++++++++ crates/mcp/src/headless.rs | 8 ++------ crates/mcp/src/tests.rs | 11 +++++++++++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/apps/gridcraft-cli/src/main.rs b/apps/gridcraft-cli/src/main.rs index c97b1d637..452448e4f 100644 --- a/apps/gridcraft-cli/src/main.rs +++ b/apps/gridcraft-cli/src/main.rs @@ -145,8 +145,7 @@ impl Args { // ---------------------------------------------------------------- helpers fn exec(s: &mut Session, id: &str, params: Value) -> Result { - let r = s.execute(id, params).map_err(|e| e.to_string()); - s.take_ui_requests(); + let r = s.execute_headless(id, params).map_err(|e| e.to_string()); if id == "file.open" && let Ok(value) = &r && let Some(warnings) = value.get("warnings").and_then(Value::as_array) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a13e5cef1..de786a1f4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -311,6 +311,25 @@ impl Session { pub fn take_ui_requests(&mut self) -> Vec { std::mem::take(&mut self.ui_requests) } + + /// Runs a command for a caller with no UI (MCP, CLI): queued UI requests are dropped, and a command that only + /// queued a file dialog because it got no file (`file.open`, `file.saveAs`, `file.save` on a workbook with no + /// file yet, `data.getData`) is an error instead of a silent success. + pub fn execute_headless(&mut self, id: &str, params: Json) -> Result { + let r = self.execute(id, params); + let requests = self.take_ui_requests(); + let asked_for_file = requests.iter().any(|q| matches!(q, UiRequest::Dialog(name, _) if name == "open" || name == "saveAs")); + if r.is_ok() && asked_for_file { + let what = match id { + "file.open" => "`path` or `base64`", + "data.getData" | "data.fromTextCsv" => "`path` or `text`", + _ => "`path`", + }; + let msg = format!("missing {what}: programmatic calls never open dialogs"); + return Err(EngineError::BadParams { cmd: id.to_string(), msg }); + } + r + } } /// Convenience: the active workbook's active sheet. diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index ec649ea08..5f96fbd25 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -34,12 +34,8 @@ impl Headless { } fn execute(&mut self, id: &str, params: Value) -> Result { - let r = self.session.execute(id, params).map_err(|e| e.to_string()); - // Programmatic calls never open dialogs; drop anything a command queued for a UI. - for req in self.session.take_ui_requests() { - log::debug!("headless: ignoring UI request {req:?}"); - } - r + // Programmatic calls never open dialogs; anything a command queued for a UI is dropped. + self.session.execute_headless(id, params).map_err(|e| e.to_string()) } fn commands(&self, params: &Value) -> Value { diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 8e37a21e7..cf590bedb 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -381,3 +381,14 @@ fn remote_backend_over_tcp() { handle.join().unwrap(); assert!(Remote::connect("127.0.0.1:1").is_err()); } + +#[test] +fn file_commands_without_a_file_fail() { + let mut s = server(); + // `file.save` on Book1, which has no file yet, would ask for one too. + for command in ["file.open", "file.saveAs", "file.save", "data.getData", "data.fromTextCsv"] { + let msg = fails(&mut s, "execute_command", json!({"command": command})); + assert!(msg.contains("missing") && msg.contains("never open dialogs"), "{command}: {msg}"); + fails(&mut s, "execute_command", json!({"command": command, "params": {}})); + } +}