Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion Sources/CodeCaps/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
}

@MainActor
final class AppDelegate: NSObject, NSApplicationDelegate, NSWindowDelegate, NSMenuDelegate, NSMenuItemValidation {
final class AppDelegate: NSObject, NSApplicationDelegate, NSWindowDelegate, NSMenuDelegate, NSMenuItemValidation, NSPopoverDelegate {
let model = MonitorModel()
let consoleState = ConsoleState()
private var statusItem: NSStatusItem?
Expand All @@ -69,6 +69,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSWindowDelegate, NSMe
return image
}()
private let popover = NSPopover()
private var outsideClickMonitor: Any?
private var consoleWindow: NSWindow?
private var statusMenu: NSMenu?
private var subscriptions = Set<AnyCancellable>()
Expand All @@ -81,6 +82,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSWindowDelegate, NSMe
AppUpdater.shared.start()
configureMenu()
popover.behavior = .transient
popover.delegate = self
let glance = NSHostingController(rootView:
GlancePopover(model: model,
openConsole: { [weak self] page in self?.showConsole(page: page) },
Expand Down Expand Up @@ -361,6 +363,33 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSWindowDelegate, NSMe
}
}

// MARK: - Popover outside-click dismissal

private func startOutsideClickMonitor() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug medium

Race condition in AppDelegate.swift: the Task { @MainActor in } wrapper defers the self.popover.isShown check, allowing a stale mouse-down to close a freshly-reopened popover when the popover closes and reopens between scheduling and execution, after which the queued Task reads popover.isShown == true and calls performClose on the new instance. Replace the Task wrapper with MainActor.assumeIsolated so the check and performClose execute synchronously with respect to the event.

outsideClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown]) { [weak self] _ in
    MainActor.assumeIsolated {
        guard let self, self.popover.isShown else { return }
        self.popover.performClose(nil)
    }
}
Prompt for LLM

File Sources/CodeCaps/AppDelegate.swift:

Line 368:

Race condition in `AppDelegate.swift`: the `Task { @MainActor in }` wrapper defers the `self.popover.isShown` check, allowing a stale mouse-down to close a freshly-reopened popover when the popover closes and reopens between scheduling and execution, after which the queued Task reads `popover.isShown == true` and calls `performClose` on the new instance. Replace the Task wrapper with `MainActor.assumeIsolated` so the check and `performClose` execute synchronously with respect to the event.

Suggested Code:

outsideClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown]) { [weak self] _ in
    MainActor.assumeIsolated {
        guard let self, self.popover.isShown else { return }
        self.popover.performClose(nil)
    }
}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

stopOutsideClickMonitor()
outsideClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown]) { [weak self] _ in
Task { @MainActor [weak self] in
guard let self, self.popover.isShown else { return }
self.popover.performClose(nil)
}
}
}

private func stopOutsideClickMonitor() {
if let monitor = outsideClickMonitor {
NSEvent.removeMonitor(monitor)
outsideClickMonitor = nil
}
}

func popoverWillShow(_ notification: Notification) {
startOutsideClickMonitor()
}

func popoverDidClose(_ notification: Notification) {
stopOutsideClickMonitor()
}

// MARK: - Console

