From b995c7c9a074effb3591ab94ca02b4d64c4d8309 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:10:29 -0400 Subject: [PATCH] Engine: a recorded script run keeps the steps that ran The recorder expanded an automate.runScript step from the workbook as it stood when the recording stopped, so a script edited or deleted during the recording changed the saved macro, or dropped the step. The run now leaves the steps it ran with the recorder (Recorder::ran, by step index, within the byte budget), and the conversion reads those, falling back to the workbook only when they could not be kept. --- crates/engine/src/cmd/extra.rs | 9 +++++++++ crates/engine/src/cmd/macros.rs | 2 +- crates/engine/src/lib.rs | 13 ++++++++++++- crates/engine/src/macros/recorder.rs | 13 +++++++++---- crates/engine/src/record.rs | 27 +++++++++++++++++++++++---- crates/engine/src/tests/macros.rs | 23 +++++++++++++++++++++++ docs/macros.md | 3 ++- 7 files changed, 79 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 87b9fe468..b3a548d75 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -1074,6 +1074,9 @@ fn run_script(s: &mut Session, p: &Json) -> Result { } let _sandbox = crate::io::ScriptSandbox::enter(); let mut n = 0; + // What ran, for a recorder in progress: the recording keeps these steps with the run, so + // editing or deleting the script before the recording stops does not change what it saved. + let mut ran_steps: Vec<(String, Json)> = Vec::new(); for st in &steps { let id = st["command"].as_str().unwrap_or(""); if id.starts_with("automate.") { @@ -1092,6 +1095,12 @@ fn run_script(s: &mut Session, p: &Json) -> Result { } ran?; n += 1; + if s.recorder.is_some() { + ran_steps.push((id.to_string(), st["params"].clone())); + } + } + if s.recorder.is_some() { + s.script_ran = Some(ran_steps); } Ok(json!({"ran": n})) } diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 6cfd069f7..3dd8fea48 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -234,7 +234,7 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star Some(n) => crate::macros::unique_name(&wb, n), None => crate::macros::next_name(&wb), }; - let mut converted = crate::macros::recorder::convert(&name, &start, &rec.steps, &wb); + let mut converted = crate::macros::recorder::convert(&name, &start, &rec.steps, &rec.ran, &wb); // Steps past the macro limit, and ones recorded in another workbook, are left out and counted // with the ones the recorder dropped. let cut = converted.cut.saturating_add(converted.mac.steps.len().saturating_sub(gridcraft_model::MAX_MACRO_STEPS)); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 983764031..d39ef44d1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -426,6 +426,9 @@ pub struct Session { pub recorder: Option, /// How many `execute` calls are running (0 between top-level commands). depth: u32, + /// The steps the last successful `automate.runScript` ran while a recorder was on, for the + /// recorder to keep with that step (see [`record::Recorder::ran`]); taken by the recording hook. + pub(crate) script_ran: Option>, /// Macros the person allowed to run in this session: (document uid, the macro's content as /// canonical JSON, from [`macros::fingerprint`]). pub macro_approvals: std::collections::HashSet<(u64, String)>, @@ -691,7 +694,15 @@ impl Session { { p.insert("on".into(), on.clone()); } - rec.push(record::RecordedStep { doc, sheet, sheet_name, selection, id: id.to_string(), params }); + let kept = rec.push(record::RecordedStep { doc, sheet, sheet_name, selection, id: id.to_string(), params }); + // A script run is kept with the steps it ran, so editing or deleting the script + // before the recording stops does not change what was recorded. + if let Some(ran) = self.script_ran.take() + && kept + && id == "automate.runScript" + { + rec.attach_ran(ran); + } } self.push_journal(id, cmd::journal_params(¶ms)); } diff --git a/crates/engine/src/macros/recorder.rs b/crates/engine/src/macros/recorder.rs index 257278b02..54ff10811 100644 --- a/crates/engine/src/macros/recorder.rs +++ b/crates/engine/src/macros/recorder.rs @@ -17,6 +17,7 @@ use gridcraft_core::{CellRef, RangeRef, Value}; use gridcraft_model::{ColType, InputKind, MAX_MACRO_STEPS, Macro, NamedInput, Step, StepKind, Workbook}; use serde_json::{Map, Value as Json, json}; +use std::collections::BTreeMap; use super::policy::{Policy, macro_policy}; use crate::Selection; @@ -47,7 +48,9 @@ pub struct Converted { } /// Builds a macro called `name` from the recorded steps. -pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &Workbook) -> Converted { +/// `ran` is what a script step ran, by step index ([`crate::record::Recorder::ran`]); a script step +/// without an entry is read from `wb`. +pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], ran: &BTreeMap>, wb: &Workbook) -> Converted { let mut skipped = Vec::new(); // Commands as they'll be played back: (id, params, sheet before, selection before, whether // they came from another macro and stay as written). At most the macro's step limit are @@ -63,7 +66,7 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: & cut = cut.saturating_add(1); } }; - for st in steps.iter().filter(|st| st.doc == start.doc) { + for (index, st) in steps.iter().enumerate().filter(|(_, st)| st.doc == start.doc) { let id = st.id.as_str(); if id == "macro.run" { // Another macro's steps, when they stand on their own: no inputs, no output and only @@ -85,8 +88,10 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: & continue; } if id == "automate.runScript" { - // An Automate script's steps, as the script runs them. - match st.params.get("name").and_then(Json::as_str).and_then(|n| crate::cmd::extra::script_steps(wb, n)) { + // An Automate script's steps as they ran, or as the script stands now when the recorder + // could not keep them. + let now = || st.params.get("name").and_then(Json::as_str).and_then(|n| crate::cmd::extra::script_steps(wb, n)); + match ran.get(&index).cloned().or_else(now) { Some(steps) => { for (cid, params) in steps { if refused(&cid, ¶ms) { diff --git a/crates/engine/src/record.rs b/crates/engine/src/record.rs index 4a5d7c4c9..d4b3599c6 100644 --- a/crates/engine/src/record.rs +++ b/crates/engine/src/record.rs @@ -8,6 +8,7 @@ use serde::Serialize; use serde_json::Value as Json; +use std::collections::BTreeMap; use crate::Selection; @@ -46,25 +47,43 @@ pub struct Recorder { /// Once a step is dropped, every later one is too, so `steps` is always the beginning of /// what ran, never a sequence with gaps. pub dropped: usize, - /// The size of `steps` serialized as JSON. + /// The size of `steps` and `ran` serialized as JSON. bytes: usize, + /// What a step ran, by its index in `steps`, for a command that runs steps stored in the + /// workbook (`automate.runScript`): the commands as they ran, so a recording keeps them even + /// when the script is edited or deleted before the recording stops. A step without an entry + /// is read from the workbook when the recording is used. + pub ran: BTreeMap>, } impl Recorder { - /// Adds a step, or counts it as dropped once the buffer is full. - pub fn push(&mut self, step: RecordedStep) { + /// Adds a step, or counts it as dropped once the buffer is full; `true` when it was kept. + pub fn push(&mut self, step: RecordedStep) -> bool { // Once a step has been dropped, or the step count is full, the rest are counted without // being serialized (a `file.open` step holds a whole file). if self.dropped > 0 || self.steps.len() >= RECORD_LIMIT { self.dropped = self.dropped.saturating_add(1); - return; + return false; } let size = serde_json::to_vec(&step).map_or(0, |v| v.len()); if self.bytes.saturating_add(size) <= RECORD_MAX_BYTES { self.bytes = self.bytes.saturating_add(size); self.steps.push(step); + true } else { self.dropped = self.dropped.saturating_add(1); + false + } + } + + /// Attaches what the last kept step ran ([`Recorder::ran`]), within the byte budget: past it, + /// the step stays and its script is read from the workbook when the recording is used. + pub fn attach_ran(&mut self, steps: Vec<(String, Json)>) { + let Some(index) = self.steps.len().checked_sub(1) else { return }; + let size = serde_json::to_vec(&steps).map_or(0, |v| v.len()); + if self.bytes.saturating_add(size) <= RECORD_MAX_BYTES { + self.bytes = self.bytes.saturating_add(size); + self.ran.insert(index, steps); } } } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 8459bbcb1..fa4f4c7ed 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -1372,6 +1372,29 @@ fn a_recorded_script_run_leaves_out_the_steps_a_macro_may_not_run() { s.execute("macro.run", json!({"name": "Rec", "onConflict": "overwrite"})).unwrap(); } +#[test] +fn a_recorded_script_run_keeps_the_steps_that_ran() { + // One script is edited and another deleted before the recording stops: the macro holds the + // steps as they ran, not the scripts as they stand when the recording is saved. + let mut s = s(); + let step = |input: &str| json!([{"command": "cell.set", "params": {"cell": "B1", "input": input}}]); + s.execute("automate.newScript", json!({"name": "Edited", "commands": step("first")})).unwrap(); + s.execute("automate.newScript", json!({"name": "Gone", "commands": step("kept")})).unwrap(); + s.execute("macro.record", json!({"relative": false})).unwrap(); + s.execute("automate.runScript", json!({"name": "Edited"})).unwrap(); + s.execute("automate.runScript", json!({"name": "Gone"})).unwrap(); + s.execute("automate.newScript", json!({"name": "Edited", "commands": step("changed")})).unwrap(); + s.execute("automate.deleteScript", json!({"name": "Gone"})).unwrap(); + let r = s.execute("macro.record", json!({"stop": true, "name": "Rec"})).unwrap(); + let steps = recorded(&mut s, "Rec"); + assert_eq!( + steps.get(..2), + Some(&["cell.set {\"cell\":\"B1\",\"input\":\"first\"}".to_string(), "cell.set {\"cell\":\"B1\",\"input\":\"kept\"}".to_string()][..]), + "{steps:?}" + ); + assert!(!r["skipped"].as_array().unwrap().contains(&json!("automate.runScript")), "{r}"); +} + #[test] fn a_recording_is_saved_in_the_workbook_where_it_started() { let mut s = s(); diff --git a/docs/macros.md b/docs/macros.md index d14c92c00..35f624156 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -155,7 +155,8 @@ inferred. Toggles such as Bold are recorded with the state they set (`"on": true them back sets them rather than flipping them. Commands a macro may not run are left out and listed in the result, also from the steps of a macro or script run while recording; a macro run while recording is replaced by its steps when it has no inputs, no output and only plain commands -(otherwise it is left out and listed), and an Automate script run by the script's steps. +(otherwise it is left out and listed), and an Automate script run by the steps it ran, kept when +it ran: editing or deleting the script before the recording stops changes nothing. ## Other commands and storage