fix(sidebar): finish popover closes whose didClose never arrives

Sources/Sidebar/AppKitList/Cells/SidebarRowSwiftUIPopoverPresenter.swift

@@ -1,5 +1,6 @@
 import AppKit
 import CmuxAppKitSupportUI
+import CmuxFoundation
 import CmuxWorkspaces
 import SwiftUI
 
@@ -29,9 +30,19 @@ final class SidebarRowSwiftUIPopoverPresenter: NSObject, NSPopoverDelegate {
     /// Lazy: cells allocate presenters eagerly, but the hosting machinery
     /// only spins up when a popover actually presents (off the scroll path).
     private lazy var hostingController = NSHostingController(rootView: AnyView(EmptyView()))
-    private var popover: NSPopover?
+    private(set) var popover: NSPopover?
     private var presentationCount = 0
     private var closingProgrammatically = false
+    /// Completes a close whose `popoverDidClose` never arrives; see
+    /// `armCloseCompletionFallback(for:)`.
+    private let closeCompletionFallback: MainActorDeferredActionScheduler
+    /// The popover the pending fallback will complete.
+    private weak var closeCompletionFallbackTarget: NSPopover?
+
+    /// How long an animated close may take before the presenter completes
+    /// it itself: NSPopover's close fade (about 0.2 s) plus a wide margin
+    /// for a busy main thread.
+    static let closeCompletionTimeout: Duration = .seconds(1)
     /// Visible refreshes arrive from the table's configure pass (inside a
     /// representable update turn); defer + coalesce them like
     /// `SidebarWorkspaceTodoPopoverHost` does instead of forcing synchronous
@@ -46,6 +57,13 @@ final class SidebarRowSwiftUIPopoverPresenter: NSObject, NSPopoverDelegate {
     /// must know whether a close already began check this as well.
     private(set) var isClosing = false
 
+    /// - Parameter closeCompletionClock: Drives the close-completion
+    ///   fallback's deadline. Tests pass a clock they advance by hand.
+    init(closeCompletionClock: any Clock<Duration> = ContinuousClock()) {
+        closeCompletionFallback = MainActorDeferredActionScheduler(clock: closeCompletionClock)
+        super.init()
+    }
+
     func present(
         _ root: AnyView,
         relativeTo rect: NSRect,
@@ -60,6 +78,13 @@ final class SidebarRowSwiftUIPopoverPresenter: NSObject, NSPopoverDelegate {
         }
         visibleUpdateScheduler.cancel()
         pendingRoot = nil
+        // Showing a hidden popover again supersedes any close still in
+        // flight for it: its pending fallback must not abandon the popover
+        // that is about to be visible.
+        closeCompletionFallback.cancel()
+        closeCompletionFallbackTarget = nil
+        isClosing = false
+        closingProgrammatically = false
         presentationCount += 1
         applyRootView(root)
         popover.show(relativeTo: rect, of: view, preferredEdge: preferredEdge)
@@ -135,10 +160,71 @@ final class SidebarRowSwiftUIPopoverPresenter: NSObject, NSPopoverDelegate {
     }
 
     func popoverWillClose(_ notification: Notification) {
+        guard isCurrentPopover(notification) else { return }
         isClosing = true
+        if let popover {
+            armCloseCompletionFallback(for: popover)
+        }
     }
 
     func popoverDidClose(_ notification: Notification) {
+        // A didClose that lands after `present` showed the same popover again
+        // belongs to the superseded close; the popover on screen stays.
+        guard isCurrentPopover(notification), popover?.isShown != true else { return }
+        finishClose()
+    }
+
+    /// A notification from a popover this presenter already gave up on (see
+    /// the fallback below) must not tear down the one presented since.
+    private func isCurrentPopover(_ notification: Notification) -> Bool {
+        guard let sender = notification.object as? NSPopover else { return true }
+        return sender === popover
+    }
+
+    /// An animated close reaches `popoverDidClose` only when its animation
+    /// finishes, and on some owned Mac minis that never happens (#14895).
+    /// The popover then stays `isShown` and the presenter stays closing, so
+    /// every later toggle closes the stuck popover again instead of showing
+    /// a new one, and a click-away is never written back to the container.
+    /// If the close hasn't finished within the timeout, finish it here:
+    /// detach from the stuck popover, take its window down, and run the same
+    /// completion `popoverDidClose` would have.
+    ///
+    /// A repeated `popoverWillClose` for the same popover keeps the first
+    /// deadline, so it cannot keep pushing the completion back.
+    private func armCloseCompletionFallback(for closing: NSPopover) {
+        if closeCompletionFallback.isScheduled, closeCompletionFallbackTarget === closing {
+            return
+        }
+        closeCompletionFallbackTarget = closing
+        closeCompletionFallback.schedule(after: Self.closeCompletionTimeout) { [weak self, weak closing] in
+            guard let self, let closing, closing === self.popover, self.isClosing else { return }
+            self.abandon(closing)
+        }
+    }
+
+    private func abandon(_ closing: NSPopover) {
+        closing.delegate = nil
+        // Only this stuck popover loses its animation; the next one is new.
+        closing.animates = false
+        let closingWindow = closing.contentViewController?.view.window
+        if closing.isShown {
+            closing.close()
+        }
+        closingWindow?.orderOut(nil)
+        // The abandoned popover keeps its hosting controller. Should its
+        // stalled close ever resume, it must not tear the content view out of
+        // the popover presented next, so that one gets its own controller.
+        // Swap before `finishClose()`, whose dismissal callback may present.
+        let abandonedController = hostingController
+        hostingController = NSHostingController(rootView: AnyView(EmptyView()))
+        abandonedController.rootView = AnyView(EmptyView())
+        finishClose()
+    }
+
+    private func finishClose() {
+        closeCompletionFallback.cancel()
+        closeCompletionFallbackTarget = nil
         isClosing = false
         visibleUpdateScheduler.cancel()
         pendingRoot = nil

Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowChecklistSection.swift

@@ -25,7 +25,7 @@ final class SidebarRowChecklistSection: NSView {
     private var orderedLines: [SidebarRowChecklistItemLine] = []
     private var freeLines: [SidebarRowChecklistItemLine] = []
     private let addRow = SidebarRowChecklistAddRow()
-    private let popoverPresenter = SidebarRowSwiftUIPopoverPresenter()
+    let popoverPresenter = SidebarRowSwiftUIPopoverPresenter()
 
     private var model: SidebarWorkspaceRowModel?
     private var actions: SidebarAppKitRowActions?

cmux.xcodeproj/project.pbxproj

@@ -3086,6 +3086,7 @@
 		C12791000000000000000003 /* SidebarRowSnapshotCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = C12791000000000000000004 /* SidebarRowSnapshotCache.swift */; };
 		C12791000000000000000005 /* SidebarRowSnapshotCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C12791000000000000000006 /* SidebarRowSnapshotCacheTests.swift */; };
 		B804A0310000000000000031 /* SidebarRowSwiftUIPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0310000000000000031 /* SidebarRowSwiftUIPopoverPresenter.swift */; };
+		D70D754898C65B4C6FA39754 /* SidebarRowSwiftUIPopoverPresenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A6437480E46CCF7BE14124 /* SidebarRowSwiftUIPopoverPresenterTests.swift */; };
 		B804A0320000000000000032 /* SidebarRowTaskStatusGlyphButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0320000000000000032 /* SidebarRowTaskStatusGlyphButton.swift */; };
 		B804A03A000000000000003A /* SidebarRowTextAccessibilityLink.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B03A000000000000003A /* SidebarRowTextAccessibilityLink.swift */; };
 		5CDA4D369D5728DCD6B39BEF /* SidebarRowTextLinkLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = 28782D804EEE07F5CC3E872C /* SidebarRowTextLinkLayout.swift */; };
