Skip to content
12 changes: 12 additions & 0 deletions Sources/CodexBar/StatusItemController+Animation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ extension StatusItemController {
}

func updateBlinkingState() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
// During the loading animation, blink ticks can overwrite the animated menu bar icon and cause flicker.
if self.needsMenuBarIconAnimation() {
self.stopBlinking()
Expand Down Expand Up @@ -422,6 +425,9 @@ extension StatusItemController {
}

@objc func handleDebugBlinkNotification() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
self.forceBlinkNow()
}

Expand Down Expand Up @@ -500,6 +506,9 @@ extension StatusItemController {
}

private func updateAnimationFrame() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
self.animationPhase += 0.045 // half-speed animation
if self.shouldMergeIcons {
self.applyIcon(phase: self.animationPhase)
Expand All @@ -519,6 +528,9 @@ extension StatusItemController {
}

@objc func handleDebugReplayNotification(_ notification: Notification) {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
if let raw = notification.userInfo?["pattern"] as? String,
let selected = LoadingPattern(rawValue: raw)
{
Expand Down
53 changes: 45 additions & 8 deletions Sources/CodexBar/StatusItemController+Menu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,26 @@ import SwiftUI

extension StatusItemController {
private static let menuCardBaseWidth: CGFloat = 310
private static let menuOpenRefreshDelay: Duration = .seconds(1.2)
private static let defaultMenuOpenRefreshDelay: Duration = .seconds(1.2)
#if DEBUG
private static var menuOpenRefreshDelayForTesting: Duration = .seconds(1.2)
static func setMenuOpenRefreshDelayForTesting(_ delay: Duration) {
self.menuOpenRefreshDelayForTesting = delay
}

static func resetMenuOpenRefreshDelayForTesting() {
self.menuOpenRefreshDelayForTesting = self.defaultMenuOpenRefreshDelay
}
#endif

private static var menuOpenRefreshDelay: Duration {
#if DEBUG
menuOpenRefreshDelayForTesting
#else
defaultMenuOpenRefreshDelay
#endif
}

private struct OpenAIWebMenuItems {
let hasUsageBreakdown: Bool
let hasCreditsHistory: Bool
Expand Down Expand Up @@ -45,7 +64,11 @@ extension StatusItemController {
if Self.menuRefreshEnabled, self.isOpenAIWebSubviewMenu(menu) {
self.store.requestOpenAIDashboardRefreshIfStale(reason: "submenu open")
}
self.openMenus[ObjectIdentifier(menu)] = menu
if Self.menuRefreshEnabled {
// Intentionally skip open-menu tracking when refresh is disabled (tests).
// If refresh is re-enabled while this menu stays open, it will not be backfilled until next open.
self.openMenus[ObjectIdentifier(menu)] = menu
}
// Removed redundant async refresh - single pass is sufficient after initial layout
return
}
Expand Down Expand Up @@ -75,7 +98,11 @@ extension StatusItemController {
self.markMenuFresh(menu)
// Heights are already set during populateMenu, no need to remeasure
}
self.openMenus[ObjectIdentifier(menu)] = menu
if Self.menuRefreshEnabled {
// Intentionally skip open-menu tracking when refresh is disabled (tests).
// If refresh is re-enabled while this menu stays open, it will not be backfilled until next open.
self.openMenus[ObjectIdentifier(menu)] = menu
}
// Only schedule refresh after menu is registered as open - refreshNow is called async
if Self.menuRefreshEnabled {
self.scheduleOpenMenuRefresh(for: menu)
Expand Down Expand Up @@ -512,14 +539,12 @@ extension StatusItemController {
}

func refreshOpenMenusIfNeeded() {
guard Self.menuRefreshEnabled else { return }
guard !self.openMenus.isEmpty else { return }
var orphanedKeys: [ObjectIdentifier] = []
for (key, menu) in self.openMenus {
guard key == ObjectIdentifier(menu) else {
// Clean up orphaned menu entries from all tracking dictionaries
self.openMenus.removeValue(forKey: key)
self.menuRefreshTasks.removeValue(forKey: key)?.cancel()
self.menuProviders.removeValue(forKey: key)
self.menuVersions.removeValue(forKey: key)
orphanedKeys.append(key)
continue
}

Expand All @@ -535,6 +560,14 @@ extension StatusItemController {
// Heights are already set during populateMenu, no need to remeasure
}
}

// Clean up orphaned menu entries from all tracking dictionaries.
for key in orphanedKeys {
self.openMenus.removeValue(forKey: key)
self.menuRefreshTasks.removeValue(forKey: key)?.cancel()
self.menuProviders.removeValue(forKey: key)
self.menuVersions.removeValue(forKey: key)
}
}

private func menuProvider(for menu: NSMenu) -> UsageProvider? {
Expand Down Expand Up @@ -562,6 +595,10 @@ extension StatusItemController {
guard let self, let menu else { return }
try? await Task.sleep(for: Self.menuOpenRefreshDelay)
guard !Task.isCancelled else { return }
guard Self.menuRefreshEnabled else { return }
#if DEBUG
self.onDelayedMenuRefreshAttemptForTesting?()
#endif
guard self.openMenus[ObjectIdentifier(menu)] != nil else { return }
guard !self.store.isRefreshing else { return }
let provider = self.menuProvider(for: menu) ?? self.resolvedMenuProvider()
Expand Down
80 changes: 78 additions & 2 deletions Sources/CodexBar/StatusItemController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,19 @@ protocol StatusItemControlling: AnyObject {
final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControlling {
// Disable SwiftUI menu cards + menu refresh work in tests to avoid swiftpm-testing-helper crashes.
static var menuCardRenderingEnabled = !SettingsStore.isRunningTests
static var menuRefreshEnabled = !SettingsStore.isRunningTests
private static let defaultMenuRefreshEnabled = !SettingsStore.isRunningTests
private(set) static var menuRefreshEnabled = !SettingsStore.isRunningTests
// TODO(maintainer): Confirm whether runtime toggles outside tests are supported. Current semantics assume
// test-only usage; enabling refresh while a menu is already open does not retro-register/schedule that menu.
#if DEBUG
static func setMenuRefreshEnabledForTesting(_ enabled: Bool) {
self.menuRefreshEnabled = enabled
}

static func resetMenuRefreshEnabledForTesting() {
self.menuRefreshEnabled = self.defaultMenuRefreshEnabled
}
#endif
typealias Factory = (UsageStore, SettingsStore, AccountInfo, UpdaterProviding, PreferencesSelection)
-> StatusItemControlling
static let defaultFactory: Factory = { store, settings, account, updater, selection in
Expand Down Expand Up @@ -45,6 +57,10 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
var fallbackMenu: NSMenu?
var openMenus: [ObjectIdentifier: NSMenu] = [:]
var menuRefreshTasks: [ObjectIdentifier: Task<Void, Never>] = [:]
#if DEBUG
var onDelayedMenuRefreshAttemptForTesting: (() -> Void)?
var isReleasedForTesting = false
#endif
var blinkTask: Task<Void, Never>?
var loginTask: Task<Void, Never>? {
didSet { self.refreshMenusForLoginStateChange() }
Expand Down Expand Up @@ -230,6 +246,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

@objc private func handleProviderConfigDidChange(_ notification: Notification) {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
let reason = notification.userInfo?["reason"] as? String ?? "unknown"
if let source = notification.object as? SettingsStore,
source !== self.settings
Expand All @@ -256,12 +275,17 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func invalidateMenus() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
self.menuContentVersion &+= 1
guard Self.menuRefreshEnabled else { return }
// Don't refresh menus while they're open - wait until they close and reopen
// This prevents expensive rebuilds while user is navigating the menu
guard self.openMenus.isEmpty else { return }
self.refreshOpenMenusIfNeeded()
Task { @MainActor in
Task { @MainActor [weak self] in
guard let self else { return }
// AppKit can ignore menu mutations while tracking; retry on the next run loop.
await Task.yield()
guard self.openMenus.isEmpty else { return }
Expand Down Expand Up @@ -295,6 +319,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func handleSettingsChange(reason: String) {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
let configChanged = self.settings.configRevision != self.lastConfigRevision
let orderChanged = self.settings.providerOrder != self.lastProviderOrder
let shouldRefreshOpenMenus = self.shouldRefreshOpenMenusForProviderSwitcher()
Expand All @@ -310,6 +337,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func updateIcons() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
// Avoid flicker: when an animation driver is active, store updates can call `updateIcons()` and
// briefly overwrite the animated frame with the static (phase=nil) icon.
let phase: Double? = self.needsMenuBarIconAnimation() ? self.animationPhase : nil
Expand All @@ -336,6 +366,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func updateVisibility() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
let anyEnabled = !self.store.enabledProviders().isEmpty
let force = self.store.debugForceAnimation
let mergeIcons = self.shouldMergeIcons
Expand Down Expand Up @@ -373,6 +406,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func refreshMenusForLoginStateChange() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
self.invalidateMenus()
if self.shouldMergeIcons {
self.attachMenus()
Expand Down Expand Up @@ -424,6 +460,9 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
}

private func rebuildProviderStatusItems() {
#if DEBUG
guard !self.isReleasedForTesting else { return }
#endif
for item in self.statusItems.values {
self.statusBar.removeStatusItem(item)
}
Expand Down Expand Up @@ -456,6 +495,43 @@ final class StatusItemController: NSObject, NSMenuDelegate, StatusItemControllin
return "\(prefix): \(base)"
}

#if DEBUG
func releaseStatusItemsForTesting() {
guard !self.isReleasedForTesting else { return }
self.isReleasedForTesting = true
self.blinkTask?.cancel()
self.loginTask?.cancel()
Comment on lines +502 to +503

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Block login task callbacks after test teardown

releaseStatusItemsForTesting() only cancels loginTask, but an in-flight switch-account task still runs its defer (activeLoginProvider = nil, loginTask = nil), and those property observers call refreshMenusForLoginStateChange() which still reaches attachMenus() without an isReleasedForTesting guard. In tests that tear down while login is in progress, this can recreate/mutate status items after teardown and reintroduce the post-release AppKit mutation race this change is trying to prevent.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 428fd68.

What changed:

  • Added a DEBUG release guard in refreshMenusForLoginStateChange() so login-task/property-observer callbacks no-op after releaseStatusItemsForTesting().

Added regression test:

  • loginStateCallbacksDoNotAttachMenusAfterRelease() in Tests/CodexBarTests/StatusMenuTests.swift to verify login-state callbacks after release do not reattach menus or recreate status items.

Validation rerun:

  • pnpm check
  • ./Scripts/compile_and_run.sh
    Both passed.

self.animationDriver?.stop()
self.animationDriver = nil
self.animationPhase = 0
self.blinkForceUntil = nil
self.blinkStates.removeAll(keepingCapacity: false)
self.blinkAmounts.removeAll(keepingCapacity: false)
self.wiggleAmounts.removeAll(keepingCapacity: false)
self.tiltAmounts.removeAll(keepingCapacity: false)

for task in self.menuRefreshTasks.values {
task.cancel()
}
self.menuRefreshTasks.removeAll(keepingCapacity: false)
self.openMenus.removeAll(keepingCapacity: false)
self.menuProviders.removeAll(keepingCapacity: false)
self.menuVersions.removeAll(keepingCapacity: false)
self.providerMenus.removeAll(keepingCapacity: false)
self.mergedMenu = nil
self.fallbackMenu = nil

self.statusItem.menu = nil
self.statusBar.removeStatusItem(self.statusItem)

for item in self.statusItems.values {
item.menu = nil
self.statusBar.removeStatusItem(item)
}
self.statusItems.removeAll(keepingCapacity: false)
}
#endif

deinit {
self.blinkTask?.cancel()
self.loginTask?.cancel()
Expand Down
33 changes: 20 additions & 13 deletions Sources/CodexBarCore/KeychainAccessPreflight.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,7 @@ public enum KeychainAccessPreflight {
}
#endif
guard !KeychainAccessGate.isDisabled else { return .notFound }
var query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: service,
kSecMatchLimit as String: kSecMatchLimitOne,
// Preflight should never trigger UI. Avoid requesting the secret payload (`kSecReturnData`) because
// some macOS configurations have been observed to show the legacy keychain prompt even when
// `kSecUseAuthenticationUIFail` is set.
kSecReturnAttributes as String: true,
]
KeychainNoUIQuery.apply(to: &query)
if let account {
query[kSecAttrAccount as String] = account
}
let query = self.makeGenericPasswordPreflightQuery(service: service, account: account)

var result: AnyObject?
let status = SecItemCopyMatching(query as CFDictionary, &result)
Expand Down Expand Up @@ -174,4 +162,23 @@ public enum KeychainAccessPreflight {
return .notFound
#endif
}

#if os(macOS)
static func makeGenericPasswordPreflightQuery(service: String, account: String?) -> [String: Any] {
var query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: service,
kSecMatchLimit as String: kSecMatchLimitOne,
// Preflight should never trigger UI. Avoid requesting the secret payload (`kSecReturnData`) because
// some macOS configurations have been observed to show the legacy keychain prompt unless the query
// is strictly non-interactive.
kSecReturnAttributes as String: true,
]
KeychainNoUIQuery.apply(to: &query)
if let account {
query[kSecAttrAccount as String] = account
}
return query
}
#endif
}
30 changes: 26 additions & 4 deletions Sources/CodexBarCore/KeychainNoUIQuery.swift
Original file line number Diff line number Diff line change
@@ -1,19 +1,41 @@
import Foundation

#if os(macOS)
import Darwin
import LocalAuthentication
import Security

enum KeychainNoUIQuery {
private static let uiFailPolicy = KeychainNoUIQuery.resolveUIFailPolicy()

static func apply(to query: inout [String: Any]) {
let context = LAContext()
context.interactionNotAllowed = true
query[kSecUseAuthenticationContext as String] = context

// NOTE: While Apple recommends using LAContext.interactionNotAllowed, that alone is not sufficient to
// prevent the legacy keychain "Allow/Deny" prompt on some configurations. We also set the UI policy to fail
// so SecItemCopyMatching returns errSecInteractionNotAllowed instead of showing UI.
query[kSecUseAuthenticationUI as String] = kSecUseAuthenticationUIFail
// Keep explicit UI-fail policy for legacy keychain behavior on macOS where
// `interactionNotAllowed` alone can still surface Allow/Deny prompts.
query[kSecUseAuthenticationUI as String] = self.uiFailPolicy as CFString
}

static func uiFailPolicyForTesting() -> String {
self.uiFailPolicy
}

private static func resolveUIFailPolicy() -> String {
// Resolve the Security symbol at runtime to preserve the true constant value
// without directly referencing deprecated API at compile time.
let securityPath = "/System/Library/Frameworks/Security.framework/Security"
guard let handle = dlopen(securityPath, RTLD_NOW) else {
return "u_AuthUIF"
}
defer { dlclose(handle) }

guard let symbol = dlsym(handle, "kSecUseAuthenticationUIFail") else {
return "u_AuthUIF"
}
let valuePointer = symbol.assumingMemoryBound(to: CFString?.self)
return (valuePointer.pointee as String?) ?? "u_AuthUIF"
}
}
#endif
Loading