From 2a4961d2918b0e6449c69ceca2d04bc611538bce Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:14:16 +0200 Subject: [PATCH 01/14] Move settings into a SettingsStore and adopt Observation AppState handled login, polling, settings and repo selection. The persisted preferences and the repo selection helpers now live in a small SettingsStore, and the snapshot used by the Reset button is view state in SettingsView. Both classes use @Observable instead of ObservableObject; views read AppState from the environment and bind to the store with @Bindable. UpdaterService stays an ObservableObject because it bridges Sparkle's KVO property with Combine. No behavior change: same UserDefaults keys, same save points. Co-Authored-By: Claude Opus 5.5 --- ActionBar.xcodeproj/project.pbxproj | 4 + Sources/ActionBar/App/ActionBarApp.swift | 6 +- Sources/ActionBar/Models/AppState.swift | 170 ++++-------------- Sources/ActionBar/Models/SettingsStore.swift | 114 ++++++++++++ Sources/ActionBar/Views/MenuContentView.swift | 12 +- Sources/ActionBar/Views/SettingsView.swift | 45 ++--- Sources/ActionBar/Views/SetupView.swift | 8 +- 7 files changed, 194 insertions(+), 165 deletions(-) create mode 100644 Sources/ActionBar/Models/SettingsStore.swift diff --git a/ActionBar.xcodeproj/project.pbxproj b/ActionBar.xcodeproj/project.pbxproj index 8cb5b95..d653be0 100644 --- a/ActionBar.xcodeproj/project.pbxproj +++ b/ActionBar.xcodeproj/project.pbxproj @@ -29,6 +29,7 @@ A10001000000000000000019 /* UpdaterService.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000019 /* UpdaterService.swift */; }; A10001000000000000000021 /* GitHubModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000021 /* GitHubModels.swift */; }; A10001000000000000000022 /* Components.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000022 /* Components.swift */; }; + A10001000000000000000023 /* SettingsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000023 /* SettingsStore.swift */; }; /* End PBXBuildFile section */ @@ -56,6 +57,7 @@ A10002000000000000000019 /* UpdaterService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdaterService.swift; sourceTree = ""; }; A10002000000000000000021 /* GitHubModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitHubModels.swift; sourceTree = ""; }; A10002000000000000000022 /* Components.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Components.swift; sourceTree = ""; }; + A10002000000000000000023 /* SettingsStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsStore.swift; sourceTree = ""; }; A10002000000000000000020 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; /* End PBXFileReference section */ @@ -117,6 +119,7 @@ A10002000000000000000003 /* WorkflowRun.swift */, A10002000000000000000004 /* AppState.swift */, A10002000000000000000021 /* GitHubModels.swift */, + A10002000000000000000023 /* SettingsStore.swift */, ); path = Models; sourceTree = ""; @@ -256,6 +259,7 @@ A10001000000000000000017 /* SettingsView.swift in Sources */, A10001000000000000000021 /* GitHubModels.swift in Sources */, A10001000000000000000022 /* Components.swift in Sources */, + A10001000000000000000023 /* SettingsStore.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Sources/ActionBar/App/ActionBarApp.swift b/Sources/ActionBar/App/ActionBarApp.swift index 4aac3e8..51f92ef 100644 --- a/Sources/ActionBar/App/ActionBarApp.swift +++ b/Sources/ActionBar/App/ActionBarApp.swift @@ -3,13 +3,13 @@ import SwiftUI @main struct ActionBarApp: App { @NSApplicationDelegateAdaptor(AppDelegate.self) private var appDelegate - @StateObject private var appState = AppState() + @State private var appState = AppState() @StateObject private var updater = UpdaterService() var body: some Scene { MenuBarExtra { MenuContentView() - .environmentObject(appState) + .environment(appState) .environmentObject(updater) } label: { MenuBarLabel(appState: appState) @@ -20,7 +20,7 @@ struct ActionBarApp: App { } private struct MenuBarLabel: View { - @ObservedObject var appState: AppState + let appState: AppState var body: some View { Image(nsImage: MenuBarIcon.image(statusDotColor: statusDotColor)) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 17df531..689f8b7 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -1,58 +1,40 @@ import AppKit -import Combine +import Observation @MainActor -final class AppState: ObservableObject { - static let defaultPollingInterval: TimeInterval = 30 - static let defaultMaxRunsPerRepo = 5 - - @Published var isAuthenticated = KeychainService.loadToken() != nil - @Published var runs: [WorkflowRun] = [] - @Published var isLoading = false - @Published var errorMessage: String? - @Published var username: String? - @Published var deviceCode: DeviceCodeResponse? - @Published var isLoggingIn = false - @Published var selectedRepoNames: Set = [] - @Published var repoSelectionSnapshot: Set = [] - @Published var availableRepos: [Repository] = [] - @Published var isLoadingRepos = false - @Published var repoListIsCapped = false - @Published var showingSettings = false - @Published var needsInstallation = false - @Published var isCheckingInstallation = false - @Published var pollingInterval = AppState.defaultPollingInterval - @Published var workflowFilterMode: WorkflowFilterMode = .off - @Published var workflowFilterText = "" - @Published var maxRunsPerRepo = AppState.defaultMaxRunsPerRepo - @Published var includeRunsFromOtherUsers = false - @Published private(set) var overallStatus: RunStatus = .unknown +@Observable +final class AppState { + var isAuthenticated = KeychainService.loadToken() != nil + var runs: [WorkflowRun] = [] + var isLoading = false + var errorMessage: String? + var username: String? + var deviceCode: DeviceCodeResponse? + var isLoggingIn = false + var availableRepos: [Repository] = [] + var isLoadingRepos = false + var repoListIsCapped = false + var showingSettings = false + var needsInstallation = false + var isCheckingInstallation = false + private(set) var overallStatus: RunStatus = .unknown + + let settings: SettingsStore private let apiClient = GitHubAPIClient() private let oauthService = OAuthService() - private let defaults = UserDefaults.standard - private var pollTask: Task? - private var installCheckTask: Task? - private var oauthTask: Task? - private var previousRunStatuses: [Int: String] = [:] // run.id -> run.status - private var hasLoadedOnce = false - private var isFetchingRuns = false - - private enum DefaultsKey { - static let selectedRepos = "selectedRepositories" - static let pollingInterval = "pollingInterval" - static let workflowFilterMode = "workflowFilterMode" - static let workflowFilterText = "workflowFilterText" - static let maxRunsPerRepo = "maxRunsPerRepo" - static let includeRunsFromOtherUsers = "includeRunsFromOtherUsers" - } - - private var hasRepoConfiguration: Bool { - defaults.object(forKey: DefaultsKey.selectedRepos) != nil + @ObservationIgnored private var pollTask: Task? + @ObservationIgnored private var installCheckTask: Task? + @ObservationIgnored private var oauthTask: Task? + @ObservationIgnored private var previousRunStatuses: [Int: String] = [:] // run.id -> run.status + @ObservationIgnored private var hasLoadedOnce = false + @ObservationIgnored private var isFetchingRuns = false + + init(settings: SettingsStore? = nil) { + self.settings = settings ?? SettingsStore() } func start() { - loadSettings() guard let token = KeychainService.loadToken() else { return } isAuthenticated = true Task { @@ -128,24 +110,10 @@ final class AppState: ObservableObject { previousRunStatuses = [:] hasLoadedOnce = false errorMessage = nil - selectedRepoNames = [] availableRepos = [] repoListIsCapped = false showingSettings = false - workflowFilterMode = .off - workflowFilterText = "" - maxRunsPerRepo = Self.defaultMaxRunsPerRepo - includeRunsFromOtherUsers = false - // The polling interval is a device preference, so it survives signing out. - for key in [ - DefaultsKey.selectedRepos, - DefaultsKey.workflowFilterMode, - DefaultsKey.workflowFilterText, - DefaultsKey.maxRunsPerRepo, - DefaultsKey.includeRunsFromOtherUsers, - ] { - defaults.removeObject(forKey: key) - } + settings.resetAccountSettings() } // MARK: - GitHub App Installation @@ -223,8 +191,7 @@ final class AppState: ObservableObject { } func updatePollingInterval(_ interval: TimeInterval) { - pollingInterval = interval - defaults.set(pollingInterval, forKey: DefaultsKey.pollingInterval) + settings.pollingInterval = interval startPolling() } @@ -235,7 +202,7 @@ final class AppState: ObservableObject { while !Task.isCancelled { // Wait in short steps against a wall-clock deadline, so a refresh happens // promptly after the Mac wakes from sleep. - let deadline = Date().addingTimeInterval(pollingInterval) + let deadline = Date().addingTimeInterval(settings.pollingInterval) while Date() < deadline, !Task.isCancelled { try? await Task.sleep(for: .seconds(1)) } @@ -266,19 +233,19 @@ final class AppState: ObservableObject { errorMessage = nil do { - let usingFallback = !hasRepoConfiguration || selectedRepoNames.isEmpty - let repos = usingFallback ? nil : Array(selectedRepoNames) - let actor = includeRunsFromOtherUsers ? nil : username + let usingFallback = !settings.hasRepoConfiguration || settings.selectedRepoNames.isEmpty + let repos = usingFallback ? nil : Array(settings.selectedRepoNames) + let actor = settings.includeRunsFromOtherUsers ? nil : username let newRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) // With nothing selected yet, adopt the repos that have recent runs as the selection. if usingFallback && !newRuns.isEmpty { - selectedRepoNames = Set(newRuns.map(\.repositoryFullName)) - saveSelectedRepos() + settings.selectedRepoNames = Set(newRuns.map(\.repositoryFullName)) + settings.saveSelectedRepos() } - let workflowFilteredRuns = newRuns.filtered(by: workflowFilterMode, patterns: workflowFilterText) - let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: maxRunsPerRepo) + let workflowFilteredRuns = newRuns.filtered(by: settings.workflowFilterMode, patterns: settings.workflowFilterText) + let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: settings.maxRunsPerRepo) if hasLoadedOnce { notifyCompletedRuns(in: visibleRuns) @@ -311,66 +278,7 @@ final class AppState: ObservableObject { } } - // MARK: - Settings - - private func loadSettings() { - if let saved = defaults.stringArray(forKey: DefaultsKey.selectedRepos) { - selectedRepoNames = Set(saved) - } - - let savedInterval = defaults.double(forKey: DefaultsKey.pollingInterval) - pollingInterval = savedInterval > 0 ? savedInterval : Self.defaultPollingInterval - - if let modeRaw = defaults.string(forKey: DefaultsKey.workflowFilterMode), - let mode = WorkflowFilterMode(rawValue: modeRaw) { - workflowFilterMode = mode - } - workflowFilterText = defaults.string(forKey: DefaultsKey.workflowFilterText) ?? "" - - let savedMaxRuns = defaults.integer(forKey: DefaultsKey.maxRunsPerRepo) - maxRunsPerRepo = savedMaxRuns > 0 ? savedMaxRuns : Self.defaultMaxRunsPerRepo - - includeRunsFromOtherUsers = defaults.bool(forKey: DefaultsKey.includeRunsFromOtherUsers) - } - - /// Persists the settings edited in the settings panel. The polling interval is saved as soon as it changes. - func saveSettings() { - saveSelectedRepos() - defaults.set(workflowFilterMode.rawValue, forKey: DefaultsKey.workflowFilterMode) - defaults.set(workflowFilterText, forKey: DefaultsKey.workflowFilterText) - defaults.set(maxRunsPerRepo, forKey: DefaultsKey.maxRunsPerRepo) - defaults.set(includeRunsFromOtherUsers, forKey: DefaultsKey.includeRunsFromOtherUsers) - } - - private func saveSelectedRepos() { - defaults.set(Array(selectedRepoNames), forKey: DefaultsKey.selectedRepos) - } - - // MARK: - Repository Selection - - func snapshotSelectedRepos() { - repoSelectionSnapshot = Set(defaults.stringArray(forKey: DefaultsKey.selectedRepos) ?? []) - } - - func resetReposToSnapshot() { - selectedRepoNames = repoSelectionSnapshot - } - - func toggleRepo(_ repoName: String) { - if selectedRepoNames.contains(repoName) { - selectedRepoNames.remove(repoName) - } else { - selectedRepoNames.insert(repoName) - } - } - - func selectAllRepos(_ repoNames: [String]) { - selectedRepoNames = Set(repoNames) - } - - func deselectAllRepos() { - selectedRepoNames = [] - } + // MARK: - Repositories func fetchAvailableRepos(force: Bool = false) { guard let token = KeychainService.loadToken() else { return } diff --git a/Sources/ActionBar/Models/SettingsStore.swift b/Sources/ActionBar/Models/SettingsStore.swift new file mode 100644 index 0000000..789e3cf --- /dev/null +++ b/Sources/ActionBar/Models/SettingsStore.swift @@ -0,0 +1,114 @@ +import Foundation +import Observation + +/// The user's preferences, persisted in `UserDefaults`. +@MainActor +@Observable +final class SettingsStore { + static let defaultPollingInterval: TimeInterval = 30 + static let defaultMaxRunsPerRepo = 5 + + var selectedRepoNames: Set = [] + var pollingInterval = SettingsStore.defaultPollingInterval { + didSet { if !isLoading { defaults.set(pollingInterval, forKey: Key.pollingInterval) } } + } + var workflowFilterMode: WorkflowFilterMode = .off + var workflowFilterText = "" + var maxRunsPerRepo = SettingsStore.defaultMaxRunsPerRepo + var includeRunsFromOtherUsers = false + + @ObservationIgnored private let defaults: UserDefaults + /// `@Observable` runs `didSet` even for assignments in `init`; this keeps loading from writing back. + @ObservationIgnored private var isLoading = true + + private enum Key { + static let selectedRepos = "selectedRepositories" + static let pollingInterval = "pollingInterval" + static let workflowFilterMode = "workflowFilterMode" + static let workflowFilterText = "workflowFilterText" + static let maxRunsPerRepo = "maxRunsPerRepo" + static let includeRunsFromOtherUsers = "includeRunsFromOtherUsers" + } + + init(defaults: UserDefaults = .standard) { + self.defaults = defaults + + if let saved = defaults.stringArray(forKey: Key.selectedRepos) { + selectedRepoNames = Set(saved) + } + + let savedInterval = defaults.double(forKey: Key.pollingInterval) + pollingInterval = savedInterval > 0 ? savedInterval : Self.defaultPollingInterval + + if let modeRaw = defaults.string(forKey: Key.workflowFilterMode), + let mode = WorkflowFilterMode(rawValue: modeRaw) { + workflowFilterMode = mode + } + workflowFilterText = defaults.string(forKey: Key.workflowFilterText) ?? "" + + let savedMaxRuns = defaults.integer(forKey: Key.maxRunsPerRepo) + maxRunsPerRepo = savedMaxRuns > 0 ? savedMaxRuns : Self.defaultMaxRunsPerRepo + + includeRunsFromOtherUsers = defaults.bool(forKey: Key.includeRunsFromOtherUsers) + isLoading = false + } + + /// Whether a repository selection has ever been saved. + var hasRepoConfiguration: Bool { + defaults.object(forKey: Key.selectedRepos) != nil + } + + var savedSelectedRepoNames: Set { + Set(defaults.stringArray(forKey: Key.selectedRepos) ?? []) + } + + /// Persists the settings edited in the settings panel. The polling interval is saved as soon as it changes. + func save() { + saveSelectedRepos() + defaults.set(workflowFilterMode.rawValue, forKey: Key.workflowFilterMode) + defaults.set(workflowFilterText, forKey: Key.workflowFilterText) + defaults.set(maxRunsPerRepo, forKey: Key.maxRunsPerRepo) + defaults.set(includeRunsFromOtherUsers, forKey: Key.includeRunsFromOtherUsers) + } + + func saveSelectedRepos() { + defaults.set(Array(selectedRepoNames), forKey: Key.selectedRepos) + } + + /// Forgets everything tied to the signed-in account. + /// The polling interval is a device preference, so it survives signing out. + func resetAccountSettings() { + selectedRepoNames = [] + workflowFilterMode = .off + workflowFilterText = "" + maxRunsPerRepo = Self.defaultMaxRunsPerRepo + includeRunsFromOtherUsers = false + for key in [ + Key.selectedRepos, + Key.workflowFilterMode, + Key.workflowFilterText, + Key.maxRunsPerRepo, + Key.includeRunsFromOtherUsers, + ] { + defaults.removeObject(forKey: key) + } + } + + // MARK: - Repository Selection + + func toggleRepo(_ repoName: String) { + if selectedRepoNames.contains(repoName) { + selectedRepoNames.remove(repoName) + } else { + selectedRepoNames.insert(repoName) + } + } + + func selectAllRepos(_ repoNames: [String]) { + selectedRepoNames = Set(repoNames) + } + + func deselectAllRepos() { + selectedRepoNames = [] + } +} diff --git a/Sources/ActionBar/Views/MenuContentView.swift b/Sources/ActionBar/Views/MenuContentView.swift index f8d79ce..54d8204 100644 --- a/Sources/ActionBar/Views/MenuContentView.swift +++ b/Sources/ActionBar/Views/MenuContentView.swift @@ -1,7 +1,7 @@ import SwiftUI struct MenuContentView: View { - @EnvironmentObject private var appState: AppState + @Environment(AppState.self) private var appState @EnvironmentObject private var updater: UpdaterService @State private var now = Date() @State private var showAbout = false @@ -17,7 +17,7 @@ struct MenuContentView: View { private var authenticatedView: some View { VStack(spacing: 0) { if appState.showingSettings { - SettingsView() + SettingsView(settings: appState.settings) .frame(minHeight: 520) } else { header @@ -117,7 +117,7 @@ struct MenuContentView: View { WorkflowRowView( run: run, now: now, - showsTriggeringActor: appState.includeRunsFromOtherUsers + showsTriggeringActor: appState.settings.includeRunsFromOtherUsers ) if run.id != appState.runs.last?.id { @@ -201,7 +201,7 @@ struct MenuContentView: View { ), ] return MenuContentView() - .environmentObject(state) + .environment(state) .environmentObject(UpdaterService()) } @@ -210,7 +210,7 @@ struct MenuContentView: View { state.isAuthenticated = true state.username = "octocat" return MenuContentView() - .environmentObject(state) + .environment(state) .environmentObject(UpdaterService()) } @@ -220,6 +220,6 @@ struct MenuContentView: View { state.username = "octocat" state.isLoading = true return MenuContentView() - .environmentObject(state) + .environment(state) .environmentObject(UpdaterService()) } diff --git a/Sources/ActionBar/Views/SettingsView.swift b/Sources/ActionBar/Views/SettingsView.swift index 3ddbeae..ef781bb 100644 --- a/Sources/ActionBar/Views/SettingsView.swift +++ b/Sources/ActionBar/Views/SettingsView.swift @@ -1,9 +1,12 @@ import SwiftUI struct SettingsView: View { - @EnvironmentObject private var appState: AppState + @Bindable var settings: SettingsStore + + @Environment(AppState.self) private var appState @Environment(\.openURL) private var openURL @State private var searchText = "" + @State private var repoSelectionSnapshot: Set = [] @State private var showSelectAllWarning = false /// Selecting more repos than this at once asks for confirmation, since each one costs an API call per refresh. @@ -44,7 +47,7 @@ struct SettingsView: View { repositoryList } .onAppear { - appState.snapshotSelectedRepos() + repoSelectionSnapshot = settings.savedSelectedRepoNames } } @@ -54,7 +57,7 @@ struct SettingsView: View { .font(.headline) Spacer() Button("Done") { - appState.saveSettings() + settings.save() appState.showingSettings = false appState.refreshNow() } @@ -71,7 +74,7 @@ struct SettingsView: View { .font(.subheadline) Spacer() Picker("Polling interval", selection: Binding( - get: { appState.pollingInterval }, + get: { settings.pollingInterval }, set: { appState.updatePollingInterval($0) } )) { ForEach(Self.intervalOptions, id: \.value) { option in @@ -91,7 +94,7 @@ struct SettingsView: View { Text("Max runs per repo") .font(.subheadline) Spacer() - Picker("Max runs per repo", selection: $appState.maxRunsPerRepo) { + Picker("Max runs per repo", selection: $settings.maxRunsPerRepo) { ForEach(1...10, id: \.self) { n in Text("\(n)").tag(n) } @@ -109,7 +112,7 @@ struct SettingsView: View { private var otherUsersSection: some View { VStack(alignment: .leading, spacing: 4) { - Toggle("Include runs from other users", isOn: $appState.includeRunsFromOtherUsers) + Toggle("Include runs from other users", isOn: $settings.includeRunsFromOtherUsers) .toggleStyle(.checkbox) .font(.subheadline) Text("Show recent workflow runs from anyone in the selected repositories.") @@ -126,7 +129,7 @@ struct SettingsView: View { Text("Workflow filter") .font(.subheadline) Spacer() - Picker("Workflow filter", selection: $appState.workflowFilterMode) { + Picker("Workflow filter", selection: $settings.workflowFilterMode) { ForEach(WorkflowFilterMode.allCases, id: \.self) { mode in Text(mode.rawValue.capitalized).tag(mode) } @@ -137,13 +140,13 @@ struct SettingsView: View { .fixedSize() } - if appState.workflowFilterMode != .off { - TextField("e.g. deploy, CI, build", text: $appState.workflowFilterText) + if settings.workflowFilterMode != .off { + TextField("e.g. deploy, CI, build", text: $settings.workflowFilterText) .textFieldStyle(.roundedBorder) .font(.caption) VStack(alignment: .leading, spacing: 1) { - Text(appState.workflowFilterMode == .include + Text(settings.workflowFilterMode == .include ? "Only show workflows matching these names." : "Hide workflows matching these names.") Text("Comma-separated, case-insensitive substring match.") @@ -195,7 +198,7 @@ struct SettingsView: View { .font(.caption) HStack { - Text("\(appState.selectedRepoNames.count) selected") + Text("\(settings.selectedRepoNames.count) selected") .font(.caption) .foregroundStyle(.secondary) Spacer() @@ -203,21 +206,21 @@ struct SettingsView: View { if filteredRepos.count > Self.selectAllWarningThreshold { showSelectAllWarning = true } else { - appState.selectAllRepos(filteredRepos.map(\.fullName)) + settings.selectAllRepos(filteredRepos.map(\.fullName)) } } .textButtonStyle(.accentColor) Button("None") { showSelectAllWarning = false - appState.deselectAllRepos() + settings.deselectAllRepos() } .textButtonStyle(.accentColor) Button("Reset") { showSelectAllWarning = false - appState.resetReposToSnapshot() + settings.selectedRepoNames = repoSelectionSnapshot } .textButtonStyle(.accentColor) - .disabled(appState.selectedRepoNames == appState.repoSelectionSnapshot) + .disabled(settings.selectedRepoNames == repoSelectionSnapshot) } if appState.repoListIsCapped { @@ -245,7 +248,7 @@ struct SettingsView: View { .fixedSize(horizontal: false, vertical: true) HStack(spacing: 12) { Button("Select All Anyway") { - appState.selectAllRepos(filteredRepos.map(\.fullName)) + settings.selectAllRepos(filteredRepos.map(\.fullName)) showSelectAllWarning = false } .textButtonStyle(.accentColor, font: .caption2.bold()) @@ -265,7 +268,7 @@ struct SettingsView: View { ScrollView { LazyVStack(spacing: 0) { ForEach(filteredRepos) { repo in - repositoryRow(repo, isSelected: appState.selectedRepoNames.contains(repo.fullName)) + repositoryRow(repo, isSelected: settings.selectedRepoNames.contains(repo.fullName)) } } } @@ -275,7 +278,7 @@ struct SettingsView: View { private func repositoryRow(_ repo: Repository, isSelected: Bool) -> some View { Button { - appState.toggleRepo(repo.fullName) + settings.toggleRepo(repo.fullName) } label: { HStack(spacing: 8) { Image(systemName: isSelected ? "checkmark.circle.fill" : "circle") @@ -311,7 +314,7 @@ private extension View { Repository(id: 2, fullName: "owner/second-repo"), Repository(id: 3, fullName: "owner/third-repo"), ] - state.selectedRepoNames = ["owner/first-repo", "owner/third-repo"] - return SettingsView() - .environmentObject(state) + state.settings.selectedRepoNames = ["owner/first-repo", "owner/third-repo"] + return SettingsView(settings: state.settings) + .environment(state) } diff --git a/Sources/ActionBar/Views/SetupView.swift b/Sources/ActionBar/Views/SetupView.swift index a54297d..2d9a076 100644 --- a/Sources/ActionBar/Views/SetupView.swift +++ b/Sources/ActionBar/Views/SetupView.swift @@ -1,7 +1,7 @@ import SwiftUI struct SetupView: View { - @EnvironmentObject private var appState: AppState + @Environment(AppState.self) private var appState @Environment(\.openURL) private var openURL var body: some View { @@ -163,7 +163,7 @@ struct SetupView: View { #Preview("Login") { SetupView() - .environmentObject(AppState()) + .environment(AppState()) } #Preview("Device Code") { @@ -175,12 +175,12 @@ struct SetupView: View { interval: 5 ) return SetupView() - .environmentObject(state) + .environment(state) } #Preview("Installation Required") { let state = AppState() state.needsInstallation = true return SetupView() - .environmentObject(state) + .environment(state) } From d3a5b9f755811b481c378a9574f52fcaf2a4c640 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:14:55 +0200 Subject: [PATCH 02/14] Save settings as soon as they change Edits in the settings panel were applied in memory right away but only written to UserDefaults when Done was pressed, so closing the menu without Done left the app running with settings that a relaunch would lose. Each setting now saves itself when it changes, like the polling interval already did. Done just closes the panel and refreshes. Since a non-empty selection is now always saved, checking whether the selectedRepositories key exists is redundant; an empty selection alone decides whether to fall back to all repos. The Reset button still restores the selection from when the panel was opened. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 3 +- Sources/ActionBar/Models/SettingsStore.swift | 47 +++++++++----------- Sources/ActionBar/Views/SettingsView.swift | 3 +- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 689f8b7..fc97869 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -233,7 +233,7 @@ final class AppState { errorMessage = nil do { - let usingFallback = !settings.hasRepoConfiguration || settings.selectedRepoNames.isEmpty + let usingFallback = settings.selectedRepoNames.isEmpty let repos = usingFallback ? nil : Array(settings.selectedRepoNames) let actor = settings.includeRunsFromOtherUsers ? nil : username let newRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) @@ -241,7 +241,6 @@ final class AppState { // With nothing selected yet, adopt the repos that have recent runs as the selection. if usingFallback && !newRuns.isEmpty { settings.selectedRepoNames = Set(newRuns.map(\.repositoryFullName)) - settings.saveSelectedRepos() } let workflowFilteredRuns = newRuns.filtered(by: settings.workflowFilterMode, patterns: settings.workflowFilterText) diff --git a/Sources/ActionBar/Models/SettingsStore.swift b/Sources/ActionBar/Models/SettingsStore.swift index 789e3cf..b986b50 100644 --- a/Sources/ActionBar/Models/SettingsStore.swift +++ b/Sources/ActionBar/Models/SettingsStore.swift @@ -1,21 +1,31 @@ import Foundation import Observation -/// The user's preferences, persisted in `UserDefaults`. +/// The user's preferences, saved to `UserDefaults` as soon as they change. @MainActor @Observable final class SettingsStore { static let defaultPollingInterval: TimeInterval = 30 static let defaultMaxRunsPerRepo = 5 - var selectedRepoNames: Set = [] + var selectedRepoNames: Set = [] { + didSet { save(Array(selectedRepoNames), forKey: Key.selectedRepos) } + } var pollingInterval = SettingsStore.defaultPollingInterval { - didSet { if !isLoading { defaults.set(pollingInterval, forKey: Key.pollingInterval) } } + didSet { save(pollingInterval, forKey: Key.pollingInterval) } + } + var workflowFilterMode: WorkflowFilterMode = .off { + didSet { save(workflowFilterMode.rawValue, forKey: Key.workflowFilterMode) } + } + var workflowFilterText = "" { + didSet { save(workflowFilterText, forKey: Key.workflowFilterText) } + } + var maxRunsPerRepo = SettingsStore.defaultMaxRunsPerRepo { + didSet { save(maxRunsPerRepo, forKey: Key.maxRunsPerRepo) } + } + var includeRunsFromOtherUsers = false { + didSet { save(includeRunsFromOtherUsers, forKey: Key.includeRunsFromOtherUsers) } } - var workflowFilterMode: WorkflowFilterMode = .off - var workflowFilterText = "" - var maxRunsPerRepo = SettingsStore.defaultMaxRunsPerRepo - var includeRunsFromOtherUsers = false @ObservationIgnored private let defaults: UserDefaults /// `@Observable` runs `didSet` even for assignments in `init`; this keeps loading from writing back. @@ -53,26 +63,9 @@ final class SettingsStore { isLoading = false } - /// Whether a repository selection has ever been saved. - var hasRepoConfiguration: Bool { - defaults.object(forKey: Key.selectedRepos) != nil - } - - var savedSelectedRepoNames: Set { - Set(defaults.stringArray(forKey: Key.selectedRepos) ?? []) - } - - /// Persists the settings edited in the settings panel. The polling interval is saved as soon as it changes. - func save() { - saveSelectedRepos() - defaults.set(workflowFilterMode.rawValue, forKey: Key.workflowFilterMode) - defaults.set(workflowFilterText, forKey: Key.workflowFilterText) - defaults.set(maxRunsPerRepo, forKey: Key.maxRunsPerRepo) - defaults.set(includeRunsFromOtherUsers, forKey: Key.includeRunsFromOtherUsers) - } - - func saveSelectedRepos() { - defaults.set(Array(selectedRepoNames), forKey: Key.selectedRepos) + private func save(_ value: Any, forKey key: String) { + guard !isLoading else { return } + defaults.set(value, forKey: key) } /// Forgets everything tied to the signed-in account. diff --git a/Sources/ActionBar/Views/SettingsView.swift b/Sources/ActionBar/Views/SettingsView.swift index ef781bb..78f541a 100644 --- a/Sources/ActionBar/Views/SettingsView.swift +++ b/Sources/ActionBar/Views/SettingsView.swift @@ -47,7 +47,7 @@ struct SettingsView: View { repositoryList } .onAppear { - repoSelectionSnapshot = settings.savedSelectedRepoNames + repoSelectionSnapshot = settings.selectedRepoNames } } @@ -57,7 +57,6 @@ struct SettingsView: View { .font(.headline) Spacer() Button("Done") { - settings.save() appState.showingSettings = false appState.refreshNow() } From 564e659aeca49feb7b844495d1ff44f8f296859d Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:15:20 +0200 Subject: [PATCH 03/14] Notify when runs that were waiting, pending or requested complete Completion notifications only fired for runs previously seen as in_progress or queued. RunStatus also treats waiting, pending and requested as queued, and runs in those states completed silently. The rule now uses RunStatus.isActive, and lives in [WorkflowRun].newlyCompleted(since:) so it can be tested. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 17 +++++------------ Sources/ActionBar/Models/WorkflowRun.swift | 7 +++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index fc97869..f65e508 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -26,7 +26,7 @@ final class AppState { @ObservationIgnored private var pollTask: Task? @ObservationIgnored private var installCheckTask: Task? @ObservationIgnored private var oauthTask: Task? - @ObservationIgnored private var previousRunStatuses: [Int: String] = [:] // run.id -> run.status + @ObservationIgnored private var previousRunStatuses: [Int: RunStatus] = [:] // run.id -> status @ObservationIgnored private var hasLoadedOnce = false @ObservationIgnored private var isFetchingRuns = false @@ -247,11 +247,13 @@ final class AppState { let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: settings.maxRunsPerRepo) if hasLoadedOnce { - notifyCompletedRuns(in: visibleRuns) + for run in visibleRuns.newlyCompleted(since: previousRunStatuses) { + NotificationService.sendCompletionNotification(run: run) + } } previousRunStatuses = [:] for run in visibleRuns { - previousRunStatuses[run.id] = run.status + previousRunStatuses[run.id] = RunStatus(from: run) } overallStatus = workflowFilteredRuns.overallStatus @@ -268,15 +270,6 @@ final class AppState { if manageLoading { isLoading = false } } - /// Notifies about runs that were queued or in progress on the previous fetch and have now completed. - private func notifyCompletedRuns(in newRuns: [WorkflowRun]) { - for run in newRuns where run.status == "completed" { - if let previousStatus = previousRunStatuses[run.id], ["in_progress", "queued"].contains(previousStatus) { - NotificationService.sendCompletionNotification(run: run) - } - } - } - // MARK: - Repositories func fetchAvailableRepos(force: Bool = false) { diff --git a/Sources/ActionBar/Models/WorkflowRun.swift b/Sources/ActionBar/Models/WorkflowRun.swift index 7d0736c..5e4beee 100644 --- a/Sources/ActionBar/Models/WorkflowRun.swift +++ b/Sources/ActionBar/Models/WorkflowRun.swift @@ -153,6 +153,13 @@ extension Array where Element == WorkflowRun { } } + /// Runs that have completed since they were last seen queued or in progress. + func newlyCompleted(since previousStatuses: [Int: RunStatus]) -> [WorkflowRun] { + filter { run in + run.status == "completed" && previousStatuses[run.id]?.isActive == true + } + } + /// Running if anything is in flight, otherwise the outcome of the most recently finished run. var overallStatus: RunStatus { if contains(where: { RunStatus(from: $0).isActive }) { From 4eecb4bf1d8750be4717bb5f24de7640836ce381 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:16:30 +0200 Subject: [PATCH 04/14] Surface errors from loading runs and the repository list A workflow runs response that failed to decode made fetchRunsForRepo return [] as if the repo had no runs. It now throws decodingError like the other requests. fetchRecentRuns still returns partial results when only some repos fail, but now reports which ones did, and the menu shows "Couldn't load runs for " (or "for N repositories", with the names in a tooltip) above the list. fetchAvailableRepos ignored errors entirely, leaving an empty or stale list with no explanation. It still keeps the previous list on failure, but settings now shows the error under the repository header. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 17 +++++++-- .../ActionBar/Services/GitHubAPIClient.swift | 36 ++++++++++++------- Sources/ActionBar/Views/MenuContentView.swift | 23 ++++++++++++ Sources/ActionBar/Views/SettingsView.swift | 8 +++++ 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index f65e508..170b906 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -6,6 +6,8 @@ import Observation final class AppState { var isAuthenticated = KeychainService.loadToken() != nil var runs: [WorkflowRun] = [] + /// Repositories whose runs failed to load on the last fetch, while others loaded fine. + var failedRepos: [String] = [] var isLoading = false var errorMessage: String? var username: String? @@ -14,6 +16,7 @@ final class AppState { var availableRepos: [Repository] = [] var isLoadingRepos = false var repoListIsCapped = false + var repoListError: String? var showingSettings = false var needsInstallation = false var isCheckingInstallation = false @@ -106,12 +109,14 @@ final class AppState { needsInstallation = false overallStatus = .unknown runs = [] + failedRepos = [] username = nil previousRunStatuses = [:] hasLoadedOnce = false errorMessage = nil availableRepos = [] repoListIsCapped = false + repoListError = nil showingSettings = false settings.resetAccountSettings() } @@ -236,7 +241,8 @@ final class AppState { let usingFallback = settings.selectedRepoNames.isEmpty let repos = usingFallback ? nil : Array(settings.selectedRepoNames) let actor = settings.includeRunsFromOtherUsers ? nil : username - let newRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) + let recentRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) + let newRuns = recentRuns.runs // With nothing selected yet, adopt the repos that have recent runs as the selection. if usingFallback && !newRuns.isEmpty { @@ -258,6 +264,7 @@ final class AppState { overallStatus = workflowFilteredRuns.overallStatus runs = visibleRuns + failedRepos = recentRuns.failedRepos hasLoadedOnce = true } catch GitHubAPIError.invalidToken { signOut() @@ -276,11 +283,15 @@ final class AppState { guard let token = KeychainService.loadToken() else { return } if !force && !availableRepos.isEmpty { return } isLoadingRepos = true + repoListError = nil Task { - // On failure, keep whatever list we had before. - if let repos = try? await apiClient.fetchAllRepos(token: token) { + do { + let repos = try await apiClient.fetchAllRepos(token: token) availableRepos = repos repoListIsCapped = repos.count >= GitHubAPIClient.maxRepositories + } catch { + // Keep whatever list we had before. + repoListError = error.localizedDescription } isLoadingRepos = false } diff --git a/Sources/ActionBar/Services/GitHubAPIClient.swift b/Sources/ActionBar/Services/GitHubAPIClient.swift index 781db91..a1459d2 100644 --- a/Sources/ActionBar/Services/GitHubAPIClient.swift +++ b/Sources/ActionBar/Services/GitHubAPIClient.swift @@ -23,6 +23,12 @@ enum GitHubAPIError: Error, LocalizedError { } } +/// Recent runs across repositories, plus the repositories whose runs couldn't be loaded. +struct RecentRuns { + let runs: [WorkflowRun] + let failedRepos: [String] +} + actor GitHubAPIClient { /// Upper bound on repositories listed for selection (and polled when nothing is selected). static let maxRepositories = 200 @@ -71,7 +77,7 @@ actor GitHubAPIClient { // MARK: - Workflow Runs - func fetchRecentRuns(token: String, actor: String? = nil, repos repoNames: [String]? = nil) async throws -> [WorkflowRun] { + func fetchRecentRuns(token: String, actor: String? = nil, repos repoNames: [String]? = nil) async throws -> RecentRuns { let repoFullNames: [String] if let repoNames { repoFullNames = repoNames @@ -79,12 +85,12 @@ actor GitHubAPIClient { repoFullNames = try await fetchAllRepos(token: token).map(\.fullName) } - let (allRuns, errors) = await withTaskGroup(of: Result<[WorkflowRun], Error>.self) { group in + let (allRuns, errors) = await withTaskGroup(of: (String, Result<[WorkflowRun], Error>).self) { group in var pendingRepos = repoFullNames.makeIterator() func enqueueNextRepo() { guard let repo = pendingRepos.next() else { return } - group.addTask { await self.fetchRunsResult(repo: repo, token: token, actor: actor) } + group.addTask { (repo, await self.fetchRunsResult(repo: repo, token: token, actor: actor)) } } for _ in 0.. $1.createdAt } - return Array(recentRuns.prefix(Self.maxRecentRuns)) + return RecentRuns( + runs: Array(recentRuns.prefix(Self.maxRecentRuns)), + failedRepos: errors.map(\.repo).sorted() + ) } private func fetchRunsResult(repo: String, token: String, actor: String?) async -> Result<[WorkflowRun], Error> { @@ -158,8 +167,11 @@ actor GitHubAPIClient { throw GitHubAPIError.invalidResponse(httpResponse.statusCode) } - guard let runsResponse = try? decoder.decode(WorkflowRunsResponse.self, from: data) else { - return [] + let runsResponse: WorkflowRunsResponse + do { + runsResponse = try decoder.decode(WorkflowRunsResponse.self, from: data) + } catch { + throw GitHubAPIError.decodingError(error) } if let cacheKey { diff --git a/Sources/ActionBar/Views/MenuContentView.swift b/Sources/ActionBar/Views/MenuContentView.swift index 54d8204..7f08635 100644 --- a/Sources/ActionBar/Views/MenuContentView.swift +++ b/Sources/ActionBar/Views/MenuContentView.swift @@ -22,6 +22,10 @@ struct MenuContentView: View { } else { header Divider() + if !appState.failedRepos.isEmpty && appState.errorMessage == nil { + failedReposWarning + Divider() + } content } @@ -110,6 +114,25 @@ struct MenuContentView: View { .padding(.vertical, 8) } + private var failedReposWarning: some View { + let repos = appState.failedRepos + return HStack(spacing: 6) { + Image(systemName: "exclamationmark.triangle.fill") + .foregroundStyle(.orange) + Text(repos.count == 1 + ? "Couldn't load runs for \(repos[0])" + : "Couldn't load runs for \(repos.count) repositories") + .lineLimit(1) + .truncationMode(.middle) + Spacer() + } + .font(.caption2) + .foregroundStyle(.secondary) + .help(repos.joined(separator: "\n")) + .padding(.horizontal, 16) + .padding(.vertical, 6) + } + private var runsList: some View { ScrollView { LazyVStack(spacing: 0) { diff --git a/Sources/ActionBar/Views/SettingsView.swift b/Sources/ActionBar/Views/SettingsView.swift index 78f541a..2ddf3ab 100644 --- a/Sources/ActionBar/Views/SettingsView.swift +++ b/Sources/ActionBar/Views/SettingsView.swift @@ -229,6 +229,14 @@ struct SettingsView: View { .frame(maxWidth: .infinity, alignment: .leading) } + if let error = appState.repoListError { + Text("Couldn't load repositories: \(error)") + .font(.caption2) + .foregroundStyle(.red) + .fixedSize(horizontal: false, vertical: true) + .frame(maxWidth: .infinity, alignment: .leading) + } + if showSelectAllWarning { selectAllWarning } From 26579a75e49cefffdc2c484f73d700bb5f1fcdca Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:16:47 +0200 Subject: [PATCH 05/14] Start the app state only once appState.start() runs from the MenuBarExtra label's onAppear, which can fire more than once. Each extra call validated the token again and started another monitoring pass, restarting polling. start() now ignores every call after the first. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/App/ActionBarApp.swift | 1 + Sources/ActionBar/Models/AppState.swift | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/Sources/ActionBar/App/ActionBarApp.swift b/Sources/ActionBar/App/ActionBarApp.swift index 51f92ef..fc50e7b 100644 --- a/Sources/ActionBar/App/ActionBarApp.swift +++ b/Sources/ActionBar/App/ActionBarApp.swift @@ -13,6 +13,7 @@ struct ActionBarApp: App { .environmentObject(updater) } label: { MenuBarLabel(appState: appState) + // onAppear can fire more than once; start() only runs the first time. .onAppear { appState.start() } } .menuBarExtraStyle(.window) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 170b906..97af07d 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -30,6 +30,7 @@ final class AppState { @ObservationIgnored private var installCheckTask: Task? @ObservationIgnored private var oauthTask: Task? @ObservationIgnored private var previousRunStatuses: [Int: RunStatus] = [:] // run.id -> status + @ObservationIgnored private var hasStarted = false @ObservationIgnored private var hasLoadedOnce = false @ObservationIgnored private var isFetchingRuns = false @@ -37,7 +38,10 @@ final class AppState { self.settings = settings ?? SettingsStore() } + /// Resumes the saved session. Safe to call more than once; only the first call does anything. func start() { + guard !hasStarted else { return } + hasStarted = true guard let token = KeychainService.loadToken() else { return } isAuthenticated = true Task { From d1538aaa2e6a8731702b7b6c9e2799d49caf03c7 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:19:01 +0200 Subject: [PATCH 06/14] Build in Swift 6 language mode Swift 6 turns on complete concurrency checking. The only thing it flagged was AppDelegate mutating NSPanel.collectionBehavior, which is main actor-isolated, from the Sendable block-based observer. AppDelegate is now explicitly @MainActor, and the window observer is a selector-based observer calling a main actor method (windows become key on the main thread). The UNUserNotificationCenterDelegate methods are nonisolated, as they effectively were before: UserNotifications doesn't promise to call them on the main thread, and an isolated conformance would trap at runtime if it didn't. Co-Authored-By: Claude Opus 5.5 --- ActionBar.xcodeproj/project.pbxproj | 4 ++-- Sources/ActionBar/App/AppDelegate.swift | 30 +++++++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/ActionBar.xcodeproj/project.pbxproj b/ActionBar.xcodeproj/project.pbxproj index d653be0..a53b82e 100644 --- a/ActionBar.xcodeproj/project.pbxproj +++ b/ActionBar.xcodeproj/project.pbxproj @@ -393,7 +393,7 @@ PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; SWIFT_EMIT_LOC_STRINGS = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; }; name = Debug; }; @@ -423,7 +423,7 @@ PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; SWIFT_EMIT_LOC_STRINGS = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; }; name = Release; }; diff --git a/Sources/ActionBar/App/AppDelegate.swift b/Sources/ActionBar/App/AppDelegate.swift index bd88645..a7c7fd6 100644 --- a/Sources/ActionBar/App/AppDelegate.swift +++ b/Sources/ActionBar/App/AppDelegate.swift @@ -1,26 +1,32 @@ import AppKit import UserNotifications +@MainActor final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCenterDelegate { - private var windowObserver: NSObjectProtocol? - func applicationDidFinishLaunching(_ notification: Notification) { UNUserNotificationCenter.current().delegate = self NotificationService.requestPermission() // Make the MenuBarExtra panel immune to "Show Desktop" / Exposé - windowObserver = NotificationCenter.default.addObserver( - forName: NSWindow.didBecomeKeyNotification, - object: nil, - queue: .main - ) { notification in - guard let panel = notification.object as? NSPanel, !(panel is NSOpenPanel) else { return } - panel.collectionBehavior.insert([.canJoinAllSpaces, .stationary, .fullScreenAuxiliary]) - } + NotificationCenter.default.addObserver( + self, + selector: #selector(windowDidBecomeKey(_:)), + name: NSWindow.didBecomeKeyNotification, + object: nil + ) } + // Windows become key on the main thread, so this is called there. + @objc private func windowDidBecomeKey(_ notification: Notification) { + guard let panel = notification.object as? NSPanel, !(panel is NSOpenPanel) else { return } + panel.collectionBehavior.insert([.canJoinAllSpaces, .stationary, .fullScreenAuxiliary]) + } + + // UserNotifications doesn't promise to call its delegate on the main thread, so these are + // nonisolated rather than inheriting the app delegate's main actor isolation. + // Show notifications even when the app is in the foreground - func userNotificationCenter( + nonisolated func userNotificationCenter( _ center: UNUserNotificationCenter, willPresent notification: UNNotification, withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void @@ -29,7 +35,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } // Open the workflow run in the browser when the notification is clicked - func userNotificationCenter( + nonisolated func userNotificationCenter( _ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void From 9cd8400afa8b4633efcb4b67d7b0716452b13c00 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Wed, 21 Oct 2026 22:23:08 +0200 Subject: [PATCH 07/14] Add an ActionBarTests unit test target and run it in CI Swift Testing coverage for the pure logic: - RunStatus(from:) for every status and conclusion, and isActive - [WorkflowRun].filtered(by:patterns:), limited(toRunsPerRepo:), overallStatus and the newlyCompleted(since:) notification rule - RelativeTime.format - Decoding the API and OAuth models with the app's decoder - SettingsStore loading, saving on change, and the sign-out reset The bundle has no test host. It compiles the model files it tests itself, so running the tests never launches the app. A launched, differently signed app could read the Keychain item, get errSecAuthFailed and delete the user's token. The JSON decoder setup moves to JSONDecoder.gitHubAPI() so the tests use the app's real configuration. The test target is in the shared scheme, CI runs it after the Release build, and there's a `make test` target. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 7 + ActionBar.xcodeproj/project.pbxproj | 106 ++++++++++++++ .../xcshareddata/xcschemes/ActionBar.xcscheme | 26 ++++ Makefile | 6 +- README.md | 2 + Sources/ActionBar/Models/GitHubModels.swift | 10 ++ .../ActionBar/Services/GitHubAPIClient.swift | 5 +- Tests/ActionBarTests/ModelDecodingTests.swift | 136 ++++++++++++++++++ Tests/ActionBarTests/RelativeTimeTests.swift | 41 ++++++ Tests/ActionBarTests/RunStatusTests.swift | 72 ++++++++++ Tests/ActionBarTests/SettingsStoreTests.swift | 97 +++++++++++++ Tests/ActionBarTests/TestRuns.swift | 24 ++++ .../ActionBarTests/WorkflowRunListTests.swift | 75 ++++++++++ 13 files changed, 602 insertions(+), 5 deletions(-) create mode 100644 Tests/ActionBarTests/ModelDecodingTests.swift create mode 100644 Tests/ActionBarTests/RelativeTimeTests.swift create mode 100644 Tests/ActionBarTests/RunStatusTests.swift create mode 100644 Tests/ActionBarTests/SettingsStoreTests.swift create mode 100644 Tests/ActionBarTests/TestRuns.swift create mode 100644 Tests/ActionBarTests/WorkflowRunListTests.swift diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5cbafbe..a5cc69d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,3 +17,10 @@ jobs: -destination 'generic/platform=macOS' \ CODE_SIGNING_ALLOWED=NO \ build + + - name: Test + run: | + xcodebuild -project ActionBar.xcodeproj -scheme ActionBar -configuration Debug \ + -destination 'platform=macOS' \ + CODE_SIGNING_ALLOWED=NO \ + test diff --git a/ActionBar.xcodeproj/project.pbxproj b/ActionBar.xcodeproj/project.pbxproj index a53b82e..5c6fbf4 100644 --- a/ActionBar.xcodeproj/project.pbxproj +++ b/ActionBar.xcodeproj/project.pbxproj @@ -30,6 +30,16 @@ A10001000000000000000021 /* GitHubModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000021 /* GitHubModels.swift */; }; A10001000000000000000022 /* Components.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000022 /* Components.swift */; }; A10001000000000000000023 /* SettingsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000023 /* SettingsStore.swift */; }; + A10001000000000000000030 /* TestRuns.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000030 /* TestRuns.swift */; }; + A10001000000000000000031 /* RunStatusTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000031 /* RunStatusTests.swift */; }; + A10001000000000000000032 /* WorkflowRunListTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000032 /* WorkflowRunListTests.swift */; }; + A10001000000000000000033 /* RelativeTimeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000033 /* RelativeTimeTests.swift */; }; + A10001000000000000000034 /* ModelDecodingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000034 /* ModelDecodingTests.swift */; }; + A10001000000000000000035 /* SettingsStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000035 /* SettingsStoreTests.swift */; }; + A10001000000000000000040 /* WorkflowRun.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000003 /* WorkflowRun.swift */; }; + A10001000000000000000041 /* GitHubModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000021 /* GitHubModels.swift */; }; + A10001000000000000000042 /* RelativeTime.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000014 /* RelativeTime.swift */; }; + A10001000000000000000043 /* SettingsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000023 /* SettingsStore.swift */; }; /* End PBXBuildFile section */ @@ -59,6 +69,13 @@ A10002000000000000000022 /* Components.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Components.swift; sourceTree = ""; }; A10002000000000000000023 /* SettingsStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsStore.swift; sourceTree = ""; }; A10002000000000000000020 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; + A10002000000000000000030 /* TestRuns.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestRuns.swift; sourceTree = ""; }; + A10002000000000000000031 /* RunStatusTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RunStatusTests.swift; sourceTree = ""; }; + A10002000000000000000032 /* WorkflowRunListTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkflowRunListTests.swift; sourceTree = ""; }; + A10002000000000000000033 /* RelativeTimeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelativeTimeTests.swift; sourceTree = ""; }; + A10002000000000000000034 /* ModelDecodingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelDecodingTests.swift; sourceTree = ""; }; + A10002000000000000000035 /* SettingsStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsStoreTests.swift; sourceTree = ""; }; + A10003000000000000000002 /* ActionBarTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = ActionBarTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -70,6 +87,13 @@ ); runOnlyForDeploymentPostprocessing = 0; }; + A10004000000000000000002 /* Frameworks */ = { + isa = PBXFrameworksBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + runOnlyForDeploymentPostprocessing = 0; + }; /* End PBXFrameworksBuildPhase section */ /* Begin PBXGroup section */ @@ -79,6 +103,7 @@ A10002000000000000000018 /* Base.xcconfig */, A10005000000000000000003 /* Sources */, A10005000000000000000008 /* Resources */, + A10005000000000000000010 /* ActionBarTests */, A10005000000000000000002 /* Products */, ); sourceTree = ""; @@ -87,6 +112,7 @@ isa = PBXGroup; children = ( A10003000000000000000001 /* ActionBar.app */, + A10003000000000000000002 /* ActionBarTests.xctest */, ); name = Products; sourceTree = ""; @@ -169,6 +195,19 @@ path = Utilities; sourceTree = ""; }; + A10005000000000000000010 /* ActionBarTests */ = { + isa = PBXGroup; + children = ( + A10002000000000000000030 /* TestRuns.swift */, + A10002000000000000000031 /* RunStatusTests.swift */, + A10002000000000000000032 /* WorkflowRunListTests.swift */, + A10002000000000000000033 /* RelativeTimeTests.swift */, + A10002000000000000000034 /* ModelDecodingTests.swift */, + A10002000000000000000035 /* SettingsStoreTests.swift */, + ); + path = Tests/ActionBarTests; + sourceTree = ""; + }; /* End PBXGroup section */ /* Begin PBXNativeTarget section */ @@ -192,6 +231,22 @@ productReference = A10003000000000000000001 /* ActionBar.app */; productType = "com.apple.product-type.application"; }; + A10006000000000000000002 /* ActionBarTests */ = { + isa = PBXNativeTarget; + buildConfigurationList = A10008000000000000000004 /* Build configuration list for PBXNativeTarget "ActionBarTests" */; + buildPhases = ( + A10007000000000000000003 /* Sources */, + A10004000000000000000002 /* Frameworks */, + ); + buildRules = ( + ); + dependencies = ( + ); + name = ActionBarTests; + productName = ActionBarTests; + productReference = A10003000000000000000002 /* ActionBarTests.xctest */; + productType = "com.apple.product-type.bundle.unit-test"; + }; /* End PBXNativeTarget section */ /* Begin PBXProject section */ @@ -219,6 +274,7 @@ projectRoot = ""; targets = ( A10006000000000000000001 /* ActionBar */, + A10006000000000000000002 /* ActionBarTests */, ); }; /* End PBXProject section */ @@ -263,6 +319,23 @@ ); runOnlyForDeploymentPostprocessing = 0; }; + A10007000000000000000003 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + A10001000000000000000030 /* TestRuns.swift in Sources */, + A10001000000000000000031 /* RunStatusTests.swift in Sources */, + A10001000000000000000032 /* WorkflowRunListTests.swift in Sources */, + A10001000000000000000033 /* RelativeTimeTests.swift in Sources */, + A10001000000000000000034 /* ModelDecodingTests.swift in Sources */, + A10001000000000000000035 /* SettingsStoreTests.swift in Sources */, + A10001000000000000000040 /* WorkflowRun.swift in Sources */, + A10001000000000000000041 /* GitHubModels.swift in Sources */, + A10001000000000000000042 /* RelativeTime.swift in Sources */, + A10001000000000000000043 /* SettingsStore.swift in Sources */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; /* End PBXSourcesBuildPhase section */ /* Begin XCBuildConfiguration section */ @@ -427,6 +500,30 @@ }; name = Release; }; + A1000A000000000000000005 /* Debug */ = { + isa = XCBuildConfiguration; + buildSettings = { + CODE_SIGN_IDENTITY = "-"; + CODE_SIGN_STYLE = Manual; + GENERATE_INFOPLIST_FILE = YES; + PRODUCT_BUNDLE_IDENTIFIER = com.ptrchm.actionbar.tests; + PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_VERSION = 6.0; + }; + name = Debug; + }; + A1000A000000000000000006 /* Release */ = { + isa = XCBuildConfiguration; + buildSettings = { + CODE_SIGN_IDENTITY = "-"; + CODE_SIGN_STYLE = Manual; + GENERATE_INFOPLIST_FILE = YES; + PRODUCT_BUNDLE_IDENTIFIER = com.ptrchm.actionbar.tests; + PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_VERSION = 6.0; + }; + name = Release; + }; /* End XCBuildConfiguration section */ /* Begin XCConfigurationList section */ @@ -448,6 +545,15 @@ defaultConfigurationIsVisible = 0; defaultConfigurationName = Release; }; + A10008000000000000000004 /* Build configuration list for PBXNativeTarget "ActionBarTests" */ = { + isa = XCConfigurationList; + buildConfigurations = ( + A1000A000000000000000005 /* Debug */, + A1000A000000000000000006 /* Release */, + ); + defaultConfigurationIsVisible = 0; + defaultConfigurationName = Release; + }; /* End XCConfigurationList section */ /* Begin XCRemoteSwiftPackageReference section */ diff --git a/ActionBar.xcodeproj/xcshareddata/xcschemes/ActionBar.xcscheme b/ActionBar.xcodeproj/xcshareddata/xcschemes/ActionBar.xcscheme index 8c13473..406acb5 100644 --- a/ActionBar.xcodeproj/xcshareddata/xcschemes/ActionBar.xcscheme +++ b/ActionBar.xcodeproj/xcshareddata/xcschemes/ActionBar.xcscheme @@ -21,6 +21,20 @@ ReferencedContainer = "container:ActionBar.xcodeproj"> + + + + + + + + + + /dev/null | grep -m1 '^\s*BUILT_PRODUCTS_DIR' | sed 's/.*= //') APP_BUNDLE = $(BUILD_DIR)/$(APP_NAME).app -.PHONY: build run clean +.PHONY: build run test clean build: xcodebuild -project $(PROJECT) -scheme $(SCHEME) -configuration Release build @@ -14,6 +14,10 @@ run: build sleep 0.5 open "$(APP_BUNDLE)" +# The tests don't launch the app, so they never touch the Keychain. +test: + xcodebuild -project $(PROJECT) -scheme $(SCHEME) -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO test + clean: xcodebuild -project $(PROJECT) -scheme $(SCHEME) clean rm -rf ~/Library/Developer/Xcode/DerivedData/$(APP_NAME)-* diff --git a/README.md b/README.md index eced5a9..930c5f3 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,8 @@ Requirements: macOS 14+ and a recent version of Xcode. 2. Copy `Config/Local.xcconfig.example` to `Config/Local.xcconfig` and set your Apple Developer Team ID and a bundle identifier registered to your team. 3. Open `ActionBar.xcodeproj` and run the `ActionBar` scheme, or use `make run`. +Run the unit tests with `make test` or the scheme's Test action. They cover the app's model logic and don't launch the app. + The app authenticates through the official ActionBar GitHub App. If you're shipping your own fork, register your own [GitHub App](https://docs.github.com/en/apps/creating-github-apps) with device flow enabled and **Actions (read)** permission, then update `GitHubApp.clientID` and `GitHubApp.installationURL` in `Sources/ActionBar/Services/OAuthService.swift`. You'll also need your own Sparkle key pair and feed URL in `Resources/Info.plist` (see below). ## Releasing diff --git a/Sources/ActionBar/Models/GitHubModels.swift b/Sources/ActionBar/Models/GitHubModels.swift index add7c04..11ab4c9 100644 --- a/Sources/ActionBar/Models/GitHubModels.swift +++ b/Sources/ActionBar/Models/GitHubModels.swift @@ -3,6 +3,16 @@ import Foundation // Responses from the GitHub REST API and OAuth device flow. // Decoded with `.convertFromSnakeCase`, so property names mirror the JSON keys. +extension JSONDecoder { + /// A decoder for GitHub REST API responses. + static func gitHubAPI() -> JSONDecoder { + let decoder = JSONDecoder() + decoder.keyDecodingStrategy = .convertFromSnakeCase + decoder.dateDecodingStrategy = .iso8601 + return decoder + } +} + struct GitHubUser: Decodable { let login: String } diff --git a/Sources/ActionBar/Services/GitHubAPIClient.swift b/Sources/ActionBar/Services/GitHubAPIClient.swift index a1459d2..0368d05 100644 --- a/Sources/ActionBar/Services/GitHubAPIClient.swift +++ b/Sources/ActionBar/Services/GitHubAPIClient.swift @@ -59,10 +59,7 @@ actor GitHubAPIClient { "X-GitHub-Api-Version": "2022-11-28", ] self.session = URLSession(configuration: config) - - self.decoder = JSONDecoder() - self.decoder.keyDecodingStrategy = .convertFromSnakeCase - self.decoder.dateDecodingStrategy = .iso8601 + self.decoder = .gitHubAPI() } func clearConditionalCache() { diff --git a/Tests/ActionBarTests/ModelDecodingTests.swift b/Tests/ActionBarTests/ModelDecodingTests.swift new file mode 100644 index 0000000..34c85d2 --- /dev/null +++ b/Tests/ActionBarTests/ModelDecodingTests.swift @@ -0,0 +1,136 @@ +import Foundation +import Testing + +struct ModelDecodingTests { + private let decoder = JSONDecoder.gitHubAPI() + + private func decode(_ type: T.Type, _ json: String) throws -> T { + try decoder.decode(type, from: Data(json.utf8)) + } + + @Test func workflowRunsResponse() throws { + let response = try decode(WorkflowRunsResponse.self, """ + { + "total_count": 1, + "workflow_runs": [{ + "id": 42, + "name": "CI", + "display_title": "Fix login bug", + "status": "completed", + "conclusion": "success", + "event": "push", + "html_url": "https://github.com/owner/repo/actions/runs/42", + "head_branch": "main", + "created_at": "2025-01-02T03:04:05Z", + "updated_at": "2025-01-02T03:14:05Z", + "repository": { "full_name": "owner/repo" }, + "actor": { "login": "octocat" }, + "triggering_actor": { "login": "hubot" } + }] + } + """) + + let run = try #require(response.workflowRuns.first) + #expect(run.id == 42) + #expect(run.name == "CI") + #expect(run.title == "Fix login bug") + #expect(run.status == "completed") + #expect(run.conclusion == "success") + #expect(run.event == "push") + #expect(run.htmlUrl == "https://github.com/owner/repo/actions/runs/42") + #expect(run.headBranch == "main") + #expect(run.createdAt == Date(timeIntervalSince1970: 1_735_787_045)) + #expect(run.updatedAt == Date(timeIntervalSince1970: 1_735_787_645)) + #expect(run.repositoryFullName == "owner/repo") + #expect(run.triggeringActorLogin == "hubot") + } + + @Test func workflowRunWithOptionalFieldsMissing() throws { + let run = try decode(WorkflowRun.self, """ + { + "id": 7, + "html_url": "https://github.com/owner/repo/actions/runs/7", + "created_at": "2025-01-02T03:04:05Z", + "updated_at": "2025-01-02T03:04:05Z", + "repository": { "full_name": "owner/repo" }, + "actor": { "login": "octocat" } + } + """) + + #expect(run.name == nil) + #expect(run.status == nil) + #expect(run.headBranch == nil) + #expect(run.title == "Workflow") + #expect(run.triggeringActorLogin == "octocat") + } + + @Test func workflowRunWithoutRepositoryFails() { + #expect(throws: DecodingError.self) { + try decode(WorkflowRun.self, """ + { + "id": 7, + "html_url": "https://github.com/owner/repo/actions/runs/7", + "created_at": "2025-01-02T03:04:05Z", + "updated_at": "2025-01-02T03:04:05Z" + } + """) + } + } + + @Test func installationRepositoriesResponse() throws { + let response = try decode(InstallationRepositoriesResponse.self, """ + { + "total_count": 2, + "repositories": [ + { "id": 1, "full_name": "owner/first", "private": false }, + { "id": 2, "full_name": "owner/second", "private": true } + ] + } + """) + + #expect(response.totalCount == 2) + #expect(response.items.map(\.fullName) == ["owner/first", "owner/second"]) + } + + @Test func installationsResponse() throws { + let response = try decode(InstallationsResponse.self, """ + { "total_count": 1, "installations": [{ "id": 99, "app_id": 1 }] } + """) + + #expect(response.totalCount == 1) + #expect(response.items.map(\.id) == [99]) + } + + @Test func deviceCodeResponse() throws { + let response = try decode(DeviceCodeResponse.self, """ + { + "device_code": "abc123", + "user_code": "ABCD-1234", + "verification_uri": "https://github.com/login/device", + "expires_in": 900, + "interval": 5 + } + """) + + #expect(response.deviceCode == "abc123") + #expect(response.userCode == "ABCD-1234") + #expect(response.verificationUri == "https://github.com/login/device") + #expect(response.interval == 5) + } + + @Test func oauthTokenResponses() throws { + let success = try decode(OAuthTokenResponse.self, """ + { "access_token": "token", "token_type": "bearer", "scope": "" } + """) + #expect(success.accessToken == "token") + #expect(success.error == nil) + + let pending = try decode(OAuthTokenResponse.self, """ + { "error": "slow_down", "error_description": "Too many requests.", "interval": 10 } + """) + #expect(pending.accessToken == nil) + #expect(pending.error == "slow_down") + #expect(pending.errorDescription == "Too many requests.") + #expect(pending.interval == 10) + } +} diff --git a/Tests/ActionBarTests/RelativeTimeTests.swift b/Tests/ActionBarTests/RelativeTimeTests.swift new file mode 100644 index 0000000..3f4f5ff --- /dev/null +++ b/Tests/ActionBarTests/RelativeTimeTests.swift @@ -0,0 +1,41 @@ +import Foundation +import Testing + +struct RelativeTimeTests { + private let now = Date(timeIntervalSince1970: 1_700_000_000) + + private func format(secondsAgo: TimeInterval) -> String { + RelativeTime.format(now.addingTimeInterval(-secondsAgo), relativeTo: now) + } + + @Test func underAMinuteIsJustNow() { + #expect(format(secondsAgo: 0) == "just now") + #expect(format(secondsAgo: 59) == "just now") + } + + @Test func futureDatesAreJustNow() { + #expect(format(secondsAgo: -120) == "just now") + } + + @Test func minutes() { + #expect(format(secondsAgo: 60) == "1m ago") + #expect(format(secondsAgo: 59 * 60 + 59) == "59m ago") + } + + @Test func hours() { + #expect(format(secondsAgo: 60 * 60) == "1h ago") + #expect(format(secondsAgo: 23 * 60 * 60 + 59 * 60) == "23h ago") + } + + @Test func days() { + #expect(format(secondsAgo: 24 * 60 * 60) == "1d ago") + #expect(format(secondsAgo: 29 * 24 * 60 * 60) == "29d ago") + } + + @Test func thirtyDaysOrMoreShowsTheDate() { + let date = now.addingTimeInterval(-30 * 24 * 60 * 60) + let formatter = DateFormatter() + formatter.dateFormat = "MMM d" + #expect(RelativeTime.format(date, relativeTo: now) == formatter.string(from: date)) + } +} diff --git a/Tests/ActionBarTests/RunStatusTests.swift b/Tests/ActionBarTests/RunStatusTests.swift new file mode 100644 index 0000000..28cc3e5 --- /dev/null +++ b/Tests/ActionBarTests/RunStatusTests.swift @@ -0,0 +1,72 @@ +import Foundation +import Testing + +struct RunStatusTests { + @Test(arguments: [ + ("in_progress", RunStatus.running), + ("queued", .queued), + ("waiting", .queued), + ("pending", .queued), + ("requested", .queued), + ]) + func activeStatuses(status: String, expected: RunStatus) { + let status = RunStatus(from: .make(status: status, conclusion: nil)) + #expect(status == expected) + #expect(status.isActive) + } + + @Test(arguments: [ + ("success", RunStatus.success), + ("failure", .failure), + ("timed_out", .failure), + ("startup_failure", .failure), + ("cancelled", .cancelled), + ("skipped", .cancelled), + ("neutral", .unknown), + ]) + func completedConclusions(conclusion: String, expected: RunStatus) { + let status = RunStatus(from: .make(status: "completed", conclusion: conclusion)) + #expect(status == expected) + #expect(!status.isActive) + } + + @Test func completedWithoutConclusionIsUnknown() { + #expect(RunStatus(from: .make(status: "completed", conclusion: nil)) == .unknown) + } + + @Test func unrecognizedOrMissingStatusIsUnknown() { + #expect(RunStatus(from: .make(status: "something_new", conclusion: nil)) == .unknown) + #expect(RunStatus(from: .make(status: nil, conclusion: nil)) == .unknown) + } + + // MARK: - Overall Status + + @Test func overallStatusOfNoRunsIsUnknown() { + #expect([WorkflowRun]().overallStatus == .unknown) + } + + @Test func overallStatusIsRunningWhileAnythingIsActive() { + let runs: [WorkflowRun] = [ + .make(id: 1, conclusion: "failure", updatedAt: Date(timeIntervalSince1970: 200)), + .make(id: 2, status: "waiting", conclusion: nil, updatedAt: Date(timeIntervalSince1970: 100)), + ] + #expect(runs.overallStatus == .running) + } + + @Test func overallStatusFollowsMostRecentlyFinishedRun() { + let runs: [WorkflowRun] = [ + .make(id: 1, conclusion: "success", updatedAt: Date(timeIntervalSince1970: 100)), + .make(id: 2, conclusion: "failure", updatedAt: Date(timeIntervalSince1970: 300)), + .make(id: 3, conclusion: "cancelled", updatedAt: Date(timeIntervalSince1970: 200)), + ] + #expect(runs.overallStatus == .failure) + } + + @Test func overallStatusIgnoresRunsThatAreNeitherActiveNorCompleted() { + let runs: [WorkflowRun] = [ + .make(id: 1, conclusion: "success", updatedAt: Date(timeIntervalSince1970: 100)), + .make(id: 2, status: "something_new", conclusion: nil, updatedAt: Date(timeIntervalSince1970: 300)), + ] + #expect(runs.overallStatus == .success) + } +} diff --git a/Tests/ActionBarTests/SettingsStoreTests.swift b/Tests/ActionBarTests/SettingsStoreTests.swift new file mode 100644 index 0000000..2c9f581 --- /dev/null +++ b/Tests/ActionBarTests/SettingsStoreTests.swift @@ -0,0 +1,97 @@ +import Foundation +import Testing + +/// Each test starts from an empty defaults suite. They share one suite, so they run one at a time. +@MainActor +@Suite(.serialized) +final class SettingsStoreTests { + private let suiteName = "com.ptrchm.actionbar.tests" + private let defaults: UserDefaults + + init() throws { + defaults = try #require(UserDefaults(suiteName: suiteName)) + defaults.removePersistentDomain(forName: suiteName) + } + + deinit { + UserDefaults().removePersistentDomain(forName: suiteName) + } + + @Test func defaultsWhenNothingIsSaved() { + let store = SettingsStore(defaults: defaults) + #expect(store.selectedRepoNames.isEmpty) + #expect(store.pollingInterval == SettingsStore.defaultPollingInterval) + #expect(store.workflowFilterMode == .off) + #expect(store.workflowFilterText == "") + #expect(store.maxRunsPerRepo == SettingsStore.defaultMaxRunsPerRepo) + #expect(!store.includeRunsFromOtherUsers) + } + + @Test func loadingDoesNotWriteAnything() { + _ = SettingsStore(defaults: defaults) + #expect(defaults.persistentDomain(forName: suiteName)?.isEmpty ?? true) + } + + @Test func changesAreSavedImmediately() { + let store = SettingsStore(defaults: defaults) + store.selectedRepoNames = ["owner/a", "owner/b"] + store.pollingInterval = 60 + store.workflowFilterMode = .exclude + store.workflowFilterText = "deploy" + store.maxRunsPerRepo = 3 + store.includeRunsFromOtherUsers = true + + let reloaded = SettingsStore(defaults: defaults) + #expect(reloaded.selectedRepoNames == ["owner/a", "owner/b"]) + #expect(reloaded.pollingInterval == 60) + #expect(reloaded.workflowFilterMode == .exclude) + #expect(reloaded.workflowFilterText == "deploy") + #expect(reloaded.maxRunsPerRepo == 3) + #expect(reloaded.includeRunsFromOtherUsers) + } + + @Test func repoSelectionHelpers() { + let store = SettingsStore(defaults: defaults) + store.toggleRepo("owner/a") + store.toggleRepo("owner/b") + store.toggleRepo("owner/a") + #expect(SettingsStore(defaults: defaults).selectedRepoNames == ["owner/b"]) + + store.selectAllRepos(["owner/a", "owner/c"]) + #expect(SettingsStore(defaults: defaults).selectedRepoNames == ["owner/a", "owner/c"]) + + store.deselectAllRepos() + #expect(SettingsStore(defaults: defaults).selectedRepoNames.isEmpty) + } + + @Test func invalidSavedValuesFallBackToDefaults() { + defaults.set(0, forKey: "pollingInterval") + defaults.set(-1, forKey: "maxRunsPerRepo") + defaults.set("bogus", forKey: "workflowFilterMode") + + let store = SettingsStore(defaults: defaults) + #expect(store.pollingInterval == SettingsStore.defaultPollingInterval) + #expect(store.maxRunsPerRepo == SettingsStore.defaultMaxRunsPerRepo) + #expect(store.workflowFilterMode == .off) + } + + @Test func resetAccountSettingsKeepsPollingInterval() { + let store = SettingsStore(defaults: defaults) + store.selectedRepoNames = ["owner/a"] + store.pollingInterval = 300 + store.workflowFilterMode = .include + store.workflowFilterText = "ci" + store.maxRunsPerRepo = 2 + store.includeRunsFromOtherUsers = true + + store.resetAccountSettings() + + #expect(store.selectedRepoNames.isEmpty) + #expect(store.pollingInterval == 300) + #expect(store.workflowFilterMode == .off) + #expect(store.workflowFilterText == "") + #expect(store.maxRunsPerRepo == SettingsStore.defaultMaxRunsPerRepo) + #expect(!store.includeRunsFromOtherUsers) + #expect(defaults.persistentDomain(forName: suiteName)?.keys.sorted() == ["pollingInterval"]) + } +} diff --git a/Tests/ActionBarTests/TestRuns.swift b/Tests/ActionBarTests/TestRuns.swift new file mode 100644 index 0000000..0e6762e --- /dev/null +++ b/Tests/ActionBarTests/TestRuns.swift @@ -0,0 +1,24 @@ +import Foundation + +extension WorkflowRun { + static func make( + id: Int = 1, + name: String? = "CI", + status: String? = "completed", + conclusion: String? = "success", + updatedAt: Date = Date(timeIntervalSince1970: 0), + repo: String = "owner/repo" + ) -> WorkflowRun { + WorkflowRun( + id: id, + name: name, + displayTitle: nil, + status: status, + conclusion: conclusion, + htmlUrl: "https://github.com/\(repo)/actions/runs/\(id)", + headBranch: "main", + updatedAt: updatedAt, + repositoryFullName: repo + ) + } +} diff --git a/Tests/ActionBarTests/WorkflowRunListTests.swift b/Tests/ActionBarTests/WorkflowRunListTests.swift new file mode 100644 index 0000000..1b45688 --- /dev/null +++ b/Tests/ActionBarTests/WorkflowRunListTests.swift @@ -0,0 +1,75 @@ +import Foundation +import Testing + +struct WorkflowRunListTests { + private let runs: [WorkflowRun] = [ + .make(id: 1, name: "CI"), + .make(id: 2, name: "Deploy to production"), + .make(id: 3, name: "Build docs"), + .make(id: 4, name: nil), + ] + + // MARK: - Workflow Filter + + @Test func filterOffKeepsEverything() { + #expect(runs.filtered(by: .off, patterns: "ci").map(\.id) == [1, 2, 3, 4]) + } + + @Test func includeKeepsMatchingNames() { + #expect(runs.filtered(by: .include, patterns: "ci, DEPLOY").map(\.id) == [1, 2]) + } + + @Test func excludeDropsMatchingNames() { + #expect(runs.filtered(by: .exclude, patterns: "ci,deploy").map(\.id) == [3, 4]) + } + + @Test func matchesSubstrings() { + #expect(runs.filtered(by: .include, patterns: "prod").map(\.id) == [2]) + } + + @Test(arguments: [WorkflowFilterMode.include, .exclude]) + func blankPatternsKeepEverything(mode: WorkflowFilterMode) { + #expect(runs.filtered(by: mode, patterns: "").map(\.id) == [1, 2, 3, 4]) + #expect(runs.filtered(by: mode, patterns: " , ,").map(\.id) == [1, 2, 3, 4]) + } + + // MARK: - Runs per Repo + + @Test func limitsRunsPerRepoPreservingOrder() { + let runs: [WorkflowRun] = [ + .make(id: 1, repo: "owner/a"), + .make(id: 2, repo: "owner/b"), + .make(id: 3, repo: "owner/a"), + .make(id: 4, repo: "owner/a"), + .make(id: 5, repo: "owner/b"), + .make(id: 6, repo: "owner/b"), + ] + #expect(runs.limited(toRunsPerRepo: 2).map(\.id) == [1, 2, 3, 5]) + #expect(runs.limited(toRunsPerRepo: 1).map(\.id) == [1, 2]) + #expect(runs.limited(toRunsPerRepo: 10).map(\.id) == [1, 2, 3, 4, 5, 6]) + } + + // MARK: - Completion Notifications + + @Test(arguments: ["in_progress", "queued", "waiting", "pending", "requested"]) + func notifiesWhenActiveRunCompletes(previousStatus: String) { + let previous = [1: RunStatus(from: .make(status: previousStatus, conclusion: nil))] + let completed = [WorkflowRun.make(id: 1, conclusion: "failure")] + #expect(completed.newlyCompleted(since: previous).map(\.id) == [1]) + } + + @Test func doesNotNotifyForRunsAlreadyFinished() { + let previous: [Int: RunStatus] = [1: .success] + #expect([WorkflowRun.make(id: 1)].newlyCompleted(since: previous).isEmpty) + } + + @Test func doesNotNotifyForRunsNotSeenBefore() { + #expect([WorkflowRun.make(id: 1)].newlyCompleted(since: [:]).isEmpty) + } + + @Test func doesNotNotifyForRunsStillActive() { + let previous: [Int: RunStatus] = [1: .queued] + let runs = [WorkflowRun.make(id: 1, status: "in_progress", conclusion: nil)] + #expect(runs.newlyCompleted(since: previous).isEmpty) + } +} From 2cc7b55a6747c4d4356ac618ceb3cf837d5bc4f6 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 08:33:01 +0200 Subject: [PATCH 08/14] Keep polling with the settings from when Settings opened Settings now save as they change, so a poll during editing used half-finished values. With an empty selection (after pressing None) it would replace the user's in-progress picks with the repos that have recent runs. Polls now use a snapshot taken when Settings opens, and Done applies the new settings. The Reset button uses the same snapshot instead of one taken in onAppear. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 34 +++++++++++++++---- Sources/ActionBar/Models/SettingsStore.swift | 19 +++++++++++ Sources/ActionBar/Views/MenuContentView.swift | 3 +- Sources/ActionBar/Views/SettingsView.swift | 16 ++++----- Tests/ActionBarTests/SettingsStoreTests.swift | 17 ++++++++++ 5 files changed, 72 insertions(+), 17 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 97af07d..169a945 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -17,7 +17,10 @@ final class AppState { var isLoadingRepos = false var repoListIsCapped = false var repoListError: String? - var showingSettings = false + private(set) var showingSettings = false + /// The run query from when Settings was opened. Polls keep using it until Settings closes, + /// so half-finished edits (like an empty selection) don't reach the menu or notifications. + private(set) var runQueryAtSettingsOpen: SettingsStore.RunQuery? var needsInstallation = false var isCheckingInstallation = false private(set) var overallStatus: RunStatus = .unknown @@ -122,6 +125,7 @@ final class AppState { repoListIsCapped = false repoListError = nil showingSettings = false + runQueryAtSettingsOpen = nil settings.resetAccountSettings() } @@ -186,6 +190,20 @@ final class AppState { startPolling() } + // MARK: - Settings + + func openSettings() { + runQueryAtSettingsOpen = settings.runQuery + showingSettings = true + fetchAvailableRepos() + } + + func closeSettings() { + showingSettings = false + runQueryAtSettingsOpen = nil + refreshNow() + } + // MARK: - Polling func refreshNow() { @@ -242,19 +260,21 @@ final class AppState { errorMessage = nil do { - let usingFallback = settings.selectedRepoNames.isEmpty - let repos = usingFallback ? nil : Array(settings.selectedRepoNames) - let actor = settings.includeRunsFromOtherUsers ? nil : username + let query = runQueryAtSettingsOpen ?? settings.runQuery + let usingFallback = query.repoNames.isEmpty + let repos = usingFallback ? nil : Array(query.repoNames) + let actor = query.includeRunsFromOtherUsers ? nil : username let recentRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) let newRuns = recentRuns.runs // With nothing selected yet, adopt the repos that have recent runs as the selection. - if usingFallback && !newRuns.isEmpty { + // Not while Settings is open, where it would overwrite the user's edits. + if usingFallback && runQueryAtSettingsOpen == nil && !newRuns.isEmpty { settings.selectedRepoNames = Set(newRuns.map(\.repositoryFullName)) } - let workflowFilteredRuns = newRuns.filtered(by: settings.workflowFilterMode, patterns: settings.workflowFilterText) - let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: settings.maxRunsPerRepo) + let workflowFilteredRuns = newRuns.filtered(by: query.workflowFilterMode, patterns: query.workflowFilterText) + let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: query.maxRunsPerRepo) if hasLoadedOnce { for run in visibleRuns.newlyCompleted(since: previousRunStatuses) { diff --git a/Sources/ActionBar/Models/SettingsStore.swift b/Sources/ActionBar/Models/SettingsStore.swift index b986b50..86db36c 100644 --- a/Sources/ActionBar/Models/SettingsStore.swift +++ b/Sources/ActionBar/Models/SettingsStore.swift @@ -8,6 +8,15 @@ final class SettingsStore { static let defaultPollingInterval: TimeInterval = 30 static let defaultMaxRunsPerRepo = 5 + /// The settings that decide which runs are fetched and shown. + struct RunQuery: Equatable { + var repoNames: Set + var workflowFilterMode: WorkflowFilterMode + var workflowFilterText: String + var maxRunsPerRepo: Int + var includeRunsFromOtherUsers: Bool + } + var selectedRepoNames: Set = [] { didSet { save(Array(selectedRepoNames), forKey: Key.selectedRepos) } } @@ -27,6 +36,16 @@ final class SettingsStore { didSet { save(includeRunsFromOtherUsers, forKey: Key.includeRunsFromOtherUsers) } } + var runQuery: RunQuery { + RunQuery( + repoNames: selectedRepoNames, + workflowFilterMode: workflowFilterMode, + workflowFilterText: workflowFilterText, + maxRunsPerRepo: maxRunsPerRepo, + includeRunsFromOtherUsers: includeRunsFromOtherUsers + ) + } + @ObservationIgnored private let defaults: UserDefaults /// `@Observable` runs `didSet` even for assignments in `init`; this keeps loading from writing back. @ObservationIgnored private var isLoading = true diff --git a/Sources/ActionBar/Views/MenuContentView.swift b/Sources/ActionBar/Views/MenuContentView.swift index 7f08635..32376d8 100644 --- a/Sources/ActionBar/Views/MenuContentView.swift +++ b/Sources/ActionBar/Views/MenuContentView.swift @@ -60,8 +60,7 @@ struct MenuContentView: View { } .help("Refresh now") Button("Settings", systemImage: "gearshape") { - appState.showingSettings = true - appState.fetchAvailableRepos() + appState.openSettings() } .help("Settings") } diff --git a/Sources/ActionBar/Views/SettingsView.swift b/Sources/ActionBar/Views/SettingsView.swift index 2ddf3ab..acfe405 100644 --- a/Sources/ActionBar/Views/SettingsView.swift +++ b/Sources/ActionBar/Views/SettingsView.swift @@ -6,7 +6,6 @@ struct SettingsView: View { @Environment(AppState.self) private var appState @Environment(\.openURL) private var openURL @State private var searchText = "" - @State private var repoSelectionSnapshot: Set = [] @State private var showSelectAllWarning = false /// Selecting more repos than this at once asks for confirmation, since each one costs an API call per refresh. @@ -19,6 +18,11 @@ struct SettingsView: View { ("5 min", 300), ] + /// What the Reset button restores. + private var repoSelectionAtOpen: Set { + appState.runQueryAtSettingsOpen?.repoNames ?? settings.selectedRepoNames + } + private var filteredRepos: [Repository] { if searchText.isEmpty { return appState.availableRepos @@ -46,9 +50,6 @@ struct SettingsView: View { Divider() repositoryList } - .onAppear { - repoSelectionSnapshot = settings.selectedRepoNames - } } private var header: some View { @@ -57,8 +58,7 @@ struct SettingsView: View { .font(.headline) Spacer() Button("Done") { - appState.showingSettings = false - appState.refreshNow() + appState.closeSettings() } .buttonStyle(.plain) .foregroundStyle(Color.accentColor) @@ -216,10 +216,10 @@ struct SettingsView: View { .textButtonStyle(.accentColor) Button("Reset") { showSelectAllWarning = false - settings.selectedRepoNames = repoSelectionSnapshot + settings.selectedRepoNames = repoSelectionAtOpen } .textButtonStyle(.accentColor) - .disabled(settings.selectedRepoNames == repoSelectionSnapshot) + .disabled(settings.selectedRepoNames == repoSelectionAtOpen) } if appState.repoListIsCapped { diff --git a/Tests/ActionBarTests/SettingsStoreTests.swift b/Tests/ActionBarTests/SettingsStoreTests.swift index 2c9f581..10e9914 100644 --- a/Tests/ActionBarTests/SettingsStoreTests.swift +++ b/Tests/ActionBarTests/SettingsStoreTests.swift @@ -64,6 +64,23 @@ final class SettingsStoreTests { #expect(SettingsStore(defaults: defaults).selectedRepoNames.isEmpty) } + @Test func runQueryReflectsCurrentSettings() { + let store = SettingsStore(defaults: defaults) + store.selectedRepoNames = ["owner/a"] + store.workflowFilterMode = .include + store.workflowFilterText = "ci" + store.maxRunsPerRepo = 2 + store.includeRunsFromOtherUsers = true + + #expect(store.runQuery == SettingsStore.RunQuery( + repoNames: ["owner/a"], + workflowFilterMode: .include, + workflowFilterText: "ci", + maxRunsPerRepo: 2, + includeRunsFromOtherUsers: true + )) + } + @Test func invalidSavedValuesFallBackToDefaults() { defaults.set(0, forKey: "pollingInterval") defaults.set(-1, forKey: "maxRunsPerRepo") From 34ba9593af5833872d20be993add7bc920f5c83f Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 08:33:44 +0200 Subject: [PATCH 09/14] Track run statuses across filtered runs and failed repos The notification history only kept the visible runs, and was rebuilt from scratch on each fetch. A run hidden for one fetch (by the filter or the per-repo limit) or in a repo that failed to load for one fetch lost its "was running" status, so it never notified when it finished. RunHistory now records every fetched run and keeps the last status for repos that failed. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 13 +++--- Sources/ActionBar/Models/WorkflowRun.swift | 37 +++++++++++++--- .../ActionBarTests/WorkflowRunListTests.swift | 43 ++++++++++++++++--- 3 files changed, 72 insertions(+), 21 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 169a945..8b80591 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -32,7 +32,7 @@ final class AppState { @ObservationIgnored private var pollTask: Task? @ObservationIgnored private var installCheckTask: Task? @ObservationIgnored private var oauthTask: Task? - @ObservationIgnored private var previousRunStatuses: [Int: RunStatus] = [:] // run.id -> status + @ObservationIgnored private var runHistory = RunHistory() @ObservationIgnored private var hasStarted = false @ObservationIgnored private var hasLoadedOnce = false @ObservationIgnored private var isFetchingRuns = false @@ -118,7 +118,7 @@ final class AppState { runs = [] failedRepos = [] username = nil - previousRunStatuses = [:] + runHistory = RunHistory() hasLoadedOnce = false errorMessage = nil availableRepos = [] @@ -277,14 +277,13 @@ final class AppState { let visibleRuns = workflowFilteredRuns.limited(toRunsPerRepo: query.maxRunsPerRepo) if hasLoadedOnce { - for run in visibleRuns.newlyCompleted(since: previousRunStatuses) { + for run in runHistory.newlyCompleted(in: visibleRuns) { NotificationService.sendCompletionNotification(run: run) } } - previousRunStatuses = [:] - for run in visibleRuns { - previousRunStatuses[run.id] = RunStatus(from: run) - } + // Track every fetched run, not just the visible ones, so a run hidden by the filter or + // the per-repo limit for one fetch still notifies if it shows up completed later. + runHistory.record(newRuns, keepingRunsFrom: recentRuns.failedRepos) overallStatus = workflowFilteredRuns.overallStatus runs = visibleRuns diff --git a/Sources/ActionBar/Models/WorkflowRun.swift b/Sources/ActionBar/Models/WorkflowRun.swift index 5e4beee..46ca59f 100644 --- a/Sources/ActionBar/Models/WorkflowRun.swift +++ b/Sources/ActionBar/Models/WorkflowRun.swift @@ -153,13 +153,6 @@ extension Array where Element == WorkflowRun { } } - /// Runs that have completed since they were last seen queued or in progress. - func newlyCompleted(since previousStatuses: [Int: RunStatus]) -> [WorkflowRun] { - filter { run in - run.status == "completed" && previousStatuses[run.id]?.isActive == true - } - } - /// Running if anything is in flight, otherwise the outcome of the most recently finished run. var overallStatus: RunStatus { if contains(where: { RunStatus(from: $0).isActive }) { @@ -174,3 +167,33 @@ extension Array where Element == WorkflowRun { return RunStatus(from: lastFinishedRun) } } + +// MARK: - Run History + +/// The last status seen for each run, to tell which runs finished since the previous fetch. +struct RunHistory { + private struct Entry { + let repo: String + let status: RunStatus + } + + private var entries: [Int: Entry] = [:] // run.id -> last seen + + /// Runs that have completed since they were last seen queued or in progress. + func newlyCompleted(in runs: [WorkflowRun]) -> [WorkflowRun] { + runs.filter { run in + run.status == "completed" && entries[run.id]?.status.isActive == true + } + } + + /// Replaces the history with `runs`. Repos in `failedRepos` weren't loaded this time, + /// so their runs keep their last status until the repo loads again. + mutating func record(_ runs: [WorkflowRun], keepingRunsFrom failedRepos: [String]) { + let failedRepos = Set(failedRepos) + var newEntries = entries.filter { failedRepos.contains($0.value.repo) } + for run in runs { + newEntries[run.id] = Entry(repo: run.repositoryFullName, status: RunStatus(from: run)) + } + entries = newEntries + } +} diff --git a/Tests/ActionBarTests/WorkflowRunListTests.swift b/Tests/ActionBarTests/WorkflowRunListTests.swift index 1b45688..946cb44 100644 --- a/Tests/ActionBarTests/WorkflowRunListTests.swift +++ b/Tests/ActionBarTests/WorkflowRunListTests.swift @@ -51,25 +51,54 @@ struct WorkflowRunListTests { // MARK: - Completion Notifications + private func history(_ runs: [WorkflowRun], failedRepos: [String] = []) -> RunHistory { + var history = RunHistory() + history.record(runs, keepingRunsFrom: failedRepos) + return history + } + @Test(arguments: ["in_progress", "queued", "waiting", "pending", "requested"]) func notifiesWhenActiveRunCompletes(previousStatus: String) { - let previous = [1: RunStatus(from: .make(status: previousStatus, conclusion: nil))] + let history = history([.make(id: 1, status: previousStatus, conclusion: nil)]) let completed = [WorkflowRun.make(id: 1, conclusion: "failure")] - #expect(completed.newlyCompleted(since: previous).map(\.id) == [1]) + #expect(history.newlyCompleted(in: completed).map(\.id) == [1]) } @Test func doesNotNotifyForRunsAlreadyFinished() { - let previous: [Int: RunStatus] = [1: .success] - #expect([WorkflowRun.make(id: 1)].newlyCompleted(since: previous).isEmpty) + let history = history([.make(id: 1)]) + #expect(history.newlyCompleted(in: [.make(id: 1)]).isEmpty) } @Test func doesNotNotifyForRunsNotSeenBefore() { - #expect([WorkflowRun.make(id: 1)].newlyCompleted(since: [:]).isEmpty) + #expect(RunHistory().newlyCompleted(in: [.make(id: 1)]).isEmpty) } @Test func doesNotNotifyForRunsStillActive() { - let previous: [Int: RunStatus] = [1: .queued] + let history = history([.make(id: 1, status: "queued", conclusion: nil)]) let runs = [WorkflowRun.make(id: 1, status: "in_progress", conclusion: nil)] - #expect(runs.newlyCompleted(since: previous).isEmpty) + #expect(history.newlyCompleted(in: runs).isEmpty) + } + + @Test func forgetsRunsThatAreNoLongerFetched() { + var history = history([.make(id: 1, status: "in_progress", conclusion: nil)]) + history.record([], keepingRunsFrom: []) + #expect(history.newlyCompleted(in: [.make(id: 1)]).isEmpty) + } + + @Test func keepsRunsFromReposThatFailedToLoad() { + var history = history([ + .make(id: 1, status: "in_progress", conclusion: nil, repo: "owner/a"), + .make(id: 2, status: "in_progress", conclusion: nil, repo: "owner/b"), + ]) + history.record([], keepingRunsFrom: ["owner/a"]) + + let completed: [WorkflowRun] = [.make(id: 1, repo: "owner/a"), .make(id: 2, repo: "owner/b")] + #expect(history.newlyCompleted(in: completed).map(\.id) == [1]) + } + + @Test func freshResultsReplaceKeptRuns() { + var history = history([.make(id: 1, status: "in_progress", conclusion: nil, repo: "owner/a")]) + history.record([.make(id: 1, repo: "owner/a")], keepingRunsFrom: ["owner/a"]) + #expect(history.newlyCompleted(in: [.make(id: 1, repo: "owner/a")]).isEmpty) } } From fdbce47f3d165303bb5ba3ba3e475a508e4be178 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 08:35:24 +0200 Subject: [PATCH 10/14] Don't lose refreshes or apply results after sign-out A refresh asked for while a fetch was in flight (like the one Done triggers) was dropped, so new settings could wait a full polling interval. It now runs once the current fetch finishes, and results fetched with settings that have since changed are discarded. Requests that finish after sign-out no longer write runs, the repo selection, errors or the username, or start polling. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 65 ++++++++++++++++++++----- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 8b80591..8585c2a 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -36,6 +36,9 @@ final class AppState { @ObservationIgnored private var hasStarted = false @ObservationIgnored private var hasLoadedOnce = false @ObservationIgnored private var isFetchingRuns = false + @ObservationIgnored private var refetchRequested = false + /// Changes on sign-out, so a fetch that was in flight doesn't apply its results afterwards. + @ObservationIgnored private var sessionID = 0 init(settings: SettingsStore? = nil) { self.settings = settings ?? SettingsStore() @@ -47,8 +50,11 @@ final class AppState { hasStarted = true guard let token = KeychainService.loadToken() else { return } isAuthenticated = true + let sessionID = self.sessionID Task { - if let user = try? await apiClient.validateToken(token) { + let user = try? await apiClient.validateToken(token) + guard sessionID == self.sessionID else { return } + if let user { username = user.login } await startMonitoring(token: token) @@ -112,6 +118,7 @@ final class AppState { await apiClient.clearConditionalCache() } KeychainService.deleteToken() + sessionID += 1 isAuthenticated = false needsInstallation = false overallStatus = .unknown @@ -133,33 +140,42 @@ final class AppState { /// Starts polling if the GitHub App is installed, otherwise waits for it to be installed. private func startMonitoring(token: String) async { + let sessionID = self.sessionID + let hasInstallations: Bool do { - if try await apiClient.hasInstallations(token: token) { - startPolling() - } else { - needsInstallation = true - startInstallCheckTimer() - } + hasInstallations = try await apiClient.hasInstallations(token: token) } catch { // Don't assume there are no installations; fetchRuns will surface real errors. + hasInstallations = true + } + guard sessionID == self.sessionID else { return } + + if hasInstallations { startPolling() + } else { + needsInstallation = true + startInstallCheckTimer() } } func checkInstallationAndStart() { guard let token = KeychainService.loadToken() else { return } isCheckingInstallation = true + let sessionID = self.sessionID Task { + defer { isCheckingInstallation = false } do { - if try await apiClient.hasInstallations(token: token) { + let hasInstallations = try await apiClient.hasInstallations(token: token) + guard sessionID == self.sessionID else { return } + if hasInstallations { installationFound() } else { errorMessage = "No installation found yet. Please install the app on GitHub first. If you did, wait a bit." } } catch { + guard sessionID == self.sessionID else { return } errorMessage = error.localizedDescription } - isCheckingInstallation = false } } @@ -245,10 +261,21 @@ final class AppState { } private func fetchRuns(manageLoading: Bool = true) async { - guard !isFetchingRuns else { return } + guard !isFetchingRuns else { + // Fetch again once the current fetch finishes, so this refresh isn't lost. + refetchRequested = true + return + } isFetchingRuns = true defer { isFetchingRuns = false } + repeat { + refetchRequested = false + await fetchRunsOnce(manageLoading: manageLoading) + } while refetchRequested && !Task.isCancelled + } + + private func fetchRunsOnce(manageLoading: Bool) async { guard let token = KeychainService.loadToken() else { isAuthenticated = false return @@ -258,6 +285,8 @@ final class AppState { isLoading = true } errorMessage = nil + defer { if manageLoading { isLoading = false } } + let sessionID = self.sessionID do { let query = runQueryAtSettingsOpen ?? settings.runQuery @@ -267,6 +296,13 @@ final class AppState { let recentRuns = try await apiClient.fetchRecentRuns(token: token, actor: actor, repos: repos) let newRuns = recentRuns.runs + guard sessionID == self.sessionID else { return } + // Settings closed with changes while this was in flight; these results are for the old ones. + guard query == runQueryAtSettingsOpen ?? settings.runQuery else { + refetchRequested = true + return + } + // With nothing selected yet, adopt the repos that have recent runs as the selection. // Not while Settings is open, where it would overwrite the user's edits. if usingFallback && runQueryAtSettingsOpen == nil && !newRuns.isEmpty { @@ -290,14 +326,14 @@ final class AppState { failedRepos = recentRuns.failedRepos hasLoadedOnce = true } catch GitHubAPIError.invalidToken { + guard sessionID == self.sessionID else { return } signOut() } catch is CancellationError { // Task was cancelled (e.g. polling restart) — don't surface to UI. } catch { + guard sessionID == self.sessionID else { return } errorMessage = error.localizedDescription } - - if manageLoading { isLoading = false } } // MARK: - Repositories @@ -307,16 +343,19 @@ final class AppState { if !force && !availableRepos.isEmpty { return } isLoadingRepos = true repoListError = nil + let sessionID = self.sessionID Task { + defer { isLoadingRepos = false } do { let repos = try await apiClient.fetchAllRepos(token: token) + guard sessionID == self.sessionID else { return } availableRepos = repos repoListIsCapped = repos.count >= GitHubAPIClient.maxRepositories } catch { // Keep whatever list we had before. + guard sessionID == self.sessionID else { return } repoListError = error.localizedDescription } - isLoadingRepos = false } } } From 8b3ac73270c54e10d112972b94917a5a1ad68396 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 10:12:36 +0200 Subject: [PATCH 11/14] Fail a multi-repo fetch only when every repo failed The fetch threw the first per-repo error whenever no runs came back, so one failing repo next to repos with no runs (or Actions disabled) replaced the menu with a full error instead of the partial-failure line. Now it only fails outright when every requested repo failed, or on an invalid token or rate limit. The rule moves next to GitHubAPIError so it can be unit tested. Co-Authored-By: Claude Opus 5.5 --- ActionBar.xcodeproj/project.pbxproj | 4 ++ Sources/ActionBar/Models/GitHubModels.swift | 48 +++++++++++++++++++ .../ActionBar/Services/GitHubAPIClient.swift | 40 +--------------- Tests/ActionBarTests/FetchFailureTests.swift | 42 ++++++++++++++++ 4 files changed, 96 insertions(+), 38 deletions(-) create mode 100644 Tests/ActionBarTests/FetchFailureTests.swift diff --git a/ActionBar.xcodeproj/project.pbxproj b/ActionBar.xcodeproj/project.pbxproj index 5c6fbf4..af6f986 100644 --- a/ActionBar.xcodeproj/project.pbxproj +++ b/ActionBar.xcodeproj/project.pbxproj @@ -31,6 +31,7 @@ A10001000000000000000022 /* Components.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000022 /* Components.swift */; }; A10001000000000000000023 /* SettingsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000023 /* SettingsStore.swift */; }; A10001000000000000000030 /* TestRuns.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000030 /* TestRuns.swift */; }; + A10001000000000000000036 /* FetchFailureTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000036 /* FetchFailureTests.swift */; }; A10001000000000000000031 /* RunStatusTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000031 /* RunStatusTests.swift */; }; A10001000000000000000032 /* WorkflowRunListTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000032 /* WorkflowRunListTests.swift */; }; A10001000000000000000033 /* RelativeTimeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10002000000000000000033 /* RelativeTimeTests.swift */; }; @@ -70,6 +71,7 @@ A10002000000000000000023 /* SettingsStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsStore.swift; sourceTree = ""; }; A10002000000000000000020 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; A10002000000000000000030 /* TestRuns.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestRuns.swift; sourceTree = ""; }; + A10002000000000000000036 /* FetchFailureTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FetchFailureTests.swift; sourceTree = ""; }; A10002000000000000000031 /* RunStatusTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RunStatusTests.swift; sourceTree = ""; }; A10002000000000000000032 /* WorkflowRunListTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkflowRunListTests.swift; sourceTree = ""; }; A10002000000000000000033 /* RelativeTimeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelativeTimeTests.swift; sourceTree = ""; }; @@ -199,6 +201,7 @@ isa = PBXGroup; children = ( A10002000000000000000030 /* TestRuns.swift */, + A10002000000000000000036 /* FetchFailureTests.swift */, A10002000000000000000031 /* RunStatusTests.swift */, A10002000000000000000032 /* WorkflowRunListTests.swift */, A10002000000000000000033 /* RelativeTimeTests.swift */, @@ -324,6 +327,7 @@ buildActionMask = 2147483647; files = ( A10001000000000000000030 /* TestRuns.swift in Sources */, + A10001000000000000000036 /* FetchFailureTests.swift in Sources */, A10001000000000000000031 /* RunStatusTests.swift in Sources */, A10001000000000000000032 /* WorkflowRunListTests.swift in Sources */, A10001000000000000000033 /* RelativeTimeTests.swift in Sources */, diff --git a/Sources/ActionBar/Models/GitHubModels.swift b/Sources/ActionBar/Models/GitHubModels.swift index 11ab4c9..68f6902 100644 --- a/Sources/ActionBar/Models/GitHubModels.swift +++ b/Sources/ActionBar/Models/GitHubModels.swift @@ -13,6 +13,54 @@ extension JSONDecoder { } } +enum GitHubAPIError: Error, LocalizedError { + case invalidToken + case rateLimited + case networkError(Error) + case invalidResponse(Int) + case decodingError(Error) + + var errorDescription: String? { + switch self { + case .invalidToken: + return "Invalid or expired token" + case .rateLimited: + return "GitHub API rate limit reached. Please wait and try again." + case .networkError(let error): + return "Network error: \(error.localizedDescription)" + case .invalidResponse(let code): + return "GitHub API returned status \(code)" + case .decodingError(let error): + return "Failed to parse response: \(error.localizedDescription)" + } + } +} + +/// Recent runs across repositories, plus the repositories whose runs couldn't be loaded. +struct RecentRuns { + let runs: [WorkflowRun] + let failedRepos: [String] +} + +extension GitHubAPIError { + /// The error that should fail a fetch across several repos, or nil to return partial results. + /// An invalid token or a rate limit on any repo fails the fetch, and so does every repo failing. + /// A repo that loaded with no runs (or with Actions disabled) still counts as loaded. + static func failingWholeFetch(_ errors: [Error], requestedRepoCount: Int) -> Error? { + if errors.contains(where: { if case GitHubAPIError.invalidToken = $0 { true } else { false } }) { + return GitHubAPIError.invalidToken + } + if errors.contains(where: { if case GitHubAPIError.rateLimited = $0 { true } else { false } }) { + return GitHubAPIError.rateLimited + } + if let firstError = errors.first, errors.count >= requestedRepoCount { + return firstError + } + return nil + } +} + + struct GitHubUser: Decodable { let login: String } diff --git a/Sources/ActionBar/Services/GitHubAPIClient.swift b/Sources/ActionBar/Services/GitHubAPIClient.swift index 0368d05..47850cb 100644 --- a/Sources/ActionBar/Services/GitHubAPIClient.swift +++ b/Sources/ActionBar/Services/GitHubAPIClient.swift @@ -1,34 +1,5 @@ import Foundation -enum GitHubAPIError: Error, LocalizedError { - case invalidToken - case rateLimited - case networkError(Error) - case invalidResponse(Int) - case decodingError(Error) - - var errorDescription: String? { - switch self { - case .invalidToken: - return "Invalid or expired token" - case .rateLimited: - return "GitHub API rate limit reached. Please wait and try again." - case .networkError(let error): - return "Network error: \(error.localizedDescription)" - case .invalidResponse(let code): - return "GitHub API returned status \(code)" - case .decodingError(let error): - return "Failed to parse response: \(error.localizedDescription)" - } - } -} - -/// Recent runs across repositories, plus the repositories whose runs couldn't be loaded. -struct RecentRuns { - let runs: [WorkflowRun] - let failedRepos: [String] -} - actor GitHubAPIClient { /// Upper bound on repositories listed for selection (and polled when nothing is selected). static let maxRepositories = 200 @@ -107,15 +78,8 @@ actor GitHubAPIClient { return (allRuns, errors) } - // Partial results are fine unless the failure affects every request. - if errors.contains(where: { if case GitHubAPIError.invalidToken = $0.error { true } else { false } }) { - throw GitHubAPIError.invalidToken - } - if errors.contains(where: { if case GitHubAPIError.rateLimited = $0.error { true } else { false } }) { - throw GitHubAPIError.rateLimited - } - if let firstError = errors.first, allRuns.isEmpty { - throw firstError.error + if let error = GitHubAPIError.failingWholeFetch(errors.map(\.error), requestedRepoCount: repoFullNames.count) { + throw error } let recentRuns = allRuns diff --git a/Tests/ActionBarTests/FetchFailureTests.swift b/Tests/ActionBarTests/FetchFailureTests.swift new file mode 100644 index 0000000..b9e588c --- /dev/null +++ b/Tests/ActionBarTests/FetchFailureTests.swift @@ -0,0 +1,42 @@ +import Foundation +import Testing + +struct FetchFailureTests { + private let serverError = GitHubAPIError.invalidResponse(500) + + @Test func noErrorsReturnsResults() { + #expect(GitHubAPIError.failingWholeFetch([], requestedRepoCount: 2) == nil) + } + + @Test func someReposFailingReturnsPartialResults() { + // One repo failed; the other loaded, even if it had no runs. + #expect(GitHubAPIError.failingWholeFetch([serverError], requestedRepoCount: 2) == nil) + } + + @Test func everyRepoFailingFailsTheFetch() throws { + let error = try #require(GitHubAPIError.failingWholeFetch([serverError, serverError], requestedRepoCount: 2)) + guard case GitHubAPIError.invalidResponse(500) = error else { + Issue.record("Expected the first repo's error, got \(error)") + return + } + } + + @Test func invalidTokenOnAnyRepoFailsTheFetch() throws { + let error = try #require(GitHubAPIError.failingWholeFetch( + [serverError, GitHubAPIError.invalidToken], + requestedRepoCount: 5 + )) + guard case GitHubAPIError.invalidToken = error else { + Issue.record("Expected invalidToken, got \(error)") + return + } + } + + @Test func rateLimitOnAnyRepoFailsTheFetch() throws { + let error = try #require(GitHubAPIError.failingWholeFetch([GitHubAPIError.rateLimited], requestedRepoCount: 5)) + guard case GitHubAPIError.rateLimited = error else { + Issue.record("Expected rateLimited, got \(error)") + return + } + } +} From 690a7bfdb67799fe3db6b99b3c12a9fb67e527bb Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 10:13:15 +0200 Subject: [PATCH 12/14] Don't show a cancelled poll's results or skip the next fetch Changing the polling interval cancels the poll task mid-fetch. Its unfinished repos came back as network errors, so the menu could show some repos as failed, or "Network error: cancelled", and the new poll's first fetch was skipped because the old one was still in flight. A cancelled fetch now throws CancellationError and is ignored, and the new poll waits for the old one to end before fetching. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 9 +++++++-- Sources/ActionBar/Services/GitHubAPIClient.swift | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 8585c2a..2bd3a72 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -239,8 +239,12 @@ final class AppState { } private func startPolling() { + let previousPollTask = pollTask stopPolling() pollTask = Task { + // Let a cancelled poll wind down first. Otherwise its fetch would still be in flight, + // this fetch would be skipped, and nothing would refresh until the next interval. + await previousPollTask?.value await fetchRuns() while !Task.isCancelled { // Wait in short steps against a wall-clock deadline, so a refresh happens @@ -325,11 +329,12 @@ final class AppState { runs = visibleRuns failedRepos = recentRuns.failedRepos hasLoadedOnce = true + } catch let error where error is CancellationError || Task.isCancelled { + // Task was cancelled (e.g. polling restart) — don't surface to UI. Requests cut off + // by the cancellation can surface as network errors, hence the `Task.isCancelled` check. } catch GitHubAPIError.invalidToken { guard sessionID == self.sessionID else { return } signOut() - } catch is CancellationError { - // Task was cancelled (e.g. polling restart) — don't surface to UI. } catch { guard sessionID == self.sessionID else { return } errorMessage = error.localizedDescription diff --git a/Sources/ActionBar/Services/GitHubAPIClient.swift b/Sources/ActionBar/Services/GitHubAPIClient.swift index 47850cb..a4a3476 100644 --- a/Sources/ActionBar/Services/GitHubAPIClient.swift +++ b/Sources/ActionBar/Services/GitHubAPIClient.swift @@ -78,6 +78,8 @@ actor GitHubAPIClient { return (allRuns, errors) } + // A cancelled fetch reports its unfinished repos as network errors; don't pass those off as failures. + try Task.checkCancellation() if let error = GitHubAPIError.failingWholeFetch(errors.map(\.error), requestedRepoCount: repoFullNames.count) { throw error } From f7ae03e1e9e6a4cee9a43e9e58e358cb68b0333f Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 10:13:39 +0200 Subject: [PATCH 13/14] Stop the installation check from starting polling after sign-out A background installation check already in flight at sign-out could still call installationFound() and start polling, including for the next account when that account has no installation yet. The check now stops if the task was cancelled or the session changed, and waiting for an installation stops any poll that's running. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index 2bd3a72..d7cd608 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -153,6 +153,7 @@ final class AppState { if hasInstallations { startPolling() } else { + stopPolling() needsInstallation = true startInstallCheckTimer() } @@ -181,12 +182,15 @@ final class AppState { private func startInstallCheckTimer() { stopInstallCheckTimer() + let sessionID = self.sessionID installCheckTask = Task { while !Task.isCancelled { try? await Task.sleep(for: .seconds(10)) guard !Task.isCancelled else { break } guard let token = KeychainService.loadToken() else { continue } - if (try? await apiClient.hasInstallations(token: token)) == true { + let hasInstallations = (try? await apiClient.hasInstallations(token: token)) == true + guard !Task.isCancelled, sessionID == self.sessionID else { break } + if hasInstallations { installationFound() break } From 392cbbf8287a6ffe9fcdd398d3c623ef78e7d0cd Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Thu, 22 Oct 2026 10:15:19 +0200 Subject: [PATCH 14/14] Apply only the latest repository list load Opening Settings and pressing refresh started two loads, and whichever finished last won. An older failure could show the red error under a list that had loaded, or an older list could replace a newer one, and the spinner stopped when the first one finished. A new load now cancels the previous one, and only the current load updates the list, the error and the spinner. Co-Authored-By: Claude Opus 5.5 --- Sources/ActionBar/Models/AppState.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Sources/ActionBar/Models/AppState.swift b/Sources/ActionBar/Models/AppState.swift index d7cd608..98f7517 100644 --- a/Sources/ActionBar/Models/AppState.swift +++ b/Sources/ActionBar/Models/AppState.swift @@ -39,6 +39,7 @@ final class AppState { @ObservationIgnored private var refetchRequested = false /// Changes on sign-out, so a fetch that was in flight doesn't apply its results afterwards. @ObservationIgnored private var sessionID = 0 + @ObservationIgnored private var repoListTask: Task? init(settings: SettingsStore? = nil) { self.settings = settings ?? SettingsStore() @@ -114,6 +115,9 @@ final class AppState { cancelLogin() stopPolling() stopInstallCheckTimer() + repoListTask?.cancel() + repoListTask = nil + isLoadingRepos = false Task { await apiClient.clearConditionalCache() } @@ -353,18 +357,20 @@ final class AppState { isLoadingRepos = true repoListError = nil let sessionID = self.sessionID - Task { - defer { isLoadingRepos = false } + // Only the latest load applies its result, so an older one finishing later can't overwrite it. + repoListTask?.cancel() + repoListTask = Task { do { let repos = try await apiClient.fetchAllRepos(token: token) - guard sessionID == self.sessionID else { return } + guard !Task.isCancelled, sessionID == self.sessionID else { return } availableRepos = repos repoListIsCapped = repos.count >= GitHubAPIClient.maxRepositories } catch { + guard !Task.isCancelled, sessionID == self.sessionID else { return } // Keep whatever list we had before. - guard sessionID == self.sessionID else { return } repoListError = error.localizedDescription } + isLoadingRepos = false } } }