diff --git a/Sources/App/NSWindow+CmuxPeerWindow.swift b/Sources/App/NSWindow+CmuxPeerWindow.swift new file mode 100644 index 000000000000..09a6b0179d04 --- /dev/null +++ b/Sources/App/NSWindow+CmuxPeerWindow.swift @@ -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 + } +} diff --git a/Sources/App/SettingsWindowPresenter.swift b/Sources/App/SettingsWindowPresenter.swift index e4294b0839fc..9b22cf5296f2 100644 --- a/Sources/App/SettingsWindowPresenter.swift +++ b/Sources/App/SettingsWindowPresenter.swift @@ -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 @@ -26,9 +23,6 @@ enum SettingsWindowPresenter { ) { self.openWindow = openWindow self.parentWindowProvider = parentWindowProvider - if let settingsWindow { - attachToPreferredParent(settingsWindow) - } if shouldOpenWhenConfigured { shouldOpenWhenConfigured = false openWindow() @@ -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 } @@ -113,11 +107,6 @@ enum SettingsWindowPresenter { #if DEBUG static func resetForTests() { - if let settingsWindow { - detachFromCurrentParent(settingsWindow) - } else { - removeParentCloseObserver() - } openWindow = nil parentWindowProvider = nil settingsWindow = nil @@ -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 diff --git a/Sources/Settings/ConfigSettingsView.swift b/Sources/Settings/ConfigSettingsView.swift index da24c2d6d502..ae2647605609 100644 --- a/Sources/Settings/ConfigSettingsView.swift +++ b/Sources/Settings/ConfigSettingsView.swift @@ -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) } diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index b3527272d731..b47fdea27edf 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -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 */; }; @@ -955,6 +956,7 @@ B9000016A1B2C3D4E5F60719 /* MultiWindowNotificationsUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultiWindowNotificationsUITests.swift; sourceTree = ""; }; D2C075029771815DD5DA1332 /* NotificationAndMenuBarTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationAndMenuBarTests.swift; sourceTree = ""; }; A5001091 /* NotificationsPage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationsPage.swift; sourceTree = ""; }; + D36090020000000000000002 /* NSWindow+CmuxPeerWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "App/NSWindow+CmuxPeerWindow.swift"; sourceTree = ""; }; B09C007F42697761B5F1A2AB /* OmnibarAndToolsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OmnibarAndToolsTests.swift; sourceTree = ""; }; 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 = ""; }; @@ -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 */, @@ -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 */, diff --git a/cmuxTests/SettingsWindowPresenterTests.swift b/cmuxTests/SettingsWindowPresenterTests.swift index 6c5483e61401..417bc20c843d 100644 --- a/cmuxTests/SettingsWindowPresenterTests.swift +++ b/cmuxTests/SettingsWindowPresenterTests.swift @@ -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 { @@ -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) @@ -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 { @@ -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")