@@ -7226,6 +7227,7 @@
 		B804B0370000000000000037 /* SidebarRowPressedDim.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/Cells/SidebarRowPressedDim.swift; sourceTree = "<group>"; };
 		C12791000000000000000006 /* SidebarRowSnapshotCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarRowSnapshotCacheTests.swift; sourceTree = "<group>"; };
 		B804B0310000000000000031 /* SidebarRowSwiftUIPopoverPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/Cells/SidebarRowSwiftUIPopoverPresenter.swift; sourceTree = "<group>"; };
+		B3A6437480E46CCF7BE14124 /* SidebarRowSwiftUIPopoverPresenterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "SidebarRowSwiftUIPopoverPresenterTests.swift"; sourceTree = "<group>"; };
 		B804B0320000000000000032 /* SidebarRowTaskStatusGlyphButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/Cells/SidebarRowTaskStatusGlyphButton.swift; sourceTree = "<group>"; };
 		B804B03A000000000000003A /* SidebarRowTextAccessibilityLink.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/Cells/SidebarRowTextAccessibilityLink.swift; sourceTree = "<group>"; };
 		28782D804EEE07F5CC3E872C /* SidebarRowTextLinkLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/Cells/SidebarRowTextLinkLayout.swift; sourceTree = "<group>"; };
@@ -12604,6 +12606,7 @@
 				AA11C5ADC15512A75E032CCA /* TerminalSurfaceTestTeardown.swift */,
 				8E40AA0639D63F947BB321B4 /* UpdateRelaunchBlockersTests.swift */,
 				A634CA3FCA39C6F02FA78DC7 /* UpdateRelaunchAgentResumeTests.swift */,
