Skip to content

Commit e3c2ade

Browse files
hhh2210claudesteipete
authored
fix: scope provider refresh state (#1911)
* fix: scope refresh-row disabling to the provider being refreshed Refreshing one provider greyed out — and single-flighted — the Refresh row for every provider, so the menu implied the others were refreshing when they were not. Replace the single global `manualRefreshTask` with a per-scope map (`.global` for the all-providers ⌘R/overview refresh, `.provider(p)` for a single provider). The disable gate now reflects only the target provider's own refresh (a global refresh still disables every row), and `startManualRefresh` guards per scope so different providers can refresh concurrently instead of one blocking the rest. `MenuCardRefreshMonitor` tracks in-flight providers as a set plus a global flag. Rewrites the refresh-row tests that encoded the old blanket behavior and adds coverage for scoped greying and concurrent per-provider refreshes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review — preserve frozen cards and exclude global/provider overlap - MenuCardRefreshMonitor: keep an already-refreshing provider's frozen card when a second provider's refresh re-supplies models (old-wins merge), so the first card no longer jumps mid-refresh — the artifact the freeze exists to prevent. - startManualRefresh: make a .global refresh mutually exclusive with any per-provider one (and vice versa); two different providers still refresh concurrently. - Merged-overview Refresh row now reflects any in-flight manual refresh, including a per-provider refresh's post-fetch status/token tail, instead of flickering. - Make the test-only manualRefreshTask / manualRefreshProvider accessors return a value only when unambiguous (global, or exactly one task), else nil; move them to +Actions. - Add tests: concurrent frozen-card retention, and overview-busy-through-tail + global refresh exclusion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: apply frozen-card old-wins merge, global/provider exclusivity guard, scoped overview greying * fix: isolate provider refresh snapshots * test: use scoped refresh tasks --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
1 parent 9ad35de commit e3c2ade

10 files changed

Lines changed: 306 additions & 85 deletions

Sources/CodexBar/MenuCardRefreshMonitor.swift

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,18 @@ final class MenuCardRefreshMonitor {
1313
typealias ModelResolver = @MainActor (UsageProvider) -> UsageMenuCardView.Model?
1414

1515
private let resolveModel: ModelResolver
16-
var isManualRefreshInFlight = false
17-
private var manualRefreshProvider: UsageProvider?
16+
/// Set while an all-providers refresh is running; it freezes every provider's card.
17+
private var globalManualRefreshInFlight = false
18+
/// Providers with an individual manual refresh in flight. Concurrent entries are allowed so
19+
/// refreshing one provider does not stall or unfreeze another.
20+
private var manualRefreshProviders: Set<UsageProvider> = []
1821
private var frozenManualRefreshModels: [UsageProvider: UsageMenuCardView.Model] = [:]
1922

23+
/// True while any manual refresh (global or per-provider) is running.
24+
var isManualRefreshInFlight: Bool {
25+
self.globalManualRefreshInFlight || !self.manualRefreshProviders.isEmpty
26+
}
27+
2028
init(resolveModel: @escaping ModelResolver) {
2129
self.resolveModel = resolveModel
2230
}
@@ -25,19 +33,34 @@ final class MenuCardRefreshMonitor {
2533
frozenModels: [UsageProvider: UsageMenuCardView.Model],
2634
provider: UsageProvider? = nil)
2735
{
28-
self.frozenManualRefreshModels = frozenModels
29-
self.manualRefreshProvider = provider
30-
self.isManualRefreshInFlight = true
36+
if let provider {
37+
self.frozenManualRefreshModels[provider] = frozenModels[provider]
38+
self.manualRefreshProviders.insert(provider)
39+
} else {
40+
self.frozenManualRefreshModels = frozenModels
41+
self.globalManualRefreshInFlight = true
42+
}
43+
}
44+
45+
/// Balances a `beginManualRefresh` with the same `provider` argument (nil ends the global refresh).
46+
func endManualRefresh(for provider: UsageProvider? = nil) {
47+
if let provider {
48+
self.manualRefreshProviders.remove(provider)
49+
self.frozenManualRefreshModels[provider] = nil
50+
} else {
51+
self.globalManualRefreshInFlight = false
52+
self.frozenManualRefreshModels.removeAll(keepingCapacity: true)
53+
}
3154
}
3255

33-
func endManualRefresh() {
34-
self.isManualRefreshInFlight = false
35-
self.manualRefreshProvider = nil
56+
func resetManualRefresh() {
57+
self.globalManualRefreshInFlight = false
58+
self.manualRefreshProviders.removeAll(keepingCapacity: true)
3659
self.frozenManualRefreshModels.removeAll(keepingCapacity: true)
3760
}
3861

3962
func isManualRefreshInFlight(for provider: UsageProvider) -> Bool {
40-
self.isManualRefreshInFlight && (self.manualRefreshProvider == nil || self.manualRefreshProvider == provider)
63+
self.globalManualRefreshInFlight || self.manualRefreshProviders.contains(provider)
4164
}
4265

4366
func model(

Sources/CodexBar/StatusItemController+Actions.swift

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import AppKit
22
import CodexBarCore
33

4+
extension StatusItemController {
5+
/// Identifies which manual refresh a task belongs to, so per-provider refreshes stay independent
6+
/// of each other and of the all-providers refresh.
7+
enum ManualRefreshScope: Hashable {
8+
case global
9+
case provider(UsageProvider)
10+
}
11+
}
12+
413
enum LoginNotificationLogic {
514
static func notificationCopy(providerName: String) -> (title: String, body: String) {
615
(
@@ -114,10 +123,17 @@ extension StatusItemController: StatusItemMenuPersistentActionDelegate {
114123
}
115124

116125
private func startManualRefresh(for provider: UsageProvider?) {
126+
let scope: ManualRefreshScope = provider.map(ManualRefreshScope.provider) ?? .global
117127
let scopedRefreshInFlight = provider.map { self.store.refreshingProviders.contains($0) }
118128
?? !self.store.refreshingProviders.isEmpty
129+
// Two different providers may refresh concurrently, but an all-providers (.global) refresh must
130+
// not overlap a per-provider one (or vice versa) — that would duplicate the shared fetch work.
131+
let conflictsWithOtherScope = scope == .global
132+
? self.manualRefreshTasks.contains { $0.key != .global }
133+
: self.manualRefreshTasks[.global] != nil
119134
guard !self.hasPreparedForAppShutdown,
120-
self.manualRefreshTask == nil,
135+
self.manualRefreshTasks[scope] == nil,
136+
!conflictsWithOtherScope,
121137
!self.store.isRefreshing,
122138
!scopedRefreshInFlight
123139
else { return }
@@ -126,9 +142,8 @@ extension StatusItemController: StatusItemMenuPersistentActionDelegate {
126142
let task = Task { @MainActor [weak self] in
127143
guard let self else { return }
128144
defer {
129-
self.manualRefreshTask = nil
130-
self.manualRefreshProvider = nil
131-
self.menuCardRefreshMonitor.endManualRefresh()
145+
self.manualRefreshTasks[scope] = nil
146+
self.menuCardRefreshMonitor.endManualRefresh(for: provider)
132147
self.updatePersistentRefreshItemsEnabled()
133148
self.prepareAttachedClosedMenusIfNeeded()
134149
}
@@ -152,8 +167,7 @@ extension StatusItemController: StatusItemMenuPersistentActionDelegate {
152167
interaction: .userInitiated)
153168
}
154169
}
155-
self.manualRefreshProvider = provider
156-
self.manualRefreshTask = task
170+
self.manualRefreshTasks[scope] = task
157171
self.menuCardRefreshMonitor.beginManualRefresh(frozenModels: frozenModels, provider: provider)
158172
self.updatePersistentRefreshItemsEnabled()
159173
}

Sources/CodexBar/StatusItemController+MenuTracking.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ extension StatusItemController {
111111

112112
var isMenuDataRefreshInFlight: Bool {
113113
self.store.isRefreshing ||
114-
self.manualRefreshTask != nil ||
114+
!self.manualRefreshTasks.isEmpty ||
115115
!self.store.refreshingProviders.isEmpty ||
116116
UsageProvider.allCases.contains { self.store.isTokenRefreshInFlight(for: $0) }
117117
}

Sources/CodexBar/StatusItemController+PersistentMenuActions.swift

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,29 @@ extension StatusItemController {
1616
}
1717

1818
func isRefreshActionInFlight(for menu: NSMenu) -> Bool {
19-
if self.manualRefreshTask != nil {
19+
// An all-providers manual refresh (⌘R / overview) legitimately busies every row.
20+
if self.manualRefreshTasks[.global] != nil {
2021
return true
2122
}
2223

2324
if self.isMergedOverviewSelected(in: menu) {
24-
// Overview refresh is global, so its busy state must mirror the global manual-refresh gate.
25-
return self.store.isRefreshing || !self.store.refreshingProviders.isEmpty
25+
// Overview stands for every provider, so it is busy while ANY manual refresh runs —
26+
// including the post-fetch tail of a per-provider refresh, after `refreshingProviders`
27+
// has cleared but its `.provider` task is still finishing status/token/credits work.
28+
return self.store.isRefreshing
29+
|| !self.manualRefreshTasks.isEmpty
30+
|| !self.store.refreshingProviders.isEmpty
2631
}
2732
if let provider = self.menuProvider(for: menu) {
28-
return self.store.isRefreshing || self.store.refreshingProviders.contains(provider)
33+
// A manual refresh of a different provider must not grey out this provider's row: only
34+
// reflect the global refresh, this provider's own manual refresh, and its store refresh.
35+
return self.store.isRefreshing
36+
|| self.manualRefreshTasks[.provider(provider)] != nil
37+
|| self.store.refreshingProviders.contains(provider)
2938
}
30-
return self.store.isRefreshing || !self.store.refreshingProviders.isEmpty
39+
return self.store.isRefreshing
40+
|| !self.manualRefreshTasks.isEmpty
41+
|| !self.store.refreshingProviders.isEmpty
3142
}
3243

3344
func isMergedOverviewSelected(in menu: NSMenu) -> Bool {

Sources/CodexBar/StatusItemController+Shutdown.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ extension StatusItemController {
2828
self.menuBarCountdownRefreshTask = nil
2929
self.loginTask?.cancel()
3030
self.loginTask = nil
31-
self.manualRefreshTask?.cancel()
32-
self.manualRefreshTask = nil
33-
self.manualRefreshProvider = nil
34-
self.menuCardRefreshMonitor.endManualRefresh()
31+
for task in self.manualRefreshTasks.values {
32+
task.cancel()
33+
}
34+
self.manualRefreshTasks.removeAll()
35+
self.menuCardRefreshMonitor.resetManualRefresh()
3536
self.screenChangeVisibilityTask?.cancel()
3637
self.screenChangeVisibilityTask = nil
3738
self.pendingScreenChangePreviousCount = nil

Sources/CodexBar/StatusItemController.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,10 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
138138
var fallbackMenu: NSMenu?
139139
var openMenus: [ObjectIdentifier: NSMenu] = [:]
140140
var menuRefreshTasks: [ObjectIdentifier: Task<Void, Never>] = [:]
141-
var manualRefreshTask: Task<Void, Never>?
142-
var manualRefreshProvider: UsageProvider?
141+
/// Manual refreshes tracked per scope so refreshing one provider neither greys out nor blocks
142+
/// a manual refresh of another. `.global` covers the all-providers refresh (⌘R / merged overview).
143+
var manualRefreshTasks: [ManualRefreshScope: Task<Void, Never>] = [:]
144+
143145
var closedMenuRebuildTasks: [ObjectIdentifier: Task<Void, Never>] = [:]
144146
var closedMenuRebuildRequests = MenuRebuildRequestRegistry<ObjectIdentifier>()
145147
var openMenuRebuildTasks: [ObjectIdentifier: Task<Void, Never>] = [:]

Tests/CodexBarTests/StatusMenuClosedPreparationTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ extension StatusMenuTests {
185185
controller._test_manualRefreshOperation = nil
186186
}
187187
controller.refreshNow()
188-
let task = try #require(controller.manualRefreshTask)
188+
let task = try #require(controller.manualRefreshTasks[.global])
189189

190190
controller.invalidateMenus()
191191
for _ in 0..<40 {

Tests/CodexBarTests/StatusMenuMergedOverviewRefreshTests.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ struct StatusMenuMergedOverviewRefreshTests {
4242
}
4343

4444
#expect(requestCount == 0)
45-
#expect(controller.manualRefreshTask == nil)
46-
#expect(controller.manualRefreshProvider == nil)
45+
#expect(controller.manualRefreshTasks.isEmpty)
4746
}
4847

4948
private func makeSettings() -> SettingsStore {

0 commit comments

Comments
 (0)