From d6cd622c49c854b708224a92e26e09815f49ebaf Mon Sep 17 00:00:00 2001 From: lonexreb Date: Sat, 18 Jul 2026 00:20:31 -0500 Subject: [PATCH 1/2] fix(sessions-launch-config): persist tab groups in launch configurations (#13898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving a window as a Launch Configuration dropped tab groups: WindowTemplate / TabTemplate had no group metadata, and open_launch_config_window re-added each tab individually without recreating groups — so a grouped window always reopened flat. Save path: WindowTemplate gains tab_groups (name/color/collapsed/pinned) and TabTemplate gains group, an index into that list. From maps each group's runtime id to its serialized index and stamps member tabs. Runtime TabGroupIds are per-session UUIDs and are deliberately not serialized; fresh ids are minted on open. Open path: open_launch_config_window recreates the groups (gated on FeatureFlag::GroupedTabs, pinned additionally on PinnedTabs — mirroring grouped-tab session restore in configure_new_workspace) and reattaches tabs by group index, dropping references that don't resolve (hand-edited YAML). Both new fields are optional/defaulted in serde, so launch configs written before tab groups existed still parse (covered by a test). Tests: group mapping from snapshot, YAML round trip, and legacy-YAML backward-compat. --- app/src/launch_configs/launch_config.rs | 60 +++++++- app/src/launch_configs/launch_config_tests.rs | 137 +++++++++++++++++- app/src/workspace/view.rs | 35 +++++ 3 files changed, 229 insertions(+), 3 deletions(-) diff --git a/app/src/launch_configs/launch_config.rs b/app/src/launch_configs/launch_config.rs index 3e4b032998a..a7d46c4d428 100644 --- a/app/src/launch_configs/launch_config.rs +++ b/app/src/launch_configs/launch_config.rs @@ -6,6 +6,7 @@ use crate::app_state::{ AppState, LeafContents, PaneNodeSnapshot, SplitDirection as StateSplitDirection, TabSnapshot, WindowSnapshot, }; +use crate::tab::SelectedTabColor; use crate::themes::theme::AnsiColorIdentifier; #[cfg(test)] @@ -38,20 +39,66 @@ impl LaunchConfig { pub struct WindowTemplate { #[serde(skip_serializing_if = "Option::is_none", default)] pub active_tab_index: Option, + /// Tab groups in this window. Tabs reference a group via + /// [`TabTemplate::group`], an index into this list (#13898). + #[serde(skip_serializing_if = "Vec::is_empty", default)] + pub tab_groups: Vec, pub tabs: Vec, } +/// A tab group saved in a launch configuration. +/// +/// Groups are referenced by member tabs through their index in +/// [`WindowTemplate::tab_groups`]; runtime [`TabGroupId`]s are not serialized +/// since they are per-session UUIDs — fresh ids are minted when the launch +/// config opens. +#[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_falsey", default)] + pub collapsed: Option, + #[serde(skip_serializing_if = "is_falsey", default)] + pub pinned: Option, +} + impl From for WindowTemplate { fn from(snapshot: WindowSnapshot) -> Self { let mut active_tab_index = None; let mut num_valid_tabs = 0; + // Map each group's runtime id to its index in the serialized list so + // member tabs can reference it (#13898). + let group_index_by_id: std::collections::HashMap<_, _> = snapshot + .tab_groups + .iter() + .enumerate() + .map(|(index, group)| (group.id, index)) + .collect(); + let tab_groups = snapshot + .tab_groups + .iter() + .map(|group| TabGroupTemplate { + name: group.name.clone(), + color: match group.color { + SelectedTabColor::Color(color) => Some(color), + _ => None, + }, + collapsed: group.collapsed.then_some(true), + pinned: group.pinned.then_some(true), + }) + .collect(); + let tabs = snapshot .tabs .into_iter() .enumerate() .filter_map(|(i, tab)| { - let tab = tab.try_into().ok()?; + let group_id = tab.group_id; + let mut tab: TabTemplate = tab.try_into().ok()?; + tab.group = group_id.and_then(|id| group_index_by_id.get(&id)).copied(); if i == snapshot.active_tab_index { active_tab_index = Some(num_valid_tabs); @@ -65,6 +112,7 @@ impl From for WindowTemplate { Self { active_tab_index, + tab_groups, tabs, } } @@ -187,6 +235,10 @@ pub struct TabTemplate { pub commands: Vec, #[serde(skip_serializing_if = "Option::is_none", default)] pub color: Option, + /// Index into [`WindowTemplate::tab_groups`] for the group this tab + /// belongs to, if any (#13898). + #[serde(skip_serializing_if = "Option::is_none", default)] + pub group: Option, } impl TabTemplate { @@ -238,6 +290,9 @@ impl TryFrom for TabTemplate { layout: snapshot.root.try_into()?, commands: Vec::new(), color, + // Group membership is assigned by `WindowTemplate::from`, which owns + // the group-id → index mapping. + group: None, }) } } @@ -278,6 +333,7 @@ pub fn make_mock_single_window_launch_config() -> LaunchConfig { active_window_index: Some(0), windows: vec![WindowTemplate { active_tab_index: Some(0), + tab_groups: Vec::new(), tabs: vec![ TabTemplate { title: Some("First Tab".to_string()), @@ -290,6 +346,7 @@ pub fn make_mock_single_window_launch_config() -> LaunchConfig { }, commands: Vec::new(), color: None, + group: None, }, TabTemplate { title: Some("Second Tab".to_string()), @@ -302,6 +359,7 @@ pub fn make_mock_single_window_launch_config() -> LaunchConfig { }, commands: Vec::new(), color: None, + group: None, }, ], }], diff --git a/app/src/launch_configs/launch_config_tests.rs b/app/src/launch_configs/launch_config_tests.rs index 6b5e34015db..e28ccfa0f11 100644 --- a/app/src/launch_configs/launch_config_tests.rs +++ b/app/src/launch_configs/launch_config_tests.rs @@ -1,12 +1,15 @@ use std::path::PathBuf; -use super::{CommandTemplate, LaunchConfig, PaneMode, PaneTemplateType}; +use super::{CommandTemplate, LaunchConfig, PaneMode, PaneTemplateType, TabGroupTemplate}; use crate::app_state::{ AppState, BranchSnapshot, LeafContents, LeafSnapshot, NotebookPaneSnapshot, PaneFlex, - PaneNodeSnapshot, SplitDirection, TabSnapshot, TerminalPaneSnapshot, WindowSnapshot, + PaneNodeSnapshot, SplitDirection, TabGroupSnapshot, TabSnapshot, TerminalPaneSnapshot, + WindowSnapshot, }; 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 { @@ -526,3 +529,133 @@ 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) } + +fn terminal_tab_snapshot(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("/some/dir".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, + } +} + +/// Regression test for #13898: saving a window with grouped tabs as a launch +/// configuration must persist the groups and each tab's group membership. +#[test] +fn test_config_from_snapshot_maps_tab_groups() { + let group_a = TabGroupId::new(); + let group_b = TabGroupId::new(); + let mut state = multi_tab_snapshot( + 0, + vec![ + terminal_tab_snapshot(Some(group_a)), + terminal_tab_snapshot(None), + terminal_tab_snapshot(Some(group_b)), + ], + ); + state.windows[0].tab_groups = vec![ + TabGroupSnapshot { + id: group_a, + name: Some("Backend".to_string()), + color: SelectedTabColor::Color(AnsiColorIdentifier::Blue), + collapsed: true, + pinned: false, + }, + TabGroupSnapshot { + id: group_b, + name: None, + color: SelectedTabColor::Unset, + collapsed: false, + pinned: false, + }, + ]; + + let template = LaunchConfig::from_snapshot("Test".into(), &state); + let window = &template.windows[0]; + + assert_eq!( + window.tab_groups, + vec![ + TabGroupTemplate { + name: Some("Backend".to_string()), + color: Some(AnsiColorIdentifier::Blue), + collapsed: Some(true), + pinned: None, + }, + TabGroupTemplate { + name: None, + color: None, + collapsed: None, + pinned: None, + }, + ], + ); + assert_eq!(window.tabs[0].group, Some(0)); + assert_eq!(window.tabs[1].group, None); + assert_eq!(window.tabs[2].group, Some(1)); +} + +/// Tab groups survive a YAML serialize → deserialize round trip (#13898). +#[test] +fn test_launch_config_yaml_round_trips_tab_groups() { + let group_a = TabGroupId::new(); + let mut state = multi_tab_snapshot( + 0, + vec![ + terminal_tab_snapshot(Some(group_a)), + terminal_tab_snapshot(None), + ], + ); + state.windows[0].tab_groups = vec![TabGroupSnapshot { + id: group_a, + name: Some("Frontend".to_string()), + color: SelectedTabColor::Unset, + collapsed: false, + pinned: false, + }]; + + let template = LaunchConfig::from_snapshot("Round Trip".into(), &state); + let yaml = serde_yaml::to_string(&template).expect("launch config should serialize"); + let parsed: LaunchConfig = serde_yaml::from_str(&yaml).expect("launch config should parse"); + + assert_eq!(parsed.windows[0].tab_groups, template.windows[0].tab_groups); + assert_eq!(parsed.windows[0].tabs[0].group, Some(0)); + assert_eq!(parsed.windows[0].tabs[1].group, None); +} + +/// Launch configs written before tab groups existed still parse (#13898). +#[test] +fn test_launch_config_yaml_without_groups_parses() { + let config: LaunchConfig = serde_yaml::from_str( + r#" +name: Legacy Config +windows: + - tabs: + - layout: + cwd: /tmp +"#, + ) + .expect("legacy launch config should parse"); + + assert!(config.windows[0].tab_groups.is_empty()); + assert_eq!(config.windows[0].tabs[0].group, None); +} diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 5d606303df3..274c28b5dc8 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3853,6 +3853,35 @@ impl Workspace { ) { let start_index = self.tabs.len(); + // Recreate the config's tab groups with fresh runtime ids so member + // tabs below can be reattached by their template group index (#13898). + // Mirrors grouped-tab restoration in `configure_new_workspace`. + 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.unwrap_or_default(), + draggable_state: Default::default(), + // Only restore pinned state when the Pinned Tabs feature is enabled. + pinned: FeatureFlag::PinnedTabs.is_enabled() + && group_template.pinned.unwrap_or_default(), + }; + let group_id = group.id; + self.tab_groups.insert(group_id, group); + group_id + }) + .collect() + } else { + Vec::new() + }; + window .tabs .iter() @@ -3867,6 +3896,12 @@ impl Workspace { self.tabs[start_index + tab_index].selected_color = tab_template .color .map_or(SelectedTabColor::Unset, SelectedTabColor::Color); + // Drop the group reference if its index doesn't resolve (e.g. + // hand-edited YAML or the GroupedTabs flag being disabled). + 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() { From 39bf677501ed76a01b2d42aeb327be6d91b514d2 Mon Sep 17 00:00:00 2001 From: lonexreb Date: Sat, 18 Jul 2026 00:36:02 -0500 Subject: [PATCH 2/2] fix(sessions-launch-config): don't persist group pinned state (#13898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: opening a launch config into a window that already has unpinned tabs could restore a group as pinned while its member tabs are appended after existing unpinned tabs, breaking the pinned-region ordering invariant. Launch configs deliberately don't persist pinned state for tabs; group pinned persistence was scope creep. Drop the template field entirely — recreated groups always open unpinned, which removes the invariant hazard by construction and matches the existing per-tab behavior. --- app/src/launch_configs/launch_config.rs | 3 --- app/src/launch_configs/launch_config_tests.rs | 2 -- app/src/workspace/view.rs | 8 +++++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/src/launch_configs/launch_config.rs b/app/src/launch_configs/launch_config.rs index a7d46c4d428..05db6df4bfe 100644 --- a/app/src/launch_configs/launch_config.rs +++ b/app/src/launch_configs/launch_config.rs @@ -60,8 +60,6 @@ pub struct TabGroupTemplate { pub color: Option, #[serde(skip_serializing_if = "is_falsey", default)] pub collapsed: Option, - #[serde(skip_serializing_if = "is_falsey", default)] - pub pinned: Option, } impl From for WindowTemplate { @@ -87,7 +85,6 @@ impl From for WindowTemplate { _ => None, }, collapsed: group.collapsed.then_some(true), - pinned: group.pinned.then_some(true), }) .collect(); diff --git a/app/src/launch_configs/launch_config_tests.rs b/app/src/launch_configs/launch_config_tests.rs index e28ccfa0f11..73f8ab1d1e4 100644 --- a/app/src/launch_configs/launch_config_tests.rs +++ b/app/src/launch_configs/launch_config_tests.rs @@ -599,13 +599,11 @@ fn test_config_from_snapshot_maps_tab_groups() { name: Some("Backend".to_string()), color: Some(AnsiColorIdentifier::Blue), collapsed: Some(true), - pinned: None, }, TabGroupTemplate { name: None, color: None, collapsed: None, - pinned: None, }, ], ); diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 274c28b5dc8..31a1990954e 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3869,9 +3869,11 @@ impl Workspace { .map_or(SelectedTabColor::Unset, SelectedTabColor::Color), collapsed: group_template.collapsed.unwrap_or_default(), draggable_state: Default::default(), - // Only restore pinned state when the Pinned Tabs feature is enabled. - pinned: FeatureFlag::PinnedTabs.is_enabled() - && group_template.pinned.unwrap_or_default(), + // Launch configs don't persist pinned state (for tabs or + // groups): a config can open into a window that already + // has unpinned tabs, and restoring a pinned group there + // would break the pinned-region ordering invariant. + pinned: false, }; let group_id = group.id; self.tab_groups.insert(group_id, group);