✨ feat(tab-groups): add tab groups with color-coded inline grouping

[kud]

🎟️ Ticket

Ticket: N/A

📄 Description

Adds tab groups to the tab bar. Users can organise tabs into named, color-coded groups that collapse and expand inline — similar to how Firefox handles tab groups.

What was added:

• Right-click any tab → "Add to New Group…" opens a sheet with a name field, 9 colored swatches, and an eyedropper for custom colors
• Groups appear in the tab bar as a colored header tab showing the group name, a color dot, a chevron, and a member count
• Single click on the group header toggles collapse/expand; double-click opens an edit sheet (rename + recolor in one place)
• Member tabs render a left-edge accent bar in the group's color
• Dragging a group header auto-collapses it first to prevent dropping a group inside itself
• Context menu exposes "Edit Group", "Move to Group" (submenu), and "Remove from Group"
• Group membership and collapse state persist across restarts via window arrangements

Key files:

File	Role
sources/iTermTabGroup.swift	Data model for a tab group
sources/PseudoTerminal+TabGroups.swift	Group management logic on the window controller
sources/iTermCreateTabGroupViewController.swift	Sheet UI for creating/editing a group (name + color picker)
ThirdParty/PSMTabBarControl/source/PSMTabBarCell.h/.m	Group decoration properties on tab cells
ThirdParty/PSMTabBarControl/source/PSMTabBarControl.h/.m	Collapse/expand animation, drag callbacks, deferred single-click
ThirdParty/PSMTabBarControl/source/PSMYosemiteTabStyle.m	Renders group header and color accent
sources/PseudoTerminal.h/.m	Context menu wiring, selection handling, persistence

✅ How to Validate

1. make run
2. Create several tabs, right-click → "Add to New Group…" — sheet should appear with name field and color picker
3. Pick a name and color, click Done — group header should appear in the tab bar
4. Single click the header to collapse; click again to expand
5. Double-click the header → edit sheet should open pre-populated with name and color
6. Drag the group header — it should auto-collapse before the drag begins
7. Right-click a tab outside the group → "Move to Group" → confirm submenu lists the group
8. Drag tabs within the group to reorder — they should stay in the group
9. Save a window arrangement, quit, restore — groups and collapse state should persist

🛠️ Developer Checklist

• Code is readable and maintainable
• Single atomic commit
• Tests included and passing (if applicable)
• Commits follow Conventional Commits

[gnachman]

I get some compiler errors:

/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:77:22: error: cannot find 'contentView' in scope
        let tabBar = contentView.tabBarControl
                     ^~~~~~~~~~~
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:115:17: error: value of optional type '[PTYTab]?' must be unwrapped to refer to member 'contains' of wrapped base type '[PTYTab]'
                allTabs.contains { Int($0.uniqueId) == id }
                ^
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:115:17: note: chain the optional using '?' to access member 'contains' only for non-'nil' base values
                allTabs.contains { Int($0.uniqueId) == id }
                ^
                       ?
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:115:17: note: force-unwrap using '!' to abort execution if the optional value contains 'nil'
                allTabs.contains { Int($0.uniqueId) == id }
                ^
                       !
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:172:64: error: value of optional type '[PTYTab]?' must be unwrapped to refer to member 'first' of wrapped base type '[PTYTab]'
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:172:64: note: chain the optional using '?' to access member 'first' only for non-'nil' base values
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
                                                                      ?
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:172:64: note: force-unwrap using '!' to abort execution if the optional value contains 'nil'
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
                                                                      !
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:178:13: error: cannot find 'contentView' in scope
            contentView.tabView.selectTabViewItem(representative.tabViewItem)
            ^~~~~~~~~~~
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:183:13: error: cannot find 'contentView' in scope
            contentView.tabView.removeTabViewItem(item)
            ^~~~~~~~~~~
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:195:23: error: cannot find 'contentView' in scope
        let tabView = contentView.tabView
                      ^~~~~~~~~~~
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:212:64: error: value of optional type '[PTYTab]?' must be unwrapped to refer to member 'first' of wrapped base type '[PTYTab]'
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:212:64: note: chain the optional using '?' to access member 'first' only for non-'nil' base values
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
                                                                      ?
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:212:64: note: force-unwrap using '!' to abort execution if the optional value contains 'nil'
        let memberTabs = group.memberTabIDs.compactMap { id in allTabs.first { Int($0.uniqueId) == id } }
                                                               ^
                                                                      !
