Merge branch 'serial-brew': brew commands run one at a time

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

BrewBar/BrewBarApp.swift

@@ -47,6 +47,14 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
     func applicationDidFinishLaunching(_: Notification) {
         Settings.registerDefaults()
+        UNUserNotificationCenter.current().delegate = self
+
+        NotificationCenter.default.addObserver(
+            self,
+            selector: #selector(windowWillClose(_:)),
+            name: NSWindow.willCloseNotification,
+            object: nil
+        )
 
         statusItem = NSStatusBar.system.statusItem(withLength: NSStatusItem.variableLength)
         statusItem.button?.imagePosition = .imageLeft // icon stays visible next to the count
@@ -117,10 +125,31 @@ class AppDelegate: NSObject, NSApplicationDelegate {
             settingsWindow = SettingsWindowController(appDelegate: self)
         }
         settingsWindow?.syncUI()
-        settingsWindow?.showWindow(nil)
+        presentWindow(of: settingsWindow)
-        // accessory app: without activation the window can open behind others
+    }
+
+    // MARK: - Window presentation
+
+    /// Accessory apps never appear in Cmd-Tab and their windows can open
+    /// behind other apps. While one of our windows is open we temporarily
+    /// become a regular app (Dock icon + Cmd-Tab entry); windowWillClose
+    /// drops back to accessory once the last window is gone.
+    func presentWindow(of controller: NSWindowController?) {
+        NSApp.setActivationPolicy(.regular)
+        controller?.showWindow(nil)
         NSApp.activate(ignoringOtherApps: true)
-        settingsWindow?.window?.makeKeyAndOrderFront(nil)
+        controller?.window?.makeKeyAndOrderFront(nil)
+    }
+
+    @objc func windowWillClose(_: Notification) {
+        // async so the closing window's isVisible has flipped to false
+        DispatchQueue.main.async {
+            let ourWindows = [self.logWindow?.window, self.settingsWindow?.window]
+            let stillOpen = ourWindows.compactMap { $0 }.contains { $0.isVisible }
+            if !stillOpen {
+                NSApp.setActivationPolicy(.accessory)
+            }
+        }
     }
 
     func resolveBrewPath() -> String {
@@ -150,27 +179,34 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     /// prompt whenever brew needs admin rights (no terminal is attached to
     /// Process). If two brew commands overlap, the later one's text wins —
     /// harmless, since prompts realistically only appear during upgrades.
-    static func writeAskpassScript(for command: String) -> URL {
+    static func writeAskpassScript(message: String) -> URL {
         let dir = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask)[0]
             .appendingPathComponent("BrewBar", isDirectory: true)
         try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
 
         // the text lands inside shell single quotes AND an AppleScript string;
         // whitelist characters that cannot break out of either
-        let safeCommand = command.filter { $0.isLetter || $0.isNumber || " ._@+=:/-".contains($0) }
+        let safeMessage = message.filter { $0.isLetter || $0.isNumber || " ._@+=:/-".contains($0) }
 
         let url = dir.appendingPathComponent("askpass.sh")
         let script = """
         #!/bin/zsh
-        osascript -e 'display dialog "BrewBar needs your administrator password to run:\\n\\nbrew \(safeCommand)" default answer "" with hidden answer with title "BrewBar" with icon caution buttons {"Cancel", "OK"} default button "OK"' -e 'text returned of result'
+        osascript -e 'display dialog "BrewBar needs your administrator password to \(safeMessage)." default answer "" with hidden answer with title "BrewBar" with icon caution buttons {"Cancel", "OK"} default button "OK"' -e 'text returned of result'
         """
         try? script.write(to: url, atomically: true, encoding: .utf8)
         try? FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: url.path)
         return url
     }
 
