tray: let Sparkle's install quit through the window-close interception

rt-tray/Sources-core/Launch/QuitReason.swift

@@ -7,16 +7,24 @@ import CoreServices
 /// never hold up. No reason at all (a plain Cmd-Q or Dock "Quit") is
 /// `false`: that's the one case the interception exists to catch.
 public enum QuitReason {
+    /// Sparkle's installer agent, `Updater.app`, which sends the install quit.
+    public static let sparkleInstallerBundleIdentifier = "org.sparkle-project.Sparkle.Updater"
+
     /// The whole quit decision: true terminates, false hands the quit to the
     /// window-close interception. A quit arriving with no window on screen
     /// has nothing to close, and it is the only path a script, the Dock, or
     /// Activity Monitor can take once the window is closed, so it stands as
-    /// a real quit.
+    /// a real quit. Sparkle's installer quits the app with the same plain
+    /// event as Cmd-Q, so while an update is installing its quit, told apart
+    /// by the sender, must go through or the install never happens.
     public static func shouldTerminate(quitConfirmed: Bool,
                                        sessionEnding: Bool,
+                                       updateInstalling: Bool,
+                                       senderBundleIdentifier: String?,
                                        reasonCode: UInt32?,
                                        windowOnScreen: Bool) -> Bool {
         if quitConfirmed || sessionEnding { return true }
+        if updateInstalling && senderBundleIdentifier == sparkleInstallerBundleIdentifier { return true }
         if isSystemInitiated(reasonCode: reasonCode) { return true }
         return !windowOnScreen
     }

rt-tray/Sources/AppDelegate.swift

@@ -93,6 +93,10 @@ class AppDelegate: NSObject, NSApplicationDelegate, @unchecked Sendable {
     /// path and visibly block the OS ("app is preventing logout/shutdown").
     /// This backstop trusts the session itself, not just the reason code.
     private var systemSessionEnding = false
+    /// Set by `UpdaterController` while Sparkle installs an update. Read
+    /// here rather than through `updater`, whose lazy init would start
+    /// Sparkle on a copy that is only quitting.
+    private var updateInstalling = false
 
     // MARK: - Lifecycle
 
@@ -509,14 +513,26 @@ class AppDelegate: NSObject, NSApplicationDelegate, @unchecked Sendable {
     /// via the existing `windowWillClose`) and cancels the termination, so
     /// daemon supervision never dies just because the window did. System
     /// shutdown/restart/logout are read from the quit AppleEvent's reason
-    /// and always honored: this interception must never block the OS.
+    /// and always honored: this interception must never block the OS. Nor
+    /// may it block Sparkle's installer, whose quit is what lets an update
+    /// install and relaunch.
     func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
-        let reasonCode = NSAppleEventManager.shared().currentAppleEvent?
-            .paramDescriptor(forKeyword: AEKeyword(kAEQuitReason))?.typeCodeValue
+        let quitEvent = NSAppleEventManager.shared().currentAppleEvent
+        let reasonCode = quitEvent?.paramDescriptor(forKeyword: AEKeyword(kAEQuitReason))?.typeCodeValue
+        let senderPID = quitEvent?.attributeDescriptor(forKeyword: AEKeyword(keySenderPIDAttr))?.int32Value ?? 0
+        let senderBundleIdentifier = senderPID > 0
+            ? NSRunningApplication(processIdentifier: senderPID)?.bundleIdentifier : nil
         let window = mattstackWindow?.window
         let windowOnScreen = (window?.isVisible ?? false) || (window?.isMiniaturized ?? false)
-        if QuitReason.shouldTerminate(quitConfirmed: quitConfirmed, sessionEnding: systemSessionEnding,
-                                      reasonCode: reasonCode, windowOnScreen: windowOnScreen) {
+        let terminate = QuitReason.shouldTerminate(quitConfirmed: quitConfirmed, sessionEnding: systemSessionEnding,
+                                                   updateInstalling: updateInstalling,
+                                                   senderBundleIdentifier: senderBundleIdentifier,
+                                                   reasonCode: reasonCode, windowOnScreen: windowOnScreen)
+        if updateInstalling {
+            TrayLog.info("quit during update install", ["sender": senderBundleIdentifier ?? "none",
+                                                        "terminate": terminate])
+        }
+        if terminate {
             return .terminateNow
         }
         Task { @MainActor in
@@ -1485,6 +1501,9 @@ class AppDelegate: NSObject, NSApplicationDelegate, @unchecked Sendable {
                 TrayState.shared.updateAvailable = version.isEmpty ? nil : version
             }
         }
+        updater.onUpdateInstallingChanged = { [weak self] installing in
+            self?.updateInstalling = installing
+        }
     }
 
     // MARK: - Polling

rt-tray/Sources/Updates/UpdaterController.swift

@@ -8,13 +8,23 @@ import MattstackCore
 final class UpdaterController: NSObject, UpdateChecking, SPUUpdaterDelegate, SPUStandardUserDriverDelegate, @unchecked Sendable {
     @objc dynamic private(set) var canCheckForUpdates = false
     var onUpdateAvailable: ((String) -> Void)?
+    /// Called on the main thread, synchronously, so the flag it sets is in
+    /// place before Sparkle's installer sends its quit event.
+    var onUpdateInstallingChanged: ((Bool) -> Void)?
     private let isBusy: () -> Bool
     private let enabled: Bool
     private var controller: SPUStandardUpdaterController?
     private var observation: NSKeyValueObservation?
 
     private let feedOverride: String?
 
+    // These delegate methods are optional, so a Sparkle rename would leave
+    // ours compiling but never called; naming them here fails the build.
+    private static let requiredDelegateSelectors = [
+        #selector(SPUUpdaterDelegate.updater(_:willInstallUpdate:)),
+        #selector(SPUUpdaterDelegate.updater(_:didFinishUpdateCycleFor:error:)),
+    ]
+
     init(isDevBuild: Bool, isBusy: @escaping () -> Bool) {
         self.isBusy = isBusy
         let info = Bundle.main.infoDictionary
@@ -82,6 +92,18 @@ final class UpdaterController: NSObject, UpdateChecking, SPUUpdaterDelegate, SPU
 
     func feedURLString(for updater: SPUUpdater) -> String? { feedOverride }
 
+    // Sparkle calls this once per update session, just before it asks its
+    // installer to quit the app, and not again when "Install and Relaunch"
+    // is retried, so the flag must hold until the cycle ends.
+    func updater(_ updater: SPUUpdater, willInstallUpdate item: SUAppcastItem) {
+        TrayLog.info("update installing", ["version": item.displayVersionString])
+        onUpdateInstallingChanged?(true)
+    }
+
+    func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) {
+        onUpdateInstallingChanged?(false)
+    }
+
     func updater(_ updater: SPUUpdater, willInstallUpdateOnQuit item: SUAppcastItem,
                  immediateInstallationBlock immediateInstallHandler: @escaping () -> Void) -> Bool {
         // NSStatusBarWindow is always present and always isVisible for this

rt-tray/Tests/MattstackCoreChecks/QuitReasonChecks.swift

@@ -10,21 +10,42 @@ let quitReasonChecks: [Check] = [
         c.expect(QuitReason.isSystemInitiated(reasonCode: UInt32(kAEQuitAll)))
     },
     Check("a quit with no window on screen is a real quit") { c in
-        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false,
+        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: false,
+                                            senderBundleIdentifier: nil,
                                             reasonCode: nil, windowOnScreen: false))
     },
     Check("a quit with the window on screen closes the window instead") { c in
-        c.expect(!QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false,
+        c.expect(!QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: false,
+                                             senderBundleIdentifier: nil,
                                              reasonCode: nil, windowOnScreen: true))
     },
     Check("tray quit, session end, and system reasons terminate even with the window open") { c in
-        c.expect(QuitReason.shouldTerminate(quitConfirmed: true, sessionEnding: false,
+        c.expect(QuitReason.shouldTerminate(quitConfirmed: true, sessionEnding: false, updateInstalling: false,
+                                            senderBundleIdentifier: nil,
                                             reasonCode: nil, windowOnScreen: true))
-        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: true,
+        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: true, updateInstalling: false,
+                                            senderBundleIdentifier: nil,
                                             reasonCode: nil, windowOnScreen: true))
-        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false,
+        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: false,
+                                            senderBundleIdentifier: nil,
                                             reasonCode: UInt32(kAEShutDown), windowOnScreen: true))
     },
+    Check("Sparkle's install quit terminates even with the window on screen") { c in
+        c.expect(QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: true,
+                                            senderBundleIdentifier: "org.sparkle-project.Sparkle.Updater",
+                                            reasonCode: nil, windowOnScreen: true))
+    },
+    Check("a quit that is not Sparkle's install still closes the window") { c in
+        c.expect(!QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: true,
+                                             senderBundleIdentifier: nil,
+                                             reasonCode: nil, windowOnScreen: true))
+        c.expect(!QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: true,
+                                             senderBundleIdentifier: "com.apple.dock",
+                                             reasonCode: nil, windowOnScreen: true))
+        c.expect(!QuitReason.shouldTerminate(quitConfirmed: false, sessionEnding: false, updateInstalling: false,
+                                             senderBundleIdentifier: "org.sparkle-project.Sparkle.Updater",
+                                             reasonCode: nil, windowOnScreen: true))
+    },
     Check("no reason, an unrecognized code, or plain kAELogOut defer to the window-close interception") { c in
         c.expect(!QuitReason.isSystemInitiated(reasonCode: nil))
         c.expect(!QuitReason.isSystemInitiated(reasonCode: 0))