fix(connections): make the connections strip's close command close the connection

CHANGELOG.md

@@ -34,8 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-- Closing a connection removes it from the workspace rail, ends its session, and closes its tunnel. It stayed listed as connected, could not be clicked, and its database connection stayed open.
-- Close Workspace on a rail row acts on the connection the row names. It acted on the connection on screen instead, closing its tabs and losing unsaved editor work.
+- Closing a connection removes it from the connections strip, ends its session, and closes its tunnel. It stayed listed as connected, could not be clicked, and its database connection stayed open.
+- **Open in New Window** on a connection in the strip moves it out of the shared window with its tabs, its session and its unsaved work.
+- The strip's close command ends the connection: every tab across every database it has open, its session, and every row it holds in the rail. It used to close only the tabs of one database and leave the row you clicked exactly where it was, which read as doing nothing. Disconnect still ends only the session and keeps the row. **File > Close Connection** does the same from the menu bar.
 - Closing a window disconnects every connection it was showing, not just one of them.
 - Hiding a database in the sidebar's database filter takes effect straight away. The tree kept listing every database until some other change happened to rebuild it.
 - An alert opens on the window you were working in. It could attach itself to a floating panel such as the Quick Switcher, which takes the alert with it when it closes.

TablePro/Core/Menu/FileMenuBuilder.swift

@@ -72,6 +72,10 @@ enum FileMenuBuilder {
                 shortcut: .closeAllTabs,
                 keyboard: keyboard
             ),