/Users/gnachman/git/iterm2-alt2/sources/PseudoTerminal+TabGroups.swift:215:23: error: cannot find 'contentView' in scope
        let tabView = contentView.tabView
                      ^~~~~~~~~~~

The idea of tab groups in general is a good and thank you for taking this on! A few comments:

• Avoid using associated objects. They are hard to reason about. Claude loves them, but it's usually not hard to find a better place to put your data. Swift/objc interop makes it harder and sometimes you'll need to make a public property in the main @interface.
• Ensure that state restoration works properly with tab groups. Tab unique IDs are not stable across restarts. Also, it looks like stashed tabs in collapsed groups are not seen by the state restoration encoder.
• Expanding and unexpanding to encode state is pretty inefficient. Try to find a way to handle it without touching the UI.
• Storing groupMemberCount rather than using group.memberTabIDs.count risks desynchronization

[kud]

@gnachman

🔧 All addressed in 19ffef9:

• Compiler errors — the contentView errors were a bridging issue: PseudoTerminal+Private.h was referencing NSMutableArray<iTermTabGroup *> but iTermTabGroup is a Swift class, which can't be used as an ObjC generic type parameter in a bridging header. Changed to plain NSMutableArray * to fix it. The optional [PTYTab]? unwraps on tabs() were fixed with ?? [] at all call sites.

• Associated objects — replaced with a proper _mutableTabGroups ivar in PseudoTerminal.m, exposed via PseudoTerminal+Private.h. The AssociatedKeys enum and computed property are gone.

• State restoration — iTermTabGroup now serialises member tabs as their index within the window's tab array rather than uniqueId. On restore, indices are mapped back to the newly-assigned uniqueId values of the restored tabs.

• Expanding/unexpanding for encode — the encode path was already model-only (toDictionary reads memberTabIDs directly). The UI side-effect was in deleteTabGroup, which was calling toggleCollapseGroup to unstash items. Extracted a unstashItems(for:) helper that re-inserts stashed items without triggering the full expand animation or double updateTabGroupDecorations call.

• groupMemberCount desync — the cell already reads from group.memberTabIDs.count live in updateTabGroupDecorations, and every mutation path calls it. Happy to go further and have the cell derive the count from the group model directly if you'd prefer that over the snapshot approach.

[gnachman]

I still get compiler errors and warnings.

[kud]

@gnachman Oh yeah sorry, it's still in draft!

[abrambailey]

Ah, this is just what I was looking for. Hopefully this comes to fruition!

[kud]

@abrambailey Working on it, I've just got a hard time to build the app on my different computers. I'm working on the DX for now to help the make works well on any machine. Once it's done, I will come back to that one.

[namp10010]

@kud ,
Thanks for working on this feature, I am keen to help to move it forward. I will try to get it compile on my machine. Just quickly, is the branch up to date?

[kud]

@namp10010 Yes :]

[namp10010]

@kud ,

I created a small PR to your branch to fix a compile issue on my machine (M1 Pro).

it compiles and runs now. hope that helps

[kud]

Thank you! @namp10010

[kud]

Good news :] — the feature has moved forward significantly since the last update. Here's where things stand:

Resolved from the previous review:

• All compiler errors fixed (bridging header, optional unwraps, contentView scope)
• Associated objects replaced with a proper _mutableTabGroups ivar
• State restoration now serialises by tab index (not uniqueId) and remaps on restore
• Collapsed groups no longer expand/collapse for encode — model-only path
• groupMemberCount removed; cells read group.memberTabIDs.count live

New since last review:

• Double-click on a group header now opens an edit sheet (rename + recolor) instead of just rename — colour picker included
• Dragging a group header auto-collapses it first to prevent self-nesting
• Single-click toggle uses a deferred timer so double-click cancels it (no accidental toggle)
• "Edit Group" replaces the separate "Rename" and "Change Colour" menu items
• 7 additional bugs found and fixed via a pre-ship code review:
  • NSColorPanel dangling-target crash
  • Stash integrity on removeTab from a collapsed group
  • Multiple simultaneous collapse completions (was single block slot, now array)
  • selectNearestTabOutside crash when header index is NSNotFound
  • Timer invalidation missing from dealloc
  • Retain cycle between iTermTabGroup ↔ headerTabViewItem
  • reconcileGroupsAfterReorder was ejecting tabs during intra-group reorders

