diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 102edc2d3..556d41759 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1386,6 +1386,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { gridcraft_formula::adjust::Edit::MoveToSheet { from: src, to_sheet: dest_name, to_row: at.row, to_col: at.col } }; rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e); + cx.edits.push((src_sheet_name.clone(), e)); cx.structural = true; } let end = at.offset_clamped((tiles_r * h) as i64 - 1, (tiles_c * w) as i64 - 1); diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 3c45031af..0078cb4fe 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -294,7 +294,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) }; shift_cell_features(sh, &e); let name = sh.name.clone(); - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); let sh = cx.sheet_mut(sheet)?; for (i, c) in (r.start.col..=r.end.col).enumerate() { sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1))); diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index a14fc2842..76c0134b3 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -21,7 +21,7 @@ pub fn specs() -> Vec { "Check Macro", [], None, - "{name, inputs?, timeLimit?: seconds} validates the inputs and dry-runs the macro on the workbook (then puts everything back) → {ok, errors?, needsApproval?, inputs, output?, conflicts, footprint}", + "{name, inputs?, onConflict?, overwrite?, timeLimit?: seconds} validates the inputs and dry-runs the macro on the workbook (then puts everything back) → {ok (macro.run with these parameters would go ahead without a question), errors?, needsApproval?, onConflict, inputs, output?, conflicts, footprint}", has_doc, check ), diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 90fe4ccc8..bcf77d4d0 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -340,6 +340,9 @@ pub struct Ctx<'a> { /// The command applied sheet protection itself (row/column inserts and deletes the /// protection allows), so cells it shifts aren't refused as edits of locked cells. pub protection_checked: bool, + /// The structural edits the command made, in order (the sheet's name, the edit), as + /// references were rewritten for them. + pub edits: Vec<(String, gridcraft_formula::adjust::Edit)>, } pub(crate) fn edit(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result) -> Result { @@ -357,9 +360,17 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) #[allow(clippy::disallowed_methods)] let t0 = if cfg!(target_arch = "wasm32") { None } else { std::env::var_os("GRIDCRAFT_PROFILE").map(|_| std::time::Instant::now()) }; let mut sel = d.selection.clone(); - let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false }; + let mut ctx = Ctx { + wb: (*d.wb).clone(), + changed: Vec::new(), + structural: false, + sel: &mut sel, + fit_rows: Vec::new(), + protection_checked: false, + edits: Vec::new(), + }; let r = f(&mut ctx)?; - let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx; + let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, edits, .. } = ctx; if !protection_checked { check_protection(&d.wb, &wb, &d.unlocked_ranges)?; } @@ -373,6 +384,9 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) } } } + if let Some(log) = d.edits.as_mut() { + log.extend(edits); + } if changed.len() <= 200_000 { fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row))); } @@ -400,7 +414,6 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) Ok(r) } -/// The most written cells a macro's footprint check records. /// The most cells a dry run's touch log keeps; a footprint whose log is full may be missing writes /// that kept a cell's value, so a shift is refused then. pub(crate) const TOUCH_LOG_LIMIT: usize = 1_000_000; diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index e6caa1dc7..dfab93736 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -223,7 +223,9 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: } /// Rewrites every formula reference (and chart series) for a structural edit on `target`. -pub(super) fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { +pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) { + cx.edits.push((target.to_string(), e.clone())); + let wb = &mut cx.wb; super::edit::rewrite_all_formulas(wb, target, e); for si in 0..wb.sheets.len() { let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default(); @@ -307,7 +309,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } } } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis, at, count }); + rewrite(cx, &name, &Edit::Insert { axis, at, count }); cx.structural = true; Ok(Json::Null) })?; @@ -372,7 +374,7 @@ fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } shift_sheet_features(sh, axis, at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count }); + rewrite(cx, &name, &Edit::Delete { axis, at, count }); } cx.structural = true; Ok(Json::Null) @@ -424,7 +426,7 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { sh.cells.shift_cols(before, count as i64); shift_sheet_features(sh, Axis::Cols, before, count, true); } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); + rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); let mut touched = Vec::new(); { @@ -462,14 +464,14 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { for cell in touched { cx.touch(sheet, cell); } - rewrite(&mut cx.wb, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); + rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); { let sh = cx.sheet_mut(sheet)?; sh.cells.shift_cols(delete_at + count, -(count as i64)); shift_sheet_features(sh, Axis::Cols, delete_at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); + rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); *cx.sel = crate::Selection { active: CellRef::new(active_row, final_start), anchor: CellRef::new(active_row, final_start), @@ -514,7 +516,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { } shift_cell_features(sh, &e); let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; @@ -549,7 +551,7 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result { shift_cell_features(sh, &e); } // References into the deleted cells become #REF!; the rest move. - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index b8ef1abe9..d47b78520 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -611,7 +611,7 @@ impl Drop for ScriptSandbox { /// thread is being torn down). fn script_sandbox_check(path: &str) -> Result<()> { if SCRIPT_SANDBOX.try_with(|c| c.get() > 0).unwrap_or(true) { - return Err(EngineError::Other(format!("{path}: scripts can't read or write files"))); + return Err(EngineError::Other(format!("{path}: scripts, and macro steps without the permission, can't read or write files"))); } Ok(()) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ed6c92713..844494ec5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -209,6 +209,9 @@ pub struct DocState { /// While a macro's footprint is measured: every cell a command wrote, by sheet name, even /// where it wrote what was already there. pub touched: Option>, + /// While a macro step runs: the structural edits its command made (the sheet's name, the + /// edit), so the macro's inputs, output and Repeat ranges follow their cells as formulas do. + pub edits: Option>, /// The references `edit.goTo` with `record: true` went to in this document, newest first, /// without names, each area written with its sheet (`Sheet1!B14:C22,Sheet1!F19:G30`). pub go_to_recent: Vec, @@ -303,6 +306,7 @@ impl DocState { scripts_trusted, trust, touched: None, + edits: None, go_to_recent: vec![], } } diff --git a/crates/engine/src/macros/debug.rs b/crates/engine/src/macros/debug.rs index 8c5b7e201..19aa03167 100644 --- a/crates/engine/src/macros/debug.rs +++ b/crates/engine/src/macros/debug.rs @@ -146,7 +146,7 @@ fn start(s: &mut Session, p: &Json) -> Result { }; let start_wb = s.doc()?.wb.clone(); let token = s.group_begin(&format!("Run Macro: {}", b.mac.name))?; - // The run's machine: it checks each step for writes over data when the footprint was partial. + // The run's machine: unless the rule is overwrite, it checks each step for writes over data. let mut machine = match super::run::make_way(s, &b) { Ok((machine, _)) => machine, Err(e) => { diff --git a/crates/engine/src/macros/policy.rs b/crates/engine/src/macros/policy.rs index 8f23a5b40..a84097986 100644 --- a/crates/engine/src/macros/policy.rs +++ b/crates/engine/src/macros/policy.rs @@ -1,6 +1,8 @@ -//! What a macro step may run. Macros run commands, so this is the one place that keeps a macro -//! from saving, opening or closing files, driving the app, running other macros, or reading and -//! writing files without the person knowing. +//! What a macro step may run. Macros run commands, so this keeps a macro from saving, opening or +//! closing files, driving the app, running other macros, or reading and writing files without the +//! person knowing. It goes by command id and the `path` parameter; the runner also runs every +//! step except an approved file step inside the scripts' file sandbox, so a command that touches a +//! file under another parameter can't either. use serde_json::Value as Json; @@ -47,6 +49,11 @@ const READS_FILES: &[&str] = &["data.getData", "data.fromTextCsv", "insert.pictu /// File commands that only produce bytes unless given a `path` (then they write a file). const EXPORTS: &[&str] = &["file.exportPdf", "file.exportCsv", "file.exportHtml", "file.print", "file.printPreview", "file.saveBytes"]; +/// Whether command `id` with `params` writes a file (an export or print given a `path`). +pub fn writes_file(id: &str, params: &Json) -> bool { + EXPORTS.contains(&id) && params.get("path").is_some_and(|p| !p.is_null()) +} + /// The policy for running command `id` with `params` as a macro step in a document with `trust`. /// /// Any `path` other than `null` counts as touching a file: a token such as `{"$ref": "Input"}` diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index bd30c3e11..337acc23d 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -9,24 +9,25 @@ //! 3. **Footprint**: unless the macro may overwrite, it first runs on the real workbook in dry-run //! mode, the changes are compared with `CellStore::diff`, and everything is put back. Cells it //! would write outside its inputs that already hold data are conflicts, handled by the output -//! rule: ask, fail, overwrite, or shift the data down or right (and check again). A dry run -//! that stops early (a file step, Solver) proves nothing about later steps, so the run then -//! checks each step as it goes. +//! rule: ask, fail, overwrite, or shift the data down or right (and check again). The +//! footprint can miss a write (a dry run stops early at a file step or Solver, and RAND or NOW +//! can send the run another way than the dry run), so the run also checks each step as it goes. //! 4. **Run**: one undo step (`Session::grouped`); an error at any step rolls everything back and //! names the step. //! //! The steps run on a small machine with an explicit frame stack, so the debugger can stop //! between any two steps and pick up again on a later frame. -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashMap}; use std::sync::Arc; use gridcraft_core::{CellRef, RangeRef, Value}; +use gridcraft_formula::adjust::Edit; use gridcraft_model::{Cell, CompareOp, InputKind, Macro, OnConflict, RepeatOf, Sheet, Step, StepKind, Workbook}; use serde::Serialize; use serde_json::{Value as Json, json}; -use super::policy::{Policy, macro_policy, previewable}; +use super::policy::{Policy, macro_policy, previewable, writes_file}; use super::resolve::{self, Env, Place, ResolveError}; use crate::{EngineError, Result, Selection, Session, Trust, UiRequest}; @@ -37,6 +38,9 @@ pub const MAX_EXECUTED_STEPS: u64 = 100_000; pub const DEFAULT_TIME_LIMIT_MS: f64 = 60_000.0; /// The most cells an input may have for its content to be checked. pub const MAX_INPUT_CELLS: u64 = 1_000_000; +/// The most bytes of parameters one run passes to its commands, all steps together: on the web +/// build, where there is no clock, this and the step limit bound the work a run does. +pub const MAX_PARAM_BYTES: u64 = 256 * 1024 * 1024; /// The most ranges a footprint lists per sheet (the counts stay exact). const MAX_LISTED_RANGES: usize = 500; @@ -57,11 +61,22 @@ pub struct StepError { /// The step would write over data and the conflict rule is ask: overwriting answers it. #[serde(skip_serializing_if = "std::ops::Not::not")] pub confirm: bool, + /// Files the run wrote before the step failed ("step 1 (file.exportCsv)"): rolling back the + /// workbook leaves them written. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub wrote_files: Vec, } impl StepError { fn new(step: &str, command: Option<&str>, message: impl Into) -> StepError { - StepError { step: step.to_string(), command: command.map(str::to_string), message: message.into(), edge: false, confirm: false } + StepError { + step: step.to_string(), + command: command.map(str::to_string), + message: message.into(), + edge: false, + confirm: false, + wrote_files: Vec::new(), + } } fn resolve(step: &str, command: Option<&str>, e: ResolveError) -> StepError { @@ -78,11 +93,30 @@ impl StepError { } pub fn into_engine(self, name: &str) -> EngineError { - let text = format!("Macro '{name}' stopped. {} Nothing was changed.", self.describe()); + let text = format!("Macro '{name}' stopped. {} {}", self.describe(), rolled_back(&self.wrote_files)); if self.confirm { EngineError::Confirm(text) } else { EngineError::Other(text) } } } +/// What a run that rolled back leaves: "Nothing was changed.", or the files it wrote before. +pub fn rolled_back(wrote_files: &[String]) -> String { + match wrote_files { + [] => "Nothing was changed.".into(), + [one] => format!("The workbook was put back; the file {one} wrote stays."), + many => format!("The workbook was put back; the files {} wrote stay.", join_and(many)), + } +} + +/// The size of `v` as JSON text, near enough to bound how much a run passes to its commands. +fn json_bytes(v: &Json) -> u64 { + match v { + Json::String(t) => t.len() as u64 + 2, + Json::Array(a) => a.iter().map(json_bytes).fold(2, u64::saturating_add), + Json::Object(o) => o.iter().map(|(k, v)| (k.len() as u64 + 3).saturating_add(json_bytes(v))).fold(2, u64::saturating_add), + _ => 8, + } +} + // ---------------------------------------------------------------- gate /// Checks every enabled step against the policy: denied commands and undeclared file access are @@ -430,27 +464,37 @@ pub struct Machine { pub partial: Option<(String, String)>, /// The last step's resolved parameters (for the debugger). pub last_params: Option, - /// Set when the footprint was partial: each step is checked for writes over data as it runs. + /// Files the run wrote ("step 1 (file.exportCsv)"): rolling the run back doesn't take them back. + pub wrote_files: Vec, + /// Repeat and If bodies by the path of their frame: a body is copied the first time its step + /// runs, not on every pass. + bodies: HashMap>>, + /// Bytes of parameters passed to commands so far, against [`MAX_PARAM_BYTES`]. + param_bytes: u64, + /// Set unless the macro may overwrite: each step is checked for writes over data as it runs, + /// since the footprint can miss a write (it stopped early, or the run took another path). guard: Option, } -/// Checks each step of a real run for writes over data the footprint couldn't see. +/// Checks each step of a real run for writes over data the footprint didn't see. #[derive(Clone, Debug)] struct Guard { /// The workbook when the run started, and for each sheet now, its index there (`None` for a /// sheet the run added). start: Arc, origin: Vec>, - /// Where the footprint stopped: "step 2 (file.exportCsv writes a file)". - partial: String, + /// Where the footprint stopped, if it did: "step 2 (file.exportCsv writes a file)". + partial: Option, /// The conflict rule is ask. ask: bool, } impl Guard { - fn new(start: Arc, partial: &Json, ask: bool) -> Guard { - let text = |k: &str| partial.get(k).and_then(Json::as_str).unwrap_or_default().to_string(); - let partial = format!("step {} ({})", text("step"), text("reason")); + fn new(start: Arc, partial: Option<&Json>, ask: bool) -> Guard { + let partial = partial.map(|p| { + let text = |k: &str| p.get(k).and_then(Json::as_str).unwrap_or_default().to_string(); + format!("step {} ({})", text("step"), text("reason")) + }); Guard { origin: (0..start.sheets.len()).map(Some).collect(), start, partial, ask } } @@ -528,6 +572,9 @@ impl Machine { stopped: None, partial: None, last_params: None, + wrote_files: Vec::new(), + bodies: HashMap::new(), + param_bytes: 0, guard: None, }; m.settle(); @@ -627,12 +674,15 @@ impl Machine { /// Runs the current step and moves to the next one. pub fn step(&mut self, s: &mut Session) -> std::result::Result<(), StepError> { - let Some((path, st)) = self.current().map(|(p, st)| (p, st.clone())) else { return Ok(()) }; + // The frame's steps are shared, so the step (and its bodies) isn't copied to run it. + let Some((path, steps, i)) = self.frames.last().map(|f| (child_path(&f.prefix, f.next), Arc::clone(&f.steps), f.next)) else { return Ok(()) }; + let Some(st) = steps.get(i) else { return Ok(()) }; if let Some(f) = self.frames.last_mut() { f.next += 1; } - let r = self.exec(s, &path, &st); - if r.is_err() { + let mut r = self.exec(s, &path, st); + if let Err(e) = r.as_mut() { + e.wrote_files.clone_from(&self.wrote_files); self.frames.clear(); } self.settle(); @@ -671,7 +721,8 @@ impl Machine { } if items > 0 && body.iter().any(|b| b.enabled) { let repeat = Some(Loop { of: *of, over: place, at: 0, count: items }); - self.frames.push(Frame { steps: Arc::new(body.clone()), next: 0, prefix: path.to_string(), repeat }); + let steps = self.body(path.to_string(), body); + self.frames.push(Frame { steps, next: 0, prefix: path.to_string(), repeat }); } Ok(()) } @@ -687,7 +738,9 @@ impl Machine { }; let (branch, name) = if compare(&v, *op, value) { (then, "then") } else { (otherwise, "else") }; if branch.iter().any(|b| b.enabled) { - self.frames.push(Frame { steps: Arc::new(branch.clone()), next: 0, prefix: format!("{path}.{name}"), repeat: None }); + let prefix = format!("{path}.{name}"); + let steps = self.body(prefix.clone(), branch); + self.frames.push(Frame { steps, next: 0, prefix, repeat: None }); } Ok(()) } @@ -699,6 +752,39 @@ impl Machine { } } + /// After a step that inserted, deleted or moved sheets, rows, columns or cells, moves the + /// inputs, the output and the ranges of the Repeats in progress with their cells, as the + /// references in formulas move. A step that deletes one of them, or changes the size of a + /// range a Repeat is going over, stops the run. + fn follow(&mut self, before: &Workbook, after: &Workbook, edits: &[(String, Edit)]) -> std::result::Result<(), String> { + let same_sheets = before.sheets.len() == after.sheets.len() && before.sheets.iter().zip(&after.sheets).all(|(b, a)| b.name == a.name); + if same_sheets && edits.is_empty() { + return Ok(()); + } + let (pairs, _) = pair_sheets(before, after); + let follow = |what: &str, p: Place| follow_place(before, after, &pairs, edits, p).ok_or_else(|| format!("It deleted the cells of {what}.")); + for (name, place, _) in self.inputs.iter_mut() { + *place = follow(name, *place)?; + } + if let Some(o) = self.output { + self.output = Some(follow("the output anchor", o)?); + } + for f in self.frames.iter_mut() { + let Some(l) = f.repeat.as_mut() else { continue }; + let over = follow(&format!("the range Repeat {} goes over", f.prefix), l.over)?; + if (over.range.height(), over.range.width()) != (l.over.range.height(), l.over.range.width()) { + return Err(format!("It changed the size of the range Repeat {} goes over.", f.prefix)); + } + l.over = over; + } + Ok(()) + } + + /// The shared copy of the body that runs under `prefix`. + fn body(&mut self, prefix: String, body: &[Step]) -> Arc> { + Arc::clone(self.bodies.entry(prefix).or_insert_with(|| Arc::new(body.to_vec()))) + } + fn env<'a>(&'a self, wb: &'a Workbook) -> Env<'a> { Env { wb, active_sheet: wb.active_sheet, inputs: &self.inputs, output: self.output, item: self.item() } } @@ -733,10 +819,27 @@ impl Machine { self.frames.clear(); return Ok(()); } + self.param_bytes = self.param_bytes.saturating_add(json_bytes(&resolved)); + if self.param_bytes > MAX_PARAM_BYTES { + return Err(fail(format!( + "The macro passed more than {} MB of parameters to its steps and was stopped.", + MAX_PARAM_BYTES / (1024 * 1024) + ))); + } + let file_step = matches!(macro_policy(id, &resolved, self.trust), Policy::NeedsPermission(_)); + let writes = writes_file(id, &resolved); self.last_params = Some(resolved.clone()); - let prev = self.guard.as_ref().and_then(|_| s.docs.iter().find(|d| d.uid == self.doc)).map(|d| d.wb.clone()); + let prev = s.docs.iter_mut().find(|d| d.uid == self.doc).map(|d| { + d.edits = Some(Vec::new()); + d.wb.clone() + }); let queued = s.ui_requests.len(); + // Only an approved file step reaches the file system: any other step runs as a script's + // does, so a command that reads or writes a file under another parameter name can't. + let sandbox = (!file_step).then(crate::io::ScriptSandbox::enter); let r = s.execute(id, resolved); + drop(sandbox); + let edits = s.docs.iter_mut().find(|d| d.uid == self.doc).and_then(|d| d.edits.take()).unwrap_or_default(); // Macros never open dialogs or web pages, and never start editing a cell; messages for // the person stay queued. let mut blocked = None; @@ -750,14 +853,24 @@ impl Machine { } } r.map_err(|e| fail(e.to_string()))?; + if writes { + self.wrote_files.push(format!("step {path} ({id})")); + } if let Some(b) = blocked { return Err(fail(b)); } + let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?; + if let Some(prev) = &prev { + self.follow(prev, &after, &edits).map_err(fail)?; + } if let (Some(g), Some(prev)) = (self.guard.as_mut(), prev) { - let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?; let what = g.check(&prev, &after, &self.inputs); if !what.is_empty() { - let msg = format!("It would write over data ({what}); the check before the run stopped at {} and couldn't see this.", g.partial); + let msg = match &g.partial { + Some(at) => format!("It would write over data ({what}); the check before the run stopped at {at} and couldn't see this."), + // A value such as RAND or NOW differed from the check's, and the run went another way. + None => format!("It would write over data ({what}); the check before the run didn't see this write."), + }; return Err(StepError { confirm: g.ask, ..fail(msg) }); } } @@ -765,6 +878,34 @@ impl Machine { } } +/// Where the cells of `p` are after a step: on the same sheet wherever it moved (`pairs`, from +/// [`pair_sheets`]), and moved by each structural edit the step made there, or `None` when the +/// step deleted them (or their sheet). +fn follow_place(before: &Workbook, after: &Workbook, pairs: &[(usize, Option)], edits: &[(String, Edit)], p: Place) -> Option { + let sheet = pairs.iter().find(|(b, _)| *b == p.sheet).and_then(|(_, a)| *a)?; + let mut name = after.sheet(sheet)?.name.clone(); + let old_name = before.sheet(p.sheet).map(|s| s.name.as_str()).unwrap_or_default(); + let mut text = format!("{}!{}", resolve::quote_sheet(&name), p.range.a1()); + for (target, e) in edits { + if target != &name && target != old_name { + continue; + } + let expr = gridcraft_formula::parse(&text).ok()?; + text = gridcraft_formula::print(&gridcraft_formula::adjust::adjust(expr, target, target, e)); + if text.contains("#REF!") { + return None; + } + // A cut and paste can take the cells to another sheet. + let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; + name = after.sheet(resolve::resolve_ref(&text, &env).ok()?.sheet)?.name.clone(); + } + let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; + if text.contains('$') { + text = text.replace('$', ""); + } + resolve::resolve_ref(&text, &env).ok() +} + /// If's comparison of a cell's value with a step's `value`, the way Excel's `=`, `<`… compare. fn compare(v: &Value, op: CompareOp, want: &Json) -> bool { let empty = v.is_empty() || v.as_text() == Some(""); @@ -813,9 +954,15 @@ fn compare(v: &Value, op: CompareOp, want: &Json) -> bool { #[derive(Clone, Debug, Default, Serialize)] #[serde(rename_all = "camelCase")] pub struct Footprint { - /// Steps executed by the end of the dry run that made this footprint (the run's one budget). + /// Steps executed and bytes of parameters passed by the end of the dry run that made this + /// footprint (the run's one budget). #[serde(skip)] pub executed: u64, + #[serde(skip)] + pub param_bytes: u64, + /// The Stop step that ended the dry run asks for the run to be rolled back. + #[serde(skip)] + pub rolls_back: bool, /// The dry run wrote more cells than the touch log keeps, so writes that kept a cell's value /// may be missing from `cells`: a shift cannot trust the block. #[serde(skip_serializing_if = "std::ops::Not::not")] @@ -896,6 +1043,7 @@ struct Saved { sheet_selections: Vec, unlocked_ranges: Vec<(String, String)>, trust: Trust, + go_to_recent: Vec, clipboard: Option, prefs: crate::Prefs, mode: crate::Mode, @@ -924,6 +1072,7 @@ impl Saved { sheet_selections: d.map(|d| d.sheet_selections.clone()).unwrap_or_default(), unlocked_ranges: d.map(|d| d.unlocked_ranges.clone()).unwrap_or_default(), trust: d.map_or(Trust::Local, |d| d.trust), + go_to_recent: d.map(|d| d.go_to_recent.clone()).unwrap_or_default(), clipboard: s.clipboard.clone(), prefs: s.prefs.clone(), mode: s.mode, @@ -950,6 +1099,7 @@ impl Saved { d.sheet_selections = self.sheet_selections; d.unlocked_ranges = self.unlocked_ranges; d.trust = self.trust; + d.go_to_recent = self.go_to_recent; } s.active = self.active.min(s.docs.len().saturating_sub(1)); s.clipboard = self.clipboard; @@ -972,8 +1122,9 @@ impl Saved { } /// Runs the macro in dry-run mode on its workbook, puts everything back, and says what it would -/// change. The dry run stops once `time_limit_ms` has passed on `clock`. -pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: f64, executed: u64) -> Result { +/// change. The dry run spends from `run`'s budget: its clock and time limit, its steps and the +/// bytes of parameters it passed. +pub fn footprint(s: &mut Session, p: &Prepared, run: &Machine) -> Result { let i = s.docs.iter().position(|d| d.uid == p.doc).ok_or(EngineError::NoDocument)?; s.set_active(i); let saved = Saved::take(s, p.doc); @@ -983,7 +1134,7 @@ pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: d.touched = Some(Vec::new()); } let mut m = Machine::new(p, true); - (m.clock, m.time_limit_ms, m.executed) = (clock, time_limit_ms, executed); + (m.clock, m.time_limit_ms, m.executed, m.param_bytes) = (run.clock, run.time_limit_ms, run.executed, run.param_bytes); let outcome = m.run_to_end(s); let (after, touched) = match s.docs.iter_mut().find(|d| d.uid == p.doc) { Some(d) => (d.wb.clone(), d.touched.take().unwrap_or_default()), @@ -995,7 +1146,8 @@ pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: let (after, touched) = if matches!(m.stopped, Some((_, true))) { (before.clone(), Vec::new()) } else { (after, touched) }; let mut fp = diff(&before, &after, &p.inputs, &touched); fp.log_full = touched.len() >= crate::cmd::TOUCH_LOG_LIMIT; - fp.executed = m.executed; + (fp.executed, fp.param_bytes) = (m.executed, m.param_bytes); + fp.rolls_back = matches!(m.stopped, Some((_, true))); fp.error = outcome.err(); fp.stopped = m.stopped.map(|(msg, _)| msg); fp.partial = m.partial.map(|(step, reason)| json!({"step": step, "reason": reason})); @@ -1006,6 +1158,11 @@ pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: /// without changing them (a formula written again), so a shift moves everything the macro writes. fn diff(before: &Workbook, after: &Workbook, inputs: &[Bound], touched: &[(String, CellRef)]) -> Footprint { let mut fp = Footprint::default(); + // One pass over the touch log (up to a million writes), not one per sheet. + let mut by_sheet: HashMap<&str, BTreeSet> = HashMap::new(); + for (n, c) in touched { + by_sheet.entry(n.as_str()).or_default().insert(*c); + } let (pairs, added) = pair_sheets(before, after); fp.sheets_added = added.into_iter().filter_map(|a| after.sheet(a).map(|s| s.name.clone())).collect(); for (bi, ai) in pairs { @@ -1015,7 +1172,7 @@ fn diff(before: &Workbook, after: &Workbook, inputs: &[Bound], touched: &[(Strin continue; }; let Some(after_sheet) = after.sheets.get(ai) else { continue }; - let rewritten: BTreeSet = touched.iter().filter(|(n, _)| *n == after_sheet.name).map(|(_, c)| *c).collect(); + let rewritten = by_sheet.remove(after_sheet.name.as_str()).unwrap_or_default(); if Arc::ptr_eq(bs, after_sheet) && rewritten.is_empty() { continue; } @@ -1140,15 +1297,33 @@ pub(crate) fn ranges(cells: &mut [CellRef]) -> Vec { /// stays together. A shift that would move one of the macro's inputs is refused: the inputs were /// checked and bound where they are, and only the block's columns (or rows) would move. fn make_room(s: &mut Session, fp: &Footprint, p: &Prepared, down: bool) -> Result<()> { + let wb = s.doc()?.wb.clone(); + if let Some(why) = shift_refusal(&wb, fp, p, down) { + return Err(EngineError::Other(why)); + } + let env = Env { wb: &wb, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; + let moves: Vec = fp + .sheets + .iter() + .filter(|s| s.conflict_count > 0) + .filter_map(|sh| sh.bounds_range.map(|block| resolve::a1(Place { sheet: sh.index, range: block }, &env))) + .collect(); + for range in moves { + s.execute("home.insertCells", json!({"range": range, "shift": if down { "down" } else { "right" }}))?; + } + Ok(()) +} + +/// Why shifting the data can't make room for `fp`, or `None`: the macro deletes sheets or +/// objects, the footprint couldn't follow every write, or the shift would move an input. +fn shift_refusal(wb: &Workbook, fp: &Footprint, p: &Prepared, down: bool) -> Option { if !fp.sheets_removed.is_empty() || fp.objects_removed > 0 { - return Err(EngineError::Other("Shifting can't make room: the macro deletes sheets or objects.".into())); + return Some("Shifting can't make room: the macro deletes sheets or objects.".into()); } if fp.log_full { - return Err(EngineError::Other("Shifting can't make room: the macro writes more cells than the footprint can follow.".into())); + return Some("Shifting can't make room: the macro writes more cells than the footprint can follow.".into()); } - let wb = s.doc()?.wb.clone(); - let env = Env { wb: &wb, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; - let mut moves = Vec::new(); + let env = Env { wb, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; for sh in fp.sheets.iter().filter(|s| s.conflict_count > 0) { let Some(block) = sh.bounds_range else { continue }; // What the insertion moves: the block's columns from its top row down, or its rows from @@ -1163,17 +1338,13 @@ fn make_room(s: &mut Session, fp: &Footprint, p: &Prepared, down: bool) -> Resul // The output is where the macro writes, so moving what is there is the point; the second // footprint catches data the shift moved into it. if let Some((name, pl)) = p.inputs.iter().map(|(n, pl, _)| (n.as_str(), *pl)).find(|(_, pl)| pl.sheet == sh.index && moved(&pl.range)) { - return Err(EngineError::Other(format!( + return Some(format!( "Shifting the data would move {name} ({}). Run with onConflict overwrite or fail, or make room first.", resolve::a1(pl, &env) - ))); + )); } - moves.push(resolve::a1(Place { sheet: sh.index, range: block }, &env)); } - for range in moves { - s.execute("home.insertCells", json!({"range": range, "shift": if down { "down" } else { "right" }}))?; - } - Ok(()) + None } // ---------------------------------------------------------------- run @@ -1196,6 +1367,7 @@ pub fn begin(s: &mut Session, name: &str, p: &Json) -> Result { .and_then(|i| d.wb.macros.get(i)) .cloned() .ok_or_else(|| EngineError::Other(format!("There is no macro called '{name}'.")))?; + check_params(p, &m)?; let asks = gate(&m, trust)?; approve(s, &m, doc, &asks, p.get("approve").and_then(Json::as_bool).unwrap_or(false))?; let prepared = prepare(s, &m, p.get("inputs"))?; @@ -1210,8 +1382,9 @@ pub fn begin(s: &mut Session, name: &str, p: &Json) -> Result { /// the machine for the run and whether it shifted. The dry runs count towards the run's time /// limit. /// -/// A partial footprint proves nothing about the steps it didn't reach, so then the machine -/// checks each step as it runs and stops the run at one that writes over data. +/// The footprint proves nothing about steps it didn't reach (it stopped early) or a path the run +/// takes and the dry run didn't (RAND, NOW), so the machine also checks each step as it runs and +/// stops the run at one that writes over data. pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { let (m, rule) = (&b.mac, b.rule); let mut machine = Machine::new(&b.prepared, false); @@ -1219,9 +1392,9 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { if rule == OnConflict::Overwrite { return Ok((machine, false)); } - // The dry runs and the run share one step budget as they share one clock. - let mut fp = footprint(s, &b.prepared, machine.clock, b.time_limit_ms, machine.executed)?; - machine.executed = fp.executed; + // The dry runs and the run share one budget (steps, parameters) as they share one clock. + let mut fp = footprint(s, &b.prepared, &machine)?; + (machine.executed, machine.param_bytes) = (fp.executed, fp.param_bytes); if let Some(e) = fp.error.take() { return Err(e.into_engine(&m.name)); } @@ -1230,8 +1403,17 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { let what = fp.describe_conflicts(); match rule { OnConflict::Ask => { + // Overwriting answers for every step, also the ones a partial footprint didn't reach. + let unseen = fp.partial.as_ref().map_or(String::new(), |p| { + let text = |k: &str| p.get(k).and_then(Json::as_str).unwrap_or_default().to_string(); + format!( + " The check stopped at step {} ({}), so overwriting also lets the steps after it write over data unchecked.", + text("step"), + text("reason") + ) + }); return Err(EngineError::Confirm(format!( - "Macro '{}' would write over cells that hold data ({what}). Overwrite them, or run again with onConflict shiftDown, shiftRight or fail", + "Macro '{}' would write over cells that hold data ({what}).{unseen} Overwrite them, or run again with onConflict shiftDown, shiftRight or fail", m.name ))); } @@ -1240,8 +1422,8 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { } OnConflict::ShiftDown | OnConflict::ShiftRight => { make_room(s, &fp, &b.prepared, rule == OnConflict::ShiftDown)?; - fp = footprint(s, &b.prepared, machine.clock, b.time_limit_ms, machine.executed)?; - machine.executed = fp.executed; + fp = footprint(s, &b.prepared, &machine)?; + (machine.executed, machine.param_bytes) = (fp.executed, fp.param_bytes); if let Some(e) = fp.error.take() { return Err(e.into_engine(&m.name)); } @@ -1256,10 +1438,8 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { } } } - if let Some(p) = &fp.partial { - let start = s.docs.iter().find(|d| d.uid == b.prepared.doc).ok_or(EngineError::NoDocument)?.wb.clone(); - machine.guard = Some(Guard::new(start, p, rule == OnConflict::Ask)); - } + let start = s.docs.iter().find(|d| d.uid == b.prepared.doc).ok_or(EngineError::NoDocument)?.wb.clone(); + machine.guard = Some(Guard::new(start, fp.partial.as_ref(), rule == OnConflict::Ask)); Ok((machine, shifted)) } @@ -1275,7 +1455,7 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { let before_run = machine.executed; machine.run_to_end(s).map_err(|e| e.into_engine(name))?; match machine.stopped { - Some((msg, true)) => Err(EngineError::Other(format!("Macro '{name}' stopped: {msg} Nothing was changed."))), + Some((msg, true)) => Err(EngineError::Other(format!("Macro '{name}' stopped: {msg} {}", rolled_back(&machine.wrote_files)))), stopped => { let mut out = json!({"macro": name, "ran": machine.executed.saturating_sub(before_run), "inputs": b.inputs}); if let Some((msg, _)) = stopped { @@ -1290,6 +1470,47 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { }) } +/// Refuses run parameters that would otherwise be ignored: an input the macro doesn't have, an +/// input given as something other than reference text, and `onConflict`, `overwrite`, `approve` +/// or `timeLimit` of the wrong kind (`null` counts as not given). +pub fn check_params(p: &Json, m: &Macro) -> Result<()> { + let refuse = |msg: String| Err(EngineError::Other(format!("Macro '{}': {msg}", m.name))); + let names = || m.inputs.iter().map(|i| i.name.clone()).collect::>().join(", "); + match p.get("inputs") { + None | Some(Json::Null) => {} + Some(Json::String(_)) if !m.inputs.is_empty() => {} + Some(Json::String(_)) => return refuse("it takes no inputs.".into()), + Some(Json::Object(o)) => { + for (k, v) in o { + if !m.inputs.iter().any(|i| i.name.eq_ignore_ascii_case(k)) { + return refuse(if m.inputs.is_empty() { + format!("it has no input called '{k}'; it takes no inputs.") + } else { + format!("it has no input called '{k}' (its inputs: {}).", names()) + }); + } + if !v.is_string() { + return refuse(format!("give the input {k} as reference text, such as \"B2:D9\".")); + } + } + } + Some(_) => return refuse("give `inputs` as reference text, such as \"B2:D9\", or as {\"Input\": \"B2:D9\"}.".into()), + } + let given = |k: &str| p.get(k).filter(|v| !v.is_null()); + if given("onConflict").is_some_and(|v| !v.is_string()) { + return refuse("`onConflict` must be ask, fail, overwrite, shiftDown or shiftRight.".into()); + } + for k in ["overwrite", "approve"] { + if given(k).is_some_and(|v| !v.is_boolean()) { + return refuse(format!("`{k}` must be true or false.")); + } + } + if given("timeLimit").is_some_and(|v| !v.as_f64().is_some_and(|t| t > 0.0)) { + return refuse("`timeLimit` must be a number of seconds above 0.".into()); + } + Ok(()) +} + /// The run's `timeLimit` (seconds) in milliseconds, or the default. fn time_limit(p: &Json) -> f64 { p.get("timeLimit").and_then(Json::as_f64).map(|t| (t * 1000.0).clamp(1.0, 600_000.0)).unwrap_or(DEFAULT_TIME_LIMIT_MS) @@ -1318,12 +1539,21 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { .cloned() .ok_or_else(|| EngineError::Other(format!("There is no macro called '{name}'.")))?; let mut out = json!({"macro": m.name, "ok": false, "onConflict": m.output.on_conflict.name()}); + let refused = |mut out: Json, e: String| { + out["errors"] = json!([e]); + Ok(out) + }; + if let Err(e) = check_params(p, &m) { + return refused(out, e.to_string()); + } + let rule = match conflict_rule(p, &m) { + Ok(r) => r, + Err(e) => return refused(out, e.to_string()), + }; + out["onConflict"] = json!(rule.name()); let asks = match gate(&m, trust) { Ok(a) => a, - Err(e) => { - out["errors"] = json!([e.to_string()]); - return Ok(out); - } + Err(e) => return refused(out, e.to_string()), }; let approved = s.macro_approvals.contains(&(doc, super::fingerprint(&m))); if !asks.is_empty() && !approved { @@ -1332,10 +1562,7 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { } let prepared = match prepare(s, &m, p.get("inputs")) { Ok(pr) => pr, - Err(e) => { - out["errors"] = json!([e.to_string()]); - return Ok(out); - } + Err(e) => return refused(out, e.to_string()), }; out["inputs"] = prepared.inputs_json(&s.doc()?.wb); if let Some(o) = prepared.output { @@ -1345,11 +1572,47 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { if !asks.is_empty() && !approved { return Ok(out); } - let fp = footprint(s, &prepared, Stopwatch::start(), time_limit(p), 0)?; - out["ok"] = json!(fp.error.is_none()); - out["conflicts"] = json!(fp.conflict_count()); + let mut budget = Machine::new(&prepared, false); + budget.time_limit_ms = time_limit(p); + let fp = footprint(s, &prepared, &budget)?; + // `ok`: the run would go ahead with this rule, without a question. + let mut problems = Vec::new(); if let Some(e) = &fp.error { - out["errors"] = json!([e.describe()]); + problems.push(e.describe()); + } + if let (true, Some(msg)) = (fp.rolls_back, &fp.stopped) { + problems.push(format!("A Stop step ends the run and rolls it back: {msg}")); + } + let shifts = matches!(rule, OnConflict::ShiftDown | OnConflict::ShiftRight); + if fp.conflict_count() > 0 { + let what = fp.describe_conflicts(); + match rule { + OnConflict::Ask => problems.push(format!("It would write over cells that hold data ({what}); the run asks first.")), + OnConflict::Fail => problems.push(format!("It would write over cells that hold data ({what}).")), + OnConflict::Overwrite => {} + OnConflict::ShiftDown | OnConflict::ShiftRight => { + if let Some(why) = shift_refusal(&s.doc()?.wb, &fp, &prepared, rule == OnConflict::ShiftDown) { + problems.push(why); + } + } + } + } + // The run spends its budget on the dry runs before it too: one, or two when it shifts. + let passes: u64 = match rule { + OnConflict::Overwrite => 1, + _ if shifts && fp.conflict_count() > 0 => 3, + _ => 2, + }; + if fp.executed.saturating_mul(passes) > MAX_EXECUTED_STEPS { + problems.push(format!("The run would execute more than {MAX_EXECUTED_STEPS} steps, counting the check before it ({} here).", fp.executed)); + } + if fp.param_bytes.saturating_mul(passes) > MAX_PARAM_BYTES { + problems.push(format!("The run would pass more than {} MB of parameters, counting the check before it.", MAX_PARAM_BYTES / (1024 * 1024))); + } + out["ok"] = json!(problems.is_empty()); + out["conflicts"] = json!(fp.conflict_count()); + if !problems.is_empty() { + out["errors"] = json!(problems); } out["footprint"] = serde_json::to_value(&fp).unwrap_or(Json::Null); Ok(out) diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 02b346e4d..0082f783d 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -1,7 +1,9 @@ //! Macros: running, input checks, the footprint dry run, the policy and the limits. +use std::collections::BTreeSet; use std::sync::Arc; +use gridcraft_core::CellRef; use serde_json::{Value as Json, json}; use super::{s, v}; @@ -166,13 +168,22 @@ fn conflicts_ask_fail_or_overwrite() { let journal = s.journal.len(); let sel = s.doc().unwrap().selection.clone(); let c = s.execute("macro.check", json!({"name": "Totals", "inputs": "B2:C5"})).unwrap(); - assert_eq!(c["ok"], true); + // Under ask the run would ask first, so it wouldn't go ahead as it is. + assert_eq!((&c["ok"], &c["onConflict"]), (&json!(false), &json!("ask"))); + assert_eq!(c["errors"], json!(["It would write over cells that hold data (E3 on Sheet1); the run asks first."])); assert_eq!(c["conflicts"], 1); assert_eq!(c["output"], "E2"); assert_eq!(c["footprint"]["sheets"][0]["cells"], json!(["E2:E3"])); assert_eq!(c["footprint"]["sheets"][0]["conflicts"], json!(["E3"])); assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); assert_eq!((s.journal.len(), &s.doc().unwrap().selection), (journal, &sel)); + // With the answer given, or with the rule overwrite, it would. + for p in [json!({"overwrite": true}), json!({"onConflict": "overwrite"})] { + let mut p = p; + (p["name"], p["inputs"]) = (json!("Totals"), json!("B2:C5")); + let c = s.execute("macro.check", p.clone()).unwrap(); + assert_eq!((&c["ok"], &c["onConflict"], c.get("errors")), (&json!(true), &json!("overwrite"), None), "{p}"); + } // The answer to the question overwrites. s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "overwrite": true})).unwrap(); @@ -180,6 +191,58 @@ fn conflicts_ask_fail_or_overwrite() { assert_eq!(v(&s, "H1").display(), "400"); } +/// The value of `cell` on the sheet called `sheet`. +fn on(s: &Session, sheet: &str, cell: &str) -> String { + let wb = &s.doc().unwrap().wb; + wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap().value(CellRef::parse(cell).unwrap()).display() +} + +#[test] +fn inputs_and_the_output_follow_their_cells_when_a_step_moves_them() { + // A sheet inserted in front: the input is still on Sheet1, not on whatever sheet is first now. + let mut s = s(); + let steps = json!([ + {"command": "home.insertSheet", "params": {"name": "Summary", "before": 0}}, + {"command": "cell.set", "params": {"cell": "Input", "input": "x"}} + ]); + put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps})); + s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); + assert_eq!((on(&s, "Sheet1", "B2"), on(&s, "Summary", "B2")), ("x".into(), "".into())); + + // Rows inserted above: the input and the output move down with their cells. + let mut s = super::s(); + set(&mut s, "B5", "in"); + let steps = json!([ + {"command": "home.insertRows", "params": {"rows": "2:3"}}, + {"command": "cell.set", "params": {"cell": "Input", "input": "x"}}, + {"command": "cell.set", "params": {"cell": "Output", "input": "y"}} + ]); + put( + &mut s, + json!({"name": "Down", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"anchor": "D5", "onConflict": "overwrite"}, "steps": steps}), + ); + s.execute("macro.run", json!({"name": "Down", "inputs": "B5"})).unwrap(); + assert_eq!((v(&s, "B7").display(), v(&s, "D7").display()), ("x".into(), "y".into())); + assert_eq!((v(&s, "B5").display(), v(&s, "D5").display()), ("".into(), "".into())); + + // A step that deletes an input's cells stops the run. + let mut s = super::s(); + set(&mut s, "B5", "in"); + let steps = json!([{"command": "home.deleteRows", "params": {"rows": "5"}}, {"command": "cell.set", "params": {"cell": "Input", "input": "x"}}]); + put(&mut s, json!({"name": "Gone", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps})); + let start = s.doc().unwrap().wb.clone(); + let err = s.execute("macro.run", json!({"name": "Gone", "inputs": "B5"})).unwrap_err().to_string(); + assert!(err.contains("Step 1 (home.deleteRows): It deleted the cells of Input."), "{err}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + + // A step inside a Repeat that changes the size of the range it goes over stops the run. + let mut s = super::s(); + let body = json!([{"command": "home.insertRows", "params": {"rows": "2"}}]); + put(&mut s, json!({"name": "Grow", "output": {"onConflict": "overwrite"}, "steps": [{"type": "repeat", "over": "A1:A3", "body": body}]})); + let err = s.execute("macro.run", json!({"name": "Grow"})).unwrap_err().to_string(); + assert!(err.contains("Step 1.1 (home.insertRows): It changed the size of the range Repeat 1 goes over."), "{err}"); +} + #[test] fn shifting_makes_room_and_is_checked_again() { for (rule, moved) in [("shiftDown", ["E4", "E5"]), ("shiftRight", ["F2", "F3"])] { @@ -378,6 +441,184 @@ fn a_dry_run_puts_the_journal_back_even_after_it_trimmed_it() { assert_eq!((&s.journal, s.recording), (&journal, recording)); } +#[test] +fn each_step_is_checked_for_writes_over_data() { + let dir = std::env::temp_dir().join(format!("gc-macro-guard-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let csv = dir.join("out.csv").to_string_lossy().to_string(); + let export = json!({"command": "file.exportCsv", "params": {"path": csv}}); + let solver = json!({"command": "data.solver", "params": {"objective": "C1", "variables": "A1", "constraints": ["A1 <= 5"], "method": "simplex", "saveModel": false}}); + let late = json!({"command": "cell.set", "params": {"cell": "B1", "input": "over"}}); + // A file step or Solver, then a step that writes over B1: the run stops there. + for first in [&export, &solver] { + let mut s = super::s(); + set(&mut s, "A1", "1"); + set(&mut s, "C1", "=A1"); + set(&mut s, "B1", "keep"); + put(&mut s, json!({"name": "Late", "inputs": [{"name": "Input"}], "permissions": ["file.exportCsv"], "steps": [first, late]})); + let start = s.doc().unwrap().wb.clone(); + for rule in ["fail", "ask"] { + let r = s.execute("macro.run", json!({"name": "Late", "inputs": "A1", "onConflict": rule, "approve": true})); + let e = match (rule, r) { + ("fail", Err(EngineError::Other(e))) | ("ask", Err(EngineError::Confirm(e))) => e, + (_, r) => panic!("{first} {rule}: {r:?}"), + }; + assert!(e.contains("Step 2 (cell.set): It would write over data (B1 on Sheet1)"), "{e}"); + // The workbook is put back; a file the run wrote stays, and the message says so. + let left = + if first == &export { "The workbook was put back; the file step 1 (file.exportCsv) wrote stays." } else { "Nothing was changed." }; + assert!(e.ends_with(left), "{e}"); + assert_eq!(v(&s, "B1").display(), "keep", "{first} {rule}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "{first} {rule}: rolled back"); + } + // The answer to the question overwrites. + s.execute("macro.run", json!({"name": "Late", "inputs": "A1", "overwrite": true})).unwrap(); + assert_eq!(v(&s, "B1").display(), "over"); + } + // A step that writes over what the macro itself wrote isn't a conflict. + let mut s = super::s(); + let again = |input: &str| json!({"command": "cell.set", "params": {"cell": "E1", "input": input}}); + put( + &mut s, + json!({"name": "Twice", "permissions": ["file.exportCsv"], "output": {"onConflict": "fail"}, "steps": [again("a"), export, again("b")]}), + ); + s.execute("macro.run", json!({"name": "Twice", "approve": true})).unwrap(); + assert_eq!(v(&s, "E1").display(), "b"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_run_that_takes_another_path_than_its_check_is_still_checked() { + // RAND gives the run other values than the dry run before it, so an If can go the other way: + // the write the footprint didn't see must still not go over data. + let mut s = s(); + let steps = json!([ + {"command": "cell.set", "params": {"cell": "A1", "input": "=RAND()"}}, + {"type": "if", "cell": "A1", "op": "gt", "value": 0.5, "then": [{"command": "cell.set", "params": {"cell": "B1", "input": "over"}}]} + ]); + put(&mut s, json!({"name": "Coin", "output": {"onConflict": "fail"}, "steps": steps})); + let mut outcomes = BTreeSet::new(); + for _ in 0..40 { + set(&mut s, "B1", "keep"); + let r = s.execute("macro.run", json!({"name": "Coin"})); + assert_eq!(v(&s, "B1").display(), "keep", "{r:?}"); + outcomes.insert(match r { + Ok(_) => "ran", + Err(e) if e.to_string().contains("would write over") => "refused", + Err(e) => panic!("{e}"), + }); + } + assert_eq!(outcomes.len(), 2, "both paths were taken: {outcomes:?}"); +} + +#[test] +fn run_parameters_that_would_be_ignored_are_refused() { + let mut s = s(); + data(&mut s); + put(&mut s, totals()); + for (p, says) in [ + (json!({"inputs": {"Inptu": "B2:C5"}}), "it has no input called 'Inptu' (its inputs: Input)."), + (json!({"inputs": {"Input": 5}}), "give the input Input as reference text"), + (json!({"inputs": ["B2:C5"]}), "give `inputs` as reference text"), + (json!({"inputs": "B2:C5", "onConflict": 1}), "`onConflict` must be"), + (json!({"inputs": "B2:C5", "overwrite": "yes"}), "`overwrite` must be true or false."), + (json!({"inputs": "B2:C5", "approve": 1}), "`approve` must be true or false."), + (json!({"inputs": "B2:C5", "timeLimit": "30"}), "`timeLimit` must be a number of seconds above 0."), + (json!({"inputs": "B2:C5", "timeLimit": 0}), "`timeLimit` must be a number of seconds above 0."), + ] { + let mut p = p; + p["name"] = json!("Totals"); + let err = s.execute("macro.run", p.clone()).unwrap_err().to_string(); + assert!(err.contains(&format!("Macro 'Totals': {says}")), "{p}: {err}"); + } + put(&mut s, json!({"name": "None", "steps": []})); + let err = s.execute("macro.run", json!({"name": "None", "inputs": "A1"})).unwrap_err().to_string(); + assert!(err.contains("Macro 'None': it takes no inputs."), "{err}"); + // `null` is the same as not given, and an input's name matches in any case. + let p = json!({"name": "Totals", "inputs": {"input": "B2:C5"}, "onConflict": null, "overwrite": true, "timeLimit": null}); + s.execute("macro.run", p).unwrap(); +} + +#[test] +fn a_run_passes_a_bounded_amount_of_parameters() { + // A step with a 3 MB parameter in a Repeat of 100 rows would hand its command 300 MB; the run + // stops at 256 MB, which matters on the web build, where there is no time limit. + let mut s = s(); + let big = "x".repeat(3_000_000); + let body = json!([{"command": "home.bold", "params": {"range": "A1", "note": big}}]); + put(&mut s, json!({"name": "Big", "output": {"onConflict": "overwrite"}, "steps": [{"type": "repeat", "over": "A1:A100", "body": body}]})); + let err = s.execute("macro.run", json!({"name": "Big"})).unwrap_err().to_string(); + assert!(err.contains("passed more than 256 MB of parameters to its steps"), "{err}"); + // 50 rows are 150 MB: the run alone stays under the limit, the dry run and the run together + // go past it, as they share the step budget. + let body = json!([{"command": "home.bold", "params": {"range": "A1", "note": "x".repeat(3_000_000)}}]); + for (rule, ok) in [("overwrite", true), ("fail", false)] { + let repeat = json!({"type": "repeat", "over": "A1:A50", "body": body}); + put(&mut s, json!({"name": "Half", "output": {"onConflict": rule}, "steps": [repeat]})); + assert_eq!(s.execute("macro.run", json!({"name": "Half"})).is_ok(), ok, "{rule}"); + } +} + +#[test] +fn the_check_says_whether_the_run_would_go_ahead() { + let mut s = s(); + // A Stop step that rolls the run back. + put(&mut s, json!({"name": "Halt", "output": {"onConflict": "fail"}, "steps": [{"type": "stop", "message": "Not today.", "rollback": true}]})); + let c = s.execute("macro.check", json!({"name": "Halt"})).unwrap(); + assert_eq!((&c["ok"], &c["errors"]), (&json!(false), &json!(["A Stop step ends the run and rolls it back: Not today."]))); + // A shift that would move an input. + set(&mut s, "B1", "taken"); + set(&mut s, "B3", "in"); + let step = json!({"command": "cell.set", "params": {"cell": "B1", "input": "new"}}); + put(&mut s, json!({"name": "Move", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "shiftDown"}, "steps": [step]})); + let c = s.execute("macro.check", json!({"name": "Move", "inputs": "B3"})).unwrap(); + assert_eq!(c["ok"], false); + assert!(c["errors"][0].as_str().unwrap().starts_with("Shifting the data would move Input (Sheet1!B3)."), "{c}"); + // A macro whose dry run takes 90,000 steps passes its own dry run, but the run spends the + // budget twice under fail, so the check says it wouldn't finish. + let spin = json!([{"type": "repeat", "over": "A1:A300", "body": [{"type": "repeat", "over": "A1:A300", "body": [{"type": "if", "cell": "A1", "op": "empty"}]}]}]); + for (rule, ok) in [("fail", false), ("overwrite", true)] { + put(&mut s, json!({"name": "Spin", "output": {"onConflict": rule}, "steps": spin})); + let c = s.execute("macro.check", json!({"name": "Spin"})).unwrap(); + assert_eq!(c["ok"], ok, "{rule}: {c}"); + assert_eq!(s.execute("macro.run", json!({"name": "Spin"})).is_ok(), ok, "{rule}: the run agrees"); + } + let c = s.execute("macro.check", json!({"name": "Spin", "onConflict": "fail"})).unwrap(); + assert!(c["errors"][0].as_str().unwrap().starts_with("The run would execute more than 100000 steps, counting the check before it"), "{c}"); + // Parameters the run would refuse, the check reports. + let c = s.execute("macro.check", json!({"name": "Spin", "onConflict": 5})).unwrap(); + assert!(c["errors"][0].as_str().unwrap().contains("`onConflict` must be"), "{c}"); +} + +#[test] +fn the_question_after_a_partial_footprint_says_what_the_answer_covers() { + let dir = std::env::temp_dir().join(format!("gc-macro-unseen-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let csv = dir.join("out.csv").to_string_lossy().to_string(); + let mut s = s(); + set(&mut s, "B1", "keep"); + set(&mut s, "C1", "also"); + let steps = json!([ + {"command": "cell.set", "params": {"cell": "B1", "input": "over"}}, + {"command": "file.exportCsv", "params": {"path": csv}}, + {"command": "cell.set", "params": {"cell": "C1", "input": "over"}} + ]); + put(&mut s, json!({"name": "Two", "permissions": ["file.exportCsv"], "steps": steps})); + let r = s.execute("macro.run", json!({"name": "Two", "approve": true})); + let Err(EngineError::Confirm(q)) = r else { panic!("{r:?}") }; + assert!(q.contains("(B1 on Sheet1). The check stopped at step 2 (file.exportCsv writes a file), so overwriting also lets the steps after it write over data unchecked."), "{q}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_dry_run_puts_back_the_go_to_list() { + let mut s = s(); + let step = json!({"command": "edit.goTo", "params": {"reference": "D4", "record": true}}); + put(&mut s, json!({"name": "Go", "steps": [step]})); + s.execute("macro.check", json!({"name": "Go"})).unwrap(); + assert_eq!(s.doc().unwrap().go_to_recent, Vec::::new()); +} + #[test] fn the_answer_to_the_question_counts_however_ask_was_given() { let mut s = s(); diff --git a/docs/macros.md b/docs/macros.md index 60d5c7d1e..a126c80d2 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -1,6 +1,6 @@ # Macros -> **Last updated:** 2026-10-24 · **Change:** minor (debugging macros: `macro.debug`) +> **Last updated:** 2026-10-25 · **Change:** minor (debugging macros: `macro.debug`) GridCraft macros are typed, declarative lists of commands, not VBA. Every step runs a GridCraft command (the same ids the ribbon, CLI and MCP use) or one of three bounded control steps. A macro @@ -59,9 +59,10 @@ The engine has no macro recorder or management commands in this version, and no whole macro (4,194,304 characters of JSON). A macro read from a file that breaks any of these is left out with a warning; it is never changed to fit. When a macro runs, a Repeat whose range has more rows or columns than its `max` is refused, and a run executes at most 100,000 steps (each - pass through a Repeat counts) and takes at most 60 seconds, both shared by the footprint dry - runs and the run itself (`timeLimit` to change the time; the web build has no clock, so there - only the step limit applies). + pass through a Repeat counts), passes its commands at most 256 MB of parameters, all steps + together, and takes at most 60 seconds, all three shared by the footprint dry runs and the run + itself (`timeLimit` to change the time; the web build has no clock, so there only the step and + parameter limits apply). ## Tokens @@ -107,24 +108,40 @@ refused rather than read as the header. including a blank one and a token, and the policy is checked again on the parameters as they are when the step runs. A macro that touches files, or that came with a workbook opened from a file, runs once the person approves it: the call fails with `approve: …` and runs with - `approve: true`. Approval lasts for the session, until the macro changes. + `approve: true`. Approval lasts for the session, until the macro changes. Every step other + than an approved file step runs inside the file sandbox that scripts use, so no other + parameter reaches a file either. 2. **Footprint.** Unless the conflict rule is `overwrite`, the macro first runs in dry-run mode on - the real workbook, and everything is put back. Cells it would write outside its - inputs that hold data, and sheets and objects it would - delete, are conflicts, handled by `onConflict` (or the macro's `output.onConflict`): `ask` (the - call fails with `confirm: …`; `overwrite: true` answers it), `fail`, `overwrite`, `shiftDown` or - `shiftRight` (Insert Cells makes room, then the footprint is checked again). Steps that touch - files, and Solver, end the dry run early; the footprint then says it is partial, and unless - the rule is `overwrite`, the run checks every step as it goes. A step that writes over cells, - sheets or objects that held data when the run started stops the run and rolls the workbook - back (a file already written stays): it asks (`confirm: …`) under `ask` and fails otherwise, - since shifting can't make room that late. + the real workbook, and everything is put back. Cells it would write outside its inputs that + hold data, and sheets and objects it would delete, are conflicts, handled by `onConflict` (or + the macro's `output.onConflict`): `ask` (the call fails with `confirm: …`; `overwrite: true` + answers it), `fail`, `overwrite`, `shiftDown` or `shiftRight` (Insert Cells makes room, then + the footprint is checked again). Steps that touch files, and Solver, end the dry run early; + the footprint then says it is partial. Unless the rule is `overwrite`, the run also checks + every step as it goes, since the footprint can miss a write: it is partial, or a value such as + RAND or NOW sends the run another way than the dry run. A step that writes over cells, sheets + or objects that held data when the run started stops the run and rolls the workbook back (a + file already written stays): it asks (`confirm: …`) under `ask` and fails otherwise, since + shifting can't make room that late. Moving cells that hold data, as inserting or deleting rows + or cells above them does, counts as writing over them, in the footprint and in this check: + both compare cells by place. 3. **Run.** One undo step, "Run Macro: Totals". Any error rolls the whole run back and names the step: `Step 3.1 (cell.set): …` (paths are 1-based; `2.1` is inside a Repeat, `4.then.1` inside - an If). + an If). The message ends "Nothing was changed.", or names the files the run already wrote, + which stay. An input the macro doesn't have, an input that isn't reference text, and + `onConflict`, `overwrite`, `approve` or `timeLimit` of the wrong kind are refused rather than + ignored. When a step inserts, deletes or moves sheets, rows, columns or cells, the inputs, the + output and the range of a Repeat in progress move with their cells, as references in formulas + do; a step that deletes one of them, or changes the size of a range a Repeat is going over, + stops the run. -`macro.check {name, inputs?, timeLimit?}` reports the bound inputs, the output anchor, any -approval needed and the footprint, without changing anything. +`macro.check {name, inputs?, onConflict?, overwrite?, timeLimit?}` reports the bound inputs, the +output anchor, any approval needed and the footprint, and puts back everything the dry run +changed. `ok` says whether `macro.run` with the same parameters would go ahead without a question; +when it wouldn't, `errors` says why: the run would fail at a step, a Stop step rolls it back, it +would write over data under `ask` or `fail`, shifting can't make room, or the run would go past +the step or parameter limit counting the dry runs before it. The random numbers RAND gives move +on, as after any recalculation. ## Debugging