From 619b4fd338a3aa0c47ae0105c477d381c56de5cc Mon Sep 17 00:00:00 2001 From: Austin Wang <38676809+austinywang@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:53:13 -0700 Subject: [PATCH 1/5] test: chief.enabled off must release the pinned Home workspace (red) Regression test for https://github.com/manaflow-ai/cmux/issues/26003. Co-Authored-By: Claude Opus 5.5 --- cmux.xcodeproj/project.pbxproj | 4 ++ cmuxTests/ChiefDisabledHomeTests.swift | 67 ++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 cmuxTests/ChiefDisabledHomeTests.swift diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index a05bb89a3fd9..bf319cccbf46 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -641,6 +641,7 @@ C41EF3684F6AEFB1FBCE143A /* ChiefControlRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C41EF4F63A64D736DCDC0FFD /* ChiefControlRequest.swift */; }; D6A4A997D5FFA4275447E76B /* ChiefControlRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 293639DF6364D4A8F4033D79 /* ChiefControlRequestTests.swift */; }; C41EF9F9E5F927F28B6946A5 /* ChiefConversationListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = C41EF3329C1652D4C80B6991 /* ChiefConversationListView.swift */; }; + E0C2D88C793FF69C01AFE7AD /* ChiefDisabledHomeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4189B3FB78ACEE79AE12B1AA /* ChiefDisabledHomeTests.swift */; }; B5EDBC73F930461BB073BBEB /* ChiefDispatchPlan.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C636D86CB494560BBF6953B /* ChiefDispatchPlan.swift */; }; BB4B1E8F6FBC43FDBBD8A4BD /* ChiefDispatchPlan.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C636D86CB494560BBF6953B /* ChiefDispatchPlan.swift */; }; C41EFE522A9F79BCD582CCDE /* ChiefEngineSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = C41EF322A13A3F710137F679 /* ChiefEngineSettings.swift */; }; @@ -5492,6 +5493,7 @@ C41EF4F63A64D736DCDC0FFD /* ChiefControlRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Chief/ChiefControlRequest.swift; sourceTree = ""; }; 293639DF6364D4A8F4033D79 /* ChiefControlRequestTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ChiefControlRequestTests.swift"; sourceTree = ""; }; C41EF3329C1652D4C80B6991 /* ChiefConversationListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Chief/ChiefConversationListView.swift; sourceTree = ""; }; + 4189B3FB78ACEE79AE12B1AA /* ChiefDisabledHomeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ChiefDisabledHomeTests.swift"; sourceTree = ""; }; 2C636D86CB494560BBF6953B /* ChiefDispatchPlan.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChiefDispatchPlan.swift; sourceTree = ""; }; C41EF322A13A3F710137F679 /* ChiefEngineSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Chief/ChiefEngineSettings.swift; sourceTree = ""; }; C41EFACFA450D15C2AD8EF63 /* ChiefFirstRunAgents.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Chief/ChiefFirstRunAgents.swift; sourceTree = ""; }; @@ -14281,6 +14283,7 @@ FBFA082D961AFAB3FFBFF6CC /* RawOSCMachinePayloadTests.swift */, E5E915CF2237052FDE02EE05 /* WorkspaceTitleRecoveryTests.swift */, AD2084D340B702301EBB1356 /* SurfaceSocketCommandTests+NewDisplay.swift */, + 4189B3FB78ACEE79AE12B1AA /* ChiefDisabledHomeTests.swift */, ); path = cmuxTests; sourceTree = ""; @@ -18473,6 +18476,7 @@ 852600010000000000000002 /* CanvasPaneContentMountTests.swift in Sources */, C4A570030000000000000001 /* CanvasShortcutContextTests.swift in Sources */, D6A4A997D5FFA4275447E76B /* ChiefControlRequestTests.swift in Sources */, + E0C2D88C793FF69C01AFE7AD /* ChiefDisabledHomeTests.swift in Sources */, F3CF0E3AB3D16A241EF1C013 /* ChiefHomeNavigationFocusTests.swift in Sources */, F6174997FACCB1C84EDEAC1C /* ChiefListAndSettingsCopyTests.swift in Sources */, 19E97DB221B879A7A25AB8AC /* ChiefPanelSessionTests.swift in Sources */, diff --git a/cmuxTests/ChiefDisabledHomeTests.swift b/cmuxTests/ChiefDisabledHomeTests.swift new file mode 100644 index 000000000000..93b92a69e06f --- /dev/null +++ b/cmuxTests/ChiefDisabledHomeTests.swift @@ -0,0 +1,67 @@ +import AppKit +import CmuxSettings +import Foundation +import Testing + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// `chief.enabled: false` releases Chief Home: the Home group unpins, the Home +/// workspace and its Chief pane close, and nothing keeps them pinned +/// (https://github.com/manaflow-ai/cmux/issues/26003). +@MainActor +@Suite("Chief disabled releases Home", .serialized) +struct ChiefDisabledHomeTests { + /// Restores `chief.enabled` after a test changes it. + private struct ChiefEnabledOverride { + let key = ChiefCatalogSection().enabled.userDefaultsKey + let original: Any? + + init(_ enabled: Bool) { + original = UserDefaults.standard.object(forKey: key) + ChiefSettings.setEnabled(enabled) + } + + func restore() { + if let original { + UserDefaults.standard.set(original, forKey: key) + } else { + UserDefaults.standard.removeObject(forKey: key) + } + } + } + + @Test func disablingChiefLetsTheHomeGroupUnpinAndHomeClose() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let groupId = try #require(home.groupId) + #expect(manager.workspaceGroups.first { $0.id == groupId }?.isPinned == true) + + // Enabled: Home stays the pinned, protected anchor. + manager.setWorkspaceGroupPinned(groupId: groupId, isPinned: false) + #expect(manager.workspaceGroups.first { $0.id == groupId }?.isPinned == true) + #expect(!manager.canCloseWorkspace(home, allowPinned: true)) + + ChiefSettings.setEnabled(false) + manager.setWorkspaceGroupPinned(groupId: groupId, isPinned: false) + #expect(manager.workspaceGroups.first { $0.id == groupId }?.isPinned == false) + #expect(manager.canCloseWorkspace(home, allowPinned: true)) + manager.closeWorkspace(home, recordHistory: false) + #expect(!manager.tabs.contains { $0.id == home.id }) + } + + @Test func disabledChiefLetsTheHomePaneClose() throws { + let setting = ChiefEnabledOverride(false) + defer { setting.restore() } + let workspace = Workspace() + defer { for panel in workspace.panels.values { panel.close() } } + let paneId = try #require(workspace.bonsplitController.focusedPaneId) + let home = try #require(workspace.newChiefSurface(inPane: paneId, isHome: true, focus: false)) + #expect(workspace.closePanel(home.id, force: true)) + } +} From 803ca3a899bfad6755cc375643597dd45df0f012 Mon Sep 17 00:00:00 2001 From: Austin Wang <38676809+austinywang@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:12:19 -0700 Subject: [PATCH 2/5] Release Chief Home when chief.enabled is off With chief.enabled false, Home was still protected by every pin/close guard and the Chief service kept running, so the Home workspace could never be unpinned or closed and Home still started a harness. - Home protection (isChiefHome, isChiefHomeGroup, the Home pane's close guards) applies only while chief.enabled is on. - Turning Chief off, opening a window, or applying a session snapshot removes Home and unpins its group; panes the user added to Home stay. - ChiefService stops (ends turns and harness processes, drops the store) when the setting turns off, and start() refuses while it is off. - workspace-group unpin on the protected Home group returns a protected error instead of OK. Fixes https://github.com/manaflow-ai/cmux/issues/26003 Co-Authored-By: Claude Opus 5.5 --- Resources/Localizable.xcstrings | 59 +++++++++++++++++ Sources/AppDelegate.swift | 2 + Sources/Chief/ChiefService+Control.swift | 2 +- Sources/Chief/ChiefService.swift | 50 +++++++++++++- Sources/ChiefHomeNavigation.swift | 25 +++++++ Sources/ContentView.swift | 3 + Sources/Panels/ChiefPanel.swift | 3 + Sources/TabManager.swift | 3 +- ...minalController+WorkspaceGroupAction.swift | 12 ++++ Sources/Workspace+ChiefPane.swift | 7 ++ Sources/Workspace.swift | 6 +- cmuxTests/ChiefDisabledHomeTests.swift | 65 +++++++++++++++++++ 12 files changed, 229 insertions(+), 8 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 20d31b355d90..2ce8b8ccb700 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -671652,6 +671652,65 @@ } } } + }, + "workspaceGroup.error.chiefHomeCannotUnpin": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "The Home group stays pinned while Chief is enabled" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "تبقى مجموعة Home مثبّتة ما دام Chief مفعّلًا" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "Die Gruppe „Home“ bleibt angeheftet, solange Chief aktiviert ist" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "El grupo Home permanece fijado mientras Chief esté activado" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "Le groupe Home reste épinglé tant que Chief est activé" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "Chief が有効な間、Home グループは固定されたままです" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "Chief가 활성화된 동안 Home 그룹은 고정된 상태로 유지됩니다" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "Chief 启用时，Home 分组会保持固定" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "Chief 啟用時，Home 群組會保持釘選" + } + } + } } }, "version": "1.0" diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index f6f3c7acb70f..87c94e546a01 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -4331,6 +4331,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent deferBrowserPanels: true, workspaceCreateIdempotencyCache: TerminalController.shared.workspaceCreateIdempotencyCache ) + // A snapshot saved with Chief on carries its Home; with Chief off, drop it. + ChiefHomeNavigation.removeHome(in: context.tabManager) context.restoreWindowDockSessionSnapshot( snapshot, notificationStore: notificationStore, diff --git a/Sources/Chief/ChiefService+Control.swift b/Sources/Chief/ChiefService+Control.swift index dc9ae781da09..7d3d91a94517 100644 --- a/Sources/Chief/ChiefService+Control.swift +++ b/Sources/Chief/ChiefService+Control.swift @@ -131,7 +131,7 @@ extension ChiefService { start() let failure = startFailure ?? ChiefStrings.unavailable if let ownerLoad { return (ownerLoad, failure) } - // Not reached: start() always sets the load. + // start() sets the load unless `chief.enabled` is off. return (Task { throw ChiefControlFailure.unavailable(failure) }, failure) } diff --git a/Sources/Chief/ChiefService.swift b/Sources/Chief/ChiefService.swift index 76575f0bff28..a7ca894ddb22 100644 --- a/Sources/Chief/ChiefService.swift +++ b/Sources/Chief/ChiefService.swift @@ -49,6 +49,9 @@ final class ChiefService { /// changes only with ``workingConversations``, so Observation tracks that. @ObservationIgnored private(set) var turnStarts: [ConversationID: ContinuousClock.Instant] = [:] @ObservationIgnored private var started = false + /// Bumped by each ``start()`` and ``stopBecauseDisabled()``; an owner load + /// that finishes after a stop sees a newer value and shuts itself down. + @ObservationIgnored private var startGeneration = 0 /// The list's preview lines by conversation (ChiefService+Previews.swift), with the row state /// each was read at. @ObservationIgnored var previewLines: [ConversationID: ChiefPreviewEntry] = [:] @@ -125,10 +128,12 @@ final class ChiefService { /// Opens the Chief home and starts the store. Idempotent, and cheap on /// the main thread: the owner loads on a background task and the store - /// connects when it is ready. + /// connects when it is ready. Does nothing while `chief.enabled` is off. func start() { - guard !started else { return } + guard !started, ChiefSettings.isEnabled else { return } started = true + startGeneration &+= 1 + let generation = startGeneration let (home, temporary) = Self.resolveHome() let settings = ChiefSettingsAccess().current engine = settings @@ -156,13 +161,48 @@ final class ChiefService { Task { @MainActor [weak self] in do { let owner = try await loading.value - self?.ownerLoaded(owner) + guard let self, self.startGeneration == generation else { + await owner.shutdown() + return + } + self.ownerLoaded(owner) } catch { + guard self?.startGeneration == generation else { return } self?.ownerFailed(error) } } } + /// `chief.enabled` turned off: ends every turn and the brain's processes + /// and drops the store, so nothing reaches a harness until Chief is + /// enabled and started again. Open Chief views keep the stopped store and + /// can no longer send. + func stopBecauseDisabled() { + guard started else { return } + started = false + startGeneration &+= 1 + let owner = self.owner + storeIfStarted?.stop() + storeIfStarted = nil + startFailure = nil + self.owner = nil + ownerLoad = nil + home = nil + allowRules = nil + brainSwitch = nil + workingWatch?.cancel() + workingWatch = nil + workingConversations = [] + turnStarts = [:] + if let settingsObserver { NotificationCenter.default.removeObserver(settingsObserver) } + settingsObserver = nil + if let quitObserver { NotificationCenter.default.removeObserver(quitObserver) } + quitObserver = nil + guard let owner else { return } + // task-owner: one shutdown of the stopped owner; ends when its turns and processes end + Task.detached { await owner.shutdown() } + } + private func ownerLoaded(_ owner: ChiefConversationOwner) { self.owner = owner for problem in owner.loadProblems { chiefServiceLog.error("load problem: \(problem, privacy: .private)") } @@ -262,6 +302,10 @@ final class ChiefService { } private func applySettingsIfChanged() { + guard ChiefSettings.isEnabled else { + stopBecauseDisabled() + return + } let settings = ChiefSettingsAccess().current guard settings != engine, let brainSwitch else { return } engine = settings diff --git a/Sources/ChiefHomeNavigation.swift b/Sources/ChiefHomeNavigation.swift index a2bf649bc978..f791c758d37a 100644 --- a/Sources/ChiefHomeNavigation.swift +++ b/Sources/ChiefHomeNavigation.swift @@ -98,6 +98,31 @@ struct ChiefHomeNavigation { return home } + /// With `chief.enabled` off, removes Chief Home and unpins the Home group, + /// so a disabled Chief leaves nothing pinned in the sidebar. A Home + /// workspace holding only Chief panes closes (a window whose only + /// workspace it is gets a fresh workspace first); one the user added other + /// panes to keeps them and loses only its Home pane. + static func removeHome(in manager: TabManager) { + guard !ChiefSettings.isEnabled, !manager.isFinalizedForWindowClose else { return } + for home in manager.tabs where home.holdsChiefHomePane { + if home.panels.values.allSatisfy({ $0 is ChiefPanel }) { + if manager.tabs.count == 1 { + guard manager.addWorkspaceIfActive(select: true) != nil else { return } + } + manager.closeWorkspace(home, recordHistory: false) + } else { + let homePanes = home.panels.values.compactMap { $0 as? ChiefPanel }.filter(\.isHome) + home.withClosedPanelHistorySuppressed { + for pane in homePanes { _ = home.closePanel(pane.id, force: true) } + } + } + } + for group in manager.workspaceGroups where group.externalID == homeGroupExternalID && group.isPinned { + manager.setWorkspaceGroupPinned(groupId: group.id, isPinned: false) + } + } + /// Opens a conversation as an ordinary workspace under Home; existing views are reused. @discardableResult static func openConversation(_ id: ConversationID, in manager: TabManager, select: Bool = true) -> Workspace? { diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index b5ebb46c093a..abfc97522ff8 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -2653,6 +2653,8 @@ struct ContentView: View { view = AnyView(view.onAppear { ChiefHomeNavigation.restoreConversations(in: tabManager) + // A session saved with Chief on restores its Home; with Chief off, drop it. + ChiefHomeNavigation.removeHome(in: tabManager) sidebarState.installVisibilityWillChangeHandler(ownerId: windowId) { isVisible in if !isVisible { restoreMainPanelFocusAfterAppKitSidebarHiddenIfNeeded() @@ -2804,6 +2806,7 @@ struct ContentView: View { _ = ChiefHomeNavigation.ensureHome(in: tabManager) } else { ChiefHomeNavigation(sidebarSelectionState: sidebarSelectionState, tabManager: tabManager).leave() + ChiefHomeNavigation.removeHome(in: tabManager) } }) diff --git a/Sources/Panels/ChiefPanel.swift b/Sources/Panels/ChiefPanel.swift index 6c28a725f86e..e16a8973a3f4 100644 --- a/Sources/Panels/ChiefPanel.swift +++ b/Sources/Panels/ChiefPanel.swift @@ -16,6 +16,9 @@ final class ChiefPanel: Panel { /// Home is protected in the split tree; dragged copies are ordinary Chief panes. let isHome: Bool + /// Whether the split tree refuses to close this pane: canonical Home while `chief.enabled` is on. + var isProtectedHome: Bool { isHome && ChiefSettings.isEnabled } + /// The conversation this pane shows; nil means Chief's default conversation. @Published var conversationID: ConversationID? diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 7fb957d7f7f2..a765680155a3 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -2441,7 +2441,8 @@ class TabManager: ObservableObject { } func isChiefHomeGroup(_ groupId: UUID) -> Bool { - workspaceGroups.contains { $0.id == groupId && $0.externalID == ChiefHomeNavigation.homeGroupExternalID } + guard ChiefSettings.isEnabled else { return false } + return workspaceGroups.contains { $0.id == groupId && $0.externalID == ChiefHomeNavigation.homeGroupExternalID } || tabs.contains { $0.groupId == groupId && $0.isChiefHome } } diff --git a/Sources/TerminalController+WorkspaceGroupAction.swift b/Sources/TerminalController+WorkspaceGroupAction.swift index bcde1a4bc712..2cdfa0ca235e 100644 --- a/Sources/TerminalController+WorkspaceGroupAction.swift +++ b/Sources/TerminalController+WorkspaceGroupAction.swift @@ -68,6 +68,18 @@ extension TerminalController { case .pin: tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: true) case .unpin: + // Chief Home stays pinned while `chief.enabled` is on; say so instead of reporting OK. + guard !tabManager.isChiefHomeGroup(groupID) else { + mutationError = .err( + code: "protected", + message: String( + localized: "workspaceGroup.error.chiefHomeCannotUnpin", + defaultValue: "The Home group stays pinned while Chief is enabled" + ), + data: ["group_id": groupID.uuidString] + ) + return + } tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: false) case .rename: guard let title else { diff --git a/Sources/Workspace+ChiefPane.swift b/Sources/Workspace+ChiefPane.swift index 7471d9d92c93..2b405639053e 100644 --- a/Sources/Workspace+ChiefPane.swift +++ b/Sources/Workspace+ChiefPane.swift @@ -6,7 +6,14 @@ import Foundation /// Workspace surface creation for Chief panes: a Chief conversation as a tab or a split. extension Workspace { /// True when this workspace owns its protected canonical Chief Home pane. + /// Home is protected only while `chief.enabled` is on; with Chief off it is + /// an ordinary workspace the user can unpin and close. var isChiefHome: Bool { + ChiefSettings.isEnabled && holdsChiefHomePane + } + + /// True when this workspace holds the canonical Home pane, whether or not Chief is enabled. + var holdsChiefHomePane: Bool { panels.values.contains { ($0 as? ChiefPanel)?.isHome == true } } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 610082f193bb..8ea45f8f9430 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -11405,7 +11405,7 @@ final class Workspace: Identifiable, ObservableObject, FilePreviewTabMetadataHos // intentionally pinned (which hides Bonsplit's close affordance), but // keep this guard for force/API paths that bypass the tab chrome. if let panelId = panelIdFromSurfaceId(tabId), - (panels[panelId] as? ChiefPanel)?.isHome == true { + (panels[panelId] as? ChiefPanel)?.isProtectedHome == true { return false } if force { forceCloseTabIds.insert(tabId) } @@ -14584,7 +14584,7 @@ extension Workspace: BonsplitDelegate { // Pinned protects normal Bonsplit UI; this delegate guard covers // callers that ask the controller to close directly. if let panelId = panelIdFromSurfaceId(tab.id), - (panels[panelId] as? ChiefPanel)?.isHome == true { + (panels[panelId] as? ChiefPanel)?.isProtectedHome == true { return false } func recordPostCloseState() { @@ -15216,7 +15216,7 @@ extension Workspace: BonsplitDelegate { let tabs = controller.tabs(inPane: pane) if tabs.contains(where: { tab in guard let panelId = panelIdFromSurfaceId(tab.id) else { return false } - return (panels[panelId] as? ChiefPanel)?.isHome == true + return (panels[panelId] as? ChiefPanel)?.isProtectedHome == true }) { return false } diff --git a/cmuxTests/ChiefDisabledHomeTests.swift b/cmuxTests/ChiefDisabledHomeTests.swift index 93b92a69e06f..f90bafd9c2fb 100644 --- a/cmuxTests/ChiefDisabledHomeTests.swift +++ b/cmuxTests/ChiefDisabledHomeTests.swift @@ -64,4 +64,69 @@ struct ChiefDisabledHomeTests { let home = try #require(workspace.newChiefSurface(inPane: paneId, isHome: true, focus: false)) #expect(workspace.closePanel(home.id, force: true)) } + + @Test func removeHomeClosesHomeAndUnpinsItsGroupWhenChiefIsOff() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let groupId = try #require(home.groupId) + let member = try #require(manager.tabs.first { !$0.holdsChiefHomePane }) + manager.addWorkspaceToGroup(workspaceId: member.id, groupId: groupId) + + // Enabled: removal is a no-op. + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.tabs.contains { $0.id == home.id }) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(!manager.tabs.contains { $0.holdsChiefHomePane }) + #expect(manager.tabs.contains { $0.id == member.id }) + #expect(manager.workspaceGroups.allSatisfy { + $0.externalID != ChiefHomeNavigation.homeGroupExternalID || !$0.isPinned + }) + #expect(ChiefHomeNavigation.ensureHome(in: manager) == nil) + } + + @Test func removeHomeLeavesAWorkspaceWhenHomeWasTheOnlyOne() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + for workspace in manager.tabs where workspace.id != home.id { + manager.closeWorkspace(workspace, recordHistory: false) + } + #expect(manager.tabs.map(\.id) == [home.id]) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.tabs.count == 1) + #expect(!manager.tabs.contains { $0.holdsChiefHomePane }) + #expect(manager.selectedWorkspace != nil) + } + + @Test func disabledChiefServiceDoesNotStart() { + let setting = ChiefEnabledOverride(false) + defer { setting.restore() } + ChiefService.shared.start() + #expect(ChiefService.shared.storeIfStarted == nil) + #expect(ChiefService.shared.store == nil) + } + + @Test func removeHomeKeepsPanesTheUserAddedToHome() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let homePane = try #require(home.panels.values.compactMap { $0 as? ChiefPanel }.first(where: \.isHome)) + let paneId = try #require(home.bonsplitController.focusedPaneId) + let terminal = try #require(home.newTerminalSurface(inPane: paneId, focus: false)) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.tabs.contains { $0.id == home.id }) + #expect(home.panels[homePane.id] == nil) + #expect(home.panels[terminal.id] != nil) + #expect(!home.holdsChiefHomePane) + } } From bf19d20e5b5a4d31d868b7a548025212beeeeea6 Mon Sep 17 00:00:00 2001 From: Austin Wang <38676809+austinywang@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:20:14 -0700 Subject: [PATCH 3/5] Report a refused workspace-group unpin as protected on the CLI path The CLI's workspace.group.unpin goes through the control coordinator, which reported OK whenever the group existed. setWorkspaceGroupPinned now returns whether it applied, and the control seam returns a typed resolution so a refused unpin is a protected error. removeHome keeps unpinning when it cannot add a replacement workspace, and a Chief stop also clears its preview cache. Co-Authored-By: Claude Opus 5.5 --- ...rolCommandCoordinator+WorkspaceGroup.swift | 18 +++++++--- .../ControlWorkspaceGroupContext.swift | 6 ++-- .../ControlWorkspaceGroupPinResolution.swift | 18 ++++++++++ .../ControlCommandContextTestStubs.swift | 2 +- ...CoordinatorWorkspaceGroupSafetyTests.swift | 33 +++++++++++++++++++ .../FakeWorkspaceGroupSafetyContext.swift | 9 +++++ Sources/Chief/ChiefService.swift | 2 ++ Sources/ChiefHomeNavigation.swift | 2 +- Sources/TabManager.swift | 16 +++++++-- ...troller+ControlWorkspaceGroupContext.swift | 12 ++++--- ...minalController+WorkspaceGroupAction.swift | 8 ++--- 11 files changed, 103 insertions(+), 23 deletions(-) create mode 100644 Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupPinResolution.swift diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlCommandCoordinator+WorkspaceGroup.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlCommandCoordinator+WorkspaceGroup.swift index 6a0a9fbc855a..0b347be7143e 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlCommandCoordinator+WorkspaceGroup.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlCommandCoordinator+WorkspaceGroup.swift @@ -229,14 +229,22 @@ extension ControlCommandCoordinator { guard let gid = uuid(params, "group_id") else { return .err(code: "invalid_params", message: "Missing or invalid group_id", data: nil) } - guard let ok = context?.controlSetWorkspaceGroupPinned( + let resolution = context?.controlSetWorkspaceGroupPinned( routing: routingSelectors(params), groupID: gid, isPinned: isPinned - ) else { + ) ?? .tabManagerUnavailable + switch resolution { + case .tabManagerUnavailable: return .err(code: "unavailable", message: "TabManager not available", data: nil) + case .notFound: + return .err(code: "not_found", message: "Group not found", data: .object(["group_id": .string(gid.uuidString)])) + case .protected(let message): + return .err(code: "protected", message: message, data: .object([ + "group_id": .string(gid.uuidString), + "is_pinned": .bool(!isPinned), + ])) + case .applied: + return .ok(.object(["group_id": .string(gid.uuidString), "is_pinned": .bool(isPinned)])) } - return ok - ? .ok(.object(["group_id": .string(gid.uuidString), "is_pinned": .bool(isPinned)])) - : .err(code: "not_found", message: "Group not found", data: .object(["group_id": .string(gid.uuidString)])) } // MARK: - Add / Remove / Anchor diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupContext.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupContext.swift index 6391ffc8ebe9..5b554c7707b4 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupContext.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupContext.swift @@ -118,13 +118,13 @@ public protocol ControlWorkspaceGroupContext: AnyObject { /// - routing: The routing selectors used for TabManager resolution. /// - groupID: The group to update. /// - isPinned: The new pinned state. - /// - Returns: `true` if the group existed and was updated, `false` if not - /// found, or `nil` if no TabManager resolved. + /// - Returns: Whether the change applied, the group was missing, the app + /// refused it, or no TabManager resolved. func controlSetWorkspaceGroupPinned( routing: ControlRoutingSelectors, groupID: UUID, isPinned: Bool - ) -> Bool? + ) -> ControlWorkspaceGroupPinResolution /// Adds a workspace to a group for `workspace.group.add`. /// diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupPinResolution.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupPinResolution.swift new file mode 100644 index 000000000000..f8b61e97d69e --- /dev/null +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupPinResolution.swift @@ -0,0 +1,18 @@ +public import Foundation + +/// The outcome of `workspace.group.pin` / `.unpin`. +/// +/// The legacy body reported success whenever the group existed, even when the +/// app refused the change (a protected group such as Chief Home stays pinned), +/// so the CLI printed `OK` for an unpin that did nothing. +public enum ControlWorkspaceGroupPinResolution: Sendable, Equatable { + /// No TabManager resolved (legacy `unavailable` / "TabManager not + /// available"). + case tabManagerUnavailable + /// The group was not found (legacy `not_found` / "Group not found"). + case notFound + /// The app refused the change. Carries the app's localized reason. + case protected(message: String) + /// The group now has the requested pinned state. + case applied +} diff --git a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift index 6fa6a56b536a..08ac6e938a22 100644 --- a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift +++ b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift @@ -260,7 +260,7 @@ extension ControlWorkspaceGroupContext { func controlDeleteWorkspaceGroup(routing: ControlRoutingSelectors, groupID: UUID) -> Int? { nil } func controlRenameWorkspaceGroup(routing: ControlRoutingSelectors, groupID: UUID, name: String) -> Bool? { nil } func controlSetWorkspaceGroupCollapsed(routing: ControlRoutingSelectors, groupID: UUID, isCollapsed: Bool) -> Bool? { nil } - func controlSetWorkspaceGroupPinned(routing: ControlRoutingSelectors, groupID: UUID, isPinned: Bool) -> Bool? { nil } + func controlSetWorkspaceGroupPinned(routing: ControlRoutingSelectors, groupID: UUID, isPinned: Bool) -> ControlWorkspaceGroupPinResolution { .tabManagerUnavailable } func controlAddWorkspaceToGroup(routing: ControlRoutingSelectors, groupID: UUID, workspaceID: UUID, placement: WorkspaceGroupNewPlacement?, referenceWorkspaceID: UUID?) -> ControlWorkspaceGroupAddResolution { .tabManagerUnavailable } diff --git a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceGroupSafetyTests.swift b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceGroupSafetyTests.swift index e529dbdcb45d..886fbcf7305c 100644 --- a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceGroupSafetyTests.swift +++ b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceGroupSafetyTests.swift @@ -189,6 +189,39 @@ struct ControlCommandCoordinatorWorkspaceGroupSafetyTests { #expect(context.deletedGroupIDs.isEmpty) } + @Test func refusedUnpinReportsProtectedInsteadOfOK() { + let context = FakeWorkspaceGroupSafetyContext() + context.pinResolution = .protected(message: "stays pinned") + let coordinator = ControlCommandCoordinator(context: context) + let groupID = UUID() + + guard case .err(let code, let message, _) = coordinator.handle(request( + "workspace.group.unpin", + ["group_id": .string(groupID.uuidString)] + )) else { + Issue.record("a refused unpin reported success") + return + } + + #expect(code == "protected") + #expect(message == "stays pinned") + } + + @Test func appliedUnpinReportsTheNewState() { + let context = FakeWorkspaceGroupSafetyContext() + let coordinator = ControlCommandCoordinator(context: context) + + guard case .ok(.object(let payload)) = coordinator.handle(request( + "workspace.group.unpin", + ["group_id": .string(UUID().uuidString)] + )) else { + Issue.record("an applied unpin did not succeed") + return + } + + #expect(payload["is_pinned"] == .bool(false)) + } + private func request( _ method: String, _ params: [String: JSONValue] = [:] diff --git a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeWorkspaceGroupSafetyContext.swift b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeWorkspaceGroupSafetyContext.swift index 08e13e0d07e4..9df2444b3606 100644 --- a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeWorkspaceGroupSafetyContext.swift +++ b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeWorkspaceGroupSafetyContext.swift @@ -11,6 +11,15 @@ final class FakeWorkspaceGroupSafetyContext: ControlCommandContext { var removeGeneratedAnchor = false var deletedGroupIDs: [UUID] = [] var deleteResult = 2 + var pinResolution: ControlWorkspaceGroupPinResolution = .applied + + func controlSetWorkspaceGroupPinned( + routing: ControlRoutingSelectors, + groupID: UUID, + isPinned: Bool + ) -> ControlWorkspaceGroupPinResolution { + pinResolution + } func controlCreateWorkspaceGroup( routing: ControlRoutingSelectors, diff --git a/Sources/Chief/ChiefService.swift b/Sources/Chief/ChiefService.swift index a7ca894ddb22..392a83099645 100644 --- a/Sources/Chief/ChiefService.swift +++ b/Sources/Chief/ChiefService.swift @@ -194,6 +194,8 @@ final class ChiefService { workingWatch = nil workingConversations = [] turnStarts = [:] + previewLines = [:] + previewReads = [] if let settingsObserver { NotificationCenter.default.removeObserver(settingsObserver) } settingsObserver = nil if let quitObserver { NotificationCenter.default.removeObserver(quitObserver) } diff --git a/Sources/ChiefHomeNavigation.swift b/Sources/ChiefHomeNavigation.swift index f791c758d37a..fb0ccf3a363d 100644 --- a/Sources/ChiefHomeNavigation.swift +++ b/Sources/ChiefHomeNavigation.swift @@ -108,7 +108,7 @@ struct ChiefHomeNavigation { for home in manager.tabs where home.holdsChiefHomePane { if home.panels.values.allSatisfy({ $0 is ChiefPanel }) { if manager.tabs.count == 1 { - guard manager.addWorkspaceIfActive(select: true) != nil else { return } + guard manager.addWorkspaceIfActive(select: true) != nil else { continue } } manager.closeWorkspace(home, recordHistory: false) } else { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index a765680155a3..4fba52a232e8 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -2463,9 +2463,21 @@ class TabManager: ObservableObject { workspaceGrouping.toggleWorkspaceGroupPinned(groupId: groupId) } - func setWorkspaceGroupPinned(groupId: UUID, isPinned: Bool) { - guard isPinned || workspaceGroupAnchor(for: groupId)?.isChiefHome != true else { return } + /// Pins or unpins a group. Returns false when the change was refused: + /// Chief Home's group stays pinned while `chief.enabled` is on. + @discardableResult + func setWorkspaceGroupPinned(groupId: UUID, isPinned: Bool) -> Bool { + guard isPinned || workspaceGroupAnchor(for: groupId)?.isChiefHome != true else { return false } workspaceGrouping.setWorkspaceGroupPinned(groupId: groupId, isPinned: isPinned) + return true + } + + /// Why an unpin of Chief Home's group was refused, for socket callers. + static var chiefHomeCannotUnpinMessage: String { + String( + localized: "workspaceGroup.error.chiefHomeCannotUnpin", + defaultValue: "The Home group stays pinned while Chief is enabled" + ) } func setWorkspaceGroupColor(groupId: UUID, hex: String?) { diff --git a/Sources/TerminalController+ControlWorkspaceGroupContext.swift b/Sources/TerminalController+ControlWorkspaceGroupContext.swift index b5dcea5bfc72..b81feb4bdab4 100644 --- a/Sources/TerminalController+ControlWorkspaceGroupContext.swift +++ b/Sources/TerminalController+ControlWorkspaceGroupContext.swift @@ -244,11 +244,13 @@ extension TerminalController: ControlWorkspaceGroupContext { routing: ControlRoutingSelectors, groupID: UUID, isPinned: Bool - ) -> Bool? { - guard let tabManager = resolveTabManager(routing: routing) else { return nil } - let ok = tabManager.workspaceGroups.contains(where: { $0.id == groupID }) - if ok { tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: isPinned) } - return ok + ) -> ControlWorkspaceGroupPinResolution { + guard let tabManager = resolveTabManager(routing: routing) else { return .tabManagerUnavailable } + guard tabManager.workspaceGroups.contains(where: { $0.id == groupID }) else { return .notFound } + guard tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: isPinned) else { + return .protected(message: TabManager.chiefHomeCannotUnpinMessage) + } + return .applied } func controlAddWorkspaceToGroup( diff --git a/Sources/TerminalController+WorkspaceGroupAction.swift b/Sources/TerminalController+WorkspaceGroupAction.swift index 2cdfa0ca235e..bbd5d8266d23 100644 --- a/Sources/TerminalController+WorkspaceGroupAction.swift +++ b/Sources/TerminalController+WorkspaceGroupAction.swift @@ -69,18 +69,14 @@ extension TerminalController { tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: true) case .unpin: // Chief Home stays pinned while `chief.enabled` is on; say so instead of reporting OK. - guard !tabManager.isChiefHomeGroup(groupID) else { + guard tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: false) else { mutationError = .err( code: "protected", - message: String( - localized: "workspaceGroup.error.chiefHomeCannotUnpin", - defaultValue: "The Home group stays pinned while Chief is enabled" - ), + message: TabManager.chiefHomeCannotUnpinMessage, data: ["group_id": groupID.uuidString] ) return } - tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: false) case .rename: guard let title else { mutationError = .err(code: "invalid_params", message: "Missing or invalid title", data: nil) From fe8bb5d8870ade8e8003e648c7b46ccd0095a872 Mon Sep 17 00:00:00 2001 From: Austin Wang <38676809+austinywang@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:37:23 -0700 Subject: [PATCH 4/5] test: avoid a rethrowing call inside #require Co-Authored-By: Claude Opus 5.5 --- cmuxTests/ChiefDisabledHomeTests.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmuxTests/ChiefDisabledHomeTests.swift b/cmuxTests/ChiefDisabledHomeTests.swift index f90bafd9c2fb..b96eb8678320 100644 --- a/cmuxTests/ChiefDisabledHomeTests.swift +++ b/cmuxTests/ChiefDisabledHomeTests.swift @@ -118,7 +118,8 @@ struct ChiefDisabledHomeTests { defer { setting.restore() } let manager = TabManager() let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) - let homePane = try #require(home.panels.values.compactMap { $0 as? ChiefPanel }.first(where: \.isHome)) + let homePanes = home.panels.values.compactMap { $0 as? ChiefPanel }.filter { $0.isHome } + let homePane = try #require(homePanes.first) let paneId = try #require(home.bonsplitController.focusedPaneId) let terminal = try #require(home.newTerminalSurface(inPane: paneId, focus: false)) From 5acc33f6cbe1cea31f46795103122d632c9ccf3c Mon Sep 17 00:00:00 2001 From: Austin Wang <38676809+austinywang@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:06:44 -0700 Subject: [PATCH 5/5] Chief Home removal keeps Dock work, hidden conversations and later pins Addresses review on https://github.com/manaflow-ai/cmux/pull/26602: - a Home workspace with Dock panels keeps them (only its Home pane goes) - automatic removal leaves the hidden-conversation list unchanged - the Home group is unpinned only alongside a removal, so a user's later pin survives the next window appear or session restore - controlOwner() rejects an owner whose load finished after Chief was turned off Co-Authored-By: Claude Opus 5.5 --- Sources/Chief/ChiefService+Control.swift | 9 +++- Sources/Chief/ChiefService.swift | 2 +- Sources/ChiefHomeNavigation.swift | 29 ++++++++++--- cmuxTests/ChiefDisabledHomeTests.swift | 52 ++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/Sources/Chief/ChiefService+Control.swift b/Sources/Chief/ChiefService+Control.swift index 7d3d91a94517..f287faca3b49 100644 --- a/Sources/Chief/ChiefService+Control.swift +++ b/Sources/Chief/ChiefService+Control.swift @@ -118,11 +118,18 @@ extension ChiefService { /// - Throws: ``ChiefControlFailure/unavailable(_:)`` when the Chief home could not be opened. func controlOwner() async throws -> ChiefConversationOwner { let load = controlOwnerLoad() + let generation = startGeneration + let owner: ChiefConversationOwner do { - return try await load.task.value + owner = try await load.task.value } catch { throw ChiefControlFailure.unavailable(startFailure ?? load.failure) } + // Chief was turned off while the load ran: that owner is shutting down. + guard startGeneration == generation, ChiefSettings.isEnabled else { + throw ChiefControlFailure.unavailable(ChiefStrings.unavailable) + } + return owner } /// Starts the Chief and returns its owner's load, for a caller off the diff --git a/Sources/Chief/ChiefService.swift b/Sources/Chief/ChiefService.swift index 392a83099645..b8a52af11ad9 100644 --- a/Sources/Chief/ChiefService.swift +++ b/Sources/Chief/ChiefService.swift @@ -51,7 +51,7 @@ final class ChiefService { @ObservationIgnored private var started = false /// Bumped by each ``start()`` and ``stopBecauseDisabled()``; an owner load /// that finishes after a stop sees a newer value and shuts itself down. - @ObservationIgnored private var startGeneration = 0 + @ObservationIgnored private(set) var startGeneration = 0 /// The list's preview lines by conversation (ChiefService+Previews.swift), with the row state /// each was read at. @ObservationIgnored var previewLines: [ConversationID: ChiefPreviewEntry] = [:] diff --git a/Sources/ChiefHomeNavigation.swift b/Sources/ChiefHomeNavigation.swift index fb0ccf3a363d..4d4428ba2763 100644 --- a/Sources/ChiefHomeNavigation.swift +++ b/Sources/ChiefHomeNavigation.swift @@ -98,27 +98,44 @@ struct ChiefHomeNavigation { return home } - /// With `chief.enabled` off, removes Chief Home and unpins the Home group, - /// so a disabled Chief leaves nothing pinned in the sidebar. A Home - /// workspace holding only Chief panes closes (a window whose only + /// With `chief.enabled` off, removes Chief Home and unpins its group, so + /// a disabled Chief leaves nothing pinned in the sidebar. A Home workspace + /// holding only Chief panes and an empty Dock closes (a window whose only /// workspace it is gets a fresh workspace first); one the user added other - /// panes to keeps them and loses only its Home pane. + /// panes or Dock panels to keeps them and loses only its Home pane. The + /// group is unpinned only alongside a removal, so a later pin by the user + /// stays. Automatic removal never hides conversations from the Home list. static func removeHome(in manager: TabManager) { guard !ChiefSettings.isEnabled, !manager.isFinalizedForWindowClose else { return } + guard manager.tabs.contains(where: \.holdsChiefHomePane) else { return } + let hiddenBefore = UserDefaults.standard.object(forKey: hiddenConversationsKey) + defer { UserDefaults.standard.set(hiddenBefore, forKey: hiddenConversationsKey) } + var removedGroupIDs: Set = [] for home in manager.tabs where home.holdsChiefHomePane { - if home.panels.values.allSatisfy({ $0 is ChiefPanel }) { + let groupID = home.groupId + let holdsOnlyChief = home.panels.values.allSatisfy { $0 is ChiefPanel } + && home._dockSplit?.panels.isEmpty != false + if holdsOnlyChief { if manager.tabs.count == 1 { guard manager.addWorkspaceIfActive(select: true) != nil else { continue } } manager.closeWorkspace(home, recordHistory: false) } else { let homePanes = home.panels.values.compactMap { $0 as? ChiefPanel }.filter(\.isHome) + // Only the Dock holds user work: keep the main area non-empty so + // removing Home closes neither the workspace nor its Dock. + if home.panels.values.allSatisfy({ $0 is ChiefPanel }), + let pane = home.bonsplitController.focusedPaneId { + _ = home.newTerminalSurface(inPane: pane, focus: false) + } home.withClosedPanelHistorySuppressed { for pane in homePanes { _ = home.closePanel(pane.id, force: true) } } } + if let groupID { removedGroupIDs.insert(groupID) } } - for group in manager.workspaceGroups where group.externalID == homeGroupExternalID && group.isPinned { + for group in manager.workspaceGroups + where removedGroupIDs.contains(group.id) && group.externalID == homeGroupExternalID && group.isPinned { manager.setWorkspaceGroupPinned(groupId: group.id, isPinned: false) } } diff --git a/cmuxTests/ChiefDisabledHomeTests.swift b/cmuxTests/ChiefDisabledHomeTests.swift index b96eb8678320..7837b71b8368 100644 --- a/cmuxTests/ChiefDisabledHomeTests.swift +++ b/cmuxTests/ChiefDisabledHomeTests.swift @@ -1,4 +1,5 @@ import AppKit +import CmuxHomeCore import CmuxSettings import Foundation import Testing @@ -130,4 +131,55 @@ struct ChiefDisabledHomeTests { #expect(home.panels[terminal.id] != nil) #expect(!home.holdsChiefHomePane) } + + @Test func removeHomeKeepsAHomeWorkspaceWithDockPanels() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let dock = try #require(home.dockSplit) + let dockPane = try #require(dock.bonsplitController.allPaneIds.first) + let dockPanelID = try #require(dock.newSurface(kind: .terminal, inPane: dockPane, focus: false)) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.tabs.contains { $0.id == home.id }) + #expect(!home.holdsChiefHomePane) + #expect(home._dockSplit?.panels[dockPanelID] != nil) + } + + @Test func removeHomeDoesNotHideConversations() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let key = ChiefHomeNavigation.hiddenConversationsKey + let originalHidden = UserDefaults.standard.object(forKey: key) + defer { UserDefaults.standard.set(originalHidden, forKey: key) } + UserDefaults.standard.removeObject(forKey: key) + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let paneId = try #require(home.bonsplitController.focusedPaneId) + _ = try #require(home.newChiefSurface(inPane: paneId, conversationID: ConversationID("conv_keep"), focus: false)) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(!manager.tabs.contains { $0.id == home.id }) + #expect(UserDefaults.standard.stringArray(forKey: key) == nil) + } + + @Test func removeHomeLeavesALaterUserPinAlone() throws { + let setting = ChiefEnabledOverride(true) + defer { setting.restore() } + let manager = TabManager() + let home = try #require(ChiefHomeNavigation.ensureHome(in: manager)) + let groupId = try #require(home.groupId) + let member = try #require(manager.tabs.first { !$0.holdsChiefHomePane }) + manager.addWorkspaceToGroup(workspaceId: member.id, groupId: groupId) + + ChiefSettings.setEnabled(false) + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.workspaceGroups.first { $0.id == groupId }?.isPinned == false) + #expect(manager.setWorkspaceGroupPinned(groupId: groupId, isPinned: true)) + ChiefHomeNavigation.removeHome(in: manager) + #expect(manager.workspaceGroups.first { $0.id == groupId }?.isPinned == true) + } }