Merge pull request #1346 from manaflow-ai/task-cmdw-click-share-close-path

Share last-surface close flow between X and Cmd+W

Resources/Localizable.xcstrings

@@ -41188,13 +41188,13 @@
         "en": {
           "stringUnit": {
             "state": "translated",
-            "value": "Cmd+W Closes Workspace on Last Surface"
+            "value": "Closing Last Surface Closes Workspace"
           }
         },
         "ja": {
           "stringUnit": {
             "state": "translated",
-            "value": "最後のサーフェスでCmd+Wがワークスペースも閉じる"
+            "value": "最後のサーフェスを閉じるとワークスペースも閉じる"
           }
         }
       }
@@ -41205,13 +41205,13 @@
         "en": {
           "stringUnit": {
             "state": "translated",
-            "value": "When the focused surface is the last one in its workspace, Cmd+W closes only the surface and keeps the workspace open."
+            "value": "Closing the last surface keeps the workspace open. Use Cmd+Shift+W to close a workspace explicitly."
           }
         },
         "ja": {
           "stringUnit": {
             "state": "translated",
-            "value": "フォーカス中のサーフェスがそのワークスペースの最後の1つでも、Cmd+Wはサーフェスだけを閉じ、ワークスペースは残します。"
+            "value": "最後のサーフェスを閉じてもワークスペースは残ります。ワークスペースを明示的に閉じるにはCmd+Shift+Wを使います。"
           }
         }
       }
@@ -41222,13 +41222,13 @@
         "en": {
           "stringUnit": {
             "state": "translated",
-            "value": "When the focused surface is the last one in its workspace, Cmd+W also closes the workspace."
+            "value": "Closing the last surface also closes its workspace."
           }
         },
         "ja": {
           "stringUnit": {
             "state": "translated",
-            "value": "フォーカス中のサーフェスがそのワークスペースの最後の1つなら、Cmd+Wでワークスペースも閉じます。"
+            "value": "最後のサーフェスを閉じると、そのワークスペースも閉じます。"
           }
         }
       }

Sources/TabManager.swift

@@ -909,6 +909,7 @@ class TabManager: ObservableObject {
             portOrdinal: ordinal,
             configTemplate: inheritedConfig
         )
