Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Sources/App/NSWindow+CmuxPeerWindow.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import AppKit

extension NSWindow {
/// Configures this window as a standard cmux top-level *peer* window.
///
/// Peer windows — Settings, the Config editor, About — sit at
/// `NSWindow.Level.normal` and obey standard macOS window ordering: clicking
/// any sibling window (including the main terminal window) brings it forward
/// and lets the peer recede behind it. This is the default every top-level
/// window the user opens should adopt.
///
/// It exists as a single, greppable seam so that "this is an ordinary
/// top-level window" is stated explicitly rather than left implicit. The
/// only sanctioned way to float a window above its siblings is to set
/// `level = .floating` deliberately at the call site with a comment
/// justifying it (e.g. DEBUG HUD/lab panels). Accidentally inheriting a
/// floating level — via an `NSPanel` default or a stray `level = .floating`
/// — is what produced the "Settings floats above the main window forever"
/// bug (https://github.com/manaflow-ai/cmux/issues/5081).
///
/// - Note: A plain `NSWindow` / SwiftUI `Window` scene already defaults to
/// `.normal`; calling this makes the invariant explicit and guards against
/// a later change (or a child-window attachment) silently re-floating the
/// window.
@MainActor
func adoptCmuxPeerWindowLevel() {
level = .normal
}
}
101 changes: 16 additions & 85 deletions Sources/App/SettingsWindowPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ enum SettingsWindowPresenter {
private static var openWindow: (@MainActor () -> Void)?
private static var parentWindowProvider: (@MainActor () -> NSWindow?)?
private static weak var settingsWindow: NSWindow?
private static weak var observedParentWindow: NSWindow?
private static weak var observedSettingsWindow: NSWindow?
private static var parentCloseObserver: NSObjectProtocol?
private static var pendingNavigationTarget: SettingsNavigationTarget?
private static var pendingContentNavigationTarget: SettingsNavigationTarget?
private static var shouldOpenWhenConfigured = false
Expand All @@ -26,9 +23,6 @@ enum SettingsWindowPresenter {
) {
self.openWindow = openWindow
self.parentWindowProvider = parentWindowProvider
if let settingsWindow {
attachToPreferredParent(settingsWindow)
}
if shouldOpenWhenConfigured {
shouldOpenWhenConfigured = false
openWindow()
Expand All @@ -42,8 +36,8 @@ enum SettingsWindowPresenter {
window.isRestorable = false
window.minSize = minimumSize
window.contentMinSize = minimumSize
window.adoptCmuxPeerWindowLevel()
clampToVisibleAreaIfNeeded(window)
attachToPreferredParent(window)
if shouldFocusAfterConfiguration {
Task { @MainActor in
guard settingsWindow === window else { return }
Expand Down Expand Up @@ -113,11 +107,6 @@ enum SettingsWindowPresenter {

#if DEBUG
static func resetForTests() {
if let settingsWindow {
detachFromCurrentParent(settingsWindow)
} else {
removeParentCloseObserver()
}
openWindow = nil
parentWindowProvider = nil
settingsWindow = nil
Expand Down Expand Up @@ -163,86 +152,28 @@ enum SettingsWindowPresenter {
if window.isMiniaturized {
window.deminiaturize(nil)
}
window.adoptCmuxPeerWindowLevel()
clampToVisibleAreaIfNeeded(window)
if let parentWindow = attachToPreferredParent(window) {
orderParentBehindSettings(parentWindow)
// Surface the preferred main window first so Settings opens layered
// above it — the standard "Settings in front of its app" presentation
// a global hotkey or app activation expects. We do this by ordering
// both windows front *as peers*, never via `addChildWindow`: a child
// window is pinned above its parent forever and can never recede when
// the user clicks the main window (the bug in
// https://github.com/manaflow-ai/cmux/issues/5081). One-time front
// ordering gives the same initial layering while leaving normal
// click-to-raise window ordering fully intact afterwards.
if let parentWindow = parentWindowProvider?(), parentWindow !== window {
if parentWindow.isMiniaturized {
parentWindow.deminiaturize(nil)
}
parentWindow.orderFront(nil)
}
NSRunningApplication.current.activate(options: [.activateAllWindows])
window.makeKeyAndOrderFront(nil)
window.orderFrontRegardless()
}

@discardableResult
private static func attachToPreferredParent(_ window: NSWindow) -> NSWindow? {
guard let parentWindow = parentWindowProvider?(),
parentWindow !== window else {
detachFromCurrentParent(window)
return nil
}

if window.parent !== parentWindow {
detachFromCurrentParent(window)
parentWindow.addChildWindow(window, ordered: .above)
}
observeParentWillClose(parentWindow, settingsWindow: window)
return parentWindow
}

private static func detachFromCurrentParent(_ window: NSWindow) {
removeParentCloseObserver()
guard let parentWindow = window.parent else { return }
parentWindow.removeChildWindow(window)
}

private static func observeParentWillClose(_ parentWindow: NSWindow, settingsWindow: NSWindow) {
guard observedParentWindow !== parentWindow || observedSettingsWindow !== settingsWindow else {
return
}

removeParentCloseObserver()
observedParentWindow = parentWindow
observedSettingsWindow = settingsWindow
// Run synchronously for normal AppKit window-close notifications so
// Settings detaches before AppKit orders out child windows.
parentCloseObserver = NotificationCenter.default.addObserver(
forName: NSWindow.willCloseNotification,
object: parentWindow,
queue: nil
) { [weak parentWindow, weak settingsWindow] _ in
guard Thread.isMainThread else {
assertionFailure("NSWindow.willCloseNotification should be delivered on the main thread")
return
}
MainActor.assumeIsolated {
detachFromClosingParent(parentWindow: parentWindow, settingsWindow: settingsWindow)
}
}
}

private static func detachFromClosingParent(parentWindow: NSWindow?, settingsWindow: NSWindow?) {
guard let settingsWindow, settingsWindow.parent === parentWindow else {
removeParentCloseObserver()
return
}
detachFromCurrentParent(settingsWindow)
}

private static func removeParentCloseObserver() {
if let parentCloseObserver {
NotificationCenter.default.removeObserver(parentCloseObserver)
}
parentCloseObserver = nil
observedParentWindow = nil
observedSettingsWindow = nil
}

private static func orderParentBehindSettings(_ window: NSWindow) {
if window.isMiniaturized {
window.deminiaturize(nil)
}
window.orderFront(nil)
}

private static func clampToVisibleAreaIfNeeded(_ window: NSWindow) {
guard let screen = window.screen ?? NSScreen.main else { return }
var frame = window.frame
Expand Down
5 changes: 4 additions & 1 deletion Sources/Settings/ConfigSettingsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ struct ConfigSettingsView: View {
window.minSize = NSSize(width: 700, height: 500)
window.tabbingMode = .disallowed
window.animationBehavior = .utilityWindow
window.level = .floating
// The Config editor is a top-level peer window, not a floating
// inspector: clicking the main window must be able to raise it above
// the editor (https://github.com/manaflow-ai/cmux/issues/5081).
window.adoptCmuxPeerWindowLevel()
window.collectionBehavior.insert(.fullScreenAuxiliary)
}

Expand Down
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@
B9000015A1B2C3D4E5F60719 /* MultiWindowNotificationsUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000016A1B2C3D4E5F60719 /* MultiWindowNotificationsUITests.swift */; };
734F49D37E543DD01C2F4FEF /* NotificationAndMenuBarTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2C075029771815DD5DA1332 /* NotificationAndMenuBarTests.swift */; };
A5001094 /* NotificationsPage.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001091 /* NotificationsPage.swift */; };
D36090020000000000000001 /* NSWindow+CmuxPeerWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = D36090020000000000000002 /* NSWindow+CmuxPeerWindow.swift */; };
4378399A7C0245EF8186F306 /* OmnibarAndToolsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B09C007F42697761B5F1A2AB /* OmnibarAndToolsTests.swift */; };
D1BEF00002A1B2C3D4E5F719 /* open in Copy CLI */ = {isa = PBXBuildFile; fileRef = D1BEF00001A1B2C3D4E5F719 /* open */; };
FEED0000000000000000F007 /* opencode-plugin.js in Resources */ = {isa = PBXBuildFile; fileRef = FEED0000000000000000F006 /* opencode-plugin.js */; };
Expand Down Expand Up @@ -955,6 +956,7 @@
B9000016A1B2C3D4E5F60719 /* MultiWindowNotificationsUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultiWindowNotificationsUITests.swift; sourceTree = "<group>"; };
D2C075029771815DD5DA1332 /* NotificationAndMenuBarTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationAndMenuBarTests.swift; sourceTree = "<group>"; };
A5001091 /* NotificationsPage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationsPage.swift; sourceTree = "<group>"; };
D36090020000000000000002 /* NSWindow+CmuxPeerWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "App/NSWindow+CmuxPeerWindow.swift"; sourceTree = "<group>"; };
B09C007F42697761B5F1A2AB /* OmnibarAndToolsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OmnibarAndToolsTests.swift; sourceTree = "<group>"; };
D1BEF00001A1B2C3D4E5F719 /* open */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = Resources/bin/open; sourceTree = SOURCE_ROOT; };
FEED0000000000000000F006 /* opencode-plugin.js */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.javascript; path = "opencode-plugin.js"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1397,6 +1399,7 @@
A50019B1 /* SettingsSearchAliases.swift */,
D3610C010000000000000002 /* CmuxSettingsJSONPathSupport.swift */,
D36090010000000000000002 /* SettingsWindowPresenter.swift */,
D36090020000000000000002 /* NSWindow+CmuxPeerWindow.swift */,
C0DE35860000000000000002 /* BackgroundWorkspacePrimeCoordinator.swift */,
A5001012 /* ContentView.swift */,
C0DE46010000000000000002 /* CMUXInstalledExtensionSidebarHostView.swift */,
Expand Down Expand Up @@ -2385,6 +2388,7 @@
3865A0033865A0033865A003 /* MenubarSearchPopover.swift in Sources */,
C0DE3150A00000000000001 /* MinimalModeSidebarControls.swift in Sources */,
A5001094 /* NotificationsPage.swift in Sources */,
D36090020000000000000001 /* NSWindow+CmuxPeerWindow.swift in Sources */,
D0B1001CA1B2C3D4E5F60001 /* PaneDropRoutingSupport.swift in Sources */,
A5001400 /* Panel.swift in Sources */,
A5001405 /* PanelContentView.swift in Sources */,
Expand Down
45 changes: 32 additions & 13 deletions cmuxTests/SettingsWindowPresenterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ final class SettingsWindowPresenterTests: XCTestCase {
XCTAssertEqual(SettingsWindowPresenter.consumePendingContentNavigationTarget(), .browserImport)
}

func testParentsSettingsAbovePreferredMainWindow() {
// Settings is a top-level *peer* window, not a child of the main window.
// A child window (`addChildWindow`) is pinned above its parent forever and
// can never recede when the user clicks the main window — that is the
// floating-Settings bug (https://github.com/manaflow-ai/cmux/issues/5081).
// These tests pin the peer invariant: configuring/focusing Settings must
// never create a parent-child relationship and must leave it at `.normal`.
func testDoesNotAttachSettingsAsChildOfPreferredMainWindow() {
let parentWindow = makeWindow(identifier: "cmux.main.\(UUID().uuidString)")
let settingsWindow = makeWindow(identifier: SettingsWindowPresenter.windowIdentifier)
defer {
Expand All @@ -93,11 +99,12 @@ final class SettingsWindowPresenterTests: XCTestCase {
)
SettingsWindowPresenter.configure(window: settingsWindow)

XCTAssertTrue(settingsWindow.parent === parentWindow)
XCTAssertTrue(parentWindow.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertNil(settingsWindow.parent)
XCTAssertFalse(parentWindow.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertEqual(settingsWindow.level, .normal)
}

func testReparentsSettingsWhenPreferredMainWindowChanges() {
func testFocusingSettingsKeepsItAsPeerWhenPreferredMainWindowChanges() {
let firstParent = makeWindow(identifier: "cmux.main.\(UUID().uuidString)")
let secondParent = makeWindow(identifier: "cmux.main.\(UUID().uuidString)")
let settingsWindow = makeWindow(identifier: SettingsWindowPresenter.windowIdentifier)
Expand All @@ -113,21 +120,20 @@ final class SettingsWindowPresenterTests: XCTestCase {
parentWindowProvider: { preferredParent }
)
SettingsWindowPresenter.configure(window: settingsWindow)
XCTAssertTrue(settingsWindow.parent === firstParent)
XCTAssertNil(settingsWindow.parent)

preferredParent = secondParent
SettingsWindowPresenter.refocusIfVisible()
XCTAssertTrue(settingsWindow.parent === firstParent)

settingsWindow.orderFront(nil)
// refocusIfVisible() runs the real performFocus ordering path.
SettingsWindowPresenter.refocusIfVisible()

XCTAssertTrue(settingsWindow.parent === secondParent)
XCTAssertNil(settingsWindow.parent)
XCTAssertFalse(firstParent.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertTrue(secondParent.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertFalse(secondParent.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertEqual(settingsWindow.level, .normal)
}

func testDetachesSettingsBeforePreferredMainWindowCloses() {
func testSettingsSurvivesPreferredMainWindowCloseAsIndependentPeer() {
let parentWindow = makeWindow(identifier: "cmux.main.\(UUID().uuidString)")
let settingsWindow = makeWindow(identifier: SettingsWindowPresenter.windowIdentifier)
defer {
Expand All @@ -141,15 +147,28 @@ final class SettingsWindowPresenterTests: XCTestCase {
)
SettingsWindowPresenter.configure(window: settingsWindow)
settingsWindow.orderFront(nil)
XCTAssertTrue(settingsWindow.parent === parentWindow)
XCTAssertNil(settingsWindow.parent)

// As an independent peer, Settings is unaffected by the main window
// closing — there is no child relationship to tear down.
NotificationCenter.default.post(name: NSWindow.willCloseNotification, object: parentWindow)

XCTAssertNil(settingsWindow.parent)
XCTAssertFalse(parentWindow.childWindows?.contains(where: { $0 === settingsWindow }) == true)
XCTAssertTrue(settingsWindow.isVisible)
}

func testAdoptCmuxPeerWindowLevelBringsFloatingWindowToNormal() {
let window = makeWindow(identifier: "cmux.peer.\(UUID().uuidString)")
defer { window.orderOut(nil) }

window.level = .floating
XCTAssertEqual(window.level, .floating)

window.adoptCmuxPeerWindowLevel()

XCTAssertEqual(window.level, .normal)
}

func testConfigureClampsOversizedSettingsFrameToVisibleArea() throws {
guard let screen = NSScreen.main else {
throw XCTSkip("No screen available for Settings frame clamping")
Expand Down
Loading