fix(sessions-launch-config): persist tab groups in launch configurations (#13898)#13937
fix(sessions-launch-config): persist tab groups in launch configurations (#13898)#13937lonexreb wants to merge 2 commits into
Conversation
…ons (warpdotdev#13898) 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<WindowSnapshot> 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.
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR persists tab-group metadata in launch configurations and recreates groups when opening saved configs. I did not find security-specific issues, and there is no approved spec context to compare against.
Concerns
- Opening a launch config into an existing window can restore a group as pinned while leaving its member tabs appended after existing unpinned tabs, which breaks the pinned-region ordering invariant.
- This is a user-facing tab restoration behavior change, but the PR description does not include screenshots or a screen recording demonstrating the grouped-tab launch config flow end to end.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| 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() |
There was a problem hiding this comment.
pinned_boundary_index after assigning memberships, or avoid restoring pinned for active-window opens.
…tdev#13898) 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.
|
Addressed in Launch configs deliberately don't persist pinned state for tabs; group pinned persistence was scope creep on my part. Dropped the On visual evidence: the save→YAML→open mapping is covered end-to-end by the three deterministic tests (group metadata, membership indices, round trip, legacy compat); happy to add a screen recording of the grouped-tab flow if a maintainer would like one. /oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR extends launch configuration templates to serialize tab group metadata and per-tab group membership, then recreates those groups when opening a launch config. It also adds regression coverage for grouped-tab snapshot mapping and YAML compatibility.
Concerns
- For this user-facing change, please include screenshots or a screen recording demonstrating grouped tabs being saved to a launch configuration and restored end to end. The PR description does not include visual evidence for the changed behavior.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves #13898.
Problem
Saving a window with grouped tabs via Save and Launch Configuration Template loses the groups — reopening the config restores all tabs flat.
Root cause
As pinpointed in triage:
WindowSnapshot.tab_groups/TabSnapshot.group_idmodel grouped-tab state and normal session restoration persists them, but the launch-config path drops the information at both ends —WindowTemplate/TabTemplatehave no group metadata fields, andopen_launch_config_windowadds each tab individually with no point whereTabGrouprecords are recreated or tabs reattached.Fix
Save path —
WindowTemplategainstab_groups(name / color / collapsed / pinned) andTabTemplategainsgroup, an index into that list.From<WindowSnapshot>maps each group's runtime id to its serialized index and stamps member tabs. RuntimeTabGroupIds are per-session UUIDs and deliberately not serialized — fresh ids are minted on open.Open path —
open_launch_config_windowrecreates the groups before adding tabs (gated onFeatureFlag::GroupedTabs;pinnedadditionally onPinnedTabs), mirroring grouped-tab session restore inconfigure_new_workspace, then reattaches each tab by its template group index. Unresolvable indices (hand-edited YAML, flag off) are dropped, matching restore's dangling-reference handling.Compatibility — both new fields are optional/defaulted in serde: launch configs written before tab groups existed parse unchanged, and configs without groups serialize without the new keys.
Example of the new YAML shape:
Tests
test_config_from_snapshot_maps_tab_groups— two groups + grouped/ungrouped/grouped tabs: templates and per-tab indices map correctly.test_launch_config_yaml_round_trips_tab_groups— serialize → parse preserves groups and membership.test_launch_config_yaml_without_groups_parses— pre-existing YAML without groups still parses (empty groups, no membership).All 14 launch-config tests pass;
cargo fmt+clippy --testsclean.