-    func runBrew(_ command: String, completion: @escaping (String) -> Void = { _ in }) {
+    /// Serial: brew commands run strictly one after another. Concurrent brew
-        DispatchQueue.global().async {
+    /// processes could each summon their own sudo password dialog (and their
+    /// log output interleaved); queueing them prevents both.
+    static let brewQueue = DispatchQueue(label: "fr.socheleau.BrewBar.brew")
+
+    /// askpassMessage customizes the sudo dialog ("...password to <message>.");
+    /// defaults to naming the brew command.
+    func runBrew(_ command: String, askpassMessage: String? = nil, completion: @escaping (String) -> Void = { _ in }) {
+        Self.brewQueue.async {
             let brew = self.resolveBrewPath()
             let cleaned = command.replacingOccurrences(of: "brew ", with: "")
             let arguments = [brew] + cleaned.split(separator: " ").map(String.init)
@@ -184,7 +220,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
             process.arguments = arguments
 
             var environment = ProcessInfo.processInfo.environment
-            environment["SUDO_ASKPASS"] = Self.writeAskpassScript(for: cleaned).path
+            environment["SUDO_ASKPASS"] = Self.writeAskpassScript(message: askpassMessage ?? "run: brew \(cleaned)").path
             process.environment = environment
 
             let pipe = Pipe()
@@ -223,15 +259,39 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusMenuItem.title = "Upgrading..."
 
-        // must match the outdated listing: greedy-listed casks are skipped by
-        // plain "upgrade" and would stay outdated forever
-        let command = Settings.includeGreedyCasks ? "upgrade --greedy" : "upgrade"
-
         runBrew("update") { _ in
-            self.runBrew(command) { _ in
+            // packages that turn outdated only after this brew update are not
-                // everything was just upgraded; don't chain an auto-upgrade
+            // in cachedOutdated yet; the final refetch will surface them
-                self.fetchOutdated(allowAutoUpgrade: false)
+            let caskNames = self.cachedOutdated.casks.map(\.name)
+            let upgradeCasksThenRefresh = {
+                self.upgradeCasksSequentially(caskNames) {
+                    // everything was just upgraded; don't chain an auto-upgrade
+                    self.fetchOutdated(allowAutoUpgrade: false)
+                }
             }
+
+            if self.cachedOutdated.formulae.isEmpty {
+                upgradeCasksThenRefresh()
+            } else {
+                self.runBrew("upgrade --formula") { _ in
+                    upgradeCasksThenRefresh()
+                }
+            }
+        }
+    }
+
+    /// Upgrades casks one at a time so each sudo prompt can name the app it
+    /// is for — a batched "upgrade --cask" gives no per-package hook.
+    func upgradeCasksSequentially(_ names: [String], completion: @escaping () -> Void) {
+        guard let next = names.first else {
+            completion()
+            return
+        }
+
+        statusMenuItem.title = "Upgrading \(next)..."
+        // an explicitly named cask upgrades even when self-updating, no --greedy needed
+        runBrew("upgrade --cask \(next)", askpassMessage: "upgrade \(next)") { _ in
+            self.upgradeCasksSequentially(Array(names.dropFirst()), completion: completion)
         }
     }
 
@@ -319,13 +379,9 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     func runAutoUpgrade(formulae: Bool, casks: Bool) {
         statusMenuItem.title = "Auto-upgrading..."
 
+        let caskNames = casks ? cachedOutdated.casks.map(\.name) : []
         let upgradeCasksThenRefresh = {
-            if casks {
+            self.upgradeCasksSequentially(caskNames) {
-                let command = Settings.includeGreedyCasks ? "upgrade --cask --greedy" : "upgrade --cask"
-                self.runBrew(command) { _ in
-                    self.fetchOutdated(allowAutoUpgrade: false)
-                }
-            } else {
                 self.fetchOutdated(allowAutoUpgrade: false)
             }
         }
@@ -370,7 +426,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     @objc func upgradeSingle(_ sender: NSMenuItem) {
         guard let formula = sender.representedObject as? String else { return }
 
-        runBrew("upgrade \(formula)") { _ in
+        runBrew("upgrade \(formula)", askpassMessage: "upgrade \(formula)") { _ in
             self.refreshAll()
         }
     }
@@ -411,7 +467,18 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         if logWindow == nil {
             logWindow = LogWindowController()
         }
-        logWindow?.showWindow(nil)
         logWindow?.update(text: logBuffer)
+        presentWindow(of: logWindow)
+    }
+}
+
+extension AppDelegate: UNUserNotificationCenterDelegate {
+    /// Without this, macOS suppresses banners while BrewBar is the active
+    /// app (e.g. right after toggling the setting with the window open).
+    func userNotificationCenter(_: UNUserNotificationCenter,
+                                willPresent _: UNNotification,
+                                withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void)
+    {
+        completionHandler([.banner])
     }
 }

BrewBar/LogWindowController.swift

@@ -14,22 +14,41 @@ class LogWindowController: NSWindowController {
 
         self.init(window: window)
 
-        window.title = "Logs"
+        window.title = "BrewBar Logs"
+        // the controller keeps the window alive; without this, closing the
+        // window would deallocate it and reopening would crash
+        window.isReleasedWhenClosed = false
+        window.center()
 
         let scrollView = NSScrollView(frame: window.contentView!.bounds)
         scrollView.autoresizingMask = [.width, .height]
+        scrollView.hasVerticalScroller = true
 
         textView = NSTextView(frame: scrollView.bounds)
         textView.isEditable = false
         textView.autoresizingMask = [.width, .height]
+        textView.font = .monospacedSystemFont(ofSize: 12, weight: .regular)
+        textView.textColor = .textColor // adapts to light/dark mode
+        textView.backgroundColor = .textBackgroundColor
+        textView.textContainerInset = NSSize(width: 8, height: 8)
 
         scrollView.documentView = textView
         window.contentView?.addSubview(scrollView)
     }
 
     func update(text: String) {
+        // only follow the tail if the user is already at the bottom — don't
+        // yank the scroll position away while they are reading older output
+        let wasAtBottom: Bool = if let scrollView = textView.enclosingScrollView {
+            scrollView.contentView.bounds.maxY >= textView.frame.height - 30
+        } else {
+            true
+        }
+
         textView.string = text
 
-        textView.scrollToEndOfDocument(nil)
+        if wasAtBottom {
+            textView.scrollToEndOfDocument(nil)
+        }
     }
 }

BrewBar/SettingsWindowController.swift

@@ -246,15 +246,62 @@ class SettingsWindowController: NSWindowController {
             return
         }
 
-        UNUserNotificationCenter.current().requestAuthorization(options: [.alert]) { granted, _ in
+        // requestAuthorization only shows the system prompt the first time
+        // ever; if permission was denied before, it silently returns false.
+        // Check the status first so the user learns what actually happened.
+        let center = UNUserNotificationCenter.current()
+        center.getNotificationSettings { notificationSettings in
             // callback arrives on a background queue; UI needs main
             DispatchQueue.main.async {
-                Settings.notifyOnNewUpdates = granted
+                switch notificationSettings.authorizationStatus {
-                sender.state = granted ? .on : .off
+                case .authorized, .provisional:
+                    Settings.notifyOnNewUpdates = true
+
+                case .notDetermined:
+                    center.requestAuthorization(options: [.alert, .sound]) { granted, error in
+                        DispatchQueue.main.async {
+                            Settings.notifyOnNewUpdates = granted
+                            sender.state = granted ? .on : .off
+                            if let error {
+                                self.showNotificationProblem(error.localizedDescription)
+                            }
+                        }
+                    }
+
+                case .denied:
+                    sender.state = .off
+                    self.showNotificationsDenied()
+
+                @unknown default:
+                    sender.state = .off
+                }
             }
         }
     }
 
+    func showNotificationsDenied() {
+        let alert = NSAlert()
+        alert.alertStyle = .warning
+        alert.messageText = "Notifications are disabled for BrewBar"
+        alert.informativeText = "macOS has notifications turned off for BrewBar. Enable them in System Settings > Notifications > BrewBar, then flip this switch again."
+        alert.addButton(withTitle: "Open System Settings")
+        alert.addButton(withTitle: "Cancel")
+
+        if alert.runModal() == .alertFirstButtonReturn,
+           let url = URL(string: "x-apple.systempreferences:com.apple.Notifications-Settings.extension")
+        {
+            NSWorkspace.shared.open(url)
+        }
+    }
+
+    func showNotificationProblem(_ message: String) {
+        let alert = NSAlert()
+        alert.alertStyle = .warning
+        alert.messageText = "Could not enable notifications"
+        alert.informativeText = message
+        alert.runModal()
+    }
+
     @objc func toggleShowCount(_ sender: NSButton) {
         Settings.showCountInMenuBar = sender.state == .on
         appDelegate?.updateMenuBarCount() // reflect immediately, no refetch needed