+            MenuItemFactory.item(
+                String(localized: "Close Connection"),
+                action: #selector(MainSplitViewController.closeConnection(_:))
+            ),
             MenuItemFactory.item(
                 String(localized: "Reopen Closed Tab"),
                 action: #selector(AppDelegate.reopenClosedTab(_:)),

TablePro/Core/Menu/ViewMenuBuilder.swift

@@ -25,7 +25,7 @@ enum ViewMenuBuilder {
                 keyboard: keyboard
             ),
             MenuItemFactory.item(
-                String(localized: "Show Workspace Rail"),
+                String(localized: "Show Connections"),
                 action: #selector(MainSplitViewController.toggleWorkspaceRail(_:)),
                 shortcut: .toggleWorkspaceRail,
                 keyboard: keyboard
@@ -84,13 +84,13 @@ enum ViewMenuBuilder {
             ),
             MenuItemFactory.separator,
             MenuItemFactory.item(
-                String(localized: "Show Previous Workspace"),
+                String(localized: "Show Previous Connection"),
                 action: #selector(MainSplitViewController.showPreviousWorkspace(_:)),
                 shortcut: .showPreviousWorkspace,
                 keyboard: keyboard
             ),
             MenuItemFactory.item(
-                String(localized: "Show Next Workspace"),
+                String(localized: "Show Next Connection"),
                 action: #selector(MainSplitViewController.showNextWorkspace(_:)),
                 shortcut: .showNextWorkspace,
                 keyboard: keyboard

TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift

@@ -0,0 +1,73 @@
+//
+//  ConnectionCloseAction.swift
+//  TablePro
+//
+
+import AppKit
+import Foundation
+
+/// The one path a user-requested connection close takes, whatever surface asked for it. A peer of
+/// `ConnectionDisconnectAction`, and the two are deliberately different: Disconnect ends the
+/// session and leaves the connection in place to reconnect, Close ends the connection.
+///
+/// The rail used to offer this as "Close Workspace", wired to the tab strip's bulk-close family, so
+/// it closed a subset of one connection's tabs and left the row it was invoked on exactly where it
+/// was. No platform or competitor precedent puts that scope on a list of open sessions: the HIG has
+/// no close verb for a sidebar row at all, the one app shipping the literal string is Xcode's File
+/// menu where it means the whole project session, and Finder's Locations rows, the true analogue of
+/// a list of live remote sessions, end the session and drop the row.
+@MainActor
+internal enum ConnectionCloseAction {
+    internal enum Decision: Equatable {
+        case closeImmediately
+        case confirmUnsavedWork
+    }
+
+    /// Pure so the case that used to fail silently is pinned by a test: a connection with no session
+    /// has nothing to lose, and asking about it produced an alert nobody could answer.
+    internal static func decision(hasSession: Bool, hasUnsavedWork: Bool) -> Decision {
+        guard hasSession, hasUnsavedWork else { return .closeImmediately }
+        return .confirmUnsavedWork
+    }
+
+    internal static func close(connectionId: UUID) async {
+        let coordinator = WindowManager.shared.coordinator(for: connectionId)
+        let decision = decision(
+            hasSession: coordinator != nil,
+            hasUnsavedWork: coordinator?.hasAnyUnsavedWork() ?? false
+        )
+        guard decision == .confirmUnsavedWork else {
+            WindowManager.shared.closeWindow(for: connectionId)
+            return
+        }
+
+        /// Shown, then asked. A data-loss alert over a connection the user cannot see names work
+        /// they have no way to look at before answering.
+        let presentingWindow = reveal(connectionId: connectionId)
+        switch await AlertHelper.confirmSaveChanges(
+            message: String(localized: "Your changes will be lost if you don't save them."),
+            window: presentingWindow
+        ) {
+        case .save:
+            coordinator?.commandActions?.saveChanges()
+        case .dontSave:
+            WindowManager.shared.closeWindow(for: connectionId)
+        case .cancel:
+            break
+        }
+    }
+
+    /// `hasAnyUnsavedWork` is coordinator state, so it answers for a connection whose content has
+    /// never been on screen. Acting on that answer needs the connection in front of the user first.
+    @discardableResult
+    private static func reveal(connectionId: UUID) -> NSWindow? {
+        guard let window = WindowManager.shared.window(for: connectionId),
+              let host = window.contentViewController as? MainSplitViewController else { return nil }
+        if let group = window.tabGroup, group.selectedWindow !== window {
+            group.selectedWindow = window
+        }
+        window.makeKeyAndOrderFront(nil)
+        host.workspaces.select(connectionId)
+        return window
+    }
+}

TablePro/Core/Services/Infrastructure/ConnectionWorkspaceHandoff.swift

@@ -0,0 +1,35 @@
+//
+//  ConnectionWorkspaceHandoff.swift
+//  TablePro
+//
+
+import Foundation
+
+/// Carries a live `ConnectionWorkspace` from the window it is leaving to the one being built for
+/// it, so Open in New Window moves the connection rather than reopening it.
+///
+/// Rebuilding it in the new window would mean a second `SessionStateFactory.create`, and everything
+/// the user has in that connection lives on the state it would replace: the open tabs, the change
+/// manager holding unsaved cell edits, the coordinator, and the undo stack. The workspace is handed
+/// over whole instead, keyed by the payload the new window is created with, mirroring how
+/// `SessionStateFactory.registerPending` hands a freshly built state to a window that does not
+/// exist yet.
+@MainActor
+internal enum ConnectionWorkspaceHandoff {
+    private static var pending: [UUID: ConnectionWorkspace] = [:]
+
+    internal static func register(_ workspace: ConnectionWorkspace, for payloadId: UUID) {
+        pending[payloadId] = workspace
+    }
+
+    internal static func consume(for payloadId: UUID) -> ConnectionWorkspace? {
+        pending.removeValue(forKey: payloadId)
+    }
+
+    /// A window that failed to build never consumes its handoff, and the workspace it was carrying
+    /// is the user's open work. Returning it lets the caller put it back where it came from.
+    @discardableResult
+    internal static func reclaim(for payloadId: UUID) -> ConnectionWorkspace? {
+        pending.removeValue(forKey: payloadId)
+    }
+}

TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift

@@ -29,6 +29,12 @@ extension MainSplitViewController {
     /// itself. Leaving it to `commandActions` made the shortcut inert on exactly the pane a
     /// user most wants to dismiss.
     @objc func closeEditorTab(_ sender: Any?) {
+        /// Close is interpreted by whatever holds the keyboard. With the connections strip focused
+        /// it means the connection highlighted there, which is the row the user is looking at.
+        if railOwnsFocus {
+            closeConnection(sender)
+            return
+        }
         guard let actions = commandActions else {
             guard let connectionId = workspaces.selectedConnectionId else { return }
             WindowManager.shared.closeWindow(for: connectionId)
@@ -37,6 +43,13 @@ extension MainSplitViewController {
         actions.closeTab()
     }
 
+    /// The contextual menu on a rail row offers this too, and the HIG requires every context-menu
+    /// command to be reachable from the menu bar.
+    @objc func closeConnection(_ sender: Any?) {
+        guard let connectionId = workspaces.selectedConnectionId else { return }
+        Task { await ConnectionCloseAction.close(connectionId: connectionId) }
+    }
+
     @objc func selectNextEditorTab(_ sender: Any?) {
         commandActions?.selectTab(offsetBy: 1)
     }

TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift

@@ -92,7 +92,7 @@ extension MainSplitViewController: NSMenuItemValidation {
         /// ours to enable and disable now.
         case #selector(newEditorTab(_:)):
             return context.isConnected
-        case #selector(closeEditorTab(_:)):
+        case #selector(closeEditorTab(_:)), #selector(closeConnection(_:)):
             return context.hasSelectedWorkspace
         case #selector(selectNextEditorTab(_:)), #selector(selectPreviousEditorTab(_:)):
             return context.isConnected
@@ -242,7 +242,7 @@ extension MainSplitViewController: NSMenuItemValidation {
         case #selector(toggleInspector(_:)):
             setTitle(isInspectorVisible ? "Hide Inspector" : "Show Inspector", on: menuItem)
         case #selector(toggleWorkspaceRail(_:)):
-            setTitle(isWorkspaceRailEnabled ? "Hide Workspace Rail" : "Show Workspace Rail", on: menuItem)
+            setTitle(isWorkspaceRailEnabled ? "Hide Connections" : "Show Connections", on: menuItem)
         case #selector(undo(_:)):
             setResolvedTitle(commandActions?.resolvedUndoTitle ?? String(localized: "Undo"), on: menuItem)
         case #selector(redo(_:)):

TablePro/Core/Services/Infrastructure/MainSplitViewController.swift

@@ -153,6 +153,12 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
             return existing
         }
 
+        /// A connection moving here from another window arrives whole. Building a new workspace for
+        /// it would replace the session state its open tabs, unsaved edits and undo stack live on.
+        if let payloadId = payload?.id, let moved = ConnectionWorkspaceHandoff.consume(for: payloadId) {
+            return workspaces.insert(moved)
+        }
+
         let resolvedConnection = DatabaseManager.shared.activeSessions[connectionId]?.connection
             ?? ConnectionStorage.shared.loadConnections().first { $0.id == connectionId }
 
@@ -1019,6 +1025,15 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         applyDefaultCollapseStateIfNoAutosave()
     }
 
+    /// Whether the connections strip holds the keyboard. A close command is interpreted by the view
+    /// that has focus, so with the strip focused it means the connection it has highlighted rather
+    /// than the editor's front tab.
+    internal var railOwnsFocus: Bool {
+        guard let navigationSidebar, navigationSidebar.isRailVisible,
+              let responder = view.window?.firstResponder as? NSView else { return false }
+        return responder.isDescendant(of: navigationSidebar.railController.view)
+    }
+
     /// A collapsed pane keeps whatever first responder it held, which would leave the window
     /// typing into a search field nobody can see.
     private func resignFirstResponderInsideChrome() {

TablePro/Core/Services/Infrastructure/WindowManager.swift

@@ -77,15 +77,54 @@ internal final class WindowManager {
         return candidates.indices.contains(index) ? candidates[index] : nil
     }
 
-    private func openInNewWindow(payload: EditorTabPayload, activate: Bool, autoConnect: Bool) {
+    /// Moves a connection the window already hosts into a window of its own, carrying its session,
+    /// tabs and unsaved work with it. The reverse of the single-window model's default, and the
+    /// workflow it took away: `NSWindow`'s own Move Tab to New Window cannot express it, because a
+    /// connection is not a window tab here.
+    ///
+    /// Refused for a window's last connection, where it would close the window and open an
+    /// identical one. The rail hides the command in that case rather than dimming it.
+    internal func canMoveToNewWindow(connectionId: UUID) -> Bool {
+        guard let host = hosts().first(where: { $0.workspaces.contains(connectionId) }) else { return false }
+        return host.workspaces.count > 1
+    }
+
+    internal func moveToNewWindow(connectionId: UUID) {
+        guard canMoveToNewWindow(connectionId: connectionId),
+              let host = hosts().first(where: { $0.workspaces.contains(connectionId) }),
+              let workspace = host.workspaces.remove(connectionId) else { return }
+        host.applySelectedWorkspace()
+
+        let payload = EditorTabPayload(connectionId: connectionId, intent: .restoreOrDefault)
+        ConnectionWorkspaceHandoff.register(workspace, for: payload.id)
+        openInNewWindow(payload: payload, activate: true, autoConnect: false, isHandoff: true)
+
+        /// The new window consumes the handoff while it builds. Anything still pending means it
+        /// never got there, and the connection would otherwise be hosted by no window at all.
+        if let stranded = ConnectionWorkspaceHandoff.reclaim(for: payload.id) {
+            host.workspaces.insert(stranded)
+            host.applySelectedWorkspace()
+        }
+    }
+
+    private func openInNewWindow(
+        payload: EditorTabPayload,
+        activate: Bool,
+        autoConnect: Bool,
+        isHandoff: Bool = false
+    ) {
         let t0 = Date()
         Self.lifecycleLogger.info(
             "[open] WindowManager.openTab start payloadId=\(payload.id, privacy: .public) connId=\(payload.connectionId, privacy: .public) intent=\(String(describing: payload.intent), privacy: .public) skipAutoExecute=\(payload.skipAutoExecute) activate=\(activate)"
         )
 
         let resolvedConnection = DatabaseManager.shared.activeSessions[payload.connectionId]?.connection
         let preCreatedSessionState: SessionStateFactory.SessionState?
-        if let resolvedConnection {
+        /// A handoff brings its own state. Building a second one here would register it pending and
+        /// leave it there, holding a coordinator for a connection that already has one.
+        if isHandoff {
+            preCreatedSessionState = nil
+        } else if let resolvedConnection {
             let state = SessionStateFactory.create(connection: resolvedConnection, payload: payload)
             SessionStateFactory.registerPending(state, for: payload.id)
             preCreatedSessionState = state