Save and restore tab groups in launch configurations

[maxmilian]

Fixes #13898

Why

Saving a launch config from a window with grouped tabs and reopening it gives you flat tabs — the groups are gone. impl From<WindowSnapshot> for WindowTemplate reads snapshot.tabs and snapshot.active_tab_index and never looks at snapshot.tab_groups, and WindowTemplate had nowhere to put them.

Worth noting because it narrows the fix: this is a gap in one path, not a missing capability. Session restore already round-trips groups end to end — persistence/sqlite.rs writes them (:1012-1038) and reads them back (:2538-2703), covered by test_sqlite_round_trips_tab_groups. So the data model was already there; only the launch-config projection ignored it.

What changed

Schema (launch_config.rs) — WindowTemplate gains tab_groups: Vec<TabGroupTemplate> and TabTemplate gains group: Option<usize>, an index into that list.

windows:
  - tabs:
      - layout: {cwd: /srv/api}
        group: 0
      - layout: {cwd: /tmp}          # ungrouped, no `group` key
    tab_groups:
      - name: backend
        color: blue

Both fields are skip_serializing_if + default, so existing configs are unaffected — a window with no groups serializes byte-for-byte as before. There is a test asserting neither key appears in that case.

Runtime TabGroupIds are deliberately not serialized. They are UUIDs minted per session, a config can be opened repeatedly, and it can be opened into a workspace that already holds groups — reusing saved ids would collide. The index indirection sidesteps that.

Restore (workspace/view.rs) — open_launch_config_window creates the groups first (fresh ids), then assigns each tab's group_id through the index. This mirrors the session-restore path deliberately, including both feature gates: groups are only created under FeatureFlag::GroupedTabs, and pinned is only honored under FeatureFlag::PinnedTabs.

The part that needed care

The save path filters out tabs that cannot be represented (notebook panes) and renumbers the survivors via num_valid_tabs. So group membership cannot be derived from tab position — the snapshot index and the template index diverge exactly when a tab drops out. Membership is instead carried on each tab's own group_id and resolved against the surviving group list.

Two consequences, both locked by tests:

• A group whose only member failed to convert is dropped, rather than restored as an empty group the user can neither see nor remove.
• When that happens, later groups shift down, and the remaining tabs' group indices shift with them.

test_config_from_snapshot_remaps_groups_around_unsaveable_tabs is the one that guards this, and I checked it actually bites rather than trusting the green run: dropping the "keep only groups that still have a member" filter turns it red with

assertion `left == right` failed: empty group must be dropped
  left: 2
 right: 1

and restoring the filter returns the suite to 14 passed.

An out-of-range group index (hand-edited YAML) leaves that tab ungrouped rather than failing the window — same posture as the session-restore path, which drops a tab's group reference when the group itself did not restore.

Tests

Three new tests in launch_config_tests.rs:

• test_config_from_snapshot_preserves_tab_groups — name/color carried, membership correct, ungrouped tabs stay ungrouped
• test_config_from_snapshot_remaps_groups_around_unsaveable_tabs — the re-indexing case above
• test_config_from_snapshot_omits_tab_groups_when_there_are_none — backward compatibility of the serialized form

The four existing WindowTemplate / TabTemplate literals in crates/integration/src/test/launch_configs.rs were updated for the new fields; no behavior change there.

Known validation limitation

cargo fmt -- --check is clean, cargo check -p warp --all-targets is clean, and cargo test -p warp --lib launch_config reports 14 passed / 0 failed.

That covers the save half. I have not exercised the restore half through a running app on a machine with GroupedTabs enabled — the flag gating and the id-minting on open_launch_config_window are verified by reading against the session-restore path they mirror, not by a manual round trip in the UI. If you would like that confirmed before merge, tell me which flag configuration to run and I will capture it.

[oz-for-oss]

@maxmilian

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

Overview

This PR extends launch-config serialization and restoration so saved launch configs can carry tab group metadata and per-tab group references, with unit coverage for the save-side projection.

Concerns

• Launch config restore accepts split group memberships from hand-edited YAML; the rest of the tab-group code assumes each group is a contiguous run.
• For this user-facing restore behavior, please include screenshots or a screen recording demonstrating grouped launch config restore end to end. The PR description explicitly says the restore half was not exercised in a running app with GroupedTabs enabled.

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

[oz-for-oss]

⚠️ [IMPORTANT] This restores arbitrary group indices directly, so a hand-edited config can assign the same group to non-contiguous tabs. The tab-group code assumes each group is a contiguous run; normalize or drop split memberships before setting group_id to avoid corrupting group operations.

[maxmilian]

Thanks — the first concern was real, and I had missed it. Pushed b9dfb3a.

Split group memberships

Confirmed against tab_bar_slots: it opens a fresh TabBarSlot::Group whenever a run breaks, so group: 0, ungrouped, group: 0 really would draw two containers sharing one id. Restore was taking membership straight from the config, so hand-edited YAML could reach a state no other path can.

Restore now keeps each group to its first contiguous run and returns later stragglers ungrouped. I considered reordering the tabs instead — it also restores the invariant — but silently moving tabs a config explicitly ordered seemed the more surprising of the two; happy to switch if you disagree.

The resolution moved out of open_launch_config_window's closure into resolve_group_memberships, which made it reachable from unit tests (it needed a ViewContext before). Four new tests: contiguous runs preserved, a run split by an ungrouped tab, a run split by another group, and out-of-range indices. I checked they bite rather than trusting the green run — dropping the guard fails two of them:

test_resolve_group_memberships_ungroups_a_split_run
  left:  [Some(0), None, Some(0)]
 right:  [Some(0), None, None]

cargo test -p warp --lib launch_config is 18 passed / 0 failed on the new head.

Visual evidence

I want to give you a straight answer rather than a promise I cannot keep: I do not think an external contributor can produce this recording, because GroupedTabs cannot be turned on in a locally built app. Every path is closed from outside:

• overrides::* is compiled out unless feature = "test-util"; a normal build returns None
• set_user_preference has no callers anywhere in the tree
• the runtime flag menu only exposes RUNTIME_FEATURE_FLAGS, which is [LocalClaudeCodexChildHarnesses]
• FLAG_STATES comes from server-side delivery

So the restore half is unreachable in my build, which is exactly why the PR description said it was unverified rather than quietly claiming otherwise.

What I can offer instead, and would rather do: an integration test in crates/integration/src/test/launch_configs.rs. That harness dispatches the real root_view:open_launch_config action, and integration tests may call FeatureFlag::GroupedTabs.set_enabled(true) (the pattern in session_restoration.rs:224). It would cover the same behavior the recording would show — groups restored with names and colors, membership assigned, the split case collapsed — and unlike a screenshot it keeps covering it.

Tell me which you'd prefer:

1. I add that integration test to this PR, or
2. you have a way to enable GroupedTabs in a local build that I've missed — point me at it and I'll capture the recording, or
3. someone with the flag verifies the restore half internally and I leave the code as is.

Happy with any of the three; I just can't do the recording unaided.