diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index d81902b1b..53856e720 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -6,15 +6,26 @@ use serde_json::Value as Json; use super::*; pub fn specs() -> Vec { - vec![cmd!( - "macro.run", - "Run Macro", - [], - None, - "{name, inputs?: {Input: \"B2:D9\"} | \"B2:D9\", onConflict?: ask|fail|overwrite|shiftDown|shiftRight, overwrite?: bool, approve?: bool, timeLimit?: seconds} runs as one undo step; any error rolls it all back → {macro, ran, inputs, stopped?, shifted?}", - has_doc, - run - )] + vec![ + cmd!( + "macro.run", + "Run Macro", + [], + None, + "{name, inputs?: {Input: \"B2:D9\"} | \"B2:D9\", onConflict?: ask|fail|overwrite|shiftDown|shiftRight, overwrite?: bool, approve?: bool, timeLimit?: seconds} runs as one undo step; any error rolls it all back → {macro, ran, inputs, stopped?, shifted?}", + has_doc, + run + ), + cmd!( + query "macro.check", + "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}", + has_doc, + check + ), + ] } fn name_param<'a>(cmd: &str, p: &'a Json) -> Result<&'a str> { @@ -25,3 +36,8 @@ fn run(s: &mut Session, p: &Json) -> Result { let name = name_param("macro.run", p)?; crate::macros::run::run(s, name, p) } + +fn check(s: &mut Session, p: &Json) -> Result { + let name = name_param("macro.check", p)?; + crate::macros::run::check(s, name, p) +} diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 6f58f18ee..90fe4ccc8 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -366,6 +366,13 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) if !structural { check_arrays(&d.wb, &wb)?; } + if let Some(log) = d.touched.as_mut() { + for (si, c) in changed.iter().take(TOUCH_LOG_LIMIT.saturating_sub(log.len())) { + if let Some(sh) = wb.sheet(*si) { + log.push((sh.name.clone(), *c)); + } + } + } if changed.len() <= 200_000 { fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row))); } @@ -393,6 +400,11 @@ 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; + /// Excel's message for an edit of a locked cell on a protected sheet. pub(crate) const PROTECTED: &str = "The cell or chart you're trying to change is on a protected sheet."; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 151c2bc06..9d4d2f93b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -206,6 +206,9 @@ pub struct DocState { /// workbook is opened from a file, carried by content pasted from such a workbook, and never /// changed by a trust decision. pub trust: Trust, + /// 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>, /// 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, @@ -299,6 +302,7 @@ impl DocState { content_kind: None, scripts_trusted, trust, + touched: None, go_to_recent: vec![], } } @@ -434,6 +438,9 @@ pub struct Session { /// Serialized size of each journal entry, measured once when it is pushed, and their sum. journal_sizes: Vec, journal_bytes: usize, + /// Entries the journal has dropped from its front to stay within its limits, ever; a dry run + /// that puts the journal back needs to know how many went while it ran. + journal_dropped: u64, pub ui_requests: Vec, /// Format painter source style (sticky when double-clicked). pub format_painter: Option<(Arc, usize, RangeRef, bool)>, @@ -558,10 +565,26 @@ impl Session { self.journal.drain(..n); self.journal_sizes.drain(..n); self.journal_bytes = kept; + self.journal_dropped = self.journal_dropped.saturating_add(n as u64); self.recording = self.recording.map(|r| r.saturating_sub(n)); } } + /// Entries dropped from the journal's front so far, for a caller that puts the journal back. + pub(crate) fn journal_dropped(&self) -> u64 { + self.journal_dropped + } + + /// Cuts the journal back to its first `keep` entries, with its size bookkeeping. + pub(crate) fn truncate_journal(&mut self, keep: usize) { + if self.journal_sizes.len() == self.journal.len() { + self.journal_sizes.truncate(keep); + self.journal_bytes = self.journal_sizes.iter().sum(); + } + self.journal.truncate(keep); + self.recording = self.recording.map(|r| r.min(self.journal.len())); + } + pub fn commands(&self) -> Vec { command_specs().iter().map(|c| c.info(self)).collect() } diff --git a/crates/engine/src/macros/policy.rs b/crates/engine/src/macros/policy.rs index 57d974920..8f23a5b40 100644 --- a/crates/engine/src/macros/policy.rs +++ b/crates/engine/src/macros/policy.rs @@ -73,6 +73,12 @@ pub fn macro_policy(id: &str, params: &Json, trust: Trust) -> Policy { } } +/// Whether the footprint dry run can preview a command. Solver can take long, so a dry run stops +/// before it and reports the footprint as partial. +pub fn previewable(id: &str) -> bool { + id != "data.solver" +} + #[cfg(test)] mod tests { use super::*; @@ -123,6 +129,7 @@ mod tests { assert_eq!(macro_policy(id, &none, Trust::Local), Policy::Allow, "{id}"); assert!(matches!(macro_policy(id, &none, Trust::FromFile), Policy::Ask(_)), "{id}"); } + assert!(!previewable("data.solver") && previewable("cell.set")); } #[test] diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 8ae1e3496..f45bcfc25 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -6,12 +6,12 @@ //! approves it (once per macro content, for the session). //! 2. **Inputs**: each declared input is bound to a range (given, its default, or the selection) //! and checked against its shape and column types, with messages that name the cells. -//! 3. **Conflicts**: unless the macro may overwrite, each step is checked as it runs. A step that -//! writes over data outside the inputs (cells that held data when the run started, sheets and -//! objects it deletes) stops the run, which asks (`ask`) or fails (the other rules), and the -//! run rolls back. Shifting the data to make room (`shiftDown`, `shiftRight`) needs to know -//! where the macro writes before it runs, which this version doesn't work out, so those rules -//! stop the run as `fail` does. +//! 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. //! 4. **Run**: one undo step (`Session::grouped`); an error at any step rolls everything back and //! names the step. //! @@ -26,9 +26,9 @@ use gridcraft_model::{Cell, CompareOp, InputKind, Macro, OnConflict, RepeatOf, S use serde::Serialize; use serde_json::{Value as Json, json}; -use super::policy::{Policy, macro_policy}; +use super::policy::{Policy, macro_policy, previewable}; use super::resolve::{self, Env, Place, ResolveError}; -use crate::{EngineError, Result, Session, Trust, UiRequest}; +use crate::{EngineError, Result, Selection, Session, Trust, UiRequest}; /// The most steps one run executes (counting each pass through a Repeat). pub const MAX_EXECUTED_STEPS: u64 = 100_000; @@ -37,7 +37,7 @@ 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 ranges a list of cells gives per sheet (the counts stay exact). +/// The most ranges a footprint lists per sheet (the counts stay exact). const MAX_LISTED_RANGES: usize = 500; /// An input bound to a range: name, where, whether its first row is a header. @@ -421,28 +421,37 @@ pub struct Machine { pub executed: u64, clock: Stopwatch, pub time_limit_ms: f64, + /// Dry run (the footprint check): steps that would touch files or can't be previewed stop + /// the run early instead of running. + pub dry_run: bool, /// A Stop step ended the run: its message, and whether it asked for a rollback. pub stopped: Option<(String, bool)>, + /// A dry run stopped before this step (path, reason): what follows isn't in the footprint. + pub partial: Option<(String, String)>, /// The last step's resolved parameters (for the debugger). pub last_params: Option, - /// Set unless the macro may overwrite: each step is checked for writes over data as it runs. + /// Set when the footprint was partial: each step is checked for writes over data as it runs. guard: Option, } -/// Checks each step of a run for writes over data the workbook held when the run started. +/// Checks each step of a real run for writes over data the footprint couldn'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, /// The conflict rule is ask. ask: bool, } impl Guard { - fn new(start: Arc, ask: bool) -> Guard { - Guard { origin: (0..start.sheets.len()).map(Some).collect(), start, ask } + 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")); + Guard { origin: (0..start.sheets.len()).map(Some).collect(), start, partial, ask } } /// What one step wrote over that held data when the run started: cells outside the inputs, @@ -503,7 +512,7 @@ fn deleted(start: &[T], prev: &[T], after: &[T], id: impl Fn(&T) -> u32) -> u } impl Machine { - pub fn new(p: &Prepared) -> Machine { + pub fn new(p: &Prepared, dry_run: bool) -> Machine { let mut m = Machine { name: p.mac.name.clone(), doc: p.doc, @@ -515,7 +524,9 @@ impl Machine { executed: 0, clock: Stopwatch::start(), time_limit_ms: DEFAULT_TIME_LIMIT_MS, + dry_run, stopped: None, + partial: None, last_params: None, guard: None, }; @@ -666,9 +677,19 @@ impl Machine { if !matches!(written, Policy::NeedsPermission(_)) { return Err(fail(format!("{id} {what} only once its parameters are filled in, so it wasn't approved."))); } + if self.dry_run { + self.partial = Some((path.to_string(), format!("{id} {what}"))); + self.frames.clear(); + return Ok(()); + } } (_, Policy::Allow | Policy::Ask(_)) => {} } + if self.dry_run && !previewable(id) { + self.partial = Some((path.to_string(), format!("{id} can't be previewed"))); + self.frames.clear(); + return Ok(()); + } 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 queued = s.ui_requests.len(); @@ -693,7 +714,7 @@ impl Machine { 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})."); + let msg = format!("It would write over data ({what}); the check before the run stopped at {} and couldn't see this.", g.partial); return Err(StepError { confirm: g.ask, ..fail(msg) }); } } @@ -743,7 +764,255 @@ fn compare(v: &Value, op: CompareOp, want: &Json) -> bool { } } -// ---------------------------------------------------------------- comparing workbooks +// ---------------------------------------------------------------- footprint + +/// What a macro would change, from a dry run on the real workbook. +#[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). + #[serde(skip)] + pub executed: u64, + /// 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")] + pub log_full: bool, + pub sheets: Vec, + pub sheets_added: Vec, + pub sheets_removed: Vec, + pub objects_added: usize, + pub objects_changed: usize, + pub objects_removed: usize, + /// The dry run stopped early before this step; later steps aren't in the footprint. + #[serde(skip_serializing_if = "Option::is_none")] + pub partial: Option, + /// The step the run would fail at. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, + /// A Stop step's message. + #[serde(skip_serializing_if = "Option::is_none")] + pub stopped: Option, +} + +/// The cells a macro would write on one sheet, outside its inputs. +#[derive(Clone, Debug, Default, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SheetFootprint { + pub sheet: String, + #[serde(skip)] + pub index: usize, + /// The written cells as ranges (at most 500 listed), and how many cells. + pub cells: Vec, + pub changed: usize, + /// Written cells that hold data now. + pub conflicts: Vec, + pub conflict_count: usize, + /// The block that holds every written cell. + #[serde(skip_serializing_if = "Option::is_none")] + pub bounds: Option, + #[serde(skip)] + pub bounds_range: Option, +} + +impl Footprint { + /// Cells that hold data and would be written, plus sheets and objects that would go. + pub fn conflict_count(&self) -> usize { + self.sheets.iter().map(|s| s.conflict_count).sum::() + self.sheets_removed.len() + self.objects_removed + } + + /// "C2:C9 on Sheet1, the sheet Old, 2 objects". + pub fn describe_conflicts(&self) -> String { + let mut parts: Vec = self + .sheets + .iter() + .filter(|s| s.conflict_count > 0) + .map(|s| { + let mut listed = s.conflicts.iter().take(3).cloned().collect::>().join(", "); + if s.conflicts.len() > 3 { + listed.push_str(", …"); + } + format!("{listed} on {}", s.sheet) + }) + .collect(); + parts.extend(self.sheets_removed.iter().map(|n| format!("the sheet {n}"))); + if self.objects_removed > 0 { + parts.push(format!("{} object{}", self.objects_removed, if self.objects_removed == 1 { "" } else { "s" })); + } + parts.join("; ") + } +} + +/// Session state a dry run may touch, put back afterwards. The workbook, its selection and its +/// history are put back by the dry run's undo group. +struct Saved { + active: usize, + docs: Vec, + /// The workbook as last saved: `sheet.activate` and the view commands move it to the + /// current workbook on a clean document, and the rollback puts the workbook back but not this. + saved: Option>, + sheet_selections: Vec, + unlocked_ranges: Vec<(String, String)>, + trust: Trust, + clipboard: Option, + prefs: crate::Prefs, + mode: crate::Mode, + /// The journal's length and how many entries it had dropped from its front; a dry run that + /// pushes past the journal's limits drops more, which a length alone cannot see. + journal: usize, + journal_dropped: u64, + recording: Option, + ui_requests: usize, + format_painter: Option<(Arc, usize, RangeRef, bool)>, + last_find: Option, + hide_ruler: bool, + hidden_windows: Vec, + draw: (String, String, f32), + comments_pane: bool, + show_all_notes: bool, +} + +impl Saved { + fn take(s: &mut Session, doc: u64) -> Saved { + let d = s.docs.iter().find(|d| d.uid == doc); + Saved { + active: s.active, + docs: s.docs.iter().map(|d| d.uid).collect(), + saved: d.map(|d| d.saved.clone()), + 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), + clipboard: s.clipboard.clone(), + prefs: s.prefs.clone(), + mode: s.mode, + journal: s.journal.len(), + journal_dropped: s.journal_dropped(), + recording: s.recording, + ui_requests: s.ui_requests.len(), + format_painter: s.format_painter.clone(), + last_find: s.last_find.clone(), + hide_ruler: s.hide_ruler, + hidden_windows: s.hidden_windows.clone(), + draw: (s.draw_tool.clone(), s.draw_color.clone(), s.draw_width), + comments_pane: s.comments_pane, + show_all_notes: s.show_all_notes, + } + } + + fn restore(self, s: &mut Session, doc: u64) { + s.docs.retain(|d| self.docs.contains(&d.uid)); + if let Some(d) = s.docs.iter_mut().find(|d| d.uid == doc) { + if let Some(saved) = self.saved { + d.saved = saved; + } + d.sheet_selections = self.sheet_selections; + d.unlocked_ranges = self.unlocked_ranges; + d.trust = self.trust; + } + s.active = self.active.min(s.docs.len().saturating_sub(1)); + s.clipboard = self.clipboard; + s.prefs = self.prefs; + s.mode = self.mode; + // Entries the dry run pushed go; entries the journal dropped meanwhile are gone for good, + // and the recording's start moves back with them. + let dropped = usize::try_from(s.journal_dropped().saturating_sub(self.journal_dropped)).unwrap_or(usize::MAX); + s.truncate_journal(self.journal.saturating_sub(dropped)); + s.recording = self.recording.map(|r| r.saturating_sub(dropped)); + s.ui_requests.truncate(self.ui_requests); + s.format_painter = self.format_painter; + s.last_find = self.last_find; + s.hide_ruler = self.hide_ruler; + s.hidden_windows = self.hidden_windows; + (s.draw_tool, s.draw_color, s.draw_width) = self.draw; + s.comments_pane = self.comments_pane; + s.show_all_notes = self.show_all_notes; + } +} + +/// 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 { + 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); + let before = s.doc()?.wb.clone(); + let token = s.group_begin("Check Macro")?; + if let Some(d) = s.docs.iter_mut().find(|d| d.uid == p.doc) { + 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); + 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()), + None => (before.clone(), Vec::new()), + }; + let rolled = s.group_rollback(token); + saved.restore(s, p.doc); + rolled?; + 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.error = outcome.err(); + fp.stopped = m.stopped.map(|(msg, _)| msg); + fp.partial = m.partial.map(|(step, reason)| json!({"step": step, "reason": reason})); + Ok(fp) +} + +/// Compares the workbook before and after a dry run. `touched` adds the cells commands wrote +/// 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(); + 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 { + let Some(bs) = before.sheets.get(bi) else { continue }; + let Some(ai) = ai else { + fp.sheets_removed.push(bs.name.clone()); + 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(); + if Arc::ptr_eq(bs, after_sheet) && rewritten.is_empty() { + continue; + } + let (added, changed, removed) = objects(bs, after_sheet); + fp.objects_added += added; + fp.objects_changed += changed; + fp.objects_removed += removed; + let in_input = |c: CellRef| inputs.iter().any(|(_, p, _)| p.sheet == bi && p.range.contains(c)); + let mut written = Vec::new(); + let mut conflicts = Vec::new(); + let mut cells: BTreeSet = bs.cells.diff(&after_sheet.cells).into_iter().collect(); + cells.extend(rewritten.iter().copied()); + for c in cells { + if in_input(c) { + continue; + } + let (b, a) = (bs.cell(c), after_sheet.cell(c)); + if recalculated_only(b, a) && !rewritten.contains(&c) { + continue; + } + written.push(c); + if b.is_some_and(has_content) && content_differs(b, a) { + conflicts.push(c); + } + } + let Some(first) = written.first() else { continue }; + let bounds = written.iter().fold(RangeRef::cell(*first), |r, c| r.union(&RangeRef::cell(*c))); + fp.sheets.push(SheetFootprint { + sheet: bs.name.clone(), + index: bi, + changed: written.len(), + conflict_count: conflicts.len(), + cells: ranges(&mut written), + conflicts: ranges(&mut conflicts), + bounds: Some(bounds.a1()), + bounds_range: Some(bounds), + }); + } + fp +} /// Each sheet of `before` with its index in `after` (`None` when deleted), and the sheets of /// `after` that are new. Sheets are matched by name; what's left is matched in order (a renamed @@ -785,6 +1054,18 @@ fn content_differs(b: Option<&Cell>, a: Option<&Cell>) -> bool { } } +/// Objects added, changed and removed between two versions of a sheet. +fn objects(b: &Sheet, a: &Sheet) -> (usize, usize, usize) { + fn count(b: &[T], a: &[T], id: impl Fn(&T) -> u32) -> (usize, usize, usize) { + let added = a.iter().filter(|x| !b.iter().any(|y| id(y) == id(x))).count(); + let removed = b.iter().filter(|x| !a.iter().any(|y| id(y) == id(x))).count(); + let changed = b.iter().filter(|x| a.iter().any(|y| id(y) == id(x) && y != *x)).count(); + (added, changed, removed) + } + let parts = [count(&b.charts, &a.charts, |o| o.id), count(&b.images, &a.images, |o| o.id), count(&b.shapes, &a.shapes, |o| o.id)]; + parts.iter().fold((0, 0, 0), |t, p| (t.0 + p.0, t.1 + p.1, t.2 + p.2)) +} + /// Cells as A1 ranges: runs along each row, then runs with the same columns on following rows. pub(crate) fn ranges(cells: &mut [CellRef]) -> Vec { cells.sort_unstable_by_key(|c| (c.row, c.col)); @@ -811,6 +1092,47 @@ pub(crate) fn ranges(cells: &mut [CellRef]) -> Vec { merged.iter().take(MAX_LISTED_RANGES).map(RangeRef::a1).collect() } +/// Inserts blank cells where the footprint lands (moving what is there down or right), so the +/// macro writes into empty cells. The block is every written cell's, so a row the macro writes +/// 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<()> { + 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())); + } + if fp.log_full { + return Err(EngineError::Other("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(); + 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 + // its left column rightwards. + let moved = |r: &RangeRef| { + if down { + r.start.col <= block.end.col && r.end.col >= block.start.col && r.end.row >= block.start.row + } else { + r.start.row <= block.end.row && r.end.row >= block.start.row && r.end.col >= block.start.col + } + }; + // 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!( + "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(()) +} + // ---------------------------------------------------------------- run /// A macro that passed its checks and is about to run. @@ -840,19 +1162,62 @@ pub fn begin(s: &mut Session, name: &str, p: &Json) -> Result { Ok(Begun { mac: m, prepared, rule, time_limit_ms, inputs }) } -/// The output rule, inside the run's undo group: the machine for the run. Unless the macro may -/// overwrite, the machine checks each step as it runs and stops the run at one that writes over -/// data the workbook held when the run started (asking under `ask`, failing otherwise); the run's -/// undo group then rolls back. Shifting needs to know where the macro writes before it runs, so -/// `shiftDown` and `shiftRight` stop the run as `fail` does. -pub fn make_way(s: &mut Session, b: &Begun) -> Result { - let mut machine = Machine::new(&b.prepared); +/// The footprint check and the output rule, inside the run's undo group: an error when the macro +/// would fail or write over data it may not, after shifting the data if the rule says so. Returns +/// 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. +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); machine.time_limit_ms = b.time_limit_ms; - if b.rule != OnConflict::Overwrite { - let start = s.docs.iter().find(|d| d.uid == b.prepared.doc).ok_or(EngineError::NoDocument)?.wb.clone(); - machine.guard = Some(Guard::new(start, b.rule == OnConflict::Ask)); + if rule == OnConflict::Overwrite { + return Ok((machine, false)); } - Ok(machine) + // 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; + if let Some(e) = fp.error.take() { + return Err(e.into_engine(&m.name)); + } + let mut shifted = false; + if fp.conflict_count() > 0 { + let what = fp.describe_conflicts(); + match rule { + OnConflict::Ask => { + 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", + m.name + ))); + } + OnConflict::Fail | OnConflict::Overwrite => { + return Err(EngineError::Other(format!("Macro '{}' would write over cells that hold data ({what}). Nothing was changed.", m.name))); + } + 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; + if let Some(e) = fp.error.take() { + return Err(e.into_engine(&m.name)); + } + if fp.conflict_count() > 0 { + return Err(EngineError::Other(format!( + "Macro '{}': shifting the data didn't make room ({}). Nothing was changed.", + m.name, + fp.describe_conflicts() + ))); + } + shifted = true; + } + } + } + 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)); + } + Ok((machine, shifted)) } /// Runs macro `name` on the active workbook: `{inputs?, onConflict?, overwrite?, approve?, @@ -861,16 +1226,21 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { let b = begin(s, name, p)?; let label = format!("Run Macro: {}", b.mac.name); s.grouped(&label, |s| { - let mut machine = make_way(s, &b)?; + let (mut machine, shifted) = make_way(s, &b)?; let name = &b.mac.name; + // `ran` counts the run's own steps; the budget also holds the dry runs'. + 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."))), stopped => { - let mut out = json!({"macro": name, "ran": machine.executed, "inputs": b.inputs}); + let mut out = json!({"macro": name, "ran": machine.executed.saturating_sub(before_run), "inputs": b.inputs}); if let Some((msg, _)) = stopped { out["stopped"] = json!(msg); } + if shifted { + out["shifted"] = json!(b.rule.name()); + } Ok(out) } } @@ -895,3 +1265,49 @@ fn conflict_rule(p: &Json, m: &Macro) -> Result { } Ok(rule) } + +/// `macro.check`: what running the macro would do, without changing anything. +pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { + let d = s.doc()?; + let (doc, trust) = (d.uid, d.trust); + let m = super::find(&d.wb, name) + .and_then(|i| d.wb.macros.get(i)) + .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 asks = match gate(&m, trust) { + Ok(a) => a, + Err(e) => { + out["errors"] = json!([e.to_string()]); + return Ok(out); + } + }; + let approved = s.macro_approvals.contains(&(doc, super::fingerprint(&m))); + if !asks.is_empty() && !approved { + // Nothing from an unapproved macro runs, not even in a dry run. + out["needsApproval"] = json!(asks); + } + let prepared = match prepare(s, &m, p.get("inputs")) { + Ok(pr) => pr, + Err(e) => { + out["errors"] = json!([e.to_string()]); + return Ok(out); + } + }; + out["inputs"] = prepared.inputs_json(&s.doc()?.wb); + if let Some(o) = prepared.output { + let wb = &s.doc()?.wb; + out["output"] = json!(resolve::a1(o, &Env { wb, active_sheet: wb.active_sheet, inputs: &[], output: None, item: None })); + } + 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()); + if let Some(e) = &fp.error { + out["errors"] = json!([e.describe()]); + } + 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 262f50af9..2f5b71a61 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -1,5 +1,4 @@ -//! Macros: running, input checks, checking each step for writes over data, the policy and the -//! limits. +//! Macros: running, input checks, the footprint dry run, the policy and the limits. use std::sync::Arc; @@ -157,25 +156,208 @@ fn conflicts_ask_fail_or_overwrite() { let start = s.doc().unwrap().wb.clone(); let r = s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5"})); let Err(EngineError::Confirm(q)) = r else { panic!("{r:?}") }; - assert!(q.contains("Step 2 (cell.set): It would write over data (E3 on Sheet1)"), "{q}"); + assert!(q.contains("would write over cells that hold data (E3 on Sheet1)"), "{q}"); assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "programmatic runs default to failing"); let err = s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": "fail"})).unwrap_err().to_string(); assert!(err.contains("Nothing was changed"), "{err}"); - assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); - // Shifting needs to know where the macro writes before it runs: it stops as fail does. - for rule in ["shiftDown", "shiftRight"] { - let err = s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": rule})).unwrap_err().to_string(); - assert!(err.contains("would write over data (E3 on Sheet1)"), "{rule}: {err}"); - assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "{rule}"); - } assert!(s.execute("macro.run", json!({"name": "Totals", "onConflict": "sideways"})).is_err()); + // macro.check shows the footprint and the conflict, and changes nothing. + 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); + 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)); + // The answer to the question overwrites. s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "overwrite": true})).unwrap(); assert_eq!(v(&s, "E3").display(), "400"); assert_eq!(v(&s, "H1").display(), "400"); } +#[test] +fn shifting_makes_room_and_is_checked_again() { + for (rule, moved) in [("shiftDown", ["E4", "E5"]), ("shiftRight", ["F2", "F3"])] { + let mut s = s(); + data(&mut s); + set(&mut s, "E2", "old 1"); + set(&mut s, "E3", "old 2"); + put(&mut s, totals()); + let r = s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": rule})).unwrap(); + assert_eq!(r["shifted"], rule); + assert_eq!((v(&s, "E2").display(), v(&s, "E3").display()), ("Total".into(), "400".into()), "{rule}"); + assert_eq!((v(&s, moved[0]).display(), v(&s, moved[1]).display()), ("old 1".into(), "old 2".into()), "{rule}"); + // The shift and the run are one undo step. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(v(&s, "E2").display(), "old 1", "{rule}"); + assert!(v(&s, moved[1]).is_empty(), "{rule}"); + } + // A cell written with what it already holds isn't a conflict, but it moves with the others, + // so rows stay together. + let mut s = super::s(); + for (cell, input) in [("F2", "x"), ("G2", "b"), ("F3", "f3"), ("G3", "g3")] { + set(&mut s, cell, input); + } + put( + &mut s, + json!({"name": "Pair", "output": {"onConflict": "shiftDown"}, "steps": [ + {"command": "cell.set", "params": {"cell": "F2", "input": "a"}}, + {"command": "cell.set", "params": {"cell": "G2", "input": "b"}} + ]}), + ); + let c = s.execute("macro.check", json!({"name": "Pair"})).unwrap(); + assert_eq!((c["conflicts"].as_u64(), &c["footprint"]["sheets"][0]["cells"]), (Some(1), &json!(["F2:G2"]))); + s.execute("macro.run", json!({"name": "Pair"})).unwrap(); + let row = |s: &Session, r: u32| (v(s, &format!("F{r}")).display(), v(s, &format!("G{r}")).display()); + assert_eq!(row(&s, 2), ("a".into(), "b".into())); + assert_eq!(row(&s, 3), ("x".into(), "b".into())); + assert_eq!(row(&s, 4), ("f3".into(), "g3".into())); + // An anchor that moves with the shifted data still lands on it: the second check refuses. + let mut s = super::s(); + data(&mut s); + set(&mut s, "E2", "old"); + s.execute("formulas.defineName", json!({"name": "Target", "refersTo": "=Sheet1!$E$2"})).unwrap(); + put( + &mut s, + json!({"name": "Named", "steps": [{"command": "cell.set", "params": {"cell": "Name:Target", "input": "new"}}], "output": {"onConflict": "shiftDown"}}), + ); + let start = s.doc().unwrap().wb.clone(); + let err = s.execute("macro.run", json!({"name": "Named"})).unwrap_err().to_string(); + assert!(err.contains("shifting the data didn't make room (E3 on Sheet1)"), "{err}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); +} + +#[test] +fn a_shift_that_would_move_an_input_is_refused() { + // A shift inserts cells over the conflicting block, which moves the block's columns (or rows) + // from there on. When one of the macro's own inputs lies in the way, its data would move and + // shear while the run keeps the old coordinates, so the shift is refused and nothing changes. + let mut s = s(); + data(&mut s); + set(&mut s, "B1", "Old title"); + put( + &mut s, + json!({"name": "Title", "inputs": [{"name": "Input", "header": true}], "steps": [ + {"command": "cell.set", "params": {"cell": "Input.cell(0,1)", "input": "Report"}} + ]}), + ); + let start = s.doc().unwrap().wb.clone(); + let err = s.execute("macro.run", json!({"name": "Title", "inputs": "B2:C5", "onConflict": "shiftDown"})).unwrap_err().to_string(); + assert!(err.contains("would move Input (") && err.contains("B2:C5"), "{err}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "nothing changed"); + assert_eq!((v(&s, "B1").display(), v(&s, "B3").display()), ("Old title".into(), "Ann".into())); + // Writing left of the input and shifting right would move it too. + put( + &mut s, + json!({"name": "Left", "inputs": [{"name": "Input", "header": true}], "steps": [ + {"command": "cell.set", "params": {"cell": "Input.offset(1,-1).cell(1,1)", "input": "x"}} + ]}), + ); + set(&mut s, "A3", "in the way"); + let err = s.execute("macro.run", json!({"name": "Left", "inputs": "B2:C5", "onConflict": "shiftRight"})).unwrap_err().to_string(); + assert!(err.contains("would move Input"), "{err}"); + // The output counts as well. + put( + &mut s, + json!({"name": "Out", "inputs": [{"name": "Input", "header": true}], "output": {"anchor": "Input.below(1)"}, "steps": [ + {"command": "cell.set", "params": {"cell": "Input.cell(0,1)", "input": "Report"}} + ]}), + ); + let err = s.execute("macro.run", json!({"name": "Out", "inputs": "B2:C5", "onConflict": "shiftDown"})).unwrap_err().to_string(); + assert!(err.contains("would move Input"), "{err}"); + // A shift that moves nothing of the macro's own still works, over the block of every written + // cell, so a row the macro writes stays together. + set(&mut s, "H1", "elsewhere"); + put( + &mut s, + json!({"name": "Wide", "inputs": [{"name": "Input", "header": true}], "steps": [ + {"command": "cell.set", "params": {"cell": "H1", "input": "over"}}, + {"command": "cell.set", "params": {"cell": "H9", "input": "far"}} + ]}), + ); + s.execute("macro.run", json!({"name": "Wide", "inputs": "B2:C5", "onConflict": "shiftDown"})).unwrap(); + assert_eq!((v(&s, "H1").display(), v(&s, "H9").display(), v(&s, "H10").display()), ("over".into(), "far".into(), "elsewhere".into())); + assert_eq!(v(&s, "B3").display(), "Ann", "the input did not move"); +} + +#[test] +fn a_shift_is_refused_when_the_footprint_cannot_follow_every_write() { + // The touch log keeps at most a million writes; past that, a write that kept a cell's value + // is missing from the footprint, so a shift could split a row the macro writes. The footprint + // says so and the shift is refused (overwrite, with no dry run, still runs). + let mut s = s(); + set(&mut s, "A1", "x"); + set(&mut s, "E2", "old"); + put( + &mut s, + json!({"name": "Flood", "steps": [ + {"command": "edit.fillDown", "params": {"range": "A1:A1000001"}}, + {"command": "cell.set", "params": {"cell": "E2", "input": "new"}} + ]}), + ); + let c = s.execute("macro.check", json!({"name": "Flood"})).unwrap(); + assert_eq!(c["footprint"]["logFull"], true, "{c}"); + let err = s.execute("macro.run", json!({"name": "Flood", "onConflict": "shiftDown"})).unwrap_err().to_string(); + assert!(err.contains("more cells than the footprint can follow"), "{err}"); + assert_eq!(v(&s, "E2").display(), "old", "nothing changed"); +} + +#[test] +fn a_dry_run_leaves_a_clean_workbook_clean() { + // `view.zoom` and `sheet.activate` move the saved mark to the current workbook on a clean + // document; the dry run's rollback puts the workbook back, so the mark must come back too. + let mut s = s(); + data(&mut s); + let d = s.doc_mut().unwrap(); + d.saved = d.wb.clone(); + assert!(!s.doc().unwrap().is_dirty()); + put(&mut s, json!({"name": "Look", "steps": [{"command": "view.zoom", "params": {"percent": 120}}]})); + s.execute("macro.check", json!({"name": "Look"})).unwrap(); + assert!(!s.doc().unwrap().is_dirty(), "a dry run changed nothing"); + // A run refused by the conflict rule changed nothing either. + put(&mut s, totals()); + set(&mut s, "E3", "taken"); + let d = s.doc_mut().unwrap(); + d.saved = d.wb.clone(); + assert!(s.execute("macro.run", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": "fail"})).is_err()); + assert!(!s.doc().unwrap().is_dirty()); +} + +#[test] +fn a_dry_run_puts_the_journal_back_even_after_it_trimmed_it() { + // Every nested command a dry run executes is journaled, so a long macro pushes the journal + // past its limit and the journal drops its oldest entries. Putting the journal back by its + // old length would then keep entries from the dry run and point a recording at the wrong + // place; the restore counts what was dropped. + let mut s = s(); + data(&mut s); + s.execute("selection.set", json!({"range": "A1"})).unwrap(); + let journal = s.journal.clone(); + s.recording = Some(journal.len().saturating_sub(1)); + put( + &mut s, + json!({"name": "Long", "steps": [{"type": "repeat", "over": "A1:A200", "body": [ + {"type": "repeat", "over": "A1:A60", "body": [{"command": "cell.set", "params": {"cell": "Row.cell(1,1)", "input": "x"}}]} + ]}]}), + ); + s.execute("macro.check", json!({"name": "Long"})).unwrap(); + let sets = |j: &[(String, Json)]| j.iter().filter(|(id, _)| id == "cell.set").count(); + assert!(sets(&s.journal) <= sets(&journal), "no dry-run entry stays"); + assert!(s.journal.len() <= journal.len(), "{} entries, had {}", s.journal.len(), journal.len()); + assert!(journal.ends_with(&s.journal), "what stays is the journal's own tail"); + assert!(s.recording.is_some_and(|r| r <= s.journal.len()), "{:?}", s.recording); + // A short dry run leaves the journal exactly as it was. + put(&mut s, json!({"name": "Short", "steps": [{"command": "cell.set", "params": {"cell": "H1", "input": "x"}}]})); + let (journal, recording) = (s.journal.clone(), s.recording); + s.execute("macro.check", json!({"name": "Short"})).unwrap(); + 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())); @@ -232,6 +414,66 @@ fn the_answer_to_the_question_counts_however_ask_was_given() { assert_eq!(v(&s, "E3").display(), "400"); } +#[test] +fn the_step_limit_is_one_budget_across_the_dry_run_and_the_run() { + // 300 x 300 passes is under the limit for one phase; the dry run and the run together are not. + let mut s = s(); + put( + &mut s, + json!({"name": "Spin", "steps": [{"type": "repeat", "over": "A1:A300", "body": [ + {"type": "repeat", "over": "A1:A300", "body": [{"type": "if", "cell": "A1", "op": "empty"}]} + ]}]}), + ); + let err = s.execute("macro.run", json!({"name": "Spin"})).unwrap_err().to_string(); + assert!(err.contains("more than 100000 steps"), "{err}"); + // With no dry run (overwrite), the same macro fits. + s.execute("macro.run", json!({"name": "Spin", "onConflict": "overwrite"})).unwrap(); +} + +#[test] +fn the_footprint_covers_other_sheets_objects_and_edges() { + let mut s = s(); + data(&mut s); + s.execute("home.insertSheet", json!({"name": "Notes"})).unwrap(); + set(&mut s, "A1", "note"); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + s.execute("insert.chart", json!({"range": "B2:C5", "type": "column", "at": "H2"})).unwrap(); + let chart = s.doc().unwrap().wb.sheet(0).unwrap().charts[0].id; + put( + &mut s, + json!({"name": "Wide", "output": {"onConflict": "fail"}, "steps": [ + {"command": "cell.set", "params": {"cell": "Notes!A1", "input": "x"}}, + {"command": "cell.set", "params": {"cell": "Notes!B2", "input": "y"}}, + {"command": "home.insertSheet", "params": {"name": "Extra"}}, + {"command": "sheet.activate", "params": {"sheet": "Sheet1"}}, + {"command": "insert.shape", "params": {"kind": "ellipse", "at": "J2"}}, + {"command": "object.delete", "params": {"kind": "chart", "id": chart}} + ]}), + ); + let c = s.execute("macro.check", json!({"name": "Wide", "inputs": {}})).unwrap(); + let fp = &c["footprint"]; + assert_eq!( + fp["sheets"], + json!([{"sheet": "Notes", "cells": ["A1", "B2"], "changed": 2, "conflicts": ["A1"], "conflictCount": 1, "bounds": "A1:B2"}]) + ); + assert_eq!(fp["sheetsAdded"], json!(["Extra"])); + assert_eq!((fp["objectsAdded"].as_u64(), fp["objectsRemoved"].as_u64()), (Some(1), Some(1))); + assert_eq!(c["conflicts"], 2, "the deleted chart and the note"); + assert_eq!(s.doc().unwrap().wb.sheets.len(), 2, "the dry run's new sheet went away"); + let err = s.execute("macro.run", json!({"name": "Wide"})).unwrap_err().to_string(); + assert!(err.contains("(A1 on Notes; 1 object)"), "{err}"); + + // Running past the edge of the sheet fails before anything changes. + put( + &mut s, + json!({"name": "Edge", "inputs": [{"name": "Input"}], "steps": [{"command": "cell.set", "params": {"cell": "Input.offset(-3,0)", "input": "x"}}]}), + ); + let err = s.execute("macro.run", json!({"name": "Edge", "inputs": "B2"})).unwrap_err().to_string(); + assert!(err.contains("Step 1 (cell.set): 'Input.offset(-3,0)' runs past the top of the sheet."), "{err}"); + let c = s.execute("macro.check", json!({"name": "Edge", "inputs": "B2"})).unwrap(); + assert_eq!((c["ok"].as_bool(), c["footprint"]["error"]["edge"].as_bool()), (Some(false), Some(true))); +} + #[test] fn repeat_if_and_stop() { let mut s = s(); @@ -312,7 +554,7 @@ fn macros_are_bounded() { assert!(err.contains("ran more than 100000 steps"), "{err}"); let err = s.execute("macro.run", json!({"name": "Spin", "timeLimit": 0.001})).unwrap_err().to_string(); assert!(err.contains("ran longer than"), "{err}"); - // A run that checks each step keeps to the time limit too. + // The footprint's dry run keeps to the time limit too, and so does the check's. put( &mut s, json!({"name": "Spin", "output": {"onConflict": "fail"}, "steps": [{"type": "repeat", "over": "A1:A400", "body": [ @@ -321,6 +563,8 @@ fn macros_are_bounded() { ); let err = s.execute("macro.run", json!({"name": "Spin", "timeLimit": 0.001})).unwrap_err().to_string(); assert!(err.contains("ran longer than"), "{err}"); + let c = s.execute("macro.check", json!({"name": "Spin", "timeLimit": 0.001})).unwrap(); + assert!(c["errors"][0].as_str().unwrap().contains("ran longer than"), "{c}"); } #[test] @@ -333,22 +577,33 @@ fn a_token_in_a_path_needs_the_permission_and_approval() { for path in [json!({"$ref": "Input"}), json!(in_dir), json!(" ")] { let step = json!({"command": "file.exportCsv", "params": {"path": path}}); put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step]})); - // Without the permission nothing runs. + // Without the permission nothing runs, not even the check's dry run. let err = s.execute("macro.run", json!({"name": "Export"})).unwrap_err().to_string(); assert!(err.contains("Step 1 (file.exportCsv) writes a file. Add file.exportCsv to the macro's permissions"), "{path}: {err}"); - // With it, the person approves first. + let c = s.execute("macro.check", json!({"name": "Export"})).unwrap(); + assert!(c["errors"][0].as_str().unwrap().contains("writes a file"), "{path}: {c}"); + assert!(c.get("footprint").is_none()); + // With it, the person approves first; the check doesn't run it before then. put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step], "permissions": ["file.exportCsv"]})); let r = s.execute("macro.run", json!({"name": "Export"})); let Err(EngineError::Approve(q)) = &r else { panic!("{path}: {r:?}") }; assert!(q.contains("file.exportCsv, which writes a file"), "{q}"); + let c = s.execute("macro.check", json!({"name": "Export"})).unwrap(); + assert_eq!(c["needsApproval"], json!(["runs file.exportCsv, which writes a file"]), "{path}"); + assert!(c.get("footprint").is_none()); assert_eq!(files(&dir), 0, "{path}"); assert!(!std::path::Path::new("A1.csv").exists() && !std::path::Path::new(" .csv").exists()); } - // Once approved, the run writes the file. + // Once approved, the check's dry run stops before the step, and only the run writes the file. let step = json!({"command": "file.exportCsv", "params": {"path": in_dir}}); put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step], "permissions": ["file.exportCsv"]})); s.execute("macro.run", json!({"name": "Export", "approve": true, "inputs": "B2"})).unwrap(); assert_eq!(files(&dir), 1); + std::fs::remove_dir_all(&dir).unwrap(); + std::fs::create_dir_all(&dir).unwrap(); + let c = s.execute("macro.check", json!({"name": "Export", "inputs": "B2"})).unwrap(); + assert_eq!(c["footprint"]["partial"]["step"], "1", "{c}"); + assert_eq!(files(&dir), 0, "the check writes nothing"); let _ = std::fs::remove_dir_all(&dir); } @@ -391,10 +646,16 @@ fn the_policy_keeps_macros_away_from_files_and_the_app() { let Err(EngineError::Approve(q)) = &r else { panic!("{r:?}") }; assert_eq!(q, "Run macro 'Import'? It runs data.fromTextCsv, which reads a file."); assert_eq!(s.doc().unwrap().wb.sheets.len(), 1); + let c = s.execute("macro.check", json!({"name": "Import"})).unwrap(); + assert_eq!(c["needsApproval"], json!(["runs data.fromTextCsv, which reads a file"])); + assert!(c.get("footprint").is_none(), "nothing unapproved runs, even dry"); s.execute("macro.run", json!({"name": "Import", "approve": true})).unwrap(); assert_eq!(s.doc().unwrap().wb.sheets.len(), 2); s.execute("edit.undo", json!({})).unwrap(); s.execute("macro.run", json!({"name": "Import"})).unwrap(); + // The dry run skips the file step and says so. + let c = s.execute("macro.check", json!({"name": "Import"})).unwrap(); + assert_eq!(c["footprint"]["partial"]["step"], "1"); // Changing the macro asks again. put(&mut s, json!({"name": "Import", "description": "changed", "steps": [step], "permissions": ["data.fromTextCsv"]})); assert!(matches!(s.execute("macro.run", json!({"name": "Import"})), Err(EngineError::Approve(_)))); @@ -439,3 +700,63 @@ fn macros_from_a_file_ask_before_running() { let b = crate::macros::parse_macro(&json!({"steps": [{"params": {"input": "x", "cell": "A1"}, "command": "cell.set"}], "name": "M"})).unwrap(); assert_eq!(crate::macros::fingerprint(&a), crate::macros::fingerprint(&b)); } + +#[test] +fn a_partial_footprint_doesnt_clear_later_writes() { + let dir = std::env::temp_dir().join(format!("gc-macro-partial-{}", 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"}}); + // The dry run stops at an approved file step or at Solver; the step after it writes over B1. + 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(); + let c = s.execute("macro.check", json!({"name": "Late", "inputs": "A1"})).unwrap(); + if c.get("needsApproval").is_none() { + assert_eq!((c["conflicts"].as_u64(), &c["footprint"]["partial"]["step"]), (Some(0), &json!("1")), "{c}"); + } + 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}"); + 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"); + // After a shift, the steps past the file step are checked too. + let mut s = super::s(); + set(&mut s, "C1", "old"); + set(&mut s, "D1", "keep"); + let step = |cell: &str| json!({"command": "cell.set", "params": {"cell": cell, "input": "new"}}); + put( + &mut s, + json!({"name": "Shift", "permissions": ["file.exportCsv"], "output": {"onConflict": "shiftDown"}, "steps": [step("C1"), export, step("D1")]}), + ); + let start = s.doc().unwrap().wb.clone(); + let err = s.execute("macro.run", json!({"name": "Shift", "approve": true})).unwrap_err().to_string(); + assert!(err.contains("Step 3 (cell.set): It would write over data (D1 on Sheet1)"), "{err}"); + assert_eq!((v(&s, "C1").display(), v(&s, "D1").display()), ("old".into(), "keep".into())); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start), "the shift is rolled back too"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/macros.md b/docs/macros.md index daf0a2c35..a56bd77a9 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -1,17 +1,16 @@ # Macros -> **Last updated:** 2026-10-25 · **Change:** minor (running macros: `macro.run`) +> **Last updated:** 2026-10-25 · **Change:** minor (checking macros before they run: the footprint, shifting, `macro.check`) 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 declares the inputs it works on, runs as one undo step, rolls all workbook changes back on an -error (a file it already wrote stays), stops before it writes over data it may not, and does not -touch files without permission. +error (a file it already wrote stays), checks where its results will land before it writes them, +and does not touch files without permission. Agents run macros with `macro.run` like any other command. -This page describes the macro format, its tokens, and running macros. The engine has no check of a -macro before it runs, no macro debugger, recorder or management commands in this version, and no -macro interface. +This page describes the macro format, its tokens, and running and checking macros. The engine has +no macro debugger, recorder or management commands in this version, and no macro interface. ## A macro @@ -60,8 +59,9 @@ macro interface. 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 (`timeLimit` to change the time; - the web build has no clock, so there only the step limit applies). + 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). ## Tokens @@ -108,17 +108,24 @@ refused rather than read as the header. 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. -2. **Conflicts.** Unless the conflict rule is `overwrite`, each step is checked as it runs. A - step that writes over cells outside the inputs that held data when the run started, or deletes - sheets or objects that were there, stops the run, which rolls the workbook back (a file already - written stays). The rule is `onConflict` (or the macro's `output.onConflict`): `ask` (the call - fails with `confirm: …`; `overwrite: true` answers it), `fail` or `overwrite`. Shifting the - data to make room (`shiftDown`, `shiftRight`) needs to know where the macro writes before it - runs, which this version doesn't work out, so those rules stop the run as `fail` does. +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. 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). +`macro.check {name, inputs?, timeLimit?}` reports the bound inputs, the output anchor, any +approval needed and the footprint, without changing anything. + ## Storage Macros are saved in the workbook: as a field in GridCraft's JSON format, and in xlsx as hidden