Fix terminal surfaces losing theme after config reload - #2707
rodchristiansen wants to merge 25 commits into
Conversation
Users can now organize workspace tabs into named, collapsible sections in the sidebar. Sections are opt-in — the sidebar behaves identically when no sections exist. - New SidebarSection model with name, collapse state, and ordered workspace membership - Section CRUD in TabManager (create, rename, delete, reorder) - Context menu: "Move to Section" submenu on workspace tabs with "New Section..." option - SidebarSectionHeaderView with disclosure chevron, inline rename, context menu, accessibility labels, and drop target - Drag workspaces onto section headers to assign them - Section state persists across sessions via SessionPersistence - Pinned workspaces always render at top regardless of section - Backward-compatible: old session snapshots restore cleanly
The new file needs to be registered in the .pbxproj so Xcode can find the SidebarSection and SidebarLayout types.
- Forward section objectWillChange to TabManager via sectionRevision counter so sidebar re-renders immediately on any section mutation - Use @focusstate on rename TextField with delayed auto-focus so cursor lands in the text field instead of the terminal - Guard onTapGesture during rename editing to prevent conflicts - Add toggleCollapsed()/setCollapsed() helpers that bump revision - Pipe Combine observers from each section to TabManager on didSet
NavigationSplitView with native glass sidebar, system titlebar, and SwiftUI toolbar on macOS 26. Terminal detail content gets 8pt external padding, 6pt internal content inset, and leading corner rounding (16pt) when the sidebar is visible. Key changes: - NavigationSplitView wraps sidebar + detail for native glass treatment - System titlebar (titlebarAppearsTransparent = false) with SwiftUI .toolbar for bell and new-tab buttons - SplitViewDividerHider clears NSSplitView divider color - Terminal content inset (6pt) keeps text within safe zone - Leading corners rounded at AppKit portal level when sidebar present - Bonsplit tab bar hidden for single-tab panes on macOS 26 - App icon auto-set skipped on macOS 26 (system handles dark variants) - Old titlebar accessory controllers skipped on macOS 26
Update bonsplit submodule: the animated split entry path hardcoded 0.5 instead of using the configured divider ratio. Include sidebar sections (id, name, collapsed state, workspace membership) in the autosave fingerprint so that section changes trigger session persistence and survive app restart.
Preserve workspace UUIDs in session snapshots so section membership survives app restart. Include sections in the autosave fingerprint so create/rename/reorder/collapse changes trigger persistence. Auto-enter rename mode when a new section is created so the user can immediately type the name.
- Add "target": "current" to cmux.json workspace commands so layouts apply to the selected workspace in-place instead of spawning a new one - Fix section rename auto-focus race: delay pendingRenameSectionId to next run loop so SwiftUI mounts the header view before the publisher emits
- Decode 'target' field in CmuxWorkspaceDefinition custom decoder so target:current actually takes effect from cmux.json - Fix within-section drag reorder: reorder section.workspaceIds instead of the flat tabs array so visual order updates correctly - Fix section rename auto-focus timing race (from prior commit)
- Fix bell button no-op: start titlebarAccessoryController on macOS 26 so notifications popover infrastructure is available - Add tab bar design comment: bonsplit shows tabs when pane.tabs > 1 - Replace dividerColor KVC with safe responds(to:)/perform selector - Add accessibility labels to toolbar buttons (bell, new tab) - Localize all WindowToolbarController toolbar item strings - Add comment about NavigationSplitView width persistence limitation - Move corner radius update inside CATransaction to prevent flicker - Replace magic number 20 with named sidebarDetectionThreshold constant - Remove unused TerminalBackgroundFill struct - Fix SplitViewDividerHider doc comment and remove unused property - Add comment explaining uniform 6pt content inset design
- Remove || true debug code left in section menu guard - Fix intra-section drag off-by-one: remove before insert so indices are stable when dragging forward - Fix tabIndexById to use flat tabs order for shift-selection and shortcut badges (not section order) - Fix dictionary mutation during iteration in panel close loop - Make target:current fail closed when no workspace is selected - Only auto-apply to single-pane workspaces to avoid tearing down restored or user-customized split configurations - Validate destination section before removing workspace from all sections in moveWorkspaceToSection - Add backwards-compat comment for optional SessionWorkspaceSnapshot.id - Replace SwiftUI import with Foundation in SidebarSection (only needs Combine + Foundation) - Remove redundant revision property and bumpRevision() calls from SidebarSection (mutations already trigger objectWillChange) - Remove redundant tabManager.objectWillChange.send() in section header
…macOS 26 - Fix bell button no-op: use SwiftUI .popover on macOS 26 instead of the broken AppKit titlebar accessory path; bridge keyboard shortcut via NotificationCenter - Move pane action buttons (terminal, browser, split right/down) from bonsplit tab bar to native toolbar as a ControlGroup on macOS 26 - Hide bonsplit split buttons when tab bar is hidden (macOS 26) - Hide old HiddenTitlebarSidebarControlsView on macOS 26 - Remove system sidebar toggle chevron (.toolbar(removing: .sidebarToggle))
- Fix asymmetric inset: only inset leading/top edges where corners are rounded, preserving terminal real estate on right/bottom - Fix misleading comment about fullSizeContentView on macOS 26 - Update SplitViewDividerHider doc to match actual implementation - Clarify hideTabBar comment: PaneContainerView conditionally shows tab bar for multi-tab panes - Improve sidebar detection heuristic comment explaining why frame-based approach is used (AppKit view cannot access SwiftUI SidebarState) - Add GeometryReader to persist sidebar width on macOS 26 when user drags the native NavigationSplitView divider
- Remove redundant sectionRevision read in sidebar body (sidebarLayout already reads it internally, establishing the SwiftUI dependency) - Fix autoApply silently skipping global-config commands: fall back to global config directory when no local config is present - Fix auto-apply race condition on rapid tab switching: capture the emitted tab ID and verify it's still selected before applying, so a quick B→C switch doesn't apply B's config to C
…s on macOS 26 - Use flexible frame for sidebar in NavigationSplitView so rows extend full width instead of floating centered with gaps - Skip attaching legacy TitlebarControlsAccessoryViewController on macOS 26 since NavigationSplitView toolbar provides its own controls - Add unread notification badge to the new toolbar bell button
After ghostty_app_update_config, each surface re-derives its config using its conditional state (light/dark). However, ghostty_surface_set_color_scheme has an internal dedup that skips when the scheme hasn't changed, so surfaces could end up with stale theme colors (e.g. light foreground on dark background) after a config reload or appearance toggle. Fix by calling ghostty_surface_update_config on each surface in refreshTerminalSurfacesAfterGhosttyConfigReload, which forces a full config re-derive with the surface's current conditional state, bypassing the color scheme dedup. Also re-apply the color scheme from the current macOS appearance to keep Swift-side tracking in sync.
|
@rodchristiansen is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR introduces a comprehensive macOS 26+ UI overhaul featuring a new sidebar sections system for workspace organization, a NavigationSplitView layout, revised titlebar behavior, workspace auto-apply functionality, and new toolbar controls. It adds session persistence for sidebar sections, updates terminal surface refresh logic, and refactors config execution to support in-place workspace updates with conditional macOS version-specific code paths throughout. Changes
Sequence DiagramssequenceDiagram
participant User
participant AppDelegate
participant CmuxConfigStore
participant TabManager
participant Workspace
participant Terminal
User->>AppDelegate: Launch app / Change workspace
AppDelegate->>CmuxConfigStore: loadAll()
CmuxConfigStore->>CmuxConfigStore: checkAutoApply(forTabId:)
CmuxConfigStore->>CmuxConfigStore: Find command where autoApply==true<br/>and target==.current
alt Command found and conditions met
CmuxConfigStore->>Workspace: Execute command in current workspace
Workspace->>Workspace: Set title, color, layout
Workspace->>Terminal: Apply layout (close non-focused panels)
else Command not applicable
CmuxConfigStore->>CmuxConfigStore: Skip (already applied or conditions unmet)
end
Terminal->>AppDelegate: Render updated workspace
sequenceDiagram
participant User
participant ContentView as ContentView<br/>(Sidebar UI)
participant TabManager
participant SidebarSection
participant SessionPersistence
User->>ContentView: Drag workspace to section header
ContentView->>TabManager: moveWorkspaceToSection(tabId, sectionId)
TabManager->>SidebarSection: addWorkspace(workspaceId, at index)
SidebarSection->>TabManager: notifySectionChange()
TabManager->>TabManager: Increment sectionRevision
TabManager->>ContentView: Update sidebarLayout (SwiftUI dependency)
ContentView->>ContentView: Re-render section groups
TabManager->>SessionPersistence: Session autosave includes sections
SessionPersistence->>SessionPersistence: Serialize sidebar section state
sequenceDiagram
participant AppDelegate
participant ContentView as macOS 26+ UI
participant WindowToolbarController
participant TabManager
AppDelegate->>ContentView: Initialize window (macOS 26+)
ContentView->>ContentView: Render NavigationSplitView layout
ContentView->>WindowToolbarController: Configure toolbar
WindowToolbarController->>WindowToolbarController: Add sidebar toggle,<br/>notifications, new tab items
User->>WindowToolbarController: Click sidebar toggle
WindowToolbarController->>AppDelegate: toggleSidebarAction()
AppDelegate->>ContentView: Update sidebarState.isVisible
ContentView->>ContentView: Animate NavigationSplitView column
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses terminal surfaces ending up with stale theme colors after a Ghostty config reload by forcing each surface to re-derive its config and re-assert the current light/dark color scheme. In addition, it introduces a set of macOS 26-specific UI changes (NavigationSplitView / Liquid Glass / toolbar behavior) and adds sidebar “sections” plus related session persistence and config behavior.
Changes:
- Force Ghostty surfaces to re-derive config + re-apply the correct color scheme after config reload.
- Add macOS 26 UI adaptations (NavigationSplitView layout, glass/titlebar/toolbar adjustments, rounded terminal corners).
- Introduce sidebar sections (grouping/collapse/reorder/move-to-section) with session persistence support, and add cmux config auto-apply / “target: current”.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Workspace.swift | Adds workspace UUID persistence in snapshots and supports restoring a workspace’s ID; macOS 26-specific bonsplit appearance tweaks. |
| Sources/WindowToolbarController.swift | Adds macOS 26 toolbar identifiers/items and switches toolbar style on macOS 26. |
| Sources/Update/UpdateTitlebarAccessory.swift | Exposes NotificationsPopoverView for reuse and skips legacy titlebar accessory attachment on macOS 26. |
| Sources/TerminalWindowPortal.swift | Rounds terminal corners on macOS 26 when a sidebar is heuristically detected. |
| Sources/TabManager.swift | Adds sidebar sections model + layout computation + autosave hashing + session snapshot/restore integration. |
| Sources/SidebarSection.swift | Introduces SidebarSection + SidebarLayout types to represent sidebar grouping. |
| Sources/SessionPersistence.swift | Extends session snapshots with optional workspace IDs and adds sidebar section snapshot types. |
| Sources/GhosttyTerminalView.swift | Adds surface reapplyColorSchemeAndConfig() and macOS 26 scroll view insets; exposes applySurfaceColorScheme to same-file callers. |
| Sources/ContentView.swift | Adds macOS 26 NavigationSplitView + SwiftUI toolbar controls, sections UI, glass modifiers, and sidebar background behavior changes. |
| Sources/CmuxConfigExecutor.swift | Adds “target: current” support to apply a workspace layout to the currently selected workspace. |
| Sources/CmuxConfig.swift | Adds autoApply, workspace target, and auto-apply wiring logic in CmuxConfigStore. |
| Sources/cmuxApp.swift | Refactors main window content construction and adjusts macOS 26 app icon behavior. |
| Sources/AppDelegate.swift | Forces surface config+scheme re-derivation after Ghostty config reload; macOS 26 titlebar/window behavior changes; adds notification toggle indirection for SwiftUI popover. |
| GhosttyTabs.xcodeproj/project.pbxproj | Adds SidebarSection.swift to the Xcode project build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clear the divider color via ObjC messaging (private API). | ||
| let selector = NSSelectorFromString("setDividerColor:") | ||
| if splitView.responds(to: selector) { | ||
| splitView.perform(selector, with: NSColor.clear) | ||
| } |
There was a problem hiding this comment.
SplitViewDividerHiderView is invoking a private AppKit API via perform(NSSelectorFromString("setDividerColor:"), ...). This is brittle across OS updates and can cause App Store rejection. Prefer a public approach (e.g., accept the default divider, adjust layout/background to visually mask it, or use a supported dividerStyle/appearance API) and remove the private selector call.
| // Clear the divider color via ObjC messaging (private API). | |
| let selector = NSSelectorFromString("setDividerColor:") | |
| if splitView.responds(to: selector) { | |
| splitView.perform(selector, with: NSColor.clear) | |
| } |
| @State private var isEditing = false | ||
| @State private var editedName = "" | ||
| @FocusState private var isTextFieldFocused: Bool | ||
| @Environment(\.colorScheme) private var colorScheme |
There was a problem hiding this comment.
@Environment(\.colorScheme) private var colorScheme is declared but never used in SidebarSectionHeaderView, which will trigger an unused-variable warning. Please remove it (or use it if it was intended to drive styling).
| @Environment(\.colorScheme) private var colorScheme |
| private var contentAndSidebarLayout: AnyView { | ||
| // On macOS 26, use NavigationSplitView so the system recognizes | ||
| // the sidebar column and applies native Liquid Glass treatment. | ||
| if #available(macOS 26.0, *) { | ||
| return AnyView( | ||
| NavigationSplitView(columnVisibility: Binding( | ||
| get: { sidebarState.isVisible ? .all : .detailOnly }, |
There was a problem hiding this comment.
The PR title/description focuses on fixing Ghostty surface theme state after config reload, but this diff also introduces substantial unrelated changes (macOS 26 NavigationSplitView/Liquid Glass work, toolbar controls, sidebar sections, config auto-apply, etc.). Please either update the PR description to cover these additions (including rationale/test plan) or split the changes into separate PRs to keep review and risk manageable.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21e9e70. Configure here.
| if #available(macOS 26.0, *) { | ||
| NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
Notifications popover dismiss/query broken on macOS 26
Medium Severity
On macOS 26, toggleNotificationsPopover was updated to use a NotificationCenter notification to drive the SwiftUI-managed isNotificationsPopoverPresented state, but dismissNotificationsPopoverIfShown and isNotificationsPopoverShown still delegate exclusively to titlebarAccessoryController, which won't own the popover on macOS 26. This means Escape-to-dismiss (line 10882) won't work and the keyboard-event consumption guard (line 10889) will always evaluate to false, letting keystrokes leak into the terminal while the popover is open.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 21e9e70. Configure here.
| // sidebarLayout reads sectionRevision internally, establishing the | ||
| // SwiftUI dependency — no separate read needed here. | ||
| let layout = tabManager.sidebarLayout | ||
| let allOrdered = layout.allWorkspacesInOrder |
There was a problem hiding this comment.
Unused allOrdered variable is dead code
Low Severity
allOrdered is computed from layout.allWorkspacesInOrder but never referenced anywhere. The sidebar body iterates layout.pinnedWorkspaces, layout.ungroupedWorkspaces, and layout.sectionGroups directly, making this a dead store.
Reviewed by Cursor Bugbot for commit 21e9e70. Configure here.
| } | ||
|
|
||
| var body: some Scene { | ||
| WindowGroup { mainWindowContent } |
There was a problem hiding this comment.
.windowStyle(.hiddenTitleBar) removed for all macOS versions
Medium Severity
The SwiftUI .windowStyle(.hiddenTitleBar) modifier was dropped during the mainWindowContent extraction and not conditionally re-applied for pre-macOS 26. While NSWindow properties (titlebarAppearsTransparent, titleVisibility) and fullSizeContentView partially compensate, .windowStyle(.hiddenTitleBar) also affects SwiftUI's own layout calculations and window creation, potentially causing subtle layout differences on macOS 13–15.
Reviewed by Cursor Bugbot for commit 21e9e70. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
Sources/SessionPersistence.swift (1)
359-359: Consider defaultingsectionsto[]after decode.Optional works for backward compatibility, but a non-optional in-memory shape (
[]when missing) reduces nil checks and downstream branching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionPersistence.swift` at line 359, The decoded model leaves var sections: [SessionSidebarSectionSnapshot]? nil; after decoding in SessionPersistence's Decodable init (or wherever the JSON decoding occurs), set sections to an empty array when missing (e.g., assign sections = sections ?? []) so the in-memory shape is non-nil and callers don't need to handle optional sections; reference the sections property and the SessionSidebarSectionSnapshot type when making the change.Sources/Workspace.swift (1)
6778-6803:hideTabBarnow describes only half of this branch.It also controls split-button visibility, so a broader name would make the 26+ appearance policy easier to read the next time this logic changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 6778 - 6803, The boolean hideTabBar in bonsplitAppearance is misleading because it also controls showSplitButtons; rename it to something reflecting both effects (e.g., useCompactChrome or hideChromeControls) and update all uses in that function: set the new variable based on the macOS availability check, use it for tabBarHeight (compact -> 0 else 33) and for showSplitButtons (show when not compact), keeping splitButtonTooltips, enableAnimations, and chromeColors unchanged; ensure the variable name is updated in the BonsplitConfiguration.Appearance initializer so the intent is clear.Sources/CmuxConfig.swift (1)
447-458: Only the first matching auto-apply command is executed.The use of
.first(where:)means if multiple commands haveautoApply: trueandtarget: .current, only the first one in load order (local config first, then global) will execute. This is likely intentional but worth documenting in user-facing config documentation so users understand the precedence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 447 - 458, The current code uses loadedCommands.first(where: { $0.autoApply == true && $0.workspace?.target == .current }) so only a single matching autoApply command runs; change this to find all matches (e.g., use loadedCommands.filter(...)) and iterate over each matching command, calling CmuxConfigExecutor.execute(...) for each and referencing commandSourcePaths[command.id] for the configSourcePath; keep inserting workspace.id into autoAppliedWorkspaceIds once per workspace and preserve loadedCommands order so local configs still take precedence.Sources/TabManager.swift (1)
5962-5970: Deduplicate restored workspace membership across sections.Restore currently filters unknown IDs but does not enforce one-section-per-workspace. A malformed or legacy snapshot with duplicates can restore the same workspace into multiple sections.
Suggested hardening
// Restore sidebar sections, filtering out workspace IDs that weren't restored. let restoredTabIds = Set(newTabs.map(\.id)) +var assignedWorkspaceIds: Set<UUID> = [] let restoredSections: [SidebarSection] = (snapshot.sections ?? []).map { sectionSnapshot in SidebarSection( id: sectionSnapshot.id, name: sectionSnapshot.name, isCollapsed: sectionSnapshot.isCollapsed, - workspaceIds: sectionSnapshot.workspaceIds.filter { restoredTabIds.contains($0) } + workspaceIds: sectionSnapshot.workspaceIds.filter { + restoredTabIds.contains($0) && assignedWorkspaceIds.insert($0).inserted + } ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TabManager.swift` around lines 5962 - 5970, The restore logic can place the same workspace ID into multiple SidebarSection instances; update the restoredSections construction to deduplicate and enforce one-section-per-workspace by tracking assigned IDs as you map snapshot.sections: create a mutable Set (e.g., assignedIds) outside the map, then for each sectionSnapshot compute workspaceIdsFiltered = sectionSnapshot.workspaceIds.filter { restoredTabIds.contains($0) && !assignedIds.contains($0) }, add those filtered IDs into assignedIds, and use workspaceIdsFiltered when creating the SidebarSection (referencing restoredTabIds, SidebarSection, restoredSections, and snapshot.sections).Sources/ContentView.swift (1)
15768-15803: Avoid full split-view tree traversal on everyupdateNSView.
updateNSViewcallsscheduleHide()each refresh; this can repeatedly walk the full window hierarchy. Consider patch-once-per-window or throttling.Lightweight guard to reduce repeated work
`@available`(macOS 26.0, *) final class SplitViewDividerHiderView: NSView { + private weak var lastPatchedWindow: NSWindow? + private var didPatchInCurrentWindow = false + override func viewDidMoveToWindow() { super.viewDidMoveToWindow() + if window !== lastPatchedWindow { + lastPatchedWindow = window + didPatchInCurrentWindow = false + } scheduleHide() } func scheduleHide() { DispatchQueue.main.async { [weak self] in - self?.hideDividers() + guard let self else { return } + guard !self.didPatchInCurrentWindow else { return } + self.hideDividers() + self.didPatchInCurrentWindow = true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 15768 - 15803, The current updateNSView(_:context:) → scheduleHide() flow causes patchSplitViews(in:) to traverse the entire window every refresh; change it to patch each window only once (or throttle repeated runs) by tracking which NSWindow instances have already been patched: add a static weak set/NSHashTable or an associated-object flag keyed on the NSWindow and check it in hideDividers()/patchSplitViews(in:) (or before scheduling work in scheduleHide()); also clear the flag if the window changes/closes (hook viewDidMoveToWindow / windowWillClose) so new windows get patched. This keeps the existing symbols (SplitViewDividerHiderView, updateNSView, scheduleHide, hideDividers, patchSplitViews, viewDidMoveToWindow) but prevents repeated full-tree traversals per refresh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 10155-10162: The macOS 26 change removed attaching titlebar
accessory views in attachUpdateAccessory(to window: NSWindow) but left
notifications toggling via a global NotificationCenter broadcast (object: nil),
causing all ContentView subscribers to toggle and desync with controller-backed
APIs; fix this by introducing a window-scoped source-of-truth for the macOS-26
path and wiring everything to it: update ContentView to filter notification
toggle events by the target NSWindow (or a unique window identifier) instead of
listening to object: nil, and modify showNotificationsPopoverFromMenuBar(),
dismissNotificationsPopoverIfShown(), and isNotificationsPopoverShown() to
consult/update the new macOS-26 window-scoped state (falling back to
titlebarAccessoryController behavior on older macOS) so controller-backed
methods remain consistent with visible popovers.
In `@Sources/CmuxConfigExecutor.swift`:
- Around line 96-112: The .current branch in CmuxConfigExecutor bypasses the
restart handling and always closes panels and applies layout; update the branch
to consult wsDef.restart before mutating the workspace: retrieve the restart
policy from wsDef (e.g., .ignore, .confirm, .always) at the start of the
wsDef.target == .current block and short-circuit appropriately (for .ignore
return without changing panels/layout; for .confirm invoke the existing confirm
prompt/path used by the restart switch to get user consent) and only perform
setCustomTitle/setCustomColor/closing panels/applyCustomLayout when the restart
policy permits; use the same logic/code path as the later restart switch so
behavior is consistent with other targets.
In `@Sources/ContentView.swift`:
- Around line 15795-15799: Remove the use of the private AppKit selector
("setDividerColor:") invoked via splitView.perform(...) in the ContentView.swift
block (the selector variable and the responds/to/perform code) and instead rely
only on public APIs: set splitView.dividerStyle = .thin and, if your deployment
target supports it, set splitView.dividerColor via the public NSSplitView API;
if dividerColor is unavailable for your target OS, remove the private workaround
and add a short TODO comment referencing splitView.dividerStyle and the need to
implement a custom NSView-based separator or document the migration plan so we
avoid private API usage.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9136-9148: The macOS 26 branch unconditionally subtracts a 6pt
inset when computing scrollFrame which can yield negative sizes or over-shifted
origins during tiny/intermediate layouts; inside the if `#available`(macOS 26.0,
*) block, clamp the inset before building scrollFrame (e.g., cap inset to no
more than half the bounds.width and bounds.height or ensure bounds.width - inset
>= 0 and bounds.height - inset >= 0) and then use that clamped inset when
computing x, width and height so scrollFrame never becomes negative or moves the
origin incorrectly.
- Around line 4448-4456: reapplyColorSchemeAndConfig currently unwraps `surface`
directly and calls `ghostty_surface_update_config`, which can dereference a
freed native pointer; instead obtain the runtime-safe surface via the wrapper
`liveSurfaceForGhosttyAccess(reason:)` before any native calls—use
`attachedView` to ensure view exists, call `liveSurfaceForGhosttyAccess(reason:
"reapplyColorSchemeAndConfig")` to get a guarded surface, and only then call
`ghostty_surface_update_config(surface, config)` (and bail out if the
live-surface wrapper returns nil) so you never call into Ghostty with a
freed/native-invalid surface.
In `@Sources/SessionPersistence.swift`:
- Around line 331-334: SessionWorkspaceSnapshot.id being optional is okay for
legacy decoding but must be enforced non-nil when creating/writing new
snapshots: add a guard that checks SessionWorkspaceSnapshot.id is non-nil in the
code path that persists/encodes new snapshots (e.g., the function that writes or
encodes snapshots such as the snapshot persistence/serialize method), and if nil
throw/return an error (or assert) so callers must supply the live workspace
UUID; keep the optional only for decoding but ensure any constructor or save
method validates id and refuses to write snapshots without an id.
In `@Sources/TabManager.swift`:
- Around line 2686-2792: reorderWorkspace(...) must ignore attempts to perform
flat-array reorders for workspaces that are inside a section; add an early
return at the start of the reorderWorkspace(tabId:toIndex:) implementation that
checks sectionForWorkspace(tabId) and returns immediately if it yields a
SidebarSection (i.e., the workspace is grouped), so the function no longer
mutates tabs ordering for sectioned items.
In `@Sources/TerminalWindowPortal.swift`:
- Around line 1499-1515: The current corner-radius heuristic in
TerminalWindowPortal using targetFrame.origin.x > 20 is too broad and rounds
inner corners for right-side panes; replace this x-offset check with a reliable
detection of leading-edge panes (reuse the existing sidebar-edge detection
helper or API already present in this class/module) so you only set
desiredRadius and maskedCorners when the pane is actually on the outer leading
edge; update the conditional around hostedView.layer?.cornerRadius and
maskedCorners to call that helper (or a method like isPaneOnLeadingEdge) instead
of comparing origin.x, keeping the macOS 26 availability guard and the
CATransaction-safe update logic.
In `@Sources/WindowToolbarController.swift`:
- Around line 164-177: The WindowToolbarController implementation is dead
(defines toolbarDefaultItemIdentifiers, toolbarAllowedItemIdentifiers and
toolbar item identifiers such as sidebarToggleIdentifier,
notificationsIdentifier, newTabIdentifier and has a start(tabManager:) method
that is never called); either remove the entire WindowToolbarController class
and its identifier constants to eliminate unused NSToolbar code, or wire it up
by instantiating WindowToolbarController and calling start(tabManager:) during
window/app setup (e.g., where NSWindow is created), attach its NSToolbar to the
window, and ensure toolbarDefaultItemIdentifiers/toolbarAllowedItemIdentifiers
are used by setting the toolbar.delegate to the controller so the defined items
(sidebarToggleIdentifier, notificationsIdentifier, newTabIdentifier,
commandItemIdentifier) become active.
- Around line 230-243: toggleSidebarAction toggles the global sidebarState on
AppDelegate which affects the key/active window rather than the window whose
toolbar was clicked; update toggleSidebarAction to call
AppDelegate.shared?.toggleSidebarInActiveMainWindow() so it uses the established
cross-window routing pattern (see newTabAction's use of
addWorkspaceInPreferredMainWindow as the model). For toggleNotificationsAction,
decide if notifications should be routed per-window—if so, replace the direct
call to AppDelegate.shared?.toggleNotificationsPopover(animated:) with the
equivalent per-window routing helper (mirror the active-main-window routing used
by toggleSidebarInActiveMainWindow); otherwise leave it as-is. Ensure you only
change toggleSidebarAction (and toggleNotificationsAction if applying per-window
routing) and keep newTabAction unchanged as the correct pattern example.
In `@vendor/bonsplit`:
- Line 1: The parent repo points to a submodule commit
(cfff8a9318f8131604681e4cb86c11925e01bf1e) that hasn't been pushed to the
bonsplit remote; push that commit to the bonsplit repository's main branch (push
the local bonsplit commit to manaflow-ai/bonsplit main), then update the parent
repo pointer. After pushing, verify the commit is reachable from origin/main
using git merge-base --is-ancestor cfff8a9318f8131604681e4cb86c11925e01bf1e
origin/main (run this inside the bonsplit checkout), and only then update/commit
the submodule pointer in the parent repo.
---
Nitpick comments:
In `@Sources/CmuxConfig.swift`:
- Around line 447-458: The current code uses loadedCommands.first(where: {
$0.autoApply == true && $0.workspace?.target == .current }) so only a single
matching autoApply command runs; change this to find all matches (e.g., use
loadedCommands.filter(...)) and iterate over each matching command, calling
CmuxConfigExecutor.execute(...) for each and referencing
commandSourcePaths[command.id] for the configSourcePath; keep inserting
workspace.id into autoAppliedWorkspaceIds once per workspace and preserve
loadedCommands order so local configs still take precedence.
In `@Sources/ContentView.swift`:
- Around line 15768-15803: The current updateNSView(_:context:) → scheduleHide()
flow causes patchSplitViews(in:) to traverse the entire window every refresh;
change it to patch each window only once (or throttle repeated runs) by tracking
which NSWindow instances have already been patched: add a static weak
set/NSHashTable or an associated-object flag keyed on the NSWindow and check it
in hideDividers()/patchSplitViews(in:) (or before scheduling work in
scheduleHide()); also clear the flag if the window changes/closes (hook
viewDidMoveToWindow / windowWillClose) so new windows get patched. This keeps
the existing symbols (SplitViewDividerHiderView, updateNSView, scheduleHide,
hideDividers, patchSplitViews, viewDidMoveToWindow) but prevents repeated
full-tree traversals per refresh.
In `@Sources/SessionPersistence.swift`:
- Line 359: The decoded model leaves var sections:
[SessionSidebarSectionSnapshot]? nil; after decoding in SessionPersistence's
Decodable init (or wherever the JSON decoding occurs), set sections to an empty
array when missing (e.g., assign sections = sections ?? []) so the in-memory
shape is non-nil and callers don't need to handle optional sections; reference
the sections property and the SessionSidebarSectionSnapshot type when making the
change.
In `@Sources/TabManager.swift`:
- Around line 5962-5970: The restore logic can place the same workspace ID into
multiple SidebarSection instances; update the restoredSections construction to
deduplicate and enforce one-section-per-workspace by tracking assigned IDs as
you map snapshot.sections: create a mutable Set (e.g., assignedIds) outside the
map, then for each sectionSnapshot compute workspaceIdsFiltered =
sectionSnapshot.workspaceIds.filter { restoredTabIds.contains($0) &&
!assignedIds.contains($0) }, add those filtered IDs into assignedIds, and use
workspaceIdsFiltered when creating the SidebarSection (referencing
restoredTabIds, SidebarSection, restoredSections, and snapshot.sections).
In `@Sources/Workspace.swift`:
- Around line 6778-6803: The boolean hideTabBar in bonsplitAppearance is
misleading because it also controls showSplitButtons; rename it to something
reflecting both effects (e.g., useCompactChrome or hideChromeControls) and
update all uses in that function: set the new variable based on the macOS
availability check, use it for tabBarHeight (compact -> 0 else 33) and for
showSplitButtons (show when not compact), keeping splitButtonTooltips,
enableAnimations, and chromeColors unchanged; ensure the variable name is
updated in the BonsplitConfiguration.Appearance initializer so the intent is
clear.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a88a287-b84d-43f2-9287-334fe1c70915
📒 Files selected for processing (15)
GhosttyTabs.xcodeproj/project.pbxprojSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigExecutor.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/SessionPersistence.swiftSources/SidebarSection.swiftSources/TabManager.swiftSources/TerminalWindowPortal.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/WindowToolbarController.swiftSources/Workspace.swiftSources/cmuxApp.swiftvendor/bonsplit
| func attachUpdateAccessory(to window: NSWindow) { | ||
| if #available(macOS 26.0, *) { | ||
| // On macOS 26, toolbar buttons are native SwiftUI .toolbar items | ||
| // in the NavigationSplitView. Skip attaching the old titlebar | ||
| // accessory views, but the controller is already started (for | ||
| // notifications popover support). | ||
| return | ||
| } |
There was a problem hiding this comment.
Unify the macOS 26 notifications popover behind one window-scoped source of truth.
These changes move 26+ away from titlebarAccessoryController by skipping attachment and toggling through a global notification with object: nil. That creates two regressions: every ContentView subscriber can toggle at once, and the rest of AppDelegate still uses controller-backed show/dismiss/query methods that no longer reflect the visible popover. In practice this can fan the popover out across all windows and break menu-bar/Escape handling on macOS 26+.
🧭 Possible starting point
func toggleNotificationsPopover(animated: Bool = true, anchorView: NSView? = nil) {
if `#available`(macOS 26.0, *) {
- NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil)
+ let targetWindow = anchorView?.window ?? NSApp.keyWindow ?? NSApp.mainWindow
+ NotificationCenter.default.post(
+ name: Self.toggleNotificationsPopoverNotification,
+ object: targetWindow
+ )
return
}
titlebarAccessoryController.toggleNotificationsPopover(animated: animated, anchorView: anchorView)
}Companion changes are still needed so Sources/ContentView.swift filters by the addressed window, and so showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and isNotificationsPopoverShown() use that same macOS-26-backed state.
Also applies to: 10171-10179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 10155 - 10162, The macOS 26 change
removed attaching titlebar accessory views in attachUpdateAccessory(to window:
NSWindow) but left notifications toggling via a global NotificationCenter
broadcast (object: nil), causing all ContentView subscribers to toggle and
desync with controller-backed APIs; fix this by introducing a window-scoped
source-of-truth for the macOS-26 path and wiring everything to it: update
ContentView to filter notification toggle events by the target NSWindow (or a
unique window identifier) instead of listening to object: nil, and modify
showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and
isNotificationsPopoverShown() to consult/update the new macOS-26 window-scoped
state (falling back to titlebarAccessoryController behavior on older macOS) so
controller-backed methods remain consistent with visible popovers.
| if wsDef.target == .current { | ||
| guard let current = tabManager.selectedWorkspace else { return } | ||
| current.setCustomTitle(workspaceName) | ||
| if let color = wsDef.color { | ||
| current.setCustomColor(color) | ||
| } | ||
| if let layout = wsDef.layout { | ||
| // Close all panels except the focused one so applyCustomLayout | ||
| // starts from a single pane and doesn't stack on existing splits. | ||
| let keep = current.focusedPanelId | ||
| let panelIdsToClose = current.panels.keys.filter { $0 != keep } | ||
| for panelId in panelIdsToClose { | ||
| current.closePanel(panelId, force: true) | ||
| } | ||
| current.applyCustomLayout(layout, baseCwd: resolvedCwd) | ||
| } | ||
| return |
There was a problem hiding this comment.
target: .current bypasses the configured restart policy.
This branch returns before the restart switch below, so restart: .ignore and restart: .confirm still force-close non-focused panels and apply the new layout. That makes reload/auto-apply destructive in cases that should currently select-or-prompt instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/CmuxConfigExecutor.swift` around lines 96 - 112, The .current branch
in CmuxConfigExecutor bypasses the restart handling and always closes panels and
applies layout; update the branch to consult wsDef.restart before mutating the
workspace: retrieve the restart policy from wsDef (e.g., .ignore, .confirm,
.always) at the start of the wsDef.target == .current block and short-circuit
appropriately (for .ignore return without changing panels/layout; for .confirm
invoke the existing confirm prompt/path used by the restart switch to get user
consent) and only perform setCustomTitle/setCustomColor/closing
panels/applyCustomLayout when the restart policy permits; use the same
logic/code path as the later restart switch so behavior is consistent with other
targets.
| // Clear the divider color via ObjC messaging (private API). | ||
| let selector = NSSelectorFromString("setDividerColor:") | ||
| if splitView.responds(to: selector) { | ||
| splitView.perform(selector, with: NSColor.clear) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there are no private selector invocations for split-view divider color.
# Expected result after fix: no matches.
rg -n --type=swift 'NSSelectorFromString\("setDividerColor:"\)|perform\(\s*selector,\s*with:'Repository: manaflow-ai/cmux
Length of output: 524
🏁 Script executed:
sed -n '15790,15810p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 913
🏁 Script executed:
# Search for additional context about NSSplitView divider manipulation in the codebase
rg -n -B 3 -A 3 'setDividerColor' Sources/Repository: manaflow-ai/cmux
Length of output: 646
🏁 Script executed:
# Check if there are comments or documentation explaining why setDividerColor is needed
rg -n -B 5 'NSSelectorFromString\("setDividerColor' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 421
🏁 Script executed:
# Search for other divider-related properties or methods on NSSplitView in the codebase
rg -n 'divider' Sources/ContentView.swift | head -20Repository: manaflow-ai/cmux
Length of output: 967
🏁 Script executed:
# Check if there are any appearance or theme-related calls that might control divider color publicly
rg -n 'NSAppearance|dividerColor|separatorColor' Sources/Repository: manaflow-ai/cmux
Length of output: 3747
🏁 Script executed:
# Look for context about why the divider needs to be cleared/made invisible
rg -n -B 10 'patchSplitViews' Sources/ContentView.swift | head -30Repository: manaflow-ai/cmux
Length of output: 1017
Remove private AppKit API usage for divider color.
The private setDividerColor: selector via perform() can trigger App Store rejections and binary compatibility issues. This approach is not maintainable across OS updates.
However, note that simply removing this code while keeping only dividerStyle = .thin may not fully hide the dividers as intended—dividerStyle controls thickness/appearance but not visibility/color. Consider:
- Using
NSSplitView.dividerColorif it's settable publicly in your target OS versions - Exploring alternative layout approaches (e.g., custom
NSViewwith manual layout) if public APIs don't support transparent dividers - If a workaround is unavoidable, document the rationale and test against the latest SDK to catch deprecation warnings early
Remove the private selector code and use only public APIs, or document why a private workaround is necessary with a clear plan to migrate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 15795 - 15799, Remove the use of the
private AppKit selector ("setDividerColor:") invoked via splitView.perform(...)
in the ContentView.swift block (the selector variable and the
responds/to/perform code) and instead rely only on public APIs: set
splitView.dividerStyle = .thin and, if your deployment target supports it, set
splitView.dividerColor via the public NSSplitView API; if dividerColor is
unavailable for your target OS, remove the private workaround and add a short
TODO comment referencing splitView.dividerStyle and the need to implement a
custom NSView-based separator or document the migration plan so we avoid private
API usage.
| func reapplyColorSchemeAndConfig() { | ||
| guard let surface, let view = attachedView else { return } | ||
| // Force the surface to re-derive its config with its current conditional state. | ||
| // ghostty_surface_set_color_scheme has an internal dedup that skips when the | ||
| // scheme hasn't changed, but after a config reload the underlying theme data | ||
| // may have changed. ghostty_surface_update_config bypasses that dedup. | ||
| if let config = GhosttyApp.shared.config { | ||
| ghostty_surface_update_config(surface, config) | ||
| } |
There was a problem hiding this comment.
Use the live-surface guard before calling into Ghostty here.
Line 4449 unwraps surface directly, but this path dereferences the native pointer during a reload pass. That bypasses liveSurfaceForGhosttyAccess(reason:), so a wrapper whose runtime surface was freed out-of-band can still reach ghostty_surface_update_config.
🛡️ Proposed fix
- func reapplyColorSchemeAndConfig() {
- guard let surface, let view = attachedView else { return }
+ `@MainActor`
+ func reapplyColorSchemeAndConfig() {
+ guard let view = attachedView,
+ let surface = liveSurfaceForGhosttyAccess(reason: "reapplyColorSchemeAndConfig") else { return }
// Force the surface to re-derive its config with its current conditional state.
// ghostty_surface_set_color_scheme has an internal dedup that skips when the
// scheme hasn't changed, but after a config reload the underlying theme data
// may have changed. ghostty_surface_update_config bypasses that dedup.
if let config = GhosttyApp.shared.config {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 4448 - 4456,
reapplyColorSchemeAndConfig currently unwraps `surface` directly and calls
`ghostty_surface_update_config`, which can dereference a freed native pointer;
instead obtain the runtime-safe surface via the wrapper
`liveSurfaceForGhosttyAccess(reason:)` before any native calls—use
`attachedView` to ensure view exists, call `liveSurfaceForGhosttyAccess(reason:
"reapplyColorSchemeAndConfig")` to get a guarded surface, and only then call
`ghostty_surface_update_config(surface, config)` (and bail out if the
live-surface wrapper returns nil) so you never call into Ghostty with a
freed/native-invalid surface.
| // On macOS 26, inset the scroll view on the leading and top edges | ||
| // to keep terminal text within the rounded corner safe zone. | ||
| // Only leading corners are rounded (when sidebar is visible), so | ||
| // right/bottom edges use no inset to maximize terminal real estate. | ||
| let scrollFrame: CGRect | ||
| if #available(macOS 26.0, *) { | ||
| let inset: CGFloat = 6 | ||
| scrollFrame = CGRect( | ||
| x: bounds.origin.x + inset, | ||
| y: bounds.origin.y, | ||
| width: bounds.width - inset, | ||
| height: bounds.height - inset | ||
| ) |
There was a problem hiding this comment.
Clamp the macOS 26 inset before building the scroll frame.
Lines 9142-9147 subtract a fixed 6pt inset unconditionally. During split churn or tiny intermediate layouts, that can produce negative frame sizes and an over-shifted leading origin, which then bleeds into the scroll/content-size reconciliation path.
📐 Proposed fix
if `#available`(macOS 26.0, *) {
let inset: CGFloat = 6
+ let leadingInset = min(inset, bounds.width)
+ let topInset = min(inset, bounds.height)
scrollFrame = CGRect(
- x: bounds.origin.x + inset,
+ x: bounds.origin.x + leadingInset,
y: bounds.origin.y,
- width: bounds.width - inset,
- height: bounds.height - inset
+ width: max(0, bounds.width - leadingInset),
+ height: max(0, bounds.height - topInset)
)
} else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 9136 - 9148, The macOS 26
branch unconditionally subtracts a 6pt inset when computing scrollFrame which
can yield negative sizes or over-shifted origins during tiny/intermediate
layouts; inside the if `#available`(macOS 26.0, *) block, clamp the inset before
building scrollFrame (e.g., cap inset to no more than half the bounds.width and
bounds.height or ensure bounds.width - inset >= 0 and bounds.height - inset >=
0) and then use that clamped inset when computing x, width and height so
scrollFrame never becomes negative or moves the origin incorrectly.
| // MARK: - Sidebar Sections | ||
|
|
||
| /// Subscribe to every section's `objectWillChange` so that any property | ||
| /// mutation (collapse, membership, name) bumps `sectionRevision` and | ||
| /// triggers a SwiftUI re-render of the sidebar layout. | ||
| private func rebindSectionObservers() { | ||
| sectionObserverCancellables.removeAll() | ||
| for section in sections { | ||
| section.objectWillChange | ||
| .receive(on: RunLoop.main) | ||
| .sink { [weak self] _ in | ||
| self?.sectionRevision &+= 1 | ||
| } | ||
| .store(in: §ionObserverCancellables) | ||
| } | ||
| } | ||
|
|
||
| private func notifySectionChange() { | ||
| sectionRevision &+= 1 | ||
| } | ||
|
|
||
| @discardableResult | ||
| func createSection(name: String) -> SidebarSection { | ||
| let section = SidebarSection(name: name) | ||
| sections.append(section) | ||
| // Delay so SwiftUI renders the new SidebarSectionHeaderView | ||
| // (and subscribes to $pendingRenameSectionId) before we emit. | ||
| DispatchQueue.main.async { [weak self] in | ||
| self?.pendingRenameSectionId = section.id | ||
| } | ||
| return section | ||
| } | ||
|
|
||
| func renameSection(sectionId: UUID, name: String) { | ||
| guard let section = sections.first(where: { $0.id == sectionId }) else { return } | ||
| section.name = name | ||
| notifySectionChange() | ||
| } | ||
|
|
||
| func deleteSection(sectionId: UUID) { | ||
| sections.removeAll { $0.id == sectionId } | ||
| } | ||
|
|
||
| func reorderSection(sectionId: UUID, toIndex targetIndex: Int) { | ||
| guard let currentIndex = sections.firstIndex(where: { $0.id == sectionId }) else { return } | ||
| let clamped = max(0, min(targetIndex, sections.count - 1)) | ||
| guard currentIndex != clamped else { return } | ||
| let section = sections.remove(at: currentIndex) | ||
| sections.insert(section, at: clamped) | ||
| } | ||
|
|
||
| func moveWorkspaceToSection(tabId: UUID, sectionId: UUID, atIndex: Int? = nil) { | ||
| // Validate destination exists before modifying any state. | ||
| guard let section = sections.first(where: { $0.id == sectionId }) else { return } | ||
| // Remove from any existing section first | ||
| for s in sections { | ||
| s.removeWorkspace(tabId) | ||
| } | ||
| section.addWorkspace(tabId, at: atIndex) | ||
| notifySectionChange() | ||
| } | ||
|
|
||
| func removeWorkspaceFromSection(tabId: UUID) { | ||
| for section in sections { | ||
| section.removeWorkspace(tabId) | ||
| } | ||
| notifySectionChange() | ||
| } | ||
|
|
||
| func sectionForWorkspace(_ tabId: UUID) -> SidebarSection? { | ||
| sections.first { $0.contains(tabId) } | ||
| } | ||
|
|
||
| var sidebarLayout: SidebarLayout { | ||
| // Read sectionRevision to establish a SwiftUI dependency so the | ||
| // layout is recomputed whenever any section property changes. | ||
| let _ = sectionRevision | ||
| let tabById = Dictionary(uniqueKeysWithValues: tabs.map { ($0.id, $0) }) | ||
| let pinnedWorkspaces = tabs.filter { $0.isPinned } | ||
|
|
||
| // Workspace IDs that are in some section (and not pinned) | ||
| var sectionedIds = Set<UUID>() | ||
| let sectionGroups: [SidebarLayout.SectionGroup] = sections.map { section in | ||
| let workspaces = section.workspaceIds.compactMap { id -> Workspace? in | ||
| guard let ws = tabById[id], !ws.isPinned else { return nil } | ||
| return ws | ||
| } | ||
| for ws in workspaces { | ||
| sectionedIds.insert(ws.id) | ||
| } | ||
| return SidebarLayout.SectionGroup(section: section, workspaces: workspaces) | ||
| } | ||
|
|
||
| let ungroupedWorkspaces = tabs.filter { !$0.isPinned && !sectionedIds.contains($0.id) } | ||
| return SidebarLayout( | ||
| pinnedWorkspaces: pinnedWorkspaces, | ||
| ungroupedWorkspaces: ungroupedWorkspaces, | ||
| sectionGroups: sectionGroups | ||
| ) | ||
| } | ||
|
|
||
| private func cleanupSectionsForRemovedWorkspace(_ workspaceId: UUID) { | ||
| for section in sections { | ||
| section.removeWorkspace(workspaceId) | ||
| } | ||
| notifySectionChange() | ||
| } |
There was a problem hiding this comment.
Block flat reorders for grouped workspaces.
Now that section membership is first-class, reorderWorkspace(...) should reject grouped tabs. Otherwise flat-array reorder calls can mutate tabs order for sectioned items and conflict with section-aware ordering behavior.
Suggested fix
`@discardableResult`
func reorderWorkspace(tabId: UUID, toIndex targetIndex: Int) -> Bool {
+ guard sectionForWorkspace(tabId) == nil else { return false }
guard let currentIndex = tabs.firstIndex(where: { $0.id == tabId }) else { return false }
if tabs.count <= 1 { return true }
let workspace = tabs[currentIndex]
let clamped = clampedReorderIndex(for: workspace, targetIndex: targetIndex)
if currentIndex == clamped { return true }
tabs.remove(at: currentIndex)
tabs.insert(workspace, at: clamped)
return true
}
`@discardableResult`
func reorderWorkspace(tabId: UUID, before beforeId: UUID? = nil, after afterId: UUID? = nil) -> Bool {
+ guard sectionForWorkspace(tabId) == nil else { return false }
+ if let beforeId, sectionForWorkspace(beforeId) != nil { return false }
+ if let afterId, sectionForWorkspace(afterId) != nil { return false }
guard tabs.contains(where: { $0.id == tabId }) else { return false }
if let beforeId {
guard let idx = tabs.firstIndex(where: { $0.id == beforeId }) else { return false }
return reorderWorkspace(tabId: tabId, toIndex: idx)
}Based on learnings: reorderWorkspace(...) must reject grouped tabs (early return); flat-array reorders are not allowed for workspaces inside a group.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TabManager.swift` around lines 2686 - 2792, reorderWorkspace(...)
must ignore attempts to perform flat-array reorders for workspaces that are
inside a section; add an early return at the start of the
reorderWorkspace(tabId:toIndex:) implementation that checks
sectionForWorkspace(tabId) and returns immediately if it yields a SidebarSection
(i.e., the workspace is grouped), so the function no longer mutates tabs
ordering for sectioned items.
| // On macOS 26, round the terminal's leading corners when a sidebar | ||
| // is visible to its left, matching the NavigationSplitView glass shape. | ||
| // Applied inside the CATransaction to prevent animation flicker. | ||
| if #available(macOS 26.0, *) { | ||
| // Detect sidebar presence via x-offset. The sidebar has a minimum | ||
| // width of 120pt, so any x > 20 reliably indicates a sidebar is | ||
| // to our left. This AppKit view cannot access SwiftUI SidebarState | ||
| // directly; the frame-based heuristic is the simplest reliable path. | ||
| let hasSidebarToLeft = targetFrame.origin.x > 20 | ||
| let desiredRadius: CGFloat = hasSidebarToLeft ? 16 : 0 | ||
| if hostedView.layer?.cornerRadius != desiredRadius { | ||
| hostedView.layer?.cornerRadius = desiredRadius | ||
| hostedView.layer?.maskedCorners = hasSidebarToLeft | ||
| ? [.layerMinXMinYCorner, .layerMinXMaxYCorner] | ||
| : [] | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't use pane X-offset as the sidebar signal.
Line 1507 will also be true for any non-leading split pane, so right-hand panes will get rounded inside corners even when the sidebar is hidden. Please gate this to panes that are actually on the outer leading edge, or reuse the existing sidebar-edge detection instead of a fixed > 20 heuristic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalWindowPortal.swift` around lines 1499 - 1515, The current
corner-radius heuristic in TerminalWindowPortal using targetFrame.origin.x > 20
is too broad and rounds inner corners for right-side panes; replace this
x-offset check with a reliable detection of leading-edge panes (reuse the
existing sidebar-edge detection helper or API already present in this
class/module) so you only set desiredRadius and maskedCorners when the pane is
actually on the outer leading edge; update the conditional around
hostedView.layer?.cornerRadius and maskedCorners to call that helper (or a
method like isPaneOnLeadingEdge) instead of comparing origin.x, keeping the
macOS 26 availability guard and the CATransaction-safe update logic.
| if #available(macOS 26.0, *) { | ||
| return [sidebarToggleIdentifier, notificationsIdentifier, newTabIdentifier, | ||
| .flexibleSpace, commandItemIdentifier] | ||
| } | ||
| return [commandItemIdentifier, .flexibleSpace] | ||
| } | ||
|
|
||
| func toolbarDefaultItemIdentifiers(_ toolbar: NSToolbar) -> [NSToolbarItem.Identifier] { | ||
| [commandItemIdentifier, .flexibleSpace] | ||
| if #available(macOS 26.0, *) { | ||
| return [sidebarToggleIdentifier, notificationsIdentifier, newTabIdentifier, | ||
| .flexibleSpace, commandItemIdentifier] | ||
| } | ||
| return [commandItemIdentifier, .flexibleSpace] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect macOS 26 toolbar composition across AppKit and SwiftUI layers.
# Expected: a single source of truth for sidebar/notifications/new-workspace controls.
rg -n -C6 'NavigationSplitView|toolbar\(removing:\s*\.sidebarToggle\)|ToolbarItemGroup|Image\(systemName:\s*"sidebar|Image\(systemName:\s*"bell|Image\(systemName:\s*"plus|newSurface|newWorkspace' Sources/ContentView.swift
rg -n -C6 'sidebarToggleIdentifier|notificationsIdentifier|newTabIdentifier|toolbarDefaultItemIdentifiers|toolbarAllowedItemIdentifiers' Sources/WindowToolbarController.swiftRepository: manaflow-ai/cmux
Length of output: 13141
🏁 Script executed:
# Find where WindowToolbarController is attached to the window
rg -n 'WindowToolbarController|NSWindow.*toolbar|window.toolbar' Sources/ --type swift -B2 -A2 | head -80Repository: manaflow-ai/cmux
Length of output: 3416
🏁 Script executed:
# Check AppDelegate window creation to see toolbar setup
rg -n 'createMainWindow|setWindowToolbar|NSToolbar' Sources/AppDelegate.swift -B3 -A3 | head -100Repository: manaflow-ai/cmux
Length of output: 2531
🏁 Script executed:
# Check if there's any code that conditionally disables SwiftUI toolbar on macOS 26
rg -n 'toolbar.*macOS.*26|#available.*macOS.*26.*toolbar' Sources/ContentView.swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Find where WindowToolbarController is created/instantiated
rg -n 'WindowToolbarController\(\)|WindowToolbarController\.shared|WindowToolbarController\.init' Sources/ --type swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check _isMinimal or visibility state in WindowToolbarController
rg -n '_isMinimal|_visible|setupMinimalToggle' Sources/WindowToolbarController.swift -B3 -A3Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Look for where ContentView's root window is created to see toolbar initialization order
rg -n 'ContentView\(\)|WindowScene|\.body.*ContentView' Sources/ContentView.swift | head -20Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check main window creation in AppDelegate to understand toolbar setup flow
sed -n '7038,7100p' Sources/AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 3074
🏁 Script executed:
# Search for WindowToolbarController creation/init in AppDelegate
rg -n 'WindowToolbarController' Sources/AppDelegate.swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check cmuxApp.swift for WindowToolbarController setup
rg -n 'WindowToolbarController' Sources/cmuxApp.swift -B3 -A3Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Look for where toolbar items are actually populated - check the full toolbar delegate method
sed -n '179,230p' Sources/WindowToolbarController.swiftRepository: manaflow-ai/cmux
Length of output: 3023
🏁 Script executed:
# Search more broadly for WindowToolbarController usage across entire repo
git ls-files -z | xargs -0 grep -l 'WindowToolbarController' 2>/dev/nullRepository: manaflow-ai/cmux
Length of output: 137
🏁 Script executed:
# Check if WindowToolbarController is even instantiated anywhere
rg 'WindowToolbarController()' --type swift -g '!Sources/WindowToolbarController.swift'Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Search for where attach method is called
rg 'attach\(to:' Sources/ --type swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 1310
🏁 Script executed:
# Check if there's a singleton or static property for WindowToolbarController
rg 'static.*WindowToolbarController|shared.*WindowToolbarController|WindowToolbarController\.shared' Sources/ --type swiftRepository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check the complete WindowToolbarController class to see if it's a singleton or how it's instantiated
head -100 Sources/WindowToolbarController.swiftRepository: manaflow-ai/cmux
Length of output: 3633
🏁 Script executed:
# Look at the top of AppDelegate to see if WindowToolbarController is created there
head -200 Sources/AppDelegate.swift | grep -n 'class AppDelegate\|init\|WindowToolbarController'Repository: manaflow-ai/cmux
Length of output: 246
🏁 Script executed:
# Check if there's any `@NSApplicationDelegateAdaptor` or initialization in cmuxApp
rg -n 'WindowToolbarController|NSApplicationDelegateAdaptor' Sources/cmuxApp.swift -B3 -A3Repository: manaflow-ai/cmux
Length of output: 711
🏁 Script executed:
# Find where WindowToolbarController.start is called
rg -n '\.start\(' Sources/ --type swift | grep -i 'windowtoolbar\|tabManager'Repository: manaflow-ai/cmux
Length of output: 339
🏁 Script executed:
# Look at AppDelegate initialization/window creation to see if WindowToolbarController is started
rg -n 'func application|didFinishLaunching|applicationDidFinishLaunching' Sources/AppDelegate.swift -A20 | head -50Repository: manaflow-ai/cmux
Length of output: 2325
🏁 Script executed:
# Check if there's a specific place where WindowToolbarController is instantiated and started
rg -n 'let.*toolbarController|var.*toolbarController|WindowToolbarController()' Sources/ --type swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 515
🏁 Script executed:
# Search for WindowToolbarController as a property in AppDelegate or elsewhere
rg -n 'var.*WindowToolbarController|let.*WindowToolbarController' Sources/ --type swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check the createMainWindow function more thoroughly to see if toolbar is set up there
sed -n '7038,7150p' Sources/AppDelegate.swift | grep -n -i 'toolbar\|window\|contentview'Repository: manaflow-ai/cmux
Length of output: 3437
🏁 Script executed:
# Look for where window's contentView is set in createMainWindow
sed -n '7100,7200p' Sources/AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 4095
🏁 Script executed:
# Do a comprehensive search for any instantiation or usage of WindowToolbarController
rg 'WindowToolbarController' Sources/ --type swiftRepository: manaflow-ai/cmux
Length of output: 164
🏁 Script executed:
# Check if WindowToolbarController is referenced in the entire project
git grep -n 'WindowToolbarController'Repository: manaflow-ai/cmux
Length of output: 761
Remove or complete the inactive WindowToolbarController implementation.
WindowToolbarController defines macOS 26 toolbar items (sidebar, notifications, new workspace) but is never instantiated or started in the codebase. Its start(tabManager:) method is never called, so the NSToolbar layer remains unused. Only ContentView's SwiftUI .toolbar() with ToolbarItemGroup(.primaryAction) is active. Either remove this dead code or complete the integration if it was intended for future use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/WindowToolbarController.swift` around lines 164 - 177, The
WindowToolbarController implementation is dead (defines
toolbarDefaultItemIdentifiers, toolbarAllowedItemIdentifiers and toolbar item
identifiers such as sidebarToggleIdentifier, notificationsIdentifier,
newTabIdentifier and has a start(tabManager:) method that is never called);
either remove the entire WindowToolbarController class and its identifier
constants to eliminate unused NSToolbar code, or wire it up by instantiating
WindowToolbarController and calling start(tabManager:) during window/app setup
(e.g., where NSWindow is created), attach its NSToolbar to the window, and
ensure toolbarDefaultItemIdentifiers/toolbarAllowedItemIdentifiers are used by
setting the toolbar.delegate to the controller so the defined items
(sidebarToggleIdentifier, notificationsIdentifier, newTabIdentifier,
commandItemIdentifier) become active.
| @objc private func toggleSidebarAction() { | ||
| _ = AppDelegate.shared?.sidebarState?.toggle() | ||
| } | ||
|
|
||
| @objc private func toggleNotificationsAction() { | ||
| _ = AppDelegate.shared?.toggleNotificationsPopover(animated: true) | ||
| } | ||
|
|
||
| @objc private func newTabAction() { | ||
| if let appDelegate = AppDelegate.shared { | ||
| if appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "toolbar.newTab") == nil { | ||
| appDelegate.openNewMainWindow(nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether toolbar actions are window-scoped or app-global.
# Expected: evidence that routing uses the clicked window context (or confirm it does not).
rg -n -C4 '\bsidebarState\b|func\s+addWorkspaceInPreferredMainWindow|func\s+openNewMainWindow|func\s+registerMainWindow' Sources/AppDelegate.swift
rg -n -C4 'toggleSidebarAction|toggleNotificationsAction|newTabAction|toolbar\(_:itemForItemIdentifier' Sources/WindowToolbarController.swift
rg -n -C4 'class\s+MainWindowRegistration|struct\s+MainWindowRegistration|mainWindow.*tabManager|active.*tabManager' Sources/AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 21055
🏁 Script executed:
# Verify WindowToolbarController ownership and window association
rg -n -B5 -A15 'class\s+WindowToolbarController|struct\s+WindowToolbarController' Sources/WindowToolbarController.swift
# Check if WindowToolbarController has a window property
rg -n 'var\s+window|weak.*window|let.*window' Sources/WindowToolbarController.swift
# Check how WindowToolbarController is instantiated and associated with windows
rg -n 'WindowToolbarController' Sources/AppDelegate.swift
# Check other toolbar-related usage in ContentView or window setup
rg -n 'WindowToolbarController' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 1096
🏁 Script executed:
wc -l Sources/WindowToolbarController.swiftRepository: manaflow-ai/cmux
Length of output: 101
🏁 Script executed:
cat -n Sources/WindowToolbarController.swiftRepository: manaflow-ai/cmux
Length of output: 11545
🏁 Script executed:
# Verify whether AppDelegate.sidebarState represents active window and check the contextForMainWindow routing pattern
rg -n -B3 -A3 'func contextForMainWindow|func preferredMainWindowContextForWorkspaceCreation|private func setActiveMainWindow' Sources/AppDelegate.swift | head -60Repository: manaflow-ai/cmux
Length of output: 896
🏁 Script executed:
# See the complete setActiveMainWindow implementation and its effects
rg -n -A30 'private func setActiveMainWindow' Sources/AppDelegate.swift | head -50Repository: manaflow-ai/cmux
Length of output: 1936
🏁 Script executed:
# Check addWorkspaceInPreferredMainWindow implementation for proper window routing
rg -n -A50 'func addWorkspaceInPreferredMainWindow' Sources/AppDelegate.swift | head -80Repository: manaflow-ai/cmux
Length of output: 2088
🏁 Script executed:
# Check if toggleSidebarInActiveMainWindow is actually being used and how it handles window routing correctly
rg -n -B5 -A25 'func toggleSidebarInActiveMainWindow' Sources/AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 1524
Use the existing cross-window routing method for sidebar and notification actions.
toggleSidebarAction() and toggleNotificationsAction() currently manipulate global AppDelegate properties without window context. In multi-window sessions, this toggles the active/key window's sidebar instead of the window whose toolbar button was clicked.
Replace AppDelegate.shared?.sidebarState?.toggle() with AppDelegate.shared?.toggleSidebarInActiveMainWindow(), which already implements the correct routing pattern (active manager → key window → main window → fallback). Similarly, check whether toggleNotificationsPopover() should route per-window or if it intentionally targets the active window; if per-window routing is needed, apply the same pattern.
The newTabAction() correctly uses addWorkspaceInPreferredMainWindow() and should be the model for other toolbar actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/WindowToolbarController.swift` around lines 230 - 243,
toggleSidebarAction toggles the global sidebarState on AppDelegate which affects
the key/active window rather than the window whose toolbar was clicked; update
toggleSidebarAction to call
AppDelegate.shared?.toggleSidebarInActiveMainWindow() so it uses the established
cross-window routing pattern (see newTabAction's use of
addWorkspaceInPreferredMainWindow as the model). For toggleNotificationsAction,
decide if notifications should be routed per-window—if so, replace the direct
call to AppDelegate.shared?.toggleNotificationsPopover(animated:) with the
equivalent per-window routing helper (mirror the active-main-window routing used
by toggleSidebarInActiveMainWindow); otherwise leave it as-is. Ensure you only
change toggleSidebarAction (and toggleNotificationsAction if applying per-window
routing) and keep newTabAction unchanged as the correct pattern example.
| @@ -1 +1 @@ | |||
| Subproject commit b2788b1e77d43f0c114dcf189aa59cae8abb47de | |||
| Subproject commit cfff8a9318f8131604681e4cb86c11925e01bf1e | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SUBMODULE_PATH="vendor/bonsplit"
EXPECTED_COMMIT="cfff8a9318f8131604681e4cb86c11925e01bf1e"
echo "== Resolve gitlink pointer from parent repo =="
POINTER="$(git ls-tree HEAD "$SUBMODULE_PATH" | awk '{print $3}')"
echo "Pointer in parent: $POINTER"
test "$POINTER" = "$EXPECTED_COMMIT"
echo "== Ensure submodule is present =="
if [ ! -e "$SUBMODULE_PATH/.git" ]; then
git submodule update --init "$SUBMODULE_PATH"
fi
echo "== Fetch bonsplit main and verify ancestry =="
git -C "$SUBMODULE_PATH" fetch origin main --quiet
if git -C "$SUBMODULE_PATH" merge-base --is-ancestor "$EXPECTED_COMMIT" origin/main; then
echo "OK: commit is reachable from origin/main"
else
echo "FAIL: commit is NOT on origin/main"
exit 1
fi
echo "== Verify key Bonsplit API symbols still exist =="
rg -n --type=swift -C2 '\bBonsplitConfiguration\.Appearance\b|\btabBarHeight\b|\bshowSplitButtons\b|\bchromeColors\b' "$SUBMODULE_PATH"
rg -n --type=swift -C2 '\bclass\s+BonsplitController\b|\bonExternalTabDrop\b|\bonTabCloseRequest\b|\btabs\s*\(inPane:\s*|\bselectedTab\s*\(inPane:\s*|\bfocusedPaneId\b' "$SUBMODULE_PATH"
echo "== Show commit metadata =="
git -C "$SUBMODULE_PATH" show --no-patch --oneline "$EXPECTED_COMMIT"Repository: manaflow-ai/cmux
Length of output: 689
Push the bonsplit commit to origin/main before updating the parent repo pointer.
The submodule pointer cfff8a9318f8131604681e4cb86c11925e01bf1e does not exist in the manaflow-ai/bonsplit remote. Git failed to fetch it with:
fatal: remote error: upload-pack: not our ref cfff8a9318f8131604681e4cb86c11925e01bf1e
Per submodule workflow, you must push the bonsplit commit to the manaflow-ai/bonsplit main branch first, then update the parent pointer. Verify the commit is reachable from origin/main with:
cd vendor/bonsplit && git merge-base --is-ancestor cfff8a9318f8131604681e4cb86c11925e01bf1e origin/main🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/bonsplit` at line 1, The parent repo points to a submodule commit
(cfff8a9318f8131604681e4cb86c11925e01bf1e) that hasn't been pushed to the
bonsplit remote; push that commit to the bonsplit repository's main branch (push
the local bonsplit commit to manaflow-ai/bonsplit main), then update the parent
repo pointer. After pushing, verify the commit is reachable from origin/main
using git merge-base --is-ancestor cfff8a9318f8131604681e4cb86c11925e01bf1e
origin/main (run this inside the bonsplit checkout), and only then update/commit
the submodule pointer in the parent repo.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/TerminalWindowPortal.swift">
<violation number="1" location="Sources/TerminalWindowPortal.swift:1507">
P3: Sidebar presence is inferred solely from `targetFrame.origin.x > 20`, so any non-leftmost split pane will be treated as sidebar-adjacent and get rounded leading corners even when no sidebar is present or only the left pane touches it.</violation>
</file>
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:2982">
P2: Notifications popover toggling listens to a global notification and blindly toggles local state, so a single toggle event can flip popover state in every window and desynchronize visibility across windows.</violation>
</file>
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:10161">
P2: macOS 26 skips accessory attachment, but menu-bar notifications still require attached accessory controllers, making the popover path a no-op.</violation>
</file>
<file name="Sources/CmuxConfig.swift">
<violation number="1" location="Sources/CmuxConfig.swift:452">
P1: Auto-apply executes local workspace config implicitly on tab/workspace activation without trust/confirmation gating, enabling untrusted cmux.json to trigger actions automatically.</violation>
</file>
<file name="Sources/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:354">
P2: `.windowStyle(.hiddenTitleBar)` was removed from the `WindowGroup` during the `mainWindowContent` extraction and not conditionally re-applied for pre-macOS 26. While NSWindow properties like `titlebarAppearsTransparent` and `titleVisibility` partially compensate, `.windowStyle(.hiddenTitleBar)` also affects SwiftUI's own layout calculations and window creation, potentially causing subtle layout differences on macOS 13–15.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }) else { return } | ||
|
|
||
| autoAppliedWorkspaceIds.insert(workspace.id) | ||
| CmuxConfigExecutor.execute( |
There was a problem hiding this comment.
P1: Auto-apply executes local workspace config implicitly on tab/workspace activation without trust/confirmation gating, enabling untrusted cmux.json to trigger actions automatically.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/CmuxConfig.swift, line 452:
<comment>Auto-apply executes local workspace config implicitly on tab/workspace activation without trust/confirmation gating, enabling untrusted cmux.json to trigger actions automatically.</comment>
<file context>
@@ -381,6 +415,47 @@ final class CmuxConfigStore: ObservableObject {
+ }) else { return }
+
+ autoAppliedWorkspaceIds.insert(workspace.id)
+ CmuxConfigExecutor.execute(
+ command: command,
+ tabManager: tabManager,
</file context>
| onDismiss: { isNotificationsPopoverPresented = false } | ||
| ) | ||
| } | ||
| .onReceive(NotificationCenter.default.publisher(for: AppDelegate.toggleNotificationsPopoverNotification)) { _ in |
There was a problem hiding this comment.
P2: Notifications popover toggling listens to a global notification and blindly toggles local state, so a single toggle event can flip popover state in every window and desynchronize visibility across windows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 2982:
<comment>Notifications popover toggling listens to a global notification and blindly toggles local state, so a single toggle event can flip popover state in every window and desynchronize visibility across windows.</comment>
<file context>
@@ -2869,6 +2882,124 @@ struct ContentView: View {
+ onDismiss: { isNotificationsPopoverPresented = false }
+ )
+ }
+ .onReceive(NotificationCenter.default.publisher(for: AppDelegate.toggleNotificationsPopoverNotification)) { _ in
+ isNotificationsPopoverPresented.toggle()
+ }
</file context>
| // in the NavigationSplitView. Skip attaching the old titlebar | ||
| // accessory views, but the controller is already started (for | ||
| // notifications popover support). | ||
| return |
There was a problem hiding this comment.
P2: macOS 26 skips accessory attachment, but menu-bar notifications still require attached accessory controllers, making the popover path a no-op.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/AppDelegate.swift, line 10161:
<comment>macOS 26 skips accessory attachment, but menu-bar notifications still require attached accessory controllers, making the popover path a no-op.</comment>
<file context>
@@ -10119,6 +10153,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
+ // in the NavigationSplitView. Skip attaching the old titlebar
+ // accessory views, but the controller is already started (for
+ // notifications popover support).
+ return
+ }
titlebarAccessoryController.start()
</file context>
| } | ||
|
|
||
| var body: some Scene { | ||
| WindowGroup { mainWindowContent } |
There was a problem hiding this comment.
P2: .windowStyle(.hiddenTitleBar) was removed from the WindowGroup during the mainWindowContent extraction and not conditionally re-applied for pre-macOS 26. While NSWindow properties like titlebarAppearsTransparent and titleVisibility partially compensate, .windowStyle(.hiddenTitleBar) also affects SwiftUI's own layout calculations and window creation, potentially causing subtle layout differences on macOS 13–15.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/cmuxApp.swift, line 354:
<comment>`.windowStyle(.hiddenTitleBar)` was removed from the `WindowGroup` during the `mainWindowContent` extraction and not conditionally re-applied for pre-macOS 26. While NSWindow properties like `titlebarAppearsTransparent` and `titleVisibility` partially compensate, `.windowStyle(.hiddenTitleBar)` also affects SwiftUI's own layout calculations and window creation, potentially causing subtle layout differences on macOS 13–15.</comment>
<file context>
@@ -316,40 +316,42 @@ struct cmuxApp: App {
+ }
+
+ var body: some Scene {
+ WindowGroup { mainWindowContent }
.commands {
CommandGroup(replacing: .appSettings) {
</file context>
| // width of 120pt, so any x > 20 reliably indicates a sidebar is | ||
| // to our left. This AppKit view cannot access SwiftUI SidebarState | ||
| // directly; the frame-based heuristic is the simplest reliable path. | ||
| let hasSidebarToLeft = targetFrame.origin.x > 20 |
There was a problem hiding this comment.
P3: Sidebar presence is inferred solely from targetFrame.origin.x > 20, so any non-leftmost split pane will be treated as sidebar-adjacent and get rounded leading corners even when no sidebar is present or only the left pane touches it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/TerminalWindowPortal.swift, line 1507:
<comment>Sidebar presence is inferred solely from `targetFrame.origin.x > 20`, so any non-leftmost split pane will be treated as sidebar-adjacent and get rounded leading corners even when no sidebar is present or only the left pane touches it.</comment>
<file context>
@@ -1496,6 +1496,23 @@ final class WindowTerminalPortal: NSObject {
+ // width of 120pt, so any x > 20 reliably indicates a sidebar is
+ // to our left. This AppKit view cannot access SwiftUI SidebarState
+ // directly; the frame-based heuristic is the simplest reliable path.
+ let hasSidebarToLeft = targetFrame.origin.x > 20
+ let desiredRadius: CGFloat = hasSidebarToLeft ? 16 : 0
+ if hostedView.layer?.cornerRadius != desiredRadius {
</file context>
Greptile SummaryThis PR fixes terminal surfaces displaying stale theme colors (e.g. light foreground on dark background) after a Ghostty config reload by adding Confidence Score: 5/5Safe to merge — the core bug fix is correct and all remaining findings are P2 style/documentation suggestions. The primary fix (reapplyColorSchemeAndConfig + ghostty_surface_update_config) is correctly sequenced after the config swap and properly uses the updated GhosttyApp.shared.config. New features (sidebar sections, session persistence, autoApply) use backwards-compatible optional fields and well-guarded logic. All open findings are P2: the nil-config silent skip is an edge case unlikely in steady state, the autoApply restriction is a documentation gap rather than a defect, and the session-restore overwrite is arguably intentional behavior. Sources/CmuxConfig.swift — autoApply behavior for non-target:current commands and session-restore interaction worth a second look before shipping docs.
|
| Filename | Overview |
|---|---|
| Sources/GhosttyTerminalView.swift | Adds reapplyColorSchemeAndConfig() which calls ghostty_surface_update_config + applySurfaceColorScheme(force:true) to bypass Ghostty's dedup after config reload; nil-config path silently skips the update_config step (P2). |
| Sources/AppDelegate.swift | Calls reapplyColorSchemeAndConfig() on each terminal panel inside refreshTerminalSurfacesAfterGhosttyConfigReload; logic is straightforward and well-sequenced after the existing forceRefresh call. |
| Sources/CmuxConfig.swift | Introduces autoApply/target fields and checkAutoApply gated on target: current workspace commands only; shell commands and target: new commands silently ignore autoApply: true (P2), and session-restored single-pane workspaces are eligible for layout overwrite (P2). |
| Sources/CmuxConfigExecutor.swift | Adds target: current branch that closes all non-focused panels and applies a custom layout in-place; logic is clean and the early-return on missing selected workspace prevents silent no-op. |
| Sources/SidebarSection.swift | New model for collapsible sidebar sections with ordered workspace IDs; addWorkspace correctly deduplicates before insertion and SidebarLayout.allWorkspacesInOrder provides a clear flat ordering for navigation. |
| Sources/SessionPersistence.swift | Adds optional id: UUID? to SessionWorkspaceSnapshot and optional sections to SessionTabManagerSnapshot for backwards-compatible persistence; schema version correctly remains at 1 since all new fields are optional. |
| Sources/TabManager.swift | Adds sections: [SidebarSection] with observer rebinding on didSet, sectionRevision bump for SwiftUI dependency tracking, and sidebarLayout computed property; section persistence and restore handle filtered workspace IDs correctly. |
Sequence Diagram
sequenceDiagram
participant User
participant GhosttyApp
participant AppDelegate
participant TerminalSurface
User->>GhosttyApp: Reload Configuration
GhosttyApp->>GhosttyApp: ghostty_app_update_config(app, newConfig)
GhosttyApp->>GhosttyApp: config = newConfig
GhosttyApp->>GhosttyApp: scheduleSurfaceRefreshAfterConfigurationReload()
Note over GhosttyApp: DispatchQueue.main.async
GhosttyApp->>AppDelegate: refreshTerminalSurfacesAfterGhosttyConfigReload()
loop Each TerminalPanel
AppDelegate->>TerminalSurface: reconcileGeometryNow()
AppDelegate->>TerminalSurface: refreshHostBackgroundAfterGhosttyConfigReload()
AppDelegate->>TerminalSurface: forceRefresh()
AppDelegate->>TerminalSurface: reapplyColorSchemeAndConfig()
Note over TerminalSurface: ghostty_surface_update_config(surface, config) bypasses set_color_scheme dedup
TerminalSurface->>TerminalSurface: applySurfaceColorScheme(force: true)
end
Reviews (1): Last reviewed commit: "Fix terminal surfaces losing theme after..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| guard let command = loadedCommands.first(where: { | ||
| $0.autoApply == true && $0.workspace?.target == .current | ||
| }) else { return } |
There was a problem hiding this comment.
autoApply silently ignored for non-target: current commands
The filter $0.autoApply == true && $0.workspace?.target == .current means autoApply: true is a no-op for both shell command entries and workspace commands with target: "new" (the default). A user who sets autoApply: true on a shell command or a target: new workspace command will get no feedback about why it never fires.
Consider either logging a warning when a non-applicable command has autoApply: true, or documenting the restriction in the CmuxCommandDefinition field comment so the constraint is visible at the schema level.
| func reapplyColorSchemeAndConfig() { | ||
| guard let surface, let view = attachedView else { return } | ||
| // Force the surface to re-derive its config with its current conditional state. | ||
| // ghostty_surface_set_color_scheme has an internal dedup that skips when the | ||
| // scheme hasn't changed, but after a config reload the underlying theme data | ||
| // may have changed. ghostty_surface_update_config bypasses that dedup. | ||
| if let config = GhosttyApp.shared.config { | ||
| ghostty_surface_update_config(surface, config) | ||
| } | ||
| // Re-apply color scheme to ensure the surface's conditional state matches | ||
| // the current macOS appearance, in case it drifted. | ||
| view.applySurfaceColorScheme(force: true) | ||
| } |
There was a problem hiding this comment.
GhosttyApp.shared.config is nil
If GhosttyApp.shared.config is nil (e.g. during a failed config allocation), ghostty_surface_update_config is skipped while applySurfaceColorScheme(force: true) still runs. This means the surface gets the scheme re-applied but not the re-derived config, leaving theme colors potentially stale — the exact bug this PR fixes. While this path is unlikely in steady state, a log line or assert would make the nil case observable.
if let config = GhosttyApp.shared.config {
ghostty_surface_update_config(surface, config)
} else {
// config unavailable — scheme will be re-applied but config re-derivation is skipped
#if DEBUG
dlog("reapplyColorSchemeAndConfig: skipped surface_update_config — no app config")
#endif
}| private func checkAutoApply(forTabId: UUID? = nil) { | ||
| guard let tabManager = trackedTabManager, | ||
| let workspace = tabManager.selectedWorkspace, | ||
| // If a specific tab ID was requested, verify it's still selected. | ||
| forTabId == nil || workspace.id == forTabId, | ||
| !autoAppliedWorkspaceIds.contains(workspace.id), | ||
| // Only auto-apply to workspaces with a single pane — don't tear | ||
| // down user-customized layouts or restored split configurations. | ||
| workspace.panels.count <= 1 | ||
| else { return } |
There was a problem hiding this comment.
autoApply overwrite
autoAppliedWorkspaceIds starts empty each session, so a restored single-pane workspace (panels.count <= 1) that hasn't been auto-applied yet will have its layout overwritten by target: current on first selection. Because closePanel(..., force: true) is called for all but the focused panel before applyCustomLayout, any scrollback or state in that pane is lost.
If the intent is to skip auto-apply on restored workspaces, you could seed autoAppliedWorkspaceIds from the restored workspace IDs during restoreFromSnapshot, or add a dedicated wasRestoredFromSession flag on Workspace.
Summary
ghostty_app_update_config, surfaces can end up with stale theme colors (e.g. light foreground on dark background) becauseghostty_surface_set_color_scheme's internal dedup skips re-derivation when the scheme hasn't changedghostty_surface_update_configon each surface after config reload, which forces a full config re-derive with the surface's current light/dark conditional state, bypassing the dedupTest plan
theme = light:X,dark:Yconditional themes configuredtheme = X)Note
Medium Risk
Medium risk due to broad UI/windowing changes (macOS 26 NavigationSplitView/toolbar/glass behavior) plus new session-persisted sidebar section/grouping state that affects workspace ordering and restore logic.
Overview
Fixes terminal surfaces showing stale theme colors after Ghostty config reload by forcing each surface to
ghostty_surface_update_configand reapplying the Swift-side color scheme (reapplyColorSchemeAndConfig).Adds workspace grouping in the sidebar via new
SidebarSectionmodel andTabManager.sidebarLayout, including section CRUD (rename/reorder/collapse), drag-and-drop support, context-menu “Move to Section”, and persistence/restore throughSessionPersistence(workspace UUIDs are now stored and restored).Introduces a macOS 26-specific window/UI path: uses
NavigationSplitViewfor native glass sidebar + SwiftUI toolbars (notifications popover handled via a notification bridge), adjusts safe-area/titlebar behavior, disables manual window glass insertion, tweaks terminal rounding/insets, and updates toolbar/accessory attachment logic for 26+.Reviewed by Cursor Bugbot for commit 21e9e70. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes terminal theme desync after config reload by forcing each surface to re-derive its config and reapply the current appearance. Also brings native macOS 26 UI updates and optional sidebar sections with session persistence.
Bug Fixes
ghostty_surface_update_configand then reapply the color scheme on every surface to prevent stale light/dark theme colors.New Features
NavigationSplitView, SwiftUI toolbar (terminal/browser/split/notifications/new tab), and rounded leading corners with safe insets; legacy titlebar accessory is skipped.workspace.target: "current"andautoApply; auto-apply runs once per workspace on switch (single-pane only), using local or global config.bonsplitto improve split divider behavior; hide its tab bar for single-tab panes on macOS 26.Written for commit 21e9e70. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements