From a3b5f8e45c0fc82f21ea56dfdb8909dd7143ba47 Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Fri, 23 Oct 2026 20:08:40 -0400
Subject: [PATCH] Engine: a command that panics leaves the undo history as it
 found it

The panic guard restored the workbook and selection but not the history, so
a nested edit made before the panic left a no-op undo step and the redo
history it had cleared stayed lost. The guard now truncates the steps made
during the command and puts the redo history back. Regression test, and the
architecture note says what the guard restores.
---
 crates/engine/src/lib.rs         | 12 +++++++++++-
 crates/engine/src/tests/seams.rs | 22 ++++++++++++++++++++++
 docs/architecture.md             |  3 ++-
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs
index ad740e3f6..9f79dd37f 100644
--- a/crates/engine/src/lib.rs
+++ b/crates/engine/src/lib.rs
@@ -532,6 +532,9 @@ impl Session {
         let (first_group, group_epoch) = (self.next_group, self.group_epoch);
         // Whether a group was open on the document before the command, and how long its history was.
         let start = before.as_ref().and_then(|(uid, ..)| self.docs.iter().find(|d| d.uid == *uid).map(|d| (self.in_group(*uid), d.undo.len())));
+        // A command that panics leaves the history as it found it: the steps commands nested in it
+        // made go with the workbook, and the redo history they cleared comes back.
+        let redo_before = before.as_ref().and_then(|(uid, ..)| self.docs.iter().find(|d| d.uid == *uid).map(|d| d.redo.clone()));
         // The recorder keeps top-level calls with the place they ran in.
         let place = (self.depth == 0 && self.recorder.is_some()).then(|| match self.active() {
             Some(d) => (d.uid, d.wb.active_sheet, d.wb.active().map(|sh| sh.name.clone()).unwrap_or_default(), d.selection.clone()),
@@ -551,13 +554,20 @@ impl Session {
             Err(p) => {
                 let msg =
                     p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::<String>().cloned()).unwrap_or_else(|| "panic".into());
-                // Restore the workbook as it was, and drop the undo groups the command opened.
+                // Restore the workbook and its history as they were, and drop the undo groups the
+                // command opened.
                 self.groups.retain(|g| g.id < first_group);
                 if let Some((uid, wb, sel)) = &before
                     && let Some(d) = self.docs.iter_mut().find(|d| d.uid == *uid)
                 {
                     d.wb = wb.clone();
                     d.selection = sel.clone();
+                    if let Some((_, len)) = start {
+                        d.undo.truncate(len);
+                    }
+                    if let Some(redo) = redo_before {
+                        d.redo = redo;
+                    }
                     d.calc.rebuild(&d.wb);
                 }
                 if self.depth == 0 {
diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs
index d3e9f78db..dedd5c67b 100644
--- a/crates/engine/src/tests/seams.rs
+++ b/crates/engine/src/tests/seams.rs
@@ -543,6 +543,28 @@ fn a_group_ended_at_the_history_limit_replaces_only_the_oldest_step() {
     assert_undo_redo(&mut s, &[("F1", "one"), ("F2", "two")]);
 }
 
+#[test]
+fn a_command_that_panics_leaves_the_history_as_it_found_it() {
+    // A command that edits through a nested command and then panics: the nested step is gone
+    // with the workbook it changed, and the redo history the nested step cleared comes back.
+    crate::cmd::register_undoable_test_command("test.editThenPanic", |s, _| {
+        put(s, "A1", "half done")?;
+        panic!("after an edit");
+    });
+    let mut s = s();
+    set(&mut s, "A1", "1");
+    set(&mut s, "A1", "2");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!((undo_labels(&s).len(), s.doc().unwrap().redo.len()), (1, 1));
+    let err = s.execute("test.editThenPanic", json!({})).unwrap_err().to_string();
+    assert!(err.contains("after an edit"), "{err}");
+    assert_eq!(v(&s, "A1").display(), "1", "the workbook is restored");
+    assert_eq!(undo_labels(&s), ["Enter Cell"], "no step for the nested edit");
+    assert_eq!(s.doc().unwrap().redo.len(), 1, "the redo history is back");
+    s.execute("edit.redo", json!({})).unwrap();
+    assert_eq!(v(&s, "A1").display(), "2");
+}
+
 // ---------------------------------------------------------------- recording buffer
 
 fn recorded_ids(s: &Session) -> Vec<String> {
diff --git a/docs/architecture.md b/docs/architecture.md
index e44e80083..aa5e1e64d 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -33,7 +33,8 @@ including tests, 2026-10-10.
    `Session::execute(id, params)`. Commands are `CommandSpec { id, label, menu, shortcut, params,
    enabled, run, journal, undoable }`; ids follow Excel's ribbon (`home.bold`, `insert.chart`,
    `data.sortAscending`) plus primitives (`cell.set`, `selection.set`, `range.setValues`).
-   `Session::execute` catches escaped panics and restores the workbook (never-crash guard).
+   `Session::execute` catches escaped panics and restores the workbook, its selection and its
+   undo and redo history as they were before the command (never-crash guard).
 2. **Edits** change the model through copy-on-write bands; undo keeps the previous snapshot.
    Undo retains at most 100 steps and trims older steps when its estimated retained cell storage
    exceeds 128 MiB. The newest step remains undoable even if that step alone exceeds the budget.