Skip to content

TabManager decomposition: Wave 3+4 sub-models (Browser/Notifications/Sidebar/Workspaces/Panes/FocusHistory) + settings convergence; TabManager becomes a composition root - #5921

Merged
azooz2003-bit merged 43 commits into
mainfrom
feat-tabmanager-models
Jun 13, 2026

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jun 11, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked diff on purpose: this PR targets feat-sidebar-git (#5907), the CmuxSidebarGit extraction it builds on. Do not retarget to main and do not rebase onto the 3c branch.

Wave-3 + Wave-4 drain of Sources/TabManager.swift per the TabManager decomposition blueprint. TabManager at this PR's base: 8,366 lines. Now: 6,067 lines (original god object: 11,841). TabManager is now a per-window composition root: it owns the window (NSWindow, chrome/title), presents NSAlerts, calls session persistence, keeps the DEBUG UI-test scaffolding, and composes the per-domain sub-models below, hosting their seams and forwarding its legacy API one-to-one (zero call-site churn outside TabManager.swift in the Wave-4 tranches).

What moved where

Wave 3 (landed earlier in this PR):

  1. CmuxSettings convergence: the FooSettings namespace-enum UserDefaults keys converge onto catalog DefaultsKey<Value> entries through a synchronous SettingsReading/SettingsWriting seam (UserDefaultsSettingsClient). Key strings byte-identical (wire format).
  2. CmuxBrowser: @MainActor @Observable BrowserModel + bounded RecentlyClosedBrowserStack behind BrowserManaging.
  3. CmuxNotifications: per-panel NotificationDismissalModel behind NotificationDismissing with synchronous NotificationDismissalHosting.
  4. CmuxSidebar: SidebarMultiSelectionModel + typed multi-selection events + pure detail-visibility resolvers.

Wave 4:

  1. CmuxWorkspaceNavigation: @MainActor @Observable FocusHistoryModel owns the focus-history back/forward stack behind FocusHistoryNavigating, hosted through the synchronous FocusHistoryHosting seam. 15 package tests.
  2. CmuxPanes: SplitDirection/ResizeDirection values + pure split-geometry planning (equalizeDividerPlan, resizeDividerAdjustment) with a stateless PaneLayoutService applying plans to BonsplitController (plan-vs-apply). 10 package tests.
  3. CmuxWorkspaces — WorkspacesModel storage move (tranche 4a): TabManager's @Published tabs / workspaceGroups / selectedTabId stored state moved into @MainActor @Observable WorkspacesModel<Tab>, generic over the WorkspaceTabRepresenting seam (id/groupId/isPinned/currentDirectory) that Workspace satisfies. The legacy @Published property-observer semantics are preserved exactly through the synchronous WorkspacesHosting hooks: objectWillChange plus the legacy Combine bridge publishers (tabsPublisher/selectedTabIdPublisher, CurrentValueSubject = new-value-at-willSet + replay-on-subscribe, the Published.Publisher contract) fire in willSet; the selection didSet side-effect chain runs verbatim from the didSet hook; hooks fire on every assignment including equal values (@Published never compared). The eleven remaining $tabs/$selectedTabId Combine subscribers (AppDelegate, CmuxConfig, ContentView, BackgroundWorkspacePrimeCoordinator, MobileWorkspaceListObserver, DEBUG scaffolding) moved to the bridge publishers after per-site analysis (dropFirst/throttle/switchToLatest/replay all subject-compatible; sinks that read tabManager.tabs during emission still observe the pre-change value).
    Boundary: tabs/groups/selection is the window's workspace-list domain state; the bridge publishers are one documented legacy seam, deleted when those subscribers modernize to @observable observation.
  4. CmuxWorkspaces — group + reorder flows (tranche 4b): the ~1,500-line group/reorder cluster left TabManager.
    • WorkspacesModel owns the invariant/traversal layer for the data it stores: top-level row derivation, pin-tier clamps, anchor-first ordering, group-run/contiguity normalization, group-order sync, membership assignment, anchor-close dissolve, selection auto-expand.
    • WorkspaceReorderCoordinator: move-to-top, single/before-after/batch reorders (owns WorkspaceReorderPlanner), sidebar drag planning, drag-inferred group membership, pin toggles/batch pinning.
    • WorkspaceGroupCoordinator: group creation (fresh anchor + child adoption + stable creation placement), createWorkspaceInGroup, member add/remove, ungroup/delete, rename, collapse/pin/color/icon/anchor, group-slot moves, auto-naming via host-provided localized format.
    • Seams: WorkspaceOrderHosting (the legacy order-change NotificationCenter + event-bus publication stays app-side) and WorkspaceGroupHosting (workspace creation/teardown on the Workspace god, selection entry point, sidebar multi-selection sync, String(localized:) format, settings read, RenderableSystemSymbol normalization).
    • 20 package tests: hook parity, tier ordering, batch-reorder validation, batch-unpin order parity, group creation/dissolve/anchor invariants, collapse focus/selection stripping.
      Boundary justification (per package): CmuxWorkspaces owns one full domain — the window's workspace list, ordering, grouping, and selection — as model + coordinators; it depends only on CmuxSettings (for the WorkspaceGroupNewPlacement value type). It does not absorb pane or navigation state (separate domains, separate packages), and nothing in it imports AppKit.

Verification

Per tranche: package swift build + swift test green (CmuxWorkspaces: 20 tests); swift_file_length_budget.py and lint-ios-package-conventions.sh exit 0; tagged cloud app build green (tabmod). Adversarial machine-diff per tranche against the pre-deletion file state: tranche 4a's moved selection willSet/didSet bodies appear as unchanged context lines in the git diff itself; tranche 4b compared all 57 moved method bodies after mechanical normalization (model./host. prefixes, Tab type substitution) — 50 byte-identical, 7 differing only at the intended host-seam inversions (workspace creation/close/select/sidebar-sync/localized-format/settings/icon-normalization hooks). Defaults keys/codecs untouched.

What stays in TabManager, and why (composition-root residue)

  • Window ownership + chrome: NSWindow weak ref, title/backdrop updates, tab-bar inset sync (blueprint-sanctioned residue).
  • NSAlert presentation + the synchronous close-confirmation flow: converting confirmClose to the planned async CloseConfirming protocol requires migrating the six Bool-returning close entry points (closeWorkspaceWithConfirmation, tab-close gestures, etc.) and their UI callers off the nested-runloop runModal contract — a behavior change (gesture handlers would return before the user decides), not a faithful lift. It needs its own observer-analysis commit; doing it by keeping a parallel sync path would create two sources of truth. The group-bookkeeping half of close (dissolveGroupsAnchoredBy) already lives in WorkspacesModel.
  • Workspace creation/close/detach choreography (addWorkspace, closeWorkspace, detachWorkspace/attachWorkspace, TabManager+DetachedWorkspace.swift): these sequence Workspace.init, panel/bonsplit access, AppDelegate, Sentry/UITestRecorder, and service wiring. Until the Workspace god decomposes (separate PR fleet), lifting them would be pure host-hook ping-pong that has to be redone when Workspace changes shape. The pure parts (placement resolution, creation snapshots, insert-index clamps) are already value-typed.
  • Session persistence extension: sessionSnapshot/restoreSessionSnapshot/sessionAutosaveFingerprint are projections over deep Workspace state; the encode/decode + file write live in SessionPersistence.swift driven by AppDelegate (app lifecycle). The SessionSnapshotRepository/SessionSnapshotStoring extraction therefore lands with the AppDelegate/Workspace fleet, where the store and its callers both live.
  • DEBUG UI-test scaffolding (~1,000 lines) and VsyncIOSurfaceTimelineState (#if DEBUG, sanctioned).
  • Domain forwarders: sidebar git/PR, browser zoom/devtools, find/search, split ops — one-line forwards into the owned sub-models/services.

Remaining (stacked slices, not this PR)

  • PaneTreeModel (CmuxPanes): the Bonsplit pane-tree state lives in Workspace/BonsplitController, not TabManager — the TabManager-side pane work (geometry planning + PaneLayoutService apply seam) is complete in this PR; the tree model itself is blocked on the Workspace god fleet.
  • Async CloseConfirming + close-coordinator (see residue note above).
  • SessionSnapshotRepository behind SessionSnapshotStoring (AppDelegate fleet).
  • UI packages (CmuxWorkspacesUI/CmuxPanesUI) once the corresponding views leave ContentView.

Hard rules followed: faithful lift first; one source of truth (legacy types deleted, call sites converged); no free functions, no namespace enums in packages; String(localized:) app-side only.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Recently closed browser panels can now be restored.
    • Workspace grouping in the sidebar with collapsible groups and customizable anchors.
    • Improved focus history navigation with back/forward menu support.
    • Split panel equalization and keyboard resizing improvements.
    • Enhanced workspace color palette customization.
    • Refined notification dismissal handling for better unread state management.
  • Improvements

    • Better sidebar detail visibility controls and workspace organization.

azooz2003-bit and others added 21 commits June 10, 2026 12:13
Extract the window RPC domain (window.list/current/focus/create/close/displays/
display) out of TerminalController into a new @mainactor @observable
ControlCommandCoordinator in CmuxControlSocket, behind the read-only
ControlCommandContext seam (app target conforms; package never imports the app
target). The coordinator owns the kind:N ControlHandleRegistry (RPC selection
state per the decomposition plan); TerminalController delegates its ensureRef/
resolveRef/removeRef to it so refs stay consistent across moved and not-yet-
moved domains.

Faithful lift: the window bodies build ControlCallResult/JSONValue payloads
whose Foundation object is identical to the legacy [String: Any] dictionaries,
so the encoded wire bytes match. Dispatch runs on the main actor inside the
existing withSocketCommandPolicy scope, so the per-read v2MainSync hops the
legacy bodies used become plain in-isolation calls and disappear. window.current
preserves both distinct legacy errors (unavailable vs not_found) via
ControlCurrentWindowResolution.

TerminalController.swift 22074 -> 21921 (budget ratcheted). 17 new package
tests (128 total) drive every window method through a fake context, asserting
byte-identical payloads, ref minting, routing-selector parsing, and the two
window.current failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restructure the seam into a per-domain protocol umbrella (ControlCommandContext:
ControlWindowContext, ...) so each domain can be built in its own files, and
port the shared TerminalControllerV2ParamParsingSupport pure helpers + ref
minting (workspaceRefs/tabRef/workspacePaneAndSurfaceRefs) into the coordinator
as JSONValue twins. Foundation for moving the remaining RPC domains.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the app-focus (app.focus_override.set, app.simulate_active), main-actor feed
(feed.jump, feed.list), and notification (create/create_for_surface/create_for_target/
list/dismiss/mark_read/open/jump_to_unread/clear) domains into the coordinator
behind their per-domain seams (ControlAppFocusContext/ControlFeedContext/
ControlNotificationContext), composed into the ControlCommandContext umbrella. The
core handle(_:) now chains per-domain handleX dispatchers.

Worker-lane methods stay app-side: feed.push/permission.reply/question.reply/
exit_plan.reply, and notification.create_for_caller (its own resolver).

Faithfulness: byte-identical payloads/errors (live socket sweep on ctl3c1 confirms
every result + error shape). Notification localized strings are resolved in the app
conformance (app bundle) and passed through ControlNotificationStrings, because
String(localized:) inside the package would bind to the package bundle and silently
drop the Japanese translations — a wire change for non-English locales.

Test fakes get benign defaults for non-window seams via ControlCommandContextTestStubs
so each fake implements only the domain it exercises (128 package tests still green).
TerminalController.swift 21952 -> 21522 (budget ratcheted).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move workspace.group.* (17 methods), pane.* (9 methods), and mobile.host.status/
mobile.workspace.list/mobile.terminal.* (+terminal.* aliases) into the coordinator
behind ControlWorkspaceGroupContext/ControlPaneContext/ControlMobileHostContext,
composed into the umbrella; core handle(_:) chains the new handlers.

Workspace Groups + Pane are full lifts (bodies deleted, payloads rebuilt as JSONValue,
localized group strings routed app-side via ControlWorkspaceGroupStrings). Mobile Host
is a faithful pass-through: its 8 bodies are SHARED with the mobile data-plane
(mobileHostHandleRPC) so they stay in TerminalController (relaxed private->internal);
the coordinator decouples via the seam and the conformance bridges V2CallResult.

Pane folds the resize support helpers (kept app-side: Bonsplit-coupled); v2SurfaceMove
relaxed private->internal for pane.join forwarding. Live socket sweep on ctl3c1 confirms
faithful payloads + errors (group create/list, pane list/create split, mobile host status).

TerminalController.swift 21522 -> 20296. 128 package tests green. Two new Pane files
>500 lines get budget entries.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regression found by the no-regression code review of the moved domains: the
ported int() did Int(value) on a JSON double, which TRAPS (crashes) on overflow/
NaN — reachable via pane.resize amount or workspace.group.move to_index with e.g.
1e30 — whereas legacy v2Int went through (params[key] as? NSNumber).intValue,
which clamps. Also int()/double() didn't coerce a JSON boolean to a number the
way the legacy as? NSNumber path did.

Both now route doubles/bools through NSNumber.intValue/.doubleValue, matching
v2Int/v2Double exactly (truncate-toward-zero, clamp out-of-range, bool->1/0).
5 regression tests cover truncation, overflow/NaN no-trap, and bool coercion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Workspace (21 methods incl. remote.*) and Surface (25 methods + debug.terminals)
move into ControlCommandCoordinator behind ControlWorkspaceContext/
ControlSurfaceContext. ~2640 lines deleted from TerminalController.swift
(20296 -> ~17650). Worker-lane workspace.remote.pty_* stay app-side.

Two shared bodies the drafting agents wrongly flagged for deletion were RESTORED
(internal/private): v2WorkspaceCreate(params:tabManager:) is still driven by the
mobile data-plane v2MobileWorkspaceCreate; workspaceCloseProtectedMessage() by the
v1 close path. surface.move + debug.terminals forward to the still-shared
v2SurfaceMove/v2DebugTerminals (relaxed internal), like pane.join. Relaxed to
internal for the conformances: tabManager, socketFastPathState, orderedPanels,
readTerminalTextRawSnapshot.

Live socket sweep on ctl3c1 confirms faithful payloads + errors across both
domains (workspace list/current/create/rename/select/next/close, surface list/
current/health/send_text+read_text round-trip/resume.get, error shapes).
133 package tests green.

KNOWN FOLLOW-UPS: workspace.create logic is duplicated (conformance reimplements +
restored shared body) — dedupe by forwarding; the 2 Workspace files >500 lines
(budget entries added) should be split; adversarial code-review verification of
these 2 domains still pending (8 prior domains verified clean).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coordinator/ had grown to 105 files. Move each domain's coordinator extension,
seam protocol, and value/resolution/snapshot types into a per-domain subfolder
(Window/AppFocus/Feed/Notification/Pane/Surface/Workspace/WorkspaceGroup/
MobileHost). The 4 shared core files stay at the Coordinator/ root:
ControlCommandContext (umbrella), ControlCommandCoordinator (core dispatch +
handle registry), ControlCommandCoordinator+Params (shared param/ref helpers),
ControlRoutingSelectors. SwiftPM globs sources recursively, so this is purely
organizational — no Package.swift/import changes. Budget paths updated for the
moved Pane/Workspace coordinator files; TC.swift budget corrected to 17680
(the two restored shared bodies grew it after the last bump).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Workspace lift had reimplemented workspace.create logic in the conformance
while the original v2WorkspaceCreate(params:tabManager:) was restored for the
mobile data-plane caller -- two copies that could diverge. Replace the typed
reimplementation with a passthrough that forwards to the single shared
v2WorkspaceCreate (relaxed private->internal) and bridges its Foundation result,
exactly like surface.move/debug.terminals/mobile. Deletes the now-unused
ControlWorkspaceCreateInputs/ControlWorkspaceCreateResolution. One source of
truth, byte-identical wire output.

Comprehensive socket sweep on ctl3c1 (all 10 domains, 38 ok + 13 expected
validation errors, zero crashes) confirms no regression: workspace.create happy
path + its cwd/layout validation errors preserved; pane.resize amount=1e30 now
clamps (invalid_state) instead of trapping (the int/double NSNumber fix). 133
package tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…view

Surface (4): surface.clear_history with a present-but-invalid surface_id silently
cleared the FOCUSED surface instead of returning not_found (wrong-target side
effect; hasSurfaceIDParam now crosses the seam like send_text); surface.split
with an unrecognized direction returned unavailable instead of invalid_params
'Missing or invalid direction (left|right|up|down)' (coordinator now validates
the parseSplitDirection token set + a drift-safe .invalidDirection case);
surface.split error precedence restored (direction -> agent-session -> divider;
the agent-session token check moved before divider parsing); surface.resume.*
explicit target restored to surface_id ?? tab_id ONLY (terminal_id is a general
routing alias but was never a resume target) and the window branch now requires
a RESOLVABLE window_id like origin.

Workspace (4): select/close/rename get the routing precheck so unresolvable
routing returns unavailable before param validation (legacy TabManager-first
order, matching reorder); workspace.current with a stale selectedTabId returns
.ok with workspace:null again instead of not_found. Dead code removed
(JSONValue.isControlNull, surfaceIDForInput).

All confirmed by live socket sweep on the rebuilt ctl3c1 (each previously-wrong
response now byte-matches origin). 133 package tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…3c-1

# Conflicts:
#	.github/swift-file-length-budget.tsv
…ce conformance

The tests-build-and-lag job failed solely on the Swift WARNING budget: the
Workspace conformance's controlWorkspaceRemotePTYAttachEnd declared
'sessionID sessionID: String' (extraneous duplicate). Behavior identical; the
job's build and lag phases were green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s (drafts integrated)

Five domains drafted by the orchestrator's agents (handed off), repaired
(browserNavContext accessor, allocateElementRef state call, v1 handlers
unhooked from the v2 chain), wired into the umbrella + dispatch, with test
stubs completed. 140 package tests green. App-side surgery follows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e coordinator

TerminalController.swift 18,033 -> 10,748 (-7,285). The five remaining domains
now dispatch through ControlCommandCoordinator: System (identify/tree/auth.login/
session.restore/settings.open/feedback.open/extension snapshot/workspace.action/
tab.action/drag_to_split/split_off), Project (project.* + markdown.open +
file.open), Debug (39 debug.* methods), Sidebar v1 (44 verbs via a new
handleSidebarV1 hook ahead of the v1 switch), Browser panel v1 (8 verbs), and
all 89 main-actor browser.* methods.

Browser per-surface state moved off the controller: ControlBrowserAutomationState
(package) + dialog responders keyed by dialogID app-side (the Sendable
V2BrowserPendingDialog redesign); cleanupSurfaceState purges the new state,
faithfully mirroring the legacy eviction. Two conformances the drafts never
included (ControlBrowserContext, ControlBrowserPanelContext) were authored
byte-faithfully from the legacy bodies. Shared bodies kept + relaxed to internal
(v2Identify, v2WorkspaceAction, v2SurfaceSplitOff, v2FileOpen, the 18 v1-debug
impls, the JS pump, the worker-lane browser.download.wait cluster).

Deliberate deltas (documented): controlFeedbackOpen drops the deprecated
.activateIgnoringOtherApps activation option (documented no-op on macOS 14+,
the project floor; keeping it fails the new-file warning budget); a sequence id
bridges Int64->Int (lossless on arm64).

Gates: package swift build + 140/140 tests; tagged app build BUILD SUCCEEDED;
live socket sweep green across all domains (system.tree, auth.login parity,
browser.open_split -> get.title returns the real page title end-to-end,
project validation errors, v1 set-status via the new hook, debug.terminals,
plus regression of the ten prior domains); zero new warnings; both budgets pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lift the embedded sidebar git/PR subsystem out of TabManager into the new
CmuxSidebarGit services package:

- @mainactor SidebarGitMetadataService (behind SidebarGitMetadataServing):
  local probe state machine, retry walk over the preserved offsets
  [0, 0.5, 1.5, 3, 6, 10]s, per-directory snapshot dedupe behind the
  injected process-wide WorkspaceGitMetadataProbeLimiter, RecursivePathWatcher
  wiring, and the 5-minute fallback re-poll.
- @mainactor PullRequestPollService (behind PullRequestProbing): PR poll
  deadlines (max(0.25, ...) floor, 10s/60s +-10% jitter, 15-min terminal
  sweeps, batch limit 3), repo cache pruning, transient-failure staleness,
  command-hint reconciliation.
- TabManager keeps thin forwarders plus a SidebarGitHosting conformance
  (synchronous read/write seam so apply-turn interleavings stay identical);
  gh/git argv, GitHub request shapes, cache keys, and badge transitions are
  unchanged byte-for-byte.
- GitPollClock moves into the package with the code that sleeps on it; the
  probe limiter singleton becomes constructor injection (process-wide
  instance at the composition root).

TabManager.swift 9992 -> 8366 lines (-1626); package +~2900 lines incl. 16
behavior tests (virtual-clock probe scheduling, projection, poll-deadline
floor, command hints, limiter). Package swift build + swift test green;
full app build green; pbxproj normalized.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"main" is a skip-lookup branch, so the poll-deadline floor test cleared the
badge without ever starting a refresh and then waited forever for a poll
timer that was never armed. Probe/PR suites now use feature/x where a real
refresh is required; the dedicated skip-lookup test keeps main. 20 tests in
4 suites green. Net +5/-4 lines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The System+Project adversarial review found file.open had been reimplemented in
the coordinator/conformance while the original v2FileOpen stayed behind (it is
driven directly by FilePreviewReviewFeedbackTests and MarkdownPanelTests) - two
copies that could drift, and a stale dispatcher comment claiming forwarding.
file.open now forwards to the single shared body and bridges its result, like
workspace.create; the reimplementation and its now-unused
ControlFileOpenResolution/ControlFileOpenSurface types are deleted.

Review verdicts so far: System+Project all faithful (this was the only finding,
not a behavior bug); Debug (39 verbs) + Sidebar v1 (44) + Browser-panel v1 (8)
all faithful, zero divergences, #if DEBUG gating verified end-to-end.

140 package tests green; app build green; live probe of file.open through the
shared body (happy path + both error shapes) byte-faithful.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h residue

The Browser adversarial review (87/89 faithful) found its only two divergences
share one root cause: focus_mode.set and zoom.set validated mode/direction
BEFORE the TabManager/handle guards (legacy order: guards first). The shared
browserFocusedAction helper gains a post-guard validate step; both methods'
validation moves there. Live-verified: double-fault now returns
unavailable/'TabManager not available', single-fault the mode/direction error.

Residue: socketFastPathState drops its 'nonisolated' (after the cutover its
only callers are the @mainactor sidebar/surface conformances; the worker-thread
fast path retired with the legacy dispatcher). ServerEventTarget's @unchecked
Sendable and the V2CallResult/V2SocketRequest twins stay deliberately: they
serve the worker-lane and kept-shared bodies, which move in a later wave (the
target itself dissolves with TerminalControlComposition in Wave 5).

Verification totals for the five stacked domains: 143 methods/verbs reviewed
per-method vs the pre-deletion originals; 141 faithful as-lifted, 2 fixed here.
140 package tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…convergence surface

Adds the synchronous typed-settings seam the TabManager Wave-3 drain reads
through: SettingsReading/SettingsWriting protocols plus the
UserDefaultsSettingsClient conformer (same DefaultsKey + SettingCodable
primitives as the actor store, no parallel mechanism). Adds the two missing
TabManager keys as a workspaceGroups catalog section
(anchorCloseSuppressed, newWorkspacePlacement with the tolerant
WorkspaceGroupNewPlacement value enum) and folds the legacy indicator-style
string mapping (rail/border/wash/lift/typography/washRail/blueWashColorRail)
into WorkspaceIndicatorStyle's SettingCodable decode so converged reads keep
resolving values written by earlier builds.

Tests: 83 package tests green (19 new: client round-trip/reset/absence,
legacy indicator decode matrix, tolerant group-placement parse).
Net: +371 lines (package only; app cutover follows).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 13, 2026 7:45pm
cmux-staging Building Building Preview, Comment Jun 13, 2026 7:45pm

@coderabbitai

coderabbitai Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

New packages extract browser history, notification dismissal, pane geometry, settings, sidebar, workspace navigation, and workspaces logic into reusable modules. The app target and tests are rewired to those package APIs, new catalog-backed settings are introduced, and the Xcode project is updated for the added packages and source files.

Changes

Package extraction and app adoption

Layer / File(s) Summary
Browser, notifications, and panes foundations
Packages/CmuxBrowser/*, Packages/CmuxNotifications/*, Packages/CmuxPanes/*
BrowserModel, RecentlyClosedBrowserStack, notification dismissal types, and pane split planning/result types are added with their package manifests and tests.
Settings and sidebar shared primitives
Packages/CmuxSettings/*, Packages/CmuxSidebar/*
Typed settings access, new catalog sections, sidebar visibility structs, and sidebar multi-selection event/model types are added with package tests.
Workspace navigation focus history
Packages/CmuxWorkspaceNavigation/*, Sources/FocusHistory.swift
Focus-history host/model/menu types move into CmuxWorkspaceNavigation, and the app-side file drops the relocated value types.
Workspaces domain model, coordinators, and reorder engine
Packages/CmuxWorkspaces/*
Workspace tab/group contracts, the observable model, invariants, reorder planner/coordinators, and package tests are added in CmuxWorkspaces.
App integration with extracted modules
Sources/AppDelegate*, Sources/CmuxConfig.swift, Sources/CommandPalette/*, Sources/ContentView.swift, Sources/Feed/*, Sources/KeyboardShortcutSettingsFileStore*, Sources/Mobile/*, Sources/Sidebar/*, Sources/TerminalController*, Sources/TerminalNotificationStore.swift, Sources/VerticalTabsSidebar+WorkspaceGroups.swift, Sources/Workspace.swift, Sources/WorkspaceIndicatorStyle+Display.swift, Sources/WorkspacePlacement+Resolution.swift, Sources/WorkspaceTabColor*, Sources/cmuxApp*, Sources/GhosttyTerminalView.swift, Sources/AppleScriptSupport.swift
The app target switches to the new modules, publisher-based observation, typed multi-selection events, catalog-backed settings reads, workspace placement resolution, indicator style display, and workspace tab color handling.
Project and dependency wiring updates
cmux.xcodeproj/project.pbxproj
The project adds the new local package references, product dependencies, framework links, and source entries for the extracted files.
App test migration to package APIs and settings catalog
cmuxTests/*
Tests are updated to use the new package types, disambiguate duplicate symbols, and assert through SettingCatalog/UserDefaultsSettingsClient instead of legacy helpers.

Sequence Diagram(s)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-tabmanager-models

@greptile-apps

greptile-apps Bot commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Wave 3+4 TabManager decomposition extracting six sub-models (BrowserModel, NotificationDismissalModel, SidebarMultiSelectionModel, FocusHistoryModel, PaneLayoutService, and a synchronous UserDefaultsSettingsClient) into dedicated SwiftPM packages, while converging legacy namespace-enum UserDefaults keys onto the typed SettingCatalog. TabManager becomes a pure composition root that attaches each model to itself as its host seam.

  • New sub-models (@MainActor @Observable) replace ~1 500 lines of inline TabManager state; all navigation loops, suppression depth guards, and invalidation logic are faithful one-for-one lifts — no behavioral change intended or observed.
  • Settings convergence: SidebarBranchLayoutSettings, SidebarWorkspaceDetailSettings, SidebarPullRequestClickabilitySettings, WorkspaceAutoReorderSettings, and the sidebar indicator-style resolver are deleted and replaced by catalog reads through UserDefaultsSettingsClient; legacy indicator strings ("rail", "wash", …) map through WorkspaceIndicatorStyle.decodeFromUserDefaults exactly as before.
  • New CmuxPanes package extracts split-equalize and resize-divider geometry into pure, tested ExternalTreeNode extensions applied by a stateless PaneLayoutService.

Confidence Score: 5/5

This PR is safe to merge; all logic lifts are faithful and the new sub-models correctly attach to their host seams with no new mutable shared state introduced.

The decomposition is structurally sound: sub-models are @mainactor @observable, weak host references prevent retain cycles, suppression depth is guarded against underflow, and the settings key strings are treated as wire-format constants. Navigation loop invariants in FocusHistoryModel and the atomic latch read-then-clear in NotificationDismissalModel both match the original TabManager behavior. The one edge case flagged (overflow removing a just-inserted entry in the preservingForwardBranch path when historyIndex is −1 with non-empty history) is a pre-existing condition carried over faithfully from the original code and is unreachable in normal operation.

No files require special attention; the largest new file (FocusHistoryModel.swift at 433 lines) is well-decomposed and covered by dedicated tests.

Important Files Changed

Filename Overview
Sources/TabManager.swift ~1500 lines removed; TabManager now composes FocusHistoryModel, NotificationDismissalModel, BrowserModel, SidebarMultiSelectionModel, and PaneLayoutService as sub-models; focusHistoryRevision @published preserved for downstream observers; attach calls present in init.
Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryModel.swift 433-line faithful lift of focus-history sub-model from TabManager; @mainactor @observable; navigation loops, invalidation, and menu snapshot logic all correct. Suppression depth guard against underflow is present.
Packages/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalModel.swift Per-window notification dismissal sub-model; correctly implements atomic read-then-clear of suppressFocusFlash in dismissFocusedPanelNotificationIfActive while preserving the latch for the explicit-focus path.
Sources/TabManager+NotificationDismissalHosting.swift New hosting conformance; all store accesses forward through AppDelegate.shared?.notificationStore (matching legacy pattern); no new global state.
Sources/TabManager+FocusHistoryHosting.swift New hosting conformance for focus history; workspace/panel existence checks and selection mutations mirror legacy optional-chained reads; private members (focusSelectedTabPanel, revision counter) remain in TabManager.swift as required.
Packages/CmuxSettings/Sources/CmuxSettings/Stores/UserDefaultsSettingsClient.swift Synchronous SettingsWriting conformer; nonisolated(unsafe) for the UserDefaults property is correctly justified; reads primary key only (not legacyUserDefaultsKeys), matching stated scope.
Packages/CmuxBrowser/Sources/CmuxBrowser/RecentlyClosed/RecentlyClosedBrowserStack.swift Bounded LIFO stack; capacity clamped to ≥1 at init; push, pop, purge-by-workspace all correct; Sendable and value-type.
Packages/CmuxPanes/Sources/CmuxPanes/PaneLayoutService.swift Stateless @mainactor service wrapping equalize and resize geometry plans; delegates plan computation to ExternalTreeNode extension; output is identical to removed TabManager inline logic.
Sources/WorkspaceTabColorSettings.swift 280-line verbatim lift of workspace tab color settings from TabManager; migration correctly erases legacy keys on persist; nextCustomColorName while-true loop is bounded by user-owned collection size (practical limit ~100).
Packages/CmuxSidebar/Sources/CmuxSidebar/MultiSelection/SidebarMultiSelectionModel.swift @observable model replacing @published sidebarSelectedWorkspaceIds; collapseSelection unconditionally posts notification matching legacy clearSidebarMultiSelection(except:); all mutation paths (subtract, intersect, replace) faithfully lifted.
Sources/ContentView.swift SidebarTabItemSettingsSnapshot migrated to SettingCatalog/UserDefaultsSettingsClient; notification handlers updated to match new SidebarMultiSelectionModel object identity check; SidebarWorkspaceAuxiliaryDetailVisibility type moved to CmuxSidebar package.
Sources/cmuxApp.swift DebugWindowConfigSnapshot and debug views migrated to catalog keys; @AppStorage initializers use inline SidebarCatalogSection()/WorkspaceColorsCatalogSection() struct instantiation per property; WorkspaceIndicatorStyle.decodeFromUserDefaults now used for legacy-string resolution.
Packages/CmuxSettings/Sources/CmuxSettings/Values/WorkspaceIndicatorStyle.swift Adds legacy-string mapping (rail→leftRail, border/wash/lift/…→solidFill) identical to the removed SidebarActiveTabIndicatorSettings.resolvedStyle; SettingCodable conformance added.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class TabManager {
        +let notificationDismissal: any NotificationDismissing
        +let browserModel: BrowserModel
        +let sidebarMultiSelection: SidebarMultiSelectionModel
        +let focusHistoryNavigation: any FocusHistoryNavigating
        +let paneLayout: PaneLayoutService
    }
    class FocusHistoryModel {
        -focusHistory: [FocusHistoryRecord]
        -historyIndex: Int
        -suppressionDepth: Int
        +navigateBack() Bool
        +navigateForward() Bool
        +recordFocusInHistory()
    }
    class NotificationDismissalModel {
        -suppressFocusFlash: Bool
        -pendingSelectionContext
        +dismissFocusedPanelNotificationIfActive()
        +dismissNotification()
    }
    class BrowserModel {
        -recentlyClosedBrowsers: RecentlyClosedBrowserStack
        +recordClosedBrowserPanel()
        +popMostRecentlyClosedBrowserPanel()
    }
    class SidebarMultiSelectionModel {
        -selectedWorkspaceIds: Set~UUID~
        +collapseSelection()
        +postDidHide()
    }
    class PaneLayoutService {
        +equalizeSplits()
        +resizeSplit()
    }
    class FocusHistoryHosting
    class NotificationDismissalHosting
    TabManager --> FocusHistoryModel : owns
    TabManager --> NotificationDismissalModel : owns
    TabManager --> BrowserModel : owns
    TabManager --> SidebarMultiSelectionModel : owns
    TabManager --> PaneLayoutService : owns
    TabManager ..|> FocusHistoryHosting
    TabManager ..|> NotificationDismissalHosting
    FocusHistoryModel ..> FocusHistoryHosting : weak host
    NotificationDismissalModel ..> NotificationDismissalHosting : weak host
Loading

Reviews (4): Last reviewed commit: "Wave-4 tranche 3: CmuxWorkspaces package..." | Re-trigger Greptile

Comment on lines +8 to +14
/// removed the stored object instead of writing `false`; preserve that
/// by resetting the key when re-enabling the dialog.
public let anchorCloseSuppressed = DefaultsKey<Bool>(
id: "workspaceGroups.anchorCloseSuppressed",
defaultValue: false,
userDefaultsKey: "workspaceGroup.anchorCloseSuppressed"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Reset-on-default contract has no type-level enforcement

The comment says "preserve that by resetting the key when re-enabling the dialog" — but SettingsWriting.set(_:for:) doesn't prevent writing false explicitly. During the migration window when legacy TabManager code still reads this key by absence (absent → show dialog, true → suppress), a migrated call site that writes client.set(false, ...) instead of client.reset(...) will store explicit false. A legacy UserDefaults.bool(forKey:) call returns false in both cases, so the UI behaves identically — but if any legacy code specifically branches on defaults.object(forKey:) != nil to distinguish "user has dismissed this once" from "never shown", it would see a spurious non-nil entry and behave differently. The same risk applies to newWorkspacePlacement (line 21–25). A brief doc comment on SettingsWriting.reset (or a dedicated migration helper) would make the write-by-removal invariant explicit enough that call sites won't accidentally reach for set.

Comment on lines +10 to +13
public enum WorkspaceGroupNewPlacement: String, CaseIterable, Sendable, Identifiable, SettingCodable {
case afterCurrent
case top
case end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 WorkspaceGroupNewPlacement duplicates all three cases of WorkspacePlacement

Both enums declare case afterCurrent, case top, case end with the same semantics and the same rawValue strings. WorkspaceGroupNewPlacement adds the tolerant parser and Identifiable, but the case set is identical. If WorkspacePlacement grows a case in a future PR (e.g., .pinned or .beforeCurrent), WorkspaceGroupNewPlacement will silently diverge and the group-placement setting will reject values the top-level setting accepts. Either extending WorkspacePlacement with the tolerant init?(rawString:) and Identifiable, or making WorkspaceGroupNewPlacement a typealias, would keep the two in sync with a single source of truth.

Comment on lines +33 to +38
public func value<Value: SettingCodable>(for key: DefaultsKey<Value>) -> Value {
Value.decodeFromUserDefaults(defaults.object(forKey: key.userDefaultsKey)) ?? key.defaultValue
}

public func valueIfPresent<Value: SettingCodable>(for key: DefaultsKey<Value>) -> Value? {
Value.decodeFromUserDefaults(defaults.object(forKey: key.userDefaultsKey))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 DefaultsKey.suite is silently ignored by all read/write methods

DefaultsKey carries an optional suite field so a key can target a custom UserDefaults suite (e.g., an App Group container). UserDefaultsSettingsClient always reads and writes against the single defaults instance it was constructed with, ignoring key.suite entirely. This is consistent with UserDefaultsSettingsStore.value(for:) and set(_:for:) (both of which also skip suite), but the inconsistency with UserDefaultsSettingsStore.resetAll — which does switch suites per key — means a key declared with suite: will silently read/write the wrong domain through this client. A doc note on the value(for:) / set(_:for:) declarations (matching the pattern in resetAll) would prevent future callers from assuming suite routing works end-to-end.

azooz2003-bit and others added 3 commits June 11, 2026 16:41
stage 3c (stacked): final five domains — System/Project/Debug/Sidebar/Browser
…ls + tests; app cutover follows)

Three Wave-3 feature-domain packages from the TabManager blueprint, each a
zero-dependency leaf with swift-tools 6.0 / macOS 14 / Swift 6 language mode:

- CmuxSidebar: SidebarMultiSelectionModel (@mainactor @observable) owning the
  sidebar multi-selection set plus typed SidebarMultiSelectionDidHideEvent /
  ShouldCollapseEvent wrappers (NotificationCenter delivery and userInfo wire
  shape byte-identical to the legacy stringly keys);
  SidebarWorkspaceAuxiliaryDetailVisibility lifted faithfully and
  SidebarWorkspaceDetailVisibility binding the two legacy
  SidebarWorkspaceDetailSettings resolvers into one value.
- CmuxBrowser: BrowserModel (@mainactor @observable) behind BrowserManaging,
  holding the bounded RecentlyClosedBrowserStack (faithful LIFO/capacity/
  workspace-purge semantics), generic over BrowserPanelRestoreSnapshot
  because the full snapshot payload is Workspace-owned and migrates with the
  Workspace decomposition.
- CmuxNotifications: NotificationDismissalModel (@mainactor @observable)
  behind NotificationDismissing, a faithful lift of TabManager's dismissal
  decision flow (context policy enum, pending-selection context,
  suppress-focus-flash latch) over a synchronous two-way
  NotificationDismissalHosting seam (same isolation rationale as
  CmuxSidebarGit: one-turn read/write interleavings must not gain
  suspension points).

Tests: 7 (CmuxSidebar) + 8 (CmuxBrowser) + 10 (CmuxNotifications) new
package tests green, covering selection mutations, event wire shape,
stack bounds, and the dismissal-state transition matrix incl. side-effect
ordering. Net: +1147 lines (packages only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…est, unused imports)

- @ObservationIgnored on the coordinator's handles registry: it is a struct
  mutated by ref() on nearly every response, so tracking it would invalidate
  any observer on every socket command (greptile).
- windowCloseOkAndNotFound now also asserts the not_found branch (coderabbit).
- Drop unused Foundation imports from ControlAppFocusContext and
  ControlMobileHostContext (coderabbit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ert the coordinator's v2 browser.* domain

PR 5778 moved the JS-evaluating browser.* methods onto a nonisolated
socket-worker lane while this branch had lifted the (pre-5778) browser
domain into the @mainactor coordinator. The two designs are incompatible
and main's is the behavioral reference, so this merge takes main's
browser implementation wholesale and deletes the coordinator's browser-v2
domain (package files, app conformances, umbrella members, tests). The
v1 browser-panel and sidebar handlers and the other 13 coordinator
domains are untouched by main and stay. mobile.terminal.paste (new in
PR 5876) dispatches from the legacy v2 switch. The browser domain gets
re-lifted in a follow-up against the worker-lane architecture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…alues + split geometry + PaneLayoutService)

Lift the pane-domain pure logic out of the app target into a new CmuxPanes
package (first package to depend on vendor/bonsplit): SplitDirection and
ResizeDirection move from TabManager+CompatibilityTypes.swift as Sendable
values; the SplitEqualizer math and TabManager.resizeSplit's candidate walk,
innermost-split selection, and 0.1-0.9 clamp become pure plan computations
as ExternalTreeNode extensions (equalizeDividerPlan / resizeDividerAdjustment,
snapshot-only inputs); a stateless @mainactor PaneLayoutService applies plans
to BonsplitController preserving the legacy post-order divider-mutation
sequence. TabManager owns one PaneLayoutService; TabManager+EqualizeSplits and
the control-socket workspace-equalize witness forward through it. Deletes
Sources/SplitEqualizer.swift and Sources/TabManager+CompatibilityTypes.swift;
ten package tests cover span weighting, orientation filtering, invalid split
ids, pixel-delta resize, child-side matching, innermost preference, clamping,
and the direction value mappings.

Machine-diff vs pre-deletion state: direction value bodies, spanCount,
candidate walk, and candidate/trace structs identical after receiver-spelling
normalization; the resize selection/clamp math identical token-for-token with
the tab/pane lookups staying app-side. Budget TSV refreshed: TabManager.swift
7515 -> 7146; cutover files +1 import line each.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lue + batch-reorder planner)

Start the CmuxWorkspaces domain package with the workspace values and pure
planning TabManager carried at file scope: WorkspaceGroup (the sidebar group
value with its anchor-lifecycle contract), WorkspaceReorderPlanItem, and
WorkspaceBatchReorderError move over as Sendable values; the batch-reorder
validation and pinned-stable final-order computation (workspaceBatchReorderPlan
+ batchWorkspaceReorderFinalIds) lift into a stateless WorkspaceReorderPlanner
operating on WorkspaceOrderSnapshot (id, isPinned) captures. TabManager owns
one planner, snapshots tabs at the call sites, and keeps the apply side
(tabs[] rebuild, group-contiguity renormalization, order-change notifications)
plus the group/pinned-aware per-item clamping, which reads live group state.
Four package tests cover requested-ahead-of-unmentioned ordering, the
pinned-ahead-of-unpinned invariant, duplicate/unknown rejection precedence,
and the empty request.

Machine-diff vs pre-deletion TabManager.swift: batchReorderPlan,
batchReorderFinalIds, and the WorkspaceGroup field list identical after
snapshot-seam renames (tabs -> current, workspacesById -> snapshotsById).

Remaining for this package (documented in the PR body): WorkspacesModel
(tabs/groups/selection stored state), the creation/reorder/group/close/
detached coordinators, SessionSnapshotRepository behind SessionSnapshotStoring,
and the async CloseConfirming seam.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (CmuxWorkspaces)

TabManager's @published tabs / workspaceGroups / selectedTabId stored state
moves into a new @mainactor @observable WorkspacesModel<Tab> in
CmuxWorkspaces, generic over a minimal WorkspaceTabRepresenting seam
(id/groupId/isPinned) that the app-target Workspace satisfies. TabManager
stays the per-window composition point: it owns the model, forwards the
legacy accessors, and implements WorkspacesHosting to run the legacy
property-observer side effects verbatim at identical timing:

- willSet hooks re-emit objectWillChange (what @published did) plus the new
  legacy Combine bridge publishers (tabsPublisher/selectedTabIdPublisher,
  CurrentValueSubject: new-value-at-willSet + replay-on-subscribe, the exact
  Published.Publisher contract).
- The selection willSet DEBUG trace and didSet side-effect chain move
  body-verbatim into the hook methods (git diff shows the bodies as
  unchanged context).
- Hooks fire on every assignment including equal values (@published parity);
  no-op guards stay in the host bodies. Covered by WorkspacesModelTests.

The eleven remaining $tabs/$selectedTabId Combine subscribers (AppDelegate,
CmuxConfig, ContentView, BackgroundWorkspacePrimeCoordinator,
MobileWorkspaceListObserver, TabManager DEBUG scaffolding) move to the
bridge publishers mechanically; per-site analysis confirmed none depend on
Published-specific semantics beyond replay + willSet-time emission
(dropFirst/throttle/switchToLatest all subject-compatible).

Budget refreshed via --write-budget (cutover precedent): +64 lines of
forwards/bridges/doc on TabManager.swift ahead of the tranche-4b drain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ordinators

The ~1,500-line group/reorder cluster leaves TabManager:

- WorkspacesModel gains the invariant/traversal layer it owns the data for
  (Model/WorkspacesModel+Ordering.swift, +GroupInvariants.swift): top-level
  row derivation, pin-tier clamps, anchorFirst, contiguity/run
  normalization, group-order sync, assignGroup, anchor-close dissolve, and
  the selection auto-expand used by the selection didSet hook.
- WorkspaceReorderCoordinator: move-to-top, single/before-after/batch
  reorders (owns the WorkspaceReorderPlanner), sidebar drag planning,
  drag-inferred group membership, pin toggles/batch pinning.
- WorkspaceGroupCoordinator: group creation (fresh anchor + child
  adoption + stable creation placement), createWorkspaceInGroup, member
  add/remove, ungroup/delete, rename, collapse/pin/color/icon/anchor,
  group-slot moves, localized auto-naming via host-provided format.
- Seams: WorkspaceOrderHosting (legacy postWorkspaceOrderDidChange
  NotificationCenter + event-bus publication stays app-side) and
  WorkspaceGroupHosting (workspace creation/teardown on the Workspace god,
  selection entry point, sidebar multi-selection sync, String(localized:)
  format, settings read, RenderableSystemSymbol normalization).
  WorkspaceTabRepresenting gains currentDirectory (group cwd inheritance).
- TabManager keeps its full legacy API as one-line forwards (zero call-site
  churn in ContentView/AppDelegate/TerminalController/socket paths) and
  implements the two hosting protocols in the class body.

Adversarial machine-diff vs the pre-tranche HEAD: 57 moved method bodies
compared after mechanical normalization (model./host. prefixes, Tab type
substitution); 50 are byte-identical, the 7 diffs are exactly the intended
host-seam inversions (addWorkspace/closeWorkspace/select/sidebar/localized
format/settings/icon-normalization hooks) plus comment rewording.

CmuxWorkspaces now depends on CmuxSettings (WorkspaceGroupNewPlacement).
20 package tests cover hook parity, tier ordering, batch reorder errors,
batch-unpin order parity, group creation/dissolve/anchor invariants, and
collapse focus/selection stripping. Budget refreshed (--write-budget):
TabManager.swift 7158 -> 6067 lines; the two coordinators are new >500-line
package files made of moved code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit and others added 2 commits June 12, 2026 11:34
Main brought 16 commits since the sync point, including the 3c
coordinator merge (6743387), renderer realization GPU reclaim plus
ghostty submodule bump to 5697db8 (aeb8847), browser proxy and
download fixes, agent launch capture trust, iOS View as Text, and
v0.64.15.

Resolutions:
- TerminalController+ControlWorkspaceContext.swift (add/add): took
  main's version verbatim; the branch side differed only by a trailing
  blank line inherited from feat-ctl-coordinator-3c-1.
- swift-file-length-budget.tsv: regenerated from actual line counts
  (--write-budget), which also drops the 4 stale entries for the 3c
  browser-v2 coordinator files that exist on neither side.
- ghostty submodule: main's 5697db8 (branch pointer 34cbf18 was
  inherited, no slice commits).

Packages/CmuxControlSocket is identical to origin/main (zero-path
diff); no inherited 3c-only socket content to strip.

Gates: budget, lint-ios-package-conventions, swift build
CmuxSidebarGit, xcodebuild Debug compile all pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Conflict resolution:
- Sources/TabManager.swift: slice wins for lifted types (NewWorkspacePlacement
  and WorkspaceAutoReorderSettings stay drained into CmuxSettings);
  createWorkspaceInGroup keeps the WorkspaceGroupCoordinator forwarding and
  threads main's new initialSurface parameter through it.
- NewWorkspaceInitialSurface (new on main) lifts into CmuxWorkspaces/Values so
  the group coordinator seam can carry it; WorkspaceGroupHosting
  .createWorkspaceForGroup gains the initialSurface parameter and TabManager's
  conformance forwards it into addWorkspace, preserving main's behavior.
- ghostty submodule pointer taken from the incoming side (5697db8, new
  ghostty_surface_set_renderer_realized API required by main's renderer code).
- .github/swift-file-length-budget.tsv regenerated to actual wc -l via
  --write-budget.
- cmuxTests/BrowserConfigTests.swift: fix the tests CI failure on this branch
  by making `import CmuxSettings` unconditional; it sat in the
  canImport(cmux) branch which DEV CI builds (cmux_DEV module) never take.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit and others added 2 commits June 12, 2026 12:44
Same trap as BrowserConfigTests in the merge commit: `import CmuxSettings`
sat inside the `#elseif canImport(cmux)` branch, which DEV CI builds
(cmux_DEV module) never take, so SettingCatalog/UserDefaultsSettingsClient
were out of scope and the `tests` job failed to compile. Move the import
unconditional in ShortcutAndCommandPaletteTests and WorkspaceGroupTests
(WorkspaceUnitTests precedent).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cyTests

SidebarWorkspaceAuxiliaryDetailVisibility lives only in the CmuxSidebar
package (no app-side duplicate), so the tests job failed to compile once
the earlier conditional-import fixes let compilation reach this file.
Audited the remaining cmuxTests/cmuxUITests files for package-only type
references without an unconditional import: this is the only one (other
flagged names still have app-side legacy declarations reachable through
@testable import).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit and others added 3 commits June 12, 2026 14:44
…pp types in tests

Two follow-ups surfaced once the tests job compiled past the earlier
missing-import failures:

1. The SidebarWorkspaceDetailSettingsTests retarget (wave-3 cutover) was
   syntactically mangled: `.showsWorkspaceDescription` /
   `.showsNotificationMessage` were spliced into the middle of the
   UserDefaultsSettingsClient read. Restored the intended assertions
   (construct SidebarWorkspaceDetailVisibility from the catalog reads,
   then assert the resolved member), matching the legacy
   resolvedWorkspaceDescriptionVisibility / NotificationMessage semantics.

2. The app target still declares legacy duplicates of several CmuxSettings
   value types (StoredShortcut, ShortcutStroke, AppIconMode,
   BrowserThemeMode, BrowserSearchEngine). Files importing CmuxSettings
   unconditionally now saw both and failed with ambiguity once main's new
   tests landed in the merge. Pin the app types with conditional private
   typealiases in WorkspaceUnitTests, ShortcutAndCommandPaletteTests,
   KeyboardShortcutContextTests, GhosttyConfigTests, BrowserConfigTests -
   these tests exercise the app-side settings paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The file's single ShortcutStroke use compares
CmuxSettings.ShortcutAction.defaultStroke (package type, defaulted-arg
init); pinning it to the app type broke the call. StoredShortcut stays
pinned to the app type (store override APIs).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit and others added 2 commits June 13, 2026 12:30
Resolve squash-lineage conflicts after 5907 (CmuxSidebarGit) squash-merged
to main:
- GhosttyTerminalView/TabManager imports: union (keep slice's CmuxPanes +
  decomposition packages alongside main's CmuxTerminalCore/CmuxSidebarGit).
- TabManager: keep the slice's composition-root form (computed selectedTabId
  + willSet/didSet hook methods, sub-models, settings init param,
  sidebarMultiSelection); drop the legacy enums (NewWorkspacePlacement,
  NewWorkspaceInitialSurface, WorkspaceAutoReorderSettings) already lifted
  into CmuxSettings/CmuxWorkspaces.
- CmuxSidebarGit byte-identical to main (no duplication).
- pbxproj: union (slice's 6 new packages + main's CmuxTerminalCore etc.),
  normalized + brace-balanced + xcodebuild -list parses.
- swift-file-length-budget.tsv regenerated from merged tree.

Silent cross-file fixes (no git conflict, caught by full app build):
- add `import CmuxWorkspaces` to MobileWorkspaceListObserver.swift and
  TerminalController+MobileWorkspaceList.swift (main added [WorkspaceGroup]
  params; slice moved WorkspaceGroup into CmuxWorkspaces).
- bridge tabManager.$workspaceGroups → workspaceGroupsPublisher
  CurrentValueSubject (slice replaced @published workspaceGroups with a
  computed property; MobileWorkspaceListObserver subscribed to the projection).

Local Debug app build: ** BUILD SUCCEEDED **.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
origin/main moved forward after the first resync (#6052 sidebar row cleanups,
#5920 CmuxIPCService MultiWindowRouter extraction). Only conflict was the
regenerable swift-file-length-budget.tsv; AppDelegate/ContentView/pbxproj
auto-merged. Budget regenerated from the merged tree; pbxproj normalized +
parses; runMultiWindowRouteCLI confirmed single-homed in CmuxIPCService.

Local Debug app build: ** BUILD SUCCEEDED **.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/SidebarWorkspaceGroupHeaderView.swift (1)

142-145: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use plural keys for the unread accessibility label.

%lld unread is wrong for 1 and bypasses the repo’s .one / .other pluralization convention. Split this into singular/plural localization keys.

Based on learnings: pluralized Swift strings should use ICU .one / .other keys.

🌐 Proposed fix
-                        .accessibilityLabel(Text(String.localizedStringWithFormat(
-                            String(localized: "workspaceGroup.unread.a11y", defaultValue: "%lld unread"),
-                            anchorUnreadCount
-                        )))
+                        .accessibilityLabel(Text(
+                            anchorUnreadCount == 1
+                                ? String(localized: "workspaceGroup.unread.a11y.one", defaultValue: "1 unread")
+                                : String(localized: "workspaceGroup.unread.a11y.other", defaultValue: "\(anchorUnreadCount) unread")
+                        ))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/SidebarWorkspaceGroupHeaderView.swift` around lines 142 - 145, The
accessibility label is using a non-pluralized format string; change it to use
ICU plural keys and a proper integer specifier and then format with the count.
Add a pluralized localization key (workspaceGroup.unread.a11y) with .one and
.other variants (e.g. ".one" -> "%d unread", ".other" -> "%d unread"), replace
the current String(localized: "workspaceGroup.unread.a11y", defaultValue: "%lld
unread") usage in the accessibilityLabel with a formatted call that uses
anchorUnreadCount (e.g. call String.localizedStringWithFormat(String(localized:
"workspaceGroup.unread.a11y"), anchorUnreadCount) and use %d instead of %lld) so
singular/plural forms are chosen correctly.

Sources: Coding guidelines, Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryModel.swift`:
- Around line 34-35: Clamp maxHistorySize to a positive integer in the
FocusHistoryModel initializer (e.g., set self.maxHistorySize = max(1,
maxHistorySize)) and update any places that drop oldest entries (the code paths
computing and calling removeFirst in the methods that prune history) to use a
safe removal count (e.g., let toRemove = min(history.count, computedRemoveCount)
before calling removeFirst(toRemove)) so removeFirst is never invoked with n >
count; change the init and the two prune sites accordingly to use these guards.

In
`@Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift`:
- Around line 52-55: The current eligibleChildren computation does an O(n²) scan
because childWorkspaceIds.compactMap calls model.tabs.contains(where:) for each
id; to fix, build a Set of existing tab IDs once (e.g., let tabIdSet =
Set(model.tabs.map { $0.id })) then change the compactMap to check
tabIdSet.contains(id) and !existingAnchorIds.contains(id) so eligibleChildren
uses O(1) membership tests; update the code around eligibleChildren,
childWorkspaceIds, and model.tabs accordingly.

In
`@Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel`+Ordering.swift:
- Around line 109-125: clampedTopLevelReorderIndex currently uses
sidebarTopLevelPinnedWorkspaceIds() directly which can include ids not present
in the provided topLevelIds (or miss promoted rows); update the function to
first limit the pinned set to only those present in the supplied topLevelIds
(e.g. pinnedSet =
Set(sidebarTopLevelPinnedWorkspaceIds()).intersection(Set(topLevelIds))) and
then use that pinnedSet when computing pinnedCount and when testing if the
workspaceId is pinned; this ensures both the count and the workspaceId check are
based on the same topLevel snapshot passed into clampedTopLevelReorderIndex.

In
`@Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`:
- Line 213: Replace the forced-try usages of "try!
`#require`(model.workspaceGroups.first(where: { $0.id == groupId }))" (and the
same pattern at the other four occurrences) by making the enclosing test
functions throw (change their signatures to "func ...() throws") and using "try
`#require`(...)" instead of "try!"; ensure each test function that contains these
expressions is updated to include "throws" so the tests propagate failures
rather than crash.

In `@Sources/SidebarWorkspaceGroupHeaderView.swift`:
- Around line 1-3: The file references WorkspaceGroup and
WorkspaceGroupNewPlacement but only imports CmuxSettings; add import
CmuxWorkspaces at the top of Sources/SidebarWorkspaceGroupHeaderView.swift so
those types resolve (i.e., include the new module import alongside the existing
imports).

In `@Sources/TabManager`+FocusHistoryHosting.swift:
- Around line 40-43: The selectWorkspace(_:) method currently assigns
selectedTabId without checking that the given workspaceId exists in tabs; update
selectWorkspace(_:) to first verify the workspaceId is present in tabs (e.g.,
via tabs.contains { $0.id == workspaceId } or appropriate lookup) and return
early (no-op) if it’s missing, only setting selectedTabId when the workspace is
actually found.

---

Outside diff comments:
In `@Sources/SidebarWorkspaceGroupHeaderView.swift`:
- Around line 142-145: The accessibility label is using a non-pluralized format
string; change it to use ICU plural keys and a proper integer specifier and then
format with the count. Add a pluralized localization key
(workspaceGroup.unread.a11y) with .one and .other variants (e.g. ".one" -> "%d
unread", ".other" -> "%d unread"), replace the current String(localized:
"workspaceGroup.unread.a11y", defaultValue: "%lld unread") usage in the
accessibilityLabel with a formatted call that uses anchorUnreadCount (e.g. call
String.localizedStringWithFormat(String(localized:
"workspaceGroup.unread.a11y"), anchorUnreadCount) and use %d instead of %lld) so
singular/plural forms are chosen correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 803ed18f-1950-4f50-9bb5-5d57ce41dc4d

📥 Commits

Reviewing files that changed from the base of the PR and between 94408ca and f8d9fb9.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (116)
  • Packages/CmuxBrowser/Package.swift
  • Packages/CmuxBrowser/Sources/CmuxBrowser/BrowserModel.swift
  • Packages/CmuxBrowser/Sources/CmuxBrowser/RecentlyClosed/BrowserManaging.swift
  • Packages/CmuxBrowser/Sources/CmuxBrowser/RecentlyClosed/BrowserPanelRestoreSnapshot.swift
  • Packages/CmuxBrowser/Sources/CmuxBrowser/RecentlyClosed/RecentlyClosedBrowserStack.swift
  • Packages/CmuxBrowser/Tests/CmuxBrowserTests/RecentlyClosedBrowserStackTests.swift
  • Packages/CmuxNotifications/Package.swift
  • Packages/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalContext.swift
  • Packages/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalHosting.swift
  • Packages/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalModel.swift
  • Packages/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissing.swift
  • Packages/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDismissalModelTests.swift
  • Packages/CmuxPanes/Package.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/Geometry/ExternalTreeNode+SplitGeometry.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/Geometry/SplitDividerAdjustment.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/Geometry/SplitEqualizePlan.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/PaneLayoutService.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/SplitEqualizeResult.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/Values/ResizeDirection.swift
  • Packages/CmuxPanes/Sources/CmuxPanes/Values/SplitDirection.swift
  • Packages/CmuxPanes/Tests/CmuxPanesTests/SplitGeometryTests.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Keys/SettingCatalog.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Keys/WorkspaceGroupsCatalogSection.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Stores/SettingsReading.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Stores/SettingsWriting.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Stores/UserDefaultsSettingsClient.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Values/WorkspaceGroupNewPlacement.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Values/WorkspaceIndicatorStyle.swift
  • Packages/CmuxSettings/Tests/CmuxSettingsTests/UserDefaultsSettingsClientTests.swift
  • Packages/CmuxSidebar/Package.swift
  • Packages/CmuxSidebar/Sources/CmuxSidebar/Detail/SidebarWorkspaceAuxiliaryDetailVisibility.swift
  • Packages/CmuxSidebar/Sources/CmuxSidebar/Detail/SidebarWorkspaceDetailVisibility.swift
  • Packages/CmuxSidebar/Sources/CmuxSidebar/MultiSelection/SidebarMultiSelectionDidHideEvent.swift
  • Packages/CmuxSidebar/Sources/CmuxSidebar/MultiSelection/SidebarMultiSelectionModel.swift
  • Packages/CmuxSidebar/Sources/CmuxSidebar/MultiSelection/SidebarMultiSelectionShouldCollapseEvent.swift
  • Packages/CmuxSidebar/Tests/CmuxSidebarTests/SidebarDetailVisibilityTests.swift
  • Packages/CmuxSidebar/Tests/CmuxSidebarTests/SidebarMultiSelectionModelTests.swift
  • Packages/CmuxWorkspaceNavigation/Package.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryEntry.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryHosting.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryModel.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryNavigating.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryRecord.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/Menu/FocusHistoryMenuDirection.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/Menu/FocusHistoryMenuItem.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/Menu/FocusHistoryMenuPosition.swift
  • Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/Menu/FocusHistoryMenuSnapshot.swift
  • Packages/CmuxWorkspaceNavigation/Tests/CmuxWorkspaceNavigationTests/FocusHistoryModelTests.swift
  • Packages/CmuxWorkspaces/Package.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupHosting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceOrderHosting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceReorderCoordinator.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspaceTabRepresenting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesHosting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+GroupInvariants.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+Ordering.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Reorder/WorkspaceBatchReorderError.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Reorder/WorkspaceOrderSnapshot.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Reorder/WorkspaceReorderPlanItem.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Reorder/WorkspaceReorderPlanner.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/NewWorkspaceInitialSurface.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceGroup.swift
  • Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift
  • Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceReorderPlannerTests.swift
  • Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspacesModelTests.swift
  • Sources/AppDelegate+FocusHistoryContextMenu.swift
  • Sources/AppDelegate+MoveTabToNewWorkspace.swift
  • Sources/AppDelegate.swift
  • Sources/AppleScriptSupport.swift
  • Sources/BackgroundWorkspacePrimeCoordinator.swift
  • Sources/CmuxConfig.swift
  • Sources/CommandPalette/CommandPaletteSettingsToggle.swift
  • Sources/ContentView+ForkAgentConversation.swift
  • Sources/ContentView.swift
  • Sources/Feed/FeedCoordinator.swift
  • Sources/FocusHistory.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/KeyboardShortcutSettingsFileStore+Template.swift
  • Sources/KeyboardShortcutSettingsFileStore.swift
  • Sources/Mobile/MobileWorkspaceListObserver.swift
  • Sources/Sidebar/SidebarAppearanceSupport.swift
  • Sources/Sidebar/SidebarState.swift
  • Sources/SidebarWorkspaceGroupHeaderView.swift
  • Sources/SidebarWorkspaceRenderItem.swift
  • Sources/SplitEqualizer.swift
  • Sources/TabManager+CompatibilityTypes.swift
  • Sources/TabManager+DetachedWorkspace.swift
  • Sources/TabManager+EqualizeSplits.swift
  • Sources/TabManager+FocusHistoryHosting.swift
  • Sources/TabManager+NotificationDismissalHosting.swift
  • Sources/TabManager.swift
  • Sources/TerminalController+ControlWorkspaceContext.swift
  • Sources/TerminalController+ControlWorkspaceGroupContext.swift
  • Sources/TerminalController+MobileWorkspaceList.swift
  • Sources/TerminalController.swift
  • Sources/TerminalNotificationStore.swift
  • Sources/VerticalTabsSidebar+WorkspaceGroups.swift
  • Sources/Workspace.swift
  • Sources/WorkspaceIndicatorStyle+Display.swift
  • Sources/WorkspacePlacement+Resolution.swift
  • Sources/WorkspaceTabColorEntry.swift
  • Sources/WorkspaceTabColorSettings.swift
  • Sources/cmuxApp+HistoryMenu.swift
  • Sources/cmuxApp.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/BrowserConfigTests.swift
  • cmuxTests/GhosttyConfigTests.swift
  • cmuxTests/KeyboardShortcutContextTests.swift
  • cmuxTests/ShortcutAndCommandPaletteTests.swift
  • cmuxTests/SidebarOrderingTests.swift
  • cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
  • cmuxTests/TabManagerSessionSnapshotTests.swift
  • cmuxTests/WorkspaceGroupTests.swift
  • cmuxTests/WorkspaceUnitTests.swift
💤 Files with no reviewable changes (2)
  • Sources/TabManager+CompatibilityTypes.swift
  • Sources/SplitEqualizer.swift
👮 Files not reviewed due to content moderation or server errors (1)
  • Sources/TerminalController.swift

Comment on lines +34 to +35
public init(maxHistorySize: Int = 50) {
self.maxHistorySize = maxHistorySize

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clamp maxHistorySize to a positive value to avoid removeFirst runtime traps.

Line 34 accepts any Int; with maxHistorySize <= 0, the overflow math at Lines 101-104 and Lines 123-125 can call removeFirst with n > count, which traps.

Suggested fix
 public init(maxHistorySize: Int = 50) {
-    self.maxHistorySize = maxHistorySize
+    self.maxHistorySize = max(1, maxHistorySize)
 }

Also applies to: 101-104, 123-125

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/CmuxWorkspaceNavigation/Sources/CmuxWorkspaceNavigation/FocusHistoryModel.swift`
around lines 34 - 35, Clamp maxHistorySize to a positive integer in the
FocusHistoryModel initializer (e.g., set self.maxHistorySize = max(1,
maxHistorySize)) and update any places that drop oldest entries (the code paths
computing and calling removeFirst in the methods that prune history) to use a
safe removal count (e.g., let toRemove = min(history.count, computedRemoveCount)
before calling removeFirst(toRemove)) so removeFirst is never invoked with n >
count; change the init and the two prune sites accordingly to use these guards.

Comment on lines +52 to +55
let eligibleChildren = childWorkspaceIds.compactMap { id -> UUID? in
guard model.tabs.contains(where: { $0.id == id }),
!existingAnchorIds.contains(id) else { return nil }
return id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid quadratic filtering when resolving eligible child workspaces.

Line 53 does model.tabs.contains(where:) inside childWorkspaceIds.compactMap, which rescans the full tabs collection per child id. In large multi-select operations this becomes O(n²) and can noticeably slow group creation.

⚡ Proposed fix
-        let eligibleChildren = childWorkspaceIds.compactMap { id -> UUID? in
-            guard model.tabs.contains(where: { $0.id == id }),
-                  !existingAnchorIds.contains(id) else { return nil }
-            return id
-        }
+        let tabIds = Set(model.tabs.map(\.id))
+        let eligibleChildren = childWorkspaceIds.compactMap { id -> UUID? in
+            guard tabIds.contains(id),
+                  !existingAnchorIds.contains(id) else { return nil }
+            return id
+        }

As per coding guidelines, .github/review-bot-rules/algorithmic-complexity.md requires flagging nested full-collection scans on scalable user-data paths.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let eligibleChildren = childWorkspaceIds.compactMap { id -> UUID? in
guard model.tabs.contains(where: { $0.id == id }),
!existingAnchorIds.contains(id) else { return nil }
return id
let tabIds = Set(model.tabs.map(\.id))
let eligibleChildren = childWorkspaceIds.compactMap { id -> UUID? in
guard tabIds.contains(id),
!existingAnchorIds.contains(id) else { return nil }
return id
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift`
around lines 52 - 55, The current eligibleChildren computation does an O(n²)
scan because childWorkspaceIds.compactMap calls model.tabs.contains(where:) for
each id; to fix, build a Set of existing tab IDs once (e.g., let tabIdSet =
Set(model.tabs.map { $0.id })) then change the compactMap to check
tabIdSet.contains(id) and !existingAnchorIds.contains(id) so eligibleChildren
uses O(1) membership tests; update the code around eligibleChildren,
childWorkspaceIds, and model.tabs accordingly.

Source: Coding guidelines

Comment on lines +503 to +510
if !model.workspaceGroups.isEmpty {
for id in changedIds {
if let workspace = workspacesById[id] {
reorderTabForPinnedState(workspace)
}
}
host?.workspaceOrderDidChange(movedWorkspaceIds: changedIds)
return changedIds

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Batch pin updates currently rescan/re-normalize per workspace, causing quadratic work.

In grouped windows, bulk pin/unpin executes reorderTabForPinnedState once per changed id. That repeatedly traverses/normalizes tabs, so multi-select batch updates degrade to O(m×n). Apply all pin-state flips first, then run one normalization/reorder pass before publishing.

As per coding guidelines, batch actions over scalable collections should avoid per-target rescans and repeated full-collection work in production paths.

Source: Coding guidelines

Comment on lines +109 to +125
func clampedTopLevelReorderIndex(
forWorkspaceId workspaceId: UUID,
targetIndex: Int,
topLevelIds: [UUID]
) -> Int {
let clamped = max(0, min(targetIndex, max(0, topLevelIds.count - 1)))
let pinnedIds = sidebarTopLevelPinnedWorkspaceIds()
let pinnedCount = topLevelIds.reduce(into: 0) { count, id in
if pinnedIds.contains(id) {
count += 1
}
}
if pinnedIds.contains(workspaceId) {
return min(clamped, max(0, pinnedCount - 1))
}
return max(clamped, pinnedCount)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Top-level clamp uses a pinned snapshot that can exclude promoted rows.

clampedTopLevelReorderIndex bases pin-tier classification on sidebarTopLevelPinnedWorkspaceIds(), but promotion flows pass a custom topLevelIds that may include a promoted workspace not present in that pinned snapshot. This can clamp a promoted pinned workspace as if it were unpinned.

Suggested fix
 func clampedTopLevelReorderIndex(
     forWorkspaceId workspaceId: UUID,
     targetIndex: Int,
     topLevelIds: [UUID]
 ) -> Int {
     let clamped = max(0, min(targetIndex, max(0, topLevelIds.count - 1)))
-    let pinnedIds = sidebarTopLevelPinnedWorkspaceIds()
+    let groupsByAnchorId = Dictionary(uniqueKeysWithValues: workspaceGroups.map { ($0.anchorWorkspaceId, $0) })
+    let tabsById = Dictionary(uniqueKeysWithValues: tabs.map { ($0.id, $0) })
+
+    func isPinnedTopLevelId(_ id: UUID) -> Bool {
+        if let group = groupsByAnchorId[id] { return group.isPinned }
+        return tabsById[id]?.isPinned == true
+    }
+
     let pinnedCount = topLevelIds.reduce(into: 0) { count, id in
-        if pinnedIds.contains(id) {
+        if isPinnedTopLevelId(id) {
             count += 1
         }
     }
-    if pinnedIds.contains(workspaceId) {
+    if isPinnedTopLevelId(workspaceId) {
         return min(clamped, max(0, pinnedCount - 1))
     }
     return max(clamped, pinnedCount)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel`+Ordering.swift
around lines 109 - 125, clampedTopLevelReorderIndex currently uses
sidebarTopLevelPinnedWorkspaceIds() directly which can include ids not present
in the provided topLevelIds (or miss promoted rows); update the function to
first limit the pinned set to only those present in the supplied topLevelIds
(e.g. pinnedSet =
Set(sidebarTopLevelPinnedWorkspaceIds()).intersection(Set(topLevelIds))) and
then use that pinnedSet when computing pinnedCount and when testing if the
workspaceId is pinned; this ensures both the count and the workspaceId check are
based on the same topLevel snapshot passed into clampedTopLevelReorderIndex.

childWorkspaceIds: [child1.id, child2.id]
)

let group = try! #require(model.workspaceGroups.first(where: { $0.id == groupId }))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace try! #require(...) with throwing test flow.

Lines 213, 244, 263, 278, and 317 use forced tries, which triggers the force_try lint error and can crash the test instead of reporting a normal assertion failure. Mark those tests as throws and use try #require(...).

✅ Suggested patch pattern
-    func createWorkspaceGroupAdoptsChildrenAndKeepsSectionContiguous() {
+    func createWorkspaceGroupAdoptsChildrenAndKeepsSectionContiguous() throws {
@@
-        let group = try! `#require`(model.workspaceGroups.first(where: { $0.id == groupId }))
+        let group = try `#require`(model.workspaceGroups.first(where: { $0.id == groupId }))

-    func deleteWorkspaceGroupClosesMembersAndClearsLastHoldout() {
+    func deleteWorkspaceGroupClosesMembersAndClearsLastHoldout() throws {
@@
-        let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))
+        let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))

-    func ungroupKeepsMemberPositionsAndDropsMembership() {
+    func ungroupKeepsMemberPositionsAndDropsMembership() throws {
@@
-        let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id]))
+        let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id]))

-    func collapseToggleMovesFocusToAnchorAndStripsHiddenSelection() {
+    func collapseToggleMovesFocusToAnchorAndStripsHiddenSelection() throws {
@@
-        let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id]))
+        let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id]))

-    func setWorkspaceGroupAnchorHoistsNewAnchorToSectionFront() {
+    func setWorkspaceGroupAnchorHoistsNewAnchorToSectionFront() throws {
@@
-        let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))
+        let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))

Also applies to: 244-244, 263-263, 278-278, 317-317

🧰 Tools
🪛 SwiftLint (0.63.3)

[Error] 213-213: Force tries should be avoided

(force_try)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`
at line 213, Replace the forced-try usages of "try!
`#require`(model.workspaceGroups.first(where: { $0.id == groupId }))" (and the
same pattern at the other four occurrences) by making the enclosing test
functions throw (change their signatures to "func ...() throws") and using "try
`#require`(...)" instead of "try!"; ensure each test function that contains these
expressions is updated to include "throws" so the tests propagate failures
rather than crash.

Source: Linters/SAST tools

Comment on lines 1 to +3
import AppKit
import SwiftUI
import CmuxSettings

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add the CmuxWorkspaces import.

WorkspaceGroup and WorkspaceGroupNewPlacement now come from the extracted workspaces module, but this file only imports CmuxSettings. That leaves the new type references unresolved unless they’re re-exported elsewhere.

🛠️ Proposed fix
 import AppKit
 import SwiftUI
+import CmuxWorkspaces
 import CmuxSettings
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/SidebarWorkspaceGroupHeaderView.swift` around lines 1 - 3, The file
references WorkspaceGroup and WorkspaceGroupNewPlacement but only imports
CmuxSettings; add import CmuxWorkspaces at the top of
Sources/SidebarWorkspaceGroupHeaderView.swift so those types resolve (i.e.,
include the new module import alongside the existing imports).

Comment on lines +40 to +43
func selectWorkspace(_ workspaceId: UUID) {
if selectedTabId != workspaceId {
selectedTabId = workspaceId
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard selectWorkspace(_:) when the workspace no longer exists.

On Line 42, selectedTabId is set without verifying workspaceId is still present in tabs. That breaks this file’s own no-op-on-missing-target contract and can leave selection pointing at a non-existent workspace.

💡 Proposed fix
     func selectWorkspace(_ workspaceId: UUID) {
-        if selectedTabId != workspaceId {
+        guard tabs.contains(where: { $0.id == workspaceId }) else { return }
+        if selectedTabId != workspaceId {
             selectedTabId = workspaceId
         }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/TabManager`+FocusHistoryHosting.swift around lines 40 - 43, The
selectWorkspace(_:) method currently assigns selectedTabId without checking that
the given workspaceId exists in tabs; update selectWorkspace(_:) to first verify
the workspaceId is present in tabs (e.g., via tabs.contains { $0.id ==
workspaceId } or appropriate lookup) and return early (no-op) if it’s missing,
only setting selectedTabId when the workspace is actually found.

@azooz2003-bit
azooz2003-bit merged commit 7481d76 into main Jun 13, 2026
32 checks passed
azooz2003-bit added a commit that referenced this pull request Jun 19, 2026
…ocusHistoryModel

The focus-history back/forward stack, suppression depth, navigation, menu
snapshots, and current-entry resolution already live in CmuxWorkspaces'
FocusHistoryModel (PR #5921 / #6356); TabManager only forwards. Two of those
forwarders, withFocusHistoryRecordingSuppressed and currentFocusHistoryEntry,
had no callers outside TabManager itself, so their five internal uses now call
focusHistoryNavigation directly and the pass-through declarations are removed.

Byte-faithful: every repointed call hits the identical FocusHistoryNavigating
method on the same focusHistoryNavigation instance the forwarders routed
through, so behavior, ordering, and the focusHistoryRevision republish are
unchanged. The remaining members stay app-target because they are the live
seam: invalidateFocusHistoryTarget/focusHistoryMenuSnapshot/navigate* are the
public API consumed by AppDelegate/ContentView/cmuxApp/Workspace/tests;
panelIdForFocusHistorySurface reads tabs; focusSelectedWorkspacePanel and
focusHistoryRevisionDidChange touch private members and the @published
focusHistoryRevision mirror that posts .tabManagerFocusHistoryRevisionDidChange
with object: self (window identity).

TabManager.swift 6176 -> 6172. Budget ratcheted down. CmuxWorkspaces 97 tests
pass; app build SUCCEEDED.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — f8d9fb91 Deployed Jun 13, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant