From c93752a008b5a9a9a717c9e2bea4742d7efb4c00 Mon Sep 17 00:00:00 2001 From: Lukas <134181853+bo2themax@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:20:00 +0200 Subject: [PATCH] macOS: suppress restart tips for auto update --- macos/Sources/App/macOS/AppDelegate.swift | 2 +- .../Features/Update/UpdateController.swift | 78 +++++++++++++++++-- .../Features/Update/UpdateDelegate.swift | 10 +-- .../Features/Update/UpdateDriver.swift | 4 +- .../Features/Update/UpdatePopoverView.swift | 46 +---------- .../Features/Update/UpdateSimulator.swift | 19 ++--- .../Features/Update/UpdateViewModel.swift | 48 +++++++++--- macos/Tests/Update/UpdateStateTests.swift | 33 +++++--- macos/Tests/Update/UpdateViewModelTests.swift | 4 +- 9 files changed, 149 insertions(+), 95 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 5003e3d32..3aebbb2a8 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -376,7 +376,7 @@ class AppDelegate: NSObject, // If we've already accepted to install an update, then we don't need to // confirm quit. The user is already expecting the update to happen. - if updateController.isInstalling { + if updateController.shouldTerminateWithoutWarning { return .terminateNow } diff --git a/macos/Sources/Features/Update/UpdateController.swift b/macos/Sources/Features/Update/UpdateController.swift index af99b4e86..2b91854f2 100644 --- a/macos/Sources/Features/Update/UpdateController.swift +++ b/macos/Sources/Features/Update/UpdateController.swift @@ -1,6 +1,7 @@ import Sparkle import Cocoa import Combine +import SwiftUI /// Standard controller for managing Sparkle updates in Ghostty. /// @@ -15,13 +16,9 @@ class UpdateController { userDriver.viewModel } - /// True if we're installing an update. - var isInstalling: Bool { - if case .installing = viewModel.state { - return true - } else { - return false - } + /// True if we're installing an update triggered manually. + var shouldTerminateWithoutWarning: Bool { + viewModel.state.shouldTerminateWithoutWarning } /// Initialize a new update controller. @@ -69,6 +66,30 @@ class UpdateController { return } + if case let .installing(installing) = viewModel.state { + // If the update is already installed, we can't actually + // cancel it, and SPUUpdater.checkForUpdates will simply fail, + // so we just show an alert to remind the user to restart. + let alert = NSAlert() + alert.alertStyle = .informational + let accessoryView = NSHostingView( + rootView: InstallingAccessoryView(installing: installing) + .frame(width: 228, alignment: .leading) + ) + accessoryView.frame = .init(origin: .zero, size: accessoryView.fittingSize) + alert.accessoryView = accessoryView + alert.addButton(withTitle: "Restart Now") + alert.addButton(withTitle: "Restart Later") + .keyEquivalent = .init([KeyboardShortcut(.escape).key.character]) + switch alert.runModal() { + case .alertFirstButtonReturn: + viewModel.state.confirm() + default: + break + } + return + } + // If we're not idle then we need to cancel any prior state. viewModel.state.cancel() @@ -91,3 +112,46 @@ class UpdateController { return true } } + +private struct InstallingAccessoryView: View { + let installing: UpdateState.Installing + + var body: some View { + VStack(alignment: .leading, spacing: 16) { + VStack(alignment: .leading, spacing: 8) { + Text("Restart Required") + .font(.system(size: 13, weight: .semibold)) + + Text("The update is ready. Please restart the application to complete the installation.") + .font(.system(size: 11)) + .foregroundColor(.secondary) + .fixedSize(horizontal: false, vertical: true) + + if let item = installing.appcastItem, let releaseNotesURL = installing.releaseNotes?.url { + VStack(alignment: .leading, spacing: 4) { + Link(destination: releaseNotesURL) { + HStack(spacing: 6) { + Text("Version:") + .foregroundColor(.secondary) + .frame(width: 60, alignment: .trailing) + Text(item.displayVersionString) + } + .font(.system(size: 11)) + } + + if let date = item.date { + HStack(spacing: 6) { + Text("Released:") + .foregroundColor(.secondary) + .frame(width: 60, alignment: .trailing) + Text(date.formatted(date: .abbreviated, time: .omitted)) + } + .font(.system(size: 11)) + } + } + .textSelection(.enabled) + } + } + } + } +} diff --git a/macos/Sources/Features/Update/UpdateDelegate.swift b/macos/Sources/Features/Update/UpdateDelegate.swift index 2459b8dfd..fbd436d92 100644 --- a/macos/Sources/Features/Update/UpdateDelegate.swift +++ b/macos/Sources/Features/Update/UpdateDelegate.swift @@ -23,12 +23,12 @@ extension UpdateDriver: SPUUpdaterDelegate { /// delegate method on the responsible driver instead. func updater(_ updater: SPUUpdater, willInstallUpdateOnQuit item: SUAppcastItem, immediateInstallationBlock immediateInstallHandler: @escaping () -> Void) -> Bool { viewModel.state = .installing(.init( - isAutoUpdate: true, - retryTerminatingApplication: immediateInstallHandler, - dismiss: { [weak viewModel] in - viewModel?.state = .idle - } + appcastItem: item, + retryTerminatingApplication: immediateInstallHandler )) + AppDelegate.logger.info("Version: \(item.displayVersionString) installed silently, waiting for relaunch...") + // Even when hasUnobtrusiveTarget is false, we don't show the alert immediately. + // We wait until the user manually checks for updates or relaunches. return true } } diff --git a/macos/Sources/Features/Update/UpdateDriver.swift b/macos/Sources/Features/Update/UpdateDriver.swift index b46e751f3..57c8d56b5 100644 --- a/macos/Sources/Features/Update/UpdateDriver.swift +++ b/macos/Sources/Features/Update/UpdateDriver.swift @@ -171,10 +171,8 @@ class UpdateDriver: NSObject, SPUUserDriver { func showInstallingUpdate(withApplicationTerminated applicationTerminated: Bool, retryTerminatingApplication: @escaping () -> Void) { viewModel.state = .installing(.init( + appcastItem: nil, retryTerminatingApplication: retryTerminatingApplication, - dismiss: { [weak viewModel] in - viewModel?.state = .idle - } )) if !hasUnobtrusiveTarget { diff --git a/macos/Sources/Features/Update/UpdatePopoverView.swift b/macos/Sources/Features/Update/UpdatePopoverView.swift index aa4e822f3..b16cbeaea 100644 --- a/macos/Sources/Features/Update/UpdatePopoverView.swift +++ b/macos/Sources/Features/Update/UpdatePopoverView.swift @@ -35,11 +35,8 @@ struct UpdatePopoverView: View { case .extracting(let extracting): ExtractingView(extracting: extracting) - case .installing(let installing): - // This is only required when `installing.isAutoUpdate == true`, - // but we keep it anyway, just in case something unexpected - // happens during installing - InstallingView(installing: installing, dismiss: dismiss) + case .installing: + EmptyView() case .notFound(let notFound): NotFoundView(notFound: notFound, dismiss: dismiss) @@ -274,45 +271,6 @@ private struct ExtractingView: View { } } -private struct InstallingView: View { - let installing: UpdateState.Installing - let dismiss: DismissAction - - var body: some View { - VStack(alignment: .leading, spacing: 16) { - VStack(alignment: .leading, spacing: 8) { - Text("Restart Required") - .font(.system(size: 13, weight: .semibold)) - - Text("The update is ready. Please restart the application to complete the installation.") - .font(.system(size: 11)) - .foregroundColor(.secondary) - .fixedSize(horizontal: false, vertical: true) - } - - HStack { - Button("Restart Later") { - installing.dismiss() - dismiss() - } - .keyboardShortcut(.cancelAction) - .controlSize(.small) - - Spacer() - - Button("Restart Now") { - installing.retryTerminatingApplication() - dismiss() - } - .keyboardShortcut(.defaultAction) - .buttonStyle(.borderedProminent) - .controlSize(.small) - } - } - .padding(16) - } -} - private struct NotFoundView: View { let notFound: UpdateState.NotFound let dismiss: DismissAction diff --git a/macos/Sources/Features/Update/UpdateSimulator.swift b/macos/Sources/Features/Update/UpdateSimulator.swift index c893993e0..c9339c0e7 100644 --- a/macos/Sources/Features/Update/UpdateSimulator.swift +++ b/macos/Sources/Features/Update/UpdateSimulator.swift @@ -274,28 +274,19 @@ enum UpdateSimulator { } } - private func simulateInstalling(_ viewModel: UpdateViewModel) { + private func simulateInstalling(_ viewModel: UpdateViewModel, appcastItem: SUAppcastItem? = nil) { viewModel.state = .installing(.init( + appcastItem: appcastItem, retryTerminatingApplication: { print("Restart button clicked in simulator - resetting to idle") viewModel.state = .idle }, - dismiss: { - viewModel.state = .idle - } )) } private func simulateAutoUpdate(_ viewModel: UpdateViewModel) { - viewModel.state = .installing(.init( - isAutoUpdate: true, - retryTerminatingApplication: { - print("Restart button clicked in simulator - resetting to idle") - viewModel.state = .idle - }, - dismiss: { - viewModel.state = .idle - } - )) + let item = SUAppcastItem.empty() + item.setValue("x.x.x", forKey: "_displayVersionString") + simulateInstalling(viewModel, appcastItem: item) } } diff --git a/macos/Sources/Features/Update/UpdateViewModel.swift b/macos/Sources/Features/Update/UpdateViewModel.swift index 59046224b..f458ce6d6 100644 --- a/macos/Sources/Features/Update/UpdateViewModel.swift +++ b/macos/Sources/Features/Update/UpdateViewModel.swift @@ -30,8 +30,8 @@ class UpdateViewModel: ObservableObject { return "Downloading…" case .extracting(let extracting): return String(format: "Preparing: %.0f%%", extracting.progress * 100) - case .installing(let install): - return install.isAutoUpdate ? "Restart to Complete Update" : "Installing…" + case let .installing(install): + return install.appcastItem != nil ? "Restart to Complete Update" : "Installing…" case .notFound: return "No Updates Available" case .error(let err): @@ -93,7 +93,11 @@ class UpdateViewModel: ObservableObject { case .extracting: return "Extracting and preparing the update" case let .installing(install): - return install.isAutoUpdate ? "Restart to Complete Update" : "Installing update and preparing to restart" + if let item = install.appcastItem { + return "The update is ready. Version: \(item.displayVersionString)" + } else { + return "Installing update and preparing to restart" + } case .notFound: return "You are running the latest version" case .error: @@ -185,8 +189,22 @@ enum UpdateState: Equatable { case extracting(Extracting) case installing(Installing) + /// True if we're installing an update triggered manually. + var shouldTerminateWithoutWarning: Bool { + if case .installing(let installing) = self { + return installing.appcastItem == nil + } else { + return false + } + } + var isHidden: Bool { if case .idle = self { return true } + if case .installing(let installing) = self { + // Hide the update pill when installing is triggered by auto update. + // There will be an alert when users check the updates themselves. + return installing.appcastItem != nil + } return false } @@ -239,10 +257,17 @@ enum UpdateState: Equatable { /// Confirms or accepts the current update state. /// - For available updates: begins installation /// - For ready-to-install: proceeds with installation - func confirm() { + /// - For installing: suppress termination warnings and restart + mutating func confirm() { switch self { case .updateAvailable(let available): available.reply(.install) + case .installing(let installing): + // Remove appcastItem so we can restart without any other alerts. + var suppressTerminationWarnings = installing + suppressTerminationWarnings.appcastItem = nil + self = .installing(suppressTerminationWarnings) + installing.retryTerminatingApplication() default: break } @@ -266,8 +291,8 @@ enum UpdateState: Equatable { return lDown.progress == rDown.progress && lDown.expectedLength == rDown.expectedLength case (.extracting(let lExt), .extracting(let rExt)): return lExt.progress == rExt.progress - case (.installing(let lInstall), .installing(let rInstall)): - return lInstall.isAutoUpdate == rInstall.isAutoUpdate + case (.installing(let lhs), .installing(let rhs)): + return lhs.appcastItem?.displayVersionString == rhs.appcastItem?.displayVersionString default: return false } @@ -378,9 +403,14 @@ enum UpdateState: Equatable { } struct Installing { - /// True if this state is triggered by ``Ghostty/UpdateDriver/updater(_:willInstallUpdateOnQuit:immediateInstallationBlock:)`` - var isAutoUpdate = false + /// Non-nil if this state is triggered by auto update + var appcastItem: SUAppcastItem? let retryTerminatingApplication: () -> Void - let dismiss: () -> Void + + var releaseNotes: ReleaseNotes? { + guard let appcastItem else { return nil } + let currentCommit = Bundle.main.infoDictionary?["GhosttyCommit"] as? String + return ReleaseNotes(displayVersionString: appcastItem.displayVersionString, currentCommit: currentCommit) + } } } diff --git a/macos/Tests/Update/UpdateStateTests.swift b/macos/Tests/Update/UpdateStateTests.swift index 97fb5a9e7..dfb21ef24 100644 --- a/macos/Tests/Update/UpdateStateTests.swift +++ b/macos/Tests/Update/UpdateStateTests.swift @@ -25,11 +25,9 @@ struct UpdateStateTests { } @Test func testInstallingEquality() { - let state1: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {}, dismiss: {})) - let state2: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {}, dismiss: {})) + let state1: UpdateState = .installing(.init(retryTerminatingApplication: {})) + let state2: UpdateState = .installing(.init(retryTerminatingApplication: {})) #expect(state1 == state2) - let state3: UpdateState = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: {}, dismiss: {})) - #expect(state3 != state2) } @Test func testPermissionRequestEquality() { @@ -100,13 +98,28 @@ struct UpdateStateTests { // MARK: - isHidden Tests - @Test func testIsIdleTrue() { - let state: UpdateState = .idle - #expect(state.isHidden == true) + @Test( + arguments: [ + (UpdateState.idle, true), + (.installing(.init(appcastItem: .empty(), retryTerminatingApplication: {})), true), + (.checking(.init(cancel: {})), false), + (.installing(.init(retryTerminatingApplication: {})), false) + ] + ) + func testIsHidden(_ state: UpdateState, expected: Bool) { + #expect(state.isHidden == expected) } - @Test func testIsIdleFalse() { - let state: UpdateState = .checking(.init(cancel: {})) - #expect(state.isHidden == false) + // MARK: - shouldTerminateWithoutWarning Tests + + @Test( + arguments: [ + (UpdateState.idle, false), + (.installing(.init(appcastItem: .empty(), retryTerminatingApplication: {})), false), + (.installing(.init(retryTerminatingApplication: {})), true) + ] + ) + func testShouldTerminateWithoutWarning(_ state: UpdateState, expected: Bool) { + #expect(state.shouldTerminateWithoutWarning == expected) } } diff --git a/macos/Tests/Update/UpdateViewModelTests.swift b/macos/Tests/Update/UpdateViewModelTests.swift index 9b747f9ec..ee22007f2 100644 --- a/macos/Tests/Update/UpdateViewModelTests.swift +++ b/macos/Tests/Update/UpdateViewModelTests.swift @@ -52,9 +52,9 @@ struct UpdateViewModelTests { @Test func testInstallingText() { let viewModel = UpdateViewModel() - viewModel.state = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {}, dismiss: {})) + viewModel.state = .installing(.init(retryTerminatingApplication: {})) #expect(viewModel.text == "Installing…") - viewModel.state = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: {}, dismiss: {})) + viewModel.state = .installing(.init(appcastItem: .empty(), retryTerminatingApplication: {})) #expect(viewModel.text == "Restart to Complete Update") }