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/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..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 @@ -131,7 +138,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..b8a52af11ad9 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(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] = [:] @@ -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,50 @@ 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 = [:] + previewLines = [:] + previewReads = [] + 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 +304,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..4d4428ba2763 100644 --- a/Sources/ChiefHomeNavigation.swift +++ b/Sources/ChiefHomeNavigation.swift @@ -98,6 +98,48 @@ struct ChiefHomeNavigation { return home } + /// 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 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 { + 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 removedGroupIDs.contains(group.id) && 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..4fba52a232e8 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 } } @@ -2462,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 bcde1a4bc712..bbd5d8266d23 100644 --- a/Sources/TerminalController+WorkspaceGroupAction.swift +++ b/Sources/TerminalController+WorkspaceGroupAction.swift @@ -68,7 +68,15 @@ extension TerminalController { case .pin: tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: true) case .unpin: - tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: false) + // Chief Home stays pinned while `chief.enabled` is on; say so instead of reporting OK. + guard tabManager.setWorkspaceGroupPinned(groupId: groupID, isPinned: false) else { + mutationError = .err( + code: "protected", + message: TabManager.chiefHomeCannotUnpinMessage, + data: ["group_id": groupID.uuidString] + ) + return + } case .rename: guard let title else { mutationError = .err(code: "invalid_params", message: "Missing or invalid title", data: nil) 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/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..7837b71b8368 --- /dev/null +++ b/cmuxTests/ChiefDisabledHomeTests.swift @@ -0,0 +1,185 @@ +import AppKit +import CmuxHomeCore +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)) + } + + @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 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)) + + 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) + } + + @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) + } +}