+        newWorkspace.owningTabManager = self
         wireClosedBrowserTracking(for: newWorkspace)
         let insertIndex = newTabInsertIndex(placementOverride: placementOverride)
         if insertIndex >= 0 && insertIndex <= tabs.count {
@@ -1357,6 +1358,7 @@ class TabManager: ObservableObject {
         AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id)
         unwireClosedBrowserTracking(for: workspace)
         workspace.teardownAllPanels()
+        workspace.owningTabManager = nil
 
         tabs.remove(at: index)
 
@@ -1379,6 +1381,7 @@ class TabManager: ObservableObject {
 
         let removed = tabs.remove(at: index)
         unwireClosedBrowserTracking(for: removed)
+        removed.owningTabManager = nil
         lastFocusedPanelByTab.removeValue(forKey: removed.id)
 
         if tabs.isEmpty {
@@ -1397,6 +1400,7 @@ class TabManager: ObservableObject {
 
     /// Attach an existing workspace to this window.
     func attachWorkspace(_ workspace: Workspace, at index: Int? = nil, select: Bool = true) {
+        workspace.owningTabManager = self
         wireClosedBrowserTracking(for: workspace)
         let insertIndex: Int = {
             guard let index else { return tabs.count }
@@ -1661,10 +1665,6 @@ class TabManager: ObservableObject {
         }
     }
 
-    private func shouldCloseWorkspaceOnLastSurfaceShortcut(_ workspace: Workspace) -> Bool {
-        LastSurfaceCloseShortcutSettings.closesWorkspace() && workspace.panels.count <= 1
-    }
-
     private func closePanelWithConfirmation(tab: Workspace, panelId: UUID) {
         guard tab.panels[panelId] != nil else {
 #if DEBUG
@@ -1690,19 +1690,13 @@ class TabManager: ObservableObject {
             "surface.close.shortcut.begin tab=\(tab.id.uuidString.prefix(5)) " +
             "panel=\(panelId.uuidString.prefix(5)) kind=\(panelKind) " +
             "panelCount=\(tab.panels.count) bonsplitTabs=\(bonsplitTabCount) " +
-            "closeWorkspaceOnLastSurface=\(shouldCloseWorkspaceOnLastSurfaceShortcut(tab) ? 1 : 0)"
+            "closeWorkspaceOnLastSurfaceSetting=\(LastSurfaceCloseShortcutSettings.closesWorkspace() ? 1 : 0)"
         )
 #endif
 
-        // Cmd+W should match Bonsplit's tab close button semantics by default. Users can opt
-        // back into the legacy behavior where closing the last surface also closes its workspace.
-        if shouldCloseWorkspaceOnLastSurfaceShortcut(tab) {
-            closeWorkspaceIfRunningProcess(tab)
-            return
-        }
-
-        // Route the default Cmd+W path through Bonsplit/Workspace close handling so it matches
-        // the tab close button behavior, including shared confirmation and replacement-panel flow.
+        // Route Cmd+W through Bonsplit/Workspace close handling so it matches the tab close
+        // button, including shared confirmation, setting-controlled last-surface behavior, and
+        // replacement-panel flow.
         let closed = tab.closePanel(panelId)
 #if DEBUG
         dlog(
@@ -4006,6 +4000,7 @@ extension TabManager {
                 workingDirectory: workspaceSnapshot.currentDirectory,
                 portOrdinal: ordinal
             )
+            workspace.owningTabManager = self
             workspace.restoreSessionSnapshot(workspaceSnapshot)
             wireClosedBrowserTracking(for: workspace)
             newTabs.append(workspace)
@@ -4015,6 +4010,7 @@ extension TabManager {
             let ordinal = Self.nextPortOrdinal
             Self.nextPortOrdinal += 1
             let fallback = Workspace(title: "Terminal 1", portOrdinal: ordinal)
+            fallback.owningTabManager = self
             wireClosedBrowserTracking(for: fallback)
             newTabs.append(fallback)
         }

Sources/Workspace.swift

@@ -946,6 +946,7 @@ final class Workspace: Identifiable, ObservableObject {
 
     /// Callback used by TabManager to capture recently closed browser panels for Cmd+Shift+T restore.
     var onClosedBrowserPanel: ((ClosedBrowserPanelRestoreSnapshot) -> Void)?
+    weak var owningTabManager: TabManager?
 
 
     // Closing tabs mutates split layout immediately; terminal views handle their own AppKit
@@ -4114,6 +4115,19 @@ final class Workspace: Identifiable, ObservableObject {
 // MARK: - BonsplitDelegate
 
 extension Workspace: BonsplitDelegate {
+    @MainActor
+    private func shouldCloseWorkspaceOnLastSurface(for tabId: TabID) -> Bool {
+        let manager = owningTabManager ?? AppDelegate.shared?.tabManagerFor(tabId: id) ?? AppDelegate.shared?.tabManager
+        guard LastSurfaceCloseShortcutSettings.closesWorkspace(),
+              panels.count <= 1,
+              panelIdFromSurfaceId(tabId) != nil,
+              let manager,
+              manager.tabs.contains(where: { $0.id == id }) else {
+            return false
+        }
+        return true
+    }
+
     @MainActor
     private func confirmClosePanel(for tabId: TabID) async -> Bool {
         let alert = NSAlert()
@@ -4529,6 +4543,12 @@ extension Workspace: BonsplitDelegate {
             return false
         }
 
+        if shouldCloseWorkspaceOnLastSurface(for: tab.id) {
+            clearStagedClosedBrowserRestoreSnapshot(for: tab.id)
+            owningTabManager?.closeWorkspaceWithConfirmation(self)
+            return false
+        }
+
         // Check if the panel needs close confirmation
         guard let panelId = panelIdFromSurfaceId(tab.id),
               let terminalPanel = terminalPanel(for: panelId) else {

Sources/cmuxApp.swift

@@ -454,7 +454,8 @@ struct cmuxApp: App {
                 // Terminal semantics:
                 // Cmd+W closes the focused tab/surface (with confirmation if needed) and keeps
                 // the workspace open by default. Cmd+Shift+W is the explicit workspace-close
-                // action, unless the user opts back into the legacy last-surface Cmd+W behavior.
+                // action, unless the user opts into closing the workspace when its last surface
+                // is closed.
                 Button(String(localized: "menu.file.closeTab", defaultValue: "Close Tab")) {
                     closePanelOrWindow()
                 }
@@ -3127,12 +3128,12 @@ struct SettingsView: View {
         if closeWorkspaceOnLastSurfaceShortcut {
             return String(
                 localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn",
-                defaultValue: "When the focused surface is the last one in its workspace, Cmd+W also closes the workspace."
+                defaultValue: "Closing the last surface also closes its workspace."
             )
         }
         return String(
             localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff",
-            defaultValue: "When the focused surface is the last one in its workspace, Cmd+W closes only the surface and keeps the workspace open."
+            defaultValue: "Closing the last surface keeps the workspace open. Use Cmd+Shift+W to close a workspace explicitly."
         )
     }
 
@@ -3504,7 +3505,7 @@ struct SettingsView: View {
                         SettingsCardDivider()
 
                         SettingsCardRow(
-                            String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Cmd+W Closes Workspace on Last Surface"),
+                            String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Closing Last Surface Closes Workspace"),
                             subtitle: closeWorkspaceOnLastSurfaceShortcutSubtitle
                         ) {
                             Toggle("", isOn: $closeWorkspaceOnLastSurfaceShortcut)

cmuxTests/CmuxWebViewKeyEquivalentTests.swift

@@ -5263,6 +5263,30 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase {
         XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId)
     }
 
+    func testClosePanelButtonKeepsWorkspaceOpenWhenItOwnsTheLastSurface() {
+        let manager = TabManager()
+        guard let workspace = manager.selectedWorkspace,
+              let initialPanelId = workspace.focusedPanelId else {
+            XCTFail("Expected selected workspace and focused panel")
+            return
+        }
+
+        let initialWorkspaceId = workspace.id
+        XCTAssertEqual(manager.tabs.count, 1)
+        XCTAssertEqual(workspace.panels.count, 1)
+
+        XCTAssertTrue(workspace.closePanel(initialPanelId))
+        drainMainQueue()
+        drainMainQueue()
+
+        XCTAssertEqual(manager.tabs.count, 1, "Closing the last surface should not remove the workspace")
+        XCTAssertEqual(manager.selectedTabId, initialWorkspaceId)
+        XCTAssertEqual(manager.tabs.first?.id, initialWorkspaceId)
+        XCTAssertNil(workspace.panels[initialPanelId], "Expected the original surface to be closed")
+        XCTAssertEqual(workspace.panels.count, 1, "Expected the workspace to stay alive with a replacement surface")
+        XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId)
+    }
+
     func testCloseCurrentPanelClosesWorkspaceWhenLastSurfaceShortcutSettingEnabled() {
         let defaults = UserDefaults.standard
         let originalSetting = defaults.object(forKey: LastSurfaceCloseShortcutSettings.key)
@@ -5289,6 +5313,39 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase {
         XCTAssertEqual(manager.selectedTabId, firstWorkspace.id)
     }
 
+    func testClosePanelButtonClosesWorkspaceWhenLastSurfaceShortcutSettingEnabled() {
+        let defaults = UserDefaults.standard
+        let originalSetting = defaults.object(forKey: LastSurfaceCloseShortcutSettings.key)
+        defaults.set(true, forKey: LastSurfaceCloseShortcutSettings.key)
+        defer {
+            if let originalSetting {
+                defaults.set(originalSetting, forKey: LastSurfaceCloseShortcutSettings.key)
+            } else {
+                defaults.removeObject(forKey: LastSurfaceCloseShortcutSettings.key)
+            }
+        }
+
+        let manager = TabManager()
+        let firstWorkspace = manager.tabs[0]
+        let secondWorkspace = manager.addWorkspace()
+
+        manager.selectWorkspace(secondWorkspace)
+        guard let secondPanelId = secondWorkspace.focusedPanelId else {
+            XCTFail("Expected focused panel in selected workspace")
+            return
+        }
+
+        XCTAssertEqual(manager.selectedTabId, secondWorkspace.id)
+        XCTAssertEqual(secondWorkspace.panels.count, 1)
+
+        XCTAssertFalse(secondWorkspace.closePanel(secondPanelId))
+        drainMainQueue()
+        drainMainQueue()
+
+        XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id])
+        XCTAssertEqual(manager.selectedTabId, firstWorkspace.id)
+    }
+
     func testCloseCurrentPanelWithLegacySettingIgnoresStaleSurfaceId() {
         let defaults = UserDefaults.standard
         let originalSetting = defaults.object(forKey: LastSurfaceCloseShortcutSettings.key)