Merge branch 'feature/cleanup'

Resolved by combining both sides in upgradeSingle (two-param runBrew
closure from fix/offline-refresh + cleanup hook from feature/cleanup)
and adapting the cleanup branch's runBrew calls to the new signature.

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

BrewBar/BrewBarApp.swift

@@ -1,4 +1,5 @@
 import Cocoa
+import Network
 import UserNotifications
 
 @main
@@ -43,6 +44,11 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     var cachedOutdated: OutdatedPackages = .none
     var lastNotifiedOutdated: Set<String> = []
 
+    let pathMonitor = NWPathMonitor()
+    /// false until the monitor's first update, so the launch refresh waits
+    /// until the network state is actually known. Main-queue only.
+    var isOnline = false
+
     // MARK: - Menu
 
     func applicationDidFinishLaunching(_: Notification) {
@@ -76,6 +82,12 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         outdatedItem.submenu = outdatedSubmenu
         outdatedItem.isHidden = true // shown once a fetch finds outdated packages
         menu.addItem(outdatedItem)
+        menu.addItem(NSMenuItem(title: "🧹 Cleanup", action: #selector(cleanupAction), keyEquivalent: ""))
+        // isAlternate swaps this in for the item right above while ⌥ is held
+        let purgeItem = NSMenuItem(title: "🧹 Cleanup (purge cache)", action: #selector(cleanupPurgeAction), keyEquivalent: "")
+        purgeItem.keyEquivalentModifierMask = .option
+        purgeItem.isAlternate = true
+        menu.addItem(purgeItem)
         menu.addItem(NSMenuItem(title: "📜 Logs", action: #selector(showLogs), keyEquivalent: "l"))
 
         menu.addItem(NSMenuItem.separator())
@@ -91,7 +103,16 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusItem.menu = menu
 
-        refreshAll()
+        // no refreshAll() here: the monitor fires once immediately after
+        // start(), and the first online update triggers the launch refresh
+        pathMonitor.pathUpdateHandler = { path in
+            // updates arrive on the monitor's queue; hop to main for AppKit
+            DispatchQueue.main.async {
+                self.networkPathChanged(online: path.status == .satisfied)
+            }
+        }
+        pathMonitor.start(queue: DispatchQueue(label: "fr.socheleau.BrewBar.network"))
+
         refreshBrewVersion()
 
         refreshTimer = Timer.scheduledTimer(withTimeInterval: refreshInterval, repeats: true) { _ in
@@ -205,7 +226,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
     /// 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 }) {
+    func runBrew(_ command: String, askpassMessage: String? = nil, completion: @escaping (_ output: String, _ success: Bool) -> Void = { _, _ in }) {
         Self.brewQueue.async {
             let brew = self.resolveBrewPath()
             let cleaned = command.replacingOccurrences(of: "brew ", with: "")
@@ -231,7 +252,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
                 try process.run()
             } catch {
                 self.log("ERROR: \(error)\n")
-                DispatchQueue.main.async { completion("") }
+                DispatchQueue.main.async { completion("", false) }
                 return
             }
 
@@ -241,8 +262,12 @@ class AppDelegate: NSObject, NSApplicationDelegate {
             let output = String(data: data, encoding: .utf8) ?? ""
 
             self.log(output)
+            let success = process.terminationStatus == 0
+            if !success {
+                self.log("✗ exited with status \(process.terminationStatus)\n")
+            }
             // completion always on main: callers update AppKit UI, which is main-thread only
-            DispatchQueue.main.async { completion(output) }
+            DispatchQueue.main.async { completion(output, success) }
         }
     }
 
@@ -252,6 +277,34 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         refreshAll()
     }
 
+    @objc func cleanupAction() {
+        runCleanup(purgeCache: false)
+    }
+
+    @objc func cleanupPurgeAction() {
+        runCleanup(purgeCache: true)
+    }
+
+    /// --prune=all also deletes cached downloads brew would normally keep
+    /// (current versions); the only cost is re-downloading at next install
+    func runCleanup(purgeCache: Bool) {
+        statusMenuItem.title = "Cleaning up..."
+        runBrew(purgeCache ? "cleanup --prune=all" : "cleanup") { _, _ in
+            self.updateStatus(count: self.cachedOutdated.count)
+        }
+    }
+
+    /// Interposed between the end of an upgrade and its closing refetch;
+    /// passes straight through unless the user opted in.
+    func cleanupAfterUpgradeIfEnabled(completion: @escaping () -> Void) {
+        guard Settings.cleanupAfterUpgrade else {
+            completion()
+            return
+        }
+        statusMenuItem.title = "Cleaning up..."
+        runBrew("cleanup") { _, _ in completion() }
+    }
+
     @objc func upgradeAll() {
         if Settings.confirmBeforeUpgradeAll {
             guard confirmUpgrade() else { return }
@@ -259,21 +312,23 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusMenuItem.title = "Upgrading..."
 
-        runBrew("update") { _ in
+        runBrew("update") { _, _ in
             // packages that turn outdated only after this brew update are not
             // in cachedOutdated yet; the final refetch will surface them
             let caskNames = self.cachedOutdated.casks.map(\.name)
             let upgradeCasksThenRefresh = {
                 self.upgradeCasksSequentially(caskNames) {
+                    self.cleanupAfterUpgradeIfEnabled {
                         // 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
+                self.runBrew("upgrade --formula") { _, _ in
                     upgradeCasksThenRefresh()
                 }
             }
@@ -290,7 +345,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusMenuItem.title = "Upgrading \(next)..."
         // an explicitly named cask upgrades even when self-updating, no --greedy needed
-        runBrew("upgrade --cask \(next)", askpassMessage: "upgrade \(next)") { _ in
+        runBrew("upgrade --cask \(next)", askpassMessage: "upgrade \(next)") { _, _ in
             self.upgradeCasksSequentially(Array(names.dropFirst()), completion: completion)
         }
     }
@@ -328,7 +383,15 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         // --greedy also lists casks that self-update (brew skips them by default)
         let command = Settings.includeGreedyCasks ? "outdated --greedy --json" : "outdated --json"
 
-        runBrew(command) { output in
+        runBrew(command) { output, success in
+            // parsing a failure's output would yield .none and masquerade as
+            // "all up to date"; keep the cached state and its icon instead
+            guard success else {
+                self.statusMenuItem.title = "⚠️ Check failed — see Logs"
+                self.setMenuBarIcon(self.cachedOutdated.isEmpty ? "brewbar-uptodate" : "brewbar-outdated")
+                return
+            }
+
             let outdated = BrewParser.parseOutdatedJSON(output)
 
             self.cachedOutdated = outdated
@@ -382,12 +445,14 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         let caskNames = casks ? cachedOutdated.casks.map(\.name) : []
         let upgradeCasksThenRefresh = {
             self.upgradeCasksSequentially(caskNames) {
+                self.cleanupAfterUpgradeIfEnabled {
                     self.fetchOutdated(allowAutoUpgrade: false)
                 }
             }
+        }
 
         if formulae {
-            runBrew("upgrade --formula") { _ in
+            runBrew("upgrade --formula") { _, _ in
                 upgradeCasksThenRefresh()
             }
         } else {
@@ -426,22 +491,54 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     @objc func upgradeSingle(_ sender: NSMenuItem) {
         guard let formula = sender.representedObject as? String else { return }
 
-        runBrew("upgrade \(formula)", askpassMessage: "upgrade \(formula)") { _ in
+        runBrew("upgrade \(formula)", askpassMessage: "upgrade \(formula)") { _, _ in
+            self.cleanupAfterUpgradeIfEnabled {
                 self.refreshAll()
             }
         }
+    }
+
+    /// Refreshing offline would silently succeed on stale data: brew update
+    /// fails but its exit code was never checked, and brew outdated compares
+    /// against the local taps without touching the network — so the app
+    /// claimed "all up to date" no matter what. Every offline→online
+    /// transition refreshes, which also covers the refreshes skipped below.
+    func networkPathChanged(online: Bool) {
+        let wasOnline = isOnline
+        isOnline = online
+
+        guard online else {
+            statusMenuItem.title = "📡 Offline"
+            return
+        }
+        if !wasOnline {
+            refreshAll()
+        }
+    }
 
     func refreshAll() {
+        guard isOnline else {
+            statusMenuItem.title = "📡 Offline"
+            return
+        }
+
         statusMenuItem.title = "Updating..."
 
-        runBrew("update") { _ in
+        // NWPathMonitor can't see a network that is up but broken (captive
+        // portal, dead DNS, git host down); brew update's exit code can.
+        // Proceeding anyway would read stale local data and report a false OK.
+        runBrew("update") { _, success in
+            guard success else {
+                self.statusMenuItem.title = "⚠️ Check failed — see Logs"
+                return
+            }
             self.fetchOutdated() // ✅ ONLY place calling outdated
         }
     }
 
     /// ✅ Brew version (correct)
     func refreshBrewVersion() {
-        runBrew("--version") { output in
+        runBrew("--version") { output, _ in
             let firstLine = BrewParser.parseVersion(output)
             self.versionItem.title = "🏷 \(firstLine)"
         }

BrewBar/Settings.swift

@@ -44,4 +44,9 @@ enum Settings {
         get { UserDefaults.standard.bool(forKey: "confirmBeforeUpgradeAll") }
         set { UserDefaults.standard.set(newValue, forKey: "confirmBeforeUpgradeAll") }
     }
+
+    static var cleanupAfterUpgrade: Bool {
+        get { UserDefaults.standard.bool(forKey: "cleanupAfterUpgrade") }
+        set { UserDefaults.standard.set(newValue, forKey: "cleanupAfterUpgrade") }
+    }
 }

BrewBar/SettingsWindowController.swift

@@ -22,6 +22,7 @@ class SettingsWindowController: NSWindowController {
     var notifyCheckbox: NSButton!
     var showCountCheckbox: NSButton!
     var confirmCheckbox: NSButton!
+    var cleanupCheckbox: NSButton!
 
     convenience init(appDelegate: AppDelegate) {
         let window = NSWindow(
@@ -125,6 +126,12 @@ class SettingsWindowController: NSWindowController {
             action: #selector(toggleConfirm(_:))
         )
 
+        cleanupCheckbox = NSButton(
+            checkboxWithTitle: "Run brew cleanup after upgrades",
+            target: self,
+            action: #selector(toggleCleanup(_:))
+        )
+
         let stack = NSStackView(views: [
             frequencyRow,
             customRow,
@@ -138,6 +145,7 @@ class SettingsWindowController: NSWindowController {
             notifyCheckbox,
             showCountCheckbox,
             confirmCheckbox,
+            cleanupCheckbox,
             launchAtLoginCheckbox,
         ])
         stack.orientation = .vertical
@@ -179,6 +187,7 @@ class SettingsWindowController: NSWindowController {
         notifyCheckbox.state = Settings.notifyOnNewUpdates ? .on : .off
         showCountCheckbox.state = Settings.showCountInMenuBar ? .on : .off
         confirmCheckbox.state = Settings.confirmBeforeUpgradeAll ? .on : .off
+        cleanupCheckbox.state = Settings.cleanupAfterUpgrade ? .on : .off
     }
 
     func separator() -> NSBox {
@@ -311,6 +320,10 @@ class SettingsWindowController: NSWindowController {
         Settings.confirmBeforeUpgradeAll = sender.state == .on
     }
 
+    @objc func toggleCleanup(_ sender: NSButton) {
+        Settings.cleanupAfterUpgrade = sender.state == .on
+    }
+
     @objc func toggleLaunchAtLogin(_ sender: NSButton) {
         do {
             if sender.state == .on {