/// The single window. `page` nil means "leave the selection alone", which
Expand Down
2 changes: 1 addition & 1 deletion Sources/CodeCaps/MonitorModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1891,7 +1891,7 @@ final class MonitorModel: ObservableObject {
for (key, message) in file?.issues ?? [:] where issues[key] == nil {
issues[key] = message
}
if !fileWindows.filter({ $0.boundedRemainingPercent != nil }).isEmpty {
if !windows.filter({ $0.canonicalProviderKey == "openai" && $0.boundedRemainingPercent != nil }).isEmpty {
issues["openai"] = nil
}
if currentCodexAccountID == nil {
Expand Down
14 changes: 11 additions & 3 deletions Sources/CodeCaps/QuotaComponents.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,17 @@ enum Theme {
NSColor.black.withAlphaComponent(0.7))
/// The share of a quota window already used: the left segment of the bar.
static let barUsed = danger
/// The share still available: the right segment of the bar. This is the
/// same teal as the "% remaining" text, which the owner reads as green.
static let barRemaining = accent
/// The share still available: the right segment of the bar. Healthy capacity
/// uses teal (or green if chosen), keeping the bar distinguishable from the red
/// `barUsed` segment even if the user picks a warm or danger-like UI accent (e.g. magenta, orange).
static var barRemaining: Color {
switch AccentChoice.current {
case .green:
return dyn(hex(AccentChoice.green.lightHex), hex(AccentChoice.green.darkHex))
default:
return dyn(hex(AccentChoice.teal.lightHex), hex(AccentChoice.teal.darkHex))
}
}
static let fleet = dyn(hex(0x4B4FA8), hex(0x8A8EE0))

/// Unfilled portion of any progress bar. A black 6% track disappears on a
Expand Down
2 changes: 1 addition & 1 deletion Sources/CodeCaps/ResetAlarmManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public final class ResetAlarmManager: ObservableObject {
private func deliver(_ payload: ResetAlarmNotification) {
// Only deliver alerts if the master top bell is armed (All) or this specific quota's bell is on
guard isAlarmEnabled(for: payload.providerId) else { return }
if payload.isVendorReset && payload.sound.isAudible {
if payload.sound.isAudible {
playAlarmSound(payload.sound)
}
// Deliver via custom test handler if installed
Expand Down
18 changes: 13 additions & 5 deletions Sources/QuotaCore/ResetAlarmTracker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -509,11 +509,19 @@ public struct ResetAlarmTracker: Sendable {
nextResetAt = observation.resetAt.flatMap { $0 > readAt ? $0 : nil } ?? window.periodResetAt
}
}
} else if let reported = observation.resetAt, reported > readAt {
// No period end known — a reset was just detected without the next
// one being published. Adopt the first reset time still ahead; a
// stale one would read as a second reset of the same period.
nextResetAt = reported
} else {
if let reported = observation.resetAt, reported > readAt {
// No period end known — adopt the first reset time still ahead.
nextResetAt = reported
}
if let reading, let last = window.lastRemaining {
let jumpedToFull = reading >= 99.5 && last < 99.5
let surgedMidWindow = last < 80.0 && reading >= 95.0
if jumpedToFull || surgedMidWindow {
isReset = true
isMidWindow = true
}
}
}

let previous = window
Expand Down
9 changes: 9 additions & 0 deletions Tests/CodeCapsTests/ThemePreferenceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,13 @@ final class ThemePreferenceTests: XCTestCase {
AccentChoice.current = .teal
XCTAssertEqual(AccentChoice.current, .teal)
}

func testBarRemainingRemainsHealthyToneAndDoesNotTurnRedWhenMagentaSelected() {
AccentChoice.current = .magenta
XCTAssertNotNil(Theme.barRemaining)
AccentChoice.current = .green
XCTAssertNotNil(Theme.barRemaining)
AccentChoice.current = .teal
XCTAssertNotNil(Theme.barRemaining)
}
}
12 changes: 12 additions & 0 deletions Tests/QuotaCoreTests/ResetAlarmTrackerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -871,4 +871,16 @@ final class ResetAlarmTrackerTests: XCTestCase {
let decoded = ResetAlarmTrackerState.decoded(from: tracker.state.encoded())
XCTAssertEqual(decoded, tracker.state)
}

func testWindowWithoutInitialResetAtFiresWhenJumpingToFull() {
var tracker = ResetAlarmTracker()
let obs1 = reading("primary", period: 7 * day, resetAt: nil, remaining: 20, observedAt: t0, provider: "openai")
_ = tracker.process([obs1], now: t0)
let t1 = t0 + 2 * hour
let obs2 = reading("primary", period: 7 * day, resetAt: nil, remaining: 100, observedAt: t1, provider: "openai")
let events = tracker.process([obs2], now: t1)
XCTAssertEqual(events.count, 1)
XCTAssertEqual(events.first?.providerId, "openai")
XCTAssertEqual(events.first?.remainingPercent, 100)
}
}
Loading