From 080b6d993b1682010a613ca41389dc953a22e6b1 Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Fri, 23 Oct 2026 16:36:34 -0400
Subject: [PATCH] Engine: a grouped action that closes its document succeeds

Session::grouped ended its group after the action, and when the action had closed the group's document (file.close, or file.open taking the place of an untouched new workbook) the group had gone with it, so the action was reported as failed although it had done what it was asked. A group that is gone because its document is gone now counts as ended; a group ended by someone else while the document is open is still an error.
---
 crates/engine/src/lib.rs         | 12 ++++++++----
 crates/engine/src/tests/seams.rs | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs
index 6e37058dd..ad740e3f6 100644
--- a/crates/engine/src/lib.rs
+++ b/crates/engine/src/lib.rs
@@ -652,11 +652,15 @@ impl Session {
     /// is rolled back and its error is returned. Groups nest; the outermost one makes the step.
     pub fn grouped<R>(&mut self, label: &str, f: impl FnOnce(&mut Session) -> Result<R>) -> Result<R> {
         let token = self.group_begin(label)?;
+        let doc = token.doc();
         match f(self) {
-            Ok(r) => {
-                self.group_end(token)?;
-                Ok(r)
-            }
+            Ok(r) => match self.group_end(token) {
+                Ok(_) => Ok(r),
+                // `f` closed the group's document (`file.close`, or `file.open` taking the place of
+                // an untouched new workbook), and the group went with it: nothing is left to undo.
+                Err(_) if !self.docs.iter().any(|d| d.uid == doc) => Ok(r),
+                Err(e) => Err(e),
+            },
             Err(e) => {
                 // The error from `f` is the one to report. A rollback fails only when the group
                 // has already ended, and then there is nothing left to undo.
diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs
index 3216cfd97..a5cd0d868 100644
--- a/crates/engine/src/tests/seams.rs
+++ b/crates/engine/src/tests/seams.rs
@@ -170,6 +170,27 @@ fn a_long_group_keeps_the_history_before_it() {
     assert_eq!(d.undo.iter().filter(|e| e.label == "Enter Cell").count(), 99);
 }
 
+#[test]
+fn a_grouped_action_that_closes_its_document_succeeds() {
+    // The group goes with the document; the action did what it was asked.
+    let mut s = s();
+    let uid = s.doc().unwrap().uid;
+    s.grouped("Close", |s| s.execute("file.close", json!({}))).unwrap();
+    assert!(s.docs.iter().all(|d| d.uid != uid) && !s.in_group(uid));
+    // Opening a file into an untouched new workbook takes its place the same way.
+    s.new_workbook();
+    let uid = s.doc().unwrap().uid;
+    let bytes = serde_json::to_vec(&gridcraft_model::Workbook::new()).unwrap();
+    s.grouped("Open", |s| s.execute("file.open", json!({"name": "theirs.json", "base64": crate::io::base64_encode(&bytes)}))).unwrap();
+    assert!(s.docs.iter().all(|d| d.uid != uid) && !s.in_group(uid) && s.groups.is_empty());
+    // A group ended by someone else while its document is still open is still an error.
+    let err = s.grouped("Ended", |s| {
+        let token = s.groups.last().map(|g| crate::GroupToken { id: g.id, doc: g.doc });
+        s.group_end(token.ok_or_else(|| EngineError::Other("no group".into()))?).map(|_| ())
+    });
+    assert!(err.unwrap_err().to_string().contains("already ended"));
+}
+
 #[test]
 fn closing_a_document_drops_its_groups() {
     let mut s = s();