Still to do before merge:

• Tests
• Screen recording for the PR description

[gnachman]

Missing Tab Style and Orientation Coverage

Tahoe style is completely unhandled. PSMTahoeTabStyle.swift has its own independent drawTabCell(_:highlightAmount:) and desiredWidth(ofTabCell:). The PR only modifies PSMYosemiteTabStyle.m. On Tahoe:

• Group headers render as normal tabs (with close button, interior text, etc.) instead of the colored pill
• closeButtonRect(forTabCell:) doesn't return NSZeroRect for group headers
• desiredWidth(ofTabCell:) doesn't account for group header sizing
• Member tabs get no accent bar

The Yosemite-derived styles (Minimal, Dark, DarkHighContrast, LightHighContrast) all inherit drawTabCell:highlightAmount: and desiredWidthOfTabCell: without overriding them, so those are covered.

Vertical orientation is not handled. The group decoration methods assume horizontal layout:

• drawGroupMemberSidebar: draws a full-width horizontal strip at the cell's bottom edge. In vertical orientation a left-edge vertical strip would make more sense.
• drawGroupHeaderDecorations: uses margins and text centering tuned for horizontal cells (short and wide). Vertical tab cells are tall and narrow — the pill and text will likely look wrong.
• The group header width calculation in desiredWidthOfTabCell: is only meaningful for horizontal layout; vertical tabs use height.

Light mode has hardcoded dark-background assumptions. The default neon style hardcodes [NSColor whiteColor] for group header text. The active-state border (colorWithWhite:1.0 alpha:0.4) and hover overlay (colorWithWhite:0.0 alpha:0.25) also don't adapt. The flat style does luminance-adaptive text, which is good — the neon path should do the same. The PSMGroupPillButton multi-select pill has the same problem (white text/borders hardcoded).

Memory Management (MRC)

PSMTabBarControl.m is MRC ([super dealloc], explicit retain/release throughout). The new code has leaks:

• Missing releases in -dealloc: _fadingInCells, _collapsingCells, _collapseCompletions, _multiSelectedTabViewItems, and _groupSelectionButton are allocated but never released.
• PSMGroupPillButton -updateTrackingAreas: The old _trackingArea is removed from the view but not released before reassignment.
• PSMGroupPillButton has no -dealloc: _trackingArea is never released on teardown.
• -groupGradientForColor: leaks: [[NSGradient alloc] initWithColors:...] is returned without autorelease.

Architectural Fragility

The PR adds 16+ isKindOfClass:[PTYTab class] guards scattered across PseudoTerminal.m (in windowWillResize:, tabViewDidChangeNumberOfTabViewItems:, fitTabsToWindow, allSessions, refreshTerminal:, tabs, sizeOfLargestTabWithExclusion:, minWidth, buildSessionSubmenu:, and every delegate method). Every existing and future code path that iterates tabViewItems must remember this check or it will crash on group header items.

A centralizing approach would reduce risk — e.g., a -[PseudoTerminal realTabViewItems] filtered accessor, or a category on NSTabViewItem that provides -isGroupHeader.

NSColorPanel Dangling Target

iTermCreateTabGroupViewController sets itself as the shared NSColorPanel's target/action but only cleans up via dismissColorPanel() called from button actions. If the sheet is dismissed externally (parent window closes, escape key), deinit doesn't clean up and the color panel retains a dangling target. Should add:

deinit {
    dismissColorPanel()
}

Naming / Rendering Inconsistencies

• drawGroupMemberSidebar: draws a horizontal top/bottom strip, not a sidebar. The PR description says "left-edge accent bar." Either the rendering or the name should change.
• updateAnimated calls [self update:NO] — the NO argument means no animation. The name suggests the opposite.

Dead Code

• NSCoding on iTermTabGroup: encode(with:) and init?(coder:) are implemented, but arrangements use the toDictionary/init?(dictionary:) path. The NSCoding conformance appears unused.
• _cellSlideOffset property on PSMTabBarCell: declared, synthesized, initialized, never read.