+				B3A6437480E46CCF7BE14124 /* SidebarRowSwiftUIPopoverPresenterTests.swift */,
 				D87FDCA31298C3B71D50F9E7 /* PasteLastScreenshotShortcutTests.swift */,
 				AFD49B60E1A7F637C0ADFEF4 /* TerminalPasteFailureNoticeTests.swift */,
 				8E924EBE2A4B7D8798B993C1 /* SidebarSelectedRowScrollTests.swift */,
@@ -17193,6 +17196,7 @@
 				5B0C00010000000000000003 /* SidebarResizerOcclusionResolverTests.swift in Sources */,
 				3D0EDA5CB6EA473DB4DB9464 /* SidebarRowPaletteTests.swift in Sources */,
 				C12791000000000000000005 /* SidebarRowSnapshotCacheTests.swift in Sources */,
+				D70D754898C65B4C6FA39754 /* SidebarRowSwiftUIPopoverPresenterTests.swift in Sources */,
 				C9A57513C9A57513C9A57513 /* SidebarScrollViewConfiguratorTests.swift in Sources */,
 				F6528A82B7124269968F9A99 /* SidebarSearchFieldTests.swift in Sources */,
 				4A435919EC29E25A2E139D7A /* SidebarSelectedRowScrollTests.swift in Sources */,

cmuxTests/SidebarRowSwiftUIPopoverPresenterTests.swift

