diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController+Commands.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController+Commands.swift index b4a4eb6dc663..2e5af1c9c1d0 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController+Commands.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController+Commands.swift @@ -67,7 +67,7 @@ extension PageInfoController { /// Site settings of `origin` (any site of this tab's profile, such as /// one whose automatic downloads were blocked), from outside the bubble. public func showSiteSettings(origin: String) { - guard let store, let provider, let url = URL(string: origin) else { return } + guard !isRetired, let store, let provider, let url = URL(string: origin) else { return } close() let site = PageInfoSite(url: url, security: url.scheme == "https" ? .secure : .insecure) windows.showSiteSettings(origin: origin, site: site, store: store, provider: provider, send: { [weak self] in self?.send($0) }) @@ -77,9 +77,12 @@ extension PageInfoController { guard let provider else { return } close() let failure = provider.pageInfoActivity.failedCertificateReason - Task { [weak self] in + certificateTask?.cancel() + // task-owner: the certificate viewer request; retirement cancels it and prevents late presentation + certificateTask = Task { [weak self] in let chain = await provider.pageInfoCertificateChain().compactMap { try? PageInfoCertificate(der: $0) } - self?.windows.showCertificate(chain, site: site, failure: failure) + guard !Task.isCancelled, let self, !self.isRetired else { return } + self.windows.showCertificate(chain, site: site, failure: failure) } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController.swift index 4fe93939eb40..cbb3454333bb 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController.swift @@ -27,6 +27,8 @@ public final class PageInfoController { private let card = PageInfoCardView() var observation: ObservationLoop? var loadTask: Task? + var certificateTask: Task? + var isRetired = false var livePermissions: [SitePermissionKind: SitePermissionSetting] = [:] var openedURL: URL? private var pageHadFocus = false @@ -78,6 +80,7 @@ public final class PageInfoController { /// Runs a command (the registry handlers call this). public func perform(_ command: PageInfoCommand) { + guard !isRetired else { return } switch command { case .show(let page): show(page) case .close: close() @@ -91,6 +94,8 @@ public final class PageInfoController { /// The pane shows another tab: a bubble for the old one closes. public func tabDidChange() { close() + certificateTask?.cancel() + certificateTask = nil windows.closeAll() } @@ -123,8 +128,12 @@ public final class PageInfoController { /// Closes the bubble's panel for good (its page is closing): ``close()`` only orders it out. func closePanel() { + isRetired = true + certificateTask?.cancel() + certificateTask = nil close() popup.closePanel() + windows.closeAll() } /// The bubble closed (``close()`` or a host dismiss rule): live data stops, the page gets diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoWindows.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoWindows.swift index e3ddd1206afe..ae44c755528c 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoWindows.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoWindows.swift @@ -7,6 +7,7 @@ final class PageInfoWindows { private var certificate: CertificateViewerWindow? private var siteSettings: SiteSettingsWindow? private var siteData: SiteDataWindow? + var ownedWindows: [NSWindow] { [certificate, siteSettings, siteData].compactMap { $0 } } /// The shell window the bubble belongs to (`WindowPlacement`). var parentWindow: () -> NSWindow? = { nil } @@ -45,7 +46,10 @@ final class PageInfoWindows { } func closeAll() { - [certificate, siteSettings, siteData].forEach { $0?.close() } + ownedWindows.forEach { $0.close() } + certificate = nil + siteSettings = nil + siteData = nil } private func present(_ window: NSWindow) { diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonConnection.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonConnection.swift index b0009b617d4e..9ea02f45f35c 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonConnection.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonConnection.swift @@ -116,6 +116,13 @@ public actor DaemonConnection { continuation.finish() } + /// Also waits for an in-flight endpoint/reconnect attempt to finish before its owner is reaped. + public func closeAndWait() async { + let reconnect = reconnectTask + close() + await reconnect?.value + } + public var isReady: Bool { if case .ready = phase { return true } return false diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonPrestart.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonPrestart.swift index adba0698f5ff..54a1f5c7e9c1 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonPrestart.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonPrestart.swift @@ -46,7 +46,7 @@ public final class DaemonPrestart: Sendable { await connection.prefetchSnapshot() return .success((connection, identity)) } catch { - await connection.close() + await connection.closeAndWait() return .failure((error as? DaemonError) ?? .launchFailed(String(describing: error))) } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonStartup.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonStartup.swift index 88d443b4f625..d0f32d9df7ea 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonStartup.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonStartup.swift @@ -92,7 +92,7 @@ public struct DaemonStartup: Sendable { identity = try await connection.start() DaemonLaunchTimings.shared.mark("daemon.handshake_end") } catch { - await connection.close() + await connection.closeAndWait() if Task.isCancelled { return nil } let failure = (error as? DaemonError) ?? .launchFailed(String(describing: error)) await onFailure(failure) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService.swift index 74c4710f534f..d50eecb82961 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService.swift @@ -228,7 +228,7 @@ public final class DaemonService { if let (exiting, identity) = connected, let handOff, await handOff(exiting, identity) { // The new connection's endpoint waits for the old daemon to // exit and starts the bundled build, which adopts the terminals. - await exiting.close() + await exiting.closeAndWait() // The first connect was in time; the deadline now covers // the successor's start. self?.armStartupDeadline() @@ -240,7 +240,7 @@ public final class DaemonService { } guard let (connection, identity) = connected else { return } guard let self, !Task.isCancelled else { - await connection.close() + await connection.closeAndWait() return } self.didConnect(connection, identity: identity) @@ -302,7 +302,7 @@ public final class DaemonService { await weakSelf?.noteStartupFailure(error) } if Task.isCancelled { - await connected?.0.close() + await connected?.0.closeAndWait() return } guard let (connection, identity) = connected else { @@ -311,7 +311,7 @@ public final class DaemonService { continue } guard let self, !Task.isCancelled else { - await connection.close() + await connection.closeAndWait() return } do { @@ -323,13 +323,13 @@ public final class DaemonService { let show = !Task.isCancelled self.shutdownConnection() if show { store.markFailed(String(describing: error)) } - await connection.close() + await connection.closeAndWait() return } self.didConnect(connection, identity: identity) logger.info("\(machineID, privacy: .public): cmux-tui \(identity.version, privacy: .public) session \(identity.session, privacy: .public)") await store.run(connection: connection, scheduler: drainScheduler) - await connection.close() + await connection.closeAndWait() if Task.isCancelled { return } guard await ends.waitAfterFailure(wake: wake, clock: clock) else { return } } @@ -467,7 +467,20 @@ public final class DaemonService { /// control socket can await it (`ActionRegistry.track`). @ObservationIgnored public var workTracker: ((ActionWork) -> Void)? + /// Teardown that must finish before the caller removes a test session's state or reaps its owner. + public func shutdownConnectionAndWait() async { + let startup = runTask + let connected = detachConnection() + await connected?.closeAndWait() + await startup?.value + } + public func shutdownConnection() { + // task-owner: teardown hop; close() is idempotent and finishes the store pump + if let connection = detachConnection() { Task { await connection.close() } } + } + + private func detachConnection() -> DaemonConnection? { // Nothing may start an owner again behind the shutdown (the unresponsive owner's restart). recovery.detach() startupDeadlineTimer?.cancel() @@ -480,8 +493,8 @@ public final class DaemonService { runTask = nil keptLayoutRelaunch?.cancel() keptLayoutRelaunch = nil - // task-owner: teardown hop; close() is idempotent and finishes the store pump - if let connection { Task { await connection.close() } } + let connected = connection connection = nil + return connected } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextMacScreen/MacScreenApprovalPanel.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextMacScreen/MacScreenApprovalPanel.swift index ed6a9b84237c..6f35c113b7b2 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextMacScreen/MacScreenApprovalPanel.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextMacScreen/MacScreenApprovalPanel.swift @@ -94,9 +94,10 @@ public final class MacScreenApprovalPrompt { } } - private func settle(_ id: String, hub: DeviceHub) { + func settle(_ id: String, hub: DeviceHub) { settled.insert(id) - current = nil + // Keep the old request until update observes the transition to no request. Clearing it + // here made update's nil == nil guard skip closing the last request's panel. update(hub.pendingRequests) } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AppTestDaemonOwnerTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AppTestDaemonOwnerTests.swift new file mode 100644 index 000000000000..6d21c928a413 --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AppTestDaemonOwnerTests.swift @@ -0,0 +1,62 @@ +@testable import CmuxNextAgentPane +@testable import CmuxNextDaemon +import Darwin +import Foundation +import Testing + +@Suite(.serialized) +struct AppTestDaemonOwnerTests { + private struct HandshakeFailed: Error {} + + private func withChild(_ body: (DaemonEndpoint, AcpmuxUnresponsiveWatch.Daemon) async throws -> Void) async throws { + let child = Process() + child.executableURL = URL(fileURLWithPath: "/bin/sleep") + child.arguments = ["3600"] + child.standardInput = FileHandle.nullDevice + try child.run() + guard let identity = AcpmuxUnresponsiveWatch.kernelProcess(pid: child.processIdentifier) else { + child.terminate() + _ = await ProcessExit.exitEvent(pid: child.processIdentifier, within: .seconds(5), clock: ContinuousClock()) + Issue.record("could not read this test child's kernel identity") + return + } + let endpoint = DaemonEndpoint(socketPath: "/unused", pid: identity.pid) + let result: Result + do { result = .success(try await body(endpoint, identity)) } + catch { result = .failure(error) } + // task-owner: joined child cleanup survives cancellation of the test task. + let exited = await Task { + await AcpmuxUnresponsiveWatch(clock: ContinuousClock()).stop(identity, grace: .seconds(1)) + }.value + #expect(exited) + try result.get() + } + + @Test func aFailedHandshakeStillStopsTheRecordedProcess() async throws { + try await withChild { endpoint, identity in + let exited = await AppTestDaemonOwner.reap(endpoint, process: identity, grace: .seconds(1)) { + throw HandshakeFailed() + } + #expect(exited) + } + } + + @Test func anUnverifiedLiveProcessIsLeftAloneAndCleanupRemainsIncomplete() async throws { + try await withChild { endpoint, identity in + let exited = await AppTestDaemonOwner.reap(endpoint, process: nil) { throw HandshakeFailed() } + #expect(!exited) + #expect(AcpmuxUnresponsiveWatch.kernelProcess(pid: identity.pid) == identity) + } + } + + @Test func aReusedPidDoesNotSignalTheReplacement() async throws { + try await withChild { endpoint, identity in + var previous = identity + previous.token.val.7 &+= 1 + let exited = await AppTestDaemonOwner.reap(endpoint, process: previous) { throw HandshakeFailed() } + #expect(exited, "the recorded process is gone") + #expect(AcpmuxUnresponsiveWatch.kernelProcess(pid: identity.pid) == identity, + "the replacement process must stay alive") + } + } +} diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift index 0a3b02c039cd..af9f57f25341 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift @@ -33,6 +33,7 @@ struct BackgroundAccessoryPolicyTests { let settings: SettingsController let configURL: URL let root: URL + let owner: AppTestDaemonOwner let presented = Presented() init() async throws { @@ -40,16 +41,22 @@ struct BackgroundAccessoryPolicyTests { root = FileManager.default.temporaryDirectory.appendingPathComponent("cn-bg-\(UUID().uuidString.prefix(8))") let session = "cn-bg-\(UUID().uuidString.prefix(8).lowercased())" let launcher = DaemonLauncher( - configuration: .init(binary: binary, session: session, stateDirectory: root.appendingPathComponent("state")), + configuration: .init(binary: binary, session: session, stateDirectory: root.appendingPathComponent("state"), exitWithProcess: ProcessInfo.processInfo.processIdentifier), environment: { ["HOME": NSHomeDirectory(), "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "SHELL": "/bin/sh", "TERM": "xterm-256color"] }) + owner = AppTestDaemonOwner(launcher) + // Finish throwing filesystem setup before adopting services into the process. + configURL = root.appendingPathComponent("cmux.json") + do { + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + try Data("{}".utf8).write(to: configURL) + } catch { + try? FileManager.default.removeItem(at: root) + throw error + } // As `CmuxNextApp.main` and `AppDelegate` launch: regular, and these services are the process's. NSApplication.shared.setActivationPolicy(.regular) services = AppServices(environment: AppEnvironment.current([:])) AppProcessRoot.shared.adopt(services) - // A cmux.json of its own (Settings needs one loaded), with the setting unset: on. - try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) - configURL = root.appendingPathComponent("cmux.json") - try Data("{}".utf8).write(to: configURL) settings = SettingsController(registry: services.registry, design: DesignSettings(), fileURL: configURL, managedReader: FixedManagedPreferenceReader(ManagedPreferences()), managedWatchFiles: []) await settings.reload() @@ -60,99 +67,117 @@ struct BackgroundAccessoryPolicyTests { // The policy each window is presented under (the hook runs after it is ordered in). let presented = presented services.windows.onPresent = { _ in presented.policies.append(NSApp.activationPolicy()) } - services.daemon.start { - DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: launcher.endpointProvider) + services.daemon.start { [owner] in + DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: owner.endpointProvider) } } - func shutDown() async { - if let connection = services.daemon.connection, let identity = services.daemon.identity { - _ = try? await connection.request(ShutdownDaemonRequest(pid: identity.pid, generation: identity.generation, endTerminals: true), - timeout: .seconds(10)) - } - services.daemon.shutdownConnection() + @discardableResult + func shutDown() async -> Bool { + let reaped = await owner.stop(services) + for controller in services.windows.controllers { controller.window?.close() } + for key in Array(services.cache.browsers.keys) { services.cache.release(key) } + let panels = NSApp.windows.filter { services.palette.owns($0) } + services.palette.hide() + for panel in panels { panel.close() } // Leave background mode as a window opening does: its menu bar item goes. services.background.windowWillAppear(activates: false) + return reaped } } + /// Always await daemon shutdown and close the app's windows/status item, including when + /// a requirement or file write throws. The caller's activation-policy defer runs afterward. + private static func withLaunch(_ body: (Launch) async throws -> Void) async throws { + // A test process may have no root yet. Its inert replacement owns no windows or actions. + let previous = AppProcessRoot.shared.services ?? AppServices(environment: AppEnvironment.current([:])) + let launch = try await Launch() + var reaped = false + defer { if reaped { try? FileManager.default.removeItem(at: launch.root) } } + let result: Result + do { result = .success(try await body(launch)) } + catch { result = .failure(error) } + // Disable this launch's background eligibility before closing windows: a queued settle + // must not recreate the status item after shutDown or change the restored activation policy. + AppProcessRoot.shared.adopt(previous) + reaped = await launch.shutDown() + try result.get() + } + /// The window closed while the restore still runs stays closed: the restore neither opens it /// again nor lands the first workspace in a new one. The launch window is the one on screen then. @Test func aWindowClosedBeforeTheRestoreFinishesStaysClosed() async throws { let app = NSApplication.shared let policyBefore = app.activationPolicy() defer { app.setActivationPolicy(policyBefore) } - let launch = try await Launch() - defer { try? FileManager.default.removeItem(at: launch.root) } - let windows = launch.services.windows - windows.restoreWhenLoaded() - try #require(!windows.restored, "the restore finished before the launch window could close") - try #require(windows.controllers.count == 1, "no launch window") - _ = launch.services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) - #expect(windows.controllers.isEmpty) - launch.presented.policies = [] - await Self.wait(within: .seconds(90)) { windows.restored && !windows.registry.isLaunching } - try #require(windows.restored, "the restore never finished") - // A reopened window would land on the next main-actor turns; test-only wait. - try? await Task.sleep(for: .milliseconds(500)) - #expect(launch.presented.policies.isEmpty, "the restore presented a closed window, even if only for a moment") - #expect(windows.controllers.isEmpty, "the restore opened \(windows.controllers.count) window(s) after the user closed the launch window") - await launch.shutDown() + try await Self.withLaunch { launch in + let windows = launch.services.windows + windows.restoreWhenLoaded() + try #require(!windows.restored, "the restore finished before the launch window could close") + try #require(windows.controllers.count == 1, "no launch window") + _ = launch.services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) + #expect(windows.controllers.isEmpty) + launch.presented.policies = [] + await Self.wait(within: .seconds(90)) { windows.restored && !windows.registry.isLaunching } + try #require(windows.restored, "the restore never finished") + // A reopened window would land on the next main-actor turns; test-only wait. + try? await Task.sleep(for: .milliseconds(500)) + #expect(launch.presented.policies.isEmpty, "the restore presented a closed window, even if only for a moment") + #expect(windows.controllers.isEmpty, "the restore opened \(windows.controllers.count) window(s) after the user closed the launch window") + } } @Test func noWindowIsAMenuBarAppAndAnOpeningWindowComesBackRegular() async throws { let app = NSApplication.shared let policyBefore = app.activationPolicy() defer { app.setActivationPolicy(policyBefore) } - let launch = try await Launch() - defer { try? FileManager.default.removeItem(at: launch.root) } - let services = launch.services - let settings = launch.settings - let configURL = launch.configURL - let presented = launch.presented - services.windows.restoreWhenLoaded() - let windows = services.windows - let hasWindow = { windows.active.flatMap { windows.registry.members(of: $0.state.id).first } != nil } - await Self.wait(within: .seconds(90), until: hasWindow) - try #require(hasWindow(), "the launch window never showed a workspace") - #expect(app.activationPolicy() == .regular, "a window is open") + try await Self.withLaunch { launch in + let services = launch.services + let settings = launch.settings + let configURL = launch.configURL + let presented = launch.presented + services.windows.restoreWhenLoaded() + let windows = services.windows + let hasWindow = { windows.active.flatMap { windows.registry.members(of: $0.state.id).first } != nil } + await Self.wait(within: .seconds(90), until: hasWindow) + try #require(hasWindow(), "the launch window never showed a workspace") + #expect(app.activationPolicy() == .regular, "a window is open") - // The user closes the last window (Close All Windows): the app keeps running, off the Dock. - _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) - await Self.wait(within: .seconds(5)) { app.activationPolicy() == .accessory } - #expect(windows.controllers.isEmpty) - #expect(app.activationPolicy() == .accessory, "no window open: no Dock icon and no Cmd-Tab entry") + // The user closes the last window (Close All Windows): the app keeps running, off the Dock. + _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) + await Self.wait(within: .seconds(5)) { app.activationPolicy() == .accessory } + #expect(windows.controllers.isEmpty) + #expect(app.activationPolicy() == .accessory, "no window open: no Dock icon and no Cmd-Tab entry") - // Show cmux (the Dock click's and the menu bar item's path): the window comes back, regular first. - presented.policies = [] - _ = services.registry.perform("showMainWindow", invocation: ActionInvocation(origin: .user)) - await Self.wait(within: .seconds(30)) { !presented.policies.isEmpty } - #expect(windows.controllers.count == 1, "Show cmux opened no window") - #expect(app.activationPolicy() == .regular) - #expect(presented.policies == [.regular], "a window was ordered in under \(presented.policies)") + // Show cmux (the Dock click's and the menu bar item's path): the window comes back, regular first. + presented.policies = [] + _ = services.registry.perform("showMainWindow", invocation: ActionInvocation(origin: .user)) + await Self.wait(within: .seconds(30)) { !presented.policies.isEmpty } + #expect(windows.controllers.count == 1, "Show cmux opened no window") + #expect(app.activationPolicy() == .regular) + #expect(presented.policies == [.regular], "a window was ordered in under \(presented.policies)") - // Settings opened while in the background opens it in a window, so the app is regular again. - _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) - await Self.wait(within: .seconds(5)) { app.activationPolicy() == .accessory } - #expect(app.activationPolicy() == .accessory) - presented.policies = [] - _ = services.registry.perform("openSettings", invocation: ActionInvocation(origin: .user)) - await Self.wait(within: .seconds(30)) { !presented.policies.isEmpty } - #expect(windows.controllers.count == 1, "Settings opened no window") - #expect(app.activationPolicy() == .regular) - #expect(presented.policies == [.regular], "the Settings window was ordered in under \(presented.policies)") + // Settings opened while in the background opens it in a window, so the app is regular again. + _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) + await Self.wait(within: .seconds(5)) { app.activationPolicy() == .accessory } + #expect(app.activationPolicy() == .accessory) + presented.policies = [] + _ = services.registry.perform("openSettings", invocation: ActionInvocation(origin: .user)) + await Self.wait(within: .seconds(30)) { !presented.policies.isEmpty } + #expect(windows.controllers.count == 1, "Settings opened no window") + #expect(app.activationPolicy() == .regular) + #expect(presented.policies == [.regular], "the Settings window was ordered in under \(presented.policies)") - // `app.hideDockIconWithoutWindows: false` keeps the old behavior: regular with no window. - try Data(#"{"app":{"hideDockIconWithoutWindows":false}}"#.utf8).write(to: configURL) - await settings.reload() - #expect(settings.snapshot.hideDockIconWithoutWindows == false) - _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) - await Self.wait(within: .seconds(5)) { windows.controllers.isEmpty } - // A background switch would land on the next main-actor turn; test-only wait. - try? await Task.sleep(for: .milliseconds(500)) - #expect(windows.controllers.isEmpty) - #expect(app.activationPolicy() == .regular, "the setting is off: the Dock icon stays with no window") - - await launch.shutDown() + // `app.hideDockIconWithoutWindows: false` keeps the old behavior: regular with no window. + try Data(#"{"app":{"hideDockIconWithoutWindows":false}}"#.utf8).write(to: configURL) + await settings.reload() + #expect(settings.snapshot.hideDockIconWithoutWindows == false) + _ = services.registry.perform("closeAllWindows", invocation: ActionInvocation(origin: .user)) + await Self.wait(within: .seconds(5)) { windows.controllers.isEmpty } + // A background switch would land on the next main-actor turn; test-only wait. + try? await Task.sleep(for: .milliseconds(500)) + #expect(windows.controllers.isEmpty) + #expect(app.activationPolicy() == .regular, "the setting is off: the Dock icon stays with no window") + } } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BrowserProfileNewTabMenuTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BrowserProfileNewTabMenuTests.swift index 8865edffe9b7..7c791aff2bf1 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BrowserProfileNewTabMenuTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BrowserProfileNewTabMenuTests.swift @@ -20,6 +20,15 @@ struct BrowserProfileNewTabMenuTests { return all(menu.items) } + /// The harness retains its services after closing its shell windows. Retire cached browser + /// pages too, so their Page Info and Extensions panels close on success and on an early throw. + private static func close(_ services: AppServices, palettePanel: NSWindow?) { + let panel = palettePanel ?? services.palette.visiblePanel + services.palette.hide() + panel?.close() + for key in Array(services.cache.browsers.keys) { services.cache.release(key) } + } + /// The browser tabs of `pane` in the daemon's records. private static func browserTabs(_ pane: PaneController, _ services: AppServices) -> [TabModel] { services.daemon.store.workspaces.flatMap(\.screens).flatMap(\.panes).filter { $0.id == pane.pane.id }.flatMap(\.tabs).filter { $0.kind == .browser } @@ -27,6 +36,8 @@ struct BrowserProfileNewTabMenuTests { @Test func theStripMenusNewTabWithBrowserProfileOpensATabOfThePickedProfile() async throws { try await PinnedPageTests.withApp { services, _ in + var palettePanel: NSWindow? + defer { Self.close(services, palettePanel: palettePanel) } services.palette.bindRegistryActions() let pane = try #require(services.windows.active?.workspaceContent?.panes.values.first) let before = Set(Self.browserTabs(pane, services).map(\.id)) @@ -47,12 +58,12 @@ struct BrowserProfileNewTabMenuTests { #expect(ran, "the click did not run") #expect(clickRefusal == nil, "the click refused: \(clickRefusal ?? "")") + // Retain the panel before waiting: a timeout must close it too. + palettePanel = services.palette.visiblePanel // The palette asks which profile; the person picks the default one. try await PinnedPageTests.until({ services.palette.model.rows.contains { $0.item.id == "option:\(BrowserProfileRecord.defaultID)" } }, report: { "palette visible \(services.palette.isVisible), rows \(services.palette.model.rows.map(\.item.id)), refusals \(refusals)" }) - // The palette only orders its panel out when it hides: close it once the test is done. - let palettePanel = services.palette.visiblePanel - defer { palettePanel?.close() } + palettePanel = palettePanel ?? services.palette.visiblePanel services.palette.model.activate(rowID: "option:\(BrowserProfileRecord.defaultID)") try await PinnedPageTests.until({ Self.browserTabs(pane, services).contains { !before.contains($0.id) } }, within: .seconds(30), report: { @@ -68,6 +79,8 @@ struct BrowserProfileNewTabMenuTests { /// dock and a normal pane on its right): each of its panes' strip menus opens the tab. @Test func onHomeEveryPanesStripMenuOpensATabOfThePickedProfile() async throws { try await PasswordManagerRouteTests.withApp { services, connection in + var palettePanel: NSWindow? + defer { Self.close(services, palettePanel: palettePanel) } services.palette.bindRegistryActions() let window = try #require(services.windows.active) services.home.ensureHomeWorkspace(connection) @@ -82,9 +95,6 @@ struct BrowserProfileNewTabMenuTests { let previous = services.registry.refusalObserver services.registry.refusalObserver = { reason, quiet in refusals.append(reason); previous?(reason, quiet) } defer { services.registry.refusalObserver = previous } - // The palette only orders its panel out when it hides: close it once the test is done. - var palettePanel: NSWindow? - defer { palettePanel?.close() } let panes = try #require(window.workspaceContent?.panes.values.map { $0 }).sorted { $0.paneKey < $1.paneKey } for pane in panes { refusals.removeAll() @@ -101,6 +111,7 @@ struct BrowserProfileNewTabMenuTests { ran = services.registry.perform(run.id, invocation: ActionInvocation(target: run.target, arguments: run.arguments)) } #expect(ran && clickRefusal == nil, "pane \(pane.paneKey): the click refused: \(clickRefusal ?? "") ran \(ran)") + palettePanel = palettePanel ?? services.palette.visiblePanel let option = "option:\(BrowserProfileRecord.defaultID)" try await PinnedPageTests.until({ services.palette.model.rows.contains { $0.item.id == option } }, report: { "pane \(pane.paneKey): palette \(services.palette.isVisible), rows \(services.palette.model.rows.map(\.item.id)), refusals \(refusals)" }) @@ -119,6 +130,7 @@ struct BrowserProfileNewTabMenuTests { @Test func theProfileBadgeMenuOpensItsTabInTheRightClickedTabsPane() async throws { let action = "browserProfile.newTab" try await PasswordManagerRouteTests.withApp { services, connection in + defer { Self.close(services, palettePanel: nil) } let window = try #require(services.windows.active) services.home.ensureHomeWorkspace(connection) try await PasswordManagerRouteTests.until({ services.home.homeWorkspace != nil }, within: .seconds(60), diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CrossMachineTabMoveTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CrossMachineTabMoveTests.swift index bc2419b6e3c6..b04d70bf290e 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CrossMachineTabMoveTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CrossMachineTabMoveTests.swift @@ -105,7 +105,26 @@ struct CrossMachineTabMoveTests { /// The other machine's tab matching `match` in the target workspace, once its store shows it. func machineTab(_ match: (TabModel) -> Bool) async throws -> TabModel { - try await poll("no matching tab on the other machine") { + let deadline = ContinuousClock.now + .seconds(20) + // A move's reply can precede the store applying its rename. Read after the + // daemon's barrier so the creation event alone cannot satisfy this lookup. + // The barrier and lookup share one deadline. Leaving the group cancels and + // joins the observation wait or deadline task, including on caller cancellation. + let daemon = self.machine.daemon + try await withThrowingTaskGroup(of: Void.self) { group in + defer { group.cancelAll() } + group.addTask { + try Task.checkCancellation() + try await daemon.appliedThroughSyncBarrier() + try Task.checkCancellation() + } + group.addTask { + try await ContinuousClock().sleep(until: deadline) + throw DaemonError.timedOut("the other machine's store did not reach the move's reply") + } + _ = try await group.next() + } + return try await poll("no matching tab on the other machine", until: deadline) { self.machine.daemon.store.workspaces.first { $0.id == self.target.id }?.screens.flatMap(\.panes).flatMap(\.tabs).first(where: match) } } @@ -118,9 +137,10 @@ struct CrossMachineTabMoveTests { return !present() } - private func poll(_ message: String, _ value: () -> T?) async throws -> T { - let deadline = ContinuousClock.now + .seconds(20) - while value() == nil, ContinuousClock.now < deadline { try? await Task.sleep(for: .milliseconds(50)) } + private func poll(_ message: String, until deadline: ContinuousClock.Instant = ContinuousClock.now + .seconds(20), _ value: () -> T?) async throws -> T { + try Task.checkCancellation() + while value() == nil, ContinuousClock.now < deadline { try await Task.sleep(for: .milliseconds(50)) } + try Task.checkCancellation() return try #require(value(), Comment(rawValue: message)) } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonClosedBrowserPageTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonClosedBrowserPageTests.swift index d6f88d0b8711..80229ad803dd 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonClosedBrowserPageTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonClosedBrowserPageTests.swift @@ -21,8 +21,8 @@ struct DaemonClosedBrowserPageTests { try await PinnedPageTests.until({ surfaces.allSatisfy { store.tab(surface: $0)?.resourceID != nil } }) let tabs = try surfaces.map { try #require(store.tab(surface: $0)) } for tab in tabs { _ = services.cache.browser(for: tab) } - // The tab left open lets its page go when the test ends, as a closed tab's does. - defer { services.cache.release(tabs[0].id) } + // Release both even if the close request or its wait fails; an already retired tab is harmless. + defer { for tab in tabs { services.cache.release(tab.id) } } #expect(tabs.allSatisfy { services.cache.hasContent(for: $0.id) }, "each tab has its page") let closing = tabs[1] diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonDrainFrameTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonDrainFrameTests.swift index b42f41c50cd1..2e84cf1a9e51 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonDrainFrameTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DaemonDrainFrameTests.swift @@ -14,6 +14,51 @@ import Testing /// and a sustained event rate above what one slice per frame applied grew the inbox until it collapsed /// into an overflow and a full snapshot apply. @MainActor @Suite struct DaemonDrainFrameTests { + @Test func shutdownJoinsAnUnadoptedStartup() async { + let entered = AsyncStream.makeStream(bufferingPolicy: .bufferingNewest(1)) + let cancelled = AsyncStream.makeStream(bufferingPolicy: .bufferingNewest(1)) + let release = AsyncStream.makeStream(bufferingPolicy: .bufferingNewest(1)) + defer { + entered.continuation.finish() + cancelled.continuation.finish() + release.continuation.finish() + } + let service = DaemonService() + service.start { + DaemonConnection { + try await withTaskCancellationHandler { + entered.continuation.yield(()) + // task-owner: controlled startup work; the test releases and joins it on shutdown + let attempt = Task { + var iterator = release.stream.makeAsyncIterator() + _ = await iterator.next() + throw CancellationError() + } + return try await attempt.value + } onCancel: { + cancelled.continuation.yield(()) + } + } + } + var started = entered.stream.makeAsyncIterator() + _ = await started.next() + #expect(service.connection == nil) + var stopped = false + // task-owner: the test joins this shutdown after releasing the controlled endpoint attempt + let shutdown = Task { @MainActor in + await service.shutdownConnectionAndWait() + stopped = true + } + var cancellation = cancelled.stream.makeAsyncIterator() + _ = await cancellation.next() + // The cancellation handler ran on detach's main-actor turn. A cancel-only teardown + // would have returned on that turn; a joined teardown waits for the explicit release. + #expect(!stopped) + release.continuation.yield(()) + await shutdown.value + #expect(stopped && service.connection == nil && !service.isStarting) + } + /// One workspace, one screen, one pane with `tabs` tabs on surfaces 1...tabs. private func service(tabs: Int, mainWork: MainWorkScheduler) -> DaemonService { let service = DaemonService(mainWork: mainWork) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DebugPaletteCaptureTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DebugPaletteCaptureTests.swift index 74dc137e86bc..9aa824543182 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DebugPaletteCaptureTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/DebugPaletteCaptureTests.swift @@ -16,9 +16,10 @@ struct DebugPaletteCaptureTests { _ = NSApplication.shared let palette = PaletteController(registry: ActionRegistry.standard(), frecencyPersistence: nil) palette.show(.commands) + // Own the panel even if the visibility assertion fails or it has already hidden. + let ownedPanel = NSApp.windows.first { palette.owns($0) } + defer { palette.hide(); ownedPanel?.close() } let panel = try #require(palette.visiblePanel) - // Hiding only orders the palette's panel out: close it too. - defer { palette.hide(); panel.close() } let path = (NSTemporaryDirectory() as NSString).appendingPathComponent("palette-capture-\(UUID().uuidString).png") defer { try? FileManager.default.removeItem(atPath: path) } let result = DebugPaletteCapture.capture(panel, to: path, windowImage: { _ in nil }) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FreshLaunchSidebarTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FreshLaunchSidebarTests.swift index 50319b87b5a7..e991d1b9b70c 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FreshLaunchSidebarTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FreshLaunchSidebarTests.swift @@ -21,13 +21,23 @@ struct FreshLaunchSidebarTests { @Test func aFreshProfilesLaunchWindowShowsItsSidebarFromItsFirstFrame() async throws { let binary = try #require(SameTreeDaemonBinary.url) let root = FileManager.default.temporaryDirectory.appendingPathComponent("cn-fresh-\(UUID().uuidString.prefix(8))") + var reaped = false + defer { if reaped { try? FileManager.default.removeItem(at: root) } } let session = "cn-fresh-\(UUID().uuidString.prefix(8).lowercased())" let launcher = DaemonLauncher( - configuration: .init(binary: binary, session: session, stateDirectory: root.appendingPathComponent("state")), + configuration: .init(binary: binary, session: session, stateDirectory: root.appendingPathComponent("state"), exitWithProcess: ProcessInfo.processInfo.processIdentifier), environment: { ["HOME": NSHomeDirectory(), "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "SHELL": "/bin/sh", "TERM": "xterm-256color"] }) + let owner = AppTestDaemonOwner(launcher) _ = NSApplication.shared let reveal = LaunchReveal(deadline: .seconds(60)) let services = AppServices(environment: AppEnvironment.current([:]), launchReveal: reveal) + defer { + for controller in services.windows.controllers { controller.window?.close() } + for key in Array(services.cache.browsers.keys) { services.cache.release(key) } + let panels = NSApp.windows.filter { services.palette.owns($0) } + services.palette.hide() + for panel in panels { panel.close() } + } AppActions.bind(services) services.palette.bindRegistryActions() services.windows.ordersWindowsIn = false @@ -35,21 +45,24 @@ struct FreshLaunchSidebarTests { let presented = Presented() services.windows.onPresent = { _ in presented.sidebarShown.append(reveal.isReady(.sidebar)) } services.daemon.start { - DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: launcher.endpointProvider) + DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: owner.endpointProvider) } services.windows.restoreWhenLoaded() - defer { try? FileManager.default.removeItem(at: root) } - let deadline = ContinuousClock.now + .seconds(30) - while presented.sidebarShown.isEmpty, ContinuousClock.now < deadline { - try? await Task.sleep(for: .milliseconds(20)) - } + let result: Result + do { + try Task.checkCancellation() + let deadline = ContinuousClock.now + .seconds(30) + while presented.sidebarShown.isEmpty, ContinuousClock.now < deadline { + try await Task.sleep(for: .milliseconds(20)) + } + try Task.checkCancellation() + result = .success(()) + } catch { result = .failure(error) } - if let connection = services.daemon.connection, let identity = services.daemon.identity { - _ = try? await connection.request(ShutdownDaemonRequest(pid: identity.pid, generation: identity.generation, endTerminals: true), - timeout: .seconds(10)) - } - services.daemon.shutdownConnection() - for controller in services.windows.controllers { controller.window?.close() } + // stop() joins its uncancelled teardown before any failure escapes and the defers + // close windows and remove the state directory, including cancellation before connection. + reaped = await owner.stop(services) + try result.get() #expect(presented.sidebarShown.first == true, "the launch window opened with its sidebar held clear: glass alone") } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FullScreenTitlebarPageModeTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FullScreenTitlebarPageModeTests.swift index 909489296741..7ee0fc335fbf 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FullScreenTitlebarPageModeTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/FullScreenTitlebarPageModeTests.swift @@ -42,9 +42,11 @@ import Testing } @Test func aRealFullScreenTitlebarIsNotAPage() throws { - let type = try #require(NSClassFromString("NSToolbarFullScreenWindow") as? NSWindow.Type) + let type = try Self.standaloneTitlebarClass() let titlebar = type.init(contentRect: CGRect(x: 0, y: 0, width: 800, height: 28), styleMask: [.borderless], backing: .buffered, defer: true) + titlebar.isReleasedWhenClosed = false + defer { titlebar.close() } // Bools only: a failing #expect describes its operands, and this class's description // asks for its full screen owner, which a standalone one does not have. let isPage = WindowOverlayHost.isPageWindow(titlebar) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HiddenPageHibernationTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HiddenPageHibernationTests.swift index e3851e8745fd..f5ee08426d6d 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HiddenPageHibernationTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HiddenPageHibernationTests.swift @@ -356,14 +356,17 @@ import Testing hibernation.pressureDidChange(.critical) folder = FileManager.default.temporaryDirectory .appendingPathComponent("cn-hidden-hibernation-\(UUID().uuidString.prefix(8).lowercased())", isDirectory: true) - try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) window.isReleasedWhenClosed = false + var completed = false + defer { if !completed { close() } } + try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) window.orderFrontRegardless() hibernation.now = { [weak self] in .now + (self?.skipped ?? .zero) } // No test here checks a page that does not answer (UnansweredPageProbeTests does): a probe // starved past its 2 s by a full parallel run would keep the page as `unanswered`, and // nothing would ask it again. hibernation.pageDeadline = .seconds(120) + completed = true } /// A loaded WebKit page showing `body`, titled `name`. diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomeOpenFocusesComposerTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomeOpenFocusesComposerTests.swift index 05aecbae8249..531cd396bb19 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomeOpenFocusesComposerTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomeOpenFocusesComposerTests.swift @@ -140,11 +140,14 @@ import Testing page.layoutSubtreeIfNeeded() page.show(ConversationID(rows[0])) let h = Harness(services: services, window: window, page: page) + var completed = false + defer { if !completed { h.close() } } await wait { h.box != nil } let hasBox = h.box != nil try #require(hasBox, "Home's message box never appeared") let listed = page.list.itemFramesInWindow.count == entries.count try #require(listed, "the list does not show the test items") + completed = true return h } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/MacScreenHubTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/MacScreenHubTests.swift index 49ca80636a99..0090bf54cf7c 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/MacScreenHubTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/MacScreenHubTests.swift @@ -857,8 +857,31 @@ struct MacScreenHubHostOrderTests { /// The host's approval prompt is visible every time, not only the first. @MainActor @Suite(.closesItsWindows) struct MacScreenApprovalPanelTests { + @Test(arguments: [false, true]) + func settlingTheLastRequestClosesItsPanel(observerFirst: Bool) throws { + let prompt = MacScreenApprovalPrompt() + defer { prompt.update([]) } + let hub = testHub() + let request = SessionRequest(id: "settled", viewerId: "v1", viewerName: "Laptop", scope: .view) + prompt.update([request]) + _ = try #require(prompt.panelFrame) + // decide() has removed the request from the hub. Its pending-request observer may + // run before the answer's settlement, or afterward; both must close the last panel. + if observerFirst { prompt.update(hub.pendingRequests) } + prompt.settle(request.id, hub: hub) + #expect(prompt.current == nil) + #expect(prompt.panelFrame == nil) + prompt.update(hub.pendingRequests) + #expect(prompt.panelFrame == nil) + + // Closing drops the old panel; a later request still gets a usable replacement. + prompt.update([SessionRequest(id: "next", viewerId: "v1", viewerName: "Laptop", scope: .full)]) + _ = try #require(prompt.panelFrame) + } + @Test func everyPromptHasAVisibleFrame() throws { let prompt = MacScreenApprovalPrompt() + defer { prompt.update([]) } let first = SessionRequest(id: "s1", viewerId: "v1", viewerName: "Laptop", viewerKeyFingerprint: String(repeating: "ab", count: 32), scope: .view) let second = SessionRequest(id: "s2", viewerId: "v1", viewerName: "Laptop", scope: .full) prompt.update([first]) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift index 0999fc65798a..e716c0e44297 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift @@ -45,20 +45,29 @@ struct PasswordManagerRouteTests { _ body: (AppServices, DaemonConnection) async throws -> Void) async throws { let binary = try #require(SameTreeDaemonBinary.url) let root = FileManager.default.temporaryDirectory.appendingPathComponent("cn-pwroute-\(UUID().uuidString.prefix(8))") - defer { try? FileManager.default.removeItem(at: root) } + var reaped = false + defer { if reaped { try? FileManager.default.removeItem(at: root) } } let launcher = DaemonLauncher( configuration: .init(binary: binary, session: "cn-pwroute-\(UUID().uuidString.prefix(8).lowercased())", - stateDirectory: root.appendingPathComponent("state")), + stateDirectory: root.appendingPathComponent("state"), exitWithProcess: ProcessInfo.processInfo.processIdentifier), environment: { ["HOME": home, "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "SHELL": "/bin/sh", "TERM": "xterm-256color"] }) + let owner = AppTestDaemonOwner(launcher) _ = NSApplication.shared let services = AppServices(environment: AppEnvironment.current([:]), agentPaneEnvironment: agentPaneEnvironment) AppActions.bind(services) services.windows.ordersWindowsIn = false services.daemon.start { - DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: launcher.endpointProvider) + DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil), endpointProvider: owner.endpointProvider) } services.windows.restoreWhenLoaded() - defer { for controller in services.windows.controllers { controller.window?.close() } } + // Registered before restore can suspend or fail, including before the test body starts. + defer { + for controller in services.windows.controllers { controller.window?.close() } + for key in Array(services.cache.browsers.keys) { services.cache.release(key) } + let panels = NSApp.windows.filter { services.palette.owns($0) } + services.palette.hide() + for panel in panels { panel.close() } + } let result: Result do { try await until({ services.daemon.store.workspaces.contains { $0.screens.contains { !$0.panes.isEmpty } } }, within: .seconds(90)) @@ -70,13 +79,7 @@ struct PasswordManagerRouteTests { try await until({ !services.windows.registry.isLaunching }, within: .seconds(30)) result = .success(try await body(services, try #require(services.daemon.connection))) } catch { result = .failure(error) } - // Stop the app's connection first: on the daemon's EOF it would start a - // fresh daemon that outlives the test. The shutdown uses its own socket. - if let connection = services.daemon.connection { - services.daemon.shutdownConnection() - await connection.close() - _ = try? await connection.shutdownDaemon(endTerminals: true) - } + reaped = await owner.stop(services) try result.get() } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PinnedPageTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PinnedPageTests.swift index 58a5971d6ef8..06b59bd2eead 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PinnedPageTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PinnedPageTests.swift @@ -1,10 +1,171 @@ import AppKit import CmuxNextActions +@testable import CmuxNextAgentPane @testable import CmuxNextApp -import CmuxNextDaemon +@testable import CmuxNextDaemon import Foundation +import Darwin import Testing +/// Owns each daemon a test launch ensured, including one whose first handshake never completed. +actor AppTestDaemonOwner { + private let launcher: DaemonLauncher + private var endpoints: Set = [] + private var processes: [DaemonEndpoint: AcpmuxUnresponsiveWatch.Daemon] = [:] + private var ensureFailed = false + private var stopping = false + private var ensures: [UUID: Task] = [:] + + init(_ launcher: DaemonLauncher) { self.launcher = launcher } + + nonisolated var endpointProvider: DaemonConnection.EndpointProvider { + { try await self.endpoint() } + } + + private func endpoint() async throws -> DaemonEndpoint { + guard !stopping else { throw CancellationError() } + let id = UUID() + // task-owner: one bounded ensure; both its caller and reap join it before dropping ownership + let ensure = Task { try await self.ensureEndpoint() } + // Keep the task in the admission set until reap joins it. The daemon can cancel this + // caller while the detached ensure is still launching, and dropping it in defer lets + // teardown miss that in-flight owner. + ensures[id] = ensure + return try await ensure.value + } + + private func ensureEndpoint() async throws -> DaemonEndpoint { + // A cancelled ensure can have detached its owner before stdout reports it. Let this + // bounded attempt finish and record ownership; stop() joins its caller before reaping. + let launcher = launcher + do { + let endpoint = try await launcher.endpointProvider() + record(endpoint) + return endpoint + } catch { + ensureFailed = true + throw error + } + } + + /// False preserves the caller's state directory for an owner whose exit is unconfirmed. + @MainActor @discardableResult func stop(_ services: AppServices) async -> Bool { + // task-owner: joined teardown; runs even when the test's task is already cancelled + await Task { @MainActor in + await services.daemon.shutdownConnectionAndWait() + await services.windows.prepareForTermination() + return await self.reap() + }.value + } + + @discardableResult func reap() async -> Bool { + stopping = true + // Reject new requests and join admitted ensures before taking the ownership census. + for ensure in Array(ensures.values) { _ = try? await ensure.value } + ensures.removeAll() + var resolved = true + if let running = await launcher.runningOwner() { + record(running.endpoint) + } else if ensureFailed { + Issue.record("daemon ensure failed and its final owner could not be established; cleanup is incomplete") + resolved = false + } + for endpoint in Array(endpoints) { + let exited: Bool + if let process = processes[endpoint] { + exited = await Self.reap(endpoint, process: process) { + let connection = DaemonConnection(endpoint: endpoint) + do { + _ = try await connection.start() + // endTerminals uses its own socket and retains this identity. Close the + // subscribed connection first so EOF cannot start a replacement owner. + await connection.closeAndWait() + let reply = try await connection.shutdownDaemon(endTerminals: true) + return reply.accepted == true + } catch { + await connection.closeAndWait() + throw error + } + } + } else { + // The endpoint was never verified as our process. Never connect to its socket: + // it may now belong to a replacement daemon. Wait briefly for it to disappear, + // preserving the state directory if ownership remains ambiguous. + if let pid = endpoint.pid, pid > 1 { + exited = await ProcessExit.exitEvent(pid: pid, within: .seconds(1), clock: ContinuousClock()) + } else { + exited = false + } + } + #expect(exited, "test daemon exit was not confirmed: \(String(describing: endpoint.pid)); preserving its state directory") + if exited { + endpoints.remove(endpoint) + processes[endpoint] = nil + } else { resolved = false } + } + return resolved + } + + /// Remember the process while ensure's endpoint still belongs to the configured binary + /// and socket. The audit token includes the kernel pid version: a recycled pid is never + /// signalled, even if it changes between verification and the signal syscall. + private func record(_ endpoint: DaemonEndpoint) { + endpoints.insert(endpoint) + guard processes[endpoint] == nil, let pid = endpoint.pid, pid > 1, pid != getpid(), + endpoint.bridge == nil, + let before = AcpmuxUnresponsiveWatch.kernelProcess(pid: pid), + AcpmuxVersionHandoff.executablePath(pid: pid) == launcher.configuration.binary.resolvingSymlinksInPath().path, + Self.listenerPID(socketPath: endpoint.socketPath) == pid, + AcpmuxUnresponsiveWatch.kernelProcess(pid: pid) == before else { return } + processes[endpoint] = before + } + + /// A full listen backlog must not block ownership recording or teardown. If this + /// nonblocking peer check cannot finish now, retain the endpoint without signal authority. + private static func listenerPID(socketPath: String) -> Int32? { + var address = sockaddr_un() + address.sun_family = sa_family_t(AF_UNIX) + guard socketPath.utf8.count < MemoryLayout.size(ofValue: address.sun_path) else { return nil } + withUnsafeMutableBytes(of: &address.sun_path) { buffer in + buffer.copyBytes(from: socketPath.utf8) + buffer[socketPath.utf8.count] = 0 + } + let descriptor = socket(AF_UNIX, SOCK_STREAM, 0) + guard descriptor >= 0 else { return nil } + defer { Darwin.close(descriptor) } + guard fcntl(descriptor, F_SETFL, O_NONBLOCK) == 0 else { return nil } + let connected = withUnsafePointer(to: &address) { + $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { + Darwin.connect(descriptor, $0, socklen_t(MemoryLayout.size)) + } + } + guard connected == 0 else { return nil } + var pid: pid_t = 0 + var size = socklen_t(MemoryLayout.size) + return getsockopt(descriptor, SOL_LOCAL, LOCAL_PEERPID, &pid, &size) == 0 && pid > 1 ? pid : nil + } + + /// Shared with the failure-path regression: graceful shutdown may throw before identify. + /// Its fallback uses the existing audit-token signaler and bounded TERM/KILL exit watches. + static func reap(_ endpoint: DaemonEndpoint, process: AcpmuxUnresponsiveWatch.Daemon?, + grace: Duration = .seconds(10), + shutdown: @Sendable () async throws -> Bool) async -> Bool { + guard let pid = endpoint.pid, pid > 1 else { return false } + if (try? await shutdown()) == true, + await ProcessExit.exitEvent(pid: pid, within: .seconds(60), clock: ContinuousClock()) { return true } + guard let process, process.pid == pid else { + // An unverified live pid is evidence to preserve, never permission to signal it. + return await ProcessExit.exitEvent(pid: pid, within: .seconds(1), clock: ContinuousClock()) + } + let watch = AcpmuxUnresponsiveWatch(clock: ContinuousClock(), process: { pid in + // Unreadable identity is not proof of exit. The retained audit token still lets + // the kernel refuse a signal to any replacement process without a pid-only fallback. + AcpmuxUnresponsiveWatch.kernelProcess(pid: pid) ?? process + }) + return await watch.stop(process, grace: grace) + } +} + /// cx-lle.6, Chrome's pinned tabs: a pinned browser tab keeps the page it was pinned on. The /// app's Pin Tab records it on the daemon, the tab reports it after it browses away, and Back to /// Pinned Page (`browser.resetPinnedTab`, tab menu, palette, `cmux browser back-to-pinned-page`) @@ -65,20 +226,29 @@ struct PinnedPageTests { static func withApp(_ body: (AppServices, DaemonConnection) async throws -> Void) async throws { let binary = try #require(SameTreeDaemonBinary.url) let root = FileManager.default.temporaryDirectory.appendingPathComponent("cn-pinned-\(UUID().uuidString.prefix(8))") - defer { try? FileManager.default.removeItem(at: root) } + var reaped = false + defer { if reaped { try? FileManager.default.removeItem(at: root) } } let launcher = DaemonLauncher( configuration: .init(binary: binary, session: "cn-pinned-\(UUID().uuidString.prefix(8).lowercased())", - stateDirectory: root.appendingPathComponent("state")), + stateDirectory: root.appendingPathComponent("state"), exitWithProcess: ProcessInfo.processInfo.processIdentifier), environment: { ["HOME": NSHomeDirectory(), "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "SHELL": "/bin/sh", "TERM": "xterm-256color"] }) + let owner = AppTestDaemonOwner(launcher) _ = NSApplication.shared let services = AppServices(environment: AppEnvironment.current([:])) AppActions.bind(services) services.windows.ordersWindowsIn = false services.daemon.start { - DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil, sessionEvents: true), endpointProvider: launcher.endpointProvider) + DaemonConnection(configuration: DaemonConnection.Configuration(terminalEnvironment: nil, sessionEvents: true), endpointProvider: owner.endpointProvider) } services.windows.restoreWhenLoaded() - defer { for controller in services.windows.controllers { controller.window?.close() } } + // Registered before restore can suspend or fail, including before the test body starts. + defer { + for controller in services.windows.controllers { controller.window?.close() } + for key in Array(services.cache.browsers.keys) { services.cache.release(key) } + let panels = NSApp.windows.filter { services.palette.owns($0) } + services.palette.hide() + for panel in panels { panel.close() } + } let result: Result do { try await until({ services.daemon.store.workspaces.contains { $0.screens.contains { !$0.panes.isEmpty } } }, within: .seconds(90)) @@ -86,13 +256,7 @@ struct PinnedPageTests { try await until({ services.windows.active?.workspaceContent?.panes.isEmpty == false }, within: .seconds(30)) result = .success(try await body(services, try #require(services.daemon.connection))) } catch { result = .failure(error) } - // Stop the app's connection first: on the daemon's EOF it would start a - // fresh daemon that outlives the test. The shutdown uses its own socket. - if let connection = services.daemon.connection { - services.daemon.shutdownConnection() - await connection.close() - _ = try? await connection.shutdownDaemon(endTerminals: true) - } + reaped = await owner.stop(services) try result.get() } } diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextBrowserTests/WebKitTabCloseRetirementTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextBrowserTests/WebKitTabCloseRetirementTests.swift index fc0389b95bfe..ea34975a3f06 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextBrowserTests/WebKitTabCloseRetirementTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextBrowserTests/WebKitTabCloseRetirementTests.swift @@ -10,6 +10,36 @@ import WebKit /// closed tab's web view now leaves its window at once and stays alive, its page open, until /// `WebViewRetirement` closes the page after the scrolling stopped. @MainActor @Suite(.serialized, .timeLimit(.minutes(1))) struct WebKitTabCloseRetirementTests { + @Test func retirementClosesPageInfoAuxiliaryWindowsAndRejectsALateCertificate() async throws { + _ = NSApplication.shared + let tab = MockBrowserEngine().makeMockTab(.init(initialURL: URL(string: "https://example.com"))) + let chrome = BrowserChromeView(tab: tab) + let pageInfo = chrome.pageInfo + defer { + chrome.closePopupPanels() + pageInfo.windows.closeAll() + tab.close() + } + let site = PageInfoSite(state: tab.state) + pageInfo.windows.showCertificate([], site: site, failure: nil) + pageInfo.perform(.siteSettings) + pageInfo.perform(.manageSiteData) + let auxiliary = pageInfo.windows.ownedWindows + #expect(auxiliary.count == 3) + #expect(auxiliary.allSatisfy { $0.isVisible }) + + // Queue a certificate response, retire synchronously before its task runs, then join it. + // No sleeps: the late response must never create another window for the retired page. + pageInfo.perform(.showCertificate) + let certificate = try #require(pageInfo.certificateTask) + BrowserPageRetirement(tab: tab, chrome: chrome).retire() + #expect(auxiliary.allSatisfy { !$0.isVisible }) + await certificate.value + pageInfo.perform(.siteSettings) + pageInfo.perform(.manageSiteData) + #expect(pageInfo.windows.ownedWindows.isEmpty) + } + @Test func aClosedTabsWebViewLeavesItsWindowButIsNotFreedWithTheTab() { weak var released: WKWebView? let window = NSWindow(contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), styleMask: [.titled],