Retry transient Sparkle download failures

.github/swift-file-length-budget.tsv

@@ -2,7 +2,7 @@
 # Format: max_lines<TAB>relative path
 # Reduce counts as files shrink. CI fails if tracked files exceed this budget.
 34499	CLI/cmux.swift
-17954	Sources/AppDelegate.swift
+17952	Sources/AppDelegate.swift
 16427	Sources/ContentView.swift
 14270	Sources/TerminalController.swift
 13172	Sources/Workspace.swift
@@ -24,8 +24,8 @@
 4483	Sources/cmuxApp.swift
 4482	Sources/Panels/FilePreviewPanel.swift
 4367	cmuxTests/BrowserPanelTests.swift
-4283	Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
 4121	Sources/BrowserWindowPortal.swift
+3964	Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
 3934	Sources/Feed/FeedPanelView.swift
 3926	cmuxTests/TabManagerUnitTests.swift
 3896	cmuxTests/WindowAndDragTests.swift

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/AttemptUpdateCoordinator.swift

@@ -66,6 +66,22 @@ struct AttemptUpdateCoordinator {
         }
     }
 
+    /// Arm the coordinator to auto-confirm the update a freshly *restarted* retry check resolves,
+    /// entering ``Phase/awaitingResult`` directly rather than ``Phase/awaitingCheckRestart``.
+    ///
+    /// Used by the download-phase transient-retry re-arm (issue #5632). Unlike
+    /// ``requestInstallLatest(currentState:)`` — invoked while a stale pre-request `.updateAvailable`
+    /// prompt may still be on screen, which `awaitingCheckRestart` exists to ignore until a real
+    /// check restarts — this arm happens while the model is the retry's own `.checking` pill, so
+    /// there is no stale prompt to gate against. Entering `awaitingResult` directly means that if the
+    /// user cancels the retry before the check restarts (idling the model, e.g. during the
+    /// controller's readiness wait), the coordinator disarms via the `awaitingResult` idle → inactive
+    /// path instead of misreading the idle as check-restart progress — which would otherwise strand
+    /// it armed to silently auto-confirm a later, unrelated update.
+    mutating func armForConfirmedRetryCheck() {
+        phase = .awaitingResult
+    }
+
     /// Feed each ``UpdateState`` change observed after a request. Returns the action the controller
     /// should perform.
     mutating func handleStateChange(_ state: UpdateState) -> Action {

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateActionDelegate.swift

@@ -3,12 +3,17 @@
 /// This is the dependency-inversion seam between the `CmuxUpdater` package and the app: the
 /// package calls up through this protocol instead of reaching `AppDelegate`/`TerminalController`
 /// directly. The app's delegate conforms and is injected into ``UpdateController`` (which holds
-/// it `weak`).
+/// it `weak`). The protocol is main-actor isolated because Sparkle callbacks and cmux update UI
+/// state are both main-actor owned.
 @MainActor
 public protocol UpdateActionDelegate: AnyObject {
     /// The user asked to retry after an update error. The host should re-initiate a check
     /// through its normal entry point.
-    func updaterRequestsRetryCheckForUpdates()
+    ///
+    /// - Parameter preservingInstallIntent: Whether the interrupted Sparkle session had already
+    ///   moved past update selection into an install/download phase and the retry should
+    ///   auto-confirm the same update after Sparkle finds it again.
+    func updaterRequestsRetryCheckForUpdates(preservingInstallIntent: Bool)
 
     /// Sparkle is about to relaunch the app to finish installing. The host should persist
     /// session state, stop its terminal/runtime, and invalidate restorable state so the

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+BundleReleaseChannel.swift

@@ -0,0 +1,29 @@
+import Foundation
+
+extension UpdateController {
+    /// Which release channel a bundle identifier belongs to, decided from the identifier alone so it
+    /// stays unit-testable, mirroring ``TransientRetryPlan``.
+    ///
+    /// cmux DEV (`com.cmuxterm.app.debug[.<tag>]`) and staging (`com.cmuxterm.app.staging[.<tag>]`)
+    /// builds are produced from local source and are not on the public release train, so they must
+    /// never be compared against the public Sparkle appcast (#6292).
+    ///
+    /// Mirrors `SocketControlSettings.isDebugLikeBundleIdentifier` + `isStagingBundleIdentifier` (in
+    /// the CmuxSettings package). The classification is duplicated here deliberately to avoid
+    /// introducing a `CmuxUpdater → CmuxSettings` package dependency edge for a small string check.
+    enum BundleReleaseChannel: Equatable {
+        /// A DEV or staging build produced from local source; never query the public appcast.
+        case devLike
+        /// A public release-train build; the appcast applies.
+        case release
+
+        nonisolated init(bundleIdentifier: String?) {
+            guard let bundleIdentifier else { self = .release; return }
+            let isDevLike = bundleIdentifier == "com.cmuxterm.app.debug"
+                || bundleIdentifier.hasPrefix("com.cmuxterm.app.debug.")
+                || bundleIdentifier == "com.cmuxterm.app.staging"
+                || bundleIdentifier.hasPrefix("com.cmuxterm.app.staging.")
+            self = isDevLike ? .devLike : .release
+        }
+    }
+}

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+ReadinessWaitDecision.swift

@@ -0,0 +1,27 @@
+extension UpdateController {
+    /// Whether the bounded readiness wait should keep polling for `canCheckForUpdates`, decided from
+    /// the model state observed at the top of each poll iteration.
+    ///
+    /// Kept as a pure value + decision — separate from the controller's effectful readiness wait — so
+    /// it is unit-testable without the live `SPUUpdater` the controller owns, mirroring
+    /// ``TransientRetryPlan``.
+    ///
+    /// The wait is entered on behalf of a pending check and must keep polling for readiness for any
+    /// pending state — a `.checking` placeholder (a plain manual check or a transient-retry pill) or
+    /// the `.updateAvailable` prompt that `attemptUpdate()` re-resolves to the latest version
+    /// (issue #6366). It stops only when the model has returned to `.idle`, the signal that the user
+    /// cancelled the pending check, so readiness arriving later cannot resurrect a dismissed check.
+    /// Stopping on every non-`.checking` state instead stranded `attemptUpdate()`'s install whenever
+    /// Sparkle was briefly not ready, leaving the model `.updateAvailable` so no fresh check ran
+    /// (autoreview follow-up to issue #5632).
+    enum ReadinessWaitDecision: Equatable {
+        /// A pending check still awaits readiness; keep polling `canCheckForUpdates`.
+        case keepPolling
+        /// The pending check was cancelled back to `.idle`; stop the wait.
+        case stop
+
+        nonisolated init(modelState state: UpdateState) {
+            self = state.isIdle ? .stop : .keepPolling
+        }
+    }
+}

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+TransientRetry.swift

@@ -0,0 +1,40 @@
+extension UpdateController {
+    /// How ``retryAfterTransientFailure(preservingInstallIntent:)`` should react to a transient
+    /// Sparkle download failure.
+    ///
+    /// Kept as a pure value + decision — separate from the controller's effectful retry method — so
+    /// it is unit-testable without the live `SPUUpdater` the controller owns, mirroring how
+    /// ``AttemptUpdateCoordinator`` isolates the install re-resolution policy.
+    enum TransientRetryPlan: Equatable {
+        /// The attempt coordinator is already sequencing an install; restart its check while
+        /// preserving the in-flight install intent so it still auto-confirms.
+        case restartMonitoredCheck
+        /// The failure landed in a Sparkle download/extract/install phase, but the coordinator was
+        /// not yet monitoring (the interrupted session was Sparkle's own, not a coordinator
+        /// re-resolve). Re-arm the coordinator so the retried check auto-confirms the update it
+        /// finds; otherwise the retry surfaces a fresh prompt and the interrupted install is
+        /// silently stranded.
+        case rearmConfirmedInstall
+        /// A plain transient retry with no install intent to preserve; run an ordinary fresh check.
+        case plainCheck
+
+        /// Pure decision for ``UpdateController/retryAfterTransientFailure(preservingInstallIntent:)``,
+        /// derived from the retry context alone so it stays unit-testable without the live
+        /// `SPUUpdater` the controller owns.
+        ///
+        /// A retry while the attempt coordinator is already monitoring just restarts its in-flight
+        /// check (intent preserved by the coordinator itself). A preserved-intent retry that arrives
+        /// *before* the coordinator is monitoring must re-arm it so the retried check auto-confirms
+        /// the update it resolves — the case that previously stranded the interrupted install by
+        /// falling through to a bare check that only surfaced a prompt (issue #5632).
+        nonisolated init(preservingInstallIntent: Bool, coordinatorIsMonitoring: Bool) {
+            if coordinatorIsMonitoring {
+                self = .restartMonitoredCheck
+            } else if preservingInstallIntent {
+                self = .rearmConfirmedInstall
+            } else {
+                self = .plainCheck
+            }
+        }
+    }
+}

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController.swift

@@ -26,7 +26,7 @@ public final class UpdateController {
     private let hostBundle: Bundle
     private let backgroundProbeInterval: TimeInterval
     /// Whether the running build is a cmux DEV/staging build that must never be compared against
-    /// the public release appcast. See ``isDevLikeBundleIdentifier(_:)``.
+    /// the public release appcast. See ``BundleReleaseChannel``.
     private let isDevLikeBundle: Bool
 
     /// Host actions the updater delegates upward (retry, relaunch prep). Forwarded to the driver.
@@ -68,7 +68,7 @@ public final class UpdateController {
     ///   - fileManager: Filesystem access for the Sparkle installation-cache workaround;
     ///     injectable so tests can avoid touching the real filesystem.
     ///   - isDevLikeBundle: Overrides whether this is a DEV/staging build. Defaults to `nil`,
-    ///     which derives it from `hostBundle.bundleIdentifier` via ``isDevLikeBundleIdentifier(_:)``.
+    ///     which derives it from `hostBundle.bundleIdentifier` via ``BundleReleaseChannel``.
     ///     Injectable because a `Bundle` with an arbitrary identifier cannot be constructed in tests.
     public init(log: any UpdateLogging,
                 clock: any UpdateClock = SystemUpdateClock(),
@@ -83,7 +83,7 @@ public final class UpdateController {
         self.fileManager = fileManager
         self.hostBundle = hostBundle
         self.backgroundProbeInterval = settings.scheduledCheckInterval
-        let isDevLikeBundle = isDevLikeBundle ?? Self.isDevLikeBundleIdentifier(hostBundle.bundleIdentifier)
+        let isDevLikeBundle = isDevLikeBundle ?? (BundleReleaseChannel(bundleIdentifier: hostBundle.bundleIdentifier) == .devLike)
         self.isDevLikeBundle = isDevLikeBundle
         settings.apply(to: defaults)
         if isDevLikeBundle {
@@ -202,16 +202,79 @@ public final class UpdateController {
         checkForUpdatesWhenReady()
     }
 
-    private func performCheckForUpdates() {
+    /// Retry after a transient Sparkle download failure. Unlike a user-started fresh check, this
+    /// preserves any in-progress install/attempt intent so a retried archive download continues
+    /// silently after Sparkle finds the same update again.
+    ///
+    /// The reaction is chosen by the pure ``TransientRetryPlan/init(preservingInstallIntent:coordinatorIsMonitoring:)``:
+    /// restart the coordinator's already-monitored check, re-arm the coordinator so the retried
+    /// check auto-confirms (when the interrupted phase carried install intent but the coordinator
+    /// was not yet monitoring — issue #6366), or run a plain fresh check.
+    ///
+    /// - Parameter preservingInstallIntent: Whether the retry request came from a Sparkle
+    ///   install/download phase whose update choice should be auto-confirmed again.
+    public func retryAfterTransientFailure(preservingInstallIntent: Bool = false) {
+        let plan = TransientRetryPlan(
+            preservingInstallIntent: preservingInstallIntent,
+            coordinatorIsMonitoring: attemptCoordinator.isMonitoring
+        )
+        log.append("retrying update after transient download failure (plan=\(plan))")
+        switch plan {
+        case .restartMonitoredCheck:
+            // Preserve the interrupted session's install intent so the retried check re-resolves and
+            // (the coordinator being already monitoring) auto-confirms the update it finds.
+            checkForUpdatesWhenReady(preservingInstallIntent: true)
+        case .rearmConfirmedInstall:
+            // Arm the attempt coordinator to auto-confirm the retried check's update, matching the
+            // pre-failure install intent — entering the coordinator's result-awaiting phase directly
+            // (see `armForConfirmedRetryCheck`) rather than via `requestInstallLatest`. The model
+            // here is the retry's own synthetic `.checking` pill, not a stale prompt, so a user
+            // cancel during the readiness wait disarms the coordinator instead of stranding it armed
+            // to silently auto-confirm a later, unrelated update. Route the check through the
+            // preserving path so the synthetic pill is not torn down and the bounded-retry count is
+            // preserved.
+            attemptCoordinator.armForConfirmedRetryCheck()
+            checkForUpdatesWhenReady(preservingInstallIntent: true)
+        case .plainCheck:
+            checkForUpdatesWhenReady(preservingInstallIntent: false)
+        }
+    }
+
+    private func performCheckForUpdates(preservingInstallIntent: Bool = false) {
         startUpdaterIfNeeded()
         ensureSparkleInstallationCache()
         // Cancel any pending deferred re-check on every path so a stale one can't fire a
         // duplicate checkForUpdates() after this new check starts.
         recheckTask?.cancel()
+        // A transient retry (preserved-intent or plain) only arrives from
+        // `retryAfterTransientFailure` after the driver's multi-second backoff, by which point
+        // Sparkle has already acknowledged and torn down the failed session at error time (see
+        // `showUpdaterError` → `acknowledgement()`). There is therefore no just-dismissed *live*
+        // session to coalesce with, so the non-idle teardown + 100ms delay below — which exists
+        // solely to let Sparkle finish aborting a session before an *immediate* re-check — does not
+        // apply. Routing a retry through it would also be wrong: the current non-idle state is then
+        // the driver's synthetic `.checking` backoff placeholder whose `cancel` aborts the retry and
+        // resets the bounded-retry failure count, so `cancelActiveStateForNewCheck()` would kill the
+        // very retry (flickering the pill to idle) and — on the readiness-delayed path, where this
+        // runs from `readyCheckTask` after the synchronous `isRestartingTransientRetry` window has
+        // closed — silently reset the `[1, 3, 8]` cap into an unbounded retry loop (autoreview P2).
+        // Start the fresh check directly instead.
+        //
+        // `canCheckForUpdates` was true at every caller (`checkForUpdatesWhenReady`,
+        // `waitForReadinessThenCheck`), so any live Sparkle session is already gone: a `.checking`
+        // here is only ever that synthetic placeholder, never a real in-flight check.
+        if preservingInstallIntent {
+            updater.checkForUpdates()
+            return
+        }
         if model.state == .idle {
             updater.checkForUpdates()
             return
         }
+        if case .checking = model.state {
+            updater.checkForUpdates()
+            return
+        }
 
         model.cancelActiveStateForNewCheck()
 
@@ -228,7 +291,7 @@ public final class UpdateController {
     }
 
     /// Check for updates once the updater reports it can.
-    private func checkForUpdatesWhenReady() {
+    private func checkForUpdatesWhenReady(preservingInstallIntent: Bool = false) {
         if isDevLikeBundle {
             // DEV/staging builds are not on the public release train. A manual check (menu,
             // custom UI, or attempt-and-install) must not query the public appcast or offer the
@@ -246,22 +309,24 @@ public final class UpdateController {
         let canCheck = updater.canCheckForUpdates
         log.append("checkForUpdatesWhenReady invoked (canCheck=\(canCheck))")
         if canCheck {
-            performCheckForUpdates()
+            performCheckForUpdates(preservingInstallIntent: preservingInstallIntent)
             return
         }
         if model.state.isIdle {
             model.setState(.checking(.init(cancel: {})))
         }
-        waitForReadinessThenCheck()
+        waitForReadinessThenCheck(preservingInstallIntent: preservingInstallIntent)
     }
 
-    private func waitForReadinessThenCheck() {
+    private func waitForReadinessThenCheck(preservingInstallIntent: Bool = false) {
         readyCheckTask = Task { @MainActor [weak self] in
             guard let self else { return }
             var remaining = self.readyRetryCount
             while remaining > 0 {
+                // Stop the wait when the pending check was cancelled; keep polling otherwise.
+                guard ReadinessWaitDecision(modelState: self.model.state) == .keepPolling else { return }
                 if self.updater.canCheckForUpdates {
-                    self.performCheckForUpdates()
+                    self.performCheckForUpdates(preservingInstallIntent: preservingInstallIntent)
                     return
                 }
                 remaining -= 1
@@ -342,7 +407,7 @@ public final class UpdateController {
         if isDevLikeBundle {
             // DEV/staging builds are not on the public release train; never probe the public
             // appcast (init also disables Sparkle's own scheduled checks). Tear down any probe a
-            // prior path may have started. See `isDevLikeBundleIdentifier(_:)` (#6292).
+            // prior path may have started. See `BundleReleaseChannel` (#6292).
             log.append("launch update probe skipped (dev/staging build)")
             backgroundProbeTask?.cancel()
             backgroundProbeTask = nil
@@ -427,23 +492,3 @@ public final class UpdateController {
         }
     }
 }
-
-extension UpdateController {
-    /// Whether `bundleIdentifier` is a cmux DEV (`com.cmuxterm.app.debug[.<tag>]`) or staging
-    /// (`com.cmuxterm.app.staging[.<tag>]`) build.
-    ///
-    /// Such builds are produced from local source and are not on the public release train, so
-    /// they must never be compared against the public Sparkle appcast (#6292).
-    ///
-    /// Mirrors `SocketControlSettings.isDebugLikeBundleIdentifier` +
-    /// `isStagingBundleIdentifier` (in the CmuxSettings package). The classification is
-    /// duplicated here deliberately to avoid introducing a `CmuxUpdater → CmuxSettings` package
-    /// dependency edge for a small string check.
-    static func isDevLikeBundleIdentifier(_ bundleIdentifier: String?) -> Bool {
-        guard let bundleIdentifier else { return false }
-        return bundleIdentifier == "com.cmuxterm.app.debug"
-            || bundleIdentifier.hasPrefix("com.cmuxterm.app.debug.")
-            || bundleIdentifier == "com.cmuxterm.app.staging"
-            || bundleIdentifier.hasPrefix("com.cmuxterm.app.staging.")
-    }
-}