From 6cd4cebfb71d2a06e5f4079fccfd2a3b94ff2502 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:18:07 -0400 Subject: [PATCH] Engine: Stop Action runs on its own and is not recorded Run from inside another command, automate.stopAction rolled back that command's own group, pulling the workbook from under the rest of its run; and the recording buffer kept it, so a recording could replay the stop inside the run it was part of. Nested, the command is refused and the group stays; the recorder leaves it out (record::NOT_RECORDED). --- crates/engine/src/cmd/extra.rs | 5 +++++ crates/engine/src/lib.rs | 12 +++++++++++- crates/engine/src/record.rs | 4 ++++ crates/engine/src/tests/seams.rs | 26 ++++++++++++++++++++++++++ docs/architecture.md | 2 ++ 5 files changed, 48 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 009d77f2a..02d45febb 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -1099,6 +1099,11 @@ fn run_script(s: &mut Session, p: &Json) -> Result { /// Automate › Stop Action: rolls back the undo groups open on the active workbook, so Undo and /// Redo work there again. fn stop_action(s: &mut Session, _: &Json) -> Result { + // Inside another command the open groups are that command's own (an action in progress): + // a step of it that stopped them would pull the workbook from under the rest of its run. + if s.depth() > 1 { + return Err(EngineError::Disabled("automate.stopAction".into(), "Stop Action runs on its own, not as a step of a script or macro.".into())); + } let uid = s.doc()?.uid; Ok(json!({"stopped": s.abort_groups(uid)})) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 1827da375..856f1777e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -623,7 +623,7 @@ impl Session { // file, `home.formatCells` with no style) is not recorded either: the dialog's own // call is, so playing the recording back runs the command once, with its parameters. let opened_dialog = self.ui_requests.get(requests_before..).is_some_and(|new| new.iter().any(|q| matches!(q, UiRequest::Dialog(..)))); - if let Some((doc, sheet, sheet_name, selection)) = place.filter(|_| !opened_dialog) + if let Some((doc, sheet, sheet_name, selection)) = place.filter(|_| !opened_dialog && !record::NOT_RECORDED.contains(&id)) && let Some(rec) = self.recorder.as_mut() { rec.push(record::RecordedStep { doc, sheet, sheet_name, selection, id: id.to_string(), params: cmd::journal_params(¶ms) }); @@ -870,6 +870,16 @@ impl Session { self.depth } + /// Runs a command as a command in progress would (one level deeper), for tests of what a + /// nested call may do. + #[cfg(test)] + pub(crate) fn execute_nested(&mut self, id: &str, params: Json) -> Result { + self.depth = self.depth.saturating_add(1); + let r = self.execute(id, params); + self.depth = self.depth.saturating_sub(1); + r + } + /// Whether an undo group is open on the document with this uid. pub fn in_group(&self, doc: u64) -> bool { self.groups.iter().any(|g| g.doc == doc) diff --git a/crates/engine/src/record.rs b/crates/engine/src/record.rs index 7804be6c9..4a5d7c4c9 100644 --- a/crates/engine/src/record.rs +++ b/crates/engine/src/record.rs @@ -18,6 +18,10 @@ pub const RECORD_LIMIT: usize = 100_000; /// without a size cap a recording of a few opens of a large workbook keeps tens of MB alive. pub const RECORD_MAX_BYTES: usize = 64 * 1024 * 1024; +/// Commands never recorded: recovery actions, not steps of the person's work. A recording that +/// replayed Stop Action would stop the run it is a step of. +pub const NOT_RECORDED: [&str; 1] = ["automate.stopAction"]; + /// One recorded command and where it ran. #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index b445377b7..45d57e6f6 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -820,6 +820,32 @@ fn a_call_that_only_opens_a_dialog_is_not_recorded() { assert_eq!(s.stop_recording().unwrap().dropped, 0); } +#[test] +fn stop_action_runs_only_on_its_own_and_is_not_recorded() { + // A step of an action in progress may not stop the action: nested, the command is refused + // and the action's group stays. Nor is Stop Action a step of the person's work: a recording + // leaves it out, so playing the recording back never stops the run it is part of. + let mut s = s(); + s.start_recording(); + let r: crate::Result<()> = s.grouped("Outer", |s| { + set(s, "A1", "one"); + let err = s.execute_nested("automate.stopAction", json!({})).unwrap_err().to_string(); + assert!(err.contains("on its own"), "{err}"); + set(s, "A2", "two"); + Ok(()) + }); + r.unwrap(); + assert_eq!(v(&s, "A2").display(), "two"); + assert_eq!(undo_labels(&s), ["Outer"], "the action kept its group"); + // On its own it stops a group left open; the recording keeps the edits and not the stop. + let _left = s.group_begin("Left open").unwrap(); + set(&mut s, "A3", "three"); + assert_eq!(s.execute("automate.stopAction", json!({})).unwrap(), json!({"stopped": true})); + assert!(v(&s, "A3").is_empty()); + assert_eq!(recorded_ids(&s), ["cell.set", "cell.set", "cell.set"]); + assert_eq!(s.stop_recording().unwrap().dropped, 0); +} + #[test] fn the_recorder_keeps_top_level_calls_with_where_they_ran() { let mut s = s(); diff --git a/docs/architecture.md b/docs/architecture.md index d243643a4..60e095611 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -65,6 +65,8 @@ Actions made of many commands (macros, a debugger that runs one step per call) u every group it opened, as `group_rollback` does. `Session::abort_groups(doc)`, and the command `automate.stopAction` on the active workbook, roll back the groups left open on a document (a debugger the person abandons, a caller that dropped its token), so Undo works there again. The + command runs on its own: a step of a script or macro may not stop the action it is part of, and + a recording leaves it out, so playing the recording back never stops the run. The history limit is applied when the outermost command finishes; until then the steps made inside it are bounded on their own. - **Recording buffer** (`record.rs`). `Session::start_recording` collects each successful