[davidalee]

Hey all. What's needed to help move this PR forward? I've been looking for exactly this feature and am encouraged to see so much progress already made on it.

[gnachman]

Hi, and thanks for putting this together. There's a lot of good work here — the sheet UI, the color picker with the eyedropper, the gradient drawing, and the overall feature scope are all well-conceived. Before either of us invests more time in revisions, I'd like to step back and talk about the underlying design, because I think we need to change the foundation before the detail-level feedback is worth addressing.

I pulled the branch down and tried it, and hit a bunch of issues:

• Cmd-N keyboard shortcuts don't reach tabs inside a group. When the group is collapsed the members are gone from the tab view entirely; when it's expanded, the header occupies an index and shifts the numbering. Because only one group can be expanded at a time, most grouped tabs are keyboard-unreachable most of the time.
• In the Tahoe tab style, clicking a group header produces a weird animation. Tahoe renders the header as a plain tab and its hover/press transitions still run even though the click toggles collapse.
• Group headers look like tabs with something drawn inside them. The tab chrome draws first and the decoration is painted on top, so the hierarchy doesn't read.
• Tab group colors don't seem to work at all with Tahoe style tabs.

Aside from that, there is a real architectural issue: the group header is a real NSTabViewItem inserted into NSTabView.tabViewItems, with identifier = iTermTabGroup. That creates a bunch of issues:

• Every iterator over tabViewItems in the codebase now has to distinguish PTYTabs from group headers. The PR adds about a dozen isKindOfClass: guards and it's still not complete; every future iterator is a latent bug.
• Collapse has to be implemented by removing tab view items and stashing them, which fights NSTabView's model and is what breaks keyboard shortcuts, accessibility, and arrangement encoding (hence the expand-encode-recollapse dance).
• Ordering invariants (members must be contiguous with the header) have to be manually enforced after every reorder.
• Group rendering lives in one tab style's draw path, so Tahoe doesn't know groups exist.
• The drag assistant has no notion of a group, so the workaround is to collapse synchronously inside mouseDragged:, which races with layout and produces visual jumps.

It would be simpler to keep NSTabView.tabViewItems the same, with only PTYTabs. Group headers can be synthetic cells rendered by PSMTabBarControl, interleaved between real tab cells based on a model the tab bar queries through a delegate callback along the lines of -orderedGroupsForTabBarControl: and -groupForTab:. iTermTabGroup holds [uniqueId] (no NSTabViewItem, which can create a retain cycle). The expanded/collapsed state of a group is reflected by the cells that PSMTabBarControl sees internally, while NSTabBarControl does not need to know about them.

Under that model, a lot complexity disappears: the isKindOfClass: guards, stashedTabViewItems, enforceContiguity, reconcileGroupsAfterReorder, moveGroupAfterLastGroup, autoManageGroupCollapseForTab, the groupCollapseManaging reentrancy guard, the animation state machine with markNextInsertionsAsAnimated:, and the expand-encode-recollapse in
populateArrangementWith: all go away. Keyboard shortcuts work because the tabs are always tabs. Tahoe works because group rendering isn't layered onto a tab cell.

A few things carry forward pretty much as-is: iTermCreateTabGroupViewController, the gradient drawing in drawGroupHeaderDecorations:, the sidebar drawing for members, the menu wiring, and the basic shape of iTermTabGroup (name / color / memberTabIDs / isCollapsed).

Two smaller points I'd separate out, independent of the core:

• The "only one group expanded at a time" behavior is a product choice, not a structural one. Firefox and Chrome don't do this. I'd prefer multiple groups can be expanded; autoManageGroupCollapseForTab and its reentrancy guard are symptoms of that coupling.
• Cmd-click multi-select plus the floating "Group (N)" pill is really a separate feature. I'd split it into a follow-up PR once the core model is in, rather than reviewing both at once.

How would you feel about taking another pass along these lines? Happy to discuss the delegate shape, the collapse-as-render-filter approach, or anything else before you start. I want to make sure we land something we're both comfortable maintaining — the feature itself is worth having, I just want the foundation to be right.

I think it would be less painful to do as several smaller PRs, since this is a pretty big project.