@@ -0,0 +1,176 @@
+import AppKit
+import SwiftUI
+import Testing
+@testable import cmux_DEV
+
+/// On some owned Mac minis an animated `NSPopover` close starts
+/// (`popoverWillClose`) but never finishes (`popoverDidClose`). The presenter
+/// must not stay "closing" forever: that left `isShown` true, so every later
+/// toggle closed the stuck popover again instead of presenting a new one.
+@Suite(.serialized)
+@MainActor
+struct SidebarRowSwiftUIPopoverPresenterTests {
+    @MainActor
+    private final class Host {
+        let anchor = NSView(frame: NSRect(x: 0, y: 0, width: 320, height: 80))
+        let window: NSWindow
+
+        init() {
+            window = NSWindow(
+                contentRect: anchor.bounds,
+                styleMask: [.borderless],
+                backing: .buffered,
+                defer: false
+            )
+            window.contentView = anchor
+            window.orderFront(nil)
+        }
+
+        func present(_ presenter: SidebarRowSwiftUIPopoverPresenter) {
+            presenter.present(
+                AnyView(Text(verbatim: "Checklist")),
+                relativeTo: NSRect(x: anchor.bounds.width - 1, y: 0, width: 1, height: 1),
+                of: anchor,
+                preferredEdge: .maxX
+            )
+        }
+
+        func tearDown(_ presenter: SidebarRowSwiftUIPopoverPresenter) {
+            presenter.onExternalDismiss = nil
+            presenter.close()
+            window.contentView = nil
+            window.close()
+        }
+    }
+
+    /// Waits until the presenter's close fallback is sleeping on `clock`.
+    private func fallbackArmed(on clock: SidebarTestManualClock) async -> Bool {
+        await AppKitTestEventPump().waitUntil(timeout: .seconds(3)) { clock.sleeperCount == 1 }
+    }
+
+    private func closeCompleted(_ presenter: SidebarRowSwiftUIPopoverPresenter) async -> Bool {
+        await AppKitTestEventPump().waitUntil(timeout: .seconds(3)) {
+            !presenter.isShown && !presenter.isClosing
+        }
+    }
+
+    @Test
+    func userCloseWhoseAnimationNeverFinishesStillCompletes() async throws {
+        let host = Host()
+        let clock = SidebarTestManualClock()
+        let presenter = SidebarRowSwiftUIPopoverPresenter(closeCompletionClock: clock)
+        defer { host.tearDown(presenter) }
+        var dismissals = 0
+        presenter.onExternalDismiss = { dismissals += 1 }
+        host.present(presenter)
+        try #require(presenter.isShown)
+
+        // AppKit starts a click-away close, and its `popoverDidClose` never
+        // arrives, as on the affected hosts.
+        presenter.popoverWillClose(Notification(name: NSPopover.willCloseNotification))
+        #expect(presenter.isClosing)
+        #expect(await fallbackArmed(on: clock), "willClose should arm a bounded close fallback")
+        #expect(presenter.isClosing, "The close is still in flight before the deadline")
+
+        clock.advance(by: .seconds(1))
+        #expect(await closeCompleted(presenter), "A close whose animation never finishes should still complete")
+        #expect(dismissals == 1, "The click-away should be reported as an external dismissal once")
+
+        // The next toggle presents a new popover instead of closing the stuck
+        // one, and that popover animates its own close again.
+        host.present(presenter)
+        #expect(presenter.isShown)
+        #expect(!presenter.isClosing)
+        #expect(presenter.popover?.animates == true)
+    }
+
+    @Test
+    func repeatedWillCloseKeepsTheFirstDeadline() async throws {
+        let host = Host()
+        let clock = SidebarTestManualClock()
+        let presenter = SidebarRowSwiftUIPopoverPresenter(closeCompletionClock: clock)
+        defer { host.tearDown(presenter) }
+        host.present(presenter)
+        try #require(presenter.isShown)
+
+        let willClose = Notification(name: NSPopover.willCloseNotification)
+        presenter.popoverWillClose(willClose)
+        #expect(await fallbackArmed(on: clock))
+        clock.advance(by: .milliseconds(600))
+        presenter.popoverWillClose(willClose)
+        await AppKitTestEventPump().drain()
+
+        // One second after the first willClose, not after the second.
+        clock.advance(by: .milliseconds(400))
+        #expect(await closeCompleted(presenter), "A repeated willClose must not push completion back")
+    }
+
+    @Test
+    func representingAHiddenPopoverCancelsThePendingFallback() async throws {
+        let host = Host()
+        let clock = SidebarTestManualClock()
+        let presenter = SidebarRowSwiftUIPopoverPresenter(closeCompletionClock: clock)
+        defer { host.tearDown(presenter) }
+        var dismissals = 0
+        presenter.onExternalDismiss = { dismissals += 1 }
+        host.present(presenter)
+        let popover = try #require(presenter.popover)
+        try #require(presenter.isShown)
+
+        // A close starts and the popover goes hidden, but its didClose has
+        // not reached the presenter yet when the container presents again.
+        presenter.popoverWillClose(Notification(name: NSPopover.willCloseNotification, object: popover))
+        #expect(await fallbackArmed(on: clock))
+        popover.delegate = nil
+        popover.animates = false
+        popover.close()
+        popover.delegate = presenter
+        try #require(!presenter.isShown)
+
+        host.present(presenter)
+        #expect(presenter.isShown)
+        #expect(!presenter.isClosing, "Presenting again supersedes the close in flight")
+        #expect(clock.sleeperCount == 0, "Presenting again cancels the superseded fallback")
+
+        // The old deadline passing must not abandon the popover now showing.
+        clock.advance(by: .seconds(1))
+        await AppKitTestEventPump().drain()
+        #expect(presenter.isShown, "The superseded fallback must not close the re-presented popover")
+        #expect(presenter.popover === popover)
+
+        // The superseded close's didClose arriving late must not tear down
+        // or report a dismissal of the popover now showing.
+        presenter.popoverDidClose(Notification(name: NSPopover.didCloseNotification, object: popover))
+        #expect(presenter.isShown, "A late didClose must not close the re-presented popover")
+        #expect(presenter.popover === popover)
+        #expect(dismissals == 0, "A late didClose is not an external dismissal")
+    }
+
+    @Test
+    func toggleCloseIsNeverAnExternalDismissal() async throws {
+        let host = Host()
+        let clock = SidebarTestManualClock()
+        let presenter = SidebarRowSwiftUIPopoverPresenter(closeCompletionClock: clock)
+        defer { host.tearDown(presenter) }
+        var dismissals = 0
+        presenter.onExternalDismiss = { dismissals += 1 }
+        host.present(presenter)
+        try #require(presenter.isShown)
+
+        // A real animated close. Healthy hosts deliver didClose; affected
+        // hosts never do. Past the fallback's deadline, either way ends the
+        // close, and neither may report the toggle as the user dismissing
+        // the popover from outside.
+        presenter.close()
+        _ = await AppKitTestEventPump().waitUntil(timeout: .seconds(3)) {
+            clock.sleeperCount == 1 || !presenter.isClosing
+        }
+        clock.advance(by: .seconds(1))
+        #expect(await closeCompleted(presenter))
+        await AppKitTestEventPump().drain()
+        #expect(dismissals == 0)
+
+        host.present(presenter)
+        #expect(presenter.isShown)
+    }
+}

cmuxTests/SidebarSelectionCoalescerTests.swift

@@ -126,6 +126,13 @@ final class SidebarTestManualClock: Clock, @unchecked Sendable {
 
     var minimumResolution: Duration { .zero }
 
+    /// Sleeps registered and not yet resumed, for deadline-bounded polls.
+    var sleeperCount: Int {
+        lock.lock()
+        defer { lock.unlock() }
+        return sleepers.count
+    }
+
     var retainedCancellationMarkerCount: Int {
         lock.lock()
         defer { lock.unlock() }