From 7994274d2eb49ba4c22ebe5b19591c06649698e6 Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 11:17:56 -0400 Subject: [PATCH 1/8] Confirm before closing busy terminal tabs --- .../Reducer/SettingsFeature.swift | 4 + .../Views/AppearanceSettingsView.swift | 4 + .../Models/GlobalSettings.swift | 9 ++ .../Models/WorktreeTerminalState.swift | 91 ++++++++++++++++++- .../Views/WorktreeTerminalTabsView.swift | 30 +++++- .../Ghostty/GhosttySurfaceView.swift | 5 + supacodeTests/SettingsFeatureTests.swift | 19 ++++ .../SettingsFilePersistenceTests.swift | 42 +++++++++ .../WorktreeTerminalManagerTests.swift | 74 +++++++++++++++ 9 files changed, 269 insertions(+), 9 deletions(-) diff --git a/SupacodeSettingsFeature/Reducer/SettingsFeature.swift b/SupacodeSettingsFeature/Reducer/SettingsFeature.swift index 6322516bc..0c8ba0105 100644 --- a/SupacodeSettingsFeature/Reducer/SettingsFeature.swift +++ b/SupacodeSettingsFeature/Reducer/SettingsFeature.swift @@ -72,6 +72,7 @@ public struct SettingsFeature { public var agentPresenceBadgesEnabled: Bool public var autoUpdateAgentIntegrationsEnabled: Bool public var confirmQuitMode: ConfirmQuitMode + public var confirmCloseTabsWithRunningProcesses: Bool public var terminateSessionsOnQuit: Bool public var remoteSessionPersistenceEnabled: Bool public var cliInstallState = CLIInstallState.checking @@ -121,6 +122,7 @@ public struct SettingsFeature { agentPresenceBadgesEnabled = settings.agentPresenceBadgesEnabled autoUpdateAgentIntegrationsEnabled = settings.autoUpdateAgentIntegrationsEnabled confirmQuitMode = settings.confirmQuitMode + confirmCloseTabsWithRunningProcesses = settings.confirmCloseTabsWithRunningProcesses terminateSessionsOnQuit = settings.terminateSessionsOnQuit remoteSessionPersistenceEnabled = settings.remoteSessionPersistenceEnabled defaultWorktreeBaseDirectoryPath = @@ -162,6 +164,7 @@ public struct SettingsFeature { agentPresenceBadgesEnabled: agentPresenceBadgesEnabled, autoUpdateAgentIntegrationsEnabled: autoUpdateAgentIntegrationsEnabled, confirmQuitMode: confirmQuitMode, + confirmCloseTabsWithRunningProcesses: confirmCloseTabsWithRunningProcesses, terminateSessionsOnQuit: terminateSessionsOnQuit, remoteSessionPersistenceEnabled: remoteSessionPersistenceEnabled ) @@ -300,6 +303,7 @@ public struct SettingsFeature { state.agentPresenceBadgesEnabled = normalizedSettings.agentPresenceBadgesEnabled state.autoUpdateAgentIntegrationsEnabled = normalizedSettings.autoUpdateAgentIntegrationsEnabled state.confirmQuitMode = normalizedSettings.confirmQuitMode + state.confirmCloseTabsWithRunningProcesses = normalizedSettings.confirmCloseTabsWithRunningProcesses state.terminateSessionsOnQuit = normalizedSettings.terminateSessionsOnQuit state.remoteSessionPersistenceEnabled = normalizedSettings.remoteSessionPersistenceEnabled state.defaultWorktreeBaseDirectoryPath = normalizedSettings.defaultWorktreeBaseDirectoryPath ?? "" diff --git a/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift b/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift index 22fb1d1a5..8f35625c5 100644 --- a/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift +++ b/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift @@ -93,6 +93,10 @@ public struct AppearanceSettingsView: View { Text("Hide Tab Bar for Single Tab") Text("Automatically hides the tab bar when only one tab is open.") } + Toggle(isOn: $store.confirmCloseTabsWithRunningProcesses) { + Text("Confirm before Closing Tabs") + Text("Ask before closing a tab that has running terminal processes.") + } Picker(selection: $store.automatedActionPolicy.sending(\.setAutomatedActionPolicy)) { ForEach(AutomatedActionPolicy.allCases, id: \.self) { policy in Text(policy.displayName).tag(policy) diff --git a/SupacodeSettingsShared/Models/GlobalSettings.swift b/SupacodeSettingsShared/Models/GlobalSettings.swift index 43271b538..a3b49bd4a 100644 --- a/SupacodeSettingsShared/Models/GlobalSettings.swift +++ b/SupacodeSettingsShared/Models/GlobalSettings.swift @@ -63,6 +63,9 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { /// entries from earlier wire-protocol revisions). public var autoUpdateAgentIntegrationsEnabled: Bool public var confirmQuitMode: ConfirmQuitMode + /// When true, closing a tab asks for confirmation if any of its terminal + /// surfaces has foreground work that Ghostty considers unsafe to interrupt. + public var confirmCloseTabsWithRunningProcesses: Bool /// When true, quitting Supacode also closes every terminal tab and tears /// down zmx sessions, local and host-side, so nothing keeps running in the /// background. Default off because persistence is the headline feature. @@ -103,6 +106,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { agentPresenceBadgesEnabled: true, autoUpdateAgentIntegrationsEnabled: true, confirmQuitMode: .auto, + confirmCloseTabsWithRunningProcesses: true, terminateSessionsOnQuit: false, remoteSessionPersistenceEnabled: true ) @@ -139,6 +143,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { agentPresenceBadgesEnabled: Bool = true, autoUpdateAgentIntegrationsEnabled: Bool = true, confirmQuitMode: ConfirmQuitMode = .auto, + confirmCloseTabsWithRunningProcesses: Bool = true, terminateSessionsOnQuit: Bool = false, remoteSessionPersistenceEnabled: Bool = true ) { @@ -173,6 +178,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { self.agentPresenceBadgesEnabled = agentPresenceBadgesEnabled self.autoUpdateAgentIntegrationsEnabled = autoUpdateAgentIntegrationsEnabled self.confirmQuitMode = confirmQuitMode + self.confirmCloseTabsWithRunningProcesses = confirmCloseTabsWithRunningProcesses self.terminateSessionsOnQuit = terminateSessionsOnQuit self.remoteSessionPersistenceEnabled = remoteSessionPersistenceEnabled } @@ -331,6 +337,9 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { } else { confirmQuitMode = Self.default.confirmQuitMode } + confirmCloseTabsWithRunningProcesses = + try container.decodeIfPresent(Bool.self, forKey: .confirmCloseTabsWithRunningProcesses) + ?? Self.default.confirmCloseTabsWithRunningProcesses terminateSessionsOnQuit = try container.decodeIfPresent(Bool.self, forKey: .terminateSessionsOnQuit) ?? Self.default.terminateSessionsOnQuit diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index b12a83335..76b7445f8 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -49,6 +49,17 @@ final class WorktreeTerminalState { let isVisible: Bool let isFocused: Bool } + struct PendingTabCloseConfirmation: Equatable { + let tabIDs: [TerminalTabID] + + var title: String { + tabIDs.count == 1 ? "Close Tab?" : "Close \(tabIDs.count) Tabs?" + } + + var actionTitle: String { + tabIDs.count == 1 ? "Close Tab" : "Close Tabs" + } + } private struct SurfaceLaunchMetadata { let usesZmx: Bool @@ -58,6 +69,7 @@ final class WorktreeTerminalState { let tabManager: TerminalTabManager private let runtime: GhosttyRuntime @ObservationIgnored private let splitPreserveZoomOnNavigation: () -> Bool + @ObservationIgnored private let surfaceNeedsCloseConfirmation: (GhosttySurfaceView) -> Bool private let worktree: Worktree @ObservationIgnored @SharedReader private var repositorySettings: RepositorySettings @@ -82,6 +94,7 @@ final class WorktreeTerminalState { @ObservationIgnored private var lastTabProgressDisplays: [TerminalTabID: TerminalTabProgressDisplay?] = [:] var socketPath: String? private(set) var shouldHideTabBar = false + private(set) var pendingTabCloseConfirmation: PendingTabCloseConfirmation? // Every mutation schedules a coalesced row-projection emit so the TCA // mirror of running scripts reconciles from this single source of truth (#573). private var blockingScripts: [TerminalTabID: BlockingScriptKind] = [:] { @@ -213,10 +226,12 @@ final class WorktreeTerminalState { runtime: GhosttyRuntime, worktree: Worktree, runSetupScript: Bool = false, - splitPreserveZoomOnNavigation: (() -> Bool)? = nil + splitPreserveZoomOnNavigation: (() -> Bool)? = nil, + surfaceNeedsCloseConfirmation: ((GhosttySurfaceView) -> Bool)? = nil ) { self.runtime = runtime self.splitPreserveZoomOnNavigation = splitPreserveZoomOnNavigation ?? { runtime.splitPreserveZoomOnNavigation() } + self.surfaceNeedsCloseConfirmation = surfaceNeedsCloseConfirmation ?? { $0.needsCloseConfirmation } self.worktree = worktree self.pendingSetupScript = runSetupScript self.tabManager = TerminalTabManager() @@ -723,8 +738,7 @@ final class WorktreeTerminalState { @discardableResult func closeFocusedTab() -> Bool { guard let tabId = tabManager.selectedTabId else { return false } - closeTab(tabId) - return true + return requestCloseTab(tabId) } @discardableResult @@ -799,7 +813,73 @@ final class WorktreeTerminalState { return true } + @discardableResult + func requestCloseTab(_ tabId: TerminalTabID) -> Bool { + requestCloseTabs([tabId]) + } + + @discardableResult + func requestCloseOtherTabs(keeping tabId: TerminalTabID) -> Bool { + requestCloseTabs(tabManager.tabs.map(\.id).filter { $0 != tabId }) + } + + @discardableResult + func requestCloseTabsToRight(of tabId: TerminalTabID) -> Bool { + guard let index = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return false } + return requestCloseTabs(Array(tabManager.tabs.dropFirst(index + 1).map(\.id))) + } + + @discardableResult + func requestCloseAllTabs() -> Bool { + requestCloseTabs(tabManager.tabs.map(\.id)) + } + + func confirmPendingTabClose() { + guard let pending = pendingTabCloseConfirmation else { return } + pendingTabCloseConfirmation = nil + for tabId in pending.tabIDs { + closeTab(tabId) + } + } + + func cancelPendingTabClose() { + pendingTabCloseConfirmation = nil + } + + private func requestCloseTabs(_ requestedTabIDs: [TerminalTabID]) -> Bool { + let existingTabIDs = requestedTabIDs.filter { requested in + tabManager.tabs.contains(where: { $0.id == requested }) + } + guard !existingTabIDs.isEmpty else { return false } + + @Shared(.settingsFile) var settingsFile + let needsConfirmation = + settingsFile.global.confirmCloseTabsWithRunningProcesses + && existingTabIDs.contains(where: tabNeedsCloseConfirmation) + if needsConfirmation { + pendingTabCloseConfirmation = PendingTabCloseConfirmation(tabIDs: existingTabIDs) + } else { + for tabId in existingTabIDs { + closeTab(tabId) + } + } + return true + } + + private func tabNeedsCloseConfirmation(_ tabId: TerminalTabID) -> Bool { + guard let tree = trees[tabId] else { return false } + return tree.leaves().contains(where: surfaceNeedsCloseConfirmation) + } + + private func removeFromPendingTabClose(_ tabId: TerminalTabID) { + guard let pending = pendingTabCloseConfirmation else { return } + let remaining = pending.tabIDs.filter { $0 != tabId } + pendingTabCloseConfirmation = + remaining.isEmpty ? nil : PendingTabCloseConfirmation(tabIDs: remaining) + } + func closeTab(_ tabId: TerminalTabID) { + removeFromPendingTabClose(tabId) let closedBlockingKind = blockingScripts.removeValue(forKey: tabId) cleanupBlockingScriptLaunchDirectory(for: tabId) // Clear lingering tab tracking for completed or non-blocking tabs. @@ -1029,6 +1109,7 @@ final class WorktreeTerminalState { } func closeAllSurfaces() { + pendingTabCloseConfirmation = nil let closingSurfaces = Array(surfaces.values) let closingSurfaceIDs = closingSurfaces.map(\.id) for surface in closingSurfaces { @@ -1655,8 +1736,7 @@ final class WorktreeTerminalState { } view.bridge.onCloseTab = { [weak self, weak view] _ in guard let self, let view, self.isLiveSurface(view) else { return false } - self.closeTab(tabId) - return true + return self.requestCloseTab(tabId) } view.bridge.onGotoTab = { [weak self, weak view] target in guard let self, let view, self.isLiveSurface(view) else { return false } @@ -2608,6 +2688,7 @@ final class WorktreeTerminalState { killZmxSessions(forSurfaceIDs: [view.id], includeRemote: includeRemoteSession) } if newTree.isEmpty { + removeFromPendingTabClose(tabId) trees.removeValue(forKey: tabId) focusedSurfaceIdByTab.removeValue(forKey: tabId) cleanupBlockingScriptLaunchDirectory(for: tabId) diff --git a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift index 19b36edb3..5bc96223a 100644 --- a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift +++ b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift @@ -23,6 +23,7 @@ struct WorktreeTerminalTabsView: View { // would reintroduce the closed-all flash on first render. let _: Void = state.ensureInitialTab(focusing: false) let unfocusedSplitOverlay = manager.unfocusedSplitOverlay() + let pendingTabClose = state.pendingTabCloseConfirmation let _ = colorScheme VStack(spacing: 0) { if !state.shouldHideTabBar { @@ -36,16 +37,16 @@ struct WorktreeTerminalTabsView: View { }, canSplit: state.tabManager.selectedTabId.flatMap { state.activeSurfaceID(for: $0) } != nil, closeTab: { tabId in - state.closeTab(tabId) + _ = state.requestCloseTab(tabId) }, closeOthers: { tabId in - state.closeOtherTabs(keeping: tabId) + _ = state.requestCloseOtherTabs(keeping: tabId) }, closeToRight: { tabId in - state.closeTabsToRight(of: tabId) + _ = state.requestCloseTabsToRight(of: tabId) }, closeAll: { - state.closeAllTabs() + _ = state.requestCloseAllTabs() }, dismissSplitZoom: { tabId in state.dismissSplitZoom(for: tabId) @@ -70,6 +71,27 @@ struct WorktreeTerminalTabsView: View { } } .animation(.easeInOut(duration: 0.2), value: state.shouldHideTabBar) + .alert( + pendingTabClose?.title ?? "Close Tab?", + isPresented: Binding( + get: { state.pendingTabCloseConfirmation != nil }, + set: { isPresented in + if !isPresented { + state.cancelPendingTabClose() + } + } + ), + presenting: pendingTabClose + ) { pending in + Button("Cancel", role: .cancel) { + state.cancelPendingTabClose() + } + Button(pending.actionTitle, role: .destructive) { + state.confirmPendingTabClose() + } + } message: { _ in + Text("One or more processes are still running. Closing will terminate them.") + } .background( WindowFocusObserverView { activity in windowActivity = activity diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift index 1dc22cd29..adc78cc29 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift @@ -279,6 +279,11 @@ final class GhosttySurfaceView: NSView, Identifiable { } } + var needsCloseConfirmation: Bool { + guard let surface else { return false } + return ghostty_surface_needs_confirm_quit(surface) + } + func closeSurface() { clearNotificationObservers() if let surface { diff --git a/supacodeTests/SettingsFeatureTests.swift b/supacodeTests/SettingsFeatureTests.swift index 57cd4a204..1fba2f4b1 100644 --- a/supacodeTests/SettingsFeatureTests.swift +++ b/supacodeTests/SettingsFeatureTests.swift @@ -30,6 +30,7 @@ struct SettingsFeatureTests { promptForWorktreeCreation: true, terminalThemeSyncEnabled: false, automatedActionPolicy: .always, + confirmCloseTabsWithRunningProcesses: false, ) @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global = loaded } @@ -60,6 +61,7 @@ struct SettingsFeatureTests { $0.fetchOriginBeforeWorktreeCreation = true $0.terminalThemeSyncEnabled = false $0.automatedActionPolicy = .always + $0.confirmCloseTabsWithRunningProcesses = false } await store.skipReceivedActions() receiveStartupHookChecks(from: store) @@ -113,6 +115,23 @@ struct SettingsFeatureTests { expectNoDifference(settingsFile.global, expectedSettings) } + @Test(.dependencies) func confirmCloseTabsWithRunningProcessesPersistsChanges() async { + var initialSettings = GlobalSettings.default + initialSettings.confirmCloseTabsWithRunningProcesses = true + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global = initialSettings } + + let store = TestStore(initialState: SettingsFeature.State(settings: initialSettings)) { + SettingsFeature() + } + + await store.send(.binding(.set(\.confirmCloseTabsWithRunningProcesses, false))) { + $0.confirmCloseTabsWithRunningProcesses = false + } + await store.receive(\.delegate.settingsChanged) + #expect(!settingsFile.global.confirmCloseTabsWithRunningProcesses) + } + @Test(.dependencies) func setSystemNotificationsEnabledPersistsChanges() async { var initialSettings = GlobalSettings.default initialSettings.systemNotificationsEnabled = false diff --git a/supacodeTests/SettingsFilePersistenceTests.swift b/supacodeTests/SettingsFilePersistenceTests.swift index ef4cf4d7e..0154e0f6b 100644 --- a/supacodeTests/SettingsFilePersistenceTests.swift +++ b/supacodeTests/SettingsFilePersistenceTests.swift @@ -386,6 +386,48 @@ struct SettingsFilePersistenceTests { #expect(reloaded.global.terminalThemeSyncEnabled == true) } + @Test(.dependencies) func decodesMissingConfirmCloseTabsWithRunningProcessesAsTrue() throws { + let legacy = LegacySettingsFile( + global: LegacyGlobalSettings( + appearanceMode: .dark, + updatesAutomaticallyCheckForUpdates: false, + updatesAutomaticallyDownloadUpdates: true + ), + repositories: [:] + ) + let data = try JSONEncoder().encode(legacy) + let storage = MutableTestStorage(initialData: data) + + let settings: SettingsFile = withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.settingsFile) var settings: SettingsFile + return settings + } + + #expect(settings.global.confirmCloseTabsWithRunningProcesses) + } + + @Test(.dependencies) func roundTripsExplicitConfirmCloseTabsWithRunningProcessesDisabled() throws { + let storage = SettingsTestStorage() + + withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.settingsFile) var settings: SettingsFile + $settings.withLock { $0.global.confirmCloseTabsWithRunningProcesses = false } + } + + let reloaded: SettingsFile = withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.settingsFile) var reloaded: SettingsFile + return reloaded + } + + #expect(!reloaded.global.confirmCloseTabsWithRunningProcesses) + } + @Test(.dependencies) func decodesMissingRemoteSessionPersistenceEnabledAsTrue() throws { let legacy = LegacySettingsFile( global: LegacyGlobalSettings( diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 210bfa04f..5c15f8ccf 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -1,9 +1,11 @@ import AppKit import Clocks import Dependencies +import DependenciesTestSupport import Foundation import GhosttyKit import IdentifiedCollections +import Sharing import SupacodeSettingsShared import Testing @@ -1054,6 +1056,78 @@ struct WorktreeTerminalManagerTests { #expect(state.surfaceStates[surfaceID] == nil) } + @Test(.dependencies) func requestCloseTabConfirmsWhenAnySplitSurfaceHasRunningProcess() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let tabId = state.createTab(focusing: true), + let initialSurface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + #expect(state.performSplitAction(.newSplit(direction: .right), for: initialSurface.id)) + let leaves = state.splitTree(for: tabId).leaves() + guard leaves.count == 2 else { + Issue.record("Expected a split tab") + return + } + runningSurfaceIDs.insert(leaves[1].id) + + #expect(state.requestCloseTab(tabId)) + #expect(state.pendingTabCloseConfirmation?.tabIDs == [tabId]) + #expect(state.tabManager.tabs.contains(where: { $0.id == tabId })) + + state.cancelPendingTabClose() + #expect(state.pendingTabCloseConfirmation == nil) + #expect(state.tabManager.tabs.contains(where: { $0.id == tabId })) + + #expect(state.requestCloseTab(tabId)) + state.confirmPendingTabClose() + #expect(state.pendingTabCloseConfirmation == nil) + #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) + } + + @Test(.dependencies) func requestCloseTabClosesIdleTabWithoutConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = true } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { _ in false } + ) + guard let tabId = state.createTab(focusing: true) else { + Issue.record("Expected a tab") + return + } + + #expect(state.requestCloseTab(tabId)) + #expect(state.pendingTabCloseConfirmation == nil) + #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) + } + + @Test(.dependencies) func disabledSettingClosesRunningTabWithoutConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = false } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { _ in true } + ) + guard let tabId = state.createTab(focusing: true) else { + Issue.record("Expected a tab") + return + } + + #expect(state.requestCloseTab(tabId)) + #expect(state.pendingTabCloseConfirmation == nil) + #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) + } @Test func closeAllSurfacesClearsPerSurfaceBookkeeping() { withDependencies { From 1a1d5a586a2b1642ecfd07df5700bd6a7c778270 Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 13:30:05 -0400 Subject: [PATCH 2/8] Stabilize merge queue duration test --- supacodeTests/PullRequestMergeQueueStatusTests.swift | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/supacodeTests/PullRequestMergeQueueStatusTests.swift b/supacodeTests/PullRequestMergeQueueStatusTests.swift index 276aecae5..b0520ca12 100644 --- a/supacodeTests/PullRequestMergeQueueStatusTests.swift +++ b/supacodeTests/PullRequestMergeQueueStatusTests.swift @@ -23,13 +23,10 @@ struct PullRequestMergeQueueStatusTests { #expect(status?.position == 3) #expect(status?.positionLabel == "Position 3") - #expect(status?.estimatedTimeLabel == "~10 \(Self.abbreviatedMinutes) left") - #expect(status?.detail == "Position 3 · ~10 \(Self.abbreviatedMinutes) left") - } - - // macOS 26.5 changed Duration.formatted's abbreviated plural minutes from "min" to "mins". - private static var abbreviatedMinutes: String { - if #available(macOS 26.5, *) { return "mins" } else { return "min" } + let expectedTimeLabels: Set = ["~10 min left", "~10 mins left"] + let expectedDetails = Set(expectedTimeLabels.map { "Position 3 · \($0)" }) + #expect(expectedTimeLabels.contains(status?.estimatedTimeLabel ?? "")) + #expect(expectedDetails.contains(status?.detail ?? "")) } @Test func dropsEstimatedTimeWhenZeroOrMissing() { From 14fcd44e4746a4b0cc29003a50882e71bf757e63 Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 16:10:44 -0400 Subject: [PATCH 3/8] Use Ghostty surface close confirmation --- .../Reducer/SettingsFeature.swift | 8 +- .../Views/AppearanceSettingsView.swift | 6 +- .../Models/GlobalSettings.swift | 18 +-- .../Models/WorktreeTerminalState.swift | 97 +++++++++++---- .../Views/WorktreeTerminalTabsView.swift | 18 +-- .../Ghostty/GhosttyRuntime.swift | 10 +- .../GhosttyRuntimeBundledOverridesTests.swift | 4 + supacodeTests/SettingsFeatureTests.swift | 14 +-- .../SettingsFilePersistenceTests.swift | 10 +- .../WorktreeTerminalManagerTests.swift | 114 ++++++++++++++++-- 10 files changed, 224 insertions(+), 75 deletions(-) diff --git a/SupacodeSettingsFeature/Reducer/SettingsFeature.swift b/SupacodeSettingsFeature/Reducer/SettingsFeature.swift index 12a44756f..5f12a5cea 100644 --- a/SupacodeSettingsFeature/Reducer/SettingsFeature.swift +++ b/SupacodeSettingsFeature/Reducer/SettingsFeature.swift @@ -76,7 +76,7 @@ public struct SettingsFeature { public var agentPresenceBadgesEnabled: Bool public var autoUpdateAgentIntegrationsEnabled: Bool public var confirmQuitMode: ConfirmQuitMode - public var confirmCloseTabsWithRunningProcesses: Bool + public var confirmCloseSurface: Bool public var terminateSessionsOnQuit: Bool public var remoteSessionPersistenceEnabled: Bool public var appVisibility: AppVisibility @@ -130,7 +130,7 @@ public struct SettingsFeature { agentPresenceBadgesEnabled = settings.agentPresenceBadgesEnabled autoUpdateAgentIntegrationsEnabled = settings.autoUpdateAgentIntegrationsEnabled confirmQuitMode = settings.confirmQuitMode - confirmCloseTabsWithRunningProcesses = settings.confirmCloseTabsWithRunningProcesses + confirmCloseSurface = settings.confirmCloseSurface terminateSessionsOnQuit = settings.terminateSessionsOnQuit remoteSessionPersistenceEnabled = settings.remoteSessionPersistenceEnabled appVisibility = settings.appVisibility @@ -173,7 +173,7 @@ public struct SettingsFeature { agentPresenceBadgesEnabled: agentPresenceBadgesEnabled, autoUpdateAgentIntegrationsEnabled: autoUpdateAgentIntegrationsEnabled, confirmQuitMode: confirmQuitMode, - confirmCloseTabsWithRunningProcesses: confirmCloseTabsWithRunningProcesses, + confirmCloseSurface: confirmCloseSurface, terminateSessionsOnQuit: terminateSessionsOnQuit, remoteSessionPersistenceEnabled: remoteSessionPersistenceEnabled, appVisibility: appVisibility @@ -310,7 +310,7 @@ public struct SettingsFeature { state.agentPresenceBadgesEnabled = normalizedSettings.agentPresenceBadgesEnabled state.autoUpdateAgentIntegrationsEnabled = normalizedSettings.autoUpdateAgentIntegrationsEnabled state.confirmQuitMode = normalizedSettings.confirmQuitMode - state.confirmCloseTabsWithRunningProcesses = normalizedSettings.confirmCloseTabsWithRunningProcesses + state.confirmCloseSurface = normalizedSettings.confirmCloseSurface state.terminateSessionsOnQuit = normalizedSettings.terminateSessionsOnQuit state.remoteSessionPersistenceEnabled = normalizedSettings.remoteSessionPersistenceEnabled state.appVisibility = normalizedSettings.appVisibility diff --git a/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift b/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift index e12d3915a..19d844007 100644 --- a/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift +++ b/SupacodeSettingsFeature/Views/AppearanceSettingsView.swift @@ -120,9 +120,9 @@ public struct AppearanceSettingsView: View { Text("Hide Tab Bar for Single Tab") Text("Automatically hides the tab bar when only one tab is open.") } - Toggle(isOn: $store.confirmCloseTabsWithRunningProcesses) { - Text("Confirm before Closing Tabs") - Text("Ask before closing a tab that has running terminal processes.") + Toggle(isOn: $store.confirmCloseSurface) { + Text("Confirm before Closing Terminals") + Text("Ask before closing a terminal that has a running process.") } Picker(selection: $store.automatedActionPolicy.sending(\.setAutomatedActionPolicy)) { ForEach(AutomatedActionPolicy.allCases, id: \.self) { policy in diff --git a/SupacodeSettingsShared/Models/GlobalSettings.swift b/SupacodeSettingsShared/Models/GlobalSettings.swift index 9966f70b4..52a18af1f 100644 --- a/SupacodeSettingsShared/Models/GlobalSettings.swift +++ b/SupacodeSettingsShared/Models/GlobalSettings.swift @@ -63,9 +63,9 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { /// entries from earlier wire-protocol revisions). public var autoUpdateAgentIntegrationsEnabled: Bool public var confirmQuitMode: ConfirmQuitMode - /// When true, closing a tab asks for confirmation if any of its terminal - /// surfaces has foreground work that Ghostty considers unsafe to interrupt. - public var confirmCloseTabsWithRunningProcesses: Bool + /// When true, user-initiated closes ask for confirmation when a terminal + /// surface has foreground work that Ghostty considers unsafe to interrupt. + public var confirmCloseSurface: Bool /// When true, quitting Supacode also closes every terminal tab and tears /// down zmx sessions, local and host-side, so nothing keeps running in the /// background. Default off because persistence is the headline feature. @@ -108,7 +108,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { agentPresenceBadgesEnabled: true, autoUpdateAgentIntegrationsEnabled: true, confirmQuitMode: .auto, - confirmCloseTabsWithRunningProcesses: true, + confirmCloseSurface: true, terminateSessionsOnQuit: false, remoteSessionPersistenceEnabled: true, appVisibility: .dock @@ -146,7 +146,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { agentPresenceBadgesEnabled: Bool = true, autoUpdateAgentIntegrationsEnabled: Bool = true, confirmQuitMode: ConfirmQuitMode = .auto, - confirmCloseTabsWithRunningProcesses: Bool = true, + confirmCloseSurface: Bool = true, terminateSessionsOnQuit: Bool = false, remoteSessionPersistenceEnabled: Bool = true, appVisibility: AppVisibility = .dock @@ -182,7 +182,7 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { self.agentPresenceBadgesEnabled = agentPresenceBadgesEnabled self.autoUpdateAgentIntegrationsEnabled = autoUpdateAgentIntegrationsEnabled self.confirmQuitMode = confirmQuitMode - self.confirmCloseTabsWithRunningProcesses = confirmCloseTabsWithRunningProcesses + self.confirmCloseSurface = confirmCloseSurface self.terminateSessionsOnQuit = terminateSessionsOnQuit self.remoteSessionPersistenceEnabled = remoteSessionPersistenceEnabled self.appVisibility = appVisibility @@ -342,9 +342,9 @@ public nonisolated struct GlobalSettings: Codable, Equatable, Sendable { } else { confirmQuitMode = Self.default.confirmQuitMode } - confirmCloseTabsWithRunningProcesses = - try container.decodeIfPresent(Bool.self, forKey: .confirmCloseTabsWithRunningProcesses) - ?? Self.default.confirmCloseTabsWithRunningProcesses + confirmCloseSurface = + try container.decodeIfPresent(Bool.self, forKey: .confirmCloseSurface) + ?? Self.default.confirmCloseSurface terminateSessionsOnQuit = try container.decodeIfPresent(Bool.self, forKey: .terminateSessionsOnQuit) ?? Self.default.terminateSessionsOnQuit diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 2aa55c5ca..abdfdabc1 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -49,15 +49,35 @@ final class WorktreeTerminalState { let isVisible: Bool let isFocused: Bool } - struct PendingTabCloseConfirmation: Equatable { - let tabIDs: [TerminalTabID] + enum PendingCloseConfirmation: Equatable { + case surface(UUID) + case tabs([TerminalTabID]) var title: String { - tabIDs.count == 1 ? "Close Tab?" : "Close \(tabIDs.count) Tabs?" + switch self { + case .surface: + return "Close Terminal?" + case .tabs(let tabIDs): + return tabIDs.count == 1 ? "Close Tab?" : "Close \(tabIDs.count) Tabs?" + } } var actionTitle: String { - tabIDs.count == 1 ? "Close Tab" : "Close Tabs" + switch self { + case .surface: + return "Close Terminal" + case .tabs(let tabIDs): + return tabIDs.count == 1 ? "Close Tab" : "Close Tabs" + } + } + + var message: String { + switch self { + case .surface: + return "The terminal still has a running process. Closing it will terminate the process." + case .tabs: + return "One or more processes are still running. Closing will terminate them." + } } } @@ -94,7 +114,7 @@ final class WorktreeTerminalState { @ObservationIgnored private var lastTabProgressDisplays: [TerminalTabID: TerminalTabProgressDisplay?] = [:] var socketPath: String? private(set) var shouldHideTabBar = false - private(set) var pendingTabCloseConfirmation: PendingTabCloseConfirmation? + private(set) var pendingCloseConfirmation: PendingCloseConfirmation? // Every mutation schedules a coalesced row-projection emit so the TCA // mirror of running scripts reconciles from this single source of truth (#573). private var blockingScripts: [TerminalTabID: BlockingScriptKind] = [:] { @@ -838,16 +858,25 @@ final class WorktreeTerminalState { requestCloseTabs(tabManager.tabs.map(\.id)) } - func confirmPendingTabClose() { - guard let pending = pendingTabCloseConfirmation else { return } - pendingTabCloseConfirmation = nil - for tabId in pending.tabIDs { - closeTab(tabId) + func confirmPendingClose() { + guard let pending = pendingCloseConfirmation else { return } + pendingCloseConfirmation = nil + switch pending { + case .surface(let surfaceID): + guard let surface = surfaces[surfaceID] else { return } + completeCloseRequest(for: surface) + case .tabs(let tabIDs): + for tabId in tabIDs { + closeTab(tabId) + } } } - func cancelPendingTabClose() { - pendingTabCloseConfirmation = nil + func cancelPendingClose() { + if case .surface(let surfaceID)? = pendingCloseConfirmation { + pendingExplicitSurfaceCloseIDs.remove(surfaceID) + } + pendingCloseConfirmation = nil } private func requestCloseTabs(_ requestedTabIDs: [TerminalTabID]) -> Bool { @@ -858,10 +887,11 @@ final class WorktreeTerminalState { @Shared(.settingsFile) var settingsFile let needsConfirmation = - settingsFile.global.confirmCloseTabsWithRunningProcesses + settingsFile.global.confirmCloseSurface && existingTabIDs.contains(where: tabNeedsCloseConfirmation) if needsConfirmation { - pendingTabCloseConfirmation = PendingTabCloseConfirmation(tabIDs: existingTabIDs) + cancelPendingClose() + pendingCloseConfirmation = .tabs(existingTabIDs) } else { for tabId in existingTabIDs { closeTab(tabId) @@ -875,15 +905,14 @@ final class WorktreeTerminalState { return tree.leaves().contains(where: surfaceNeedsCloseConfirmation) } - private func removeFromPendingTabClose(_ tabId: TerminalTabID) { - guard let pending = pendingTabCloseConfirmation else { return } - let remaining = pending.tabIDs.filter { $0 != tabId } - pendingTabCloseConfirmation = - remaining.isEmpty ? nil : PendingTabCloseConfirmation(tabIDs: remaining) + private func removeFromPendingClose(tabId: TerminalTabID) { + guard case .tabs(let tabIDs)? = pendingCloseConfirmation else { return } + let remaining = tabIDs.filter { $0 != tabId } + pendingCloseConfirmation = remaining.isEmpty ? nil : .tabs(remaining) } func closeTab(_ tabId: TerminalTabID) { - removeFromPendingTabClose(tabId) + removeFromPendingClose(tabId: tabId) let closedBlockingKind = blockingScripts.removeValue(forKey: tabId) cleanupBlockingScriptLaunchDirectory(for: tabId) // Clear lingering tab tracking for completed or non-blocking tabs. @@ -1114,7 +1143,7 @@ final class WorktreeTerminalState { } func closeAllSurfaces() { - pendingTabCloseConfirmation = nil + cancelPendingClose() let closingSurfaces = Array(surfaces.values) let closingSurfaceIDs = closingSurfaces.map(\.id) for surface in closingSurfaces { @@ -1781,9 +1810,9 @@ final class WorktreeTerminalState { guard self.isLiveSurface(view) else { return } self.handleContextSignal(surfaceID: view.id, id: id, metadata: metadata) } - view.bridge.onCloseRequest = { [weak self, weak view] _ in + view.bridge.onCloseRequest = { [weak self, weak view] needsConfirmation in guard let self, let view else { return } - self.handleCloseRequest(for: view) + self.handleCloseRequest(for: view, needsConfirmation: needsConfirmation) } view.onFocusChange = { [weak self, weak view] focused in guard let self, let view, focused else { return } @@ -2235,6 +2264,9 @@ final class WorktreeTerminalState { /// Also cancels any held agent OSC 9 and forgets the last-custom-notification /// instant so a future surface ID can't reuse stale dedupe state. private func discardSurfaceBookkeeping(for surfaceID: UUID) { + if pendingCloseConfirmation == .surface(surfaceID) { + pendingCloseConfirmation = nil + } pendingAgentOSCNotifications.removeValue(forKey: surfaceID)?.cancel() lastCustomNotificationAt.removeValue(forKey: surfaceID) surfaces.removeValue(forKey: surfaceID) @@ -2552,7 +2584,22 @@ final class WorktreeTerminalState { } } - private func handleCloseRequest(for view: GhosttySurfaceView) { + private func handleCloseRequest(for view: GhosttySurfaceView, needsConfirmation: Bool) { + guard surfaces[view.id] === view else { return } + @Shared(.settingsFile) var settingsFile + if needsConfirmation, + pendingExplicitSurfaceCloseIDs.contains(view.id), + settingsFile.global.confirmCloseSurface + { + cancelPendingClose() + pendingExplicitSurfaceCloseIDs.insert(view.id) + pendingCloseConfirmation = .surface(view.id) + return + } + completeCloseRequest(for: view) + } + + private func completeCloseRequest(for view: GhosttySurfaceView) { guard surfaces[view.id] === view else { return } let isExplicitClose = pendingExplicitSurfaceCloseIDs.remove(view.id) != nil if shouldHandleAsUnexpectedZmxClose( @@ -2693,7 +2740,7 @@ final class WorktreeTerminalState { killZmxSessions(forSurfaceIDs: [view.id], includeRemote: includeRemoteSession) } if newTree.isEmpty { - removeFromPendingTabClose(tabId) + removeFromPendingClose(tabId: tabId) trees.removeValue(forKey: tabId) focusedSurfaceIdByTab.removeValue(forKey: tabId) cleanupBlockingScriptLaunchDirectory(for: tabId) diff --git a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift index 5bc96223a..e28fd1c4a 100644 --- a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift +++ b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift @@ -23,7 +23,7 @@ struct WorktreeTerminalTabsView: View { // would reintroduce the closed-all flash on first render. let _: Void = state.ensureInitialTab(focusing: false) let unfocusedSplitOverlay = manager.unfocusedSplitOverlay() - let pendingTabClose = state.pendingTabCloseConfirmation + let pendingClose = state.pendingCloseConfirmation let _ = colorScheme VStack(spacing: 0) { if !state.shouldHideTabBar { @@ -72,25 +72,25 @@ struct WorktreeTerminalTabsView: View { } .animation(.easeInOut(duration: 0.2), value: state.shouldHideTabBar) .alert( - pendingTabClose?.title ?? "Close Tab?", + pendingClose?.title ?? "Close Terminal?", isPresented: Binding( - get: { state.pendingTabCloseConfirmation != nil }, + get: { state.pendingCloseConfirmation != nil }, set: { isPresented in if !isPresented { - state.cancelPendingTabClose() + state.cancelPendingClose() } } ), - presenting: pendingTabClose + presenting: pendingClose ) { pending in Button("Cancel", role: .cancel) { - state.cancelPendingTabClose() + state.cancelPendingClose() } Button(pending.actionTitle, role: .destructive) { - state.confirmPendingTabClose() + state.confirmPendingClose() } - } message: { _ in - Text("One or more processes are still running. Closing will terminate them.") + } message: { pending in + Text(pending.message) } .background( WindowFocusObserverView { activity in diff --git a/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift b/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift index 71a76e31d..5855bfc4c 100644 --- a/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift +++ b/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift @@ -566,8 +566,8 @@ final class GhosttyRuntime { return min(max(value, 0), 1) } - /// Applies Supacode-specific config (padding values) that takes precedence - /// over user settings. + /// Applies Supacode-specific config that takes precedence over user + /// settings. /// /// No `background-opacity` override: surfaces render translucent at the /// theme's opacity and keep their own OSC 11 color. The window tint behind @@ -578,9 +578,15 @@ final class GhosttyRuntime { /// override): surfaces run the real shell with zmx injected as a Ghostty /// `command-wrapper`, so Ghostty resolves and integrates the shell exactly as /// it would without zmx, honoring the user's `command` / `shell-integration`. + /// + /// Supacode owns close-confirmation policy and UI. Keeping Ghostty's + /// predicate enabled makes its callback report prompt safety independently + /// of the user's Ghostty setting; `GlobalSettings.confirmCloseSurface` + /// decides whether Supacode presents the alert. internal static let bundledOverridesString = """ window-padding-x = 14 window-padding-y = 12,0 + confirm-close-surface = true """ /// Reports Supacode in `TERM_PROGRAM` so programs detect the real host diff --git a/supacodeTests/GhosttyRuntimeBundledOverridesTests.swift b/supacodeTests/GhosttyRuntimeBundledOverridesTests.swift index a703773ac..b0dd50915 100644 --- a/supacodeTests/GhosttyRuntimeBundledOverridesTests.swift +++ b/supacodeTests/GhosttyRuntimeBundledOverridesTests.swift @@ -48,6 +48,10 @@ struct GhosttyRuntimeBundledOverridesTests { #expect(!GhosttyRuntime.bundledOverridesString.contains("shell-integration")) } + @Test func bundledOverridesKeepSurfaceCloseDetectionEnabled() { + #expect(GhosttyRuntime.bundledOverridesString.contains("confirm-close-surface = true")) + } + /// Each line in the heredoc is parsed as a Ghostty `key = value` directive /// by `ghostty_config_load_file`. Catches accidental free-form text edits. @Test func bundledOverridesAreKeyValueDirectives() { diff --git a/supacodeTests/SettingsFeatureTests.swift b/supacodeTests/SettingsFeatureTests.swift index 7142352c3..10dabba99 100644 --- a/supacodeTests/SettingsFeatureTests.swift +++ b/supacodeTests/SettingsFeatureTests.swift @@ -30,7 +30,7 @@ struct SettingsFeatureTests { promptForWorktreeCreation: true, terminalThemeSyncEnabled: false, automatedActionPolicy: .always, - confirmCloseTabsWithRunningProcesses: false, + confirmCloseSurface: false, ) @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global = loaded } @@ -61,7 +61,7 @@ struct SettingsFeatureTests { $0.fetchOriginBeforeWorktreeCreation = true $0.terminalThemeSyncEnabled = false $0.automatedActionPolicy = .always - $0.confirmCloseTabsWithRunningProcesses = false + $0.confirmCloseSurface = false } await store.skipReceivedActions() receiveStartupHookChecks(from: store) @@ -115,9 +115,9 @@ struct SettingsFeatureTests { expectNoDifference(settingsFile.global, expectedSettings) } - @Test(.dependencies) func confirmCloseTabsWithRunningProcessesPersistsChanges() async { + @Test(.dependencies) func confirmCloseSurfacePersistsChanges() async { var initialSettings = GlobalSettings.default - initialSettings.confirmCloseTabsWithRunningProcesses = true + initialSettings.confirmCloseSurface = true @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global = initialSettings } @@ -125,11 +125,11 @@ struct SettingsFeatureTests { SettingsFeature() } - await store.send(.binding(.set(\.confirmCloseTabsWithRunningProcesses, false))) { - $0.confirmCloseTabsWithRunningProcesses = false + await store.send(.binding(.set(\.confirmCloseSurface, false))) { + $0.confirmCloseSurface = false } await store.receive(\.delegate.settingsChanged) - #expect(!settingsFile.global.confirmCloseTabsWithRunningProcesses) + #expect(!settingsFile.global.confirmCloseSurface) } @Test(.dependencies) func setSystemNotificationsEnabledPersistsChanges() async { diff --git a/supacodeTests/SettingsFilePersistenceTests.swift b/supacodeTests/SettingsFilePersistenceTests.swift index 9310d5a05..a0e889541 100644 --- a/supacodeTests/SettingsFilePersistenceTests.swift +++ b/supacodeTests/SettingsFilePersistenceTests.swift @@ -387,7 +387,7 @@ struct SettingsFilePersistenceTests { #expect(reloaded.global.terminalThemeSyncEnabled == true) } - @Test(.dependencies) func decodesMissingConfirmCloseTabsWithRunningProcessesAsTrue() throws { + @Test(.dependencies) func decodesMissingConfirmCloseSurfaceAsTrue() throws { let legacy = LegacySettingsFile( global: LegacyGlobalSettings( appearanceMode: .dark, @@ -406,17 +406,17 @@ struct SettingsFilePersistenceTests { return settings } - #expect(settings.global.confirmCloseTabsWithRunningProcesses) + #expect(settings.global.confirmCloseSurface) } - @Test(.dependencies) func roundTripsExplicitConfirmCloseTabsWithRunningProcessesDisabled() throws { + @Test(.dependencies) func roundTripsExplicitConfirmCloseSurfaceDisabled() throws { let storage = SettingsTestStorage() withDependencies { $0.settingsFileStorage = storage.storage } operation: { @Shared(.settingsFile) var settings: SettingsFile - $settings.withLock { $0.global.confirmCloseTabsWithRunningProcesses = false } + $settings.withLock { $0.global.confirmCloseSurface = false } } let reloaded: SettingsFile = withDependencies { @@ -426,7 +426,7 @@ struct SettingsFilePersistenceTests { return reloaded } - #expect(!reloaded.global.confirmCloseTabsWithRunningProcesses) + #expect(!reloaded.global.confirmCloseSurface) } @Test(.dependencies) func decodesMissingRemoteSessionPersistenceEnabledAsTrue() throws { diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 9fa060821..3f8d544b1 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -1056,9 +1056,98 @@ struct WorktreeTerminalManagerTests { #expect(state.surfaceStates[surfaceID] == nil) } + + @Test(.dependencies) func explicitSurfaceCloseConfirmsWhenProcessNeedsConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(surface.id)) + #expect(state.hasTab(tabId)) + + state.cancelPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.hasTab(tabId)) + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: true) + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(tabId)) + } + + @Test(.dependencies) func confirmedSplitSurfaceCloseRemovesOnlyTargetPane() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + guard let tabId = state.createTab(focusing: true), + let initialSurface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + #expect(state.performSplitAction(.newSplit(direction: .right), for: initialSurface.id)) + let leaves = state.splitTree(for: tabId).leaves() + guard leaves.count == 2 else { + Issue.record("Expected a split tab") + return + } + let target = leaves[1] + + #expect(state.performBindingAction("close_surface", onSurfaceID: target.id)) + target.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(target.id)) + + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.hasTab(tabId)) + #expect(state.splitTree(for: tabId).leaves().map(\.id) == [initialSurface.id]) + } + + @Test(.dependencies) func explicitIdleSurfaceCloseSkipsConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: false) + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(tabId)) + } + + @Test(.dependencies) func disabledSettingClosesRunningSurfaceWithoutConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = false } + let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(tabId)) + } @Test(.dependencies) func requestCloseTabConfirmsWhenAnySplitSurfaceHasRunningProcess() { @Shared(.settingsFile) var settingsFile - $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = true } + $settingsFile.withLock { $0.global.confirmCloseSurface = true } var runningSurfaceIDs: Set = [] let state = WorktreeTerminalState( runtime: GhosttyRuntime(), @@ -1080,22 +1169,22 @@ struct WorktreeTerminalManagerTests { runningSurfaceIDs.insert(leaves[1].id) #expect(state.requestCloseTab(tabId)) - #expect(state.pendingTabCloseConfirmation?.tabIDs == [tabId]) + #expect(state.pendingCloseConfirmation == .tabs([tabId])) #expect(state.tabManager.tabs.contains(where: { $0.id == tabId })) - state.cancelPendingTabClose() - #expect(state.pendingTabCloseConfirmation == nil) + state.cancelPendingClose() + #expect(state.pendingCloseConfirmation == nil) #expect(state.tabManager.tabs.contains(where: { $0.id == tabId })) #expect(state.requestCloseTab(tabId)) - state.confirmPendingTabClose() - #expect(state.pendingTabCloseConfirmation == nil) + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) } @Test(.dependencies) func requestCloseTabClosesIdleTabWithoutConfirmation() { @Shared(.settingsFile) var settingsFile - $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = true } + $settingsFile.withLock { $0.global.confirmCloseSurface = true } let state = WorktreeTerminalState( runtime: GhosttyRuntime(), worktree: makeWorktree(), @@ -1107,13 +1196,13 @@ struct WorktreeTerminalManagerTests { } #expect(state.requestCloseTab(tabId)) - #expect(state.pendingTabCloseConfirmation == nil) + #expect(state.pendingCloseConfirmation == nil) #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) } @Test(.dependencies) func disabledSettingClosesRunningTabWithoutConfirmation() { @Shared(.settingsFile) var settingsFile - $settingsFile.withLock { $0.global.confirmCloseTabsWithRunningProcesses = false } + $settingsFile.withLock { $0.global.confirmCloseSurface = false } let state = WorktreeTerminalState( runtime: GhosttyRuntime(), worktree: makeWorktree(), @@ -1125,7 +1214,7 @@ struct WorktreeTerminalManagerTests { } #expect(state.requestCloseTab(tabId)) - #expect(state.pendingTabCloseConfirmation == nil) + #expect(state.pendingCloseConfirmation == nil) #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) } @@ -1313,10 +1402,12 @@ struct WorktreeTerminalManagerTests { #expect(remoteKills.contains(.init(authority: "devbox", sessionID: sessionID))) } - @Test func closedTabStaleRenderDoesNotResurrectSurface() { + @Test(.dependencies) func closedTabStaleRenderDoesNotResurrectSurface() { // A SwiftUI pane can re-render its tab during the tab-close transition; // the lazy splitTree(for:) create must not mint a replacement surface for // the dead tab, or an invisible surface leaks a local+host session pair. + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } let probe = ZmxTestProbe(listing: []) let worktree = makeRemoteWorktree() let manager = makeZmxBackedManager(probe: probe, worktree: worktree) @@ -1330,6 +1421,7 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) surface.bridge.closeSurface(processAlive: true) + state.confirmPendingClose() #expect(state.hasTab(tabID) == false) #expect(state.splitTree(for: tabID).isEmpty) From e337c2802dcd10b278bf3ff2e9b54b777f7c63f8 Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 16:51:02 -0400 Subject: [PATCH 4/8] Align close confirmation with its effect --- .../Models/WorktreeTerminalState.swift | 41 ++++++++++++------- .../WorktreeTerminalManagerTests.swift | 12 +++++- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index abdfdabc1..772f4b6f3 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -50,32 +50,40 @@ final class WorktreeTerminalState { let isFocused: Bool } enum PendingCloseConfirmation: Equatable { - case surface(UUID) + case surface(UUID, closesTab: Bool) case tabs([TerminalTabID]) var title: String { switch self { - case .surface: - return "Close Terminal?" + case .surface(_, closesTab: false): + return "Close Pane?" + case .surface(_, closesTab: true): + return "Close Tab?" + case .tabs(let tabIDs) where tabIDs.count == 1: + return "Close Tab?" case .tabs(let tabIDs): - return tabIDs.count == 1 ? "Close Tab?" : "Close \(tabIDs.count) Tabs?" + return "Close \(tabIDs.count) Tabs?" } } var actionTitle: String { switch self { - case .surface: - return "Close Terminal" - case .tabs(let tabIDs): - return tabIDs.count == 1 ? "Close Tab" : "Close Tabs" + case .surface(_, closesTab: false): + return "Close Pane" + case .surface(_, closesTab: true): + return "Close Tab" + case .tabs(let tabIDs) where tabIDs.count == 1: + return "Close Tab" + case .tabs: + return "Close Tabs" } } var message: String { switch self { - case .surface: - return "The terminal still has a running process. Closing it will terminate the process." - case .tabs: + case .surface(_, closesTab: false): + return "The pane still has a running process. Closing it will terminate the process." + case .surface(_, closesTab: true), .tabs: return "One or more processes are still running. Closing will terminate them." } } @@ -862,7 +870,7 @@ final class WorktreeTerminalState { guard let pending = pendingCloseConfirmation else { return } pendingCloseConfirmation = nil switch pending { - case .surface(let surfaceID): + case .surface(let surfaceID, _): guard let surface = surfaces[surfaceID] else { return } completeCloseRequest(for: surface) case .tabs(let tabIDs): @@ -873,7 +881,7 @@ final class WorktreeTerminalState { } func cancelPendingClose() { - if case .surface(let surfaceID)? = pendingCloseConfirmation { + if case .surface(let surfaceID, _)? = pendingCloseConfirmation { pendingExplicitSurfaceCloseIDs.remove(surfaceID) } pendingCloseConfirmation = nil @@ -2264,7 +2272,9 @@ final class WorktreeTerminalState { /// Also cancels any held agent OSC 9 and forgets the last-custom-notification /// instant so a future surface ID can't reuse stale dedupe state. private func discardSurfaceBookkeeping(for surfaceID: UUID) { - if pendingCloseConfirmation == .surface(surfaceID) { + if case .surface(let pendingSurfaceID, _)? = pendingCloseConfirmation, + pendingSurfaceID == surfaceID + { pendingCloseConfirmation = nil } pendingAgentOSCNotifications.removeValue(forKey: surfaceID)?.cancel() @@ -2591,9 +2601,10 @@ final class WorktreeTerminalState { pendingExplicitSurfaceCloseIDs.contains(view.id), settingsFile.global.confirmCloseSurface { + let closesTab = tabID(containing: view.id).flatMap { trees[$0] }?.leaves().count == 1 cancelPendingClose() pendingExplicitSurfaceCloseIDs.insert(view.id) - pendingCloseConfirmation = .surface(view.id) + pendingCloseConfirmation = .surface(view.id, closesTab: closesTab) return } completeCloseRequest(for: view) diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 3f8d544b1..4a46709eb 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -1070,7 +1070,12 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) surface.bridge.closeSurface(processAlive: true) - #expect(state.pendingCloseConfirmation == .surface(surface.id)) + let pending = state.pendingCloseConfirmation + #expect(pending == .surface(surface.id, closesTab: true)) + let tabClose = WorktreeTerminalState.PendingCloseConfirmation.tabs([tabId]) + #expect(pending?.title == tabClose.title) + #expect(pending?.actionTitle == tabClose.actionTitle) + #expect(pending?.message == tabClose.message) #expect(state.hasTab(tabId)) state.cancelPendingClose() @@ -1104,7 +1109,10 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: target.id)) target.bridge.closeSurface(processAlive: true) - #expect(state.pendingCloseConfirmation == .surface(target.id)) + let pending = state.pendingCloseConfirmation + #expect(pending == .surface(target.id, closesTab: false)) + #expect(pending?.title == "Close Pane?") + #expect(pending?.actionTitle == "Close Pane") state.confirmPendingClose() #expect(state.pendingCloseConfirmation == nil) From 1330289bb32098e8bebb1170b7ce47c921df896d Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 17:07:32 -0400 Subject: [PATCH 5/8] Use generic terminal close warning --- .../Models/WorktreeTerminalState.swift | 40 ++++--------------- .../WorktreeTerminalManagerTests.swift | 10 +++-- 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 772f4b6f3..8ed265ebd 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -50,42 +50,19 @@ final class WorktreeTerminalState { let isFocused: Bool } enum PendingCloseConfirmation: Equatable { - case surface(UUID, closesTab: Bool) + case surface(UUID) case tabs([TerminalTabID]) var title: String { - switch self { - case .surface(_, closesTab: false): - return "Close Pane?" - case .surface(_, closesTab: true): - return "Close Tab?" - case .tabs(let tabIDs) where tabIDs.count == 1: - return "Close Tab?" - case .tabs(let tabIDs): - return "Close \(tabIDs.count) Tabs?" - } + "Close Terminal?" } var actionTitle: String { - switch self { - case .surface(_, closesTab: false): - return "Close Pane" - case .surface(_, closesTab: true): - return "Close Tab" - case .tabs(let tabIDs) where tabIDs.count == 1: - return "Close Tab" - case .tabs: - return "Close Tabs" - } + "Close Terminal" } var message: String { - switch self { - case .surface(_, closesTab: false): - return "The pane still has a running process. Closing it will terminate the process." - case .surface(_, closesTab: true), .tabs: - return "One or more processes are still running. Closing will terminate them." - } + "One or more terminal processes are still running. Closing will terminate them." } } @@ -870,7 +847,7 @@ final class WorktreeTerminalState { guard let pending = pendingCloseConfirmation else { return } pendingCloseConfirmation = nil switch pending { - case .surface(let surfaceID, _): + case .surface(let surfaceID): guard let surface = surfaces[surfaceID] else { return } completeCloseRequest(for: surface) case .tabs(let tabIDs): @@ -881,7 +858,7 @@ final class WorktreeTerminalState { } func cancelPendingClose() { - if case .surface(let surfaceID, _)? = pendingCloseConfirmation { + if case .surface(let surfaceID)? = pendingCloseConfirmation { pendingExplicitSurfaceCloseIDs.remove(surfaceID) } pendingCloseConfirmation = nil @@ -2272,7 +2249,7 @@ final class WorktreeTerminalState { /// Also cancels any held agent OSC 9 and forgets the last-custom-notification /// instant so a future surface ID can't reuse stale dedupe state. private func discardSurfaceBookkeeping(for surfaceID: UUID) { - if case .surface(let pendingSurfaceID, _)? = pendingCloseConfirmation, + if case .surface(let pendingSurfaceID)? = pendingCloseConfirmation, pendingSurfaceID == surfaceID { pendingCloseConfirmation = nil @@ -2601,10 +2578,9 @@ final class WorktreeTerminalState { pendingExplicitSurfaceCloseIDs.contains(view.id), settingsFile.global.confirmCloseSurface { - let closesTab = tabID(containing: view.id).flatMap { trees[$0] }?.leaves().count == 1 cancelPendingClose() pendingExplicitSurfaceCloseIDs.insert(view.id) - pendingCloseConfirmation = .surface(view.id, closesTab: closesTab) + pendingCloseConfirmation = .surface(view.id) return } completeCloseRequest(for: view) diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 4a46709eb..d6621c7d1 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -1071,11 +1071,13 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) surface.bridge.closeSurface(processAlive: true) let pending = state.pendingCloseConfirmation - #expect(pending == .surface(surface.id, closesTab: true)) + #expect(pending == .surface(surface.id)) let tabClose = WorktreeTerminalState.PendingCloseConfirmation.tabs([tabId]) #expect(pending?.title == tabClose.title) #expect(pending?.actionTitle == tabClose.actionTitle) #expect(pending?.message == tabClose.message) + #expect(pending?.title == "Close Terminal?") + #expect(pending?.actionTitle == "Close Terminal") #expect(state.hasTab(tabId)) state.cancelPendingClose() @@ -1110,9 +1112,9 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: target.id)) target.bridge.closeSurface(processAlive: true) let pending = state.pendingCloseConfirmation - #expect(pending == .surface(target.id, closesTab: false)) - #expect(pending?.title == "Close Pane?") - #expect(pending?.actionTitle == "Close Pane") + #expect(pending == .surface(target.id)) + #expect(pending?.title == "Close Terminal?") + #expect(pending?.actionTitle == "Close Terminal") state.confirmPendingClose() #expect(state.pendingCloseConfirmation == nil) From 02fb08a0413c283aeef7d0f13432f8643616433d Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 18:31:56 -0400 Subject: [PATCH 6/8] Preserve active close confirmation target --- .../WorktreeTerminalManager.swift | 6 +- .../Models/WorktreeTerminalState.swift | 21 ++-- .../WorktreeTerminalManagerTests.swift | 98 +++++++++++++++++-- 3 files changed, 108 insertions(+), 17 deletions(-) diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 1a8d54552..69bce6048 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -13,6 +13,7 @@ private let terminalLogger = SupaLogger("Terminal") @Observable final class WorktreeTerminalManager { private let runtime: GhosttyRuntime + @ObservationIgnored private let surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? private(set) var socketServer: AgentHookSocketServer? private var states: [Worktree.ID: WorktreeTerminalState] = [:] @ObservationIgnored @@ -143,9 +144,11 @@ final class WorktreeTerminalManager { socketServer: AgentHookSocketServer? = nil, clock: C = ContinuousClock(), eventBufferCap: Int = WorktreeTerminalManager.defaultEventBufferCap, + surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? = nil ) { self.eventBufferCap = eventBufferCap self.runtime = runtime + self.surfaceBindingActionPerformer = surfaceBindingActionPerformer self.focusedSurfaceBackground = runtime.backgroundColor() self.hookEventSleep = { duration in try await clock.sleep(for: duration) } self.layoutDebounceSleep = { duration in try await clock.sleep(for: duration) } @@ -529,7 +532,8 @@ final class WorktreeTerminalManager { let state = WorktreeTerminalState( runtime: runtime, worktree: worktree, - runSetupScript: runSetupScript + runSetupScript: runSetupScript, + surfaceBindingActionPerformer: surfaceBindingActionPerformer ) state.socketPath = socketServer?.socketPath // Load saved layout snapshot for restoration (skip when a setup script is pending). diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 33ba7b9a4..6505b1ca5 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -75,6 +75,7 @@ final class WorktreeTerminalState { private let runtime: GhosttyRuntime @ObservationIgnored private let splitPreserveZoomOnNavigation: () -> Bool @ObservationIgnored private let surfaceNeedsCloseConfirmation: (GhosttySurfaceView) -> Bool + @ObservationIgnored private let surfaceBindingActionPerformer: (GhosttySurfaceView, String) -> Void private let worktree: Worktree @ObservationIgnored @SharedReader private var repositorySettings: RepositorySettings @@ -243,11 +244,13 @@ final class WorktreeTerminalState { worktree: Worktree, runSetupScript: Bool = false, splitPreserveZoomOnNavigation: (() -> Bool)? = nil, - surfaceNeedsCloseConfirmation: ((GhosttySurfaceView) -> Bool)? = nil + surfaceNeedsCloseConfirmation: ((GhosttySurfaceView) -> Bool)? = nil, + surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? = nil ) { self.runtime = runtime self.splitPreserveZoomOnNavigation = splitPreserveZoomOnNavigation ?? { runtime.splitPreserveZoomOnNavigation() } self.surfaceNeedsCloseConfirmation = surfaceNeedsCloseConfirmation ?? { $0.needsCloseConfirmation } + self.surfaceBindingActionPerformer = surfaceBindingActionPerformer ?? { $0.performBindingAction($1) } self.worktree = worktree self.pendingSetupScript = runSetupScript self.tabManager = TerminalTabManager() @@ -831,7 +834,7 @@ final class WorktreeTerminalState { if action == "close_surface" { pendingExplicitSurfaceCloseIDs.insert(surface.id) } - surface.performBindingAction(action) + surfaceBindingActionPerformer(surface, action) } @discardableResult @@ -893,13 +896,13 @@ final class WorktreeTerminalState { tabManager.tabs.contains(where: { $0.id == requested }) } guard !existingTabIDs.isEmpty else { return false } + guard pendingCloseConfirmation == nil else { return true } @Shared(.settingsFile) var settingsFile let needsConfirmation = settingsFile.global.confirmCloseSurface && existingTabIDs.contains(where: tabNeedsCloseConfirmation) if needsConfirmation { - cancelPendingClose() pendingCloseConfirmation = .tabs(existingTabIDs) } else { for tabId in existingTabIDs { @@ -2656,13 +2659,19 @@ final class WorktreeTerminalState { private func handleCloseRequest(for view: GhosttySurfaceView, needsConfirmation: Bool) { guard surfaces[view.id] === view else { return } + let isExplicitClose = pendingExplicitSurfaceCloseIDs.contains(view.id) + if isExplicitClose, pendingCloseConfirmation != nil { + if pendingCloseConfirmation != .surface(view.id) { + pendingExplicitSurfaceCloseIDs.remove(view.id) + } + return + } + @Shared(.settingsFile) var settingsFile if needsConfirmation, - pendingExplicitSurfaceCloseIDs.contains(view.id), + isExplicitClose, settingsFile.global.confirmCloseSurface { - cancelPendingClose() - pendingExplicitSurfaceCloseIDs.insert(view.id) pendingCloseConfirmation = .surface(view.id) return } diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 7739de8a0..74e3d19ce 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -1131,7 +1131,11 @@ struct WorktreeTerminalManagerTests { @Test(.dependencies) func explicitSurfaceCloseConfirmsWhenProcessNeedsConfirmation() { @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global.confirmCloseSurface = true } - let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) guard let tabId = state.createTab(focusing: true), let surface = state.splitTree(for: tabId).root?.leftmostLeaf() else { @@ -1165,7 +1169,11 @@ struct WorktreeTerminalManagerTests { @Test(.dependencies) func confirmedSplitSurfaceCloseRemovesOnlyTargetPane() { @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global.confirmCloseSurface = true } - let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) guard let tabId = state.createTab(focusing: true), let initialSurface = state.splitTree(for: tabId).root?.leftmostLeaf() else { @@ -1193,10 +1201,55 @@ struct WorktreeTerminalManagerTests { #expect(state.splitTree(for: tabId).leaves().map(\.id) == [initialSurface.id]) } + @Test(.dependencies) func secondSurfaceCloseDoesNotRetargetPendingConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) + guard let tabId = state.createTab(focusing: true), + let initialSurface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + #expect(state.performSplitAction(.newSplit(direction: .right), for: initialSurface.id)) + let leaves = state.splitTree(for: tabId).leaves() + guard leaves.count == 2 else { + Issue.record("Expected a split tab") + return + } + let secondSurface = leaves[1] + + #expect(state.performBindingAction("close_surface", onSurfaceID: secondSurface.id)) + secondSurface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(secondSurface.id)) + + #expect(state.performBindingAction("close_surface", onSurfaceID: initialSurface.id)) + initialSurface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(secondSurface.id)) + + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.hasTab(tabId)) + #expect(state.splitTree(for: tabId).leaves().map(\.id) == [initialSurface.id]) + + #expect(state.performBindingAction("close_surface", onSurfaceID: initialSurface.id)) + initialSurface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(initialSurface.id)) + state.cancelPendingClose() + } + @Test(.dependencies) func explicitIdleSurfaceCloseSkipsConfirmation() { @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global.confirmCloseSurface = true } - let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) guard let tabId = state.createTab(focusing: true), let surface = state.splitTree(for: tabId).root?.leftmostLeaf() else { @@ -1213,7 +1266,11 @@ struct WorktreeTerminalManagerTests { @Test(.dependencies) func disabledSettingClosesRunningSurfaceWithoutConfirmation() { @Shared(.settingsFile) var settingsFile $settingsFile.withLock { $0.global.confirmCloseSurface = false } - let state = WorktreeTerminalState(runtime: GhosttyRuntime(), worktree: makeWorktree()) + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) guard let tabId = state.createTab(focusing: true), let surface = state.splitTree(for: tabId).root?.leftmostLeaf() else { @@ -1465,7 +1522,11 @@ struct WorktreeTerminalManagerTests { // host-side session dies alongside the local one. let probe = ZmxTestProbe(listing: []) let worktree = makeRemoteWorktree() - let manager = makeZmxBackedManager(probe: probe, worktree: worktree) + let manager = makeZmxBackedManager( + probe: probe, + worktree: worktree, + surfaceBindingActionPerformer: { _, _ in } + ) let state = manager.state(for: worktree) guard let tabID = state.createTab(focusing: true), let surface = state.splitTree(for: tabID).root?.leftmostLeaf() @@ -1491,7 +1552,11 @@ struct WorktreeTerminalManagerTests { $settingsFile.withLock { $0.global.confirmCloseSurface = true } let probe = ZmxTestProbe(listing: []) let worktree = makeRemoteWorktree() - let manager = makeZmxBackedManager(probe: probe, worktree: worktree) + let manager = makeZmxBackedManager( + probe: probe, + worktree: worktree, + surfaceBindingActionPerformer: { _, _ in } + ) let state = manager.state(for: worktree) guard let tabID = state.createTab(focusing: true), let surface = state.splitTree(for: tabID).root?.leftmostLeaf() @@ -1956,7 +2021,10 @@ struct WorktreeTerminalManagerTests { @Test func explicitExitedZmxSurfaceCloseDoesNotRecoverLiveSession() async { let probe = ZmxTestProbe(listing: []) - let manager = makeZmxBackedManager(probe: probe) + let manager = makeZmxBackedManager( + probe: probe, + surfaceBindingActionPerformer: { _, _ in } + ) let state = manager.state(for: makeWorktree()) guard let tabId = state.createTab(focusing: false), let surface = state.splitTree(for: tabId).root?.leftmostLeaf() @@ -1984,7 +2052,10 @@ struct WorktreeTerminalManagerTests { @Test func closeSurfaceBindingActionDoesNotRecoverLiveSession() async { let probe = ZmxTestProbe(listing: []) - let manager = makeZmxBackedManager(probe: probe) + let manager = makeZmxBackedManager( + probe: probe, + surfaceBindingActionPerformer: { _, _ in } + ) let state = manager.state(for: makeWorktree()) guard let tabId = state.createTab(focusing: false), let surface = state.splitTree(for: tabId).root?.leftmostLeaf() @@ -3064,7 +3135,11 @@ struct WorktreeTerminalManagerTests { /// `worktree` seeds the pre-created state INSIDE the dependency scope, so /// its `@Dependency(\.zmxClient)` captures the probe-backed client. Tests /// must fetch the state with the same worktree id. - private func makeZmxBackedManager(probe: ZmxTestProbe, worktree: Worktree? = nil) -> WorktreeTerminalManager { + private func makeZmxBackedManager( + probe: ZmxTestProbe, + worktree: Worktree? = nil, + surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? = nil + ) -> WorktreeTerminalManager { let zmxURL = makeFakeZmxBinary() return withDependencies { @@ -3076,7 +3151,10 @@ struct WorktreeTerminalManagerTests { listSessionsWithClients: { await probe.listSessionsWithClients() }, ) } operation: { - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) + let manager = WorktreeTerminalManager( + runtime: GhosttyRuntime(), + surfaceBindingActionPerformer: surfaceBindingActionPerformer + ) _ = manager.state(for: worktree ?? makeWorktree()) return manager } From 4cf37fc71b45fd3a098ffe1f67e3652ba0a28b12 Mon Sep 17 00:00:00 2001 From: Adam Gall Date: Wed, 15 Jul 2026 18:56:03 -0400 Subject: [PATCH 7/8] Stabilize synthetic surface close test --- supacodeTests/AgentBusyStateTests.swift | 11 ++++++++--- supacodeTests/AgentPresence+TestHelpers.swift | 8 +++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/supacodeTests/AgentBusyStateTests.swift b/supacodeTests/AgentBusyStateTests.swift index f492e3721..b465070dc 100644 --- a/supacodeTests/AgentBusyStateTests.swift +++ b/supacodeTests/AgentBusyStateTests.swift @@ -595,7 +595,7 @@ struct AgentBusyStateTests { $0.continuousClock = ImmediateClock() } operation: { withRetentionLimit(.oneHundred) { - let fixture = makeStateWithSurface() + let fixture = makeStateWithSurface(surfaceBindingActionPerformer: { _, _ in }) // Split so the tab keeps a sibling pane; the split focuses the sibling, // leaving the original pane unfocused so its notification lands unread. let sibling = UUID() @@ -720,8 +720,13 @@ struct AgentBusyStateTests { Self.makeHookEvent(name, agent: agent, surfaceID: surfaceID, pid: pid) } - private func makeStateWithSurface(worktree: Worktree? = nil) -> SurfaceFixture { - let (manager, presence) = WorktreeTerminalManager.withPresenceHarness() + private func makeStateWithSurface( + worktree: Worktree? = nil, + surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? = nil + ) -> SurfaceFixture { + let (manager, presence) = WorktreeTerminalManager.withPresenceHarness( + surfaceBindingActionPerformer: surfaceBindingActionPerformer + ) let resolvedWorktree = worktree ?? makeWorktree() let state = manager.state(for: resolvedWorktree) { false } diff --git a/supacodeTests/AgentPresence+TestHelpers.swift b/supacodeTests/AgentPresence+TestHelpers.swift index 66c732ace..2f1b66660 100644 --- a/supacodeTests/AgentPresence+TestHelpers.swift +++ b/supacodeTests/AgentPresence+TestHelpers.swift @@ -114,9 +114,15 @@ extension WorktreeTerminalManager { runtime: GhosttyRuntime = GhosttyRuntime(), socketServer: AgentHookSocketServer? = nil, clock: some Clock = ContinuousClock(), + surfaceBindingActionPerformer: ((GhosttySurfaceView, String) -> Void)? = nil ) -> (manager: WorktreeTerminalManager, presence: PresenceTestHarness) { let harness = PresenceTestHarness() - let manager = WorktreeTerminalManager(runtime: runtime, socketServer: socketServer, clock: clock) + let manager = WorktreeTerminalManager( + runtime: runtime, + socketServer: socketServer, + clock: clock, + surfaceBindingActionPerformer: surfaceBindingActionPerformer + ) harness.attach(to: manager) return (manager, harness) } From 08b7b3a875d3e4b6a08587e1544f35e92c22751a Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Tue, 21 Jul 2026 23:32:54 +0200 Subject: [PATCH 8/8] Harden terminal close-confirmation flow Drive the confirmation alert off the pending target's captured payload so a dismissal written back through the SwiftUINavigation item binding can no longer race the button actions: confirm closes the captured target even if the published value was cleared first, and cancel reliably clears the surface's explicit-close flag so a later unexpected zmx exit still reattaches instead of tearing down. Route every user-facing close command through the confirmation. Ghostty's palette/keybind close-tab now honors its scope, so "close others" and "close to the right" confirm like the context menu instead of collapsing to a single-tab close. Bypass the confirmation only for programmatic surface destroys, which the deeplink/CLI layer already gates upstream, so their acknowledgement is not blocked on an interactive alert. Fold the identical alert copy onto the type, drop the now-dead closeOtherTabs, closeTabsToRight, and closeAllTabs, and log the two silent close paths. Cover the bypass path, captured-payload confirm and cancel, the bulk-close wrappers, the Ghostty close-tab modes, multi-tab confirmation, and destroyed-while-pending cleanup. --- .../WorktreeTerminalManager.swift | 2 +- .../Models/WorktreeTerminalState.swift | 94 +++--- .../Views/WorktreeTerminalTabsView.swift | 34 +-- .../WorktreeTerminalManagerTests.swift | 285 +++++++++++++++++- 4 files changed, 342 insertions(+), 73 deletions(-) diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 1304cb57b..9b1a95793 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -527,7 +527,7 @@ final class WorktreeTerminalManager { existing.enableSetupScriptIfNeeded() } // Reload snapshot if the state has no tabs (e.g., setting was just enabled). - // If `hasAttemptedInitialTab` is sticky-true (closeAllTabs path), the snapshot + // If `hasAttemptedInitialTab` is sticky-true (every tab was closed), the snapshot // stays staged but ensureInitialTab won't consume it; that's intentional. if existing.tabManager.tabs.isEmpty, existing.pendingLayoutSnapshot == nil, diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 6505b1ca5..92262939f 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -53,17 +53,10 @@ final class WorktreeTerminalState { case surface(UUID) case tabs([TerminalTabID]) - var title: String { - "Close Terminal?" - } - - var actionTitle: String { - "Close Terminal" - } - - var message: String { - "One or more terminal processes are still running. Closing will terminate them." - } + // Copy is identical for surface and tab closes, so it lives on the type. + static let title = "Close Terminal?" + static let actionTitle = "Close Terminal" + static let message = "One or more terminal processes are still running. Closing will terminate them." } private struct SurfaceLaunchMetadata { @@ -88,6 +81,8 @@ final class WorktreeTerminalState { @ObservationIgnored private var surfaceLaunchMetadata: [UUID: SurfaceLaunchMetadata] = [:] // Surfaces the user explicitly closed, so an unexpected zmx exit isn't mistaken for one and reattached. @ObservationIgnored private var pendingExplicitSurfaceCloseIDs: Set = [] + // Explicit closes that skip the confirmation alert (programmatic destroys already gated upstream). + @ObservationIgnored private var bypassCloseConfirmationSurfaceIDs: Set = [] @ObservationIgnored private var surfaceGenerationByTab: [TerminalTabID: Int] = [:] @ObservationIgnored private var focusedSurfaceIdByTab: [TerminalTabID: UUID] = [:] /// Per-tab projection cache. `WorktreeTerminalState` recomputes from `trees` @@ -113,7 +108,7 @@ final class WorktreeTerminalState { private var blockingScriptLaunchDirectories: [TerminalTabID: URL] = [:] private var lastBlockingScriptTabByKind: [BlockingScriptKind: TerminalTabID] = [:] private var pendingSetupScript: Bool - /// Sticky after first attempt so a reselect after `closeAllTabs` doesn't auto-recreate. + /// Sticky after first attempt so a reselect after closing every tab doesn't auto-recreate. /// Intentionally never reset; resetting would re-arm the bug. @ObservationIgnored private(set) var hasAttemptedInitialTab = false @ObservationIgnored var pendingLayoutSnapshot: TerminalLayoutSnapshot? @@ -796,11 +791,15 @@ final class WorktreeTerminalState { "closeSurface: surface \(surfaceID) not found. Known: \(surfaces.keys.map(\.uuidString))") return false } - requestExplicitSurfaceClose(surface) + // Programmatic destroys (deeplink/CLI) resolve confirmation upstream, so skip the alert here. + requestExplicitSurfaceClose(surface, confirm: false) return true } - private func requestExplicitSurfaceClose(_ surface: GhosttySurfaceView) { + private func requestExplicitSurfaceClose(_ surface: GhosttySurfaceView, confirm: Bool = true) { + if !confirm { + bypassCloseConfirmationSurfaceIDs.insert(surface.id) + } performBindingAction("close_surface", on: surface) } @@ -872,10 +871,19 @@ final class WorktreeTerminalState { func confirmPendingClose() { guard let pending = pendingCloseConfirmation else { return } + confirmPendingClose(pending) + } + + // Takes the target explicitly so the alert confirms against the captured + // payload, never a published value a concurrent dismissal may have cleared. + func confirmPendingClose(_ pending: PendingCloseConfirmation) { pendingCloseConfirmation = nil switch pending { case .surface(let surfaceID): - guard let surface = surfaces[surfaceID] else { return } + guard let surface = surfaces[surfaceID] else { + terminalStateLogger.debug("confirmPendingClose: surface \(surfaceID) already gone.") + return + } completeCloseRequest(for: surface) case .tabs(let tabIDs): for tabId in tabIDs { @@ -885,12 +893,25 @@ final class WorktreeTerminalState { } func cancelPendingClose() { - if case .surface(let surfaceID)? = pendingCloseConfirmation { + guard let pending = pendingCloseConfirmation else { return } + cancelPendingClose(pending) + } + + // Takes the target explicitly so a dismissal that clears the published value + // first can't strip the surface's explicit-close flag out from under cancel. + func cancelPendingClose(_ pending: PendingCloseConfirmation) { + if case .surface(let surfaceID) = pending { pendingExplicitSurfaceCloseIDs.remove(surfaceID) } pendingCloseConfirmation = nil } + // The alert binding writes nil back on dismissal; the buttons own the real + // transitions, so this only clears without any cancel side effects. + func dismissPendingCloseConfirmation() { + pendingCloseConfirmation = nil + } + private func requestCloseTabs(_ requestedTabIDs: [TerminalTabID]) -> Bool { let existingTabIDs = requestedTabIDs.filter { requested in tabManager.tabs.contains(where: { $0.id == requested }) @@ -957,28 +978,6 @@ final class WorktreeTerminalState { return true } - func closeOtherTabs(keeping tabId: TerminalTabID) { - let ids = tabManager.tabs.map(\.id).filter { $0 != tabId } - for id in ids { - closeTab(id) - } - } - - func closeTabsToRight(of tabId: TerminalTabID) { - guard let index = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return } - let ids = tabManager.tabs.dropFirst(index + 1).map(\.id) - for id in ids { - closeTab(id) - } - } - - func closeAllTabs() { - let ids = tabManager.tabs.map(\.id) - for id in ids { - closeTab(id) - } - } - func splitTree( for tabId: TerminalTabID, inheritingFromSurfaceId: UUID? = nil, @@ -1792,9 +1791,18 @@ final class WorktreeTerminalState { guard self.isLiveSurface(view) else { return false } return self.createTab(inheritingFromSurfaceId: view.id) != nil } - view.bridge.onCloseTab = { [weak self, weak view] _ in + view.bridge.onCloseTab = { [weak self, weak view] mode in guard let self, let view, self.isLiveSurface(view) else { return false } - return self.requestCloseTab(tabId) + // Ghostty's palette/keybind close-tab carries the scope; honor each so + // "close others" / "close to the right" route through confirmation too. + switch mode { + case GHOSTTY_ACTION_CLOSE_TAB_MODE_OTHER: + return self.requestCloseOtherTabs(keeping: tabId) + case GHOSTTY_ACTION_CLOSE_TAB_MODE_RIGHT: + return self.requestCloseTabsToRight(of: tabId) + default: + return self.requestCloseTab(tabId) + } } view.bridge.onGotoTab = { [weak self, weak view] target in guard let self, let view, self.isLiveSurface(view) else { return false } @@ -2338,6 +2346,7 @@ final class WorktreeTerminalState { surfaces.removeValue(forKey: surfaceID) surfaceLaunchMetadata.removeValue(forKey: surfaceID) pendingExplicitSurfaceCloseIDs.remove(surfaceID) + bypassCloseConfirmationSurfaceIDs.remove(surfaceID) surfaceStates.removeValue(forKey: surfaceID) } @@ -2659,6 +2668,11 @@ final class WorktreeTerminalState { private func handleCloseRequest(for view: GhosttySurfaceView, needsConfirmation: Bool) { guard surfaces[view.id] === view else { return } + if bypassCloseConfirmationSurfaceIDs.remove(view.id) != nil { + terminalStateLogger.debug("handleCloseRequest: bypassing confirmation for \(view.id).") + completeCloseRequest(for: view) + return + } let isExplicitClose = pendingExplicitSurfaceCloseIDs.contains(view.id) if isExplicitClose, pendingCloseConfirmation != nil { if pendingCloseConfirmation != .surface(view.id) { diff --git a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift index 07729aeb0..fa798cf47 100644 --- a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift +++ b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift @@ -26,7 +26,6 @@ struct WorktreeTerminalTabsView: View { // the focused background is unchanged (e.g. only `split-divider-color` moved). let _ = manager.configGeneration let unfocusedSplitOverlay = manager.unfocusedSplitOverlay() - let pendingClose = state.pendingCloseConfirmation let dividerColor = manager.splitDividerColor() let _ = colorScheme VStack(spacing: 0) { @@ -77,26 +76,21 @@ struct WorktreeTerminalTabsView: View { } .animation(.easeInOut(duration: 0.2), value: state.shouldHideTabBar) .alert( - pendingClose?.title ?? "Close Terminal?", - isPresented: Binding( - get: { state.pendingCloseConfirmation != nil }, - set: { isPresented in - if !isPresented { - state.cancelPendingClose() - } - } + item: Binding( + get: { state.pendingCloseConfirmation }, + set: { if $0 == nil { state.dismissPendingCloseConfirmation() } } ), - presenting: pendingClose - ) { pending in - Button("Cancel", role: .cancel) { - state.cancelPendingClose() - } - Button(pending.actionTitle, role: .destructive) { - state.confirmPendingClose() - } - } message: { pending in - Text(pending.message) - } + title: { _ in Text(WorktreeTerminalState.PendingCloseConfirmation.title) }, + actions: { pending in + Button("Cancel", role: .cancel) { + state.cancelPendingClose(pending) + } + Button(WorktreeTerminalState.PendingCloseConfirmation.actionTitle, role: .destructive) { + state.confirmPendingClose(pending) + } + }, + message: { _ in Text(WorktreeTerminalState.PendingCloseConfirmation.message) } + ) .background( WindowFocusObserverView { activity in windowActivity = activity diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 74e3d19ce..3419932b4 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -77,7 +77,9 @@ struct WorktreeTerminalManagerTests { let state = manager.state(for: worktree) state.ensureInitialTab(focusing: false) - state.closeAllTabs() + for tab in state.tabManager.tabs { + state.closeTab(tab.id) + } state.ensureInitialTab(focusing: false) @@ -370,7 +372,9 @@ struct WorktreeTerminalManagerTests { #expect(state.currentProjection().runningScripts.isEmpty) continuation.resume() } - state.closeAllTabs() + for tab in state.tabManager.tabs { + state.closeTab(tab.id) + } } } @@ -1147,12 +1151,8 @@ struct WorktreeTerminalManagerTests { surface.bridge.closeSurface(processAlive: true) let pending = state.pendingCloseConfirmation #expect(pending == .surface(surface.id)) - let tabClose = WorktreeTerminalState.PendingCloseConfirmation.tabs([tabId]) - #expect(pending?.title == tabClose.title) - #expect(pending?.actionTitle == tabClose.actionTitle) - #expect(pending?.message == tabClose.message) - #expect(pending?.title == "Close Terminal?") - #expect(pending?.actionTitle == "Close Terminal") + #expect(WorktreeTerminalState.PendingCloseConfirmation.title == "Close Terminal?") + #expect(WorktreeTerminalState.PendingCloseConfirmation.actionTitle == "Close Terminal") #expect(state.hasTab(tabId)) state.cancelPendingClose() @@ -1190,10 +1190,7 @@ struct WorktreeTerminalManagerTests { #expect(state.performBindingAction("close_surface", onSurfaceID: target.id)) target.bridge.closeSurface(processAlive: true) - let pending = state.pendingCloseConfirmation - #expect(pending == .surface(target.id)) - #expect(pending?.title == "Close Terminal?") - #expect(pending?.actionTitle == "Close Terminal") + #expect(state.pendingCloseConfirmation == .surface(target.id)) state.confirmPendingClose() #expect(state.pendingCloseConfirmation == nil) @@ -1356,6 +1353,229 @@ struct WorktreeTerminalManagerTests { #expect(!state.tabManager.tabs.contains(where: { $0.id == tabId })) } + @Test(.dependencies) func programmaticSurfaceDestroyBypassesConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.closeSurface(id: surface.id)) + surface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(tabId)) + } + + @Test(.dependencies) func confirmingCapturedTargetClosesEvenAfterDismissalClearedState() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: true) + guard let pending = state.pendingCloseConfirmation else { + Issue.record("Expected a pending confirmation") + return + } + + // Simulate SwiftUI writing the dismissal back through the alert binding + // before the confirm button's action runs on the same tap. + state.dismissPendingCloseConfirmation() + #expect(state.pendingCloseConfirmation == nil) + state.confirmPendingClose(pending) + #expect(!state.hasTab(tabId)) + } + + @Test(.dependencies) func requestCloseOtherTabsConfirmsThenClosesExactlyOthers() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let first = state.createTab(focusing: true), + let second = state.createTab(focusing: true), + let third = state.createTab(focusing: true), + let secondSurface = state.splitTree(for: second).root?.leftmostLeaf() + else { + Issue.record("Expected three tabs") + return + } + runningSurfaceIDs.insert(secondSurface.id) + + #expect(state.requestCloseOtherTabs(keeping: first)) + #expect(state.pendingCloseConfirmation == .tabs([second, third])) + + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.tabManager.tabs.map(\.id) == [first]) + } + + @Test(.dependencies) func requestCloseTabsToRightTargetsOnlyRightwardTabs() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let first = state.createTab(focusing: true), + let second = state.createTab(focusing: true), + let third = state.createTab(focusing: true), + let thirdSurface = state.splitTree(for: third).root?.leftmostLeaf() + else { + Issue.record("Expected three tabs") + return + } + runningSurfaceIDs.insert(thirdSurface.id) + + #expect(state.requestCloseTabsToRight(of: first)) + #expect(state.pendingCloseConfirmation == .tabs([second, third])) + + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.tabManager.tabs.map(\.id) == [first]) + } + + @Test(.dependencies) func requestCloseAllTabsConfirmsThenClosesEveryTab() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let first = state.createTab(focusing: true), + let second = state.createTab(focusing: true), + let third = state.createTab(focusing: true), + let secondSurface = state.splitTree(for: second).root?.leftmostLeaf() + else { + Issue.record("Expected three tabs") + return + } + runningSurfaceIDs.insert(secondSurface.id) + + #expect(state.requestCloseAllTabs()) + #expect(state.pendingCloseConfirmation == .tabs([first, second, third])) + + state.confirmPendingClose() + #expect(state.pendingCloseConfirmation == nil) + #expect(state.tabManager.tabs.isEmpty) + } + + @Test(.dependencies) func ghosttyCloseTabModesRouteThroughConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let first = state.createTab(focusing: true), + let second = state.createTab(focusing: true), + let third = state.createTab(focusing: true), + let firstSurface = state.splitTree(for: first).root?.leftmostLeaf(), + let secondSurface = state.splitTree(for: second).root?.leftmostLeaf() + else { + Issue.record("Expected three tabs") + return + } + runningSurfaceIDs.insert(secondSurface.id) + + // "Close Other Tabs" from the first tab confirms because a sibling is busy. + #expect(firstSurface.bridge.onCloseTab?(GHOSTTY_ACTION_CLOSE_TAB_MODE_OTHER) == true) + #expect(state.pendingCloseConfirmation == .tabs([second, third])) + state.cancelPendingClose() + + // "Close Tabs to the Right" of the first tab targets the same siblings. + #expect(firstSurface.bridge.onCloseTab?(GHOSTTY_ACTION_CLOSE_TAB_MODE_RIGHT) == true) + #expect(state.pendingCloseConfirmation == .tabs([second, third])) + state.cancelPendingClose() + + // "Close Tab" scopes to the invoking (idle) tab and closes immediately. + #expect(firstSurface.bridge.onCloseTab?(GHOSTTY_ACTION_CLOSE_TAB_MODE_THIS) == true) + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(first)) + } + + @Test(.dependencies) func closingTabIndependentlyNarrowsPendingTabPayload() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + var runningSurfaceIDs: Set = [] + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceNeedsCloseConfirmation: { runningSurfaceIDs.contains($0.id) } + ) + guard let first = state.createTab(focusing: true), + let second = state.createTab(focusing: true), + let third = state.createTab(focusing: true), + let secondSurface = state.splitTree(for: second).root?.leftmostLeaf() + else { + Issue.record("Expected three tabs") + return + } + runningSurfaceIDs.insert(secondSurface.id) + + #expect(state.requestCloseAllTabs()) + #expect(state.pendingCloseConfirmation == .tabs([first, second, third])) + + state.closeTab(first) + #expect(state.pendingCloseConfirmation == .tabs([second, third])) + #expect(!state.hasTab(first)) + + state.closeTab(second) + state.closeTab(third) + #expect(state.pendingCloseConfirmation == nil) + } + + @Test(.dependencies) func tearingDownPendingSurfaceTabClearsConfirmation() { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let state = WorktreeTerminalState( + runtime: GhosttyRuntime(), + worktree: makeWorktree(), + surfaceBindingActionPerformer: { _, _ in } + ) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + + #expect(state.performBindingAction("close_surface", onSurfaceID: surface.id)) + surface.bridge.closeSurface(processAlive: true) + #expect(state.pendingCloseConfirmation == .surface(surface.id)) + + state.closeTab(tabId) + #expect(state.pendingCloseConfirmation == nil) + #expect(!state.hasTab(tabId)) + } + @Test func closeAllSurfacesClearsPerSurfaceBookkeeping() { withDependencies { $0.date.now = Date(timeIntervalSince1970: 1_234) @@ -1810,6 +2030,47 @@ struct WorktreeTerminalManagerTests { #expect(await probe.killedSessions() == []) } + @Test(.dependencies) func canceledSurfaceCloseClearsExplicitFlagSoUnexpectedExitReattaches() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.confirmCloseSurface = true } + let probe = ZmxTestProbe(listing: []) + let manager = makeZmxBackedManager( + probe: probe, + surfaceBindingActionPerformer: { _, _ in } + ) + let state = manager.state(for: makeWorktree()) + guard let tabId = state.createTab(focusing: true), + let surface = state.splitTree(for: tabId).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + let surfaceID = surface.id + await probe.setListing([.init(name: session(for: surfaceID), clients: 0)]) + + // Park a surface-close confirmation, then cancel it the way the alert does: + // the item binding nils the published value before the Cancel action runs. + #expect(state.performBindingAction("close_surface", onSurfaceID: surfaceID)) + surface.bridge.closeSurface(processAlive: true) + guard let pending = state.pendingCloseConfirmation else { + Issue.record("Expected a pending confirmation") + return + } + state.dismissPendingCloseConfirmation() + state.cancelPendingClose(pending) + + // The explicit-close flag must have been cleared, so a later unexpected exit + // reattaches the live session instead of tearing it down. + surface.bridge.closeSurface(processAlive: false) + await probe.waitForListCalls(atLeast: 1) + await waitUntil("zmx surface replacement") { + guard let replacement = state.splitTree(for: tabId).root?.leftmostLeaf() else { return false } + return replacement.id == surfaceID && replacement !== surface + } + #expect(state.tabManager.tabs.contains(where: { $0.id == tabId })) + #expect(await probe.killedSessions() == []) + } + @Test func unexpectedDetachedZmxSurfaceWithLiveSessionReattachesAndKeepsTab() async { let probe = ZmxTestProbe(listing: []) let manager = makeZmxBackedManager(probe: probe)