cmux-tui: closing a screen, pane or workspace keeps its tab groups for reopen (cx-gzh.31)

cmux-tui/crates/cmux-tui-core/src/mux/tab_groups.rs

@@ -753,9 +753,6 @@ impl Mux {
                     .iter()
                     .map(|surface| tab_public_id(state, *surface))
                     .collect::<anyhow::Result<Vec<_>>>()?;
-                if let Some(record) = edit.groups.groups.get(&group_id) {
-                    edit.closed_group = Some(group_archive(state, &group_id, record, &tabs));
-                }
                 let panes = members
                     .iter()
                     .filter_map(|surface| state.pane_of(*surface))

cmux-tui/crates/cmux-tui-core/src/mux/tab_strip.rs

@@ -75,9 +75,6 @@ pub(crate) struct StripEdit {
     /// recorded as one closed-history group that forms it again
     /// (RECOVERABLE-BY-DEFAULT).
     pub(crate) ungrouped: Option<Value>,
-    /// A group that closes with its tabs (Close Group): its archive joins
-    /// the closed-history group of those tabs, whose reopen forms it again.
-    pub(crate) closed_group: Option<Value>,
     pub(crate) result: StripResult,
 }
 
@@ -108,9 +105,11 @@ impl Mux {
                     tab_state: Vec::new(),
                     saved: None,
                     ungrouped: None,
-                    closed_group: None,
                     result: StripResult::None,
                 };
+                // A screen, pane or workspace close leaves the rows of the
+                // groups it emptied: drop them before the edit sees them.
+                prune_tab_groups(&projected, &mut edit.groups);
                 let result = mutate(&mux, &mut projected, &mut edit)?;
                 Mux::rebuild_split_screen_index(&mut projected);
                 prune_tab_groups(&projected, &mut edit.groups);
@@ -200,8 +199,7 @@ fn strip_state_write(
     before: &crate::workspace_registry::PresentationSnapshot,
     edit: StripEdit,
 ) -> crate::resource_mutation::PlanStateWrite {
-    let StripEdit { groups, pins, tab_state, saved, ungrouped, closed_group, result: result_kind } =
-        edit;
+    let StripEdit { groups, pins, tab_state, saved, ungrouped, result: result_kind } = edit;
     let mut touched_tabs = BTreeSet::new();
     for (tab, group) in &before.tab_groups.members {
         if groups.members.get(tab) != Some(group) {
@@ -244,9 +242,6 @@ fn strip_state_write(
                 archive,
             )?;
         }
-        if let Some(archive) = &closed_group {
-            crate::state::closed_history_store::attach_closed_tab_group(transaction, archive)?;
-        }
         write_tab_groups(transaction, &groups)?;
         for (tab, pinned, url) in &pins {
             set_tab_pinned(transaction, tab, *pinned, url.as_deref())?;

cmux-tui/crates/cmux-tui-core/src/state.rs

@@ -34,6 +34,7 @@ pub(crate) mod closed_history_store;
 mod closed_history_tests;
 #[cfg(test)]
 mod closed_relaunch_tests;
+mod closed_tab_groups;
 pub(crate) mod command_history_store;
 pub(crate) mod commit;
 pub(crate) mod conversation_tabs;

cmux-tui/crates/cmux-tui-core/src/state/closed_history.rs

@@ -13,7 +13,7 @@
 //! request's result, so a retry with the same key replays it.
 
 use crate::Actor;
-use crate::mux::tab_groups::pane_by_public_id;
+use crate::mux::tab_groups::{group_members, pane_by_public_id};
 use crate::mux::tab_strip::StripRequest;
 use crate::mux::*;
 use crate::state::closed_history_query::{
@@ -282,18 +282,28 @@ impl Mux {
         for member in &restore {
             self.reopen_member(&mutation.actor, &closed_id, member, &mut reopened)?;
         }
-        if let Some(group) = record.get("tab_group").filter(|group| group.is_object()) {
-            // A closed group (Close Group) names its closed tabs: it forms
-            // again around those this reopen brought back.
+        // An ungrouped group (`tab_group`, no member) forms again whole. The
+        // groups a close took (`tab_groups`; `tab_group` on a Close Group
+        // recorded before cx-gzh.31) name their closed tabs: each forms
+        // again around those this reopen brought back.
+        let closed = !restore.is_empty();
+        let groups = record
+            .get("tab_group")
+            .filter(|group| group.is_object())
+            .into_iter()
+            .chain(record["tab_groups"].as_array().into_iter().flatten());
+        for group in groups {
             let mut group = group.clone();
-            let closed = !restore.is_empty();
             if closed {
                 let tabs = group["tab_ids"]
                     .as_array()
                     .into_iter()
                     .flatten()
                     .filter_map(|tab| reopened.reopened_as.get(tab.as_str()?).cloned())
                     .collect::<Vec<_>>();
+                if tabs.is_empty() {
+                    continue;
+                }
                 group["tab_ids"] = serde_json::json!(tabs);
             }
             self.reopen_tab_group(&mutation.actor, &group, closed, &mut reopened)?;
@@ -360,7 +370,10 @@ impl Mux {
     ) -> anyhow::Result<()> {
         let Some(id) = archive["id"].as_str().map(str::to_string) else { return Ok(()) };
         let presentation = self.presentation_snapshot();
-        let live = presentation.tab_groups.groups.contains_key(&id);
+        // A group row a close left behind (no strip edit pruned it yet)
+        // holds no open tab: it is not live, and forms again.
+        let live = self
+            .with_state(|state| !group_members(state, &presentation.tab_groups, &id).is_empty());
         if !live || closed {
             let wanted = archive["tab_ids"]
                 .as_array()

cmux-tui/crates/cmux-tui-core/src/state/closed_history_store.rs

@@ -382,11 +382,16 @@ fn insert_group(
     transaction: &Transaction<'_>,
     members: Vec<Value>,
     window: Option<String>,
+    tab_groups: Vec<Value>,
 ) -> anyhow::Result<()> {
-    let (closed_id, extra) = match take_pending_group(transaction)? {
+    let (closed_id, mut extra) = match take_pending_group(transaction)? {
         Some((closed_id, extra)) => (closed_id, Some(extra)),
         None => (new_closed_id(), None),
     };
+    if !tab_groups.is_empty() {
+        let fields = extra.get_or_insert_with(|| json!({}));
+        fields["tab_groups"] = Value::Array(tab_groups);
+    }
     insert_record(transaction, closed_id, members, window, extra)
 }
 
@@ -530,68 +535,6 @@ pub(crate) fn record_ungrouped_group(
     insert_record(transaction, new_closed_id(), Vec::new(), window, Some(extra))
 }
 
-/// Close of a whole tab group: `archive` (as [`record_ungrouped_group`]
-/// takes it, naming the closed tabs) rides on the group that recorded its
-/// tabs, so reopening them forms the group again. Nothing recorded them
-/// (an incognito workspace): nothing to attach.
-pub(crate) fn attach_closed_tab_group(
-    transaction: &Transaction<'_>,
-    archive: &Value,
-) -> anyhow::Result<()> {
-    let Some(first) = archive["tab_ids"].get(0).and_then(Value::as_str) else { return Ok(()) };
-    if !super::store::state_tables_ready(transaction)? {
-        return Ok(());
-    }
-    let found = transaction
-        .query_row(
-            "SELECT g.closed_id, g.record_json FROM closed_groups AS g
-             WHERE EXISTS (SELECT 1 FROM json_tree(g.record_json, '$.members') AS t
-                           WHERE t.key = 'tab_id' AND t.atom = ?1)
-             ORDER BY g.seq DESC LIMIT 1",
-            [first],
-            |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)),
-        )
-        .optional()?;
-    let Some((closed_id, record)) = found else { return Ok(()) };
-    let mut record: Value = serde_json::from_str(&record)?;
-    // Only the group this close recorded holds every one of its tabs, as
-    // tab members or inside the screen the close emptied.
-    let mut recorded = HashSet::new();
-    closed_tab_ids(&record["members"], &mut recorded);
-    if !archive["tab_ids"]
-        .as_array()
-        .into_iter()
-        .flatten()
-        .all(|tab| tab.as_str().is_some_and(|tab| recorded.contains(tab)))
-    {
-        return Ok(());
-    }
-    record["tab_group"] = archive.clone();
-    transaction.execute(
-        "UPDATE closed_groups SET record_json = ?2 WHERE closed_id = ?1",
-        params![closed_id, serde_json::to_string(&record)?],
-    )?;
-    queue_change(transaction, &state_upsert("closed", &closed_id, public_item(&record)))
-}
-
-/// Every closed tab id in `value`, at any depth.
-fn closed_tab_ids<'a>(value: &'a Value, ids: &mut HashSet<&'a str>) {
-    match value {
-        Value::Array(items) => items.iter().for_each(|item| closed_tab_ids(item, ids)),
-        Value::Object(fields) => {
-            for (key, field) in fields {
-                match field.as_str() {
-                    Some(id) if key == "tab_id" => {
-                        ids.insert(id);
-                    }
-                    _ => closed_tab_ids(field, ids),
-                }
-            }
-        }
-        _ => {}
-    }
-}
-
 /// A new closed group id.
 pub(crate) fn new_closed_id() -> String {
     format!("closed_{}", new_uuid_v4().replace('-', ""))
@@ -628,7 +571,7 @@ pub(crate) fn flush_pending_group(transaction: &Transaction<'_>) -> anyhow::Resu
     let pending: bool =
         transaction
             .query_row("SELECT EXISTS(SELECT 1 FROM closed_group_pending)", [], |row| row.get(0))?;
-    if pending { insert_group(transaction, Vec::new(), None) } else { Ok(()) }
+    if pending { insert_group(transaction, Vec::new(), None, Vec::new()) } else { Ok(()) }
 }
 
 /// The highest level among the members: a group with a screen is a screen
@@ -741,6 +684,15 @@ pub(crate) fn capture_closed(
     }
     let closing_workspaces = closing.workspaces.iter().cloned().collect::<HashSet<_>>();
     let closing_screens = closing.screens.iter().cloned().collect::<HashSet<_>>();
+    // Before `capture_workspace` forgets which closing workspaces are
+    // ephemeral: their groups are never recorded.
+    let closing_tabs = closing.tabs.iter().cloned().collect::<HashSet<_>>();
+    let tab_groups = super::closed_tab_groups::closing_tab_groups(
+        transaction,
+        &closing_tabs,
+        &closing_screens,
+        &closing_workspaces,
+    )?;
     let mut capture = Capture::default();
     for workspace_id in &closing.workspaces {
         capture_workspace(transaction, workspace_id, &mut capture)?;
@@ -770,7 +722,7 @@ pub(crate) fn capture_closed(
     if capture.members.is_empty() {
         return Ok(());
     }
-    insert_group(transaction, capture.members, capture.window)
+    insert_group(transaction, capture.members, capture.window, tab_groups)
 }
 
 fn capture_screen(

cmux-tui/crates/cmux-tui-core/src/state/closed_tab_groups.rs

@@ -0,0 +1,87 @@
+//! The tab groups a close takes with it (cx-gzh.31). Each group with a
+//! closing tab rides on the closed-history group as an archive naming its
+//! closed tabs, so reopening them forms it again: Close Group, a closed
+//! screen, pane or workspace, or the last tab of a group.
+
+use std::collections::HashSet;
+
+use rusqlite::{Connection, OptionalExtension};
+use serde_json::{Value, json};
+
+/// The archives of the live tab groups holding a tab that closes: one in
+/// `tabs`, or in a screen of `screens` or a workspace of `workspaces`.
+/// Each names only its closing tabs, in strip order, in the fields
+/// `record_ungrouped_group` takes. Groups in an ephemeral workspace are
+/// never recorded.
+pub(super) fn closing_tab_groups(
+    connection: &Connection,
+    tabs: &HashSet<String>,
+    screens: &HashSet<String>,
+    workspaces: &HashSet<String>,
+) -> anyhow::Result<Vec<Value>> {
+    let ready = connection
+        .query_row(
+            "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'tab_group_members'",
+            [],
+            |_| Ok(()),
+        )
+        .optional()?
+        .is_some();
+    if !ready {
+        return Ok(Vec::new());
+    }
+    let rows = {
+        let mut statement = connection.prepare(
+            "SELECT g.group_id, g.pane_id, g.name, g.color, g.collapsed, g.icon,
+                    m.tab_id, s.public_id, s.workspace_id
+             FROM tab_group_members AS m
+             JOIN tab_groups AS g ON g.group_id = m.group_id
+             JOIN resource_tabs AS t ON t.public_id = m.tab_id AND t.deleted_revision IS NULL
+                                        AND t.pane_id = g.pane_id
+             JOIN resource_panes AS p ON p.public_id = t.pane_id
+             JOIN resource_screens AS s ON s.public_id = p.screen_id
+             ORDER BY g.group_id ASC, t.position ASC",
+        )?;
+        statement
+            .query_map([], |row| {
+                Ok((
+                    row.get::<_, String>(0)?,
+                    row.get::<_, String>(1)?,
+                    row.get::<_, String>(2)?,
+                    row.get::<_, String>(3)?,
+                    row.get::<_, i64>(4)?,
+                    row.get::<_, Option<String>>(5)?,
+                    row.get::<_, String>(6)?,
+                    row.get::<_, String>(7)?,
+                    row.get::<_, String>(8)?,
+                ))
+            })?
+            .collect::<Result<Vec<_>, _>>()?
+    };
+    let mut archives: Vec<Value> = Vec::new();
+    for (group, pane, name, color, collapsed, icon, tab, screen, workspace) in rows {
+        if !(tabs.contains(&tab) || screens.contains(&screen) || workspaces.contains(&workspace))
+            || super::closed_history_store::is_ephemeral(connection, &workspace)?
+        {
+            continue;
+        }
+        match archives.last_mut().filter(|archive| archive["id"] == group.as_str()) {
+            Some(archive) => {
+                if let Some(tab_ids) = archive["tab_ids"].as_array_mut() {
+                    tab_ids.push(json!(tab));
+                }
+            }
+            None => archives.push(json!({
+                "id": group,
+                "pane_id": pane,
+                "workspace_id": workspace,
+                "name": name,
+                "color": color,
+                "collapsed": collapsed != 0,
+                "icon": icon,
+                "tab_ids": [tab],
+            })),
+        }
+    }
+    Ok(archives)
+}

cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/reopen_closed_kinds.rs

@@ -621,6 +621,66 @@ fn a_closed_tab_group_that_emptied_its_screen_reopens_as_its_group() {
     assert_eq!(groups[0]["tab_ids"], serde_json::Value::Array(back), "{groups:?}");
 }
 
+#[test]
+fn a_closed_screen_reopens_with_its_tab_group() {
+    let _exclusive = exclusive_process_test();
+    let (harness, mut app, _) = start("reopen-screen-group");
+    let workspace = kinds_workspace(&mut app);
+    let first = workspace["screens"][0]["resource_id"].clone();
+    app.send(serde_json::json!({"cmd":"new-screen","workspace":workspace["id"],
+                                "cols":80,"rows":24}));
+    let screen = kinds_workspace(&mut app)["screens"]
+        .as_array()
+        .into_iter()
+        .flatten()
+        .find(|screen| screen["resource_id"] != first)
+        .cloned()
+        .expect("the new screen is listed");
+    let pane = screen["panes"][0]["id"].as_u64().expect("the new screen has a pane");
+    for _ in 0..2 {
+        app.send(serde_json::json!({"cmd":"new-tab","pane":pane}));
+    }
+    let tabs = kinds_workspace(&mut app)["screens"]
+        .as_array()
+        .into_iter()
+        .flatten()
+        .find(|listed| listed["resource_id"] == screen["resource_id"])
+        .map(|listed| listed["panes"][0]["tabs"].as_array().cloned().unwrap_or_default())
+        .unwrap_or_default()
+        .iter()
+        .filter_map(|tab| tab["tab_resource_id"].as_str().map(str::to_string))
+        .collect::<Vec<_>>();
+    assert_eq!(tabs.len(), 3, "{tabs:?}");
+    resource_request(
+        &harness.socket,
+        "screen-group-create",
+        "tab_group.create",
+        serde_json::json!({"machine":"current","session":"current","tabs":tabs[1..],
+                           "name":"Build","color":"blue"}),
+        Some("screen-group-create"),
+    );
+    let group_id = tab_groups(&harness, "screen-group-list-1")[0]["id"]
+        .as_str()
+        .expect("the group has an id")
+        .to_string();
+
+    let (item, reopened) = close_records_and_reopens(
+        &harness,
+        "screen.close",
+        serde_json::json!({"screen":screen["resource_id"]}),
+        "screen-group",
+    );
+    assert_eq!(item["kind"], "screen", "{item}");
+    let back = reopened["tab_ids"].as_array().cloned().unwrap_or_default();
+    assert_eq!(back.len(), 3, "every tab reopens once: {reopened}");
+    let groups = tab_groups(&harness, "screen-group-list-2");
+    assert_eq!(groups.len(), 1, "the reopen formed no group: {groups:?}");
+    assert_eq!(groups[0]["id"], group_id.as_str());
+    assert_eq!(groups[0]["name"], "Build");
+    assert_eq!(groups[0]["color"], "blue");
+    assert_eq!(groups[0]["tab_ids"], serde_json::json!(back[1..]), "{groups:?}");
+}
+
 fn screen_groups(harness: &RecoveryHarness, request: &str) -> Vec<serde_json::Value> {
     let reply = resource_request(
         &harness.socket,