From 55a8f3a60883f60b511ceb7978a736f888603d02 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Sat, 1 Aug 2026 22:49:13 +0800 Subject: [PATCH 1/4] Save and restore tab groups in launch configurations Launch configs stored windows, tabs, panes, titles and colors but not tab groups, so a saved workspace reopened with every tab flat. The projection `From for WindowTemplate` never read `snapshot.tab_groups`, and WindowTemplate had nowhere to put them -- session restore already round-trips groups through sqlite, so only this path lost them. WindowTemplate gains `tab_groups`, TabTemplate gains a `group` index into it. Both are skipped when empty, so configs saved from ungrouped windows serialize exactly as before. Runtime TabGroupIds are not stored: they are per-session UUIDs, and a config can be reopened into a workspace that already holds groups, so open_launch_config_window mints fresh ids and resolves membership through the index. Membership rides on each tab's own group_id rather than its position, because the save path drops tabs it cannot represent and renumbers the survivors -- the snapshot index and the template index diverge exactly then. A group whose only member was dropped does not survive either, so a config never restores an empty group the user can neither see nor remove. Restore mirrors the session-restore path's feature gating: groups only under GroupedTabs, pinned state only under PinnedTabs. --- app/src/launch_configs/launch_config.rs | 80 +++++++++- app/src/launch_configs/launch_config_tests.rs | 149 +++++++++++++++++- app/src/workspace/view.rs | 35 ++++ crates/integration/src/test/launch_configs.rs | 8 + 4 files changed, 266 insertions(+), 6 deletions(-) diff --git a/app/src/launch_configs/launch_config.rs b/app/src/launch_configs/launch_config.rs index 3e4b032998a..aa594874b4a 100644 --- a/app/src/launch_configs/launch_config.rs +++ b/app/src/launch_configs/launch_config.rs @@ -3,8 +3,8 @@ use std::path::PathBuf; use serde::{Deserialize, Deserializer, Serialize}; use crate::app_state::{ - AppState, LeafContents, PaneNodeSnapshot, SplitDirection as StateSplitDirection, TabSnapshot, - WindowSnapshot, + AppState, LeafContents, PaneNodeSnapshot, SplitDirection as StateSplitDirection, + TabGroupSnapshot, TabSnapshot, WindowSnapshot, }; use crate::themes::theme::AnsiColorIdentifier; @@ -39,6 +39,37 @@ pub struct WindowTemplate { #[serde(skip_serializing_if = "Option::is_none", default)] pub active_tab_index: Option, pub tabs: Vec, + /// Tab groups in this window, in tab-bar order. A tab joins one by + /// index through [`TabTemplate::group`]; runtime `TabGroupId`s are not + /// serialized because they are regenerated on every restore. + #[serde(skip_serializing_if = "Vec::is_empty", default)] + pub tab_groups: Vec, +} + +/// A tab group as stored in a launch config. +#[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] +pub struct TabGroupTemplate { + #[serde(skip_serializing_if = "Option::is_none", default)] + pub name: Option, + #[serde(skip_serializing_if = "Option::is_none", default)] + pub color: Option, + #[serde(skip_serializing_if = "is_false", default)] + pub collapsed: bool, + #[serde(skip_serializing_if = "is_false", default)] + pub pinned: bool, +} + +impl From<&TabGroupSnapshot> for TabGroupTemplate { + fn from(snapshot: &TabGroupSnapshot) -> Self { + Self { + name: snapshot.name.clone(), + // Groups have no default-directory color to fall back to, so the + // manual selection is the whole story here. + color: snapshot.color.resolve(None), + collapsed: snapshot.collapsed, + pinned: snapshot.pinned, + } + } } impl From for WindowTemplate { @@ -46,12 +77,16 @@ impl From for WindowTemplate { let mut active_tab_index = None; let mut num_valid_tabs = 0; - let tabs = snapshot + // A tab can fail to convert, so group membership has to be carried on + // the tab's own `group_id` rather than inferred from its position -- + // the surviving tabs are renumbered below and the two indices diverge. + let tabs_with_groups = snapshot .tabs .into_iter() .enumerate() .filter_map(|(i, tab)| { - let tab = tab.try_into().ok()?; + let group_id = tab.group_id; + let tab: TabTemplate = tab.try_into().ok()?; if i == snapshot.active_tab_index { active_tab_index = Some(num_valid_tabs); @@ -59,17 +94,43 @@ impl From for WindowTemplate { num_valid_tabs += 1; - Some(tab) + Some((tab, group_id)) + }) + .collect::>(); + + // Keep only groups that still have a member, so a config never + // restores an empty group the user cannot see or remove. + let tab_groups = snapshot + .tab_groups + .iter() + .filter(|group| { + tabs_with_groups + .iter() + .any(|(_, group_id)| *group_id == Some(group.id)) + }) + .collect::>(); + + let tabs = tabs_with_groups + .into_iter() + .map(|(mut tab, group_id)| { + tab.group = group_id + .and_then(|group_id| tab_groups.iter().position(|group| group.id == group_id)); + tab }) .collect::>(); Self { active_tab_index, tabs, + tab_groups: tab_groups.into_iter().map(TabGroupTemplate::from).collect(), } } } +fn is_false(val: &bool) -> bool { + !*val +} + fn is_falsey(val: &Option) -> bool { val.is_none_or(|v| !v) } @@ -187,6 +248,9 @@ pub struct TabTemplate { pub commands: Vec, #[serde(skip_serializing_if = "Option::is_none", default)] pub color: Option, + /// Index into [`WindowTemplate::tab_groups`], when this tab is grouped. + #[serde(skip_serializing_if = "Option::is_none", default)] + pub group: Option, } impl TabTemplate { @@ -238,6 +302,9 @@ impl TryFrom for TabTemplate { layout: snapshot.root.try_into()?, commands: Vec::new(), color, + // Resolved by `From`, which is the only place + // that knows the window's surviving group list. + group: None, }) } } @@ -277,9 +344,11 @@ pub fn make_mock_single_window_launch_config() -> LaunchConfig { name: "Mocked Config".to_string(), active_window_index: Some(0), windows: vec![WindowTemplate { + tab_groups: vec![], active_tab_index: Some(0), tabs: vec![ TabTemplate { + group: None, title: Some("First Tab".to_string()), layout: PaneTemplateType::PaneTemplate { is_focused: Some(true), @@ -292,6 +361,7 @@ pub fn make_mock_single_window_launch_config() -> LaunchConfig { color: None, }, TabTemplate { + group: None, title: Some("Second Tab".to_string()), layout: PaneTemplateType::PaneTemplate { is_focused: Some(true), diff --git a/app/src/launch_configs/launch_config_tests.rs b/app/src/launch_configs/launch_config_tests.rs index 36fc49d4c7e..2e0ddffa27f 100644 --- a/app/src/launch_configs/launch_config_tests.rs +++ b/app/src/launch_configs/launch_config_tests.rs @@ -3,8 +3,11 @@ use std::path::PathBuf; use super::{CommandTemplate, LaunchConfig, PaneMode, PaneTemplateType}; use crate::app_state::{ AppState, BranchSnapshot, LeafContents, LeafSnapshot, NotebookPaneSnapshot, PaneFlex, - PaneNodeSnapshot, SplitDirection, TabSnapshot, TerminalPaneSnapshot, WindowSnapshot, + PaneNodeSnapshot, SplitDirection, TabGroupSnapshot, TabSnapshot, TerminalPaneSnapshot, + WindowSnapshot, }; +use crate::themes::theme::AnsiColorIdentifier; +use crate::workspace::tab_group::TabGroupId; use crate::drive::OpenWarpDriveObjectSettings; use crate::tab::SelectedTabColor; @@ -528,3 +531,147 @@ fn test_config_with_active_tab_being_filtered() { let template = LaunchConfig::from_snapshot("Test".into(), &state); assert_eq!(template.windows[0].active_tab_index, None) } + +// --------------------------------------------------------------------------- +// Tab groups (#13898) +// --------------------------------------------------------------------------- + +fn terminal_tab(cwd: &str, group_id: Option) -> TabSnapshot { + TabSnapshot { + custom_title: None, + default_directory_color: None, + selected_color: SelectedTabColor::default(), + root: PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(TerminalPaneSnapshot { + uuid: vec![], + cwd: Some(cwd.into()), + is_active: true, + is_read_only: false, + shell_launch_data: None, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: vec![], + active_conversation_id: None, + }), + }), + left_panel: None, + right_panel: None, + group_id, + pinned: false, + } +} + +/// A tab that cannot be saved into a launch config, so it drops out of the +/// template and shifts every later tab's index. +fn unsaveable_tab(group_id: Option) -> TabSnapshot { + TabSnapshot { + custom_title: None, + default_directory_color: None, + selected_color: SelectedTabColor::default(), + root: PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Notebook(NotebookPaneSnapshot::CloudNotebook { + notebook_id: None, + settings: OpenWarpDriveObjectSettings::default(), + }), + }), + left_panel: None, + right_panel: None, + group_id, + pinned: false, + } +} + +fn grouped_snapshot(tabs: Vec, tab_groups: Vec) -> AppState { + let mut state = multi_tab_snapshot(0, tabs); + state.windows[0].tab_groups = tab_groups; + state +} + +fn group(name: &str, id: TabGroupId) -> TabGroupSnapshot { + TabGroupSnapshot { + id, + name: Some(name.to_string()), + color: SelectedTabColor::Color(AnsiColorIdentifier::Blue), + collapsed: false, + pinned: false, + } +} + +#[test] +fn test_config_from_snapshot_preserves_tab_groups() { + let group_id = TabGroupId::new(); + let state = grouped_snapshot( + vec![ + terminal_tab("/a", Some(group_id)), + terminal_tab("/b", None), + terminal_tab("/c", Some(group_id)), + ], + vec![group("backend", group_id)], + ); + + let config = LaunchConfig::from_snapshot("test".to_string(), &state); + let window = &config.windows[0]; + + assert_eq!(window.tab_groups.len(), 1); + assert_eq!(window.tab_groups[0].name.as_deref(), Some("backend")); + assert_eq!(window.tab_groups[0].color, Some(AnsiColorIdentifier::Blue)); + + // Membership survives, and an ungrouped tab stays ungrouped. + assert_eq!(window.tabs[0].group, Some(0)); + assert_eq!(window.tabs[1].group, None); + assert_eq!(window.tabs[2].group, Some(0)); +} + +#[test] +fn test_config_from_snapshot_remaps_groups_around_unsaveable_tabs() { + // The first group's only tab cannot be saved, so that group must not + // survive -- and the second group's index must shift down with it. + // Membership is carried on each tab's `group_id`, never on its position, + // which is what makes this hold once the tab list is renumbered. + let dropped_group = TabGroupId::new(); + let kept_group = TabGroupId::new(); + + let state = grouped_snapshot( + vec![ + unsaveable_tab(Some(dropped_group)), + terminal_tab("/a", None), + terminal_tab("/b", Some(kept_group)), + ], + vec![group("cloud", dropped_group), group("local", kept_group)], + ); + + let config = LaunchConfig::from_snapshot("test".to_string(), &state); + let window = &config.windows[0]; + + assert_eq!(window.tabs.len(), 2); + assert_eq!(window.tab_groups.len(), 1, "empty group must be dropped"); + assert_eq!(window.tab_groups[0].name.as_deref(), Some("local")); + + assert_eq!(window.tabs[0].group, None); + assert_eq!( + window.tabs[1].group, + Some(0), + "the surviving group moved from index 1 to 0" + ); +} + +#[test] +fn test_config_from_snapshot_omits_tab_groups_when_there_are_none() { + // Configs saved from ungrouped windows must serialize exactly as before, + // so existing launch configs keep round-tripping unchanged. + let state = grouped_snapshot(vec![terminal_tab("/a", None)], vec![]); + + let config = LaunchConfig::from_snapshot("test".to_string(), &state); + + assert!(config.windows[0].tab_groups.is_empty()); + assert_eq!(config.windows[0].tabs[0].group, None); + + let yaml = serde_yaml::to_string(&config).expect("serializes"); + assert!(!yaml.contains("tab_groups"), "got:\n{yaml}"); + assert!(!yaml.contains("group:"), "got:\n{yaml}"); +} diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 8c0280d99fa..0b148663cd0 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3860,6 +3860,36 @@ impl Workspace { ) { let start_index = self.tabs.len(); + // Create the groups first so the per-tab assignments below have + // something to point at. Ids are minted here rather than restored: + // a launch config can be opened repeatedly, and into a workspace that + // already holds groups, so reusing saved ids would collide. + let group_ids: Vec = if FeatureFlag::GroupedTabs.is_enabled() { + window + .tab_groups + .iter() + .map(|group_template| { + let group = TabGroup { + id: TabGroupId::new(), + name: group_template.name.clone(), + color: group_template + .color + .map_or(SelectedTabColor::Unset, SelectedTabColor::Color), + collapsed: group_template.collapsed, + draggable_state: Default::default(), + // Mirrors the session-restore path: only honor pinned + // state while the Pinned Tabs feature is enabled. + pinned: FeatureFlag::PinnedTabs.is_enabled() && group_template.pinned, + }; + let id = group.id; + self.tab_groups.insert(id, group); + id + }) + .collect() + } else { + Vec::new() + }; + window .tabs .iter() @@ -3874,6 +3904,11 @@ impl Workspace { self.tabs[start_index + tab_index].selected_color = tab_template .color .map_or(SelectedTabColor::Unset, SelectedTabColor::Color); + // An out-of-range index (hand-edited YAML) leaves the tab + // ungrouped rather than failing the whole window. + self.tabs[start_index + tab_index].group_id = tab_template + .group + .and_then(|group_index| group_ids.get(group_index).copied()); }); if !window.tabs.is_empty() { diff --git a/crates/integration/src/test/launch_configs.rs b/crates/integration/src/test/launch_configs.rs index 54b9f2853ec..c41abd57fa1 100644 --- a/crates/integration/src/test/launch_configs.rs +++ b/crates/integration/src/test/launch_configs.rs @@ -169,8 +169,10 @@ pub fn test_launch_config_single_child_branch() -> Builder { name: "Mocked config".to_owned(), active_window_index: Some(0), windows: vec![WindowTemplate { + tab_groups: vec![], active_tab_index: Some(0), tabs: vec![TabTemplate { + group: None, title: Some("First tab".to_owned()), layout: PaneTemplateType::PaneBranchTemplate { split_direction: SplitDirection::Horizontal, @@ -298,9 +300,11 @@ pub fn test_with_launch_config_with_active_tab_index() -> Builder { name: "Mocked config".to_owned(), active_window_index: Some(0), windows: vec![WindowTemplate { + tab_groups: vec![], active_tab_index: Some(1), tabs: vec![ TabTemplate { + group: None, title: None, layout: PaneTemplateType::PaneBranchTemplate { split_direction: SplitDirection::Horizontal, @@ -358,8 +362,10 @@ pub fn test_with_launch_config_with_active_pane() -> Builder { name: "Mocked config".to_owned(), active_window_index: Some(0), windows: vec![WindowTemplate { + tab_groups: vec![], active_tab_index: Some(0), tabs: vec![TabTemplate { + group: None, title: None, layout: PaneTemplateType::PaneBranchTemplate { split_direction: SplitDirection::Horizontal, @@ -437,8 +443,10 @@ pub fn test_with_launch_config_with_no_active_pane() -> Builder { name: "Mocked config".to_owned(), active_window_index: Some(0), windows: vec![WindowTemplate { + tab_groups: vec![], active_tab_index: Some(0), tabs: vec![TabTemplate { + group: None, title: None, layout: PaneTemplateType::PaneBranchTemplate { split_direction: SplitDirection::Horizontal, From b9dfb3a139804af08f3a0ea832c4cf4f03991b18 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Sun, 2 Aug 2026 08:57:52 +0800 Subject: [PATCH 2/4] Keep each restored tab group to one contiguous run The tab bar collapses each contiguous run of same-group tabs into one group container (tab_bar_slots), pushing a fresh Group slot whenever the run breaks. Restore accepted membership straight from the config, so a hand-edited YAML reading group 0, ungrouped, group 0 would draw two containers sharing one id -- a state no other code path can reach. resolve_group_memberships now honors the first run of each group and returns later stragglers ungrouped, alongside the existing out-of-range handling. Reordering the tabs would also restore the invariant, but silently moving tabs the config explicitly ordered is the more surprising of the two. Pulling the resolution out of open_launch_config_window's closure also makes it reachable from unit tests, which needed a ViewContext before. --- app/src/launch_configs/launch_config.rs | 36 +++++++++ app/src/launch_configs/launch_config_tests.rs | 79 ++++++++++++++++++- app/src/workspace/view.rs | 14 +++- 3 files changed, 122 insertions(+), 7 deletions(-) diff --git a/app/src/launch_configs/launch_config.rs b/app/src/launch_configs/launch_config.rs index aa594874b4a..554c869a55c 100644 --- a/app/src/launch_configs/launch_config.rs +++ b/app/src/launch_configs/launch_config.rs @@ -131,6 +131,42 @@ fn is_false(val: &bool) -> bool { !*val } +/// Resolves each tab's group index for restore, keeping every group to a +/// single contiguous run. +/// +/// The tab bar collapses each *contiguous* run of same-group tabs into one +/// group container (`Workspace::tab_bar_slots`), so interleaved membership -- +/// group 0, an ungrouped tab, group 0 again -- would render as two containers +/// sharing one id, which no other code path can produce. Configs written by +/// `From` are always contiguous because a live window is, so +/// this only bites on hand-edited YAML. +/// +/// The first run of each group wins and later stragglers come back ungrouped. +/// Reordering the tabs would also restore the invariant, but silently moving +/// tabs the config explicitly ordered is the more surprising of the two. +/// Out-of-range indices are dropped the same way. +pub fn resolve_group_memberships(tabs: &[TabTemplate], group_count: usize) -> Vec> { + let mut closed: Vec = vec![false; group_count]; + let mut previous: Option = None; + + tabs.iter() + .map(|tab| { + let group = tab + .group + .filter(|index| *index < group_count) + .filter(|index| !closed[*index]); + + if previous != group + && let Some(previous) = previous + { + closed[previous] = true; + } + previous = group; + group + }) + .collect() +} + fn is_falsey(val: &Option) -> bool { val.is_none_or(|v| !v) } diff --git a/app/src/launch_configs/launch_config_tests.rs b/app/src/launch_configs/launch_config_tests.rs index 2e0ddffa27f..d60442968ee 100644 --- a/app/src/launch_configs/launch_config_tests.rs +++ b/app/src/launch_configs/launch_config_tests.rs @@ -1,15 +1,15 @@ use std::path::PathBuf; -use super::{CommandTemplate, LaunchConfig, PaneMode, PaneTemplateType}; +use super::{CommandTemplate, LaunchConfig, PaneMode, PaneTemplateType, TabTemplate}; use crate::app_state::{ AppState, BranchSnapshot, LeafContents, LeafSnapshot, NotebookPaneSnapshot, PaneFlex, PaneNodeSnapshot, SplitDirection, TabGroupSnapshot, TabSnapshot, TerminalPaneSnapshot, WindowSnapshot, }; -use crate::themes::theme::AnsiColorIdentifier; -use crate::workspace::tab_group::TabGroupId; use crate::drive::OpenWarpDriveObjectSettings; use crate::tab::SelectedTabColor; +use crate::themes::theme::AnsiColorIdentifier; +use crate::workspace::tab_group::TabGroupId; fn single_tab_snapshot(root: PaneNodeSnapshot) -> AppState { AppState { @@ -675,3 +675,76 @@ fn test_config_from_snapshot_omits_tab_groups_when_there_are_none() { assert!(!yaml.contains("tab_groups"), "got:\n{yaml}"); assert!(!yaml.contains("group:"), "got:\n{yaml}"); } + +fn tab_in_group(group: Option) -> TabTemplate { + TabTemplate { + title: None, + layout: PaneTemplateType::PaneTemplate { + cwd: PathBuf::from("/tmp"), + commands: vec![], + is_focused: None, + pane_mode: PaneMode::Terminal, + shell: None, + }, + commands: vec![], + color: None, + group, + } +} + +#[test] +fn test_resolve_group_memberships_keeps_contiguous_runs_intact() { + let tabs = vec![ + tab_in_group(Some(0)), + tab_in_group(Some(0)), + tab_in_group(None), + tab_in_group(Some(1)), + ]; + + assert_eq!( + super::resolve_group_memberships(&tabs, 2), + vec![Some(0), Some(0), None, Some(1)] + ); +} + +#[test] +fn test_resolve_group_memberships_ungroups_a_split_run() { + // The tab bar renders each contiguous run as its own container, so + // honoring the second run would draw two containers with one group id. + let tabs = vec![ + tab_in_group(Some(0)), + tab_in_group(None), + tab_in_group(Some(0)), + ]; + + assert_eq!( + super::resolve_group_memberships(&tabs, 1), + vec![Some(0), None, None], + "the group's second run must not reopen it" + ); +} + +#[test] +fn test_resolve_group_memberships_ungroups_a_run_split_by_another_group() { + let tabs = vec![ + tab_in_group(Some(0)), + tab_in_group(Some(1)), + tab_in_group(Some(0)), + ]; + + assert_eq!( + super::resolve_group_memberships(&tabs, 2), + vec![Some(0), Some(1), None] + ); +} + +#[test] +fn test_resolve_group_memberships_drops_out_of_range_indices() { + // Hand-edited YAML pointing past the end of `tab_groups`. + let tabs = vec![tab_in_group(Some(7)), tab_in_group(Some(0))]; + + assert_eq!( + super::resolve_group_memberships(&tabs, 1), + vec![None, Some(0)] + ); +} diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 0b148663cd0..f19e0cc228f 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3890,6 +3890,15 @@ impl Workspace { Vec::new() }; + // `tab_bar_slots` turns every *contiguous* run of same-group tabs into + // one group container, so interleaved membership would render as two + // containers sharing one id. `resolve_group_memberships` collapses that + // to the first run of each group; see its docs for why. + let memberships = crate::launch_configs::launch_config::resolve_group_memberships( + &window.tabs, + group_ids.len(), + ); + window .tabs .iter() @@ -3904,10 +3913,7 @@ impl Workspace { self.tabs[start_index + tab_index].selected_color = tab_template .color .map_or(SelectedTabColor::Unset, SelectedTabColor::Color); - // An out-of-range index (hand-edited YAML) leaves the tab - // ungrouped rather than failing the whole window. - self.tabs[start_index + tab_index].group_id = tab_template - .group + self.tabs[start_index + tab_index].group_id = memberships[tab_index] .and_then(|group_index| group_ids.get(group_index).copied()); }); From 9d62ca1afecfb70e9ea335537f27a9291c535dd6 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Mon, 3 Aug 2026 13:49:40 +0800 Subject: [PATCH 3/4] Cover launch config tab group restore with an integration test Adds test_launch_config_restores_tab_groups, which dispatches the real root_view:open_launch_config with a config carrying two named, colored groups and asserts the restored window: group names and colors come back, each tab lands in the group it was saved under, and a group split across an ungrouped tab collapses to its first contiguous run. The new assert_tab_groups helper numbers groups by first appearance in the tab list, so it pins down membership and identity together -- two tabs share an index only when they share a TabGroupId. --- .../workspace/assertions.rs | 77 ++++++++++++++- crates/integration/src/bin/integration.rs | 1 + crates/integration/src/test/launch_configs.rs | 99 +++++++++++++++++++ .../integration/tests/integration/ui_tests.rs | 1 + 4 files changed, 177 insertions(+), 1 deletion(-) diff --git a/app/src/integration_testing/workspace/assertions.rs b/app/src/integration_testing/workspace/assertions.rs index 91e2c84376f..d97ee4fd63a 100644 --- a/app/src/integration_testing/workspace/assertions.rs +++ b/app/src/integration_testing/workspace/assertions.rs @@ -1,7 +1,10 @@ -use warpui::async_assert_eq; use warpui::integration::AssertionCallback; +use warpui::{async_assert, async_assert_eq}; use crate::integration_testing::view_getters::workspace_view; +use crate::tab::SelectedTabColor; +use crate::themes::theme::AnsiColorIdentifier; +use crate::workspace::tab_group::TabGroupId; pub fn assert_focused_tab_index(tab_index: usize) -> AssertionCallback { Box::new(move |app, window_id| { @@ -12,6 +15,78 @@ pub fn assert_focused_tab_index(tab_index: usize) -> AssertionCallback { }) } +/// Assert how the workspace's tabs are grouped, along with the name and color +/// of each group. +/// +/// `expected_memberships` holds one entry per tab, in tab order: `None` for an +/// ungrouped tab, or an index into `expected_groups`. Groups are numbered by +/// the order they first appear in the tab list, so this also pins down which +/// tabs share a single group -- two tabs only get the same index when they +/// carry the same `TabGroupId`. +/// +/// Group colors are given the way a launch config writes them -- `None` for a +/// group that saved no color -- and are compared against the `SelectedTabColor` +/// restore is expected to produce. +pub fn assert_tab_groups( + expected_memberships: Vec>, + expected_groups: Vec<(Option<&'static str>, Option)>, +) -> AssertionCallback { + Box::new(move |app, window_id| { + let workspace = workspace_view(app, window_id); + workspace.read(app, |view, _ctx| { + let mut group_order: Vec = Vec::new(); + let memberships: Vec> = view + .tabs + .iter() + .map(|tab| { + tab.group_id.map(|id| { + group_order + .iter() + .position(|seen| *seen == id) + .unwrap_or_else(|| { + group_order.push(id); + group_order.len() - 1 + }) + }) + }) + .collect(); + + if memberships != expected_memberships { + return async_assert!( + false, + "Expected tab group memberships {expected_memberships:?}, but there were {memberships:?}" + ); + } + + let groups: Vec<(Option, SelectedTabColor)> = group_order + .iter() + .map(|id| { + let group = view + .tab_groups + .get(id) + .expect("A grouped tab's group must exist in the workspace"); + (group.name.clone(), group.color) + }) + .collect(); + let expected: Vec<(Option, SelectedTabColor)> = expected_groups + .iter() + .map(|&(name, color)| { + ( + name.map(str::to_owned), + color.map_or(SelectedTabColor::Unset, SelectedTabColor::Color), + ) + }) + .collect(); + + async_assert_eq!( + groups, + expected, + "Expected restored groups {expected:?}, but there were {groups:?}" + ) + }) + }) +} + /// Assert that there are a particular number of tabs in the workspace. pub fn assert_tab_count(tab_count: usize) -> AssertionCallback { Box::new(move |app, window_id| { diff --git a/crates/integration/src/bin/integration.rs b/crates/integration/src/bin/integration.rs index 0ace5c586d0..184c116457d 100644 --- a/crates/integration/src/bin/integration.rs +++ b/crates/integration/src/bin/integration.rs @@ -254,6 +254,7 @@ fn register_tests() -> HashMap<&'static str, BoxedBuilderFn> { register_test!(test_with_launch_config_with_active_tab_index); register_test!(test_with_launch_config_with_active_pane); register_test!(test_with_launch_config_with_no_active_pane); + register_test!(test_launch_config_restores_tab_groups); register_test!(test_find_query_not_evaluated_on_terminal_mode_change); register_test!(test_bash_bootstraps_with_prompt_command_array); register_test!(test_bash_bootstraps_with_prompt_command_array_that_sets_ps1); diff --git a/crates/integration/src/test/launch_configs.rs b/crates/integration/src/test/launch_configs.rs index c41abd57fa1..4f26e3c9937 100644 --- a/crates/integration/src/test/launch_configs.rs +++ b/crates/integration/src/test/launch_configs.rs @@ -513,3 +513,102 @@ pub fn test_with_launch_config_with_no_active_pane() -> Builder { .add_assertion(assert_focused_pane_index(0, 0)), ) } + +/// Opening a launch config that carries tab groups should rebuild those groups +/// in the new window: names and colors restored, each tab back in the group it +/// was saved under. +/// +/// The config here also hand-writes a membership a live window can never +/// produce -- group "Backend" on both sides of an ungrouped tab. The tab bar +/// renders each *contiguous* run as one container, so restore keeps the first +/// run and returns the straggler ungrouped rather than drawing two containers +/// that share an id. +pub fn test_launch_config_restores_tab_groups() -> Builder { + use warp::integration_testing::workspace::assert_tab_groups; + use warp::launch_configs::launch_config::{ + LaunchConfig, PaneMode, PaneTemplateType, TabGroupTemplate, TabTemplate, WindowTemplate, + }; + use warp::themes::theme::AnsiColorIdentifier; + + FeatureFlag::GroupedTabs.set_enabled(true); + + fn tab(title: &str, group: Option) -> TabTemplate { + TabTemplate { + group, + title: Some(title.to_owned()), + layout: PaneTemplateType::PaneTemplate { + is_focused: Some(true), + cwd: PathBuf::from("/some/path"), + commands: Vec::new(), + pane_mode: PaneMode::Terminal, + shell: None, + }, + commands: Vec::new(), + color: None, + } + } + + fn create_launch_config() -> LaunchConfig { + LaunchConfig { + name: "Mocked config".to_owned(), + active_window_index: Some(0), + windows: vec![WindowTemplate { + tab_groups: vec![ + TabGroupTemplate { + name: Some("Backend".to_owned()), + color: Some(AnsiColorIdentifier::Blue), + collapsed: false, + pinned: false, + }, + TabGroupTemplate { + name: Some("Frontend".to_owned()), + color: None, + collapsed: false, + pinned: false, + }, + ], + active_tab_index: Some(0), + tabs: vec![ + tab("api", Some(0)), + tab("worker", Some(0)), + tab("scratch", None), + tab("stray", Some(0)), + tab("web", Some(1)), + ], + }], + } + } + + new_builder() + .with_step(wait_until_bootstrapped_single_pane_for_tab(0)) + .with_step( + new_step_with_default_assertions("Assert we have only 1 window open at start") + .add_assertion(assert_num_windows_open(1)), + ) + .with_step( + new_step_with_default_assertions("Open a launch config carrying tab groups") + .with_action(move |app, _, _| { + app.dispatch_global_action( + "root_view:open_launch_config", + warp::root_view::OpenLaunchConfigArg { + launch_config: create_launch_config(), + ui_location: get_launch_config_ui_location(), + open_in_active_window: false, + }, + ); + }), + ) + .with_step( + new_step_with_default_assertions("Assert the groups came back with their tabs") + .add_assertion(assert_tab_count(5)) + .add_assertion(assert_tab_groups( + // "stray" asked for "Backend" again after an ungrouped tab, + // so it restores ungrouped. + vec![Some(0), Some(0), None, None, Some(1)], + vec![ + (Some("Backend"), Some(AnsiColorIdentifier::Blue)), + (Some("Frontend"), None), + ], + )), + ) +} diff --git a/crates/integration/tests/integration/ui_tests.rs b/crates/integration/tests/integration/ui_tests.rs index 7e9a25ff900..9b2686d2eb2 100644 --- a/crates/integration/tests/integration/ui_tests.rs +++ b/crates/integration/tests/integration/ui_tests.rs @@ -124,6 +124,7 @@ integration_tests! { test_with_launch_config_with_active_tab_index, test_with_launch_config_with_active_pane, test_with_launch_config_with_no_active_pane, + test_launch_config_restores_tab_groups, test_find_query_not_evaluated_on_terminal_mode_change, test_custom_open_completions_menu_binding, test_ssh_with_shell_override, From a92b9b4c99b29f53ecf0777d9bac36e5417fc7a9 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Mon, 3 Aug 2026 14:07:23 +0800 Subject: [PATCH 4/4] Drop restored tab groups that no tab joins open_launch_config_window inserted a TabGroup for every entry in the config before resolving memberships, so a hand-authored config could leave a group nothing points at -- invisible in the tab bar and unreachable from any menu. The save path already drops groups whose members were all unsaveable; restore now matches it. Memberships are resolved first and an id is minted only for a group that kept a member, which also covers the group whose last tab was stripped by the contiguous-run collapse. assert_tab_groups now requires the workspace to hold exactly the listed groups, and the integration config gains an 'Orphan' group no tab joins. --- .../workspace/assertions.rs | 13 ++++ app/src/workspace/view.rs | 74 +++++++++++-------- crates/integration/src/test/launch_configs.rs | 22 ++++-- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/app/src/integration_testing/workspace/assertions.rs b/app/src/integration_testing/workspace/assertions.rs index d97ee4fd63a..5f7d02a0c2a 100644 --- a/app/src/integration_testing/workspace/assertions.rs +++ b/app/src/integration_testing/workspace/assertions.rs @@ -27,6 +27,9 @@ pub fn assert_focused_tab_index(tab_index: usize) -> AssertionCallback { /// Group colors are given the way a launch config writes them -- `None` for a /// group that saved no color -- and are compared against the `SelectedTabColor` /// restore is expected to produce. +/// +/// The workspace must hold exactly the groups listed, so a group that no tab +/// belongs to is a failure rather than something the caller can omit. pub fn assert_tab_groups( expected_memberships: Vec>, expected_groups: Vec<(Option<&'static str>, Option)>, @@ -58,6 +61,16 @@ pub fn assert_tab_groups( ); } + if view.tab_groups.len() != group_order.len() { + return async_assert!( + false, + "Expected the workspace to hold exactly {} group(s), the ones its tabs \ + belong to, but it holds {} -- the extras have no members", + group_order.len(), + view.tab_groups.len() + ); + } + let groups: Vec<(Option, SelectedTabColor)> = group_order .iter() .map(|id| { diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index f19e0cc228f..4f96a1f6d0c 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3860,45 +3860,55 @@ impl Workspace { ) { let start_index = self.tabs.len(); - // Create the groups first so the per-tab assignments below have - // something to point at. Ids are minted here rather than restored: - // a launch config can be opened repeatedly, and into a workspace that - // already holds groups, so reusing saved ids would collide. - let group_ids: Vec = if FeatureFlag::GroupedTabs.is_enabled() { - window - .tab_groups - .iter() - .map(|group_template| { - let group = TabGroup { - id: TabGroupId::new(), - name: group_template.name.clone(), - color: group_template - .color - .map_or(SelectedTabColor::Unset, SelectedTabColor::Color), - collapsed: group_template.collapsed, - draggable_state: Default::default(), - // Mirrors the session-restore path: only honor pinned - // state while the Pinned Tabs feature is enabled. - pinned: FeatureFlag::PinnedTabs.is_enabled() && group_template.pinned, - }; - let id = group.id; - self.tab_groups.insert(id, group); - id - }) - .collect() - } else { - Vec::new() - }; - // `tab_bar_slots` turns every *contiguous* run of same-group tabs into // one group container, so interleaved membership would render as two // containers sharing one id. `resolve_group_memberships` collapses that // to the first run of each group; see its docs for why. + let group_count = if FeatureFlag::GroupedTabs.is_enabled() { + window.tab_groups.len() + } else { + 0 + }; let memberships = crate::launch_configs::launch_config::resolve_group_memberships( &window.tabs, - group_ids.len(), + group_count, ); + // Only mint ids for groups that kept a member. A hand-authored config + // can name a group no tab joins, and the collapse above can strip a + // group's last tab; inserting those anyway would leave empty groups in + // workspace state that nothing can reach. This mirrors the save path, + // which already drops groups whose members were all unsaveable. + // + // Ids are minted here rather than restored: a launch config can be + // opened repeatedly, and into a workspace that already holds groups, so + // reusing saved ids would collide. + let group_ids: Vec> = window + .tab_groups + .iter() + .enumerate() + .map(|(group_index, group_template)| { + if !memberships.contains(&Some(group_index)) { + return None; + } + let group = TabGroup { + id: TabGroupId::new(), + name: group_template.name.clone(), + color: group_template + .color + .map_or(SelectedTabColor::Unset, SelectedTabColor::Color), + collapsed: group_template.collapsed, + draggable_state: Default::default(), + // Mirrors the session-restore path: only honor pinned + // state while the Pinned Tabs feature is enabled. + pinned: FeatureFlag::PinnedTabs.is_enabled() && group_template.pinned, + }; + let id = group.id; + self.tab_groups.insert(id, group); + Some(id) + }) + .collect(); + window .tabs .iter() @@ -3914,7 +3924,7 @@ impl Workspace { .color .map_or(SelectedTabColor::Unset, SelectedTabColor::Color); self.tabs[start_index + tab_index].group_id = memberships[tab_index] - .and_then(|group_index| group_ids.get(group_index).copied()); + .and_then(|group_index| group_ids.get(group_index).copied().flatten()); }); if !window.tabs.is_empty() { diff --git a/crates/integration/src/test/launch_configs.rs b/crates/integration/src/test/launch_configs.rs index 4f26e3c9937..2871455b180 100644 --- a/crates/integration/src/test/launch_configs.rs +++ b/crates/integration/src/test/launch_configs.rs @@ -518,11 +518,14 @@ pub fn test_with_launch_config_with_no_active_pane() -> Builder { /// in the new window: names and colors restored, each tab back in the group it /// was saved under. /// -/// The config here also hand-writes a membership a live window can never -/// produce -- group "Backend" on both sides of an ungrouped tab. The tab bar -/// renders each *contiguous* run as one container, so restore keeps the first -/// run and returns the straggler ungrouped rather than drawing two containers -/// that share an id. +/// The config here also hand-writes two shapes a live window can never produce: +/// +/// - group "Backend" on both sides of an ungrouped tab. The tab bar renders +/// each *contiguous* run as one container, so restore keeps the first run and +/// returns the straggler ungrouped rather than drawing two containers that +/// share an id. +/// - group "Orphan", which no tab joins. Restore must not put it in workspace +/// state, where nothing could reach it. pub fn test_launch_config_restores_tab_groups() -> Builder { use warp::integration_testing::workspace::assert_tab_groups; use warp::launch_configs::launch_config::{ @@ -566,6 +569,12 @@ pub fn test_launch_config_restores_tab_groups() -> Builder { collapsed: false, pinned: false, }, + TabGroupTemplate { + name: Some("Orphan".to_owned()), + color: Some(AnsiColorIdentifier::Red), + collapsed: false, + pinned: false, + }, ], active_tab_index: Some(0), tabs: vec![ @@ -605,6 +614,9 @@ pub fn test_launch_config_restores_tab_groups() -> Builder { // "stray" asked for "Backend" again after an ungrouped tab, // so it restores ungrouped. vec![Some(0), Some(0), None, None, Some(1)], + // "Orphan" is absent: assert_tab_groups requires the + // workspace to hold exactly these, so a memberless group + // left behind would fail here. vec![ (Some("Backend"), Some(AnsiColorIdentifier::Blue)), (Some("Frontend"), None),