diff --git a/cmux-tui/crates/cmux-tui-core/src/mux/tab_groups.rs b/cmux-tui/crates/cmux-tui-core/src/mux/tab_groups.rs index 7c89419d4dca..93d9ecf4a08a 100644 --- a/cmux-tui/crates/cmux-tui-core/src/mux/tab_groups.rs +++ b/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::>>()?; - 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)) diff --git a/cmux-tui/crates/cmux-tui-core/src/mux/tab_strip.rs b/cmux-tui/crates/cmux-tui-core/src/mux/tab_strip.rs index 84bf3665287c..2d3310f77b25 100644 --- a/cmux-tui/crates/cmux-tui-core/src/mux/tab_strip.rs +++ b/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, - /// 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, 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())?; diff --git a/cmux-tui/crates/cmux-tui-core/src/state.rs b/cmux-tui/crates/cmux-tui-core/src/state.rs index 31ee6ccfe820..025f67ff74a0 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state.rs +++ b/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; diff --git a/cmux-tui/crates/cmux-tui-core/src/state/closed_history.rs b/cmux-tui/crates/cmux-tui-core/src/state/closed_history.rs index 5bd203de92cd..445f0e8a7b02 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/closed_history.rs +++ b/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,11 +282,18 @@ 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() @@ -294,6 +301,9 @@ impl Mux { .flatten() .filter_map(|tab| reopened.reopened_as.get(tab.as_str()?).cloned()) .collect::>(); + 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() diff --git a/cmux-tui/crates/cmux-tui-core/src/state/closed_history_store.rs b/cmux-tui/crates/cmux-tui-core/src/state/closed_history_store.rs index 6260801481e0..4b5bdc39c4d4 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/closed_history_store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/closed_history_store.rs @@ -382,11 +382,16 @@ fn insert_group( transaction: &Transaction<'_>, members: Vec, window: Option, + tab_groups: Vec, ) -> 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::>(); let closing_screens = closing.screens.iter().cloned().collect::>(); + // Before `capture_workspace` forgets which closing workspaces are + // ephemeral: their groups are never recorded. + let closing_tabs = closing.tabs.iter().cloned().collect::>(); + 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( diff --git a/cmux-tui/crates/cmux-tui-core/src/state/closed_tab_groups.rs b/cmux-tui/crates/cmux-tui-core/src/state/closed_tab_groups.rs new file mode 100644 index 000000000000..8e64e88a8781 --- /dev/null +++ b/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, + screens: &HashSet, + workspaces: &HashSet, +) -> anyhow::Result> { + 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>(5)?, + row.get::<_, String>(6)?, + row.get::<_, String>(7)?, + row.get::<_, String>(8)?, + )) + })? + .collect::, _>>()? + }; + let mut archives: Vec = 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) +} diff --git a/cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/reopen_closed_kinds.rs b/cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/reopen_closed_kinds.rs index 8a0fae30d647..b71e2f8263a5 100644 --- a/cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/reopen_closed_kinds.rs +++ b/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::>(); + 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 { let reply = resource_request( &harness.socket,